[edk2-devel] [PATCH v1 2/2] StandaloneMmPkg: Fix HOB space and heap space conflicted issue
Nhi Pham via groups.io
nhi=os.amperecomputing.com at groups.io
Wed Aug 16 08:55:52 UTC 2023
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 <nhi at 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 <huangming at 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 <huangming at 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 (#107793): https://edk2.groups.io/g/devel/message/107793
Mute This Topic: https://groups.io/mt/89020085/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list