Remove Secure Boot workarounds

- Removing the magic number checks that would block sb functionality
  if things didn't appear secure
- Remove Best Effort Policy and all of its related code
- Remove the legacy PCR extension
- Remove the non-secure header preservation path.
- Always load HB base image header from the bl to hb data path
  vs settings unsecurely out of pnor

Change-Id: Ie638384ac50ed47850985c959ea7a32e5757d64e
RTC: 178520
RTC: 155374
RTC: 173489
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/49925
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Reviewed-by: Nicholas E. Bofferding <bofferdn@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: Michael Baiocchi <mbaiocch@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>

15 files changed, 109 insertions, 613 deletions

diff --git a/src/usr/pnor/pnor_common.C b/src/usr/pnor/pnor_common.C
index 20772964d..d262ebe82 100644
--- a/src/usr/pnor/pnor_common.C
+++ b/src/usr/pnor/pnor_common.C
@@ -303,32 +303,6 @@ errlHndl_t PNOR::parseTOC( uint8_t* i_tocBuffer,SectionData_t * o_TOC)
                               cur_entry->name);
 
             }
-
-#ifndef __HOSTBOOT_RUNTIME
-            if (PNOR::hasNonSecureHeader(o_TOC[l_secId]))
-            {
-                // Never extend the base image through this path, it will be
-                // handled elsewhere
-                if(l_secId != PNOR::HB_BASE_CODE)
-                {
-                    // For non-secure sections with a SHA512 header, the
-                    // flash address has incremented past the header, so
-                    // back up by the header size (accounting for ECC) in order
-                    // to extend the header
-                    auto addr = o_TOC[l_secId].flashAddr;
-                    size_t headerSize =
-                       (o_TOC[l_secId].integrity == FFS_INTEG_ECC_PROTECT) ?
-                       PAGESIZE_PLUS_ECC : PAGESIZE;
-                    addr -= headerSize;
-
-                    l_errhdl = PNOR::extendHash(addr, headerSize, l_secId);
-                    if (l_errhdl)
-                    {
-                        break;
-                    }
-                }
-            }
-#endif
         }
 
         for(int tmpId = 0;
@@ -346,50 +320,6 @@ errlHndl_t PNOR::parseTOC( uint8_t* i_tocBuffer,SectionData_t * o_TOC)
     return l_errhdl;
 }
 
-#ifndef __HOSTBOOT_RUNTIME
-errlHndl_t PNOR::extendHash(uint64_t i_addr, size_t i_size,
-                            const PNOR::SectionId i_sectionId)
-{
-    errlHndl_t l_errhdl = NULL;
-
-    do {
-
-    const char* l_name = PNOR::SectionIdToString(i_sectionId);
-
-    // Read data from the PNOR DD
-    uint8_t* l_buf = new uint8_t[i_size]();
-    TARGETING::Target* l_target = TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL;
-    l_errhdl = DeviceFW::deviceRead(l_target, l_buf, i_size,
-                                    DEVICE_PNOR_ADDRESS(0,i_addr));
-    if (l_errhdl)
-    {
-        break;
-    }
-
-    SHA512_t l_hash = {0};
-    SECUREBOOT::hashBlob(l_buf, i_size, l_hash);
-    l_errhdl = TRUSTEDBOOT::pcrExtend(TRUSTEDBOOT::PCR_0,
-        PNOR::PAYLOAD == i_sectionId?
-            TRUSTEDBOOT::EV_COMPACT_HASH:
-            (PNOR::isCoreRootOfTrustSection(i_sectionId)?
-                TRUSTEDBOOT::EV_S_CRTM_CONTENTS:
-                TRUSTEDBOOT::EV_POST_CODE),
-        l_hash,
-        sizeof(SHA512_t),
-        l_name);
-    delete[] l_buf;
-
-    if (l_errhdl)
-    {
-        break;
-    }
-
-    } while(0);
-
-    return l_errhdl;
-}
-#endif
-
 bool PNOR::isInhibitedSection(const uint32_t i_section)
 {
 #ifdef CONFIG_SECUREBOOT
@@ -444,95 +374,6 @@ bool PNOR::isInhibitedSection(const uint32_t i_section)
 #endif
 }
 
