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 <fsp-CI-jenkins+hostboot@us.ibm.com>
Reviewed-by: STEPHEN GLANCY <sglancy@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: HWSV CI <hwsv-ci+hostboot@us.ibm.com>
Tested-by: Hostboot CI <hostboot-ci+hostboot@us.ibm.com>
Reviewed-by: JACOB L. HARVEY <jlharvey@us.ibm.com>
Reviewed-by: Louis Stermole <stermole@us.ibm.com>
Dev-Ready: Brent Wieman <bwieman@us.ibm.com>
Reviewed-by: Christian R. Geddes <crgeddes@us.ibm.com>
Reviewed-by: Jennifer A. Stofer <stofer@us.ibm.com>
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/46195
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Daniel M. Crowell <dcrowell@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>

3 files changed, 190 insertions, 71 deletions

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<TARGET_TYPE_MCS>& 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<uint8_t*> l_mt_blobs(PORTS_PER_MCS, nullptr);
@@ -4698,6 +4698,7 @@ fapi2::ReturnCode eff_dimm::decode_vpd(const fapi2::Target<TARGET_TYPE_MCS>& 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<fapi2::TARGET_TYPE_MCBIST>(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<TARGET_TYPE_MCS>& 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<TARGET_TYPE_MCS>& 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<TARGET_TYPE_MCS>& 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<TARGET_TYPE_MCS>& 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<TARGET_TYPE_MCS>& 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<TARGET_TYPE_MCS>& 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 <lib/shared/mss_const.H>
 #include <lib/mss_utils.H>
 #include <lib/mss_attribute_accessors.H>
+#include <generic/memory/lib/utils/find.H>
+#include <generic/memory/lib/utils/index.H>
 
 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<uint8_t*>& 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<fapi2::TARGET_TYPE_MCS>
 /// @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<fapi2::TARG
     constexpr uint64_t l_num_bytes_to_copy = 1;
     constexpr uint64_t l_offset = 0;
 
-    memcpy(&l_value, i_blobs[0] + l_offset, l_num_bytes_to_copy);
+    uint8_t* l_blob = nullptr;
+    select_mt_blob(i_blobs, &l_blob);
+    memcpy(&l_value, l_blob + l_offset, l_num_bytes_to_copy);
 
     FAPI_TRY( FAPI_ATTR_SET(fapi2::ATTR_MSS_VPD_MT_0_VERSION_LAYOUT, i_target, l_value),
               "Unable to decode and set ATTR_MSS_VPD_MT_0_VERSION_LAYOUT start: %d, len: %d for %s", l_offset, l_num_bytes_to_copy,
@@ -76,7 +133,9 @@ inline fapi2::ReturnCode vpd_mt_1_version_data(const fapi2::Target<fapi2::TARGET
     constexpr uint64_t l_num_bytes_to_copy = 1;
     constexpr uint64_t l_offset = 1;
 
-    memcpy(&l_value, i_blobs[0] + l_offset, l_num_bytes_to_copy);
+    uint8_t* l_blob = nullptr;
+    select_mt_blob(i_blobs, &l_blob);
+    memcpy(&l_value, l_blob + l_offset, l_num_bytes_to_copy);
 
     FAPI_TRY( FAPI_ATTR_SET(fapi2::ATTR_MSS_VPD_MT_1_VERSION_DATA, i_target, l_value),
               "Unable to decode and set ATTR_MSS_VPD_MT_1_VERSION_DATA start: %d, len: %d for %s", l_offset, l_num_bytes_to_copy,
@@ -100,7 +159,9 @@ inline fapi2::ReturnCode vpd_mt_2_signature_hash(const fapi2::Target<fapi2::TARG
     constexpr uint64_t l_num_bytes_to_copy = 4;
     constexpr uint64_t l_offset = 2;
 
-    memcpy(&l_value, i_blobs[0] + l_offset, l_num_bytes_to_copy);
+    uint8_t* l_blob = nullptr;
+    select_mt_blob(i_blobs, &l_blob);
+    memcpy(&l_value, l_blob + l_offset, l_num_bytes_to_copy);
     l_value = be32toh(l_value);
 
     FAPI_TRY( FAPI_ATTR_SET(fapi2::ATTR_MSS_VPD_MT_2_SIGNATURE_HASH, i_target, l_value),
@@ -133,8 +194,9 @@ inline fapi2::ReturnCode vpd_mt_dimm_rcd_ibt_ca(const fapi2::Target<fapi2::TARGE
 
     constexpr uint64_t l_num_bytes_to_copy = l_length / mss::PORTS_PER_MCS;
 
-    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+    for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARG
 
     constexpr uint64_t l_num_bytes_to_copy = l_length / mss::PORTS_PER_MCS;
 
-    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+    for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARGE
 
     constexpr uint64_t l_num_bytes_to_copy = l_length / mss::PORTS_PER_MCS;
 
-    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+    for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARG
 
     constexpr uint64_t l_num_bytes_to_copy = l_length / mss::PORTS_PER_MCS;
 
-    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+    for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::T
 
     constexpr uint64_t l_num_bytes_to_copy = l_length / mss::PORTS_PER_MCS;
 
-    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+    for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARGET_T
 
     constexpr uint64_t l_num_bytes_to_copy = l_length / mss::PORTS_PER_MCS;
 
-    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+    for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARGET_
 
     constexpr uint64_t l_num_bytes_to_copy = l_length / mss::PORTS_PER_MCS;
 
-    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+    for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARGET_TY
 
     constexpr uint64_t l_num_bytes_to_copy = l_length / mss::PORTS_PER_MCS;
 
-    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+    for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARGET_T
     uint8_t l_layer_version = 0;
     FAPI_TRY( mss::vpd_mt_0_version_layout(i_target, l_layer_version));
 
-    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+    for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::T
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2:
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::T
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARGET
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARGET
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARGET
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::T
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARGE
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARG
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TAR
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TAR
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARGET_TYPE_MC
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARGET_TYPE_MC
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARGET_TYPE_
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARGET_T
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARGET_TYP
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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<fapi2::TARGET
     {
         const uint64_t l_length = l_num_bytes_to_copy / mss::PORTS_PER_MCS;
 
-        for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
+        for (const auto& p : mss::find_targets<fapi2::TARGET_TYPE_MCA>(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 @@
       <priority>HIGH</priority>
     </callout>
   </hwpError>
+
+  <hwpError>
+    <rc>RC_MSS_INVALID_VPD_KEYWORD_MAX</rc>
+    <description>
+      VPD keyword is too big for space allocated for it.
+    </description>
+    <ffdc>MAX</ffdc>
+    <ffdc>ACTUAL</ffdc>
+    <ffdc>KEYWORD</ffdc>
+    <callout>
+      <hw>
+        <hwid>VPD_PART</hwid>
+        <refTarget>MCS_TARGET</refTarget>
+      </hw>
+      <priority>HIGH</priority>
+    </callout>
+  </hwpError>
+
 </hwpErrors>