commit 72d01aabedc138be48f560c69e948b182ecaa326
author Chris Cain <cjcain@us.ibm.com> Tue Jun 14 16:28:03 2022 -0500
committer Chris Cain <cjcain@us.ibm.com> Wed Jun 22 09:11:58 2022 -0500
tree d02bdb500e930ca0e60a6f6e622ddc575eba115d
parent 3576d65ea65864afc78c3eb79f31f660f598e1c3

Fix correlation between OCC StateSensorPDRs and procs

occ-control was not correlating the OCC Active sensors with the correct
processor. Code change will now use the Sensor ID to know which
OCC/proc is active. Hostboot will also be making a change to ensure that
the Sensor IDs are always numbered according to processor order (p0, p1,
etc)

Wait for PHYP to start before reading PLDM sensors:
occ-control caches the PLDM sensor IDs to limit the dbus queries.
The cache was supposed to be cleared when the OS was powered off, but
the existing code only cleared it when CurrentHostState was Off.
Got a defect where occ-control was using invalid/old sensor IDs when
getting notifications of OCC Active sensors. This causes the app to try
communicating with the wrong or invalid OCC.

Code change will clear the sensor cache anytime PHYP is not running, and
will populate the cache once PHYP is running.

Tested on hardware with various boot types and resets.

Change-Id: I4b32aa848768296065d6570466475f5b17771d2e
Signed-off-by: Chris Cain <cjcain@us.ibm.com>

pldm.cpp

diff --git a/pldm.cpp b/pldm.cpp
index f4a1c73..1ce23f4 100644
--- a/pldm.cpp
+++ b/pldm.cpp
@@ -117,8 +117,7 @@
     {
         auto pdrPtr =
             reinterpret_cast<const pldm_state_sensor_pdr*>(pdr.data());
-        uint32_t key = (static_cast<uint32_t>(pdrPtr->container_id) << 16) |
-                       static_cast<uint32_t>(pdrPtr->entity_instance);
+        uint32_t key = pdrPtr->sensor_id;
         entityInstMap.emplace(key, static_cast<SensorID>(pdrPtr->sensor_id));
     }
 
@@ -132,6 +131,12 @@
 
 void Interface::sensorEvent(sdbusplus::message::message& msg)
 {
+    if (!open_power::occ::utils::isHostRunning())
+    {
+        clearData();
+        return;
+    }
+
     if (!isOCCSensorCacheValid())
     {
         fetchSensorInfo(PLDM_STATE_SET_OPERATIONAL_RUNNING_STATUS,
@@ -144,13 +149,14 @@
                         SBESensorOffset);
     }
 
-    TerminusID tid{};
+    TerminusID sensorTid{};
     SensorID sensorId{};
     SensorOffset msgSensorOffset{};
     EventState eventState{};
     EventState previousEventState{};
 
-    msg.read(tid, sensorId, msgSensorOffset, eventState, previousEventState);
+    msg.read(sensorTid, sensorId, msgSensorOffset, eventState,
+             previousEventState);
 
     if (msgSensorOffset == OCCSensorOffset)
     {
@@ -237,30 +243,49 @@
     }
 }
 
-void Interface::hostStateEvent(sdbusplus::message::message& msg)
-{
-    std::map<std::string, std::variant<std::string>> properties{};
-    std::string interface;
-    msg.read(interface, properties);
-    const auto stateEntry = properties.find("CurrentHostState");
-    if (stateEntry != properties.end())
-    {
-        auto stateEntryValue = stateEntry->second;
-        auto propVal = std::get<std::string>(stateEntryValue);
-        if (propVal == "xyz.openbmc_project.State.Host.HostState.Off")
-        {
-            clearData();
-        }
-    }
-}
-
 void Interface::clearData()
 {
-    sensorToOCCInstance.clear();
-    occInstanceToEffecter.clear();
-
-    sensorToSBEInstance.clear();
-    sbeInstanceToEffecter.clear();
+    if (!sensorToOCCInstance.empty())
+    {
+        log<level::INFO>(
+            fmt::format("clearData: Clearing sensorToOCCInstance ({} entries)",
+                        sensorToOCCInstance.size())
+                .c_str());
+        for (auto entry : sensorToOCCInstance)
+        {
+            log<level::INFO>(
+                fmt::format("clearData: OCC{} / sensorID: 0x{:04X}",
+                            entry.second, entry.first)
+                    .c_str());
+        }
+        sensorToOCCInstance.clear();
+    }
+    if (!occInstanceToEffecter.empty())
+    {
+        log<level::DEBUG>(
+            fmt::format(
+                "clearData: Clearing occInstanceToEffecter ({} entries)",
+                occInstanceToEffecter.size())
+                .c_str());
+        occInstanceToEffecter.clear();
+    }
+    if (!sensorToSBEInstance.empty())
+    {
+        log<level::DEBUG>(
+            fmt::format("clearData: Clearing sensorToSBEInstance ({} entries)",
+                        sensorToSBEInstance.size())
+                .c_str());
+        sensorToSBEInstance.clear();
+    }
+    if (!sbeInstanceToEffecter.empty())
+    {
+        log<level::DEBUG>(
+            fmt::format(
+                "clearData: Clearing sbeInstanceToEffecter ({} entries)",
+                sbeInstanceToEffecter.size())
+                .c_str());
+        sbeInstanceToEffecter.clear();
+    }
 }
 
 void Interface::fetchEffecterInfo(uint16_t stateSetId,
@@ -326,8 +351,7 @@
     {
         auto pdrPtr =
             reinterpret_cast<const pldm_state_effecter_pdr*>(pdr.data());
-        uint32_t key = (static_cast<uint32_t>(pdrPtr->container_id) << 16) |
-                       static_cast<uint32_t>(pdrPtr->entity_instance);
+        uint32_t key = pdrPtr->effecter_id;
         entityInstMap.emplace(key, static_cast<SensorID>(pdrPtr->effecter_id));
     }
 
@@ -850,6 +874,8 @@
                 "checkActiveSensor: Unable to find PLDM sensor for OCC{}",
                 instance)
                 .c_str());
+        // Clear cache to recollect the sensor ids
+        clearData();
     }
 }