Re: [PATCH] BaseTools/GenFv: Support SecCore and PeiCore in different FV

["Feng, Bob C" <bob.c.feng@intel.com>]

This patch looks good to me.

Reviewed-by: Bob Feng <bob.c.feng@intel.com>


-----Original Message-----
From: Chiu, Chasel 
Sent: Wednesday, December 26, 2018 10:01 PM
To: edk2-devel@lists.01.org
Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>
Subject: [PATCH] BaseTools/GenFv: Support SecCore and PeiCore in different FV

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1423

There is usage model that SecCore and PeiCore are in different FVs.
Update BaseTools to support this usage model.

Test: Verified on internal platform with the case SecCore and PeiCore in different FVs and built/booted successfully.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
---
 BaseTools/Source/C/GenFv/GenFvInternalLib.c | 82 +++++++++++++++++++++++++++++++++++++++++-----------------------------------------
 1 file changed, 41 insertions(+), 41 deletions(-)

diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
index 6a874f4e94..32bbcce0a6 100644
--- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
+++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
@@ -1655,43 +1655,42 @@ Returns:
   //
   // Find the PEI Core
   //
+  PeiCorePhysicalAddress = 0;
   Status = GetFileByType (EFI_FV_FILETYPE_PEI_CORE, 1, &PeiCoreFile);
-  if (EFI_ERROR (Status) || PeiCoreFile == NULL) {
-    Error (NULL, 0, 3000, "Invalid", "could not find the PEI core in the FV.");
-    return EFI_ABORTED;
-  }
-  //
-  // PEI Core found, now find PE32 or TE section
-  //
-  Status = GetSectionByType (PeiCoreFile, EFI_SECTION_PE32, 1, &Pe32Section);
-  if (Status == EFI_NOT_FOUND) {
-    Status = GetSectionByType (PeiCoreFile, EFI_SECTION_TE, 1, &Pe32Section);
-  }
+  if (!EFI_ERROR (Status) && (PeiCoreFile != NULL)) {
+    //
+    // PEI Core found, now find PE32 or TE section
+    //
+    Status = GetSectionByType (PeiCoreFile, EFI_SECTION_PE32, 1, &Pe32Section);
+    if (Status == EFI_NOT_FOUND) {
+      Status = GetSectionByType (PeiCoreFile, EFI_SECTION_TE, 1, &Pe32Section);
+    }
 
-  if (EFI_ERROR (Status)) {
-    Error (NULL, 0, 3000, "Invalid", "could not find either a PE32 or a TE section in PEI core file.");
-    return EFI_ABORTED;
-  }
+    if (EFI_ERROR (Status)) {
+      Error (NULL, 0, 3000, "Invalid", "could not find either a PE32 or a TE section in PEI core file.");
+      return EFI_ABORTED;
+    }
 
-  SecHeaderSize = GetSectionHeaderLength(Pe32Section.CommonHeader);
-  Status = GetPe32Info (
-            (VOID *) ((UINTN) Pe32Section.Pe32Section + SecHeaderSize),
-            &EntryPoint,
-            &BaseOfCode,
-            &MachineType
-            );
+    SecHeaderSize = GetSectionHeaderLength(Pe32Section.CommonHeader);
+    Status = GetPe32Info (
+              (VOID *) ((UINTN) Pe32Section.Pe32Section + SecHeaderSize),
+              &EntryPoint,
+              &BaseOfCode,
+              &MachineType
+              );
 
-  if (EFI_ERROR (Status)) {
-    Error (NULL, 0, 3000, "Invalid", "could not get the PE32 entry point for the PEI core.");
-    return EFI_ABORTED;
+    if (EFI_ERROR (Status)) {
+      Error (NULL, 0, 3000, "Invalid", "could not get the PE32 entry point for the PEI core.");
+      return EFI_ABORTED;
+    }
+    //
+    // Physical address is FV base + offset of PE32 + offset of the entry point
+    //
+    PeiCorePhysicalAddress = FvInfo->BaseAddress;
+    PeiCorePhysicalAddress += (UINTN) Pe32Section.Pe32Section + SecHeaderSize - (UINTN) FvImage->FileImage;
+    PeiCorePhysicalAddress += EntryPoint;
+    DebugMsg (NULL, 0, 9, "PeiCore physical entry point address", 
+ "Address = 0x%llX", (unsigned long long) PeiCorePhysicalAddress);
   }
-  //
-  // Physical address is FV base + offset of PE32 + offset of the entry point
-  //
-  PeiCorePhysicalAddress = FvInfo->BaseAddress;
-  PeiCorePhysicalAddress += (UINTN) Pe32Section.Pe32Section + SecHeaderSize - (UINTN) FvImage->FileImage;
-  PeiCorePhysicalAddress += EntryPoint;
-  DebugMsg (NULL, 0, 9, "PeiCore physical entry point address", "Address = 0x%llX", (unsigned long long) PeiCorePhysicalAddress);
 
   if (MachineType == EFI_IMAGE_MACHINE_IA64) {
     //
@@ -1749,16 +1748,17 @@ Returns:
     *SecCoreEntryAddressPtr = SecCorePhysicalAddress;
 
   } else if (MachineType == EFI_IMAGE_MACHINE_IA32 || MachineType == EFI_IMAGE_MACHINE_X64) {
-    //
-    // Get the location to update
-    //
-    Ia32ResetAddressPtr  = (UINT32 *) ((UINTN) FvImage->Eof - IA32_PEI_CORE_ENTRY_OFFSET);
-
-    //
-    // Write lower 32 bits of physical address for Pei Core entry
-    //
-    *Ia32ResetAddressPtr = (UINT32) PeiCorePhysicalAddress;
+    if (PeiCorePhysicalAddress != 0) {
+      //
+      // Get the location to update
+      //
+      Ia32ResetAddressPtr  = (UINT32 *) ((UINTN) FvImage->Eof - 
+ IA32_PEI_CORE_ENTRY_OFFSET);
 
+      //
+      // Write lower 32 bits of physical address for Pei Core entry
+      //
+      *Ia32ResetAddressPtr = (UINT32) PeiCorePhysicalAddress;
+    }
     //
     // Write SecCore Entry point relative address into the jmp instruction in reset vector.
     //
--
2.13.3.windows.1