From: EvanLloyd <evan.lloyd@arm.com>

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.

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.


Sami Mujawar (1):
  MdeModulePkg: Dynamic Tables Framework

 MdeModulePkg/MdeModulePkg.dec                                                                       |  13 +
 MdeModulePkg/Universal/DynamicTables/DynamicTables.dsc.inc                                          |  45 ++
 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiDbg2LibArm/AcpiDbg2LibArm.inf                       |  49 ++
 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiFadtLibArm/AcpiFadtLibArm.inf                       |  47 ++
 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiGtdtLibArm/AcpiGtdtLibArm.inf                       |  46 ++
 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLibArm.inf                       |  47 ++
 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibArm.inf                       |  47 ++
 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiRawLibArm/AcpiRawLibArm.inf                         |  44 ++
 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf                       |  44 ++
 MdeModulePkg/Library/DynamicTables/Common/TableHelperLib/TableHelperLib.inf                         |  39 ++
 MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf              |  57 ++
 MdeModulePkg/Universal/DynamicTables/DynamicTableManagerDxe/DynamicTableManagerDxe.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 +++
 MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/DynamicTableFactory.h                   |  91 +++
 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c                          | 440 ++++++++++++
 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c                          | 562 +++++++++++++++
 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c                          | 652 +++++++++++++++++
 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c                          | 732 ++++++++++++++++++++
 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMcfgLibArm/McfgGenerator.c                          | 336 +++++++++
 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiRawLibArm/RawGenerator.c                            | 177 +++++
 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c                          | 323 +++++++++
 MdeModulePkg/Library/DynamicTables/Common/TableHelperLib/TableHelper.c                              | 165 +++++
 MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/AcpiTableFactory/AcpiTableFactory.c     | 227 ++++++
 MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/DynamicTableFactoryDxe.c                |  84 +++
 MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/SmbiosTableFactory/SmbiosTableFactory.c | 227 ++++++
 MdeModulePkg/Universal/DynamicTables/DynamicTableManagerDxe/DynamicTableManagerDxe.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/AcpiDbg2LibArm.inf
 create mode 100644 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiFadtLibArm/AcpiFadtLibArm.inf
 create mode 100644 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiGtdtLibArm/AcpiGtdtLibArm.inf
 create mode 100644 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLibArm.inf
 create mode 100644 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibArm.inf
 create mode 100644 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiRawLibArm/AcpiRawLibArm.inf
 create mode 100644 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf
 create mode 100644 MdeModulePkg/Library/DynamicTables/Common/TableHelperLib/TableHelperLib.inf
 create mode 100644 MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf
 create mode 100644 MdeModulePkg/Universal/DynamicTables/DynamicTableManagerDxe/DynamicTableManagerDxe.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/DynamicTableFactory.h
 create mode 100644 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
 create mode 100644 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c
 create mode 100644 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
 create mode 100644 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c
 create mode 100644 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMcfgLibArm/McfgGenerator.c
 create mode 100644 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiRawLibArm/RawGenerator.c
 create mode 100644 MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
 create mode 100644 MdeModulePkg/Library/DynamicTables/Common/TableHelperLib/TableHelper.c
 create mode 100644 MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/AcpiTableFactory/AcpiTableFactory.c
 create mode 100644 MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/DynamicTableFactoryDxe.c
 create mode 100644 MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/SmbiosTableFactory/SmbiosTableFactory.c
 create mode 100644 MdeModulePkg/Universal/DynamicTables/DynamicTableManagerDxe/DynamicTableManagerDxe.c
 create mode 100644 MdeModulePkg/Universal/DynamicTables/DynamicTables.fdf.inc

-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

I omitted some info here.
The code can be examined at:
https://github.com/EvanLloyd/tianocore/tree/139_dynamic_tables_v1
and
https://github.com/EvanLloyd/edk2-platforms/tree/139_dynamic_tables_v1

Evan

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> evan.lloyd@arm.com
> Sent: 02 October 2017 20:48
> 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
>
> From: EvanLloyd <evan.lloyd@arm.com>
>
> 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.
>
> 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.

