Secure Boot: Fix lid load from HB reserved memory issues at runtime

- Force all PNOR sections we load from HB rserved memory to be secure
  Only exception is the RINGOVD section, in which we use a fake header
- Add fake header when Secureboot compiled out or a section is never
  signed as there is no secure header preserved in virtual memory

RTC: 171708
RTC: 180063
Change-Id: Ibbbd7be24ee7b199e73451c63b2c2d1f86a2c2d8
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/49020
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Reviewed-by: Nicholas E. Bofferding <bofferdn@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Reviewed-by: Marshall J. Wilks <mjwilks@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>

15 files changed, 441 insertions, 163 deletions

diff --git a/src/include/usr/pnor/pnor_const.H b/src/include/usr/pnor/pnor_const.H
index e820bdac6..81ebf0385 100644
--- a/src/include/usr/pnor/pnor_const.H
+++ b/src/include/usr/pnor/pnor_const.H
@@ -123,11 +123,9 @@ struct SectionInfo_t
     bool Volatile;   /**< Section loses contents on non HB reboots */
     bool secure;        /**< Indicates if a section is secure */
     bool clearOnEccErr; /**< Indicates on ECC errors, clear and reboot*/
-#ifdef CONFIG_SECUREBOOT
     size_t secureProtectedPayloadSize; /**< Cache the secure payload size so
                                             that the secure container only
                                             needs to be parsed once */
-#endif
 };
 
 /**
diff --git a/src/include/usr/runtime/common/runtime_utils.H b/src/include/usr/runtime/common/runtime_utils.H
index d63a24d7b..c7f318bc1 100644
--- a/src/include/usr/runtime/common/runtime_utils.H
+++ b/src/include/usr/runtime/common/runtime_utils.H
@@ -45,4 +45,15 @@ extern const PreVerifyVector preVerifiedPnorSections;
  */
 bool isPreVerifiedSection(const PNOR::SectionId i_section);
 
+/**
+ * @brief Determines whether the given section is pre-verified and if so, if
+ *        it's expected to be secure
+ *
+ * @param[in] i_section PNOR section to test.
+ *
+ * @return bool True if pre-verified section is expected to be secure,
+*              false otherwise.
+ */
+bool isPreVerifiedSectionSecure(const PNOR::SectionId i_section);
+
 }
\ No newline at end of file
diff --git a/src/include/usr/runtime/populate_hbruntime.H b/src/include/usr/runtime/populate_hbruntime.H
index 03d8b80de..9f0466172 100644
--- a/src/include/usr/runtime/populate_hbruntime.H
+++ b/src/include/usr/runtime/populate_hbruntime.H
@@ -23,6 +23,8 @@
 /*                                                                        */
 /* IBM_PROLOG_END_TAG                                                     */
 #include <hdat/hdat.H>
+#include <vector>
+#include <utility>
 
 namespace RUNTIME
 {
@@ -68,9 +70,13 @@ errlHndl_t unmapVirtAddr(uint64_t i_addr);
  *  @brief Pre verify Pnor sections and load into HB reserved memory
  *
  *  @param[in] i_sec - pnor section to pre-verify and load
+ *  @param[in] i_verified - Indicates if pnor section is expected to have a
+ *                          secure header.
+ *                          e.g. RINGOVD currently never has a secure header
  *
  *  @return Error handle if error
  */
-errlHndl_t hbResvLoadSecureSection (const PNOR::SectionId i_sec);
+errlHndl_t hbResvLoadSecureSection (const PNOR::SectionId i_sec,
+                                    const bool i_verified);
 
 } // End of Namespace
\ No newline at end of file
diff --git a/src/include/usr/runtime/preverifiedlidmgr.H b/src/include/usr/runtime/preverifiedlidmgr.H
index 185a0d24f..4b087e102 100644
--- a/src/include/usr/runtime/preverifiedlidmgr.H
+++ b/src/include/usr/runtime/preverifiedlidmgr.H
@@ -171,6 +171,14 @@ class PreVerifiedLidMgr
     // Depends on the payload kind
     uint64_t (*getNextAddress)(const size_t);
 
+    // Add fake headers during pnor loads
+    // Use Case: Secureboot compiled out or unsigned sections need a header
+    //           added so runtime can parse it for the section size
+    static bool cv_addFakeHdrs;
+
+    // Current Pnor section ID we are processing. Used to generate fake header
+    static PNOR::SectionId cv_curPnorSecId;
+
     /**
      * @brief Get aligned reserved memory size for OPAL
      *        Note: Historically we have used the aligned size for OPAL
diff --git a/src/include/usr/secureboot/containerheader.H b/src/include/usr/secureboot/containerheader.H
index 255566c9d..f7e924d9b 100644
--- a/src/include/usr/secureboot/containerheader.H
+++ b/src/include/usr/secureboot/containerheader.H
@@ -28,6 +28,8 @@
 #include <errl/errlentry.H>
 #include <secureboot/service.H>
 #include <securerom/ROM.H>
+#include <limits.h>
+#include <array>
 
 // Forward Declaration
 class SecureRomManagerTest;
@@ -54,18 +56,37 @@ class ContainerHeader
         ContainerHeader(const void* i_header):
             iv_isValid(false),iv_hdrBytesRead(0)
         {
-            assert(i_header != NULL);
+            assert(i_header != nullptr);
             iv_pHdrStart = reinterpret_cast<const uint8_t*>(i_header);
-            memset(&iv_headerInfo, 0x00, sizeof(iv_headerInfo));
-            memset(iv_hwKeyHash, 0, sizeof(SHA512_t));
-            memset(iv_componentId,0x00,sizeof(iv_componentId));
+            initVars();
             parse_header(i_header);
         };
 
         /**
+         * @brief ContainerHeader
+         *
+         * This constructor generates a fake header with minimal information
+         *
+         * @param[in] i_totalSize   Total Container Size
+         * @param[in] i_compId      Component ID
+         */
+        ContainerHeader(const size_t i_totalSize,
+                        const char* i_compId):
+            iv_isValid(false),iv_hdrBytesRead(0),iv_fakeHeader{}
+        {
+            initVars();
+            genFakeHeader(i_totalSize, i_compId);
+        };
+
+        /**
+         * @brief Initialize internal variables
+         */
+        void initVars();
+
+        /**
          * @brief Destructor
          */
-        ~ContainerHeader(){};
+        ~ContainerHeader(){}
 
         /**
          * @brief Retrieves total container size (includes header, payload text,
@@ -158,6 +179,13 @@ class ContainerHeader
          */
         const char* componentId() const;
 
+        /**
+         *  @brief Returns the container's fake header
+         *
+         *  @return const uint8_t* fake header
+         */
+        const uint8_t* fakeHeader() const;
+
     private:
         /**
          * @brief Default Constructor in private to prevent being instantiated
@@ -259,6 +287,18 @@ class ContainerHeader
         void safeMemCpyAndInc(void* i_dest, const uint8_t* &io_hdr,
                               const size_t i_size);
 
+        // Pointer to fake header generated
+        std::array<uint8_t,PAGE_SIZE> iv_fakeHeader;
+
+        /**
+         * @brief Generate fake header with limited information
+         *
+         * @param[in] i_totalSize   Total container size
+         * @param[in] i_compId      Component ID
+         */
+        void genFakeHeader(const size_t i_totalSize,
+                           const char* const i_compId);
+
         friend class ::SecureRomManagerTest;
 };
 
diff --git a/src/usr/pnor/runtime/rt_pnor.C b/src/usr/pnor/runtime/rt_pnor.C
index a9c3e4d1e..16707f774 100644
--- a/src/usr/pnor/runtime/rt_pnor.C
+++ b/src/usr/pnor/runtime/rt_pnor.C
@@ -41,6 +41,8 @@
 #include <runtime/customize_attrs_for_payload.H>
 #include <securerom/ROM.H>
 #include <config.h>
+#include "../pnor_utils.H"
+#include <runtime/common/runtime_utils.H>
 
 // Trace definition
 extern trace_desc_t* g_trac_pnor;
@@ -155,30 +157,43 @@ errlHndl_t RtPnor::getSectionInfo(PNOR::SectionId i_section,
     errlHndl_t l_err = nullptr;
     do
     {
-
-        // TODO: RTC:180063 change this to error out on secure sections as it
-        //                  did previously in HB commit cefc4c
         // Check if Section is invalid or inhibited from loading at runtime.
         bool l_inhibited = false;
+        bool l_secure = PNOR::isEnforcedSecureSection(i_section);
+        bool l_preVerified = RUNTIME::isPreVerifiedSection(i_section);
 #ifdef CONFIG_SECUREBOOT
         l_inhibited = PNOR::isInhibitedSection(i_section);
 #endif
-        if (i_section == PNOR::INVALID_SECTION || l_inhibited)
+        if (i_section == PNOR::INVALID_SECTION || l_inhibited || l_secure ||
+            l_preVerified)
         {
             TRACFCOMP(g_trac_pnor, "RtPnor::getSectionInfo: Invalid Section %d",
                       static_cast<int>(i_section));
+
+            if (l_preVerified)
+            {
+                TRACFCOMP(g_trac_pnor, ERR_MRK"RtPnor::getSectionInfo: pre-verified sections should be loaded via Hostboot Reserved Memory ");
+            }
 #ifdef CONFIG_SECUREBOOT
-            if (l_inhibited)
+            else if (l_inhibited)
             {
                 TRACFCOMP(g_trac_pnor, ERR_MRK"RtPnor::getSectionInfo: attribute overrides inhibited by secureboot");
             }
+            else if (l_secure)
+            {
+                TRACFCOMP(g_trac_pnor, ERR_MRK"RtPnor::getSectionInfo: secure sections should be loaded via Hostboot Reserved Memory");
+            }
 #endif
+
             /*@
              * @errortype
              * @moduleid    PNOR::MOD_RTPNOR_GETSECTIONINFO
              * @reasoncode  PNOR::RC_RTPNOR_INVALID_SECTION
              * @userdata1   PNOR::SectionId
-             * @userdata2[0:63]   Inhibited by secureboot
+             * @userdata2[0:15]   Inhibited by secureboot
+             * @userdata2[16:31]  Indication of a secure section
+             * @userdata2[32:47]  Indication of a pre-verified section
+             * @userdata2[48:63]  0
              * @devdesc     invalid section passed to getSectionInfo  or
              *              section prohibited by secureboot
              */
@@ -186,7 +201,10 @@ errlHndl_t RtPnor::getSectionInfo(PNOR::SectionId i_section,
                                             PNOR::MOD_RTPNOR_GETSECTIONINFO,
                                             PNOR::RC_RTPNOR_INVALID_SECTION,
                                             i_section,
-                                            l_inhibited,
+                                            FOUR_UINT16_TO_UINT64(l_inhibited,
+                                                                  l_secure,
+                                                                  l_preVerified,
+                                                                  0),
                                             true);
             break;
         }
@@ -215,58 +233,44 @@ errlHndl_t RtPnor::getSectionInfo(PNOR::SectionId i_section,
         bool l_ecc = (iv_TOC[i_section].integrity&FFS_INTEG_ECC_PROTECT) ?
                       true : false;
 
-        // TODO: RTC:180063 change this to error out on secure sections as it
-        //                  did previously in HB commit cefc4c
-        // Only do mapping and read from device to set vaddr if not a secure
-        // section. Secure sections should load from HB resv memory and will set
-        // vaddr to 0
-        if (iv_TOC[i_section].secure)
+        void* l_pWorking = nullptr;
+        void* l_pClean   = nullptr;
+
+        //find the section in the map first
+        if(iv_pnorMap.find(i_section) != iv_pnorMap.end())
         {
-            TRACFCOMP(g_trac_pnor,"RtPnor::getSectionInfo: Warning> Section is secure, so must be loaded from Hb resv memory. vaddr will be set to 0");
-            o_info.vaddr = 0;
+            //get the addresses from the map
+            PnorAddrPair_t l_addrPair = iv_pnorMap[i_section];
+            l_pWorking = l_addrPair.first;
+            l_pClean   = l_addrPair.second;
         }
         else
         {
-            void* l_pWorking = nullptr;
-            void* l_pClean   = nullptr;
-
-            //find the section in the map first
-            if(iv_pnorMap.find(i_section) != iv_pnorMap.end())
+            //malloc twice -- one working copy and one clean copy
+            //So, we can diff and write only the dirty bytes
+            l_pWorking = malloc(l_sizeBytes);
+            l_pClean   = malloc(l_sizeBytes);
+
+            //offset = 0 : read the entire section
+            l_err = readFromDevice(iv_masterProcId, i_section, 0, l_sizeBytes,
+                                   l_ecc, l_pWorking);
+            if(l_err)
             {
-                //get the addresses from the map
-                PnorAddrPair_t l_addrPair = iv_pnorMap[i_section];
-                l_pWorking = l_addrPair.first;
-                l_pClean   = l_addrPair.second;
+                TRACFCOMP(g_trac_pnor, "RtPnor::getSectionInfo:readFromDevice"
+                      " failed");
+                break;
             }
-            else
-            {
-                //malloc twice -- one working copy and one clean copy
-                //So, we can diff and write only the dirty bytes
-                l_pWorking = malloc(l_sizeBytes);
-                l_pClean   = malloc(l_sizeBytes);
-
-                //offset = 0 : read the entire section
-                l_err = readFromDevice(iv_masterProcId, i_section, 0, l_sizeBytes,
-                                       l_ecc, l_pWorking);
-                if(l_err)
-                {
-                    TRACFCOMP(g_trac_pnor, "RtPnor::getSectionInfo:readFromDevice"
-                          " failed");
-                    break;
-                }
 
-                //copy data to another pointer to save a clean copy of data
-                memcpy(l_pClean, l_pWorking, l_sizeBytes);
+            //copy data to another pointer to save a clean copy of data
+            memcpy(l_pClean, l_pWorking, l_sizeBytes);
 
-                //save it in the map
-                iv_pnorMap [i_section] = PnorAddrPair_t(l_pWorking, l_pClean);
-            }
-            o_info.vaddr = reinterpret_cast<uint64_t>(l_pWorking);
+            //save it in the map
+            iv_pnorMap [i_section] = PnorAddrPair_t(l_pWorking, l_pClean);
         }
-
         //return the data in the struct
         o_info.id           = i_section;
         o_info.name         = SectionIdToString(i_section);
+        o_info.vaddr        = reinterpret_cast<uint64_t>(l_pWorking);
         o_info.flashAddr    = iv_TOC[i_section].flashAddr;
         o_info.size         = l_sizeBytes;
         o_info.eccProtected = l_ecc;
diff --git a/src/usr/runtime/common/runtime_utils.C b/src/usr/runtime/common/runtime_utils.C
index 04a42f502..7b900389f 100644
--- a/src/usr/runtime/common/runtime_utils.C
+++ b/src/usr/runtime/common/runtime_utils.C
@@ -58,4 +58,22 @@ bool isPreVerifiedSection(const PNOR::SectionId i_section)
     return l_result;
 }
 
+bool isPreVerifiedSectionSecure(const PNOR::SectionId i_section)
+{
+    bool l_result = false;
+    auto it = find_if(preVerifiedPnorSections.begin(),
+                      preVerifiedPnorSections.end(),
+                      [&i_section](const PreVerifyPair& p)
+                      {
+                          return p.first == i_section;
+                      });
+
+    if (it != preVerifiedPnorSections.end())
+    {
+        l_result = it->second;
+    }
+
+    return l_result;
+}
+
 }
\ No newline at end of file
diff --git a/src/usr/runtime/populate_hbruntime.C b/src/usr/runtime/populate_hbruntime.C
index 199b55ecb..eb7a5a7b5 100644
--- a/src/usr/runtime/populate_hbruntime.C
+++ b/src/usr/runtime/populate_hbruntime.C
@@ -76,7 +76,6 @@
 #include <pnor/pnor_reasoncodes.H>
 #include <runtime/common/runtime_utils.H>
 
-
 namespace RUNTIME
 {
 
@@ -557,7 +556,8 @@ errlHndl_t fill_RsvMem_hbData(uint64_t & io_start_address,
     return l_elog;
 }
 
-errlHndl_t hbResvLoadSecureSection (const PNOR::SectionId i_sec)
+errlHndl_t hbResvLoadSecureSection (const PNOR::SectionId i_sec,
+                                    bool i_verified)
 {
     TRACFCOMP( g_trac_runtime,ENTER_MRK"hbResvloadSecureSection() sec %s",
               PNOR::SectionIdToString(i_sec));
@@ -603,16 +603,18 @@ errlHndl_t hbResvLoadSecureSection (const PNOR::SectionId i_sec)
         auto l_pnorVaddr = l_info.vaddr;
         auto l_imgSize = l_info.size;
 
-    // If section is signed, only the protected size was loaded into memory
-#ifdef CONFIG_SECUREBOOT
-        if (l_info.secure)
+        // If section is signed, only the protected size was loaded into memory
+        if (i_verified)
         {
+#ifdef CONFIG_SECUREBOOT
             l_imgSize = l_info.secureProtectedPayloadSize;
             // Include secure header
             l_pnorVaddr -= PAGESIZE;
+#endif
+            // Add size for secure header.
+            // NOTE: if SB compiled out, a header will be injected later
             l_imgSize += PAGESIZE;
         }
-#endif
 
         // Load Pnor section into HB reserved memory
         l_elog = PreVerifiedLidMgr::loadFromPnor(i_sec, l_pnorVaddr, l_imgSize);
@@ -1070,7 +1072,7 @@ errlHndl_t populate_HbRsvMem(uint64_t i_nodeId)
         // Handle all Pre verified PNOR sections
         for (const auto & secIdPair : preVerifiedPnorSections)
         {
-            l_elog = hbResvLoadSecureSection(secIdPair.first);
+            l_elog = hbResvLoadSecureSection(secIdPair.first, secIdPair.second);
             if (l_elog)
             {
                 break;
diff --git a/src/usr/runtime/preverifiedlidmgr.C b/src/usr/runtime/preverifiedlidmgr.C
index 16cf15c10..62671860f 100644
--- a/src/usr/runtime/preverifiedlidmgr.C
+++ b/src/usr/runtime/preverifiedlidmgr.C
@@ -36,6 +36,8 @@
 #include <arch/ppc.H>
 #include <targeting/common/target.H>
 #include <targeting/common/attributes.H>
+#include <secureboot/containerheader.H>
+#include <runtime/common/runtime_utils.H>
 
 extern trace_desc_t *g_trac_runtime;
 
@@ -47,6 +49,8 @@ PreVerifiedLidMgr::ResvMemInfo PreVerifiedLidMgr::cv_resvMemInfo {};
 PreVerifiedLidMgr::ResvMemInfo PreVerifiedLidMgr::cv_phypResvMemInfo {};
 mutex_t PreVerifiedLidMgr::cv_mutex = MUTEX_INITIALIZER;
 mutex_t PreVerifiedLidMgr::cv_loadImageMutex = MUTEX_INITIALIZER;
+bool PreVerifiedLidMgr::cv_addFakeHdrs = false;
+PNOR::SectionId PreVerifiedLidMgr::cv_curPnorSecId = PNOR::INVALID_SECTION;
 
 /********************
  Public Methods
@@ -156,6 +160,27 @@ errlHndl_t PreVerifiedLidMgr::_loadFromPnor(const PNOR::SectionId i_sec,
     TRACFCOMP(g_trac_runtime, ENTER_MRK"PreVerifiedLidMgr::_loadFromPnor - sec %s",
               PNOR::SectionIdToString(i_sec));
 
+#ifdef CONFIG_SECUREBOOT
+    // If SB compiled in, only add fake secure header if the section is never
+    // signed. e.g. RINGOVD section
+    // Otherwise always add fake secure header when SB compiled out
+    if (!RUNTIME::isPreVerifiedSectionSecure(i_sec))
+    {
+#endif
+        // Check if Header is mising
+        if (!PNOR::cmpSecurebootMagicNumber(
+                reinterpret_cast<uint8_t*>(i_addr)))
+        {
+            TRACFCOMP(g_trac_runtime, "PreVerifiedLidMgr::_loadFromPnor adding fake header to %s",
+                      PNOR::SectionIdToString(i_sec));
+            // Add fake headers to pnor loads
+            cv_addFakeHdrs = true;
+            cv_curPnorSecId = i_sec;
+        }
+#ifdef CONFIG_SECUREBOOT
+    }
+#endif
+
     errlHndl_t l_errl = nullptr;
 
     do {
@@ -206,10 +231,11 @@ errlHndl_t PreVerifiedLidMgr::_loadFromPnor(const PNOR::SectionId i_sec,
         {
             char l_lidStr[Util::lidIdStrLength] {};
             snprintf (l_lidStr, Util::lidIdStrLength, "%08X",l_lids.lid);
+            assert(i_size > PAGE_SIZE, "PreVerifiedLidMgr::_loadFromPnor - caller did not include size of header for total size");
             l_errl = RUNTIME::setNextHbRsvMemEntry(HDAT::RHB_TYPE_VERIFIED_LIDS,
                                                    cv_pResvMemInfo->rangeId,
                                                    cv_pResvMemInfo->curAddr+PAGE_SIZE,
-                                                   i_size,
+                                                   i_size-PAGE_SIZE,
                                                    l_lidStr);
             if(l_errl)
             {
@@ -253,6 +279,10 @@ errlHndl_t PreVerifiedLidMgr::_loadFromPnor(const PNOR::SectionId i_sec,
 
     } while(0);
 
+    // Force fake header bool to be false and clear cur PNOR section id
+    cv_addFakeHdrs = false;
+    cv_curPnorSecId = PNOR::INVALID_SECTION;
+
     TRACFCOMP( g_trac_runtime, EXIT_MRK"PreVerifiedLidMgr::_loadFromPnor");
 
     mutex_unlock(&cv_loadImageMutex);
@@ -271,6 +301,9 @@ errlHndl_t PreVerifiedLidMgr::_loadFromMCL(const uint32_t i_lidId,
     TRACFCOMP(g_trac_runtime, ENTER_MRK"PreVerifiedLidMgr::_loadFromMCL lid = 0x%X",
               i_lidId);
 
+    // Force fake header bool to be false in MCL path
+    cv_addFakeHdrs = false;
+
     errlHndl_t l_errl = nullptr;
 
     // Switch to Different Memory Info for PHYP component
@@ -396,11 +429,32 @@ errlHndl_t PreVerifiedLidMgr::loadImage(const uint64_t i_imgAddr,
     TRACDCOMP(g_trac_runtime, "PreVerifiedLidMgr::loadImage - curAddr 0x%X, size 0x%X, vaddr 0x%X",
               cv_pResvMemInfo->curAddr, i_imgSize, l_tmpVaddr);
 
-    // Include Header page from pnor image.
-    // NOTE: Do not use aligned size for memcpy
-    memcpy(reinterpret_cast<void*>(l_tmpVaddr),
-           reinterpret_cast<void*>(i_imgAddr),
-           i_imgSize);
+    // Inject a fake header when loading from PNOR and secureboot is compiled
+    // out.
+    if(cv_addFakeHdrs)
+    {
+        TRACDCOMP(g_trac_runtime, "PreVerifiedLidMgr::loadImage fake header load");
+        SECUREBOOT::ContainerHeader l_fakeHdr(i_imgSize,
+                                            SectionIdToString(cv_curPnorSecId));
+        // Inject Fake header into reserved memory
+        memcpy(reinterpret_cast<void*>(l_tmpVaddr),
+               l_fakeHdr.fakeHeader(),
+               PAGE_SIZE);
+        // Include rest of image after header
+        // NOTE: Do not use aligned size for memcpy
+        assert(i_imgSize > PAGE_SIZE, "PreVerifiedLidMgr::loadImage - caller did not include size of header for total size");
+        memcpy(reinterpret_cast<void*>(l_tmpVaddr+PAGE_SIZE),
+               reinterpret_cast<void*>(i_imgAddr),
+               i_imgSize-PAGE_SIZE);
+    }
+    else
+    {
+        TRACDCOMP(g_trac_runtime, "PreVerifiedLidMgr::loadImage default load");
+        // NOTE: Do not use aligned size for memcpy
+        memcpy(reinterpret_cast<void*>(l_tmpVaddr),
+               reinterpret_cast<void*>(i_imgAddr),
+               i_imgSize);
+    }
 
     l_errl = RUNTIME::unmapVirtAddr(l_tmpVaddr);
     if(l_errl)
diff --git a/src/usr/runtime/test/testpreverifiedlidmgr.H b/src/usr/runtime/test/testpreverifiedlidmgr.H
index 3863ed27e..26879574e 100644
--- a/src/usr/runtime/test/testpreverifiedlidmgr.H
+++ b/src/usr/runtime/test/testpreverifiedlidmgr.H
@@ -83,7 +83,8 @@ class PreVerifiedLidMgrTest : public CxxTest::TestSuite
         // Handle all Pre verified PNOR sections
         for (const auto & secIdPair : RUNTIME::preVerifiedPnorSections)
         {
-            l_errl = RUNTIME::hbResvLoadSecureSection(secIdPair.first);
+            l_errl = RUNTIME::hbResvLoadSecureSection(secIdPair.first,
+                                                      secIdPair.second);
             if (l_errl)
             {
                 errlCommit(l_errl, RUNTIME_COMP_ID);
@@ -100,16 +101,20 @@ class PreVerifiedLidMgrTest : public CxxTest::TestSuite
         // Each section has 2 lids each (Header, Content) except the RINGOVD
         // section. It only has 1 or is inhibited in secure mode
         size_t l_numSections = RUNTIME::preVerifiedPnorSections.size();
-        size_t l_expectedLids = (2 * l_numSections) - 1;
+        // See utillidpnor.C for more info on num of lids
+        size_t l_expectedLids = (2 * l_numSections);
         if (SECUREBOOT::enabled())
         {
-            l_expectedLids--;
+            // RINGOVD not permitted in secure mode
+            l_expectedLids -= 2;
         }
 
         // Ensure the expected number of lids were loaded.
         if (l_preVerLidMgr.cv_lidsLoaded.size() != l_expectedLids)
         {
-            TS_FAIL("testLoadFromPnor> Num of lids loaded not correct");
+            TS_FAIL("testLoadFromPnor> Num of lids loaded not correct %d expected %d",
+                    l_preVerLidMgr.cv_lidsLoaded.size(),
+                    l_expectedLids);
             break;
         }
 
diff --git a/src/usr/secureboot/base/test/securerommgrtest.H b/src/usr/secureboot/base/test/securerommgrtest.H
index 4a445b52f..1355cc75d 100644
--- a/src/usr/secureboot/base/test/securerommgrtest.H
+++ b/src/usr/secureboot/base/test/securerommgrtest.H
@@ -45,6 +45,7 @@
 #include "../../../vfs/vfsrp.H"
 #include <sys/vfs.h>
 #include <kernel/console.H>
+#include <pnor/pnorif.H>
 
 // Quick change for unit testing
 //#define TRACUCOMP(args...)  TRACFCOMP(args)
@@ -525,6 +526,40 @@ class SecureRomManagerTest : public CxxTest::TestSuite
             unloadSignedFile( signedFile_pageAddr, signedFile_size);
         }
     }
+
+    void test_fakeHeader(void)
+    {
+        TRACFCOMP(g_trac_secure,"SecureRomManagerTest::test_fakeHeader");
+
+        const size_t l_totalContainerSize = 0x10000;
+        // Purposely make a comp id larger than SW_HDR_COMP_ID_SIZE_BYTES
+        // otherwise strncmp below needs a different size
+        const char* l_compId = "FAKEHEADERTEST";
+
+        // Simple call constructor to create fake header and make sure it
+        // does not cause an error
+        SECUREBOOT::ContainerHeader l_fakeHdr(l_totalContainerSize, l_compId);
+
+        // Check if Header is mising
+        if (!PNOR::cmpSecurebootMagicNumber(l_fakeHdr.fakeHeader()))
+        {
+            TS_FAIL("SecureRomManagerTest::test_fakeHeader: missing magic number");
+        }
+
+        // Payload Text Size should be the total container size minus the header
+        if(l_fakeHdr.payloadTextSize() != (l_totalContainerSize - PAGE_SIZE))
+        {
+            TS_FAIL("SecureRomManagerTest::test_fakeHeader: payload text size was not parsed correctly");
+        }
+
+        // Ensure the parsed component ID matches what was passed in through
+        // SW_HDR_COMP_ID_SIZE_BYTES
+        if(strncmp(l_fakeHdr.componentId(), l_compId,
+                   SW_HDR_COMP_ID_SIZE_BYTES) != 0)
+        {
+            TS_FAIL("SecureRomManagerTest::test_fakeHeader: component ID was not parsed correctly");
+        }
+    }
 };
 
 /**********************************************************************/
diff --git a/src/usr/secureboot/common/containerheader.C b/src/usr/secureboot/common/containerheader.C
index 069a587d9..dd43551d2 100644
--- a/src/usr/secureboot/common/containerheader.C
+++ b/src/usr/secureboot/common/containerheader.C
@@ -34,7 +34,7 @@ namespace SECUREBOOT
 
 void ContainerHeader::parse_header(const void* i_header)
 {
-    assert(i_header != NULL);
+    assert(i_header != nullptr);
     const uint8_t* l_hdr = reinterpret_cast<const uint8_t*>(i_header);
 
     /*---- Parse ROM_container_raw ----*/
@@ -94,6 +94,68 @@ void ContainerHeader::parse_header(const void* i_header)
     print();
 }
 
+void ContainerHeader::initVars()
+{
+    memset(&iv_headerInfo, 0x00, sizeof(iv_headerInfo));
+    memset(iv_hwKeyHash, 0, sizeof(SHA512_t));
+    memset(iv_componentId,0x00,sizeof(iv_componentId));
+}
+
+void ContainerHeader::genFakeHeader(const size_t i_totalSize,
+                                    const char* const i_compId)
+{
+    SecureHeaderInfo info {};
+    assert(iv_fakeHeader.data() != nullptr, "Internal fake header buffer nullptr");
+
+    uint8_t* l_hdr = reinterpret_cast<uint8_t*>(iv_fakeHeader.data());
+
+     /*---- ROM_container_raw ----*/
+    info.hw_hdr.magic_number = ROM_MAGIC_NUMBER;
+    info.hw_hdr.version = CONTAINER_VERSION;
+    info.hw_hdr.container_size = i_totalSize;
+    // The rom code has a placeholder for the prefix in the first struct so
+    // skip it
+    size_t l_size = offsetof(ROM_container_raw, prefix);
+    memcpy(l_hdr, &info.hw_hdr, l_size);
+    l_hdr += l_size;
+
+    /*---- ROM_prefix_header_raw ----*/
+    info.hw_prefix_hdr.ver_alg.version = HEADER_VERSION;
+    info.hw_prefix_hdr.ver_alg.hash_alg = HASH_ALG_SHA512;
+    info.hw_prefix_hdr.ver_alg.sig_alg = SIG_ALG_ECDSA521;
+    info.hw_prefix_hdr.sw_key_count = 1;
+    info.hw_prefix_hdr.payload_size = sizeof(ecc_key_t);
+
+    l_size = offsetof(ROM_prefix_header_raw, ecid);
+    l_size += info.hw_prefix_hdr.ecid_count * ECID_SIZE;
+    memcpy(l_hdr, &info.hw_prefix_hdr, l_size);
+    l_hdr += l_size;
+
+    /*---- Parse ROM_prefix_data_raw ----*/
+    // Skip over variable number of sw keys as they are already zeroed out
+    l_size = offsetof(ROM_prefix_data_raw, sw_pkey_p);
+    l_size += info.hw_prefix_hdr.sw_key_count * sizeof(ecc_key_t);
+    l_hdr += l_size;
+
+    /*---- ROM_sw_header_raw ----*/
+    info.sw_hdr.ver_alg.version = 1;
+    strncpy(info.sw_hdr.component_id, i_compId,SW_HDR_COMP_ID_SIZE_BYTES);
+    info.sw_hdr.ver_alg.hash_alg = HASH_ALG_SHA512;
+    info.sw_hdr.ver_alg.sig_alg = SIG_ALG_ECDSA521;
+    info.sw_hdr.payload_size = i_totalSize - PAGE_SIZE;
+
+    l_size = offsetof(ROM_sw_header_raw, ecid);
+    l_size += info.hw_prefix_hdr.ecid_count * ECID_SIZE;
+    memcpy(l_hdr, &info.sw_hdr, l_size);
+    l_hdr += l_size;
+
+    /*---- Parse ROM_sw_sig_raw ----*/
+    // No-op already zeroed out
+
+    iv_pHdrStart = reinterpret_cast<const uint8_t*>(iv_fakeHeader.data());
+    parse_header(iv_fakeHeader.data());
+}
+
 void ContainerHeader::print() const
 {
 #ifdef HOSTBOOT_DEBUG
@@ -218,9 +280,9 @@ void ContainerHeader::validate()
 void ContainerHeader::safeMemCpyAndInc(void* i_dest, const uint8_t* &io_hdr,
                                        const size_t i_size)
 {
-    assert(i_dest != NULL, "ContainerHeader: dest ptr NULL");
-    assert(io_hdr != NULL, "ContainerHeader: current header location ptr NULL");
-    assert(iv_pHdrStart != NULL, "ContainerHeader: start of header ptr NULL");
+    assert(i_dest != nullptr, "ContainerHeader: dest ptr NULL");
+    assert(io_hdr != nullptr, "ContainerHeader: current header location ptr NULL");
+    assert(iv_pHdrStart != nullptr, "ContainerHeader: start of header ptr NULL");
 
     TRACDCOMP(g_trac_secure,"dest: 0x%X src: 0x%X size: 0x%X",i_dest, io_hdr, i_size);
 
@@ -265,4 +327,10 @@ void ContainerHeader::genHwKeyHash()
 }
 #endif
 
+const uint8_t* ContainerHeader::fakeHeader() const
+{
+    assert(iv_fakeHeader.data() != nullptr, "Fake header should not be nullptr");
+    return iv_fakeHeader.data();
+}
+
 }; //end of SECUREBOOT namespace
diff --git a/src/usr/secureboot/runtime/test/testsecureboot_rt.H b/src/usr/secureboot/runtime/test/testsecureboot_rt.H
index 33ca4cd48..f728357e2 100644
--- a/src/usr/secureboot/runtime/test/testsecureboot_rt.H
+++ b/src/usr/secureboot/runtime/test/testsecureboot_rt.H
@@ -44,6 +44,7 @@
 #include <secureboot/settings.H>
 #include <pnor/pnorif.H>
 #include <pnor/pnor_reasoncodes.H>
+#include "../../../pnor/pnor_utils.H"
 
 class SecurebootRtTestSuite: public CxxTest::TestSuite
 {
@@ -163,70 +164,62 @@ class SecurebootRtTestSuite: public CxxTest::TestSuite
     /**
      * @brief Helper to test case that runs getSectionInfo scenarios and checks
      *        for desired results.
+     *        If secure, should throw an error
+     *        Otherwise no error
      * @param[in] i_id, Pnor Section ID
-     * @param[in] i_secure, Indicates if section is expected to be secure or not
      *
      * @return N/A
      */
-    void runAccessSecurePnorTest(PNOR::SectionId i_id, bool i_secure)
+    void runAccessSecurePnorTest(PNOR::SectionId i_id)
     {
         errlHndl_t l_errl = nullptr;
+        bool l_secure = PNOR::isEnforcedSecureSection(i_id);
         PNOR::SectionInfo_t l_info;
 
-        l_errl = PNOR::getSectionInfo(i_id, l_info);
-        if(l_errl)
-        {
-            TS_FAIL("testAccessSecurePnorSection: Failed for section %s",
-                        PNOR::SectionIdToString(i_id));
-            errlCommit(l_errl, SECURE_COMP_ID);
-        }
+        SB_ENTER("runAccessSecurePnorTest %s", PNOR::SectionIdToString(i_id));
 
-        // TODO: RTC:180063 change this test case back to how it was before
-        //                  having secure sections return vaddr = 0
-        //                  previously in HB commit cefc4c
-        // If we expect the section to be secure, make sure it returns secure
-        // and a vaddr of 0
-        if (i_secure)
+        l_errl = PNOR::getSectionInfo(i_id, l_info);
+        if(l_secure)
         {
-            if (l_info.secure != 1)
+            SB_INF("runAccessSecurePnorTest is secure");
+            if (l_errl &&
+                (l_errl->reasonCode() == PNOR::RC_RTPNOR_INVALID_SECTION))
             {
-                TS_FAIL("testAccessSecurePnorSection: Did not return %s as a secure section",
-                            PNOR::SectionIdToString(i_id));
+                SB_INF("runAccessSecurePnorTest caught correct error");
+                delete l_errl;
+                l_errl = nullptr;
             }
-            else if (l_info.vaddr != 0)
+            else
             {
-                TS_FAIL("testAccessSecurePnorSection: Did not return a vaddr of 0 for secure section %s",
-                            PNOR::SectionIdToString(i_id));
+                TS_FAIL("testAccessSecurePnorSection: unexpected reason code for Secure Section %s. Expected RC 0x%.4X Actual RC 0x%.4X",
+                        PNOR::SectionIdToString(i_id),
+                        PNOR::RC_RTPNOR_INVALID_SECTION,
+                        l_errl->reasonCode());
+                errlCommit(l_errl, SECURE_COMP_ID);
             }
         }
-        // If we expect the section to be secure, make sure it returns secure
-        // and a vaddr of 0
-        else
+        else if(l_errl)
         {
-            if (l_info.vaddr == 0)
-            {
-                TS_FAIL("testAccessSecurePnorSection: Did not return a vaddr of non-zero for a non-secure section %s",
-                            PNOR::SectionIdToString(i_id));
-            }
+            TS_FAIL("testAccessSecurePnorSection: getSectionInfo failed for section %s",
+                        PNOR::SectionIdToString(i_id));
+            errlCommit(l_errl, SECURE_COMP_ID);
         }
+
+        SB_EXIT("runAccessSecurePnorTest");
     }
 
-    // TODO: RTC:180063 change this test case back to how it was before
-    //                  having secure sections return vaddr = 0 previously
-    //                  in HB commit cefc4c
     void testAccessSecurePnorSection()
     {
+#ifdef CONFIG_SECUREBOOT
         SB_ENTER("testAccessSecurePnorSection");
 
+        // Should thow an error for trying to read a secure section
+        runAccessSecurePnorTest(PNOR::OCC);
+        runAccessSecurePnorTest(PNOR::HB_EXT_CODE);
 
-        // Ensure we get a vaddr of 0 at Runtime
-        runAccessSecurePnorTest(PNOR::OCC, true);
-        runAccessSecurePnorTest(PNOR::HB_EXT_CODE, true);
-
-        // Ensure we get a vaddr of  at Runtime
-        runAccessSecurePnorTest(PNOR::TEST, false);
-
-        SB_EXIT("testAccessSecurePnorSection");
+        // No error for trying to read a secure section
+        runAccessSecurePnorTest(PNOR::TEST);
+#endif
     }
 
 
diff --git a/src/usr/util/runtime/utillidmgr_rt.C b/src/usr/util/runtime/utillidmgr_rt.C
index 52490cac1..0e45de3f0 100644
--- a/src/usr/util/runtime/utillidmgr_rt.C
+++ b/src/usr/util/runtime/utillidmgr_rt.C
@@ -35,6 +35,7 @@
 #include <trace/interface.H>
 #include "../utilbase.H"
 #include <util/utillidpnor.H>
+#include <pnor/pnor_reasoncodes.H>
 
 UtilLidMgr::UtilLidMgr(uint32_t i_lidId) :
     iv_isLidInPnor(false), iv_lidBuffer(nullptr), iv_lidSize(0),
@@ -142,13 +143,14 @@ errlHndl_t UtilLidMgr::loadLid()
     {
         if(iv_isLidInHbResvMem)
         {
+            const auto pnorSectionId = Util::getLidPnorSection(
+                static_cast<Util::LidId>(iv_lidId));
+
             UTIL_FT("UtilLidMgr::loadLid> iv_isLidInHbResvMem=true");
             iv_lidBuffer = reinterpret_cast<void*>(g_hostInterfaces->
                 get_reserved_mem(
-                   PNOR::SectionIdToString(
-                      Util::getLidPnorSection(
-                          static_cast<Util::LidId>(iv_lidId))),
-                                 0));
+                   PNOR::SectionIdToString(pnorSectionId),
+                   0));
 
             // If nullptr returned, set size to 0 to indicate we could not find
             // the lid in HB resv memory
@@ -160,30 +162,48 @@ errlHndl_t UtilLidMgr::loadLid()
             else
             {
                 UTIL_FT("UtilLidMgr::loadLid - resv mem section found");
-                // If section is secure, adjust size and buffer pointer
-                // TODO: RTC:180063 if getSectionInfo is modified to not support
-                //                  secure sections, then need a different
-                //                  method.
-                if(iv_lidPnorInfo.secure)
-                {
-                    UTIL_FT("UtilLidMgr::loadLid - resv mem section is secure");
-                    // Build a container header object to parse protected size
-                    SECUREBOOT::ContainerHeader l_conHdr(iv_lidBuffer);
-                    iv_lidSize = l_conHdr.payloadTextSize();
-
-                    // Increment by page size to not expose secure header
-                    iv_lidBuffer = static_cast<uint8_t*>(iv_lidBuffer) +
-                                   PAGESIZE;
-                }
-                else
+
+                // Ensure Section has a Secure Header
+                if (!PNOR::cmpSecurebootMagicNumber(
+                        reinterpret_cast<uint8_t*>(iv_lidBuffer)))
                 {
-                    // If no secure header, just use PNOR size
-                    // NOTE: Unsigned sections with sha512Version have already
-                    //       skipped over the header and decreased size while
-                    //       parsing the PNOR TOC
-                    iv_lidSize = iv_lidPnorInfo.size;
-                    UTIL_FT("UtilLidMgr::loadLid - iv_lidSize=%d", iv_lidSize );
+                    UTIL_FT(ERR_MRK"UtilLidMgr::loadLid: currently don't support "
+                        "a reserved memory area without a secure header. Section = %s",
+                        PNOR::SectionIdToString(pnorSectionId));
+
+                    uint64_t l_actualBytes = 0;
+                    memcpy(&l_actualBytes,
+                           iv_lidBuffer,
+                           sizeof(ROM_MAGIC_NUMBER));
+
+                    /*@
+                     * @errortype       ERRL_SEV_INFORMATIONAL
+                     * @moduleid        Util::UTIL_LIDMGR_RT
+                     * @reasoncode      PNOR::RC_BAD_SECURE_MAGIC_NUM
+                     * @userdata1       Section attempting to be loaded
+                     * @userdata2       First 4 bytes of vaddr
+                     * @devdesc         Error loading lid from reserved memory without secure header
+                     * @custdesc        Firmware Error
+                     */
+                    l_errl = new ERRORLOG::ErrlEntry(
+                        ERRORLOG::ERRL_SEV_INFORMATIONAL,
+                        Util::UTIL_LIDMGR_RT,
+                        PNOR::RC_BAD_SECURE_MAGIC_NUM,
+                        pnorSectionId,
+                        l_actualBytes,
+                        true/*SW Error*/);
+                    break;
                 }
+
+                UTIL_FT("UtilLidMgr::loadLid - resv mem section has secure header");
+
+                // Build a container header object to parse protected size
+                SECUREBOOT::ContainerHeader l_conHdr(iv_lidBuffer);
+                iv_lidSize = l_conHdr.payloadTextSize();
+
+                // Increment by page size to not expose secure header
+                iv_lidBuffer = static_cast<uint8_t*>(iv_lidBuffer) +
+                               PAGESIZE;
             }
         }
         else if(iv_isLidInVFS)
diff --git a/src/usr/util/utillidpnor.C b/src/usr/util/utillidpnor.C
index de69964a2..25c90c73e 100644
--- a/src/usr/util/utillidpnor.C
+++ b/src/usr/util/utillidpnor.C
@@ -26,15 +26,15 @@
 #include <util/utillidmgr.H>
 #include <util/utillidpnor.H>
 #include <config.h>
-#ifdef CONFIG_SECUREBOOT
 #include <pnor/pnorif.H>
 #include <errl/errlmanager.H>
-#endif
 
 #include <utility>
 #include <map>
 #include <trace/interface.H>
 #include "utilbase.H"
+#include <initservice/initserviceif.H>
+#include <pnor/pnor_reasoncodes.H>
 
 namespace Util
 {
@@ -100,7 +100,7 @@ PNOR::SectionId getLidPnorSection(const LidId i_lid)
 bool UtilLidMgr::getLidPnorSectionInfo(uint32_t i_lidId,
                                        PNOR::SectionInfo_t &o_lidPnorInfo)
 {
-    errlHndl_t l_err = NULL;
+    errlHndl_t l_err = nullptr;
     bool l_lidInPnor = false;
 
     // Search if a lid id maps to pnor section
@@ -110,30 +110,58 @@ bool UtilLidMgr::getLidPnorSectionInfo(uint32_t i_lidId,
     // LidToPnor will return INVALID_SECITON if no mapping found
     if (l_secId == PNOR::INVALID_SECTION)
     {
-        UTIL_FT("UtilLidMgr::getLidPnorSection lid 0x%X not in PNOR", i_lidId);
+        UTIL_FT("UtilLidMgr::getLidPnorSectionInfo lid 0x%X not in PNOR", i_lidId);
         o_lidPnorInfo.id = PNOR::INVALID_SECTION;
     }
     // A mapping was found
     else
     {
+        // PNOR section is optional or lid is not in PNOR, so just delete error
+        // During IPL
+        //    PNOR section may be optional
+        // In Runtime
+        //    FSP - prohibit access to PNOR
+        //    OP - PNOR access of pre-verifed HB reserved memory sections not allowed.
+#ifdef __HOSTBOOT_RUNTIME
+        // Do not allow PNOR access at runtime on FSP based machines
+        if(INITSERVICE::spBaseServicesEnabled())
+        {
+            break;
+        }
+#endif
         l_err = PNOR::getSectionInfo(l_secId, o_lidPnorInfo);
-        // Section is optional or lid is not in PNOR, so just delete error
-        if (l_err)
+        if (l_err &&
+#ifdef __HOSTBOOT_RUNTIME
+            (l_err->reasonCode() == PNOR::RC_RTPNOR_INVALID_SECTION)
+#else
+            (l_err->reasonCode() == PNOR::RC_INVALID_SECTION)
+#endif
+           )
         {
             o_lidPnorInfo.id = PNOR::INVALID_SECTION;
             delete l_err;
-            l_err = NULL;
+            l_err = nullptr;
+            UTIL_FT("UtilLidMgr::getLidPnorSectionInfo Lid 0x%X ignore getSectionInfo error",
+                    i_lidId);
+            break;
+        }
+        else if (l_err)
+        {
+            UTIL_FT(ERR_MRK"UtilLidMgr::getLidPnorSectionInfo Lid 0x%X getSectionInfo error shutting down rc=0x%08X",
+                    l_err->reasonCode());
+            errlCommit(l_err, UTIL_COMP_ID);
+            break;
         }
         else
         {
             l_lidInPnor = true;
-            UTIL_FT("UtilLidMgr::getLidPnorSection Lid 0x%X in PNOR", i_lidId);
+            UTIL_FT("UtilLidMgr::getLidPnorSectionInfo Lid 0x%X in PNOR", i_lidId);
 #ifdef CONFIG_SECUREBOOT
 #ifndef __HOSTBOOT_RUNTIME
             // The lid could be securely signed in PNOR
             if(o_lidPnorInfo.secure)
             {
-                UTIL_FT("UtilLidMgr::getLidPnorSection verify Lid in PNOR");
+                UTIL_FT("UtilLidMgr::getLidPnorSectionInfo verify Lid in PNOR");
 
                 // Load the secure section
                 l_err = loadSecureSection(l_secId);
@@ -157,18 +185,6 @@ bool UtilLidMgr::getLidPnorSectionInfo(uint32_t i_lidId,
             }
 #endif
 #endif
-
-#ifdef __HOSTBOOT_RUNTIME
-            //use this check for HBRT due to secure lid load setting vaddr
-            //to zero -- which causes isSectionEmpty to segfault
-            if( !o_lidPnorInfo.vaddr )
-            {
-                UTIL_FT("UtilLidMgr::getLidPnorSection PNOR section %s is empty or secure",
-                        PNOR::SectionIdToString(l_secId));
-                l_lidInPnor = false;
-                break;
-            }
-#endif
         }
     }
     } while(0);