Secureboot best effort compatibility support

Change-Id: I1671459ca58684b14f65f322ff6dccddcaad40a2
RTC: 170685
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/38104
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>
Reviewed-by: Michael Baiocchi <mbaiocch@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>

16 files changed, 277 insertions, 204 deletions

diff --git a/src/bootloader/bootloader.C b/src/bootloader/bootloader.C
index 305033f44..4c0e70034 100644
--- a/src/bootloader/bootloader.C
+++ b/src/bootloader/bootloader.C
@@ -137,19 +137,6 @@ namespace Bootloader{
                sizeof(BlToHbData));
     }
 
-    /**
-     * @brief  Memcmp a vaddr to the known secureboot magic number
-     *
-     * @param[in]   i_vaddr: vaddr of secureboot header to check for magic number
-     *                       Note: must point to a buffer of size >= 4 bytes
-     *
-     * @return  bool - True if the magic number and starting bytes of the vaddr
-     *                 match. False otherwise.
-     */
-    bool cmpSecurebootMagicNumber(const uint8_t* i_vaddr)
-    {
-        return memcmp(&ROM_MAGIC_NUMBER, i_vaddr, sizeof(ROM_MAGIC_NUMBER))==0;
-    }
 
     /**
      * @brief Verify Container against system hash keys
@@ -179,8 +166,8 @@ namespace Bootloader{
             BOOTLOADER_TRACE(BTLDR_TRC_MAIN_VERIFY_NO_EYECATCH);
         }
         // # @TODO RTC:170136 terminate in this case
-        else if ( !cmpSecurebootMagicNumber(reinterpret_cast<const uint8_t*>
-                                            (i_pContainer)))
+        else if ( !PNOR::cmpSecurebootMagicNumber(
+                    reinterpret_cast<const uint8_t*>(i_pContainer)))
         {
             BOOTLOADER_TRACE(BTLDR_TRC_MAIN_VERIFY_NO_MAGIC_NUM);
         }
@@ -328,7 +315,7 @@ namespace Bootloader{
                 verifyContainer(l_src_addr);
 
                 // Increment past secure header
-                if (isSecureSection(PNOR::HB_BASE_CODE))
+                if (isEnforcedSecureSection(PNOR::HB_BASE_CODE))
                 {
                     l_src_addr += PAGE_SIZE/sizeof(uint64_t);
                     l_hbbLength -= PAGE_SIZE;
diff --git a/src/build/buildpnor/genPnorImages.pl b/src/build/buildpnor/genPnorImages.pl
index df2988b49..10aaf0917 100755
--- a/src/build/buildpnor/genPnorImages.pl
+++ b/src/build/buildpnor/genPnorImages.pl
@@ -500,8 +500,8 @@ sub manipulateImages
         # Sections that have secureboot support. Secureboot still must be
         # enabled for secureboot actions on these partitions to occur.
         # @TODO securebootp9 re-enable with SBE/SBEC/PAYLOAD secureboot ports
-        my $isNormalSecure =  ($eyeCatch eq "SBKT")
-                             || ($eyeCatch eq "HBRT");
+        my $isNormalSecure =  0; #($eyeCatch eq "SBKT")
+                             #|| ($eyeCatch eq "HBRT");
                              #|| ($eyeCatch eq "SBE")
                              #|| ($eyeCatch eq "SBEC")
                              #|| ($eyeCatch eq "PAYLOAD")
@@ -509,9 +509,9 @@ sub manipulateImages
                              #|| ($eyeCatch eq "CAPP")
                              #|| ($eyeCatch eq "BOOTKERNEL");
 
-        my $isSpecialSecure =    ($eyeCatch eq "HBB")
-                              || ($eyeCatch eq "HBI")
-                              || ($eyeCatch eq "HBD");
+        my $isSpecialSecure =    ($eyeCatch eq "HBB");
+                              #|| ($eyeCatch eq "HBI")
+                              #|| ($eyeCatch eq "HBD");
 
         my $openSigningFlags = OP_SIGNING_FLAG.$sb_hdrs{DEFAULT}{flags};
         my $secureboot_hdr =  $sb_hdrs{DEFAULT}{file};
@@ -566,7 +566,7 @@ sub manipulateImages
                 $fsp_prefix.=".header";
                 # Add secure container header
                 # @TODO RTC:155374 Remove when official signing supported
-                if ($secureboot)
+                if ($secureboot && $isSpecialSecure)
                 {
                     $callerHwHdrFields{configure} = 1;
                     # @TODO securebootp9 re-enable hash page table with vfs page table port
diff --git a/src/build/debug/Hostboot/BlTrace.pm b/src/build/debug/Hostboot/BlTrace.pm
index 1c629d707..6a867f3de 100644
--- a/src/build/debug/Hostboot/BlTrace.pm
+++ b/src/build/debug/Hostboot/BlTrace.pm
@@ -66,6 +66,7 @@ my %traceText = (
     "E6" => "Utils checkHeader header size invalid",
     "E7" => "Utils parseEntries invalid section",
     "E8" => "Utils SectionIdToString PNOR section id out of range",
+    "E9" => "Utils cmpSecurebootMagicNumber requested address to compare is a nullptr",
     "F0" => "Main getHBBSection returned",
     "F1" => "Main removeECC returned error",
     "F2" => "PNOR Access readTOC checkForNullBuffer null buffer",
diff --git a/src/include/bootloader/bootloader_trace.H b/src/include/bootloader/bootloader_trace.H
index fb86e1c8b..2747e0d97 100644
--- a/src/include/bootloader/bootloader_trace.H
+++ b/src/include/bootloader/bootloader_trace.H
@@ -142,7 +142,10 @@ enum BootloaderTraces
     BTLDR_TRC_UTILS_PARSE_INVALID_SECTION    = 0xE7,
 
     /** Bootloader Utils SectionIdToString PNOR section id out of range*/
-    BTLDR_TRC_UTILS_PARSE_PNOR_SECID_OUT_OF_RANGE    = 0xE8,
+    BTLDR_TRC_UTILS_PNOR_SECID_OUT_OF_RANGE    = 0xE8,
+
+    /** Bootloader Utils cmpSecurebootMagicNumber requested address to compare is a nullptr*/
+    BTLDR_TRC_UTILS_CMP_MAGIC_NUM_NULLPTR    = 0xE9,
 
     /** Bootloader main getHBBSection returned error */
     BTLDR_TRC_MAIN_GETHBBSECTION_FAIL        = 0xF0,
diff --git a/src/include/bootloader/hbblreasoncodes.H b/src/include/bootloader/hbblreasoncodes.H
index 22abf9059..e071c40b8 100644
--- a/src/include/bootloader/hbblreasoncodes.H
+++ b/src/include/bootloader/hbblreasoncodes.H
@@ -48,7 +48,8 @@ namespace Bootloader
         MOD_PNORACC_READTOC              = 0x03, /**< bl_pnorAccess.C : read TOC */
         MOD_PNORACC_GETHBBSECT           = 0x04, /**< bl_pnorAccess.C : get HBB sect */
         MOD_BOOTLOADER_VERIFY            = 0x05, /**< bootloader.C : verifyContainer */
-        MOD_BOOTLOADER_PNOR_SECID_TO_STR = 0x06 /**< pnor_utils.C : SectionIdToString */
+        MOD_BOOTLOADER_PNOR_SECID_TO_STR = 0x06, /**< pnor_utils.C : SectionIdToString */
+        MOD_BOOTLOADER_PNOR_CMP_MAGIC_NUM = 0x07 /**< pnor_utils.C : cmpSecurebootMagicNumber */
 
     };
 
@@ -73,6 +74,7 @@ namespace Bootloader
         RC_PARSE_ENTRIES_ERR       = HBBL_COMP_ID | 0x0B, /**< Parse Entries Error */
         RC_NO_HBB_IN_TOC           = HBBL_COMP_ID | 0x0C, /**< No HBB Sect in TOC */
         RC_PNOR_SECID_OUT_OF_RANGE = HBBL_COMP_ID | 0x0D, /**< Requested PNOR SecId DNE in string array */
+        RC_PNOR_NULLPTR            = HBBL_COMP_ID | 0x0E, /**< Requested address to compare is a nullptr */
     };
 
 };  // end Bootloader
