Re: [PATCH] MdeModulePkg/BootGraphicsResourceTableDxe: don't allocate below 4 GB

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 21 March 2017 at 02:37, Gao, Liming <liming.gao@intel.com> wrote:
> Yes. Spec has no such limitation. I agree this change.
>
> Reviewed-by: Liming Gao <liming.gao@intel.com>
>

Pushed as 09da11081915, thanks.

> Thanks
> Liming
>>-----Original Message-----
>>From: Zeng, Star
>>Sent: Monday, March 20, 2017 9:10 AM
>>To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org;
>>Tian, Feng <feng.tian@intel.com>
>>Cc: Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming
>><liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
>>Subject: RE: [PATCH] MdeModulePkg/BootGraphicsResourceTableDxe: don't
>>allocate below 4 GB
>>
>>I am ok with this patch.
>>
>>Jiewen and Liming, do you have any comments?
>>
>>Thanks,
>>Star
>>-----Original Message-----
>>From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>>Sent: Monday, March 20, 2017 1:20 AM
>>To: edk2-devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; Tian, Feng
>><feng.tian@intel.com>
>>Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>Subject: [PATCH] MdeModulePkg/BootGraphicsResourceTableDxe: don't
>>allocate below 4 GB
>>
>>The BGRT table has an 8 byte field for the memory address of the image data,
>>and yet the driver explicitly allocates below 4 GB. This results in an ASSERT()
>>on systems that do not have any memory below 4 GB to begin with.
>>
>>Since neither the PI, the UEFI or the ACPI spec contain any mention of why
>>this data should reside below 4 GB, replace the allocation call with an ordinary
>>AllocatePages() call.
>>
>>Contributed-under: TianoCore Contribution Agreement 1.0
>>Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>---
>>
>>MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphi
>>csResourceTableDxe.c | 41 ++------------------
>> 1 file changed, 3 insertions(+), 38 deletions(-)
>>
>>diff --git
>>a/MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGrap
>>hicsResourceTableDxe.c
>>b/MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGrap
>>hicsResourceTableDxe.c
>>index 804ffa5a6bb6..6a7165a95489 100644
>>---
>>a/MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGrap
>>hicsResourceTableDxe.c
>>+++
>>b/MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGrap
>>h
>>+++ icsResourceTableDxe.c
>>@@ -227,43 +227,6 @@ BgrtAcpiTableChecksum (  }
>>
>> /**
>>-  Allocate EfiBootServicesData below 4G memory address.
>>-
>>-  This function allocates EfiBootServicesData below 4G memory address.
>>-
>>-  @param[in]  Size   Size of memory to allocate.
>>-
>>-  @return Allocated address for output.
>>-
>>-**/
>>-VOID *
>>-BgrtAllocateBsDataMemoryBelow4G (
>>-  IN UINTN       Size
>>-  )
>>-{
>>-  UINTN                 Pages;
>>-  EFI_PHYSICAL_ADDRESS  Address;
>>-  EFI_STATUS            Status;
>>-  VOID                  *Buffer;
>>-
>>-  Pages   = EFI_SIZE_TO_PAGES (Size);
>>-  Address = 0xffffffff;
>>-
>>-  Status = gBS->AllocatePages (
>>-                  AllocateMaxAddress,
>>-                  EfiBootServicesData,
>>-                  Pages,
>>-                  &Address
>>-                  );
>>-  ASSERT_EFI_ERROR (Status);
>>-
>>-  Buffer = (VOID *) (UINTN) Address;
>>-  ZeroMem (Buffer, Size);
>>-
>>-  return Buffer;
>>-}
>>-
>>-/**
>>   Install Boot Graphics Resource Table to ACPI table.
>>
>>   @return Status code.
>>@@ -358,11 +321,13 @@ InstallBootGraphicsResourceTable (
>>     // The image should be stored in EfiBootServicesData, allowing the system
>>to reclaim the memory
>>     //
>>     BmpSize = (mLogoWidth * 3 + PaddingSize) * mLogoHeight + sizeof
>>(BMP_IMAGE_HEADER);
>>-    ImageBuffer = BgrtAllocateBsDataMemoryBelow4G (BmpSize);
>>+    ImageBuffer = AllocatePages (EFI_SIZE_TO_PAGES (BmpSize));
>>     if (ImageBuffer == NULL) {
>>       return EFI_OUT_OF_RESOURCES;
>>     }
>>
>>+    ZeroMem (ImageBuffer, BmpSize);
>>+
>>     mBmpImageHeaderTemplate.Size = (UINT32) BmpSize;
>>     mBmpImageHeaderTemplate.ImageSize = (UINT32) BmpSize - sizeof
>>(BMP_IMAGE_HEADER);
>>     mBmpImageHeaderTemplate.PixelWidth = (UINT32) mLogoWidth;
>>--
>>2.7.4
>