Skip to content

Commit

Permalink
Fix bad arguments to additional FFDC capture in BPM Update code
Browse files Browse the repository at this point in the history 
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 <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Reviewed-by: Daniel M Crowell <dcrowell@us.ibm.com>
  • Loading branch information
@MRaybuck @dcrowell77
MRaybuck authored and dcrowell77 committed Sep 14, 2019
1 parent 7a27f46 commit 60fbee2
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 70 deletions.
6 changes: 3 additions & 3 deletions src/include/usr/isteps/nvdimm/nvdimm.H
View file
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/usr/isteps/nvdimm/bpm_update.C
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
158 changes: 95 additions & 63 deletions src/usr/isteps/nvdimm/nvdimm.C
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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<ATTR_NVDIMM_READING_PAGE4>();
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 60fbee2

Please sign in to comment.