Implement FAPI_PLAT_CALL_SUBROUTINE macro

HWP writers need a way to request certain HWPs to run on the SBE
for non-master processor targets while we are in secureBoot mode.
This macro will check these conditions and if we are on a non-master
in secureMode then we will issue a fifo chipop to the slave sbe
in order to request the SBE to perform the HWP. This commit also
adds some simple test cases to verify the failure paths. Also
fapi2 test cases are re-enabled in this commit.

Change-Id: Ic42fe2df5692fb5994985d92b2fd767ea7c1234a
RTC: 179062
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/46580
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-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>

9 files changed, 367 insertions, 24 deletions

diff --git a/src/include/usr/fapi2/hwpf_fapi2_reasoncodes.H b/src/include/usr/fapi2/hwpf_fapi2_reasoncodes.H
index 16c871263..98eddff7b 100644
--- a/src/include/usr/fapi2/hwpf_fapi2_reasoncodes.H
+++ b/src/include/usr/fapi2/hwpf_fapi2_reasoncodes.H
@@ -59,6 +59,7 @@ namespace fapi2
         MOD_FAPI2_PLAT_PARSE_WOF_TABLES             = 0x10,
         MOD_FAPI2_CVPD_ACCESS                       = 0x11,
         MOD_FAPI2_BAD_DQ_BITMAP                     = 0x12,
+        MOD_FAPI2_GET_CHIP_CFAM_TARGET              = 0x13,
     };
 
     /**
@@ -125,6 +126,7 @@ namespace fapi2
         RC_MM_SET_PERMISSION_FAILED    = FAPI2_COMP_ID | 0x31,
         RC_MM_REMOVE_PAGES_FAILED      = FAPI2_COMP_ID | 0x32,
         RC_MM_SET_PERMISSION2_FAILED   = FAPI2_COMP_ID | 0x33,
+        RC_INVALID_PARENT_TARGET_FOUND = FAPI2_COMP_ID | 0x34,
 
         // HWP generated errors
         RC_HWP_GENERATED_ERROR         = HWPF_COMP_ID | 0x0f,
diff --git a/src/include/usr/fapi2/plat_hw_access.H b/src/include/usr/fapi2/plat_hw_access.H
index 94faa7443..3972f137f 100644
--- a/src/include/usr/fapi2/plat_hw_access.H
+++ b/src/include/usr/fapi2/plat_hw_access.H
@@ -220,6 +220,14 @@ void checkPibMask(errlHndl_t& io_errLog );
 // No spy access interface as HB doesn't allow spy access.
 // --------------------------------------------------------------------------
 
+/**
+* @brief Determine if a given target is on the master proc chip
+* @param[in]  i_Target   TARGETING::Target which op is being called on
+* @param[out] i_isMaster True if on master proc chip, false if not
+* @return errlHndl_t
+*/
+errlHndl_t isOnMasterProc(TARGETING::Target * i_target, bool & o_isMaster);
+
 
 } // End namespace
 
diff --git a/src/include/usr/fapi2/subroutine_executor.H b/src/include/usr/fapi2/subroutine_executor.H
index 7d9acda1b..82ca296a3 100644
--- a/src/include/usr/fapi2/subroutine_executor.H
+++ b/src/include/usr/fapi2/subroutine_executor.H
@@ -37,8 +37,50 @@
 #define SUBROUTINEEXECUTOR_H_
 
 #include <fapi2_subroutine_executor.H>
-
+#include <errl/errlmanager.H>
+#include <string.h>
+#include <stdarg.h>
+#include <sbeio/sbeioif.H>
 #include <plat_trace.H>
