From ddf6a1cb4c2bc112c59608e0e3d79abc5d2826a3 Mon Sep 17 00:00:00 2001 From: Manojkiran Eda Date: Wed, 1 Sep 2021 19:19:51 +0530 Subject: [PATCH] Fix the looping behaviour in the getstatesensorreading command This bug was actually caused by two host sensors that has the exact same entity id, instance id, container id but being used to implement different state sets. The old host code had two sensors on processor module that implements both stateset 1 & 11 that caused this regression and using this PR we have improved the sensor parsing abilities so that we can compare the state setId as well before deciding that a sensor is unique. Signed-off-by: Manojkiran Eda --- common/types.hpp | 3 ++- host-bmc/host_pdr_handler.cpp | 26 ++++++++++++++++++++------ libpldmresponder/pdr_utils.cpp | 7 +++++-- libpldmresponder/platform.cpp | 5 +++-- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/common/types.hpp b/common/types.hpp index 213feebc8..579da23c1 100644 --- a/common/types.hpp +++ b/common/types.hpp @@ -48,7 +48,8 @@ using PossibleStates = std::set; //!< composite efffecter/sensor using CompositeSensorStates = std::vector; using EntityInfo = std::tuple; -using SensorInfo = std::tuple; +using SensorInfo = + std::tuple>; } // namespace pdr diff --git a/host-bmc/host_pdr_handler.cpp b/host-bmc/host_pdr_handler.cpp index b7dff7290..868f4f395 100644 --- a/host-bmc/host_pdr_handler.cpp +++ b/host-bmc/host_pdr_handler.cpp @@ -4,6 +4,7 @@ #include "libpldm/fru.h" #include "libpldm/requester/pldm.h" +#include "libpldm/state_set.h" #include "oem/ibm/libpldm/fru.h" #include "custom_dbus.hpp" @@ -780,10 +781,12 @@ void HostPDRHandler::setHostSensorState(const PDRList& stateSensorPDRs, pldm::pdr::EntityInfo entityInfo{}; pldm::pdr::CompositeSensorStates compositeSensorStates{}; + std::vector stateSetIds{}; try { - std::tie(entityInfo, compositeSensorStates) = + std::tie(entityInfo, compositeSensorStates, + stateSetIds) = lookupSensorInfo(sensorEntry); } catch (const std::out_of_range& e) @@ -791,7 +794,8 @@ void HostPDRHandler::setHostSensorState(const PDRList& stateSensorPDRs, try { sensorEntry.terminusID = PLDM_TID_RESERVED; - std::tie(entityInfo, compositeSensorStates) = + std::tie(entityInfo, compositeSensorStates, + stateSetIds) = lookupSensorInfo(sensorEntry); } catch (const std::out_of_range& e) @@ -1149,7 +1153,9 @@ void HostPDRHandler::setOperationStatus() { pldm::pdr::EntityInfo entityInfo{}; pldm::pdr::CompositeSensorStates compositeSensorStates{}; - std::tie(entityInfo, compositeSensorStates) = sensor.second; + std::vector stateSetIds{}; + std::tie(entityInfo, compositeSensorStates, stateSetIds) = + sensor.second; const auto& [containerId, entityType, entityInstance] = entityInfo; if (node.entity_type != entityType || @@ -1159,9 +1165,17 @@ void HostPDRHandler::setOperationStatus() continue; } uint8_t state = 0; - // Get sensorOpState property by the getStateSensorReadings command. - getPresentStateBySensorReadigs(sensor.first.sensorID, state, - sensorMapIndex->first); + if (compositeSensorStates.size() == 1 && + + stateSetIds[0] == PLDM_STATE_SET_OPERATIONAL_FAULT_STATUS) + { + // set the dbus property only when its not a composite sensor + // and the state set it PLDM_STATE_SET_OPERATIONAL_FAULT_STATUS + // Get sensorOpState property by the getStateSensorReadings + // command. + getPresentStateBySensorReadigs(sensor.first.sensorID, state, + sensorMapIndex->first); + } } } } diff --git a/libpldmresponder/pdr_utils.cpp b/libpldmresponder/pdr_utils.cpp index 8da283c6d..2a32101db 100644 --- a/libpldmresponder/pdr_utils.cpp +++ b/libpldmresponder/pdr_utils.cpp @@ -150,6 +150,7 @@ std::tuple CompositeSensorStates sensors{}; auto statesPtr = pdr->possible_states; auto compositeSensorCount = pdr->composite_sensor_count; + std::vector stateSetIds{}; while (compositeSensorCount--) { @@ -173,6 +174,8 @@ std::tuple updateStates); sensors.emplace_back(std::move(possibleStates)); + stateSetIds.emplace_back(state->state_set_id); + if (compositeSensorCount) { statesPtr += sizeof(state_sensor_possible_states) + @@ -184,8 +187,8 @@ std::tuple std::make_tuple(static_cast(pdr->container_id), static_cast(pdr->entity_type), static_cast(pdr->entity_instance)); - auto sensorInfo = - std::make_tuple(std::move(entityInfo), std::move(sensors)); + auto sensorInfo = std::make_tuple(std::move(entityInfo), std::move(sensors), + std::move(stateSetIds)); return std::make_tuple(pdr->terminus_handle, pdr->sensor_id, std::move(sensorInfo)); } diff --git a/libpldmresponder/platform.cpp b/libpldmresponder/platform.cpp index b7d5fd0bd..72f0b4360 100644 --- a/libpldmresponder/platform.cpp +++ b/libpldmresponder/platform.cpp @@ -409,10 +409,11 @@ int Handler::sensorEvent(const pldm_msg* request, size_t payloadLength, pldm::pdr::EntityInfo entityInfo{}; pldm::pdr::CompositeSensorStates compositeSensorStates{}; + std::vector stateSetIds{}; try { - std::tie(entityInfo, compositeSensorStates) = + std::tie(entityInfo, compositeSensorStates, stateSetIds) = hostPDRHandler->lookupSensorInfo(sensorEntry); } catch (const std::out_of_range& e) @@ -423,7 +424,7 @@ int Handler::sensorEvent(const pldm_msg* request, size_t payloadLength, try { sensorEntry.terminusID = PLDM_TID_RESERVED; - std::tie(entityInfo, compositeSensorStates) = + std::tie(entityInfo, compositeSensorStates, stateSetIds) = hostPDRHandler->lookupSensorInfo(sensorEntry); } // If there is no mapping for events return PLDM_SUCCESS