From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-x233.google.com (mail-wr0-x233.google.com [IPv6:2a00:1450:400c:c0c::233]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 79DCF80292 for ; Sat, 18 Mar 2017 08:00:46 -0700 (PDT) Received: by mail-wr0-x233.google.com with SMTP id u108so67850108wrb.3 for ; Sat, 18 Mar 2017 08:00:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=GtL8J37fgTo1Q3wKm7SW1ffyDsnao5EubzhJwWTWoiY=; b=C+xoGnsm4Z2mueu+AS0y4sLrQNJ6j0dxiGfNpCRuwYipRgVBq9SV4VHC8/mOimYUvQ cU5KaDNPo6gVeRmXmflzBlI9dbABYFM4Aea1Gao52sLjQy/JpczqaIC+3Vm1TdaT+sGy UY8K3qGIx8xbYKT/naPN/txg9e15kVlC+oHrs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=GtL8J37fgTo1Q3wKm7SW1ffyDsnao5EubzhJwWTWoiY=; b=am0OmhSI+ao+D7BOCJsUf3bnfOgcp+GLTXMpvO+OmzZvIcKe9KxlcpdOSUWDNjBp6N u05ehOTbPZjWLLj1/qwbeQT2lWJ5GLMJqh6tJg3ZSz+wVLOAg7PLVIj96a5ePw/2PcTU p+V5vCga4g38lLrYqaFgMIla189XF6DeZnDUsA2sxWqApOF9cQyASShlbMWPk0vD+ROB 1dEbSEsPugOjHvPD62nV6/Ujyc+Od68v6e/pe6APai93e8NRpeRafnbc+XFiO64uzS6X T/ZRjPfMC19kQT+2WD4fpbUt+J+yjbxjgkdG+xmGfDlS8vjGWkvnx6ll69DtW3+PuG7U uyYg== X-Gm-Message-State: AFeK/H0tRhS2PH1rRAuBg+vtimdR20xvE4J2XuEaP6bYiG+/QWU4zFFsLsI7+y79Eu4haZEe X-Received: by 10.223.133.182 with SMTP id 51mr18516413wrt.39.1489849244537; Sat, 18 Mar 2017 08:00:44 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id x133sm6515960wme.22.2017.03.18.08.00.43 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 18 Mar 2017 08:00:43 -0700 (PDT) Date: Sat, 18 Mar 2017 15:00:42 +0000 From: Leif Lindholm To: Laszlo Ersek Cc: edk2-devel-01 , Ard Biesheuvel Message-ID: <20170318150041.GL16034@bivouac.eciton.net> References: <20170317204731.31488-1-lersek@redhat.com> <20170317204731.31488-6-lersek@redhat.com> MIME-Version: 1.0 In-Reply-To: <20170317204731.31488-6-lersek@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) 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: Sat, 18 Mar 2017 15:00:47 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Mar 17, 2017 at 09:47:24PM +0100, Laszlo Ersek wrote: > 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). I greatly approve, but since you've already done this generically - is there any good reason to keep this in ArmPkg? I am strongly looking to get rid of "things that happen to have been implemented for ARM" from there and pruning it down to contain only things that are architecturelly ARM-specific. MdeModulePkg? Regards, Leif > 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|0x00000001 > > 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 > >