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.120; helo=mga04.intel.com; envelope-from=jiewen.yao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) (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 336D820945C00 for ; Mon, 9 Oct 2017 19:25:38 -0700 (PDT) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 09 Oct 2017 19:29:05 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,502,1500966000"; d="scan'208";a="160808884" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga005.fm.intel.com with ESMTP; 09 Oct 2017 19:29:06 -0700 Received: from fmsmsx116.amr.corp.intel.com (10.18.116.20) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 9 Oct 2017 19:29:05 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx116.amr.corp.intel.com (10.18.116.20) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 9 Oct 2017 19:29:05 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.175]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.152]) with mapi id 14.03.0319.002; Tue, 10 Oct 2017 10:29:03 +0800 From: "Yao, Jiewen" To: "evan.lloyd@arm.com" , "edk2-devel@lists.01.org" CC: "\"Matteo.Carlini@arm.com\"@arm.com" <"Matteo.Carlini@arm.com"@arm.com>, "\"nd@arm.com\"@arm.com" <"nd@arm.com"@arm.com>, "\"ard.biesheuvel@linaro.org\"@arm.com" <"ard.biesheuvel@linaro.org"@arm.com>, "\"Stephanie.Hughes-Fitt@arm.com\"@arm.com" <"Stephanie.Hughes-Fitt@arm.com"@arm.com>, "\"thomas.abraham@arm.com\"@arm.com" <"thomas.abraham@arm.com"@arm.com>, "\"Arvind.Chauhan@arm.com\"@arm.com" <"Arvind.Chauhan@arm.com"@arm.com>, "\"leif.lindholm@linaro.org\"@arm.com" <"leif.lindholm@linaro.org"@arm.com>, "\"Daniil.Egranov@arm.com\"@arm.com" <"Daniil.Egranov@arm.com"@arm.com>, "Yao, Jiewen" Thread-Topic: [edk2] [PATCH 0/2] Dynamic Tables Thread-Index: AQHTO7djeQQIreRpJ06XOQr/pQI976LcWwYg Date: Tue, 10 Oct 2017 02:29:02 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503A9D7016@shsmsx102.ccr.corp.intel.com> References: <20171002194753.4316-1-evan.lloyd@arm.com> In-Reply-To: <20171002194753.4316-1-evan.lloyd@arm.com> Accept-Language: zh-CN, 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.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH 0/2] Dynamic Tables 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, 10 Oct 2017 02:25:39 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable HI Evan Thanks for the contribution. This is a very big feature. It may talk us more time to review and evaluate= . At same time, one of our key MdeModule package maintainer is in paternity l= eave. It may be longer than usual. I notice you only defined ARM namespace in this patch, and implemented ARM = library instance. Also most consumers of ConfigurationManager are from ARM platform package. = So if it urgent from ARM platform, you may consider to check into ArmPkg at= first. I only have a quick look at the patch. Would you please share more on the d= esign philosophy? 1) It seems the final goal is still to generate ACPI table/SMBIOS table/Dev= Tree. You just introduce a way to manage how these tables are generated, ri= ght? 2) Below definition is defined by the MdeModulePkg/Include/DynamicTables/Co= nfigurationManagerObject.h. Is there any industry standard to define below index? Or it is just defined= by EDKII, and anyone can add extension here? +Object ID's in the ARM Namespace: + 0 - Reserved + 1 - Boot Architecture Info + 2 - CPU Info + 3 - Power Management Profile Info + 4 - GICC Info + 5 - GICD Info + 6 - GIC MSI Frame Info + 7 - GIC Redistributor Info + 8 - GIC ITS Info + 9 - Serial Console Port Info + 10 - Serial Debug Port Info + 12 - Generic Timer Info + 13 - Platform GT Block Info + 14 - Platform Generic Watchdog + 15 - PCI Configuration Space Info + 16 - Hypervisor Vendor Id 3) I am not sure if you have known about datahub protocol. (IntelFrameworkP= kg\Include\Protocol\DataHub.h) Long time ago, we have platform module filling the SMBIOS needed informatio= n to datahub (such as CPU INFO, Memory Info). The SMBIOS table is derived from datahub protocol. The setup driver can als= o from datahub. But later, we think it is an overdesign and datahub is no longer used in th= e new IA platform. People feel it is easier to fill industry defined SMBIOS record directly, t= han to fill the EDK defined datahub record. They do not need to learn 2 different styles of data record format. To me, this seems similar to datahub. Please help us understand the key dif= ference. 4) In addition, EDKII/PI defined PCD (platform configuration database). It = is an architecture way to manage the configuration data. We are also implementing structure PCD to let platform fill data easily. (h= ttps://github.com/tianocore/edk2-staging/tree/StructurePcd) I found some configuration can be as simple as a PCD, such as + // Boot architecture information + { EFI_ACPI_6_1_ARM_PSCI_COMPLIANT }, // BootArchFlags + + // Power management profile information + { EFI_ACPI_6_1_PM_PROFILE_ENTERPRISE_SERVER }, // PowerManagement Pro= file With the new structure PCD design, below definition can also be a structure= PCD. + // SPCR Serial Port + { + FixedPcdGet64 (PcdSerialRegisterBase), // UINT64 BaseAddress + FixedPcdGet32 (PL011UartInterrupt), // UINT32 Interrupt + FixedPcdGet64 (PcdUartDefaultBaudRate), // UINT64 BaudRate + FixedPcdGet32 (PL011UartClkInHz) // UINT32 Clock + }, + // Debug Serial Port + { + FixedPcdGet64 (PcdSerialDbgRegisterBase), // UINT64 BaseAddress + 38, // UINT32 Interrupt + FixedPcdGet64 (PcdSerialDbgUartBaudRate), // UINT64 BaudRate + FixedPcdGet32 (PcdSerialDbgUartClkInHz) // UINT32 Clock + }, What if we just use PCD to define these CPU info, GICC info? Do we really a= nother ConfigurationManager? All in all, if we can compare the difference of below design with pros/cons= , that will be great. That will help us understand more about the new design. A) DataHub (IntelFrameworkPkg, do not recommend to use.) B) PCD (MdePkg, in PI specification) and structure PCD (EDKII staging) C) ConfigurationManager (this patch) Thank you Yao Jiewen > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > evan.lloyd@arm.com > Sent: Tuesday, October 3, 2017 3:48 AM > To: edk2-devel@lists.01.org > Cc: "Matteo.Carlini@arm.com"@arm.com; "nd@arm.com"@arm.com; > "ard.biesheuvel@linaro.org"@arm.com; > "Stephanie.Hughes-Fitt@arm.com"@arm.com; > "thomas.abraham@arm.com"@arm.com; > "Arvind.Chauhan@arm.com"@arm.com; "leif.lindholm@linaro.org"@arm.com; > "Daniil.Egranov@arm.com"@arm.com > Subject: [edk2] [PATCH 0/2] Dynamic Tables >=20 > From: EvanLloyd >=20 > Historically, ACPI code, SMBIOS tables, and UEFI firmware were > often developed in isolation from each other. This introduced > several problems, not least of which was duplication of platform > information between the various source trees. > In addition, variants of platforms introduced a plethora of > alternative builds of ACPI, SMBIOS and EDK2, with the concomitant > risk of getting the mixture wrong in a build. >=20 > In the effort to resolve these problems, the solution prototyped > here was devised. The basic idea is to obtain the "variant" > information from a management node. That means the firmware image > can be platform independent, with ACPI, SMBIOS (and potentially > other) tables generated with information from the management > node. This example has the framework for that, but the > configuration information is supplied directly, as an interim solution > until a suitable management node implementation exists yet. >=20 >=20 > Sami Mujawar (1): > MdeModulePkg: Dynamic Tables Framework >=20 > MdeModulePkg/MdeModulePkg.dec > | 13 + > MdeModulePkg/Universal/DynamicTables/DynamicTables.dsc.inc > | 45 ++ >=20 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiDbg2LibArm/AcpiDbg2Lib > Arm.inf | 49 ++ >=20 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiFadtLibArm/AcpiFadtLibAr > m.inf | 47 ++ >=20 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiGtdtLibArm/AcpiGtdtLibA > rm.inf | 46 ++ >=20 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLib > Arm.inf | 47 ++ >=20 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLib > Arm.inf | 47 ++ >=20 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiRawLibArm/AcpiRawLibAr > m.inf | 44 ++ >=20 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibAr > m.inf | 44 ++ >=20 > MdeModulePkg/Library/DynamicTables/Common/TableHelperLib/TableHelperLi > b.inf | 39 ++ >=20 > MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/DynamicTa > bleFactoryDxe.inf | 57 ++ >=20 > MdeModulePkg/Universal/DynamicTables/DynamicTableManagerDxe/DynamicT > ableManagerDxe.inf | 47 ++ > MdeModulePkg/Include/DynamicTables/AcpiTableGenerator.h > | 280 ++++++++ > MdeModulePkg/Include/DynamicTables/ArmNameSpaceObjects.h > | 367 ++++++++++ > MdeModulePkg/Include/DynamicTables/ConfigurationManagerHelper.h > | 112 +++ > MdeModulePkg/Include/DynamicTables/ConfigurationManagerObject.h > | 158 +++++ > MdeModulePkg/Include/DynamicTables/SmbiosTableGenerator.h > | 235 +++++++ > MdeModulePkg/Include/DynamicTables/StandardNameSpaceObjects.h > | 93 +++ > MdeModulePkg/Include/DynamicTables/TableGenerator.h > | 235 +++++++ > MdeModulePkg/Include/Library/TableHelperLib.h > | 67 ++ > MdeModulePkg/Include/Protocol/ConfigurationManagerProtocol.h > | 121 ++++ > MdeModulePkg/Include/Protocol/DynamicTableFactoryProtocol.h > | 113 +++ >=20 > MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/DynamicTa > bleFactory.h | 91 +++ >=20 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiDbg2LibArm/Dbg2Genera > tor.c | 440 ++++++++++++ >=20 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiFadtLibArm/FadtGenerat > or.c | 562 +++++++++++++++ >=20 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerat > or.c | 652 +++++++++++++++++ >=20 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMadtLibArm/MadtGener > ator.c | 732 ++++++++++++++++++++ >=20 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMcfgLibArm/McfgGenera > tor.c | 336 +++++++++ >=20 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiRawLibArm/RawGenerat > or.c | 177 +++++ >=20 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerat > or.c | 323 +++++++++ > MdeModulePkg/Library/DynamicTables/Common/TableHelperLib/TableHelper.c > | 165 +++++ >=20 > MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/AcpiTableFa > ctory/AcpiTableFactory.c | 227 ++++++ >=20 > MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/DynamicTa > bleFactoryDxe.c | 84 +++ >=20 > MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/SmbiosTabl > eFactory/SmbiosTableFactory.c | 227 ++++++ >=20 > MdeModulePkg/Universal/DynamicTables/DynamicTableManagerDxe/DynamicT > ableManagerDxe.c | 531 ++++++++++++++ > MdeModulePkg/Universal/DynamicTables/DynamicTables.fdf.inc > | 35 + > 36 files changed, 6888 insertions(+) > create mode 100644 > MdeModulePkg/Universal/DynamicTables/DynamicTables.dsc.inc > create mode 100644 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiDbg2LibArm/AcpiDbg2Lib > Arm.inf > create mode 100644 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiFadtLibArm/AcpiFadtLibAr > m.inf > create mode 100644 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiGtdtLibArm/AcpiGtdtLibA > rm.inf > create mode 100644 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLib > Arm.inf > create mode 100644 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLib > Arm.inf > create mode 100644 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiRawLibArm/AcpiRawLibAr > m.inf > create mode 100644 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibAr > m.inf > create mode 100644 > MdeModulePkg/Library/DynamicTables/Common/TableHelperLib/TableHelperLi > b.inf > create mode 100644 > MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/DynamicTa > bleFactoryDxe.inf > create mode 100644 > MdeModulePkg/Universal/DynamicTables/DynamicTableManagerDxe/DynamicT > ableManagerDxe.inf > create mode 100644 > MdeModulePkg/Include/DynamicTables/AcpiTableGenerator.h > create mode 100644 > MdeModulePkg/Include/DynamicTables/ArmNameSpaceObjects.h > create mode 100644 > MdeModulePkg/Include/DynamicTables/ConfigurationManagerHelper.h > create mode 100644 > MdeModulePkg/Include/DynamicTables/ConfigurationManagerObject.h > create mode 100644 > MdeModulePkg/Include/DynamicTables/SmbiosTableGenerator.h > create mode 100644 > MdeModulePkg/Include/DynamicTables/StandardNameSpaceObjects.h > create mode 100644 MdeModulePkg/Include/DynamicTables/TableGenerator.h > create mode 100644 MdeModulePkg/Include/Library/TableHelperLib.h > create mode 100644 > MdeModulePkg/Include/Protocol/ConfigurationManagerProtocol.h > create mode 100644 > MdeModulePkg/Include/Protocol/DynamicTableFactoryProtocol.h > create mode 100644 > MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/DynamicTa > bleFactory.h > create mode 100644 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiDbg2LibArm/Dbg2Genera > tor.c > create mode 100644 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiFadtLibArm/FadtGenerat > or.c > create mode 100644 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerat > or.c > create mode 100644 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMadtLibArm/MadtGener > ator.c > create mode 100644 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMcfgLibArm/McfgGenera > tor.c > create mode 100644 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiRawLibArm/RawGenerat > or.c > create mode 100644 > MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerat > or.c > create mode 100644 > MdeModulePkg/Library/DynamicTables/Common/TableHelperLib/TableHelper.c > create mode 100644 > MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/AcpiTableFa > ctory/AcpiTableFactory.c > create mode 100644 > MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/DynamicTa > bleFactoryDxe.c > create mode 100644 > MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/SmbiosTabl > eFactory/SmbiosTableFactory.c > create mode 100644 > MdeModulePkg/Universal/DynamicTables/DynamicTableManagerDxe/DynamicT > ableManagerDxe.c > create mode 100644 > MdeModulePkg/Universal/DynamicTables/DynamicTables.fdf.inc >=20 > -- > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759") >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel