From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.24; helo=mga09.intel.com; envelope-from=liming.gao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 68B552270D32B for ; Thu, 12 Apr 2018 08:16:59 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Apr 2018 08:16:58 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,442,1517904000"; d="scan'208";a="32179275" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga008.fm.intel.com with ESMTP; 12 Apr 2018 08:16:58 -0700 Received: from fmsmsx118.amr.corp.intel.com (10.18.116.18) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 12 Apr 2018 08:16:58 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx118.amr.corp.intel.com (10.18.116.18) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 12 Apr 2018 08:16:57 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.239]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.43]) with mapi id 14.03.0319.002; Thu, 12 Apr 2018 23:16:55 +0800 From: "Gao, Liming" To: Laszlo Ersek , Ard Biesheuvel CC: "edk2-devel@lists.01.org" , Leif Lindholm Thread-Topic: [edk2] [PATCH 09/10] ArmVirtPkg/PlatformHasAcpiDtDxe: depend on gEfiVariableArchProtocolGuid Thread-Index: AQHT0fkaaxe1tSFUAECxKqYRK4QCXaP8JNiAgAAsCgCAAOq/gA== Date: Thu, 12 Apr 2018 15:16:54 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E20D3F9@SHSMSX104.ccr.corp.intel.com> References: <20180412005540.26651-1-lersek@redhat.com> <20180412005540.26651-10-lersek@redhat.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH 09/10] ArmVirtPkg/PlatformHasAcpiDtDxe: depend on gEfiVariableArchProtocolGuid X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Apr 2018 15:16:59 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Laszlo: We also have the similar idea that auto appends Variable read or Variable= write protocol. Build tool can know which PCD is configured as DynamicHii.= But, it doesn't know whether this PCD is consumed or produced in the drive= r entry point. If PCD is used in driver entry point, Build tool needs to ap= pend Variable arch protocol depex. Otherwise, BaseTool doesn't need do it. = Further, I think BaseTools can use PCD usage described in Pcd section. CONS= UMES means PCD read in the driver entry point, PRODUCES means PCD write in = the driver entry point. Seemly, this solution can work well. But, it depend= s on the correct PCD usage in module INF. If PCD usage is not described or = wrongly described, it may cause system boot failure. Consider its impact, w= e have not added this support.=20 [Pcd] gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## SOMETIMES_CONS= UMES gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## PRODUCES gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## SOMETIMES_PROD= UCES Thanks Liming > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of La= szlo Ersek > Sent: Thursday, April 12, 2018 5:06 PM > To: Ard Biesheuvel > Cc: edk2-devel@lists.01.org; Leif Lindholm > Subject: Re: [edk2] [PATCH 09/10] ArmVirtPkg/PlatformHasAcpiDtDxe: depend= on gEfiVariableArchProtocolGuid >=20 > On 04/12/18 08:28, Ard Biesheuvel wrote: > > On 12 April 2018 at 02:55, Laszlo Ersek wrote: > >> PlatformHasAcpiDtDxe consumes the DynamicHii PCD called > >> "gArmVirtTokenSpaceGuid.PcdForceNoAcpi". The PcdGetBool() library call > >> terminates in gRT->GetVariable(), in the MdeModulePkg/Universal/PCD/Dx= e > >> driver. Put "gEfiVariableArchProtocolGuid" on PlatformHasAcpiDtDxe's D= EPEX > >> so that we not attempt the call before the PCD driver can successfully > >> read non-volatile variables. > >> > > > > OK, so does this mean that UefiRuntimeServicesTableLib deliberately > > does not depex on gEfiVariableArchProtocolGuid, >=20 > Yes, I do think so; the runtime services are expected to become > available gradually. For example, "MdePkg/Include/Protocol/Variable.h" > has a great long comment on this. >=20 > > and it will return > > EFI_NOT_AVAILABLE_YET when called too early? If that is the case, I > > would assume it is the responsibility of the PcdLib implementation to > > deal with that, not the PcdLib clients. >=20 > Yes, this idea occurred to me as well, but I think it is not correct. > The PcdLib instance for the DXE phase is responsible for talking to the > PCD protocol, and indeed it has a depex on that protocol > (MdePkg/Library/DxePcdLib/DxePcdLib.inf). However, if a module uses > dynamic PCDs, but none of those dynamic PCDs are HII (i.e., backed by > non-volatile UEFI variables), then IMO neither the PcdLib instance, nor > the PCD DXE driver, should depend on the variable read service > (gEfiVariableArchProtocolGuid). >=20 > (Well the PCD DXE driver is shared / used by all of the DXE modules, so > it couldn't depend on gEfiVariableArchProtocolGuid in any case at all.) >=20 > Ultimately, in the optimal case, the PcdLib instance for DXE should > "know" if any PCDs used by the main driver module are DynamicHII, and in > that case, the PcdLib instance should create a dependency on the > variable read (and possibly write) services. I think edk2 simply doesn't > support this use case at this moment (it is ultimately a DSC decision > what type we give to a PCD, choosing from the possibilities listed in > the DEC), so we have to take care of it ourselves. I do see that this > limits the reusability of individual drivers across different DSC files > -- if another DSC makes the same PCD fixed, for example, then the > gEfiVariableArchProtocolGuid dependency becomes useless / needlessly > restrictive in the driver. >=20 > We could factor out just the gEfiVariableArchProtocolGuid depex into > another NULL class lib, and hook that into PlatformHasAcpiDtDxe in the > DSC file, because in the DSC we do know that the PCD is DynamicHII. If > you feel strongly about it, it's doable, but I certainly think it's > overkill. PlatformHasAcpiDtDxe got "Platform" in its name; it's pretty > unlikely to be reused in any other DSC. (Even ArmVirtXen has a different > implementation, which does not use the PCD.) I feel the proposed patch > should be fine -- what's your take? >=20 > Thanks! > Laszlo >=20 > > > > > >> Cc: Ard Biesheuvel > >> Cc: Leif Lindholm > >> Cc: Steve Capper > >> Cc: Supreeth Venkatesh > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: Laszlo Ersek > >> --- > >> ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf > b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf > >> index 5351e741aa41..8346f7f24b32 100644 > >> --- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf > >> +++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf > >> @@ -44,7 +44,7 @@ [Guids] > >> gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL > >> > >> [Pcd] > >> gArmVirtTokenSpaceGuid.PcdForceNoAcpi > >> > >> [Depex] > >> - TRUE > >> + gEfiVariableArchProtocolGuid > >> -- > >> 2.14.1.3.gb7cf6e02401b > >> > >> >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel