Re: [PATCH v2 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine

[Laszlo Ersek <lersek@redhat.com>]

On 05/24/18 11:09, Ard Biesheuvel wrote:
> Add a routine to DxeServicesLib that abstracts the allocation of memory
> that should be accessible by PEI after a warm reboot. We will use it to
> replace open coded implementations that limit the address to < 4 GB,
> which may not be possible on non-Intel systems that have no 32-bit
> addressable memory at all.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  MdePkg/Include/Library/DxeServicesLib.h          | 23 +++++++-
>  MdePkg/Library/DxeServicesLib/DxeServicesLib.c   | 55 ++++++++++++++++++++
>  MdePkg/Library/DxeServicesLib/DxeServicesLib.inf |  3 ++
>  3 files changed, 80 insertions(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Include/Library/DxeServicesLib.h b/MdePkg/Include/Library/DxeServicesLib.h
> index 7c1c62236d96..20aee68af558 100644
> --- a/MdePkg/Include/Library/DxeServicesLib.h
> +++ b/MdePkg/Include/Library/DxeServicesLib.h
> @@ -305,5 +305,26 @@ GetFileDevicePathFromAnyFv (
>    OUT       EFI_DEVICE_PATH_PROTOCOL  **FvFileDevicePath
>    );
>  
> -#endif
> +/**
> +  Allocates one or more 4KB pages of a given type from a memory region that is
> +  accessible to PEI.
> +
> +  Allocates the number of 4KB pages of type 'MemoryType' and returns a
> +  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
> +  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
> +  memory remaining to satisfy the request, then NULL is returned.
>  
> +  @param[in]  MemoryType            The memory type to allocate
> +  @param[in]  Pages                 The number of 4 KB pages to allocate.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocatePeiAccessiblePages (
> +  IN EFI_MEMORY_TYPE  MemoryType,
> +  IN UINTN            Pages
> +  );
> +
> +#endif
> diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
> index 1827c9216fbc..6719aabe5d04 100644
> --- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
> +++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
> @@ -16,6 +16,7 @@
>  
>  #include <PiDxe.h>
>  #include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Library/DevicePathLib.h>
> @@ -1093,3 +1094,57 @@ Done:
>  
>    return Status;
>  }
> +
> +/**
> +  Allocates one or more 4KB pages of a given type from a memory region that is
> +  accessible to PEI.
> +
> +  Allocates the number of 4KB pages of type 'MemoryType' and returns a
> +  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
> +  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
> +  memory remaining to satisfy the request, then NULL is returned.
> +
> +  @param[in]  MemoryType            The memory type to allocate
> +  @param[in]  Pages                 The number of 4 KB pages to allocate.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocatePeiAccessiblePages (
> +  IN EFI_MEMORY_TYPE  MemoryType,
> +  IN UINTN            Pages
> +  )
> +{
> +  EFI_STATUS                  Status;
> +  EFI_ALLOCATE_TYPE           AllocType;
> +  EFI_PHYSICAL_ADDRESS        Memory;
> +#ifdef MDE_CPU_X64
> +  EFI_HOB_HANDOFF_INFO_TABLE  *PhitHob;
> +#endif
> +
> +  if (Pages == 0) {
> +    return NULL;
> +  }
> +
> +  AllocType = AllocateAnyPages;
> +#ifdef MDE_CPU_X64
> +  //
> +  // On X64 systems, a X64 build of DXE may be combined with a 32-bit build of
> +  // PEI, and so we need to check the memory limit set by PEI, and allocate
> +  // below 4 GB if the limit is set to 4 GB or lower.
> +  //
> +  PhitHob = (EFI_HOB_HANDOFF_INFO_TABLE *)GetHobList ();
> +  if (PhitHob->EfiFreeMemoryTop <= MAX_UINT32) {
> +    AllocType = AllocateMaxAddress;
> +    Memory = MAX_UINT32;
> +  }
> +#endif
> +
> +  Status = gBS->AllocatePages (AllocType, MemoryType, Pages, &Memory);
> +  if (EFI_ERROR (Status)) {
> +    return NULL;
> +  }
> +  return (VOID *)(UINTN)Memory;
> +}
> diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> index bd2faf2f6f2d..b0306c09872f 100644
> --- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> +++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> @@ -44,6 +44,9 @@ [LibraryClasses]
>    UefiLib
>    UefiBootServicesTableLib
>  
> +[LibraryClasses.common.X64]
> +  HobLib
> +
>  [Guids]
>    gEfiFileInfoGuid                              ## SOMETIMES_CONSUMES ## UNDEFINED
>  
> 

Perhaps the #include <Library/HobLib.h> directive could be guarded by
#ifdef MDE_CPU_X64 as well, just for symmetry with the HobLib lib class,
but it's not too important.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>