+#include <secureboot/service.H>
+#include <plat_hw_access.H>
+
+/**
+* @brief Given a hwp name, and its parameters, serialize the parmeters and pass the serialized
+*        data to the SBE via a FIFO chipop
+* @param[in]  i_hwpName  String representing the name of the hwp to be called
+* @param[in]  i_Target   TARGETING::Target which op is being called on
+* @param[in]  types      Any variable length of arguments, which along with target should get passed
+*                        in as parameters to the given HWP
+* @return errlHndl_t Error log handle on failure.
+*/
+template<class... Types>
+errlHndl_t requestHwpViaSbe(const char * i_hwpName, TARGETING::Target * i_target, Types... types)
+{
+    errlHndl_t l_errl = nullptr;
+
+    //Determine argument byte size
+    size_t l_sizeOfArgInBytes =0;
+    using expander = int[];
+    (void) expander{ 0, (l_sizeOfArgInBytes+=sizeof(types), 0)... };
+
+    //Set up the buffer which will be passed to chip op send function
+    uint8_t l_buffer[l_sizeOfArgInBytes];
+    uint8_t* l_bufferPtr = &l_buffer[0];
+    memset(l_bufferPtr, 0, l_sizeOfArgInBytes);
+
+    // Serialize the arguments into the buffer
+    (void) expander{ 0, ((memcpy(l_bufferPtr,&types,sizeof(types)),l_bufferPtr+=sizeof(types)), 0)... };
+
+    //Call the chip op send function to request the SBE to call the HWP
+    l_errl = SBEIO::sendSecureHwpRequest(i_target, l_bufferPtr, l_sizeOfArgInBytes, i_hwpName);
+
+    return l_errl;
+}
+
+
+//Macros that return the 1st argument
+#define _GET_1ST_ARG(N, ...) N
 
 /**
  * @brief Subroutine Executor macro example code - Platforms will need to
@@ -48,11 +90,49 @@
  * execute the Subroutine (e.g. dlopening a shared library)
  */
 #define FAPI_PLAT_CALL_SUBROUTINE(RC, FUNC, _args...) \
+{ \
+    do \
     { \
-        FAPI_DBG("executing FAPI_PLAT_CALL_SUBROUTINE macro"); \
-        RC = FUNC(_args); \
-    }
-
-//@todo - RTC:179062 - Add real chipop support
+        errlHndl_t l_errl = nullptr; \
+        /* Read the FUNC as a string and pass it to the conversion method \
+        to determine what hwp we need to request */ \
+        const char* l_function = #FUNC; \
+        /*Read the target argument, which is the first argument in the list */ \
+        TARGETING::Target* l_target = \
+        reinterpret_cast<TARGETING::Target*>(_GET_1ST_ARG(_args).get()); \
+        /*Check if secureboot is enabled and if the target exists on the master proc*/ \
+        bool isSecure = SECUREBOOT::enabled(); \
+        bool isMaster = false; \
+        l_errl = isOnMasterProc(l_target, isMaster); \
+        if(l_errl) \
+        { \
+            FAPI_INF("subroutine_executor: Failed trying to determine is target was on master chip"); \
+            RC.setPlatDataPtr(reinterpret_cast<void *> (l_errl)); \
+            break; \
+        } \
+        /*Run hwp on host if targ is on master, we are not in securemode*/ \
+        if(isMaster || !isSecure) \
+        { \
+            FAPI_INF("subroutine_executor: isSecure = %d   isMaster = %d .. executing hwp %s on host", \
+                      isSecure, isMaster, l_function); \
+            RC = FUNC(_args); \
+        } \
+        /*Otherwise request the HWP via chipop to the SBE*/ \
+        else \
+        { \
+            FAPI_INF("subroutine_executor: isSecure = %d   isMaster = %d .. executing hwp %s on host", \
+                    isSecure, isMaster, l_function); \
+            l_errl =  requestHwpViaSbe(l_function, l_target, _args); \
+            /*For now until SBE support comes fallback to running on host if chipop fails*/ \
+            if(l_errl) \
+            { \
+                /*Commit the error as informational and attempt hwp */ \
+                l_errl->setSev(ERRORLOG::ERRL_SEV_INFORMATIONAL); \
+                errlCommit(l_errl, SBEIO_COMP_ID); \
+                RC = FUNC(_args);\
+            } \
+        } \
+    } while(0); \
+}
 
-#endif
+#endif
\ No newline at end of file
diff --git a/src/include/usr/sbeio/sbeioif.H b/src/include/usr/sbeio/sbeioif.H
index 565aa165b..357a29a95 100644
--- a/src/include/usr/sbeio/sbeioif.H
+++ b/src/include/usr/sbeio/sbeioif.H
@@ -274,7 +274,7 @@ namespace SBEIO
     * @param[in] i_dataSize     Size of blob of data that contains additional parameters
     *                           for the requests HWP
     *
-    * @param[in] i_hwpStringLen size of the hwp name string at beginning of data pointer
+    * @param[in] i_hwpName      Pointer to string of chars representing hwp name
     *
     * @return errlHndl_t Error log handle on failure.
     *
@@ -282,7 +282,7 @@ namespace SBEIO
     errlHndl_t sendSecureHwpRequest(TARGETING::Target * i_target,
                                     uint8_t * i_dataPointer,
                                     uint64_t i_dataSize,
-                                    uint64_t i_hwpStringLen);
+                                    const char * i_hwpName);
 
 
 } //end namespace SBEIO
diff --git a/src/usr/fapi2/plat_hw_access.C b/src/usr/fapi2/plat_hw_access.C
index d0969d760..e39632ccc 100644
--- a/src/usr/fapi2/plat_hw_access.C
+++ b/src/usr/fapi2/plat_hw_access.C
@@ -362,7 +362,7 @@ void checkPibMask(errlHndl_t& io_errLog )
 errlHndl_t getCfamChipTarget(const TARGETING::Target* i_target,
                              TARGETING::Target*& o_chipTarget)
 {
-    errlHndl_t l_err = NULL;
+    errlHndl_t l_err = nullptr;
 
     // Default to input target
     o_chipTarget = const_cast<TARGETING::Target*>(i_target);
@@ -389,6 +389,22 @@ errlHndl_t getCfamChipTarget(const TARGETING::Target* i_target,
         {
             // Something is wrong here, can't have more than one parent chip
             FAPI_ERR("getCfamChipTarget: Invalid number of parent chip for this target chiplet - # parent chips %d", l_list.size());
+            /*@
+            * @errortype
+            * @moduleid     fapi2::MOD_FAPI2_GET_CHIP_CFAM_TARGET
+            * @reasoncode   fapi2::RC_INVALID_PARENT_TARGET_FOUND
+            * @userdata1    Number of parent proc chips found
+            * @userdata2    HUID of input target
+            * @devdesc      Detecting more than 1 parent proc targets
+            * @custdesc     Internal firmware error
+            */
+            l_err = new ERRORLOG::ErrlEntry(ERRORLOG::ERRL_SEV_UNRECOVERABLE,
+                                            fapi2::MOD_FAPI2_GET_CHIP_CFAM_TARGET,
+                                            fapi2::RC_INVALID_PARENT_TARGET_FOUND,
+                                            l_list.size(),
+                                            TARGETING::get_huid(i_target),
+                                            true /*SW error*/);
+            l_err->collectTrace(FAPI_TRACE_NAME);
         }
     }
     return l_err;
@@ -1039,6 +1055,32 @@ uint8_t platGetPIBErrorMask(void)
 // No spy access interface as HB doesn't allow spy access.
 // --------------------------------------------------------------------------
 
+/**
+* @brief Determine if a given target is on the master proc chip
+* @param[in]  i_Target   TARGETING::Target which op is being called on
+* @param[out] i_isMaster True if on master proc chip, false if not
+* @return errlHndl_t
+*/
+errlHndl_t isOnMasterProc(TARGETING::Target * i_target, bool & o_isMaster)
+{
+    errlHndl_t l_errl = nullptr;
+    assert(i_target != nullptr, "isOnMasterProc:: Cannot pass nullptr target to isOnMasterProc");
+    TARGETING::Target* l_pMasterProcChip = nullptr;
+    TARGETING::Target* l_pParentProcChip = nullptr;
+    TARGETING::targetService().masterProcChipTargetHandle( l_pMasterProcChip );
+    assert(l_pMasterProcChip != nullptr, "isOnMasterProc:: Unable to find the system's master proc chip target handle");
+    o_isMaster = false;
+    l_errl = getCfamChipTarget(i_target, l_pMasterProcChip);
+
+    if(l_errl == nullptr)
+    {
+        if(l_pMasterProcChip == l_pParentProcChip)
+        {
+            o_isMaster = true;
+        }
+    }
+    return l_errl;
+}
 
 } // End namespace
 
