[PATCH v3 0/8] Add ACPI support for Kvmtool

[PATCH v3 0/8] Add ACPI support for Kvmtool

[PierreGondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3742
V1: https://edk2.groups.io/g/devel/message/76990
V2:
- New patch: "DynamicTablesPkg: Print specifier macro for
  CM_OBJECT_ID" [Laszlo]
- Only add AcpiView for ArmVirtKvmTool instead of all ArmVirt
  platforms. This is done using a 'ACPIVIEW_ENABLE' switch.
  [Laszlo]
- Only generate ACPI tables for AARCH64. [Laszlo]
- Various modifications (error handling, patch organization,
  coding style, etc). [Laszlo]
V3:
- To fix bugs reported by Linux (signaled by Ard):
  - Add patch to parse and populate PMU information.
  - Add patch to strictly comply to the first model at ACPI
    6.4 s6.2.13 to describe PCI legacy interrupts using _PRT.
  - Add address size limit to IORT PCI root complex.

The changes can be seen at:
https://github.com/PierreARM/edk2/tree/1456_Add_ACPI_support_for_Kvmtool_v3
The results of the CI can be seen at:
https://github.com/tianocore/edk2/pull/2451/checks?check_run_id=4982549008

Kvmtool dynamically generates a device tree describing the platform
to boot on. Using the patch-sets listed below, the DynamicTables
framework generates ACPI tables describing a similar platform.

This patch-set:
 - adds a ConfigurationManager and make use of the DynamicTablesPkg
   in for Kvmtool for AARCH64, allowing to generate ACPI tables
 - adds the acpiview command line utility to the ArmVirtPkg
   platform that request if via the ACPIVIEW_ENABLE macro
 - update ArmVirtPkg.ci.yaml to add new words and use the
   DynamicTablesPkg
 - adds a print specifier macro for the CM_OBJECT_ID type

With this patchset, KvmTool on AARCH64 will use ACPI tables instead
of a Device Tree (cf PcdForceNoAcpi Pcd).

Pierre Gondois (4):
  DynamicTablesPkg: Print specifier macro for CM_OBJECT_ID
  DynamicTablesPkg: FdtHwInfoParserLib: Parse Pmu info
  DynamicTablesPkg: AcpiSsdtPcieLibArm: Fix _PRT description
  ArmVirtPkg: Add cspell exceptions

Sami Mujawar (4):
  ArmVirtPkg/Kvmtool: Add DSDT ACPI table
  ArmVirtPkg/Kvmtool: Add Configuration Manager
  ArmVirtPkg/Kvmtool: Enable ACPI support
  ArmVirtPkg/Kvmtool: Enable Acpiview

 ArmVirtPkg/ArmVirt.dsc.inc                    |    5 +-
 ArmVirtPkg/ArmVirtKvmTool.dsc                 |   22 +-
 ArmVirtPkg/ArmVirtKvmTool.fdf                 |   15 +-
 ArmVirtPkg/ArmVirtPkg.ci.yaml                 |    6 +-
 .../KvmtoolCfgMgrDxe/AslTables/Dsdt.asl       |   21 +
 .../KvmtoolCfgMgrDxe/ConfigurationManager.c   | 1065 +++++++++++++++++
 .../KvmtoolCfgMgrDxe/ConfigurationManager.h   |  125 ++
 .../ConfigurationManagerDxe.inf               |   54 +
 .../Include/ConfigurationManagerObject.h      |    7 +-
 .../AcpiSsdtPcieLibArm/SsdtPcieGenerator.c    |   47 +-
 .../FdtHwInfoParserLib/Gic/ArmGicCParser.c    |  131 +-
 .../FdtHwInfoParserLib/Gic/ArmGicCParser.h    |    8 +-
 12 files changed, 1491 insertions(+), 15 deletions(-)
 create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/AslTables/Dsdt.asl
 create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.c
 create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.h
 create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf

-- 
2.25.1

[PATCH v3 1/8] DynamicTablesPkg: Print specifier macro for CM_OBJECT_ID

[PierreGondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

Add a macro that specifies the format for printing CM_OBJECT_ID.
This allows to print the CM_OBJECT_ID is a consistent way in the
output logs.

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---

Notes:
    v2:
    - New patch, requested by Laszlo.

 DynamicTablesPkg/Include/ConfigurationManagerObject.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/DynamicTablesPkg/Include/ConfigurationManagerObject.h b/DynamicTablesPkg/Include/ConfigurationManagerObject.h
index 60d825a2b253..74ad25d5d94a 100644
--- a/DynamicTablesPkg/Include/ConfigurationManagerObject.h
+++ b/DynamicTablesPkg/Include/ConfigurationManagerObject.h
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2017 - 2019, ARM Limited. All rights reserved.
+  Copyright (c) 2017 - 2022, ARM Limited. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -84,6 +84,11 @@ Object ID's in the ARM Namespace:
 */
 typedef UINT32 CM_OBJECT_ID;
 
+//
+// Helper macro to format a CM_OBJECT_ID.
+//
+#define FMT_CM_OBJECT_ID  "0x%lx"
+
 /** A mask for Object ID
 */
 #define OBJECT_ID_MASK  0xFF
-- 
2.25.1

[PATCH v3 2/8] DynamicTablesPkg: FdtHwInfoParserLib: Parse Pmu info

[PierreGondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

Parse the Pmu interrupts if a pmu compatible node is present,
and populate the MADT GicC structure accordingly.

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---

Notes:
    v3:
     - New patch. [Pierre]

 .../FdtHwInfoParserLib/Gic/ArmGicCParser.c    | 131 +++++++++++++++++-
 .../FdtHwInfoParserLib/Gic/ArmGicCParser.h    |   8 +-
 2 files changed, 135 insertions(+), 4 deletions(-)

diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
index b4e6729a4ab2..961607378449 100644
--- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
+++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
@@ -1,13 +1,14 @@
 /** @file
   Arm Gic cpu parser.
 
-  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
+  Copyright (c) 2021 - 2022, Arm Limited. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
   - linux/Documentation/devicetree/bindings/arm/cpus.yaml
   - linux/Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml
   - linux/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
+  - linux/Documentation/devicetree/bindings/arm/pmu.yaml
 **/
 
 #include "FdtHwInfoParser.h"
@@ -34,6 +35,21 @@ STATIC CONST COMPATIBILITY_INFO  CpuCompatibleInfo = {
   CpuCompatibleStr
 };
 
+/** Pmu compatible strings.
+
+  Any other "compatible" value is not supported by this module.
+*/
+STATIC CONST COMPATIBILITY_STR  PmuCompatibleStr[] = {
+  { "arm,armv8-pmuv3" }
+};
+
+/** COMPATIBILITY_INFO structure for the PmuCompatibleStr.
+*/
+CONST COMPATIBILITY_INFO  PmuCompatibleInfo = {
+  ARRAY_SIZE (PmuCompatibleStr),
+  PmuCompatibleStr
+};
+
 /** Parse a "cpu" node.
 
   @param [in]  Fdt              Pointer to a Flattened Device Tree (Fdt).
@@ -639,6 +655,110 @@ GicCv3IntcNodeParser (
   return EFI_SUCCESS;
 }
 
+/** Parse a Pmu compatible node, extracting Pmu information.
+
+  This function modifies a CM_OBJ_DESCRIPTOR object.
+  The following CM_ARM_GICC_INFO fields are patched:
+    - PerformanceInterruptGsiv;
+
+  @param [in]       Fdt              Pointer to a Flattened Device Tree (Fdt).
+  @param [in]       GicIntcNode      Offset of a Gic compatible
+                                     interrupt-controller node.
+  @param [in, out]  GicCCmObjDesc    The CM_ARM_GICC_INFO to patch.
+
+  @retval EFI_SUCCESS             The function completed successfully.
+  @retval EFI_ABORTED             An error occurred.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+GicCPmuNodeParser (
+  IN      CONST VOID               *Fdt,
+  IN            INT32              GicIntcNode,
+  IN  OUT       CM_OBJ_DESCRIPTOR  *GicCCmObjDesc
+  )
+{
+  EFI_STATUS        Status;
+  INT32             IntCells;
+  INT32             PmuNode;
+  UINT32            PmuNodeCount;
+  UINT32            PmuIrq;
+  UINT32            Index;
+  CM_ARM_GICC_INFO  *GicCInfo;
+  CONST UINT8       *Data;
+  INT32             DataSize;
+
+  if (GicCCmObjDesc == NULL) {
+    ASSERT (0);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  GicCInfo = (CM_ARM_GICC_INFO *)GicCCmObjDesc->Data;
+  PmuNode  = 0;
+
+  // Count the number of pmu nodes.
+  Status = FdtCountCompatNodeInBranch (
+             Fdt,
+             0,
+             &PmuCompatibleInfo,
+             &PmuNodeCount
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
+  if (PmuNodeCount == 0) {
+    return EFI_NOT_FOUND;
+  }
+
+  Status = FdtGetNextCompatNodeInBranch (
+             Fdt,
+             0,
+             &PmuCompatibleInfo,
+             &PmuNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    if (Status == EFI_NOT_FOUND) {
+      // Should have found the node.
+      Status = EFI_ABORTED;
+    }
+  }
+
+  // Get the number of cells used to encode an interrupt.
+  Status = FdtGetInterruptCellsInfo (Fdt, GicIntcNode, &IntCells);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
+  Data = fdt_getprop (Fdt, PmuNode, "interrupts", &DataSize);
+  if ((Data == NULL) || (DataSize != (IntCells * sizeof (UINT32)))) {
+    // If error or not 1 interrupt.
+    ASSERT (0);
+    return EFI_ABORTED;
+  }
+
+  PmuIrq = FdtGetInterruptId ((CONST UINT32 *)Data);
+
+  // Only supports PPI 23 for now.
+  // According to BSA 1.0 s3.6 PPI assignments, PMU IRQ ID is 23. A non BSA
+  // compliant system may assign a different IRQ for the PMU, however this
+  // is not implemented for now.
+  if (PmuIrq != BSA_PMU_IRQ) {
+    ASSERT (0);
+    return EFI_ABORTED;
+  }
+
+  for (Index = 0; Index < GicCCmObjDesc->Count; Index++) {
+    GicCInfo[Index].PerformanceInterruptGsiv = PmuIrq;
+  }
+
+  return EFI_SUCCESS;
+}
+
 /** CM_ARM_GICC_INFO parser function.
 
   This parser expects FdtBranch to be the "\cpus" node node.
@@ -649,7 +769,7 @@ GicCv3IntcNodeParser (
     UINT32  AcpiProcessorUid;                 // {Populated}
     UINT32  Flags;                            // {Populated}
     UINT32  ParkingProtocolVersion;           // {default = 0}
-    UINT32  PerformanceInterruptGsiv;         // {default = 0}
+    UINT32  PerformanceInterruptGsiv;         // {Populated}
     UINT64  ParkedAddress;                    // {default = 0}
     UINT64  PhysicalBaseAddress;              // {Populated}
     UINT64  GICV;                             // {Populated}
@@ -764,6 +884,13 @@ ArmGicCInfoParser (
     goto exit_handler;
   }
 
+  // Parse the Pmu Interrupt.
+  Status = GicCPmuNodeParser (Fdt, IntcNode, NewCmObjDesc);
+  if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
+    ASSERT (0);
+    goto exit_handler;
+  }
+
   // Add all the CmObjs to the Configuration Manager.
   Status = AddMultipleCmObj (FdtParserHandle, NewCmObjDesc, 0, NULL);
   if (EFI_ERROR (Status)) {
diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.h b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.h
index 2a0f966bf0c2..fd980484a28d 100644
--- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.h
+++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.h
@@ -1,7 +1,7 @@
 /** @file
   Arm Gic cpu parser.
 
-  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
+  Copyright (c) 2021 - 2022, Arm Limited. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
@@ -12,6 +12,10 @@
 #ifndef ARM_GICC_PARSER_H_
 #define ARM_GICC_PARSER_H_
 
+/* According to BSA 1.0 s3.6 PPI assignments, PMU IRQ ID is 23.
+*/
+#define BSA_PMU_IRQ  23
+
 /** CM_ARM_GICC_INFO parser function.
 
   This parser expects FdtBranch to be the "\cpus" node node.
@@ -22,7 +26,7 @@
     UINT32  AcpiProcessorUid;                 // {Populated}
     UINT32  Flags;                            // {Populated}
     UINT32  ParkingProtocolVersion;           // {default = 0}
-    UINT32  PerformanceInterruptGsiv;         // {default = 0}
+    UINT32  PerformanceInterruptGsiv;         // {Populated}
     UINT64  ParkedAddress;                    // {default = 0}
     UINT64  PhysicalBaseAddress;              // {Populated}
     UINT64  GICV;                             // {Populated}
-- 
2.25.1

Re: [edk2-devel] [PATCH v3 2/8] DynamicTablesPkg: FdtHwInfoParserLib: Parse Pmu info

[Ard Biesheuvel]

On Fri, 28 Jan 2022 at 16:41, PierreGondois <pierre.gondois@arm.com> wrote:
>
> From: Pierre Gondois <Pierre.Gondois@arm.com>
>
> Parse the Pmu interrupts if a pmu compatible node is present,
> and populate the MADT GicC structure accordingly.
>
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
>
> Notes:
>     v3:
>      - New patch. [Pierre]
>
>  .../FdtHwInfoParserLib/Gic/ArmGicCParser.c    | 131 +++++++++++++++++-
>  .../FdtHwInfoParserLib/Gic/ArmGicCParser.h    |   8 +-
>  2 files changed, 135 insertions(+), 4 deletions(-)
>
> diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
> index b4e6729a4ab2..961607378449 100644
> --- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
> +++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
> @@ -1,13 +1,14 @@
>  /** @file
>    Arm Gic cpu parser.
>
> -  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
> +  Copyright (c) 2021 - 2022, Arm Limited. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
>    @par Reference(s):
>    - linux/Documentation/devicetree/bindings/arm/cpus.yaml
>    - linux/Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml
>    - linux/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
> +  - linux/Documentation/devicetree/bindings/arm/pmu.yaml
>  **/
>
>  #include "FdtHwInfoParser.h"
> @@ -34,6 +35,21 @@ STATIC CONST COMPATIBILITY_INFO  CpuCompatibleInfo = {
>    CpuCompatibleStr
>  };
>
> +/** Pmu compatible strings.
> +
> +  Any other "compatible" value is not supported by this module.
> +*/
> +STATIC CONST COMPATIBILITY_STR  PmuCompatibleStr[] = {
> +  { "arm,armv8-pmuv3" }
> +};
> +
> +/** COMPATIBILITY_INFO structure for the PmuCompatibleStr.
> +*/
> +CONST COMPATIBILITY_INFO  PmuCompatibleInfo = {
> +  ARRAY_SIZE (PmuCompatibleStr),
> +  PmuCompatibleStr
> +};
> +
>  /** Parse a "cpu" node.
>
>    @param [in]  Fdt              Pointer to a Flattened Device Tree (Fdt).
> @@ -639,6 +655,110 @@ GicCv3IntcNodeParser (
>    return EFI_SUCCESS;
>  }
>
> +/** Parse a Pmu compatible node, extracting Pmu information.
> +
> +  This function modifies a CM_OBJ_DESCRIPTOR object.
> +  The following CM_ARM_GICC_INFO fields are patched:
> +    - PerformanceInterruptGsiv;
> +
> +  @param [in]       Fdt              Pointer to a Flattened Device Tree (Fdt).
> +  @param [in]       GicIntcNode      Offset of a Gic compatible
> +                                     interrupt-controller node.
> +  @param [in, out]  GicCCmObjDesc    The CM_ARM_GICC_INFO to patch.
> +
> +  @retval EFI_SUCCESS             The function completed successfully.
> +  @retval EFI_ABORTED             An error occurred.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +GicCPmuNodeParser (
> +  IN      CONST VOID               *Fdt,
> +  IN            INT32              GicIntcNode,
> +  IN  OUT       CM_OBJ_DESCRIPTOR  *GicCCmObjDesc
> +  )
> +{
> +  EFI_STATUS        Status;
> +  INT32             IntCells;
> +  INT32             PmuNode;
> +  UINT32            PmuNodeCount;
> +  UINT32            PmuIrq;
> +  UINT32            Index;
> +  CM_ARM_GICC_INFO  *GicCInfo;
> +  CONST UINT8       *Data;
> +  INT32             DataSize;
> +
> +  if (GicCCmObjDesc == NULL) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  GicCInfo = (CM_ARM_GICC_INFO *)GicCCmObjDesc->Data;
> +  PmuNode  = 0;
> +
> +  // Count the number of pmu nodes.
> +  Status = FdtCountCompatNodeInBranch (
> +             Fdt,
> +             0,
> +             &PmuCompatibleInfo,
> +             &PmuNodeCount
> +             );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);

If you use ASSERT_EFI_ERROR() here, you at least get the type of error
as DEBUG output, whereas ASSERT(0) does not give any context
whatsoever.

> +    return Status;
> +  }
> +
> +  if (PmuNodeCount == 0) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  Status = FdtGetNextCompatNodeInBranch (
> +             Fdt,
> +             0,
> +             &PmuCompatibleInfo,
> +             &PmuNode
> +             );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);

Same here

> +    if (Status == EFI_NOT_FOUND) {
> +      // Should have found the node.
> +      Status = EFI_ABORTED;
> +    }
> +  }
> +
> +  // Get the number of cells used to encode an interrupt.
> +  Status = FdtGetInterruptCellsInfo (Fdt, GicIntcNode, &IntCells);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);

and here

> +    return Status;
> +  }
> +
> +  Data = fdt_getprop (Fdt, PmuNode, "interrupts", &DataSize);
> +  if ((Data == NULL) || (DataSize != (IntCells * sizeof (UINT32)))) {
> +    // If error or not 1 interrupt.
> +    ASSERT (0);
> +    return EFI_ABORTED;
> +  }
> +
> +  PmuIrq = FdtGetInterruptId ((CONST UINT32 *)Data);
> +
> +  // Only supports PPI 23 for now.
> +  // According to BSA 1.0 s3.6 PPI assignments, PMU IRQ ID is 23. A non BSA
> +  // compliant system may assign a different IRQ for the PMU, however this
> +  // is not implemented for now.
> +  if (PmuIrq != BSA_PMU_IRQ) {
> +    ASSERT (0);
> +    return EFI_ABORTED;
> +  }
> +
> +  for (Index = 0; Index < GicCCmObjDesc->Count; Index++) {
> +    GicCInfo[Index].PerformanceInterruptGsiv = PmuIrq;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
>  /** CM_ARM_GICC_INFO parser function.
>
>    This parser expects FdtBranch to be the "\cpus" node node.
> @@ -649,7 +769,7 @@ GicCv3IntcNodeParser (
>      UINT32  AcpiProcessorUid;                 // {Populated}
>      UINT32  Flags;                            // {Populated}
>      UINT32  ParkingProtocolVersion;           // {default = 0}
> -    UINT32  PerformanceInterruptGsiv;         // {default = 0}
> +    UINT32  PerformanceInterruptGsiv;         // {Populated}
>      UINT64  ParkedAddress;                    // {default = 0}
>      UINT64  PhysicalBaseAddress;              // {Populated}
>      UINT64  GICV;                             // {Populated}
> @@ -764,6 +884,13 @@ ArmGicCInfoParser (
>      goto exit_handler;
>    }
>
> +  // Parse the Pmu Interrupt.
> +  Status = GicCPmuNodeParser (Fdt, IntcNode, NewCmObjDesc);
> +  if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
> +    ASSERT (0);

and here

> +    goto exit_handler;

shouldn't this be CamelCase?

> +  }
> +
>    // Add all the CmObjs to the Configuration Manager.
>    Status = AddMultipleCmObj (FdtParserHandle, NewCmObjDesc, 0, NULL);
>    if (EFI_ERROR (Status)) {
> diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.h b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.h
> index 2a0f966bf0c2..fd980484a28d 100644
> --- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.h
> +++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.h
> @@ -1,7 +1,7 @@
>  /** @file
>    Arm Gic cpu parser.
>
> -  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
> +  Copyright (c) 2021 - 2022, Arm Limited. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
>    @par Reference(s):
> @@ -12,6 +12,10 @@
>  #ifndef ARM_GICC_PARSER_H_
>  #define ARM_GICC_PARSER_H_
>
> +/* According to BSA 1.0 s3.6 PPI assignments, PMU IRQ ID is 23.
> +*/
> +#define BSA_PMU_IRQ  23
> +
>  /** CM_ARM_GICC_INFO parser function.
>
>    This parser expects FdtBranch to be the "\cpus" node node.
> @@ -22,7 +26,7 @@
>      UINT32  AcpiProcessorUid;                 // {Populated}
>      UINT32  Flags;                            // {Populated}
>      UINT32  ParkingProtocolVersion;           // {default = 0}
> -    UINT32  PerformanceInterruptGsiv;         // {default = 0}
> +    UINT32  PerformanceInterruptGsiv;         // {Populated}
>      UINT64  ParkedAddress;                    // {default = 0}
>      UINT64  PhysicalBaseAddress;              // {Populated}
>      UINT64  GICV;                             // {Populated}
> --
> 2.25.1
>
>
>
> ------------
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#86169): https://edk2.groups.io/g/devel/message/86169
> Mute This Topic: https://groups.io/mt/88746969/1131722
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ardb@kernel.org]
> ------------
>
>

[PATCH v3 3/8] DynamicTablesPkg: AcpiSsdtPcieLibArm: Fix _PRT description

[PierreGondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

In ACPI 6.4, s6.2.13, _PRT objects describing PCI legacy interrupts
can be defined following 2 models.
In the first model, a _SRS object must be described to modify the PCI
interrupt. The _CRS and _PRS object allows to describe the PCI
interrupt (level/edge triggered, active high/low).
In the second model, the PCI interrupt cannot be described with a
similar granularity. PCI interrupts are by default level triggered,
active low.

GicV2 SPI interrupts are level or edge triggered, active high. To
correctly describe PCI interrupts, the first model is used, even though
Arm Base Boot Requirements v1.0 requires to use the second mode.

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---

Notes:
    v3:
     - New patch. [Pierre]

 .../AcpiSsdtPcieLibArm/SsdtPcieGenerator.c    | 47 +++++++++++++++++--
 1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
index 3e22587d4a25..6823a98795c8 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
@@ -1,7 +1,7 @@
 /** @file
   SSDT Pcie Table Generator.
 
-  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2021 - 2022, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -295,6 +295,10 @@ GeneratePciDeviceInfo (
     Method (_CRS, 0) {
       Return CRS0
       })
+    Method (_PRS, 0) {
+      Return CRS0
+      })
+    Method (_SRS, 1) { }
     Method (_DIS) { }
   }
 
@@ -302,9 +306,12 @@ GeneratePciDeviceInfo (
   PCI Firmware Specification - Revision 3.3,
   s3.5. "Device State at Firmware/Operating System Handoff"
 
-  The _PRS and _SRS are not supported, cf Arm Base Boot Requirements v1.0:
+  Warning: The Arm Base Boot Requirements v1.0 states the following:
   "The _PRS (Possible Resource Settings) and _SRS (Set Resource Settings)
   are not supported."
+  However, the first model to describe PCI legacy interrupts is used (cf. ACPI
+  6.4 s6.2.13) as PCI defaults (Level Triggered, Active Low) are not compatible
+  with GICv2 (must be Active High).
 
   @param [in]       Irq         Interrupt controller interrupt.
   @param [in]       IrqFlags    Interrupt flags.
@@ -416,7 +423,41 @@ GenerateLinkDevice (
     return Status;
   }
 
-  // ASL:Method (_DIS, 1) {}
+  // ASL:
+  // Method (_PRS, 0) {
+  //   Return (CRS0)
+  // }
+  Status = AmlCodeGenMethodRetNameString (
+             "_PRS",
+             "CRS0",
+             0,
+             FALSE,
+             0,
+             LinkNode,
+             NULL
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
+  // ASL:Method (_SRS, 1) {}
+  // Not possible to disable interrupts.
+  Status = AmlCodeGenMethodRetNameString (
+             "_SRS",
+             NULL,
+             1,
+             FALSE,
+             0,
+             LinkNode,
+             NULL
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
+  // ASL:Method (_DIS, 0) {}
   // Not possible to disable interrupts.
   Status = AmlCodeGenMethodRetNameString (
              "_DIS",
-- 
2.25.1

Re: [PATCH v3 3/8] DynamicTablesPkg: AcpiSsdtPcieLibArm: Fix _PRT description

[Ard Biesheuvel]

(+ Marc)

On Fri, 28 Jan 2022 at 16:41, <Pierre.Gondois@arm.com> wrote:
>
> From: Pierre Gondois <Pierre.Gondois@arm.com>
>
> In ACPI 6.4, s6.2.13, _PRT objects describing PCI legacy interrupts
> can be defined following 2 models.
> In the first model, a _SRS object must be described to modify the PCI
> interrupt. The _CRS and _PRS object allows to describe the PCI
> interrupt (level/edge triggered, active high/low).
> In the second model, the PCI interrupt cannot be described with a
> similar granularity. PCI interrupts are by default level triggered,
> active low.
>
> GicV2 SPI interrupts are level or edge triggered, active high. To
> correctly describe PCI interrupts, the first model is used, even though
> Arm Base Boot Requirements v1.0 requires to use the second mode.
>

There are two different issues here:

- using separate 'interrupt link' device objects with an Interrupt()
resource rather than a simple GSIV number
- whether _PRS and _SRS need to be implemented on those link objects.

The latter is simply not true - _PRS and _SRS are optional, and
pointless if there is only a single possible value, so there is really
no need to add them here.

As for the choice between the link object or the GSIV number: I don't
think INTx interrupts on ARM systems are actually level low, and the
GSIV option is widely used, also in platforms that exist in
edk2-platforms, without any reported issues.

I've cc'ed Marc, perhaps he can shed some light on this, but I'd
prefer to stick to the GSIV approach if we can, as it is much simpler.



> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
>
> Notes:
>     v3:
>      - New patch. [Pierre]
>
>  .../AcpiSsdtPcieLibArm/SsdtPcieGenerator.c    | 47 +++++++++++++++++--
>  1 file changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> index 3e22587d4a25..6823a98795c8 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> @@ -1,7 +1,7 @@
>  /** @file
>    SSDT Pcie Table Generator.
>
> -  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
> +  Copyright (c) 2021 - 2022, Arm Limited. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -295,6 +295,10 @@ GeneratePciDeviceInfo (
>      Method (_CRS, 0) {
>        Return CRS0
>        })
> +    Method (_PRS, 0) {
> +      Return CRS0
> +      })
> +    Method (_SRS, 1) { }
>      Method (_DIS) { }
>    }
>
> @@ -302,9 +306,12 @@ GeneratePciDeviceInfo (
>    PCI Firmware Specification - Revision 3.3,
>    s3.5. "Device State at Firmware/Operating System Handoff"
>
> -  The _PRS and _SRS are not supported, cf Arm Base Boot Requirements v1.0:
> +  Warning: The Arm Base Boot Requirements v1.0 states the following:
>    "The _PRS (Possible Resource Settings) and _SRS (Set Resource Settings)
>    are not supported."
> +  However, the first model to describe PCI legacy interrupts is used (cf. ACPI
> +  6.4 s6.2.13) as PCI defaults (Level Triggered, Active Low) are not compatible
> +  with GICv2 (must be Active High).
>
>    @param [in]       Irq         Interrupt controller interrupt.
>    @param [in]       IrqFlags    Interrupt flags.
> @@ -416,7 +423,41 @@ GenerateLinkDevice (
>      return Status;
>    }
>
> -  // ASL:Method (_DIS, 1) {}
> +  // ASL:
> +  // Method (_PRS, 0) {
> +  //   Return (CRS0)
> +  // }
> +  Status = AmlCodeGenMethodRetNameString (
> +             "_PRS",
> +             "CRS0",
> +             0,
> +             FALSE,
> +             0,
> +             LinkNode,
> +             NULL
> +             );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +    return Status;
> +  }
> +
> +  // ASL:Method (_SRS, 1) {}
> +  // Not possible to disable interrupts.
> +  Status = AmlCodeGenMethodRetNameString (
> +             "_SRS",
> +             NULL,
> +             1,
> +             FALSE,
> +             0,
> +             LinkNode,
> +             NULL
> +             );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +    return Status;
> +  }
> +
> +  // ASL:Method (_DIS, 0) {}
>    // Not possible to disable interrupts.
>    Status = AmlCodeGenMethodRetNameString (
>               "_DIS",
> --
> 2.25.1
>

Re: [PATCH v3 3/8] DynamicTablesPkg: AcpiSsdtPcieLibArm: Fix _PRT description

[Marc Zyngier]

On Sat, 29 Jan 2022 15:52:02 +0000,
Ard Biesheuvel <ardb@kernel.org> wrote:
> 
> (+ Marc)
> 
> On Fri, 28 Jan 2022 at 16:41, <Pierre.Gondois@arm.com> wrote:
> >
> > From: Pierre Gondois <Pierre.Gondois@arm.com>
> >
> > In ACPI 6.4, s6.2.13, _PRT objects describing PCI legacy interrupts
> > can be defined following 2 models.
> > In the first model, a _SRS object must be described to modify the PCI
> > interrupt. The _CRS and _PRS object allows to describe the PCI
> > interrupt (level/edge triggered, active high/low).
> > In the second model, the PCI interrupt cannot be described with a
> > similar granularity. PCI interrupts are by default level triggered,
> > active low.
> >
> > GicV2 SPI interrupts are level or edge triggered, active high. To
> > correctly describe PCI interrupts, the first model is used, even though
> > Arm Base Boot Requirements v1.0 requires to use the second mode.
> >
> 
> There are two different issues here:
> 
> - using separate 'interrupt link' device objects with an Interrupt()
> resource rather than a simple GSIV number
> - whether _PRS and _SRS need to be implemented on those link objects.
> 
> The latter is simply not true - _PRS and _SRS are optional, and
> pointless if there is only a single possible value, so there is really
> no need to add them here.
> 
> As for the choice between the link object or the GSIV number: I don't
> think INTx interrupts on ARM systems are actually level low, and the
> GSIV option is widely used, also in platforms that exist in
> edk2-platforms, without any reported issues.
> 
> I've cc'ed Marc, perhaps he can shed some light on this, but I'd
> prefer to stick to the GSIV approach if we can, as it is much simpler.

I don't immediately see the point either. Yes, the GIC only supports
level-high interrupts. However, all the PCIe implementations connected
to it are inverting the level. If they don't, that's even simpler (the
HW is broken).

Is this to address an apparent disconnect with the spec?

[...]

> > -  The _PRS and _SRS are not supported, cf Arm Base Boot Requirements v1.0:
> > +  Warning: The Arm Base Boot Requirements v1.0 states the following:
> >    "The _PRS (Possible Resource Settings) and _SRS (Set Resource Settings)
> >    are not supported."
> > +  However, the first model to describe PCI legacy interrupts is used (cf. ACPI
> > +  6.4 s6.2.13) as PCI defaults (Level Triggered, Active Low) are not compatible
> > +  with GICv2 (must be Active High).

nit: GICv3 has the exact same behaviour. Why is v2 singled out?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v3 3/8] DynamicTablesPkg: AcpiSsdtPcieLibArm: Fix _PRT description

[PierreGondois]

Hi,

On 1/29/22 7:20 PM, Marc Zyngier wrote:
> On Sat, 29 Jan 2022 15:52:02 +0000,
> Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> (+ Marc)
>>
>> On Fri, 28 Jan 2022 at 16:41, <Pierre.Gondois@arm.com> wrote:
>>>
>>> From: Pierre Gondois <Pierre.Gondois@arm.com>
>>>
>>> In ACPI 6.4, s6.2.13, _PRT objects describing PCI legacy interrupts
>>> can be defined following 2 models.
>>> In the first model, a _SRS object must be described to modify the PCI
>>> interrupt. The _CRS and _PRS object allows to describe the PCI
>>> interrupt (level/edge triggered, active high/low).
>>> In the second model, the PCI interrupt cannot be described with a
>>> similar granularity. PCI interrupts are by default level triggered,
>>> active low.
>>>
>>> GicV2 SPI interrupts are level or edge triggered, active high. To
>>> correctly describe PCI interrupts, the first model is used, even though
>>> Arm Base Boot Requirements v1.0 requires to use the second mode.
>>>
>>
>> There are two different issues here:
>>
>> - using separate 'interrupt link' device objects with an Interrupt()
>> resource rather than a simple GSIV number
>> - whether _PRS and _SRS need to be implemented on those link objects.
>>
>> The latter is simply not true - _PRS and _SRS are optional, and
>> pointless if there is only a single possible value, so there is really
>> no need to add them here.
>>
>> As for the choice between the link object or the GSIV number: I don't
>> think INTx interrupts on ARM systems are actually level low, and the
>> GSIV option is widely used, also in platforms that exist in
>> edk2-platforms, without any reported issues.
>>
>> I've cc'ed Marc, perhaps he can shed some light on this, but I'd
>> prefer to stick to the GSIV approach if we can, as it is much simpler.
> 
> I don't immediately see the point either. Yes, the GIC only supports
> level-high interrupts. However, all the PCIe implementations connected
> to it are inverting the level. If they don't, that's even simpler (the
> HW is broken).
> 
> Is this to address an apparent disconnect with the spec?
> 
> [...]
> 

1. _PRS/_SRS methods
I agree they are optional and meaningless here as interrupts are not dynamically configurable.
However linux is checking that:
- the interrupt used in _CRS is one of the possible interrupts advertised in _PRS. If not, then a warning is issued and the interrupt is not used.
- the _SRS method is present. If not, setting the interrupt fails.
If _PRS and _SRS method are really optional, it seems these checks should not happen.
For now, using a link object without _PRS/_SRS doesn't work.

Note:
The first check was initially done because an invalid interrupt was advertised in _CRS when valid interrupts were available in _PRS.
https://bugzilla.kernel.org/show_bug.cgi?id=2665

2. GSIV vs link object
The fist motivation was to accurately describe the interrupts.
Even though GIC interrupts must be active high, PCI interrupts are active low by default (according to spec), and the GSIV model doesn't allow to describe the polarity/activation state.

Another point that came out is that in linux, GSIV interrupts for PCI are configured as level triggered by default.
 From "Base System Architecture 1.0", sE.4 and sE.6, PCI interrupts can be level or edge triggered. More specifically, KvmTool configures PCI interrupts as edge triggered.
So the only way to describe an edge interrupt is to use a link object.


>>> -  The _PRS and _SRS are not supported, cf Arm Base Boot Requirements v1.0:
>>> +  Warning: The Arm Base Boot Requirements v1.0 states the following:
>>>     "The _PRS (Possible Resource Settings) and _SRS (Set Resource Settings)
>>>     are not supported."
>>> +  However, the first model to describe PCI legacy interrupts is used (cf. ACPI
>>> +  6.4 s6.2.13) as PCI defaults (Level Triggered, Active Low) are not compatible
>>> +  with GICv2 (must be Active High).
> 
> nit: GICv3 has the exact same behaviour. Why is v2 singled out?

No specific reason, GICv3 should have been in the comment aswell.

> 
> Thanks,
> 
> 	M.
> 

Regards,
Pierre

Re: [PATCH v3 3/8] DynamicTablesPkg: AcpiSsdtPcieLibArm: Fix _PRT description

[Marc Zyngier]

On Mon, 31 Jan 2022 12:59:11 +0000,
Pierre Gondois <pierre.gondois@arm.com> wrote:
> 
> Hi,
> 
> On 1/29/22 7:20 PM, Marc Zyngier wrote:
> > On Sat, 29 Jan 2022 15:52:02 +0000,
> > Ard Biesheuvel <ardb@kernel.org> wrote:
> >> 
> >> (+ Marc)
> >> 
> >> On Fri, 28 Jan 2022 at 16:41, <Pierre.Gondois@arm.com> wrote:
> >>> 
> >>> From: Pierre Gondois <Pierre.Gondois@arm.com>
> >>> 
> >>> In ACPI 6.4, s6.2.13, _PRT objects describing PCI legacy interrupts
> >>> can be defined following 2 models.
> >>> In the first model, a _SRS object must be described to modify the PCI
> >>> interrupt. The _CRS and _PRS object allows to describe the PCI
> >>> interrupt (level/edge triggered, active high/low).
> >>> In the second model, the PCI interrupt cannot be described with a
> >>> similar granularity. PCI interrupts are by default level triggered,
> >>> active low.
> >>> 
> >>> GicV2 SPI interrupts are level or edge triggered, active high. To
> >>> correctly describe PCI interrupts, the first model is used, even though
> >>> Arm Base Boot Requirements v1.0 requires to use the second mode.
> >>> 
> >> 
> >> There are two different issues here:
> >> 
> >> - using separate 'interrupt link' device objects with an Interrupt()
> >> resource rather than a simple GSIV number
> >> - whether _PRS and _SRS need to be implemented on those link objects.
> >> 
> >> The latter is simply not true - _PRS and _SRS are optional, and
> >> pointless if there is only a single possible value, so there is really
> >> no need to add them here.
> >> 
> >> As for the choice between the link object or the GSIV number: I don't
> >> think INTx interrupts on ARM systems are actually level low, and the
> >> GSIV option is widely used, also in platforms that exist in
> >> edk2-platforms, without any reported issues.
> >> 
> >> I've cc'ed Marc, perhaps he can shed some light on this, but I'd
> >> prefer to stick to the GSIV approach if we can, as it is much simpler.
> > 
> > I don't immediately see the point either. Yes, the GIC only supports
> > level-high interrupts. However, all the PCIe implementations connected
> > to it are inverting the level. If they don't, that's even simpler (the
> > HW is broken).
> > 
> > Is this to address an apparent disconnect with the spec?
> > 
> > [...]
> > 
> 
> 1. _PRS/_SRS methods
> I agree they are optional and meaningless here as interrupts are not dynamically configurable.
> However linux is checking that:
> - the interrupt used in _CRS is one of the possible interrupts advertised in _PRS. If not, then a warning is issued and the interrupt is not used.
> - the _SRS method is present. If not, setting the interrupt fails.
> If _PRS and _SRS method are really optional, it seems these checks should not happen.
> For now, using a link object without _PRS/_SRS doesn't work.
> 
> Note:
> The first check was initially done because an invalid interrupt was advertised in _CRS when valid interrupts were available in _PRS.
> https://bugzilla.kernel.org/show_bug.cgi?id=2665
> 
> 2. GSIV vs link object
> The fist motivation was to accurately describe the interrupts.
> Even though GIC interrupts must be active high, PCI interrupts are
> active low by default (according to spec), and the GSIV model
> doesn't allow to describe the polarity/activation state.

And any operating system that groks ACPI on arm64 already knows about
this quirks.

> Another point that came out is that in linux, GSIV interrupts for
> PCI are configured as level triggered by default.

That's part of the PCI spec. INTx is level triggered, no ifs, no
buts. Otherwise, you can't implement interrupt sharing, which legacy
PCI requires with INTx. So Linux has nothing to do with this.

> From "Base System Architecture 1.0", sE.4 and sE.6, PCI interrupts
> can be level or edge triggered.

You are confusing MSIs, which *MUST* be edge, and INTx which *MUST* be
level. These are two very different thing, and you really should not
conflate the two.

> More specifically, KvmTool configures PCI interrupts as edge
> triggered.

Well, that's a gross bug in kvmtool. I guess that it doesn't really
matter for virtio devices, but this should be fixed.

> So the only way to describe an edge interrupt is to use a link object.

MSIs should never be described in ACPI, as they are entirely SW
programmable (there is no static allocation). Only INTx must be
described, and that's strictly level.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v3 3/8] DynamicTablesPkg: AcpiSsdtPcieLibArm: Fix _PRT description

[Ard Biesheuvel]

On Mon, 31 Jan 2022 at 14:53, Marc Zyngier <maz@kernel.org> wrote:
>
> On Mon, 31 Jan 2022 12:59:11 +0000,
> Pierre Gondois <pierre.gondois@arm.com> wrote:
> >
> > Hi,
> >
> > On 1/29/22 7:20 PM, Marc Zyngier wrote:
> > > On Sat, 29 Jan 2022 15:52:02 +0000,
> > > Ard Biesheuvel <ardb@kernel.org> wrote:
> > >>
> > >> (+ Marc)
> > >>
> > >> On Fri, 28 Jan 2022 at 16:41, <Pierre.Gondois@arm.com> wrote:
> > >>>
> > >>> From: Pierre Gondois <Pierre.Gondois@arm.com>
> > >>>
> > >>> In ACPI 6.4, s6.2.13, _PRT objects describing PCI legacy interrupts
> > >>> can be defined following 2 models.
> > >>> In the first model, a _SRS object must be described to modify the PCI
> > >>> interrupt. The _CRS and _PRS object allows to describe the PCI
> > >>> interrupt (level/edge triggered, active high/low).
> > >>> In the second model, the PCI interrupt cannot be described with a
> > >>> similar granularity. PCI interrupts are by default level triggered,
> > >>> active low.
> > >>>
> > >>> GicV2 SPI interrupts are level or edge triggered, active high. To
> > >>> correctly describe PCI interrupts, the first model is used, even though
> > >>> Arm Base Boot Requirements v1.0 requires to use the second mode.
> > >>>
> > >>
> > >> There are two different issues here:
> > >>
> > >> - using separate 'interrupt link' device objects with an Interrupt()
> > >> resource rather than a simple GSIV number
> > >> - whether _PRS and _SRS need to be implemented on those link objects.
> > >>
> > >> The latter is simply not true - _PRS and _SRS are optional, and
> > >> pointless if there is only a single possible value, so there is really
> > >> no need to add them here.
> > >>
> > >> As for the choice between the link object or the GSIV number: I don't
> > >> think INTx interrupts on ARM systems are actually level low, and the
> > >> GSIV option is widely used, also in platforms that exist in
> > >> edk2-platforms, without any reported issues.
> > >>
> > >> I've cc'ed Marc, perhaps he can shed some light on this, but I'd
> > >> prefer to stick to the GSIV approach if we can, as it is much simpler.
> > >
> > > I don't immediately see the point either. Yes, the GIC only supports
> > > level-high interrupts. However, all the PCIe implementations connected
> > > to it are inverting the level. If they don't, that's even simpler (the
> > > HW is broken).
> > >
> > > Is this to address an apparent disconnect with the spec?
> > >
> > > [...]
> > >
> >
> > 1. _PRS/_SRS methods
> > I agree they are optional and meaningless here as interrupts are not dynamically configurable.
> > However linux is checking that:
> > - the interrupt used in _CRS is one of the possible interrupts advertised in _PRS. If not, then a warning is issued and the interrupt is not used.
> > - the _SRS method is present. If not, setting the interrupt fails.
> > If _PRS and _SRS method are really optional, it seems these checks should not happen.
> > For now, using a link object without _PRS/_SRS doesn't work.
> >
> > Note:
> > The first check was initially done because an invalid interrupt was advertised in _CRS when valid interrupts were available in _PRS.
> > https://bugzilla.kernel.org/show_bug.cgi?id=2665
> >
> > 2. GSIV vs link object
> > The fist motivation was to accurately describe the interrupts.
> > Even though GIC interrupts must be active high, PCI interrupts are
> > active low by default (according to spec), and the GSIV model
> > doesn't allow to describe the polarity/activation state.
>
> And any operating system that groks ACPI on arm64 already knows about
> this quirks.
>
> > Another point that came out is that in linux, GSIV interrupts for
> > PCI are configured as level triggered by default.
>
> That's part of the PCI spec. INTx is level triggered, no ifs, no
> buts. Otherwise, you can't implement interrupt sharing, which legacy
> PCI requires with INTx. So Linux has nothing to do with this.
>
> > From "Base System Architecture 1.0", sE.4 and sE.6, PCI interrupts
> > can be level or edge triggered.
>
> You are confusing MSIs, which *MUST* be edge, and INTx which *MUST* be
> level. These are two very different thing, and you really should not
> conflate the two.
>
> > More specifically, KvmTool configures PCI interrupts as edge
> > triggered.
>
> Well, that's a gross bug in kvmtool. I guess that it doesn't really
> matter for virtio devices, but this should be fixed.
>
> > So the only way to describe an edge interrupt is to use a link object.
>
> MSIs should never be described in ACPI, as they are entirely SW
> programmable (there is no static allocation). Only INTx must be
> described, and that's strictly level.
>

Thanks for clearing that up Marc.

So we can drop the link objects after all, which means we are not
forced to add 'optional' methods just to appease Linux. This is far
better IMHO.

In the mean time, I expect that Marc or myself will get kvmtool fixed,
but this doesn't actually matter with GSIVs anyway.

Thanks,
Ard.

[PATCH v3 4/8] ArmVirtPkg: Add cspell exceptions

[PierreGondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

The cpsell tool checks for unknown words in the upstream CI.
Add some new words to the list of exceptions.

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---

Notes:
    v2:
    - Adding 'acpiview'. Since the patch change, Laszlo's
      Reviewed-by is not added. [Pierre]

 ArmVirtPkg/ArmVirtPkg.ci.yaml | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirtPkg.ci.yaml b/ArmVirtPkg/ArmVirtPkg.ci.yaml
index 67b0e9594f3e..d5d63ddd4fd7 100644
--- a/ArmVirtPkg/ArmVirtPkg.ci.yaml
+++ b/ArmVirtPkg/ArmVirtPkg.ci.yaml
@@ -6,7 +6,7 @@
 #
 # Copyright (c) Microsoft Corporation
 # Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
-# Copyright (c) 2020, ARM Limited. All rights reserved.
+# Copyright (c) 2020 - 2022, ARM Limited. All rights reserved.<BR>
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
@@ -48,6 +48,7 @@
             "MdePkg/MdePkg.dec",
             "MdeModulePkg/MdeModulePkg.dec",
             "ArmVirtPkg/ArmVirtPkg.dec",
+            "DynamicTablesPkg/DynamicTablesPkg.dec",
             "NetworkPkg/NetworkPkg.dec",
             "ArmPkg/ArmPkg.dec",
             "OvmfPkg/OvmfPkg.dec",
@@ -98,6 +99,9 @@
         "AuditOnly": False,           # Fails right now with over 270 errors
         "IgnoreFiles": [],           # use gitignore syntax to ignore errors in matching files
         "ExtendWords": [
+            "acpiview",
+            "armltd",
+            "ssdts",
             "setjump",
             "plong",
             "lparam",
-- 
2.25.1

[PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table

[PierreGondois]

From: Sami Mujawar <sami.mujawar@arm.com>

Most ACPI tables for Kvmtool firmware are dynamically
generated. The AML code is also generated at runtime
for most components in appropriate SSDTs.

Although there may not be much to describe in the DSDT,
the DSDT table is mandatory.

Therefore, add an empty stub for DSDT.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v2:
     - Coding style modifications. [Laszlo]

 .../KvmtoolCfgMgrDxe/AslTables/Dsdt.asl       | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/AslTables/Dsdt.asl

diff --git a/ArmVirtPkg/KvmtoolCfgMgrDxe/AslTables/Dsdt.asl b/ArmVirtPkg/KvmtoolCfgMgrDxe/AslTables/Dsdt.asl
new file mode 100644
index 000000000000..5010beabfc4b
--- /dev/null
+++ b/ArmVirtPkg/KvmtoolCfgMgrDxe/AslTables/Dsdt.asl
@@ -0,0 +1,21 @@
+/** @file
+  Differentiated System Description Table Fields (DSDT)
+
+  Copyright (c) 2021 - 2022, ARM Ltd. All rights reserved.<BR>
+    SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "ARMLTD", "ARM-KVMT", 1) {
+  Scope (_SB) {
+    //
+    // Most ACPI tables for Kvmtool firmware are
+    // dynamically generated. The AML code is also
+    // generated at runtime for most components in
+    // appropriate SSDTs.
+    // Although there may not be much to describe
+    // in the DSDT, the DSDT table is mandatory.
+    // Therefore, add an empty stub for DSDT.
+    //
+  } // Scope (_SB)
+}
-- 
2.25.1

Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table

[Rebecca Cran]

On 1/28/22 08:41, PierreGondois wrote:

> +  Differentiated System Description Table Fields (DSDT)
> +
> +  Copyright (c) 2021 - 2022, ARM Ltd. All rights reserved.<BR>
> +    SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "ARMLTD", "ARM-KVMT", 1) {

The Revision field should probably be 2, not 1. ACPI 6.4 says:


2. This field also sets the global integer width for the AML
interpreter. Values less than two will cause the inter-
preter to use 32-bit integers and math. Values of two
and greater will cause the interpreter to use full 64-bit
integers and math.

-- 
Rebecca Cran

Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table

[Sami Mujawar]

[-- Attachment #1: Type: text/plain, Size: 1194 bytes --]

Hi Rebecca,

Thanks for catching this.
I think we also need to add a check in Acpiview to report this issue. However, that would be another patch series.

Regards,

Sami Mujawar

From: Rebecca Cran <quic_rcran@quicinc.com>
Date: Monday, 31 January 2022 at 15:17
To: devel@edk2.groups.io <devel@edk2.groups.io>, Pierre Gondois <Pierre.Gondois@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>, Sami Mujawar <Sami.Mujawar@arm.com>
Subject: Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table
On 1/28/22 08:41, PierreGondois wrote:

> +  Differentiated System Description Table Fields (DSDT)
> +
> +  Copyright (c) 2021 - 2022, ARM Ltd. All rights reserved.<BR>
> +    SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "ARMLTD", "ARM-KVMT", 1) {

The Revision field should probably be 2, not 1. ACPI 6.4 says:


2. This field also sets the global integer width for the AML
interpreter. Values less than two will cause the inter-
preter to use 32-bit integers and math. Values of two
and greater will cause the interpreter to use full 64-bit
integers and math.

--
Rebecca Cran

[-- Attachment #2: Type: text/html, Size: 4106 bytes --]

Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table

[PierreGondois]

Hi Rebecca,

On 1/31/22 4:21 PM, Sami Mujawar wrote:
> Hi Rebecca,
> 
> Thanks for catching this.
> 
> I think we also need to adda check in Acpiview to report this issue. However,
> that would be another patch series.
> 
> Regards,
> 
> Sami Mujawar
> 
> *From: *Rebecca Cran <quic_rcran@quicinc.com>
> *Date: *Monday, 31 January 2022 at 15:17
> *To: *devel@edk2.groups.io <devel@edk2.groups.io>, Pierre Gondois
> <Pierre.Gondois@arm.com>
> *Cc: *Ard Biesheuvel <ardb+tianocore@kernel.org>, Sami Mujawar
> <Sami.Mujawar@arm.com>
> *Subject: *Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table
> 
> On 1/28/22 08:41, PierreGondois wrote:
> 
>> +  Differentiated System Description Table Fields (DSDT)
>> +
>> +  Copyright (c) 2021 - 2022, ARM Ltd. All rights reserved.<BR>
>> +    SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "ARMLTD", "ARM-KVMT", 1) {
> 
> The Revision field should probably be 2, not 1. ACPI 6.4 says:
> 
> 
> 2. This field also sets the global integer width for the AML
> interpreter. Values less than two will cause the inter-
> preter to use 32-bit integers and math. Values of two
> and greater will cause the interpreter to use full 64-bit
> integers and math.
> 
> -- 
> Rebecca Cran
> 

Yes indeed. I forgot to add it in the v4... I will send a v5.
Regards,
Pierre

Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table

[Ard Biesheuvel]

On Tue, 1 Feb 2022 at 17:55, Pierre Gondois <pierre.gondois@arm.com> wrote:
>
> Hi Rebecca,
>
> On 1/31/22 4:21 PM, Sami Mujawar wrote:
> > Hi Rebecca,
> >
> > Thanks for catching this.
> >
> > I think we also need to adda check in Acpiview to report this issue. However,
> > that would be another patch series.
> >
> > Regards,
> >
> > Sami Mujawar
> >
> > *From: *Rebecca Cran <quic_rcran@quicinc.com>
> > *Date: *Monday, 31 January 2022 at 15:17
> > *To: *devel@edk2.groups.io <devel@edk2.groups.io>, Pierre Gondois
> > <Pierre.Gondois@arm.com>
> > *Cc: *Ard Biesheuvel <ardb+tianocore@kernel.org>, Sami Mujawar
> > <Sami.Mujawar@arm.com>
> > *Subject: *Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table
> >
> > On 1/28/22 08:41, PierreGondois wrote:
> >
> >> +  Differentiated System Description Table Fields (DSDT)
> >> +
> >> +  Copyright (c) 2021 - 2022, ARM Ltd. All rights reserved.<BR>
> >> +    SPDX-License-Identifier: BSD-2-Clause-Patent
> >> +
> >> +**/
> >> +
> >> +DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "ARMLTD", "ARM-KVMT", 1) {
> >
> > The Revision field should probably be 2, not 1. ACPI 6.4 says:
> >
> >
> > 2. This field also sets the global integer width for the AML
> > interpreter. Values less than two will cause the inter-
> > preter to use 32-bit integers and math. Values of two
> > and greater will cause the interpreter to use full 64-bit
> > integers and math.
> >
> > --
> > Rebecca Cran
> >
>
> Yes indeed. I forgot to add it in the v4... I will send a v5.

Please give me a minute before sending another version.

I am still seeing

Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table

[Ard Biesheuvel]

On Tue, 1 Feb 2022 at 17:56, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 1 Feb 2022 at 17:55, Pierre Gondois <pierre.gondois@arm.com> wrote:
> >
> > Hi Rebecca,
> >
> > On 1/31/22 4:21 PM, Sami Mujawar wrote:
> > > Hi Rebecca,
> > >
> > > Thanks for catching this.
> > >
> > > I think we also need to adda check in Acpiview to report this issue. However,
> > > that would be another patch series.
> > >
> > > Regards,
> > >
> > > Sami Mujawar
> > >
> > > *From: *Rebecca Cran <quic_rcran@quicinc.com>
> > > *Date: *Monday, 31 January 2022 at 15:17
> > > *To: *devel@edk2.groups.io <devel@edk2.groups.io>, Pierre Gondois
> > > <Pierre.Gondois@arm.com>
> > > *Cc: *Ard Biesheuvel <ardb+tianocore@kernel.org>, Sami Mujawar
> > > <Sami.Mujawar@arm.com>
> > > *Subject: *Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table
> > >
> > > On 1/28/22 08:41, PierreGondois wrote:
> > >
> > >> +  Differentiated System Description Table Fields (DSDT)
> > >> +
> > >> +  Copyright (c) 2021 - 2022, ARM Ltd. All rights reserved.<BR>
> > >> +    SPDX-License-Identifier: BSD-2-Clause-Patent
> > >> +
> > >> +**/
> > >> +
> > >> +DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "ARMLTD", "ARM-KVMT", 1) {
> > >
> > > The Revision field should probably be 2, not 1. ACPI 6.4 says:
> > >
> > >
> > > 2. This field also sets the global integer width for the AML
> > > interpreter. Values less than two will cause the inter-
> > > preter to use 32-bit integers and math. Values of two
> > > and greater will cause the interpreter to use full 64-bit
> > > integers and math.
> > >
> > > --
> > > Rebecca Cran
> > >
> >
> > Yes indeed. I forgot to add it in the v4... I will send a v5.
>
> Please give me a minute before sending another version.
>
> I am still seeing

Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table

[Ard Biesheuvel]

On Tue, 1 Feb 2022 at 17:56, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 1 Feb 2022 at 17:55, Pierre Gondois <pierre.gondois@arm.com> wrote:
> >
> > Hi Rebecca,
> >
> > On 1/31/22 4:21 PM, Sami Mujawar wrote:
> > > Hi Rebecca,
> > >
> > > Thanks for catching this.
> > >
> > > I think we also need to adda check in Acpiview to report this issue. However,
> > > that would be another patch series.
> > >
> > > Regards,
> > >
> > > Sami Mujawar
> > >
> > > *From: *Rebecca Cran <quic_rcran@quicinc.com>
> > > *Date: *Monday, 31 January 2022 at 15:17
> > > *To: *devel@edk2.groups.io <devel@edk2.groups.io>, Pierre Gondois
> > > <Pierre.Gondois@arm.com>
> > > *Cc: *Ard Biesheuvel <ardb+tianocore@kernel.org>, Sami Mujawar
> > > <Sami.Mujawar@arm.com>
> > > *Subject: *Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table
> > >
> > > On 1/28/22 08:41, PierreGondois wrote:
> > >
> > >> +  Differentiated System Description Table Fields (DSDT)
> > >> +
> > >> +  Copyright (c) 2021 - 2022, ARM Ltd. All rights reserved.<BR>
> > >> +    SPDX-License-Identifier: BSD-2-Clause-Patent
> > >> +
> > >> +**/
> > >> +
> > >> +DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "ARMLTD", "ARM-KVMT", 1) {
> > >
> > > The Revision field should probably be 2, not 1. ACPI 6.4 says:
> > >
> > >
> > > 2. This field also sets the global integer width for the AML
> > > interpreter. Values less than two will cause the inter-
> > > preter to use 32-bit integers and math. Values of two
> > > and greater will cause the interpreter to use full 64-bit
> > > integers and math.
> > >
> > > --
> > > Rebecca Cran
> > >
> >
> > Yes indeed. I forgot to add it in the v4... I will send a v5.
>
> Please give me a minute before sending another version.
>
> I am still seeing

No ACPI PMU IRQ for CPU26

errors and I am trying to figure out why.

Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table

[PierreGondois]

On 2/1/22 5:56 PM, Ard Biesheuvel wrote:
> On Tue, 1 Feb 2022 at 17:56, Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> On Tue, 1 Feb 2022 at 17:55, Pierre Gondois <pierre.gondois@arm.com> wrote:
>>>
>>> Hi Rebecca,
>>>
>>> On 1/31/22 4:21 PM, Sami Mujawar wrote:
>>>> Hi Rebecca,
>>>>
>>>> Thanks for catching this.
>>>>
>>>> I think we also need to adda check in Acpiview to report this issue. However,
>>>> that would be another patch series.
>>>>
>>>> Regards,
>>>>
>>>> Sami Mujawar
>>>>
>>>> *From: *Rebecca Cran <quic_rcran@quicinc.com>
>>>> *Date: *Monday, 31 January 2022 at 15:17
>>>> *To: *devel@edk2.groups.io <devel@edk2.groups.io>, Pierre Gondois
>>>> <Pierre.Gondois@arm.com>
>>>> *Cc: *Ard Biesheuvel <ardb+tianocore@kernel.org>, Sami Mujawar
>>>> <Sami.Mujawar@arm.com>
>>>> *Subject: *Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table
>>>>
>>>> On 1/28/22 08:41, PierreGondois wrote:
>>>>
>>>>> +  Differentiated System Description Table Fields (DSDT)
>>>>> +
>>>>> +  Copyright (c) 2021 - 2022, ARM Ltd. All rights reserved.<BR>
>>>>> +    SPDX-License-Identifier: BSD-2-Clause-Patent
>>>>> +
>>>>> +**/
>>>>> +
>>>>> +DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "ARMLTD", "ARM-KVMT", 1) {
>>>>
>>>> The Revision field should probably be 2, not 1. ACPI 6.4 says:
>>>>
>>>>
>>>> 2. This field also sets the global integer width for the AML
>>>> interpreter. Values less than two will cause the inter-
>>>> preter to use 32-bit integers and math. Values of two
>>>> and greater will cause the interpreter to use full 64-bit
>>>> integers and math.
>>>>
>>>> --
>>>> Rebecca Cran
>>>>
>>>
>>> Yes indeed. I forgot to add it in the v4... I will send a v5.
>>
>> Please give me a minute before sending another version.
>>
>> I am still seeing
> 
> No ACPI PMU IRQ for CPU26
> 
> errors and I am trying to figure out why.
> 

There is a --pmu option in kvmtool, we are not populating pmu
interrupts if kvmtool doesn't receive this option. Maybe this is it ?

Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table

[Ard Biesheuvel]

On Tue, 1 Feb 2022 at 18:02, Pierre Gondois <pierre.gondois@arm.com> wrote:
>
>
>
> On 2/1/22 5:56 PM, Ard Biesheuvel wrote:
> > On Tue, 1 Feb 2022 at 17:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> >>
> >> On Tue, 1 Feb 2022 at 17:55, Pierre Gondois <pierre.gondois@arm.com> wrote:
> >>>
> >>> Hi Rebecca,
> >>>
> >>> On 1/31/22 4:21 PM, Sami Mujawar wrote:
> >>>> Hi Rebecca,
> >>>>
> >>>> Thanks for catching this.
> >>>>
> >>>> I think we also need to adda check in Acpiview to report this issue. However,
> >>>> that would be another patch series.
> >>>>
> >>>> Regards,
> >>>>
> >>>> Sami Mujawar
> >>>>
> >>>> *From: *Rebecca Cran <quic_rcran@quicinc.com>
> >>>> *Date: *Monday, 31 January 2022 at 15:17
> >>>> *To: *devel@edk2.groups.io <devel@edk2.groups.io>, Pierre Gondois
> >>>> <Pierre.Gondois@arm.com>
> >>>> *Cc: *Ard Biesheuvel <ardb+tianocore@kernel.org>, Sami Mujawar
> >>>> <Sami.Mujawar@arm.com>
> >>>> *Subject: *Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table
> >>>>
> >>>> On 1/28/22 08:41, PierreGondois wrote:
> >>>>
> >>>>> +  Differentiated System Description Table Fields (DSDT)
> >>>>> +
> >>>>> +  Copyright (c) 2021 - 2022, ARM Ltd. All rights reserved.<BR>
> >>>>> +    SPDX-License-Identifier: BSD-2-Clause-Patent
> >>>>> +
> >>>>> +**/
> >>>>> +
> >>>>> +DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "ARMLTD", "ARM-KVMT", 1) {
> >>>>
> >>>> The Revision field should probably be 2, not 1. ACPI 6.4 says:
> >>>>
> >>>>
> >>>> 2. This field also sets the global integer width for the AML
> >>>> interpreter. Values less than two will cause the inter-
> >>>> preter to use 32-bit integers and math. Values of two
> >>>> and greater will cause the interpreter to use full 64-bit
> >>>> integers and math.
> >>>>
> >>>> --
> >>>> Rebecca Cran
> >>>>
> >>>
> >>> Yes indeed. I forgot to add it in the v4... I will send a v5.
> >>
> >> Please give me a minute before sending another version.
> >>
> >> I am still seeing
> >
> > No ACPI PMU IRQ for CPU26
> >
> > errors and I am trying to figure out why.
> >
>
> There is a --pmu option in kvmtool, we are not populating pmu
> interrupts if kvmtool doesn't receive this option. Maybe this is it ?

Yes you are right.

So this is all looking fine now - I tested booting Linux with
pci=nomsi, and the legacy interrupts are level and working as
expected.

Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table

[PierreGondois]

On 2/1/22 6:04 PM, Ard Biesheuvel wrote:
> On Tue, 1 Feb 2022 at 18:02, Pierre Gondois <pierre.gondois@arm.com> wrote:
>>
>>
>>
>> On 2/1/22 5:56 PM, Ard Biesheuvel wrote:
>>> On Tue, 1 Feb 2022 at 17:56, Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>
>>>> On Tue, 1 Feb 2022 at 17:55, Pierre Gondois <pierre.gondois@arm.com> wrote:
>>>>>
>>>>> Hi Rebecca,
>>>>>
>>>>> On 1/31/22 4:21 PM, Sami Mujawar wrote:
>>>>>> Hi Rebecca,
>>>>>>
>>>>>> Thanks for catching this.
>>>>>>
>>>>>> I think we also need to adda check in Acpiview to report this issue. However,
>>>>>> that would be another patch series.
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> Sami Mujawar
>>>>>>
>>>>>> *From: *Rebecca Cran <quic_rcran@quicinc.com>
>>>>>> *Date: *Monday, 31 January 2022 at 15:17
>>>>>> *To: *devel@edk2.groups.io <devel@edk2.groups.io>, Pierre Gondois
>>>>>> <Pierre.Gondois@arm.com>
>>>>>> *Cc: *Ard Biesheuvel <ardb+tianocore@kernel.org>, Sami Mujawar
>>>>>> <Sami.Mujawar@arm.com>
>>>>>> *Subject: *Re: [edk2-devel] [PATCH v3 5/8] ArmVirtPkg/Kvmtool: Add DSDT ACPI table
>>>>>>
>>>>>> On 1/28/22 08:41, PierreGondois wrote:
>>>>>>
>>>>>>> +  Differentiated System Description Table Fields (DSDT)
>>>>>>> +
>>>>>>> +  Copyright (c) 2021 - 2022, ARM Ltd. All rights reserved.<BR>
>>>>>>> +    SPDX-License-Identifier: BSD-2-Clause-Patent
>>>>>>> +
>>>>>>> +**/
>>>>>>> +
>>>>>>> +DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "ARMLTD", "ARM-KVMT", 1) {
>>>>>>
>>>>>> The Revision field should probably be 2, not 1. ACPI 6.4 says:
>>>>>>
>>>>>>
>>>>>> 2. This field also sets the global integer width for the AML
>>>>>> interpreter. Values less than two will cause the inter-
>>>>>> preter to use 32-bit integers and math. Values of two
>>>>>> and greater will cause the interpreter to use full 64-bit
>>>>>> integers and math.
>>>>>>
>>>>>> --
>>>>>> Rebecca Cran
>>>>>>
>>>>>
>>>>> Yes indeed. I forgot to add it in the v4... I will send a v5.
>>>>
>>>> Please give me a minute before sending another version.
>>>>
>>>> I am still seeing
>>>
>>> No ACPI PMU IRQ for CPU26
>>>
>>> errors and I am trying to figure out why.
>>>
>>
>> There is a --pmu option in kvmtool, we are not populating pmu
>> interrupts if kvmtool doesn't receive this option. Maybe this is it ?
> 
> Yes you are right.
> 
> So this is all looking fine now - I tested booting Linux with
> pci=nomsi, and the legacy interrupts are level and working as
> expected.
> 

Ok nice. Thanks for testing. I will send the v5 shortly.

[PATCH v3 6/8] ArmVirtPkg/Kvmtool: Add Configuration Manager

[PierreGondois]

From: Sami Mujawar <sami.mujawar@arm.com>

Add Configuration Manager to enable ACPI tables for Kvmtool
firmware. The Configuration Manager for Kvmtool uses the DT
Hardware Information Parser module (FdtHwInfoParser) to parse
the DT provided by Kvmtool. The FdtHwInfoParser parses the DT
and invokes the callback function HW_INFO_ADD_OBJECT to add
the Configuration Manager objects to the Platform Information
repository.

The information for some Configuration Manager objects may not
be available in the DT. Such objects are initialised locally
by the Configuration Manager.

Support for the following ACPI tables is provided:
 - DBG2
 - DSDT (Empty stub)
 - FADT
 - GTDT
 - MADT
 - SPCR
 - SSDT (Cpu Hierarchy)
 - SSDT (Pcie bus)

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v2:
    - Only keep AARCH64 as a valid architecture. [Laszlo]
    - Move modifications to ArmVirtKvmTool.dsc to the next
      patch. [Laszlo]
    - Various coding style/error handling/ASSERT corrections.
      [Laszlo]
    - Remove dependency to PcdForceNoAcpi and rely on DEPEX
      to load the module. [Laszlo]
    - Not possible to make 'struct PlatformRepositoryInfo'
      an unamed struct since this is a redifinition.
      [Laszlo/Pierre]
    - Define 'FMT_CM_OBJECT_ID' print formatter in an earlier
      patch and use it in this patch. [Laszlo/Pierre]
    
    v3:
    - Add address size limit for IORT PCI root complex. [Pierre]

 .../KvmtoolCfgMgrDxe/ConfigurationManager.c   | 1065 +++++++++++++++++
 .../KvmtoolCfgMgrDxe/ConfigurationManager.h   |  125 ++
 .../ConfigurationManagerDxe.inf               |   54 +
 3 files changed, 1244 insertions(+)
 create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.c
 create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.h
 create mode 100644 ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf

diff --git a/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.c b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.c
new file mode 100644
index 000000000000..e8c1b13c8f2c
--- /dev/null
+++ b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.c
@@ -0,0 +1,1065 @@
+/** @file
+  Configuration Manager Dxe
+
+  Copyright (c) 2021 - 2022, Arm Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Glossary:
+    - Cm or CM   - Configuration Manager
+    - Obj or OBJ - Object
+**/
+
+#include <IndustryStandard/DebugPort2Table.h>
+#include <IndustryStandard/IoRemappingTable.h>
+#include <IndustryStandard/MemoryMappedConfigurationSpaceAccessTable.h>
+#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DynamicPlatRepoLib.h>
+#include <Library/HobLib.h>
+#include <Library/HwInfoParserLib.h>
+#include <Library/IoLib.h>
+#include <Library/PcdLib.h>
+#include <Library/TableHelperLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/AcpiTable.h>
+#include <Protocol/ConfigurationManagerProtocol.h>
+
+#include "ConfigurationManager.h"
+
+//
+// The platform configuration repository information.
+//
+STATIC
+EDKII_PLATFORM_REPOSITORY_INFO  mKvmtoolPlatRepositoryInfo = {
+  //
+  // Configuration Manager information
+  //
+  { CONFIGURATION_MANAGER_REVISION, CFG_MGR_OEM_ID },
+
+  //
+  // ACPI Table List
+  //
+  {
+    //
+    // FADT Table
+    //
+    {
+      EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+      EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_REVISION,
+      CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt),
+      NULL
+    },
+    //
+    // GTDT Table
+    //
+    {
+      EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
+      EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE_REVISION,
+      CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdGtdt),
+      NULL
+    },
+    //
+    // MADT Table
+    //
+    {
+      EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
+      EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
+      CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMadt),
+      NULL
+    },
+    //
+    // SPCR Table
+    //
+    {
+      EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
+      EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION,
+      CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdSpcr),
+      NULL
+    },
+    //
+    // DSDT Table
+    //
+    {
+      EFI_ACPI_6_3_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
+      0, // Unused
+      CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdDsdt),
+      (EFI_ACPI_DESCRIPTION_HEADER *)dsdt_aml_code
+    },
+    //
+    // SSDT Cpu Hierarchy Table
+    //
+    {
+      EFI_ACPI_6_3_SECONDARY_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
+      0, // Unused
+      CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdSsdtCpuTopology),
+      NULL
+    },
+    //
+    // DBG2 Table
+    //
+    {
+      EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
+      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION,
+      CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdDbg2),
+      NULL
+    },
+    //
+    // PCI MCFG Table
+    //
+    {
+      EFI_ACPI_6_3_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE,
+      EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_SPACE_ACCESS_TABLE_REVISION,
+      CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMcfg),
+      NULL
+    },
+    //
+    // SSDT table describing the PCI root complex
+    //
+    {
+      EFI_ACPI_6_3_SECONDARY_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
+      0, // Unused
+      CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdSsdtPciExpress),
+      NULL
+    },
+    //
+    // IORT Table
+    //
+    {
+      EFI_ACPI_6_3_IO_REMAPPING_TABLE_SIGNATURE,
+      EFI_ACPI_IO_REMAPPING_TABLE_REVISION,
+      CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdIort),
+      NULL
+    },
+  },
+
+  //
+  // Power management profile information
+  //
+  { EFI_ACPI_6_3_PM_PROFILE_ENTERPRISE_SERVER },    // PowerManagement Profile
+
+  //
+  // ITS group node
+  //
+  {
+    //
+    // Reference token for this Iort node
+    //
+    REFERENCE_TOKEN (ItsGroupInfo),
+    //
+    // The number of ITS identifiers in the ITS node.
+    //
+    1,
+    //
+    // Reference token for the ITS identifier array
+    //
+    REFERENCE_TOKEN (ItsIdentifierArray)
+  },
+
+  //
+  // ITS identifier array
+  //
+  {
+    { 0 },                            // The ITS Identifier
+  },
+
+  //
+  // Root Complex node info
+  //
+  {
+    //
+    // Reference token for this Iort node
+    //
+    REFERENCE_TOKEN (RootComplexInfo),
+    //
+    // Number of ID mappings
+    //
+    1,
+    //
+    // Reference token for the ID mapping array
+    //
+    REFERENCE_TOKEN (DeviceIdMapping[0]),
+    //
+    // Memory access properties : Cache coherent attributes
+    //
+    EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA,
+    //
+    // Memory access properties : Allocation hints
+    //
+    0,
+    //
+    // Memory access properties : Memory access flags
+    //
+    0,
+    //
+    // ATS attributes
+    //
+    EFI_ACPI_IORT_ROOT_COMPLEX_ATS_UNSUPPORTED,
+    //
+    // PCI segment number
+    //
+    0,
+    ///
+    /// Memory address size limit
+    ///
+    MEMORY_ADDRESS_SIZE_LIMIT
+  },
+
+  //
+  // Array of Device ID mappings
+  //
+  {
+    //
+    // Device ID mapping for Root complex node
+    // RootComplex -> ITS Group
+    //
+    {
+      //
+      // Input base
+      //
+      0x0,
+      //
+      // Number of input IDs
+      //
+      0x0000FFFF,
+      //
+      // Output Base
+      //
+      0x0,
+      //
+      // Output reference
+      //
+      REFERENCE_TOKEN (ItsGroupInfo),
+      //
+      // Flags
+      //
+      0
+    },
+  },
+};
+
+/**
+  A helper function for returning the Configuration Manager Objects.
+
+  @param [in]       CmObjectId     The Configuration Manager Object ID.
+  @param [in]       Object         Pointer to the Object(s).
+  @param [in]       ObjectSize     Total size of the Object(s).
+  @param [in]       ObjectCount    Number of Objects.
+  @param [in, out]  CmObjectDesc   Pointer to the Configuration Manager Object
+                                   descriptor describing the requested Object.
+
+  @retval EFI_SUCCESS           Success.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+HandleCmObject (
+  IN  CONST CM_OBJECT_ID                CmObjectId,
+  IN        VOID                        *Object,
+  IN  CONST UINTN                       ObjectSize,
+  IN  CONST UINTN                       ObjectCount,
+  IN  OUT   CM_OBJ_DESCRIPTOR   *CONST  CmObjectDesc
+  )
+{
+  CmObjectDesc->ObjectId = CmObjectId;
+  CmObjectDesc->Size     = ObjectSize;
+  CmObjectDesc->Data     = Object;
+  CmObjectDesc->Count    = ObjectCount;
+  DEBUG ((
+    DEBUG_INFO,
+    "INFO: CmObjectId = " FMT_CM_OBJECT_ID ", "
+                                           "Ptr = 0x%p, Size = %lu, Count = %lu\n",
+    CmObjectId,
+    CmObjectDesc->Data,
+    CmObjectDesc->Size,
+    CmObjectDesc->Count
+    ));
+  return EFI_SUCCESS;
+}
+
+/**
+  A helper function for returning the Configuration Manager Objects that
+  match the token.
+
+  @param [in]  This               Pointer to the Configuration Manager Protocol.
+  @param [in]  CmObjectId         The Configuration Manager Object ID.
+  @param [in]  Object             Pointer to the Object(s).
+  @param [in]  ObjectSize         Total size of the Object(s).
+  @param [in]  ObjectCount        Number of Objects.
+  @param [in]  Token              A token identifying the object.
+  @param [in]  HandlerProc        A handler function to search the object
+                                  referenced by the token.
+  @param [in, out]  CmObjectDesc  Pointer to the Configuration Manager Object
+                                  descriptor describing the requested Object.
+
+  @retval EFI_SUCCESS           Success.
+  @retval EFI_INVALID_PARAMETER A parameter is invalid.
+  @retval EFI_NOT_FOUND         The required object information is not found.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+HandleCmObjectRefByToken (
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  This,
+  IN  CONST CM_OBJECT_ID                                  CmObjectId,
+  IN        VOID                                          *Object,
+  IN  CONST UINTN                                         ObjectSize,
+  IN  CONST UINTN                                         ObjectCount,
+  IN  CONST CM_OBJECT_TOKEN                               Token,
+  IN  CONST CM_OBJECT_HANDLER_PROC                        HandlerProc,
+  IN  OUT   CM_OBJ_DESCRIPTOR                     *CONST  CmObjectDesc
+  )
+{
+  EFI_STATUS  Status;
+
+  CmObjectDesc->ObjectId = CmObjectId;
+  if (Token == CM_NULL_TOKEN) {
+    CmObjectDesc->Size  = ObjectSize;
+    CmObjectDesc->Data  = Object;
+    CmObjectDesc->Count = ObjectCount;
+    Status              = EFI_SUCCESS;
+  } else {
+    Status = HandlerProc (This, CmObjectId, Token, CmObjectDesc);
+  }
+
+  DEBUG ((
+    DEBUG_INFO,
+    "INFO: Token = 0x%p, CmObjectId = " FMT_CM_OBJECT_ID ", "
+                                                         "Ptr = 0x%p, Size = %lu, Count = %lu\n",
+    (VOID *)Token,
+    CmObjectId,
+    CmObjectDesc->Data,
+    CmObjectDesc->Size,
+    CmObjectDesc->Count
+    ));
+  return Status;
+}
+
+/**
+  Return an ITS identifier array.
+
+  @param [in]  This        Pointer to the Configuration Manager Protocol.
+  @param [in]  CmObjectId  The Configuration Manager Object ID.
+  @param [in]  Token       A token for identifying the object
+  @param [out] CmObject    Pointer to the Configuration Manager Object
+                           descriptor describing the requested Object.
+
+  @retval EFI_SUCCESS           Success.
+  @retval EFI_INVALID_PARAMETER A parameter is invalid.
+  @retval EFI_NOT_FOUND         The required object information is not found.
+**/
+EFI_STATUS
+EFIAPI
+GetItsIdentifierArray (
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  This,
+  IN  CONST CM_OBJECT_ID                                  CmObjectId,
+  IN  CONST CM_OBJECT_TOKEN                               Token,
+  OUT       CM_OBJ_DESCRIPTOR                     *CONST  CmObject
+  )
+{
+  EDKII_PLATFORM_REPOSITORY_INFO  *PlatformRepo;
+
+  if ((This == NULL) || (CmObject == NULL)) {
+    ASSERT (This != NULL);
+    ASSERT (CmObject != NULL);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  PlatformRepo = This->PlatRepoInfo;
+
+  if (Token != (CM_OBJECT_TOKEN)&PlatformRepo->ItsIdentifierArray) {
+    return EFI_NOT_FOUND;
+  }
+
+  CmObject->ObjectId = CmObjectId;
+  CmObject->Size     = sizeof (PlatformRepo->ItsIdentifierArray);
+  CmObject->Data     = (VOID *)&PlatformRepo->ItsIdentifierArray;
+  CmObject->Count    = ARRAY_SIZE (PlatformRepo->ItsIdentifierArray);
+  return EFI_SUCCESS;
+}
+
+/**
+  Return a device Id mapping array.
+
+  @param [in]  This        Pointer to the Configuration Manager Protocol.
+  @param [in]  CmObjectId  The Configuration Manager Object ID.
+  @param [in]  Token       A token for identifying the object
+  @param [out] CmObject    Pointer to the Configuration Manager Object
+                           descriptor describing the requested Object.
+
+  @retval EFI_SUCCESS           Success.
+  @retval EFI_INVALID_PARAMETER A parameter is invalid.
+  @retval EFI_NOT_FOUND         The required object information is not found.
+**/
+EFI_STATUS
+EFIAPI
+GetDeviceIdMappingArray (
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  This,
+  IN  CONST CM_OBJECT_ID                                  CmObjectId,
+  IN  CONST CM_OBJECT_TOKEN                               Token,
+  OUT       CM_OBJ_DESCRIPTOR                     *CONST  CmObject
+  )
+{
+  EDKII_PLATFORM_REPOSITORY_INFO  *PlatformRepo;
+
+  if ((This == NULL) || (CmObject == NULL)) {
+    ASSERT (This != NULL);
+    ASSERT (CmObject != NULL);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  PlatformRepo = This->PlatRepoInfo;
+
+  if (Token != (CM_OBJECT_TOKEN)&PlatformRepo->DeviceIdMapping[0]) {
+    return EFI_NOT_FOUND;
+  }
+
+  CmObject->ObjectId = CmObjectId;
+  CmObject->Size     = sizeof (CM_ARM_ID_MAPPING);
+  CmObject->Data     = (VOID *)Token;
+  CmObject->Count    = 1;
+  return EFI_SUCCESS;
+}
+
+/**
+  Function pointer called by the parser to add information.
+
+  Callback function that the parser can use to add new
+  CmObj. This function must copy the CmObj data and not rely on
+  the parser preserving the CmObj memory.
+  This function is responsible of the Token allocation.
+
+  @param  [in]  ParserHandle  A handle to the parser instance.
+  @param  [in]  Context       A pointer to the caller's context provided in
+                              HwInfoParserInit ().
+  @param  [in]  CmObjDesc     CM_OBJ_DESCRIPTOR containing the CmObj(s) to add.
+  @param  [out] Token         If provided and success, contain the token
+                              generated for the CmObj.
+
+  @retval EFI_SUCCESS             The function completed successfully.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+HwInfoAdd (
+  IN        HW_INFO_PARSER_HANDLE  ParserHandle,
+  IN        VOID                   *Context,
+  IN  CONST CM_OBJ_DESCRIPTOR      *CmObjDesc,
+  OUT       CM_OBJECT_TOKEN        *Token OPTIONAL
+  )
+{
+  EFI_STATUS                      Status;
+  EDKII_PLATFORM_REPOSITORY_INFO  *PlatformRepo;
+
+  if ((ParserHandle == NULL)  ||
+      (Context == NULL)       ||
+      (CmObjDesc == NULL))
+  {
+    ASSERT (ParserHandle != NULL);
+    ASSERT (Context != NULL);
+    ASSERT (CmObjDesc != NULL);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  PlatformRepo = (EDKII_PLATFORM_REPOSITORY_INFO *)Context;
+
+  DEBUG_CODE_BEGIN ();
+  //
+  // Print the received objects.
+  //
+  ParseCmObjDesc (CmObjDesc);
+  DEBUG_CODE_END ();
+
+  Status = DynPlatRepoAddObject (
+             PlatformRepo->DynamicPlatformRepo,
+             CmObjDesc,
+             Token
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+  }
+
+  return Status;
+}
+
+/**
+  Cleanup the platform configuration repository.
+
+  @param [in]  This        Pointer to the Configuration Manager Protocol.
+
+  @retval EFI_SUCCESS             Success
+  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+CleanupPlatformRepository (
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  This
+  )
+{
+  EFI_STATUS                      Status;
+  EDKII_PLATFORM_REPOSITORY_INFO  *PlatformRepo;
+
+  if (This == NULL) {
+    ASSERT (This != NULL);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  PlatformRepo = This->PlatRepoInfo;
+
+  //
+  // Shutdown the dynamic repo and free all objects.
+  //
+  Status = DynamicPlatRepoShutdown (PlatformRepo->DynamicPlatformRepo);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  //
+  // Shutdown parser.
+  //
+  Status = HwInfoParserShutdown (PlatformRepo->FdtParserHandle);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+  }
+
+  return Status;
+}
+
+/**
+  Initialize the platform configuration repository.
+
+  @param [in]  This        Pointer to the Configuration Manager Protocol.
+
+  @retval EFI_SUCCESS             Success
+  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
+  @retval EFI_OUT_OF_RESOURCES    An allocation has failed.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+InitializePlatformRepository (
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  This
+  )
+{
+  EFI_STATUS                      Status;
+  EDKII_PLATFORM_REPOSITORY_INFO  *PlatformRepo;
+  VOID                            *Hob;
+
+  if (This == NULL) {
+    ASSERT (This != NULL);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Hob = GetFirstGuidHob (&gFdtHobGuid);
+  if ((Hob == NULL) || (GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64))) {
+    ASSERT (FALSE);
+    ASSERT (GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64));
+    return EFI_NOT_FOUND;
+  }
+
+  PlatformRepo          = This->PlatRepoInfo;
+  PlatformRepo->FdtBase = (VOID *)*(UINTN *)GET_GUID_HOB_DATA (Hob);
+
+  //
+  // Initialise the dynamic platform repository.
+  //
+  Status = DynamicPlatRepoInit (&PlatformRepo->DynamicPlatformRepo);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  //
+  // Initialise the FDT parser
+  //
+  Status = HwInfoParserInit (
+             PlatformRepo->FdtBase,
+             PlatformRepo,
+             HwInfoAdd,
+             &PlatformRepo->FdtParserHandle
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    goto ErrorHandler;
+  }
+
+  Status = HwInfoParse (PlatformRepo->FdtParserHandle);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    goto ErrorHandler;
+  }
+
+  Status = DynamicPlatRepoFinalise (PlatformRepo->DynamicPlatformRepo);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    goto ErrorHandler;
+  }
+
+  return EFI_SUCCESS;
+
+ErrorHandler:
+  CleanupPlatformRepository (This);
+  return Status;
+}
+
+/**
+  Return a standard namespace object.
+
+  @param [in]      This        Pointer to the Configuration Manager Protocol.
+  @param [in]      CmObjectId  The Configuration Manager Object ID.
+  @param [in]      Token       An optional token identifying the object. If
+                               unused this must be CM_NULL_TOKEN.
+  @param [in, out] CmObject    Pointer to the Configuration Manager Object
+                               descriptor describing the requested Object.
+
+  @retval EFI_SUCCESS           Success.
+  @retval EFI_INVALID_PARAMETER A parameter is invalid.
+  @retval EFI_NOT_FOUND         The required object information is not found.
+**/
+EFI_STATUS
+EFIAPI
+GetStandardNameSpaceObject (
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  This,
+  IN  CONST CM_OBJECT_ID                                  CmObjectId,
+  IN  CONST CM_OBJECT_TOKEN                               Token OPTIONAL,
+  IN  OUT   CM_OBJ_DESCRIPTOR                     *CONST  CmObject
+  )
+{
+  EFI_STATUS                      Status;
+  EDKII_PLATFORM_REPOSITORY_INFO  *PlatformRepo;
+  UINTN                           AcpiTableCount;
+  CM_OBJ_DESCRIPTOR               CmObjDesc;
+
+  if ((This == NULL) || (CmObject == NULL)) {
+    ASSERT (This != NULL);
+    ASSERT (CmObject != NULL);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Status       = EFI_NOT_FOUND;
+  PlatformRepo = This->PlatRepoInfo;
+
+  switch (GET_CM_OBJECT_ID (CmObjectId)) {
+    case EStdObjCfgMgrInfo:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 &PlatformRepo->CmInfo,
+                 sizeof (PlatformRepo->CmInfo),
+                 1,
+                 CmObject
+                 );
+      break;
+
+    case EStdObjAcpiTableList:
+      AcpiTableCount = ARRAY_SIZE (PlatformRepo->CmAcpiTableList);
+
+      //
+      // Get Pci config space information.
+      //
+      Status = DynamicPlatRepoGetObject (
+                 PlatformRepo->DynamicPlatformRepo,
+                 CREATE_CM_ARM_OBJECT_ID (EArmObjPciConfigSpaceInfo),
+                 CM_NULL_TOKEN,
+                 &CmObjDesc
+                 );
+      if (Status == EFI_NOT_FOUND) {
+        //
+        // The last 3 tables are for PCIe. If PCIe information is not
+        // present, Kvmtool was launched without the PCIe option.
+        // Therefore, reduce the table count by 3.
+        //
+        AcpiTableCount -= 3;
+      } else if (EFI_ERROR (Status)) {
+        ASSERT_EFI_ERROR (Status);
+        return Status;
+      }
+
+      //
+      // Get the Gic version.
+      //
+      Status = DynamicPlatRepoGetObject (
+                 PlatformRepo->DynamicPlatformRepo,
+                 CREATE_CM_ARM_OBJECT_ID (EArmObjGicDInfo),
+                 CM_NULL_TOKEN,
+                 &CmObjDesc
+                 );
+      if (EFI_ERROR (Status)) {
+        ASSERT_EFI_ERROR (Status);
+        return Status;
+      }
+
+      if (((CM_ARM_GICD_INFO *)CmObjDesc.Data)->GicVersion < 3) {
+        //
+        // IORT is only required for GicV3/4
+        //
+        AcpiTableCount -= 1;
+      }
+
+      Status = HandleCmObject (
+                 CmObjectId,
+                 PlatformRepo->CmAcpiTableList,
+                 (sizeof (PlatformRepo->CmAcpiTableList[0]) * AcpiTableCount),
+                 AcpiTableCount,
+                 CmObject
+                 );
+      break;
+
+    default:
+      Status = EFI_NOT_FOUND;
+      DEBUG ((
+        DEBUG_ERROR,
+        "ERROR: CmObjectId " FMT_CM_OBJECT_ID ". Status = %r\n",
+        CmObjectId,
+        Status
+        ));
+      break;
+  }
+
+  return Status;
+}
+
+/**
+  Return an ARM namespace object.
+
+  @param [in]      This        Pointer to the Configuration Manager Protocol.
+  @param [in]      CmObjectId  The Configuration Manager Object ID.
+  @param [in]      Token       An optional token identifying the object. If
+                               unused this must be CM_NULL_TOKEN.
+  @param [in, out] CmObject    Pointer to the Configuration Manager Object
+                               descriptor describing the requested Object.
+
+  @retval EFI_SUCCESS           Success.
+  @retval EFI_INVALID_PARAMETER A parameter is invalid.
+  @retval EFI_NOT_FOUND         The required object information is not found.
+**/
+EFI_STATUS
+EFIAPI
+GetArmNameSpaceObject (
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  This,
+  IN  CONST CM_OBJECT_ID                                  CmObjectId,
+  IN  CONST CM_OBJECT_TOKEN                               Token OPTIONAL,
+  IN  OUT   CM_OBJ_DESCRIPTOR                     *CONST  CmObject
+  )
+{
+  EFI_STATUS                      Status;
+  EDKII_PLATFORM_REPOSITORY_INFO  *PlatformRepo;
+
+  if ((This == NULL) || (CmObject == NULL)) {
+    ASSERT (This != NULL);
+    ASSERT (CmObject != NULL);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Status       = EFI_NOT_FOUND;
+  PlatformRepo = This->PlatRepoInfo;
+
+  //
+  // First check among the static objects.
+  //
+  switch (GET_CM_OBJECT_ID (CmObjectId)) {
+    case EArmObjPowerManagementProfileInfo:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 &PlatformRepo->PmProfileInfo,
+                 sizeof (PlatformRepo->PmProfileInfo),
+                 1,
+                 CmObject
+                 );
+      break;
+
+    case EArmObjItsGroup:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 &PlatformRepo->ItsGroupInfo,
+                 sizeof (PlatformRepo->ItsGroupInfo),
+                 1,
+                 CmObject
+                 );
+      break;
+
+    case EArmObjGicItsIdentifierArray:
+      Status = HandleCmObjectRefByToken (
+                 This,
+                 CmObjectId,
+                 PlatformRepo->ItsIdentifierArray,
+                 sizeof (PlatformRepo->ItsIdentifierArray),
+                 ARRAY_SIZE (PlatformRepo->ItsIdentifierArray),
+                 Token,
+                 GetItsIdentifierArray,
+                 CmObject
+                 );
+      break;
+
+    case EArmObjRootComplex:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 &PlatformRepo->RootComplexInfo,
+                 sizeof (PlatformRepo->RootComplexInfo),
+                 1,
+                 CmObject
+                 );
+      break;
+
+    case EArmObjIdMappingArray:
+      Status = HandleCmObjectRefByToken (
+                 This,
+                 CmObjectId,
+                 PlatformRepo->DeviceIdMapping,
+                 sizeof (PlatformRepo->DeviceIdMapping),
+                 ARRAY_SIZE (PlatformRepo->DeviceIdMapping),
+                 Token,
+                 GetDeviceIdMappingArray,
+                 CmObject
+                 );
+      break;
+
+    default:
+      //
+      // No match found among the static objects.
+      // Check the dynamic objects.
+      //
+      Status = DynamicPlatRepoGetObject (
+                 PlatformRepo->DynamicPlatformRepo,
+                 CmObjectId,
+                 Token,
+                 CmObject
+                 );
+      break;
+  } // switch
+
+  if (Status == EFI_NOT_FOUND) {
+    DEBUG ((
+      DEBUG_INFO,
+      "INFO: CmObjectId " FMT_CM_OBJECT_ID ". Status = %r\n",
+      CmObjectId,
+      Status
+      ));
+  } else {
+    ASSERT_EFI_ERROR (Status);
+  }
+
+  return Status;
+}
+
+/**
+  Return an OEM namespace object.
+
+  @param [in]      This        Pointer to the Configuration Manager Protocol.
+  @param [in]      CmObjectId  The Configuration Manager Object ID.
+  @param [in]      Token       An optional token identifying the object. If
+                               unused this must be CM_NULL_TOKEN.
+  @param [in, out] CmObject    Pointer to the Configuration Manager Object
+                               descriptor describing the requested Object.
+
+  @retval EFI_SUCCESS           Success.
+  @retval EFI_INVALID_PARAMETER A parameter is invalid.
+  @retval EFI_NOT_FOUND         The required object information is not found.
+**/
+EFI_STATUS
+EFIAPI
+GetOemNameSpaceObject (
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  This,
+  IN  CONST CM_OBJECT_ID                                  CmObjectId,
+  IN  CONST CM_OBJECT_TOKEN                               Token OPTIONAL,
+  IN  OUT   CM_OBJ_DESCRIPTOR                     *CONST  CmObject
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = EFI_SUCCESS;
+  if ((This == NULL) || (CmObject == NULL)) {
+    ASSERT (This != NULL);
+    ASSERT (CmObject != NULL);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  switch (GET_CM_OBJECT_ID (CmObjectId)) {
+    default:
+      Status = EFI_NOT_FOUND;
+      DEBUG ((
+        DEBUG_ERROR,
+        "ERROR: CmObjectId " FMT_CM_OBJECT_ID ". Status = %r\n",
+        CmObjectId,
+        Status
+        ));
+      break;
+  }
+
+  return Status;
+}
+
+/**
+  The GetObject function defines the interface implemented by the
+  Configuration Manager Protocol for returning the Configuration
+  Manager Objects.
+
+  @param [in]      This        Pointer to the Configuration Manager Protocol.
+  @param [in]      CmObjectId  The Configuration Manager Object ID.
+  @param [in]      Token       An optional token identifying the object. If
+                               unused this must be CM_NULL_TOKEN.
+  @param [in, out] CmObject    Pointer to the Configuration Manager Object
+                               descriptor describing the requested Object.
+
+  @retval EFI_SUCCESS           Success.
+  @retval EFI_INVALID_PARAMETER A parameter is invalid.
+  @retval EFI_NOT_FOUND         The required object information is not found.
+**/
+EFI_STATUS
+EFIAPI
+ArmKvmtoolPlatformGetObject (
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  This,
+  IN  CONST CM_OBJECT_ID                                  CmObjectId,
+  IN  CONST CM_OBJECT_TOKEN                               Token OPTIONAL,
+  IN  OUT   CM_OBJ_DESCRIPTOR                     *CONST  CmObject
+  )
+{
+  EFI_STATUS  Status;
+
+  if ((This == NULL) || (CmObject == NULL)) {
+    ASSERT (This != NULL);
+    ASSERT (CmObject != NULL);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  switch (GET_CM_NAMESPACE_ID (CmObjectId)) {
+    case EObjNameSpaceStandard:
+      Status = GetStandardNameSpaceObject (This, CmObjectId, Token, CmObject);
+      break;
+    case EObjNameSpaceArm:
+      Status = GetArmNameSpaceObject (This, CmObjectId, Token, CmObject);
+      break;
+    case EObjNameSpaceOem:
+      Status = GetOemNameSpaceObject (This, CmObjectId, Token, CmObject);
+      break;
+    default:
+      Status = EFI_INVALID_PARAMETER;
+      DEBUG ((
+        DEBUG_ERROR,
+        "ERROR: Unknown Namespace CmObjectId " FMT_CM_OBJECT_ID ". "
+                                                                "Status = %r\n",
+        CmObjectId,
+        Status
+        ));
+      break;
+  }
+
+  return Status;
+}
+
+/**
+  The SetObject function defines the interface implemented by the
+  Configuration Manager Protocol for updating the Configuration
+  Manager Objects.
+
+  @param [in]      This        Pointer to the Configuration Manager Protocol.
+  @param [in]      CmObjectId  The Configuration Manager Object ID.
+  @param [in]      Token       An optional token identifying the object. If
+                               unused this must be CM_NULL_TOKEN.
+  @param [in]      CmObject    Pointer to the Configuration Manager Object
+                               descriptor describing the Object.
+
+  @retval EFI_UNSUPPORTED  This operation is not supported.
+**/
+EFI_STATUS
+EFIAPI
+ArmKvmtoolPlatformSetObject (
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  This,
+  IN  CONST CM_OBJECT_ID                                  CmObjectId,
+  IN  CONST CM_OBJECT_TOKEN                               Token OPTIONAL,
+  IN        CM_OBJ_DESCRIPTOR                     *CONST  CmObject
+  )
+{
+  return EFI_UNSUPPORTED;
+}
+
+//
+// A structure describing the configuration manager protocol interface.
+//
+STATIC
+CONST
+EDKII_CONFIGURATION_MANAGER_PROTOCOL  mKvmtoolPlatformConfigManagerProtocol = {
+  CREATE_REVISION (1,          0),
+  ArmKvmtoolPlatformGetObject,
+  ArmKvmtoolPlatformSetObject,
+  &mKvmtoolPlatRepositoryInfo
+};
+
+/**
+  Entrypoint of Configuration Manager Dxe.
+
+  @param  ImageHandle
+  @param  SystemTable
+
+  @retval EFI_SUCCESS
+  @retval EFI_LOAD_ERROR
+  @retval EFI_OUT_OF_RESOURCES
+**/
+EFI_STATUS
+EFIAPI
+ConfigurationManagerDxeInitialize (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = gBS->InstallProtocolInterface (
+                  &ImageHandle,
+                  &gEdkiiConfigurationManagerProtocolGuid,
+                  EFI_NATIVE_INTERFACE,
+                  (VOID *)&mKvmtoolPlatformConfigManagerProtocol
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "ERROR: Failed to get Install Configuration Manager Protocol." \
+      " Status = %r\n",
+      Status
+      ));
+    return Status;
+  }
+
+  Status = InitializePlatformRepository (
+             &mKvmtoolPlatformConfigManagerProtocol
+             );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "ERROR: Failed to initialize the Platform Configuration Repository." \
+      " Status = %r\n",
+      Status
+      ));
+    goto ErrorHandler;
+  }
+
+  return Status;
+
+ErrorHandler:
+  gBS->UninstallProtocolInterface (
+         &ImageHandle,
+         &gEdkiiConfigurationManagerProtocolGuid,
+         (VOID *)&mKvmtoolPlatformConfigManagerProtocol
+         );
+  return Status;
+}
+
+/**
+  Unload function for this image.
+
+  @param ImageHandle   Handle for the image of this driver.
+
+  @retval EFI_SUCCESS  Driver unloaded successfully.
+  @retval other        Driver can not unloaded.
+**/
+EFI_STATUS
+EFIAPI
+ConfigurationManagerDxeUnloadImage (
+  IN EFI_HANDLE  ImageHandle
+  )
+{
+  return CleanupPlatformRepository (&mKvmtoolPlatformConfigManagerProtocol);
+}
diff --git a/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.h b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.h
new file mode 100644
index 000000000000..3373948bc4eb
--- /dev/null
+++ b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManager.h
@@ -0,0 +1,125 @@
+/** @file
+
+  Copyright (c) 2021 - 2022, Arm Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Glossary:
+    - Cm or CM   - Configuration Manager
+    - Obj or OBJ - Object
+**/
+
+#ifndef CONFIGURATION_MANAGER_H_
+#define CONFIGURATION_MANAGER_H_
+
+///
+/// C array containing the compiled AML template.
+/// This symbol is defined in the auto generated C file
+/// containing the AML bytecode array.
+///
+extern CHAR8  dsdt_aml_code[];
+
+///
+/// The configuration manager version.
+///
+#define CONFIGURATION_MANAGER_REVISION  CREATE_REVISION (1, 0)
+
+///
+/// The OEM ID
+///
+#define CFG_MGR_OEM_ID  { 'A', 'R', 'M', 'L', 'T', 'D' }
+
+///
+/// Memory address size limit. Assume the whole address space.
+///
+#define MEMORY_ADDRESS_SIZE_LIMIT  64
+
+/** A function that prepares Configuration Manager Objects for returning.
+
+  @param [in]  This        Pointer to the Configuration Manager Protocol.
+  @param [in]  CmObjectId  The Configuration Manager Object ID.
+  @param [in]  Token       A token for identifying the object.
+  @param [out] CmObject    Pointer to the Configuration Manager Object
+                           descriptor describing the requested Object.
+
+  @retval EFI_SUCCESS           Success.
+  @retval EFI_INVALID_PARAMETER A parameter is invalid.
+  @retval EFI_NOT_FOUND         The required object information is not found.
+**/
+typedef EFI_STATUS (*CM_OBJECT_HANDLER_PROC) (
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  This,
+  IN  CONST CM_OBJECT_ID                                  CmObjectId,
+  IN  CONST CM_OBJECT_TOKEN                               Token,
+  IN  OUT   CM_OBJ_DESCRIPTOR                     *CONST  CmObject
+  );
+
+///
+/// A helper macro for mapping a reference token.
+///
+#define REFERENCE_TOKEN(Field)                                     \
+          (CM_OBJECT_TOKEN)((UINT8*)&mKvmtoolPlatRepositoryInfo +  \
+           OFFSET_OF (EDKII_PLATFORM_REPOSITORY_INFO, Field))
+
+///
+/// The number of ACPI tables to install
+///
+#define PLAT_ACPI_TABLE_COUNT  10
+
+///
+/// A structure describing the platform configuration
+/// manager repository information
+///
+typedef struct PlatformRepositoryInfo {
+  ///
+  /// Configuration Manager Information.
+  ///
+  CM_STD_OBJ_CONFIGURATION_MANAGER_INFO    CmInfo;
+
+  ///
+  /// List of ACPI tables
+  ///
+  CM_STD_OBJ_ACPI_TABLE_INFO               CmAcpiTableList[PLAT_ACPI_TABLE_COUNT];
+
+  ///
+  /// Power management profile information
+  ///
+  CM_ARM_POWER_MANAGEMENT_PROFILE_INFO     PmProfileInfo;
+
+  ///
+  /// ITS Group node
+  ///
+  CM_ARM_ITS_GROUP_NODE                    ItsGroupInfo;
+
+  ///
+  /// ITS Identifier array
+  ///
+  CM_ARM_ITS_IDENTIFIER                    ItsIdentifierArray[1];
+
+  ///
+  /// PCI Root complex node
+  ///
+  CM_ARM_ROOT_COMPLEX_NODE                 RootComplexInfo;
+
+  ///
+  /// Array of DeviceID mapping
+  ///
+  CM_ARM_ID_MAPPING                        DeviceIdMapping[1];
+
+  ///
+  /// Dynamic platform repository.
+  /// CmObj created by parsing the Kvmtool device tree are stored here.
+  ///
+  DYNAMIC_PLATFORM_REPOSITORY_INFO         *DynamicPlatformRepo;
+
+  ///
+  /// Base address of the FDT.
+  ///
+  VOID                                     *FdtBase;
+
+  ///
+  /// A handle to the FDT HwInfoParser.
+  ///
+  HW_INFO_PARSER_HANDLE                    FdtParserHandle;
+} EDKII_PLATFORM_REPOSITORY_INFO;
+
+#endif // CONFIGURATION_MANAGER_H_
diff --git a/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
new file mode 100644
index 000000000000..a333966a8c96
--- /dev/null
+++ b/ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
@@ -0,0 +1,54 @@
+## @file
+#  Configuration Manager Dxe
+#
+#  Copyright (c) 2021 - 2022, Arm Limited. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001B
+  BASE_NAME                      = ConfigurationManagerDxe
+  FILE_GUID                      = 3C80D366-510C-4154-BB3A-E12439AD337C
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = ConfigurationManagerDxeInitialize
+  UNLOAD_IMAGE                   = ConfigurationManagerDxeUnloadImage
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = AARCH64
+#
+
+[Sources]
+  AslTables/Dsdt.asl
+  ConfigurationManager.c
+  ConfigurationManager.h
+  ConfigurationManagerDxe.inf
+
+[Packages]
+  ArmVirtPkg/ArmVirtPkg.dec
+  DynamicTablesPkg/DynamicTablesPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  DynamicPlatRepoLib
+  HobLib
+  HwInfoParserLib
+  PrintLib
+  TableHelperLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+  UefiRuntimeServicesTableLib
+
+[Protocols]
+  gEdkiiConfigurationManagerProtocolGuid
+
+[Guids]
+  gFdtHobGuid
+
+[Depex]
+  gEdkiiPlatformHasAcpiGuid
-- 
2.25.1

[PATCH v3 7/8] ArmVirtPkg/Kvmtool: Enable ACPI support

[PierreGondois]

From: Sami Mujawar <sami.mujawar@arm.com>

A Configuration Manager that uses the Dynamic Tables framework
to generate ACPI tables for Kvmtool Guests has been provided.
This Configuration Manager uses the FdtHwInfoParser module to
parse the Kvmtool Device Tree and generate the required
Configuration Manager objects for generating the ACPI tables.

Therefore, enable ACPI table generation for Kvmtool.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3742
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---

Notes:
    v2:
    - Remove Pcds that are redefined to their default value,
      and modules already included in ArmVirtPkg/ArmVirt.dsc.inc
      [Laszlo]
    - Use guards as '!if $(ARCH) == AARCH64' to conditionnaly
      generate ACPI tables. This allows to prevent the 32bits
      ARM/ACPI combination without restricting the DynamicTablesPkg
      to AARCH64. This means that KvmTool on ARM will use the DT.
      Not adding Laszlo's Acked-by since this is not exactly what
      was suggested. [Laszlo/Pierre]

 ArmVirtPkg/ArmVirtKvmTool.dsc | 17 ++++++++++++++---
 ArmVirtPkg/ArmVirtKvmTool.fdf | 15 ++++++++++++++-
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
index 9d23072d8fa8..1ded9df0fc5d 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.dsc
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -29,6 +29,10 @@ [Defines]
 
 !include ArmVirtPkg/ArmVirt.dsc.inc
 
+!if $(ARCH) == AARCH64
+!include DynamicTablesPkg/DynamicTables.dsc.inc
+!endif
+
 !include MdePkg/MdeLibs.dsc.inc
 
 [LibraryClasses.common]
@@ -71,6 +75,9 @@ [LibraryClasses.common]
   PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf
   SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
 
+  HwInfoParserLib|DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtHwInfoParserLib.inf
+  DynamicPlatRepoLib|DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
+
 [LibraryClasses.common.SEC, LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM]
   PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
   PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf
@@ -195,9 +202,6 @@ [PcdsDynamicDefault.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoHorizontalResolution|640
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoVerticalResolution|480
 
-  ## Force DTB
-  gArmVirtTokenSpaceGuid.PcdForceNoAcpi|TRUE
-
   # Setup Flash storage variables
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x40000
@@ -353,3 +357,10 @@ [Components.common]
   }
   OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
   OvmfPkg/Virtio10Dxe/Virtio10.inf
+
+!if $(ARCH) == AARCH64
+  #
+  # ACPI Support
+  #
+  ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
+!endif
diff --git a/ArmVirtPkg/ArmVirtKvmTool.fdf b/ArmVirtPkg/ArmVirtKvmTool.fdf
index 14a5fce43a09..9e006e83ee5c 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.fdf
+++ b/ArmVirtPkg/ArmVirtKvmTool.fdf
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.
+#  Copyright (c) 2018 - 2022, ARM Limited. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -201,6 +201,19 @@ [FV.FvMain]
   INF OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
   INF OvmfPkg/Virtio10Dxe/Virtio10.inf
 
+!if $(ARCH) == AARCH64
+  #
+  # ACPI Support
+  #
+  INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+  #
+  # Dynamic Table fdf
+  #
+  !include DynamicTablesPkg/DynamicTables.fdf.inc
+
+  INF ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf
+!endif
+
   #
   # TianoCore logo (splash screen)
   #
-- 
2.25.1

[PATCH v3 8/8] ArmVirtPkg/Kvmtool: Enable Acpiview

[PierreGondois]

From: Sami Mujawar <sami.mujawar@arm.com>

Acpiview is a command line tool allowing to display, dump, or check
installed ACPI tables. Add a 'ACPIVIEW_ENABLE' switch to enable it
on an ArmVirt platform.

The switch is set for the ArmVirtKvmTool platform.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v2:
    - Only add AcpiView for ArmVirtKvmTool instead of all
      ArmVirt platforms. This is done using a
      'ACPIVIEW_ENABLE' switch. [Laszlo]

 ArmVirtPkg/ArmVirt.dsc.inc    | 5 ++++-
 ArmVirtPkg/ArmVirtKvmTool.dsc | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 5a1598d90ca7..98b95c7407fd 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
+#  Copyright (c) 2011 - 2022, ARM Limited. All rights reserved.
 #  Copyright (c) 2014, Linaro Limited. All rights reserved.
 #  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
 #  Copyright (c) Microsoft Corporation.
@@ -397,6 +397,9 @@ [Components.common]
       NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
       NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
       NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
+!if $(ACPIVIEW_ENABLE) == TRUE
+      NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
+!endif
       NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
       NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
 !if $(NETWORK_IP6_ENABLE) == TRUE
diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
index 1ded9df0fc5d..8ddd6164d974 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.dsc
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -1,7 +1,7 @@
 #  @file
 #  Workspace file for KVMTool virtual platform.
 #
-#  Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.
+#  Copyright (c) 2018 - 2022, ARM Limited. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -27,6 +27,9 @@ [Defines]
   SKUID_IDENTIFIER               = DEFAULT
   FLASH_DEFINITION               = ArmVirtPkg/ArmVirtKvmTool.fdf
 
+[Defines.AARCH64]
+  DEFINE ACPIVIEW_ENABLE         = TRUE
+
 !include ArmVirtPkg/ArmVirt.dsc.inc
 
 !if $(ARCH) == AARCH64
-- 
2.25.1