Fix CSID: 2 slave ranks, termination in RCBCX

Change-Id: Ie19985e858f5d90b7ca74319c080b156d758ec6a
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/41515
Reviewed-by: Louis Stermole <stermole@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Hostboot CI <hostboot-ci+hostboot@us.ibm.com>
Reviewed-by: STEPHEN GLANCY <sglancy@us.ibm.com>
Reviewed-by: Jennifer A. Stofer <stofer@us.ibm.com>
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/41799
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>

6 files changed, 169 insertions, 59 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 26c3a950e..c3303cc48 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
@@ -68,23 +68,34 @@ enum rc08_encode
 {
     CID_START = 6,
     CID_LENGTH = 2,
-    ALL_ENABLE = 0b00,
-    ONE_ZERO_ENABLE = 0b01,
-    TWO_ONE_ENABLE = 0b10,
-    ALL_DISABLE = 0b11,
     DA17_START = 4,
     DA17_LENGTH = 1,
     DA17_QA17_LOCATION = 4,
     DA17_QA17_ENABLE = 0b0,
     DA17_QA17_DISABLE = 0b1,
     QXPAR_LOCATION = 5,
-    PARITY_ENABLE = 0,
+    RC08_PARITY_ENABLE = 0,
     PARITY_DISABLE = 1,
     MAX_SLAVE_RANKS = 8,
+    NUM_SLAVE_RANKS_ENCODED_IN_ONE_BIT = 2,
     NUM_SLAVE_RANKS_ENCODED_IN_TWO_BITS = 4,
+    NUM_SLAVE_RANKS_ENCODED_IN_THREE_BITS = 8,
 };
 
 ///
+/// @brief bit encodings for Frequencies RCBX
+/// @note valid frequency values for Nimbus systems
+/// From DDR4 Register v1.0
+///
+enum rcbx_encode
+{
+    DC0_POS = 7,
+    DC1_POS = 6,
+    DC2_POS = 5,
+    DC_ENABLE = 0b0,
+    DC_DISABLE = 0b1,
+};
+///
 /// @brief bit encodings for Frequencies RC0A (RC0A)
 /// @note valid frequency values for Nimbus systems
 /// From DDR4 Register v1.0
@@ -1209,45 +1220,52 @@ fapi_try_exit:
 }
 
 ///
-/// @brief Determines & sets effective config for DIMM RC08
+/// @brief Determines how many chip ID bits are needed for the iv_dimm
+/// @param[out] o_qs a qsid encoding denoting if 0, 1, 2, or all three QSID's are needed
 /// @return fapi2::FAPI2_RC_SUCCESS if okay
-/// @note DA[1:0] enable/ disable QxC
 ///