diff --git a/src/usr/fapi2/runtime/test/makefile b/src/usr/fapi2/runtime/test/makefile
index 88cb33044..a8a44b36c 100644
--- a/src/usr/fapi2/runtime/test/makefile
+++ b/src/usr/fapi2/runtime/test/makefile
@@ -35,7 +35,7 @@ include ../../test/fapi2Test.mk
 $(info TESTS before is ${TESTS})
 TMPVAR1 := ${TESTS}
 #RTC: 181003 need to determine why fapi2HwpErrorBufferTest.H is getting data storage exception in RT
-TESTS = $(filter-out ${ROOTPATH}/src/usr/fapi2/test/getVpdTest.H ${ROOTPATH}/src/usr/fapi2/test/fapi2GetVpdTest.H ${ROOTPATH}/src/usr/fapi2/test/fapi2HwpErrorBufferTest.H ,  ${TMPVAR1})
+TESTS = $(filter-out ${ROOTPATH}/src/usr/fapi2/test/getVpdTest.H ${ROOTPATH}/src/usr/fapi2/test/fapi2GetVpdTest.H ${ROOTPATH}/src/usr/fapi2/test/fapi2HwpErrorBufferTest.H ${ROOTPATH}/src/usr/fapi2/test/fapi2SubroutineExecutorTest.H  ,  ${TMPVAR1})
 TMPVAR2 := ${OBJS}
 OBJS = $(filter-out getVpdTest.o, ${TMPVAR2})
 include ${ROOTPATH}/config.mk
