pldm: Build BIOS Table Lazily
Entity manager service hosts the compatible interface and populates the
Names property with the chassis type. It is seen that the Names property
is updated pretty late on the way to BMC ready state. And by that time
pldmd is already creating the bios table assuming that the system type
is not detected.
To fix this behavior, rather than creating the bios table early, we need
to wait until we get the system type from the entity manager daemon.
System Type is fetched from Entity Manager Decorator.Compatible
interface [1].
PLDM registers the service name only after building the BIOS tables, to
avoids failures to bios updates from other applications
Tested: Power off/On successfully in Simulator
[1] https://github.com/openbmc/entity-manager/commit/9bac6409d4bd684f058517f41de33ba1d17e5666
Change-Id: I4d431061eaaf8842f6382c2e83807f725653e19b
Signed-off-by: Archana Kakani <archana.kakani@ibm.com>
diff --git a/libpldmresponder/bios.cpp b/libpldmresponder/bios.cpp
index a6c0c99..48a3f28 100644
--- a/libpldmresponder/bios.cpp
+++ b/libpldmresponder/bios.cpp
@@ -71,13 +71,12 @@
Handler::Handler(
int fd, uint8_t eid, pldm::InstanceIdDb* instanceIdDb,
pldm::requester::Handler<pldm::requester::Request>* handler,
- pldm::responder::platform_config::Handler* platformConfigHandler) :
+ pldm::responder::platform_config::Handler* platformConfigHandler,
+ pldm::responder::bios::Callback requestPLDMServiceName) :
biosConfig(BIOS_JSONS_DIR, BIOS_TABLES_DIR, &dbusHandler, fd, eid,
- instanceIdDb, handler, platformConfigHandler)
+ instanceIdDb, handler, platformConfigHandler,
+ requestPLDMServiceName)
{
- biosConfig.removeTables();
- biosConfig.buildTables();
-
handlers.emplace(
PLDM_SET_DATE_TIME,
[this](pldm_tid_t, const pldm_msg* request, size_t payloadLength) {
diff --git a/libpldmresponder/bios.hpp b/libpldmresponder/bios.hpp
index 6923063..8cb18c0 100644
--- a/libpldmresponder/bios.hpp
+++ b/libpldmresponder/bios.hpp
@@ -35,10 +35,13 @@
* @param[in] instanceIdDb - pointer to an InstanceIdDb object
* @param[in] handler - PLDM request handler
* @param[in] platformConfigHandler - pointer to platform config object
+ * @param[in] requestPLDMServiceName - Callback for registering the PLDM
+ * service
*/
Handler(int fd, uint8_t eid, pldm::InstanceIdDb* instanceIdDb,
pldm::requester::Handler<pldm::requester::Request>* handler,
- pldm::responder::platform_config::Handler* platformConfigHandler);
+ pldm::responder::platform_config::Handler* platformConfigHandler,
+ pldm::responder::bios::Callback requestPLDMServiceName);
/** @brief Handler for GetDateTime
*
diff --git a/libpldmresponder/bios_config.cpp b/libpldmresponder/bios_config.cpp
index c5c549c..6cc9c24 100644
--- a/libpldmresponder/bios_config.cpp
+++ b/libpldmresponder/bios_config.cpp
@@ -9,6 +9,7 @@
#include <phosphor-logging/lg2.hpp>
#include <xyz/openbmc_project/BIOSConfig/Manager/server.hpp>
+#include <filesystem>
#include <fstream>
#ifdef OEM_IBM
@@ -44,12 +45,24 @@
const char* jsonDir, const char* tableDir, DBusHandler* const dbusHandler,
int fd, uint8_t eid, pldm::InstanceIdDb* instanceIdDb,
pldm::requester::Handler<pldm::requester::Request>* handler,
- pldm::responder::platform_config::Handler* platformConfigHandler) :
+ pldm::responder::platform_config::Handler* platformConfigHandler,
+ pldm::responder::bios::Callback requestPLDMServiceName) :
jsonDir(jsonDir),
tableDir(tableDir), dbusHandler(dbusHandler), fd(fd), eid(eid),
instanceIdDb(instanceIdDb), handler(handler),
- platformConfigHandler(platformConfigHandler)
+ platformConfigHandler(platformConfigHandler),
+ requestPLDMServiceName(requestPLDMServiceName)
+{
+ fs::create_directories(tableDir);
+ removeTables();
+ if (isSystemTypeAvailable())
+ {
+ initBIOSAttributes(sysType);
+ }
+ listenPendingAttributes();
+}
+bool BIOSConfig::isSystemTypeAvailable()
{
if (platformConfigHandler)
{
@@ -58,10 +71,29 @@
{
sysType = systemType.value();
}
+ else
+ {
+ platformConfigHandler->registerSystemTypeCallback(std::bind(
+ &BIOSConfig::initBIOSAttributes, this, std::placeholders::_1));
+ return false;
+ }
}
- fs::create_directories(tableDir);
+ return true;
+}
+
+void BIOSConfig::initBIOSAttributes(const std::string& systemType)
+{
+ sysType = systemType;
+ fs::path dir{jsonDir / sysType};
+ if (!fs::exists(dir))
+ {
+ error("System specific bios attribute directory {DIR} does not exit",
+ "DIR", dir.string());
+ return;
+ }
constructAttributes();
- listenPendingAttributes();
+ buildTables();
+ requestPLDMServiceName();
}
void BIOSConfig::buildTables()
@@ -495,6 +527,7 @@
void BIOSConfig::constructAttributes()
{
+ info("Bios Attribute file path: {PATH}", "PATH", (jsonDir / sysType));
load(jsonDir / sysType / stringJsonFile, [this](const Json& entry) {
constructAttribute<BIOSStringAttribute>(entry);
});
diff --git a/libpldmresponder/bios_config.hpp b/libpldmresponder/bios_config.hpp
index 36a7739..2cb13e4 100644
--- a/libpldmresponder/bios_config.hpp
+++ b/libpldmresponder/bios_config.hpp
@@ -58,6 +58,7 @@
using PendingObj = std::tuple<AttributeType, CurrentValue>;
using PendingAttributes = std::map<AttributeName, PendingObj>;
+using Callback = std::function<void()>;
/** @class BIOSConfig
* @brief Manager BIOS Attributes
@@ -81,13 +82,16 @@
* @param[in] instanceIdDb - pointer to an InstanceIdDb object
* @param[in] handler - PLDM request handler
* @param[in] platformConfigHandler - pointer to platform config Handler
+ * @param[in] requestPLDMServiceName - Callback for claiming the PLDM
+ * service name Called only after building BIOS tables.
*/
explicit BIOSConfig(
const char* jsonDir, const char* tableDir,
pldm::utils::DBusHandler* const dbusHandler, int fd, uint8_t eid,
pldm::InstanceIdDb* instanceIdDb,
pldm::requester::Handler<pldm::requester::Request>* handler,
- pldm::responder::platform_config::Handler* platformConfigHandler);
+ pldm::responder::platform_config::Handler* platformConfigHandler,
+ pldm::responder::bios::Callback requestPLDMServiceName);
/** @brief Set attribute value on dbus and attribute value table
* @param[in] entry - attribute value entry
@@ -126,6 +130,13 @@
int setBIOSTable(uint8_t tableType, const Table& table,
bool updateBaseBIOSTable = true);
+ /** @brief Construct the BIOS Attributes and build the tables
+ * after receiving system type from entity manager.
+ * @param[in] String - System Type
+ * @return void
+ */
+ void initBIOSAttributes(const std::string& sysType);
+
private:
/** @enum Index into the fields in the BaseBIOSTable
*/
@@ -163,6 +174,9 @@
/** @brief platform config Handler*/
pldm::responder::platform_config::Handler* platformConfigHandler;
+ /** @brief Callback for registering the PLDM service name */
+ pldm::responder::bios::Callback requestPLDMServiceName;
+
// vector persists all attributes
using BIOSAttributes = std::vector<std::unique_ptr<BIOSAttribute>>;
BIOSAttributes biosAttributes;
@@ -189,6 +203,13 @@
void processBiosAttrChangeNotification(
const DbusChObjProperties& chProperties, uint32_t biosAttrIndex);
+ /** @brief Method to get know if the system type is received from entity
+ * manager or if we want to use the default bios json files.
+ * @return - Returns true is the system type is received from EM or
+ * if default option is chosen
+ */
+ bool isSystemTypeAvailable();
+
/** @brief Construct an attribute and persist it
* @tparam T - attribute type
* @param[in] entry - json entry
diff --git a/libpldmresponder/platform_config.cpp b/libpldmresponder/platform_config.cpp
index 9c56385..87fd334 100644
--- a/libpldmresponder/platform_config.cpp
+++ b/libpldmresponder/platform_config.cpp
@@ -40,6 +40,10 @@
{
// get only the first system type
systemType = names.front();
+ if (sysTypeCallback)
+ {
+ sysTypeCallback(systemType);
+ }
}
if (!systemType.empty())
@@ -119,6 +123,11 @@
return std::nullopt;
}
+void Handler::registerSystemTypeCallback(SystemTypeCallback callback)
+{
+ sysTypeCallback = callback;
+}
+
} // namespace platform_config
} // namespace responder
diff --git a/libpldmresponder/platform_config.hpp b/libpldmresponder/platform_config.hpp
index 265a419..3934c61 100644
--- a/libpldmresponder/platform_config.hpp
+++ b/libpldmresponder/platform_config.hpp
@@ -19,6 +19,8 @@
"xyz.openbmc_project.Inventory.Decorator.Compatible";
static constexpr auto namesProperty = "Names";
+using SystemTypeCallback = std::function<void(const std::string&)>;
+
class Handler : public CmdHandler
{
public:
@@ -32,9 +34,10 @@
"xyz.openbmc_project.EntityManager"),
std::bind(&Handler::systemCompatibleCallback, this,
std::placeholders::_1));
+ sysTypeCallback = nullptr;
}
- /** @brief Interface to get the system type information
+ /** @brief Interface to get the system type information using Dbus query
*
* @return - the system type information
*/
@@ -43,12 +46,18 @@
/** @brief D-Bus Interface added signal match for Entity Manager */
void systemCompatibleCallback(sdbusplus::message_t& msg);
+ /** @brief Registers the callback from other objects */
+ void registerSystemTypeCallback(SystemTypeCallback callback);
+
private:
/** @brief system type/model */
std::string systemType;
/** @brief D-Bus Interface added signal match for Entity Manager */
std::unique_ptr<sdbusplus::bus::match_t> systemCompatibleMatchCallBack;
+
+ /** @brief Registered Callback */
+ SystemTypeCallback sysTypeCallback;
};
} // namespace platform_config
diff --git a/libpldmresponder/test/libpldmresponder_bios_config_test.cpp b/libpldmresponder/test/libpldmresponder_bios_config_test.cpp
index d073e85..3a15992 100644
--- a/libpldmresponder/test/libpldmresponder_bios_config_test.cpp
+++ b/libpldmresponder/test/libpldmresponder_bios_config_test.cpp
@@ -91,13 +91,11 @@
MockdBusHandler dbusHandler;
MockSystemConfig mockSystemConfig;
- EXPECT_CALL(mockSystemConfig, getPlatformName()).WillOnce(Return(""));
- ON_CALL(dbusHandler, getDbusPropertyVariant(_, _, _))
- .WillByDefault(Throw(std::exception()));
+ EXPECT_CALL(mockSystemConfig, getPlatformName())
+ .WillOnce(Return(std::filesystem::path("bios_jsons")));
- BIOSConfig biosConfig("./bios_jsons", tableDir.c_str(), &dbusHandler, 0, 0,
- nullptr, nullptr, &mockSystemConfig);
- biosConfig.buildTables();
+ BIOSConfig biosConfig("./", tableDir.c_str(), &dbusHandler, 0, 0, nullptr,
+ nullptr, &mockSystemConfig, []() {});
auto stringTable = biosConfig.getBIOSTable(PLDM_BIOS_STRING_TABLE);
auto attrTable = biosConfig.getBIOSTable(PLDM_BIOS_ATTR_TABLE);
auto attrValueTable = biosConfig.getBIOSTable(PLDM_BIOS_ATTR_VAL_TABLE);
@@ -271,9 +269,7 @@
BIOSConfig biosConfig("./system_type1/bios_jsons", tableDir.c_str(),
&dbusHandler, 0, 0, nullptr, nullptr,
- &mockSystemConfig);
-
- biosConfig.buildTables();
+ &mockSystemConfig, []() {});
auto stringTable = biosConfig.getBIOSTable(PLDM_BIOS_STRING_TABLE);
auto attrTable = biosConfig.getBIOSTable(PLDM_BIOS_ATTR_TABLE);
@@ -333,6 +329,76 @@
}
}
+TEST_F(TestBIOSConfig, setBIOSTable)
+{
+ MockdBusHandler dbusHandler;
+ MockSystemConfig mockSystemConfig;
+
+ EXPECT_CALL(mockSystemConfig, getPlatformName()).WillOnce(Return("jsons"));
+ BIOSConfig biosConfig("./", tableDir.c_str(), &dbusHandler, 0, 0, nullptr,
+ nullptr, &mockSystemConfig, []() {});
+
+ std::set<std::string> strings{"pvm_system_name", "pvm_stop_at_standby",
+ "fw_boot_side", "fw_next_boot_side"};
+
+ Table table;
+ for (const auto& elem : strings)
+ {
+ table::string::constructEntry(table, elem);
+ }
+
+ table::appendPadAndChecksum(table);
+ auto rc = biosConfig.setBIOSTable(PLDM_BIOS_STRING_TABLE, table);
+ EXPECT_EQ(rc, PLDM_SUCCESS);
+
+ auto stringTable = biosConfig.getBIOSTable(PLDM_BIOS_STRING_TABLE);
+ EXPECT_TRUE(stringTable);
+}
+
+TEST_F(TestBIOSConfig, getBIOSTableFailure)
+{
+ MockdBusHandler dbusHandler;
+ MockSystemConfig mockSystemConfig;
+
+ EXPECT_CALL(mockSystemConfig, getPlatformName()).WillOnce(Return("jsons"));
+ BIOSConfig biosConfig("./", tableDir.c_str(), &dbusHandler, 0, 0, nullptr,
+ nullptr, &mockSystemConfig, []() {});
+
+ auto stringTable = biosConfig.getBIOSTable(PLDM_BIOS_STRING_TABLE);
+ auto attrTable = biosConfig.getBIOSTable(PLDM_BIOS_ATTR_TABLE);
+ auto attrValueTable = biosConfig.getBIOSTable(PLDM_BIOS_ATTR_VAL_TABLE);
+
+ EXPECT_FALSE(stringTable);
+ EXPECT_FALSE(attrTable);
+ EXPECT_FALSE(attrValueTable);
+}
+
+TEST_F(TestBIOSConfig, setAttrValueFailure)
+{
+ MockdBusHandler dbusHandler;
+ MockSystemConfig mockSystemConfig;
+
+ EXPECT_CALL(mockSystemConfig, getPlatformName()).WillOnce(Return("jsons"));
+ BIOSConfig biosConfig("./", tableDir.c_str(), &dbusHandler, 0, 0, nullptr,
+ nullptr, &mockSystemConfig, []() {});
+
+ std::vector<uint8_t> attrValueEntry{
+ 0, 0, /* attr handle */
+ 1, /* attr type string read-write */
+ 4, 0, /* current string length */
+ 'a', 'b', 'c', 'd', /* defaut value string handle index */
+ };
+
+ uint16_t attrHandle{10};
+ attrValueEntry[0] = attrHandle & 0xff;
+ attrValueEntry[1] = (attrHandle >> 8) & 0xff;
+
+ auto rc = biosConfig.setAttrValue(attrValueEntry.data(),
+ attrValueEntry.size(), false);
+ std::cout << "Error in settig Attribute " << rc << std::endl;
+ EXPECT_EQ(rc, PLDM_BIOS_TABLE_UNAVAILABLE);
+}
+
TEST_F(TestBIOSConfig, setAttrValue)
{
MockdBusHandler dbusHandler;
@@ -340,9 +406,7 @@
EXPECT_CALL(mockSystemConfig, getPlatformName()).WillOnce(Return(""));
BIOSConfig biosConfig("./bios_jsons", tableDir.c_str(), &dbusHandler, 0, 0,
- nullptr, nullptr, &mockSystemConfig);
- biosConfig.removeTables();
- biosConfig.buildTables();
+ nullptr, nullptr, &mockSystemConfig, []() {});
auto stringTable = biosConfig.getBIOSTable(PLDM_BIOS_STRING_TABLE);
auto attrTable = biosConfig.getBIOSTable(PLDM_BIOS_ATTR_TABLE);