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

["Nhi Pham via groups.io" <nhi=os.amperecomputing.com@groups.io>]

Hi Oliver, thank you for taking a look at this patch.

On 9/2/2023 3:43 AM, Oliver Smith-Denny wrote:
> On 8/31/2023 1:20 AM, Nhi Pham via groups.io wrote:
>> Hi Ard,
>>
>> Thanks for your response on this patch. Please see my reply inline...
>>
>> On 8/30/2023 8:10 PM, Ard Biesheuvel wrote:
>>> [EXTERNAL EMAIL NOTICE: This email originated from an external 
>>> sender. Please be mindful of safe email handling and proprietary 
>>> information protection practices.]
>>>
>>>
>>> On Wed, 16 Aug 2023 at 10:56, Nhi Pham 
>>> <nhi@amperemail.onmicrosoft.com> 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 <nhi@os.amperecomputing.com>
>>>>
>>> Thanks for reminding me.
>>>
>>> So if the HOB creation is completely broken, are we sure this is the
>>> correct fix? Wouldn't it be better to update FreeMemoryTop and
>>> FreeMemoryBottom to the correct values?
>>
>> Per your question, I had a deep dive into the HOB today. I think I 
>> need to clarify further the issue to make sure that we are on the 
>> same page.
>>
>> 1. When the base of the HOB list (HobStart) is reallocated in the MM 
>> heap memory, the Hob->EfiEndOfHobList must be adjusted accordingly 
>> compared to the new HOB base. Currently, we are missing that. That 
>> causes the CreateHob() function does not work. The GetNextHob() 
>> function always look up the old HOB list instead of the new HOB list.
>>
>> 2. The Memory Allocation StandaloneMmPkg/Core/Page.c does not update 
>> the Hob->EfiFreeMemoryTop, this may cause the MM heap space and HOB 
>> space to be overlapped at some points.
>>
>> It sounds like the issue on the memory allocation in StandaloneMM. 
>> For #1, I think we should write a new patch for it. For #2, could you 
>> advise?
>>
>
> If I am understanding this correctly, this is only an issue when
> HOBs are created in StMM, i.e. not from HOBs that are passed in. Is this
> correct?
Yes, the issue only occurs when HOB are created in StandaloneMM by the 
HOB library instance 
StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
>
> If so, is HOB creation in StMM and supported use case? The only instance
I think it is intended to work as the CreateHob() function is implemented.
> a quick search turns up is the ARM StMM Core entry, where some
> information from TF-A is converted to HOB format. Do we have any other
> use cases (and curious more on this use case). My thought process would
> be that StMM would not create any HOBs. Depending on FW configuration,
> it may receive HOBs from PEI.

I have a use case when enabling the UEFI Variable driver running in 
StandaloneMM. Instead of using the PCDs, the in-memory NVRAM region is 
allocated **dynamically** at boot time in the StMM secure memory. Then, 
they will be passed into the gVariableFlashInfoHobGuid for being 
consumed by other variable MM drivers.

>
> In PEI/DXE to StMM communication, we should be using the
> MM_Communicate interface.
>
> But in StMM to StMM communication, I would expect we would go through
> interfaces like protocols, PCDs, or UEFI variables. Any reason for HOBs
> that I'm not thinking of? I'm wondering if the answer here is to
> address what seemingly is one (or a very small set) of use cases and
> remove the HOB creation code in StMM altogether.
>
> Thanks,
> Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108269): https://edk2.groups.io/g/devel/message/108269
Mute This Topic: https://groups.io/mt/89020085/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-