On 9/23/19 6:02 PM, Dong, Guo wrote: > > This is not dead code. > This actual bug didn't cause issues since BlSupportDxe just allocate > resources reported from bootloaders.
Ah OK, thanks Guo. > Anyway, this is a great enhancement from spec to capture such bugs. > > Thanks, > Guo > >> -----Original Message----- >> From: Philippe Mathieu-Daudé [mailto:[email protected]] >> Sent: Monday, September 23, 2019 8:08 AM >> To: [email protected]; [email protected] >> Cc: You, Benjamin <[email protected]>; Dong, Guo >> <[email protected]>; Ma, Maurice <[email protected]> >> Subject: Re: [edk2-devel] [PATCH 35/35] UefiPayloadPkg/BlSupportDxe: fix >> ReserveResourceInGcd() calls >> >> On 9/17/19 9:49 PM, Laszlo Ersek wrote: >>> The last parameter of ReserveResourceInGcd() is "ImageHandle", >>> forwarded in turn to gDS->AllocateMemorySpace() or gDS- >>> AllocateIoSpace() as "owner" >>> image handle. >>> >>> But BlDxeEntryPoint() passes "SystemTable" as "ImageHandle". >>> >>> Compilers have not flagged it because EFI_HANDLE (the type of >>> "ImageHandle") is unfortunately specified as (VOID*), and >>> (EFI_SYSTEM_TABLE*) converts to (VOID*) silently. >>> >>> Hand the entry point function's "ImageHandle" parameter to >>> ReserveResourceInGcd(). This fixes an actual bug. >> >> Wow very buggy, so I assume this is mostly dead code, right? >> >>> Cc: Benjamin You <[email protected]> >>> Cc: Guo Dong <[email protected]> >>> Cc: Maurice Ma <[email protected]> >>> Signed-off-by: Laszlo Ersek <[email protected]> >>> --- >>> >>> Notes: >>> build-tested only >>> >>> UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c >>> b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c >>> index bcee4cd9bc41..28dfc8fc5545 100644 >>> --- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c >>> +++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c >>> @@ -106,10 +106,10 @@ BlDxeEntryPoint ( >>> // >>> // Report MMIO/IO Resources >>> // >>> - Status = ReserveResourceInGcd (TRUE, >>> EfiGcdMemoryTypeMemoryMappedIo, 0xFEC00000, SIZE_4KB, 0, >> SystemTable); >>> // IOAPIC >>> + Status = ReserveResourceInGcd (TRUE, >>> + EfiGcdMemoryTypeMemoryMappedIo, 0xFEC00000, SIZE_4KB, 0, >>> + ImageHandle); // IOAPIC >>> ASSERT_EFI_ERROR (Status); >>> >>> - Status = ReserveResourceInGcd (TRUE, >>> EfiGcdMemoryTypeMemoryMappedIo, 0xFED00000, SIZE_1KB, 0, >> SystemTable); >>> // HPET >>> + Status = ReserveResourceInGcd (TRUE, >>> + EfiGcdMemoryTypeMemoryMappedIo, 0xFED00000, SIZE_1KB, 0, >>> + ImageHandle); // HPET >>> ASSERT_EFI_ERROR (Status); >>> >>> // >>> >> >> Reviewed-by: Philippe Mathieu-Daude <[email protected]> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47860): https://edk2.groups.io/g/devel/message/47860 Mute This Topic: https://groups.io/mt/34180240/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
