Add a default message for IPMI SEL entries added by command
This change adds a default message to the Redfish hooks so that
all Add SEL Entry or Platform Event commands will go to Redfish
instead of to the SEL.
SEL entries that can be parsed will show their specific message,
all other entries will show the default message which includes
the raw IPMI data sent in the command. This will allow using only
Redfish for external events without losing any data from
unexpected entries.
Since this change elminates the need for logging to the IPMI SEL,
this change also simplifies the overrides of the commands and
converts the Add SEL Entry command to the new IPMI API in the
process.
Tested:
Ran the following IPMI commands and verified that they each added
a new Redfish default log entry:
ipmitool raw 0xa 0x44 0x00 0x00 0xc0 0 0 0 0 1 2 3 4 5 6 7 8 9
ipmitool raw 0x4 0x2 0x81 0x4 0x1 0x6 0x1 0xa4
Change-Id: Ic60b89b1541e5ec789406c1c1e005b723db8d106
Signed-off-by: Jason M. Bills <jason.m.bills@linux.intel.com>
diff --git a/src/ipmi_to_redfish_hooks.cpp b/src/ipmi_to_redfish_hooks.cpp
index ff85081..be4dd3a 100644
--- a/src/ipmi_to_redfish_hooks.cpp
+++ b/src/ipmi_to_redfish_hooks.cpp
@@ -40,6 +40,29 @@
hexStr = stream.str();
}
+static bool defaultMessageHook(const std::string& ipmiRaw)
+{
+ // Log the record as a default Redfish message instead of a SEL record
+
+ static const std::string openBMCMessageRegistryVersion("0.1");
+ std::string messageID =
+ "OpenBMC." + openBMCMessageRegistryVersion + ".SELEntryAdded";
+
+ std::vector<std::string> messageArgs;
+ messageArgs.push_back(ipmiRaw);
+
+ // Log the Redfish message to the journal with the appropriate metadata
+ std::string journalMsg = "SEL Entry Added: " + ipmiRaw;
+ std::string messageArgsString = boost::algorithm::join(messageArgs, ",");
+ phosphor::logging::log<phosphor::logging::level::INFO>(
+ journalMsg.c_str(),
+ phosphor::logging::entry("REDFISH_MESSAGE_ID=%s", messageID.c_str()),
+ phosphor::logging::entry("REDFISH_MESSAGE_ARGS=%s",
+ messageArgsString.c_str()));
+
+ return true;
+}
+
// Record a BIOS message as a Redfish message instead of a SEL record
static bool biosMessageHook(const SELData& selData, const std::string& ipmiRaw)
{
@@ -70,8 +93,7 @@
messageID += ".MemoryRASConfigurationEnabled";
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
// Get the message data from eventData2 and eventData3
@@ -117,8 +139,7 @@
break;
}
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
@@ -133,8 +154,7 @@
messageID += ".BIOSPOSTError";
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
// Get the message data from eventData2 and eventData3
@@ -153,8 +173,7 @@
break;
}
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
@@ -172,8 +191,7 @@
messageID += ".IntelUPILinkWidthReducedToQuarter";
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
// Get the message data from eventData2
@@ -187,8 +205,7 @@
break;
}
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
@@ -206,8 +223,7 @@
messageID += ".MemoryRASModeEnabled";
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
// Get the message data from eventData2 and eventData3
@@ -259,8 +275,7 @@
break;
}
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
@@ -275,21 +290,18 @@
messageID += ".BIOSBoot";
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
}
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
@@ -348,8 +360,7 @@
messageID += ".MirroringRedundancyDegraded";
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
// Get the message data from eventData2 and eventData3
@@ -378,8 +389,7 @@
break;
}
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
@@ -397,8 +407,7 @@
messageID += ".MemoryECCUncorrectable";
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
// Get the message data from eventData2 and eventData3
@@ -424,8 +433,7 @@
break;
}
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
@@ -443,8 +451,7 @@
messageID += ".LegacyPCISERR";
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
// Get the message data from eventData2 and eventData3
@@ -464,8 +471,7 @@
break;
}
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
@@ -527,8 +533,7 @@
messageID += ".PCIeFatalMCBlockedTLP";
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
// Get the message data from eventData2 and eventData3
@@ -548,8 +553,7 @@
break;
}
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
@@ -591,8 +595,7 @@
messageID += ".PCIeCorrectableUnspecifiedAERError";
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
// Get the message data from eventData2 and eventData3
@@ -612,8 +615,7 @@
break;
}
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
@@ -631,8 +633,7 @@
messageID += ".SparingRedundancyDegraded";
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
// Get the message data from eventData2 and eventData3
@@ -663,8 +664,7 @@
break;
}
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
@@ -689,15 +689,13 @@
".MemoryParityCommandAndAddress";
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
}
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
// Get the message data from eventData2 and eventData3
@@ -726,8 +724,7 @@
break;
}
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
@@ -749,8 +746,7 @@
".PCIeFatalUnspecifiedNonAERFatalError";
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
// Get the message data from eventData2 and eventData3
@@ -770,8 +766,7 @@
break;
}
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
@@ -786,8 +781,7 @@
messageID += ".BIOSRecoveryStart";
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
@@ -800,15 +794,13 @@
messageID += ".BIOSRecoveryComplete";
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
}
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
@@ -842,14 +834,12 @@
break;
}
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
break;
default:
- messageID += ".Unknown";
- messageArgs.push_back(ipmiRaw);
+ return defaultMessageHook(ipmiRaw);
break;
}
@@ -880,35 +870,52 @@
break;
}
- // No hooks handled the request, so let it go to the SEL
- return false;
+ // No hooks handled the request, so let it go to default
+ return defaultMessageHook(ipmiRaw);
}
} // namespace redfish_hooks
-bool checkRedfishHooks(AddSELRequest* selRequest)
+bool checkRedfishHooks(uint16_t recordID, uint8_t recordType,
+ uint32_t timestamp, uint16_t generatorID, uint8_t evmRev,
+ uint8_t sensorType, uint8_t sensorNum, uint8_t eventType,
+ uint8_t eventData1, uint8_t eventData2,
+ uint8_t eventData3)
{
- // First check that this is a system event record type since that
- // determines the definition of the rest of the data
- if (selRequest->recordType != ipmi::sel::systemEvent)
- {
- // OEM record type, so let it go to the SEL
- return false;
- }
-
// Save the raw IPMI string of the request
std::string ipmiRaw;
- const boost::beast::span<uint8_t> selBytes(
- reinterpret_cast<uint8_t*>(selRequest), sizeof(AddSELRequest));
- redfish_hooks::toHexStr(selBytes, ipmiRaw);
+ std::array selBytes = {static_cast<uint8_t>(recordID),
+ static_cast<uint8_t>(recordID >> 8),
+ recordType,
+ static_cast<uint8_t>(timestamp),
+ static_cast<uint8_t>(timestamp >> 8),
+ static_cast<uint8_t>(timestamp >> 16),
+ static_cast<uint8_t>(timestamp >> 24),
+ static_cast<uint8_t>(generatorID),
+ static_cast<uint8_t>(generatorID >> 8),
+ evmRev,
+ sensorType,
+ sensorNum,
+ eventType,
+ eventData1,
+ eventData2,
+ eventData3};
+ redfish_hooks::toHexStr(boost::beast::span<uint8_t>(selBytes), ipmiRaw);
+
+ // First check that this is a system event record type since that
+ // determines the definition of the rest of the data
+ if (recordType != ipmi::sel::systemEvent)
+ {
+ // OEM record type, so let it go to the SEL
+ return redfish_hooks::defaultMessageHook(ipmiRaw);
+ }
// Extract the SEL data for the hook
- redfish_hooks::SELData selData = {
- .generatorID = selRequest->record.system.generatorID,
- .sensorNum = selRequest->record.system.sensorNum,
- .eventType = selRequest->record.system.eventType,
- .offset = selRequest->record.system.eventData[0] & 0x0F,
- .eventData2 = selRequest->record.system.eventData[1],
- .eventData3 = selRequest->record.system.eventData[2]};
+ redfish_hooks::SELData selData = {.generatorID = generatorID,
+ .sensorNum = sensorNum,
+ .eventType = eventType,
+ .offset = eventData1 & 0x0F,
+ .eventData2 = eventData2,
+ .eventData3 = eventData3};
return redfish_hooks::startRedfishHook(selData, ipmiRaw);
}
diff --git a/src/sensorcommands.cpp b/src/sensorcommands.cpp
index ba64fe4..ab654f7 100644
--- a/src/sensorcommands.cpp
+++ b/src/sensorcommands.cpp
@@ -312,38 +312,11 @@
return ipmi::responseReqDataLenInvalid();
}
- // Send this request to the Redfish hooks to see if it should be a
- // Redfish message instead. If so, no need to add it to the SEL, so
- // just return success.
- if (intel_oem::ipmi::sel::checkRedfishHooks(
- generatorID, evmRev, sensorType, sensorNum, eventType, eventData1,
- eventData2.value_or(0xFF), eventData3.value_or(0xFF)))
- {
- return ipmi::responseSuccess();
- }
-
- bool assert = eventType & directionMask ? false : true;
- std::vector<uint8_t> eventData;
- eventData.push_back(eventData1);
- eventData.push_back(eventData2.value_or(0xFF));
- eventData.push_back(eventData3.value_or(0xFF));
-
- std::string sensorPath = getPathFromSensorNumber(sensorNum);
- std::string service =
- ipmi::getService(dbus, ipmiSELAddInterface, ipmiSELPath);
- sdbusplus::message::message writeSEL = dbus.new_method_call(
- service.c_str(), ipmiSELPath, ipmiSELAddInterface, "IpmiSelAdd");
- writeSEL.append(ipmiSELAddMessage, sensorPath, eventData, assert,
- static_cast<uint16_t>(generatorID));
- try
- {
- dbus.call(writeSEL);
- }
- catch (sdbusplus::exception_t &e)
- {
- phosphor::logging::log<phosphor::logging::level::ERR>(e.what());
- return ipmi::responseUnspecifiedError();
- }
+ // Send this request to the Redfish hooks to log it as a Redfish message
+ // instead. There is no need to add it to the SEL, so just return success.
+ intel_oem::ipmi::sel::checkRedfishHooks(
+ generatorID, evmRev, sensorType, sensorNum, eventType, eventData1,
+ eventData2.value_or(0xFF), eventData3.value_or(0xFF));
return ipmi::responseSuccess();
}
diff --git a/src/storagecommands.cpp b/src/storagecommands.cpp
index 7aa8950..1ae2002 100644
--- a/src/storagecommands.cpp
+++ b/src/storagecommands.cpp
@@ -895,111 +895,24 @@
return ipmi::responseUnspecifiedError();
}
-ipmi_ret_t ipmiStorageAddSELEntry(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
- ipmi_request_t request,
- ipmi_response_t response,
- ipmi_data_len_t data_len,
- ipmi_context_t context)
+ipmi::RspType<uint16_t> ipmiStorageAddSELEntry(
+ uint16_t recordID, uint8_t recordType, uint32_t timestamp,
+ uint16_t generatorID, uint8_t evmRev, uint8_t sensorType, uint8_t sensorNum,
+ uint8_t eventType, uint8_t eventData1, uint8_t eventData2,
+ uint8_t eventData3)
{
- static constexpr char const* ipmiSELObject =
- "xyz.openbmc_project.Logging.IPMI";
- static constexpr char const* ipmiSELPath =
- "/xyz/openbmc_project/Logging/IPMI";
- static constexpr char const* ipmiSELAddInterface =
- "xyz.openbmc_project.Logging.IPMI";
- static const std::string ipmiSELAddMessage =
- "IPMI SEL entry logged using IPMI Add SEL Entry command.";
- uint16_t recordID = 0;
- sdbusplus::bus::bus bus{ipmid_get_sd_bus_connection()};
-
- if (*data_len != sizeof(AddSELRequest))
- {
- *data_len = 0;
- return IPMI_CC_REQ_DATA_LEN_INVALID;
- }
- AddSELRequest* req = static_cast<AddSELRequest*>(request);
-
// Per the IPMI spec, need to cancel any reservation when a SEL entry is
// added
cancelSELReservation();
- // Send this request to the Redfish hooks to see if it should be a Redfish
- // message instead. If so, no need to add it to the SEL, so just return
- // success.
- if (intel_oem::ipmi::sel::checkRedfishHooks(req))
- {
- *static_cast<uint16_t*>(response) = 0xFFFF;
- *data_len = sizeof(recordID);
- return IPMI_CC_OK;
- }
+ // Send this request to the Redfish hooks to log it as a Redfish message
+ // instead. There is no need to add it to the SEL, so just return success.
+ intel_oem::ipmi::sel::checkRedfishHooks(
+ recordID, recordType, timestamp, generatorID, evmRev, sensorType,
+ sensorNum, eventType, eventData1, eventData2, eventData3);
- if (req->recordType == intel_oem::ipmi::sel::systemEvent)
- {
- std::string sensorPath =
- getPathFromSensorNumber(req->record.system.sensorNum);
- std::vector<uint8_t> eventData(
- req->record.system.eventData,
- req->record.system.eventData +
- intel_oem::ipmi::sel::systemEventSize);
- bool assert = !(req->record.system.eventType & deassertionEvent);
- uint16_t genId = req->record.system.generatorID;
- sdbusplus::message::message writeSEL = bus.new_method_call(
- ipmiSELObject, ipmiSELPath, ipmiSELAddInterface, "IpmiSelAdd");
- writeSEL.append(ipmiSELAddMessage, sensorPath, eventData, assert,
- genId);
- try
- {
- sdbusplus::message::message writeSELResp = bus.call(writeSEL);
- writeSELResp.read(recordID);
- }
- catch (sdbusplus::exception_t& e)
- {
- phosphor::logging::log<phosphor::logging::level::ERR>(e.what());
- *data_len = 0;
- return IPMI_CC_UNSPECIFIED_ERROR;
- }
- }
- else if (req->recordType >= intel_oem::ipmi::sel::oemTsEventFirst &&
- req->recordType <= intel_oem::ipmi::sel::oemEventLast)
- {
- std::vector<uint8_t> eventData;
- if (req->recordType <= intel_oem::ipmi::sel::oemTsEventLast)
- {
- eventData =
- std::vector<uint8_t>(req->record.oemTs.eventData,
- req->record.oemTs.eventData +
- intel_oem::ipmi::sel::oemTsEventSize);
- }
- else
- {
- eventData = std::vector<uint8_t>(
- req->record.oem.eventData,
- req->record.oem.eventData + intel_oem::ipmi::sel::oemEventSize);
- }
- sdbusplus::message::message writeSEL = bus.new_method_call(
- ipmiSELObject, ipmiSELPath, ipmiSELAddInterface, "IpmiSelAddOem");
- writeSEL.append(ipmiSELAddMessage, eventData, req->recordType);
- try
- {
- sdbusplus::message::message writeSELResp = bus.call(writeSEL);
- writeSELResp.read(recordID);
- }
- catch (sdbusplus::exception_t& e)
- {
- phosphor::logging::log<phosphor::logging::level::ERR>(e.what());
- *data_len = 0;
- return IPMI_CC_UNSPECIFIED_ERROR;
- }
- }
- else
- {
- *data_len = 0;
- return IPMI_CC_PARM_OUT_OF_RANGE;
- }
-
- *static_cast<uint16_t*>(response) = recordID;
- *data_len = sizeof(recordID);
- return IPMI_CC_OK;
+ uint16_t responseID = 0xFFFF;
+ return ipmi::responseSuccess(responseID);
}
ipmi::RspType<uint8_t> ipmiStorageClearSEL(ipmi::Context::ptr ctx,
@@ -1103,10 +1016,9 @@
ipmi::Privilege::Operator, ipmiStorageGetSELEntry);
// <Add SEL Entry>
- ipmiPrintAndRegister(
- NETFUN_STORAGE,
- static_cast<ipmi_cmd_t>(IPMINetfnStorageCmds::ipmiCmdAddSEL), NULL,
- ipmiStorageAddSELEntry, PRIVILEGE_OPERATOR);
+ ipmi::registerHandler(ipmi::prioOpenBmcBase, ipmi::netFnStorage,
+ static_cast<ipmi::Cmd>(ipmi::storage::cmdAddSelEntry),
+ ipmi::Privilege::Operator, ipmiStorageAddSELEntry);
// <Clear SEL>
ipmi::registerHandler(ipmi::prioOpenBmcBase, ipmi::netFnStorage,