Add assoc-trim-path option and Board match
When a custom object path is being used (not the default object path
that comes from IPMI), allow the Board interface to be a valid
interface for associating the Inventory object with, not just the
System interface.
The purpose is to support large systems of the blade server style,
which have CPU and DIMM attached to a Board that is itself attached to
a System, not attached to the System directly.
Adding the assoc-trim-path option, disabled by default, to drop the
rightmost path component when setting up this association. This
bypasses the Board and associates the CPU and DIMM directly with the
underlying System.
Cleaning up the systemInfoUpdate() function, to correct a logic error
in which the match rule would not be set up for later if an exception
happened the first time. Also sleeping, to work around a race condition
with Object Mapper.
Tested: I tested it both with and without the assoc-trim-path option,
and it seemed to have the desired effect. With this option enabled, I
was able to attach CPU and DIMM to the proper System, even though there
was a Board object in the hierarchy between them.
Change-Id: Ibb5302e01b9d1b0453bdb14092ede594a9e71415
Signed-off-by: Josh Lehan <krellan@google.com>
diff --git a/include/mdrv2.hpp b/include/mdrv2.hpp
index 5516fac..315bdab 100644
--- a/include/mdrv2.hpp
+++ b/include/mdrv2.hpp
@@ -59,6 +59,8 @@
"/xyz/openbmc_project/inventory/system";
static constexpr const char* systemInterface =
"xyz.openbmc_project.Inventory.Item.System";
+static constexpr const char* boardInterface =
+ "xyz.openbmc_project.Inventory.Item.Board";
constexpr const int limitEntryLen = 0xff;
// Avoid putting multiple interfaces with same name on same object
diff --git a/meson.options b/meson.options
index 8072041..1a4cdf8 100644
--- a/meson.options
+++ b/meson.options
@@ -20,6 +20,13 @@
)
option(
+ 'assoc-trim-path',
+ type: 'feature',
+ value: 'disabled',
+ description: 'Trim one object path component from CPU and DIMM associations'
+)
+
+option(
'cpuinfo',
type: 'feature',
value: 'enabled',
diff --git a/src/mdrv2.cpp b/src/mdrv2.cpp
index 383709b..8067dc7 100644
--- a/src/mdrv2.cpp
+++ b/src/mdrv2.cpp
@@ -415,7 +415,14 @@
mapperInterface, "GetSubTreePaths");
method.append(mapperAncestorPath);
method.append(0);
- method.append(std::vector<std::string>({systemInterface}));
+
+ // If customized, also accept Board as anchor, not just System
+ std::vector<std::string> desiredInterfaces{systemInterface};
+ if (requireExactMatch)
+ {
+ desiredInterfaces.emplace_back(boardInterface);
+ }
+ method.append(desiredInterfaces);
try
{
@@ -434,42 +441,6 @@
motherboardPath = std::move(paths[i]);
break;
}
-
- if (motherboardPath.empty())
- {
- phosphor::logging::log<phosphor::logging::level::ERR>(
- "Failed to get system motherboard dbus path. Setting up a "
- "match rule");
-
- if (!motherboardConfigMatch)
- {
- motherboardConfigMatch =
- std::make_unique<sdbusplus::bus::match_t>(
- *bus,
- sdbusplus::bus::match::rules::interfacesAdded() +
- sdbusplus::bus::match::rules::argNpath(
- 0, matchParentPath),
- [this](sdbusplus::message_t& msg) {
- sdbusplus::message::object_path objectName;
- boost::container::flat_map<
- std::string,
- boost::container::flat_map<
- std::string, std::variant<std::string, uint64_t>>>
- msgData;
- msg.read(objectName, msgData);
- if (msgData.contains(systemInterface))
- {
- systemInfoUpdate();
- }
- });
- }
- }
- else
- {
- lg2::info(
- "Found Inventory anchor object for SMBIOS content {I}: {M}",
- "I", smbiosInventoryPath, "M", motherboardPath);
- }
}
catch (const sdbusplus::exception_t& e)
{
@@ -481,6 +452,76 @@
phosphor::logging::entry("ERROR=%s", e.what()));
}
+ if (motherboardPath.empty())
+ {
+ phosphor::logging::log<phosphor::logging::level::ERR>(
+ "Failed to get system motherboard dbus path. Setting up a "
+ "match rule");
+
+ if (motherboardConfigMatch)
+ {
+ lg2::info("Motherboard match rule already exists");
+ }
+ else
+ {
+ motherboardConfigMatch = std::make_unique<sdbusplus::bus::match_t>(
+ *bus,
+ sdbusplus::bus::match::rules::interfacesAdded() +
+ sdbusplus::bus::match::rules::argNpath(0, matchParentPath),
+ [this, requireExactMatch](sdbusplus::message_t& msg) {
+ sdbusplus::message::object_path objectName;
+ boost::container::flat_map<
+ std::string,
+ boost::container::flat_map<
+ std::string, std::variant<std::string, uint64_t>>>
+ msgData;
+ msg.read(objectName, msgData);
+ bool gotMatch = false;
+
+ if (msgData.contains(systemInterface))
+ {
+ lg2::info("Successful match on system interface");
+ gotMatch = true;
+ }
+
+ // If customized, also accept Board as anchor, not just System
+ if (requireExactMatch && msgData.contains(boardInterface))
+ {
+ lg2::info("Successful match on board interface");
+ gotMatch = true;
+ }
+
+ if (gotMatch)
+ {
+ // There is a race condition here: our desired interface
+ // has just been created, triggering the D-Bus callback,
+ // but Object Mapper has not been told of it yet. The
+ // mapper must also add it. Stall for time, so it can.
+ sleep(2);
+ systemInfoUpdate();
+ }
+ });
+ }
+ }
+ else
+ {
+#ifdef ASSOC_TRIM_PATH
+ // When enabled, chop off last component of motherboardPath, to trim one
+ // layer, so that associations are built to the underlying chassis
+ // itself, not the system boards in the chassis. This is for
+ // compatibility with traditional systems which only have one
+ // motherboard per chassis.
+ std::filesystem::path foundPath(motherboardPath);
+ motherboardPath = foundPath.parent_path().string();
+#endif
+
+ lg2::info("Found Inventory anchor object for SMBIOS content {I}: {M}",
+ "I", smbiosInventoryPath, "M", motherboardPath);
+ }
+
+ lg2::info("Using Inventory anchor object for SMBIOS content {I}: {M}", "I",
+ smbiosInventoryPath, "M", motherboardPath);
+
std::optional<size_t> num = getTotalCpuSlot();
if (!num)
{
diff --git a/src/meson.build b/src/meson.build
index 4a54d28..620eae4 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -3,6 +3,10 @@
cpp_args_smbios += ['-DDIMM_DBUS']
endif
+if get_option('assoc-trim-path').allowed()
+ cpp_args_smbios += ['-DASSOC_TRIM_PATH']
+endif
+
if get_option('dimm-only-locator').allowed()
cpp_args_smbios += ['-DDIMM_ONLY_LOCATOR']
endif