PEL: DRAM: Remove the Debug Section for PHAL Errors
In commit 8ae618, when the system attempts to call out FRUs, PEL is
designed to include DRAM manufacturer details if the FRU being
identified is a DIMM. The FRU type is determined using the PHAL API
"getFRUType", and if this fails, the corresponding PHAL error message
is added to the PEL debug section to aid in debugging.
However, there are cases where some FRUs, such as Planar, Fan, Power
Supply, etc., are not represented in the PHAL device tree. The PHAL
device tree primarily serves CEC (Central Electronic Complex)
hardware, which is crucial for booting the host processor. For these
unmodeled FRUs, PHAL returns 'Location code not found,' which is then
recorded in the PEL debug section.
Since these failures are expected, the PHAL error messages should be
removed from the PEL debug section to prevent confusion, as errors for
non-modeled FRUs in the PHAL device tree are expected.
The downside is that this removal could reduce debuggability for real
errors, such as a valid location code not being found or the need for
updates to the PHAL predefined FRU list. The expectation is that the
PHAL API "getFRUType" should have sufficient trace information for
debugging such issues. The phosphor-logging is not designed to capture
traces for PHAL error scenarios because of the limitations with the
PHAL device tree mentioned above.
Signed-off-by: Arya K Padman <aryakpadman@gmail.com>
Change-Id: I9a0011252667173f9e5f6aecbab2e7cd76174afa
diff --git a/extensions/openpower-pels/data_interface.cpp b/extensions/openpower-pels/data_interface.cpp
index 2922656..2ae9f1a 100644
--- a/extensions/openpower-pels/data_interface.cpp
+++ b/extensions/openpower-pels/data_interface.cpp
@@ -956,8 +956,7 @@
_locationCache.insert({locCode, isFRUDIMM});
}
-std::expected<bool, std::string>
- DataInterfaceBase::isDIMM(const std::string& locCode)
+bool DataInterfaceBase::isDIMM(const std::string& locCode)
{
auto isDIMMType = isDIMMLocCode(locCode);
if (isDIMMType.has_value())
@@ -965,45 +964,22 @@
return isDIMMType.value();
}
#ifndef PEL_ENABLE_PHAL
- return std::unexpected<std::string>(
- std::format("PHAL feature is not enabled, so the LocationCode:[{}] "
- "cannot be determined as DIMM",
- locCode));
+ return false;
#else
else
{
// Invoke pHAL API inorder to fetch the FRU Type
auto fruType = openpower::phal::pdbg::getFRUType(locCode);
+ bool isDIMMFRU{false};
if (fruType.has_value())
{
- bool isDIMMFRU{false};
if (fruType.value() == ENUM_ATTR_TYPE_DIMM)
{
isDIMMFRU = true;
}
addDIMMLocCode(locCode, isDIMMFRU);
- return isDIMMFRU;
}
- else
- {
- std::string msg{std::format("Failed to determine the HW Type, "
- "LocationCode:[{}]",
- locCode)};
- if (openpower::phal::exception::errMsgMap.contains(fruType.error()))
- {
- msg = std::format(
- "{} PHALErrorMsg:[{}]", msg,
- openpower::phal::exception::errMsgMap.at(fruType.error()));
- }
- else
- {
- msg = std::format(
- "{} PHALErrorMsg:[Unknown PHALErrorCode:{}]", msg,
- std::to_underlying<openpower::phal::exception::ERR_TYPE>(
- fruType.error()));
- }
- return std::unexpected<std::string>(msg);
- }
+ return isDIMMFRU;
}
#endif
}
diff --git a/extensions/openpower-pels/data_interface.hpp b/extensions/openpower-pels/data_interface.hpp
index 5ba9c7f..7ed5001 100644
--- a/extensions/openpower-pels/data_interface.hpp
+++ b/extensions/openpower-pels/data_interface.hpp
@@ -507,13 +507,11 @@
*
* @param[in] locCode - The location code of the FRU
*
- * @return std::expected<bool, std::string>
- * - Returns bool value if FRU type is able to determine successfully.
- * - true , if the given locCode is DIMM
- * - false, if the given locCode is not a DIMM
- * - Returns an error message in string format if an error occurs.
+ * @return - true, if the given location code is DIMM
+ * - false, if the given location code is not DIMM or if it fails to
+ * determine the FRU type.
*/
- std::expected<bool, std::string> isDIMM(const std::string& locCode);
+ bool isDIMM(const std::string& locCode);
/**
* @brief Check whether the given location code present in the cache
diff --git a/extensions/openpower-pels/pel.cpp b/extensions/openpower-pels/pel.cpp
index ad0c7c1..fbedb8b 100644
--- a/extensions/openpower-pels/pel.cpp
+++ b/extensions/openpower-pels/pel.cpp
@@ -747,14 +747,7 @@
}
else
{
- auto isDIMMLocCode =
- const_cast<DataInterfaceBase&>(dataIface).isDIMM(locCode);
- if (isDIMMLocCode.has_value())
- {
- return isDIMMLocCode.value();
- }
- debugData[AdDIMMInfoFetchError].emplace_back(isDIMMLocCode.error());
- return false;
+ return const_cast<DataInterfaceBase&>(dataIface).isDIMM(locCode);
}
};
auto addAdDIMMDetails = [&dataIface, &adSysInfoData,
diff --git a/test/openpower-pels/pel_test.cpp b/test/openpower-pels/pel_test.cpp
index 45328cc..fb88395 100644
--- a/test/openpower-pels/pel_test.cpp
+++ b/test/openpower-pels/pel_test.cpp
@@ -1397,7 +1397,7 @@
}
// When PEL has FRU callouts but PHAL is not enabled.
-TEST_F(PELTest, TestDimmsCalloutInfoWithNoPHAL)
+TEST_F(PELTest, TestNoDimmsCallout)
{
message::Entry entry;
uint64_t timestamp = 5;
@@ -1412,40 +1412,29 @@
"CalloutList": [
{
"Priority": "high",
- "LocCode": "P0-DIMM0"
- },
- {
- "Priority": "low",
- "LocCode": "P0-DIMM1"
+ "LocCode": "P0-PROC0"
}
]
}
]
)"_json;
- EXPECT_CALL(dataIface, expandLocationCode("P0-DIMM0", 0))
- .WillOnce(Return("U98D-P0-DIMM0"));
- EXPECT_CALL(dataIface, expandLocationCode("P0-DIMM1", 0))
- .WillOnce(Return("U98D-P0-DIMM1"));
+ EXPECT_CALL(dataIface, expandLocationCode("P0-PROC0", 0))
+ .WillOnce(Return("U98D-P0-PROC0"));
- EXPECT_CALL(dataIface, getInventoryFromLocCode("P0-DIMM0", 0, false))
+ EXPECT_CALL(dataIface, getInventoryFromLocCode("P0-PROC0", 0, false))
.WillOnce(Return(std::vector<std::string>{
- "/xyz/openbmc_project/inventory/system/chassis/motherboard/dimm0"}));
- EXPECT_CALL(dataIface, getInventoryFromLocCode("P0-DIMM1", 0, false))
- .WillOnce(Return(std::vector<std::string>{
- "/xyz/openbmc_project/inventory/system/chassis/motherboard/dimm1"}));
+ "/xyz/openbmc_project/inventory/system/chassis/motherboard/dcm0/cpu0"}));
+
+ // Add some location code in expanded format to DIMM cache memory
+ dataIface.addDIMMLocCode("U98D-P0-PROC0", false);
PEL pel{entry, 42, timestamp, phosphor::logging::Entry::Level::Error,
ad, ffdc, dataIface, journal};
nlohmann::json dimmInfoJson = getDIMMInfo(pel);
- nlohmann::json expected_data = R"(
- [
- "PHAL feature is not enabled, so the LocationCode:[U98D-P0-DIMM0] cannot be determined as DIMM",
- "PHAL feature is not enabled, so the LocationCode:[U98D-P0-DIMM1] cannot be determined as DIMM"
- ]
- )"_json;
+ nlohmann::json expected_data{};
EXPECT_EQ(expected_data, dimmInfoJson);
}