Trace Processor Security Registers; add them to Secure Error Logs

This commit adds a trace of the Security Switch and CBS Control/Status
registers for all processors in the system.  These registers are also
captured for Security-specific error logs.

Change-Id: I245815c720725a9aaf15a3cbad9a50b3288fc1f9
RTC:165205
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/37290
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>
Reviewed-by: Stephen M. Cprek <smcprek@us.ibm.com>
Reviewed-by: Nicholas E. Bofferding <bofferdn@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>

8 files changed, 459 insertions, 14 deletions

diff --git a/src/include/usr/secureboot/service.H b/src/include/usr/secureboot/service.H
index 73b254fd8..86da6a64b 100644
--- a/src/include/usr/secureboot/service.H
+++ b/src/include/usr/secureboot/service.H
@@ -64,6 +64,17 @@ namespace SECUREBOOT
      */
     errlHndl_t initializeSecureRomManager(void);
 
+    /**
+     * @brief Trace the Security Settings on each functional processor
+     *
+     *  @param[in] i_doConsoleTrace Optional variable that determines if
+     *                              register values are traced to the CONSOLE.
+     *                              Default is false
+     *
+     *  @return errlHndl_t  nullptr on success, else pointer to error log
+     */
+    errlHndl_t traceSecuritySettings(bool i_doConsoleTrace = false);
+
 
     /** @brief Determines if Secureboot is enabled.
      */
diff --git a/src/include/usr/secureboot/settings.H b/src/include/usr/secureboot/settings.H
index 08681e08e..1de694948 100644
--- a/src/include/usr/secureboot/settings.H
+++ b/src/include/usr/secureboot/settings.H
@@ -69,7 +69,8 @@ namespace SECUREBOOT
         SabBit         = 0x0800000000000000ull, // Secure access
         JumperStateBit = 0x0400000000000000ull, // Secure jumper
 
-        StatusRegister = 0x00050001ull,
+        StatusRegister    = 0x00050001ull, // used for SCOM operation
+        StatusRegisterFsi = 0x00002804ull, // used for FSI operation
     };
 
     /** @class Settings
diff --git a/src/usr/isteps/istep06/call_host_update_master_tpm.C b/src/usr/isteps/istep06/call_host_update_master_tpm.C
index d378d9cd7..b08ca5bce 100644
--- a/src/usr/isteps/istep06/call_host_update_master_tpm.C
+++ b/src/usr/isteps/istep06/call_host_update_master_tpm.C
@@ -5,7 +5,7 @@
 /*                                                                        */
 /* OpenPOWER HostBoot Project                                             */
 /*                                                                        */
-/* Contributors Listed Below - COPYRIGHT 2015,2016                        */
+/* Contributors Listed Below - COPYRIGHT 2015,2017                        */
 /* [+] International Business Machines Corp.                              */
 /*                                                                        */
 /*                                                                        */
@@ -30,6 +30,7 @@
 #include <isteps/hwpisteperror.H>
 #include <trustedbootif.H>
 #include <initservice/isteps_trace.H>
+#include <secureboot/service.H>
 
 namespace ISTEP_06
 {
@@ -41,17 +42,29 @@ void* call_host_update_master_tpm( void *io_pArgs )
     TRACDCOMP( ISTEPS_TRACE::g_trac_isteps_trace,
                "call_host_update_master_tpm entry" );
 
-#ifdef CONFIG_TPMDD
     errlHndl_t l_err = NULL;
+
+#ifdef CONFIG_TPMDD
     // Initialize the master TPM
     l_err = (errlHndl_t)TRUSTEDBOOT::host_update_master_tpm(io_pArgs);
     if (l_err)
     {
         l_stepError.addErrorDetails(l_err);
-        errlCommit( l_err, SECURE_COMP_ID );
+        ERRORLOG::errlCommit( l_err, SECURE_COMP_ID );
     }
 #endif
 
+    l_err = SECUREBOOT::traceSecuritySettings(true);
+    if (l_err)
+    {
+        TRACFCOMP( ISTEPS_TRACE::g_trac_isteps_trace,
+                   "call_host_update_master_tpm: Error back from "
+                   "SECUREBOOT::traceSecuritySettings: rc=0x%X, plid=0x%X",
+                   ERRL_GETRC_SAFE(l_err), ERRL_GETPLID_SAFE(l_err));
+        l_stepError.addErrorDetails(l_err);
+        ERRORLOG::errlCommit( l_err, SECURE_COMP_ID );
+    }
+
     TRACDCOMP( ISTEPS_TRACE::g_trac_isteps_trace,
                "call_host_update_master_tpm exit" );
 
diff --git a/src/usr/isteps/istep10/call_host_update_redundant_tpm.C b/src/usr/isteps/istep10/call_host_update_redundant_tpm.C
index 2d9a8a8cd..ffdf52980 100644
--- a/src/usr/isteps/istep10/call_host_update_redundant_tpm.C
+++ b/src/usr/isteps/istep10/call_host_update_redundant_tpm.C
@@ -55,6 +55,25 @@ namespace ISTEP_10
 {
 void* call_host_update_redundant_tpm (void *io_pArgs)
 {
+    TRACFCOMP(ISTEPS_TRACE::g_trac_isteps_trace,
+                ENTER_MRK"call_host_update_redundant_tpm");
+
+    errlHndl_t err = nullptr;
+
+    // Before update procedure, trace security settings
+    err = SECUREBOOT::traceSecuritySettings();
+    if (err)
+    {
+        TRACFCOMP( ISTEPS_TRACE::g_trac_isteps_trace,
+                   "call_host_update_redundant_tpm: Error back from "
+                   "SECUREBOOT::traceSecuritySettings: rc=0x%X, plid=0x%X",
+                   ERRL_GETRC_SAFE(err), ERRL_GETPLID_SAFE(err));
+
+        // Commit log, but continue
+        ERRORLOG::errlCommit( err, SECURE_COMP_ID );
+    }
+
+    // Start of update procedure
     #ifdef CONFIG_SECUREBOOT
     do {
 
@@ -103,8 +122,6 @@ void* call_host_update_redundant_tpm (void *io_pArgs)
                 >(l_protectTpm);
         }
 
-        errlHndl_t err = nullptr;
-
         const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP> l_fapiTarg(pProc);
 
         FAPI_INVOKE_HWP(err, p9_update_security_ctrl, l_fapiTarg);
@@ -169,6 +186,9 @@ void* call_host_update_redundant_tpm (void *io_pArgs)
 
     #endif // CONFIG_SECUREBOOT
 
+    TRACFCOMP(ISTEPS_TRACE::g_trac_isteps_trace,
+                EXIT_MRK"call_host_update_redundant_tpm");
+
     return nullptr;
 }
 
diff --git a/src/usr/secureboot/base/securerommgr.C b/src/usr/secureboot/base/securerommgr.C
index 4f92b3d14..1a21f3350 100644
--- a/src/usr/secureboot/base/securerommgr.C
+++ b/src/usr/secureboot/base/securerommgr.C
@@ -208,7 +208,7 @@ errlHndl_t SecureRomManager::initialize()
                                    reinterpret_cast<uint64_t>(iv_securerom),
                                    true /*Add HB Software Callout*/ );
 
-            l_errl->collectTrace(SECURE_COMP_NAME,256);
+            l_errl->collectTrace(SECURE_COMP_NAME,ERROR_TRACE_SIZE);
             break;
 
         }
@@ -216,7 +216,6 @@ errlHndl_t SecureRomManager::initialize()
         /***************************************************************/
         /*  Retrieve HW Hash Keys From The System                      */
         /***************************************************************/
-
         SecureRomManager::getHwKeyHash();
 
         TRACFCOMP(g_trac_secure,INFO_MRK"SecureRomManager::initialize(): SUCCESSFUL:"
@@ -336,7 +335,7 @@ errlHndl_t SecureRomManager::verifyContainer(void * i_container,
             // Callout code to force a rewrite of the contents
             //@todo RTC:93870 - Define new callout for verification fail
 
-            l_errl->collectTrace(SECURE_COMP_NAME,256);
+            l_errl->collectTrace(SECURE_COMP_NAME,ERROR_TRACE_SIZE);
             break;
 
         }
diff --git a/src/usr/secureboot/base/service.C b/src/usr/secureboot/base/service.C
index 05a29acc3..075a7313c 100644
--- a/src/usr/secureboot/base/service.C
+++ b/src/usr/secureboot/base/service.C
@@ -34,6 +34,7 @@
 #include <errl/errlentry.H>
 #include <errl/errlmanager.H>
 #include <errl/errludtarget.H>
+#include <errl/errludlogregister.H>
 #include <initservice/initserviceif.H>
 #include <secureboot/settings.H>
 #include <secureboot/header.H>
@@ -41,6 +42,7 @@
 #include <kernel/misc.H>
 #include <kernel/console.H>
 #include <console/consoleif.H>
+#include <util/misc.H>
 
 #include "../common/securetrace.H"
 
@@ -58,6 +60,44 @@ namespace SECUREBOOT
 // TODO securebootp9 - Do a diff of this file with the p8 version make sure
 // all the missing parts are brought in.
 
+
+// Local structure and function prototypes used below
+/**
+ *  @brief Structure used to capture values of Processor Security Registers
+ */
+struct SecureRegisterValues
+{
+    TARGETING::Target * tgt;
+    uint32_t addr;
+    uint64_t data;
+};
+
+/**
+ *  @brief Retrieve values of Security Registers of the processors in the system
+ *
+ *  @param[out] o_regs  Vector of SecureRegisterValue structs that contain
+ *                      processor security register values
+ *                      NOTE:  The state of the system/processors (ie, SCOM vs
+ *                      FSI) determines which registers can be included
+ *
+ * @return errlHndl_t  nullptr on success, else pointer to error log
+ */
+errlHndl_t getAllSecurityRegisters(std::vector<SecureRegisterValues> & o_regs);
+
+/**
+ *  @brief Adds the values of the Security Registers of the processors in the
+ *         system to an existing error log
+ *
+ *  @param[in/out] io_err  Error Log that the values of the security registers
+ *                         will be added to
+ *                         NOTE:  The state of the system/processors (ie, SCOM
+ *                         vs FSI) determines which registers can be included
+ *
+ *  @return N/A
+ */
+void addSecurityRegistersToErrlog(errlHndl_t & io_err);
+
+
 void* initializeBase(void* unused)
 {
     errlHndl_t l_errl = NULL;
@@ -144,10 +184,371 @@ void handleSecurebootFailure(errlHndl_t &io_err, bool i_waitForShutdown)
     // Add Verification callout
     io_err->addProcedureCallout(HWAS::EPUB_PRC_FW_VERIFICATION_ERR,
                                HWAS::SRCI_PRIORITY_HIGH);
+
+    // Add security register values
+    addSecurityRegistersToErrlog(io_err);
+    io_err->collectTrace(SECURE_COMP_NAME,ERROR_TRACE_SIZE);
+
     errlCommit(io_err, SECURE_COMP_ID);
 
     // Shutdown with Secureboot error status
     INITSERVICE::doShutdown(l_rc, !i_waitForShutdown);
 }
 
+
+errlHndl_t getAllSecurityRegisters(std::vector<SecureRegisterValues> & o_regs)
+{
+    SB_ENTER("getAllSecurityRegisters: isTargetingLoaded=%d",
+             Util::isTargetingLoaded());
+    errlHndl_t err = nullptr;
+
+    // Clear output vector
+    o_regs.clear();
+
+    SecureRegisterValues l_secRegValues;
+
+    do
+    {
+
+    TARGETING::TargetHandleList procList;
+    TARGETING::Target* masterProcChipTargetHandle = nullptr;
+
+    if ( Util::isTargetingLoaded()  )
+    {
+        // Try to get a list of functional processors
+
+        // Get Target Service, and the system target.
+        TargetService& tS = targetService();
+        TARGETING::Target* sys = nullptr;
+        (void) tS.getTopLevelTarget( sys );
+        assert(sys, "getAllSecurityRegisters() system target is nullptr");
+
+        TARGETING::getAllChips(procList,
+                               TARGETING::TYPE_PROC,
+                               true); // true: return functional targets
+
+        // Get the Master Proc Chip Target for comparisons later
+        err = tS.queryMasterProcChipTargetHandle(masterProcChipTargetHandle);
+
+        if (err)
+        {
+            SB_ERR("getAllSecurityRegisters: "
+                   "queryMasterProcChipTargetHandle returned error: "
+                   "RC=0x%X, PLID=0x%X",
+                   ERRL_GETRC_SAFE(err),
+                   ERRL_GETPLID_SAFE(err));
+
+            // Commit error and continue
+            errlCommit( err, SECURE_COMP_ID );
+            masterProcChipTargetHandle = nullptr;
+
+            // Since we can't get master proc, don't trust targeting and
+            // just use MASTER_PROCESSOR_CHIP_TARGET_SENTINEL
+            procList.clear();
+        }
+    }
+
+    if ( procList.size() != 0 )
+    {
+        // Grab data from all of the targets
+        uint64_t scomData = 0x0;
+        size_t   op_expected_size  = 0x0;
+        size_t   op_actual_size = 0x0;
+        uint64_t op_addr  = 0x0;
+
+
+        for( auto procTgt : procList )
+        {
+            SB_DBG("getAllSecurityRegisters: procTgt=0x%X: useXscom=%d",
+                   TARGETING::get_huid(procTgt), procTgt->getAttr<ATTR_SCOM_SWITCHES>().useXscom);
+
+            /****************************************/
+            // Get ProcSecurity::SwitchRegister
+            /****************************************/
+            // can only get register if processor target is scommable
+            // If the proc chip supports xscom..
+            if (procTgt->getAttr<ATTR_SCOM_SWITCHES>().useXscom)
+            {
+                l_secRegValues.tgt=procTgt;
+                l_secRegValues.addr=static_cast<uint32_t>(ProcSecurity::SwitchRegister);
+                err = getSecuritySwitch(l_secRegValues.data,
+                                        l_secRegValues.tgt);
+
+                if( err )
+                {
+                    // Something failed on the read.  Commit the error
+                    // here but continue
+                    SB_ERR("getAllSecurityRegisters: Error from getSecuritySwitch: "
+                           "(0x%X) from Target 0x%.8X: RC=0x%X, PLID=0x%X",
+                           l_secRegValues.addr,
+                           TARGETING::get_huid(l_secRegValues.tgt),
+                           ERRL_GETRC_SAFE(err), ERRL_GETPLID_SAFE(err));
+
+                    // Commit error and continue
+                    errlCommit( err, SECURE_COMP_ID );
+                    continue;
+                }
+                o_regs.push_back(l_secRegValues);
+            }
+
+            /****************************************/
+            // Get ProcCbsControl::StatusRegister
+            /****************************************/
+            // Check to see if current target is master processor
+            if ( procTgt == masterProcChipTargetHandle)
+            {
+                SB_DBG("getAllSecurityRegisters: procTgt=0x%X is MASTER. ",
+                       TARGETING::get_huid(procTgt));
+
+                // Read ProcCbsControl::StatusRegister via SCOM
+                scomData = 0x0;
+                op_actual_size = sizeof(scomData);
+                op_expected_size = op_actual_size;
+                op_addr = static_cast<uint64_t>(ProcCbsControl::StatusRegister);
+
+                err = deviceRead( procTgt,
+                                  &scomData,
+                                  op_actual_size,
+                                  DEVICE_SCOM_ADDRESS(op_addr) );
+            }
+            else
+            {
+                SB_DBG("getAllSecurityRegisters: procTgt=0x%X is NOT MASTER. ",
+                       TARGETING::get_huid(procTgt));
+
+                // Not Master, so read ProcCbsControl::StatusRegister via FSI
+                scomData = 0x0;
+                op_actual_size = 4; // size for FSI
+                op_expected_size = op_actual_size;
+                op_addr = static_cast<uint64_t>(ProcCbsControl::StatusRegisterFsi);
+
+                err = deviceRead( procTgt,
+                                  &scomData,
+                                  op_actual_size,
+                                  DEVICE_FSI_ADDRESS(op_addr) );
+            }
+
+            assert(op_actual_size == op_expected_size,"getAllSecurityRegisters: BUG! size returned from device write (%d) is not the expected size of %d", op_actual_size, op_expected_size);
+
+            if( err )
+            {
+                // Something failed on the read.  Commit the error
+                // here but continue
+                SB_ERR("getAllSecurityRegisters: Error reading CBS Control Reg "
+                       "(0x%X) from Target 0x%.8X: RC=0x%X, PLID=0x%X",
+                       op_addr, TARGETING::get_huid(procTgt),
+                       ERRL_GETRC_SAFE(err), ERRL_GETPLID_SAFE(err));
+
+                // Commit error and continue
+                errlCommit( err, SECURE_COMP_ID );
+                continue;
+            }
+            // push back result
+            l_secRegValues.tgt=procTgt;
+            l_secRegValues.addr=op_addr;
+            l_secRegValues.data=scomData;
+            o_regs.push_back(l_secRegValues);
+
+        } // end of targeting loop
+
+    } // TargetList has some targets
+
+    else
+    {
+        // Since targeting is NOT loaded or TargetList is empty only capture
+        // data for MASTER_PROCESSOR_CHIP_TARGET_SENTINEL
+        l_secRegValues.tgt=TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL;
+        l_secRegValues.addr=static_cast<uint32_t>(ProcSecurity::SwitchRegister);
+        err = getSecuritySwitch(l_secRegValues.data,
+                                l_secRegValues.tgt);
+
+        if( err )
+        {
+            // Something failed on the read.  Commit the error
+            // here but continue
+            SB_ERR("getAllSecurityRegisters: Error from getSecuritySwitch: "
+                   "(0x%X) from Target 0x%.8X: RC=0x%X, PLID=0x%X",
+                   l_secRegValues.addr,
+                   TARGETING::get_huid(l_secRegValues.tgt),
+                   ERRL_GETRC_SAFE(err), ERRL_GETPLID_SAFE(err));
+
+            // Commit error and continue
+            errlCommit( err, SECURE_COMP_ID );
+            break;
+        }
+        o_regs.push_back(l_secRegValues);
+
+
+    } // using MASTER_PROCESSOR_CHIP_TARGET_SENTINEL
+
+
+    } while(0);
+
+    SB_EXIT("getAllSecurityRegisters(): err rc=0x%X, plid=0x%X, "
+            "o_regs.size()=%d",
+            ERRL_GETRC_SAFE(err), ERRL_GETPLID_SAFE(err),
+            o_regs.size());
+
+    return err;
+}
+
+errlHndl_t traceSecuritySettings(bool i_doConsoleTrace)
+{
+    SB_ENTER("traceSecuritySettings(): i_doConsoleTrace=%d", i_doConsoleTrace);
+    errlHndl_t err = nullptr;
+
+    std::vector<SecureRegisterValues> registerList;
+    uint64_t l_SMDBits = 0;
+    uint64_t l_SABBits = 0;
+    TARGETING::ATTR_POSITION_type l_pos = 0;
+
+    do
+    {
+
+    err = getAllSecurityRegisters(registerList);
+
+    if (err)
+    {
+        SB_ERR("traceSecuritySettings: getAllSecurityRegisters returned error: "
+               "RC=0x%X, PLID=0x%X",
+               ERRL_GETRC_SAFE(err),
+               ERRL_GETPLID_SAFE(err));
+        break;
+    }
+
+    for( auto l_reg : registerList )
+    {
+        SB_DBG("traceSecuritySettings: register: tgt=0x%X, addr=0x%lX, data=0x%.16llX ",
+               TARGETING::get_huid(l_reg.tgt),
+               l_reg.addr, l_reg.data);
+
+        if ( l_reg.addr == static_cast<uint32_t>(ProcSecurity::SwitchRegister) )
+        {
+            SB_INF("procTgt=0x%X: ProcSecurity::SwitchRegister(0x%x): 0x%.16llX: "
+                   "SabBit=%d, SULBit=%d, SDBBit=%d, CMFSIBit=%d",
+                   TARGETING::get_huid(l_reg.tgt), l_reg.addr, l_reg.data,
+                   l_reg.data & static_cast<uint64_t>(ProcSecurity::SabBit)
+                    ? 1 : 0,
+                   l_reg.data & static_cast<uint64_t>(ProcSecurity::SULBit)
+                    ? 1 : 0 ,
+                   l_reg.data & static_cast<uint64_t>(ProcSecurity::SDBBit)
+                    ? 1 : 0 ,
+                   l_reg.data & static_cast<uint64_t>(ProcSecurity::CMFSIBit)
+                    ? 1 : 0 );
+        }
+
+        else if ( ( l_reg.addr == static_cast<uint32_t>(ProcCbsControl::StatusRegister) ) ||
+                  ( l_reg.addr == static_cast<uint32_t>(ProcCbsControl::StatusRegisterFsi) ) )
+        {
+            SB_INF("procTgt=0x%X: ProcCbsControl::StatusRegister(0x%x): 0x%.16llX: "
+                   "SabBit=%d, SmdBit=%d",
+                   TARGETING::get_huid(l_reg.tgt), l_reg.addr, l_reg.data,
+                   l_reg.data & static_cast<uint64_t>(ProcCbsControl::SabBit)
+                    ? 1 : 0,
+                   l_reg.data & static_cast<uint64_t>(ProcCbsControl::JumperStateBit)
+                    ? 1 : 0 );
+
+           if (i_doConsoleTrace == true)
+           {
+
+               // Process this register for console output below
+               l_pos=l_reg.tgt->getAttr<TARGETING::ATTR_POSITION>();
+
+               if (l_reg.data & static_cast<uint64_t>(ProcCbsControl::SabBit))
+               {
+                   l_SABBits |= (0x8000000000000000 >> l_pos);
+               }
+               if (l_reg.data & static_cast<uint64_t>(ProcCbsControl::JumperStateBit))
+               {
+                   l_SMDBits |= (0x8000000000000000 >> l_pos);
+               }
+           }
+
+        }
+
+    } // output vector loop
+
+    if (i_doConsoleTrace == true)
+    {
+#if (!defined(CONFIG_CONSOLE_OUTPUT_TRACE) && defined(CONFIG_CONSOLE))
+        // Using 2 uint32_t's due to CONSOLE BUG displaying uint64_t
+        CONSOLE::displayf("SECURE", "Security Access Bit> 0x%.8X%.8X",
+                          l_SABBits>>32, l_SABBits&0xFFFFFFFF );
+
+        CONSOLE::displayf("SECURE", "Secure Mode Disable (via Jumper)> 0x%.8X%.8X",
+                          l_SMDBits>>32, l_SMDBits&0xFFFFFFFF );
+#endif
+        SB_INF("Security Access Bit> 0x%.16llX", l_SABBits);
+        SB_INF("Secure Mode Disable (via Jumper)> 0x%.16llX", l_SMDBits);
+    }
+
+
+    } while(0);
+
+    SB_EXIT("traceSecuritySettings(): err rc=0x%X, plid=0x%X",
+            ERRL_GETRC_SAFE(err), ERRL_GETPLID_SAFE(err));
+
+    return err;
+}
+
+
+void addSecurityRegistersToErrlog(errlHndl_t & io_err)
+{
+    SB_ENTER("addSecurityRegistersToErrlog(): io_err rc=0x%X, plid=0x%X",
+             ERRL_GETRC_SAFE(io_err), ERRL_GETPLID_SAFE(io_err));
+
+    errlHndl_t new_err = nullptr;
+
+
+    std::vector<SecureRegisterValues> registerList;
+
+    do
+    {
+
+    new_err = getAllSecurityRegisters(registerList);
+
+    if (new_err)
+    {
+        SB_ERR("addSecurityRegistersToErrlog: getAllSecurityRegisters returned "
+               "error: RC=0x%X, PLID=0x%X. Commiting this error and NOT adding "
+               "data to io_err",
+               ERRL_GETRC_SAFE(new_err),
+               ERRL_GETPLID_SAFE(new_err));
+
+        // Commit error and break
+        errlCommit(new_err, SECURE_COMP_ID );
+        break;
+    }
+
+    for( auto l_reg : registerList )
+    {
+
+        if (l_reg.addr == static_cast<uint32_t>(ProcCbsControl::StatusRegisterFsi))
+        {
+            ERRORLOG::ErrlUserDetailsLogRegister l_logReg(l_reg.tgt,
+                                                      &l_reg.data,
+                                                      sizeof(l_reg.data),
+                                                      DEVICE_FSI_ADDRESS(l_reg.addr));
+            l_logReg.addToLog(io_err);
+        }
+        else
+        {
+            ERRORLOG::ErrlUserDetailsLogRegister l_logReg(l_reg.tgt,
+                                                      &l_reg.data,
+                                                      sizeof(l_reg.data),
+                                                      DEVICE_SCOM_ADDRESS(l_reg.addr));
+            l_logReg.addToLog(io_err);
+        }
+
+
+    } // end of registerList loop
+
+    } while(0);
+
+    SB_EXIT("addSecurityRegistersToErrlog(): io_err rc=0x%X, plid=0x%X",
+            ERRL_GETRC_SAFE(io_err), ERRL_GETPLID_SAFE(io_err));
+
+    return;
+}
+
 } //namespace SECUREBOOT
diff --git a/src/usr/secureboot/base/settings.C b/src/usr/secureboot/base/settings.C
index 83e5365e3..9e1cb2ee0 100644
--- a/src/usr/secureboot/base/settings.C
+++ b/src/usr/secureboot/base/settings.C
@@ -40,11 +40,6 @@ namespace SECUREBOOT
     using namespace TARGETING;
     using namespace ERRORLOG;
 
-    // symbolic constant for the trace size
-    enum {
-        ERROR_TRACE_SIZE = 256,
-    };
-
     void Settings::_init()
     {
         uint64_t l_regValue = 0;
diff --git a/src/usr/secureboot/common/securetrace.H b/src/usr/secureboot/common/securetrace.H
index 06d3bc6b5..7c7accb54 100644
--- a/src/usr/secureboot/common/securetrace.H
+++ b/src/usr/secureboot/common/securetrace.H
@@ -38,6 +38,11 @@ namespace SECUREBOOT
 
 extern trace_desc_t* g_trac_secure;
 
+// symbolic constant for the trace size
+enum {
+    ERROR_TRACE_SIZE = 256,
+};
+
 }
 
 #define SB_ENTER(args...) \