From 84e9979022484372224d5b1a4ed44d7d2989bfe3 Mon Sep 17 00:00:00 2001 From: Andre Marin Date: Wed, 13 Sep 2017 22:01:52 -0500 Subject: Modify VPD decoder to take into account deconfigured ports Fixing lab observation where version layout attribute was not being set correctly when port 0 was deconfigured. Made decoder handle the case for deconfigured ports. Change-Id: Ib8f951bcd6c8e4f4945ada7dbf35e64d07cea194 Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/46194 Tested-by: FSP CI Jenkins Reviewed-by: STEPHEN GLANCY Tested-by: Jenkins Server Tested-by: HWSV CI Tested-by: Hostboot CI Reviewed-by: JACOB L. HARVEY Reviewed-by: Louis Stermole Dev-Ready: Brent Wieman Reviewed-by: Christian R. Geddes Reviewed-by: Jennifer A. Stofer Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/46195 Tested-by: Jenkins OP Build CI Tested-by: Daniel M. Crowell Reviewed-by: Daniel M. Crowell --- .../p9/procedures/hwp/memory/lib/dimm/eff_dimm.C | 98 ++++++++------ .../p9/procedures/hwp/memory/lib/mss_vpd_decoder.H | 145 ++++++++++++++++----- .../xml/error_info/p9_memory_mss_eff_config.xml | 18 +++ 3 files changed, 190 insertions(+), 71 deletions(-) (limited to 'src/import/chips/p9/procedures') diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/dimm/eff_dimm.C b/src/import/chips/p9/procedures/hwp/memory/lib/dimm/eff_dimm.C index f3b3c628d..1e5706714 100644 --- a/src/import/chips/p9/procedures/hwp/memory/lib/dimm/eff_dimm.C +++ b/src/import/chips/p9/procedures/hwp/memory/lib/dimm/eff_dimm.C @@ -4687,9 +4687,9 @@ fapi_try_exit: /// fapi2::ReturnCode eff_dimm::decode_vpd(const fapi2::Target& i_target) { - uint8_t l_mr_blob[mss::VPD_KEYWORD_MAX] = {0}; - uint8_t l_cke_blob[mss::VPD_KEYWORD_MAX] = {0}; - uint8_t l_dq_blob[mss::VPD_KEYWORD_MAX] = {0}; + uint8_t l_mr_blob[mss::VPD_KEYWORD_MAX] = {}; + uint8_t l_cke_blob[mss::VPD_KEYWORD_MAX] = {}; + uint8_t l_dq_blob[mss::VPD_KEYWORD_MAX] = {}; uint64_t l_freq = 0; std::vector l_mt_blobs(PORTS_PER_MCS, nullptr); @@ -4698,6 +4698,7 @@ fapi2::ReturnCode eff_dimm::decode_vpd(const fapi2::Target& i_t // For sanity. Not sure this will break us, but we're certainly making assumptions below. static_assert(MAX_DIMM_PER_PORT == 2, "Max DIMM per port isn't 2"); FAPI_TRY( mss::freq(find_target(i_target), l_freq)); + // We need to set up all VPD info before calling getVPD, the API assumes this // For MR we need to tell the VPDInfo the frequency (err ... mt/s - why is this mhz?) l_vpd_info.iv_freq_mhz = l_freq; @@ -4722,12 +4723,13 @@ fapi2::ReturnCode eff_dimm::decode_vpd(const fapi2::Target& i_t { if (mss::count_dimm(p) == 0) { + FAPI_INF("No DIMMs found for %s... skipping", mss::c_str(i_target)); continue; } // Find our blob in the vector of blob pointers uint8_t* l_mt_blob = l_mt_blobs[mss::index(p)]; - uint64_t l_rank_count_dimm[MAX_DIMM_PER_PORT] = {0}; + uint64_t l_rank_count_dimm[MAX_DIMM_PER_PORT] = {}; // If we don't have any DIMM, don't worry about it. This will just drop the blob full of 0's into our index. // This will fill the VPD attributes with 0's which is perfectly ok. @@ -4739,6 +4741,7 @@ fapi2::ReturnCode eff_dimm::decode_vpd(const fapi2::Target& i_t } // This value will, of course, be 0 if there is no DIMM in the port. + // Which shouldn't happen w/the DIMM check above. l_vpd_info.iv_rank_count_dimm_0 = l_rank_count_dimm[0]; l_vpd_info.iv_rank_count_dimm_1 = l_rank_count_dimm[1]; @@ -4747,33 +4750,35 @@ fapi2::ReturnCode eff_dimm::decode_vpd(const fapi2::Target& i_t // Get the MCS blob for this specific rank combination *only if* we have DIMM. Remember, // Cronus can give us functional MCA which have no DIMM - and we'd puke getting the VPD. - if ((l_vpd_info.iv_rank_count_dimm_0 != 0) || (l_vpd_info.iv_rank_count_dimm_1 != 0)) - { - // If getVPD returns us an error, then we don't have VPD for the DIMM configuration. - // This is the root of our plug-rules: if you want a configuration of DIMM to be - // supported, it needs to have VPD defined. Likewise, if you don't want a configuration - // of DIMM supported be sure to leave it out of the VPD. Note that we don't return a specific - // plug-rule error as f/w (Dan) suggested this would duplicate errors leading to confusion. - l_vpd_info.iv_vpd_type = fapi2::MemVpdData::MT; - - // Check the max for giggles. Programming bug so we should assert. - FAPI_TRY( fapi2::getVPD(i_target, l_vpd_info, nullptr), - "Failed to retrieve MT size from VPD"); - - if (l_vpd_info.iv_size > mss::VPD_KEYWORD_MAX) - { - FAPI_ERR("VPD MT keyword is too big for our array"); - fapi2::Assert(false); - } - - // Log any error code from getVPD separately in case the error code is meaningful - FAPI_TRY( fapi2::getVPD(i_target, l_vpd_info, &(l_mt_blob[0])) ); - } + // Which again, shouldn't happen w/the DIMM check above. + + // If getVPD returns us an error, then we don't have VPD for the DIMM configuration. + // This is the root of our plug-rules: if you want a configuration of DIMM to be + // supported, it needs to have VPD defined. Likewise, if you don't want a configuration + // of DIMM supported be sure to leave it out of the VPD. Note that we don't return a specific + // plug-rule error as f/w (Dan) suggested this would duplicate errors leading to confusion. + l_vpd_info.iv_vpd_type = fapi2::MemVpdData::MT; + + // Check the max for giggles. Programming bug so we should assert. + FAPI_TRY( fapi2::getVPD(i_target, l_vpd_info, nullptr), + "Failed to retrieve MT size from VPD"); + + FAPI_ASSERT( l_vpd_info.iv_size <= mss::VPD_KEYWORD_MAX, + fapi2::MSS_INVALID_VPD_KEYWORD_MAX(). + set_MAX(mss::VPD_KEYWORD_MAX). + set_ACTUAL(l_vpd_info.iv_size). + set_KEYWORD(fapi2::MemVpdData::MT). + set_MCS_TARGET(i_target), + "VPD MT keyword size retrieved: %d, is larger than max: %d for %s", + l_vpd_info.iv_size, mss::VPD_KEYWORD_MAX, mss::c_str(i_target)); + + // Log any error code from getVPD separately in case the error code is meaningful + FAPI_TRY( fapi2::getVPD(i_target, l_vpd_info, &(l_mt_blob[0])) ); // Only get the MR blob if we have a freq. It's possible for Cronus to give us an MCS which // is connected to a controller which has 0 DIMM installed. In this case, we won't have // a frequency, and thus we'd fail getting the VPD. So we initiaized the VPD to 0's and if - // there's no freq, we us a 0 filled VPD. + // there's no freq, we use a 0 filled VPD. if (l_vpd_info.iv_freq_mhz != 0) { l_vpd_info.iv_vpd_type = fapi2::MemVpdData::MR; @@ -4782,11 +4787,14 @@ fapi2::ReturnCode eff_dimm::decode_vpd(const fapi2::Target& i_t FAPI_TRY( fapi2::getVPD(i_target, l_vpd_info, nullptr), "Failed to retrieve MR size from VPD"); - if (l_vpd_info.iv_size > mss::VPD_KEYWORD_MAX) - { - FAPI_ERR("VPD MR keyword is too big for our array"); - fapi2::Assert(false); - } + FAPI_ASSERT( l_vpd_info.iv_size <= mss::VPD_KEYWORD_MAX, + fapi2::MSS_INVALID_VPD_KEYWORD_MAX(). + set_MAX(mss::VPD_KEYWORD_MAX). + set_ACTUAL(l_vpd_info.iv_size). + set_KEYWORD(fapi2::MemVpdData::MR). + set_MCS_TARGET(i_target), + "VPD MR keyword size retrieved: %d, is larger than max: %d for %s", + l_vpd_info.iv_size, mss::VPD_KEYWORD_MAX, mss::c_str(i_target)); // Log any error code from getVPD separately in case the error code is meaningful FAPI_TRY( fapi2::getVPD(i_target, l_vpd_info, &(l_mr_blob[0])) ); @@ -4800,11 +4808,14 @@ fapi2::ReturnCode eff_dimm::decode_vpd(const fapi2::Target& i_t FAPI_TRY( fapi2::getVPD(i_target, l_vpd_info, nullptr), "Failed to retrieve CK size from VPD"); - if (l_vpd_info.iv_size > mss::VPD_KEYWORD_MAX) - { - FAPI_ERR("VPD CK keyword is too big for our array"); - fapi2::Assert(false); - } + FAPI_ASSERT( l_vpd_info.iv_size <= mss::VPD_KEYWORD_MAX, + fapi2::MSS_INVALID_VPD_KEYWORD_MAX(). + set_MAX(mss::VPD_KEYWORD_MAX). + set_ACTUAL(l_vpd_info.iv_size). + set_KEYWORD(fapi2::MemVpdData::CK). + set_MCS_TARGET(i_target), + "VPD CK keyword size retrieved: %d, is larger than max: %d for %s", + l_vpd_info.iv_size, mss::VPD_KEYWORD_MAX, mss::c_str(i_target)); FAPI_TRY( fapi2::getVPD(i_target, l_vpd_info, &(l_cke_blob[0])), "Failed to retrieve DQ VPD"); @@ -4816,11 +4827,14 @@ fapi2::ReturnCode eff_dimm::decode_vpd(const fapi2::Target& i_t FAPI_TRY( fapi2::getVPD(i_target, l_vpd_info, nullptr), "Failed to retrieve DQ size from VPD"); - if (l_vpd_info.iv_size > mss::VPD_KEYWORD_MAX) - { - FAPI_ERR("VPD DQ keyword is too big for our array"); - fapi2::Assert(false); - } + FAPI_ASSERT( l_vpd_info.iv_size <= mss::VPD_KEYWORD_MAX, + fapi2::MSS_INVALID_VPD_KEYWORD_MAX(). + set_MAX(mss::VPD_KEYWORD_MAX). + set_ACTUAL(l_vpd_info.iv_size). + set_KEYWORD(fapi2::MemVpdData::DQ). + set_MCS_TARGET(i_target), + "VPD DQ keyword size retrieved: %d, is larger than max: %d for %s", + l_vpd_info.iv_size, mss::VPD_KEYWORD_MAX, mss::c_str(i_target)); FAPI_TRY( fapi2::getVPD(i_target, l_vpd_info, &(l_dq_blob[0])), "Failed to retrieve DQ VPD"); diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/mss_vpd_decoder.H b/src/import/chips/p9/procedures/hwp/memory/lib/mss_vpd_decoder.H index f34eac317..01fe34174 100644 --- a/src/import/chips/p9/procedures/hwp/memory/lib/mss_vpd_decoder.H +++ b/src/import/chips/p9/procedures/hwp/memory/lib/mss_vpd_decoder.H @@ -33,12 +33,67 @@ #include #include #include +#include +#include namespace mss { namespace decoder { /// +/// @brief Helper function to select a valid MT blob +/// @param[in] i_blobs a std::vector of pointers to VPD blobs for this MCS's MCAs +/// @param[out] o_blob returns a valid blob of data for the MCS for MT decoding +/// +inline void select_mt_blob(const std::vector& i_blobs, uint8_t** o_blob) +{ + constexpr uint64_t EQUAL_MEM_CONTENTS = 0; + + if( i_blobs.size() != mss::PORTS_PER_MCS ) + { + // Asserting out instead of collecting FFDC since this is a + // programming bug that shouldn't occur. + FAPI_ERR("Invalid blob size received %d, expected: %d", + i_blobs.size(), mss::PORTS_PER_MCS); + + fapi2::Assert(false); + } + + // We are looking for cases where one of the blobs is filled with zeros, + // in this case we start with blob index 0, usually due to a deconfigured port. + // We want to select the non-zero blob so that our VPD "header" information is + // correct for version layout, signature hash, and version data. + constexpr uint8_t l_zero_array[mss::VPD_KEYWORD_MAX] = {}; + + if( memcmp(i_blobs[0], l_zero_array, mss::VPD_KEYWORD_MAX) == EQUAL_MEM_CONTENTS ) + { + *o_blob = i_blobs[1]; + return; + } + + // If we are here than there are three possible reasons: + // 1) + // i_blobs[0] and i_blobs[1] are equivalent and + // the array contents of each index are 0, in which will give the VPD contents + // 0 values...which is OK. Or that the DIMMs are the same (they have the same + // rank config per port). + // + // 2) + // i_blobs[0] and i_blobs[1] are NOT equivalent but neither has a 0 filled blob + // and that they have rank config that differs per port. But to set our version + // layout, signature hash, or version data correctly, any blob will do since + // this data doesn't change per port. + // + // For the two cases above we just select to return an arbitrary blob, + // in which i_blobs[0] will suffice. + // + // 3) + // i_blob[1] is filled with zeros so we end up selecting i_blob[0] which will + // be our non-zero filled VPD blob. + *o_blob = i_blobs[0]; + return; +} +/// /// @brief ATTR_MSS_VPD_MT_0_VERSION_LAYOUT decode and set /// @param[in] i_target fapi2::Target /// @param[in] i_blobs a std::vector of pointers to VPD blobs for this MCS's MCAs @@ -52,7 +107,9 @@ inline fapi2::ReturnCode vpd_mt_0_version_layout(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_offset = l_start + (l_index * l_num_bytes_to_copy); @@ -171,8 +233,9 @@ inline fapi2::ReturnCode vpd_mt_dimm_rcd_ibt_cke(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_offset = l_start + (l_index * l_num_bytes_to_copy); @@ -209,8 +272,9 @@ inline fapi2::ReturnCode vpd_mt_dimm_rcd_ibt_cs(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_offset = l_start + (l_index * l_num_bytes_to_copy); @@ -247,8 +311,9 @@ inline fapi2::ReturnCode vpd_mt_dimm_rcd_ibt_odt(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_offset = l_start + (l_index * l_num_bytes_to_copy); @@ -285,8 +350,9 @@ inline fapi2::ReturnCode vpd_mt_dram_drv_imp_dq_dqs(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_offset = l_start + (l_index * l_num_bytes_to_copy); @@ -323,8 +389,9 @@ inline fapi2::ReturnCode vpd_mt_dram_rtt_nom(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_offset = l_start + (l_index * l_num_bytes_to_copy); @@ -361,8 +428,9 @@ inline fapi2::ReturnCode vpd_mt_dram_rtt_park(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_offset = l_start + (l_index * l_num_bytes_to_copy); @@ -399,8 +467,9 @@ inline fapi2::ReturnCode vpd_mt_dram_rtt_wr(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_offset = l_start + (l_index * l_num_bytes_to_copy); @@ -446,8 +515,9 @@ inline fapi2::ReturnCode vpd_mt_mc_bias_trim(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); constexpr uint64_t l_offset = 86; constexpr uint64_t l_num_bytes_to_copy = 2; constexpr uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS; @@ -528,8 +598,9 @@ inline fapi2::ReturnCode vpd_mt_mc_dq_acboost_rd_up(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index]), l_blob + l_start, l_length); @@ -605,8 +676,9 @@ inline fapi2::ReturnCode vpd_mt_mc_dq_acboost_wr_down(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index]), l_blob + l_start, l_length); @@ -682,8 +754,9 @@ inline fapi2::ReturnCode vpd_mt_mc_dq_acboost_wr_up(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index]), l_blob + l_start, l_length); @@ -757,8 +830,9 @@ inline fapi2::ReturnCode vpd_mt_mc_dq_ctle_cap(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index]), l_blob + l_start, l_length); @@ -832,8 +906,9 @@ inline fapi2::ReturnCode vpd_mt_mc_dq_ctle_res(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index]), l_blob + l_start, l_length); @@ -899,8 +974,9 @@ inline fapi2::ReturnCode vpd_mt_mc_drv_imp_clk(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index]), l_blob + l_start, l_length); @@ -961,8 +1037,9 @@ inline fapi2::ReturnCode vpd_mt_mc_drv_imp_cmd_addr(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index]), l_blob + l_start, l_length); @@ -1023,8 +1100,9 @@ inline fapi2::ReturnCode vpd_mt_mc_drv_imp_cntl(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index]), l_blob + l_start, l_length); @@ -1085,8 +1163,9 @@ inline fapi2::ReturnCode vpd_mt_mc_drv_imp_cscid(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index]), l_blob + l_start, l_length); @@ -1147,8 +1226,9 @@ inline fapi2::ReturnCode vpd_mt_mc_drv_imp_dq_dqs(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index][0]), l_blob + l_start, l_length); @@ -1209,8 +1289,9 @@ inline fapi2::ReturnCode vpd_mt_mc_rcv_imp_dq_dqs(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index][0]), l_blob + l_start, l_length); @@ -1272,8 +1353,9 @@ inline fapi2::ReturnCode vpd_mt_odt_rd(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index][0][0]), l_blob + l_start, l_length); @@ -1335,8 +1417,9 @@ inline fapi2::ReturnCode vpd_mt_odt_wr(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index][0][0]), l_blob + l_start, l_length); @@ -1399,8 +1482,9 @@ inline fapi2::ReturnCode vpd_mt_preamble(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index]), l_blob + l_start, l_length); @@ -1461,8 +1545,9 @@ inline fapi2::ReturnCode vpd_mt_vref_dram_wr(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index]), l_blob + l_start, l_length); @@ -1523,8 +1608,9 @@ inline fapi2::ReturnCode vpd_mt_vref_mc_rd(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index]), l_blob + l_start, l_length); @@ -1591,8 +1677,9 @@ inline fapi2::ReturnCode vpd_mt_windage_rd_ctr(const fapi2::Target(i_target)) { + const auto l_index = mss::index(p); const uint8_t* l_blob = i_blobs[l_index]; const uint64_t l_start = l_offset + (l_index * l_length); memcpy(&(l_value[l_index]), l_blob + l_start, l_length); diff --git a/src/import/chips/p9/procedures/xml/error_info/p9_memory_mss_eff_config.xml b/src/import/chips/p9/procedures/xml/error_info/p9_memory_mss_eff_config.xml index 049a4cc24..7c45b1d8a 100644 --- a/src/import/chips/p9/procedures/xml/error_info/p9_memory_mss_eff_config.xml +++ b/src/import/chips/p9/procedures/xml/error_info/p9_memory_mss_eff_config.xml @@ -502,4 +502,22 @@ HIGH + + + RC_MSS_INVALID_VPD_KEYWORD_MAX + + VPD keyword is too big for space allocated for it. + + MAX + ACTUAL + KEYWORD + + + VPD_PART + MCS_TARGET + + HIGH + + + -- cgit v1.2.1