Skip to content

Commit

Permalink
Remove logErrors in plug_rules
Browse files Browse the repository at this point in the history 
Change-Id: I32a1a280dfcc723298208bb2f9170a3aaf709fb8
Original-Change-Id: I4ddbe7b5ee6595ddfef7a46381ddc3f45adac48f
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/41830
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Hostboot CI <hostboot-ci+hostboot@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>
Reviewed-by: Louis Stermole <stermole@us.ibm.com>
Reviewed-by: STEPHEN GLANCY <sglancy@us.ibm.com>
Reviewed-by: Jennifer A. Stofer <stofer@us.ibm.com>
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/66309
Tested-by: Daniel M. Crowell <dcrowell@us.ibm.com>
  • Loading branch information
@JacobHarvey @dcrowell77
JacobHarvey authored and dcrowell77 committed Sep 19, 2018
1 parent 5505f90 commit 17a5d5f
Showing 1 changed file with 16 additions and 46 deletions.
62 changes: 16 additions & 46 deletions src/import/chips/p9/procedures/hwp/memory/lib/eff_config/plug_rules.C
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ fapi2::ReturnCode empty_slot_zero(const fapi2::Target<fapi2::TARGET_TYPE_MCA>& i
fapi2::current_err = fapi2::FAPI2_RC_SUCCESS;
const auto l_dimms = mss::find_targets<TARGET_TYPE_DIMM>(i_target);

// If there's one dimm, make sure it's in slot 0
if ( l_dimms.size() == 1 )
{
FAPI_ASSERT( mss::index(l_dimms[0]) == 0,
Expand All @@ -176,18 +177,10 @@ fapi2::ReturnCode empty_slot_zero( const fapi2::Target<fapi2::TARGET_TYPE_MCS>&

for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target) )
{
// Doing this so we can check both ports instead of just one mca
fapi2::current_err = empty_slot_zero( p );

if ( fapi2::current_err != fapi2::FAPI2_RC_SUCCESS )
{
fapi2::logError( (l_rc = fapi2::current_err));
}
FAPI_TRY( empty_slot_zero( p ) );
}

// Need to return an error or else the logs will be ignored
fapi2::current_err = (fapi2::current_err == fapi2::FAPI2_RC_SUCCESS) ? l_rc : fapi2::current_err;

fapi_try_exit:
return fapi2::current_err;
}

Expand Down Expand Up @@ -252,16 +245,10 @@ fapi2::ReturnCode check_dead_load( const fapi2::Target<fapi2::TARGET_TYPE_MCS>&

for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target) )
{
fapi2::current_err = check_dead_load( p );

if ( fapi2::current_err != fapi2::FAPI2_RC_SUCCESS )
{
fapi2::logError( (l_rc = fapi2::current_err));
}
FAPI_TRY( check_dead_load( p ) );
}

fapi2::current_err = (fapi2::current_err == fapi2::FAPI2_RC_SUCCESS) ? l_rc : fapi2::current_err;

fapi_try_exit:
return fapi2::current_err;
}

Expand All @@ -279,14 +266,15 @@ fapi2::ReturnCode check_gen( const std::vector<dimm::kind>& i_kinds )
// This should never fail ... but Just In Case a little belt-and-suspenders never hurt.
// TODO RTC:160395 This needs to change for controllers which support different generations
// Nimbus only supports DDR4 for now
FAPI_ASSERT_NOEXIT( k.iv_dram_generation == fapi2::ENUM_ATTR_EFF_DRAM_GEN_DDR4 ||
k.iv_dram_generation == fapi2::ENUM_ATTR_EFF_DRAM_GEN_EMPTY,
fapi2::MSS_PLUG_RULES_INVALID_DRAM_GEN()
.set_DRAM_GEN(k.iv_dimm_type)
.set_DIMM_TARGET(k.iv_target),
"%s is not DDR4 it is %d", mss::c_str(k.iv_target), k.iv_dram_generation );
FAPI_ASSERT( k.iv_dram_generation == fapi2::ENUM_ATTR_EFF_DRAM_GEN_DDR4 ||
k.iv_dram_generation == fapi2::ENUM_ATTR_EFF_DRAM_GEN_EMPTY,
fapi2::MSS_PLUG_RULES_INVALID_DRAM_GEN()
.set_DRAM_GEN(k.iv_dimm_type)
.set_DIMM_TARGET(k.iv_target),
"%s is not DDR4 it is %d", mss::c_str(k.iv_target), k.iv_dram_generation );
}

fapi_try_exit:
return fapi2::current_err;
}

Expand Down Expand Up @@ -429,7 +417,6 @@ fapi_try_exit:
fapi2::ReturnCode plug_rule::enforce_plug_rules(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target)
{
// Check per-MCS plug rules. If those all pass, check each of our MCA

const auto l_dimms = mss::find_targets<TARGET_TYPE_DIMM>(i_target);
fapi2::ReturnCode l_rc (fapi2::FAPI2_RC_SUCCESS);

Expand All @@ -452,29 +439,14 @@ fapi2::ReturnCode plug_rule::enforce_plug_rules(const fapi2::Target<fapi2::TARGE
return FAPI2_RC_SUCCESS;
}

// We want to be careful here. The idea is to execute all the plug rules and commit erros before
// failing out. That way, a giant configuration doesn't take 45m to boot, to find a DIMM out of place
// the another 45m to find the next DIMM, etc.
for (const auto& p : mss::find_targets<TARGET_TYPE_MCA>(i_target))
{
// Cronus only exits if the procedure returns an error, so we need to keep track
// To do this, we are using l_rc and current_err
// current_err is not confined by scope,
// So current_err can be set to SUCCESS within the called function and could overwrite bad values
// l_rc will keep track if there are any errors and won't be overwritten by a success accidentally
fapi2::current_err = enforce_plug_rules(p);

if ( fapi2::current_err != fapi2::FAPI2_RC_SUCCESS )
{
// Sets current_err to bad and doesn't override with a "good" status
fapi2::logError( l_rc = fapi2::current_err );
}
// Difference between cronus and hostboot make it really annoying to loop over the targets,
// So we'll just error out if we find a bad port, this could make 2 deconfig loops instead of one,
// but it's worth for proper behavior and really shouldn't happen often
FAPI_TRY( enforce_plug_rules(p) );
}

// if current_err is marked as good, set to l_rc
// l_rc error has already been logged, so if current_err has an error report that
fapi2::current_err = (fapi2::current_err == fapi2::FAPI2_RC_SUCCESS) ? l_rc : fapi2::current_err;

fapi_try_exit:
return fapi2::current_err;
}
Expand Down Expand Up @@ -529,6 +501,4 @@ fapi2::ReturnCode plug_rule::enforce_plug_rules(const fapi2::Target<fapi2::TARGE
fapi_try_exit:
return fapi2::current_err;
}


}// mss

0 comments on commit 17a5d5f

Please sign in to comment.