Re: [PATCH v2 5/5] MdeModulePkg/FirmwarePerformanceDataTableDxe: use AllocatePeiAccessiblePages

[Laszlo Ersek <lersek@redhat.com>]

On 05/24/18 11:09, Ard Biesheuvel wrote:
> Replace the call to and implementation of the function
> FpdtAllocateReservedMemoryBelow4G() with a call to
> AllocatePeiAccessiblePages, which boils down to the same on X64,
> but does not crash non-X64 systems that lack memory below 4 GB.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> Note that the ZeroMem() call is dropped, but it is redundant anyway, given
> that in both cases, the subsequent CopyMem() call supersedes it immediately.
> 
>  MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c   | 51 ++++----------------
>  MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf |  1 +
>  2 files changed, 10 insertions(+), 42 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
> index e719e9e482cb..ded817f37301 100644
> --- a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
> +++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
> @@ -32,6 +32,7 @@
>  #include <Library/UefiRuntimeServicesTableLib.h>
>  #include <Library/BaseLib.h>
>  #include <Library/DebugLib.h>
> +#include <Library/DxeServicesLib.h>
>  #include <Library/TimerLib.h>
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/MemoryAllocationLib.h>
> @@ -179,46 +180,6 @@ FpdtAcpiTableChecksum (
>    Buffer[ChecksumOffset] = CalculateCheckSum8 (Buffer, Size);
>  }
>  
> -/**
> -  Allocate EfiReservedMemoryType below 4G memory address.
> -
> -  This function allocates EfiReservedMemoryType below 4G memory address.
> -
> -  @param[in]  Size   Size of memory to allocate.
> -
> -  @return Allocated address for output.
> -
> -**/
> -VOID *
> -FpdtAllocateReservedMemoryBelow4G (
> -  IN UINTN       Size
> -  )
> -{
> -  UINTN                 Pages;
> -  EFI_PHYSICAL_ADDRESS  Address;
> -  EFI_STATUS            Status;
> -  VOID                  *Buffer;
> -
> -  Buffer  = NULL;
> -  Pages   = EFI_SIZE_TO_PAGES (Size);
> -  Address = 0xffffffff;
> -
> -  Status = gBS->AllocatePages (
> -                  AllocateMaxAddress,
> -                  EfiReservedMemoryType,
> -                  Pages,
> -                  &Address
> -                  );
> -  ASSERT_EFI_ERROR (Status);
> -
> -  if (!EFI_ERROR (Status)) {
> -    Buffer = (VOID *) (UINTN) Address;
> -    ZeroMem (Buffer, Size);
> -  }
> -
> -  return Buffer;
> -}
> -
>  /**
>    Callback function upon VariableArchProtocol and LockBoxProtocol
>    to allocate S3 performance table memory and save the pointer to LockBox.
> @@ -287,7 +248,10 @@ FpdtAllocateS3PerformanceTableMemory (
>          //
>          // Fail to allocate at specified address, continue to allocate at any address.
>          //
> -        mAcpiS3PerformanceTable = (S3_PERFORMANCE_TABLE *) FpdtAllocateReservedMemoryBelow4G (sizeof (S3_PERFORMANCE_TABLE));
> +        mAcpiS3PerformanceTable = (S3_PERFORMANCE_TABLE *) AllocatePeiAccessiblePages (
> +                                                             EfiReservedMemoryType,
> +                                                             EFI_SIZE_TO_PAGES (sizeof (S3_PERFORMANCE_TABLE))
> +                                                             );
>        }
>        DEBUG ((EFI_D_INFO, "FPDT: ACPI S3 Performance Table address = 0x%x\n", mAcpiS3PerformanceTable));
>        if (mAcpiS3PerformanceTable != NULL) {
> @@ -368,7 +332,10 @@ InstallFirmwarePerformanceDataTable (
>        //
>        // Fail to allocate at specified address, continue to allocate at any address.
>        //
> -      mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
> +      mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) AllocatePeiAccessiblePages (
> +                                                               EfiReservedMemoryType,
> +                                                               EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
> +                                                               );
>      }
>      DEBUG ((DEBUG_INFO, "FPDT: ACPI Boot Performance Table address = 0x%x\n", mAcpiBootPerformanceTable));
>      if (mAcpiBootPerformanceTable == NULL) {
> diff --git a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
> index 8757bbd0aaa9..3d2dd6eb732f 100644
> --- a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
> +++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
> @@ -44,6 +44,7 @@ [LibraryClasses]
>    UefiRuntimeServicesTableLib
>    BaseLib
>    DebugLib
> +  DxeServicesLib
>    TimerLib
>    BaseMemoryLib
>    MemoryAllocationLib
> 

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