Re: [edk2-devel] [PATCH v1 2/2] StandaloneMmPkg: Fix HOB space and heap space conflicted issue

Mon, 28 Aug 2023 19:49:22 -0700 

Just a gentle ping!

Hi Ard, can you provide your feedback on this patch?

Thanks,

-Nhi

On 8/16/2023 3:55 PM, Nhi Pham via groups.io wrote:

Hi Ard and Ming,
I have been seeing an issue with StandaloneMM HobLib that can be fixed by this patch as well.
The function CreateHob() in the HobLib instance StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf does not work at all. The HobList is early created by the StandaloneMmCoreEntryPoint then it is relocated on the heap memory by StandaloneMmCore. But the FreeMemoryTop and FreeMemoryBottom are not updated accordingly and the HOB free memory top is overlapped with the heap space. This causes the CreateHob() function to not work as expected. Introducing the PcdMemoryHobSize is reasonable to fix this issue. 

I tested this patch in my end.

Tested-by: Nhi Pham <n...@os.amperecomputing.com>

Thanks,

-Nhi

On 5/12/2022 5:09 PM, Ming Huang via groups.io wrote:


在 5/3/22 5:10 PM, Ard Biesheuvel 写道:
On Wed, 9 Feb 2022 at 13:26, Ming Huang <huangm...@linux.alibaba.com> wrote: 
The heap space will be rewrote if a StandloneMmPkg module create HOB
by BuildGuidHob() interface and write data to HOB space.

Can you elaborate? What is supposed to happen and why, and what is
happening instead?

I tried my best to explain the issue:

-----------------------------<--HandOffHob->EfiFreeMemoryTop
|                           |
|                           |
|                           |
|                           |
|                           |
|                           |
|          mMmMemoryMap     |
|---------------------------|<--HandOffHob->EfiFreeMemoryBottom
|          HobEnd           |
|---------------------------|<--HandOffHob->EfiEndOfHobList
|                           |
|          Hob #1           |
-----------------------------<--MmHobStart
1 The mMmMemoryMap which use for free page is on above the HobEnd.
2 Create a hob by BuildGuidHob(), the HobEnd will move up and cover
   the structure or list using by free page.

After this patch, there is a pre-allocation space for creating hob.

-----------------------------<--HandOffHob->EfiFreeMemoryTop
|                           |
|                           |
|                           |
|                           |
|         mMmMemoryMap      |
|---------------------------|<--HandOffHob->EfiFreeMemoryBottom
|          Hob free space   | by PcdMemoryHobSize
|---------------------------|
|          HobEnd           |
|---------------------------|<--HandOffHob->EfiEndOfHobList
|                           |
|          Hob #1           |
-----------------------------<--MmHobStart


Add a PCD PcdMemoryHobSize for pre-allocation a space to create HOB to
fix this issue.

Signed-off-by: Ming Huang <huangm...@linux.alibaba.com>
---
  StandaloneMmPkg/Core/StandaloneMmCore.c   | 17 ++++++++++++++++-
  StandaloneMmPkg/Core/StandaloneMmCore.inf |  3 +++
  StandaloneMmPkg/StandaloneMmPkg.dec       |  2 ++
  3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c b/StandaloneMmPkg/Core/StandaloneMmCore.c 
index d221f1d111..1cf259d946 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.c
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
@@ -512,6 +512,9 @@ StandaloneMmMain (
    EFI_MMRAM_DESCRIPTOR            *MmramRanges;
    UINTN                           MmramRangeCount;
    EFI_HOB_FIRMWARE_VOLUME         *BfvHob;
+  EFI_HOB_HANDOFF_INFO_TABLE      *HandOffHobNew;
+  EFI_HOB_HANDOFF_INFO_TABLE      *HandOffHobOrg;
+  UINT64                          MaxHobSize = PcdGet64 (PcdMemoryHobSize); 

    ProcessLibraryConstructorList (HobStart, &gMmCoreMmst);

@@ -619,10 +622,22 @@ StandaloneMmMain (
    //
    HobSize = GetHobListSize (HobStart);
    DEBUG ((DEBUG_INFO, "HobSize - 0x%x\n", HobSize));
-  MmHobStart = AllocatePool (HobSize);
+  ASSERT (HobSize <= MaxHobSize);
+  MmHobStart = AllocatePool (MaxHobSize);
    DEBUG ((DEBUG_INFO, "MmHobStart - 0x%x\n", MmHobStart));
    ASSERT (MmHobStart != NULL);
    CopyMem (MmHobStart, HobStart, HobSize);
+  //
+  // Initlialize the new HOB table
+  //
+  HandOffHobOrg = (EFI_HOB_HANDOFF_INFO_TABLE *)HobStart;
+  HandOffHobNew = (EFI_HOB_HANDOFF_INFO_TABLE *)MmHobStart;
+  HandOffHobNew->EfiEndOfHobList = (EFI_PHYSICAL_ADDRESS)MmHobStart +
+    (HandOffHobOrg->EfiEndOfHobList - (EFI_PHYSICAL_ADDRESS)HobStart); +  HandOffHobNew->EfiFreeMemoryBottom = HandOffHobNew->EfiEndOfHobList + +                                       sizeof (EFI_HOB_GENERIC_HEADER); +  HandOffHobNew->EfiFreeMemoryTop = (EFI_PHYSICAL_ADDRESS)MmHobStart + MaxHobSize; 
+
    Status = MmInstallConfigurationTable (&gMmCoreMmst, &gEfiHobListGuid, MmHobStart, HobSize); 
    ASSERT_EFI_ERROR (Status);
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf b/StandaloneMmPkg/Core/StandaloneMmCore.inf 
index c44b9ff333..37e6135d73 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.inf
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.inf
@@ -76,6 +76,9 @@
    gEfiEventExitBootServicesGuid
    gEfiEventReadyToBootGuid

+[FixedPcd]
+  gStandaloneMmPkgTokenSpaceGuid.PcdMemoryHobSize
+
  #
  # This configuration fails for CLANGPDB, which does not support PIE in the GCC 
  # sense. Such however is required for ARM family StandaloneMmCore
diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec b/StandaloneMmPkg/StandaloneMmPkg.dec 
index 46784d94e4..cf554676e2 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dec
+++ b/StandaloneMmPkg/StandaloneMmPkg.dec
@@ -48,3 +48,5 @@
    gEfiStandaloneMmNonSecureBufferGuid      = { 0xf00497e3, 0xbfa2, 0x41a1, { 0x9d, 0x29, 0x54, 0xc2, 0xe9, 0x37, 0x21, 0xc5 }}     gEfiArmTfCpuDriverEpDescriptorGuid       = { 0x6ecbd5a1, 0xc0f8, 0x4702, { 0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }} 

+[PcdsFixedAtBuild]
+ gStandaloneMmPkgTokenSpaceGuid.PcdMemoryHobSize|0x00000000|UINT64|0x00000004 
--
2.17.1



















-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108075): https://edk2.groups.io/g/devel/message/108075
Mute This Topic: https://groups.io/mt/89020085/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to