diff --git a/src/include/usr/pnor/pnor_const.H b/src/include/usr/pnor/pnor_const.H
index 0a96e3180..f9220ed93 100644
--- a/src/include/usr/pnor/pnor_const.H
+++ b/src/include/usr/pnor/pnor_const.H
@@ -81,7 +81,7 @@ enum SectionId
     // Size and layout of this structure must be maintained for debug framework.
 struct SectionInfo_t
 {
-    SectionInfo_t(): id(INVALID_SECTION) {}
+    SectionInfo_t(): id(INVALID_SECTION), secure(false) {}
     SectionId id;       /**< Identifier for this section */
     const char* name;   /**< Name of the section */
     uint64_t vaddr;     /**< Virtual address of the start of the section */
@@ -92,6 +92,7 @@ struct SectionInfo_t
     bool sha512perEC;   /**< Version Checking perEC */
     bool readOnly;      /**< Section is read only */
     bool reprovision;   /**< Erase this section during a reprovision */
+    bool secure;        /**< Indicates if a section is secure */
 #ifdef CONFIG_SECUREBOOT
     size_t secureProtectedPayloadSize; /**< Cache the secure payload size so
                                             that the secure container only
@@ -100,6 +101,22 @@ struct SectionInfo_t
 };
 
 /**
+  * Internal information to deal with the sections of PNOR
+  */
+struct SectionData_t {
+    SectionData_t(): secure(false) {}
+    PNOR::SectionId id;  /**< Identifier for this section */
+    uint64_t virtAddr;  /**< Virtual address for the start of the section */
+    uint32_t flashAddr;  /**< Address in flash */
+    uint32_t size;/**< Actual size of content in bytes (not including ECC)*/
+    uint8_t chip;  /**< Chip Select */
+    uint8_t version; /**< Version Checking */
+    uint16_t integrity; /**< Data Integrity */
+    uint8_t misc; /**< Misc Flags */
+    uint8_t secure; /**< Indicates if a section is secure */
+} PACKED;
+
+/**
  * PNOR Sides
  */
 enum SideId
diff --git a/src/include/usr/pnor/pnorif.H b/src/include/usr/pnor/pnorif.H
index 311c6c4b4..7f55748f8 100644
--- a/src/include/usr/pnor/pnorif.H
+++ b/src/include/usr/pnor/pnorif.H
@@ -5,7 +5,7 @@
 /*                                                                        */
 /* OpenPOWER HostBoot Project                                             */
 /*                                                                        */
-/* Contributors Listed Below - COPYRIGHT 2011,2016                        */
+/* Contributors Listed Below - COPYRIGHT 2011,2017                        */
 /* [+] Google Inc.                                                        */
 /* [+] International Business Machines Corp.                              */
 /*                                                                        */
@@ -112,37 +112,8 @@ errlHndl_t loadSecureSection(SectionId i_section);
  */
 errlHndl_t unloadSecureSection(SectionId i_section);
 
-/**
- * @brief  Memcmp a vaddr to the known secureboot magic number
- *
- * @param[in]   i_vaddr: vaddr of secureboot header to check for magic number
- *                       Note: must point to a buffer of size >= 4 bytes
- *
- * @return  bool - True if the magic number and starting bytes of the vaddr
- *                 match. False otherwise.
- */
-bool cmpSecurebootMagicNumber(const uint8_t* i_vaddr);
-
-/**
- * @brief  Returns true if a PNOR section has the secureboot container
- *         header magic number at the beginning. This is mainly used to
- *         ignore unwanted PNOR sections like secureboot key transition.
- *         It indicates the section has valid content to be securely
- *         loaded, otherwise the section content will not be loaded.
- *         If a section does not have the header but needs to be loaded,
- *         it will fail ROM verify later on anyhow.
- *         Note: Does not work with HBB section and will assert if attempted
- *
- * @param[in]   i_section: PNOR section to check first bytes of.
- * @param[out]  o_valid:  true if section has the correct magic number at
- *                        the beginning
- *
- * @return  errlHndl_t - NULL if success, errlHndl_t otherwise.
- *                               */
-errlHndl_t hasSecurebootMagicNumber(SectionId i_section, bool &o_valid);
 #endif // CONFIG_SECUREBOOT
 
-
 /**
  * @brief Determines whether the given section is inhibited by secure boot
  *        for containing attribute overrides.
@@ -206,6 +177,20 @@ errlHndl_t validateAltMaster( void );
  */
 void getPnorInfo( PnorInfo_t& o_pnorInfo );
 
+/**
+ * @brief  Check if PNOR section appears to be secure and sets the
+ *         internal TOC of PnorRp accordingly.
+ *         Note: The setting of the flag is based on the Secureboot policy.
+ *
+ * @param[in] i_secId Section ID or index in the TOC to modify
+ * @param[in] io_TOC  Pointer to internal array of section data that
+ *                    represents the TOC of pnor flash
+ *                    Asserts if nullptr
+ *
+ * @return errlHndl_t  Error log if request was invalid
+ */
+errlHndl_t setSecure(const uint32_t i_secId, SectionData_t* io_TOC);
+
 } // PNOR
 
 #endif
diff --git a/src/usr/pnor/pnor_common.C b/src/usr/pnor/pnor_common.C
index f766e3937..bc5e2b173 100644
--- a/src/usr/pnor/pnor_common.C
+++ b/src/usr/pnor/pnor_common.C
@@ -235,12 +235,15 @@ errlHndl_t PNOR::parseTOC( uint8_t* i_tocBuffer,SectionData_t * o_TOC)
 
         ffs_entry* l_err_entry = NULL;
 
-        PNOR::parseEntries(l_ffs_hdr, l_errCode, o_TOC, l_err_entry);
-
-        if(l_errCode != NO_ERROR)
+        l_errhdl = PNOR::parseEntries(l_ffs_hdr, l_errCode, o_TOC, l_err_entry);
+        if (l_errhdl)
         {
-            TRACFCOMP(g_trac_pnor, "PNOR::parseTOC parseEntries"
-                            " parse entries returned an error");
+            TRACFCOMP(g_trac_pnor, "PNOR::parseTOC parseEntries returned an error log");
+            break;
+        }
+        else if(l_errCode != NO_ERROR)
+        {
+            TRACFCOMP(g_trac_pnor, "PNOR::parseTOC parseEntries returned an error code");
             o_TOC = NULL;
             /* @errortype
             * @moduleid PNOR::MOD_PNORRP_READTOC
@@ -295,9 +298,7 @@ errlHndl_t PNOR::parseTOC( uint8_t* i_tocBuffer,SectionData_t * o_TOC)
 
             // @TODO RTC 168021 Remove legacy extensions when all
             // secure sections are supported
-            auto isSecure = PNOR::isSecureSection(l_secId);
-            if (   o_TOC[l_secId].version == FFS_VERS_SHA512
-                && !isSecure)
+            if (PNOR::hasNonSecureHeader(o_TOC[l_secId]))
             {
                 // Never extend the base image through this path, it will be
                 // handled elsewhere
@@ -385,3 +386,44 @@ 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);
+
+#ifndef __HOSTBOOT_RUNTIME
+#ifdef CONFIG_SECUREBOOT_BEST_EFFORT
+    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);
+        auto l_buf = new uint8_t[l_size]();
+        auto l_target = 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.
+        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 8 bytes match the Secureboot Magic Number
+        io_TOC[i_secId].secure &= PNOR::cmpSecurebootMagicNumber(l_buf);
+    }
+#endif
+#endif
+
+    } while (0);
+
+    return l_errhdl;
+}
diff --git a/src/usr/pnor/pnor_utils.C b/src/usr/pnor/pnor_utils.C
index c7e77cbb7..d6e1ffad0 100644
--- a/src/usr/pnor/pnor_utils.C
+++ b/src/usr/pnor/pnor_utils.C
@@ -51,8 +51,15 @@ extern trace_desc_t* g_trac_pnor;
 
 #include "common/ffs_hb.H"
 #include <util/align.H>
+#include <config.h>
+#include <securerom/ROM.H>
 
-/*
+#ifndef BOOTLOADER
+// Includes needed when hostboot uses this file
+#include <pnor/pnorif.H>
+#endif
+
+/**
  * @brief calculates the checksum on data(ffs header/entry) and will return
  *    0 if the checksums match
  */
@@ -145,21 +152,21 @@ void PNOR::checkHeader (ffs_hdr* i_ffs_hdr,
     if(i_ffs_hdr->magic != FFS_MAGIC)
     {
         PNOR_UTIL_TRACE_W_BRK(BTLDR_TRC_UTILS_CHECKHEADER_MAGIC,
-                               "E>PNOR::parseTOC: Invalid magic"
+                               "E>PNOR::checkHeader: Invalid magic"
                 " number in FFS header: 0x%.4X",i_ffs_hdr->magic);
         io_errCode |= INVALID_MAGIC;
     }
     if(i_ffs_hdr->version != SUPPORTED_FFS_VERSION)
     {
         PNOR_UTIL_TRACE_W_BRK(BTLDR_TRC_UTILS_CHECKHEADER_VERSION,
-          "E>PNOR::parseTOC:Unsupported FFS"
+          "E>PNOR::checkHeader:Unsupported FFS"
                 " Header version: 0x%.4X", i_ffs_hdr->version);
         io_errCode |= UNSUPPORTED_FFS;
     }
     if(i_ffs_hdr->entry_size != sizeof(ffs_entry))
     {
         PNOR_UTIL_TRACE_W_BRK(BTLDR_TRC_UTILS_CHECKHEADER_ENTRYSIZE,
-                               "E>PNOR::parseTOC: Unexpected"
+                               "E>PNOR::checkHeader: Unexpected"
                 " entry_size(0x%.8x) in FFS header: 0x%.4X",
                 i_ffs_hdr->entry_size);
         io_errCode |= INVALID_ENTRY_SIZE;
@@ -167,14 +174,14 @@ void PNOR::checkHeader (ffs_hdr* i_ffs_hdr,
     if(i_ffs_hdr->entry_count == 0)
     {
         PNOR_UTIL_TRACE_W_BRK(BTLDR_TRC_UTILS_CHECKHEADER_ENTRYCNT,
-                               "E>PNOR::parseTOC:"
+                               "E>PNOR::checkHeader:"
                                 " FFS Header pointer to entries is NULL.");
         io_errCode |= NO_ENTRIES;
     }
     if(i_ffs_hdr->block_size != PAGESIZE)
     {
         PNOR_UTIL_TRACE_W_BRK(BTLDR_TRC_UTILS_CHECKHEADER_BLOCKSIZE,
-                              "E>PNOR::parseTOC:  Unsupported"
+                              "E>PNOR::checkHeader:  Unsupported"
                               " Block Size(0x%.4X). PNOR Blocks must be 4k",
                               i_ffs_hdr->block_size);
         io_errCode |= INVALID_BLOCK_SIZE;
@@ -182,7 +189,7 @@ void PNOR::checkHeader (ffs_hdr* i_ffs_hdr,
     if(i_ffs_hdr->block_count == 0)
     {
         PNOR_UTIL_TRACE_W_BRK(BTLDR_TRC_UTILS_CHECKHEADER_BLOCKCNT,
-                              "E>PNOR::parseTOC:  Unsupported"
+                              "E>PNOR::checkHeader:  Unsupported"
                 " Block COunt(0x%.4X). Device cannot be zero"
                 " blocks in length.",i_ffs_hdr->block_count);
         io_errCode |= INVALID_BLOCK_COUNT;
@@ -193,7 +200,7 @@ void PNOR::checkHeader (ffs_hdr* i_ffs_hdr,
         ((i_ffs_hdr->block_size*i_ffs_hdr->size)-sizeof(ffs_hdr)))
     {
         PNOR_UTIL_TRACE_W_BRK(BTLDR_TRC_UTILS_CHECKHEADER_HDRSIZE,
-                              "E>PNOR::parseTOC:  FFS Entries"
+                              "E>PNOR::checkHeader:  FFS Entries"
                         " (0x%.16X) go past end of FFS Table.",spaceUsed);
         io_errCode |= INVALID_HEADER_SIZE;
 
@@ -229,11 +236,20 @@ void PNOR::getSectionEnum (ffs_entry* i_entry,
   *         During the iteration we are checking that the entries are valid
   *        and we set the sectionData_t for each section in the TOC.
   */
-void PNOR::parseEntries (ffs_hdr* i_ffs_hdr,
-                         uint32_t& io_errCode,
-                         SectionData_t * io_TOC,
-                         ffs_entry*& o_err_entry)
+#ifdef BOOTLOADER
+void
+#else
+errlHndl_t
+#endif
+PNOR::parseEntries (ffs_hdr* i_ffs_hdr,
+                    uint32_t& io_errCode,
+                    PNOR::SectionData_t * io_TOC,
+                    ffs_entry*& o_err_entry)
 {
+#ifndef BOOTLOADER
+    errlHndl_t l_errhdl = nullptr;
+#endif
+
     //Walk through all the entries in the table and parse the data.
     for(uint32_t i=0; i<i_ffs_hdr->entry_count; i++)
     {
@@ -244,7 +260,7 @@ void PNOR::parseEntries (ffs_hdr* i_ffs_hdr,
         if( PNOR::pnor_ffs_checksum(cur_entry, FFS_ENTRY_SIZE) != 0)
         {
             PNOR_UTIL_TRACE_W_BRK(BTLDR_TRC_UTILS_PARSE_CHECKSUM_ERROR,
-                                   "E>PNOR::parseTOC:  "
+                                   "E>PNOR::parseEntries:  "
                                    "Check sum error while parseing entry ",
                                    "%d in TOC", i);
             io_errCode |= ENTRY_ERR;
@@ -260,7 +276,7 @@ void PNOR::parseEntries (ffs_hdr* i_ffs_hdr,
         if(secId == PNOR::INVALID_SECTION)
         {
             PNOR_UTIL_TRACE(BTLDR_TRC_UTILS_PARSE_INVALID_SECTION,
-                            "PNOR::parseTOC: "
+                            "PNOR::parseEntries: "
                             "Unsupported section found while parsing entry ",
                             "%d in TOC \n Entry name is \"%s\"", i,
                             cur_entry->name);
@@ -289,7 +305,7 @@ void PNOR::parseEntries (ffs_hdr* i_ffs_hdr,
                 (i_ffs_hdr->block_count*PAGESIZE))
         {
             PNOR_UTIL_TRACE_W_BRK(BTLDR_TRC_UTILS_PARSE_EXCEEDS_FLASH,
-                                  "E>PNOR::parseTOC:  "
+                                  "E>PNOR::parseEntries:  "
                                    "Exceeded flash while parsing entry ",
                                    "%d in TOC \n Entry name is \"%s\"", i,
                                     cur_entry->name);
@@ -306,8 +322,18 @@ void PNOR::parseEntries (ffs_hdr* i_ffs_hdr,
                                 ((io_TOC[secId].size * 8 ) / 9);
         }
 
-        if (io_TOC[secId].version == FFS_VERS_SHA512
-            && !PNOR::isSecureSection(secId))
+#ifdef BOOTLOADER
+        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;
+        }
+#endif
+
+        if (PNOR::hasNonSecureHeader(io_TOC[secId]))
         {
               //increment flash addr for sha header
             if (io_TOC[secId].integrity == FFS_INTEG_ECC_PROTECT)
@@ -323,27 +349,28 @@ void PNOR::parseEntries (ffs_hdr* i_ffs_hdr,
             // adjust the size to reflect that
             io_TOC[secId].size -= PAGESIZE;
         }
-
     } // For TOC Entries
+
+#ifndef BOOTLOADER
+    return l_errhdl;
+#endif
+
 }
 
-bool PNOR::isSecureSection(const uint32_t i_section)
+bool PNOR::isEnforcedSecureSection(const uint32_t i_section)
 {
 #ifdef CONFIG_SECUREBOOT
     #ifdef BOOTLOADER
         return i_section == HB_BASE_CODE;
-    // TODO securebootp9 uncomment these sections as they become ready for
-    // inclusion in p9. Remove this comment after the last one.
     #else
-    //    return i_section == HB_EXT_CODE ||
-    //           i_section == HB_DATA ||
-    //           i_section == SBE_IPL ||
-    //           i_section == CENTAUR_SBE ||
-    //           i_section == PAYLOAD ||
-    //           i_section == SBKT ||
-    //           i_section == OCC ||
-    //           i_section == HB_RUNTIME;
-        return false;
+        return i_section == HB_EXT_CODE ||
+               i_section == HB_DATA ||
+               i_section == SBE_IPL ||
+               i_section == CENTAUR_SBE ||
+               i_section == PAYLOAD ||
+               i_section == SBKT ||
+               i_section == OCC ||
+               i_section == HB_RUNTIME;
     #endif
 #else
     return false;
@@ -403,7 +430,7 @@ const char * PNOR::SectionIdToString( uint32_t i_secIdIndex )
 #ifdef BOOTLOADER
     if(i_secIdIndex >= (PNOR::NUM_SECTIONS))
     {
-        PNOR_UTIL_TRACE(BTLDR_TRC_UTILS_PARSE_PNOR_SECID_OUT_OF_RANGE);
+        PNOR_UTIL_TRACE(BTLDR_TRC_UTILS_PNOR_SECID_OUT_OF_RANGE);
         /*@
          * @errortype
          * @moduleid     Bootloader::MOD_BOOTLOADER_PNOR_SECID_TO_STR
@@ -427,3 +454,33 @@ const char * PNOR::SectionIdToString( uint32_t i_secIdIndex )
     return SectionIdToStringArr[i_secIdIndex];
 }
 
+bool PNOR::cmpSecurebootMagicNumber(const uint8_t* i_vaddr)
+{
+    // Bootloader does not support asserts
+#ifdef BOOTLOADER
+    if(i_vaddr == nullptr)
+    {
+        PNOR_UTIL_TRACE(BTLDR_TRC_UTILS_CMP_MAGIC_NUM_NULLPTR);
+        /*@
+         * @errortype
+         * @moduleid     Bootloader::MOD_BOOTLOADER_PNOR_CMP_MAGIC_NUM
+         * @reasoncode   Bootloader::RC_PNOR_NULLPTR
+         * @devdesc      Requested address to compare is a nullptr
+         * @custdesc     A problem occurred while running processor
+         *               boot code.
+         */
+        bl_terminate(Bootloader::MOD_BOOTLOADER_PNOR_CMP_MAGIC_NUM,
+                     Bootloader::RC_PNOR_NULLPTR);
+    }
+#else
+    assert(i_vaddr != nullptr, "cmpSecurebootMagicNumber requested address to compare is a nullptr ");
+#endif
+
+    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 8bb4504f1..fad0dee38 100644
--- a/src/usr/pnor/pnor_utils.H
+++ b/src/usr/pnor/pnor_utils.H
@@ -31,6 +31,10 @@
 #include "ffs.h"
 #include <config.h>
 
+#ifndef BOOTLOADER
+#include <errl/errlentry.H>
+#endif
+
 /** @file pnor_utils.H
  *  @brief Provides the utility functions used by different PNOR
  *         classes.
@@ -104,22 +108,6 @@ enum
     LPC_FW_SPACE = 0xF0000000,
 };
 
-
-
-/**
-  * Internal information to deal with the sections of PNOR
-  */
-struct SectionData_t {
-    PNOR::SectionId id;  /**< Identifier for this section */
-    uint64_t virtAddr;  /**< Virtual address for the start of the section */
-    uint32_t flashAddr;  /**< Address in flash */
-    uint32_t size;/**< Actual size of content in bytes (not including ECC)*/
-    uint8_t chip;  /**< Chip Select */
-    uint8_t version; /**< Version Checking */
-    uint16_t integrity; /**< Data Integrity */
-    uint8_t misc; /**< Misc Flags */
-} PACKED;
-
 /**
   * @brief Creates a 4-byte Cyclic Redundancy Check (CRC) on the data
   *        provided. The last iteration of the for-loop includes the ffs
@@ -162,8 +150,8 @@ void initializeSections(SectionData_t io_toc[NUM_SECTIONS]);
   *                         casted as an ffs_hdr
   */
 void checkForNullBuffer(uint8_t* i_tocBuffer,
-                           uint32_t& o_errCode,
-                          ffs_hdr*& );
+                        uint32_t& o_errCode,
+                        ffs_hdr*& );
 
 
 /**
@@ -213,21 +201,30 @@ void getSectionEnum (ffs_entry* i_entry,
   *
   * @param[out]  o_err_entry      Pass out bad entry so user can know which
   *                               entry has errors
-  *
+  * if BOOTLOADER
+  * @return N/A
+  * else
+  * @return     errlHndl_t        error log if error, nullptr otherwise
 */
-void parseEntries (ffs_hdr* i_ffs_hdr,
-                    uint32_t& io_errCode,
-                    SectionData_t* io_TOC,
-                    ffs_entry*& o_err_entry);
+#ifdef BOOTLOADER
+void
+#else
+errlHndl_t
+#endif
+parseEntries (ffs_hdr* i_ffs_hdr,
+              uint32_t& io_errCode,
+              SectionData_t * io_TOC,
+              ffs_entry*& o_err_entry);
 
 /**
  * @brief Determines whether the given section is secured by secure boot
+ *        This checks against a hardcoded list of what must be secure.
  *
  * @param[in] i_section PNOR section to test.
  *
  * @return bool True if secure section, false otherwise.
  */
-bool isSecureSection(const uint32_t i_section);
+bool isEnforcedSecureSection(const uint32_t i_section);
 
 /**
   * @brief Returns string representation of a PNOR SectionId.
@@ -243,6 +240,28 @@ bool isSecureSection(const uint32_t i_section);
 */
 const char * SectionIdToString( uint32_t i_secIdIndex );
 
+/**
+ * @brief  Memcmp a vaddr to the known secureboot magic number
+ *
+ * @param[in]   i_vaddr: vaddr of secureboot header to check for magic number
+ *                       Note: must point to a buffer of size >= 4 bytes
+ *                       Nullptr asserts or terminates bootloader
+ *
+ * @return  bool - True if the magic number and starting bytes of the vaddr
+ *                 match. False otherwise.
+ */
+bool cmpSecurebootMagicNumber(const uint8_t* i_vaddr);
+
+/**
+ * @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 329e3cac9..52ba22b57 100644
--- a/src/usr/pnor/pnorrp.C
+++ b/src/usr/pnor/pnorrp.C
@@ -527,11 +527,13 @@ errlHndl_t PnorRP::getSectionInfo( PNOR::SectionId i_section,
             o_info.name = SectionIdToString(id);
 
 #ifdef CONFIG_SECUREBOOT
+            o_info.secure = iv_TOC[id].secure;
             o_info.secureProtectedPayloadSize = 0; // for non secure sections
                                                    // the protected payload size
                                                    // defaults to zero
-            // handle secure sections in SPnorRP's address space
-            if (PNOR::isSecureSection(o_info.id))
+            // If a secure section and has a secure header handle secure
+            // sections in SPnorRP's address space
+            if (o_info.secure)
             {
                 uint8_t* l_vaddr = reinterpret_cast<uint8_t*>(iv_TOC[id].virtAddr);
                 // By adding VMM_VADDR_SPNOR_DELTA twice we can translate a pnor
@@ -563,17 +565,17 @@ errlHndl_t PnorRP::getSectionInfo( PNOR::SectionId i_section,
                     memcpy(&l_badMagicHeader, l_vaddr, sizeof(ROM_MAGIC_NUMBER));
                     TRACFCOMP( g_trac_pnor, ERR_MRK"PnorRP::getSectionInfo: magic number not valid to parse container for section = %s magic number = 0x%X",
                                o_info.name, l_badMagicHeader);
-                /*@
-                 * @errortype
-                 * @severity     ERRORLOG::ERRL_SEV_UNRECOVERABLE
-                 * @moduleid     PNOR::MOD_PNORRP_GETSECTIONINFO
-                 * @reasoncode   PNOR::RC_BAD_SECURE_MAGIC_NUM
-                 * @userdata1    Requested Section
-                 * @userdata2    Bad magic number
-                 * @devdesc      PNOR section does not have the known secureboot magic number
-                 * @custdesc     Corrupted flash image or firmware error during system boot
-                 */
-                l_errhdl = new ERRORLOG::ErrlEntry(ERRORLOG::ERRL_SEV_UNRECOVERABLE,
+                    /*@
+                     * @errortype
+                     * @severity     ERRORLOG::ERRL_SEV_UNRECOVERABLE
+                     * @moduleid     PNOR::MOD_PNORRP_GETSECTIONINFO
+                     * @reasoncode   PNOR::RC_BAD_SECURE_MAGIC_NUM
+                     * @userdata1    Requested Section
+                     * @userdata2    Bad magic number
+                     * @devdesc      PNOR section does not have the known secureboot magic number
+                     * @custdesc     Corrupted flash image or firmware error during system boot
+                     */
+                    l_errhdl = new ERRORLOG::ErrlEntry(ERRORLOG::ERRL_SEV_UNRECOVERABLE,
                                                        PNOR::MOD_PNORRP_GETSECTIONINFO,
                                                        PNOR::RC_BAD_SECURE_MAGIC_NUM,
                                                        TO_UINT64(i_section),
diff --git a/src/usr/pnor/spnorrp.C b/src/usr/pnor/spnorrp.C
index 5f03f46c8..c17d493ef 100644
--- a/src/usr/pnor/spnorrp.C
+++ b/src/usr/pnor/spnorrp.C
@@ -278,9 +278,26 @@ uint64_t SPnorRP::verifySections(SectionId i_id, LoadRecord* o_rec)
 
             break;
         }
+
         TRACDCOMP(g_trac_pnor,"SPnorRP::verifySections getSectionInfo"
                 " succeeded for sec = %s", l_info.name);
 
+        if (!l_info.secure)
+        {
+#ifdef CONFIG_SECUREBOOT_BEST_EFFORT
+            TRACFCOMP(g_trac_pnor,"PNOR::loadSecureSection> called on unsecured section - Best effort policy skipping");
+            break;
+#else
+            TRACFCOMP(g_trac_pnor,ERR_MRK"PNOR::loadSecureSection> called on "
+                "unsecured section");
+
+            // TODO securebootp9 revisit this assert code and replace with error log
+            // code if it is deemed that this assert could happen in the field
+            assert(false,"PNOR::loadSection> section %i is not a secure section",
+                                                                    i_id);
+#endif
+        }
+
         l_info.vaddr -= PAGESIZE; // back up a page to expose the secure header
         l_info.size += PAGESIZE; // add a page to size to account for the header
 
@@ -682,34 +699,6 @@ errlHndl_t PNOR::loadSecureSection(const SectionId i_section)
     // Send message to secure provider to load the section
     errlHndl_t err = NULL;
 
-    if (!isSecureSection(i_section))
-    {
-        TRACFCOMP(g_trac_pnor,ERR_MRK"PNOR::loadSecureSection> called on "
-            "unsecured section");
-        // TODO securebootp9 remove below temporary code after all of the
-        // sections in the below if condition have been fully ported and added
-        // to isSecureSection.
-        // start temporary code
-        if (i_section == PNOR::HB_EXT_CODE ||
-            i_section == PNOR::HB_DATA ||
-            i_section == PNOR::SBE_IPL ||
-            i_section == PNOR::CENTAUR_SBE ||
-            i_section == PNOR::PAYLOAD ||
-            i_section == PNOR::OCC ||
-            i_section == PNOR::HB_RUNTIME)
-        {
-            // For now, ignore the attempt to load this section securely.
-            // Returning from the middle of a function is excusable because
-            // it keeps the temp code in one place, making it easier to remove.
-            return NULL;
-        }
-        // end temporary code
-        // TODO securebootp9 revisit this assert code and replace with error log
-        // code if it is deemed that this assert could happen in the field
-        assert(false,"PNOR::loadSection> section %i is not a secure section",
-                                                                    i_section);
-    }
-
     msg_q_t spnorQ = msg_q_resolve(SPNORRP_MSG_Q);
 
     assert(spnorQ != NULL);
@@ -917,46 +906,4 @@ errlHndl_t SPnorRP::keyTransitionCheck(const uint8_t *i_vaddr) const
     }while(0);
 
     return l_errl;
-}
-
-bool PNOR::cmpSecurebootMagicNumber(const uint8_t* i_vaddr)
-{
-    return memcmp(&ROM_MAGIC_NUMBER, i_vaddr, sizeof(ROM_MAGIC_NUMBER)) == 0;
-}
-
-errlHndl_t PNOR::hasSecurebootMagicNumber(const SectionId i_section,
-                                          bool &o_valid)
-{
-    errlHndl_t l_errl = NULL;
-    SectionInfo_t l_info;
-
-    // Force to false
-    o_valid = false;
-
-    // This will not work for HBB
-    assert(i_section != HB_BASE_CODE, "hasSecurebootMagicNumber() does not work for HBB section");
-
-    bool isSecure = PNOR::isSecureSection(i_section);
-    do {
-        l_errl = getSectionInfo(i_section, l_info);
-        if (l_errl)
-        {
-            TRACFCOMP(g_trac_pnor, ERR_MRK"PNOR::hasSecurebootMagicNumber(): - getSectionInfo failed");
-            break;
-        }
-
-        // Use PNOR vaddr
-        if(isSecure)
-        {
-            // back up a page to expose the secure header
-            l_info.vaddr = l_info.vaddr - VMM_VADDR_SPNOR_DELTA
-                                        - VMM_VADDR_SPNOR_DELTA
-                                        - PAGESIZE;
-        }
-        o_valid = cmpSecurebootMagicNumber(reinterpret_cast<uint8_t*>
-                                           (l_info.vaddr));
-    }while(0);
-
-    return l_errl;
-}
-
+}
\ No newline at end of file
diff --git a/src/usr/pnor/test/pnorrptest.H b/src/usr/pnor/test/pnorrptest.H
index e2be5cba6..f3d94ef99 100644
--- a/src/usr/pnor/test/pnorrptest.H
+++ b/src/usr/pnor/test/pnorrptest.H
@@ -818,7 +818,7 @@ class PnorRpTest : public CxxTest::TestSuite
         errlHndl_t pError=NULL;
         do {
 
-        if (!PNOR::isSecureSection(PNOR::SBE_IPL))
+        if (!PNOR::isEnforcedSecureSection(PNOR::SBE_IPL))
         {
             break;
         }
diff --git a/src/usr/secureboot/HBconfig b/src/usr/secureboot/HBconfig
index 0ef8bc748..e52e25101 100644
--- a/src/usr/secureboot/HBconfig
+++ b/src/usr/secureboot/HBconfig
@@ -2,7 +2,15 @@
 config SECUREBOOT
     default y
     help
-        Enable secure boot
+        Enable and enforce secure boot
+
+# @TODO RTC:155374 Remove this in the future
+config SECUREBOOT_BEST_EFFORT
+    default y if SECUREBOOT
+    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 y if (SECUREBOOT && TPMDD)
diff --git a/src/usr/secureboot/base/makefile b/src/usr/secureboot/base/makefile
index 58d79be86..c58c54487 100644
--- a/src/usr/secureboot/base/makefile
+++ b/src/usr/secureboot/base/makefile
@@ -47,5 +47,7 @@ EXTRAINCDIR += ${ROOTPATH}/src/usr/secureboot/trusted/base
 VPATH += ${ROOTPATH}/src/usr/secureboot/trusted/base
 VPATH += ${ROOTPATH}/src/securerom
 
+EXTRAINCDIR += ${ROOTPATH}/src/usr/pnor/
+
 CFLAGS += -iquote ../
 include ${ROOTPATH}/config.mk
diff --git a/src/usr/secureboot/trusted/base/trustedboot_base.C b/src/usr/secureboot/trusted/base/trustedboot_base.C
index c4a149368..8dfe6cb0a 100644
--- a/src/usr/secureboot/trusted/base/trustedboot_base.C
+++ b/src/usr/secureboot/trusted/base/trustedboot_base.C
@@ -50,6 +50,7 @@
 #include "../trustedbootCmds.H"
 #include "../trustedbootUtils.H"
 #include "trustedbootMsg.H"
+#include <pnor_utils.H>
 
 // ----------------------------------------------
 // Trace definitions