oem-ibm: Introducing sensor design for SBE ocmb dump (ibm-openbmc#622)

This commit introduces new oem sensor PDRs of state set id -
PLDM_OEM_IBM_SBE_DUMP_UPDATE_STATE(32777) whose state will be set for
ocmb dump progress status. This change implements the new sensor based
dump design only for ocmb sbe dumps whereas the sbe PROC dump design
remains unchanged. As part of this change PLDM will be updating the
sensor state with dump progress state. The remote terminus will be using
polling mechanism on getStateSensorReadings command for dump status
update.

Tested:
1. Used pldmtool to set numeric effecter for triggering dump.
2. Tested using sbe error injection( unrecoverable by HRESET ) during
IPL [1] and during runtime[1]. Also verified the sensor readings post
dump completion. Additionally verified SBE PROC dump [2].

[1]: https://gist.github.com/riyadixitagra/1b02a56477d98bfda158e2112aba0476
[2]: https://gist.github.com/riyadixitagra/3f61ec85722acf2f1983a9f2d8f5e4f0

Signed-off-by: Riya Dixit <[email protected]>

Author: riyadixitagra

libpldmresponder/platform.cpp

@@ -772,6 +772,11 @@ Response Handler::setNumericEffecterValue(const pldm_msg* request,
 
     int rc = decode_set_numeric_effecter_value_req(
         request, payloadLength, &effecterId, &effecterDataSize, effecterValue);
+    if (rc)
+    {
+        error("Failed to decode set numeric effecter value request, RC = {RC}",
+              "RC", rc);
+    }
 
     const pldm::utils::DBusHandler dBusIntf;
     uint16_t entityType{};

oem/ibm/libpldmresponder/oem_ibm_handler.cpp

@@ -66,6 +66,14 @@ int pldm::responder::oem_ibm_platform::Handler::
             sensorOpState = fetchRealSAIStatus();
             presentState = PLDM_SENSOR_NORMAL;
         }
+        else if ((entityType == PLDM_ENTITY_MEMORY_MODULE) &&
+                 (stateSetId == PLDM_OEM_IBM_SBE_DUMP_UPDATE_STATE))
+        {
+            sensorOpState = fetchDimmStateSensor(entityInstance);
+            stateField.push_back({PLDM_SENSOR_ENABLED, PLDM_SENSOR_UNKNOWN,
+                                  PLDM_SENSOR_UNKNOWN, sensorOpState});
+            break;
+        }
         else
         {
             rc = PLDM_PLATFORM_INVALID_STATE_VALUE;
@@ -162,9 +170,14 @@ int pldm::responder::oem_ibm_platform::Handler::
 
         else
         {
-            error("Invalid entity type received: {ENTITY_TYPE}", "ENTITY_TYPE",
-                  entityType);
+            error(
+                "Invalid entity type received: {ENTITY_TYPE} for entityInstance '{INSTANCE}'",
+                "ENTITY_TYPE", entityType, "INSTANCE", entityInstance);
+            return PLDM_ERROR_INVALID_DATA;
         }
+        info(
+            "Processing setNumericEffecter on ID {ID} for effecter type {TYPE} and entity instance {INST}",
+            "ID", effecterId, "TYPE", entityType, "INST", entityInstance);
         rc = setNumericEffecter(entityInstance, value, entityType);
     }
     return rc;
@@ -879,6 +892,57 @@ void buildAllNumericEffecterDimmPDR(oem_ibm_platform::Handler* platformHandler,
     }
 }
 
+void buildAllDimmSensorPDR(oem_ibm_platform::Handler* platformHandler,
+                           uint16_t entityType, uint16_t stateSetID,
+                           pdr_utils::Repo& repo)
+{
+    size_t pdrSize = 0;
+    pdrSize = sizeof(pldm_state_sensor_pdr) +
+              sizeof(state_sensor_possible_states);
+    std::vector<uint8_t> entry{};
+    entry.resize(pdrSize);
+    pldm_state_sensor_pdr* pdr =
+        reinterpret_cast<pldm_state_sensor_pdr*>(entry.data());
+    if (!pdr)
+    {
+        error("Failed to get record by PDR type, ERROR:{ERR}", "ERR", lg2::hex,
+              static_cast<unsigned>(PLDM_PLATFORM_INVALID_SENSOR_ID));
+        return;
+    }
+    auto dimm_info = platformHandler->generateDimmIds();
+    for (const auto& dimm : dimm_info)
+    {
+        pdr->hdr.record_handle = 0;
+        pdr->hdr.version = 1;
+        pdr->hdr.type = PLDM_STATE_SENSOR_PDR;
+        pdr->hdr.record_change_num = 0;
+        pdr->hdr.length = sizeof(pldm_state_sensor_pdr) - sizeof(pldm_pdr_hdr);
+        pdr->terminus_handle = TERMINUS_HANDLE;
+        pdr->sensor_id = platformHandler->getNextSensorId();
+        pdr->entity_type = entityType;
+        pdr->entity_instance = dimm;
+        dumpStatusMap[dimm] = DimmDumpState::UNAVAILABLE;
+        pdr->container_id = 1;
+        pdr->sensor_init = PLDM_NO_INIT;
+        pdr->sensor_auxiliary_names_pdr = false;
+        pdr->composite_sensor_count = 1;
+
+        auto* possibleStatesPtr = pdr->possible_states;
+        auto possibleStates =
+            reinterpret_cast<state_sensor_possible_states*>(possibleStatesPtr);
+        possibleStates->state_set_id = stateSetID;
+        possibleStates->possible_states_size = 1;
+        auto state =
+            reinterpret_cast<state_sensor_possible_states*>(possibleStates);
+        if (stateSetID == PLDM_OEM_IBM_SBE_DUMP_UPDATE_STATE)
+            state->states[0].byte = 7;
+        pldm::responder::pdr_utils::PdrEntry pdrEntry{};
+        pdrEntry.data = entry.data();
+        pdrEntry.size = pdrSize;
+        repo.addRecord(pdrEntry);
+    }
+}
+
 void pldm::responder::oem_ibm_platform::Handler::buildOEMPDR(
     pdr_utils::Repo& repo)
 {
@@ -943,6 +1007,8 @@ void pldm::responder::oem_ibm_platform::Handler::buildOEMPDR(
     buildAllNumericEffecterDimmPDR(
         this, PLDM_ENTITY_MEMORY_MODULE, ENTITY_INSTANCE_0,
         PLDM_OEM_IBM_SBE_SEMANTIC_ID, repo, instanceDimmMap);
+    buildAllDimmSensorPDR(this, PLDM_ENTITY_MEMORY_MODULE,
+                          PLDM_OEM_IBM_SBE_DUMP_UPDATE_STATE, repo);
     auto sensorId = findStateSensorId(
         repo.getPdr(), 0, PLDM_OEM_IBM_ENTITY_FIRMWARE_UPDATE,
         ENTITY_INSTANCE_0, 1, PLDM_OEM_IBM_VERIFICATION_STATE);
@@ -1015,15 +1081,59 @@ int encodeEventMsg(uint8_t eventType, const std::vector<uint8_t>& eventDataVec,
     return rc;
 }
 
+void pldm::responder::oem_ibm_platform::Handler::setDimmStateSensor(
+    bool status, uint16_t entityInstance)
+{
+    auto pdrs = findStateSensorPDR(TERMINUS_ID, PLDM_ENTITY_MEMORY_MODULE,
+                                   PLDM_OEM_IBM_SBE_DUMP_UPDATE_STATE, pdrRepo);
+    if (pdrs.empty())
+    {
+        error(
+            "Failed to find state sensor PDR of entity type 'PLDM_ENTITY_MEMORY_MODULE' and state set id 'PLDM_OEM_IBM_SBE_DUMP_UPDATE_STATE' for entity instance = {ENT_INSTANCE}",
+            "ENT_INSTANCE", entityInstance);
+        return;
+    }
+    for (auto& pdr : pdrs)
+    {
+        auto stateSensorPDR =
+            reinterpret_cast<pldm_state_sensor_pdr*>(pdr.data());
+        if (entityInstance == stateSensorPDR->entity_instance)
+        {
+            auto sid = stateSensorPDR->sensor_id;
+            info(
+                "Sending state sensor event for sensor ID {SID} with status {STATUS}",
+                "SID", sid, "STATUS", status);
+            if (status)
+            {
+                sendStateSensorEvent(stateSensorPDR->sensor_id,
+                                     PLDM_STATE_SENSOR_STATE, 0,
+                                     uint8_t(DimmDumpState::SUCCESS),
+                                     uint8_t(dumpStatusMap[entityInstance]));
+                dumpStatusMap[entityInstance] = DimmDumpState::SUCCESS;
+            }
+            else
+            {
+                sendStateSensorEvent(stateSensorPDR->sensor_id,
+                                     PLDM_STATE_SENSOR_STATE, 0,
+                                     uint8_t(DimmDumpState::RETRY),
+                                     uint8_t(dumpStatusMap[entityInstance]));
+                dumpStatusMap[entityInstance] = DimmDumpState::RETRY;
+            }
+        }
+    }
+}
+
+int pldm::responder::oem_ibm_platform::Handler::fetchDimmStateSensor(
+    uint16_t entityInstance)
+{
+    return dumpStatusMap[entityInstance];
+}
+
 void pldm::responder::oem_ibm_platform::Handler::setHostEffecterState(
     bool status, uint16_t entityTypeReceived, uint16_t entityInstance)
 {
     pldm::pdr::EntityType entityType;
-    if (entityTypeReceived == PLDM_ENTITY_MEMORY_MODULE)
-    {
-        entityType = PLDM_ENTITY_MEMORY_MODULE;
-    }
-    else if (entityTypeReceived == PLDM_ENTITY_PROC)
+    if (entityTypeReceived == PLDM_ENTITY_PROC)
     {
         entityType = PLDM_ENTITY_PROC;
     }
@@ -1056,6 +1166,9 @@ void pldm::responder::oem_ibm_platform::Handler::setHostEffecterState(
 
             auto request = reinterpret_cast<pldm_msg*>(requestMsg.data());
             std::vector<set_effecter_state_field> stateField;
+            info(
+                "State effecter ID {EFFECTER_ID} will be set with state as - {STATUS}",
+                "EFFECTER_ID", effecterId, "STATUS", status);
             if (status == true)
             {
                 stateField.push_back(set_effecter_state_field{
@@ -1142,15 +1255,29 @@ void pldm::responder::oem_ibm_platform::Handler::monitorDump(
             if (propVal ==
                 "xyz.openbmc_project.Common.Progress.OperationStatus.Completed")
             {
-                setHostEffecterState(true, entityType, entityInstance);
+                if (entityType == PLDM_ENTITY_MEMORY_MODULE)
+                {
+                    setDimmStateSensor(true, entityInstance);
+                }
+                else
+                {
+                    setHostEffecterState(true, entityType, entityInstance);
+                }
             }
             else if (
                 propVal ==
                     "xyz.openbmc_project.Common.Progress.OperationStatus.Failed" ||
                 propVal ==
                     "xyz.openbmc_project.Common.Progress.OperationStatus.Aborted")
             {
-                setHostEffecterState(false, entityType, entityInstance);
+                if (entityType == PLDM_ENTITY_MEMORY_MODULE)
+                {
+                    setDimmStateSensor(false, entityInstance);
+                }
+                else
+                {
+                    setHostEffecterState(false, entityType, entityInstance);
+                }
             }
         }
         sbeDumpMatch = nullptr;
@@ -1206,7 +1333,9 @@ int pldm::responder::oem_ibm_platform::Handler::setNumericEffecter(
 
         sdbusplus::message::object_path reply;
         response.read(reply);
-
+        info(
+            "Setting numeric effecter of type {TYPE} with entity instance {INST}",
+            "TYPE", entityType, "INST", entityInstance);
         monitorDump(reply, entityType, entityInstance);
     }
     catch (const std::exception& e)
@@ -1216,7 +1345,14 @@ int pldm::responder::oem_ibm_platform::Handler::setNumericEffecter(
             "ERR", e);
         // case when the dump policy is disabled but we set the host effecter as
         // true and the host moves on
-        setHostEffecterState(true, entityType, entityInstance);
+        if (entityType == PLDM_ENTITY_MEMORY_MODULE)
+        {
+            setDimmStateSensor(true, entityInstance);
+        }
+        else
+        {
+            setHostEffecterState(true, entityType, entityInstance);
+        }
     }
     return PLDM_SUCCESS;
 }

oem/ibm/libpldmresponder/oem_ibm_handler.hpp

@@ -21,8 +21,19 @@
 #include <sdeventplus/event.hpp>
 #include <sdeventplus/utility/timer.hpp>
 
+enum ibm_oem_pldm_state_set_dimm_dump_state_values
+{
+    UNAVAILABLE = 0,
+    SUCCESS = 0x1,
+    RETRY = 0x2,
+};
+
 typedef ibm_oem_pldm_state_set_firmware_update_state_values CodeUpdateState;
 
+typedef ibm_oem_pldm_state_set_dimm_dump_state_values DimmDumpState;
+
+static std::map<uint16_t, int> dumpStatusMap;
+
 namespace pldm
 {
 namespace responder
@@ -489,6 +500,18 @@ class Handler : public oem_platform::Handler
     /** @brief update containerID in PDRs */
     void updateContainerID();
 
+    /** @brief read the state of a dimm sensor
+     *   @param entityInstance - the entity instance id of the dimm sensor
+     *   @return the state of the sensor
+     */
+    int fetchDimmStateSensor(uint16_t entityInstance);
+
+    /** @brief Methode to set the dimm sensor state
+     *   @param status - the status of dump creation
+     *   @param entityInstance - the entity instance id of the sensor
+     */
+    void setDimmStateSensor(bool status, uint16_t entityInstance);
+
     /** @brief setNumericEffecter
      *
      *  @param[in] entityInstance - the entity Instance

oem/ibm/test/libpldmresponder_oem_platform_test.cpp

@@ -233,7 +233,7 @@ TEST(generateStateEffecterOEMPDR, testGoodRequest)
                 getSubtree("/xyz/openbmc_project/inventory/system", 0,
                            std::vector<std::string>{
                                "xyz.openbmc_project.Inventory.Item.Dimm"}))
-        .WillOnce(Return(mockDimmResponse));
+        .WillRepeatedly(Return(mockDimmResponse));
 
     std::unique_ptr<MockOemPlatformHandler> mockoemPlatformHandler =
         std::make_unique<MockOemPlatformHandler>(
@@ -395,7 +395,7 @@ TEST(generateStateSensorOEMPDR, testGoodRequest)
                 getSubtree("/xyz/openbmc_project/inventory/system", 0,
                            std::vector<std::string>{
                                "xyz.openbmc_project.Inventory.Item.Dimm"}))
-        .WillOnce(Return(mockDimmResponse));
+        .WillRepeatedly(Return(mockDimmResponse));
 
     std::unique_ptr<MockOemPlatformHandler> mockoemPlatformHandler =
         std::make_unique<MockOemPlatformHandler>(

pldmtool/oem/ibm/oem_ibm_state_set.hpp

@@ -58,6 +58,7 @@ extern const std::map<uint16_t, std::string> OemIBMstateSet{
     {PLDM_OEM_IBM_FIRMWARE_UPDATE_STATE, "OEM IBM Firmware Update State"},
     {PLDM_OEM_IBM_BOOT_STATE, "OEM IBM Boot State"},
     {PLDM_OEM_IBM_VERIFICATION_STATE, "OEM IBM Verification State"},
+    {PLDM_OEM_IBM_SBE_DUMP_UPDATE_STATE, "OEM IBM SBE Dump Update State"},
     {PLDM_OEM_IBM_SYSTEM_POWER_STATE, "OEM IBM System Power State"}};
 
 /** @brief Map for PLDM OEM IBM firmware update possible state values