Change VPD to better account for deconfigured chiplets

  Update VPD blob creation for deconfiged chiplets
  Update decoder to not skip VPD for mising chiplets
  Correct actions for BB lock

Change-Id: Id84e485f803d3ee033af61c1b313b19a03dbf38f
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/29697
Tested-by: Jenkins Server <pfd-jenkins+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: Christian R. Geddes <crgeddes@us.ibm.com>
Reviewed-by: STEPHEN GLANCY <sglancy@us.ibm.com>
Reviewed-by: William G. Hoffa <wghoffa@us.ibm.com>
Reviewed-by: Jennifer A. Stofer <stofer@us.ibm.com>
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/29700
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>

7 files changed, 280 insertions, 151 deletions

diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/eff_config/eff_config.C b/src/import/chips/p9/procedures/hwp/memory/lib/eff_config/eff_config.C
index 74b6c39d7..f1d937748 100644
--- a/src/import/chips/p9/procedures/hwp/memory/lib/eff_config/eff_config.C
+++ b/src/import/chips/p9/procedures/hwp/memory/lib/eff_config/eff_config.C
@@ -3526,6 +3526,16 @@ fapi2::ReturnCode eff_config::decode_vpd(const fapi2::Target<TARGET_TYPE_MCS>& i
             fapi2::Assert(false);
         }
 
+        // Make sure to create 0 filled blobs for all the possible blobs, not just for the
+        // chiplets which are configured. This prevents the decoder from accessing nullptrs
+        // but the code which uses the VPD will only access the information for the chiplets
+        // which exist - so the 0's are meaningless
+        for (auto& b : l_mt_blobs)
+        {
+            b = new uint8_t[mss::VPD_KEYWORD_MAX];
+            memset(b, 0, mss::VPD_KEYWORD_MAX);
+        }
+
         // For MT we need to fill in the rank information
         // But, of course, the rank information can differ per port. However, the vpd interface doesn't
         // allow this in a straight-forward way. So, we have to get VPD blobs for MCS which contain
