From 60fbee21b04909a953e4a8a742d6e74f52c43724 Mon Sep 17 00:00:00 2001 From: Matthew Raybuck Date: Fri, 13 Sep 2019 10:24:48 -0500 Subject: [PATCH] Fix bad arguments to additional FFDC capture in BPM Update code In bpm_update.C there were two places where the wrong errl was being given as a parameter to FFDC functions. The errl supplied was nullptr causing hostboot to crash as a result. This commit fixes that by suppling the correct errl. Change-Id: I50e8459d2b3602177ca740059a706613402c3e95 RTC:212448 Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/83781 Tested-by: Jenkins Server Tested-by: Jenkins OP Build CI Tested-by: FSP CI Jenkins Tested-by: Jenkins OP HW Reviewed-by: Daniel M Crowell --- src/include/usr/isteps/nvdimm/nvdimm.H | 6 +- src/usr/isteps/nvdimm/bpm_update.C | 8 +- src/usr/isteps/nvdimm/nvdimm.C | 158 +++++++++++++++---------- 3 files changed, 102 insertions(+), 70 deletions(-) diff --git a/src/include/usr/isteps/nvdimm/nvdimm.H b/src/include/usr/isteps/nvdimm/nvdimm.H index 9ccfa0e7fa9..2aae34c820e 100644 --- a/src/include/usr/isteps/nvdimm/nvdimm.H +++ b/src/include/usr/isteps/nvdimm/nvdimm.H @@ -406,7 +406,7 @@ errlHndl_t compareCksum(TARGETING::Target* i_nvdimm, * * @param[in] i_nvdimm - nvdimm target * - * @param[inout] io_err - error log to add FFDC data + * @param[inout] io_err - error log to add FFDC data. Must not be nullptr * */ void nvdimmAddVendorLog(TARGETING::Target *i_nvdimm, errlHndl_t& io_err); @@ -419,7 +419,7 @@ void nvdimmAddVendorLog(TARGETING::Target *i_nvdimm, errlHndl_t& io_err); * * @param[in] i_nvdimm - nvdimm target * - * @param[in] io_err - error log to add FFDC data + * @param[in] io_err - error log to add FFDC data. Must not be nullptr * */ void nvdimmAddUpdateRegs(TARGETING::Target *i_nvdimm, errlHndl_t& io_err); @@ -440,7 +440,7 @@ void nvdimmAddUpdateRegs(TARGETING::Target *i_nvdimm, errlHndl_t& io_err); * * @param[in] i_nvdimm - nvdimm target * - * @param[inout] io_err - error log to add FFDC data + * @param[inout] io_err - error log to add FFDC data. Must not be nullptr * */ void nvdimmAddPage4Regs(TARGETING::Target *i_nvdimm, errlHndl_t& io_err); diff --git a/src/usr/isteps/nvdimm/bpm_update.C b/src/usr/isteps/nvdimm/bpm_update.C index 7910c1e5bcf..1208d784169 100644 --- a/src/usr/isteps/nvdimm/bpm_update.C +++ b/src/usr/isteps/nvdimm/bpm_update.C @@ -1255,8 +1255,8 @@ errlHndl_t Bpm::enterUpdateMode() BPM_RC::BPM_ENTER_UPDATE_MODE, TARGETING::get_huid(iv_nvdimm)); infoErrl->collectTrace(BPM_COMP_NAME); - nvdimmAddPage4Regs(iv_nvdimm,errl); - nvdimmAddVendorLog(iv_nvdimm, errl); + nvdimmAddVendorLog(iv_nvdimm, infoErrl); + nvdimmAddPage4Regs(iv_nvdimm, infoErrl); ERRORLOG::errlCommit(infoErrl, BPM_COMP_ID); } while(0); @@ -1350,8 +1350,8 @@ errlHndl_t Bpm::exitUpdateMode() HWAS::BPM_PART_TYPE, HWAS::SRCI_PRIORITY_HIGH); infoErrl->collectTrace(BPM_COMP_NAME); - nvdimmAddPage4Regs(iv_nvdimm,errl); - nvdimmAddVendorLog(iv_nvdimm, errl); + nvdimmAddVendorLog(iv_nvdimm, infoErrl); + nvdimmAddPage4Regs(iv_nvdimm, infoErrl); ERRORLOG::errlCommit(infoErrl, BPM_COMP_ID); } while(0); diff --git a/src/usr/isteps/nvdimm/nvdimm.C b/src/usr/isteps/nvdimm/nvdimm.C index 0a35b1151dc..8ef591a6ac2 100644 --- a/src/usr/isteps/nvdimm/nvdimm.C +++ b/src/usr/isteps/nvdimm/nvdimm.C @@ -4191,6 +4191,16 @@ void nvdimmAddVendorLog( TARGETING::Target* i_nvdimm, errlHndl_t& io_err ) break; } + if (io_err == nullptr) + { + // A nullptr was given when it should not have been. Emit a trace + // and break out of this function. + TRACFCOMP(g_trac_nvdimm, ERR_MRK + "nvdimmAddVendorLog() io_err was nullptr!! Skip adding additional FFDC."); + break; + } + + // Set the vendor log attribute so we don't recursively // execute the nvdimmAddVendorLog function l_vendorLog = 0x1; @@ -4312,6 +4322,80 @@ void nvdimmAddVendorLog( TARGETING::Target* i_nvdimm, errlHndl_t& io_err ) } +/* + * @brief Add NVDIMM Update regs to FFDC for errors encountered + * during NVDIMM update process + */ +void nvdimmAddUpdateRegs( TARGETING::Target* i_nvdimm, errlHndl_t& io_err ) +{ + errlHndl_t l_err = nullptr; + + do { + + if (io_err == nullptr) + { + // A nullptr was given when it should not have been. Emit a trace + // and break out of this function. + TRACFCOMP(g_trac_nvdimm, ERR_MRK + "nvdimmAddUpdateRegs() io_err was nullptr!! Skip adding additional FFDC."); + break; + } + + ERRORLOG::ErrlUserDetailsLogRegister l_regUD(i_nvdimm); + const uint32_t l_regList[] = { + NVDIMM_READY, + FIRMWARE_OPS_STATUS, + NVDIMM_CMD_STATUS0, + FIRMWARE_OPS_TIMEOUT0, + FIRMWARE_OPS_TIMEOUT1, + FW_REGION_CRC0, + FW_REGION_CRC1, + MODULE_HEALTH, + MODULE_HEALTH_STATUS0, + MODULE_HEALTH_STATUS1, + ERROR_THRESHOLD_STATUS, + ENCRYPTION_CONFIG_STATUS, + FW_SLOT_INFO, + SLOT0_ES_FWREV0, + SLOT0_ES_FWREV1, + SLOT1_ES_FWREV0, + SLOT1_ES_FWREV1, + SLOT1_SUBFWREV, + CSAVE_INFO, + CSAVE_FAIL_INFO1, + RESTORE_STATUS, + RESTORE_FAIL_INFO, + }; + uint8_t l_readData = 0; + + for (auto l_reg : l_regList) + { + l_err = nvdimmReadReg(i_nvdimm, + l_reg, + l_readData); + if (l_err) + { + TRACFCOMP(g_trac_nvdimm, ERR_MRK + "nvdimmAddUpdateRegs() nvdimm[%X] error reading 0x%X", + get_huid(i_nvdimm), l_reg); + + // Don't commit, just delete the error and continue + delete l_err; + l_err = nullptr; + continue; + } + + l_regUD.addDataBuffer(&l_readData, + sizeof(l_readData), + DEVICE_NVDIMM_ADDRESS(l_reg)); + } + + l_regUD.addToLog(io_err); + + } while(0); +} + + /* * @brief Add Page 4 regs to FFDC * Added to all NVDIMM HW errors @@ -4322,6 +4406,16 @@ void nvdimmAddPage4Regs( TARGETING::Target* i_nvdimm, errlHndl_t& io_err ) do { + if (io_err == nullptr) + { + // A nullptr was given when it should not have been. Emit a trace + // and break out of this function. + TRACFCOMP(g_trac_nvdimm, ERR_MRK + "nvdimmAddPage4Regs() io_err was nullptr!! Skip adding additional FFDC."); + break; + } + + // Get the page4 attribute, if set we are already // reading the page4 regs, exit auto l_page4 = i_nvdimm->getAttr(); @@ -4353,7 +4447,7 @@ void nvdimmAddPage4Regs( TARGETING::Target* i_nvdimm, errlHndl_t& io_err ) for (auto l_reg : l_regList) { l_err = nvdimmReadReg(i_nvdimm, - l_regList[l_reg], + l_reg, l_readData); if (l_err) { @@ -4381,68 +4475,6 @@ void nvdimmAddPage4Regs( TARGETING::Target* i_nvdimm, errlHndl_t& io_err ) } while(0); } - -/* - * @brief Add NVDIMM Update regs to FFDC for errors encountered - * during NVDIMM update process - */ -void nvdimmAddUpdateRegs( TARGETING::Target* i_nvdimm, errlHndl_t& io_err ) -{ - errlHndl_t l_err = nullptr; - - ERRORLOG::ErrlUserDetailsLogRegister l_regUD(i_nvdimm); - const uint32_t l_regList[] = { - NVDIMM_READY, - FIRMWARE_OPS_STATUS, - NVDIMM_CMD_STATUS0, - FIRMWARE_OPS_TIMEOUT0, - FIRMWARE_OPS_TIMEOUT1, - FW_REGION_CRC0, - FW_REGION_CRC1, - MODULE_HEALTH, - MODULE_HEALTH_STATUS0, - MODULE_HEALTH_STATUS1, - ERROR_THRESHOLD_STATUS, - ENCRYPTION_CONFIG_STATUS, - FW_SLOT_INFO, - SLOT0_ES_FWREV0, - SLOT0_ES_FWREV1, - SLOT1_ES_FWREV0, - SLOT1_ES_FWREV1, - SLOT1_SUBFWREV, - CSAVE_INFO, - CSAVE_FAIL_INFO1, - RESTORE_STATUS, - RESTORE_FAIL_INFO, - }; - uint8_t l_readData = 0; - - for (auto l_reg : l_regList) - { - l_err = nvdimmReadReg(i_nvdimm, - l_regList[l_reg], - l_readData); - if (l_err) - { - TRACFCOMP(g_trac_nvdimm, ERR_MRK - "nvdimmAddUpdateRegs() nvdimm[%X] error reading 0x%X", - get_huid(i_nvdimm), l_reg); - - // Don't commit, just delete the error and continue - delete l_err; - l_err = nullptr; - continue; - } - - l_regUD.addDataBuffer(&l_readData, - sizeof(l_readData), - DEVICE_NVDIMM_ADDRESS(l_reg)); - } - - l_regUD.addToLog(io_err); -} - - /* * @brief Utility function to send the value of * ATTR_NVDIMM_ARMED to the FSP