BIOS: Fix readOnly BIOS attributes
Previously, the readOnly property is true when without a D-Bus section in BIOS
JSON. Now, the readOnly property should be determined by a new readOnly field,
which should be introduced in the JSON. The checks are updated to ensure that
if there is D-Bus mapping then only D-Bus lookups are done.
Tested: test with json
https://gist.github.com/lxwinspur/2afffff2e445fddf90dfed6eceef4014
busctl get-property xyz.openbmc_project.BIOSConfigManager
/xyz/openbmc_project/bios_config/manager xyz.openbmc_project.BIOSConfig.Manager
BaseBIOSTable
a{s(sbsssvva(sv))} 5
"CodeUpdatePolicy"
"xyz.openbmc_project.BIOSConfig.Manager.AttributeType.Enumeration"
true "" "" "" s "Concurrent" s "Concurrent" 2
"xyz.openbmc_project.BIOSConfig.Manager.BoundType.OneOf" s "Concurrent"
"xyz.openbmc_project.BIOSConfig.Manager.BoundType.OneOf" s "Disruptive"
"Led"
"xyz.openbmc_project.BIOSConfig.Manager.AttributeType.Enumeration"
false "" "" "" s "Off" s "Off" 2
"xyz.openbmc_project.BIOSConfig.Manager.BoundType.OneOf" s "On"
"xyz.openbmc_project.BIOSConfig.Manager.BoundType.OneOf" s "Off"
"Model"
"xyz.openbmc_project.BIOSConfig.Manager.AttributeType.String"
false "" "" "" s "powersupply0" s "FP5280G2" 2
"xyz.openbmc_project.BIOSConfig.Manager.BoundType.MinStringLength" x 1
"xyz.openbmc_project.BIOSConfig.Manager.BoundType.MaxStringLength" x 100
"OUTLET"
"xyz.openbmc_project.BIOSConfig.Manager.AttributeType.Integer"
false "" "" "" x 9149282306036291456 x 0 3
"xyz.openbmc_project.BIOSConfig.Manager.BoundType.LowerBound" x 0
"xyz.openbmc_project.BIOSConfig.Manager.BoundType.UpperBound" x 68002
"xyz.openbmc_project.BIOSConfig.Manager.BoundType.ScalarIncrement" x 1
"str_example3"
"xyz.openbmc_project.BIOSConfig.Manager.AttributeType.String"
true "" "" "" s "ef" s "ef" 2
"xyz.openbmc_project.BIOSConfig.Manager.BoundType.MinStringLength" x 1
"xyz.openbmc_project.BIOSConfig.Manager.BoundType.MaxStringLength" x 100
Signed-off-by: George Liu <liuxiwei@inspur.com>
Change-Id: I73d2dcb9fc65a1aa8bbd060cfb9d44e708b47aae
diff --git a/libpldmresponder/bios_attribute.cpp b/libpldmresponder/bios_attribute.cpp
index b0b834e..c519cb7 100644
--- a/libpldmresponder/bios_attribute.cpp
+++ b/libpldmresponder/bios_attribute.cpp
@@ -18,16 +18,32 @@
BIOSAttribute::BIOSAttribute(const Json& entry,
DBusHandler* const dbusHandler) :
name(entry.at("attribute_name")),
- readOnly(!entry.contains("dbus")), dbusHandler(dbusHandler)
+ readOnly(false), dbusHandler(dbusHandler)
{
+ try
+ {
+ readOnly = entry.at("readOnly");
+ }
+ catch (const std::exception& e)
+ {
+ // No action required, readOnly is initialised to false
+ }
+
if (!readOnly)
{
- std::string objectPath = entry.at("dbus").at("object_path");
- std::string interface = entry.at("dbus").at("interface");
- std::string propertyName = entry.at("dbus").at("property_name");
- std::string propertyType = entry.at("dbus").at("property_type");
+ try
+ {
+ std::string objectPath = entry.at("dbus").at("object_path");
+ std::string interface = entry.at("dbus").at("interface");
+ std::string propertyName = entry.at("dbus").at("property_name");
+ std::string propertyType = entry.at("dbus").at("property_type");
- dBusMap = {objectPath, interface, propertyName, propertyType};
+ dBusMap = {objectPath, interface, propertyName, propertyType};
+ }
+ catch (const std::exception& e)
+ {
+ // No action required, dBusMap whill have no value
+ }
}
}
diff --git a/libpldmresponder/bios_attribute.hpp b/libpldmresponder/bios_attribute.hpp
index 7a15c09..b812844 100644
--- a/libpldmresponder/bios_attribute.hpp
+++ b/libpldmresponder/bios_attribute.hpp
@@ -89,7 +89,7 @@
const std::string name;
/** Weather this attribute is read-only */
- const bool readOnly;
+ bool readOnly;
protected:
/** @brief dbus backend, nullopt if this attribute is read-only*/
diff --git a/libpldmresponder/bios_enum_attribute.cpp b/libpldmresponder/bios_enum_attribute.cpp
index e799cee..a5c7ee4 100644
--- a/libpldmresponder/bios_enum_attribute.cpp
+++ b/libpldmresponder/bios_enum_attribute.cpp
@@ -32,7 +32,7 @@
}
assert(defaultValues.size() == 1);
defaultValue = defaultValues[0];
- if (!readOnly)
+ if (dBusMap.has_value())
{
auto dbusValues = entry.at("dbus").at("property_values");
buildValMap(dbusValues);
@@ -123,7 +123,7 @@
uint8_t BIOSEnumAttribute::getAttrValueIndex()
{
auto defaultValueIndex = getValueIndex(defaultValue, possibleValues);
- if (readOnly)
+ if (readOnly || !dBusMap.has_value())
{
return defaultValueIndex;
}
@@ -150,10 +150,6 @@
uint8_t BIOSEnumAttribute::getAttrValueIndex(const PropertyValue& propValue)
{
auto defaultValueIndex = getValueIndex(defaultValue, possibleValues);
- if (readOnly)
- {
- return defaultValueIndex;
- }
try
{
@@ -176,7 +172,7 @@
const pldm_bios_attr_table_entry* attrEntry,
const BIOSStringTable& stringTable)
{
- if (readOnly)
+ if (readOnly || !dBusMap.has_value())
{
return;
}
@@ -256,7 +252,19 @@
std::string value = std::get<std::string>(attributevalue);
entry->attr_type = 0;
entry->value[0] = 1; // number of current values, default 1
- entry->value[1] = getAttrValueIndex(value);
+
+ if (readOnly)
+ {
+ entry->value[1] = getValueIndex(defaultValue, possibleValues);
+ }
+ else if (!dBusMap.has_value())
+ {
+ entry->value[1] = getValueIndex(value, possibleValues);
+ }
+ else
+ {
+ entry->value[1] = getAttrValueIndex(value);
+ }
}
} // namespace bios
diff --git a/libpldmresponder/bios_integer_attribute.cpp b/libpldmresponder/bios_integer_attribute.cpp
index a4bc14b..43a94d2 100644
--- a/libpldmresponder/bios_integer_attribute.cpp
+++ b/libpldmresponder/bios_integer_attribute.cpp
@@ -46,7 +46,7 @@
const pldm_bios_attr_val_table_entry* attrValueEntry,
const pldm_bios_attr_table_entry*, const BIOSStringTable&)
{
- if (readOnly)
+ if (readOnly || !dBusMap.has_value())
{
return;
}
@@ -160,7 +160,7 @@
uint64_t BIOSIntegerAttribute::getAttrValue()
{
- if (readOnly)
+ if (readOnly || !dBusMap.has_value())
{
return integerInfo.defaultValue;
}
diff --git a/libpldmresponder/bios_string_attribute.cpp b/libpldmresponder/bios_string_attribute.cpp
index 19819c9..41538b6 100644
--- a/libpldmresponder/bios_string_attribute.cpp
+++ b/libpldmresponder/bios_string_attribute.cpp
@@ -60,7 +60,7 @@
const pldm_bios_attr_val_table_entry* attrValueEntry,
const pldm_bios_attr_table_entry*, const BIOSStringTable&)
{
- if (readOnly)
+ if (readOnly || !dBusMap.has_value())
{
return;
}
@@ -72,7 +72,7 @@
std::string BIOSStringAttribute::getAttrValue()
{
- if (readOnly)
+ if (readOnly || !dBusMap.has_value())
{
return stringInfo.defString;
}
diff --git a/test/bios_jsons/enum_attrs.json b/test/bios_jsons/enum_attrs.json
index 24c20ff..5ad3ad2 100644
--- a/test/bios_jsons/enum_attrs.json
+++ b/test/bios_jsons/enum_attrs.json
@@ -4,6 +4,7 @@
"attribute_name" : "HMCManagedState",
"possible_values" : [ "On", "Off" ],
"default_values" : [ "On" ],
+ "readOnly" : false,
"dbus":
{
"object_path" : "/xyz/abc/def",
@@ -17,6 +18,7 @@
"attribute_name" : "FWBootSide",
"possible_values" : [ "Perm", "Temp" ],
"default_values" : [ "Perm" ],
+ "readOnly" : false,
"dbus":
{
"object_path" : "/xyz/abc/def",
@@ -30,6 +32,7 @@
"attribute_name" : "InbandCodeUpdate",
"possible_values" : [ "Allowed", "NotAllowed" ],
"default_values" : [ "Allowed" ],
+ "readOnly" : false,
"dbus":
{
"object_path" : "/xyz/abc/def",
@@ -42,7 +45,8 @@
{
"attribute_name" : "CodeUpdatePolicy",
"possible_values" : [ "Concurrent", "Disruptive" ],
- "default_values" : [ "Concurrent" ]
+ "default_values" : [ "Concurrent" ],
+ "readOnly" : true
}
]
}
diff --git a/test/bios_jsons/integer_attrs.json b/test/bios_jsons/integer_attrs.json
index b99a88b..576433e 100644
--- a/test/bios_jsons/integer_attrs.json
+++ b/test/bios_jsons/integer_attrs.json
@@ -6,6 +6,7 @@
"upper_bound" : 15,
"scalar_increment" : 1,
"default_value" : 0,
+ "readOnly" : false,
"dbus":{
"object_path" : "/xyz/openbmc_project/avsbus",
"interface" : "xyz.openbmc.AvsBus.Manager",
@@ -18,14 +19,16 @@
"lower_bound" : 1,
"upper_bound" : 15,
"scalar_increment" : 1,
- "default_value" : 2
+ "default_value" : 2,
+ "readOnly" : true
},
{
"attribute_name" : "INTEGER_INVALID_CASE",
"lower_bound" : 1,
"upper_bound" : 15,
"scalar_increment" : 2,
- "default_value" : 3
+ "default_value" : 3,
+ "readOnly" : true
}
]
}
diff --git a/test/bios_jsons/string_attrs.json b/test/bios_jsons/string_attrs.json
index 6133412..e5b3920 100644
--- a/test/bios_jsons/string_attrs.json
+++ b/test/bios_jsons/string_attrs.json
@@ -7,6 +7,7 @@
"maximum_string_length" : 100,
"default_string_length" : 3,
"default_string" : "abc",
+ "readOnly" : false,
"dbus" : {
"object_path" : "/xyz/abc/def",
"interface" : "xyz.openbmc_project.str_example1.value",
@@ -21,6 +22,7 @@
"maximum_string_length" : 100,
"default_string_length" : 0,
"default_string" : "",
+ "readOnly" : false,
"dbus" : {
"object_path" : "/xyz/abc/def",
"interface" : "xyz.openbmc_project.str_example2.value",
@@ -34,7 +36,8 @@
"minimum_string_length" : 1,
"maximum_string_length" : 100,
"default_string_length" : 2,
- "default_string" : "ef"
+ "default_string" : "ef",
+ "readOnly" : true
}
]
}
diff --git a/test/libpldmresponder_bios_attribute_test.cpp b/test/libpldmresponder_bios_attribute_test.cpp
index 8abaac7..1a7e74a 100644
--- a/test/libpldmresponder_bios_attribute_test.cpp
+++ b/test/libpldmresponder_bios_attribute_test.cpp
@@ -42,7 +42,8 @@
TEST(BIOSAttribute, CtorTest)
{
auto jsonReadOnly = R"({
- "attribute_name" : "ReadOnly"
+ "attribute_name" : "ReadOnly",
+ "readOnly" : true
})"_json;
TestAttribute readOnly{jsonReadOnly, nullptr};
@@ -58,6 +59,7 @@
auto jsonReadWrite = R"({
"attribute_name":"ReadWrite",
+ "readOnly" : false,
"dbus":
{
"object_path" : "/xyz/abc/def",
@@ -76,16 +78,4 @@
EXPECT_EQ(dbusMap->interface, "xyz.openbmc.FWBoot.Side");
EXPECT_EQ(dbusMap->propertyName, "Side");
EXPECT_EQ(dbusMap->propertyType, "bool");
-
- auto jsonReadWriteError = R"({
- "attribute_name":"ReadWrite",
- "dbus":
- {
- "object_path" : "/xyz/abc/def",
- "interface" : "xyz.openbmc.FWBoot.Side",
- "property_name" : "Side"
- }
- })"_json; // missing property_type.
-
- EXPECT_THROW((TestAttribute{jsonReadWriteError, nullptr}), Json::exception);
}
diff --git a/test/libpldmresponder_bios_enum_attribute_test.cpp b/test/libpldmresponder_bios_enum_attribute_test.cpp
index b6b53be..4a91dcb 100644
--- a/test/libpldmresponder_bios_enum_attribute_test.cpp
+++ b/test/libpldmresponder_bios_enum_attribute_test.cpp
@@ -34,7 +34,8 @@
auto jsonEnumReadOnly = R"({
"attribute_name" : "CodeUpdatePolicy",
"possible_values" : [ "Concurrent", "Disruptive" ],
- "default_values" : [ "Concurrent" ]
+ "default_values" : [ "Concurrent" ],
+ "readOnly" : true
})"_json;
BIOSEnumAttribute enumReadOnly{jsonEnumReadOnly, nullptr};
@@ -47,7 +48,8 @@
auto jsonEnumReadOnlyError = R"({
"attribute_name" : "CodeUpdatePolicy",
"possible_value" : [ "Concurrent", "Disruptive" ],
- "default_values" : [ "Concurrent" ]
+ "default_values" : [ "Concurrent" ],
+ "readOnly" : true
})"_json; // possible_value -> possible_values
EXPECT_THROW((BIOSEnumAttribute{jsonEnumReadOnlyError, nullptr}),
Json::exception);
@@ -56,6 +58,7 @@
"attribute_name" : "FWBootSide",
"possible_values" : [ "Perm", "Temp" ],
"default_values" : [ "Perm" ],
+ "readOnly" : false,
"dbus":
{
"object_path" : "/xyz/abc/def",
@@ -79,7 +82,8 @@
auto jsonEnumReadOnly = R"({
"attribute_name" : "CodeUpdatePolicy",
"possible_values" : [ "Concurrent", "Disruptive" ],
- "default_values" : [ "Disruptive" ]
+ "default_values" : [ "Disruptive" ],
+ "readOnly" : true
})"_json;
std::vector<uint8_t> expectedAttrEntry{
@@ -116,6 +120,7 @@
"attribute_name" : "CodeUpdatePolicy",
"possible_values" : [ "Concurrent", "Disruptive" ],
"default_values" : [ "Disruptive" ],
+ "readOnly" : false,
"dbus":
{
"object_path" : "/xyz/abc/def",
@@ -165,6 +170,7 @@
"attribute_name" : "CodeUpdatePolicy",
"possible_values" : [ "Concurrent", "Disruptive" ],
"default_values" : [ "Disruptive" ],
+ "readOnly" : false,
"dbus":
{
"object_path" : "/xyz/abc/def",
diff --git a/test/libpldmresponder_bios_integer_attribute_test.cpp b/test/libpldmresponder_bios_integer_attribute_test.cpp
index 48c228b..4004ae9 100644
--- a/test/libpldmresponder_bios_integer_attribute_test.cpp
+++ b/test/libpldmresponder_bios_integer_attribute_test.cpp
@@ -31,7 +31,8 @@
"lower_bound" : 1,
"upper_bound" : 15,
"scalar_increment" : 1,
- "default_value" : 2
+ "default_value" : 2,
+ "readOnly" : true
})"_json;
BIOSIntegerAttribute integerReadOnly{jsonIntegerReadOnly, nullptr};
@@ -48,7 +49,8 @@
"lower_bound" : 1,
"upper_bound" : 15,
"scalar_increment" : 1,
- "default_valu" : 2
+ "default_valu" : 2,
+ "readOnly" : true
})"_json; // default_valu -> default_value
EXPECT_THROW((BIOSIntegerAttribute{jsonIntegerReadOnlyError, nullptr}),
Json::exception);
@@ -59,6 +61,7 @@
"upper_bound" : 15,
"scalar_increment" : 1,
"default_value" : 0,
+ "readOnly" : false,
"dbus":{
"object_path" : "/xyz/openbmc_project/avsbus",
"interface" : "xyz.openbmc.AvsBus.Manager",
@@ -82,7 +85,8 @@
"lower_bound" : 1,
"upper_bound" : 15,
"scalar_increment" : 1,
- "default_value" : 2
+ "default_value" : 2,
+ "readOnly" : true
})"_json;
std::vector<uint8_t> expectedAttrEntry{
@@ -114,6 +118,7 @@
"upper_bound" : 15,
"scalar_increment" : 1,
"default_value" : 2,
+ "readOnly" : false,
"dbus":{
"object_path" : "/xyz/openbmc_project/avsbus",
"interface" : "xyz.openbmc.AvsBus.Manager",
@@ -163,6 +168,7 @@
"upper_bound" : 15,
"scalar_increment" : 1,
"default_value" : 2,
+ "readOnly" : false,
"dbus":{
"object_path" : "/xyz/openbmc_project/avsbus",
"interface" : "xyz.openbmc.AvsBus.Manager",
diff --git a/test/libpldmresponder_bios_string_attribute_test.cpp b/test/libpldmresponder_bios_string_attribute_test.cpp
index 953d47d..01079c3 100644
--- a/test/libpldmresponder_bios_string_attribute_test.cpp
+++ b/test/libpldmresponder_bios_string_attribute_test.cpp
@@ -33,7 +33,8 @@
"minimum_string_length" : 1,
"maximum_string_length" : 100,
"default_string_length" : 2,
- "default_string" : "ef"
+ "default_string" : "ef",
+ "readOnly" : true
})"_json;
BIOSStringAttribute stringReadOnly{jsonStringReadOnly, nullptr};
EXPECT_EQ(stringReadOnly.name, "str_example3");
@@ -65,6 +66,7 @@
"maximum_string_length" : 100,
"default_string_length" : 3,
"default_string" : "abc",
+ "readOnly" : false,
"dbus" : {
"object_path" : "/xyz/abc/def",
"interface" : "xyz.openbmc_project.str_example1.value",
@@ -89,7 +91,8 @@
"minimum_string_length" : 1,
"maximum_string_length" : 100,
"default_string_length" : 3,
- "default_string" : "abc"
+ "default_string" : "abc",
+ "readOnly" : true
})"_json;
std::vector<uint8_t> expectedAttrEntry{
@@ -124,6 +127,7 @@
"maximum_string_length" : 100,
"default_string_length" : 3,
"default_string" : "abc",
+ "readOnly" : false,
"dbus" : {
"object_path" : "/xyz/abc/def",
"interface" : "xyz.openbmc_project.str_example1.value",
@@ -172,6 +176,7 @@
"maximum_string_length" : 100,
"default_string_length" : 3,
"default_string" : "abc",
+ "readOnly" : false,
"dbus" : {
"object_path" : "/xyz/abc/def",
"interface" : "xyz.openbmc_project.str_example1.value",