PEL: Init PHAL once the PNOR partition symlink is setup
As part of commit 8ae618, PHAL initialization was moved to pelstartup
to avoid multiple initialization calls. However, this change caused an
issue where the phosphor-logging service was pointing to the default
PHAL device tree, which lacks necessary attributes.
Root cause:
At the time the phosphor-logging service starts, the PNOR partition
symlink (which should point to the correct PHAL device tree) is not
yet created. This absence causes phosphor-logging service map to the
default device tree. The underlying issue is that the
openpower-update-bios-attr-table service, responsible for creating all
symlinks, starts after phosphor-logging due to a dependency on
phosphor-logging from a dependent service of
openpower-update-bios-attr-table service..
Proposed solution:
The phosphor-logging service will now subscribe to the 'JobRemoved'
DBUS signal from systemd service after the successful execution of
openpower--update-bios-attr-table service. Upon receiving this signal,
phosphor-logging will init PHAL, ensuring that PHAL initialization
occurs only after all symlinks have been set up.
Tested :
Before fix, journal traces has the error from dtb file open as below:
```
p10b systemd[1]: Starting OpenPower Software Update Manager...
p10b systemd[1]: Starting Phosphor Log Manager...
p10b systemd[1]: Started OpenPower Software Update Manager.
p10b systemd[1]: Started Phosphor Log Manager.
p10b phosphor-log-manager[409]: Unable to open dtb file
'/var/lib/phosphor-software-manager/pnor/rw/DEVTREE'
p10b systemd[1]: Starting Update BIOS attr table with host
firmware well-known names...
```
After fix, system is starting able to start the services without dtb
file open error.
```
p10b systemd[1]: Starting Phosphor Log Manager...
p10b phosphor-log-manager[408]: The send PELs to host setting changed
to False
p10b systemd[1]: Started Phosphor Log Manager.
p10b systemd[1]: Starting Set POWER host firmware well-known names...
p10b systemd[1]: openpower-process-host-firmware.service: Deactivated
successfully.
p10b systemd[1]: Finished Set POWER host firmware well-known names.
p10b systemd[1]: Starting Update BIOS attr table with host firmware
well-known names...
p10b systemd[1]: openpower-update-bios-attr-table.service: Deactivated
successfully.
p10b systemd[1]: Finished Update BIOS attr table with host firmware
well-known names.
```
Change-Id: I96b06581ebbc9afd6f8020f3540ed71bf06e5c19
Signed-off-by: Arya K Padman <aryakpadman@gmail.com>
diff --git a/extensions/openpower-pels/data_interface.cpp b/extensions/openpower-pels/data_interface.cpp
index 3cef1ab..2922656 100644
--- a/extensions/openpower-pels/data_interface.cpp
+++ b/extensions/openpower-pels/data_interface.cpp
@@ -22,7 +22,11 @@
#include <xyz/openbmc_project/State/BMC/server.hpp>
#include <xyz/openbmc_project/State/Boot/Progress/server.hpp>
+#include <filesystem>
+
#ifdef PEL_ENABLE_PHAL
+#include <libekb.H>
+#include <libpdbg.h>
#include <libphal.H>
#endif
@@ -47,6 +51,7 @@
constexpr auto pldm = "xyz.openbmc_project.PLDM";
constexpr auto inventoryManager = "xyz.openbmc_project.Inventory.Manager";
constexpr auto entityManager = "xyz.openbmc_project.EntityManager";
+constexpr auto systemd = "org.freedesktop.systemd1";
} // namespace service_name
namespace object_path
@@ -66,6 +71,7 @@
constexpr auto hwIsolation = "/xyz/openbmc_project/hardware_isolation";
constexpr auto biosConfigMgr = "/xyz/openbmc_project/bios_config/manager";
constexpr auto bootRawProgress = "/xyz/openbmc_project/state/boot/raw0";
+constexpr auto systemd = "/org/freedesktop/systemd1";
} // namespace object_path
namespace interface
@@ -101,6 +107,7 @@
constexpr auto invPowerSupply =
"xyz.openbmc_project.Inventory.Item.PowerSupply";
constexpr auto inventoryManager = "xyz.openbmc_project.Inventory.Manager";
+constexpr auto systemdMgr = "org.freedesktop.systemd1.Manager";
} // namespace interface
using namespace sdbusplus::server::xyz::openbmc_project::state::boot;
@@ -109,6 +116,8 @@
const DBusInterfaceList hotplugInterfaces{interface::invFan,
interface::invPowerSupply};
+static constexpr auto PDBG_DTB_PATH =
+ "/var/lib/phosphor-software-manager/hostfw/running/DEVTREE";
std::pair<std::string, std::string>
DataInterfaceBase::extractConnectorFromLocCode(
@@ -127,7 +136,8 @@
return {base, connector};
}
-DataInterface::DataInterface(sdbusplus::bus_t& bus) : _bus(bus)
+DataInterface::DataInterface(sdbusplus::bus_t& bus) :
+ _bus(bus), _systemdSlot(nullptr)
{
readBMCFWVersion();
readServerFWVersion();
@@ -222,6 +232,19 @@
}
}
}));
+
+#ifdef PEL_ENABLE_PHAL
+ if (isPHALDevTreeExist())
+ {
+ initPHAL();
+ }
+ else
+ {
+ // Watch the "openpower-update-bios-attr-table" service to init
+ // PHAL libraries
+ subscribeToSystemdSignals();
+ }
+#endif // PEL_ENABLE_PHAL
}
DBusPropertyMap DataInterface::getAllProperties(
@@ -1111,5 +1134,138 @@
// Tell the subscribers.
setFruPresent(locCode);
}
+
+#ifdef PEL_ENABLE_PHAL
+bool DataInterface::isPHALDevTreeExist() const
+{
+ try
+ {
+ if (std::filesystem::exists(PDBG_DTB_PATH))
+ {
+ return true;
+ }
+ }
+ catch (const std::exception& e)
+ {
+ lg2::error("Failed to check device tree {PHAL_DEVTREE_PATH} existence, "
+ "{ERROR}",
+ "PHAL_DEVTREE_PATH", PDBG_DTB_PATH, "ERROR", e);
+ }
+ return false;
+}
+
+void DataInterface::initPHAL()
+{
+ if (setenv("PDBG_DTB", PDBG_DTB_PATH, 1))
+ {
+ // Log message and continue,
+ // This is to help continue creating PEL in raw format.
+ lg2::error("Failed to set PDBG_DTB: ({ERRNO})", "ERRNO",
+ strerror(errno));
+ }
+
+ if (!pdbg_targets_init(NULL))
+ {
+ lg2::error("pdbg_targets_init failed");
+ return;
+ }
+
+ if (libekb_init())
+ {
+ lg2::error("libekb_init failed, skipping ffdc processing");
+ return;
+ }
+}
+
+void DataInterface::subscribeToSystemdSignals()
+{
+ try
+ {
+ auto method =
+ _bus.new_method_call(service_name::systemd, object_path::systemd,
+ interface::systemdMgr, "Subscribe");
+ _systemdSlot = method.call_async([this](sdbusplus::message_t&& msg) {
+ // Initializing with nullptr to indicate that it is not subscribed
+ // to any signal.
+ this->_systemdSlot = sdbusplus::slot_t(nullptr);
+ if (msg.is_method_error())
+ {
+ auto* error = msg.get_error();
+ lg2::error("Failed to subscribe JobRemoved systemd signal, "
+ "errorName: {ERR_NAME}, errorMsg: {ERR_MSG}, "
+ "ERR_NAME",
+ error->name, "ERR_MSG", error->message);
+ return;
+ }
+
+ namespace sdbusRule = sdbusplus::bus::match::rules;
+ this->_systemdMatch =
+ std::make_unique<decltype(this->_systemdMatch)::element_type>(
+ this->_bus,
+ sdbusRule::type::signal() +
+ sdbusRule::member("JobRemoved") +
+ sdbusRule::path(object_path::systemd) +
+ sdbusRule::interface(interface::systemdMgr),
+ [this](sdbusplus::message_t& msg) {
+ uint32_t jobID;
+ sdbusplus::message::object_path jobObjPath;
+ std::string jobUnitName, jobUnitResult;
+
+ msg.read(jobID, jobObjPath, jobUnitName, jobUnitResult);
+ if ((jobUnitName ==
+ "openpower-update-bios-attr-table.service") &&
+ (jobUnitResult == "done"))
+ {
+ // Unsubscribe immediately after the signal is
+ // received to avoid unwanted signal
+ // reception. And initialize PHAL once the
+ // unsubscription is success.
+ this->unsubscribeFromSystemdSignals();
+ }
+ });
+ });
+ }
+ catch (const sdbusplus::exception_t& e)
+ {
+ lg2::error(
+ "Exception occured while handling JobRemoved systemd signal, "
+ "exception: {ERROR}",
+ "ERROR", e);
+ }
+}
+
+void DataInterface::unsubscribeFromSystemdSignals()
+{
+ try
+ {
+ auto method =
+ _bus.new_method_call(service_name::systemd, object_path::systemd,
+ interface::systemdMgr, "Unsubscribe");
+ _systemdSlot = method.call_async([this](sdbusplus::message_t&& msg) {
+ // Unsubscribing the _systemdSlot from the subscribed signal
+ this->_systemdSlot = sdbusplus::slot_t(nullptr);
+ if (msg.is_method_error())
+ {
+ auto* error = msg.get_error();
+ lg2::error(
+ "Failed to unsubscribe from JobRemoved systemd signal, "
+ "errorName: {ERR_NAME}, errorMsg: {ERR_MSG}, "
+ "ERR_NAME",
+ error->name, "ERR_MSG", error->message);
+ return;
+ }
+ this->initPHAL();
+ });
+ }
+ catch (const sdbusplus::exception_t& e)
+ {
+ lg2::error(
+ "Exception occured while unsubscribing from JobRemoved systemd signal, "
+ "exception: {ERROR}",
+ "ERROR", e);
+ }
+}
+#endif // PEL_ENABLE_PHAL
+
} // namespace pels
} // namespace openpower
diff --git a/extensions/openpower-pels/data_interface.hpp b/extensions/openpower-pels/data_interface.hpp
index 1626293..5ba9c7f 100644
--- a/extensions/openpower-pels/data_interface.hpp
+++ b/extensions/openpower-pels/data_interface.hpp
@@ -1002,6 +1002,37 @@
*/
static std::string addLocationCodePrefix(const std::string& locationCode);
+#ifdef PEL_ENABLE_PHAL
+ /**
+ * @brief A helper API to check whether the PHAL device tree is exists,
+ * ensuring the PHAL init API can be invoked.
+ *
+ * @return true if the PHAL device tree is exists, otherwise false
+ */
+ bool isPHALDevTreeExist() const;
+
+ /**
+ * @brief A helper API to init PHAL libraries
+ *
+ * @return None
+ */
+ void initPHAL();
+
+ /**
+ * @brief A helper API to subscribe to systemd signals
+ *
+ * @return None
+ */
+ void subscribeToSystemdSignals();
+
+ /**
+ * @brief A helper API to unsubscribe to systemd signals
+ *
+ * @return None
+ */
+ void unsubscribeFromSystemdSignals();
+#endif // PEL_ENABLE_PHAL
+
/**
* @brief The D-Bus property or interface watchers that have callbacks
* registered that will set members in this class when
@@ -1023,6 +1054,19 @@
* @brief The sdbusplus bus object for making D-Bus calls.
*/
sdbusplus::bus_t& _bus;
+
+#ifdef PEL_ENABLE_PHAL
+ /**
+ * @brief Watcher to check "openpower-update-bios-attr-table" service
+ * is "done" to init PHAL libraires
+ */
+ std::unique_ptr<sdbusplus::bus::match_t> _systemdMatch;
+#endif // PEL_ENABLE_PHAL
+
+ /**
+ * @brief A slot object for async dbus call
+ */
+ sdbusplus::slot_t _systemdSlot;
};
} // namespace pels
diff --git a/extensions/openpower-pels/entry_points.cpp b/extensions/openpower-pels/entry_points.cpp
index 0b58212..dd00103 100644
--- a/extensions/openpower-pels/entry_points.cpp
+++ b/extensions/openpower-pels/entry_points.cpp
@@ -25,11 +25,6 @@
#include <format>
-#ifdef PEL_ENABLE_PHAL
-#include "libekb.H"
-#include "libpdbg.h"
-#endif
-
namespace openpower
{
namespace pels
@@ -64,33 +59,6 @@
manager = std::make_unique<Manager>(logManager, std::move(dataIface),
std::move(logger), std::move(journal));
#endif
-
-#ifdef PEL_ENABLE_PHAL
- // PDBG_DTB environment variable set to CEC device tree path
- static constexpr auto PDBG_DTB_PATH =
- "/var/lib/phosphor-software-manager/pnor/rw/DEVTREE";
-
- if (setenv("PDBG_DTB", PDBG_DTB_PATH, 1))
- {
- // Log message and continue,
- // This is to help continue creating PEL in raw format.
- lg2::error("Failed to set PDBG_DTB: ({ERRNO})", "ERRNO",
- strerror(errno));
- }
-
- if (!pdbg_targets_init(NULL))
- {
- lg2::error("pdbg_targets_init failed");
- return;
- }
-
- if (libekb_init())
- {
- lg2::error("libekb_init failed, skipping ffdc processing");
- return;
- }
-
-#endif
}
REGISTER_EXTENSION_FUNCTION(pelStartup)