From 1cf8acef718aeb4901878a02cab41859727757bc Mon Sep 17 00:00:00 2001 From: Stephen Glancy Date: Thu, 26 Oct 2017 09:18:53 -0500 Subject: Fixes broadcast mode memdiags crash 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 Tested-by: Jenkins Server Tested-by: Hostboot CI Tested-by: FSP CI Jenkins Reviewed-by: Louis Stermole Reviewed-by: ANDRE A. MARIN Dev-Ready: STEPHEN GLANCY Reviewed-by: Daniel M. Crowell Reviewed-by: Jennifer A. Stofer Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/48879 Tested-by: Jenkins OP Build CI Tested-by: Jenkins OP HW Reviewed-by: Christian R. Geddes --- .../p9/procedures/hwp/memory/lib/mcbist/mcbist.C | 60 ++++++++++++++++------ .../p9/procedures/hwp/memory/lib/mcbist/mcbist.H | 2 +- .../p9/procedures/hwp/memory/lib/mcbist/memdiags.C | 16 ++++++ .../hwp/memory/lib/mss_attribute_accessors.H | 20 ++++++++ .../memory/lib/workarounds/mcbist_workarounds.C | 1 + .../xml/attribute_info/memory_mrw_attributes.xml | 13 +++++ .../xml/error_info/p9_memory_mss_memdiags.xml | 13 +++++ .../common/xmltohb/hb_customized_attrs.xml | 6 +++ 8 files changed, 114 insertions(+), 17 deletions(-) (limited to 'src') diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/mcbist/mcbist.C b/src/import/chips/p9/procedures/hwp/memory/lib/mcbist/mcbist.C index 1569d31b1..267b69573 100644 --- a/src/import/chips/p9/procedures/hwp/memory/lib/mcbist/mcbist.C +++ b/src/import/chips/p9/procedures/hwp/memory/lib/mcbist/mcbist.C @@ -79,7 +79,8 @@ namespace mcbist /// @return FAPI2_RC_SUCCSS iff ok /// template< > -fapi2::ReturnCode load_maint_pattern( const fapi2::Target& i_target, const pattern& i_pattern, +fapi2::ReturnCode load_maint_pattern( const fapi2::Target& i_target, + const pattern& i_pattern, const bool i_invert ) { // Init the fapi2 return code @@ -733,25 +734,52 @@ const mss::states is_broadcast_capable(const std::vector const mss::states is_broadcast_capable(const fapi2::Target& 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(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(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(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(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; } /// diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/mcbist/mcbist.H b/src/import/chips/p9/procedures/hwp/memory/lib/mcbist/mcbist.H index ac432c09d..146d9d2ca 100644 --- a/src/import/chips/p9/procedures/hwp/memory/lib/mcbist/mcbist.H +++ b/src/import/chips/p9/procedures/hwp/memory/lib/mcbist/mcbist.H @@ -2974,7 +2974,7 @@ fapi2::ReturnCode load_data_compare_mask( const fapi2::Target& i_target, const mcbist::program& 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 diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/mcbist/memdiags.C b/src/import/chips/p9/procedures/hwp/memory/lib/mcbist/memdiags.C index 7899f9105..91a2cac7f 100644 --- a/src/import/chips/p9/procedures/hwp/memory/lib/mcbist/memdiags.C +++ b/src/import/chips/p9/procedures/hwp/memory/lib/mcbist/memdiags.C @@ -382,6 +382,22 @@ fapi2::ReturnCode operation::multi_port_init() l_dimms = mss::find_targets(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(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); diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/mss_attribute_accessors.H b/src/import/chips/p9/procedures/hwp/memory/lib/mss_attribute_accessors.H index 8c0b79105..92686bc9b 100644 --- a/src/import/chips/p9/procedures/hwp/memory/lib/mss_attribute_accessors.H +++ b/src/import/chips/p9/procedures/hwp/memory/lib/mss_attribute_accessors.H @@ -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(), 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 diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/workarounds/mcbist_workarounds.C b/src/import/chips/p9/procedures/hwp/memory/lib/workarounds/mcbist_workarounds.C index eac039a06..de06e36a4 100644 --- a/src/import/chips/p9/procedures/hwp/memory/lib/workarounds/mcbist_workarounds.C +++ b/src/import/chips/p9/procedures/hwp/memory/lib/workarounds/mcbist_workarounds.C @@ -138,6 +138,7 @@ fapi2::ReturnCode end_of_rank( const fapi2::Target& 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; diff --git a/src/import/chips/p9/procedures/xml/attribute_info/memory_mrw_attributes.xml b/src/import/chips/p9/procedures/xml/attribute_info/memory_mrw_attributes.xml index 2938df196..8c7e0e2ad 100755 --- a/src/import/chips/p9/procedures/xml/attribute_info/memory_mrw_attributes.xml +++ b/src/import/chips/p9/procedures/xml/attribute_info/memory_mrw_attributes.xml @@ -616,4 +616,17 @@ 0 mrw_temp_refresh_mode + + + ATTR_MSS_MRW_MEMDIAGS_BCMODE + TARGET_TYPE_SYSTEM + + A switch for memdiags broadcast mode + + uint8 + + + ENABLE = 0, DISABLE = 1 + mrw_memdiags_bcmode + diff --git a/src/import/chips/p9/procedures/xml/error_info/p9_memory_mss_memdiags.xml b/src/import/chips/p9/procedures/xml/error_info/p9_memory_mss_memdiags.xml index 5b3dc2cf7..b6e88ed5b 100644 --- a/src/import/chips/p9/procedures/xml/error_info/p9_memory_mss_memdiags.xml +++ b/src/import/chips/p9/procedures/xml/error_info/p9_memory_mss_memdiags.xml @@ -371,5 +371,18 @@ + + RC_MSS_MEMDIAGS_BCMODE_INVALID_ADDRESS + An limited address scope was passed into memdiags that is not on the first port in broadcast mode + MCA_TARGET + START_ADDRESS + MCA_START_ADDRESS + PATTERN + + CODE + HIGH + + + diff --git a/src/usr/targeting/common/xmltohb/hb_customized_attrs.xml b/src/usr/targeting/common/xmltohb/hb_customized_attrs.xml index 830512147..ca8fdddbd 100644 --- a/src/usr/targeting/common/xmltohb/hb_customized_attrs.xml +++ b/src/usr/targeting/common/xmltohb/hb_customized_attrs.xml @@ -659,6 +659,12 @@ 0x2 volatile-zeroed + + + ATTR_MSS_MRW_MEMDIAGS_BCMODE + 0x1 + + -- cgit v1.2.1