Fix predicateHwasChanged to only check subscribed bits

Found issue while running testGard11 in hwasGardTest.H
OBUS bricks were coming back changed but they did not change.

Change-Id: Icf2d27b06f8bf44dc2e0aefb881d9e2b50dcc677
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/67476
Reviewed-by: Nicholas E. Bofferding <bofferdn@us.ibm.com>
Reviewed-by: Christian R. Geddes <crgeddes@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-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: Daniel M. Crowell <dcrowell@us.ibm.com>

2 files changed, 186 insertions, 21 deletions

diff --git a/src/usr/targeting/common/predicates/predicatehwaschanged.C b/src/usr/targeting/common/predicates/predicatehwaschanged.C
index f694a4f85..1b801dbd6 100644
--- a/src/usr/targeting/common/predicates/predicatehwaschanged.C
+++ b/src/usr/targeting/common/predicates/predicatehwaschanged.C
@@ -39,6 +39,8 @@
 // Other Host Boot Components
 
 // Targeting Component
+#include <targeting/adapters/types.H>
+#include <targeting/common/trace.H>
 #include <targeting/common/attributes.H>
 #include <targeting/common/target.H>
 #include <targeting/common/predicates/predicates.H>
@@ -47,7 +49,6 @@
 //******************************************************************************
 // Macros
 //******************************************************************************
-
 #undef TARG_NAMESPACE
 #undef TARG_CLASS
 #undef TARG_FN
