From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (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 9205D80385 for ; Tue, 21 Mar 2017 01:41:27 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga105.fm.intel.com with ESMTP; 21 Mar 2017 01:41:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,198,1486454400"; d="scan'208";a="63044966" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga002.jf.intel.com with ESMTP; 21 Mar 2017 01:41:26 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 21 Mar 2017 01:41:26 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.88]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.132]) with mapi id 14.03.0248.002; Tue, 21 Mar 2017 16:41:24 +0800 From: "Zeng, Star" To: Laszlo Ersek , edk2-devel-01 , "Kinney, Michael D" , "Yao, Jiewen" CC: Leif Lindholm , Ard Biesheuvel , "Zeng, Star" Thread-Topic: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library Thread-Index: AQHSn1/Ct9FsN6en9E+84QMKKTQOM6GdAZxQ///vKwCAAItusIABFthA///k6wCAAIahsA== Date: Tue, 21 Mar 2017 08:41:23 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B836A43@shsmsx102.ccr.corp.intel.com> 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> In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 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 08:41:27 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable It is not been implemented yet. Just for our discussion. I should use futur= e tense in the sentence, sorry. Thanks, Star -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Lasz= lo 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 A= CPI Protocol, and plug-in library On 03/21/17 03:27, Zeng, Star wrote: > Just an idea. >=20 > There is a way to do not introduce a new PCD, but reuse PcdAcpiExposedTab= leVersions. > When PcdAcpiExposedTableVersions is configured to *0* (means no ANY ACPI = table should be exposed), AcpiTableDxe returns EFI_UNSUPPORTED in the drive= r entrypoint. Hm, I don't see that: $ git grep EFI_UNSUPPORTED -- MdeModulePkg/Universal/Acpi/AcpiTableDxe/ [no matches] Thanks Laszlo >=20 >=20 > Thanks, > Star > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of=20 > Zeng, Star > Sent: Monday, March 20, 2017 5:57 PM > To: Laszlo Ersek ; edk2-devel-01=20 > ; Kinney, Michael D=20 > ; Yao, Jiewen > Cc: Leif Lindholm ; Ard Biesheuvel=20 > > Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform=20 > Has ACPI Protocol, and plug-in library >=20 > Oh got it, returning unsuccessfully in constructor of library does not=20 > work. :( >=20 > For (1), I definitely have no any comments. :) For (2), I got no strong r= eason to support or reject it. We'd better could get comments from Mike and= Jiewen. >=20 > Thanks, > Star > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of=20 > Laszlo Ersek > Sent: Monday, March 20, 2017 5:18 PM > To: Zeng, Star ; edk2-devel-01=20 > > Cc: Leif Lindholm ; Ard Biesheuvel=20 > > Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform=20 > Has ACPI Protocol, and plug-in library >=20 > On 03/20/17 03:23, Zeng, Star wrote: >> Laszlo, >> >> Could the implementation have multiple instances of=20 >> PlatformHasAcpiLib to return unsuccessfully in the constructor when=20 >> EFI_ACPI_TABLE_PROTOCOL is not needed? And then eliminate=20 >> PlatformHasAcpiDtDxe, gEdkiiPlatformHasAcpiProtocolGuid and=20 >> gEdkiiPlatformHasDeviceTreeProtocolGuid? >=20 > If a library constructor fails, then the client module does not see an ex= plicit error (or automatic unloading); instead, an assertion failure is tri= ggered in auto-generated code. >=20 > 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: >=20 >> VOID >> EFIAPI >> ProcessLibraryConstructorList ( >> IN EFI_HANDLE ImageHandle, >> IN EFI_SYSTEM_TABLE *SystemTable >> ) >> { >> EFI_STATUS Status; >> >> Status =3D HobLibConstructor (ImageHandle, SystemTable); >> ASSERT_EFI_ERROR (Status); >> >> Status =3D FdtPL011SerialPortLibInitialize (); >> ASSERT_EFI_ERROR (Status); >> >> Status =3D BaseDebugLibSerialPortConstructor (); >> ASSERT_EFI_ERROR (Status); >> >> Status =3D UefiBootServicesTableLibConstructor (ImageHandle, SystemTab= le); >> ASSERT_EFI_ERROR (Status); >> >> Status =3D UefiRuntimeServicesTableLibConstructor (ImageHandle, System= Table); >> ASSERT_EFI_ERROR (Status); >> >> Status =3D UefiLibConstructor (ImageHandle, SystemTable); >> ASSERT_EFI_ERROR (Status); >> >> Status =3D PlatformHasAcpiInitialize (); <--------------- constructor = of PlatformHasAcpiLib plug-in lib >> ASSERT_EFI_ERROR (Status); >> >> } >=20 > What we could do instead would be a new library class, with an explicit i= nitialization function. However, for that to work, AcpiTableDxe would have = to be extended with a new library class dependency and a call to the init f= unction. Additionally, out-of-tree platform DSC files would have to resolve= this library class to the default instance. >=20 > 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. >=20 > In summary, there are two alternatives in my opinion: >=20 > (1) If we prefer not to modify MdeModulePkg: stick with this series. The = only update I'd see justified would be to move gEdkiiPlatformHasDeviceTreeP= rotocolGuid to EmbeddedPkg as Leif requested. >=20 > (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. >=20 > Thanks, > Laszlo >=20 >> >> Thanks, >> Star >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf=20 >> Of Laszlo Ersek >> Sent: Saturday, March 18, 2017 4:47 AM >> To: edk2-devel-01 >> Cc: Leif Lindholm ; Ard Biesheuvel=20 >> >> Subject: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has=20 >> 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 desc= ription. This is not necessarily mutually exclusive with a Device Tree-base= d hardware description. A platform driver is supposed to produce a single i= nstance 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=20 >> "MdeModulePkg/Universal/Acpi/AcpiTableDxe", through the PlatformHasAcpiL= ib plug-in. By linking this library into AcpiTableDxe via NULL resolution i= n the platform DSC, the platform makes EFI_ACPI_TABLE_PROTOCOL and (if enab= led) EFI_ACPI_SDT_PROTOCOL dependent on the above dynamic decision. >> >> In turn, other (platform and universal) DXE drivers that produce ACPI ta= bles will wait for EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL, via DEP= EX, protocol notify, or a simple gBS->LocateProtocol() in a "late enough" c= allback (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 / Ar= m doesn't look future-proof enough; future UEFI platforms could face the sa= me issue.) In addition, an effort is made to avoid the phrase "AcpiPlatform= ", as that belongs to drivers / libraries that produce platform specific AC= PI content (as opposed to deciding whether the entire firmware will have ac= cess 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=20 >> c4b4da2f95bb..0e49360a386a 100644 >> --- a/ArmPkg/ArmPkg.dec >> +++ b/ArmPkg/ArmPkg.dec >> @@ -52,6 +52,10 @@ [Ppis] >> ## Include/Ppi/ArmMpCoreInfo.h >> gArmMpCoreInfoPpiGuid =3D { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d,= =20 >> 0xab, 0x44, 0xe7, 0x54, 0xa8, 0xfc} } >> >> +[Protocols] >> + ## Include/Protocol/PlatformHasAcpi.h >> + gEdkiiPlatformHasAcpiProtocolGuid =3D { 0xf0966b41, 0xc23f, 0x4= 1b9, { 0x96, 0x04, 0x0f, 0xf7, 0xe1, 0x11, 0x96, 0x5a } } >> + >> [PcdsFeatureFlag.common] >> =20 >> 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 #=20 >> +EFI_ACPI_TABLE_PROTOCOL and (if enabled) EFI_ACPI_SDT_PROTOCOL=20 >> +depend on the # platform's dynamic decision whether to expose an=20 >> +ACPI-based hardware # description to the operating system. >> +# >> +# Universal and platform specific DXE drivers that produce ACPI=20 >> +tables depend # on EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL in t= urn. >> +# >> +# Copyright (C) 2017, Red Hat, Inc. >> +# >> +# This program and the accompanying materials are licensed and made=20 >> +available # under the terms and conditions of the BSD License which=20 >> +accompanies this # distribution. The full text of the license may be=20 >> +found at # http://opensource.org/licenses/bsd-license.php >> +# >> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"=20 >> +BASIS, WITHOUT # WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPR= ESS OR IMPLIED. >> +## >> + >> +[Defines] >> + INF_VERSION =3D 1.25 >> + BASE_NAME =3D PlatformHasAcpiLib >> + FILE_GUID =3D 29beb028-0958-447b-be0a-12229235d7= 7d >> + MODULE_TYPE =3D BASE >> + VERSION_STRING =3D 1.0 >> + LIBRARY_CLASS =3D PlatformHasAcpiLib|DXE_DRIVER >> + CONSTRUCTOR =3D 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=20 >> + that the platform provides the operating system with an ACPI-based=20 >> + hardware description. Note that this is not necessarily mutually=20 >> + exclusive with a Device Tree-based hardware description. A=20 >> + platform driver is supposed to produce a single instance of the=20 >> + protocol (with NULL contents), if appropriate. >> + >> + Copyright (C) 2017, Red Hat, Inc. >> + >> + This program and the accompanying materials are licensed and made=20 >> + available under the terms and conditions of the BSD License that=20 >> + accompanies this distribution. The full text of the license may be=20 >> + found at http://opensource.org/licenses/bsd-license.php. >> + >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"=20 >> +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=20 >> + EFI_ACPI_TABLE_PROTOCOL and (if enabled) EFI_ACPI_SDT_PROTOCOL=20 >> + depend on the platform's dynamic decision whether to expose an=20 >> + ACPI-based hardware description to the operating system. >> + >> + Universal and platform specific DXE drivers that produce ACPI=20 >> + tables depend on EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL in t= urn. >> + >> + Copyright (C) 2017, Red Hat, Inc. >> + >> + This program and the accompanying materials are licensed and made=20 >> + available under the terms and conditions of the BSD License which=20 >> + accompanies this distribution. The full text of the license may be=20 >> + found at http://opensource.org/licenses/bsd-license.php >> + >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"=20 >> +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 >> >=20 > _______________________________________________ > 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 >=20 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel