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:philmd@redhat.com]
>> Sent: Monday, September 23, 2019 8:08 AM
>> To: devel@edk2.groups.io; lersek@redhat.com
>> Cc: You, Benjamin <benjamin.you@intel.com>; Dong, Guo
>> <guo.dong@intel.com>; Ma, Maurice <maurice.ma@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.you@intel.com>
>>> Cc: Guo Dong <guo.dong@intel.com>
>>> Cc: Maurice Ma <maurice.ma@intel.com>
>>> Signed-off-by: Laszlo Ersek <lersek@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 <philmd@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-