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:phi...@redhat.com] >> Sent: Monday, September 23, 2019 8:08 AM >> To: devel@edk2.groups.io; ler...@redhat.com >> Cc: You, Benjamin <benjamin....@intel.com>; Dong, Guo >> <guo.d...@intel.com>; Ma, Maurice <maurice...@intel.com> >> 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 <benjamin....@intel.com> >>> Cc: Guo Dong <guo.d...@intel.com> >>> Cc: Maurice Ma <maurice...@intel.com> >>> Signed-off-by: Laszlo Ersek <ler...@redhat.com> >>> --- >>> >>> 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 <phi...@redhat.com> -=-=-=-=-=-=-=-=-=-=-=- 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: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
