Skip to content

Commit

Permalink
Fixes broadcast mode memdiags crash
Browse files Browse the repository at this point in the history 
Does three things:
1) Adds a switch to disable BC mode in memdiags
2) Adds a check for valid addressing in memdiags
3) Disables broadcast mode on a DD1 part

Change-Id: Ia79c9247bdeb157ed2277afb9cb52a5f25c1c032
CQ:SW406221
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/48876
Tested-by: HWSV CI <hwsv-ci+hostboot@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Hostboot CI <hostboot-ci+hostboot@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Reviewed-by: Louis Stermole <stermole@us.ibm.com>
Reviewed-by: ANDRE A. MARIN <aamarin@us.ibm.com>
Dev-Ready: STEPHEN GLANCY <sglancy@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>
Reviewed-by: Jennifer A. Stofer <stofer@us.ibm.com>
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/48879
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Reviewed-by: Christian R. Geddes <crgeddes@us.ibm.com>
  • Loading branch information
@esteban012 @crgeddes
esteban012 authored and crgeddes committed Nov 1, 2017
1 parent 5b272bf commit 1cf8ace
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 17 deletions.
60 changes: 44 additions & 16 deletions src/import/chips/p9/procedures/hwp/memory/lib/mcbist/mcbist.C
View file
Expand Up @@ -79,7 +79,8 @@ namespace mcbist
/// @return FAPI2_RC_SUCCSS iff ok
///
template< >
fapi2::ReturnCode load_maint_pattern( const fapi2::Target<TARGET_TYPE_MCBIST>& i_target, const pattern& i_pattern,
fapi2::ReturnCode load_maint_pattern( const fapi2::Target<TARGET_TYPE_MCBIST>& i_target,
const pattern& i_pattern,
const bool i_invert )
{
// Init the fapi2 return code
Expand Down Expand Up @@ -733,25 +734,52 @@ const mss::states is_broadcast_capable(const std::vector<fapi2::Target<fapi2::TA
template< >
const mss::states is_broadcast_capable(const fapi2::Target<fapi2::TARGET_TYPE_MCBIST>& i_target)
{
// Steps to determine if this MCBIST is broadcastable
// 1) Check the number of DIMM's on each MCA - true only if they all match
// 2) Check that all of the DIMM kinds are equal - if the are, then we can do broadcast mode
// 3) if both 1 and 2 are true, then broadcast capable, otherwise false
// First off, check if we need to disable broadcast mode due to a chip size bug
// Note: the bug check is decidedly more complicated than the EC check, but we'll just disable BC mode out of safety concerns
// Better to go slow and steady and initialize the chip properly than to go fast and leave the memory initialized poorly
if( mss::chip_ec_feature_mcbist_end_of_rank(i_target) )
{
FAPI_INF("%s A chip bug prevents broadcast mode. Chip is not brodcast capable", mss::c_str(i_target));
return mss::states::NO;
}

// If BC mode is disabled in the MRW, then it's disabled here
uint8_t l_bc_mode_enable = 0;
FAPI_TRY(mss::mrw_memdiags_bcmode(l_bc_mode_enable));

if( l_bc_mode_enable == fapi2::ENUM_ATTR_MSS_MRW_MEMDIAGS_BCMODE_DISABLE )
{
FAPI_INF("%s MRW attribute has broadcast mode disabled", mss::c_str(i_target));
return mss::states::NO;
}

// Now that we are guaranteed to have a chip that could run broadcast mode, do the following steps to check whether our config is broadcast capable:
{
// Steps to determine if this MCBIST is broadcastable
// 1) Check the number of DIMM's on each MCA - true only if they all match
// 2) Check that all of the DIMM kinds are equal - if they are, then we can do broadcast mode
// 3) if both 1 and 2 are true, then broadcast capable, otherwise false

// 1) Check the number of DIMM's on each MCA - if they don't match, then no
const auto l_mca_check = is_broadcast_capable(mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target));
// 1) Check the number of DIMM's on each MCA - if they don't match, then no
const auto l_mca_check = is_broadcast_capable(mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target));

// 2) Check that all of the DIMM kinds are equal - if the are, then we can do broadcast mode
const auto l_dimms = mss::find_targets<fapi2::TARGET_TYPE_DIMM>(i_target);
const auto l_dimm_kinds = mss::dimm::kind::vector(l_dimms);
const auto l_dimm_kind_check = is_broadcast_capable(l_dimm_kinds);
// 2) Check that all of the DIMM kinds are equal - if they are, then we can do broadcast mode
const auto l_dimms = mss::find_targets<fapi2::TARGET_TYPE_DIMM>(i_target);
const auto l_dimm_kinds = mss::dimm::kind::vector(l_dimms);
const auto l_dimm_kind_check = is_broadcast_capable(l_dimm_kinds);

// 3) if both 1/2 are true, then broadcastable, otherwise false
const auto l_capable = (l_mca_check == mss::states::YES && l_dimm_kind_check == mss::states::YES) ?
mss::states::YES : mss::states::NO;
// 3) if both 1/2 are true, then broadcastable, otherwise false
const auto l_capable = ((l_mca_check == mss::states::YES) && (l_dimm_kind_check == mss::states::YES)) ?
mss::states::YES : mss::states::NO;

FAPI_INF("%s %s broadcast capable", mss::c_str(i_target), (l_capable == mss::states::YES) ? "is" : "is not");
return l_capable;
FAPI_INF("%s %s broadcast capable", mss::c_str(i_target), (l_capable == mss::states::YES) ? "is" : "is not");
return l_capable;
}

fapi_try_exit:
FAPI_ERR("%s failed to get an MRW attribute, an egregious error. Returning NOT broadcast capable",
mss::c_str(i_target));
return mss::states::NO;
}

///
Expand Down
2 changes: 1 addition & 1 deletion src/import/chips/p9/procedures/hwp/memory/lib/mcbist/mcbist.H
View file
Expand Up @@ -2974,7 +2974,7 @@ fapi2::ReturnCode load_data_compare_mask( const fapi2::Target<T>& i_target,
const mcbist::program<T>& i_program );

///
/// @brief Load MCBIST Thre load_addr_gen
/// @brief Load MCBIST Thresholds
/// @tparam T, the fapi2::TargetType - derived
/// @tparam TT, the mcbistTraits associated with T - derived
/// @param[in] i_target the target to effect
Expand Down
16 changes: 16 additions & 0 deletions src/import/chips/p9/procedures/hwp/memory/lib/mcbist/memdiags.C
View file
Expand Up @@ -382,6 +382,22 @@ fapi2::ReturnCode operation<TARGET_TYPE_MCBIST>::multi_port_init()
l_dimms = mss::find_targets<fapi2::TARGET_TYPE_DIMM>(l_mcas[0]);

FAPI_INF("%s Updating l_dimms size to be %lu", mss::c_str(iv_target), l_dimms.size());

// Bomb out if we have incorrect addresses
// The following asssert catches the error earlier and keeps the error meaningful
{
// Note: we are guaranteed to have at least one DIMM, as we are not broadcast capable without DIMM's
// The ports are also required to have the same number and type of DIMM's to be broadcast capable
// As such, we can be guaranteed that we have at least one DIMM below
const uint64_t l_portdimm_this_dimm = mss::relative_pos<TARGET_TYPE_MCBIST>(l_dimms[0]);
FAPI_ASSERT(l_portdimm_this_dimm == l_portdimm_start_address,
fapi2::MSS_MEMDIAGS_BCMODE_INVALID_ADDRESS()
.set_MCA_TARGET(l_mcas[0])
.set_START_ADDRESS(l_portdimm_start_address)
.set_MCA_START_ADDRESS(l_portdimm_this_dimm)
.set_PATTERN(iv_const.iv_pattern),
"%s was passed an invalid address", mss::c_str(l_mcas[0]));
}
}

configure_multiport_subtests(l_dimms);
Expand Down
20 changes: 20 additions & 0 deletions src/import/chips/p9/procedures/hwp/memory/lib/mss_attribute_accessors.H
View file
Expand Up @@ -21341,6 +21341,26 @@ fapi_try_exit:
return fapi2::current_err;
}

///
/// @brief ATTR_MSS_MRW_MEMDIAGS_BCMODE getter
/// @param[out] uint8_t& reference to store the value
/// @note Generated by gen_accessors.pl generateParameters (SYSTEM)
/// @return fapi2::ReturnCode - FAPI2_RC_SUCCESS iff get is OK
/// @note A switch for memdiags broadcast
/// mode
///
inline fapi2::ReturnCode mrw_memdiags_bcmode(uint8_t& o_value)
{

FAPI_TRY( FAPI_ATTR_GET(fapi2::ATTR_MSS_MRW_MEMDIAGS_BCMODE, fapi2::Target<fapi2::TARGET_TYPE_SYSTEM>(), o_value) );
return fapi2::current_err;

fapi_try_exit:
FAPI_ERR("failed accessing ATTR_MSS_MRW_MEMDIAGS_BCMODE: 0x%lx (system target)",
uint64_t(fapi2::current_err));
return fapi2::current_err;
}


