From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: philmd@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Mon, 23 Sep 2019 09:04:49 -0700 Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C166785538 for ; Mon, 23 Sep 2019 16:04:48 +0000 (UTC) Received: by mail-wm1-f72.google.com with SMTP id o8so4949997wmc.2 for ; Mon, 23 Sep 2019 09:04:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:openpgp:message-id :date:user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=jumV8yzCveu+98A1mKxMCXxeQD+oRXWfqFMDDTEFiQE=; b=Eeq8Q5XVFDliEM9xkqbQD0zMIltzZBbR5WPF4/GrGLyWsa3ufsf7O1H6M/a0F45RoQ UImoicnPWe56f9uHld3OxMtXxYMjyJpaX/BQUm1tInZeXSR52cLassDB47fVXhiDYm2u Y+WCIkyNZX2JqIcbZm2RugXM+XIFD2OaHXNZRu9oWu5gL2ggSIC5Er5Mu/6lC9wlKcUz UtTWEEV6Fej3KR5FLEjbgMy+B6twVMSYD2d+1XiRRIWxEH6Xd5JWJE8dKr3qClrbQwOp X8el+PL7VYNaBlsqX9+4obMZY9l35ljpWtuVAobj9AcwwjaoAMgPjGGnFwNGXXn+Rv4X ATMQ== X-Gm-Message-State: APjAAAUg02TkOfZXqOFK0tXV9/zF00HtFPxcQCuvNJPsIgSSiJO3WOmy BvZIiq0rtj7hUUHXCjWObxr6WQ0DpFstZ6uDiSI7xYd1rHoZLbWAf/XNxDJya7ztd4LRlzRdw9j 8IpamlcccyMydkQ== X-Received: by 2002:a05:600c:54a:: with SMTP id k10mr300594wmc.127.1569254687245; Mon, 23 Sep 2019 09:04:47 -0700 (PDT) X-Google-Smtp-Source: APXvYqzkm4rb5WtgFQoWmeWxwIpbGQFGMc4pG6lE2SHHbIUlDeHSGk+u/AywZP9jsEI16jGMfiMt/g== X-Received: by 2002:a05:600c:54a:: with SMTP id k10mr300581wmc.127.1569254687051; Mon, 23 Sep 2019 09:04:47 -0700 (PDT) Received: from [192.168.1.115] (240.red-88-21-68.staticip.rima-tde.net. [88.21.68.240]) by smtp.gmail.com with ESMTPSA id i5sm10964138wmd.21.2019.09.23.09.04.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 23 Sep 2019 09:04:46 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH 35/35] UefiPayloadPkg/BlSupportDxe: fix ReserveResourceInGcd() calls To: "Dong, Guo" , "devel@edk2.groups.io" , "lersek@redhat.com" Cc: "You, Benjamin" , "Ma, Maurice" References: <20190917194935.24322-1-lersek@redhat.com> <20190917194935.24322-36-lersek@redhat.com> <91544023-5e46-83a7-c749-84bfb0a28ccb@redhat.com> <0DE6ECBAEEB99B4DA9564FF580F3580A498EE597@FMSMSX119.amr.corp.intel.com> From: =?UTF-8?B?UGhpbGlwcGUgTWF0aGlldS1EYXVkw6k=?= Openpgp: id=89C1E78F601EE86C867495CBA2A3FD6EDEADC0DE; url=http://pgp.mit.edu/pks/lookup?op=get&search=0xA2A3FD6EDEADC0DE Message-ID: <8ff38cc1-4976-ffa1-a3e3-7b2afaa8c0b1@redhat.com> Date: Mon, 23 Sep 2019 18:04:45 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 MIME-Version: 1.0 In-Reply-To: <0DE6ECBAEEB99B4DA9564FF580F3580A498EE597@FMSMSX119.amr.corp.intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 9/23/19 6:02 PM, Dong, Guo wrote: >=20 > This is not dead code.=20 > This actual bug didn't cause issues since BlSupportDxe just allocate re= sources reported from bootloaders.=20 Ah OK, thanks Guo. > Anyway, this is a great enhancement from spec to capture such bugs. >=20 > Thanks, > Guo >=20 >> -----Original Message----- >> From: Philippe Mathieu-Daud=C3=A9 [mailto:philmd@redhat.com] >> Sent: Monday, September 23, 2019 8:08 AM >> To: devel@edk2.groups.io; lersek@redhat.com >> Cc: You, Benjamin ; Dong, Guo >> ; Ma, Maurice >> Subject: Re: [edk2-devel] [PATCH 35/35] UefiPayloadPkg/BlSupportDxe: f= ix >> 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 >>> Cc: Guo Dong >>> Cc: Maurice Ma >>> Signed-off-by: Laszlo Ersek >>> --- >>> >>> 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 =3D ReserveResourceInGcd (TRUE, >>> EfiGcdMemoryTypeMemoryMappedIo, 0xFEC00000, SIZE_4KB, 0, >> SystemTable); >>> // IOAPIC >>> + Status =3D ReserveResourceInGcd (TRUE, >>> + EfiGcdMemoryTypeMemoryMappedIo, 0xFEC00000, SIZE_4KB, 0, >>> + ImageHandle); // IOAPIC >>> ASSERT_EFI_ERROR (Status); >>> >>> - Status =3D ReserveResourceInGcd (TRUE, >>> EfiGcdMemoryTypeMemoryMappedIo, 0xFED00000, SIZE_1KB, 0, >> SystemTable); >>> // HPET >>> + Status =3D ReserveResourceInGcd (TRUE, >>> + EfiGcdMemoryTypeMemoryMappedIo, 0xFED00000, SIZE_1KB, 0, >>> + ImageHandle); // HPET >>> ASSERT_EFI_ERROR (Status); >>> >>> // >>> >> >> Reviewed-by: Philippe Mathieu-Daude