[[Evan Lloyd]] Oops, sorry about the superfluous yet.
>
>
> Sami Mujawar (1):
>   MdeModulePkg: Dynamic Tables Framework
>
>  MdeModulePkg/MdeModulePkg.dec                                                                       |
> 13 +
>  MdeModulePkg/Universal/DynamicTables/DynamicTables.dsc.inc
> |  45 ++
>
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiDbg2LibArm/AcpiDbg
> 2LibArm.inf                       |  49 ++
>
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiFadtLibArm/AcpiFadt
> LibArm.inf                       |  47 ++
>
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiGtdtLibArm/AcpiGtdt
> LibArm.inf                       |  46 ++
>
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMadtLibArm/AcpiMa
> dtLibArm.inf                       |  47 ++
>
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMcfgLibArm/AcpiMcf
> gLibArm.inf                       |  47 ++
>
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiRawLibArm/AcpiRaw
> LibArm.inf                         |  44 ++
>
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcr
> LibArm.inf                       |  44 ++
>
> MdeModulePkg/Library/DynamicTables/Common/TableHelperLib/TableHelp
> erLib.inf                         |  39 ++
>
> MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/Dynami
> cTableFactoryDxe.inf              |  57 ++
>
> MdeModulePkg/Universal/DynamicTables/DynamicTableManagerDxe/Dyna
> micTableManagerDxe.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 +++
>
> MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/Dynami
> cTableFactory.h                   |  91 +++
>
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiDbg2LibArm/Dbg2Ge
> nerator.c                          | 440 ++++++++++++
>
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiFadtLibArm/FadtGen
> erator.c                          | 562 +++++++++++++++
>
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiGtdtLibArm/GtdtGen
> erator.c                          | 652 +++++++++++++++++
>
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMadtLibArm/MadtGe
> nerator.c                          | 732 ++++++++++++++++++++
>
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMcfgLibArm/McfgGe
> nerator.c                          | 336 +++++++++
>
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiRawLibArm/RawGen
> erator.c                            | 177 +++++
>
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiSpcrLibArm/SpcrGen
> erator.c                          | 323 +++++++++
>
> MdeModulePkg/Library/DynamicTables/Common/TableHelperLib/TableHelp
> er.c                              | 165 +++++
>
> MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/AcpiTa
> bleFactory/AcpiTableFactory.c     | 227 ++++++
>
> MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/Dynami
> cTableFactoryDxe.c                |  84 +++
>
> MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/Smbios
> TableFactory/SmbiosTableFactory.c | 227 ++++++
>
> MdeModulePkg/Universal/DynamicTables/DynamicTableManagerDxe/Dyna
> micTableManagerDxe.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/AcpiDbg
> 2LibArm.inf
>  create mode 100644
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiFadtLibArm/AcpiFadt
> LibArm.inf
>  create mode 100644
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiGtdtLibArm/AcpiGtdt
> LibArm.inf
>  create mode 100644
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMadtLibArm/AcpiMa
> dtLibArm.inf
>  create mode 100644
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMcfgLibArm/AcpiMcf
> gLibArm.inf
>  create mode 100644
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiRawLibArm/AcpiRaw
> LibArm.inf
>  create mode 100644
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcr
> LibArm.inf
>  create mode 100644
> MdeModulePkg/Library/DynamicTables/Common/TableHelperLib/TableHelp
> erLib.inf
>  create mode 100644
> MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/Dynami
> cTableFactoryDxe.inf
>  create mode 100644
> MdeModulePkg/Universal/DynamicTables/DynamicTableManagerDxe/Dyna
> micTableManagerDxe.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/Dynami
> cTableFactory.h
>  create mode 100644
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiDbg2LibArm/Dbg2Ge
> nerator.c
>  create mode 100644
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiFadtLibArm/FadtGen
> erator.c
>  create mode 100644
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiGtdtLibArm/GtdtGen
> erator.c
>  create mode 100644
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMadtLibArm/MadtGe
> nerator.c
>  create mode 100644
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMcfgLibArm/McfgGe
> nerator.c
>  create mode 100644
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiRawLibArm/RawGen
> erator.c
>  create mode 100644
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiSpcrLibArm/SpcrGen
> erator.c
>  create mode 100644
> MdeModulePkg/Library/DynamicTables/Common/TableHelperLib/TableHelp
> er.c
>  create mode 100644
> MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/AcpiTa
> bleFactory/AcpiTableFactory.c
>  create mode 100644
> MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/Dynami
> cTableFactoryDxe.c
>  create mode 100644
> MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/Smbios
> TableFactory/SmbiosTableFactory.c
>  create mode 100644
> MdeModulePkg/Universal/DynamicTables/DynamicTableManagerDxe/Dyna
> micTableManagerDxe.c
>  create mode 100644
> MdeModulePkg/Universal/DynamicTables/DynamicTables.fdf.inc
>
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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 leave. 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 design philosophy?

1) It seems the final goal is still to generate ACPI table/SMBIOS table/DevTree. You just introduce a way to manage how these tables are generated, right?

2) Below definition is defined by the MdeModulePkg/Include/DynamicTables/ConfigurationManagerObject.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. (IntelFrameworkPkg\Include\Protocol\DataHub.h)
Long time ago, we have platform module filling the SMBIOS needed information to datahub (such as CPU INFO, Memory Info).
The SMBIOS table is derived from datahub protocol. The setup driver can also from datahub.
But later, we think it is an overdesign and datahub is no longer used in the new IA platform.
People feel it is easier to fill industry defined SMBIOS record directly, than 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 difference.

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. (https://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 Profile

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 another 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
> 
> From: EvanLloyd <evan.lloyd@arm.com>
> 
> 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.
> 
> 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.
> 
> 
> Sami Mujawar (1):
>   MdeModulePkg: Dynamic Tables Framework
> 
>  MdeModulePkg/MdeModulePkg.dec
> |  13 +
>  MdeModulePkg/Universal/DynamicTables/DynamicTables.dsc.inc
> |  45 ++
> 
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiDbg2LibArm/AcpiDbg2Lib
> Arm.inf                       |  49 ++
> 
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiFadtLibArm/AcpiFadtLibAr
> m.inf                       |  47 ++
> 
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiGtdtLibArm/AcpiGtdtLibA
> rm.inf                       |  46 ++
> 
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLib
> Arm.inf                       |  47 ++
> 
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLib
> Arm.inf                       |  47 ++
> 
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiRawLibArm/AcpiRawLibAr
> m.inf                         |  44 ++
> 
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibAr
> m.inf                       |  44 ++
> 
> MdeModulePkg/Library/DynamicTables/Common/TableHelperLib/TableHelperLi
> b.inf                         |  39 ++
> 
> MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/DynamicTa
> bleFactoryDxe.inf              |  57 ++
> 
> 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 +++
> 
> MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/DynamicTa
> bleFactory.h                   |  91 +++
> 
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiDbg2LibArm/Dbg2Genera
> tor.c                          | 440 ++++++++++++
> 
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiFadtLibArm/FadtGenerat
> or.c                          | 562 +++++++++++++++
> 
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerat
> or.c                          | 652 +++++++++++++++++
> 
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMadtLibArm/MadtGener
> ator.c                          | 732 ++++++++++++++++++++
> 
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiMcfgLibArm/McfgGenera
> tor.c                          | 336 +++++++++
> 
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiRawLibArm/RawGenerat
> or.c                            | 177 +++++
> 
> MdeModulePkg/Library/DynamicTables/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerat
> or.c                          | 323 +++++++++
>  MdeModulePkg/Library/DynamicTables/Common/TableHelperLib/TableHelper.c
> | 165 +++++
> 
> MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/AcpiTableFa
> ctory/AcpiTableFactory.c     | 227 ++++++
> 
> MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/DynamicTa
> bleFactoryDxe.c                |  84 +++
> 
> MdeModulePkg/Universal/DynamicTables/DynamicTableFactoryDxe/SmbiosTabl
> eFactory/SmbiosTableFactory.c | 227 ++++++
> 
> 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
> 
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> 
> _______________________________________________
> 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

Hi Jiewen
(I hope that is your personal name, not your surname - but it is a bit hard for us barbarians to tell, sorry.)
Thank you for the very helpful feedback.
Responses are inline below.

> -----Original Message-----
> From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
> Sent: 10 October 2017 03:29
> To: Evan Lloyd <Evan.Lloyd@arm.com>; 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; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: RE: [edk2] [PATCH 0/2] Dynamic Tables
>
> 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
> leave. 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.

[[Evan Lloyd]] This sounds sensible, and I will discuss it with Leif.  Another option might be to start up a new module.

>
>
>
> I only have a quick look at the patch. Would you please share more on the
> design philosophy?

[[Evan Lloyd]] You are quite right - we will put a document together on this.

>
> 1) It seems the final goal is still to generate ACPI table/SMBIOS table/DevTree.
> You just introduce a way to manage how these tables are generated, right?

