bios: Add support for enum value display names
This commits adds changes to add support for the value
display names for the Enum BIOS attributes.
GET on
/redfish/v1/Registries/BiosAttributeRegistry/BiosAttributeRegistry/
shows list of Attributes under RegistryEntries. Each
attribute corresponds to a knob in BIOS.
If knob in BIOS is of type Enumeration, it will have
'option' with 'text' and 'value'. This text should be
mapped to attribute->Value->ValueDisplayName.
This change only applies to ENUM type of BIOS attributes
and does not make sense for Integer and String attributes.
Hence, we are sending attribute name in place of the
value display name field for these attributes.
The corresponding bios-setting-mgr change which mandated
this change in PLDM is at-
[1] https://github.com/openbmc/bios-settings-mgr/commit/1a448ad88fdaec7e082b4a1c437f7f3c990402cd
Signed-off-by: Sagar Srinivas <sagar.srinivas@ibm.com>
Change-Id: Ib2882c93e81a70332e4897b196ae812b63cf6184
diff --git a/libpldmresponder/bios_attribute.hpp b/libpldmresponder/bios_attribute.hpp
index 46c34cd..c07d86f 100644
--- a/libpldmresponder/bios_attribute.hpp
+++ b/libpldmresponder/bios_attribute.hpp
@@ -22,6 +22,7 @@
{
using Json = nlohmann::json;
+using ValueDisplayNamesMap = std::map<uint16_t, std::vector<std::string>>;
/** @class BIOSAttribute
* @brief Provide interfaces to implement specific types of attributes
@@ -99,6 +100,8 @@
const std::string helpText;
+ ValueDisplayNamesMap valueDisplayNamesMap;
+
protected:
/** @brief dbus backend, nullopt if this attribute is read-only*/
std::optional<pldm::utils::DBusMapping> dBusMap;
diff --git a/libpldmresponder/bios_config.cpp b/libpldmresponder/bios_config.cpp
index aea24a0..1514664 100644
--- a/libpldmresponder/bios_config.cpp
+++ b/libpldmresponder/bios_config.cpp
@@ -249,6 +249,8 @@
MenuPath menuPath{};
CurrentValue currentValue{};
DefaultValue defaultValue{};
+ std::vector<ValueDisplayName> valueDisplayNames;
+ std::map<uint16_t, std::vector<std::string>> valueDisplayNamesMap;
Option options{};
auto attrValueHandle =
@@ -291,6 +293,9 @@
biosAttributes[attrHandle % biosAttributes.size()]->helpText;
displayName =
biosAttributes[attrHandle % biosAttributes.size()]->displayName;
+ valueDisplayNamesMap =
+ biosAttributes[attrHandle % biosAttributes.size()]
+ ->valueDisplayNamesMap;
}
switch (attrType)
@@ -298,6 +303,13 @@
case PLDM_BIOS_ENUMERATION:
case PLDM_BIOS_ENUMERATION_READ_ONLY:
{
+ if (valueDisplayNamesMap.contains(attrHandle))
+ {
+ const std::vector<ValueDisplayName>& vdn =
+ valueDisplayNamesMap[attrHandle];
+ valueDisplayNames.insert(valueDisplayNames.end(),
+ vdn.begin(), vdn.end());
+ }
auto getValue = [](uint16_t handle,
const Table& table) -> std::string {
auto stringEntry = pldm_bios_table_string_find_by_handle(
@@ -334,7 +346,8 @@
options.push_back(
std::make_tuple("xyz.openbmc_project.BIOSConfig."
"Manager.BoundType.OneOf",
- getValue(pvHandls[i], *stringTable)));
+ getValue(pvHandls[i], *stringTable),
+ valueDisplayNames[i]));
}
auto count =
@@ -380,18 +393,18 @@
uint32_t scalar;
pldm_bios_table_attr_entry_integer_decode(
attrEntry, &lower, &upper, &scalar, &def);
- options.push_back(
- std::make_tuple("xyz.openbmc_project.BIOSConfig.Manager."
- "BoundType.LowerBound",
- static_cast<int64_t>(lower)));
- options.push_back(
- std::make_tuple("xyz.openbmc_project.BIOSConfig.Manager."
- "BoundType.UpperBound",
- static_cast<int64_t>(upper)));
- options.push_back(
- std::make_tuple("xyz.openbmc_project.BIOSConfig.Manager."
- "BoundType.ScalarIncrement",
- static_cast<int64_t>(scalar)));
+ options.push_back(std::make_tuple(
+ "xyz.openbmc_project.BIOSConfig.Manager."
+ "BoundType.LowerBound",
+ static_cast<int64_t>(lower), attributeName));
+ options.push_back(std::make_tuple(
+ "xyz.openbmc_project.BIOSConfig.Manager."
+ "BoundType.UpperBound",
+ static_cast<int64_t>(upper), attributeName));
+ options.push_back(std::make_tuple(
+ "xyz.openbmc_project.BIOSConfig.Manager."
+ "BoundType.ScalarIncrement",
+ static_cast<int64_t>(scalar), attributeName));
defaultValue = static_cast<int64_t>(def);
break;
}
@@ -420,11 +433,11 @@
options.push_back(
std::make_tuple("xyz.openbmc_project.BIOSConfig.Manager."
"BoundType.MinStringLength",
- static_cast<int64_t>(min)));
+ static_cast<int64_t>(min), attributeName));
options.push_back(
std::make_tuple("xyz.openbmc_project.BIOSConfig.Manager."
"BoundType.MaxStringLength",
- static_cast<int64_t>(max)));
+ static_cast<int64_t>(max), attributeName));
defaultValue = defString.data();
break;
}
diff --git a/libpldmresponder/bios_config.hpp b/libpldmresponder/bios_config.hpp
index 77a0764..10cc013 100644
--- a/libpldmresponder/bios_config.hpp
+++ b/libpldmresponder/bios_config.hpp
@@ -47,7 +47,9 @@
using DefaultValue = std::variant<int64_t, std::string>;
using OptionString = std::string;
using OptionValue = std::variant<int64_t, std::string>;
-using Option = std::vector<std::tuple<OptionString, OptionValue>>;
+using ValueDisplayName = std::string;
+using Option =
+ std::vector<std::tuple<OptionString, OptionValue, ValueDisplayName>>;
using BIOSTableObj =
std::tuple<AttributeType, ReadonlyStatus, DisplayName, Description,
MenuPath, CurrentValue, DefaultValue, Option>;
diff --git a/libpldmresponder/bios_enum_attribute.cpp b/libpldmresponder/bios_enum_attribute.cpp
index ba8895e..dd37057 100644
--- a/libpldmresponder/bios_enum_attribute.cpp
+++ b/libpldmresponder/bios_enum_attribute.cpp
@@ -33,6 +33,12 @@
{
defaultValues.emplace_back(val);
}
+
+ Json vdn = entry.at("value_names");
+ for (auto& val : vdn)
+ {
+ valueDisplayNames.emplace_back(val);
+ }
assert(defaultValues.size() == 1);
defaultValue = defaultValues[0];
if (dBusMap.has_value())
@@ -189,6 +195,14 @@
dbusHandler->setDbusProperty(*dBusMap, it->first);
}
+void BIOSEnumAttribute::populateValueDisplayNamesMap(uint16_t attrHandle)
+{
+ for (auto& vdn : valueDisplayNames)
+ {
+ valueDisplayNamesMap[attrHandle].push_back(vdn);
+ }
+}
+
void BIOSEnumAttribute::constructEntry(
const BIOSStringTable& stringTable, Table& attrTable, Table& attrValueTable,
std::optional<std::variant<int64_t, std::string>> optAttributeValue)
@@ -201,14 +215,15 @@
pldm_bios_table_attr_entry_enum_info info = {
stringTable.findHandle(name), readOnly,
(uint8_t)possibleValuesHandle.size(), possibleValuesHandle.data(),
- (uint8_t)defaultIndices.size(), defaultIndices.data(),
- };
+ (uint8_t)defaultIndices.size(), defaultIndices.data()};
auto attrTableEntry = table::attribute::constructEnumEntry(attrTable,
&info);
auto [attrHandle, attrType,
_] = table::attribute::decodeHeader(attrTableEntry);
+ populateValueDisplayNamesMap(attrHandle);
+
std::vector<uint8_t> currValueIndices(1, 0);
if (optAttributeValue.has_value())
diff --git a/libpldmresponder/bios_enum_attribute.hpp b/libpldmresponder/bios_enum_attribute.hpp
index b175cc4..e26a32a 100644
--- a/libpldmresponder/bios_enum_attribute.hpp
+++ b/libpldmresponder/bios_enum_attribute.hpp
@@ -69,6 +69,7 @@
private:
std::vector<std::string> possibleValues;
+ std::vector<std::string> valueDisplayNames;
std::string defaultValue;
/** @brief Get index of the given value in possible values
@@ -88,6 +89,11 @@
getPossibleValuesHandle(const BIOSStringTable& stringTable,
const std::vector<std::string>& pVs);
+ /** @brief Method to populate the valueDisplayNamesMap
+ * @param[in] attrHandle - attribute handle
+ */
+ void populateValueDisplayNamesMap(uint16_t attrHandle);
+
using ValMap = std::map<pldm::utils::PropertyValue, std::string>;
/** @brief Map of value on dbus and pldm */
diff --git a/libpldmresponder/test/libpldmresponder_bios_enum_attribute_test.cpp b/libpldmresponder/test/libpldmresponder_bios_enum_attribute_test.cpp
index bb0f652..775cd15 100644
--- a/libpldmresponder/test/libpldmresponder_bios_enum_attribute_test.cpp
+++ b/libpldmresponder/test/libpldmresponder_bios_enum_attribute_test.cpp
@@ -37,6 +37,7 @@
auto jsonEnumReadOnly = R"({
"attribute_name" : "CodeUpdatePolicy",
"possible_values" : [ "Concurrent", "Disruptive" ],
+ "value_names" : [ "Concurrent", "Disruptive" ],
"default_values" : [ "Concurrent" ],
"readOnly" : true,
"helpText" : "HelpText",
@@ -53,6 +54,7 @@
auto jsonEnumReadOnlyError = R"({
"attribute_name" : "CodeUpdatePolicy",
"possible_value" : [ "Concurrent", "Disruptive" ],
+ "value_names" : [ "Concurrent", "Disruptive" ],
"default_values" : [ "Concurrent" ],
"readOnly" : true,
"helpText" : "HelpText",
@@ -64,6 +66,7 @@
auto jsonEnumReadWrite = R"({
"attribute_name" : "FWBootSide",
"possible_values" : [ "Perm", "Temp" ],
+ "value_names" : [ "Perm", "Temp" ],
"default_values" : [ "Perm" ],
"readOnly" : false,
"helpText" : "HelpText",
@@ -91,6 +94,7 @@
auto jsonEnumReadOnly = R"({
"attribute_name" : "CodeUpdatePolicy",
"possible_values" : [ "Concurrent", "Disruptive" ],
+ "value_names" : [ "Concurrent", "Disruptive" ],
"default_values" : [ "Disruptive" ],
"readOnly" : true,
"helpText" : "HelpText",
@@ -130,6 +134,7 @@
auto jsonEnumReadWrite = R"({
"attribute_name" : "CodeUpdatePolicy",
"possible_values" : [ "Concurrent", "Disruptive" ],
+ "value_names" : [ "Concurrent", "Disruptive" ],
"default_values" : [ "Disruptive" ],
"readOnly" : false,
"helpText" : "HelpText",
@@ -182,6 +187,7 @@
auto jsonEnumReadWrite = R"({
"attribute_name" : "CodeUpdatePolicy",
"possible_values" : [ "Concurrent", "Disruptive" ],
+ "value_names" : [ "Concurrent", "Disruptive" ],
"default_values" : [ "Disruptive" ],
"readOnly" : false,
"helpText" : "HelpText",