-
-errlHndl_t PNOR::setSecure(const uint32_t i_secId,
-                           PNOR::SectionData_t* io_TOC)
-{
-    errlHndl_t l_errhdl = nullptr;
-
-    assert(io_TOC != nullptr, "PNOR::setSecure received a NULL toc to modify");
-
-    do {
-
-    // Set secure field based on enforced policy
-    io_TOC[i_secId].secure = PNOR::isEnforcedSecureSection(i_secId);
-
-    // HBRT does not support best effort policy. Use enforced secure policy only.
-#ifndef __HOSTBOOT_RUNTIME
-    if(SECUREBOOT::bestEffortPolicy())
-    {
-        if (io_TOC[i_secId].secure)
-        {
-            // Apply best effort policy by checking if the section appears to have a
-            // secure header
-            size_t l_size = sizeof(ROM_MAGIC_NUMBER);
-            uint8_t l_buf[l_size] = {0};
-            auto l_target = TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL;
-            // Read first 4 bytes of section data from the PNOR DD
-            // Note: Do not need to worry about ECC as the 9th byte is the first
-            //       ECC byte.
-            l_errhdl = DeviceFW::deviceRead(l_target, l_buf, l_size,
-                              DEVICE_PNOR_ADDRESS(0,io_TOC[i_secId].flashAddr));
-            if (l_errhdl)
-            {
-                break;
-            }
-
-            // Check if first 4 bytes match the Secureboot Magic Number
-            io_TOC[i_secId].secure &= PNOR::cmpSecurebootMagicNumber(l_buf);
-        }
-    }
-#endif
-
-    } while (0);
-
-    return l_errhdl;
-}
-
-// @TODO RTC 173489
-// Remove API once FSP fully supports signing of PNOR sections that did not
-// previously have a sha512 header
-errlHndl_t PNOR::hasKnownHeader(
-    const PNOR::SectionId      i_secId,
-    const PNOR::SectionData_t& i_TOC,
-          bool&                o_knownHeader)
-{
-    errlHndl_t pError = nullptr;
-    bool knownHeader = true;
-
-    do {
-
-    // Left symbolic constant defined in the function so it's easier to strip
-    // out later and nothing becomes dependent on it
-    const char VERSION_MAGIC[] = "VERSION";
-    const auto versionMagicSize = sizeof(VERSION_MAGIC);
-    const auto secureMagicSize = sizeof(ROM_MAGIC_NUMBER);
-    auto size = std::max(versionMagicSize,secureMagicSize);
-    assert(size <= sizeof(uint64_t),"non-ECC request size exceeded. "
-        "Expected size of <= %d but got %d",sizeof(uint64_t),size);
-    uint8_t buf[size] = {0};
-
-    pError = readHeaderMagic(i_secId,i_TOC,size,buf);
-    if(pError)
-    {
-        break;
-    }
-
-    auto secureHeader = PNOR::cmpSecurebootMagicNumber(buf);
-    decltype(secureHeader) versionHeader =
-        (memcmp(buf,VERSION_MAGIC,versionMagicSize) == 0);
-    if(!secureHeader && !versionHeader)
-    {
-        knownHeader = false;
-    }
-
-    o_knownHeader = knownHeader;
-
-    } while (0);
-
-    return pError;
-}
-
 bool PNOR::isSectionEmpty(const PNOR::SectionId i_section)
 {
     errlHndl_t l_errhdl = nullptr;
diff --git a/src/usr/pnor/pnor_common.H b/src/usr/pnor/pnor_common.H
index 448d44cc5..8ada2c03b 100644
--- a/src/usr/pnor/pnor_common.H
+++ b/src/usr/pnor/pnor_common.H
@@ -73,24 +73,6 @@ namespace PNOR {
      void physicalToMmioOffset(uint64_t  i_hbbAddress,
                                uint64_t& o_mmioOffset);
 
-    // @TODO RTC 178520 Remove legacy extensions when all secure sections
-    // are supported
-    /**
-     * @brief Reads version header of section, hashes it, and extends to tpm
-     *        buffer list.
-     *
-     * @parm i_addr       Offset into flash to read
-     * @parm i_size       Number of bytes to read
-     * @parm i_sectionId  Section ID of PNOR section
-     *
-     * @return Error from operation
-     */
-#ifndef __HOSTBOOT_RUNTIME
-    errlHndl_t extendHash(uint64_t i_addr,
-                          size_t i_size,
-                          const PNOR::SectionId i_sectionId);
-#endif
-
 }
 
 #endif
diff --git a/src/usr/pnor/pnor_utils.C b/src/usr/pnor/pnor_utils.C
index bf8a195c2..9e0753066 100644
--- a/src/usr/pnor/pnor_utils.C
+++ b/src/usr/pnor/pnor_utils.C
@@ -321,42 +321,21 @@ PNOR::parseEntries (ffs_hdr* i_ffs_hdr,
                                 ((io_TOC[secId].size * 8 ) / 9);
         }
 
-        // @TODO RTC 173489
-        // Remove once FSP fully supports signing of PNOR sections that did
-        // not previously have a sha512 header.  Until then, turn off the SHA512
-        // bit if it doesn't match known header types
-#ifndef BOOTLOADER
-        if(io_TOC[secId].version & FFS_VERS_SHA512)
-        {
-            bool hasKnownHeader = true;
-            l_errhdl = PNOR::hasKnownHeader(static_cast<SectionId>(secId),
-                io_TOC[secId],hasKnownHeader);
-            if(l_errhdl)
-            {
-                break;
-            }
-
-            if(!hasKnownHeader)
-            {
-                io_TOC[secId].version &= ~FFS_VERS_SHA512;
-            }
-        }
-#endif
-
-#ifdef BOOTLOADER
+        // isEnforcedSecureSection should always handle SB compiled in or not,
+        // but if that ever changes, force flag to false in PNOR TOC.
+#ifdef CONFIG_SECUREBOOT
         io_TOC[secId].secure = PNOR::isEnforcedSecureSection(secId);
 #else
-        // Check if PNOR section has a secureHeader or not.
-        l_errhdl = PNOR::setSecure(secId, io_TOC);
-        if (l_errhdl)
-        {
-            break;
-        }
+        io_TOC[secId].secure = false;
 #endif
 
-        if (PNOR::hasNonSecureHeader(io_TOC[secId]))
+        // If secureboot is compiled in, skip header if not a secure section
+        // Otherwise always skip header as the secure flag is always false and
+        // SpnorRp will not handle skipping the header if one is indicated in PNOR
+        if ( (io_TOC[secId].version & FFS_VERS_SHA512)
+              && !io_TOC[secId].secure)
         {
-              //increment flash addr for sha header
+            //increment flash addr for sha header
             if (io_TOC[secId].integrity == FFS_INTEG_ECC_PROTECT)
             {
                 io_TOC[secId].flashAddr += PAGESIZE_PLUS_ECC ;
@@ -370,6 +349,7 @@ PNOR::parseEntries (ffs_hdr* i_ffs_hdr,
             // adjust the size to reflect that
             io_TOC[secId].size -= PAGESIZE;
         }
+
     } // For TOC Entries
 
 #ifndef BOOTLOADER
@@ -510,9 +490,3 @@ bool PNOR::cmpSecurebootMagicNumber(const uint8_t* i_vaddr)
 
     return memcmp(&ROM_MAGIC_NUMBER, i_vaddr, sizeof(ROM_MAGIC_NUMBER))==0;
 }
-
-bool PNOR::hasNonSecureHeader(const PNOR::SectionData_t& i_secInfo)
-{
-    return (i_secInfo.version & FFS_VERS_SHA512) &&
-           !i_secInfo.secure;
-}
diff --git a/src/usr/pnor/pnor_utils.H b/src/usr/pnor/pnor_utils.H
index a608234fb..338f2595b 100644
--- a/src/usr/pnor/pnor_utils.H
+++ b/src/usr/pnor/pnor_utils.H
@@ -237,16 +237,6 @@ bool isEnforcedSecureSection(const uint32_t i_section);
  */
 bool isCoreRootOfTrustSection(const PNOR::SectionId i_section);
 
-/**
- * @brief Determines if a section has a Header and that it is not a Secure Header
- *
- * @param[out] i_secInfo  PNOR information of section
- *
- * @return True if it appears to not have a secure header
- *         False otherwise
- */
-bool hasNonSecureHeader(const PNOR::SectionData_t& i_secInfo);
-
 }  // End namespace PNOR
 
 
diff --git a/src/usr/pnor/pnorrp.C b/src/usr/pnor/pnorrp.C
index da2548ebc..a874d3735 100644
--- a/src/usr/pnor/pnorrp.C
+++ b/src/usr/pnor/pnorrp.C
@@ -160,36 +160,6 @@ errlHndl_t PNOR::fixECC(PNOR::SectionId i_section)
     return Singleton<PnorRP>::instance().fixECC(i_section);
 }
 
-// @TODO RTC 173489
-// Remove API once FSP fully supports signing of PNOR sections that did not
-// previously have a sha512 header
-errlHndl_t PNOR::readHeaderMagic(
-    const PNOR::SectionId      i_secId,
-    const PNOR::SectionData_t& i_TOC,
-    const size_t               i_size,
-          void* const          o_pData)
-{
-    errlHndl_t pError = nullptr;
-    assert(o_pData != nullptr,"Output buffer pointer was nullptr");
-
-    do {
-
-    size_t size = i_size;
-    auto pTarget = TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL;
-    // Read first <=8 bytes of section data from the PNOR DD
-    // Note: Do not need to worry about ECC as the 9th byte is the first
-    //       ECC byte.
-    pError = DeviceFW::deviceRead(pTarget, o_pData, size,
-                         DEVICE_PNOR_ADDRESS(0,i_TOC.flashAddr));
-    if (pError)
-    {
-        break;
-    }
-
-    } while(0);
-
-    return pError;
-}
 
 /**
  * STATIC
@@ -369,55 +339,6 @@ void PnorRP::initDaemon()
             break;
         }
 
-        // @TODO RTC 178520 Remove the non-secure extension path and
-        // always used the converged HBB extension path.
-
-        // If secured, extend base image (HBB) when Hostboot first starts.
-        // Since HBB is never re-loaded, inhibit extending this image in
-        // runtime code.
-        #ifndef __HOSTBOOT_RUNTIME
-        #ifdef CONFIG_SECUREBOOT
-        if(!SECUREBOOT::enabled())
-        {
-            // If compliant bootloader was present, it saved the HBB header
-            // to a known location accessible to HBB.  Until that bootloader
-            // is widely distributed, when in non-secure mode in lab,
-            // manufacturing, etc., read the header directly from PNOR.
-            PNOR::SideInfo_t pnorInfo = {PNOR::WORKING};
-            l_errhdl = PnorRP::getSideInfo(PNOR::WORKING, pnorInfo);
-            if(l_errhdl != nullptr)
-            {
-                break;
-            }
-
-            const SectionData_t* const pHbb = &iv_TOC[PNOR::HB_BASE_CODE];
-            const bool ecc = (pHbb->integrity == FFS_INTEG_ECC_PROTECT) ?
-                true :false;
-
-            uint8_t pHeader[PAGESIZE] = {0};
-            uint64_t fatalError = 0;
-            l_errhdl = readFromDevice(
-                    pnorInfo.hbbAddress,
-                    pHbb->chip,
-                    ecc,
-                    pHeader,
-                    fatalError);
-
-            // If fatalError != 0 there is an uncorrectable ECC error (UE).
-            // In that case, continue on with inaccurate data, as
-            // readFromDevice API will initiate a shutdown
-            if(l_errhdl != nullptr)
-            {
-                break;
-            }
-
-            // Cache the header
-            (void)SECUREBOOT::baseHeader().setNonSecurely(
-                pHeader);
-        }
-        #endif
-        #endif
-
         // start task to wait on the queue
         task_create( wait_for_message, NULL );
     } while(0);
diff --git a/src/usr/pnor/runtime/rt_pnor.C b/src/usr/pnor/runtime/rt_pnor.C
index 16707f774..276691003 100644
--- a/src/usr/pnor/runtime/rt_pnor.C
+++ b/src/usr/pnor/runtime/rt_pnor.C
@@ -86,24 +86,6 @@ errlHndl_t PNOR::clearSection(PNOR::SectionId i_section)
     return Singleton<RtPnor>::instance().clearSection(i_section);
 }
 
-// @TODO RTC 173489
-// Remove API once FSP fully supports signing of PNOR sections that did not
-// previously have a sha512 header
-errlHndl_t PNOR::readHeaderMagic(
-    const PNOR::SectionId      i_secId,
-    const PNOR::SectionData_t& i_TOC,
-    const size_t               i_size,
-          void* const          o_pData)
-{
-    errlHndl_t pError = RtPnor::readFromDevice (RtPnor::iv_masterProcId,
-                                                i_secId,
-                                                0,
-                                                i_size,
-                                                false,
-                                                o_pData);
-    return pError;
-}
-
 void PNOR::getPnorInfo( PnorInfo_t& o_pnorInfo )
 {
     o_pnorInfo.mmioOffset = LPC_SFC_MMIO_OFFSET | LPC_FW_SPACE;
@@ -409,7 +391,7 @@ errlHndl_t RtPnor::readFromDevice (uint64_t i_procId,
                                    uint64_t i_offset,
                                    size_t i_size,
                                    bool i_ecc,
-                                   void* o_data)
+                                   void* o_data) const
 {
     TRACFCOMP(g_trac_pnor, ENTER_MRK"RtPnor::readFromDevice: i_offset=0x%X, "
            "i_procId=%d sec=%d size=0x%X ecc=%d", i_offset, i_procId, i_section,
diff --git a/src/usr/pnor/runtime/rt_pnor.H b/src/usr/pnor/runtime/rt_pnor.H
index b2433a19f..ab612bebd 100644
--- a/src/usr/pnor/runtime/rt_pnor.H
+++ b/src/usr/pnor/runtime/rt_pnor.H
@@ -131,25 +131,12 @@ class RtPnor
          *
          * @return Error from device
          */
-        // @TODO RTC 173489
-        // Remove static once FSP fully supports signing of PNOR sections that
-        // did not previously have a sha512 header.  Also add the const method
-        // qualifier back in
-        static errlHndl_t readFromDevice (uint64_t i_procId,
+         errlHndl_t readFromDevice (uint64_t i_procId,
                                    PNOR::SectionId i_section,
                                    uint64_t i_offset,
                                    size_t i_size,
                                    bool i_ecc,
-                                   void* o_data);
-
-        // @TODO RTC 173489
-        // Remove API once FSP fully supports signing of PNOR sections that did
-        // not previously have a sha512 header
-        friend errlHndl_t PNOR::readHeaderMagic(
-                  PNOR::SectionId      i_secId,
-            const PNOR::SectionData_t& i_TOC,
-                  size_t               i_size,
-                  void* const          o_pData);
+                                   void* o_data) const;
 
         /**
          * @brief  Write data back to the PNOR device
diff --git a/src/usr/pnor/spnorrp.C b/src/usr/pnor/spnorrp.C
index 90c53d712..29dd6de64 100644
--- a/src/usr/pnor/spnorrp.C
+++ b/src/usr/pnor/spnorrp.C
@@ -328,38 +328,30 @@ uint64_t SPnorRP::verifySections(SectionId i_id,
 
         if (!l_info.secure)
         {
-            if(SECUREBOOT::bestEffortPolicy())
-            {
-                TRACFCOMP(g_trac_pnor,"PNOR::verifySections> called on unsecured section - Best effort policy skipping");
-                break;
-            }
-            else
-            {
-                TRACFCOMP(g_trac_pnor,ERR_MRK"PNOR::verifySections> called on "
+            TRACFCOMP(g_trac_pnor,ERR_MRK"PNOR::verifySections> called on "
                     "unsecured section");
 
-                /*@
-                 * @errortype
-                 * @severity     ERRL_SEV_CRITICAL_SYS_TERM
-                 * @moduleid     PNOR::MOD_SPNORRP_VERIFYSECTIONS
-                 * @reasoncode   PNOR::RC_UNSIGNED_PNOR_SECTION
-                 * @userdata1    PNOR section requested to verify
-                 * @userdata2    0
-                 * @devdesc      Cannot verify unsigned PNOR section
-                 * @custdesc     Security failure: unable to securely load
-                 *               requested firmware.
-                 */
-                l_errhdl = new ERRORLOG::ErrlEntry(
-                                       ERRORLOG::ERRL_SEV_CRITICAL_SYS_TERM,
-                                       PNOR::MOD_SPNORRP_VERIFYSECTIONS,
-                                       PNOR::RC_UNSIGNED_PNOR_SECTION,
-                                       TO_UINT64(i_id),
-                                       0,
-                                       true /*Add HB SW Callout*/);
-                l_errhdl->collectTrace(PNOR_COMP_NAME);
-                l_errhdl->collectTrace(SECURE_COMP_NAME);
-                break;
-            }
+            /*@
+             * @errortype
+             * @severity     ERRL_SEV_CRITICAL_SYS_TERM
+             * @moduleid     PNOR::MOD_SPNORRP_VERIFYSECTIONS
+             * @reasoncode   PNOR::RC_UNSIGNED_PNOR_SECTION
+             * @userdata1    PNOR section requested to verify
+             * @userdata2    0
+             * @devdesc      Cannot verify unsigned PNOR section
+             * @custdesc     Security failure: unable to securely load
+             *               requested firmware.
+             */
+            l_errhdl = new ERRORLOG::ErrlEntry(
+                                   ERRORLOG::ERRL_SEV_CRITICAL_SYS_TERM,
+                                   PNOR::MOD_SPNORRP_VERIFYSECTIONS,
+                                   PNOR::RC_UNSIGNED_PNOR_SECTION,
+                                   TO_UINT64(i_id),
+                                   0,
+                                   true /*Add HB SW Callout*/);
+            l_errhdl->collectTrace(PNOR_COMP_NAME);
+            l_errhdl->collectTrace(SECURE_COMP_NAME);
+            break;
         }
         else
         {
@@ -881,16 +873,8 @@ void SPnorRP::waitForMessage()
 
                             size_t l_sizeWithHdr = PAGESIZE + l_rec->textSize;
 
-                            bool l_wasLoadedAsBestEffort = false;
-                            if (l_rec->textSize == 0 &&
-                                SECUREBOOT::bestEffortPolicy())
-                            {
-                                // indicate that this section had been loaded
-                                // as "best effort"
-                                l_wasLoadedAsBestEffort = true;
-                            }
                             // if the section has an unsecured portion
-                            else if (l_sizeWithHdr != l_rec->infoSize)
+                            if (l_sizeWithHdr != l_rec->infoSize)
                             {
                                 TRACFCOMP( g_trac_pnor, ERR_MRK"SPnorRP::waitForMessage> Attempting to unload an unsupported section: 0x%X textsize+hdr: 0x%llX infosize: 0x%llX (the two sizes must be equal)", l_id, l_sizeWithHdr, l_rec->infoSize);
                                 /*@
@@ -922,15 +906,6 @@ void SPnorRP::waitForMessage()
                                 break;
                             }
 
-                            if (l_wasLoadedAsBestEffort)
-                            {
-                                l_rec->secAddr = nullptr;
-                                l_rec->textSize = 0;
-                                l_rec->infoSize = 0;
-                                l_rec->refCount = 0;
-                                break;
-                            }
-
                             l_errhdl = removePages(l_rec->secAddr,
                                                    l_sizeWithHdr);
                             if (l_errhdl)
diff --git a/src/usr/secureboot/HBconfig b/src/usr/secureboot/HBconfig
index 7a6c29cf3..af987887c 100644
--- a/src/usr/secureboot/HBconfig
+++ b/src/usr/secureboot/HBconfig
@@ -4,14 +4,6 @@ config SECUREBOOT
     help
         Enable and enforce secure boot
 
-# @TODO RTC:178520 Remove this in the future
-config SECUREBOOT_BEST_EFFORT
-    default n
-    depends on SECUREBOOT
-    help
-        Enable Best effort Secureboot. Should only be used for
-        phasing in new secureboot functionality.
-
 config DRTM #TODO RTC: 170487 Disable for relevant platforms
     default n
     depends on (SECUREBOOT && TPMDD)
diff --git a/src/usr/secureboot/base/header.C b/src/usr/secureboot/base/header.C
index f2c41069c..77d756934 100644
--- a/src/usr/secureboot/base/header.C
+++ b/src/usr/secureboot/base/header.C
@@ -36,28 +36,15 @@ namespace SECUREBOOT
         return Singleton<Header>::instance();
     }
 
-    // @TODO RTC 178520 Converge on a single method of reading the secure
-    // header
-    void Header::loadSecurely()
+    void Header::loadHeader()
     {
-        const void* const pSecureHeader = g_BlToHbDataManager.getHbbHeader();
+        const void* const pHeader = g_BlToHbDataManager.getHbbHeader();
 
         // Fatal code bug if called with nullptr pointer
-        assert(pSecureHeader != nullptr,
-            "BUG! In Header::loadSecurely(), expected valid address for base "
-            "image header in secure mode, but got nullptr.");
-        _set(pSecureHeader);
-    }
-
-    // @TODO RTC 178520 Converge on a single method of reading the secure
-    // header
-    void Header::setNonSecurely(
-        const void* const i_pHeader)
-    {
-        // Fatal code bug if called with nullptr pointer
-        assert(i_pHeader != nullptr,"BUG! In Header::setNonSecurely(), "
-            "caller passed a nullptr header address.");
-        _set(i_pHeader);
+        assert(pHeader != nullptr,
+            "BUG! In Header::loadHeader(), expected valid address for base "
+            "image header, but got nullptr.");
+        _set(pHeader);
     }
 
     void Header::_set(
diff --git a/src/usr/secureboot/base/securerommgr.C b/src/usr/secureboot/base/securerommgr.C
index b894be4a9..d5dac06d8 100644
--- a/src/usr/secureboot/base/securerommgr.C
+++ b/src/usr/secureboot/base/securerommgr.C
@@ -66,11 +66,6 @@ errlHndl_t initializeSecureRomManager(void)
     return Singleton<SecureRomManager>::instance().initialize();
 }
 
-bool secureRomValidPolicy()
-{
-    return Singleton<SecureRomManager>::instance().secureRomValidPolicy();
-}
-
 /**
  * @brief Verify Signed Container
  */
@@ -78,11 +73,8 @@ errlHndl_t verifyContainer(void * i_container, const SHA512_t* i_hwKeyHash)
 {
     errlHndl_t l_errl = nullptr;
 
-    if(Singleton<SecureRomManager>::instance().secureRomValidPolicy())
-    {
-        l_errl = Singleton<SecureRomManager>::instance().
+    l_errl = Singleton<SecureRomManager>::instance().
                                        verifyContainer(i_container,i_hwKeyHash);
-    }
 
     return l_errl;
 }
@@ -148,15 +140,8 @@ errlHndl_t verifyComponentId(
  */
 void hashBlob(const void * i_blob, size_t i_size, SHA512_t o_buf)
 {
-    if(Singleton<SecureRomManager>::instance().secureRomValidPolicy())
-    {
-        return Singleton<SecureRomManager>::instance().
+    return Singleton<SecureRomManager>::instance().
                                                hashBlob(i_blob, i_size, o_buf);
-    }
-    else
-    {
-        memset(o_buf, 0, sizeof(SHA512_t));
-    }
 }
 
 /**
@@ -165,11 +150,8 @@ void hashBlob(const void * i_blob, size_t i_size, SHA512_t o_buf)
  */
 void hashConcatBlobs(const blobPair_t &i_blobs, SHA512_t o_buf)
 {
-    if(Singleton<SecureRomManager>::instance().secureRomValidPolicy())
-    {
         return Singleton<SecureRomManager>::instance().
                                                 hashConcatBlobs(i_blobs, o_buf);
-    }
 }
 
 /*
@@ -177,11 +159,7 @@ void hashConcatBlobs(const blobPair_t &i_blobs, SHA512_t o_buf)
  */
 void getHwKeyHash(SHA512_t o_hash)
 {
-    // @TODO RTC:170136 remove isValid check
-    if(Singleton<SecureRomManager>::instance().secureRomValidPolicy())
-    {
-        return Singleton<SecureRomManager>::instance().getHwKeyHash(o_hash);
-    }
+    return Singleton<SecureRomManager>::instance().getHwKeyHash(o_hash);
 }
 
 sbFuncVer_t getSecRomFuncVersion(const sbFuncType_t i_funcType)
@@ -219,43 +197,27 @@ errlHndl_t SecureRomManager::initialize()
     do{
         // Check if bootloader to hostboot data is valid.
         iv_secureromValid = g_BlToHbDataManager.isValid();
-
+        // Enforce that the securerom is valid
         if (!iv_secureromValid)
         {
-            // Allow skipping functionality if secure rom is invalid if best
-            // effort policy enabled
-            if(SECUREBOOT::bestEffortPolicy())
-            {
-                TRACFCOMP(g_trac_secure,INFO_MRK"SecureRomManager::initialize(): SecureROM invalid, skipping functionality");
-#ifdef CONFIG_CONSOLE
-                CONSOLE::displayf(SECURE_COMP_NAME, "SecureROM invalid - skipping functionality");
-#endif
-                printk("SecureRomManager SecureROM invalid -- skipping functionality\n");
-                // Can skip the rest of this function
-                break;
-            }
-            // Otherwise enforce securerom to be valid.
-            else
-            {
-                TRACFCOMP(g_trac_secure,ERR_MRK"SecureRomManager::initialize(): SecureROM invalid");
+            TRACFCOMP(g_trac_secure,ERR_MRK"SecureRomManager::initialize(): SecureROM invalid");
 #ifdef CONFIG_CONSOLE
-                CONSOLE::displayf(SECURE_COMP_NAME, ERR_MRK"SecureROM invalid");
+            CONSOLE::displayf(SECURE_COMP_NAME, ERR_MRK"SecureROM invalid");
 #endif
-                printk("ERR> SecureRomManager SecureROM invalid\n");
-                /*@
-                 * @errortype
-                 * @moduleid     SECUREBOOT::MOD_SECURE_ROM_INIT
-                 * @reasoncode   SECUREBOOT::RC_SECROM_INVALID
-                 * @devdesc      Valid securerom not present
-                 * @custdesc     Security failure occurred during the IPL of
-                 *               the system.
-                 */
-                l_errl = new ERRORLOG::ErrlEntry( ERRORLOG::ERRL_SEV_UNRECOVERABLE,
-                                                  SECUREBOOT::MOD_SECURE_ROM_INIT,
-                                                  SECUREBOOT::RC_SECROM_INVALID);
-                l_errl->collectTrace(SECURE_COMP_NAME,ERROR_TRACE_SIZE);
-                break;
-            }
+            printk("ERR> SecureRomManager SecureROM invalid\n");
+            /*@
+             * @errortype
+             * @moduleid     SECUREBOOT::MOD_SECURE_ROM_INIT
+             * @reasoncode   SECUREBOOT::RC_SECROM_INVALID
+             * @devdesc      Valid securerom not present
+             * @custdesc     Security failure occurred during the IPL of
+             *               the system.
+             */
+            l_errl = new ERRORLOG::ErrlEntry( ERRORLOG::ERRL_SEV_UNRECOVERABLE,
+                                              SECUREBOOT::MOD_SECURE_ROM_INIT,
+                                              SECUREBOOT::RC_SECROM_INVALID);
+            l_errl->collectTrace(SECURE_COMP_NAME,ERROR_TRACE_SIZE);
+            break;
         }
 
         TRACFCOMP(g_trac_secure,"SecureRomManager::initialize(): SecureROM valid, enabling functionality");
@@ -366,12 +328,6 @@ errlHndl_t SecureRomManager::verifyContainer(void * i_container,
 
     do{
 
-        // Check if secureboot data is valid.
-        if (!secureRomValidPolicy())
-        {
-            // Can skip the rest of this function
-            break;
-        }
         // Check to see if ROM has already been initialized
         // This should have been done early in IPL so assert if this
         // is not the case as system is in a bad state
@@ -477,29 +433,25 @@ void SecureRomManager::hashBlob(const void * i_blob, size_t i_size, SHA512_t o_b
 
     TRACDCOMP(g_trac_secure,INFO_MRK"SecureRomManager::hashBlob()");
 
-    // Check if secureboot data is valid.
-    if (secureRomValidPolicy())
-    {
-        // Check to see if ROM has already been initialized
-        // This should have been done early in IPL so assert if this
-        // is not the case as system is in a bad state
-        assert(iv_securerom != nullptr);
+    // Check to see if ROM has already been initialized
+    // This should have been done early in IPL so assert if this
+    // is not the case as system is in a bad state
+    assert(iv_securerom != nullptr);
 
-        // Set startAddr to ROM_SHA512() function at an offset of Secure ROM
-        uint64_t l_rom_SHA512_startAddr =
-                                    reinterpret_cast<uint64_t>(iv_securerom) +
-                                    getSecRomFuncOffset(SB_FUNC_TYPES::SHA512);
+    // Set startAddr to ROM_SHA512() function at an offset of Secure ROM
+    uint64_t l_rom_SHA512_startAddr =
+                                reinterpret_cast<uint64_t>(iv_securerom) +
+                                getSecRomFuncOffset(SB_FUNC_TYPES::SHA512);
 
-        call_rom_SHA512(reinterpret_cast<void*>(l_rom_SHA512_startAddr),
-                        reinterpret_cast<const sha2_byte*>(i_blob),
-                        i_size,
-                        reinterpret_cast<SHA512_t*>(o_buf));
+    call_rom_SHA512(reinterpret_cast<void*>(l_rom_SHA512_startAddr),
+                    reinterpret_cast<const sha2_byte*>(i_blob),
+                    i_size,
+                    reinterpret_cast<SHA512_t*>(o_buf));
 
-        TRACUCOMP(g_trac_secure,"SecureRomManager::hashBlob(): "
-                  "call_rom_SHA512: blob=%p size=0x%X addr=%p (iv_d_p=%p)",
-                   i_blob, i_size, l_rom_SHA512_startAddr,
-                   iv_securerom);
-    }
+    TRACUCOMP(g_trac_secure,"SecureRomManager::hashBlob(): "
+              "call_rom_SHA512: blob=%p size=0x%X addr=%p (iv_d_p=%p)",
+               i_blob, i_size, l_rom_SHA512_startAddr,
+               iv_securerom);
 
     TRACDCOMP(g_trac_secure,EXIT_MRK"SecureRomManager::hashBlob()");
 }
@@ -510,42 +462,18 @@ void SecureRomManager::hashBlob(const void * i_blob, size_t i_size, SHA512_t o_b
 void SecureRomManager::hashConcatBlobs(const blobPair_t &i_blobs,
                                       SHA512_t o_buf) const
 {
-    // Check if secureboot data is valid.
-    if (secureRomValidPolicy())
+    std::vector<uint8_t> concatBuf;
+    for (const auto &it : i_blobs)
     {
-        std::vector<uint8_t> concatBuf;
-        for (const auto &it : i_blobs)
-        {
-            assert(it.first != nullptr, "BUG! In SecureRomManager::hashConcatBlobs(), "
-                "User passed in nullptr blob pointer");
-            const uint8_t* const blob =  static_cast<const uint8_t*>(it.first);
-            const auto blobSize = it.second;
-            concatBuf.insert(concatBuf.end(), blob, blob + blobSize);
-        }
-
-        // Call hash blob on new concatenated buffer
-        hashBlob(concatBuf.data(),concatBuf.size(),o_buf);
+        assert(it.first != nullptr, "BUG! In SecureRomManager::hashConcatBlobs(), "
+            "User passed in nullptr blob pointer");
+        const uint8_t* const blob =  static_cast<const uint8_t*>(it.first);
+        const auto blobSize = it.second;
+        concatBuf.insert(concatBuf.end(), blob, blob + blobSize);
     }
-}
 
-bool SecureRomManager::secureRomValidPolicy() const
-{
-    bool l_policy = true;
-    if (bestEffortPolicy())
-    {
-        // Set policy based on secure ROM status
-        l_policy = iv_secureromValid;
-    }
-    else
-    {
-        // Assert secure rom is valid in this mode.
-        // The initialize function should have created an error log already if
-        // this case is false, so this code path should not be hit.
-        assert(iv_secureromValid==true, "SecureRomManager cannot operate with invalid secure rom");
-        l_policy = true;
-    }
-
-    return l_policy;
+    // Call hash blob on new concatenated buffer
+    hashBlob(concatBuf.data(),concatBuf.size(),o_buf);
 }
 
 /********************
@@ -557,12 +485,8 @@ bool SecureRomManager::secureRomValidPolicy() const
  */
 void SecureRomManager::getHwKeyHash()
 {
-    // Check if secureboot data is valid.
-    if (secureRomValidPolicy())
-    {
-        iv_key_hash  = reinterpret_cast<const SHA512_t*>(
+    iv_key_hash  = reinterpret_cast<const SHA512_t*>(
                                            g_BlToHbDataManager.getHwKeysHash());
-    }
 }
 
 /**
@@ -570,11 +494,7 @@ void SecureRomManager::getHwKeyHash()
  */
 void SecureRomManager::getHwKeyHash(SHA512_t o_hash)
 {
-    // Check if secureboot data is valid.
-    if (secureRomValidPolicy())
-    {
-        memcpy(o_hash, iv_key_hash, sizeof(SHA512_t));
-    }
+    memcpy(o_hash, iv_key_hash, sizeof(SHA512_t));
 }
 
 const SecureRomManager::SecRomFuncTypeOffsetMap_t
diff --git a/src/usr/secureboot/base/securerommgr.H b/src/usr/secureboot/base/securerommgr.H
index b221d2c10..4358c4981 100644
--- a/src/usr/secureboot/base/securerommgr.H
+++ b/src/usr/secureboot/base/securerommgr.H
@@ -97,15 +97,6 @@ class SecureRomManager
         void hashConcatBlobs (const blobPair_t &i_blobs, SHA512_t o_buf) const;
 
         /*
-         * @brief Determines if best effort policy is enabled and allowed when
-         *        SecureROM is invalid.
-         *        Asserts secure rom is valid if bestEffortPolicy is false
-         *
-         * @return bool - True if enabled, false otherwise
-         */
-        bool secureRomValidPolicy() const;
-
-        /*
          * @brief Get offset of function from the start of the SecureROM
          *
          * @param[in]  i_funcType   Secure Boot function type to get version of
diff --git a/src/usr/secureboot/base/service.C b/src/usr/secureboot/base/service.C
index ce4f5ecbf..21900a5fa 100644
--- a/src/usr/secureboot/base/service.C
+++ b/src/usr/secureboot/base/service.C
@@ -315,11 +315,8 @@ void* initializeBase(void* unused)
         }
 #endif
 
-        // Load original secureboot header.
-        if (enabled())
-        {
-            Singleton<Header>::instance().loadSecurely();
-        }
+        // Load original header.
+        Singleton<Header>::instance().loadHeader();
     } while(0);
 
     return l_errl;
@@ -332,11 +329,6 @@ bool enabled()
 }
 #endif
 
-bool bestEffortPolicy()
-{
-    return Singleton<Settings>::instance().getBestEffortPolicy();
-}
-
 errlHndl_t getSecuritySwitch(uint64_t& o_regValue, TARGETING::Target* i_pProc)
 {
     return Singleton<Settings>::instance().getSecuritySwitch(o_regValue,
diff --git a/src/usr/secureboot/base/settings.C b/src/usr/secureboot/base/settings.C
index 386a330ba..eb25aea11 100644
--- a/src/usr/secureboot/base/settings.C
+++ b/src/usr/secureboot/base/settings.C
@@ -101,21 +101,6 @@ namespace SECUREBOOT
                 securitySwitchValue,cbsValue);
         }
         #endif
-
-#ifdef CONFIG_SECUREBOOT_BEST_EFFORT
-        iv_bestEffortPolicy = true;
-#else
-        if (iv_enabled)
-        {
-            iv_bestEffortPolicy = false;
-        }
-        else
-        {
-            iv_bestEffortPolicy = true;
-        }
-#endif
-        SB_INF("getBestEffortPolicy() state:%i",iv_bestEffortPolicy);
-        printk("SECUREBOOT::bestEffortPolicy() state:%i\n", iv_bestEffortPolicy);
     }
 
     bool Settings::getEnabled() const
@@ -123,11 +108,6 @@ namespace SECUREBOOT
         return iv_enabled;
     }
 
-    bool Settings::getBestEffortPolicy() const
-    {
-        return iv_bestEffortPolicy;
-    }
-
     errlHndl_t Settings::getJumperState(SecureJumperState& o_state,
                                         Target* i_pProc) const
     {
diff --git a/src/usr/secureboot/trusted/base/trustedboot_base.C b/src/usr/secureboot/trusted/base/trustedboot_base.C
index 2ec67ef81..801869439 100644
--- a/src/usr/secureboot/trusted/base/trustedboot_base.C
+++ b/src/usr/secureboot/trusted/base/trustedboot_base.C
@@ -436,18 +436,6 @@ errlHndl_t extendBaseImage()
     TRACDBIN(g_trac_trustedboot,"Base Header",pHbbHeader,
         TRUSTEDBOOT::DEFAULT_BIN_TRACE_SIZE);
 
-    // TODO: RTC 168021
-    // Need to remove this when HBB has a secure header across all platforms
-    // -or- a more general compatibility mechanism has been created allowing
-    // some platforms to stage in support
-    if(!PNOR::cmpSecurebootMagicNumber(
-           reinterpret_cast<const uint8_t*>(pHbbHeader)))
-    {
-        TRACDCOMP(g_trac_trustedboot, INFO_MRK " HBB header is not a secure "
-            "header; inhibiting extending base image measurement");
-        break;
-    }
-
     // Build a container header object from the raw header
     SECUREBOOT::ContainerHeader hbbContainerHeader;
     pError = hbbContainerHeader.setHeader(pHbbHeader);
@@ -457,33 +445,27 @@ errlHndl_t extendBaseImage()
         break;
     }
 
-    const void* pHbbVa = nullptr;
-    if(!SECUREBOOT::enabled())
+    // TPM extension of PNOR sections operates differently when SecureMode is
+    // enabled/disabled.  Provide all possible info and let TPM code handle
+    // the logic
+    PNOR::SectionInfo_t l_info;
+    pError = getSectionInfo(PNOR::HB_BASE_CODE, l_info);
+    if(pError)
     {
-        PNOR::SectionInfo_t l_info;
-
-        // @TODO RTC 168021 Remove this path since header will always be
-        // cached
-        pError = getSectionInfo(PNOR::HB_BASE_CODE, l_info);
-        if(pError)
-        {
-            TRACFCOMP(g_trac_trustedboot, ERR_MRK "Failed in call to "
-                "getSectionInfo for HBB section");
-            break;
-        }
+        TRACFCOMP(g_trac_trustedboot, ERR_MRK "Failed in call to "
+            "getSectionInfo for HBB section");
+        break;
+    }
 
-        if(l_info.vaddr == 0)
-        {
-            assert(false,"BUG! In extendBaseImage(), HBB virtual address "
-                "was 0");
-        }
+    if(l_info.vaddr == 0)
+    {
+        assert(false,"BUG! In extendBaseImage(), HBB virtual address was 0");
+    }
 
-        pHbbVa = reinterpret_cast<const void*>(
-            l_info.vaddr);
+    const void* pHbbVa = reinterpret_cast<const void*>(l_info.vaddr);
 
-        TRACDBIN(g_trac_trustedboot,"PNOR Base Code",pHbbVa,
-            TRUSTEDBOOT::DEFAULT_BIN_TRACE_SIZE);
-    }
+    TRACDBIN(g_trac_trustedboot,"PNOR Base Code",pHbbVa,
+             TRUSTEDBOOT::DEFAULT_BIN_TRACE_SIZE);
 
     // Extend the HBB measurement to the TPM
     pError = extendPnorSectionHash(