Fix Mirror programming for Cumulus

Only attempt to setup Mirror if memory configuration allows.

Change-Id: I3bbaf681817d82c37eb330c5d12ab3a494998c2a
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/44744
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@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: Joseph J. McGill <jmcgill@us.ibm.com>
Dev-Ready: Joseph J. McGill <jmcgill@us.ibm.com>
Reviewed-by: LENNARD G. STREAT <lstreat@us.ibm.com>
Reviewed-by: Jennifer A. Stofer <stofer@us.ibm.com>
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/44757
Reviewed-by: Hostboot Team <hostboot@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>

3 files changed, 203 insertions, 117 deletions

diff --git a/src/import/chips/p9/procedures/hwp/nest/p9_mss_eff_grouping.C b/src/import/chips/p9/procedures/hwp/nest/p9_mss_eff_grouping.C
index bc844b8bc..ea8376593 100644
--- a/src/import/chips/p9/procedures/hwp/nest/p9_mss_eff_grouping.C
+++ b/src/import/chips/p9/procedures/hwp/nest/p9_mss_eff_grouping.C
@@ -208,7 +208,7 @@ fapi2::ReturnCode EffGroupingProcAttrs::calcProcBaseAddr(
     const EffGroupingSysAttrs i_sysAttrs)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
+
     uint64_t l_memBaseAddr1, l_mmioBaseAddr;
 
     // Get the Mirror/Non-mirror base addresses
@@ -244,7 +244,6 @@ fapi2::ReturnCode EffGroupingProcAttrs::getAttrs(
     const EffGroupingSysAttrs i_sysAttrs)
 {
     FAPI_DBG("Entering EffGroupingProcAttrs::getAttrs");
-    fapi2::ReturnCode l_rc;
 
     // Get Nest Hardware Trace Macro (NHTM) bar size
     FAPI_TRY(FAPI_ATTR_GET(fapi2::ATTR_PROC_NHTM_BAR_SIZE, i_target, iv_nhtmBarSize),
@@ -343,7 +342,6 @@ fapi2::ReturnCode EffGroupingMcaAttrs::getAttrs(
     const fapi2::Target<fapi2::TARGET_TYPE_MCA>& i_target)
 {
     FAPI_DBG("Entering EffGroupingMcaAttrs::getAttrs");
-    fapi2::ReturnCode l_rc;
 
     // Get the amount of memory behind this MCA target
     // Note: DIMM must be enabled to be accounted for.
@@ -403,7 +401,6 @@ fapi2::ReturnCode EffGroupingDmiAttrs::getAttrs(
     const fapi2::Target<fapi2::TARGET_TYPE_DMI>& i_target)
 {
     FAPI_DBG("Entering EffGroupingDmiAttrs::getAttrs");
-    fapi2::ReturnCode l_rc;
 
     // Get the membuf attached to this DMI
     auto l_attachedMembuf = i_target.getChildren<fapi2::TARGET_TYPE_MEMBUF_CHIP>();
@@ -541,7 +538,6 @@ fapi2::ReturnCode EffGroupingMemInfo::getMemInfo (
     const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
 
     // Memory info will be filled in differently for Nimbus vs Cumulus
     // due to chip structure
@@ -620,6 +616,7 @@ struct EffGroupingData
     EffGroupingData()
     {
         memset(iv_data, 0, sizeof(iv_data));
+        memset(iv_mirrorOn, 0, sizeof(iv_mirrorOn));
 
         for (uint8_t l_port = 0; l_port < NUM_MC_PORTS_PER_PROC; l_port++)
         {
@@ -638,6 +635,10 @@ struct EffGroupingData
 
     // The total non-mirrored memory size in GB
     uint32_t iv_totalSizeNonMirr = 0;
+
+    // Indicates if mirror group is to be created
+    // from data of this non-mirror group
+    uint8_t iv_mirrorOn[DATA_GROUPS / 2];
 };
 
 
@@ -806,27 +807,24 @@ void EffGroupingBaseSizeData::setBaseSizeData(
         {
             uint8_t l_index = ii + MIRR_OFFSET;
 
-            // Set base address for distinct mirrored ranges
-            iv_mirror_bases[ii] = i_groupData.iv_data[l_index][BASE_ADDR];
-            iv_mirror_bases_ack[ii] = i_groupData.iv_data[l_index][BASE_ADDR];
-
-            // Set sizes for distinct mirrored ranges
-            if (i_groupData.iv_data[ii][PORTS_IN_GROUP] > 1) // ii -> Non-mirror index
+            if (i_groupData.iv_data[l_index][PORTS_IN_GROUP] != 0)
             {
+                // Set base address for distinct mirrored ranges
+                iv_mirror_bases[ii] = i_groupData.iv_data[l_index][BASE_ADDR];
+                iv_mirror_bases_ack[ii] = i_groupData.iv_data[l_index][BASE_ADDR];
+                // Set sizes for distinct mirrored ranges
                 iv_mirror_sizes[ii] = (i_groupData.iv_data[ii][PORT_SIZE] *
                                        i_groupData.iv_data[ii][PORTS_IN_GROUP]) / 2;
-            }
+                iv_mirror_sizes_ack[ii] = i_groupData.iv_data[l_index][GROUP_SIZE];
 
-            iv_mirror_sizes_ack[ii] = i_groupData.iv_data[l_index][GROUP_SIZE];
-
-            // Convert to full byte addresses
-            iv_mirror_bases[ii]     <<= 30;
-            iv_mirror_bases_ack[ii] <<= 30;
-            iv_mirror_sizes[ii]     <<= 30;
-            iv_mirror_sizes_ack[ii] <<= 30;
+                // Convert to full byte addresses
+                iv_mirror_bases[ii]     <<= 30;
+                iv_mirror_bases_ack[ii] <<= 30;
+                iv_mirror_sizes[ii]     <<= 30;
+                iv_mirror_sizes_ack[ii] <<= 30;
+            }
 
             FAPI_DBG("Mirror: %d", ii);
-
             FAPI_DBG("    i_groupData.iv_data[%d][BASE_ADDR] = 0x%.16llX (%d GB)",
                      l_index,  i_groupData.iv_data[l_index][BASE_ADDR],
                      i_groupData.iv_data[l_index][BASE_ADDR] >> 30);
@@ -962,7 +960,6 @@ fapi2::ReturnCode EffGroupingBaseSizeData::set_HTM_OCC_base_addr(
     const EffGroupingProcAttrs& i_procAttrs)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
 
     // Hold mem bases & sizes for mirror/non-mirror
     uint8_t l_numRegions = 0;
@@ -1000,7 +997,7 @@ fapi2::ReturnCode EffGroupingBaseSizeData::set_HTM_OCC_base_addr(
     if (l_htmOccSize == 0)
     {
         FAPI_INF("set_HTM_OCC_base_addr: No HTM/OCC memory requested.");
-        return l_rc;
+        goto fapi_try_exit;
     }
 
     // Setup mem base and size working array depending on mirror setting
@@ -1176,7 +1173,6 @@ fapi2::ReturnCode EffGroupingBaseSizeData::setBaseSizeAttr(
     EffGroupingData& io_groupData)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
 
     //----------------------------------------------------------------------
     //  Setting attributes
@@ -1264,7 +1260,6 @@ fapi2::ReturnCode EffGroupingBaseSizeData::setBaseSizeAttr(
     //----------------------------------------------------------------------
     //  Display attribute values
     //----------------------------------------------------------------------
-
     for (uint8_t ii = 0; ii < NUM_NON_MIRROR_REGIONS; ii++)
     {
         FAPI_INF("ATTR_PROC_MEM_BASES    [%u]: 0x%.16llX (%d GB)",
@@ -1362,7 +1357,6 @@ fapi2::ReturnCode grouping_checkValidAttributes(
     const EffGroupingProcAttrs& i_procAttrs)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
 
     // If mirror is disabled, then can not be in FLIPPED mode
     if (!i_sysAttrs.iv_hwMirrorEnabled)
@@ -2316,7 +2310,6 @@ fapi2::ReturnCode grouping_findUngroupedPorts(
     const EffGroupingData& i_groupData)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
 
     // std_pair<MC number, target>
     std::map<uint8_t, fapi2::Target<T>> l_unGroupedPair;
@@ -2501,6 +2494,74 @@ void grouping_sortGroups(EffGroupingData& io_groupData)
 }
 
 ///
+/// @brief Determine if mirror groups are to be created for existing groups.
+///
+///        Mirror group is created when these conditions are all met:
+///           - Processor is Cumulus
+///           - ATTR_MRW_HW_MIRRORING_ENABLE = true
+///           - Number of MC ports is 2, 4, 6, or 8 and 2 ports are in the same
+///             MCS/MI port pair (see MCFGP(1:4) programming in MC workbook)
+///
+/// @param[in]      i_target       Reference to TARGET_TYPE_PROC_CHIP target
+/// @param[in]      i_sysAttrs     System attribute setting
+/// @param[in/out]  io_groupData   Grouping data
+///
+/// @return FAPI2_RC_SUCCESS if success, else error code.
+///
+void setupMirrorGroup(
+    const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
+    const EffGroupingSysAttrs& i_sysAttrs,
+    EffGroupingData& io_groupData)
+{
+    FAPI_DBG("Entering setupMirrorGroup");
+
+    // Get the MI chiplets
+    auto l_miChiplets = i_target.getChildren<fapi2::TARGET_TYPE_MI>();
+
+    // No mirroring if Nimbus or ATTR_MRW_HW_MIRRORING_ENABLE is off
+    if ( (l_miChiplets.size() == 0) || (!i_sysAttrs.iv_hwMirrorEnabled) )
+    {
+        FAPI_INF("setupMirrorGroup: No mirror group - Num MI chiplets %d, "
+                 "ATTR_MRW_HW_MIRRORING_ENABLE = %d",
+                 l_miChiplets.size(), i_sysAttrs.iv_hwMirrorEnabled);
+        goto fapi_try_exit;
+    }
+
+    // Loop thru groups to see if mirror group is possible
+    for (uint8_t l_group = 0; l_group < io_groupData.iv_numGroups; l_group++)
+    {
+        // If group of 4, 6, or 8, mirror is allowed
+        // Note: For group of 4/6/8, the ports are always in the same MC
+        //       port pair per design.
+        if ( (io_groupData.iv_data[l_group][PORTS_IN_GROUP] == 4) ||
+             (io_groupData.iv_data[l_group][PORTS_IN_GROUP] == 6) ||
+             (io_groupData.iv_data[l_group][PORTS_IN_GROUP] == 8) )
+        {
+            io_groupData.iv_mirrorOn[l_group] = 1;
+        }
+
+        // For group of 2, determine if both ports are in the same MCS/MI
+        else if (io_groupData.iv_data[l_group][PORTS_IN_GROUP] == 2)
+        {
+            if ( (io_groupData.iv_data[l_group][MEMBER_IDX(0)] / 2) ==
+                 (io_groupData.iv_data[l_group][MEMBER_IDX(1)] / 2) )
+            {
+                io_groupData.iv_mirrorOn[l_group] = 1;
+            }
+        }
+
+        FAPI_INF("setupMirrorGroup: Group %d, PortsInGroup %d, Mirror = %d",
+                 l_group, io_groupData.iv_data[l_group][PORTS_IN_GROUP],
+                 io_groupData.iv_mirrorOn[l_group]);
+
+    } // Group loop
+
+fapi_try_exit:
+    FAPI_DBG("Exiting setupMirrorGroup");
+    return;
+}
+
+///
 /// @brief Calculate Mirror Memory base and alt-base addresses
 ///
 /// @param[in] i_target           Reference to processor chip target
@@ -2517,12 +2578,11 @@ fapi2::ReturnCode grouping_calcMirrorMemory(
     EffGroupingData& io_groupData)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
 
     // Calculate mirrored group size and non mirrored group size
     for (uint8_t pos = 0; pos < io_groupData.iv_numGroups; pos++)
     {
-        if (io_groupData.iv_data[pos][PORTS_IN_GROUP] > 1)
+        if (io_groupData.iv_mirrorOn[pos])
         {
             uint8_t l_mirrorOffset = pos + MIRR_OFFSET;
 
@@ -2690,7 +2750,6 @@ fapi2::ReturnCode grouping_setATTR_MSS_MEM_MC_IN_GROUP(
     const EffGroupingData& i_groupData)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
 
     fapi2::buffer<uint8_t> MC_IN_GP;
     uint8_t l_mcPort_in_group[NUM_MC_PORTS_PER_PROC];
@@ -2772,24 +2831,28 @@ void grouping_traceData(const EffGroupingSysAttrs& i_sysAttrs,
         {
             uint8_t l_mirrorOffset = ii + MIRR_OFFSET;
 
-            FAPI_INF("MIRROR - Group %u: ", l_mirrorOffset);
-            FAPI_INF("    MC port size %d GB", i_groupData.iv_data[l_mirrorOffset][PORT_SIZE]);
-            FAPI_INF("    Num of ports %d", i_groupData.iv_data[l_mirrorOffset][PORTS_IN_GROUP]);
-            FAPI_INF("    Group size  %d GB", i_groupData.iv_data[l_mirrorOffset][GROUP_SIZE]);
-            FAPI_INF("    Base addr 0x%08x", i_groupData.iv_data[l_mirrorOffset][BASE_ADDR]);
-
-            for (uint8_t jj = 0; jj < NUM_OF_ALT_MEM_REGIONS; jj++)
+            // Only display valid mirrored group
+            if (i_groupData.iv_data[l_mirrorOffset][GROUP_SIZE] > 0)
             {
-                FAPI_INF("    ALT-BAR(%d) valid %d ", jj, i_groupData.iv_data[l_mirrorOffset][ALT_VALID(jj)]);
-                FAPI_INF("    ALT-BAR(%d) size %d ", jj, i_groupData.iv_data[l_mirrorOffset][ALT_SIZE(jj)]);
-                FAPI_INF("    ALT-BAR(%d) base addr 0x%08X", jj, i_groupData.iv_data[l_mirrorOffset][ALT_BASE_ADDR(jj)]);
-            }
+                FAPI_INF("MIRROR - Group %u: ", l_mirrorOffset);
+                FAPI_INF("    MC port size %d GB", i_groupData.iv_data[l_mirrorOffset][PORT_SIZE]);
+                FAPI_INF("    Num of ports %d", i_groupData.iv_data[l_mirrorOffset][PORTS_IN_GROUP]);
+                FAPI_INF("    Group size  %d GB", i_groupData.iv_data[l_mirrorOffset][GROUP_SIZE]);
+                FAPI_INF("    Base addr 0x%08x", i_groupData.iv_data[l_mirrorOffset][BASE_ADDR]);
 
-            // Display MC in groups
-            for (uint8_t jj = 0; jj < i_groupData.iv_data[l_mirrorOffset][PORTS_IN_GROUP]; jj++)
-            {
-                FAPI_INF("    Contains MC %d",
-                         i_groupData.iv_data[l_mirrorOffset][MEMBER_IDX(jj)]);
+                for (uint8_t jj = 0; jj < NUM_OF_ALT_MEM_REGIONS; jj++)
+                {
+                    FAPI_INF("    ALT-BAR(%d) valid %d ", jj, i_groupData.iv_data[l_mirrorOffset][ALT_VALID(jj)]);
+                    FAPI_INF("    ALT-BAR(%d) size %d ", jj, i_groupData.iv_data[l_mirrorOffset][ALT_SIZE(jj)]);
+                    FAPI_INF("    ALT-BAR(%d) base addr 0x%08X", jj, i_groupData.iv_data[l_mirrorOffset][ALT_BASE_ADDR(jj)]);
+                }
+
+                // Display MC in groups
+                for (uint8_t jj = 0; jj < i_groupData.iv_data[l_mirrorOffset][PORTS_IN_GROUP]; jj++)
+                {
+                    FAPI_INF("    Contains MC %d",
+                             i_groupData.iv_data[l_mirrorOffset][MEMBER_IDX(jj)]);
+                }
             }
         }
     }
@@ -2806,7 +2869,6 @@ fapi2::ReturnCode p9_mss_eff_grouping(
     const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
 
     // Create data structures for grouping operation
     EffGroupingSysAttrs l_sysAttrs;
@@ -2814,6 +2876,7 @@ fapi2::ReturnCode p9_mss_eff_grouping(
     EffGroupingMemInfo l_memInfo;
     EffGroupingBaseSizeData l_baseSizeData;
     EffGroupingData l_groupData;
+    bool l_mirrorIsOn = false;
 
     // ----------------------------------------------
     // Get the attributes needed for memory grouping
@@ -2914,8 +2977,21 @@ fapi2::ReturnCode p9_mss_eff_grouping(
 
     FAPI_INF("Total non-mirrored size %u GB", l_groupData.iv_totalSizeNonMirr);
 
-    if (l_sysAttrs.iv_hwMirrorEnabled)
+    // Set mirror groups
+    setupMirrorGroup(i_target, l_sysAttrs, l_groupData);
+
+    for (uint8_t l_group = 0; l_group < l_groupData.iv_numGroups; l_group++)
+    {
+        if (l_groupData.iv_mirrorOn[l_group] == 1)
+        {
+            l_mirrorIsOn = true;
+            break;
+        }
+    }
+
+    if (l_mirrorIsOn)
     {
+        FAPI_INF("Mirror memory configured");
         // Calculate base and alt-base addresses
         FAPI_TRY(grouping_calcMirrorMemory(i_target, l_procAttrs, l_groupData),
                  "Error from grouping_calcMirrorMemory, l_rc 0x%.8X",
@@ -2925,6 +3001,7 @@ fapi2::ReturnCode p9_mss_eff_grouping(
     {
         // ATTR_MRW_HW_MIRRORING_ENABLE is false
         // Calculate base and alt-base addresses
+        FAPI_INF("No mirror memory configured");
         grouping_calcNonMirrorMemory(l_procAttrs, l_groupData);
     }
 
diff --git a/src/import/chips/p9/procedures/hwp/nest/p9_mss_eff_grouping.H b/src/import/chips/p9/procedures/hwp/nest/p9_mss_eff_grouping.H
index 3f29b1810..3f4f2f465 100644
--- a/src/import/chips/p9/procedures/hwp/nest/p9_mss_eff_grouping.H
+++ b/src/import/chips/p9/procedures/hwp/nest/p9_mss_eff_grouping.H
@@ -70,7 +70,7 @@ const uint8_t BASE_ADDR = 3;         // Base Address
 // Number of memory regions
 const uint8_t NUM_NON_MIRROR_REGIONS = 8;
 const uint8_t NUM_MIRROR_REGIONS = 4;
-const uint8_t NUM_OF_CHTM_REGIONS = 24;        // 24 CHTM memory regions
+const uint8_t NUM_OF_CHTM_REGIONS = 24;   // 24 CHTM memory regions
 const uint8_t NUM_OF_ALT_MEM_REGIONS = 2; // 2 memory holes
 
 // ------------------------------------------------------
diff --git a/src/import/chips/p9/procedures/hwp/nest/p9_mss_setup_bars.C b/src/import/chips/p9/procedures/hwp/nest/p9_mss_setup_bars.C
index 191b64cea..c2dbbd186 100644
--- a/src/import/chips/p9/procedures/hwp/nest/p9_mss_setup_bars.C
+++ b/src/import/chips/p9/procedures/hwp/nest/p9_mss_setup_bars.C
@@ -48,10 +48,8 @@
 ///----------------------------------------------------------------------------
 /// Constant definitions
 ///----------------------------------------------------------------------------
-const uint8_t MAX_MCS_PER_PROC     = 4;        // 4 MCS per proc chip
 const uint8_t MAX_MC_PORTS_PER_MCS = 2;        // 2 MC ports per MCS
 const uint8_t NO_CHANNEL_PER_GROUP = 0xFF;     // Init value of channel per group
-const uint8_t MAX_ALT_MEM_REGIONS  = 2;        // Max num of memory holes
 
 ///----------------------------------------------------------------------------
 /// Data structure definitions
@@ -145,9 +143,9 @@ struct mcPortGroupInfo_t
     uint8_t channelId;
 
     // ALT_MEM
-    uint8_t altMemValid[MAX_ALT_MEM_REGIONS];
-    uint32_t altMemSize[MAX_ALT_MEM_REGIONS];
-    uint32_t altBaseAddr[MAX_ALT_MEM_REGIONS];
+    uint8_t altMemValid[NUM_OF_ALT_MEM_REGIONS];
+    uint32_t altMemSize[NUM_OF_ALT_MEM_REGIONS];
+    uint32_t altBaseAddr[NUM_OF_ALT_MEM_REGIONS];
 };
 
 /**
@@ -189,14 +187,14 @@ struct mcBarData_t
     uint32_t MCFGPM_groupBaseAddr;
 
     // Info to program MCFGPA reg
-    bool     MCFGPA_HOLE_valid[MAX_ALT_MEM_REGIONS];
-    uint32_t MCFGPA_HOLE_LOWER_addr[MAX_ALT_MEM_REGIONS];
-    uint32_t MCFGPA_HOLE_UPPER_addr[MAX_ALT_MEM_REGIONS];
+    bool     MCFGPA_HOLE_valid[NUM_OF_ALT_MEM_REGIONS];
+    uint32_t MCFGPA_HOLE_LOWER_addr[NUM_OF_ALT_MEM_REGIONS];
+    uint32_t MCFGPA_HOLE_UPPER_addr[NUM_OF_ALT_MEM_REGIONS];
 
     // Info to program MCFGPMA reg
-    bool     MCFGPMA_HOLE_valid[MAX_ALT_MEM_REGIONS];
-    uint32_t MCFGPMA_HOLE_LOWER_addr[MAX_ALT_MEM_REGIONS];
-    uint32_t MCFGPMA_HOLE_UPPER_addr[MAX_ALT_MEM_REGIONS];
+    bool     MCFGPMA_HOLE_valid[NUM_OF_ALT_MEM_REGIONS];
+    uint32_t MCFGPMA_HOLE_LOWER_addr[NUM_OF_ALT_MEM_REGIONS];
+    uint32_t MCFGPMA_HOLE_UPPER_addr[NUM_OF_ALT_MEM_REGIONS];
 };
 
 ///----------------------------------------------------------------------------
@@ -266,7 +264,6 @@ fapi2::ReturnCode getMcMemSize(
     uint64_t& o_mcSize)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
 
     // Figure out the amount of memory behind this MCS
     // by adding up all memory from its MCA ports
@@ -301,7 +298,6 @@ fapi2::ReturnCode getMcMemSize(
     uint64_t& o_mcSize)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
 
     // Figure out the amount of memory behind this MI
     // by adding up all memory from its DMI ports
@@ -349,7 +345,6 @@ void getGroupDataMcMemSize(
     uint64_t& o_mcSize)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
 
     // Loop thru non-mirror groups (0-7)
     for (uint8_t l_group = 0; l_group < (DATA_GROUPS / 2); l_group++)
@@ -408,7 +403,7 @@ fapi2::ReturnCode validateGroupData(
     const uint32_t i_groupData[DATA_GROUPS][DATA_ELEMENTS])
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
+
     uint64_t l_mcSize = 0;
     uint64_t l_mcSizeGroupData = 0;
     uint8_t l_portFound[NUM_MC_PORTS_PER_PROC];
@@ -487,7 +482,6 @@ fapi2::ReturnCode getGroupSizeEncodedValue(
     uint32_t& o_value)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
 
     bool l_sizeFound = false;
 
@@ -516,7 +510,7 @@ fapi2::ReturnCode getGroupSizeEncodedValue(
                      .set_MC_POS(l_mcPos)
                      .set_GROUP_SIZE(i_groupSize),
                      "Error: Can't locate Group size value in GROUP_SIZE_TABLE. "
-                     "MC pos: %d, GroupSize u% GB.", l_mcPos, i_groupSize );
+                     "MC pos: %d, GroupSize %u GB.", l_mcPos, i_groupSize );
     }
 
 fapi_try_exit:
@@ -540,7 +534,6 @@ fapi2::ReturnCode getNonMirrorBarData(const fapi2::Target<T>& i_mcTarget,
                                       mcBarData_t& o_mcBarData)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
 
     // This function assign the MCFGP_MC_CHANNELS_PER_GROUP value
     // to the MC according to the rule listed in the Nimbus/Cumulus workbook.
@@ -682,7 +675,7 @@ fapi2::ReturnCode getNonMirrorBarData(const fapi2::Target<T>& i_mcTarget,
     // ----------------------------------------------------
 
     // Alternate Memory MCFGPA
-    for (uint8_t ii = 0; ii < MAX_ALT_MEM_REGIONS; ii++)
+    for (uint8_t ii = 0; ii < NUM_OF_ALT_MEM_REGIONS; ii++)
     {
         if ( i_portInfo[0].altMemValid[ii] )
         {
@@ -725,20 +718,34 @@ fapi_try_exit:
 ///
 /// @param[in]  i_mcTarget     MC target (MCS/MI)
 /// @param[in]  i_portInfo     The port group info
-/// @param[in]  o_mcBarData    MC BAR data
+/// @param[in]  io_mcBarData   MC BAR data
 ///
 /// @return FAPI2_RC_SUCCESS if success, else error code.
 ///
 template<fapi2::TargetType T>
 fapi2::ReturnCode getMirrorBarData(const fapi2::Target<T>& i_mcTarget,
                                    const mcPortGroupInfo_t i_portInfo[],
-                                   mcBarData_t& o_mcBarData)
+                                   mcBarData_t& io_mcBarData)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
+
+    // ---------------------------------------------------
+    // Build MC register values for mirror groups
+    // ---------------------------------------------------
+
+    // Check MCFGP value to see if mirror is possible
+    // (See Table 1 of P9 Cumulus Memory Controller Workbook)
+    //
+    if ( (io_mcBarData.MCFGP_chan_per_group < 0b0101) ||
+         (io_mcBarData.MCFGP_chan_per_group > 0b1000) )
+    {
+        FAPI_IMP("Mirror is not possible with MCFGP = 0x%.8X, NO MIRROR is "
+                 "programmed. ", io_mcBarData.MCFGP_chan_per_group);
+        goto fapi_try_exit;
+    }
 
     // Set MCFGPM_VALID
-    o_mcBarData.MCFGPM_valid = true;
+    io_mcBarData.MCFGPM_valid = true;
 
     // ----------------------------------------------------
     // Determine data for MCFGPM register
@@ -746,33 +753,32 @@ fapi2::ReturnCode getMirrorBarData(const fapi2::Target<T>& i_mcTarget,
 
     // MCFGPM Group size
     FAPI_TRY(getGroupSizeEncodedValue(i_mcTarget, i_portInfo[1].groupSize,
-                                      o_mcBarData.MCFGPM_group_size),
+                                      io_mcBarData.MCFGPM_group_size),
              "getGroupSizeEncodedValue() returns error, l_rc 0x%.8X",
              (uint64_t)fapi2::current_err);
 
     // Group base address
-    o_mcBarData.MCFGPM_groupBaseAddr = i_portInfo[1].groupBaseAddr;
-
+    io_mcBarData.MCFGPM_groupBaseAddr = i_portInfo[1].groupBaseAddr;
 
     // ----------------------------------------------------
     // Determine data for MCFGPMA registers
     // ----------------------------------------------------
     // Alternate Memory MCFGPMA
-    for (uint8_t ii = 0; ii < MAX_ALT_MEM_REGIONS; ii++)
+    for (uint8_t ii = 0; ii < NUM_OF_ALT_MEM_REGIONS; ii++)
     {
         if ( i_portInfo[1].altMemValid[ii] )
         {
-            o_mcBarData.MCFGPMA_HOLE_valid[ii] = 1;
-            o_mcBarData.MCFGPMA_HOLE_LOWER_addr[ii] =
+            io_mcBarData.MCFGPMA_HOLE_valid[ii] = 1;
+            io_mcBarData.MCFGPMA_HOLE_LOWER_addr[ii] =
                 i_portInfo[1].altBaseAddr[ii];
-            o_mcBarData.MCFGPMA_HOLE_UPPER_addr[ii] =
+            io_mcBarData.MCFGPMA_HOLE_UPPER_addr[ii] =
                 i_portInfo[1].altBaseAddr[ii] + i_portInfo[1].altMemSize[ii];
         }
         else
         {
-            o_mcBarData.MCFGPMA_HOLE_valid[ii] = 0;
-            o_mcBarData.MCFGPMA_HOLE_LOWER_addr[ii] = 0;
-            o_mcBarData.MCFGPMA_HOLE_UPPER_addr[ii] = 0;
+            io_mcBarData.MCFGPMA_HOLE_valid[ii] = 0;
+            io_mcBarData.MCFGPMA_HOLE_LOWER_addr[ii] = 0;
+            io_mcBarData.MCFGPMA_HOLE_UPPER_addr[ii] = 0;
         }
 
     }
@@ -795,17 +801,25 @@ void displayMCPortInfoData(const mcPortGroupInfo_t i_portInfo[])
     for (uint8_t ii = 0; ii < MAX_MC_PORTS_PER_MCS; ii++)
     {
         FAPI_INF("    Port %u:", ii);
-        FAPI_INF("        myGroup %u", i_portInfo[ii].myGroup);
-        FAPI_INF("        numPortsInGroup %u", i_portInfo[ii].numPortsInGroup);
-        FAPI_INF("        groupSize %u", i_portInfo[ii].groupSize);
-        FAPI_INF("        groupBaseAddr 0x%.16llX", i_portInfo[ii].groupBaseAddr);
-        FAPI_INF("        channelId %u", i_portInfo[ii].channelId);
 
-        for (uint8_t jj = 0; jj < MAX_ALT_MEM_REGIONS; jj++)
+        if (i_portInfo[ii].numPortsInGroup > 0)
         {
-            FAPI_INF("        altMemValid[%u] %u", jj, i_portInfo[ii].altMemValid[jj]);
-            FAPI_INF("        altMemSize[%u]  %u", jj, i_portInfo[ii].altMemSize[jj]);
-            FAPI_INF("        altBaseAddr[%u] 0x%.16llX", jj, i_portInfo[ii].altBaseAddr[jj]);
+            FAPI_INF("        myGroup %u", i_portInfo[ii].myGroup);
+            FAPI_INF("        numPortsInGroup %u", i_portInfo[ii].numPortsInGroup);
+            FAPI_INF("        groupSize %u", i_portInfo[ii].groupSize);
+            FAPI_INF("        groupBaseAddr 0x%.16llX", i_portInfo[ii].groupBaseAddr);
+            FAPI_INF("        channelId %u", i_portInfo[ii].channelId);
+
+            for (uint8_t jj = 0; jj < NUM_OF_ALT_MEM_REGIONS; jj++)
+            {
+                FAPI_INF("        altMemValid[%u] %u", jj, i_portInfo[ii].altMemValid[jj]);
+                FAPI_INF("        altMemSize[%u]  %u", jj, i_portInfo[ii].altMemSize[jj]);
+                FAPI_INF("        altBaseAddr[%u] 0x%.16llX", jj, i_portInfo[ii].altBaseAddr[jj]);
+            }
+        }
+        else
+        {
+            FAPI_INF("        Not configured");
         }
     }
 
@@ -835,7 +849,7 @@ void displayMCBarData(const uint8_t i_mcPosition,
     FAPI_INF("        MCFGPM_group_size %u", i_mcBarData.MCFGPM_group_size);
     FAPI_INF("        MCFGPM_groupBaseAddr 0x%.16llX", i_mcBarData.MCFGPM_groupBaseAddr);
 
-    for (uint8_t jj = 0; jj < MAX_ALT_MEM_REGIONS; jj++)
+    for (uint8_t jj = 0; jj < NUM_OF_ALT_MEM_REGIONS; jj++)
     {
         FAPI_INF("        MCFGPA_HOLE_valid[%u]      %u", jj, i_mcBarData.MCFGPA_HOLE_valid[jj]);
         FAPI_INF("        MCFGPA_HOLE_LOWER_addr[%u] %u", jj, i_mcBarData.MCFGPA_HOLE_LOWER_addr[jj]);
@@ -868,7 +882,6 @@ void getPortData(const bool i_nonMirror,
                  mcPortGroupInfo_t o_portInfo[MAX_MC_PORTS_PER_MCS])
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
 
     // Non-mirrored groups: 0->7
     // Mirrored groups: 8->11
@@ -881,8 +894,6 @@ void getPortData(const bool i_nonMirror,
         l_endGroup = (MIRR_OFFSET + NUM_MIRROR_REGIONS);
     }
 
-    FAPI_INF("getPortData for %s group", i_nonMirror ? "non-mirrored" : "mirrored");
-
     // Loop thru specified groups
     for (uint8_t l_group = l_startGroup; l_group < l_endGroup; l_group++)
     {
@@ -913,7 +924,7 @@ void getPortData(const bool i_nonMirror,
                 o_portInfo[l_mcPortNum].channelId = l_memberIdx;
 
                 // ALT memory regions
-                for (uint8_t ii = 0; ii < MAX_ALT_MEM_REGIONS; ii++)
+                for (uint8_t ii = 0; ii < NUM_OF_ALT_MEM_REGIONS; ii++)
                 {
                     if (i_groupData[l_group][ALT_VALID(ii)])
                     {
@@ -928,7 +939,7 @@ void getPortData(const bool i_nonMirror,
 
     } // Group loop
 
-    // If one of MC port is configured in a group, get the BAR data
+    // Set channel ID for certain scenario
     if ( (o_portInfo[0].numPortsInGroup > 0) ||
          (o_portInfo[1].numPortsInGroup > 0) )
     {
@@ -942,11 +953,13 @@ void getPortData(const bool i_nonMirror,
             o_portInfo[0].channelId = o_portInfo[1].channelId;
             o_portInfo[1].channelId = 0;
         }
-
-        // Display MC port info data
-        displayMCPortInfoData(o_portInfo);
     }
 
+    // Display MC port info data
+    FAPI_INF("getPortData from %s group - Results for MCS/MI pos %d",
+             i_nonMirror ? "NON-MIRROR" : "MIRROR", i_mcPos);
+    displayMCPortInfoData(o_portInfo);
+
     FAPI_DBG("Exit");
     return;
 }
@@ -969,7 +982,7 @@ fapi2::ReturnCode buildMCBarData(
     std::vector<std::pair<fapi2::Target<T>, mcBarData_t>>& o_mcBarDataPair)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
+
     char l_targetStr[fapi2::MAX_ECMD_STRING_LEN];
     const fapi2::Target<fapi2::TARGET_TYPE_SYSTEM> FAPI_SYSTEM;
     fapi2::ATTR_MRW_HW_MIRRORING_ENABLE_Type l_mirror_ctl;
@@ -1011,25 +1024,21 @@ fapi2::ReturnCode buildMCBarData(
                      "getNonMirrorBarData() returns error, l_rc 0x%.8X",
                      (uint64_t)fapi2::current_err);
 
-            // ---------------------------------------------------
-            // Build MC register values for mirror groups
-            // Nimbus doesn't use mirror, so this block should
-            // only be run on Cumulus if mirror memory is enabled
-            // ---------------------------------------------------
-            // ******* Need to also program Mirror memory for Cumulus. ********
-            if ( (l_mirror_ctl == fapi2::ENUM_ATTR_MRW_HW_MIRRORING_ENABLE_TRUE) &&
-                 (l_mcBarData.MCFGPM_valid == false) )
+            // ---------------------------------------------------------------
+            // Set MC register values for mirror groups
+            //   - Nimbus:  No mirror
+            //   - Cumulus: If ATTR_MRW_HW_MIRRORING_ENABLE = true
+            // ---------------------------------------------------------------
+            if (l_mirror_ctl == fapi2::ENUM_ATTR_MRW_HW_MIRRORING_ENABLE_TRUE)
             {
-                FAPI_INF("Building MC register data for mirrored group - Cumulus only");
-
+                FAPI_INF("ATTR_MRW_HW_MIRRORING_ENABLE is enabled: checking mirrored groups");
                 mcPortGroupInfo_t l_portInfoMirrored[MAX_MC_PORTS_PER_MCS];
-
                 // Get port data from mirrored groups (false = mirrored)
                 getPortData(false, l_unitPos, i_groupData, l_portInfoMirrored);
 
-                // If one of MC port is configured in a group, get the BAR data
-                if ( (l_portInfoMirrored[0].numPortsInGroup > 0) ||
-                     (l_portInfoMirrored[1].numPortsInGroup > 0) )
+                // If at least 2MC ports/group, get the mirror BAR data
+                if ( (l_portInfoMirrored[0].numPortsInGroup >= 2) &&
+                     (l_portInfoMirrored[1].numPortsInGroup >= 2) )
                 {
                     // ---- Build MCFGM data based on port group info ----
                     FAPI_TRY(getMirrorBarData(l_mc, l_portInfoMirrored, l_mcBarData),
@@ -1069,7 +1078,7 @@ fapi2::ReturnCode writeMCBarData(
     const std::vector<std::pair<fapi2::Target<T>, mcBarData_t>>& i_mcBarDataPair)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
+
     fapi2::buffer<uint64_t> l_scomData(0);
 
     fapi2::ATTR_MSS_INTERLEAVE_GRANULARITY_Type l_interleave_granule_size;
@@ -1293,7 +1302,7 @@ fapi2::ReturnCode unmaskMCFIR(
     const std::vector<std::pair<fapi2::Target<T>, mcBarData_t>>& i_mcBarDataPair)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
+
     fapi2::buffer<uint64_t> l_mcfiraction;
     fapi2::buffer<uint64_t> l_mcfirmask_and;
 
@@ -1340,7 +1349,7 @@ fapi2::ReturnCode p9_mss_setup_bars(
     const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target)
 {
     FAPI_DBG("Entering");
-    fapi2::ReturnCode l_rc;
+
     uint8_t l_mem_ipl_complete = 1;
 
     // Stores data read from ATTR_MSS_MCS_GROUP_32