L3 Update - p9_thread_control HWP

Change-Id: If97d1f54a1e5d8514d56969458713bcff51dc399
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/43376
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Reviewed-by: Joseph J. McGill <jmcgill@us.ibm.com>
Tested-by: PPE CI <ppe-ci+hostboot@us.ibm.com>
Tested-by: Hostboot CI <hostboot-ci+hostboot@us.ibm.com>
Reviewed-by: Richard J. Knight <rjknight@us.ibm.com>
Reviewed-by: James N. Klazynski <jklazyns@us.ibm.com>
Reviewed-by: Jennifer A. Stofer <stofer@us.ibm.com>
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/43380
Reviewed-by: Hostboot Team <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>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>

3 files changed, 241 insertions, 275 deletions

diff --git a/src/import/chips/p9/procedures/hwp/core/p9_thread_control.C b/src/import/chips/p9/procedures/hwp/core/p9_thread_control.C
index 7ed4b1811..655dbd919 100644
--- a/src/import/chips/p9/procedures/hwp/core/p9_thread_control.C
+++ b/src/import/chips/p9/procedures/hwp/core/p9_thread_control.C
@@ -22,26 +22,33 @@
 /* permissions and limitations under the License.                         */
 /*                                                                        */
 /* IBM_PROLOG_END_TAG                                                     */
+
+///----------------------------------------------------------------------------
 ///
 /// @file p9_thread_control.C
-/// @brief Implementation of sreset, start, stop and step
 ///
-
-// *HWP HWP Owner: Nick Klazynski <jklazyns@us.ibm.com>
-// *HWP FW Owner:  Thi Tran <thi@us.ibm.com>
-// *HWP Team:  Quad
-// *HWP Level: 2
-// Current Status: Only start function tested as working
-// *HWP Consumed by: FSP:HB:HS
-
+/// @brief Core Thread start/stop/step/query/activate operations
+///        See detailed description in header file.
+///
+/// *HWP HWP Owner   : Nick Klazynski <jklazyns@us.ibm.com>
+/// *HWP FW Owner    : Thi Tran <thi@us.ibm.com>
+/// *HWP Team        : Quad
+/// *HWP Consumed by : FSP:HB:HS
+/// *HWP Level       : 3
+///----------------------------------------------------------------------------
+
+//------------------------------------------------------------------------------
+// Includes
+//------------------------------------------------------------------------------
 #include <fapi2.H>
 #include <p9_thread_control.H>
+#include <p9_quad_scom_addresses_fld.H>
 
-using fapi2::TARGET_TYPE_EX;
 using fapi2::TARGET_TYPE_CORE;
 
-using fapi2::FAPI2_RC_SUCCESS;
-
+//------------------------------------------------------------------------------
+// Constants
+//------------------------------------------------------------------------------
 // The control bits for each thread are contained in DIRECT_CONTROLS
 // in regular offsets. This map allows us to go from a thread_bitset
 // to a generic register with the proper bits set. We can then shift
@@ -68,11 +75,11 @@ static const uint64_t g_control_reg_map[] =
 };
 
 static const uint64_t g_poll_for_running = 10;
+static const uint8_t PTC_STEP_COMP_POLL_LIMIT = 10;
 
 //--------------------------------------------------------------------------
 // Function definitions
 //--------------------------------------------------------------------------
-
 fapi2::ReturnCode p9_thread_control_sreset(
     const fapi2::Target<TARGET_TYPE_CORE>& i_target,
     const uint8_t i_threads, const bool i_warncheck,
@@ -102,7 +109,7 @@ fapi2::ReturnCode p9_thread_control_query(
 //--------------------------------------------------------------------------
 /// @brief threads_running : static funtion to encapsulate the running state
 /// @param[in] i_target core target
-/// @param[in] i_thread normal core thread bitset (0b0000..0b1111)
+/// @param[in] i_threads normal core thread bitset (0b0000..0b1111)
 /// @param[out] o_rasStatusReg  Complete RAS status reg 64-bit buffer.
 /// @param[out] o_ok true if the threads are running
 /// @return FAPI2_RC_SUCCESS if the underlying hw operations succeeded
@@ -131,7 +138,7 @@ fapi_try_exit:
 //--------------------------------------------------------------------------
 /// @brief threads_in_maint : static funtion to encapsulate the maint state
 /// @param[in] i_target core target
-/// @param[in] i_thread normal core thread bitset (0b0000..0b1111)
+/// @param[in] i_threads normal core thread bitset (0b0000..0b1111)
 /// @param[out] o_rasStatusReg  Complete RAS status reg 64-bit buffer.
 /// @param[out] o_ok true if the threads are in maint mode
 /// @return FAPI2_RC_SUCCESS if the underlying hw operations succeeded
@@ -154,7 +161,7 @@ fapi_try_exit:
 //--------------------------------------------------------------------------
 /// @brief all_threads_stopped : static funtion to encapsulate the stopped state
 /// @param[in] i_target core target
-/// @param[in] i_thread normal core thread bitset (0b0000..0b1111)
+/// @param[in] i_threads normal core thread bitset (0b0000..0b1111)
 /// @param[out] o_rasStatusReg  Complete RAS status reg 64-bit buffer.
 /// @param[out] o_ok true if the threads are stopped
 /// @return FAPI2_RC_SUCCESS if the underlying hw operations succeeded
@@ -179,7 +186,7 @@ fapi_try_exit:
 /// @brief all_threads_step_done : static funtion to encapsulate the step
 /// complete state
 /// @param[in] i_target core target
-/// @param[in] i_thread normal core thread bitset (0b0000..0b1111)
+/// @param[in] i_threads normal core thread bitset (0b0000..0b1111)
 /// @param[out] o_rasStatusReg  Complete RAS status reg 64-bit buffer.
 /// @param[out] o_ok true if the threads are done stepping
 /// @return FAPI2_RC_SUCCESS if the underlying hw operations succeeded
@@ -203,7 +210,7 @@ fapi_try_exit:
 /// @brief threads_step_ready : static funtion to encapsulate the step
 /// ready state
 /// @param[in] i_target core target
-/// @param[in] i_thread normal core thread bitset (0b0000..0b1111)
+/// @param[in] i_threads normal core thread bitset (0b0000..0b1111)
 /// @param[out] o_rasStatusReg  Complete RAS status reg 64-bit buffer.
 /// @param[out] o_ok true if the threads are ready to step
 /// @return FAPI2_RC_SUCCESS if the underlying hw operations succeeded
@@ -264,13 +271,23 @@ fapi2::ReturnCode p9_thread_control(
             FAPI_TRY(p9_thread_control_query(i_target, i_threads,
                                              o_rasStatusReg, o_state));
             break;
+
+        default: // full proof, in case this routine is called with an
+            // invalid casted value
+            PTC_ASSERT_WARN(false,
+                            i_warncheck,
+                            fapi2::P9_THREAD_CONTROL_INVALID_COMMAND()
+                            .set_CORE_TARGET(i_target)
+                            .set_THREAD(i_threads)
+                            .set_COMMAND(i_command),
+                            "Invalid p9_thread_control command: %d", i_command);
+            break;
     };
 
 fapi_try_exit:
-    FAPI_INF("p9_thread_control : Exit (core)");
+    FAPI_INF("p9_thread_control : Exit");
 
     return fapi2::current_err;
-
 }
 
 //--------------------------------------------------------------------------
@@ -303,12 +320,12 @@ fapi2::ReturnCode p9_thread_control_query(
 
     // Setup mask values
     const uint64_t l_running_mask =
-        (g_control_reg_map[i_threads] >> CORE_MAINT_MODE) |
-        (g_control_reg_map[i_threads] >> THREAD_QUIESCED);
+        (g_control_reg_map[i_threads] >> C_RAS_STATUS_T0_CORE_MAINT) |
+        (g_control_reg_map[i_threads] >> C_RAS_STATUS_T0_THREAD_QUIESCED);
     const uint64_t l_step_ready_mask =
-        (g_control_reg_map[i_threads] >> CORE_MAINT_MODE) |
-        (g_control_reg_map[i_threads] >> THREAD_QUIESCED) |
-        (g_control_reg_map[i_threads] >> ICT_EMPTY);
+        (g_control_reg_map[i_threads] >> C_RAS_STATUS_T0_CORE_MAINT) |
+        (g_control_reg_map[i_threads] >> C_RAS_STATUS_T0_THREAD_QUIESCED) |
+        (g_control_reg_map[i_threads] >> C_RAS_STATUS_T0_ICT_EMPTY);
 
     // Get C_RAS_STATUS reg
     FAPI_TRY(fapi2::getScom(i_target, C_RAS_STATUS, o_rasStatusReg),
@@ -331,35 +348,35 @@ fapi2::ReturnCode p9_thread_control_query(
 
     // Check for THREAD_STATE_MAINT
     if ( o_rasStatusReg &
-         g_control_reg_map[i_threads] >> CORE_MAINT_MODE )
+         g_control_reg_map[i_threads] >> C_RAS_STATUS_T0_CORE_MAINT )
     {
         o_state |= THREAD_STATE_MAINT;
     }
 
     // Check for THREAD_STATE_QUIESCED
     if ( o_rasStatusReg &
-         g_control_reg_map[i_threads] >> THREAD_QUIESCED )
+         g_control_reg_map[i_threads] >> C_RAS_STATUS_T0_THREAD_QUIESCED )
     {
         o_state |= THREAD_STATE_QUIESCED;
     }
 
     // Check for THREAD_STATE_ICT_EMPTY
     if ( o_rasStatusReg &
-         g_control_reg_map[i_threads] >> ICT_EMPTY )
+         g_control_reg_map[i_threads] >> C_RAS_STATUS_T0_ICT_EMPTY )
     {
         o_state |= THREAD_STATE_ICT_EMPTY;
     }
 
     // Check for THREAD_STATE_LSU_QUIESCED
     if ( o_rasStatusReg &
-         g_control_reg_map[i_threads] >> LSU_QUIESCED )
+         g_control_reg_map[i_threads] >> C_RAS_STATUS_T0_LSU_QUIESCED )
     {
         o_state |= THREAD_STATE_LSU_QUIESCED;
     }
 
     // Check for THREAD_STATE_ISTEP_SUCCESS
     if ( o_rasStatusReg &
-         g_control_reg_map[i_threads] >> STEP_SUCCESS )
+         g_control_reg_map[i_threads] >> C_RAS_STATUS_T0_STEP_SUCCESS )
     {
         o_state |= THREAD_STATE_ISTEP_SUCCESS;
     }
@@ -380,43 +397,6 @@ fapi_try_exit:
 }
 
 //--------------------------------------------------------------------------
-/// @brief p9_thread_control: utility subroutine to control thread state
-//-------------------------------------------------------------------------
-fapi2::ReturnCode p9_thread_control(
-    const fapi2::Target<TARGET_TYPE_EX>& i_target,
-    const uint8_t i_threads,
-    const ThreadCommands i_command,
-    fapi2::buffer<uint64_t>& o_rasStatusReg,
-    const bool i_warncheck)
-{
-    uint64_t l_state = 0;
-    FAPI_INF("p9_thread_control : Start (ex) threads: 0x%x)", i_threads);
-
-    // Grab the normal core children and iterate over them.
-    // TODO: Assumes core 0 is l_cores[0]
-    auto l_cores = i_target.getChildren<TARGET_TYPE_CORE>();
-    uint8_t l_ordinal = 0;
-
-    for( auto coreItr = l_cores.begin(); coreItr != l_cores.end(); ++coreItr, ++l_ordinal )
-    {
-        // It is quite possible that this fused core bitset only has thread-bits set
-        // for one core or the other. Don't bother to call the control function if
-        // we don't have any threads to control.
-        const uint8_t l_thread_set = fapi2::thread_bitset_f2n(l_ordinal, i_threads);
-
-        if (l_thread_set != 0)
-        {
-            FAPI_TRY(p9_thread_control(*coreItr, l_thread_set, i_command,
-                                       i_warncheck, o_rasStatusReg, l_state));
-        }
-    }
-
-fapi_try_exit:
-    FAPI_INF("p9_thread_control : Exit (ex)");
-    return fapi2::current_err;
-}
-
-//--------------------------------------------------------------------------
 /// @brief p9_thread_control_sreset: utility subroutine to sreset a thread
 /// @param[in] i_target core target
 /// @param[in] i_threads normal core thread bitset (0b0000..0b1111)
@@ -433,48 +413,49 @@ fapi2::ReturnCode p9_thread_control_sreset(
 {
     FAPI_DBG("p9_thread_control_sreset : Initiating sreset command to core PC logic for threads 0x%x",
              i_threads);
+
     // No Precondition for Sreset; power management is handled by platform
     // Clear blocking interrupts
     {
         fapi2::buffer<uint64_t> l_mode_data;
 
-        // SW375288: Reads to C_RAS_MODEREG causes SPR corruption.  For now, the code will assume no other
-        // bits are set and only set/clear mr_fence_interrupts
-        //FAPI_TRY(fapi2::getScom(i_target, C_RAS_MODEREG, l_mode_data),
-        //         "p9_thread_control_step: getScom error when reading ras_modreg for threads 0x%x",
-        //         i_threads);
-        //l_mode_data.clearBit<RAS_MODE_MR_FENCE_INTERRUPTS>();
-
+        // SW375288: Reads to C_RAS_MODEREG causes SPR corruption.
+        // For now, the code will assume no other bits are set and only
+        // set/clear mr_fence_interrupts
+        // FAPI_TRY(fapi2::getScom(i_target, C_RAS_MODEREG, l_mode_data),
+        //         "p9_thread_control_step: getScom error when reading "
+        //         "ras_modreg for threads 0x%x", i_threads);
+        // l_mode_data.clearBit<C_RAS_MODEREG_MR_FENCE_INTERRUPTS>();
         FAPI_TRY(fapi2::putScom(i_target, C_RAS_MODEREG, l_mode_data),
-                 "p9_thread_control_step: putScom error when issuing ras_modreg step mode for threads 0x%x",
-                 i_threads);
+                 "p9_thread_control_step: putScom error when issuing "
+                 "ras_modreg step mode for threads 0x%x", i_threads);
     }
 
     // Setup & Initiate SReset Command
     {
         fapi2::buffer<uint64_t> l_scom_data(
-            g_control_reg_map[i_threads] >> SRESET_REQUEST);
-
+            g_control_reg_map[i_threads] >> C_DIRECT_CONTROLS_DC_T0_SRESET_REQUEST);
         FAPI_TRY(fapi2::putScom(i_target, C_DIRECT_CONTROLS, l_scom_data),
-                 "p9_thread_control_sreset: putScom error when issuing sp_sreset for threads 0x%x",
-                 i_threads);
+                 "p9_thread_control_sreset: putScom error when issuing "
+                 "sp_sreset for threads 0x%x", i_threads);
     }
 
     // Post-conditions check
-    // TODO: Check for instructions having been executed?
     {
         bool l_running = false;
         FAPI_TRY(threads_running(i_target, i_threads, o_rasStatusReg, l_running),
-                 "p9_thread_control_sreset: unable to determine if threads are running. threads: 0x%x",
-                 i_threads);
-
+                 "p9_thread_control_sreset: unable to determine if threads "
+                 "are running. threads: 0x%x", i_threads);
         PTC_ASSERT_WARN(l_running == true,
                         i_warncheck,
                         fapi2::P9_THREAD_CONTROL_SRESET_FAIL()
                         .set_CORE_TARGET(i_target)
-                        .set_THREAD(i_threads),
-                        "p9_thread_control_sreset: ERROR: Thread SRESET issued, but the threads aren't running. "
-                        "SReset might have failed for threads 0x%x", i_threads);
+                        .set_THREAD(i_threads)
+                        .set_C_RAS_STATUS_REG(o_rasStatusReg),
+                        "p9_thread_control_sreset: ERROR: Thread SRESET issued, "
+                        "but the threads aren't running. SReset might have "
+                        "failed for threads 0x%x, C_RAS_STATUS reg 0x%.16llX",
+                        i_threads, o_rasStatusReg);
     }
 
     FAPI_INF("p9_thread_control_sreset : sreset command issued for threads 0x%x",
@@ -491,7 +472,7 @@ fapi_try_exit:
 /// @param[in] i_warncheck convert pre/post checks errors to warnings
 /// @param[out] o_rasStatusReg  Complete RAS status reg 64-bit buffer.
 /// @return FAPI2_RC_SUCCESS if operation was successful,
-///          RC_P9_THREAD_CONTROL_START_FAIL if start command failed,
+///         RC_P9_THREAD_CONTROL_START_FAIL if start command failed,
 ///         else error
 //--------------------------------------------------------------------------
 fapi2::ReturnCode p9_thread_control_start(
@@ -506,58 +487,62 @@ fapi2::ReturnCode p9_thread_control_start(
     {
         bool l_in_maint = false;
         FAPI_TRY(threads_in_maint(i_target, i_threads, o_rasStatusReg, l_in_maint),
-                 "p9_thread_control_start: unable to determine if threads are in maint mode. threads: 0x%x",
-                 i_threads);
+                 "p9_thread_control_start: unable to determine if threads are "
+                 "in maint mode. threads: 0x%x",  i_threads);
 
         PTC_ASSERT_WARN(l_in_maint == true,
                         i_warncheck,
                         fapi2::P9_THREAD_CONTROL_START_PRE_NOMAINT()
                         .set_CORE_TARGET(i_target)
-                        .set_THREAD(i_threads),
-                        "p9_thread_control_start: ERROR: Cannot issue Thread Start because the threads aren't in maint mode (threads=%x).",
-                        i_threads);
+                        .set_THREAD(i_threads)
+                        .set_C_RAS_STATUS_REG(o_rasStatusReg),
+                        "p9_thread_control_start: ERROR: Cannot issue Thread Start "
+                        "because the threads aren't in maint mode (threads=%x), "
+                        "C_RAS_STATUS reg 0x%.16llX", i_threads, o_rasStatusReg);
     }
 
     // Clear blocking interrupts
     {
         fapi2::buffer<uint64_t> l_mode_data;
 
-        // SW375288: Reads to C_RAS_MODEREG causes SPR corruption.  For now, the code will assume no other
-        // bits are set and only set/clear mr_fence_interrupts
-        //FAPI_TRY(fapi2::getScom(i_target, C_RAS_MODEREG, l_mode_data),
-        //         "p9_thread_control_step: getScom error when reading ras_modreg for threads 0x%x",
-        //         i_threads);
-        //l_mode_data.clearBit<RAS_MODE_MR_FENCE_INTERRUPTS>();
+        // SW375288: Reads to C_RAS_MODEREG causes SPR corruption.
+        // For now, the code will assume no other bits are set and only
+        // set/clear mr_fence_interrupts
+        // FAPI_TRY(fapi2::getScom(i_target, C_RAS_MODEREG, l_mode_data),
+        //         "p9_thread_control_step: getScom error when reading "
+        //         "ras_modreg for threads 0x%x", i_threads);
+        //l_mode_data.clearBit<C_RAS_MODEREG_MR_FENCE_INTERRUPTS>();
         FAPI_TRY(fapi2::putScom(i_target, C_RAS_MODEREG, l_mode_data),
-                 "p9_thread_control_step: putScom error when issuing ras_modreg step mode for threads 0x%x",
-                 i_threads);
+                 "p9_thread_control_step: putScom error when issuing ras_modreg "
+                 "step mode for threads 0x%x", i_threads);
     }
 
     // Start the threads
     {
-        fapi2::buffer<uint64_t> l_scom_data(g_control_reg_map[i_threads] >> CORE_START);
-
+        fapi2::buffer<uint64_t> l_scom_data(
+            g_control_reg_map[i_threads] >> C_DIRECT_CONTROLS_DC_T0_CORE_START);
         FAPI_TRY(fapi2::putScom(i_target, C_DIRECT_CONTROLS, l_scom_data),
-                 "p9_thread_control_start: putScom error when issuing sp_start for threads 0x%x",
-                 i_threads);
+                 "p9_thread_control_start: putScom error when issuing sp_start "
+                 "for threads 0x%x", i_threads);
     }
 
     // Post-conditions check
-    // TODO: Perhaps only run this section if i_warncheck==true to save an extranious scom
-    //       Verify understanding and desire for this funtionality before implementing in all thread_control functions
     {
         bool l_running = false;
         FAPI_TRY(threads_running(i_target, i_threads, o_rasStatusReg, l_running),
-                 "p9_thread_control_start: unable to determine if threads are running. threads: 0x%x",
-                 i_threads);
+                 "p9_thread_control_start: unable to determine if threads are "
+                 "running. threads: 0x%x", i_threads);
 
         PTC_ASSERT_WARN(l_running == true,
                         i_warncheck,
                         fapi2::P9_THREAD_CONTROL_START_FAIL()
                         .set_CORE_TARGET(i_target)
-                        .set_THREAD(i_threads),
-                        "p9_thread_control_start: ERROR: Thread Start issued, but the threads aren't running. "
-                        "Start might have failed for threads 0x%x", i_threads);
+                        .set_THREAD(i_threads)
+                        .set_C_RAS_STATUS_REG(o_rasStatusReg),
+                        "p9_thread_control_start: ERROR: Thread Start issued, "
+                        "but the threads aren't running. Start might have "
+                        "failed for threads 0x%x, C_RAS_STATUS reg 0x%.16llX",
+                        i_threads, o_rasStatusReg);
     }
 
     FAPI_INF("p9_thread_control_start : start command issued for threads 0x%x",
@@ -589,41 +574,44 @@ fapi2::ReturnCode p9_thread_control_stop(
     {
         fapi2::buffer<uint64_t> l_mode_data;
 
-        // SW375288: Reads to C_RAS_MODEREG causes SPR corruption.  For now, the code will assume no other
-        // bits are set and only set/clear mr_fence_interrupts
-        //FAPI_TRY(fapi2::getScom(i_target, C_RAS_MODEREG, l_mode_data),
-        //         "p9_thread_control_step: getScom error when reading ras_modreg for threads 0x%x",
-        //         i_threads);
-
-        l_mode_data.setBit<RAS_MODE_MR_FENCE_INTERRUPTS>();
+        // SW375288: Reads to C_RAS_MODEREG causes SPR corruption.  For now,
+        // the code will assume no other bits are set and only set/clear
+        // mr_fence_interrupts.
+        // FAPI_TRY(fapi2::getScom(i_target, C_RAS_MODEREG, l_mode_data),
+        //         "p9_thread_control_step: getScom error when reading "
+        //         "ras_modreg for threads 0x%x", i_threads);
+        l_mode_data.setBit<C_RAS_MODEREG_MR_FENCE_INTERRUPTS>();
         FAPI_TRY(fapi2::putScom(i_target, C_RAS_MODEREG, l_mode_data),
-                 "p9_thread_control_step: putScom error when issuing ras_modreg step mode for threads 0x%x",
-                 i_threads);
+                 "p9_thread_control_step: putScom error when issuing ras_modreg "
+                 "step mode for threads 0x%x", i_threads);
     }
 
     // Stop the threads
     {
-        fapi2::buffer<uint64_t> l_scom_data(g_control_reg_map[i_threads] >> CORE_STOP);
-
+        fapi2::buffer<uint64_t> l_scom_data(
+            g_control_reg_map[i_threads] >> C_DIRECT_CONTROLS_DC_T0_CORE_STOP);
         FAPI_TRY(fapi2::putScom(i_target, C_DIRECT_CONTROLS, l_scom_data),
-                 "p9_thread_control_stop: putScom error when issuing sp_stop for threads 0x%x",
-                 i_threads);
+                 "p9_thread_control_stop: putScom error when issuing sp_stop "
+                 "for threads 0x%x", i_threads);
     }
 
     // Post-conditions check
     {
         bool l_stopped = false;
         FAPI_TRY(threads_stopped(i_target, i_threads, o_rasStatusReg, l_stopped),
-                 "p9_thread_control_stop: unable to determine if threads are stopped. threads: 0x%x",
-                 i_threads);
+                 "p9_thread_control_stop: unable to determine if threads are "
+                 "stopped. threads: 0x%x", i_threads);
 
         PTC_ASSERT_WARN(l_stopped == true,
                         i_warncheck,
                         fapi2::P9_THREAD_CONTROL_STOP_FAIL()
                         .set_CORE_TARGET(i_target)
-                        .set_THREAD(i_threads),
-                        "p9_thread_control_stop: ERROR: Thread Stop issued, but the threads are running. "
-                        "Stop might have failed for threads 0x%x", i_threads);
+                        .set_THREAD(i_threads)
+                        .set_C_RAS_STATUS_REG(o_rasStatusReg),
+                        "p9_thread_control_stop: ERROR: Thread Stop issued, "
+                        "but the threads are running. Stop might have failed "
+                        "for threads 0x%x, C_RAS_STATUS reg 0x%.16llX",
+                        i_threads, o_rasStatusReg);
     }
     FAPI_INF("p9_thread_control_stop : stop command issued for threads 0x%x",
              i_threads);
@@ -648,33 +636,37 @@ fapi2::ReturnCode p9_thread_control_step(
     const uint8_t i_threads, const bool i_warncheck,
     fapi2::buffer<uint64_t>& o_rasStatusReg)
 {
-    FAPI_DBG("p9_thread_control_stop : Initiating step command to core PC logic for threads 0x%x",
-             i_threads);
+    FAPI_DBG("p9_thread_control_stop : Initiating step command to core PC "
+             "logic for threads 0x%x", i_threads);
 
     // Preconditions
     {
         bool l_step_ready = false;
         FAPI_TRY(threads_step_ready(i_target, i_threads, o_rasStatusReg, l_step_ready),
-                 "p9_thread_control_step: unable to determine if threads are ready to step. threads: 0x%x",
-                 i_threads);
+                 "p9_thread_control_step: unable to determine if threads are "
+                 "ready to step. threads: 0x%x", i_threads);
 
         PTC_ASSERT_WARN(l_step_ready == true,
                         i_warncheck,
                         fapi2::P9_THREAD_CONTROL_STEP_PRE_NOTSTOPPING()
                         .set_CORE_TARGET(i_target)
-                        .set_THREAD(i_threads),
-                        "p9_thread_control_step: ERROR: Thread cannot be stepped because they are not ready to step (threads=%x).", i_threads);
+                        .set_THREAD(i_threads)
+                        .set_C_RAS_STATUS_REG(o_rasStatusReg),
+                        "p9_thread_control_step: ERROR: Thread cannot be "
+                        "stepped because they are not ready to step (threads=%x), "
+                        "C_RAS_STATUS reg 0x%.16llX", i_threads, o_rasStatusReg);
     }
 
 
     // Setup single step mode and issue step.
     {
-        fapi2::buffer<uint64_t> l_step_data(g_control_reg_map[i_threads] >> CORE_STEP);
+        fapi2::buffer<uint64_t> l_step_data(
+            g_control_reg_map[i_threads] >> C_DIRECT_CONTROLS_DC_T0_CORE_STEP);
 
         // Set issue the step
         FAPI_TRY(fapi2::putScom(i_target, C_DIRECT_CONTROLS, l_step_data),
-                 "p9_thread_control_step: putScom error when issuing step command for threads 0x%x",
-                 i_threads);
+                 "p9_thread_control_step: putScom error when issuing step "
+                 "command for threads 0x%x", i_threads);
     }
 
 
@@ -687,7 +679,8 @@ fapi2::ReturnCode p9_thread_control_step(
         {
             FAPI_DBG("polling for step done. governor: %d", l_governor);
             FAPI_TRY(threads_step_done(i_target, i_threads, o_rasStatusReg, l_step_done),
-                     "p9_thread_control_step: thread step issued but something went wrong polling for step_done for threads 0x%x",
+                     "p9_thread_control_step: thread step issued but something "
+                     "went wrong polling for step_done for threads 0x%x",
                      i_threads);
         }
         while((l_step_done != true) && l_governor--);
@@ -698,13 +691,15 @@ fapi2::ReturnCode p9_thread_control_step(
                         fapi2::P9_THREAD_CONTROL_STEP_FAIL()
                         .set_CORE_TARGET(i_target)
                         .set_THREAD(i_threads)
+                        .set_C_RAS_STATUS_REG(o_rasStatusReg)
                         .set_PTC_STEP_COMP_POLL_LIMIT(PTC_STEP_COMP_POLL_LIMIT),
-                        "p9_thread_control_stop: ERROR: Thread Step failed. Complete bits aren't set after %d poll atempts. WARNING: C_RAS_STATUS "
-                        "bit still in single instruction mode. Threads 0x%x", PTC_STEP_COMP_POLL_LIMIT,
-                        i_threads);
+                        "p9_thread_control_stop: ERROR: Thread Step failed."
+                        "Complete bits aren't set after %d poll atempts. "
+                        "WARNING: C_RAS_STATUS bit still in single instruction "
+                        "mode. Threads 0x%x, C_RAS_STATUS reg 0x%.16llX",
+                        PTC_STEP_COMP_POLL_LIMIT, i_threads, o_rasStatusReg);
     }
 