///
/// @brief ATTR_MSS_VPD_MR_0_VERSION_LAYOUT getter
Expand Down
1 change: 1 addition & 0 deletions src/import/chips/p9/procedures/hwp/memory/lib/workarounds/mcbist_workarounds.C
View file
Expand Up @@ -138,6 +138,7 @@ fapi2::ReturnCode end_of_rank( const fapi2::Target<TARGET_TYPE_MCBIST>& i_target

// If we're here, we need to fix up our program. We need to set our stop to stop immediate, which implies
// we don't do broadcasts and we can't do read, we have to do display.
FAPI_INF("%s replacing reads and changing broadcast mode due to a chip bug", mss::c_str(i_target));
replace_read_helper(io_program);

return fapi2::FAPI2_RC_SUCCESS;
Expand Down
13 changes: 13 additions & 0 deletions src/import/chips/p9/procedures/xml/attribute_info/memory_mrw_attributes.xml
View file
Expand Up @@ -616,4 +616,17 @@
<default> 0 </default>
<mssAccessorName>mrw_temp_refresh_mode</mssAccessorName>
</attribute>

<attribute>
<id>ATTR_MSS_MRW_MEMDIAGS_BCMODE</id>
<targetType>TARGET_TYPE_SYSTEM</targetType>
<description>
A switch for memdiags broadcast mode
</description>
<valueType>uint8</valueType>
<platInit/>
<initToZero/>
<enum> ENABLE = 0, DISABLE = 1 </enum>
<mssAccessorName>mrw_memdiags_bcmode</mssAccessorName>
</attribute>
</attributes>
13 changes: 13 additions & 0 deletions src/import/chips/p9/procedures/xml/error_info/p9_memory_mss_memdiags.xml
View file
Expand Up @@ -371,5 +371,18 @@
</callout>
</hwpError>

<hwpError>
<rc>RC_MSS_MEMDIAGS_BCMODE_INVALID_ADDRESS</rc>
<description>An limited address scope was passed into memdiags that is not on the first port in broadcast mode</description>
<ffdc>MCA_TARGET</ffdc>
<ffdc>START_ADDRESS</ffdc>
<ffdc>MCA_START_ADDRESS</ffdc>
<ffdc>PATTERN</ffdc>
<callout>
<procedure>CODE</procedure>
<priority>HIGH</priority>
</callout>
</hwpError>

</hwpErrors>

6 changes: 6 additions & 0 deletions src/usr/targeting/common/xmltohb/hb_customized_attrs.xml
View file
Expand Up @@ -659,6 +659,12 @@
<default>0x2</default>
<persistency>volatile-zeroed</persistency>
</attribute>

<attribute>
<id>ATTR_MSS_MRW_MEMDIAGS_BCMODE</id>
<default>0x1</default>
<no_export/>
</attribute>
<!-- =====================================================================
End of customizations definitions
================================================================= -->
Expand Down

0 comments on commit 1cf8ace

Please sign in to comment.