[[Evan Lloyd]] Yes.

>
> 2) Below definition is defined by the
> MdeModulePkg/Include/DynamicTables/ConfigurationManagerObject.h.
> Is there any industry standard to define below index? Or it is just defined by
> EDKII, and anyone can add extension here?

[[Evan Lloyd]] This is an initial submission for discussion, but there is not currently a relevant standard.  We are providing a "proof of concept" for discussion and review.  The ids are currently internal to edk2.  We do anticipate some sort of standard definition (for the information supplier) but that might be no more than a version of ConfigurationManagerObject.h

>
> +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.
> (IntelFrameworkPkg\Include\Protocol\DataHub.h)

[[Evan Lloyd]] We did not.  We have not previously paid much attention to IntelFrameworkPkg  😊
We will have a look at the DataHub stuff though, to see how it fits.

> Long time ago, we have platform module filling the SMBIOS needed
> information to datahub (such as CPU INFO, Memory Info).
> The SMBIOS table is derived from datahub protocol. The setup driver can
> also from datahub.
> But later, we think it is an overdesign and datahub is no longer used in the
> new IA platform.
> People feel it is easier to fill industry defined SMBIOS record directly, than 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
> difference.

[[Evan Lloyd]]  From your description, there does seem to be an element of similarity with our submitted implementation.
However, our current code does not yet cover later options of obtaining the configuration data from a remote node, etc.
Our ultimate aim is to have a single UEFI image run on a range of platforms, taking guidance from the remote on what tables to publish with what content.  You say "People feel it is easier to fill industry defined SMBIOS record directly, than to fill the EDK defined datahub record."  I do not question your statement, in fact I am sure you are right.  However, might those people be the ones who make a living filling "industry defined SMBIOS record directly"?  Our experience is that there is significant overhead in building complete sets of platform dependent tables for each variant of a machine, and we would like to reduce that.

>
> 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.
> (https://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 Profile
>
> 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
> another ConfigurationManager?

[[Evan Lloyd]] PCDs are very useful.  However they (especially FixedPcds) are build time options.  Given the aim of a single UEFI image, they do not quite work.  We want the UEFI to be able to get the variant config from an external entity.  In theory, a complex enough PCD database could be used to provide the config information, and could be loaded from a remote, but it would be a very elaborate solution.

>
>
> 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)

[[Evan Lloyd]] We will need to study DataHub to see what is there, but we will make a response when we have.

>
>
> 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
> >
> > From: EvanLloyd <evan.lloyd@arm.com>
> >
> > 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.
> >
> > 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.
> >
> >
...
> > --
> > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 10/10/17 20:52, Evan Lloyd wrote:
> Hi Jiewen
> (I hope that is your personal name, not your surname - but it is a bit hard for us barbarians to tell, sorry.)

Meta: I've been shamelessly exploiting the (apparent) custom that
@intel.com email addresses come in the following shapes:

<GivenName.Surname@intel.com>
<GivenName.InitialOfMiddleName.Surname@intel.com>

If my heuristic has been wrong all this time, at least now I'll learn
about it :)

(Yes, I've noticed the use of commas in some names; they don't help. The
order they dictate is not uniform between cultures. Email address
schemes are more reliable, within an organization anyway.)

Thanks,
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Thank you!

Comment in line.


From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Evan Lloyd
Sent: Wednesday, October 11, 2017 2:52 AM
To: Yao, Jiewen <jiewen.yao@intel.com>; 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: Re: [edk2] [PATCH 0/2] Dynamic Tables