-
 fapi_try_exit:
     return fapi2::current_err;
 }
diff --git a/src/import/chips/p9/procedures/hwp/core/p9_thread_control.H b/src/import/chips/p9/procedures/hwp/core/p9_thread_control.H
index a87a3c8dd..a0f887877 100755
--- a/src/import/chips/p9/procedures/hwp/core/p9_thread_control.H
+++ b/src/import/chips/p9/procedures/hwp/core/p9_thread_control.H
@@ -5,7 +5,7 @@
 /*                                                                        */
 /* OpenPOWER HostBoot Project                                             */
 /*                                                                        */
-/* Contributors Listed Below - COPYRIGHT 2015,2016                        */
+/* Contributors Listed Below - COPYRIGHT 2015,2017                        */
 /* [+] International Business Machines Corp.                              */
 /*                                                                        */
 /*                                                                        */
@@ -22,46 +22,45 @@
 /* permissions and limitations under the License.                         */
 /*                                                                        */
 /* IBM_PROLOG_END_TAG                                                     */
-//------------------------------------------------------------------------------
-// *! TITLE       : p9_thread_control
-// *! DESCRIPTION : Core Thread start/stop/step/query/activate operations
-// *!               Use to start (start or sreset) thread instruction execution,
-// *!                 stop instruction execution, or single instruction step.
-// *!               Also used to query the state of a thread.
-//------------------------------------------------------------------------------
-
-// *HWP HWP Owner: Nick Klazynski <jklazyns@us.ibm.com>
-// *HWP FW Owner:  Thi Tran <thi@us.ibm.com>
-// *HWP Team:  Quad
-// *HWP Level: 2
-// *HWP Consumed by: FSP:HB:HS
 
+///----------------------------------------------------------------------------
+///
+/// @file p9_thread_control.H
+///
+/// @brief Core Thread start/stop/step/query/activate operations
+///        Use to start (start or sreset) thread instruction execution,
+///        stop instruction execution, or single instruction step.
+///        Also used to query the state of a thread.
+///
+/// *HWP HWP Owner   : Nick Klazynski <jklazyns@us.ibm.com>
+/// *HWP FW Owner    : Thi Tran <thi@us.ibm.com>
+/// *HWP Team        : Quad
+/// *HWP Consumed by : FSP:HB:HS
+/// *HWP Level       : 3
+///----------------------------------------------------------------------------
 #ifndef _P9_THREAD_CONTROL_H_
 #define _P9_THREAD_CONTROL_H_
 
+//------------------------------------------------------------------------------
+// Includes
+//------------------------------------------------------------------------------
 #include <fapi2.H>
-#include <return_code.H>
-#include <error_scope.H>
-
-#include "p9_quad_scom_addresses.H"
-
-extern "C"
-{
+#include <p9_quad_scom_addresses.H>
 
 //------------------------------------------------------------------------------
 // Constants
 //------------------------------------------------------------------------------
-    const uint8_t MAX_NUM_OF_THREADS = 4;
+const uint8_t MAX_NUM_OF_THREADS = 4;
 
 // THREAD_STATE bit definitions
-    const uint64_t THREAD_STATE_RUNNING            = 0x8000000000000000ULL;
-    const uint64_t THREAD_STATE_STOP               = 0x4000000000000000ULL;
-    const uint64_t THREAD_STATE_QUIESCED           = 0x2000000000000000ULL;
-    const uint64_t THREAD_STATE_MAINT              = 0x1000000000000000ULL;
-    const uint64_t THREAD_STATE_ICT_EMPTY          = 0x0800000000000000ULL;
-    const uint64_t THREAD_STATE_LSU_QUIESCED       = 0x0400000000000000ULL;
-    const uint64_t THREAD_STATE_ISTEP_SUCCESS      = 0x0200000000000000ULL;
-    const uint64_t THREAD_STATE_ISTEP_READY        = 0x0100000000000000ULL;
+const uint64_t THREAD_STATE_RUNNING            = 0x8000000000000000ULL;
+const uint64_t THREAD_STATE_STOP               = 0x4000000000000000ULL;
+const uint64_t THREAD_STATE_QUIESCED           = 0x2000000000000000ULL;
+const uint64_t THREAD_STATE_MAINT              = 0x1000000000000000ULL;
+const uint64_t THREAD_STATE_ICT_EMPTY          = 0x0800000000000000ULL;
+const uint64_t THREAD_STATE_LSU_QUIESCED       = 0x0400000000000000ULL;
+const uint64_t THREAD_STATE_ISTEP_SUCCESS      = 0x0200000000000000ULL;
+const uint64_t THREAD_STATE_ISTEP_READY        = 0x0100000000000000ULL;
 
 // A macro to wrap the warning check boiler plate
 // If the action failed and i_warncheck is set add a trace and continue anyway
@@ -80,43 +79,14 @@ extern "C"
         }                                                               \
     }                                                                   \
 
-
 // ProcThreadControl input commands
-// If you make this an enum, the compiler can
-// check that a case statement has all the elements
-// covered.
     enum ThreadCommands
     {
-        PTC_CMD_SRESET            = 0,
-        PTC_CMD_STEP              = 1,
-        PTC_CMD_START             = 2,
-        PTC_CMD_STOP              = 3,
-        PTC_CMD_QUERY             = 4,
-    };
-
-    enum ThreadRasStatus
-    {
-        CORE_MAINT_MODE = 0,
-        THREAD_QUIESCED = 1,
-        ICT_EMPTY       = 2,
-        LSU_QUIESCED    = 3,
-        STEP_SUCCESS    = 4,
-    };
-
-    enum PTC_Constants
-    {
-        RAS_MODE_MR_FENCE_INTERRUPTS = 57,
-        PTC_STEP_COMP_POLL_LIMIT     = 10,
-    };
-
-// Bit positions in the DIRECT_CONTROL register
-    enum ThreadControl
-    {
-        CLEAR_MAINT      = 3,
-        SRESET_REQUEST   = 4,
-        CORE_STEP        = 5,
-        CORE_START       = 6,
-        CORE_STOP        = 7,
+        PTC_CMD_SRESET  = 0,
+        PTC_CMD_STEP    = 1,
+        PTC_CMD_START   = 2,
+        PTC_CMD_STOP    = 3,
+        PTC_CMD_QUERY   = 4,
     };
 
 /// @typedef p9_thread_control_FP_t
@@ -126,58 +96,43 @@ extern "C"
         const uint8_t, const ThreadCommands, const bool,
         fapi2::buffer<uint64_t>&, uint64_t&);
 
-//--------------------------------------------------------------------------
-/// @brief p9_thread_control: utility subroutine to control thread state
-/// @param[in] i_target ex target
-/// @param[in] i_threads fused thread bitset (0b00000000..0b11111111)
-/// @param[in] i_command  one of
-///               PTC_CMD_SRESET => initiate sreset thread command
-///               PTC_CMD_START  => initiate start thread command
-///               PTC_CMD_STOP   => initiate stop thread command
-///               PTC_CMD_STEP   => initiate step thread command
-///               PTC_CMD_QUERY  => query and return thread state
-/// @param[in] i_warncheck convert pre/post checks errors to warnings
-/// @return FAPI_RC_SUCCESS if operation was successful,
-///         function-specific fail codes (see function definitions),
-///         else error
-//-------------------------------------------------------------------------
-//fapi2::ReturnCode p9_thread_control(const fapi2::Target<fapi2::TARGET_TYPE_EX>& i_target,
-//        const uint8_t i_threads, const ThreadCommands i_command,
-//        const bool i_warncheck);
+    extern "C"
+    {
 
-//--------------------------------------------------------------------------
+///--------------------------------------------------------------------------
 /// @brief p9_thread_control: utility subroutine to control thread state
 ///
-/// @param[in] i_target core target
-/// @param[in] i_threads     Desired thread bit set, multiple thread settings
-///                          are allowed.
+/// @param[in] i_target    Core target
+/// @param[in] i_threads   Desired thread bit set, multiple thread settings
+///                        are allowed.
 ///                             0b0000         No thread (No-op)
 ///                             0b1000         Thread 0
 ///                             0b0100         Thread 1
 ///                             0b0010         Thread 2
 ///                             0b0001         Thread 3
-/// @param[in] i_command  one of
-///               PTC_CMD_SRESET => initiate sreset thread command
-///               PTC_CMD_START  => initiate start thread command
-///               PTC_CMD_STOP   => initiate stop thread command
-///               PTC_CMD_STEP   => initiate step thread command
-///               PTC_CMD_QUERY  => query and return thread state
+/// @param[in] i_command   One of
+///                            PTC_CMD_SRESET => initiate sreset thread command
+///                            PTC_CMD_START  => initiate start thread command
+///                            PTC_CMD_STOP   => initiate stop thread command
+///                            PTC_CMD_STEP   => initiate step thread command
+///                            PTC_CMD_QUERY  => query and return thread state
 /// @param[in] i_warncheck convert pre/post checks errors to warnings
 /// @param[out] o_rasStatusReg  Complete RAS status reg 64-bit buffer after
 ///                             executing command.
 /// @param[out] o_state Thread state, only valid for PTC_CMD_QUERY command.
-///                    See p9_thread_control.H for THREAD_STATE bit definitions.
+///                     See THREAD_STATE bit definitions above.
 ///
 /// @return FAPI_RC_SUCCESS if operation was successful,
 ///         function-specific fail codes (see function definitions),
 ///         else error
-//--------------------------------------------------------------------------
-    fapi2::ReturnCode p9_thread_control(const fapi2::Target<fapi2::TARGET_TYPE_CORE>& i_target,
-                                        const uint8_t i_threads, const ThreadCommands i_command,
-                                        const bool i_warncheck,
-                                        fapi2::buffer<uint64_t>& o_rasStatusReg,
-                                        uint64_t& o_state);
-
-} // extern
+///--------------------------------------------------------------------------
+        fapi2::ReturnCode p9_thread_control(
+            const fapi2::Target<fapi2::TARGET_TYPE_CORE>& i_target,
+            const uint8_t i_threads, const ThreadCommands i_command,
+            const bool i_warncheck,
+            fapi2::buffer<uint64_t>& o_rasStatusReg,
+            uint64_t& o_state);
+
+    } // extern
 
 #endif // _P9_THREAD_CONTROL_H_
diff --git a/src/import/chips/p9/procedures/xml/error_info/p9_thread_control_errors.xml b/src/import/chips/p9/procedures/xml/error_info/p9_thread_control_errors.xml
index eae637806..cc727c4c2 100644
--- a/src/import/chips/p9/procedures/xml/error_info/p9_thread_control_errors.xml
+++ b/src/import/chips/p9/procedures/xml/error_info/p9_thread_control_errors.xml
@@ -5,7 +5,7 @@
 <!--                                                                        -->
 <!-- OpenPOWER HostBoot Project                                             -->
 <!--                                                                        -->
-<!-- Contributors Listed Below - COPYRIGHT 2015,2016                        -->
+<!-- Contributors Listed Below - COPYRIGHT 2015,2017                        -->
 <!-- [+] International Business Machines Corp.                              -->
 <!--                                                                        -->
 <!--                                                                        -->
