PEL: Add the DRAM manufacturer info of DIMM callout in UD section
Add the called out DIMMs DRAM manufacturer info to the user data
section of the PEL to assist the service engineers in identifying the
manufacturer of the faulty DRAMs packaged within the DIMM module
directly from the logs, aiding in quick resolution.
The changes also moves the pdbg target and libekb initialization to
the PEL startup which avoids the need of multiple initialization as
the existing design.
When a PEL calls out a DIMM FRU, the DRAM manufacturer ID and the
expanded location code of those DIMMs are added to the SysInfo user
data section of the generated PEL in JSON format under the key 'DIMMs
Additional Info'.
In case of any errors occur during the collection or processing of
the manufacturer data, the error messages will be logged in the 'PEL
Internal Debug Data' section as a JSON array under the key 'DIMMs Info
Fetch Error' as a separate user data section.
Tested :
Below is a portion of PEL(callout section and User Data section are
shown) which callout the DIMM P0-C32.
```
"Hex Word 9": "00000000",
"Callout Section": {
"Callout Count": "1",
"Callouts": [{
"FRU Type": "Normal Hardware FRU",
"Priority": "Mandatory, replace all with this
type as a unit",
"Location Code": "UXXX.YYY.WWW004A-P0-C32",
"Part Number": "7777777",
"CCIN": "1234",
"Serial Number": "YYYYYY"
}]
}
```
"User Data": {
"Section Version": "1",
"Sub-section type": "1",
"Created by": "bmc error logging",
"BMCLoad": "0.65 0.69 0.64",
"BMCState": "Ready",
"BMCUptime": "0y 0d 0h 17m 43s",
"BootState": "Unspecified",
"ChassisState": "Off",
"DIMMs Additional Info": [
{
"DRAM Manufacturer ID": [
"0x88",
"0xAA"
]
"Location Code": "UXXX.YYY.WWW004A-P0-C32",
}
],
"FW Version ID": "fw1060.20-4-1060.2432.20240729a (NL1060_068)",
"HostState": "Off",
"System IM": "50001001"
}
```
Change-Id: I2ff81c66e63b99e8e84378ec78f586fb9b6322d7
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 9e2a7e5..3cef1ab 100644
--- a/extensions/openpower-pels/data_interface.cpp
+++ b/extensions/openpower-pels/data_interface.cpp
@@ -22,6 +22,10 @@
#include <xyz/openbmc_project/State/BMC/server.hpp>
#include <xyz/openbmc_project/State/Boot/Progress/server.hpp>
+#ifdef PEL_ENABLE_PHAL
+#include <libphal.H>
+#endif
+
// Use a timeout of 10s for D-Bus calls so if there are
// timeouts the callers of the PEL creation method won't
// also timeout.
@@ -880,6 +884,107 @@
return std::get<1>(rawProgress);
}
+std::optional<std::vector<uint8_t>>
+ DataInterface::getDIProperty(const std::string& locationCode) const
+{
+ std::vector<uint8_t> viniDI;
+
+ try
+ {
+ // Note : The hardcoded value 0 should be changed when comes to
+ // multinode system.
+ auto objectPath = getInventoryFromLocCode(locationCode, 0, true);
+
+ DBusValue value;
+ getProperty(service_name::inventoryManager, objectPath[0],
+ interface::viniRecordVPD, "DI", value);
+
+ viniDI = std::get<std::vector<uint8_t>>(value);
+ }
+ catch (const std::exception& e)
+ {
+ lg2::warning(
+ "Failed reading DI property for the location code : {LOC_CODE} from "
+ "interface: {IFACE} exception: {ERROR}",
+ "LOC_CODE", locationCode, "IFACE", interface::viniRecordVPD,
+ "ERROR", e);
+ return std::nullopt;
+ }
+
+ return viniDI;
+}
+
+std::optional<bool>
+ DataInterfaceBase::isDIMMLocCode(const std::string& locCode) const
+{
+ if (_locationCache.contains(locCode))
+ {
+ return _locationCache.at(locCode);
+ }
+ else
+ {
+ return std::nullopt;
+ }
+}
+
+void DataInterfaceBase::addDIMMLocCode(const std::string& locCode,
+ bool isFRUDIMM)
+{
+ _locationCache.insert({locCode, isFRUDIMM});
+}
+
+std::expected<bool, std::string>
+ DataInterfaceBase::isDIMM(const std::string& locCode)
+{
+ auto isDIMMType = isDIMMLocCode(locCode);
+ if (isDIMMType.has_value())
+ {
+ 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));
+#else
+ else
+ {
+ // Invoke pHAL API inorder to fetch the FRU Type
+ auto fruType = openpower::phal::pdbg::getFRUType(locCode);
+ 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);
+ }
+ }
+#endif
+}
+
void DataInterface::startFruPlugWatch()
{
// Add a watch on inventory InterfacesAdded and then find all
diff --git a/extensions/openpower-pels/data_interface.hpp b/extensions/openpower-pels/data_interface.hpp
index a2f55f4..1626293 100644
--- a/extensions/openpower-pels/data_interface.hpp
+++ b/extensions/openpower-pels/data_interface.hpp
@@ -7,8 +7,10 @@
#include <sdbusplus/bus.hpp>
#include <sdbusplus/bus/match.hpp>
+#include <expected>
#include <filesystem>
#include <fstream>
+#include <unordered_map>
namespace openpower
{
@@ -487,6 +489,56 @@
*/
virtual std::vector<uint8_t> getRawProgressSRC() const = 0;
+ /**
+ * @brief Returns the FRUs DI property value hosted on the VINI iterface for
+ * the given location code.
+ *
+ * @param[in] locationCode - The location code of the FRU
+ *
+ * @return std::optional<std::vector<uint8_t>> - The FRUs DI or
+ * std::nullopt
+ */
+ virtual std::optional<std::vector<uint8_t>>
+ getDIProperty(const std::string& locationCode) const = 0;
+
+ /**
+ * @brief Wrpper API to call pHAL API 'getFRUType()' and check whether the
+ * given location code is DIMM or not
+ *
+ * @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.
+ */
+ std::expected<bool, std::string> isDIMM(const std::string& locCode);
+
+ /**
+ * @brief Check whether the given location code present in the cache
+ * memory
+ *
+ * @param[in] locCode - The location code of the FRU
+ *
+ * @return true, if the given location code present in cache and is a DIMM
+ * false, if the given location code present in cache, but a non
+ * DIMM FRU
+ * std::nullopt, if the given location code is not present in the
+ * cache.
+ */
+ std::optional<bool> isDIMMLocCode(const std::string& locCode) const;
+
+ /**
+ * @brief add the given location code to the cache memory
+ *
+ * @param[in] locCode - The location code of the FRU
+ * @param[in] isFRUDIMM - true indicates the FRU is a DIMM
+ * false indicates the FRU is a non DIMM
+ *
+ */
+ void addDIMMLocCode(const std::string& locCode, bool isFRUDIMM);
+
protected:
/**
* @brief Sets the host on/off state and runs any
@@ -601,6 +653,14 @@
* @brief The boot state property
*/
std::string _bootState;
+
+ /**
+ * @brief A cache storage for location code and its FRU Type
+ * - The key 'std::string' represents the locationCode of the FRU
+ * - The bool value - true indicates the FRU is a DIMM
+ * false indicates the FRU is a non DIMM.
+ */
+ std::unordered_map<std::string, bool> _locationCache;
};
/**
@@ -837,6 +897,18 @@
*/
std::vector<uint8_t> getRawProgressSRC() const override;
+ /**
+ * @brief Returns the FRUs DI property value hosted on the VINI iterface for
+ * the given location code.
+ *
+ * @param[in] locationCode - The location code of the FRU
+ *
+ * @return std::optional<std::vector<uint8_t>> - The FRUs DI or
+ * std::nullopt
+ */
+ std::optional<std::vector<uint8_t>>
+ getDIProperty(const std::string& locationCode) const override;
+
private:
/**
* @brief Reads the BMC firmware version string and puts it into
diff --git a/extensions/openpower-pels/entry_points.cpp b/extensions/openpower-pels/entry_points.cpp
index 29b9d33..0b58212 100644
--- a/extensions/openpower-pels/entry_points.cpp
+++ b/extensions/openpower-pels/entry_points.cpp
@@ -25,6 +25,11 @@
#include <format>
+#ifdef PEL_ENABLE_PHAL
+#include "libekb.H"
+#include "libpdbg.h"
+#endif
+
namespace openpower
{
namespace pels
@@ -72,6 +77,19 @@
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
}
diff --git a/extensions/openpower-pels/meson.build b/extensions/openpower-pels/meson.build
index 478a83c..fb6282b 100644
--- a/extensions/openpower-pels/meson.build
+++ b/extensions/openpower-pels/meson.build
@@ -68,6 +68,7 @@
'severity.cpp',
'user_header.cpp',
'temporary_file.cpp',
+ '../../util.cpp',
extra_sources,
)
diff --git a/extensions/openpower-pels/pel.cpp b/extensions/openpower-pels/pel.cpp
index ee8ebf6..ad0c7c1 100644
--- a/extensions/openpower-pels/pel.cpp
+++ b/extensions/openpower-pels/pel.cpp
@@ -43,6 +43,7 @@
#include <format>
#include <iostream>
+#include <ranges>
namespace openpower
{
@@ -51,6 +52,7 @@
namespace pv = openpower::pels::pel_values;
constexpr auto unknownValue = "Unknown";
+constexpr auto AdDIMMInfoFetchError = "DIMMs Info Fetch Error";
PEL::PEL(const message::Entry& regEntry, uint32_t obmcLogID, uint64_t timestamp,
phosphor::logging::Entry::Level severity,
@@ -87,7 +89,7 @@
}
#endif
- std::map<std::string, std::vector<std::string>> debugData;
+ DebugData debugData;
nlohmann::json callouts;
_ph = std::make_unique<PrivateHeader>(regEntry.componentID, obmcLogID,
@@ -112,6 +114,9 @@
auto src =
std::make_unique<SRC>(regEntry, additionalData, callouts, dataIface);
+ nlohmann::json adSysInfoData(nlohmann::json::value_t::object);
+ addAdDetailsForDIMMsCallout(src, dataIface, adSysInfoData, debugData);
+
if (!src->getDebugData().empty())
{
// Something didn't go as planned
@@ -126,7 +131,8 @@
auto mtms = std::make_unique<FailingMTMS>(dataIface);
_optionalSections.push_back(std::move(mtms));
- auto ud = util::makeSysInfoUserDataSection(additionalData, dataIface);
+ auto ud = util::makeSysInfoUserDataSection(additionalData, dataIface, true,
+ adSysInfoData);
addUserDataSection(std::move(ud));
// Check for pel severity of type - 0x51 = critical error, system
@@ -722,6 +728,60 @@
}
}
+void PEL::addAdDetailsForDIMMsCallout(
+ const std::unique_ptr<SRC>& src, const DataInterfaceBase& dataIface,
+ nlohmann::json& adSysInfoData, DebugData& debugData)
+{
+ if (!src->callouts())
+ {
+ // No callouts
+ return;
+ }
+
+ auto isDIMMCallout = [&dataIface, &debugData](const auto& callout) {
+ auto locCode{callout->locationCode()};
+ if (locCode.empty())
+ {
+ // Not a hardware callout. No action required
+ return false;
+ }
+ else
+ {
+ auto isDIMMLocCode =
+ const_cast<DataInterfaceBase&>(dataIface).isDIMM(locCode);
+ if (isDIMMLocCode.has_value())
+ {
+ return isDIMMLocCode.value();
+ }
+ debugData[AdDIMMInfoFetchError].emplace_back(isDIMMLocCode.error());
+ return false;
+ }
+ };
+ auto addAdDIMMDetails = [&dataIface, &adSysInfoData,
+ &debugData](const auto& callout) {
+ auto dimmLocCode{callout->locationCode()};
+
+ auto diPropVal = dataIface.getDIProperty(dimmLocCode);
+ if (!diPropVal.has_value())
+ {
+ std::string errMsg{
+ std::format("Failed reading DI property from "
+ "VINI Interface for the LocationCode:[{}]",
+ dimmLocCode)};
+ debugData[AdDIMMInfoFetchError].emplace_back(errMsg);
+ }
+ else
+ {
+ util::addDIMMInfo(dimmLocCode, diPropVal.value(), adSysInfoData);
+ }
+ };
+
+ auto DIMMsCallouts = src->callouts()->callouts() |
+ std::views::filter(isDIMMCallout);
+
+ std::ranges::for_each(DIMMsCallouts, addAdDIMMDetails);
+}
+
namespace util
{
@@ -848,7 +908,7 @@
std::unique_ptr<UserData> makeSysInfoUserDataSection(
const AdditionalData& ad, const DataInterfaceBase& dataIface,
- bool addUptime)
+ bool addUptime, const nlohmann::json& adSysInfoData)
{
nlohmann::json json;
@@ -862,6 +922,11 @@
addBMCUptime(json, dataIface);
}
+ if (!adSysInfoData.empty())
+ {
+ json.update(adSysInfoData);
+ }
+
return makeJSONUserDataSection(json);
}
@@ -994,6 +1059,20 @@
return out;
}
+void addDIMMInfo(const std::string& locationCode,
+ const std::vector<std::uint8_t>& diPropVal,
+ nlohmann::json& adSysInfoData)
+{
+ nlohmann::json dimmInfoObj;
+ dimmInfoObj["Location Code"] = locationCode;
+ std::ranges::transform(
+ diPropVal, std::back_inserter(dimmInfoObj["DRAM Manufacturer ID"]),
+ [](const auto& diPropEachByte) {
+ return std::format("{:#04x}", diPropEachByte);
+ });
+ adSysInfoData["DIMMs Additional Info"] += dimmInfoObj;
+}
+
} // namespace util
} // namespace pels
diff --git a/extensions/openpower-pels/pel.hpp b/extensions/openpower-pels/pel.hpp
index 8abefa4..5586187 100644
--- a/extensions/openpower-pels/pel.hpp
+++ b/extensions/openpower-pels/pel.hpp
@@ -30,6 +30,7 @@
};
using PelFFDC = std::vector<PelFFDCfile>;
+using DebugData = std::map<std::string, std::vector<std::string>>;
constexpr uint8_t jsonCalloutSubtype = 0xCA;
@@ -416,6 +417,20 @@
const JournalBase& journal);
/**
+ * @brief API to collect the addditional details of the DIMM callouts in
+ * the PEL as a JSON object in adSysInfoData.
+ *
+ * @param[in] src - Unique pointer to the SRC object
+ * @param[in] dataIface - The data interface object
+ * @param[out] adSysInfoData - The additional data to SysInfo in json format
+ * @param[out] debugData - The map of string and vector of string to store
+ * the debug data if any.
+ */
+ void addAdDetailsForDIMMsCallout(
+ const std::unique_ptr<SRC>& src, const DataInterfaceBase& dataIface,
+ nlohmann::json& adSysInfoData, DebugData& debugData);
+
+ /**
* @brief The PEL Private Header section
*/
std::unique_ptr<PrivateHeader> _ph;
@@ -466,12 +481,16 @@
* @param[in] dataIface - The data interface object
* @param[in] addUptime - Whether to add the uptime attribute the default is
* true
+ * @param[in] adSysInfoData - The additional data to SysInfo in json format.
+ * Default value will be empty.
*
* @return std::unique_ptr<UserData> - The section
*/
std::unique_ptr<UserData> makeSysInfoUserDataSection(
const AdditionalData& ad, const DataInterfaceBase& dataIface,
- bool addUptime = true);
+ bool addUptime = true,
+ const nlohmann::json& adSysInfoData =
+ nlohmann::json(nlohmann::json::value_t::object));
/**
* @brief Reads data from an opened file descriptor.
@@ -493,6 +512,19 @@
makeFFDCuserDataSection(uint16_t componentID, const PelFFDCfile& file);
/**
+ * @brief To create JSON object with the given location code and the hex
+ * converted value of the given DI property.
+ *
+ * @param[in] locationCode - The location code of the DIMM
+ * @param[in] diPropVal - The DI property value of the DIMM
+ * @param[out] adSysInfoData - Holds the created JSON object
+ */
+
+void addDIMMInfo(const std::string& locationCode,
+ const std::vector<std::uint8_t>& diPropVal,
+ nlohmann::json& adSysInfoData);
+
+/**
* @brief Flattens a vector of strings into a vector of bytes suitable
* for storing in a PEL section.
*
diff --git a/extensions/openpower-pels/phal_service_actions.cpp b/extensions/openpower-pels/phal_service_actions.cpp
index aaa3c9d..f40821e 100644
--- a/extensions/openpower-pels/phal_service_actions.cpp
+++ b/extensions/openpower-pels/phal_service_actions.cpp
@@ -192,12 +192,6 @@
ATTR_PHYS_BIN_PATH_Type physBinPath;
std::copy(entityPath.begin(), entityPath.end(), physBinPath);
// libphal api to deconfigure the target
- if (!pdbg_targets_init(NULL))
- {
- lg2::error(
- "pdbg_targets_init failed, skipping deconfig record update");
- return;
- }
openpower::phal::pdbg::deconfigureTgt(physBinPath, plid);
}
catch (const std::exception& e)
diff --git a/extensions/openpower-pels/sbe_ffdc_handler.cpp b/extensions/openpower-pels/sbe_ffdc_handler.cpp
index 36198a1..de0d510 100644
--- a/extensions/openpower-pels/sbe_ffdc_handler.cpp
+++ b/extensions/openpower-pels/sbe_ffdc_handler.cpp
@@ -247,18 +247,6 @@
// formated FFDC data structure after FFDC packet processing
FFDC ffdc;
- if (!pdbg_targets_init(NULL))
- {
- lg2::error("pdbg_targets_init failed, skipping ffdc processing");
- return;
- }
-
- if (libekb_init())
- {
- lg2::error("libekb_init failed, skipping ffdc processing");
- return;
- }
-
try
{
// libekb provided wrapper function to convert SBE FFDC