* [PATCH] MdeModulePkg/BootGraphicsResourceTableDxe: don't allocate below 4 GB
@ 2017-03-19 17:19 Ard Biesheuvel
2017-03-20 1:09 ` Zeng, Star
0 siblings, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2017-03-19 17:19 UTC (permalink / raw)
To: edk2-devel, star.zeng, feng.tian; +Cc: Ard Biesheuvel
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/BootGraphicsResourceTableDxe.c | 41 ++------------------
1 file changed, 3 insertions(+), 38 deletions(-)
diff --git a/MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.c b/MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.c
index 804ffa5a6bb6..6a7165a95489 100644
--- a/MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.c
+++ b/MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] MdeModulePkg/BootGraphicsResourceTableDxe: don't allocate below 4 GB
2017-03-19 17:19 [PATCH] MdeModulePkg/BootGraphicsResourceTableDxe: don't allocate below 4 GB Ard Biesheuvel
@ 2017-03-20 1:09 ` Zeng, Star
2017-03-21 2:37 ` Gao, Liming
0 siblings, 1 reply; 4+ messages in thread
From: Zeng, Star @ 2017-03-20 1:09 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org, Tian, Feng
Cc: Yao, Jiewen, Gao, Liming, Zeng, Star
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/BootGraphicsResourceTableDxe.c | 41 ++------------------
1 file changed, 3 insertions(+), 38 deletions(-)
diff --git a/MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.c b/MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.c
index 804ffa5a6bb6..6a7165a95489 100644
--- a/MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.c
+++ b/MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraph
+++ 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] MdeModulePkg/BootGraphicsResourceTableDxe: don't allocate below 4 GB
2017-03-20 1:09 ` Zeng, Star
@ 2017-03-21 2:37 ` Gao, Liming
2017-03-21 7:11 ` Ard Biesheuvel
0 siblings, 1 reply; 4+ messages in thread
From: Gao, Liming @ 2017-03-21 2:37 UTC (permalink / raw)
To: Zeng, Star, Ard Biesheuvel, edk2-devel@lists.01.org, Tian, Feng
Cc: Yao, Jiewen
Yes. Spec has no such limitation. I agree this change.
Reviewed-by: Liming Gao <liming.gao@intel.com>
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] MdeModulePkg/BootGraphicsResourceTableDxe: don't allocate below 4 GB
2017-03-21 2:37 ` Gao, Liming
@ 2017-03-21 7:11 ` Ard Biesheuvel
0 siblings, 0 replies; 4+ messages in thread
From: Ard Biesheuvel @ 2017-03-21 7:11 UTC (permalink / raw)
To: Gao, Liming; +Cc: Zeng, Star, edk2-devel@lists.01.org, Tian, Feng, Yao, Jiewen
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-03-21 7:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-19 17:19 [PATCH] MdeModulePkg/BootGraphicsResourceTableDxe: don't allocate below 4 GB Ard Biesheuvel
2017-03-20 1:09 ` Zeng, Star
2017-03-21 2:37 ` Gao, Liming
2017-03-21 7:11 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox