[edk2-devel] [PATCH V2 14/14] MdeModulePkg: Pool and page functions accept memory when OOM occurs
Ni, Ray
ray.ni at intel.com
Tue Aug 30 00:29:14 UTC 2022
Can you please explain more about this patch?
I am a little nervous when seeing patches that change the fundamental memory services.
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Lendacky, Thomas via groups.io
> Sent: Tuesday, August 30, 2022 4:47 AM
> To: Xu, Min M <min.m.xu at intel.com>; devel at edk2.groups.io
> Cc: Gao, Jiaqi <jiaqi.gao at intel.com>; Wang, Jian J <jian.j.wang at intel.com>; Gao, Liming <gaoliming at byosoft.com.cn>; Bi,
> Dandan <dandan.bi at intel.com>; Aktas, Erdem <erdemaktas at google.com>; James Bottomley <jejb at linux.ibm.com>; Yao,
> Jiewen <jiewen.yao at intel.com>; Gerd Hoffmann <kraxel at redhat.com>
> Subject: Re: [edk2-devel] [PATCH V2 14/14] MdeModulePkg: Pool and page functions accept memory when OOM occurs
>
> On 8/27/22 01:21, Min Xu wrote:
> > From: Jiaqi Gao <jiaqi.gao at intel.com>
> >
> > RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3937
> >
> > When CoreAllocatePages() / CoreAllocatePool() meets error of
> > EFI_OUT_OF_RESOURCES, locate the EdkiiMemoryAcceptProtocol and accept extra
> > memory dynamically.
> >
> > Firstly, find the unaccpeted memory region with enough size in GCD
>
> s/unaccpeted/unaccepted/
>
> > entries. Then locate the EdkiiMemoryAcceptProtocol and accept the memory.
> > Finally, update the GCD memory and gMemoryMap entries.
> >
> > After updating the memory infomation, CoreInternalAllocatePages() /
> > CoreInternalAllocatePool() will be recalled to allocate pages / pool.
>
> What path does allocation take when called through boot services? If I set
> a 256MB accepted memory size, I can get to the bootloader and select my
> kernel. But then the kernel dies in efi_relocate_kernel() with:
>
> EFI stub: ERROR: Failed to allocate usable memory for kernel.
> EFI stub: ERROR: efi_relocate_kernel() failed!
> EFI stub: ERROR: efi_main() failed!
>
> because both efi_bs_call(allocate_pages, ...) and efi_low_alloc_above() fail.
>
> Similar to DXE, should OVMF accept more memory through this path to let
> the kernel boot?
>
> Thanks,
> Tom
>
> >
> > Cc: Jian J Wang <jian.j.wang at intel.com>
> > Cc: Liming Gao <gaoliming at byosoft.com.cn>
> > Cc: Dandan Bi <dandan.bi at intel.com>
> > Cc: Erdem Aktas <erdemaktas at google.com>
> > Cc: James Bottomley <jejb at linux.ibm.com>
> > Cc: Jiewen Yao <jiewen.yao at intel.com>
> > Cc: Tom Lendacky <thomas.lendacky at amd.com>
> > Cc: Gerd Hoffmann <kraxel at redhat.com>
> > Signed-off-by: Jiaqi Gao <jiaqi.gao at intel.com>
> > Signed-off-by: Min Xu <min.m.xu at intel.com>
> > ---
> > MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
> > MdeModulePkg/Core/Dxe/Mem/Imem.h | 16 +++
> > MdeModulePkg/Core/Dxe/Mem/Page.c | 190 ++++++++++++++++++++++++++++++
> > MdeModulePkg/Core/Dxe/Mem/Pool.c | 14 +++
> > 4 files changed, 221 insertions(+)
> >
> > diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
> > index e4bca895773d..371ba45357be 100644
> > --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> > +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> > @@ -169,6 +169,7 @@
> > gEfiVariableArchProtocolGuid ## CONSUMES
> > gEfiCapsuleArchProtocolGuid ## CONSUMES
> > gEfiWatchdogTimerArchProtocolGuid ## CONSUMES
> > + gEdkiiMemoryAcceptProtocolGuid ## CONSUMES
> >
> > [Pcd]
> > gEfiMdeModulePkgTokenSpaceGuid.PcdLoadFixAddressBootTimeCodePageNumber ## SOMETIMES_CONSUMES
> > diff --git a/MdeModulePkg/Core/Dxe/Mem/Imem.h b/MdeModulePkg/Core/Dxe/Mem/Imem.h
> > index 2f0bf2bf631f..22e0d0e44030 100644
> > --- a/MdeModulePkg/Core/Dxe/Mem/Imem.h
> > +++ b/MdeModulePkg/Core/Dxe/Mem/Imem.h
> > @@ -47,6 +47,22 @@ typedef struct {
> > // Internal prototypes
> > //
> >
> > +/**
> > + Internal function. Used by the pool and page functions to accept memory
> > + when OOM occurs.
> > +
> > + @param Type The type of allocation to perform.
> > + @param AcceptSize Size of memory to be accepted.
> > + @param Memory Accept memory address
> > +
> > +**/
> > +EFI_STATUS
> > +AcceptMemoryResource (
> > + IN EFI_ALLOCATE_TYPE Type,
> > + IN UINTN AcceptSize,
> > + IN OUT EFI_PHYSICAL_ADDRESS *Memory
> > + );
> > +
> > /**
> > Internal function. Used by the pool functions to allocate pages
> > to back pool allocation requests.
> > diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
> > index 160289c1f9ec..513792a7fe04 100644
> > --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> > +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> > @@ -10,6 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > #include "Imem.h"
> > #include "HeapGuard.h"
> > #include <Pi/PrePiDxeCis.h>
> > +#include <Protocol/MemoryAccept.h>
> >
> > //
> > // Entry for tracking the memory regions for each memory type to coalesce similar memory types
> > @@ -379,6 +380,176 @@ CoreFreeMemoryMapStack (
> > mFreeMapStack -= 1;
> > }
> >
> > +/**
> > + Used to accept memory when OOM occurs.
> > +
> > + @param Type The type of allocation to perform.
> > + @param AcceptSize Size of memory to be accepted.
> > + @param Memory Accept memory address
> > +
> > +**/
> > +EFI_STATUS
> > +AcceptMemoryResource (
> > + IN EFI_ALLOCATE_TYPE Type,
> > + IN UINTN AcceptSize,
> > + IN OUT EFI_PHYSICAL_ADDRESS *Memory
> > + )
> > +{
> > + #ifdef MDE_CPU_X64
> > +
> > + LIST_ENTRY *Link;
> > + EFI_GCD_MAP_ENTRY *GcdEntry;
> > + EFI_GCD_MAP_ENTRY UnacceptedEntry;
> > + EDKII_MEMORY_ACCEPT_PROTOCOL *MemoryAcceptProtocol;
> > + UINTN Start;
> > + UINTN End;
> > + EFI_STATUS Status;
> > +
> > + //
> > + // We accept n*32MB at one time to improve the efficiency.
> > + //
> > + AcceptSize = (AcceptSize + SIZE_32MB - 1) & ~(SIZE_32MB - 1);
> > +
> > + if (AcceptSize == 0) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + Status = gBS->LocateProtocol (&gEdkiiMemoryAcceptProtocolGuid, NULL, (VOID **)&MemoryAcceptProtocol);
> > + if (EFI_ERROR (Status)) {
> > + return EFI_UNSUPPORTED;
> > + }
> > +
> > + if (Type == AllocateAddress) {
> > + Start = *Memory;
> > + End = *Memory + AcceptSize;
> > + }
> > +
> > + if (Type == AllocateMaxAddress) {
> > + if (*Memory < EFI_PAGE_MASK) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + if ((*Memory & EFI_PAGE_MASK) != EFI_PAGE_MASK) {
> > + //
> > + // Change MaxAddress to be 1 page lower
> > + //
> > + *Memory -= EFI_PAGE_SIZE;
> > +
> > + //
> > + // Set MaxAddress to a page boundary
> > + //
> > + *Memory &= ~(UINT64)EFI_PAGE_MASK;
> > +
> > + //
> > + // Set MaxAddress to end of the page
> > + //
> > + *Memory |= EFI_PAGE_MASK;
> > + }
> > + }
> > +
> > + //
> > + // Traverse the mGcdMemorySpaceMap to find out the unaccepted
> > + // memory entry with big enough size.
> > + //
> > + Link = mGcdMemorySpaceMap.ForwardLink;
> > + while (Link != &mGcdMemorySpaceMap) {
> > + GcdEntry = CR (Link, EFI_GCD_MAP_ENTRY, Link, EFI_GCD_MAP_SIGNATURE);
> > + if (GcdEntry->GcdMemoryType == EFI_GCD_MEMORY_TYPE_UNACCEPTED) {
> > + if (Type == AllocateMaxAddress) {
> > + if (GcdEntry->BaseAddress + AcceptSize - 1 > *Memory) {
> > + Link = Link->ForwardLink;
> > + continue;
> > + }
> > + } else if (Type == AllocateAddress) {
> > + if ((GcdEntry->BaseAddress > *Memory) || (GcdEntry->EndAddress < *Memory + AcceptSize - 1)) {
> > + Link = Link->ForwardLink;
> > + continue;
> > + }
> > + }
> > +
> > + //
> > + // Is the entry big enough?
> > + //
> > + if (AcceptSize <= GcdEntry->EndAddress - GcdEntry->BaseAddress + 1) {
> > + UnacceptedEntry = *GcdEntry;
> > + if (Type != AllocateAddress) {
> > + Start = UnacceptedEntry.BaseAddress;
> > + End = UnacceptedEntry.BaseAddress + AcceptSize - 1;
> > + }
> > +
> > + break;
> > + }
> > + }
> > +
> > + Link = Link->ForwardLink;
> > + }
> > +
> > + if (Link == &mGcdMemorySpaceMap) {
> > + return EFI_OUT_OF_RESOURCES;
> > + }
> > +
> > + //
> > + // Accept memory using the interface provide by the protocol.
> > + //
> > + Status = MemoryAcceptProtocol->AcceptMemory (MemoryAcceptProtocol, Start, AcceptSize);
> > + if (EFI_ERROR (Status)) {
> > + return EFI_OUT_OF_RESOURCES;
> > + }
> > +
> > + //
> > + // If memory is accepted successfully, remove the target memory space from GCD.
> > + //
> > + CoreRemoveMemorySpace (UnacceptedEntry.BaseAddress, UnacceptedEntry.EndAddress -
> UnacceptedEntry.BaseAddress + 1);
> > +
> > + //
> > + // Add the remain lower part of unaccepted memory to the
> > + // Gcd memory space and memory map.
> > + //
> > + if (Start > UnacceptedEntry.BaseAddress) {
> > + CoreAddMemorySpace (
> > + EFI_GCD_MEMORY_TYPE_UNACCEPTED,
> > + UnacceptedEntry.BaseAddress,
> > + Start - UnacceptedEntry.BaseAddress,
> > + UnacceptedEntry.Capabilities
> > + );
> > + }
> > +
> > + //
> > + // Update accepted part of the memory entry to type of EfiGcdMemoryTypeSystemMemory
> > + // and add the range to the memory map.
> > + //
> > + CoreAddMemorySpace (
> > + EfiGcdMemoryTypeSystemMemory,
> > + Start,
> > + AcceptSize,
> > + //
> > + // Hardcode memory space attributes.
> > + //
> > + EFI_MEMORY_CPU_CRYPTO | EFI_MEMORY_XP | EFI_MEMORY_RO | EFI_MEMORY_RP
> > + );
> > +
> > + //
> > + // Add the remain higher part of unaccepted memory to the
> > + // Gcd memory space and memory map.
> > + //
> > + if (UnacceptedEntry.EndAddress > End) {
> > + CoreAddMemorySpace (
> > + EFI_GCD_MEMORY_TYPE_UNACCEPTED,
> > + End + 1,
> > + UnacceptedEntry.EndAddress - End,
> > + UnacceptedEntry.Capabilities
> > + );
> > + }
> > +
> > + return EFI_SUCCESS;
> > +
> > + #else
> > +
> > + return EFI_UNSUPPORTED;
> > +
> > + #endif
> > +}
> > +
> > /**
> > Find untested but initialized memory regions in GCD map and convert them to be DXE allocatable.
> >
> > @@ -1486,6 +1657,25 @@ CoreAllocatePages (
> > Memory,
> > NeedGuard
> > );
> > + #ifdef MDE_CPU_X64
> > +
> > + if (Status == EFI_OUT_OF_RESOURCES) {
> > + Status = AcceptMemoryResource (Type, NumberOfPages << EFI_PAGE_SHIFT, Memory);
> > + if (!EFI_ERROR (Status)) {
> > + Status = CoreInternalAllocatePages (
> > + Type,
> > + MemoryType,
> > + NumberOfPages,
> > + Memory,
> > + NeedGuard
> > + );
> > + } else {
> > + Status = EFI_OUT_OF_RESOURCES;
> > + }
> > + }
> > +
> > + #endif
> > +
> > if (!EFI_ERROR (Status)) {
> > CoreUpdateProfile (
> > (EFI_PHYSICAL_ADDRESS)(UINTN)RETURN_ADDRESS (0),
> > diff --git a/MdeModulePkg/Core/Dxe/Mem/Pool.c b/MdeModulePkg/Core/Dxe/Mem/Pool.c
> > index 7aaf501600cf..9e8c8611c1ef 100644
> > --- a/MdeModulePkg/Core/Dxe/Mem/Pool.c
> > +++ b/MdeModulePkg/Core/Dxe/Mem/Pool.c
> > @@ -273,6 +273,20 @@ CoreAllocatePool (
> > EFI_STATUS Status;
> >
> > Status = CoreInternalAllocatePool (PoolType, Size, Buffer);
> > +
> > + #ifdef MDE_CPU_X64
> > +
> > + if (Status == EFI_OUT_OF_RESOURCES) {
> > + Status = AcceptMemoryResource (AllocateAnyPages, Size, NULL);
> > + if (!EFI_ERROR (Status)) {
> > + Status = CoreInternalAllocatePool (PoolType, Size, Buffer);
> > + } else {
> > + Status = EFI_OUT_OF_RESOURCES;
> > + }
> > + }
> > +
> > + #endif
> > +
> > if (!EFI_ERROR (Status)) {
> > CoreUpdateProfile (
> > (EFI_PHYSICAL_ADDRESS)(UINTN)RETURN_ADDRESS (0),
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92939): https://edk2.groups.io/g/devel/message/92939
Mute This Topic: https://groups.io/mt/93285612/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