Hi Jiewen
(I hope that is your personal name, not your surname - but it is a bit hard for us barbarians to tell, sorry.)

[Jiewen] Ah sorry about that. You are right. Jiewen is my given name. ☺

Thank you for the very helpful feedback.
Responses are inline below.

> -----Original Message-----
> From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
> Sent: 10 October 2017 03:29
> To: Evan Lloyd <Evan.Lloyd@arm.com<mailto:Evan.Lloyd@arm.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: "Matteo.Carlini@arm.com"@arm.com<mailto:%22Matteo.Carlini@arm.com%22@arm.com>; "nd@arm.com"@arm.com<mailto:%22nd@arm.com%22@arm.com>;
> "ard.biesheuvel@linaro.org"@arm.com<mailto:%22ard.biesheuvel@linaro.org%22@arm.com>; "Stephanie.Hughes-
> Fitt@arm.com"@arm.com<mailto:Fitt@arm.com%22@arm.com>; "thomas.abraham@arm.com"@arm.com<mailto:%22thomas.abraham@arm.com%22@arm.com>;
> "Arvind.Chauhan@arm.com"@arm.com<mailto:%22Arvind.Chauhan@arm.com%22@arm.com>;
> "leif.lindholm@linaro.org"@arm.com<mailto:%22leif.lindholm@linaro.org%22@arm.com>;
> "Daniil.Egranov@arm.com"@arm.com<mailto:%22Daniil.Egranov@arm.com%22@arm.com>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> Subject: RE: [edk2] [PATCH 0/2] Dynamic Tables
>
> 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
> leave. 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.

[[Evan Lloyd]] This sounds sensible, and I will discuss it with Leif.  Another option might be to start up a new module.

[Jiewen] Right. I also replied mail from Sean Brogan (MSFT), who has concern on putting this to MdeModulePkg. But the mail was so long that it was filtered before.
If ArmPkg can be the first landing zone, that is great.

>
>
>
> I only have a quick look at the patch. Would you please share more on the
> design philosophy?

[[Evan Lloyd]] You are quite right - we will put a document together on this.

>
> 1) It seems the final goal is still to generate ACPI table/SMBIOS table/DevTree.
> You just introduce a way to manage how these tables are generated, right?

[[Evan Lloyd]] Yes.

>
> 2) Below definition is defined by the
> MdeModulePkg/Include/DynamicTables/ConfigurationManagerObject.h.
> Is there any industry standard to define below index? Or it is just defined by
> EDKII, and anyone can add extension here?

[[Evan Lloyd]] This is an initial submission for discussion, but there is not currently a relevant standard.  We are providing a "proof of concept" for discussion and review.  The ids are currently internal to edk2.  We do anticipate some sort of standard definition (for the information supplier) but that might be no more than a version of ConfigurationManagerObject.h

>
> +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.
> (IntelFrameworkPkg\Include\Protocol\DataHub.h)

[[Evan Lloyd]] We did not.  We have not previously paid much attention to IntelFrameworkPkg  😊
We will have a look at the DataHub stuff though, to see how it fits.

> Long time ago, we have platform module filling the SMBIOS needed
> information to datahub (such as CPU INFO, Memory Info).
> The SMBIOS table is derived from datahub protocol. The setup driver can
> also from datahub.
> But later, we think it is an overdesign and datahub is no longer used in the
> new IA platform.
> People feel it is easier to fill industry defined SMBIOS record directly, than 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
> difference.

[[Evan Lloyd]]  From your description, there does seem to be an element of similarity with our submitted implementation.
However, our current code does not yet cover later options of obtaining the configuration data from a remote node, etc.
Our ultimate aim is to have a single UEFI image run on a range of platforms, taking guidance from the remote on what tables to publish with what content.  You say "People feel it is easier to fill industry defined SMBIOS record directly, than to fill the EDK defined datahub record."  I do not question your statement, in fact I am sure you are right.  However, might those people be the ones who make a living filling "industry defined SMBIOS record directly"?  Our experience is that there is significant overhead in building complete sets of platform dependent tables for each variant of a machine, and we would like to reduce that.

