GCD initialization and memory allocation HOBs


Jeff Brasen
 

It looks like CoreInitializeMemoryServices (https://github.com/tianocore/edk2/blob/dd5c7e3c5282b084daa5bbf0ec229cec699b2c17/MdeModulePkg/Core/Dxe/Gcd/Gcd.c#L2116) does not honor any memory allocations outside of the HOB list itself. Should we add code to this function to make sure the region this function select does not end up over a reserved region that the HOB producer phase marked as already allocated? Either that or codify the specific requirements a bit more on to make sure cases like this are invalid. (producer might have to split up system resource entries, etc)

We can work on a change if we decide that we want to improve GCD but wanted to get feedback before we started on this.

Thanks,
Jeff


Laszlo Ersek
 

On 09/24/20 20:40, Jeff Brasen wrote:
It looks like CoreInitializeMemoryServices
(https://github.com/tianocore/edk2/blob/dd5c7e3c5282b084daa5bbf0ec229cec699b2c17/MdeModulePkg/Core/Dxe/Gcd/Gcd.c#L2116)
does not honor any memory allocations outside of the HOB list itself.
Should we add code to this function to make sure the region this
function select does not end up over a reserved region that the HOB
producer phase marked as already allocated? Either that or codify the
specific requirements a bit more on to make sure cases like this are
invalid. (producer might have to split up system resource entries,
etc)
Do you mean

(1) a physically reserved memory address range
(EFI_RESOURCE_MEMORY_RESERVED), or

(2) normal RAM (EFI_RESOURCE_SYSTEM_MEMORY) that is meant to be
"repurposed" (= pre-allocated) in PEI as a particular UEFI memory type?


EFI_HOB_RESOURCE_DESCRIPTOR "does not describe how memory is used but
instead describes the attributes of the physical memory present".

CoreInitializeMemoryServices() honors (1).

CoreInitializeMemoryServices() does not honor (2); however, the later
function CoreInitializeGcdServices() does honor (2).


* Regarding (1):

My understanding is that the HOB producer phase should describe
"physically reserved" memory regions with EFI_RESOURCE_MEMORY_RESERVED
resource descriptor HOBs, and these should not overlap normal memory
regions, which are described with EFI_RESOURCE_SYSTEM_MEMORY descriptor
HOBs.

Furthermore, whether (a part of) a memory resource is allocated or not,
is *orthogonal* to whether the memory resource is "physically reserved
memory" or "normal RAM". In my opinion, the diagram at

PI 1.7
Volume 2 (DXE Core Interface)
7.2.2 GCD Memory Resources
Figure 2. GCD Memory State Transitions

applies.

So the HOB producer phase should firstly describe the area in question
as EFI_RESOURCE_MEMORY_RESERVED (without overlapping any other resource
descriptor in memory address space).

Secondly, if the HOB producer phase wants to prevent DXE modules from
allocating reserved memory out of this resource, then the HOB producer
phase should *also* cover the entire range with a memalloc HOB
that uses EfiReservedMemoryType.

The first step above (= describing the area with an
EFI_RESOURCE_MEMORY_RESERVED HOB) will activate the following branch in
CoreInitializeMemoryServices():

//
// Skip Resource Descriptor HOBs that do not describe tested system memory
//
ResourceHob = Hob.ResourceDescriptor;
if (ResourceHob->ResourceType != EFI_RESOURCE_SYSTEM_MEMORY) {
continue;
}

* Regarding (2):

Normal (= not physically reserved) RAM that is allocated in PEI -- using
memalloc HOBs with various UEFI memory types, such as BS Data or AcpiNVS
-- is set aside in the CoreInitializeGcdServices() function.

Is that too late for your purpose? If so, why?

AIUI, CoreInitializeGcdServices() does make sure that DXE-phase calls to
gDS->AllocateMemorySpace(), gBS->AllocatePool(), and friends, will not
trample over the original memalloc HOBs.

Thanks,
Laszlo


Jeff Brasen
 

#2 is the case we are in, we have data that is loaded by our pre-UEFI bootloaders into memory that should behave as boot services memory (OS can reclaim this) When we setup our HOB list we added a normal system memory resource that covers both the HOB list and this other data. We then add a memory allocation HOB to mark this memory as boot services data. We are seeing cases where CoreInitializeMemoryServices Is selecting the region that has this data and is both causing corruption as well as preventing the memory allocation hob from being processed correctly as the memory is not free when GCD parses it.

We can easily work around this particular issue by splitting the system memory resource so that GCD then goes to look for space in the free region in the hob or at the beginning of the resource that contains the HOB list. But if we don't have enough space in those regions then it will look for other memory regions and it doesn't seem like the best practice to design the HOB producer with the specific implementation of GCD services.

It seems that if we have memory allocation data that is in outside the HOB list itself we could hit this problem, and this seems valid per my reading of the PI spec.

-----Original Message-----
From: discuss@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Friday, September 25, 2020 3:06 AM
To: discuss@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>
Cc: Jan Bobek <jbobek@nvidia.com>; Ashish Singhal <ashishsingha@nvidia.com>
Subject: Re: [edk2-discuss] GCD initialization and memory allocation HOBs

External email: Use caution opening links or attachments


On 09/24/20 20:40, Jeff Brasen wrote:
It looks like CoreInitializeMemoryServices
(https://github.com/tianocore/edk2/blob/dd5c7e3c5282b084daa5bbf0ec229c
ec699b2c17/MdeModulePkg/Core/Dxe/Gcd/Gcd.c#L2116)
does not honor any memory allocations outside of the HOB list itself.
Should we add code to this function to make sure the region this
function select does not end up over a reserved region that the HOB
producer phase marked as already allocated? Either that or codify the
specific requirements a bit more on to make sure cases like this are
invalid. (producer might have to split up system resource entries,
etc)
Do you mean

(1) a physically reserved memory address range (EFI_RESOURCE_MEMORY_RESERVED), or

(2) normal RAM (EFI_RESOURCE_SYSTEM_MEMORY) that is meant to be "repurposed" (= pre-allocated) in PEI as a particular UEFI memory type?

EFI_HOB_RESOURCE_DESCRIPTOR "does not describe how memory is used but instead describes the attributes of the physical memory present".

CoreInitializeMemoryServices() honors (1).

CoreInitializeMemoryServices() does not honor (2); however, the later function CoreInitializeGcdServices() does honor (2).


* Regarding (1):

My understanding is that the HOB producer phase should describe "physically reserved" memory regions with EFI_RESOURCE_MEMORY_RESERVED resource descriptor HOBs, and these should not overlap normal memory regions, which are described with EFI_RESOURCE_SYSTEM_MEMORY descriptor HOBs.

Furthermore, whether (a part of) a memory resource is allocated or not, is *orthogonal* to whether the memory resource is "physically reserved memory" or "normal RAM". In my opinion, the diagram at

PI 1.7
Volume 2 (DXE Core Interface)
7.2.2 GCD Memory Resources
Figure 2. GCD Memory State Transitions

applies.

So the HOB producer phase should firstly describe the area in question as EFI_RESOURCE_MEMORY_RESERVED (without overlapping any other resource descriptor in memory address space).

Secondly, if the HOB producer phase wants to prevent DXE modules from allocating reserved memory out of this resource, then the HOB producer phase should *also* cover the entire range with a memalloc HOB that uses EfiReservedMemoryType.

The first step above (= describing the area with an EFI_RESOURCE_MEMORY_RESERVED HOB) will activate the following branch in
CoreInitializeMemoryServices():

//
// Skip Resource Descriptor HOBs that do not describe tested system memory
//
ResourceHob = Hob.ResourceDescriptor;
if (ResourceHob->ResourceType != EFI_RESOURCE_SYSTEM_MEMORY) {
continue;
}

* Regarding (2):

Normal (= not physically reserved) RAM that is allocated in PEI -- using memalloc HOBs with various UEFI memory types, such as BS Data or AcpiNVS
-- is set aside in the CoreInitializeGcdServices() function.

Is that too late for your purpose? If so, why?

AIUI, CoreInitializeGcdServices() does make sure that DXE-phase calls to
gDS->AllocateMemorySpace(), gBS->AllocatePool(), and friends, will not
trample over the original memalloc HOBs.

Thanks,
Laszlo