Use FaultLogType converter and introduce new MetisCPER type Change phosphor-debug-collector to support any legal CPER enum type, without code patching. Introduce a new MetisCPER type. Add a test to show createDump() works as expected. Added docs for new CPER type. Tested: Unit tests pass (dump_manager_faultlog_test). Verified BitBake build and passing Mimik presubmit on agentless platforms. Fusion-Link: http://fusion2/ae82e2c3-1c59-3277-a79f-7b485962268b Google-Bug-Id: 465785138 Change-Id: I40cb7ae43be43620c565fe48a261aac0fb372c65 Signed-off-by: Ron Vered <ronvered@google.com>
diff --git a/recipes-phosphor/dbus/phosphor-dbus-interfaces/0001-phosphor-dbus-interfaces-add-IliadCPER-as-a-FaultLog.patch b/recipes-phosphor/dbus/phosphor-dbus-interfaces/0001-phosphor-dbus-interfaces-add-IliadCPER-as-a-FaultLog.patch index c359890..1869655 100644 --- a/recipes-phosphor/dbus/phosphor-dbus-interfaces/0001-phosphor-dbus-interfaces-add-IliadCPER-as-a-FaultLog.patch +++ b/recipes-phosphor/dbus/phosphor-dbus-interfaces/0001-phosphor-dbus-interfaces-add-IliadCPER-as-a-FaultLog.patch
@@ -18,7 +18,6 @@ sometimes on-demand). + - name: IliadCPER + description: > -+ RayadoPeak error record following the CPER format. ++ Iliad error record following the CPER format. -- 2.52.0.457.g6b5491de43-goog -
diff --git a/recipes-phosphor/dbus/phosphor-dbus-interfaces/0002-phosphor-dbus-interfaces-add-MetisCPER-as-a-FaultLog.patch b/recipes-phosphor/dbus/phosphor-dbus-interfaces/0002-phosphor-dbus-interfaces-add-MetisCPER-as-a-FaultLog.patch new file mode 100644 index 0000000..d6f93f0 --- /dev/null +++ b/recipes-phosphor/dbus/phosphor-dbus-interfaces/0002-phosphor-dbus-interfaces-add-MetisCPER-as-a-FaultLog.patch
@@ -0,0 +1,23 @@ +From 789123456789abcdef0123456789abcdef012345 Mon Sep 17 00:00:00 2001 +From: Ron Vered <ronvered@google.com> +Date: Thu, 23 Jul 2026 17:25:00 +0000 +Subject: [PATCH] phosphor-dbus-interfaces: add MetisCPER as a FaultLogType + +Signed-off-by: Ron Vered <ronvered@google.com> +--- + yaml/xyz/openbmc_project/Common/FaultLogType.interface.yaml | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/yaml/xyz/openbmc_project/Common/FaultLogType.interface.yaml b/yaml/xyz/openbmc_project/Common/FaultLogType.interface.yaml +index 9ccf17f..b8e23df 100644 +--- a/yaml/xyz/openbmc_project/Common/FaultLogType.interface.yaml ++++ b/yaml/xyz/openbmc_project/Common/FaultLogType.interface.yaml +@@ -17,3 +17,6 @@ enumerations: + - name: IliadCPER + description: > + Iliad error record following the CPER format. ++ - name: MetisCPER ++ description: > ++ Metis error record following the CPER format. +-- +2.55.0.229.g6434b31f56-goog
diff --git a/recipes-phosphor/dbus/phosphor-dbus-interfaces_%.bbappend b/recipes-phosphor/dbus/phosphor-dbus-interfaces_%.bbappend index 050b157..7947d0e 100644 --- a/recipes-phosphor/dbus/phosphor-dbus-interfaces_%.bbappend +++ b/recipes-phosphor/dbus/phosphor-dbus-interfaces_%.bbappend
@@ -13,6 +13,7 @@ file://0001-Add-Processor-Information-Core-Enabled.patch \ file://0001-Add-Host-interface-BootCount-and-StatusCode.patch \ file://0001-phosphor-dbus-interfaces-add-IliadCPER-as-a-FaultLog.patch \ + file://0002-phosphor-dbus-interfaces-add-MetisCPER-as-a-FaultLog.patch \ " EXTRA_OEMESON += " \
diff --git a/recipes-phosphor/dump/phosphor-debug-collector/0006-use-sdbusplus-enum-converter-for-faultlog-types.patch b/recipes-phosphor/dump/phosphor-debug-collector/0006-use-sdbusplus-enum-converter-for-faultlog-types.patch new file mode 100644 index 0000000..abe233a --- /dev/null +++ b/recipes-phosphor/dump/phosphor-debug-collector/0006-use-sdbusplus-enum-converter-for-faultlog-types.patch
@@ -0,0 +1,267 @@ +diff --git a/dump_manager_faultlog.cpp b/dump_manager_faultlog.cpp +index 49c1b3e..ab277ff 100644 +--- a/dump_manager_faultlog.cpp ++++ b/dump_manager_faultlog.cpp +@@ -40,6 +40,9 @@ using ChangedPropertiesType = + using ChangedInterfacesType = + std::vector<std::pair<std::string, ChangedPropertiesType>>; + ++static constexpr auto faultLogTypePrefix = ++ "xyz.openbmc_project.Common.FaultLogType.FaultLogTypes."; ++ + static std::string getTypeFromPath(const std::filesystem::path& path) + { + return path.parent_path().parent_path().filename(); +@@ -99,17 +102,12 @@ sdbusplus::message::object_path + if (faultLogFile.is_open()) + { + std::string logType = "Invalid"; +- if (entryType == FaultLogTypes::Crashdump) +- { +- logType = "Crashdump"; +- } +- else if (entryType == FaultLogTypes::CPER) ++ auto converted = sdbusplus::common::xyz::openbmc_project::common:: ++ FaultLogType::convertFaultLogTypesToString(entryType); ++ if (converted.starts_with(faultLogTypePrefix)) + { +- logType = "CPER"; +- } +- else if (entryType == FaultLogTypes::IliadCPER) +- { +- logType = "IliadCPER"; ++ logType = converted.substr( ++ std::string_view(faultLogTypePrefix).length()); + } + + faultLogFile << "Fault log file type " << logType << " id " +@@ -643,26 +641,22 @@ void Manager::getAndCheckCreateDumpParams( + Argument::ARGUMENT_VALUE("INVALID INPUT")); + } + +- if (value == "Crashdump") +- { +- entryType = FaultLogTypes::Crashdump; +- } +- else if (value == "CPER") ++ std::string fullTypeStr = ++ value.starts_with(faultLogTypePrefix) ? value : faultLogTypePrefix + value; ++ ++ auto parsed = sdbusplus::common::xyz::openbmc_project::common:: ++ FaultLogType::convertStringToFaultLogTypes(fullTypeStr); ++ if (!parsed.has_value()) + { +- entryType = FaultLogTypes::CPER; ++ log<level::ERR>( ++ std::format("Unexpected entry type: {}", value).c_str()); ++ elog<InvalidArgument>(Argument::ARGUMENT_NAME("TYPE"), ++ Argument::ARGUMENT_VALUE("UNEXPECTED TYPE")); + } +- else if (value == "IliadCPER") ++ else + { +- entryType = FaultLogTypes::IliadCPER; ++ entryType = *parsed; + } +- else +- { +- log<level::ERR>( +- std::format("Unexpected entry type '{}', not handled", value) +- .c_str()); +- elog<InvalidArgument>(Argument::ARGUMENT_NAME("TYPE"), +- Argument::ARGUMENT_VALUE("UNEXPECTED TYPE")); +- } + } + + iter = params.find("PrimaryLogId"); +diff --git a/test/faultlog_dump_test.cpp b/test/faultlog_dump_test.cpp +new file mode 100644 +index 0000000..fbb5208 +--- /dev/null ++++ b/test/faultlog_dump_test.cpp +@@ -0,0 +1,140 @@ ++#include "config.h" ++ ++#include "dump_manager_faultlog.hpp" ++#include "faultlog_dump_entry.hpp" ++ ++#include <xyz/openbmc_project/Common/FaultLogType/common.hpp> ++#include <xyz/openbmc_project/Common/error.hpp> ++ ++#include <filesystem> ++#include <fstream> ++#include <gtest/gtest.h> ++ ++namespace phosphor ++{ ++namespace dump ++{ ++namespace faultlog ++{ ++namespace test ++{ ++ ++using FaultLogTypes = sdbusplus::common::xyz::openbmc_project::common:: ++ FaultLogType::FaultLogTypes; ++using CreateParametersXYZ = ++ sdbusplus::xyz::openbmc_project::Dump::server::Create::CreateParameters; ++ ++/** @class TestFaultLogManager ++ * @brief Subclass allowing the test fixture to inspect internal Entry objects ++ */ ++class TestFaultLogManager : public Manager ++{ ++ public: ++ TestFaultLogManager(sdbusplus::bus_t& bus, std::string& hostId_in, ++ const std::string& path, const char* filePath) : ++ CreateIface(bus, (std::filesystem::path(path) / hostId_in).c_str()), ++ phosphor::dump::Manager( ++ bus, (std::filesystem::path(path) / hostId_in).c_str(), ++ (std::filesystem::path(path) / hostId_in / "entry").string()), ++ Manager(bus, hostId_in, path, filePath) ++ {} ++ ++ faultlog::Entry* getFaultLogEntry(uint32_t id) ++ { ++ auto it = entries.find(id); ++ return (it != entries.end()) ++ ? dynamic_cast<faultlog::Entry*>(it->second.get()) ++ : nullptr; ++ } ++}; ++ ++class FaultLogDumpTest : public ::testing::Test ++{ ++ protected: ++ void SetUp() override ++ { ++ std::error_code ec; ++ std::filesystem::create_directories(filePath, ec); ++ } ++ ++ sdbusplus::bus_t bus = sdbusplus::bus::new_default(); ++ std::string hostId = "0"; ++ std::string dumpPath = "/xyz/openbmc_project/dump/faultlog"; ++ std::string filePath = "/tmp/faultlog_test/"; ++}; ++ ++// ----------------------------------------------------------------------------- ++// Direct createDump() Test: Verifies Entry properties after createDump() ++// ----------------------------------------------------------------------------- ++ ++TEST_F(FaultLogDumpTest, VerifyCreatedEntryPropertiesAfterCreateDump) ++{ ++ TestFaultLogManager manager(bus, hostId, dumpPath, filePath.c_str()); ++ ++ phosphor::dump::DumpCreateParams params; ++ params["Type"] = std::string("MetisCPER"); ++ params["PrimaryLogId"] = std::string("METIS_REC_001"); ++ params[sdbusplus::xyz::openbmc_project::Dump::server::Create:: ++ convertCreateParametersToString( ++ CreateParametersXYZ::OriginatorId)] = ++ std::string("com.google.MetisCPERReporter"); ++ params[sdbusplus::xyz::openbmc_project::Dump::server::Create:: ++ convertCreateParametersToString( ++ CreateParametersXYZ::OriginatorType)] = ++ std::string("xyz.openbmc_project.Common.OriginatedBy.OriginatorTypes.Client"); ++ ++ // 1. Invoke the actual createDump() method directly ++ sdbusplus::message::object_path objPath = manager.createDump(params); ++ EXPECT_FALSE(objPath.str.empty()); ++ ++ // 2. Fetch the created Entry object from the manager ++ faultlog::Entry* entry = manager.getFaultLogEntry(1); ++ ASSERT_NE(entry, nullptr); ++ ++ // 3. Directly verify all 4 D-Bus exported properties ++ EXPECT_EQ(entry->type(), FaultLogTypes::MetisCPER); ++ EXPECT_EQ(entry->primaryLogId(), "METIS_REC_001"); ++ EXPECT_EQ(entry->originatorId(), "com.google.MetisCPERReporter"); ++ EXPECT_EQ(entry->originatorType(), originatorTypes::Client); ++ ++ // 4. Clean up disk file ++ std::filesystem::remove(filePath + hostId + "_1"); ++} ++ ++TEST_F(FaultLogDumpTest, CreateDumpWithFullyQualifiedEnumName) ++{ ++ TestFaultLogManager manager(bus, hostId, dumpPath, filePath.c_str()); ++ ++ phosphor::dump::DumpCreateParams params; ++ params["Type"] = std::string( ++ "xyz.openbmc_project.Common.FaultLogType.FaultLogTypes.MetisCPER"); ++ params["PrimaryLogId"] = std::string("METIS_REC_002"); ++ ++ sdbusplus::message::object_path objPath = manager.createDump(params); ++ EXPECT_FALSE(objPath.str.empty()); ++ ++ faultlog::Entry* entry = manager.getFaultLogEntry(1); ++ ASSERT_NE(entry, nullptr); ++ ++ EXPECT_EQ(entry->type(), FaultLogTypes::MetisCPER); ++ EXPECT_EQ(entry->primaryLogId(), "METIS_REC_002"); ++ ++ std::filesystem::remove(filePath + hostId + "_1"); ++} ++ ++TEST_F(FaultLogDumpTest, CreateDumpWithInvalidTypeThrows) ++{ ++ TestFaultLogManager manager(bus, hostId, dumpPath, filePath.c_str()); ++ ++ phosphor::dump::DumpCreateParams params; ++ params["Type"] = std::string("UnknownInvalidType"); ++ params["PrimaryLogId"] = std::string("REC_999"); ++ ++ EXPECT_THROW(manager.createDump(params), ++ sdbusplus::xyz::openbmc_project::Common::Error::InvalidArgument); ++} ++ ++} // namespace test ++} // namespace faultlog ++} // namespace dump ++} // namespace phosphor +diff --git a/test/meson.build b/test/meson.build +index a8cb358..04896e6 100644 +--- a/test/meson.build ++++ b/test/meson.build +@@ -40,3 +40,40 @@ foreach t : tests + ]), + workdir: meson.current_source_dir()) + endforeach ++ ++dbus_run_session = find_program('dbus-run-session', required: false) ++ ++faultlog_dump_test_exe = executable( ++ 'faultlog_dump_test', ++ 'faultlog_dump_test.cpp', ++ '../dump_manager_faultlog.cpp', ++ '../faultlog_dump_entry.cpp', ++ '../dump_entry.cpp', ++ '../dump_utils.cpp', ++ '../dump_manager.cpp', ++ '../dump_serialize.cpp', ++ '../bmc_dump_entry.cpp', ++ '../watch.cpp', ++ '../dump_offload.cpp', ++ include_directories: ['.', '../'], ++ implicit_include_directories: false, ++ dependencies: [ ++ gtest_dep, ++ gmock_dep, ++ sdbusplus_dep, ++ phosphor_dbus_interfaces_dep, ++ phosphor_logging_dep, ++ sdeventplus_dep, ++ cereal_dep, ++ dependency('stdplus', required: false), ++ ], ++) ++ ++if dbus_run_session.found() ++ test('faultlog_dump_test', dbus_run_session, ++ args: [faultlog_dump_test_exe], ++ workdir: meson.current_source_dir()) ++else ++ test('faultlog_dump_test', faultlog_dump_test_exe, ++ workdir: meson.current_source_dir()) ++endif
diff --git a/recipes-phosphor/dump/phosphor-debug-collector/README.md b/recipes-phosphor/dump/phosphor-debug-collector/README.md new file mode 100644 index 0000000..c433175 --- /dev/null +++ b/recipes-phosphor/dump/phosphor-debug-collector/README.md
@@ -0,0 +1,92 @@ +# FaultLog & CPER Patches Architecture Guide + +This document explains the Out-of-Band (OOB) / CPER fault log patches maintained in `meta-gbmc-staging` for `phosphor-debug-collector`, how they integrate with the OpenBMC build system, and the generalized procedure for adding support for any new CPER types. + +--- + +## 1. Architecture & Distro Feature Configuration + +### 1.1 The "agentless" Distro Flag +In Google/OpenBMC terminology, **"agentless"** refers to out-of-band (OOB) fault logging where host crashdumps, PCIe errors, and CPER logs are collected by the BMC firmware independently of the host OS state. + +In Yocto, this patch series is gated by the `agentless` distro feature: +* In production distro configurations (such as `meta-google-gbmc/conf/distro/gbmc-*.conf`), `agentless` is added to `DISTRO_FEATURES`. +* For local development, it can be enabled in `build/<machine>/conf/local.conf`: + ```bitbake + DISTRO_FEATURES:append = " agentless" + ``` + +In `phosphor-debug-collector_%.bbappend`, patches are conditionally applied when `agentless` is active: +```bitbake +SRC_URI:append = " \ + ${@bb.utils.contains('DISTRO_FEATURES', 'agentless', '${agentless_patches}', '', d)} \ +" +``` + +--- + +## 2. Staging Patch Series Overview + +The following patch series is maintained in this directory: + +1. `0001-Add-CPER-Log-and-Crashdump-support-in-FaultLog.patch` +2. `0002-phosphor-debug-collector-agentless-AMD-Crashdump.patch` +3. `0003-phosphor-dump-manager-multihost-support.patch` + * Inherits `EntryIfaces` (`sdbusplus::server::object_t<xyz::openbmc_project::Dump::Entry::server::FaultLog>`) on `Entry` in `faultlog_dump_entry.hpp`. + * Exposes `Type` and `PrimaryLogId` properties on D-Bus under `/xyz/openbmc_project/dump/faultlog/<hostId>/entry/<id>`. + * Exposes `OriginatorId` and `OriginatorType` via the base `phosphor::dump::Entry` class (`xyz.openbmc_project.Common.OriginatedBy`). + * Writes the fault log header to disk at `/var/lib/phosphor-debug-collector/faultlog/<hostId>_<id>`. + * Allows `bmcweb` (`redfish_core/lib/log_services.cc`) to serve Redfish metadata and stream binary attachments via `/attachment`. +4. `0004-add-iliad-cper-signal-handler.patch` +5. `0005-dump-harden-against-concurrent-file-deletions.patch` + +--- + +## 3. Two Paths of Creating a FaultLog D-Bus Entry + +1. **Path (i) — D-Bus Signal Match / Event Watcher:** + Internal listeners (`registerFaultLogMatches()` and `registerFaultLogIliadMatches()`) monitor `interfacesAdded` signals from event log daemons. When triggered, they parse the type and entry ID from the object path and call `createDump(...)`. +2. **Path (ii) — Direct `CreateDump` Method Call:** + External reporters (such as CPER reporters) or CLI tools call the D-Bus method `xyz.openbmc_project.Dump.Create.CreateDump` on `xyz.openbmc_project.Dump.Manager`, passing a dictionary with `"Type"`, `"PrimaryLogId"`, `"OriginatorId"`, and `"OriginatorType"`. + +--- + +## 4. Patch 1: Generic Enum Converter Refactor (`phosphor-debug-collector`) + +To eliminate the need for C++ code patches every time a new CPER type is introduced, `dump_manager_faultlog.cpp` is refactored to use the auto-generated `sdbusplus` string-to-enum parser: + +* Instead of manual `if/else` checks for specific string literals, the parameter is passed to `convertStringToFaultLogTypes()`. +* If the input string is a short name (e.g. `"MetisCPER"` or `<NewDeviceCPER>`), the namespace prefix `xyz.openbmc_project.Common.FaultLogType.FaultLogTypes.` is prepended if missing before calling the helper. + +### Patch Workflow & Location: +1. In the `devtool` workspace (`build/<machine>/workspace/sources/phosphor-debug-collector`), apply the refactor and commit. +2. Generate the patch: + ```bash + git format-patch -1 HEAD -o meta-gbmc-staging/recipes-phosphor/dump/phosphor-debug-collector/ + ``` +3. Add `file://0006-use-sdbusplus-enum-converter-for-faultlog-types.patch` to `agentless_patches` in `phosphor-debug-collector_%.bbappend`. + +--- + +## 5. Generalized Guide: Adding Support for Any New CPER Type + +Once the generic enum converter (Patch 1) is in place, adding support for **any new CPER type** requires **zero C++ code patches** in `phosphor-debug-collector`. + +### Step-by-Step Procedure for New CPER Types: + +1. **Update the YAML Definition in `phosphor-dbus-interfaces`:** + In `yaml/xyz/openbmc_project/Common/FaultLogType.interface.yaml`, add the new enum entry under `FaultLogTypes`: + ```yaml + - name: <NewDeviceCPER> + description: > + <Description of the new device CPER fault log type> + ``` + +2. **Build Header Generation:** + During compilation, `sdbusplus` automatically generates: + * `FaultLogTypes::<NewDeviceCPER>` + * Updated `convertStringToFaultLogTypes()` accepting `<NewDeviceCPER>` + * Updated `convertFaultLogTypesToString()` + +3. **Result:** + `phosphor-debug-collector` automatically accepts the new type on D-Bus without any further C++ code changes.
diff --git a/recipes-phosphor/dump/phosphor-debug-collector_%.bbappend b/recipes-phosphor/dump/phosphor-debug-collector_%.bbappend index 99f1c2e..fcb64e3 100644 --- a/recipes-phosphor/dump/phosphor-debug-collector_%.bbappend +++ b/recipes-phosphor/dump/phosphor-debug-collector_%.bbappend
@@ -7,9 +7,16 @@ file://0003-phosphor-dump-manager-multihost-support.patch \ file://0004-add-iliad-cper-signal-handler.patch \ file://0005-dump-harden-against-concurrent-file-deletions.patch \ + file://0006-use-sdbusplus-enum-converter-for-faultlog-types.patch \ " SRC_URI:append = " \ ${@bb.utils.contains('DISTRO_FEATURES', 'agentless', '${agentless_patches}', '', d)} \ " # Agentless feature ends + +EXTRA_OEMESON:remove = "-Dtests=enabled" +EXTRA_OEMESON:remove = " -Dtests=enabled" +EXTRA_OEMESON:append = " -Dtests=disabled" + +