From e8b8f27d39f6633ff89f2dc541db77c802988685 Mon Sep 17 00:00:00 2001 From: Mike Baiocchi Date: Fri, 10 Feb 2017 09:06:46 -0600 Subject: 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 Tested-by: Jenkins OP Build CI Tested-by: FSP CI Jenkins Reviewed-by: Stephen M. Cprek Reviewed-by: Nicholas E. Bofferding Reviewed-by: Daniel M. Crowell --- src/include/usr/secureboot/service.H | 11 + src/include/usr/secureboot/settings.H | 3 +- .../isteps/istep06/call_host_update_master_tpm.C | 19 +- .../istep10/call_host_update_redundant_tpm.C | 24 +- src/usr/secureboot/base/securerommgr.C | 5 +- src/usr/secureboot/base/service.C | 401 +++++++++++++++++++++ src/usr/secureboot/base/settings.C | 5 - src/usr/secureboot/common/securetrace.H | 5 + 8 files changed, 459 insertions(+), 14 deletions(-) (limited to 'src') 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 #include #include +#include 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 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(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 #include #include +#include #include #include #include @@ -41,6 +42,7 @@ #include #include #include +#include #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 & 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 & 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().useXscom); + + /****************************************/ + // Get ProcSecurity::SwitchRegister + /****************************************/ + // can only get register if processor target is scommable + // If the proc chip supports xscom.. + if (procTgt->getAttr().useXscom) + { + l_secRegValues.tgt=procTgt; + l_secRegValues.addr=static_cast(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(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(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(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 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(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(ProcSecurity::SabBit) + ? 1 : 0, + l_reg.data & static_cast(ProcSecurity::SULBit) + ? 1 : 0 , + l_reg.data & static_cast(ProcSecurity::SDBBit) + ? 1 : 0 , + l_reg.data & static_cast(ProcSecurity::CMFSIBit) + ? 1 : 0 ); + } + + else if ( ( l_reg.addr == static_cast(ProcCbsControl::StatusRegister) ) || + ( l_reg.addr == static_cast(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(ProcCbsControl::SabBit) + ? 1 : 0, + l_reg.data & static_cast(ProcCbsControl::JumperStateBit) + ? 1 : 0 ); + + if (i_doConsoleTrace == true) + { + + // Process this register for console output below + l_pos=l_reg.tgt->getAttr(); + + if (l_reg.data & static_cast(ProcCbsControl::SabBit)) + { + l_SABBits |= (0x8000000000000000 >> l_pos); + } + if (l_reg.data & static_cast(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 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(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...) \ -- cgit v1.2.1