diff --git a/src/usr/fapi2/test/fapi2SubroutineExecutorTest.H b/src/usr/fapi2/test/fapi2SubroutineExecutorTest.H
new file mode 100644
index 000000000..5cb044ae5
--- /dev/null
+++ b/src/usr/fapi2/test/fapi2SubroutineExecutorTest.H
@@ -0,0 +1,215 @@
+/* IBM_PROLOG_BEGIN_TAG                                                   */
+/* This is an automatically generated prolog.                             */
+/*                                                                        */
+/* $Source: src/usr/fapi2/test/fapi2SubroutineExecutorTest.H $            */
+/*                                                                        */
+/* OpenPOWER HostBoot Project                                             */
+/*                                                                        */
+/* Contributors Listed Below - COPYRIGHT 2016,2017                        */
+/* [+] International Business Machines Corp.                              */
+/*                                                                        */
+/*                                                                        */
+/* Licensed under the Apache License, Version 2.0 (the "License");        */
+/* you may not use this file except in compliance with the License.       */
+/* You may obtain a copy of the License at                                */
+/*                                                                        */
+/*     http://www.apache.org/licenses/LICENSE-2.0                         */
+/*                                                                        */
+/* Unless required by applicable law or agreed to in writing, software    */
+/* distributed under the License is distributed on an "AS IS" BASIS,      */
+/* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or        */
+/* implied. See the License for the specific language governing           */
+/* permissions and limitations under the License.                         */
+/*                                                                        */
+/* IBM_PROLOG_END_TAG                                                     */
+#include <fapi2.H>
+#include <fapi2_subroutine_executor.H>
+
+//--------------------------------------------------------------------------
+/// @file  fapi2SubroutineExecutorTest.C
+///
+/// @brief This does tests of the FAPI try and FAPI assert MACROs without
+///        needing full ReturnCode support.
+//--------------------------------------------------------------------------
+
+namespace fapi2
+{
+
+    class Fapi2SubroutineExecutorTest : public CxxTest::TestSuite
+{
+    private:
+
+        // a pretend HWP that we will use for simple testing
+        //note that function name does not match anything defined as acceptable hwp names in sbe_secureHwp.C
+        fapi2::ReturnCode procedure_to_call(TARGETING::Target * i_target, uint32_t i_data, uint32_t& o_data )
+        {
+            fapi2::ReturnCode l_rc = FAPI2_RC_SUCCESS;
+
+            if(i_data == 0 )
+            {
+                o_data = 1;
+            }
+            else if( i_data == 1 )
+            {
+                l_rc = FAPI2_RC_FALSE;
+            }
+            return l_rc;
+        }
+
+        // a pretend HWP that we will use for simple testing
+        //note that function name matches what is defined as acceptable hwp name in sbe_secureHwp.C
+        fapi2::ReturnCode test_hwp(TARGETING::Target * i_target, uint32_t i_data, uint32_t& o_data )
+        {
+            fapi2::ReturnCode l_rc = FAPI2_RC_SUCCESS;
+
+            if(i_data == 0 )
+            {
+                o_data = 1;
+            }
+            else if( i_data == 1 )
+            {
+                l_rc = FAPI2_RC_FALSE;
+            }
+            return l_rc;
+        }
+
+    // Request to run a hwp that is on Hostboot's supported list
+    // We expect that hostboot will send a secureHwp chip op
+    // but it will fail because SBE doesn't support and instead
+    // we will just run hwp on host
+    // Test that the function runs okay
+    fapi2::ReturnCode  p9_fapi_subroutineExecutor_ValidHwp(  )
+    {
+        FAPI_INF("p9_fapi_subroutineExecutor_ValidHwp starting ... ");
+        fapi2::ReturnCode l_rc = FAPI2_RC_SUCCESS;
+        // Get a list of all of the proc chips and use the first we find as our target
+        // then cast it to a fapi2 target
+        TARGETING::TargetHandleList l_chipList;
+        TARGETING::getAllChips(l_chipList, TARGETING::TYPE_PROC, true);
+        assert(l_chipList.size() > 0, "Could not find a functional proc chip, something is wrong");
+        TARGETING::Target * l_proc = l_chipList[0];
+
+        Target<fapi2::TARGET_TYPE_PROC_CHIP> fapi2_procTarget(l_proc);
+
+        uint32_t  l_var1 = 0;
+        uint32_t  l_var2 = 0;
+
+
+        FAPI_PLAT_CALL_SUBROUTINE(l_rc, test_hwp, fapi2_procTarget, l_var1, l_var2);
+
+        if(l_var2 != 1)
+        {
+            l_rc = 0xDEAD;
+        }
+        FAPI_INF("p9_fapi_subroutineExecutor_ValidHwp complete ... ");
+        return l_rc;
+    }
+
+    // Request to run a hwp that isn't on Hostboot's supported list
+    // We expect that hostboot will still send secureHwp chip op
+    // but it will fail and instead we will just run hwp on host
+    // Test that the function runs okay
+    fapi2::ReturnCode  p9_fapi_subroutineExecutor_InvalidHwp(  )
+    {
+        FAPI_INF("p9_fapi_subroutineExecutor_InvalidHwp starting ... ");
+        fapi2::ReturnCode l_rc = FAPI2_RC_SUCCESS;
+        // Get a list of all of the proc chips and use the first we find as our target
+        // then cast it to a fapi2 target
+        TARGETING::TargetHandleList l_chipList;
+        TARGETING::getAllChips(l_chipList, TARGETING::TYPE_PROC, true);
+        assert(l_chipList.size() > 0, "Could not find a functional proc chip, something is wrong");
+        TARGETING::Target * l_proc = l_chipList[0];
+        Target<fapi2::TARGET_TYPE_PROC_CHIP> fapi2_procTarget(l_proc);
+
+        uint32_t  l_var1 = 0;
+        uint32_t  l_var2 = 0;
+        FAPI_PLAT_CALL_SUBROUTINE(l_rc, procedure_to_call, fapi2_procTarget, l_var1, l_var2);
+
+        if(l_rc == FAPI2_RC_FALSE)
+        {
+            FAPI_INF("p9_fapi_subroutineExecutor_InvalidHwp:: l_var1: %x l_var2: %x   l_rc: FALSE", l_var1, l_var2);
+        }
+        else
+        {
+            FAPI_INF("p9_fapi_subroutineExecutor_InvalidHwp:: l_var1: %x l_var2: %x   l_rc: SUCESS", l_var1, l_var2);
+        }
+
+        if(l_var2 != 1)
+        {
+            l_rc = 0xDEAD;
+        }
+        FAPI_INF("p9_fapi_subroutineExecutor_InvalidHwp complete ... ");
+        return l_rc;
+    }
+
+    // Request to run a hwp that isn't on Hostboot's supported list
+    // We expect that hostboot will still send secureHwp chip op
+    // but it will fail and instead we will just run hwp on host
+    // This test checks that if the hwp fails we get the RC correctly
+    fapi2::ReturnCode  p9_fapi_subroutineExecutor_InvalidHwp_RcCheck(  )
+    {
+        FAPI_INF("p9_fapi_subroutineExecutor_InvalidHwp_RcCheck started ... ");
+        fapi2::ReturnCode l_rc = FAPI2_RC_SUCCESS;
+        // Get a list of all of the proc chips and use the first we find as our target
+        // then cast it to a fapi2 target
+        TARGETING::TargetHandleList l_chipList;
+        TARGETING::getAllChips(l_chipList, TARGETING::TYPE_PROC, true);
+        TARGETING::Target * l_proc = l_chipList[0];
+        assert(l_chipList.size() > 0, "Could not find a functional proc chip, something is wrong");
+        Target<fapi2::TARGET_TYPE_PROC_CHIP> fapi2_procTarget(l_proc);
+
+        uint32_t  l_var1 = 1;
+        uint32_t  l_var2 = 1;
+        FAPI_PLAT_CALL_SUBROUTINE(l_rc, procedure_to_call, fapi2_procTarget, l_var1, l_var2);
+        if(l_rc == FAPI2_RC_FALSE)
+        {
+            FAPI_INF("p9_fapi_subroutineExecutor_InvalidHwp_RcCheck:: l_var1: %x l_var2: %x   l_rc: FALSE", l_var1, l_var2);
+        }
+        else
+        {
+            FAPI_INF("p9_fapi_subroutineExecutor_InvalidHwp_RcCheck:: l_var1: %x l_var2: %x   l_rc: SUCESS", l_var1, l_var2);
+        }
+        FAPI_INF("p9_fapi_subroutineExecutor_InvalidHwp_RcCheck complete ... ");
+        return l_rc;
+    }
+
+  public:
+    //******************************************************************************
+    // test_fapi2SubroutineExecutor
+    //******************************************************************************
+
+    void  test_fapi2SubroutineExecutor()
+    {
+        int numTests = 0;
+        int numFails = 0;
+        fapi2::ReturnCode l_rc = FAPI2_RC_SUCCESS;
+
+        FAPI_INF("test_fapi2SubroutineExecutor starting ... ");
+        numTests++;
+        l_rc = p9_fapi_subroutineExecutor_InvalidHwp();
+        if (l_rc != FAPI2_RC_SUCCESS)
+        {
+            numFails++;
+            TS_FAIL(" p9_fapi_subroutineExecutor_InvalidHwp returned bad RC!");
+        }
+
+        numTests++;
+        l_rc = p9_fapi_subroutineExecutor_InvalidHwp_RcCheck();
+        if (l_rc  != FAPI2_RC_FALSE)
+        {
+            numFails++;
+            TS_FAIL(" p9_fapi_subroutineExecutor_InvalidHwp_RcCheck failed to return bad RC!");
+        }
+
+        //TODO RTC:180100 once the SBE support comes in we want SBE team to add support
+        //for a test HWP so we can validate the chipOp request path works. This also
+        //requires a 2 socket sytem model (might not be ready until cumulus)
+        //p9_fapi_subroutineExecutor_ValidHwp
+
+        FAPI_INF("test_fapi2SubroutineExecutor:: Test Complete. %d/%d fails", numFails, numTests);
+
+    } // end main testcase driver
+
+}; // end class
+
+} // end namespace fapi2
diff --git a/src/usr/sbeio/sbe_fifodd.H b/src/usr/sbeio/sbe_fifodd.H
index ae921034a..9bae59d91 100644
--- a/src/usr/sbeio/sbe_fifodd.H
+++ b/src/usr/sbeio/sbe_fifodd.H
@@ -143,7 +143,6 @@ class SbeFifo
             uint8_t  chipletId;
             uint8_t * dataPtr;
             fifoSecureHwpRequest(uint64_t i_dataSizeBytes,
-                                 uint64_t i_hwpStringLen,
                                  uint8_t * i_dataPtr) :
             reserved(0), commandClass(SBE_FIFO_CLASS_SECURE_HWP), command(0), targetType(0), chipletId(0)
             {
@@ -155,7 +154,7 @@ class SbeFifo
                     wordCnt++;
                 }
                 dataPtr = reinterpret_cast<uint8_t *>(malloc(i_dataSizeBytes));