-fapi2::ReturnCode eff_dimm::dimm_rc08()
+fapi2::ReturnCode eff_dimm::calculate_chip_ids( qsid& o_qs)
 {
-    uint8_t l_stack_type = 0;
-    uint8_t l_attrs_dimm_rc08[PORTS_PER_MCS][MAX_DIMM_PER_PORT] = {};
-    fapi2::buffer<uint8_t> l_buffer = 0;
     uint8_t l_total_ranks = 0;
     uint8_t l_master_ranks = 0;
     uint8_t l_num_slave_ranks = 0;
+    uint8_t l_stack_type = 0;
 
-    FAPI_TRY( eff_dimm_ddr4_rc08(iv_mcs, &l_attrs_dimm_rc08[0][0]) );
-    FAPI_TRY(iv_pDecoder->num_package_ranks_per_dimm(l_master_ranks) );
-
-    // Pulling this one from the decoder
-    FAPI_TRY( iv_pDecoder->logical_ranks_per_dimm(l_total_ranks),
-              "Failed to get logical_ranks_per_dimm from SPD %s", mss::c_str(iv_dimm) );
+    FAPI_TRY( mss::eff_num_master_ranks_per_dimm(iv_dimm, l_master_ranks) );
+    FAPI_TRY( mss::eff_num_ranks_per_dimm(iv_dimm, l_total_ranks) );
 
-    // Calling the this eff_dimm's setter function and then the function to get the attribute
+    // Calling this eff_dimm's setter function and then the function to get the attribute
     // While this is less efficient than calling the decoder function, it makes testing 3DS DIMMs easier
     // This allows us to use attribute overrides to test the DIMMS as SDP's
-    FAPI_TRY( primary_stack_type());
     FAPI_TRY( eff_prim_stack_type(iv_dimm, l_stack_type));
 
-    // Little assert, but this shouldn't be possibly and should be caught in the decoder function
-    fapi2::Assert(l_master_ranks != 0);
+    // Little assert, but this shouldn't be possible and should be caught in the decoder function
+    if (l_master_ranks == 0)
+    {
+        FAPI_ERR("Error getting master_ranks attribute. Should have already been caught if bad SPD");
+        fapi2::Assert(false);
+    }
 
     // Slave ranks = total ranks (aka logical ranks)  / master ranks (aka package ranks)
     l_num_slave_ranks = l_total_ranks / l_master_ranks;
 
+    // Let's assert that our math is correct. total_ranks % l_master_ranks should equal 0. It should be cleanly divisible
+    FAPI_ASSERT( !(l_total_ranks % l_master_ranks),
+                 fapi2::MSS_INVALID_SPD_SLAVE_RANKS()
+                 .set_LOGICAL_RANKS(l_total_ranks)
+                 .set_MASTER_RANKS(l_master_ranks)
+                 .set_DIMM_TARGET(iv_dimm),
+                 "%s Logical ranks(%d) are not divisible by master ranks(%d). Bad SPD?",
+                 mss::c_str(iv_dimm), l_total_ranks, l_master_ranks);
+
     // If we are 3DS we need to enable chip ID signal with QxC
     switch( l_stack_type)
     {
         case fapi2::ENUM_ATTR_EFF_PRIM_STACK_TYPE_DDP_QDP:
         case fapi2::ENUM_ATTR_EFF_PRIM_STACK_TYPE_SDP:
             // Don't need the chip ID signals enabled because no slave ranks
-            l_buffer.insertFromRight<CID_START, CID_LENGTH> (ALL_DISABLE);
+            FAPI_INF("Disabling chip IDs");
+            o_qs = ALL_DISABLE;
             break;
 
         // If 3DS, we have slave ranks and thus need some chip ID bits to be enabled
@@ -1277,7 +1295,7 @@ fapi2::ReturnCode eff_dimm::dimm_rc08()
                      l_num_slave_ranks);
 
             // Double check we calculated this correctly
-            FAPI_ASSERT( ((l_num_slave_ranks != 0) ||  (l_num_slave_ranks < 8)),
+            FAPI_ASSERT( ((l_num_slave_ranks != 0) ||  (l_num_slave_ranks < NUM_SLAVE_RANKS_ENCODED_IN_THREE_BITS)),
                          fapi2::MSS_INVALID_CALCULATED_NUM_SLAVE_RANKS()
                          .set_NUM_SLAVE_RANKS(l_num_slave_ranks)
                          .set_NUM_TOTAL_RANKS(l_total_ranks)
@@ -1289,18 +1307,24 @@ fapi2::ReturnCode eff_dimm::dimm_rc08()
                          l_total_ranks,
                          l_master_ranks);
 
-            // Only need 2 bits to encode 4 slave ranks with chip IDs
-            if (l_num_slave_ranks < NUM_SLAVE_RANKS_ENCODED_IN_TWO_BITS)
+            // These are based off of log base 2.
+            if (l_num_slave_ranks <= NUM_SLAVE_RANKS_ENCODED_IN_ONE_BIT)
             {
-
-                l_buffer.insertFromRight<CID_START, CID_LENGTH>( ONE_ZERO_ENABLE);
+                o_qs = ZERO_ENABLE;
+            }
+            // Only need 2 bits to encode 4 slave ranks per master rank with chip IDs
+            else if (l_num_slave_ranks <= NUM_SLAVE_RANKS_ENCODED_IN_TWO_BITS)
+            {
+                o_qs = ZERO_ONE_ENABLE;
             }
             else
             {
-                // 4-8 slave ranks, Gonna need all three bits
-                l_buffer.insertFromRight<CID_START, CID_LENGTH>( ALL_ENABLE);
+                // 5-8 slave ranks, Gonna need all three bits
+                o_qs = ALL_ENABLE;
             }
 
+            FAPI_INF("%s Chip ID bits are %x", mss::c_str(iv_dimm), o_qs);
+
             break;
 
         default:
@@ -1315,16 +1339,32 @@ fapi2::ReturnCode eff_dimm::dimm_rc08()
                          l_stack_type);
     };
 
