From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.136; helo=mga12.intel.com; envelope-from=nathaniel.l.desimone@intel.com; receiver=edk2-devel@lists.01.org Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) (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 1BA2E2210D9F3 for ; Fri, 23 Mar 2018 16:40:04 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 23 Mar 2018 16:46:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,352,1517904000"; d="scan'208";a="214144356" Received: from orsmsx104.amr.corp.intel.com ([10.22.225.131]) by fmsmga006.fm.intel.com with ESMTP; 23 Mar 2018 16:46:37 -0700 Received: from orsmsx158.amr.corp.intel.com (10.22.240.20) by ORSMSX104.amr.corp.intel.com (10.22.225.131) with Microsoft SMTP Server (TLS) id 14.3.319.2; Fri, 23 Mar 2018 16:46:37 -0700 Received: from orsmsx114.amr.corp.intel.com ([169.254.8.211]) by ORSMSX158.amr.corp.intel.com ([169.254.10.54]) with mapi id 14.03.0319.002; Fri, 23 Mar 2018 16:46:37 -0700 From: "Desimone, Nathaniel L" To: Evan Lloyd , Sami Mujawar , "edk2-devel@lists.01.org" CC: nd , Stephanie Hughes-Fitt , "leif.lindholm@linaro.org" Thread-Topic: [edk2] [staging/dynamictables PATCH 0/2] Dynamic Tables Framework core Thread-Index: AQHTv5WgcjJxjPPl7Eqc+5A2QrIwiKPZrjnggAF3igCAA1DToA== Date: Fri, 23 Mar 2018 23:46:36 +0000 Message-ID: <02A34F284D1DA44BB705E61F7180EF0A9810FAFF@ORSMSX114.amr.corp.intel.com> References: <20180319151847.85204-1-sami.mujawar@arm.com> <02A34F284D1DA44BB705E61F7180EF0A980EFDBB@ORSMSX114.amr.corp.intel.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.22.254.138] MIME-Version: 1.0 Subject: Re: [staging/dynamictables PATCH 0/2] Dynamic Tables Framework core X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 23 Mar 2018 23:40:05 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Evan, Sorry it didn't notice during my first read that SMBIOS/devicetree was also= supported. This is a rather large code drop and it is taking some time to = go through the entire thing. DynamicTablesDxe seems fine to me. With regard to the EFI_CONFIGURATION_MANAGER_PROTOCOL, note that the only p= rotocols that are allowed to be marked with the EFI_ prefix are architectur= al protocols defined by either the UEFI specification or the PI specificati= on. Since this protocol is not spec defined, it cannot be called EFI_CONFIG= UARTION_MANAGER_PROTOCOL. The convention is that protocols that we consider= to be a standard feature for the EDKII implementation of the UEFI specific= ation are given the EDKII_ prefix. For an example, see MdeModulePkg/Include= /Ppi/UfsHostController.h, note that the UFS host controller PPI is not spec= . defined so the EDKII_ prefix is used. I'm not sure if we are ready to cal= l this an MdeModulePkg addition yet, so let's keep the prefixes off for now= . I'd recommend the name DYNAMIC_TABLES_PLATFORM_PROTOCOL instead of EFI_CO= NFIGURATION_MANAGER_PROTOCOL. One concern I have is there seems to be very tight coupling between the Dyn= amicTablesPkg and the platform code. It doesn't seem like DynamicTablesPkg = is able to much by itself and depends on a very large quantity of platform = code. I have not been able to find the implementation for GetEStdObjAcpiTableList= () anywhere in the code provided either in the fork of edk2-staging or edk2= -platforms. Perhaps you forgot to upload something? Thanks, Nate -----Original Message----- From: Evan Lloyd [mailto:Evan.Lloyd@arm.com]=20 Sent: Wednesday, March 21, 2018 6:23 AM To: Desimone, Nathaniel L ; Sami Mujawar ; edk2-devel@lists.01.org Cc: nd ; Stephanie Hughes-Fitt ;= leif.lindholm@linaro.org Subject: RE: [edk2] [staging/dynamictables PATCH 0/2] Dynamic Tables Framew= ork core Hi Nathaniel. > -----Original Message----- > From: edk2-devel On Behalf Of=20 > Desimone, Nathaniel L > Sent: 20 March 2018 22:08 > To: Sami Mujawar ; edk2-devel@lists.01.org > Cc: nd ; Stephanie Hughes-Fitt Fitt@arm.com>; leif.lindholm@linaro.org > Subject: Re: [edk2] [staging/dynamictables PATCH 0/2] Dynamic Tables=20 > Framework core >=20 > Please don't use the name DynamicTableManagerDxe. What does it manage? > DynamicTables? How does adding the word "Manager" provide any useful=20 > information for the reader? I recommend the name "DynamicAcpiTableDxe". [[Evan Lloyd]] The problem with " DynamicAcpiTableDxe" is that the table manager can be us= ed for SMBIOS tables and even, should you be so inclined (perverse?) Device= Tree. The "DynamicTableManager" name relates to the fact that the compone= nt generates and submits tables (of whatever sort) as opposed to the Config= urationManager component which obtains, collates and provides the Configura= tion information. > It makes it clear this this code is specifically for generating ACPI=20 > tables at runtime (as opposed to some sort of unnamed table), and it=20 > removes the unnecessary prose. [[Evan Lloyd]] As noted (and implied by the name), the code is not "specifi= cally for generating ACPI tables" That being said, we'd be happy to change the names if you can suggest somet= hing clearer. DynamicTablesDxe might do - the "Manager" part is, as you indicate, pretty = redundant. That implies changing ConfigurationManager to something like "DynamicConfig= ". Would that strike you as clearer? Regards, Evan >=20 > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of=20 > Sami Mujawar > Sent: Monday, March 19, 2018 8:19 AM > To: edk2-devel@lists.01.org > Cc: nd@arm.com; leif.lindholm@linaro.org; Stephanie.Hughes-=20 > Fitt@arm.com > Subject: [edk2] [staging/dynamictables PATCH 0/2] Dynamic Tables=20 > Framework core >=20 > The Dynamic Tables Framework is a prototyped as a solution for=20 > automatically generating the firmware tables based on hardware=20 > description. >=20 > This patchset is the Dynamic Tables Framework core and implement the=20 > generic/standard modules for dynamically generating ACPI 6.2 tables=20 > for ARM platform. The platform specific modules are in the devel-=20 > dynamictables branch in the edk2-platforms repository at: > https://github.com/tianocore/edk2-platforms/tree/devel-dynamictables >=20 > The first patch in this patchset 'MdePkg: SMMUv3 updates for IORT' > is a precursor for the Dynamic Tables Framework and has been submitted=20 > independently to the edk2-devel mailing list where it is currently=20 > awaiting acceptance. >=20 > The sources for this patchset can be seen at: > https://github.com/samimujawar/edk2- > staging/tree/187_dynamictables_v1 >=20 > Sami Mujawar (2): > MdePkg: SMMUv3 updates for IORT table definitions > DynamicTablesPkg: Dynamic Tables Framework >=20 >=20 > DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/AcpiTableFactory/Acpi > TableFactory.c | 226 +++ >=20 > DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DeviceTreeTableFactor > y/DeviceTreeTableFactory.c | 225 +++ >=20 > DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactory. > h | 125 ++ >=20 > DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactory > Dxe.c | 84 + >=20 > DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactory > Dxe.inf | 59 + >=20 > DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/SmbiosTableFactory/S > mbiosTableFactory.c | 226 +++ >=20 > DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManag > erDxe.c | 533 +++++ >=20 > DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManag > erDxe.inf | 48 + > DynamicTablesPkg/DynamicTables.dsc.inc > | 46 + > DynamicTablesPkg/DynamicTables.fdf.inc > | 35 + > DynamicTablesPkg/DynamicTablesPkg.dec > | 42 + > DynamicTablesPkg/Include/AcpiTableGenerator.h > | 282 +++ > DynamicTablesPkg/Include/ArmNameSpaceObjects.h > | 587 ++++++ > DynamicTablesPkg/Include/ConfigurationManagerHelper.h > | 119 ++ > DynamicTablesPkg/Include/ConfigurationManagerObject.h > | 176 ++ > DynamicTablesPkg/Include/DeviceTreeTableGenerator.h > | 182 ++ > DynamicTablesPkg/Include/Library/TableHelperLib.h > | 70 + > DynamicTablesPkg/Include/Protocol/ConfigurationManagerProtocol.h > | 128 ++ > DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h > | 140 ++ > DynamicTablesPkg/Include/SmbiosTableGenerator.h > | 240 +++ > DynamicTablesPkg/Include/StandardNameSpaceObjects.h > | 116 ++ > DynamicTablesPkg/Include/TableGenerator.h > | 252 +++ >=20 > DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/AcpiDbg2LibArm.inf > | 47 + > DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c > | 440 +++++ > DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/AcpiFadtLibArm.inf > | 41 + > DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c > | 666 +++++++ > DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/AcpiGtdtLibArm.inf > | 41 + > DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c > | 670 +++++++ > DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf > | 41 + > DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c > | 2046 ++++++++++++++++++++ > DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.h > | 50 + >=20 > DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLibArm.in > f | 41 + > DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c > | 717 +++++++ > DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibArm.inf > | 41 + > DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/McfgGenerator.c > | 342 ++++ > DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/AcpiRawLibArm.inf > | 41 + > DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/RawGenerator.c > | 142 ++ > DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf > | 41 + > DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c > | 324 ++++ > DynamicTablesPkg/Library/Common/TableHelperLib/TableHelper.c > | 164 ++ > DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf > | 35 + > MdePkg/Include/IndustryStandard/IoRemappingTable.h > | 11 +- > 42 files changed, 9881 insertions(+), 1 deletion(-) create mode=20 > 100644=20 > DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/AcpiTableFactory/Acpi > TableFactory.c > create mode 100644 > DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DeviceTreeTableFactor > y/DeviceTreeTableFactory.c > create mode 100644 > DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactory. > h > create mode 100644 > DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactory > Dxe.c > create mode 100644 > DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactory > Dxe.inf > create mode 100644 > DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/SmbiosTableFactory/S > mbiosTableFactory.c > create mode 100644 > DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManag > erDxe.c > create mode 100644 > DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManag > erDxe.inf > create mode 100644 DynamicTablesPkg/DynamicTables.dsc.inc > create mode 100644 DynamicTablesPkg/DynamicTables.fdf.inc > create mode 100644 DynamicTablesPkg/DynamicTablesPkg.dec > create mode 100644 DynamicTablesPkg/Include/AcpiTableGenerator.h > create mode 100644 DynamicTablesPkg/Include/ArmNameSpaceObjects.h > create mode 100644 > DynamicTablesPkg/Include/ConfigurationManagerHelper.h > create mode 100644 > DynamicTablesPkg/Include/ConfigurationManagerObject.h > create mode 100644 > DynamicTablesPkg/Include/DeviceTreeTableGenerator.h > create mode 100644 DynamicTablesPkg/Include/Library/TableHelperLib.h > create mode 100644 > DynamicTablesPkg/Include/Protocol/ConfigurationManagerProtocol.h > create mode 100644 > DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h > create mode 100644 DynamicTablesPkg/Include/SmbiosTableGenerator.h > create mode 100644 > DynamicTablesPkg/Include/StandardNameSpaceObjects.h > create mode 100644 DynamicTablesPkg/Include/TableGenerator.h > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/AcpiDbg2LibArm.inf > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/AcpiFadtLibArm.inf > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/AcpiGtdtLibArm.inf > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.h > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLibArm.in > f > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibArm.inf > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/McfgGenerator.c > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/AcpiRawLibArm.inf > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/RawGenerator.c > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf > create mode 100644 > DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c > create mode 100644 > DynamicTablesPkg/Library/Common/TableHelperLib/TableHelper.c > create mode 100644 > DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf >=20 > -- > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' >=20 >=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