-                memcpy(dataPtr, i_dataPtr + i_hwpStringLen, i_dataSizeBytes);
+                memcpy(dataPtr, i_dataPtr, i_dataSizeBytes);
             }
             ~fifoSecureHwpRequest(){ free(dataPtr);};
 
diff --git a/src/usr/sbeio/sbe_secureHwp.C b/src/usr/sbeio/sbe_secureHwp.C
index 208c853fb..f6b3117ba 100644
--- a/src/usr/sbeio/sbe_secureHwp.C
+++ b/src/usr/sbeio/sbe_secureHwp.C
@@ -49,7 +49,7 @@ namespace SBEIO
 {
     //List out name of valid hwps that have chipop equivalents
     //these static variable will be used by
-    static const char* test_hwp = "p9_pm_ocb_indir_access"; // SBE_FIFO_CMD_PLACEHOLDER_HWP
+    static const char* test_hwp = "test_hwp"; // SBE_FIFO_CMD_PLACEHOLDER_HWP
 
     /**
     * @brief Convert a hwp name passed in as a string to a chipOp code
@@ -57,7 +57,7 @@ namespace SBEIO
     * @return fifoSecureHwpMessage  returns a chipOp representing the HWP, if found
     *                               otherwise returns UNSUPPORTED_HWP enum
     */
-    SbeFifo::fifoSecureHwpMessage convertHwpStringToOpCode(char* i_hwpName)
+    SbeFifo::fifoSecureHwpMessage convertHwpStringToOpCode(const char* i_hwpName)
     {
         //Default to undefined HWP
         SbeFifo::fifoSecureHwpMessage l_hwpOpCode = SbeFifo::fifoSecureHwpMessage::SBE_FIFO_CMD_UNSUPPORTED_HWP;
@@ -82,7 +82,7 @@ namespace SBEIO
     * @param[in] i_dataSize     Size of blob of data that contains additional parameters
     *                           for the requests HWP
     *
-    * @param[in] i_hwpStringLen size of the hwp name string at beginning of data pointer
+    * @param[in] i_hwpName      Pointer to string of chars representing hwp name
     *
     * @return errlHndl_t Error log handle on failure.
     *
@@ -90,19 +90,16 @@ namespace SBEIO
     errlHndl_t sendSecureHwpRequest(TARGETING::Target * i_target,
                                     uint8_t * i_dataPointer,
                                     uint64_t i_dataSize,
-                                    uint64_t i_hwpStringLen)
+                                    const char * i_hwpName)
     {
         errlHndl_t errl = nullptr;
         do
         {
             SBE_TRACD(ENTER_MRK "sendSecureHwpRequest");
+            //First we need to figure out if this is a proc, if it isn't
+            //then we need to find its parent proccessor chip
             auto l_targType =  i_target->getAttr<TARGETING::ATTR_TYPE>();
             TARGETING::Target * l_proc;
-
-            //Copy out the hwp name string into a local buffer
-            char l_hwpName[i_hwpStringLen];
-            memcpy(l_hwpName, i_dataPointer, i_hwpStringLen);
-
             if(l_targType == TARGETING::TYPE_PROC)
             {
                 l_proc = i_target;
@@ -112,11 +109,11 @@ namespace SBEIO
                 l_proc = const_cast<TARGETING::Target *>(getParentChip(i_target));
             }
 
-            SbeFifo::fifoSecureHwpRequest       l_fifoRequest(i_dataSize, i_hwpStringLen, i_dataPointer);
+            SbeFifo::fifoSecureHwpRequest       l_fifoRequest(i_dataSize, i_dataPointer);
             SbeFifo::fifoStandardResponse       l_fifoResponse;
 
             //Command is computed by converting hwp string to function
-            l_fifoRequest.command      = convertHwpStringToOpCode(l_hwpName);
+            l_fifoRequest.command      = convertHwpStringToOpCode(i_hwpName);
             l_fifoRequest.targetType   = translateToSBETargetType(i_target);
             l_fifoRequest.chipletId    = getChipletIDForSBE(i_target);