[PATCH 1/1] UefiPayloadPkg: Update the Module Info rather than create new Info.

[PATCH 1/1] UefiPayloadPkg: Update the Module Info rather than create new Info.

[Guomin Jiang]

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

1. DxeCore will use ModuleInfo to install LoadedImage protocol for DxeCore.
2. DxeIpl will create the ModuleInfo of UniversalPayload. and
   UniversalPayload will create the ModuleInfo of DxeCore.
3. Only the first ModuleInfo will be used by DxeCore to install
   LoadedImage.
4. So it will create the mismatched ModuleInfo for DxeCore.

Changes:
1. When found the ModuleInfo, update it with DxeCore Info only.
2. Create new ModuleInfo if found no ModuleInfo.

Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
---
 .../Library/PayloadEntryHobLib/Hob.c          | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c b/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c
index 61cd11ba0335..b840e7914f7a 100644
--- a/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c
+++ b/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c
@@ -331,6 +331,30 @@ BuildModuleHob (
   ASSERT (((MemoryAllocationModule & (EFI_PAGE_SIZE - 1)) == 0) &&
           ((ModuleLength & (EFI_PAGE_SIZE - 1)) == 0));
 
+  Hob = GetFirstHob (EFI_HOB_TYPE_MEMORY_ALLOCATION);
+  ASSERT (Hob != NULL);
+  while ((Hob = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, Hob)) != NULL) {
+    if (CompareGuid (&Hob->MemoryAllocationHeader.Name, &gEfiHobMemoryAllocModuleGuid)) {
+      //
+      // Update Dxe Core HOB.
+      //
+      Hob->MemoryAllocationHeader.MemoryBaseAddress = MemoryAllocationModule;
+      Hob->MemoryAllocationHeader.MemoryLength      = ModuleLength;
+      Hob->MemoryAllocationHeader.MemoryType        = EfiBootServicesCode;
+
+      ZeroMem (Hob->MemoryAllocationHeader.Reserved, sizeof (Hob->MemoryAllocationHeader.Reserved));
+
+      CopyGuid (&Hob->ModuleName, ModuleName);
+      Hob->EntryPoint = EntryPoint;
+
+      //
+      // The Dxe Core HOB should only one. When find it, update and return
+      //
+      return;
+    }
+    Hob = GET_NEXT_HOB (Hob);
+  }
+
   Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEMORY_ALLOCATION_MODULE));
 
   CopyGuid (&(Hob->MemoryAllocationHeader.Name), &gEfiHobMemoryAllocModuleGuid);
-- 
2.30.0.windows.2

Re: [edk2-devel] [PATCH 1/1] UefiPayloadPkg: Update the Module Info rather than create new Info.

[Zhiguang Liu]

[-- Attachment #1: Type: text/plain, Size: 3501 bytes --]

Hi Guomin,

Thanks for finding this issue.
How do you find this issue? Does it cause any bugs?

For the code change, I have some suggestions.
I don't suggest changing HobLib because it will increase the difference between the payload entry HobLib and standard HobLib.
I suggest changing the code which copies all hobs to new hob list in line 341.
For now, we only skip handoff hob, and you could add a function to determine whether to skip the Hob and let it skip handoff hob and ModuleInfo Hob.
This will help a lot because I also plan to skip some other type hobs in the future to enable other feature.

Thanks,
Zhiguang

________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Guomin Jiang <guomin.jiang@intel.com>
Sent: Tuesday, November 30, 2021 12:23
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Dong, Guo <guo.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Ma, Maurice <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>
Subject: [edk2-devel] [PATCH 1/1] UefiPayloadPkg: Update the Module Info rather than create new Info.

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

1. DxeCore will use ModuleInfo to install LoadedImage protocol for DxeCore.
2. DxeIpl will create the ModuleInfo of UniversalPayload. and
   UniversalPayload will create the ModuleInfo of DxeCore.
3. Only the first ModuleInfo will be used by DxeCore to install
   LoadedImage.
4. So it will create the mismatched ModuleInfo for DxeCore.

Changes:
1. When found the ModuleInfo, update it with DxeCore Info only.
2. Create new ModuleInfo if found no ModuleInfo.

Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
---
 .../Library/PayloadEntryHobLib/Hob.c          | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c b/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c
index 61cd11ba0335..b840e7914f7a 100644
--- a/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c
+++ b/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c
@@ -331,6 +331,30 @@ BuildModuleHob (
   ASSERT (((MemoryAllocationModule & (EFI_PAGE_SIZE - 1)) == 0) &&
           ((ModuleLength & (EFI_PAGE_SIZE - 1)) == 0));

+  Hob = GetFirstHob (EFI_HOB_TYPE_MEMORY_ALLOCATION);
+  ASSERT (Hob != NULL);
+  while ((Hob = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, Hob)) != NULL) {
+    if (CompareGuid (&Hob->MemoryAllocationHeader.Name, &gEfiHobMemoryAllocModuleGuid)) {
+      //
+      // Update Dxe Core HOB.
+      //
+      Hob->MemoryAllocationHeader.MemoryBaseAddress = MemoryAllocationModule;
+      Hob->MemoryAllocationHeader.MemoryLength      = ModuleLength;
+      Hob->MemoryAllocationHeader.MemoryType        = EfiBootServicesCode;
+
+      ZeroMem (Hob->MemoryAllocationHeader.Reserved, sizeof (Hob->MemoryAllocationHeader.Reserved));
+
+      CopyGuid (&Hob->ModuleName, ModuleName);
+      Hob->EntryPoint = EntryPoint;
+
+      //
+      // The Dxe Core HOB should only one. When find it, update and return
+      //
+      return;
+    }
+    Hob = GET_NEXT_HOB (Hob);
+  }
+
   Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEMORY_ALLOCATION_MODULE));

   CopyGuid (&(Hob->MemoryAllocationHeader.Name), &gEfiHobMemoryAllocModuleGuid);
--
2.30.0.windows.2







[-- Attachment #2: Type: text/html, Size: 7386 bytes --]