PEL: Support 'PossibleSubsystems' in msg registry
Add a 'PossibleSubsystems' field to the message registry schema where
someone can list which subsystems they might pass into the PEL_SUBSYSTEM
AdditionalData property field when creating a PEL.
The PossibleSubsystems value will be used by a off-BMC script that
generates field documentation for SRCs, so it knows what all SRC ASCII
string values are possible for that entry since the subsystem is part of
them. For example, if that field was ['processor', 'memory'], then that
means the possible SRCs are BD10AAAA and BD20AAAA for reason code
'AAAA'.
The Subsystem and PossibleSubsystems fields are mutually exclusive in
the message registry, so code changes had to be made to support the
subsystem field now being optional.
There is now a chance that even though the subsystem is supposed to be
passed in using PEL_SUBSYSTEM, it isn't. In that case, the 'Other'
subsystem will be used, since it seems fairly generic.
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
Change-Id: Ic8087fc46b2a192459b7287186f8972610e84730
diff --git a/extensions/openpower-pels/ascii_string.cpp b/extensions/openpower-pels/ascii_string.cpp
index 7d4d4f3..7e0f493 100644
--- a/extensions/openpower-pels/ascii_string.cpp
+++ b/extensions/openpower-pels/ascii_string.cpp
@@ -51,7 +51,9 @@
}
else // BMC Error
{
- setByte(2, entry.subsystem);
+ // If subsystem wasn't specified, it should get set later in
+ // the SRC constructor. Default to 'other' in case it doesn't.
+ setByte(2, entry.subsystem ? entry.subsystem.value() : 0x70);
}
// Then the reason code
diff --git a/extensions/openpower-pels/registry.cpp b/extensions/openpower-pels/registry.cpp
index 2c66e0f..b853bca 100644
--- a/extensions/openpower-pels/registry.cpp
+++ b/extensions/openpower-pels/registry.cpp
@@ -610,7 +610,11 @@
{
Entry entry;
entry.name = (*e)["Name"];
- entry.subsystem = helper::getSubsystem((*e)["Subsystem"]);
+
+ if (e->contains("Subsystem"))
+ {
+ entry.subsystem = helper::getSubsystem((*e)["Subsystem"]);
+ }
if (e->contains("ActionFlags"))
{
diff --git a/extensions/openpower-pels/registry.hpp b/extensions/openpower-pels/registry.hpp
index 180090c..bb26c70 100644
--- a/extensions/openpower-pels/registry.hpp
+++ b/extensions/openpower-pels/registry.hpp
@@ -132,7 +132,7 @@
/**
* @brief The PEL subsystem field.
*/
- uint8_t subsystem;
+ std::optional<uint8_t> subsystem;
/**
* @brief The optional PEL severity field. If not specified, the PEL
diff --git a/extensions/openpower-pels/registry/README.md b/extensions/openpower-pels/registry/README.md
index 89d4578..ac0c266 100644
--- a/extensions/openpower-pels/registry/README.md
+++ b/extensions/openpower-pels/registry/README.md
@@ -46,12 +46,24 @@
### Subsystem
This field is part of the PEL User Header section, and is used to specify
the subsystem pertaining to the error. It is an enumeration that maps to the
-actual PEL value.
+actual PEL value. If the subsystem isn't known ahead of time, it can be passed
+in at the time of PEL creation using the 'PEL\_SUBSYSTEM' AdditionalData field.
+In this case, 'Subsystem' isn't required, though 'PossibleSubsystems' is.
```
"Subsystem": "power_supply"
```
+### PossibleSubsystems
+This field is used by scripts that build documentation from the message
+registry to know which subsystems are possible for an error when it can't be
+hardcoded using the 'Subsystem' field. It is mutually exclusive with the
+'Subsystem' field.
+
+```
+"PossibleSubsystems": ["memory", "processor"]
+```
+
### Severity
This field is part of the PEL User Header section, and is used to specify
the PEL severity. It is an optional field, if it isn't specified, then the
diff --git a/extensions/openpower-pels/registry/schema/schema.json b/extensions/openpower-pels/registry/schema/schema.json
index 3bec95d..6fa6cf4 100644
--- a/extensions/openpower-pels/registry/schema/schema.json
+++ b/extensions/openpower-pels/registry/schema/schema.json
@@ -35,6 +35,8 @@
"Subsystem": {"$ref": "#/definitions/subsystem" },
+ "PossibleSubsystems": {"$ref": "#/definitions/possibleSubsystems" },
+
"Severity": {"$ref": "#/definitions/severity" },
"MfgSeverity": {"$ref": "#/definitions/mfgSeverity" },
@@ -56,13 +58,23 @@
"Callouts": {"$ref": "#/definitions/callouts"}
},
- "required": ["Name", "SRC", "Subsystem", "Documentation"],
+ "required": ["Name", "SRC", "Documentation"],
"additionalProperties": false,
"not":
{
"required": ["CalloutsUsingAD", "Callouts"]
- }
+ },
+
+ "oneOf":
+ [
+ {
+ "required": ["Subsystem"]
+ },
+ {
+ "required": ["PossibleSubsystems"]
+ }
+ ]
}
},
@@ -195,6 +207,18 @@
"user_error", "corrosion"]
},
+ "possibleSubsystems":
+ {
+ "description": "Required when the PEL creator uses PEL_SUBSYSTEM in the AdditionalData property to pass in the subsystem. Used by scripts that generate documentation to build all possible SRC ASCII strings for this error.",
+ "type": "array",
+ "items":
+ {
+ "$ref": "#/definitions/subsystem"
+ },
+ "minItems": 1,
+ "uniqueItems": true
+ },
+
"systemAndSeverity":
{
"description": "A severity entry that has an optional system type qualifier. Used when the severity needs to be based on the system type.",
diff --git a/extensions/openpower-pels/user_header.cpp b/extensions/openpower-pels/user_header.cpp
index 7aed654..b2365a9 100644
--- a/extensions/openpower-pels/user_header.cpp
+++ b/extensions/openpower-pels/user_header.cpp
@@ -58,26 +58,44 @@
_header.subType = 0;
_header.componentID = static_cast<uint16_t>(ComponentID::phosphorLogging);
- _eventSubsystem = entry.subsystem;
+ std::optional<uint8_t> subsys;
// Check for additional data - PEL_SUBSYSTEM
auto ss = additionalData.getValue("PEL_SUBSYSTEM");
if (ss)
{
auto eventSubsystem = std::stoul(*ss, NULL, 16);
- std::string subsystem =
+ std::string subsystemString =
pv::getValue(eventSubsystem, pel_values::subsystemValues);
- if (subsystem == "invalid")
+ if (subsystemString == "invalid")
{
log<level::WARNING>(
- fmt::format("UH: Invalid SubSystem value:{:#X}", eventSubsystem)
+ fmt::format(
+ "UH: Invalid SubSystem value in PEL_SUBSYSTEM: {:#X}",
+ eventSubsystem)
.c_str());
}
else
{
- _eventSubsystem = eventSubsystem;
+ subsys = eventSubsystem;
}
}
+ else
+ {
+ subsys = entry.subsystem;
+ }
+
+ if (subsys)
+ {
+ _eventSubsystem = *subsys;
+ }
+ else
+ {
+ // Gotta use something, how about 'others'.
+ log<level::WARNING>(
+ "No PEL subystem value supplied for error, using 'others'");
+ _eventSubsystem = 0x70;
+ }
_eventScope = entry.eventScope.value_or(
static_cast<uint8_t>(EventScope::entirePlatform));
diff --git a/test/openpower-pels/ascii_string_test.cpp b/test/openpower-pels/ascii_string_test.cpp
index 96ee4ad..2d23847 100644
--- a/test/openpower-pels/ascii_string_test.cpp
+++ b/test/openpower-pels/ascii_string_test.cpp
@@ -84,3 +84,16 @@
EXPECT_THROW(src::AsciiString as{stream}, std::out_of_range);
}
+
+TEST(AsciiStringTest, MissingSubsystemTest)
+{
+ message::Entry entry;
+ entry.src.type = 0xBD;
+ entry.src.reasonCode = 0xABCD;
+
+ src::AsciiString as{entry};
+ auto data = as.get();
+
+ // Default subsystem is 0x70
+ EXPECT_EQ(data, "BD70ABCD ");
+}
diff --git a/test/openpower-pels/registry_test.cpp b/test/openpower-pels/registry_test.cpp
index c7d88c9..5cfd5e5 100644
--- a/test/openpower-pels/registry_test.cpp
+++ b/test/openpower-pels/registry_test.cpp
@@ -101,6 +101,21 @@
"dump that provides debug information."
]
}
+ },
+
+ {
+ "Name": "xyz.openbmc_project.Common.Error.Timeout",
+ "PossibleSubsystems": ["processor", "memory"],
+
+ "SRC":
+ {
+ "ReasonCode": "0x2030"
+ },
+ "Documentation":
+ {
+ "Description": "A PGOOD Fault",
+ "Message": "PS had a PGOOD Fault"
+ }
}
]
}
@@ -651,3 +666,14 @@
}
}
}
+
+TEST_F(RegistryTest, TestNoSubsystem)
+{
+ auto path = RegistryTest::writeData(registryData);
+ Registry registry{path};
+
+ auto entry = registry.lookup("xyz.openbmc_project.Common.Error.Timeout",
+ LookupType::name);
+ ASSERT_TRUE(entry);
+ EXPECT_FALSE(entry->subsystem);
+}
diff --git a/test/openpower-pels/user_header_test.cpp b/test/openpower-pels/user_header_test.cpp
index 59199fb..233f08b 100644
--- a/test/openpower-pels/user_header_test.cpp
+++ b/test/openpower-pels/user_header_test.cpp
@@ -313,19 +313,56 @@
TEST(UserHeaderTest, UseEventLogPELSubsystem)
{
using namespace openpower::pels::message;
- Entry regEntry;
- regEntry.name = "test";
- regEntry.subsystem = 5;
- regEntry.actionFlags = 0xC000;
- regEntry.eventType = 1;
- regEntry.eventScope = 2;
+ {
+ Entry regEntry;
- MockDataInterface dataIface;
- std::vector<std::string> adData{"PEL_SUBSYSTEM=0x25"};
- AdditionalData ad{adData};
+ regEntry.name = "test";
+ regEntry.subsystem = 5;
+ regEntry.actionFlags = 0xC000;
+ regEntry.eventType = 1;
+ regEntry.eventScope = 2;
- UserHeader uh(regEntry, phosphor::logging::Entry::Level::Critical, ad,
- dataIface);
- ASSERT_EQ(uh.subsystem(), 0x25);
+ MockDataInterface dataIface;
+ std::vector<std::string> adData{"PEL_SUBSYSTEM=0x25"};
+ AdditionalData ad{adData};
+
+ UserHeader uh(regEntry, phosphor::logging::Entry::Level::Critical, ad,
+ dataIface);
+ ASSERT_EQ(uh.subsystem(), 0x25);
+ }
+ {
+ // No subsystem in registry, and invalid PEL_SUBSYSTEM
+ Entry regEntry;
+
+ regEntry.name = "test";
+ regEntry.actionFlags = 0xC000;
+ regEntry.eventType = 1;
+ regEntry.eventScope = 2;
+
+ MockDataInterface dataIface;
+ std::vector<std::string> adData{"PEL_SUBSYSTEM=0x99"};
+ AdditionalData ad{adData};
+
+ UserHeader uh(regEntry, phosphor::logging::Entry::Level::Critical, ad,
+ dataIface);
+ ASSERT_EQ(uh.subsystem(), 0x70); // others
+ }
+ {
+ // No subsystem in registry or PEL_SUBSYSTEM
+ Entry regEntry;
+
+ regEntry.name = "test";
+ regEntry.actionFlags = 0xC000;
+ regEntry.eventType = 1;
+ regEntry.eventScope = 2;
+
+ MockDataInterface dataIface;
+ std::vector<std::string> adData;
+ AdditionalData ad{adData};
+
+ UserHeader uh(regEntry, phosphor::logging::Entry::Level::Critical, ad,
+ dataIface);
+ ASSERT_EQ(uh.subsystem(), 0x70); // others
+ }
}