>
> 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.
> (https://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 Profile
>
> 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
> another ConfigurationManager?

[[Evan Lloyd]] PCDs are very useful.  However they (especially FixedPcds) are build time options.  Given the aim of a single UEFI image, they do not quite work.  We want the UEFI to be able to get the variant config from an external entity.  In theory, a complex enough PCD database could be used to provide the config information, and could be loaded from a remote, but it would be a very elaborate solution.

[Jiewen] PCD can be static or dynamic. PCD has lots of usage.
For dynamic PCD, you can fill a PCD at BIOS boot time, not build time.
For multi-sku, you can fill a SKU PCD to support different SKU at build time or boot time.
For structure PCD, you can fill a C-like data structure to a PCD value.

In one word, we are able to use PCD to get the variant config from an external entity.
If you think there is something that PCD cannot do, please let us know.


>
>
> 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)

[[Evan Lloyd]] We will need to study DataHub to see what is there, but we will make a response when we have.

[Jiewen] Thank you!
If you have any question on DataHub or PCD, please let us know.

>
>
> Thank you
> Yao Jiewen
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > evan.lloyd@arm.com<mailto:evan.lloyd@arm.com>
> > Sent: Tuesday, October 3, 2017 3:48 AM
> > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > Cc: "Matteo.Carlini@arm.com"@arm.com<mailto:%22Matteo.Carlini@arm.com%22@arm.com>; "nd@arm.com"@arm.com<mailto:%22nd@arm.com%22@arm.com>;
> > "ard.biesheuvel@linaro.org"@arm.com<mailto:%22ard.biesheuvel@linaro.org%22@arm.com>;
> > "Stephanie.Hughes-Fitt@arm.com"@arm.com<mailto:%22Stephanie.Hughes-Fitt@arm.com%22@arm.com>;
> > "thomas.abraham@arm.com"@arm.com<mailto:%22thomas.abraham@arm.com%22@arm.com>;
> > "Arvind.Chauhan@arm.com"@arm.com<mailto:%22Arvind.Chauhan@arm.com%22@arm.com>;
> "leif.lindholm@linaro.org"@arm.com<mailto:%22leif.lindholm@linaro.org%22@arm.com>;
> > "Daniil.Egranov@arm.com"@arm.com<mailto:%22Daniil.Egranov@arm.com%22@arm.com>
> > Subject: [edk2] [PATCH 0/2] Dynamic Tables
> >
> > From: EvanLloyd <evan.lloyd@arm.com<mailto:evan.lloyd@arm.com>>
> >
> > 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.
> >
> > 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.
> >
> >
...
> > --
> > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > https://lists.01.org/mailman/listinfo/edk2-devel
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto: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

On Tue, Oct 10, 2017 at 06:52:02PM +0000, Evan Lloyd wrote:
> > 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.
> 
> [[Evan Lloyd]] This sounds sensible, and I will discuss it with
> Leif.  Another option might be to start up a new module.

My preference would be for this to start as a branch in edk2-staging.

While there, we can bikeshed over the naming and placement (but I will
admit to a certain affinity for Sean's suggestion to keep it separate).

I do not think it is a good fit for ArmPkg.

If Evan can send out a request for creation as described in
https://github.com/tianocore/edk2-staging/blob/about/README,
I am happy to do the mechanics.

/
    Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 12 October 2017 at 16:43, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Tue, Oct 10, 2017 at 06:52:02PM +0000, Evan Lloyd wrote:
>> > 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.
>>
>> [[Evan Lloyd]] This sounds sensible, and I will discuss it with
>> Leif.  Another option might be to start up a new module.
>
> My preference would be for this to start as a branch in edk2-staging.
>
> While there, we can bikeshed over the naming and placement (but I will
> admit to a certain affinity for Sean's suggestion to keep it separate).
>
> I do not think it is a good fit for ArmPkg.
>

+1

We are working very hard to reduce ArmPkg to pieces that are actually
specific to the ARM architecture. The fact that the Intel engineers
may not be interested in using this new dynamic table feature for
their platforms does not make it an ARM thing.

> If Evan can send out a request for creation as described in
> https://github.com/tianocore/edk2-staging/blob/about/README,
> I am happy to do the mechanics.
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel