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@edk2.groups.io <devel@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...@intel.com>; devel@edk2.groups.io > Cc: Gao, Jiaqi <jiaqi....@intel.com>; Wang, Jian J <jian.j.w...@intel.com>; > Gao, Liming <gaolim...@byosoft.com.cn>; Bi, > Dandan <dandan...@intel.com>; Aktas, Erdem <erdemak...@google.com>; James > Bottomley <j...@linux.ibm.com>; Yao, > Jiewen <jiewen....@intel.com>; Gerd Hoffmann <kra...@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....@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.w...@intel.com> > > Cc: Liming Gao <gaolim...@byosoft.com.cn> > > Cc: Dandan Bi <dandan...@intel.com> > > Cc: Erdem Aktas <erdemak...@google.com> > > Cc: James Bottomley <j...@linux.ibm.com> > > Cc: Jiewen Yao <jiewen....@intel.com> > > Cc: Tom Lendacky <thomas.lenda...@amd.com> > > Cc: Gerd Hoffmann <kra...@redhat.com> > > Signed-off-by: Jiaqi Gao <jiaqi....@intel.com> > > Signed-off-by: Min Xu <min.m...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
