gpio-presence: track parent inventory compatible
When detecting multiple instances of a device such as a fan module
```
{
"Exposes": [
{
"Name": "FAN1",
"PresencePinNames": [
"FAN1_PRSNT"
],
"PresencePinValues": [
1
],
"Type": "GPIODeviceDetect"
},
{
"Name": "FAN2",
"PresencePinNames": [
"FAN2_PRSNT"
],
"PresencePinValues": [
1
],
"Type": "GPIODeviceDetect"
}
],
"Name": "Minerva Fan Board 6",
"Probe": [
"xyz.openbmc_project.FruDevice({'BOARD_PRODUCT_NAME': 'Minerva Fan Board.*', 'PRODUCT_MANUFACTURER': 'Quanta', 'BUS': 21, 'BOARD_INFO_AM4': 'pwr-silergy'})",
"OR",
"xyz.openbmc_project.FruDevice({'BOARD_PRODUCT_NAME': 'Minerva Fan Board DVT', 'PRODUCT_MANUFACTURER': 'Quanta', 'BUS': 21, 'BOARD_PART_NUMBER': '3XF0MFB0030'})"
],
"Type": "Board",
"xyz.openbmc_project.Inventory.Decorator.Asset": {
"BuildDate": "$BOARD_MANUFACTURE_DATE",
"Manufacturer": "$BOARD_MANUFACTURER",
"Model": "$BOARD_PRODUCT_NAME",
"PartNumber": "$BOARD_PART_NUMBER",
"SerialNumber": "$BOARD_SERIAL_NUMBER",
"SparePartNumber": "$BOARD_INFO_AM1"
},
"xyz.openbmc_project.Inventory.Decorator.AssetTag": {
"AssetTag": "$PRODUCT_ASSET_TAG"
},
"xyz.openbmc_project.Inventory.Decorator.Compatible": {
"Names": [
"com.meta.Hardware.Minerva.FanBoard"
]
}
}
```
we want to use a single configuration file for that FRU to avoid
duplication.
```
{
"Exposes": [],
"Name": "$Name",
"Probe": [
"xyz.openbmc_project.Inventory.Source.DevicePresence({'Name': 'FAN*'})"
],
"Type": "Board",
"xyz.openbmc_project.Inventory.Decorator.Asset": {
"Manufacturer": "Unknown",
"Model": "Unknown",
"PartNumber": "Unknown",
"SerialNumber": "Unknown",
"SparePartNumber": "05-100051"
}
}
```
That creates the requirement to have `Name` property to be something
which can show up for example in the fan (or whichever) inventory.
This requirement conflicts with the requirement for `Probe` statements
to be unique and not trigger any unintended probing of unrelated
configurations.
Since `FAN 2` is a good name to use in context of a single system, but
not sufficiently unique to probe a configuration.
This patch populates a `Compatible` field on the
`xyz.openbmc_project.Inventory.Source.DevicePresence` interface which
allows to rewrite above `Probe` statement as:
```
"xyz.openbmc_project.Inventory.Source.DevicePresence({'Name': 'FAN*', 'Compatible': 'com.meta.Hardware.Minerva*'})"
```
which will be unique and not probe accidentally on other systems.
The PDI change is [4].
The value for `Compatible` field is taken from the parent inventory item
object path, which can have an optional
"xyz.openbmc_project.Inventory.Decorator.Compatible" configured.
From my perspective, entity-manager does not currently support probing
on properties which have an array type [3], so we simply take the first
value if there are multiple compatible strings on the decorator.
In case there is no such decorator, the `Compatible` string will be
empty.
Alternatives considered:
- configuring a `Compatible` property on `GPIODeviceDetect` schema.
This creates configuration bloat as all such instances are already
identified by `Name` in context of a single system.
- expanding `Probe` statement to also probe on the baseboard.
This is undesirable as currently `Probe` statement is kind of broken
[2], does not support nested expressions and also does not handle
boolean operator precedence correctly.
Tested:
Unit Tests have been adjusted.
On Tyan S8030 added following example record for some gpio.
```
{
"Name": "ExampleDevice",
"PresencePinNames": [ "BMC_SPD_SEL" ],
"PresencePinValues": [ 0 ],
"Type": "GPIODeviceDetect"
},
```
compatible decorator is not set, so compatible is empty when it detects
the Example Device.
```
xyz.openbmc_project.Inventory.Source.DevicePresence interface - - -
.Compatible property s "" emits-change writable
.Name property s "ExampleDevice" emits-change writable
```
Adding following compatible decorator
```
"xyz.openbmc_project.Inventory.Decorator.Compatible": {
"Names": [
"com.tyan.Hardware.Mainboard.S8030",
"com.tyan.Hardware.Mainboard.S8030GM"
]
}
```
And the daemon picks it up
```
Aug 22 14:43:35 s8030-bmc-30303035c0c1 gpio-presence-sensor[7336]: found configuration interface at xyz.openbmc_project.EntityManager /xyz/openbmc_project/inventory/system/board/Tyan_S8030_Baseboard/ExampleDevice xyz.openbmc_project.Configuration.GPIODeviceDetect
Aug 22 14:43:35 s8030-bmc-30303035c0c1 gpio-presence-sensor[7336]: Found 'Compatible' decorator on parent inventory path of /xyz/openbmc_project/inventory/system/board/Tyan_S8030_Baseboard/ExampleDevice
Aug 22 14:43:35 s8030-bmc-30303035c0c1 gpio-presence-sensor[7336]: adding configuration for /xyz/openbmc_project/inventory/system/board/Tyan_S8030_Baseboard/ExampleDevice
Aug 22 14:43:36 s8030-bmc-30303035c0c1 gpio-presence-sensor[7336]: found valid configuration at object path /xyz/openbmc_project/inventory/system/board/Tyan_S8030_Baseboard/ExampleDevice
Aug 22 14:43:36 s8030-bmc-30303035c0c1 gpio-presence-sensor[7336]: Watching gpio events for BMC_SPD_SEL
Aug 22 14:43:36 s8030-bmc-30303035c0c1 gpio-presence-sensor[7336]: GPIO line BMC_SPD_SEL went low
Aug 22 14:43:36 s8030-bmc-30303035c0c1 gpio-presence-sensor[7336]: Updating dbus interface for config ExampleDevice
Aug 22 14:43:36 s8030-bmc-30303035c0c1 gpio-presence-sensor[7336]: Detected ExampleDevice as present, adding dbus interface
```
Then the Compatible string is exposed as expected
```
xyz.openbmc_project.Inventory.Source.DevicePresence interface - - -
.Compatible property s "com.tyan.Hardware.Mainboard.S8030" emits-change writable
.Name property s "ExampleDevice" emits-change writable
```
References:
[1] https://gerrit.openbmc.org/c/openbmc/entity-manager/+/82819
[2] https://github.com/openbmc/entity-manager/blob/e86be7c648b6fa6e457928b4348e561b852b79bb/src/entity_manager/perform_probe.cpp#L165
[3] https://github.com/openbmc/entity-manager/blob/e86be7c648b6fa6e457928b4348e561b852b79bb/src/utils.cpp#L116
[4] https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/83061
Change-Id: I9eec81c59d1910a05bf0776f9732281473861c63
Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>
diff --git a/src/gpio-presence/device_presence.cpp b/src/gpio-presence/device_presence.cpp
index 1402996..11a76a9 100644
--- a/src/gpio-presence/device_presence.cpp
+++ b/src/gpio-presence/device_presence.cpp
@@ -25,8 +25,10 @@
DevicePresence::DevicePresence(
sdbusplus::async::context& ctx, const std::vector<std::string>& gpioNames,
const std::vector<uint64_t>& gpioValues, const std::string& deviceName,
- const std::unordered_map<std::string, bool>& gpioState) :
- deviceName(deviceName), gpioState(gpioState), ctx(ctx)
+ const std::unordered_map<std::string, bool>& gpioState,
+ const std::vector<std::string>& parentInvCompatible) :
+ deviceName(deviceName), gpioState(gpioState), ctx(ctx),
+ parentInventoryCompatible(parentInvCompatible)
{
for (size_t i = 0; i < gpioNames.size(); i++)
{
@@ -92,8 +94,13 @@
info("Detected {NAME} as present, adding dbus interface", "NAME",
deviceName);
+ const std::string firstCompatible =
+ parentInventoryCompatible.empty() ? ""
+ : parentInventoryCompatible[0];
+
detectedIface = std::make_unique<DevicePresenceInterface>(
- ctx, objPath.str.c_str(), DevicePresenceProperties{deviceName});
+ ctx, objPath.str.c_str(),
+ DevicePresenceProperties{deviceName, firstCompatible});
detectedIface->emit_added();
}
diff --git a/src/gpio-presence/device_presence.hpp b/src/gpio-presence/device_presence.hpp
index cf1a283..b9f2ccd 100644
--- a/src/gpio-presence/device_presence.hpp
+++ b/src/gpio-presence/device_presence.hpp
@@ -30,7 +30,8 @@
const std::vector<std::string>& gpioNames,
const std::vector<uint64_t>& gpioValues,
const std::string& deviceName,
- const std::unordered_map<std::string, bool>& gpioState);
+ const std::unordered_map<std::string, bool>& gpioState,
+ const std::vector<std::string>& parentInvCompatible);
auto updateGPIOPresence(const std::string& gpioLine) -> void;
@@ -53,6 +54,8 @@
sdbusplus::async::context& ctx;
+ const std::vector<std::string> parentInventoryCompatible;
+
auto updateDbusInterfaces() -> void;
// property added when the hw is detected
diff --git a/src/gpio-presence/gpio_presence_manager.cpp b/src/gpio-presence/gpio_presence_manager.cpp
index a543f50..46bbca7 100644
--- a/src/gpio-presence/gpio_presence_manager.cpp
+++ b/src/gpio-presence/gpio_presence_manager.cpp
@@ -14,6 +14,7 @@
#include <sdbusplus/message/native_types.hpp>
#include <xyz/openbmc_project/Configuration/GPIODeviceDetect/client.hpp>
#include <xyz/openbmc_project/Configuration/GPIODeviceDetect/common.hpp>
+#include <xyz/openbmc_project/Inventory/Decorator/Compatible/client.hpp>
#include <memory>
#include <ranges>
@@ -24,6 +25,7 @@
namespace gpio_presence
{
+const std::string entityManagerBusName = "xyz.openbmc_project.EntityManager";
GPIOPresenceManager::GPIOPresenceManager(sdbusplus::async::context& ctx) :
ctx(ctx), manager(ctx, "/"),
@@ -139,7 +141,7 @@
{
auto props = co_await sdbusplus::client::xyz::openbmc_project::
configuration::GPIODeviceDetect<>(ctx)
- .service("xyz.openbmc_project.EntityManager")
+ .service(entityManagerBusName)
.path(obj.str)
.properties();
@@ -150,9 +152,11 @@
co_return;
}
+ const auto parentInvCompatible = co_await getParentInventoryCompatible(obj);
+
auto devicePresence = std::make_unique<DevicePresence>(
ctx, props.presence_pin_names, props.presence_pin_values, props.name,
- gpioState);
+ gpioState, parentInvCompatible);
if (devicePresence)
{
@@ -160,6 +164,36 @@
}
}
+auto GPIOPresenceManager::getParentInventoryCompatible(
+ const sdbusplus::message::object_path& obj)
+ -> sdbusplus::async::task<std::vector<std::string>>
+{
+ const auto parentInvObjPath = obj.parent_path();
+
+ auto clientCompatible = sdbusplus::client::xyz::openbmc_project::inventory::
+ decorator::Compatible<>(ctx)
+ .service(entityManagerBusName)
+ .path(parentInvObjPath.str);
+
+ try
+ {
+ auto parentCompatibleHardware = co_await clientCompatible.names();
+ lg2::debug(
+ "Found 'Compatible' decorator on parent inventory path of {PATH}",
+ "PATH", obj);
+ co_return parentCompatibleHardware;
+ }
+ catch (std::exception& e)
+ {
+ // pass, since it is an optional interface
+ lg2::debug("Did not find interface {INTF} on path {PATH}", "INTF",
+ sdbusplus::common::xyz::openbmc_project::inventory::
+ decorator::Compatible::interface,
+ "PATH", parentInvObjPath);
+ co_return {};
+ }
+}
+
auto GPIOPresenceManager::removeConfig(const std::string& objPath) -> void
{
if (!presenceMap.contains(objPath))
diff --git a/src/gpio-presence/gpio_presence_manager.hpp b/src/gpio-presence/gpio_presence_manager.hpp
index d490fe3..bbf9f9e 100644
--- a/src/gpio-presence/gpio_presence_manager.hpp
+++ b/src/gpio-presence/gpio_presence_manager.hpp
@@ -55,6 +55,12 @@
auto addConfigFromDbusAsync(sdbusplus::message::object_path obj)
-> sdbusplus::async::task<void>;
+ // fetch the parent inventory items 'Compatible' Decorator
+ // @param[in] obj object path of our configuration
+ auto getParentInventoryCompatible(
+ const sdbusplus::message::object_path& obj)
+ -> sdbusplus::async::task<std::vector<std::string>>;
+
// delete our configuration for the object at 'objPath'
// @param[in] objPath path of the object we want to forget
auto removeConfig(const std::string& objPath) -> void;
diff --git a/test/test_gpio_presence.cpp b/test/test_gpio_presence.cpp
index 8ab309b..d6d5609 100644
--- a/test/test_gpio_presence.cpp
+++ b/test/test_gpio_presence.cpp
@@ -37,8 +37,11 @@
std::vector<std::string> gpioNames = {gpioName};
std::vector<uint64_t> gpioValues = {0};
+ std::vector<std::string> parentInvCompatible = {};
+
auto c = std::make_unique<gpio_presence::DevicePresence>(
- ctx, gpioNames, gpioValues, name, sensor.gpioState);
+ ctx, gpioNames, gpioValues, name, sensor.gpioState,
+ parentInvCompatible);
sensor.addConfig(name, std::move(c));
@@ -67,8 +70,14 @@
std::vector<std::string> gpioNames = {gpioName};
std::vector<uint64_t> gpioValues = {0};
+ std::vector<std::string> parentInvCompatible = {
+ "com.ibm.Hardware.Chassis.Model.BlueRidge4U",
+ "com.ibm.Hardware.Chassis.Model.BlueRidge",
+ };
+
auto c = std::make_unique<gpio_presence::DevicePresence>(
- ctx, gpioNames, gpioValues, name, sensor.gpioState);
+ ctx, gpioNames, gpioValues, name, sensor.gpioState,
+ parentInvCompatible);
sdbusplus::message::object_path objPath = c->getObjPath();
@@ -87,6 +96,10 @@
assert(nameFound == "cable0");
+ auto compatibleFound = co_await client.compatible();
+
+ EXPECT_EQ(compatibleFound, "com.ibm.Hardware.Chassis.Model.BlueRidge4U");
+
ctx.request_stop();
co_return;
@@ -112,8 +125,11 @@
std::vector<std::string> gpioNames = {gpioName};
std::vector<uint64_t> gpioValues = {0};
+ std::vector<std::string> parentInvCompatible = {};
+
auto c = std::make_unique<gpio_presence::DevicePresence>(
- ctx, gpioNames, gpioValues, name, sensor.gpioState);
+ ctx, gpioNames, gpioValues, name, sensor.gpioState,
+ parentInvCompatible);
sdbusplus::message::object_path objPath = c->getObjPath();