@@ -3533,14 +3543,10 @@ fapi2::ReturnCode eff_config::decode_vpd(const fapi2::Target<TARGET_TYPE_MCS>& i
         // blob to the decoder for each MCA, regardless of whether the port configurations are the same.
         for (const auto& p : find_targets<TARGET_TYPE_MCA>(i_target))
         {
-            uint8_t* l_mt_blob = new uint8_t[mss::VPD_KEYWORD_MAX];
+            // 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};
 
-            // Make sure the mt blob is all 0's. In the event that we get an MCA who has 0 ranks (no DIMM)
-            // we don't want to get the VPD (that will fail) but we can't give up. Cronus will
-            // give us MCA with 0 DIMM, so we'll just use a 0-filled VPD for those MCA.
-            memset(l_mt_blob, 0, mss::VPD_KEYWORD_MAX);
-
             // 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.
             for (const auto& d : mss::find_targets<TARGET_TYPE_DIMM>(p))
@@ -3565,9 +3571,6 @@ fapi2::ReturnCode eff_config::decode_vpd(const fapi2::Target<TARGET_TYPE_MCS>& i
                 // plug-rule error as f/w (Dan) suggested this would duplicate errors leading to confusion.
                 FAPI_TRY( mss::getVPD(i_target, l_vpd_info, &(l_mt_blob[0])) );
             }
-
-            // Put it in the vector in the right place.
-            l_mt_blobs[mss::index(p)] = l_mt_blob;
         }
     }
 
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 27de1c1f2..1c693bd36 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
@@ -219,6 +219,12 @@ fapi2::ReturnCode execute( const fapi2::Target<TARGET_TYPE_MCBIST>& i_target,
     bool l_poll_result = false;
     poll_parameters l_poll_parameters;
 
+    // Before we go off into the bushes, lets see if there are any instructions in the
+    // program. If not, we can save everyone the hassle
+    FAPI_ASSERT(0 != i_program.iv_subtests.size(),
+                fapi2::MSS_MEMDIAGS_NO_MCBIST_SUBTESTS().set_TARGET(i_target),
+                "Attempt to run an MCBIST program with no subtests on %s", mss::c_str(i_target));
+
     FAPI_TRY( clear_errors(i_target) );
 
     // Slam the address generator config
diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/mcbist/sim.C b/src/import/chips/p9/procedures/hwp/memory/lib/mcbist/sim.C
index 50d79b40b..cd92114f3 100644
--- a/src/import/chips/p9/procedures/hwp/memory/lib/mcbist/sim.C
+++ b/src/import/chips/p9/procedures/hwp/memory/lib/mcbist/sim.C
@@ -40,6 +40,7 @@
 #include <lib/mcbist/mcbist.H>
 #include <lib/mcbist/patterns.H>
 #include <lib/mcbist/sim.H>
+#include <lib/utils/count_dimm.H>
 
 using fapi2::TARGET_TYPE_MCBIST;
 using fapi2::TARGET_TYPE_MCA;
@@ -79,6 +80,13 @@ fapi2::ReturnCode sf_init( const fapi2::Target<TARGET_TYPE_MCBIST>& i_target,
 
         size_t l_rank_address_pair = 0;
 
+        // No point in bothering if we don't have any DIMM
+        if (mss::count_dimm(p) == 0)
+        {
+            FAPI_INF("No DIMM on %s, not running sf_init", mss::c_str(p));
+            continue;
+        }
+
         // In sim we know a few things ...
         // Get the primary ranks for this port. We know there can only be 4, and we know we only trained the primary
         // ranks. Therefore, we only need to clean up the primary ranks. And because there's 4 max, we can do it
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 4ed05a8fe..62ab40dc5 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
@@ -26,10 +26,13 @@
 #ifndef MSS_VPD_ACCESS_H_
 #define MSS_VPD_ACCESS_H_
 
+#include <endian.h>
+
 #include <fapi2.H>
-#include <lib/mss_utils.H>
+
 #include <mss.H>
-#include <endian.h>
+#include <lib/shared/mss_const.H>
+#include <lib/mss_utils.H>
 
 namespace mss
 {
@@ -108,13 +111,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_dimm_rcd_ibt_ca(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2][2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 4 / l_mcas.size();
+    constexpr uint64_t l_length = 4 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 6 + (l_index * l_length);
         memcpy(&(l_value[l_index][0]), l_blob + l_start, l_length);
@@ -137,13 +144,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_dimm_rcd_ibt_cke(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2][2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 4 / l_mcas.size();
+    constexpr uint64_t l_length = 4 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 10 + (l_index * l_length);
         memcpy(&(l_value[l_index][0]), l_blob + l_start, l_length);
@@ -166,13 +177,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_dimm_rcd_ibt_cs(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2][2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 4 / l_mcas.size();
+    constexpr uint64_t l_length = 4 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 14 + (l_index * l_length);
         memcpy(&(l_value[l_index][0]), l_blob + l_start, l_length);
@@ -195,13 +210,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_dimm_rcd_ibt_odt(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2][2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 4 / l_mcas.size();
+    constexpr uint64_t l_length = 4 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 18 + (l_index * l_length);
         memcpy(&(l_value[l_index][0]), l_blob + l_start, l_length);
@@ -224,13 +243,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_dram_drv_imp_dq_dqs(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2][2][4];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 16 / l_mcas.size();
+    constexpr uint64_t l_length = 16 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 22 + (l_index * l_length);
         memcpy(&(l_value[l_index][0][0]), l_blob + l_start, l_length);
@@ -253,13 +276,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_dram_rtt_nom(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2][2][4];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 16 / l_mcas.size();
+    constexpr uint64_t l_length = 16 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 38 + (l_index * l_length);
         memcpy(&(l_value[l_index][0][0]), l_blob + l_start, l_length);
@@ -282,13 +309,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_dram_rtt_park(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2][2][4];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 16 / l_mcas.size();
+    constexpr uint64_t l_length = 16 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 54 + (l_index * l_length);
         memcpy(&(l_value[l_index][0][0]), l_blob + l_start, l_length);
@@ -311,13 +342,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_dram_rtt_wr(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2][2][4];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 16 / l_mcas.size();
+    constexpr uint64_t l_length = 16 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 70 + (l_index * l_length);
         memcpy(&(l_value[l_index][0][0]), l_blob + l_start, l_length);
@@ -350,13 +385,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_mc_dq_acboost_rd_up(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint32_t l_value[2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 8 / l_mcas.size();
+    constexpr uint64_t l_length = 8 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 86 + (l_index * l_length);
         memcpy(&(l_value[l_index]), l_blob + l_start, l_length);
@@ -394,13 +433,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_mc_dq_acboost_wr_down(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint32_t l_value[2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 8 / l_mcas.size();
+    constexpr uint64_t l_length = 8 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 94 + (l_index * l_length);
         memcpy(&(l_value[l_index]), l_blob + l_start, l_length);
@@ -438,13 +481,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_mc_dq_acboost_wr_up(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint32_t l_value[2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 8 / l_mcas.size();
+    constexpr uint64_t l_length = 8 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 102 + (l_index * l_length);
         memcpy(&(l_value[l_index]), l_blob + l_start, l_length);
@@ -480,13 +527,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_mc_dq_ctle_cap(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint64_t l_value[2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 16 / l_mcas.size();
+    constexpr uint64_t l_length = 16 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 110 + (l_index * l_length);
         memcpy(&(l_value[l_index]), l_blob + l_start, l_length);
@@ -522,13 +573,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_mc_dq_ctle_res(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint64_t l_value[2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 16 / l_mcas.size();
+    constexpr uint64_t l_length = 16 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 126 + (l_index * l_length);
         memcpy(&(l_value[l_index]), l_blob + l_start, l_length);
@@ -556,13 +611,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_mc_drv_imp_clk(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 2 / l_mcas.size();
+    constexpr uint64_t l_length = 2 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 142 + (l_index * l_length);
         memcpy(&(l_value[l_index]), l_blob + l_start, l_length);
@@ -585,13 +644,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_mc_drv_imp_cmd_addr(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 2 / l_mcas.size();
+    constexpr uint64_t l_length = 2 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 144 + (l_index * l_length);
         memcpy(&(l_value[l_index]), l_blob + l_start, l_length);
@@ -614,13 +677,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_mc_drv_imp_cntl(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 2 / l_mcas.size();
+    constexpr uint64_t l_length = 2 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 146 + (l_index * l_length);
         memcpy(&(l_value[l_index]), l_blob + l_start, l_length);
@@ -643,13 +710,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_mc_drv_imp_cscid(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 2 / l_mcas.size();
+    constexpr uint64_t l_length = 2 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 148 + (l_index * l_length);
         memcpy(&(l_value[l_index]), l_blob + l_start, l_length);
@@ -672,13 +743,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_mc_drv_imp_dq_dqs(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2][5];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 10 / l_mcas.size();
+    constexpr uint64_t l_length = 10 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 150 + (l_index * l_length);
         memcpy(&(l_value[l_index][0]), l_blob + l_start, l_length);
@@ -701,13 +776,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_mc_rcv_imp_dq_dqs(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2][5];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 10 / l_mcas.size();
+    constexpr uint64_t l_length = 10 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 160 + (l_index * l_length);
         memcpy(&(l_value[l_index][0]), l_blob + l_start, l_length);
@@ -730,13 +809,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_odt_rd(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
                                        const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2][2][4];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 16 / l_mcas.size();
+    constexpr uint64_t l_length = 16 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 170 + (l_index * l_length);
         memcpy(&(l_value[l_index][0][0]), l_blob + l_start, l_length);
@@ -759,13 +842,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_odt_wr(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
                                        const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2][2][4];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 16 / l_mcas.size();
+    constexpr uint64_t l_length = 16 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 186 + (l_index * l_length);
         memcpy(&(l_value[l_index][0][0]), l_blob + l_start, l_length);
@@ -789,13 +876,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_preamble(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 2 / l_mcas.size();
+    constexpr uint64_t l_length = 2 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 202 + (l_index * l_length);
         memcpy(&(l_value[l_index]), l_blob + l_start, l_length);
@@ -818,13 +909,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_vref_dram_wr(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint8_t l_value[2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 2 / l_mcas.size();
+    constexpr uint64_t l_length = 2 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 204 + (l_index * l_length);
         memcpy(&(l_value[l_index]), l_blob + l_start, l_length);
@@ -847,13 +942,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_vref_mc_rd(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     uint32_t l_value[2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 8 / l_mcas.size();
+    constexpr uint64_t l_length = 8 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 206 + (l_index * l_length);
         memcpy(&(l_value[l_index]), l_blob + l_start, l_length);
@@ -881,13 +980,17 @@ fapi_try_exit:
 inline fapi2::ReturnCode vpd_mt_windage_rd_ctr(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         const std::vector<uint8_t*>& i_blobs)
 {
+    if (i_blobs.size() != mss::PORTS_PER_MCS)
+    {
+        FAPI_ERR("%s found vpd blob vector with incorrect number of elements %d", mss::c_str(i_target), i_blobs.size());
+        fapi2::Assert(false);
+    }
+
     int16_t l_value[2];
-    const auto& l_mcas = mss::find_targets<fapi2::TARGET_TYPE_MCA>(i_target);
-    const uint64_t l_length = 4 / l_mcas.size();
+    constexpr uint64_t l_length = 4 / mss::PORTS_PER_MCS;
 
-    for (const auto& l_mca : l_mcas)
+    for (size_t l_index = 0; l_index < mss::PORTS_PER_MCS; ++l_index)
     {
-        const uint64_t l_index = mss::index(l_mca);
         const uint8_t* l_blob = i_blobs[l_index];
         const uint64_t l_start = 214 + (l_index * l_length);
         memcpy(&(l_value[l_index]), l_blob + l_start, l_length);
@@ -2097,4 +2200,4 @@ fapi_try_exit:
 
 
 } // close ns mss
-#endif
\ No newline at end of file
+#endif
diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/phy/ddr_phy.C b/src/import/chips/p9/procedures/hwp/memory/lib/phy/ddr_phy.C
index 32900b643..47956d355 100644
--- a/src/import/chips/p9/procedures/hwp/memory/lib/phy/ddr_phy.C
+++ b/src/import/chips/p9/procedures/hwp/memory/lib/phy/ddr_phy.C
@@ -52,6 +52,7 @@
 #include <lib/utils/find.H>
 #include <lib/utils/dump_regs.H>
 #include <lib/utils/scom.H>
+#include <lib/utils/count_dimm.H>
 
 using fapi2::TARGET_TYPE_MCBIST;
 using fapi2::TARGET_TYPE_PROC_CHIP;
@@ -587,6 +588,14 @@ fapi2::ReturnCode phy_scominit(const fapi2::Target<TARGET_TYPE_MCBIST>& i_target
 
     for (const auto& p : mss::find_targets<TARGET_TYPE_MCA>(i_target))
     {
+
+        // No point in bothering if we don't have any DIMM
+        if (mss::count_dimm(p) == 0)
+        {
+            FAPI_INF("No DIMM on %s, not running phy_scominit", mss::c_str(p));
+            continue;
+        }
+
         // The following registers must be configured to the correct operating environment:
 
         // Section 5.2.1.3 PC Rank Pair 0 on page 177
diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/utils/find.H b/src/import/chips/p9/procedures/hwp/memory/lib/utils/find.H
index 235eded6a..f6443ec90 100644
--- a/src/import/chips/p9/procedures/hwp/memory/lib/utils/find.H
+++ b/src/import/chips/p9/procedures/hwp/memory/lib/utils/find.H
@@ -105,8 +105,8 @@ inline fapi2::Target<fapi2::TARGET_TYPE_MCBIST> find_target( const fapi2::Target
 /// @return a vector of fapi2::TARGET_TYPE_DIMM
 ///
 template<>
-inline std::vector< fapi2::Target<fapi2::TARGET_TYPE_DIMM> > find_targets( const fapi2::Target<fapi2::TARGET_TYPE_MCS>&
-        i_target )
+inline std::vector< fapi2::Target<fapi2::TARGET_TYPE_DIMM> >
+find_targets( const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target )
 {
     std::vector< fapi2::Target<fapi2::TARGET_TYPE_DIMM> > l_dimms;
 
@@ -126,8 +126,8 @@ inline std::vector< fapi2::Target<fapi2::TARGET_TYPE_DIMM> > find_targets( const
 /// @return a vector of fapi2::TARGET_TYPE_DIMM
 ///
 template<>
-inline std::vector< fapi2::Target<fapi2::TARGET_TYPE_DIMM> > find_targets
-( const fapi2::Target<fapi2::TARGET_TYPE_MCBIST>& i_target )
+inline std::vector< fapi2::Target<fapi2::TARGET_TYPE_DIMM> >
+find_targets( const fapi2::Target<fapi2::TARGET_TYPE_MCBIST>& i_target )
 {
     std::vector< fapi2::Target<fapi2::TARGET_TYPE_DIMM> > l_dimms;
 
diff --git a/src/import/chips/p9/procedures/hwp/memory/p9_mss_eff_config.C b/src/import/chips/p9/procedures/hwp/memory/p9_mss_eff_config.C
index 4a9f4fc84..b2f4649fd 100644
--- a/src/import/chips/p9/procedures/hwp/memory/p9_mss_eff_config.C
+++ b/src/import/chips/p9/procedures/hwp/memory/p9_mss_eff_config.C
@@ -199,43 +199,43 @@ fapi2::ReturnCode p9_mss_eff_config( const fapi2::Target<fapi2::TARGET_TYPE_MCS>
         FAPI_TRY( l_eff_config.additive_latency(l_dimm) );
         FAPI_TRY( l_eff_config.data_mask(l_dimm) );
 
-        // Hard-coded RIT protect attribute set (currently not taken account in eff_config)
-        {
-            uint32_t l_m_dram_clocks[mss::PORTS_PER_MCS] = {0x200, 0x200};
-            FAPI_TRY( FAPI_ATTR_SET( fapi2::ATTR_MSS_RUNTIME_MEM_M_DRAM_CLOCKS, i_target, l_m_dram_clocks ) );
-        }
-        //TODO RTC:160060 verify attributes are needed, best place to put them, and correct values
-        // TK hard coded for now, should change at some point. Most of these are set in eff_config_thermal
-        {
-            uint32_t l_mem_watt_target[mss::PORTS_PER_MCS][mss::MAX_DIMM_PER_PORT] = {{0x640, 0x640}, {0x640, 0x640}};
-            FAPI_TRY( FAPI_ATTR_SET( fapi2::ATTR_MSS_MEM_WATT_TARGET, i_target,
-                                     l_mem_watt_target ) );
-        }
-        //Values taken from Mike Pardeik. Calculated to reach 90% util, will be removed once eff_config_thermal is implemented
-        {
-            uint32_t l_throttled_n_commands[mss::PORTS_PER_MCS] = {0x73, 0x73};
-            FAPI_TRY( FAPI_ATTR_SET( fapi2::ATTR_MSS_RUNTIME_MEM_THROTTLED_N_COMMANDS_PER_PORT, i_target,
-                                     l_throttled_n_commands ) );
-        }
-
-        {
-            uint32_t l_throttled_n_commands[mss::PORTS_PER_MCS] = {0x73, 0x73};
-            FAPI_TRY( FAPI_ATTR_SET( fapi2::ATTR_MSS_RUNTIME_MEM_THROTTLED_N_COMMANDS_PER_SLOT, i_target,
-                                     l_throttled_n_commands ) );
-        }
-
-        {
-            uint16_t l_cal_step[mss::PORTS_PER_MCS] = {0xFAC0, 0xFAC0};
-            FAPI_TRY( FAPI_ATTR_SET(fapi2::ATTR_MSS_CAL_STEP_ENABLE, i_target, l_cal_step) );
-        }
-
-        {
-            uint8_t l_temp_refresh_mode[mss::PORTS_PER_MCS] = {0x01, 0x01};
-            FAPI_TRY( FAPI_ATTR_SET( fapi2::ATTR_EFF_TEMP_REFRESH_MODE, i_target, l_temp_refresh_mode ) );
-        }
-
     }// dimm
 
+    // Hard-coded RIT protect attribute set (currently not taken account in eff_config)
+    {
+        uint32_t l_m_dram_clocks[mss::PORTS_PER_MCS] = {0x200, 0x200};
+        FAPI_TRY( FAPI_ATTR_SET( fapi2::ATTR_MSS_RUNTIME_MEM_M_DRAM_CLOCKS, i_target, l_m_dram_clocks ) );
+    }
+    //TODO RTC:160060 verify attributes are needed, best place to put them, and correct values
+    // TK hard coded for now, should change at some point. Most of these are set in eff_config_thermal
+    {
+        uint32_t l_mem_watt_target[mss::PORTS_PER_MCS][mss::MAX_DIMM_PER_PORT] = {{0x640, 0x640}, {0x640, 0x640}};
+        FAPI_TRY( FAPI_ATTR_SET( fapi2::ATTR_MSS_MEM_WATT_TARGET, i_target,
+                                 l_mem_watt_target ) );
+    }
+    //Values taken from Mike Pardeik. Calculated to reach 90% util, will be removed once eff_config_thermal is implemented
+    {
+        uint32_t l_throttled_n_commands[mss::PORTS_PER_MCS] = {0x73, 0x73};
+        FAPI_TRY( FAPI_ATTR_SET( fapi2::ATTR_MSS_RUNTIME_MEM_THROTTLED_N_COMMANDS_PER_PORT, i_target,
+                                 l_throttled_n_commands ) );
+    }
+
+    {
+        uint32_t l_throttled_n_commands[mss::PORTS_PER_MCS] = {0x73, 0x73};
+        FAPI_TRY( FAPI_ATTR_SET( fapi2::ATTR_MSS_RUNTIME_MEM_THROTTLED_N_COMMANDS_PER_SLOT, i_target,
+                                 l_throttled_n_commands ) );
+    }
+
+    {
+        uint16_t l_cal_step[mss::PORTS_PER_MCS] = {0xFAC0, 0xFAC0};
+        FAPI_TRY( FAPI_ATTR_SET(fapi2::ATTR_MSS_CAL_STEP_ENABLE, i_target, l_cal_step) );
+    }
+
+    {
+        uint8_t l_temp_refresh_mode[mss::PORTS_PER_MCS] = {0x01, 0x01};
+        FAPI_TRY( FAPI_ATTR_SET( fapi2::ATTR_EFF_TEMP_REFRESH_MODE, i_target, l_temp_refresh_mode ) );
+    }
+
     // Check plug rules. We check the MCS, and this will iterate down to children as needed.
     FAPI_TRY( l_eff_config.enforce_plug_rules(i_target) );