PEL: Handle location codes for connectors
There are cases where PELs need to callout physical system connectors,
such as a USB or other cable connector. This would be done by
specifying the connector's location code in either the message registry
JSON or device callout JSON. The problem is that the connector may not
be modeled by the inventory, and even if it was, there would need to be
a way to get the serial number, etc for the FRU that connector is on to
use in the callout
Instead of forcing all connectors to be modeled in the inventory along
with a copy of the SN/FN/CC from the parent FRU just to do a callout,
this commit will just pop off the connector segment of the location
code, a '-Tx', before trying to expand it or getting its related
inventory item. That way, it will be operating on the parent FRU which
would be in the inventory with its SN/FN/CC. In the case of the
location code expansion, the connector segment will then just be
re-appended afterwards.
This commit also makes a change to the device path callouts code to
expand the location code of the callout from the device path JSON
instead of trying to look it up later, which would have lost the
connector portion.
Note that when a cable itself wants to be called out, there usually
would need be a symbolic FRU specified saying which cable it is, along
with the connector location codes. In that case, the SN/FN/CC of the
parent FRU would not show up in the PEL.
Change-Id: I8e0eb58dfe630858f75e64b11ac13432bff4d2d1
diff --git a/extensions/openpower-pels/data_interface.cpp b/extensions/openpower-pels/data_interface.cpp
index 2ef99f4..89b1f54 100644
--- a/extensions/openpower-pels/data_interface.cpp
+++ b/extensions/openpower-pels/data_interface.cpp
@@ -85,6 +85,23 @@
using sdbusplus::exception::SdBusError;
using namespace phosphor::logging;
+std::pair<std::string, std::string>
+ DataInterfaceBase::extractConnectorFromLocCode(
+ const std::string& locationCode)
+{
+ auto base = locationCode;
+ std::string connector{};
+
+ auto pos = base.find("-T");
+ if (pos != std::string::npos)
+ {
+ connector = base.substr(pos);
+ base = base.substr(0, pos);
+ }
+
+ return {base, connector};
+}
+
DataInterface::DataInterface(sdbusplus::bus::bus& bus) : _bus(bus)
{
readBMCFWVersion();
@@ -414,18 +431,28 @@
std::string DataInterface::expandLocationCode(const std::string& locationCode,
uint16_t /*node*/) const
{
+ // Location codes for connectors are the location code of the FRU they are
+ // on, plus a '-Tx' segment. Remove this last segment before expanding it
+ // and then add it back in afterwards. This way, the connector doesn't have
+ // to be in the model just so that it can be expanded.
+ auto [baseLoc, connectorLoc] = extractConnectorFromLocCode(locationCode);
+
auto method =
_bus.new_method_call(service_name::vpdManager, object_path::vpdManager,
interface::vpdManager, "GetExpandedLocationCode");
- method.append(addLocationCodePrefix(locationCode),
- static_cast<uint16_t>(0));
+ method.append(addLocationCodePrefix(baseLoc), static_cast<uint16_t>(0));
auto reply = _bus.call(method);
std::string expandedLocationCode;
reply.read(expandedLocationCode);
+ if (!connectorLoc.empty())
+ {
+ expandedLocationCode += connectorLoc;
+ }
+
return expandedLocationCode;
}
@@ -436,17 +463,23 @@
std::string methodName = expanded ? "GetFRUsByExpandedLocationCode"
: "GetFRUsByUnexpandedLocationCode";
+ // Remove the connector segment, if present, so that this method call
+ // returns an inventory path that getHWCalloutFields() can be used with.
+ // (The serial number, etc, aren't stored on the connector in the
+ // inventory, and may not even be modeled.)
+ auto [baseLoc, connectorLoc] = extractConnectorFromLocCode(locationCode);
+
auto method =
_bus.new_method_call(service_name::vpdManager, object_path::vpdManager,
interface::vpdManager, methodName.c_str());
if (expanded)
{
- method.append(locationCode);
+ method.append(baseLoc);
}
else
{
- method.append(addLocationCodePrefix(locationCode), node);
+ method.append(addLocationCodePrefix(baseLoc), node);
}
auto reply = _bus.call(method);
diff --git a/extensions/openpower-pels/data_interface.hpp b/extensions/openpower-pels/data_interface.hpp
index d18223a..7a203ae 100644
--- a/extensions/openpower-pels/data_interface.hpp
+++ b/extensions/openpower-pels/data_interface.hpp
@@ -302,6 +302,23 @@
*/
virtual bool getQuiesceOnError() const = 0;
+ /**
+ * @brief Split location code into base and connector segments
+ *
+ * A location code that ends in '-Tx', where 'x' is a number,
+ * represents a connector, such as a USB cable connector.
+ *
+ * This function splits the passed in location code into a
+ * base and connector segment. e.g.:
+ * P0-T1 -> ['P0', '-T1']
+ * P0 -> ['P0', '']
+ *
+ * @param[in] locationCode - location code to split
+ * @return pair<string, string> - The base and connector segments
+ */
+ static std::pair<std::string, std::string>
+ extractConnectorFromLocCode(const std::string& locationCode);
+
protected:
/**
* @brief Sets the host on/off state and runs any
diff --git a/extensions/openpower-pels/src.cpp b/extensions/openpower-pels/src.cpp
index 9c0c3dd..b3e3e89 100644
--- a/extensions/openpower-pels/src.cpp
+++ b/extensions/openpower-pels/src.cpp
@@ -1133,13 +1133,25 @@
}
}
+ std::optional<std::string> locCode;
+
+ try
+ {
+ locCode = dataIface.expandLocationCode(callout.locationCode, 0);
+ }
+ catch (const std::exception& e)
+ {
+ auto msg = fmt::format("Unable to expand location code {}: {}",
+ callout.locationCode, e.what());
+ addDebugData(msg);
+ }
+
try
{
auto inventoryPath = dataIface.getInventoryFromLocCode(
callout.locationCode, 0, false);
- addInventoryCallout(inventoryPath, priority, std::nullopt,
- dataIface);
+ addInventoryCallout(inventoryPath, priority, locCode, dataIface);
}
catch (const std::exception& e)
{
diff --git a/test/openpower-pels/data_interface_test.cpp b/test/openpower-pels/data_interface_test.cpp
new file mode 100644
index 0000000..28e455f
--- /dev/null
+++ b/test/openpower-pels/data_interface_test.cpp
@@ -0,0 +1,24 @@
+#include "extensions/openpower-pels/data_interface.hpp"
+
+#include <gtest/gtest.h>
+
+using namespace openpower::pels;
+
+TEST(DataInterfaceTest, ExtractConnectorLocCode)
+{
+ {
+ auto [base, connector] =
+ DataInterface::extractConnectorFromLocCode("Ufcs-P0-C2-T11");
+
+ EXPECT_EQ(base, "Ufcs-P0-C2");
+ EXPECT_EQ(connector, "-T11");
+ }
+
+ {
+ auto [base, connector] =
+ DataInterface::extractConnectorFromLocCode("Ufcs-P0-C2");
+
+ EXPECT_EQ(base, "Ufcs-P0-C2");
+ EXPECT_TRUE(connector.empty());
+ }
+}
diff --git a/test/openpower-pels/meson.build b/test/openpower-pels/meson.build
index ebfd608..f8121f8 100644
--- a/test/openpower-pels/meson.build
+++ b/test/openpower-pels/meson.build
@@ -2,6 +2,7 @@
'additional_data': {},
'ascii_string': {},
'bcd_time': {},
+ 'data_interface': {},
'device_callouts': {},
'event_logger': {},
'extended_user_data': {},
diff --git a/test/openpower-pels/pel_test.cpp b/test/openpower-pels/pel_test.cpp
index 984f2ec..318a218 100644
--- a/test/openpower-pels/pel_test.cpp
+++ b/test/openpower-pels/pel_test.cpp
@@ -834,9 +834,8 @@
.Times(2)
.WillRepeatedly(Return(names));
- EXPECT_CALL(dataIface,
- getLocationCode(
- "/xyz/openbmc_project/inventory/chassis/motherboard/cpu0"))
+ EXPECT_CALL(dataIface, expandLocationCode("P1", 0))
+ .Times(1)
.WillOnce(Return("UXXX-P1"));
EXPECT_CALL(dataIface, getInventoryFromLocCode("P1", 0, false))
diff --git a/test/openpower-pels/src_test.cpp b/test/openpower-pels/src_test.cpp
index dbb831f..3149378 100644
--- a/test/openpower-pels/src_test.cpp
+++ b/test/openpower-pels/src_test.cpp
@@ -865,19 +865,15 @@
.WillRepeatedly(
Return("/xyz/openbmc_project/inventory/chassis/motherboard/bmc"));
- EXPECT_CALL(dataIface,
- getLocationCode(
- "/xyz/openbmc_project/inventory/chassis/motherboard/cpu0"))
+ EXPECT_CALL(dataIface, expandLocationCode("P1-C40", 0))
.Times(3)
.WillRepeatedly(Return("Ufcs-P1-C40"));
- EXPECT_CALL(
- dataIface,
- getLocationCode("/xyz/openbmc_project/inventory/chassis/motherboard"))
+
+ EXPECT_CALL(dataIface, expandLocationCode("P1", 0))
.Times(3)
.WillRepeatedly(Return("Ufcs-P1"));
- EXPECT_CALL(dataIface,
- getLocationCode(
- "/xyz/openbmc_project/inventory/chassis/motherboard/bmc"))
+
+ EXPECT_CALL(dataIface, expandLocationCode("P1-C15", 0))
.Times(3)
.WillRepeatedly(Return("Ufcs-P1-C15"));