+fapi_try_exit:
+    return fapi2::current_err;
+}
+///
+/// @brief Determines & sets effective config for DIMM RC08
+/// @return fapi2::FAPI2_RC_SUCCESS if okay
+/// @note DA[1:0] enable/ disable QxC
+///
+fapi2::ReturnCode eff_dimm::dimm_rc08()
+{
+    uint8_t l_attrs_dimm_rc08[PORTS_PER_MCS][MAX_DIMM_PER_PORT] = {};
+    fapi2::buffer<uint8_t> l_buffer = 0;
+
+    qsid l_qs_enabled = ALL_DISABLE;
+    FAPI_TRY( eff_dimm::calculate_chip_ids(l_qs_enabled) );
+    l_buffer.insertFromRight<CID_START, CID_LENGTH>(l_qs_enabled);
+
     // Let's set the other bits
-    l_buffer.writeBit<QXPAR_LOCATION>(PARITY_ENABLE);
+    l_buffer.writeBit<QXPAR_LOCATION>(RC08_PARITY_ENABLE);
 
-    // We only need the 17th Address bit if we have 16G dense drams
-    // TK: come back and do disable if not needed
     l_buffer.writeBit<DA17_QA17_LOCATION>(DA17_QA17_ENABLE);
 
+    FAPI_TRY( eff_dimm_ddr4_rc08(iv_mcs, &l_attrs_dimm_rc08[0][0]) );
     l_attrs_dimm_rc08[iv_port_index][iv_dimm_index] = l_buffer;
 
-    FAPI_INF( "%s: RC08 setting: %d", mss::c_str(iv_dimm), l_attrs_dimm_rc08[iv_port_index][iv_dimm_index] );
+    FAPI_INF( "%s: RC08 setting: 0x%02x", mss::c_str(iv_dimm), l_attrs_dimm_rc08[iv_port_index][iv_dimm_index] );
 
     FAPI_TRY( FAPI_ATTR_SET(fapi2::ATTR_EFF_DIMM_DDR4_RC08, iv_mcs, l_attrs_dimm_rc08) );
 
@@ -1810,12 +1850,47 @@ fapi_try_exit:
 ///
 fapi2::ReturnCode eff_dimm::dimm_rcbx()
 {
+    fapi2::buffer<uint8_t> l_buf;
+
     // Retrieve MCS attribute data
     uint8_t l_attrs_dimm_rc_bx[PORTS_PER_MCS][MAX_DIMM_PER_PORT] = {};
-    FAPI_TRY( eff_dimm_ddr4_rc_bx(iv_mcs, &l_attrs_dimm_rc_bx[0][0]) );
 
-    // Update MCS attribute
-    l_attrs_dimm_rc_bx[iv_port_index][iv_dimm_index] = iv_pDecoder->iv_raw_card.iv_rcbx;
+    qsid l_qs_enabled = ALL_DISABLE;
+    FAPI_TRY( eff_dimm::calculate_chip_ids( l_qs_enabled) );
+
+    switch (l_qs_enabled)
+    {
+        case ALL_DISABLE:
+            l_buf.writeBit<DC0_POS>(DC_DISABLE)
+            .writeBit<DC1_POS>(DC_DISABLE)
+            .writeBit<DC2_POS>(DC_DISABLE);
+            break;
+
+        case ZERO_ENABLE:
+            l_buf.writeBit<DC0_POS>(DC_ENABLE)
+            .writeBit<DC1_POS>(DC_DISABLE)
+            .writeBit<DC2_POS>(DC_DISABLE);
+            break;
+
+        case ZERO_ONE_ENABLE:
+            l_buf.writeBit<DC0_POS>(DC_ENABLE)
+            .writeBit<DC1_POS>(DC_ENABLE)
+            .writeBit<DC2_POS>(DC_DISABLE);
+            break;
+
+        case ALL_ENABLE:
+            l_buf.writeBit<DC0_POS>(DC_ENABLE)
+            .writeBit<DC1_POS>(DC_ENABLE)
+            .writeBit<DC2_POS>(DC_ENABLE);
+            break;
+
+        default:
+            FAPI_ERR("%s Error with C++ enum in eff_dimm.C. %d recieved", mss::c_str(iv_dimm), l_qs_enabled);
+            fapi2::Assert(false);
+    }
+
+    FAPI_TRY( eff_dimm_ddr4_rc_bx(iv_mcs, &l_attrs_dimm_rc_bx[0][0]) );
+    l_attrs_dimm_rc_bx[iv_port_index][iv_dimm_index] = l_buf;
 
     FAPI_INF( "%s: RCBX setting: %d", mss::c_str(iv_dimm), l_attrs_dimm_rc_bx[iv_port_index][iv_dimm_index] );
     FAPI_TRY( FAPI_ATTR_SET(fapi2::ATTR_EFF_DIMM_DDR4_RC_Bx, iv_mcs, l_attrs_dimm_rc_bx) );
@@ -3744,7 +3819,6 @@ fapi_try_exit:
     return fapi2::current_err;
 }
 
-
 ///
 /// @brief Determines & sets effective config for DIMM BC00
 /// @return fapi2::FAPI2_RC_SUCCESS if okay
diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/dimm/eff_dimm.H b/src/import/chips/p9/procedures/hwp/memory/lib/dimm/eff_dimm.H
index 3c20dcd87..4e047e540 100644
--- a/src/import/chips/p9/procedures/hwp/memory/lib/dimm/eff_dimm.H
+++ b/src/import/chips/p9/procedures/hwp/memory/lib/dimm/eff_dimm.H
@@ -67,6 +67,19 @@ class eff_dimm
         uint8_t iv_refresh_mode;
         uint8_t iv_refresh_rate_request;
 
+        ///
+        ///@brief Encoding for QSID for RC08,
+        ///@note  placing here so we can use the enum for calculate_chip_id and for RCBX
+        ///
+        enum qsid
+        {
+            ALL_ENABLE = 0b00,
+            ZERO_ENABLE = 0b10,
+            ZERO_ONE_ENABLE = 0b01,
+            ALL_DISABLE = 0b11,
+
+        };
+
         //Delete the default
         eff_dimm () = delete;
 
@@ -280,6 +293,13 @@ class eff_dimm
         fapi2::ReturnCode dimm_rc06_07();
 
         ///
+        /// @brief Determines how many chip select ID bits are needed for the iv_dimm
+        /// @param[out] o_qs a qsid encoding denoting if 0, 1, 2, or all three QSID's are needed
+        /// @return fapi2::FAPI2_RC_SUCCESS if okay
+        ///
+        fapi2::ReturnCode calculate_chip_ids( qsid& o_qs);
+
+        ///
         /// @brief Determines & sets effective config for DIMM RC08
         /// @return fapi2::FAPI2_RC_SUCCESS if okay
         ///
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 7adf99bbf..ae4c7860c 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
@@ -137,6 +137,31 @@
   </hwpError>
 
   <hwpError>
+    <rc>RC_MSS_INVALID_SPD_SLAVE_RANKS</rc>
+    <description>
+        Seems logical ranks are not master ranks* slave ranks.
+        Error from SPD is most likely.
+    </description>
+    <ffdc>LOGICAL_RANKS</ffdc>
+    <ffdc>MASTER_RANKS</ffdc>
+    <callout>
+      <procedure>CODE</procedure>
+      <priority>HIGH</priority>
+    </callout>
+    <callout>
+      <target>DIMM_TARGET</target>
+      <priority>MEDIUM</priority>
+    </callout>
+    <deconfigure>
+      <target>DIMM_TARGET</target>
+    </deconfigure>
+    <gard>
+      <target>DIMM_TARGET</target>
+    </gard>
+  </hwpError>
+
+
+  <hwpError>
     <rc>RC_MSS_BAD_SDRAM_DENSITY_DECODER</rc>
     <description>
       SPD decoder messed up and returned a 0. Should have been caught already
diff --git a/src/import/generic/memory/lib/spd/common/rcw_settings.H b/src/import/generic/memory/lib/spd/common/rcw_settings.H
index 57527dbe5..8f6675c41 100644
--- a/src/import/generic/memory/lib/spd/common/rcw_settings.H
+++ b/src/import/generic/memory/lib/spd/common/rcw_settings.H
@@ -66,7 +66,6 @@ struct rcw_settings
     uint64_t iv_rc8x;
     uint64_t iv_rc9x;
     uint64_t iv_rcax;
-    uint64_t iv_rcbx;
 
     ///
     /// @brief default ctor
@@ -113,7 +112,6 @@ struct rcw_settings
     /// @param[in] i_rc8x setting for register control word (RC8X)
     /// @param[in] i_rc9x setting for register control word (RC9X)
     /// @param[in] i_rcax setting for register control word (RCAX)
-    /// @param[in] i_rcbx setting for register control word (RCBX)
     ///
     constexpr rcw_settings( const uint64_t i_rc00,
                             const uint64_t i_rc01,
@@ -130,8 +128,7 @@ struct rcw_settings
                             const uint64_t i_rc6x,
                             const uint64_t i_rc8x,
                             const uint64_t i_rc9x,
-                            const uint64_t i_rcax,
-                            const uint64_t i_rcbx )
+                            const uint64_t i_rcax)
         : iv_rc00(i_rc00),
           iv_rc01(i_rc01),
           iv_rc02(i_rc02),