@@ -58,6 +59,7 @@
 
 namespace TARGETING
 {
+extern TARG_TD_t g_trac_targeting;
 
 #define TARG_NAMESPACE "TARGETING::"
 #define TARG_CLASS "PredicateHwasChanged::"
@@ -78,6 +80,8 @@ PredicateHwasChanged::~PredicateHwasChanged()
 
 PredicateHwasChanged& PredicateHwasChanged::reset()
 {
+    // resets to an invalid cleared state,
+    // user needs to identify a bit to check after this reset
     memset(&iv_valid,0x00,sizeof(iv_valid));
     memset(&iv_desired,0x00,sizeof(iv_desired));
     return *this;
@@ -95,12 +99,25 @@ bool PredicateHwasChanged::operator()(
     hwasStateChangedFlag actual = { rawValue: 0};
     CPPASSERT(sizeof(actual.attribute) <=
                 sizeof(actual.rawValue));
+
     actual.attribute =
             i_pTarget->getAttr<ATTR_HWAS_STATE_CHANGED_FLAG>();
     ATTR_HWAS_STATE_CHANGED_SUBSCRIPTION_MASK_type subscriptionMask =
             i_pTarget->getAttr<ATTR_HWAS_STATE_CHANGED_SUBSCRIPTION_MASK>();
 
-    return ((subscriptionMask != 0) &&
+    TRACDCOMP(g_trac_targeting,
+       TARG_FUNC "actual: 0x%.8X, subscription: 0x%.8X, iv_valid: 0x%.8X, iv_desired: 0x%.8X",
+       actual.rawValue, subscriptionMask, iv_valid.rawValue, iv_desired.rawValue);
+
+    // Catch a coding error where nothing is being setup to check
+    TARG_ASSERT(iv_valid.rawValue != 0, "PredicateHwasChanged::operator() - "
+        "No valid bits being checked for change");
+
+    // Only check for change on the bits subscribed to.
+    // Skip if not monitoring any of those subscribed bits.
+    // Then verify the bits we want to compare are a match to their
+    // desired changed state
+    return (((iv_valid.rawValue & subscriptionMask) != 0) &&
             (actual.rawValue & (iv_valid.rawValue & subscriptionMask)) ==
             (iv_desired.rawValue & (iv_valid.rawValue & subscriptionMask)));
 
diff --git a/src/usr/targeting/common/test/testcommontargeting.H b/src/usr/targeting/common/test/testcommontargeting.H
index ff30566d5..efae2a340 100644
--- a/src/usr/targeting/common/test/testcommontargeting.H
+++ b/src/usr/targeting/common/test/testcommontargeting.H
@@ -669,6 +669,7 @@ class CommonTargetingTestSuite: public CxxTest::TestSuite
         TARG_TS_TRACE(EXIT_MRK "testPredicateHwas" );
     }
 
+
     void testPredicateHwasChanged()
     {
         TARG_TS_TRACE(ENTER_MRK "testPredicateHwasChanged" );
@@ -679,6 +680,72 @@ class CommonTargetingTestSuite: public CxxTest::TestSuite
         using namespace TARGETING;
         TargetService& l_targetService = targetService();
 
+        PredicateHwasChanged hwasChangedPredicate;
+
+        // Test: Walk every possible bit combo in both polarities
+        //       on every target
+        for (TargetIterator targ_iter = l_targetService.begin();
+             targ_iter != l_targetService.end(); ++targ_iter)
+        {
+            Target* l_pTarget = *targ_iter;
+
+            // Save off flag
+            ATTR_HWAS_STATE_CHANGED_FLAG_type savedChgState =
+                  l_pTarget->getAttr<ATTR_HWAS_STATE_CHANGED_FLAG>();
+
+            // Get all permutations allowed by the hwas subscription mask
+            std::vector<uint64_t> l_subscribed_bit_ranges;
+            ATTR_HWAS_STATE_CHANGED_SUBSCRIPTION_MASK_type l_mask =
+                l_pTarget->getAttr<ATTR_HWAS_STATE_CHANGED_SUBSCRIPTION_MASK>();
+            for (uint64_t i = 1; i <= l_mask; ++i)
+            {
+                // If number is contained within masked bits, then
+                // this number is a valid setting for this mask
+                if ((i & l_mask) == i)
+                {
+                    l_subscribed_bit_ranges.push_back(i);
+                }
+            }
+
+            // Now run each permutation
+            // (note: auto keyword not supported for FSP compile)
+            for(std::vector<uint64_t>::iterator
+                it = l_subscribed_bit_ranges.begin();
+                it != l_subscribed_bit_ranges.end();
+                ++it)
+            {
+                uint64_t l_expected_bits = *it;
+                l_pTarget->setAttr<ATTR_HWAS_STATE_CHANGED_FLAG>
+                    (l_expected_bits);
+
+                // Testing polarity 1
+                hwasChangedPredicate.reset().changedBit(
+                    static_cast<HWAS_CHANGED_BIT>(l_expected_bits), true);
+                if(!hwasChangedPredicate(l_pTarget))
+                {
+                    TARG_TS_FAIL("Expected HWAS predicate to match the "
+                        "0x%.8X target's HWAS state, using 0x%.8X",
+                        get_huid(l_pTarget),l_expected_bits);
+                }
+
+                // Testing polarity 0
+                hwasChangedPredicate.reset().changedBit
+                    (static_cast<HWAS_CHANGED_BIT>(l_expected_bits), false);
+                l_pTarget->setAttr<ATTR_HWAS_STATE_CHANGED_FLAG>
+                    (~l_expected_bits);
+                if(!hwasChangedPredicate(l_pTarget))
+                {
+                    TARG_TS_FAIL("Expected HWAS predicate to match the 0x%.8X "
+                      "target's HWAS negative polarity state, using 0x%.8X",
+                      get_huid(l_pTarget),~l_expected_bits);
+                }
+            }
+
+            // restore original HWAS_STATE_CHANGED_FLAG
+            l_pTarget->setAttr<ATTR_HWAS_STATE_CHANGED_FLAG>(savedChgState);
+        }
+
+
         // Get master proc chip to work with
         Target* l_pTarget = NULL;
         (void) l_targetService.masterProcChipTargetHandle(l_pTarget);
@@ -688,26 +755,108 @@ class CommonTargetingTestSuite: public CxxTest::TestSuite
             break;
         }
 
-        PredicateHwasChanged hwasChangedPredicate;
-
         ATTR_HWAS_STATE_CHANGED_FLAG_type savedHwasChangedState =
-            l_pTarget->getAttr<ATTR_HWAS_STATE_CHANGED_FLAG>();
+                l_pTarget->getAttr<ATTR_HWAS_STATE_CHANGED_FLAG>();
 
-        // Test: Given a target with any HWAS state, default HWAS predicate
-        //     should return a match
-        for(size_t i=0; i<32; ++i)
+
+        // Test: Check changing non-subscribed bit.  Should return false
+        ATTR_HWAS_STATE_CHANGED_SUBSCRIPTION_MASK_type l_hwasSubscriptionMask =
+                l_pTarget->getAttr<ATTR_HWAS_STATE_CHANGED_SUBSCRIPTION_MASK>();
+        if (l_hwasSubscriptionMask != 0)
+        {
+            // find first unsubscribed bit
+            ATTR_HWAS_STATE_CHANGED_SUBSCRIPTION_MASK_type iBit = 1;
+            while (iBit & l_hwasSubscriptionMask)
+            {
+                iBit = iBit << 1;  // try next bit
+            }
+
+            // setup check for unsubscribed bit
+            hwasChangedPredicate.reset().changedBit(
+                  static_cast<HWAS_CHANGED_BIT>(iBit), true);
+
+            // set the bit
+            l_pTarget->setAttr<ATTR_HWAS_STATE_CHANGED_FLAG>(iBit);
+            if(hwasChangedPredicate(l_pTarget))
+            {
+                TARG_TS_FAIL("A change to an unsubscribed bit (0x%.8X) in "
+                  "target's mask (0x%.8X) should not cause a predicate change",
+                  iBit, l_hwasSubscriptionMask);
+            }
+        }
+        else
         {
-            ATTR_HWAS_STATE_CHANGED_FLAG_type sweepState
-                = { static_cast<uint8_t>(i&1) };
-            l_pTarget->setAttr<ATTR_HWAS_STATE_CHANGED_FLAG>(sweepState);
+            TARG_TS_FAIL("0x%.8X target has no HWAS subscription mask",
+                get_huid(l_pTarget));
+        }
+
+
+
+        // Location of two bits allowed to change for the target
+        ATTR_HWAS_STATE_CHANGED_SUBSCRIPTION_MASK_type chgBit1 = 0;
+        ATTR_HWAS_STATE_CHANGED_SUBSCRIPTION_MASK_type chgBit2 = 0;
+
+        // Test: Subscribe to at least two bits and change only one.
+        //       Expect FALSE (predicate is doing exclusive match)
+        if (l_hwasSubscriptionMask != 0)
+        {
+            // find first subscribed bit
+            chgBit1 = 1;
+            while (!(chgBit1 & l_hwasSubscriptionMask))
+            {
+                chgBit1 = chgBit1 << 1;  // try next bit
+            }
+            if (l_hwasSubscriptionMask & (~chgBit1))
+            {
+                // find second subscribed bit
+                chgBit2 = (chgBit1 << 1);
+                while (!(chgBit2 & l_hwasSubscriptionMask))
+                {
+                    chgBit2 = chgBit2 << 1;
+                }
+
+                // setup check for subscribed bits
+                hwasChangedPredicate.reset().changedBit(
+                      static_cast<HWAS_CHANGED_BIT>(chgBit1 | chgBit2), true);
+
+                // set the bit
+                l_pTarget->setAttr<ATTR_HWAS_STATE_CHANGED_FLAG>(chgBit1);
+                if(hwasChangedPredicate(l_pTarget))
+                {
+                    TARG_TS_FAIL("A 1-bit HWAS_STATE_CHANGED_FLAG change "
+                      "(0%.8X) while monitoring 2 bits (0x%.8X) caused a "
+                      "changedPredicate", chgBit1, (chgBit1 | chgBit2));
+                }
+            }
+            else
+            {
+                TARG_TS_FAIL("0x%.8X target only has one bit subscribed 0x%.8X",
+                  get_huid(l_pTarget), l_hwasSubscriptionMask);
+            }
+        }
+
+        // Test: Check for a combo of one bit cleared and one bit set,
+        //       expect true
+        if (chgBit1 && chgBit2)
+        {
+            // setup check for set subscribed bit
+            hwasChangedPredicate.reset().changedBit(
+                  static_cast<HWAS_CHANGED_BIT>(chgBit1), true);
+            // setup check for set subscribed bit
+            hwasChangedPredicate.changedBit(
+                  static_cast<HWAS_CHANGED_BIT>(chgBit2), false);
+
+            // set the bit (the second bit is cleared)
+            l_pTarget->setAttr<ATTR_HWAS_STATE_CHANGED_FLAG>(chgBit1);
 
             if(!hwasChangedPredicate(l_pTarget))
             {
-                TARG_TS_FAIL("Expected default HWAS predicate to match the "
-                    "target's HWAS state, using i = %d",i);
+                TARG_TS_FAIL("Expected this %.8X bit set and this %.8X bit "
+                  "cleared to result in changedPredicate", chgBit1, chgBit2);
             }
         }
 
+
         // Test: Given a target with cleared HWAS GARD bit, predicate
         //    looking for GARD bit should return false
         hwasChangedPredicate.reset().changedBit(HWAS_CHANGED_BIT_GARD, true);
@@ -719,20 +868,19 @@ class CommonTargetingTestSuite: public CxxTest::TestSuite
                 "target's cleared HWAS GARD bit");
         }
 
-        // Test: Having previously not matched a target, resetting the HWAS
-        //     predicate should match the target again
-        hwasChangedPredicate.reset();
+        // Test: Look to match the negated HWAS_CHANGED_BIT_GARD bit.
+        //       This should match and return true
+        hwasChangedPredicate.reset().changedBit(HWAS_CHANGED_BIT_GARD, false);
         if(!hwasChangedPredicate(l_pTarget))
         {
-            TARG_TS_FAIL("Expected HWAS reset predicate to match the "
-                "target's HWAS state");
+            TARG_TS_FAIL("Expected negative HWAS predicate to match the "
+                "target's cleared HWAS GARD bit");
         }
 
+
         // Test: Given a target with set HWAS GARD bit, predicate
         //    looking for GARD bit should return true
         hwasChangedPredicate.reset().changedBit(HWAS_CHANGED_BIT_GARD, true);
-        //ATTR_HWAS_STATE_CHANGED_FLAG_type functionalHwasChangedState =
-        //HWAS_CHANGED_BIT_GARD;
         l_pTarget->setAttr<ATTR_HWAS_STATE_CHANGED_FLAG>(HWAS_CHANGED_BIT_GARD);
         if(!hwasChangedPredicate(l_pTarget))
         {
@@ -771,7 +919,7 @@ class CommonTargetingTestSuite: public CxxTest::TestSuite
                 "target's all set HWAS state");
         }
 
-        // Restore state.
+        // Restore state
         l_pTarget->setAttr<ATTR_HWAS_STATE_CHANGED_FLAG>
                 (savedHwasChangedState);