@@ -28,23 +28,11 @@
   <!-- ********************************************************************* -->
   <hwpError>
     <sbeError/>
-    <rc>RC_P9_THREAD_CONTROL_SRESET_PRE_FAIL</rc>
-    <description>SReset command precondition not met: Not all threads are running.</description>
-    <ffdc>CORE_TARGET</ffdc>
-    <ffdc>THREAD</ffdc>
-    <callout>
-      <procedure>CODE</procedure>
-      <priority>HIGH</priority>
-    </callout>
-  </hwpError>
-
-  <!-- ********************************************************************* -->
-  <hwpError>
-    <sbeError/>
     <rc>RC_P9_THREAD_CONTROL_SRESET_FAIL</rc>
+    <description>SReset command failed: Not all threads are running after sreset command.</description>
     <ffdc>CORE_TARGET</ffdc>
     <ffdc>THREAD</ffdc>
-    <description>SReset command failed: Not all threads are running after sreset command.</description>
+    <ffdc>C_RAS_STATUS_REG</ffdc>
     <callout>
       <target>CORE_TARGET</target>
       <priority>HIGH</priority>
@@ -60,7 +48,6 @@
       <target>CORE_TARGET</target>
     </gard>
   </hwpError>
-
   <!-- ********************************************************************* -->
   <hwpError>
     <sbeError/>
@@ -68,12 +55,12 @@
     <description>Start command precondition not met: RAS STAT Maintenance bit is not set.</description>
     <ffdc>CORE_TARGET</ffdc>
     <ffdc>THREAD</ffdc>
+    <ffdc>C_RAS_STATUS_REG</ffdc>
     <callout>
       <procedure>CODE</procedure>
       <priority>HIGH</priority>
     </callout>
   </hwpError>
-
   <!-- ********************************************************************* -->
   <hwpError>
     <sbeError/>
@@ -81,25 +68,22 @@
     <description>Start command failed: RAS STAT instruction completed bit was not set after start command.</description>
     <ffdc>CORE_TARGET</ffdc>
     <ffdc>THREAD</ffdc>
+    <ffdc>C_RAS_STATUS_REG</ffdc>
     <callout>
-      <procedure>CODE</procedure>
+      <target>CORE_TARGET</target>
       <priority>HIGH</priority>
     </callout>
-  </hwpError>
-
-  <!-- ********************************************************************* -->
-  <hwpError>
-    <sbeError/>
-    <rc>RC_P9_THREAD_CONTROL_STOP_PRE_NOTRUNNING</rc>
-    <description>Stop command precondition not met: Not all threads are running.</description>
-    <ffdc>CORE_TARGET</ffdc>
-    <ffdc>THREAD</ffdc>
     <callout>
       <procedure>CODE</procedure>
-      <priority>HIGH</priority>
+      <priority>LOW</priority>
     </callout>
+    <deconfigure>
+      <target>CORE_TARGET</target>
+    </deconfigure>
+    <gard>
+      <target>CORE_TARGET</target>
+    </gard>
   </hwpError>
-
   <!-- ********************************************************************* -->
   <hwpError>
     <sbeError/>
@@ -107,10 +91,23 @@
     <description>Stop command issued to core PC, but RAS STAT maintenance bit is not set.</description>
     <ffdc>CORE_TARGET</ffdc>
     <ffdc>THREAD</ffdc>
+    <ffdc>C_RAS_STATUS_REG</ffdc>
     <callout>
-      <procedure>CODE</procedure>
+      <target>CORE_TARGET</target>
       <priority>HIGH</priority>
     </callout>
+    <callout>
+      <procedure>CODE</procedure>
+      <priority>LOW</priority>
+    </callout>
+    <deconfigure>
+      <target>CORE_TARGET</target>
+    </deconfigure>
+    <!-- Note:  FW will attempt to ensure instructions are stopped in many
+         different cases with no interlock that we're in a reasonable state
+         for the HWP to succeed.  Therefore, we should not gard out the HW,
+         only deconfigure it.
+    -->
   </hwpError>
 
   <!-- ********************************************************************* -->
@@ -120,6 +117,7 @@
     <description>Step command precondition not met: Not all threads are stopped.</description>
     <ffdc>CORE_TARGET</ffdc>
     <ffdc>THREAD</ffdc>
+    <ffdc>C_RAS_STATUS_REG</ffdc>
     <callout>
       <procedure>CODE</procedure>
       <priority>HIGH</priority>
@@ -133,6 +131,7 @@
     <description>Step command issued to core PC, but RAS STAT run bit is still set.</description>
     <ffdc>CORE_TARGET</ffdc>
     <ffdc>THREAD</ffdc>
+    <ffdc>C_RAS_STATUS_REG</ffdc>
     <ffdc>PTC_STEP_COMP_POLL_LIMIT</ffdc>
     <callout>
       <target>CORE_TARGET</target>
@@ -142,6 +141,23 @@
       <procedure>CODE</procedure>
       <priority>LOW</priority>
     </callout>
+    <!-- Note:  FW would only attempt to step in a debug support capacity,
+         we don't want to remove (deconfig/gard) HW for a fail in this use case
+    -->
+  </hwpError>
+
+  <!-- ********************************************************************* -->
+  <hwpError>
+    <sbeError/>
+    <rc>RC_P9_THREAD_CONTROL_INVALID_COMMAND</rc>
+    <description>p9_thread_control is called with an invalid command </description>
+    <ffdc>CORE_TARGET</ffdc>
+    <ffdc>THREAD</ffdc>
+    <ffdc>COMMAND</ffdc>
+    <callout>
+      <procedure>CODE</procedure>
+      <priority>HIGH</priority>
+    </callout>
   </hwpError>
 
 </hwpErrors>