@@ -147,8 +144,7 @@ struct rcw_settings
           iv_rc6x(i_rc6x),
           iv_rc8x(i_rc8x),
           iv_rc9x(i_rc9x),
-          iv_rcax(i_rcax),
-          iv_rcbx(i_rcbx)
+          iv_rcax(i_rcax)
     {}
 
     ///
diff --git a/src/import/generic/memory/lib/spd/lrdimm/ddr4/lrdimm_raw_cards.C b/src/import/generic/memory/lib/spd/lrdimm/ddr4/lrdimm_raw_cards.C
index f4b943b2c..a63138843 100644
--- a/src/import/generic/memory/lib/spd/lrdimm/ddr4/lrdimm_raw_cards.C
+++ b/src/import/generic/memory/lib/spd/lrdimm/ddr4/lrdimm_raw_cards.C
@@ -65,8 +65,7 @@ rcw_settings lrdimm_rc_b0( 0x00, // RC00
                            0x00, // RC6C
                            0x00, // RC8X
                            0x00, // RC9X
-                           0x00, // RCAx
-                           0x07);// RCBX
+                           0x00); // RCAX
 
 namespace lrdimm
 {
diff --git a/src/import/generic/memory/lib/spd/rdimm/ddr4/rdimm_raw_cards.C b/src/import/generic/memory/lib/spd/rdimm/ddr4/rdimm_raw_cards.C
index 0f757153d..383fb7e09 100644
--- a/src/import/generic/memory/lib/spd/rdimm/ddr4/rdimm_raw_cards.C
+++ b/src/import/generic/memory/lib/spd/rdimm/ddr4/rdimm_raw_cards.C
@@ -64,8 +64,7 @@ rcw_settings rdimm_rc_c1( 0x02, // RC00
                           0x00, // RC6X
                           0x00, // RC8X
                           0x00, // RC9X
-                          0x00, // RCAX
-                          0x07);// RCBX
+                          0x00); // RCAX
 
 ///
 /// @brief raw card C2 settings
@@ -86,8 +85,7 @@ rcw_settings rdimm_rc_c2( 0x02, // RC00
                           0x00, // RC6X
                           0x00, // RC8X
                           0x00, // RC9X
-                          0x00, // RCAX
-                          0x07);// RCBX
+                          0x00); // RCAX
 
 ///
 /// @brief raw card A1 settings
@@ -107,8 +105,7 @@ rcw_settings  rdimm_rc_a1( 0x02, // RC00
                            0x00, // RC6X
                            0x00, // RC8X
                            0x00, // RC9X
-                           0x00, // RCAX
-                           0x07);// RCBX
+                           0x00); // RCAX
 
 ///
 /// @brief raw card B1 settings
@@ -129,8 +126,8 @@ rcw_settings rdimm_rc_b1( 0x02, // RC00
                           0x00, // RC6X
                           0x00, // RC8X
                           0x00, // RC9X
-                          0x00, // RCAX
-                          0x07);// RCBX
+                          0x00);// RCAX
+
 
 ///
 /// @brief raw card B2 settings
@@ -150,8 +147,8 @@ rcw_settings rdimm_rc_b2( 0x02, // RC00
                           0x00, // RC6X
                           0x00, // RC8X
                           0x00, // RC9X
-                          0x00, // RCAX
-                          0x07);// RCBX
+                          0x00);// RCAX
+
 
 ////
 /// @brief raw card for custom dimms
@@ -171,8 +168,8 @@ rcw_settings rdimm_rc_custom ( 0x02, // RC00
                                0x00, // RC6X
                                0x00, // RC8X
                                0x00, // RC9X
-                               0x00, // RCAX
-                               0x07);// RCBX
+                               0x00);// RCAX
+
 
 ///
 /// @brief raw card VBU settings
@@ -192,8 +189,7 @@ rcw_settings rdimm_rc_vbu( 0x02, // RC00
                            0x00, // RC6X
                            0x00, // RC8X
                            0x00, // RC9X
-                           0x00, // RCAX
-                           0x07);// RCBX
+                           0x00); // RCAX
 
 namespace rdimm
 {