From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 4D34980472 for ; Tue, 21 Mar 2017 02:05:26 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7A2D981243; Tue, 21 Mar 2017 09:05:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 7A2D981243 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 7A2D981243 Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-42.phx2.redhat.com [10.3.116.42]) by smtp.corp.redhat.com (Postfix) with ESMTP id 95A4B7849C; Tue, 21 Mar 2017 09:05:24 +0000 (UTC) To: "Zeng, Star" , edk2-devel-01 , "Kinney, Michael D" , "Yao, Jiewen" References: <20170317204731.31488-1-lersek@redhat.com> <20170317204731.31488-6-lersek@redhat.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B835B4B@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B8360CA@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B836569@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B836A43@shsmsx102.ccr.corp.intel.com> Cc: Leif Lindholm , Ard Biesheuvel From: Laszlo Ersek Message-ID: <251675d6-6748-c2c6-4c9e-44495716a894@redhat.com> Date: Tue, 21 Mar 2017 10:05:22 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103B836A43@shsmsx102.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Tue, 21 Mar 2017 09:05:26 +0000 (UTC) Subject: Re: [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Mar 2017 09:05:26 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 03/21/17 09:41, Zeng, Star wrote: > It is not been implemented yet. Just for our discussion. I should use future tense in the sentence, sorry. Ah, I see. Yeah, it makes sense. (Although it still wouldn't allow us to avoid a NULL class library plug-in for AcpiTableDxe, for setting the PCD just in time -- the information source for the PCD is generally available to us only within the DXE phase, not in PEI.) Thanks Laszlo > Thanks, > Star > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek > Sent: Tuesday, March 21, 2017 4:38 PM > To: Zeng, Star ; edk2-devel-01 ; Kinney, Michael D ; Yao, Jiewen > Cc: Leif Lindholm ; Ard Biesheuvel > Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library > > On 03/21/17 03:27, Zeng, Star wrote: >> Just an idea. >> >> There is a way to do not introduce a new PCD, but reuse PcdAcpiExposedTableVersions. >> When PcdAcpiExposedTableVersions is configured to *0* (means no ANY ACPI table should be exposed), AcpiTableDxe returns EFI_UNSUPPORTED in the driver entrypoint. > > Hm, I don't see that: > > $ git grep EFI_UNSUPPORTED -- MdeModulePkg/Universal/Acpi/AcpiTableDxe/ > [no matches] > > Thanks > Laszlo > >> >> >> Thanks, >> Star >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of >> Zeng, Star >> Sent: Monday, March 20, 2017 5:57 PM >> To: Laszlo Ersek ; edk2-devel-01 >> ; Kinney, Michael D >> ; Yao, Jiewen >> Cc: Leif Lindholm ; Ard Biesheuvel >> >> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform >> Has ACPI Protocol, and plug-in library >> >> Oh got it, returning unsuccessfully in constructor of library does not >> work. :( >> >> For (1), I definitely have no any comments. :) For (2), I got no strong reason to support or reject it. We'd better could get comments from Mike and Jiewen. >> >> Thanks, >> Star >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of >> Laszlo Ersek >> Sent: Monday, March 20, 2017 5:18 PM >> To: Zeng, Star ; edk2-devel-01 >> >> Cc: Leif Lindholm ; Ard Biesheuvel >> >> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform >> Has ACPI Protocol, and plug-in library >> >> On 03/20/17 03:23, Zeng, Star wrote: >>> Laszlo, >>> >>> Could the implementation have multiple instances of >>> PlatformHasAcpiLib to return unsuccessfully in the constructor when >>> EFI_ACPI_TABLE_PROTOCOL is not needed? And then eliminate >>> PlatformHasAcpiDtDxe, gEdkiiPlatformHasAcpiProtocolGuid and >>> gEdkiiPlatformHasDeviceTreeProtocolGuid? >> >> If a library constructor fails, then the client module does not see an explicit error (or automatic unloading); instead, an assertion failure is triggered in auto-generated code. >> >> For example, in the >> "Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/AARCH64/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe/DEBUG/AutoGen.c" >> file, with this series applied, we find: >> >>> VOID >>> EFIAPI >>> ProcessLibraryConstructorList ( >>> IN EFI_HANDLE ImageHandle, >>> IN EFI_SYSTEM_TABLE *SystemTable >>> ) >>> { >>> EFI_STATUS Status; >>> >>> Status = HobLibConstructor (ImageHandle, SystemTable); >>> ASSERT_EFI_ERROR (Status); >>> >>> Status = FdtPL011SerialPortLibInitialize (); >>> ASSERT_EFI_ERROR (Status); >>> >>> Status = BaseDebugLibSerialPortConstructor (); >>> ASSERT_EFI_ERROR (Status); >>> >>> Status = UefiBootServicesTableLibConstructor (ImageHandle, SystemTable); >>> ASSERT_EFI_ERROR (Status); >>> >>> Status = UefiRuntimeServicesTableLibConstructor (ImageHandle, SystemTable); >>> ASSERT_EFI_ERROR (Status); >>> >>> Status = UefiLibConstructor (ImageHandle, SystemTable); >>> ASSERT_EFI_ERROR (Status); >>> >>> Status = PlatformHasAcpiInitialize (); <--------------- constructor of PlatformHasAcpiLib plug-in lib >>> ASSERT_EFI_ERROR (Status); >>> >>> } >> >> What we could do instead would be a new library class, with an explicit initialization function. However, for that to work, AcpiTableDxe would have to be extended with a new library class dependency and a call to the init function. Additionally, out-of-tree platform DSC files would have to resolve this library class to the default instance. >> >> Introducing a boolean dynamic PCD for the same would be much simpler then; the DEC default would cover out-of-tree platform DSCs without any change in behavior. >> >> In summary, there are two alternatives in my opinion: >> >> (1) If we prefer not to modify MdeModulePkg: stick with this series. The only update I'd see justified would be to move gEdkiiPlatformHasDeviceTreeProtocolGuid to EmbeddedPkg as Leif requested. >> >> (2) If we are willing to modify MdeModulePkg for this problem: introduce a new dynamic PCD such as "PcdAcpiTableEnable", and make AcpiTableDxe exit early if the PCD is FALSE. >> >> Thanks, >> Laszlo >> >>> >>> Thanks, >>> Star >>> -----Original Message----- >>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf >>> Of Laszlo Ersek >>> Sent: Saturday, March 18, 2017 4:47 AM >>> To: edk2-devel-01 >>> Cc: Leif Lindholm ; Ard Biesheuvel >>> >>> Subject: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has >>> ACPI Protocol, and plug-in library >>> >>> The presence of this protocol in the DXE protocol database implies that the platform provides the operating system with an ACPI-based hardware description. This is not necessarily mutually exclusive with a Device Tree-based hardware description. A platform driver is supposed to produce a single instance of the protocol (with NULL contents), if appropriate. >>> >>> The decision to produce the protocol is platform specific; for example, it could depend on an HII checkbox / underlying non-volatile UEFI variable. >>> >>> The protocol is meant to be consumed by >>> "MdeModulePkg/Universal/Acpi/AcpiTableDxe", through the PlatformHasAcpiLib plug-in. By linking this library into AcpiTableDxe via NULL resolution in the platform DSC, the platform makes EFI_ACPI_TABLE_PROTOCOL and (if enabled) EFI_ACPI_SDT_PROTOCOL dependent on the above dynamic decision. >>> >>> In turn, other (platform and universal) DXE drivers that produce ACPI tables will wait for EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL, via DEPEX, protocol notify, or a simple gBS->LocateProtocol() in a "late enough" callback (such as Ready To Boot). >>> >>> Because this protocol is not standard, it is prefixed with EDKII / Edkii, as seen elsewhere in MdeModulePkg and SecurityPkg, for example. (ARM / Arm doesn't look future-proof enough; future UEFI platforms could face the same issue.) In addition, an effort is made to avoid the phrase "AcpiPlatform", as that belongs to drivers / libraries that produce platform specific ACPI content (as opposed to deciding whether the entire firmware will have access to EFI_ACPI_TABLE_PROTOCOL). >>> >>> Cc: Ard Biesheuvel >>> Cc: Leif Lindholm >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Laszlo Ersek >>> --- >>> ArmPkg/ArmPkg.dec | 4 ++ >>> ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf | 40 ++++++++++++++++++++ >>> ArmPkg/Include/Protocol/PlatformHasAcpi.h | 34 +++++++++++++++++ >>> ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.c | 36 ++++++++++++++++++ >>> 4 files changed, 114 insertions(+) >>> >>> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec index >>> c4b4da2f95bb..0e49360a386a 100644 >>> --- a/ArmPkg/ArmPkg.dec >>> +++ b/ArmPkg/ArmPkg.dec >>> @@ -52,6 +52,10 @@ [Ppis] >>> ## Include/Ppi/ArmMpCoreInfo.h >>> gArmMpCoreInfoPpiGuid = { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d, >>> 0xab, 0x44, 0xe7, 0x54, 0xa8, 0xfc} } >>> >>> +[Protocols] >>> + ## Include/Protocol/PlatformHasAcpi.h >>> + gEdkiiPlatformHasAcpiProtocolGuid = { 0xf0966b41, 0xc23f, 0x41b9, { 0x96, 0x04, 0x0f, 0xf7, 0xe1, 0x11, 0x96, 0x5a } } >>> + >>> [PcdsFeatureFlag.common] >>> >>> gArmTokenSpaceGuid.PcdCpuDxeProduceDebugSupport|FALSE|BOOLEAN|0x00000 >>> 0 >>> 01 >>> >>> diff --git a/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf >>> b/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf >>> new file mode 100644 >>> index 000000000000..c83da4d8e98a >>> --- /dev/null >>> +++ b/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf >>> @@ -0,0 +1,40 @@ >>> +## @file >>> +# A hook-in library for MdeModulePkg/Universal/Acpi/AcpiTableDxe. >>> +# >>> +# Plugging this library instance into AcpiTableDxe makes # >>> +EFI_ACPI_TABLE_PROTOCOL and (if enabled) EFI_ACPI_SDT_PROTOCOL >>> +depend on the # platform's dynamic decision whether to expose an >>> +ACPI-based hardware # description to the operating system. >>> +# >>> +# Universal and platform specific DXE drivers that produce ACPI >>> +tables depend # on EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL in turn. >>> +# >>> +# Copyright (C) 2017, Red Hat, Inc. >>> +# >>> +# This program and the accompanying materials are licensed and made >>> +available # under the terms and conditions of the BSD License which >>> +accompanies this # distribution. The full text of the license may be >>> +found at # http://opensource.org/licenses/bsd-license.php >>> +# >>> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >>> +BASIS, WITHOUT # WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >>> +## >>> + >>> +[Defines] >>> + INF_VERSION = 1.25 >>> + BASE_NAME = PlatformHasAcpiLib >>> + FILE_GUID = 29beb028-0958-447b-be0a-12229235d77d >>> + MODULE_TYPE = BASE >>> + VERSION_STRING = 1.0 >>> + LIBRARY_CLASS = PlatformHasAcpiLib|DXE_DRIVER >>> + CONSTRUCTOR = PlatformHasAcpiInitialize >>> + >>> +[Sources] >>> + PlatformHasAcpiLib.c >>> + >>> +[Packages] >>> + ArmPkg/ArmPkg.dec >>> + MdePkg/MdePkg.dec >>> + >>> +[Depex] >>> + gEdkiiPlatformHasAcpiProtocolGuid >>> diff --git a/ArmPkg/Include/Protocol/PlatformHasAcpi.h >>> b/ArmPkg/Include/Protocol/PlatformHasAcpi.h >>> new file mode 100644 >>> index 000000000000..3cd0cfe4515d >>> --- /dev/null >>> +++ b/ArmPkg/Include/Protocol/PlatformHasAcpi.h >>> @@ -0,0 +1,34 @@ >>> +/** @file >>> + EDKII Platform Has ACPI Protocol >>> + >>> + The presence of this protocol in the DXE protocol database implies >>> + that the platform provides the operating system with an ACPI-based >>> + hardware description. Note that this is not necessarily mutually >>> + exclusive with a Device Tree-based hardware description. A >>> + platform driver is supposed to produce a single instance of the >>> + protocol (with NULL contents), if appropriate. >>> + >>> + Copyright (C) 2017, Red Hat, Inc. >>> + >>> + This program and the accompanying materials are licensed and made >>> + available under the terms and conditions of the BSD License that >>> + accompanies this distribution. The full text of the license may be >>> + found at http://opensource.org/licenses/bsd-license.php. >>> + >>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >>> +BASIS, WITHOUT >>> + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >>> +**/ >>> + >>> + >>> +#ifndef __EDKII_PLATFORM_HAS_ACPI_PROTOCOL_H__ >>> +#define __EDKII_PLATFORM_HAS_ACPI_PROTOCOL_H__ >>> + >>> +#define EDKII_PLATFORM_HAS_ACPI_PROTOCOL_GUID \ >>> + { \ >>> + 0xf0966b41, 0xc23f, 0x41b9, \ >>> + { 0x96, 0x04, 0x0f, 0xf7, 0xe1, 0x11, 0x96, 0x5a } \ >>> + } >>> + >>> +extern EFI_GUID gEdkiiPlatformHasAcpiProtocolGuid; >>> + >>> +#endif >>> diff --git a/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.c >>> b/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.c >>> new file mode 100644 >>> index 000000000000..79072da21c2b >>> --- /dev/null >>> +++ b/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.c >>> @@ -0,0 +1,36 @@ >>> +/** @file >>> + A hook-in library for MdeModulePkg/Universal/Acpi/AcpiTableDxe. >>> + >>> + Plugging this library instance into AcpiTableDxe makes >>> + EFI_ACPI_TABLE_PROTOCOL and (if enabled) EFI_ACPI_SDT_PROTOCOL >>> + depend on the platform's dynamic decision whether to expose an >>> + ACPI-based hardware description to the operating system. >>> + >>> + Universal and platform specific DXE drivers that produce ACPI >>> + tables depend on EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL in turn. >>> + >>> + Copyright (C) 2017, Red Hat, Inc. >>> + >>> + This program and the accompanying materials are licensed and made >>> + available under the terms and conditions of the BSD License which >>> + accompanies this distribution. The full text of the license may be >>> + found at http://opensource.org/licenses/bsd-license.php >>> + >>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >>> +BASIS, WITHOUT >>> + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >>> +**/ >>> + >>> +#include >>> + >>> +RETURN_STATUS >>> +EFIAPI >>> +PlatformHasAcpiInitialize ( >>> + VOID >>> + ) >>> +{ >>> + // >>> + // Do nothing, just imbue AcpiTableDxe with an >>> + // EDKII_PLATFORM_HAS_ACPI_PROTOCOL dependency. >>> + // >>> + return RETURN_SUCCESS; >>> +} >>> -- >>> 2.9.3 >>> >>> >>> _______________________________________________ >>> edk2-devel mailing list >>> edk2-devel@lists.01.org >>> https://lists.01.org/mailman/listinfo/edk2-devel >>> >> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel >> > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel >