DynamicTablesPkg can be used by RISC-V platforms to generate ACPI tables
from FDT passed from previous stage FW. However, DynamicTablesPkg
currently is ARM specific even though several parsers and ACPI
generators can be used across architectures. For ex: SSDT (PCIe), SSDT
(CPU), MCFG, SPCR, DBG2, FADT, SRAT, Raw (DSDT) are mostly common across
architectures. Only MADT, IORT and GTDT are ARM specific.

This series tries to refactor the DynamicTablesPkg so that RISC-V
support can be added fairly easily later.

While the approach taken in this series reduces the code duplication as
much as possible, it has the side effect of large changes and impacts
platforms that support DynamicTablesPkg today (ex: JunoPkg,
ArmKvmTool etc). Other approach I tried initially was to duplicate most
of the things from Arm to RISC-V. But it was too much of
code duplication which is bad in my opinion.

To help the review, the series is also available at
https://github.com/vlsunil/edk2/tree/dyn_tables_refactor_rfc_v1

Corresponding changes in edk2-platforms are at
https://github.com/vlsunil/edk2-platforms/tree/dyn_tables_approach2_v1

I have built tested the changes for following platforms enabling
DYNAMIC_TABLES_FRAMEWORK.
  - ArmJuno.dsc
  - ArmVExpress-FVP-AArch64.dsc 
  - ArmVirtKvmTool.dsc

But due to the nature of the changes, if the approach is fine, I will
need some help to test those platforms which support DynamicTables.

The series passes CI tests.

Looking forward for your feedback!. I am happy to try if there are any
better methods to add support for other architectures.

Addition of RISC-V support PoC code (on top of this series) is available
at 
https://github.com/vlsunil/edk2/tree/dyn_tables_riscv_rfc_v1

PS: PoC code for the code duplication approach is available at
https://github.com/vlsunil/edk2/commits/dyn_tables_approch1_v1/.
This was just quick work. It has the advantage of lesser impact on
existing ARM platforms. But looks too bad to me because of code
duplication.

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Pierre Gondois <pierre.gondois@arm.com>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: Andrei Warkentin <andrei.warkentin@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: AbdulLateef Attar (Abdul Lateef) <AbdulLateef.Attar@amd.com>
Cc: Jeff Brasen <jbrasen@nvidia.com>

Sunil V L (20):
  DynamicTablesPkg: FdtHwInfoParserLib: Move ARM parsers to Arm
    directory
  DynamicTablesPkg: Move common ACPI libraries from folder
  DynamicTablesPkg: Update ACPI common libraries BASE_NAME
  DynamicTablesPkg: Rename ArmNameSpaceObjects to ArchNameSpaceObjects
  DynamicTablesPkg: DynamicPlatRepoLib: Rename ArmObjList as ArchObjList
  DynamicTablesPkg: Rename EObjNameSpaceArm to EObjNameSpaceArch
  DynamicTablesPkg: FdtHwInfoParserLib: Separate ARM specific code
  DynamicTablesPkg: FdtHwInfoParserLib: Pci: Make it common for
    architectures
  DynamicTablesPkg: FdtHwInfoParserLib: Make Serial parser arch neutral
  DynamicTablesPkg: FdtHwInfoParserLib: Move ArmLib.h to ArmGicCParser.c
  DynamicTablesPkg: TableHelperLib: Populate OemTableId using
    ConfigManager
  DynamicTablesPkg: Add CreatorId in
    CM_STD_OBJ_CONFIGURATION_MANAGER_INFO
  DynamicTablesPkg: FdtHwInfoParserLib: Move ARM utilities to separate
    file
  DynamicTablesPkg/FdtHwInfoParserLib: Add function for INTC address
    cells
  DynamicTablesPkg: AcpiSsdtCpuTopologyLib: Move GICC from common code
  DynamicTablesPkg: AcpiFadtLib: Move ArmBootArch to ARM specific file
  DynamicTablesPkg: DynamicTableManagerDxe: Refactor to allow other
    archs
  DynamicTablesPkg: X86: Add interfaces to succeed CI tests
  DynamicTablesPkg/AmlLib: Fix typo
  DynamicTablesPkg.ci.yaml: Update exception and ignore list

 DynamicTablesPkg/DynamicTables.dsc.inc        |  45 +-
 .../DynamicTableManagerDxe.inf                |   9 +-
 .../AcpiDbg2Lib/AcpiDbg2Lib.inf}              |  15 +-
 .../AcpiFadtLib/AcpiFadtLib.inf}              |  13 +-
 .../AcpiMcfgLib/AcpiMcfgLib.inf}              |   9 +-
 .../AcpiPcctLib/AcpiPcctLib.inf}              |   2 +-
 .../AcpiPpttLib/AcpiPpttLib.inf}              |   2 +-
 .../AcpiRawLib/AcpiRawLib.inf}                |   9 +-
 .../AcpiSpcrLib/AcpiSpcrLib.inf}              |   9 +-
 .../AcpiSratLib/AcpiSratLib.inf}              |   2 +-
 .../SsdtCpuTopologyLib.inf}                   |   9 +-
 .../AcpiSsdtPcieLib/SsdtPcieLib.inf}          |   2 +-
 .../SsdtSerialPortLib.inf}                    |   6 +-
 .../FdtHwInfoParserLib/FdtHwInfoParserLib.inf |  48 +-
 .../KvmtoolCfgMgrDxe/ConfigurationManager.h   |  10 +-
 .../DynamicTableManagerDxe.h                  |  65 +++
 ...eSpaceObjects.h => ArchNameSpaceObjects.h} | 428 ++++++++--------
 .../Include/ConfigurationManagerObject.h      |   8 +-
 .../Include/Library/AmlLib/AmlLib.h           |   1 +
 .../Include/Library/SsdtPcieSupportLib.h      |  12 +-
 .../Include/Library/SsdtSerialPortFixupLib.h  |   8 +-
 .../Include/StandardNameSpaceObjects.h        |   1 +
 .../Acpi/Common/AcpiFadtLib/FadtGenerator.h   |  32 ++
 .../AcpiPcctLib}/PcctGenerator.h              |   4 +-
 .../AcpiPpttLib}/PpttGenerator.h              |   0
 .../SsdtCpuTopologyGenerator.h                |  78 ++-
 .../AcpiSsdtPcieLib}/SsdtPcieGenerator.h      |   0
 .../DynamicPlatRepoInternal.h                 |   4 +-
 .../{ => Arm}/BootArch/ArmBootArchParser.h    |   6 +-
 .../GenericTimer/ArmGenericTimerParser.h      |   6 +-
 .../{ => Arm}/Gic/ArmGicCParser.h             |   6 +-
 .../{ => Arm}/Gic/ArmGicDParser.h             |   6 +-
 .../{ => Arm}/Gic/ArmGicDispatcher.h          |   6 +-
 .../{ => Arm}/Gic/ArmGicItsParser.h           |   6 +-
 .../{ => Arm}/Gic/ArmGicMsiFrameParser.h      |   6 +-
 .../{ => Arm}/Gic/ArmGicRParser.h             |   6 +-
 .../FdtHwInfoParserLib/CmObjectDescUtility.h  |   6 +-
 .../FdtHwInfoParserLib/FdtHwInfoParser.h      |  27 +
 .../FdtHwInfoParserInclude.h                  |   1 -
 .../Library/FdtHwInfoParserLib/FdtUtility.h   |   9 +
 ...igSpaceParser.h => PciConfigSpaceParser.h} |  30 +-
 ...mSerialPortParser.h => SerialPortParser.h} |  16 +-
 .../KvmtoolCfgMgrDxe/ConfigurationManager.c   |  20 +-
 .../ArmDynamicTableManagerDxe.c               | 154 ++++++
 .../DynamicTableManagerDxe.c                  | 145 +-----
 .../X86DynamicTableManagerDxe.c               |  44 ++
 .../Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c   |  62 +--
 .../Acpi/Arm/AcpiIortLibArm/IortGenerator.c   | 182 +++----
 .../Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c   | 124 ++---
 .../SsdtCmn600Generator.c                     |  34 +-
 .../AcpiDbg2Lib}/Dbg2Generator.c              |  14 +-
 .../Common/AcpiFadtLib/ArmFadtGenerator.c     |  80 +++
 .../AcpiFadtLib}/FadtGenerator.c              | 115 +----
 .../AcpiMcfgLib}/McfgGenerator.c              |  22 +-
 .../AcpiPcctLib}/PcctGenerator.c              | 148 +++---
 .../AcpiPpttLib}/PpttGenerator.c              |  86 ++--
 .../AcpiRawLib}/RawGenerator.c                |   0
 .../AcpiSpcrLib}/SpcrGenerator.c              |  12 +-
 .../AcpiSratLib}/SratGenerator.c              |  82 ++--
 .../ArmSsdtCpuTopologyGenerator.c             | 179 +++++++
 .../SsdtCpuTopologyGenerator.c                | 282 ++++-------
 .../AcpiSsdtPcieLib}/SsdtPcieGenerator.c      | 112 ++---
 .../SsdtSerialPortGenerator.c                 |  12 +-
 .../Common/AmlLib/CodeGen/AmlCodeGen.c        |   3 +-
 .../Library/Common/AmlLib/Parser/AmlParser.c  |   2 +-
 .../Library/Common/AmlLib/Tree/AmlNode.c      |   2 +-
 .../DynamicPlatRepoLib/CmObjectTokenFixer.c   |  22 +-
 .../DynamicPlatRepoLib/DynamicPlatRepo.c      |  54 +-
 .../Common/DynamicPlatRepoLib/TokenMapper.c   |   8 +-
 .../SsdtPcieSupportLib/SsdtPcieSupportLib.c   |  12 +-
 .../SsdtSerialPortFixupLib.c                  |  38 +-
 .../ConfigurationManagerObjectParser.c        | 460 +++++++++---------
 .../Common/TableHelperLib/TableHelper.c       |  32 +-
 .../Arm/ArmFdtHwInfoParser.c                  |  83 ++++
 .../FdtHwInfoParserLib/Arm/ArmFdtUtility.c    | 114 +++++
 .../{ => Arm}/BootArch/ArmBootArchParser.c    |  28 +-
 .../GenericTimer/ArmGenericTimerParser.c      |  36 +-
 .../{ => Arm}/Gic/ArmGicCParser.c             | 121 ++---
 .../{ => Arm}/Gic/ArmGicDParser.c             |  30 +-
 .../{ => Arm}/Gic/ArmGicDispatcher.c          |  18 +-
 .../{ => Arm}/Gic/ArmGicItsParser.c           |  40 +-
 .../{ => Arm}/Gic/ArmGicMsiFrameParser.c      |  32 +-
 .../{ => Arm}/Gic/ArmGicRParser.c             |  36 +-
 .../FdtHwInfoParserLib/CmObjectDescUtility.c  |  10 +-
 .../FdtHwInfoParserLib/FdtHwInfoParser.c      |  78 +--
 .../Library/FdtHwInfoParserLib/FdtUtility.c   |  71 ---
 ...igSpaceParser.c => PciConfigSpaceParser.c} |  71 ++-
 ...mSerialPortParser.c => SerialPortParser.c} |  94 ++--
 DynamicTablesPkg/DynamicTablesPkg.ci.yaml     |   7 +-
 89 files changed, 2382 insertions(+), 1997 deletions(-)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiDbg2LibArm/AcpiDbg2LibArm.inf => Common/AcpiDbg2Lib/AcpiDbg2Lib.inf} (85%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiFadtLibArm/AcpiFadtLibArm.inf => Common/AcpiFadtLib/AcpiFadtLib.inf} (82%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiMcfgLibArm/AcpiMcfgLibArm.inf => Common/AcpiMcfgLib/AcpiMcfgLib.inf} (85%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiPcctLibArm/AcpiPcctLibArm.inf => Common/AcpiPcctLib/AcpiPcctLib.inf} (90%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf => Common/AcpiPpttLib/AcpiPpttLib.inf} (90%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiRawLibArm/AcpiRawLibArm.inf => Common/AcpiRawLib/AcpiRawLib.inf} (85%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf => Common/AcpiSpcrLib/AcpiSpcrLib.inf} (86%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiSratLibArm/AcpiSratLibArm.inf => Common/AcpiSratLib/AcpiSratLib.inf} (90%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyLibArm.inf => Common/AcpiSsdtCpuTopologyLib/SsdtCpuTopologyLib.inf} (81%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm.inf => Common/AcpiSsdtPcieLib/SsdtPcieLib.inf} (91%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiSsdtSerialPortLibArm/SsdtSerialPortLibArm.inf => Common/AcpiSsdtSerialPortLib/SsdtSerialPortLib.inf} (87%)
 create mode 100644 DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.h
 rename DynamicTablesPkg/Include/{ArmNameSpaceObjects.h => ArchNameSpaceObjects.h} (72%)
 create mode 100644 DynamicTablesPkg/Library/Acpi/Common/AcpiFadtLib/FadtGenerator.h
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiPcctLibArm => Common/AcpiPcctLib}/PcctGenerator.h (81%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiPpttLibArm => Common/AcpiPpttLib}/PpttGenerator.h (100%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiSsdtCpuTopologyLibArm => Common/AcpiSsdtCpuTopologyLib}/SsdtCpuTopologyGenerator.h (60%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiSsdtPcieLibArm => Common/AcpiSsdtPcieLib}/SsdtPcieGenerator.h (100%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/{ => Arm}/BootArch/ArmBootArchParser.h (90%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/{ => Arm}/GenericTimer/ArmGenericTimerParser.h (93%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/{ => Arm}/Gic/ArmGicCParser.h (94%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/{ => Arm}/Gic/ArmGicDParser.h (92%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/{ => Arm}/Gic/ArmGicDispatcher.h (93%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/{ => Arm}/Gic/ArmGicItsParser.h (91%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/{ => Arm}/Gic/ArmGicMsiFrameParser.h (90%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/{ => Arm}/Gic/ArmGicRParser.h (90%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/Pci/{ArmPciConfigSpaceParser.h => PciConfigSpaceParser.h} (84%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/Serial/{ArmSerialPortParser.h => SerialPortParser.h} (79%)
 create mode 100644 DynamicTablesPkg/Drivers/DynamicTableManagerDxe/ArmDynamicTableManagerDxe.c
 create mode 100644 DynamicTablesPkg/Drivers/DynamicTableManagerDxe/X86DynamicTableManagerDxe.c
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiDbg2LibArm => Common/AcpiDbg2Lib}/Dbg2Generator.c (95%)
 create mode 100644 DynamicTablesPkg/Library/Acpi/Common/AcpiFadtLib/ArmFadtGenerator.c
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiFadtLibArm => Common/AcpiFadtLib}/FadtGenerator.c (84%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiMcfgLibArm => Common/AcpiMcfgLib}/McfgGenerator.c (92%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiPcctLibArm => Common/AcpiPcctLib}/PcctGenerator.c (87%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiPpttLibArm => Common/AcpiPpttLib}/PpttGenerator.c (93%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiRawLibArm => Common/AcpiRawLib}/RawGenerator.c (100%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiSpcrLibArm => Common/AcpiSpcrLib}/SpcrGenerator.c (95%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiSratLibArm => Common/AcpiSratLib}/SratGenerator.c (91%)
 create mode 100644 DynamicTablesPkg/Library/Acpi/Common/AcpiSsdtCpuTopologyLib/ArmSsdtCpuTopologyGenerator.c
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiSsdtCpuTopologyLibArm => Common/AcpiSsdtCpuTopologyLib}/SsdtCpuTopologyGenerator.c (82%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiSsdtPcieLibArm => Common/AcpiSsdtPcieLib}/SsdtPcieGenerator.c (90%)
 rename DynamicTablesPkg/Library/Acpi/{Arm/AcpiSsdtSerialPortLibArm => Common/AcpiSsdtSerialPortLib}/SsdtSerialPortGenerator.c (95%)
 create mode 100644 DynamicTablesPkg/Library/FdtHwInfoParserLib/Arm/ArmFdtHwInfoParser.c
 create mode 100644 DynamicTablesPkg/Library/FdtHwInfoParserLib/Arm/ArmFdtUtility.c
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/{ => Arm}/BootArch/ArmBootArchParser.c (82%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/{ => Arm}/GenericTimer/ArmGenericTimerParser.c (86%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/{ => Arm}/Gic/ArmGicCParser.c (86%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/{ => Arm}/Gic/ArmGicDParser.c (85%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/{ => Arm}/Gic/ArmGicDispatcher.c (91%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/{ => Arm}/Gic/ArmGicItsParser.c (83%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/{ => Arm}/Gic/ArmGicMsiFrameParser.c (84%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/{ => Arm}/Gic/ArmGicRParser.c (86%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/Pci/{ArmPciConfigSpaceParser.c => PciConfigSpaceParser.c} (89%)
 rename DynamicTablesPkg/Library/FdtHwInfoParserLib/Serial/{ArmSerialPortParser.c => SerialPortParser.c} (85%)

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113462): https://edk2.groups.io/g/devel/message/113462
Mute This Topic: https://groups.io/mt/103622702/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sunil V L
> via groups.io
> Sent: Tuesday, January 9, 2024 9:29 AM
> DynamicTablesPkg can be used by RISC-V platforms to generate ACPI tables
> from FDT passed from previous stage FW. However, DynamicTablesPkg
> currently is ARM specific even though several parsers and ACPI generators can
> be used across architectures. For ex: SSDT (PCIe), SSDT (CPU), MCFG, SPCR,
> DBG2, FADT, SRAT, Raw (DSDT) are mostly common across architectures. Only
> MADT, IORT and GTDT are ARM specific.
> 
> This series tries to refactor the DynamicTablesPkg so that RISC-V support can
> be added fairly easily later.

It looks like instead of moving the common code to EObjNameSpaceStandard namespace or a new (Arch? Common?) namespace, you're renaming the entire EObjNameSpaceArm namespace to EObjNameSpaceArch. It seems to me that if ARM code vs. common code is being separated out, then the EObjNameSpaceArm namespace should continue to be used for the ARM-specific code and a common namespace should be used for the common code.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113493): https://edk2.groups.io/g/devel/message/113493
Mute This Topic: https://groups.io/mt/103622702/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Hi Jeshua,

On Tue, Jan 09, 2024 at 06:37:43PM +0000, Jeshua Smith wrote:
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sunil V L
> > via groups.io
> > Sent: Tuesday, January 9, 2024 9:29 AM
> > DynamicTablesPkg can be used by RISC-V platforms to generate ACPI tables
> > from FDT passed from previous stage FW. However, DynamicTablesPkg
> > currently is ARM specific even though several parsers and ACPI generators can
> > be used across architectures. For ex: SSDT (PCIe), SSDT (CPU), MCFG, SPCR,
> > DBG2, FADT, SRAT, Raw (DSDT) are mostly common across architectures. Only
> > MADT, IORT and GTDT are ARM specific.
> > 
> > This series tries to refactor the DynamicTablesPkg so that RISC-V support can
> > be added fairly easily later.
> 
> It looks like instead of moving the common code to EObjNameSpaceStandard namespace or a new (Arch? Common?) namespace, you're renaming the entire EObjNameSpaceArm namespace to EObjNameSpaceArch. It seems to me that if ARM code vs. common code is being separated out, then the EObjNameSpaceArm namespace should continue to be used for the ARM-specific code and a common namespace should be used for the common code.

I agree. I started with separating common things into new common space
and create one for risc-v. However, I dropped that approach for two
reasons.

1) The commit "b2bbe3df5470 DynamicTablesPkg: Remove PPTT ID structure
from ACPI 6.4 generator" when removed one of the enums from ArmObjectID,
didn't change the other values for other enums but reserved the removed
one. So, I thought there may be some assumptions which will break if the
enum value changes.

2) DynamicPlatformRepositoryInfo structure has ArmCmObjList and
ArmCmObjArray. With separate spaces for Arm, RiscV and Common, list
management needs some redesign and I was not sure it is worth it.

Hence, I thought a single list of all possible Obj Ids for all
architectures and common things would be a good trade off. But I can go
back to that approach in v2 if above issues are fine.

Thanks!
Sunil


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113511): https://edk2.groups.io/g/devel/message/113511
Mute This Topic: https://groups.io/mt/103622702/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

> > It looks like instead of moving the common code to
> EObjNameSpaceStandard namespace or a new (Arch? Common?) namespace,
> you're renaming the entire EObjNameSpaceArm namespace to
> EObjNameSpaceArch. It seems to me that if ARM code vs. common code is
> being separated out, then the EObjNameSpaceArm namespace should
> continue to be used for the ARM-specific code and a common namespace
> should be used for the common code.
> 
> I agree. I started with separating common things into new common space and
> create one for risc-v. However, I dropped that approach for two reasons.
> 
> 1) The commit "b2bbe3df5470 DynamicTablesPkg: Remove PPTT ID structure
> from ACPI 6.4 generator" when removed one of the enums from
> ArmObjectID, didn't change the other values for other enums but reserved the
> removed one. So, I thought there may be some assumptions which will break
> if the enum value changes.

I'm not familiar with why that was done. Hopefully someone else can comment. I do know that edk2/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c has arrays (StdNamespaceObjectParser and ArmNamespaceObjectParser) that need to be kept in sync with all of the namespace enums, but other than that I'm not aware of any places that need to be changes when the enums are changed.

> 2) DynamicPlatformRepositoryInfo structure has ArmCmObjList and
> ArmCmObjArray. With separate spaces for Arm, RiscV and Common, list
> management needs some redesign and I was not sure it is worth it.
> 
> Hence, I thought a single list of all possible Obj Ids for all architectures and
> common things would be a good trade off. But I can go back to that approach
> in v2 if above issues are fine.

Hopefully ARM can give input on the best direction before you make more changes. The DynamicPlatRepo currently only supports the ARM namespace, but comments such as "only Arm objects are supported for now." (line 144) seem to imply that support for more namespaces was considered.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113542): https://edk2.groups.io/g/devel/message/113542
Mute This Topic: https://groups.io/mt/103622702/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Hi All,

DynamicTablesPkg currently supports Arm architecture, and we welcome the adoption by other architectures.

Following is my proposal for moving forward.

Goals:
- reuse common code
- streamline the adoption by other architectures
- minimise the impact on migration of the existing platforms
- maintain flexibility across architectural components
- use this opportunity to integrate Dynamic SMBIOS support
  (Ref: https://edk2.groups.io/g/devel/message/107254)

The following steps would help in achieving the goals:
1.  Create an edk2 staging branch. For the edk2-platforms updates, I will create a branch on my Github fork (note this is required as there is no staging repo for edk2-platforms).
2. The design aspects and changes shall be discussed on the mailing list with patches to support the details.
3. A new section in DynamicTablesPkg\Readme.md shall be added to reflect the design updates, e.g. changes to CM Objects, Namespace definitions, etc.
4.  The design changes should typically be supported by patches for the DynamicTables core framework and demonstrate the impact on the existing platform code by typically providing patches for at least one existing platform (possibly edk2-platforms/Platform/ARM/[Juno | FVP]).
5. The design changes should be small and typically be reflected in separate patch series.
6. The first phase would be to partition the codebase into common code vs architectural specific code. This would involve moving files and reflecting the associated changes such that the build does not break.
7. Define a new namespace e.g. “ArchCommon” for the common architectural components.
8. Identify the CM_ARM_OBJECTs that can be moved to the “ArchCommon” namespace. As part of this identify if any object needs to be dropped, e.g. EArmObjReserved29
9. Identify overlap of SMBIOS objects with existing CM Objects.
10. Submit patches to move CM objects from Arm Namespace to ArchCommon Namespace. Ideally one object (and any dependencies) should be moved at a time.
11. Submit patches to migrate upstream platforms that use DynamicTablesPkg
12. Define a new namespace for RISCV specific objects
13. Submit patches for enabling RISCV
14. In the next phase support for Dynamic SMBIOS can be enabled.

Notes: 
a. Periodically rebase with edk2 & edk2-platforms master branch to sync with latest changes.
b. We can decide to merge the updates after point 11 above to edk2 & edk2-platforms master branch.
c. Similarly, the RISCV support can be merged after point 13.

I will send out a request for creating the staging branch shortly.

Regards,

Sami Mujawar

On 10/01/2024, 21:56, "Jeshua Smith" <jeshuas@nvidia.com <mailto:jeshuas@nvidia.com>> wrote:


> > It looks like instead of moving the common code to
> EObjNameSpaceStandard namespace or a new (Arch? Common?) namespace,
> you're renaming the entire EObjNameSpaceArm namespace to
> EObjNameSpaceArch. It seems to me that if ARM code vs. common code is
> being separated out, then the EObjNameSpaceArm namespace should
> continue to be used for the ARM-specific code and a common namespace
> should be used for the common code.
> 
> I agree. I started with separating common things into new common space and
> create one for risc-v. However, I dropped that approach for two reasons.
> 
> 1) The commit "b2bbe3df5470 DynamicTablesPkg: Remove PPTT ID structure
> from ACPI 6.4 generator" when removed one of the enums from
> ArmObjectID, didn't change the other values for other enums but reserved the
> removed one. So, I thought there may be some assumptions which will break
> if the enum value changes.


I'm not familiar with why that was done. Hopefully someone else can comment. I do know that edk2/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c has arrays (StdNamespaceObjectParser and ArmNamespaceObjectParser) that need to be kept in sync with all of the namespace enums, but other than that I'm not aware of any places that need to be changes when the enums are changed.


> 2) DynamicPlatformRepositoryInfo structure has ArmCmObjList and
> ArmCmObjArray. With separate spaces for Arm, RiscV and Common, list
> management needs some redesign and I was not sure it is worth it.
> 
> Hence, I thought a single list of all possible Obj Ids for all architectures and
> common things would be a good trade off. But I can go back to that approach
> in v2 if above issues are fine.


Hopefully ARM can give input on the best direction before you make more changes. The DynamicPlatRepo currently only supports the ARM namespace, but comments such as "only Arm objects are supported for now." (line 144) seem to imply that support for more namespaces was considered.





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114142): https://edk2.groups.io/g/devel/message/114142
Mute This Topic: https://groups.io/mt/103622702/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Hi Sami,

On Mon, Jan 22, 2024 at 05:15:44PM +0000, Sami Mujawar wrote:
> Hi All,
> 
> DynamicTablesPkg currently supports Arm architecture, and we welcome the adoption by other architectures.
> 
> Following is my proposal for moving forward.
> 
> Goals:
> - reuse common code
> - streamline the adoption by other architectures
> - minimise the impact on migration of the existing platforms
> - maintain flexibility across architectural components
> - use this opportunity to integrate Dynamic SMBIOS support
>   (Ref: https://edk2.groups.io/g/devel/message/107254)
> 
> The following steps would help in achieving the goals:
> 1.  Create an edk2 staging branch. For the edk2-platforms updates, I will create a branch on my Github fork (note this is required as there is no staging repo for edk2-platforms).
> 2. The design aspects and changes shall be discussed on the mailing list with patches to support the details.
> 3. A new section in DynamicTablesPkg\Readme.md shall be added to reflect the design updates, e.g. changes to CM Objects, Namespace definitions, etc.
> 4.  The design changes should typically be supported by patches for the DynamicTables core framework and demonstrate the impact on the existing platform code by typically providing patches for at least one existing platform (possibly edk2-platforms/Platform/ARM/[Juno | FVP]).
> 5. The design changes should be small and typically be reflected in separate patch series.
> 6. The first phase would be to partition the codebase into common code vs architectural specific code. This would involve moving files and reflecting the associated changes such that the build does not break.
> 7. Define a new namespace e.g. “ArchCommon” for the common architectural components.
> 8. Identify the CM_ARM_OBJECTs that can be moved to the “ArchCommon” namespace. As part of this identify if any object needs to be dropped, e.g. EArmObjReserved29
> 9. Identify overlap of SMBIOS objects with existing CM Objects.
> 10. Submit patches to move CM objects from Arm Namespace to ArchCommon Namespace. Ideally one object (and any dependencies) should be moved at a time.
> 11. Submit patches to migrate upstream platforms that use DynamicTablesPkg
> 12. Define a new namespace for RISCV specific objects
> 13. Submit patches for enabling RISCV
> 14. In the next phase support for Dynamic SMBIOS can be enabled.
> 
> Notes: 
> a. Periodically rebase with edk2 & edk2-platforms master branch to sync with latest changes.
> b. We can decide to merge the updates after point 11 above to edk2 & edk2-platforms master branch.
> c. Similarly, the RISCV support can be merged after point 13.
> 
> I will send out a request for creating the staging branch shortly.
> 
This is great!. I think staging branches is a great idea considering the
amount of changes this work needs. Thank you very much!.

I can send small patch sets to partition the code base as per #6 along
with documentation changes. Let me know once staging branches are
created.

Thanks!
Sunil


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114181): https://edk2.groups.io/g/devel/message/114181
Mute This Topic: https://groups.io/mt/103622702/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-