From 25dfca20f6cbb9ef2dbebcb0a3cf2fb11d9ec48f Mon Sep 17 00:00:00 2001 From: Archana Kakani Date: Sun, 8 Dec 2024 23:00:38 -0600 Subject: [PATCH] libresponder: Fixing issues in CM path (#636) There is multiple issues faced during the CM. This commit fixes below issues 1.Port PDRs not getting generated after adding cable card concurrently 2.Multiple adapter were seen under single I/O Slot 3.No adapter PDRs for the cards under NVMe Slots (Drive Backplane) When resolving issues, the design is that the Adapter PDR will always remain in the PDR repository, regardless of whether the card is physically present or absent. Tested: Using busctl command Signed-off-by: Archana Kakani --- libpldmresponder/fru.cpp | 156 +++++++++++++++++++++++++---- oem/ibm/libpldmresponder/utils.cpp | 23 +++-- 2 files changed, 153 insertions(+), 26 deletions(-) diff --git a/libpldmresponder/fru.cpp b/libpldmresponder/fru.cpp index c6c73a8a..790b47fd 100644 --- a/libpldmresponder/fru.cpp +++ b/libpldmresponder/fru.cpp @@ -1,6 +1,9 @@ #include "fru.hpp" #include "common/utils.hpp" +#ifdef OEM_IBM +#include "oem/ibm/libpldmresponder/utils.hpp" +#endif #include #include @@ -379,6 +382,12 @@ uint32_t FruImpl::populateRecords( void FruImpl::removeIndividualFRU(const std::string& fruObjPath) { uint16_t rsi = objectPathToRSIMap[fruObjPath]; + if (!rsi) + { + info("No Pdrs to delete for the object path {PATH}", "PATH", + fruObjPath); + return; + } pldm_entity removeEntity; uint16_t terminusHdl{}; uint16_t entityType{}; @@ -398,14 +407,38 @@ void FruImpl::removeIndividualFRU(const std::string& fruObjPath) auto removeBmcEntityRc = pldm_entity_association_pdr_remove_contained_entity( pdrRepo, &removeEntity, false, &updateRecordHdlBmc); + error("delete/remove contained entity from bmc record handle: {HANDLE}", "HANDLE", updateRecordHdlBmc); + + pldm::responder::pdr_utils::PdrEntry pdrEntry; + uint8_t* pdrData = nullptr; + auto record = pldm_pdr_find_record(pdrRepo, updateRecordHdlBmc, &pdrData, + &pdrEntry.size, + &pdrEntry.handle.nextRecordHandle); + if (record) + { + error("Found BMC Record {REC}", "REC", updateRecordHdlBmc); + } + bmcEventDataOps = record ? PLDM_RECORDS_MODIFIED : PLDM_RECORDS_DELETED; auto removeHostEntityRc = pldm_entity_association_pdr_remove_contained_entity( pdrRepo, &removeEntity, true, &updateRecordHdlHost); + error("delete/remove contained entity host record handle: {HANDLE}", "HANDLE", updateRecordHdlHost); + record = pldm_pdr_find_record(pdrRepo, updateRecordHdlHost, &pdrData, + &pdrEntry.size, + &pdrEntry.handle.nextRecordHandle); + if (record) + { + error("Found Host Record {REC}", "REC", updateRecordHdlHost); + } + hostEventDataOps = record ? PLDM_RECORDS_MODIFIED : PLDM_RECORDS_DELETED; pldm_pdr_remove_fru_record_set_by_rsi(pdrRepo, rsi, false, &deleteRecordHdl); + error("delete record handle: {HANDLE} from rsi: {RSI}", "HANDLE", + deleteRecordHdl, "RSI", rsi); + pldm_entity_association_tree_delete_node(entityTree, removeEntity); pldm_entity_association_tree_delete_node(bmcEntityTree, removeEntity); @@ -419,6 +452,7 @@ void FruImpl::removeIndividualFRU(const std::string& fruObjPath) static_cast(removeEntity.entity_container_id)); associatedEntityMap.erase(fruObjPath); + error("delete RSI: {HANDLE}", "HANDLE", rsi); deleteFruRecord(rsi); std::vector handlesTobeDeleted; @@ -437,6 +471,8 @@ void FruImpl::removeIndividualFRU(const std::string& fruObjPath) effecterDbusObjMaps.erase(ids); if (delEffecterHdl != 0) { + error("delete effecter ID: {ID} and record handle: {HANDLE}", "ID", + ids, "HANDLE", delEffecterHdl); handlesTobeDeleted.push_back(delEffecterHdl); } } @@ -450,6 +486,8 @@ void FruImpl::removeIndividualFRU(const std::string& fruObjPath) sensorDbusObjMaps.erase(ids); if (delSensorHdl != 0) { + error("delete sensor ID: {ID} and record handle: {HANDLE}", "ID", + ids, "HANDLE", delSensorHdl); handlesTobeDeleted.push_back(delSensorHdl); } } @@ -469,15 +507,30 @@ void FruImpl::removeIndividualFRU(const std::string& fruObjPath) ? handlesTobeDeleted.push_back(updateRecordHdlHost) : handlesTobeModified.push_back(updateRecordHdlHost); } + + + error("Modified Handles"); + for(const auto &modified : handlesTobeModified) + { + error("{H}", "H", modified); + } + error("Deleted Handles"); + for(const auto &deleted : handlesTobeDeleted) + { + error("{H}", "H", deleted); + } + // Adapter PDRs can have deleted records if (!handlesTobeDeleted.empty()) { + error("Sending repo change event for deleted handler"); sendPDRRepositoryChgEventbyPDRHandles( std::move(handlesTobeDeleted), std::move(std::vector(1, PLDM_RECORDS_DELETED))); } if (!handlesTobeModified.empty()) { + error("Sending repo change event fored handler"); sendPDRRepositoryChgEventbyPDRHandles( std::move(handlesTobeModified), std::move(std::vector(1, PLDM_RECORDS_MODIFIED))); @@ -538,6 +591,14 @@ void FruImpl::buildIndividualFRU(const std::string& fruInterface, pldm_entity parentEntity{}; static uint32_t bmc_record_handle = 0; uint32_t newRecordHdl{}; + + if (fruInterface == "xyz.openbmc_project.Inventory.Item.PCIeDevice") + { + // Delete the existing adapter PDR before creating new one + info("Removing old PDRs {PATH}", "PATH", fruObjectPath); + removeIndividualFRU(fruObjectPath); + } + try { entity.entity_type = parser.getEntityType(fruInterface); @@ -580,7 +641,18 @@ void FruImpl::buildIndividualFRU(const std::string& fruInterface, pldm_entity_association_tree_add(bmcEntityTree, &entity, 0xFFFF, bmcTreeParentNode, PLDM_ENTITY_ASSOCIAION_PHYSICAL); + info( + "2 Building Individual FRU [{FRU_OBJ_PATH}] with entityid [ {ENTITY_TYPE}, {ENTITY_NUM}, {ENTITY_ID} ] Parent :[ {P_ENTITY_TYP}, {P_ENTITY_NUM}, {P_ENTITY_ID} ]", + "FRU_OBJ_PATH", fruObjectPath, "ENTITY_TYPE", + static_cast(node_entity.entity_type), "ENTITY_NUM", + static_cast(node_entity.entity_instance_num), "ENTITY_ID", + static_cast(node_entity.entity_container_id), + "P_ENTITY_TYP", static_cast(parent.entity_type), + "P_ENTITY_NUM", static_cast(parent.entity_instance_num), + "P_ENTITY_ID", static_cast(parent.entity_container_id)); + objects = DBusHandler::getManagedObj( + "xyz.openbmc_project.Inventory.Manager", inventoryPath); for (const auto& object : objects) { if (object.first.str == fruObjectPath) @@ -588,6 +660,9 @@ void FruImpl::buildIndividualFRU(const std::string& fruInterface, const auto& interfaces = object.second; newRecordHdl = populateRecords(interfaces, recordInfos, entity, fruObjectPath, true); + error( + "build record handle: {HANDLE} FRU object path: {OBJPATH}", + "HANDLE", newRecordHdl, "OBJPATH", fruObjectPath); associatedEntityMap.emplace(fruObjectPath, entity); break; } @@ -637,11 +712,13 @@ void FruImpl::buildIndividualFRU(const std::string& fruInterface, found = false; pldm_entity_association_find_parent_entity(pdrRepo, &parent, true, &updatedRecordHdlHost, &found); + error("build host record handle: {HANDLE}", "HANDLE", updatedRecordHdlHost); if (found) { pldm_entity_association_pdr_add_contained_entity_to_remote_pdr( pdrRepo, &entity, updatedRecordHdlHost); hostEventDataOps = PLDM_RECORDS_MODIFIED; + error("Modify build host record handle: {HANDLE}", "HANDLE", updatedRecordHdlHost); } else { @@ -649,6 +726,8 @@ void FruImpl::buildIndividualFRU(const std::string& fruInterface, &parent, &entity, &updatedRecordHdlHost); hostEventDataOps = PLDM_RECORDS_ADDED; + error(" build host record handle: {HANDLE}", "HANDLE", updatedRecordHdlHost); + } // create the relevant state effecter and sensor PDRs for the new fru record @@ -662,6 +741,7 @@ void FruImpl::buildIndividualFRU(const std::string& fruInterface, for (auto& ids : recordHdlList) { + error("build record handle from list: {HANDLE}", "HANDLE", ids); handlesTobeAdded.push_back(ids); } if (updatedRecordHdlBmc != 0) @@ -676,6 +756,16 @@ void FruImpl::buildIndividualFRU(const std::string& fruInterface, ? handlesTobeModified.push_back(updatedRecordHdlHost) : handlesTobeAdded.push_back(updatedRecordHdlHost); } + error("Modified Handles"); + for(const auto &modified : handlesTobeModified) + { + error("{H}", "H", modified); + } + error("Added Handles"); + for(const auto &deleted : handlesTobeAdded) + { + error("{H}", "H", deleted); + } if (!handlesTobeAdded.empty()) { sendPDRRepositoryChgEventbyPDRHandles( @@ -922,37 +1012,67 @@ void FruImpl::processFruPresenceChange(const DbusChangedProps& chProperties, } std::vector portObjects; - static constexpr auto portInterface = - "xyz.openbmc_project.Inventory.Item.Connector"; - if (oemUtilsHandler) + if (newPropVal) { - if (fruInterface == "xyz.openbmc_project.Inventory.Item.PCIeDevice") + info("Presence changed to true for {PATH}", "PATH", fruObjPath); + // oemUtilsHandler check is added to make sure that it is OEM_IBM path + if (fruInterface == "xyz.openbmc_project.Inventory.Item.PCIeDevice" && + oemUtilsHandler) { - if (!oemUtilsHandler->checkFruPresence(fruObjPath.c_str())) + if (pldm::responder::utils::checkIfIBMFru(fruObjPath)) { + static constexpr auto portInterface = + "xyz.openbmc_project.Inventory.Item.Connector"; + + // if the added fru is IBM cable card, build the adapter + // PDRs and the port PDRs. + buildIndividualFRU(fruInterface, fruObjPath); + portObjects = oemUtilsHandler->findPortObjects(fruObjPath); + info("Ports under the cable card: {NUM}", "NUM", + (int)portObjects.size()); + for (auto portObject : portObjects) + { + buildIndividualFRU(portInterface, portObject); + } + return; + } + else + { + // PDRs realted to industry standard card are not + // rebuilt. return; } - portObjects = oemUtilsHandler->findPortObjects(fruObjPath); } - // as per current code the ports do not have Present property - } - - if (newPropVal) - { buildIndividualFRU(fruInterface, fruObjPath); - for (auto portObject : portObjects) - { - buildIndividualFRU(portInterface, portObject); - } } else { - for (auto portObject : portObjects) + info("Presence changed to false for {PATH}", "PATH", fruObjPath); + if (fruInterface == "xyz.openbmc_project.Inventory.Item.PCIeDevice") + { + // If the card is removed from the system, we do remove the port + // pdrs(if present) but not the adapter PDR as adapter PDR should be + // always there inspite of the presence of the card. + if (oemUtilsHandler) + { + portObjects = oemUtilsHandler->findPortObjects(fruObjPath); + for (auto portObject : portObjects) + { + removeIndividualFRU(portObject); + } + } + else + { + // Non OEM-IBM path, remove the pcieDevice pdrs + removeIndividualFRU(fruObjPath); + } + } + else { - removeIndividualFRU(portObject); + info("Removing Individual FRU {PATH}", "PATH", fruObjPath); + removeIndividualFRU(fruObjPath); } - removeIndividualFRU(fruObjPath); } } diff --git a/oem/ibm/libpldmresponder/utils.cpp b/oem/ibm/libpldmresponder/utils.cpp index 82ef0e5b..e6164134 100644 --- a/oem/ibm/libpldmresponder/utils.cpp +++ b/oem/ibm/libpldmresponder/utils.cpp @@ -608,25 +608,31 @@ bool pldm::responder::oem_ibm_utils::Handler::checkFruPresence( // the port is considered as present. this is so because // the ports do not have "Present" property std::string pcieAdapter("pcie_card"); + std::string nvmeAdapter("_drive"); std::string portStr("cxp_"); + std::string connectorStr("_connector"); std::string newObjPath = objPath; bool isPresent = true; - if (newObjPath.find(pcieAdapter) != std::string::npos) + if ((newObjPath.find(pcieAdapter) != std::string::npos) || + (newObjPath.find(nvmeAdapter) != std::string::npos)) { - if (newObjPath.find(portStr) != std::string::npos) + if ((newObjPath.find(portStr) != std::string::npos) || + (newObjPath.find(connectorStr) != std::string::npos)) { newObjPath = pldm::utils::findParent(objPath); } else { - return true; // industry std cards + // Adapter PDRs are always created underneath the slot for + // Insustry standard cards as well as IBM cable cards. + // Phyp expects the FRU records for industry std cards to be always + // built, irrespective of presence. This is needed by PHYP to send + // the Fan floor information + return true; // industry std cards/IBM cable cards } } - // Phyp expects the FRU records for industry std cards to be always - // built, irrespective of presence - static constexpr auto presentInterface = "xyz.openbmc_project.Inventory.Item"; static constexpr auto presentProperty = "Present"; @@ -638,8 +644,9 @@ bool pldm::responder::oem_ibm_utils::Handler::checkFruPresence( } catch (const sdbusplus::exception::SdBusError& e) { - error("D-Bus method call to get the FRU presence failed ERROR={ERROR}", - "ERROR", e); + error( + "D-Bus method call to get the FRU presence of {FRU} failed ERROR={ERR} and return is {V}", + "FRU", newObjPath, "ERR", e.what(), "V", (int)isPresent); } return isPresent; }