Hostboot Serviceability Review Part 2

Resolving TODOs and ensuring error logs have correct callouts

Change-Id: Ic2e65427487fb91553ffe4ed6e3ed922004963ba
RTC: 92837
Reviewed-on: http://gfw160.aus.stglabs.ibm.com:8080/gerrit/8374
Tested-by: Jenkins Server
Reviewed-by: A. Patrick Williams III <iawillia@us.ibm.com>

9 files changed, 65 insertions, 136 deletions

diff --git a/src/usr/targeting/attrrp.C b/src/usr/targeting/attrrp.C
index d51dcc422..6a27ba14a 100644
--- a/src/usr/targeting/attrrp.C
+++ b/src/usr/targeting/attrrp.C
@@ -228,16 +228,16 @@ namespace TARGETING
                  *              request that is invalid for the attribute
                  *              section containing the address.
                  */
+                const bool hbSwError = true;
                 errlHndl_t l_errl = new ErrlEntry(ERRL_SEV_UNRECOVERABLE,
                                                   TARG_MOD_ATTRRP,
                                                   TARG_RC_ATTR_MSG_FAIL,
                                                   vAddr,
                                                   TWO_UINT32_TO_UINT64(
                                                         msg->type,
-                                                        section)
-                                                  );
+                                                        section),
+                                                  hbSwError);
                 errlCommit(l_errl,TARG_COMP_ID);
-
             }
 
             // Respond to kernel request.
@@ -377,11 +377,13 @@ namespace TARGETING
                          *
                          *   @devdesc    Found unhandled attribute section type
                          */
-                    l_errl = new ErrlEntry(ERRL_SEV_UNRECOVERABLE,
-                                           TARG_MOD_ATTRRP,
-                                           TARG_RC_UNHANDLED_ATTR_SEC_TYPE,
-                                           iv_sections[i].type);
-                    break;
+                        const bool hbSwError = true;
+                        l_errl = new ErrlEntry(ERRL_SEV_UNRECOVERABLE,
+                                               TARG_MOD_ATTRRP,
+                                               TARG_RC_UNHANDLED_ATTR_SEC_TYPE,
+                                               iv_sections[i].type,
+                                               0, hbSwError);
+                        break;
                 }
 
                 if(l_errl)
@@ -415,11 +417,12 @@ namespace TARGETING
                      *              memory block for an attribute section, the
                      *              kernel returned an error.
                      */
+                    const bool hbSwError = true;
                     l_errl = new ErrlEntry(ERRL_SEV_UNRECOVERABLE,
                                            TARG_MOD_ATTRRP,
                                            TARG_RC_MM_BLOCK_FAIL,
                                            iv_sections[i].vmmAddress,
-                                           rc);
+                                           rc, hbSwError);
                     break;
                 }
 
@@ -452,11 +455,13 @@ namespace TARGETING
                      *              a virtual memory block for an attribute
                      *              section, the kernel returned an error.
                      */
+                    const bool hbSwError = true;
                     l_errl = new ErrlEntry(ERRL_SEV_UNRECOVERABLE,
                                            TARG_MOD_ATTRRP,
                                            TARG_RC_MM_PERM_FAIL,
                                            iv_sections[i].vmmAddress,
-                                           TWO_UINT32_TO_UINT64(rc, l_perm));
+                                           TWO_UINT32_TO_UINT64(rc, l_perm),
+                                           hbSwError);
                     break;
                 }
 
diff --git a/src/usr/targeting/attrrp_common.C b/src/usr/targeting/attrrp_common.C
index 3d013005b..a93c13451 100644
--- a/src/usr/targeting/attrrp_common.C
+++ b/src/usr/targeting/attrrp_common.C
@@ -176,8 +176,5 @@ namespace TARGETING
                 break;
             }
         }
-        // $TODO what if we dont find it??
     }
-
-
 }
diff --git a/src/usr/targeting/common/genHwsvMrwXml.pl b/src/usr/targeting/common/genHwsvMrwXml.pl
index 75e72a3af..4154028a5 100755
--- a/src/usr/targeting/common/genHwsvMrwXml.pl
+++ b/src/usr/targeting/common/genHwsvMrwXml.pl
@@ -1992,9 +1992,6 @@ sub generate_proc
 
     }
 
-    #TODO RTC [59707]
-    #Update Lane equalization values
-
     # add EEPROM attributes
     addEeproms($sys, $node, $proc);
 
diff --git a/src/usr/targeting/common/target.C b/src/usr/targeting/common/target.C
index 6d619a7ab..96067fb25 100644
--- a/src/usr/targeting/common/target.C
+++ b/src/usr/targeting/common/target.C
@@ -5,7 +5,7 @@
 /*                                                                        */
 /* IBM CONFIDENTIAL                                                       */
 /*                                                                        */
-/* COPYRIGHT International Business Machines Corp. 2011,2013              */
+/* COPYRIGHT International Business Machines Corp. 2011,2014              */
 /*                                                                        */
 /* p1                                                                     */
 /*                                                                        */
@@ -268,9 +268,10 @@ mutex_t* Target::_getHbMutexAttr(
     void* l_pAttr = NULL;
     (void)_getAttrPtr(i_attribute,l_pAttr);
 
-    //@TODO Remove assert once release has stablized
-    TARG_ASSERT(l_pAttr,"TARGETING::Target::_getHbMutexAttr<%d>: _getAttrPtr "
-           "returned NULL",i_attribute);
+    if (unlikely(l_pAttr == NULL))
+    {
+        targAssert(GET_HB_MUTEX_ATTR, i_attribute);
+    }
 
     return static_cast<mutex_t*>(l_pAttr);
 
@@ -515,6 +516,41 @@ uint8_t Target::getAttrTankTargetUnitPos() const
 }
 
 //******************************************************************************
+// Target::targAssert()
+//******************************************************************************
+void Target::targAssert(TargAssertReason i_reason,
+                        uint32_t i_ffdc)
+{
+    switch (i_reason)
+    {
+    case SET_ATTR:
+        TARG_ASSERT(false,
+            "TARGETING::Target::setAttr<0x%7x>: trySetAttr returned false",
+            i_ffdc);
+        break;
+    case GET_ATTR:
+        TARG_ASSERT(false,
+            "TARGETING::Target::getAttr<0x%7x>: tryGetAttr returned false",
+            i_ffdc);
+        break;
+    case GET_ATTR_AS_STRING:
+        TARG_ASSERT(false,
+            "TARGETING::Target::getAttrAsString<0x%7x>: tryGetAttr returned false",
+            i_ffdc);
+        break;
+    case GET_HB_MUTEX_ATTR:
+        TARG_ASSERT(false,
+            "TARGETING::Target::_getHbMutexAttr<0x%7x>: _getAttrPtr returned NULL",
+            i_ffdc);
+        break;
+    default:
+        TARG_ASSERT(false,
+            "TARGETING function asserted for unknown reason (0x%x)",
+            i_ffdc);
+    }
+}
+
+//******************************************************************************
 // Target::installWriteAttributeCallback
 //******************************************************************************
 bool Target::installWriteAttributeCallback(
diff --git a/src/usr/targeting/common/test/testcommontargeting.H b/src/usr/targeting/common/test/testcommontargeting.H
index 0f1cceee1..d94315d41 100644
--- a/src/usr/targeting/common/test/testcommontargeting.H
+++ b/src/usr/targeting/common/test/testcommontargeting.H
@@ -5,7 +5,7 @@
 /*                                                                        */
 /* IBM CONFIDENTIAL                                                       */
 /*                                                                        */
-/* COPYRIGHT International Business Machines Corp. 2011,2013              */
+/* COPYRIGHT International Business Machines Corp. 2011,2014              */
 /*                                                                        */
 /* p1                                                                     */
 /*                                                                        */
@@ -1414,73 +1414,6 @@ class CommonTargetingTestSuite: public CxxTest::TestSuite
         TARG_TS_TRACE(EXIT_MRK "testFapiToHbMacros" );
     }
 
-    void testFapiAttributes ()
-    {
-
-        TARG_TS_TRACE(ENTER_MRK "testFapiAttributes" );
-
-        using namespace TARGETING;
-        using namespace fapi;
-
-        fapi::ReturnCode l_rc;
-
-        //----------------------------------------------------------------------
-        // Test ATTR_MSS_EFF_PRIMARY_RANK
-        //----------------------------------------------------------------------
-
-// @TODO ATTR_MSS_EFF_PRIMARY_RANK - Enable once we pull in XML file with this attribute.
-#if ATTR_MSS_EFF_PRIMARY_RANK
-        // Find all MBA's in this system
-        PredicateCTM l_mba_pred(CLASS_UNIT,TYPE_MBA);
-        TargetRangeFilter l_mba_filter(
-                                       targetService().begin(),
-                                       targetService().end(),
-                                       &l_mba_pred);
-        // Loop through all MBA's and validate the attribute
-        for(;l_mba_filter;++l_mba_filter)
-        {
-            if(l_mba_filter->getAttr<ATTR_TYPE>() != TYPE_MBA)
-            {
-                TARG_TS_FAIL("Should only have returned type MBA!");
-                break;
-            }
-            else
-            {
-                uint8_t l_uint8_mss_eff_primary_rank[4][4];
-
-                fapi::Target* l_pFapiTarget = new fapi::Target(TARGET_TYPE_MBA_CHIPLET, *l_mba_filter);
-
-                // Test get
-                l_rc = FAPI_ATTR_GET(ATTR_MSS_EFF_PRIMARY_RANK, l_pFapiTarget,l_uint8_mss_eff_primary_rank);
-                if (l_rc)
-                {
-                    TARG_TS_FAIL("hwpTestAttributes: ATTR_MSS_EFF_PRIMARY_RANK. Error from GET");
-                    break;
-                }
-
-                // Test set
-                l_rc = FAPI_ATTR_SET(ATTR_MSS_EFF_PRIMARY_RANK, l_pFapiTarget, l_uint8_mss_eff_primary_rank);
-                if (l_rc)
-                {
-                    TARG_TS_FAIL("hwpTestAttributes: ATTR_MSS_EFF_PRIMARY_RANK. Error from SET");
-                    break;
-                }
-
-                delete l_pFapiTarget;
-                l_pFapiTarget = NULL;
-                TARG_TS_TRACE("hwpTestAttributes: Test of ATTR_MSS_EFF_PRIMARY_RANK Passed!");
-            }
-        }
-#endif
-        TARG_TS_TRACE(EXIT_MRK "testFapiAttributes" );
-
-        if(l_rc)
-        {
-            errlHndl_t l_pErrLog = static_cast<errlHndl_t>(l_rc.releasePlatData());
-            COMMON::ERRORLOG::errlCommit(l_pErrLog,TARG_COMP_ID);
-        }
-    }
-
     void testPodNumericalTypes()
     {
         TARG_TS_TRACE(ENTER_MRK "testPodNumericalTypes" );
diff --git a/src/usr/targeting/common/xmltohb/attribute_types.xml b/src/usr/targeting/common/xmltohb/attribute_types.xml
index 5ba9d01c3..64d2d83af 100644
--- a/src/usr/targeting/common/xmltohb/attribute_types.xml
+++ b/src/usr/targeting/common/xmltohb/attribute_types.xml
@@ -1927,26 +1927,6 @@
 </attribute>
 
 <attribute>
-    <id>MVPD_FREQ_CORE_NOMINAL</id>
-    <description>
-        Chip attribute.
-        The nominal frequency of the processor cores in MHz.
-        TODO This attribute should be from MVPD.
-             When MVPD function is present, this attribute should be deleted.
-             The corresponding HWPF attribute request should map to a function
-             that calls the MVPD driver (in the same way as DIMM SPD).
-             For now, just default to 3GHz
-    </description>
-    <simpleType><uint32_t><default>3000</default></uint32_t></simpleType>
-    <persistency>non-volatile</persistency>
-    <readable/>
-    <hwpfToHbAttrMap>
-        <id>ATTR_MVPD_FREQ_CORE_NOMINAL</id>
-        <macro>DIRECT</macro>
-    </hwpfToHbAttrMap>
-</attribute>
-
-<attribute>
     <id>HUID</id>
     <description>
         Hardware Unit ID
@@ -2938,24 +2918,12 @@
 
 <attribute>
     <id>MNFG_FLAGS</id>
-    <description> Provides the mnfg flags
-        in a single value.  The bits are
-        masked together so the single value
-        will have data on several mnfg flags.
-        Should note that the MNFG_FLAG_BIT enum
-        is used to provide the value of the
-        different mnfg flags.  A user can bitwise
-        'OR' several of the MNFG_FLAG_BIT values
-        together to indicate that multiple mnfg
-        flags are set.
-        @TODO via RTC: 35451
-        The HostBoot side can only read the mnfg
-        flag attribute. However the FSP side will
-        need to read and write the mnfg flag
-        attributes.  For now making it so both sides
-        are read and writeable but will need to revisit
-        to find a better way to handle persistency when
-        the hostboot and FSP differ.
+    <description>
+        Provides the manufacturing flags. This is a bitfield.
+        Multiple flags can be set at once. Use MNFG_FLAG_BIT
+        to decode. Expected use-case is for FSP to write this
+        attribute based on the MNFG component flags and for
+        HWSV/Hostboot to read it.
     </description>
     <simpleType>
         <uint64_t>
@@ -4750,8 +4718,7 @@ Divider for the 1us PBAX hang pulse.  A hang is detected after two divided hang
 
       Consumer: proc_pcbs_init.C
 
-      TODO: Dean said this may either be provided by the Machine Readable
-            Workbook or Todd R's power management def file.
+      Provided by the Machine Readable Workbook
     </description>
     <simpleType>
         <uint8_t></uint8_t>
@@ -4981,9 +4948,7 @@ Divider for the 1us PBAX hang pulse.  A hang is detected after two divided hang
 
       Consumers: proc_build_gpstate_table.C (among others)
 
-      TODO: Dean's proposal is that each platform will iterate over all chips,
-            reading the super-turbo frequency from MVPD #V and set this attribute
-            to the lowest value.
+      Set by the HWSV freq/voltage service based on MVPD #V
     </description>
     <simpleType>
         <uint32_t></uint32_t>
diff --git a/src/usr/targeting/common/xmltohb/common.mk b/src/usr/targeting/common/xmltohb/common.mk
index 24b400440..09e634eb1 100644
--- a/src/usr/targeting/common/xmltohb/common.mk
+++ b/src/usr/targeting/common/xmltohb/common.mk
@@ -5,7 +5,7 @@
 #
 # IBM CONFIDENTIAL
 #
-# COPYRIGHT International Business Machines Corp. 2011,2013
+# COPYRIGHT International Business Machines Corp. 2011,2014
 #
 # p1
 #
@@ -72,7 +72,6 @@ FAPI_ATTR_SOURCES =          \
     dimm_attributes.xml      \
     unit_attributes.xml      \
     freq_attributes.xml      \
-    proc_mvpd_attributes.xml \
     ei_bus_attributes.xml    \
     dram_initialization/proc_setup_bars/proc_setup_bars_memory_attributes.xml \
     dram_initialization/proc_setup_bars/proc_setup_bars_l3_attributes.xml \
diff --git a/src/usr/targeting/common/xmltohb/target_types.xml b/src/usr/targeting/common/xmltohb/target_types.xml
index b5580daf2..79fcd61db 100644
--- a/src/usr/targeting/common/xmltohb/target_types.xml
+++ b/src/usr/targeting/common/xmltohb/target_types.xml
@@ -314,8 +314,6 @@
     <attribute><id>HWAS_STATE_CHANGED_SUBSCRIPTION_MASK</id>
         <default>0x00000001</default> <!-- GARD -->
     </attribute>
-    <!-- TODO. MVPD_FREQ_CORE_NOMINAL to be deleted when MVPD available -->
-    <attribute><id>MVPD_FREQ_CORE_NOMINAL</id></attribute>
     <attribute><id>VPD_REC_NUM</id></attribute>
     <!-- New attributes for mss/proc_setup_bars -->
     <!--    proc_setup_bars_memory_attributes.xml   -->
diff --git a/src/usr/targeting/common/xmltohb/xmltohb.pl b/src/usr/targeting/common/xmltohb/xmltohb.pl
index 2cb9147a8..acd2fef97 100755
--- a/src/usr/targeting/common/xmltohb/xmltohb.pl
+++ b/src/usr/targeting/common/xmltohb/xmltohb.pl
@@ -4989,7 +4989,6 @@ sub generateTargetingImage {
     my($vmmConstsFile, $attributes, $Target_t) = @_;
 
     # 128 MB virtual memory offset between sections
-    #@TODO Need the final value after full host boot support is implemented.
     my $vmmSectionOffset = 128 * 1024 * 1024; # 128MB
 
     # Virtual memory addresses corresponding to the start of the targeting image