From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=40.92.72.93; helo=eur03-ve1-obe.outbound.protection.outlook.com; envelope-from=marvin.haeuser@outlook.com; receiver=edk2-devel@lists.01.org Received: from EUR03-VE1-obe.outbound.protection.outlook.com (mail-oln040092072093.outbound.protection.outlook.com [40.92.72.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 9A1BE2034860B for ; Wed, 9 May 2018 08:38:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outlook.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=Jv1mZMbtuF1pdDHguJfE5mQI5K2yN5sGOdzsdHDzKkI=; b=JQaLNLBMB8ryDIbUQl6dC6HwiZyMFo88+Ks3UHuEHf+E0pxnotY91i9j6lrB2CUFc8uhdViDBe0LgGHRXVxCckTKVcvn3zpzRDANrYih0meWJ8Eom4knJblLUnJxphCqJMWUPulayIPJlGEJddDD8jaWkxQVRwWUGfPx7LOaqnqQ7RtHkyL+p98hakfw7U+XyyoB2WcHEPg/sb2hJipmDiPZ5rOXc3IPDM3NebLeV5DnuqiddNJ97MQybqmbLvKcdHbN9UTQ84XB1TwbGFrJEkFsc3Fixmhr9ltGfJ8vTMnjW9A5Ih7Lba3gCHCFm7p1wKlys4JVCASVPBPmRrycAg== Received: from AM5EUR03FT060.eop-EUR03.prod.protection.outlook.com (10.152.16.54) by AM5EUR03HT224.eop-EUR03.prod.protection.outlook.com (10.152.17.104) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.20.735.16; Wed, 9 May 2018 15:37:57 +0000 Received: from VI1PR0801MB1790.eurprd08.prod.outlook.com (10.152.16.52) by AM5EUR03FT060.mail.protection.outlook.com (10.152.16.160) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.20.735.16 via Frontend Transport; Wed, 9 May 2018 15:37:57 +0000 Received: from VI1PR0801MB1790.eurprd08.prod.outlook.com ([fe80::7c79:584b:2e12:783e]) by VI1PR0801MB1790.eurprd08.prod.outlook.com ([fe80::7c79:584b:2e12:783e%17]) with mapi id 15.20.0755.012; Wed, 9 May 2018 15:37:57 +0000 From: Marvin H?user To: "edk2-devel@lists.01.org" CC: "Zeng, Star" , Laszlo Ersek , "jiewen.yao@intel.com" , "michael.d.kinney@intel.com" , "ruiyu.ni@intel.com" , "eric.dong@intel.com" Thread-Topic: [PATCH 1/2] MdeModulePkg: Add PlatformAcpiLib LibraryClass. Thread-Index: AQHT5yPk+OtHJaVgR0WrYxIFBUV3I6QnHwkAgABm62A= Date: Wed, 9 May 2018 15:37:56 +0000 Message-ID: References: <0C09AFA07DD0434D9E2A0C6AEB0483103BAE9AE2@shsmsx102.ccr.corp.intel.com> In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103BAE9AE2@shsmsx102.ccr.corp.intel.com> Accept-Language: de-DE, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-incomingtopheadermarker: OriginalChecksum:2057D4AD19EC604BBA78DAD236191012623D8F38B257412D5B03B508347827F1; UpperCasedChecksum:B2C6064531952CAF6775BC9ADB3406BDB57C9A5B971A5002D95949F14ED1946E; SizeAsReceived:7623; Count:47 x-tmn: [u5mbwsff+wboaUqktTZccbqNIqCWx9Ms] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM5EUR03HT224; 7:azKnW66/3JFKosfKxLNEhwh5TG4CGR/xyR55iVhmXCHmGRvc6H8dOi252Xs23xZTU/ybjXTRRkjkecHa+aEE3Z0oF22xXXFFYKxTAcoxWRaPcJB1sEQoeBS4skJw/TthYP9vjp1I6RFMAZR5nP3I4wud+jAOPotwb3SeFy1Yu7oBtdMLy449Qmg5mumzgcgfCnLC0fyRcNsAjXmwXHGPepYsWCGN2CV9hNxEOjmPZBPtK/IucLrC+p4AX5PinMQi x-incomingheadercount: 47 x-eopattributedmessage: 0 x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(201702061078)(5061506573)(5061507331)(1603103135)(2017031320274)(2017031324274)(2017031323274)(2017031322404)(1603101448)(1601125420)(1701031045); SRVR:AM5EUR03HT224; x-ms-traffictypediagnostic: AM5EUR03HT224: x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(444000031); SRVR:AM5EUR03HT224; BCL:0; PCL:0; RULEID:; SRVR:AM5EUR03HT224; x-forefront-prvs: 0667289FF8 x-forefront-antispam-report: SFV:NSPM; SFS:(7070007)(189003)(199004)(51874003)(13464003)(16799955002)(476003)(486006)(3280700002)(3660700001)(8936002)(2900100001)(8676002)(6436002)(25786009)(45080400002)(426003)(72206003)(15188155005)(446003)(11346002)(2501003)(966005)(82202002)(229853002)(97736004)(5250100002)(81156014)(6306002)(5640700003)(53376002)(55016002)(6246003)(6916009)(20460500001)(4326008)(104016004)(106356001)(2351001)(87572001)(102836004)(6346003)(99286004)(54906003)(105586002)(575784001)(26005)(86362001)(74316002)(305945005)(5660300001)(14454004)(68736007)(33656002)(76176011)(7696005)(59450400001)(53546011); DIR:OUT; SFP:1901; SCL:1; SRVR:AM5EUR03HT224; H:VI1PR0801MB1790.eurprd08.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:; received-spf: None (protection.outlook.com: outlook.com does not designate permitted sender hosts) authentication-results: spf=none (sender IP is ) smtp.mailfrom=Marvin.Haeuser@outlook.com; x-microsoft-antispam-message-info: X8p7HKgVA1Ubi91f5WZQ4Sshm4c54e/eWDzJtESMnxVu8uTM1DWLBS2vlGhLwrmpUcJrx9Ufgva2ASo8tHSSY7KgmbZiKIq3SxNLzkBHCe5kzVkE2ZRdx8hCL7oDzPCuXEQ69nqyvNPKsr9M0W+1fWpNTc8U8nmoMe0JmVnx/+ZaN8JRtAAFiPVxVMVLUYVf MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: c590720a-a2a3-495e-a864-08d5b5c2d69c X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 7181d4b0-87d6-4f4e-ba33-0d3746212cec X-MS-Exchange-CrossTenant-Network-Message-Id: c590720a-a2a3-495e-a864-08d5b5c2d69c X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 7181d4b0-87d6-4f4e-ba33-0d3746212cec X-MS-Exchange-CrossTenant-originalarrivaltime: 09 May 2018 15:37:57.1042 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Internet X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5EUR03HT224 Subject: Re: [PATCH 1/2] MdeModulePkg: Add PlatformAcpiLib LibraryClass. 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: Wed, 09 May 2018 15:38:02 -0000 Content-Language: de-DE Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable I continued to CC all people that were CC'd in Star's mail, even if I am no= t sure if those are interested in this patch. If anyone doesn't want to be = CC'd, I will drop them of course. Hey Star, Indeed I expected it to be just for reference, however as most of the drive= rs you linked share the FV-loading code with AcpiPlatformDxe, having it "ma= inlined" makes sense to me. Especially as the "Intel platform forks" of this driver seem to all have a = common memory leak which the sample driver does not have: https://github.co= m/tianocore/edk2/blob/master/MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/Ac= piPlatform.c#L244 If they were using the mainlined driver, the fix could be made once for Mde= ModulePkg and all other platforms would profit. Furthermore, I think it is a good idea to have the platform code not rely o= n where the tables come from (here FV), but instead just be requested to pe= rform the required updates, not having to care about table origin, checksum= fixup, etc. If I am not mistaken, all the additional platform code seen in those driver= s can be implemented well via a library constructor. Maybe the driver deserves a new name though, I'm not sure whether "AcpiPlat= formDxe" is actually fitting. MinPlatform uses the "AcpiPlatform" name to generate fundamental ACPI tabl= es, while "BoardAcpiDxe" is the "AcpiPlatformDxe" + "PlatformAcpiLib" equiv= alent, if I am not mistaken. Thanks in advance. Best regards, Marvin > -----Original Message----- > From: Zeng, Star > Sent: Wednesday, May 9, 2018 11:33 AM > To: Marvin.Haeuser@outlook.com; edk2-devel@lists.01.org > Cc: Yao, Jiewen ; Kinney, Michael D > ; Laszlo Ersek (lersek@redhat.com) > ; Ni, Ruiyu ; Dong, Eric > ; Zeng, Star > Subject: RE: [PATCH 1/2] MdeModulePkg: Add PlatformAcpiLib LibraryClass. >=20 > Marvin, >=20 > It is good direction to reuse more code. > What am I concerning about are > 1. There are comments "Sample ACPI Platform Driver" in AcpiPlatformDxe.in= f > and AcpiPlatformDxe.c, the current position of AcpiPlatformDxe seems just > for reference. > 2. Almost all the platforms have their own AcpiPlatformDxe, for example > https://github.com/tianocore/edk2/tree/master/OvmfPkg/AcpiPlatformDxe > https://github.com/tianocore/edk2/tree/master/QuarkPlatformPkg/Acpi/Dx > e/AcpiPlatform > https://github.com/tianocore/edk2/tree/master/Vlv2TbltDevicePkg/AcpiPla > tform > ... >=20 > Should more evaluation be done on them to see whether we can do more or > we just suggest platform should implement its own AcpiPlatformDxe? >=20 > Including more experts. >=20 >=20 > Thanks, > Star > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Marvin H?user > Sent: Wednesday, May 9, 2018 7:26 AM > To: edk2-devel@lists.01.org > Cc: Ni, Ruiyu ; Dong, Eric ; Zen= g, > Star > Subject: [edk2] [PATCH 1/2] MdeModulePkg: Add PlatformAcpiLib > LibraryClass. >=20 > PlatformAcpiLib can be consumed by the generic ACPI Platform driver to > allow platform specific updates to the ACPI tables loaded from the > configured Firmware Volume. This allows for more platforms to incorporate > the generic ACPI Platform driver. >=20 > This commit also provides a NULL implementation of PlatformAcpiLib. >=20 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marvin Haeuser > --- > MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull.c > | 36 ++++++++++++++++++++ > MdeModulePkg/Include/Library/PlatformAcpiLib.h = | 36 > ++++++++++++++++++++ >=20 > MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull.i > nf | 35 +++++++++++++++++++ > MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull.u > ni | 18 ++++++++++ > MdeModulePkg/MdeModulePkg.dec = | 4 +++ > MdeModulePkg/MdeModulePkg.dsc = | 2 ++ > 6 files changed, 131 insertions(+) >=20 > diff --git > a/MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull > .c > b/MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull > .c > new file mode 100644 > index 000000000000..5d5d0d051e1b > --- /dev/null > +++ > b/MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNu > +++ ll.c > @@ -0,0 +1,36 @@ > +/** @file > + Null Platform ACPI Library instance. > + > +Copyright (c) 2018, Intel Corporation. All rights reserved.
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. > + > +**/ > + > +#include > + > +#include > + > +/** > + Performs platform specific updates to CurrentTable. > + > + @param[in,out] CurrentTable The table to perform updates on. > + > + @retval RETURN_SUCCESS The platform specific ACPI table updates were > applied > + successfully. > + @retval other The platform specific ACPI table updates could= not be > + applied. > + > +**/ > +RETURN_STATUS > +PlatformAcpiUpdateTable ( > + IN OUT EFI_ACPI_COMMON_HEADER *CurrentTable > + ) > +{ > + return RETURN_SUCCESS; > +} > diff --git a/MdeModulePkg/Include/Library/PlatformAcpiLib.h > b/MdeModulePkg/Include/Library/PlatformAcpiLib.h > new file mode 100644 > index 000000000000..a3e367f3ab61 > --- /dev/null > +++ b/MdeModulePkg/Include/Library/PlatformAcpiLib.h > @@ -0,0 +1,36 @@ > +/** @file > + Platform ACPI library. Platform can provide an implementation of this > + library class to provide an ACPI table update routine that may be > +required > + for some type of platform initialization. > + > +Copyright (c) 2018, Intel Corporation. All rights reserved.
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 __PLATFORM_ACPI_LIB__ > +#define __PLATFORM_ACPI_LIB__ > + > +/** > + Performs platform specific updates to CurrentTable. > + > + @param[in,out] CurrentTable The table to perform updates on. > + > + @retval RETURN_SUCCESS The platform specific ACPI table updates were > applied > + successfully. > + @retval other The platform specific ACPI table updates could= not be > + applied. > + > +**/ > +RETURN_STATUS > +PlatformAcpiUpdateTable ( > + IN OUT EFI_ACPI_COMMON_HEADER *CurrentTable > + ); > + > +#endif // __PLATFORM_ACPI_LIB__ > diff --git > a/MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull > .inf > b/MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull > .inf > new file mode 100644 > index 000000000000..a548ecdd91a7 > --- /dev/null > +++ > b/MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNu > +++ ll.inf > @@ -0,0 +1,35 @@ > +## @file > +# Null Platform ACPI Library instance. > +# > +# Copyright (c) 2018, Intel Corporation. All rights reserved.
# # > +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 =3D 0x00010005 > + BASE_NAME =3D BasePlatformAcpiLibNull > + MODULE_UNI_FILE =3D BasePlatformAcpiLibNull.uni > + FILE_GUID =3D 0957C6BA-0559-46E8-8180-CD0317F0AEE= 7 > + MODULE_TYPE =3D BASE > + VERSION_STRING =3D 1.0 > + LIBRARY_CLASS =3D PlatformAcpiLib > + > +# > +# The following information is for reference only and not required by th= e > build tools. > +# > +# VALID_ARCHITECTURES =3D IA32 X64 IPF EBC > +# > + > +[Sources] > + BasePlatformAcpiLibNull.c > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > diff --git > a/MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull > .uni > b/MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull > .uni > new file mode 100644 > index 000000000000..d709a3cad8b5 > --- /dev/null > +++ > b/MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNu > +++ ll.uni > @@ -0,0 +1,18 @@ > +// /** @file > +// Null Platform ACPI Library instance. > +// > +// Null Platform ACPI Library instance. > +// > +// Copyright (c) 2018, Intel Corporation. All rights reserved.
// > +// 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. > +// > +// **/ > + > +#string STR_MODULE_ABSTRACT #language en-US "Null Platform ACPI > Library instance" > +#string STR_MODULE_DESCRIPTION #language en-US "Null Platform ACPI > Library instance." > diff --git a/MdeModulePkg/MdeModulePkg.dec > b/MdeModulePkg/MdeModulePkg.dec index cc397185f7b9..8e3d392b3314 > 100644 > --- a/MdeModulePkg/MdeModulePkg.dec > +++ b/MdeModulePkg/MdeModulePkg.dec > @@ -93,6 +93,10 @@ [LibraryClasses] > # > DebugAgentLib|Include/Library/DebugAgentLib.h >=20 > + ## @libraryclass Provide platform specific ACPI Table update functio= ns. > + # > + PlatformAcpiLib|Include/Library/PlatformAcpiLib.h > + > ## @libraryclass Provide platform specific hooks. > # > PlatformHookLib|Include/Library/PlatformHookLib.h > diff --git a/MdeModulePkg/MdeModulePkg.dsc > b/MdeModulePkg/MdeModulePkg.dsc index ec24a50c7d0a..b18506a9af9c > 100644 > --- a/MdeModulePkg/MdeModulePkg.dsc > +++ b/MdeModulePkg/MdeModulePkg.dsc > @@ -91,6 +91,7 @@ [LibraryClasses] >=20 > PeCoffExtraActionLib|MdePkg/Library/BasePeCoffExtraActionLibNull/BaseP > eCoffExtraActionLibNull.inf >=20 > PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanc > eLibNull.inf >=20 > DebugAgentLib|MdeModulePkg/Library/DebugAgentLibNull/DebugAgentLi > bNull.inf > + > + > PlatformAcpiLib|MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatf > + ormAcpiLibNull.inf >=20 > PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePla > tformHookLibNull.inf >=20 > ResetSystemLib|MdeModulePkg/Library/BaseResetSystemLibNull/BaseRes > etSystemLibNull.inf > SmbusLib|MdePkg/Library/DxeSmbusLib/DxeSmbusLib.inf > @@ -300,6 +301,7 @@ [Components] >=20 > MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib. > inf >=20 > MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeRep > ortStatusCodeLib.inf > MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf > + > + > MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull.i > + nf >=20 > MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull > .inf >=20 > MdeModulePkg/Library/DxeDebugPrintErrorLevelLib/DxeDebugPrintErrorLe > velLib.inf > MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf > -- > 2.17.0.windows.1 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel