Fix HBRT Adjunct Hang due to Multi-Node Sync System Attributes Bugs

Make fixes to the sync system attributes block of code that caused it to make
the HBRT adjunct partition fail with one of various symptoms including a crash,
stack corruption, or HBRT command failing.

Also refactor the getNextTarget() code to reduce additional tracing that was
added recently since too many traces are being generated.

Change-Id: I0d51d3a20ac87ce4b3378310b2164d20a185d76f
RTC: 190816
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/57278
Reviewed-by: Christian R. Geddes <crgeddes@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Reviewed-by: Prachi Gupta <pragupta@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Reviewed-by: William G. Hoffa <wghoffa@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>

3 files changed, 47 insertions, 68 deletions

diff --git a/src/usr/targeting/common/target.C b/src/usr/targeting/common/target.C
index 90bfd6850..37489a774 100644
--- a/src/usr/targeting/common/target.C
+++ b/src/usr/targeting/common/target.C
@@ -182,9 +182,9 @@ bool Target::_trySetAttr(
     // Get the node ID associated with the input target
     NODE_ID l_nodeId = NODE0;
     l_attrRP->getNodeId(this, l_nodeId);
-    bool isSysTarget = /* ((this->getAttr<ATTR_CLASS>() == CLASS_SYS) &&
+    bool isSysTarget = ((this->getAttr<ATTR_CLASS>() == CLASS_SYS) &&
                         (this->getAttr<ATTR_TYPE>() == TYPE_SYS))
-                     ? true : @TODO RTC:190816 */ false;
+                     ? true : false;
 #endif
 
     // Figure out if effort should be expended figuring out the target's type/
@@ -244,8 +244,10 @@ bool Target::_trySetAttr(
 #ifdef __HOSTBOOT_RUNTIME
         if(isSysTarget)
         {
+            uint8_t l_nodeCount = l_attrRP->getNodeCount();
+
             for(NODE_ID l_nodeX = NODE0;
-                l_nodeX < l_attrRP->getNodeCount();
+                l_nodeX < l_nodeCount;
                 ++l_nodeX)
             {
                 if(l_nodeX == l_nodeId)
@@ -254,30 +256,18 @@ bool Target::_trySetAttr(
                     continue;
                 }
 
-                // Get target map pointer for the node
-                void* l_pMap = l_attrRP->getTargetMapPtr(l_nodeX);
-
-                // Get pointer to targets
-                Target (*l_pTargets)[] = reinterpret_cast<Target(*)[]>(l_pMap);
-
-                // Get pointer to number of targets
-                uint32_t *l_pNumTargets =
-                    reinterpret_cast<uint32_t*>(l_pMap) - 1;
-
                 // Walk through targets
-                for(uint32_t l_targetNum = 0;
-                    l_targetNum < *l_pNumTargets;
-                    ++l_targetNum)
+                for(TargetIterator pIt = targetService().begin(l_nodeX);
+                    pIt != TARGETING::targetService().end();
+                    ++pIt)
                 {
-                    Target* l_pTarget = &(*(l_pTargets))[l_targetNum];
-
                     // Check for system target
-                    if((l_pTarget->getAttr<ATTR_CLASS>() == CLASS_SYS) &&
-                       (l_pTarget->getAttr<ATTR_TYPE>() == TYPE_SYS))
+                    if(((*pIt)->getAttr<ATTR_CLASS>() == CLASS_SYS) &&
+                       ((*pIt)->getAttr<ATTR_TYPE>() == TYPE_SYS))
                     {
                         // Get pointer to the attribute being set
                         void* l_pAttrDataNodeX = NULL;
-                        l_pTarget->_getAttrPtr(i_attr, l_pAttrDataNodeX);
+                        (*pIt)->_getAttrPtr(i_attr, l_pAttrDataNodeX);
                         if (l_pAttrData)
                         {
                             // Set the attribute for this node
diff --git a/src/usr/targeting/common/targetservice.C b/src/usr/targeting/common/targetservice.C
index 3db2f75c7..940c5dd39 100644
--- a/src/usr/targeting/common/targetservice.C
+++ b/src/usr/targeting/common/targetservice.C
@@ -510,68 +510,52 @@ Target* TargetService::getNextTarget(const Target* i_pTarget) const
 
     if(l_pTarget != NULL)
     {
-        for(uint8_t i_node=0; i_node<MAX_NODE_ID; ++i_node)
+        for(uint8_t l_node=0; l_node<MAX_NODE_ID; ++l_node)
         {
-            if((iv_nodeData[i_node].initialized) &&
-                (iv_nodeData[i_node].maxTargets > 0) &&
-                ((l_pTarget >= &(*(iv_nodeData[i_node].targets))[0]) &&
-                    (l_pTarget <= &(*(iv_nodeData[i_node].targets))[
-                                iv_nodeData[i_node].maxTargets - 1])))
+            // Node data is not initialized or has 0 targets
+            if( !(iv_nodeData[l_node].initialized) ||
+                (iv_nodeData[l_node].maxTargets == 0))
             {
-                if( l_pTarget == &(*(iv_nodeData[i_node].targets))[iv_nodeData[
-                            i_node].maxTargets - 1] )
+                TARG_ERR("getNextTarget: For node %d, initialized %d, "
+                         "maximum targets %d",
+                         l_node,
+                         iv_nodeData[l_node].initialized,
+                         iv_nodeData[l_node].maxTargets);
+            }
+            // Starting target is last target on its node
+            else if( l_pTarget == &(*(iv_nodeData[l_node].targets))[iv_nodeData[
+                            l_node].maxTargets - 1] )
+            {
+                // Go for next node
+                uint8_t l_nextNode = getNextInitializedNode(
+                                        static_cast<NODE_ID>(l_node));
+                if(l_nextNode < MAX_NODE_ID)
                 {
-                    // Go for next node
-                    uint8_t l_nextNode = getNextInitializedNode(
-                                            static_cast<NODE_ID>(i_node));
-                    TARG_INF("getNextTarget: Using next node %d", l_nextNode);
-                    if(l_nextNode < MAX_NODE_ID)
-                    {
-                        l_pTarget = &(*(iv_nodeData[l_nextNode].targets))[0];
-                        l_targetFound = true;
-                        break;
-                    }
-                    else
-                    {
-                        l_targetFound = false;
-                        break;
-                    }
+                    TARG_DBG("getNextTarget: Switched to node %d", l_nextNode);
+                    l_pTarget = &(*(iv_nodeData[l_nextNode].targets))[0];
+                    l_targetFound = true;
+                    break;
                 }
                 else
                 {
-                    ++l_pTarget;
-                    l_targetFound = true;
+                    l_targetFound = false;
                     break;
                 }
             }
-            else if( !(iv_nodeData[i_node].initialized) ||
-                     (iv_nodeData[i_node].maxTargets == 0))
+            // Starting target is in range for its node, but not last target
+            else if((l_pTarget >= &(*(iv_nodeData[l_node].targets))[0]) &&
+                    (l_pTarget < &(*(iv_nodeData[l_node].targets))[
+                                iv_nodeData[l_node].maxTargets - 1]))
             {
-                TARG_ERR("getNextTarget: For node %d, initialized %d, "
-                         "maximum targets %d",
-                         i_node,
-                         iv_nodeData[i_node].initialized,
-                         iv_nodeData[i_node].maxTargets);
-            }
-        }
-
-        if(l_targetFound == false)
-        {
-            for(uint8_t i_node=0; i_node<MAX_NODE_ID; ++i_node)
-            {
-                TARG_ERR("getNextTarget: Node %d targets: first %p, "
-                         "current %p, last %p",
-                         i_node,
-                         &(*(iv_nodeData[i_node].targets))[0],
-                         l_pTarget,
-                         &(*(iv_nodeData[i_node].targets))[
-                                iv_nodeData[i_node].maxTargets - 1]);
+                ++l_pTarget;
+                l_targetFound = true;
+                break;
             }
         }
     }
     if(l_targetFound == false)
     {
-        TARG_ERR("getNextTarget: Target not found");
+        TARG_DBG("getNextTarget: Target not found");
         l_pTarget = NULL;
     }
 
diff --git a/src/usr/targeting/runtime/attrrp_rt.C b/src/usr/targeting/runtime/attrrp_rt.C
index 9f8cce8a6..55be8eaf9 100644
--- a/src/usr/targeting/runtime/attrrp_rt.C
+++ b/src/usr/targeting/runtime/attrrp_rt.C
@@ -332,6 +332,8 @@ namespace TARGETING
                 if(iv_nodeContainer[i_nodeId].pTargetMap == nullptr)
                 {
                     // Locate targeting image for this node in reserved memory
+                    TARG_INF("getTargetMapPtr Locating reserved memory "
+                             "targeting image for the node %d", i_nodeId);
                     uint64_t attr_size = 0;
                     iv_nodeContainer[i_nodeId].pTargetMap =
                         reinterpret_cast<void*>(
@@ -366,6 +368,7 @@ namespace TARGETING
                 else
                 {
                     // This should return pTargetMap from here
+                    l_pTargetMap = iv_nodeContainer[i_nodeId].pTargetMap;
                     break;
                 }
             } while(0);
@@ -382,6 +385,8 @@ namespace TARGETING
             errlCommit(l_errl, TARG_COMP_ID);
         }
 
+        TARG_DBG("getTargetMapPtr returning %p for node %d",
+                 l_pTargetMap, i_nodeId);
         return l_pTargetMap;
         #undef TARG_FN
     }