Handle comments from pre-verify and runtime lid loading commits

Change-Id: I224079808493c062f04b7c3a59d45128a8f2e699
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/49875
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Reviewed-by: Michael Baiocchi <mbaiocch@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>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>

7 files changed, 38 insertions, 20 deletions

diff --git a/src/include/usr/runtime/common/runtime_utils.H b/src/include/usr/runtime/common/runtime_utils.H
index c7f318bc1..afb88bd47 100644
--- a/src/include/usr/runtime/common/runtime_utils.H
+++ b/src/include/usr/runtime/common/runtime_utils.H
@@ -29,7 +29,10 @@
 namespace RUNTIME
 {
 
+// Pair indicating if a PNOR section is expected to have a secure header
+// regardless of the compiler options
 typedef std::pair<PNOR::SectionId, bool> PreVerifyPair;
+// A vector of all PNOR sections that are pre-verified for runtime
 typedef std::vector<PreVerifyPair> PreVerifyVector;
 
 // PNOR sections that will be pre-verified and loaded into HB reserved memory
diff --git a/src/include/usr/runtime/populate_hbruntime.H b/src/include/usr/runtime/populate_hbruntime.H
index 9f0466172..c2c0c224e 100644
--- a/src/include/usr/runtime/populate_hbruntime.H
+++ b/src/include/usr/runtime/populate_hbruntime.H
@@ -70,13 +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
+ *  @param[in] i_secHdrExpected - 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,
-                                    const bool i_verified);
+                                    const bool i_secHdrExpected);
 
 } // End of Namespace
\ No newline at end of file
diff --git a/src/include/usr/secureboot/containerheader.H b/src/include/usr/secureboot/containerheader.H
index f7e924d9b..b5edfb325 100644
--- a/src/include/usr/secureboot/containerheader.H
+++ b/src/include/usr/secureboot/containerheader.H
@@ -63,7 +63,7 @@ class ContainerHeader
         };
 
         /**
-         * @brief ContainerHeader
+         * @brief ContainerHeader - generate fake header
          *
          * This constructor generates a fake header with minimal information
          *
diff --git a/src/usr/runtime/common/runtime_utils.C b/src/usr/runtime/common/runtime_utils.C
index 7b900389f..d87847f85 100644
--- a/src/usr/runtime/common/runtime_utils.C
+++ b/src/usr/runtime/common/runtime_utils.C
@@ -27,11 +27,11 @@
 namespace RUNTIME
 {
 
-// -- Verified Images
+// -- Images expected to have secure headers
 //   -- OCC
 //   -- WOFDATA
 //   -- HCODE
-// -- Non-verified Images
+// -- Images that never have secure headers
 ///  -- RINGOVD
 const PreVerifyVector preVerifiedPnorSections {
     {PNOR::OCC, true},
diff --git a/src/usr/runtime/populate_hbruntime.C b/src/usr/runtime/populate_hbruntime.C
index eb7a5a7b5..2c97ac2f7 100644
--- a/src/usr/runtime/populate_hbruntime.C
+++ b/src/usr/runtime/populate_hbruntime.C
@@ -557,7 +557,7 @@ errlHndl_t fill_RsvMem_hbData(uint64_t & io_start_address,
 }
 
 errlHndl_t hbResvLoadSecureSection (const PNOR::SectionId i_sec,
-                                    bool i_verified)
+                                    const bool i_secHdrExpected)
 {
     TRACFCOMP( g_trac_runtime,ENTER_MRK"hbResvloadSecureSection() sec %s",
               PNOR::SectionIdToString(i_sec));
@@ -603,16 +603,21 @@ 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
-        if (i_verified)
+        // Check if the section is expected to have a secure header regardless
+        // of compile options
+        if (i_secHdrExpected)
         {
 #ifdef CONFIG_SECUREBOOT
+            // If section is signed, only the protected size was loaded into memory
             l_imgSize = l_info.secureProtectedPayloadSize;
             // Include secure header
+            // NOTE: we do not preserve the header in virtual memory when SB
+            // is compiled out. So "-PAGESIZE" only works when SB is compiled in
             l_pnorVaddr -= PAGESIZE;
 #endif
             // Add size for secure header.
-            // NOTE: if SB compiled out, a header will be injected later
+            // NOTE: if SB compiled out, a header will be injected later so
+            // preserve space for the header.
             l_imgSize += PAGESIZE;
         }
 
diff --git a/src/usr/runtime/test/testpreverifiedlidmgr.H b/src/usr/runtime/test/testpreverifiedlidmgr.H
index 26879574e..47ba6c61b 100644
--- a/src/usr/runtime/test/testpreverifiedlidmgr.H
+++ b/src/usr/runtime/test/testpreverifiedlidmgr.H
@@ -98,14 +98,17 @@ class PreVerifiedLidMgrTest : public CxxTest::TestSuite
             break;
         }
 
-        // Each section has 2 lids each (Header, Content) except the RINGOVD
-        // section. It only has 1 or is inhibited in secure mode
+        // Each section has 2 lids each (Header, Content)
+        // Note: even the RINGOVD section adds a Header element, although it is
+        //       INVALID_LID
+        // See runtime_utils.C for full list of PNOR sections and utillidpnor.C
+        //     for the mappings (PnorToLidsMap)
         size_t l_numSections = RUNTIME::preVerifiedPnorSections.size();
-        // See utillidpnor.C for more info on num of lids
         size_t l_expectedLids = (2 * l_numSections);
         if (SECUREBOOT::enabled())
         {
-            // RINGOVD not permitted in secure mode
+            // RINGOVD not permitted in secure mode. Meaning the Header and
+            // Content lid will be missing.
             l_expectedLids -= 2;
         }
 
diff --git a/src/usr/secureboot/runtime/test/testsecureboot_rt.H b/src/usr/secureboot/runtime/test/testsecureboot_rt.H
index f728357e2..380b9eb0c 100644
--- a/src/usr/secureboot/runtime/test/testsecureboot_rt.H
+++ b/src/usr/secureboot/runtime/test/testsecureboot_rt.H
@@ -191,11 +191,18 @@ class SecurebootRtTestSuite: public CxxTest::TestSuite
             }
             else
             {
-                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 (l_errl)
+                {
+                    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);
+                }
+                else
+                {
+                    TS_FAIL("testAccessSecurePnorSection: no error found when one was expected");
+                }
             }
         }
         else if(l_errl)