[PATCH v3 0/3] DynamicTablesPkg: Pcie generation updates

[PATCH v3 0/3] DynamicTablesPkg: Pcie generation updates

[Jeff Brasen]

Add fixes/features to dynamic PCIe support

 - Correct issue with translation in generated ACPI tables.
 - Allow for more than 16 controllers to be generated.
 - Add support library to allow for override of _OSC and slot info.

Updates:
 v3 - Remove segment as UID patch, convert override to library
 v2 - Add IsPosDecode logic to translation patch


Jeff Brasen (3):
  DynamicTablesPkg: AcpiSsdtPcieLibArm: Correct translation value
  DynamicTablesPkg: AcpiSsdtPcieLibArm: Support UID > 0xF
  DynamicTablesPkg: AcpiSsdtPcieLibArm: Create support library

 .../Include/Library/SsdtPcieSupportLib.h      |  73 +++++++
 .../AcpiSsdtPcieLibArm/SsdtPcieGenerator.c    | 191 ++---------------
 .../AcpiSsdtPcieLibArm/SsdtPcieGenerator.h    |  17 +-
 .../Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm.inf |   2 +-
 .../SsdtPcieOscTemplate.asl                   |   0
 .../SsdtPcieSupportLib/SsdtPcieSupportLib.c   | 200 ++++++++++++++++++
 .../SsdtPcieSupportLib/SsdtPcieSupportLib.inf |  30 +++
 .../SsdtPcieSupportLibPrivate.h               |  25 +++
 8 files changed, 349 insertions(+), 189 deletions(-)
 create mode 100644 DynamicTablesPkg/Include/Library/SsdtPcieSupportLib.h
 rename DynamicTablesPkg/Library/{Acpi/Arm/AcpiSsdtPcieLibArm => Common/SsdtPcieSupportLib}/SsdtPcieOscTemplate.asl (100%)
 create mode 100644 DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.c
 create mode 100644 DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
 create mode 100644 DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLibPrivate.h

-- 
2.25.1

[PATCH v3 1/3] DynamicTablesPkg: AcpiSsdtPcieLibArm: Correct translation value

[Jeff Brasen]

The translation value in ACPI should be the difference between the CPU and PCIe address.

Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
 .../Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
index a34018151f..d4c5f47b07 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
@@ -540,6 +540,7 @@ GeneratePciCrs (
   UINT32                       RefCount;
   CM_ARM_PCI_ADDRESS_MAP_INFO  *AddrMapInfo;
   AML_OBJECT_NODE_HANDLE       CrsNode;
+  BOOLEAN                      IsPosDecode;
 
   ASSERT (Generator != NULL);
   ASSERT (CfgMgrProtocol != NULL);
@@ -609,6 +610,11 @@ GeneratePciCrs (
     }
 
     Translation = (AddrMapInfo->CpuAddress != AddrMapInfo->PciAddress);
+    if (AddrMapInfo->CpuAddress >= AddrMapInfo->PciAddress) {
+      IsPosDecode = TRUE;
+    } else {
+      IsPosDecode = FALSE;
+    }
 
     switch (AddrMapInfo->SpaceCode) {
       case PCI_SS_IO:
@@ -616,12 +622,12 @@ GeneratePciCrs (
                    FALSE,
                    TRUE,
                    TRUE,
-                   TRUE,
+                   IsPosDecode,
                    3,
                    0,
                    AddrMapInfo->PciAddress,
                    AddrMapInfo->PciAddress + AddrMapInfo->AddressSize - 1,
-                   Translation ? AddrMapInfo->CpuAddress : 0,
+                   Translation ? AddrMapInfo->CpuAddress - AddrMapInfo->PciAddress : 0,
                    AddrMapInfo->AddressSize,
                    0,
                    NULL,
@@ -635,7 +641,7 @@ GeneratePciCrs (
       case PCI_SS_M32:
         Status = AmlCodeGenRdDWordMemory (
                    FALSE,
-                   TRUE,
+                   IsPosDecode,
                    TRUE,
                    TRUE,
                    TRUE,
@@ -643,7 +649,7 @@ GeneratePciCrs (
                    0,
                    AddrMapInfo->PciAddress,
                    AddrMapInfo->PciAddress + AddrMapInfo->AddressSize - 1,
-                   Translation ? AddrMapInfo->CpuAddress : 0,
+                   Translation ? AddrMapInfo->CpuAddress - AddrMapInfo->PciAddress : 0,
                    AddrMapInfo->AddressSize,
                    0,
                    NULL,
@@ -657,7 +663,7 @@ GeneratePciCrs (
       case PCI_SS_M64:
         Status = AmlCodeGenRdQWordMemory (
                    FALSE,
-                   TRUE,
+                   IsPosDecode,
                    TRUE,
                    TRUE,
                    TRUE,
@@ -665,7 +671,7 @@ GeneratePciCrs (
                    0,
                    AddrMapInfo->PciAddress,
                    AddrMapInfo->PciAddress + AddrMapInfo->AddressSize - 1,
-                   Translation ? AddrMapInfo->CpuAddress : 0,
+                   Translation ? AddrMapInfo->CpuAddress - AddrMapInfo->PciAddress : 0,
                    AddrMapInfo->AddressSize,
                    0,
                    NULL,
-- 
2.25.1

[PATCH v3 2/3] DynamicTablesPkg: AcpiSsdtPcieLibArm: Support UID > 0xF

[Jeff Brasen]

Add support for PCIe devices with UID > 0xF.
This is done by using the next value in the name so
PCI5, PC26, etc

Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
 .../Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c  | 5 ++++-
 .../Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.h  | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
index d4c5f47b07..68ecae96be 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
@@ -824,7 +824,10 @@ GeneratePciDevice (
 
   // Write the name of the PCI device.
   CopyMem (AslName, "PCIx", AML_NAME_SEG_SIZE + 1);
-  AslName[AML_NAME_SEG_SIZE - 1] = AsciiFromHex (Uid);
+  AslName[AML_NAME_SEG_SIZE - 1] = AsciiFromHex (Uid & 0xF);
+  if (Uid > 0xF) {
+    AslName[AML_NAME_SEG_SIZE - 2] = AsciiFromHex ((Uid >> 4) & 0xF);
+  }
 
   // ASL: Device (PCIx) {}
   Status = AmlCodeGenDevice (AslName, ScopeNode, &PciNode);
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.h b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.h
index 59a0d601a3..515a3e1785 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.h
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.h
@@ -31,7 +31,7 @@
         Corresponding changes would be needed to support the Name and
         UID fields describing the Pci root complexes.
 */
-#define MAX_PCI_ROOT_COMPLEXES_SUPPORTED  16
+#define MAX_PCI_ROOT_COMPLEXES_SUPPORTED  256
 
 // _SB scope of the AML namespace.
 #define SB_SCOPE  "\\_SB_"
-- 
2.25.1

[PATCH v3 3/3] DynamicTablesPkg: AcpiSsdtPcieLibArm: Create support library

[Jeff Brasen]

Add support library to allow for customization of _OSC and slot info.
The functions in the library are unchanged,
with the exception of adding PciInfo pointer to the APIs.

Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
 .../Include/Library/SsdtPcieSupportLib.h      |  73 +++++++
 .../AcpiSsdtPcieLibArm/SsdtPcieGenerator.c    | 168 +--------------
 .../AcpiSsdtPcieLibArm/SsdtPcieGenerator.h    |  15 --
 .../Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm.inf |   2 +-
 .../SsdtPcieOscTemplate.asl                   |   0
 .../SsdtPcieSupportLib/SsdtPcieSupportLib.c   | 200 ++++++++++++++++++
 .../SsdtPcieSupportLib/SsdtPcieSupportLib.inf |  30 +++
 .../SsdtPcieSupportLibPrivate.h               |  25 +++
 8 files changed, 332 insertions(+), 181 deletions(-)
 create mode 100644 DynamicTablesPkg/Include/Library/SsdtPcieSupportLib.h
 rename DynamicTablesPkg/Library/{Acpi/Arm/AcpiSsdtPcieLibArm => Common/SsdtPcieSupportLib}/SsdtPcieOscTemplate.asl (100%)
 create mode 100644 DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.c
 create mode 100644 DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
 create mode 100644 DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLibPrivate.h

diff --git a/DynamicTablesPkg/Include/Library/SsdtPcieSupportLib.h b/DynamicTablesPkg/Include/Library/SsdtPcieSupportLib.h
new file mode 100644
index 0000000000..f65431ef28
--- /dev/null
+++ b/DynamicTablesPkg/Include/Library/SsdtPcieSupportLib.h
@@ -0,0 +1,73 @@
+/** @file
+  Ssdt PCie Support Library
+
+  Copyright (c) 2021 - 2022, Arm Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef SSDT_PCIE_SUPPORT_LIB_H_
+#define SSDT_PCIE_SUPPORT_LIB_H_
+
+#pragma pack(1)
+
+/** Structure used to map integer to an index.
+*/
+typedef struct MappingTable {
+  /// Mapping table.
+  /// Contains the Index <-> integer mapping
+  UINT32    *Table;
+
+  /// Last used index of the Table.
+  /// Bound by MaxIndex.
+  UINT32    LastIndex;
+
+  /// Number of entries in the Table.
+  UINT32    MaxIndex;
+} MAPPING_TABLE;
+
+#pragma pack()
+
+/** Add an _OSC template method to the PciNode.
+
+  The _OSC method is provided as an AML blob. The blob is
+  parsed and attached at the end of the PciNode list of variable elements.
+
+  @param [in]       PciInfo     Pci device information.
+  @param [in, out]  PciNode     Pci node to amend.
+
+  @retval EFI_SUCCESS             The function completed successfully.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCES    Could not allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AddOscMethod (
+  IN      CONST CM_ARM_PCI_CONFIG_SPACE_INFO  *PciInfo,
+  IN  OUT   AML_OBJECT_NODE_HANDLE            PciNode
+  );
+
+/** Generate Pci slots devices.
+
+  PCI Firmware Specification - Revision 3.3,
+  s4.8 "Generic ACPI PCI Slot Description" requests to describe the PCI slot
+  used. It should be possible to enumerate them, but this is additional
+  information.
+
+  @param [in]       PciInfo     Pci device information.
+  @param [in]  MappingTable     The mapping table structure.
+  @param [in, out]  PciNode     Pci node to amend.
+
+  @retval EFI_SUCCESS            Success.
+  @retval EFI_INVALID_PARAMETER  Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCES   Failed to allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+GeneratePciSlots (
+  IN      CONST CM_ARM_PCI_CONFIG_SPACE_INFO  *PciInfo,
+  IN      CONST MAPPING_TABLE                 *MappingTable,
+  IN  OUT       AML_OBJECT_NODE_HANDLE        PciNode
+  );
+
+#endif // SSDT_PCIE_SUPPORT_LIB_H_
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
index 68ecae96be..62cec3753c 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
@@ -29,6 +29,7 @@
 #include <Library/AcpiHelperLib.h>
 #include <Library/TableHelperLib.h>
 #include <Library/AmlLib/AmlLib.h>
+#include <Library/SsdtPcieSupportLib.h>
 #include <Protocol/ConfigurationManagerProtocol.h>
 
 #include "SsdtPcieGenerator.h"
@@ -280,86 +281,6 @@ GeneratePciDeviceInfo (
   return Status;
 }
 
-/** Generate Pci slots devices.
-
-  PCI Firmware Specification - Revision 3.3,
-  s4.8 "Generic ACPI PCI Slot Description" requests to describe the PCI slot
-  used. It should be possible to enumerate them, but this is additional
-  information.
-
-  @param [in]  MappingTable  The mapping table structure.
-  @param [in, out]  PciNode     Pci node to amend.
-
-  @retval EFI_SUCCESS            Success.
-  @retval EFI_INVALID_PARAMETER  Invalid parameter.
-  @retval EFI_OUT_OF_RESOURCES   Failed to allocate memory.
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-GeneratePciSlots (
-  IN      CONST MAPPING_TABLE           *MappingTable,
-  IN  OUT       AML_OBJECT_NODE_HANDLE  PciNode
-  )
-{
-  EFI_STATUS              Status;
-  UINT32                  Index;
-  UINT32                  LastIndex;
-  UINT32                  DeviceId;
-  CHAR8                   AslName[AML_NAME_SEG_SIZE + 1];
-  AML_OBJECT_NODE_HANDLE  DeviceNode;
-
-  ASSERT (MappingTable != NULL);
-  ASSERT (PciNode != NULL);
-
-  // Generic device name is "Dxx".
-  CopyMem (AslName, "Dxx_", AML_NAME_SEG_SIZE + 1);
-
-  LastIndex = MappingTable->LastIndex;
-
-  // There are at most 32 devices on a Pci bus.
-  if (LastIndex >= 32) {
-    ASSERT (0);
-    return EFI_INVALID_PARAMETER;
-  }
-
-  for (Index = 0; Index < LastIndex; Index++) {
-    DeviceId                       = MappingTable->Table[Index];
-    AslName[AML_NAME_SEG_SIZE - 3] = AsciiFromHex (DeviceId & 0xF);
-    AslName[AML_NAME_SEG_SIZE - 2] = AsciiFromHex ((DeviceId >> 4) & 0xF);
-
-    // ASL:
-    // Device (Dxx) {
-    //   Name (_ADR, <address value>)
-    // }
-    Status = AmlCodeGenDevice (AslName, PciNode, &DeviceNode);
-    if (EFI_ERROR (Status)) {
-      ASSERT (0);
-      return Status;
-    }
-
-    /* ACPI 6.4 specification, Table 6.2: "ADR Object Address Encodings"
-       High word-Device #, Low word-Function #. (for example, device 3,
-       function 2 is 0x00030002). To refer to all the functions on a device #,
-       use a function number of FFFF).
-    */
-    Status = AmlCodeGenNameInteger (
-               "_ADR",
-               (DeviceId << 16) | 0xFFFF,
-               DeviceNode,
-               NULL
-               );
-    if (EFI_ERROR (Status)) {
-      ASSERT (0);
-      return Status;
-    }
-
-    // _SUN object is not generated as we don't know which slot will be used.
-  }
-
-  return Status;
-}
-
 /** Generate a _PRT object (Pci Routing Table) for the Pci device.
 
   Cf. ACPI 6.4 specification, s6.2.13 "_PRT (PCI Routing Table)"
@@ -495,7 +416,7 @@ GeneratePrt (
   PrtNode = NULL;
 
   // Generate the Pci slots once all the device have been added.
-  Status = GeneratePciSlots (&Generator->DeviceTable, PciNode);
+  Status = GeneratePciSlots (PciInfo, &Generator->DeviceTable, PciNode);
   if (EFI_ERROR (Status)) {
     ASSERT (0);
     goto exit_handler;
@@ -695,89 +616,6 @@ GeneratePciCrs (
   return Status;
 }
 
-/** Add an _OSC template method to the PciNode.
-
-  The _OSC method is provided as an AML blob. The blob is
-  parsed and attached at the end of the PciNode list of variable elements.
-
-  @param [in, out]  PciNode     Pci node to amend.
-
-  @retval EFI_SUCCESS             The function completed successfully.
-  @retval EFI_INVALID_PARAMETER   Invalid parameter.
-  @retval EFI_OUT_OF_RESOURCES    Could not allocate memory.
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-AddOscMethod (
-  IN  OUT   AML_OBJECT_NODE_HANDLE  PciNode
-  )
-{
-  EFI_STATUS                   Status;
-  EFI_STATUS                   Status1;
-  EFI_ACPI_DESCRIPTION_HEADER  *SsdtPcieOscTemplate;
-  AML_ROOT_NODE_HANDLE         OscTemplateRoot;
-  AML_OBJECT_NODE_HANDLE       OscNode;
-
-  ASSERT (PciNode != NULL);
-
-  // Parse the Ssdt Pci Osc Template.
-  SsdtPcieOscTemplate = (EFI_ACPI_DESCRIPTION_HEADER *)
-                        ssdtpcieosctemplate_aml_code;
-
-  OscNode         = NULL;
-  OscTemplateRoot = NULL;
-  Status          = AmlParseDefinitionBlock (
-                      SsdtPcieOscTemplate,
-                      &OscTemplateRoot
-                      );
-  if (EFI_ERROR (Status)) {
-    DEBUG ((
-      DEBUG_ERROR,
-      "ERROR: SSDT-PCI-OSC: Failed to parse SSDT PCI OSC Template."
-      " Status = %r\n",
-      Status
-      ));
-    return Status;
-  }
-
-  Status = AmlFindNode (OscTemplateRoot, "\\_OSC", &OscNode);
-  if (EFI_ERROR (Status)) {
-    goto error_handler;
-  }
-
-  Status = AmlDetachNode (OscNode);
-  if (EFI_ERROR (Status)) {
-    goto error_handler;
-  }
-
-  Status = AmlAttachNode (PciNode, OscNode);
-  if (EFI_ERROR (Status)) {
-    // Free the detached node.
-    AmlDeleteTree (OscNode);
-    goto error_handler;
-  }
-
-error_handler:
-  // Cleanup
-  Status1 = AmlDeleteTree (OscTemplateRoot);
-  if (EFI_ERROR (Status1)) {
-    DEBUG ((
-      DEBUG_ERROR,
-      "ERROR: SSDT-PCI-OSC: Failed to cleanup AML tree."
-      " Status = %r\n",
-      Status1
-      ));
-    // If Status was success but we failed to delete the AML Tree
-    // return Status1 else return the original error code, i.e. Status.
-    if (!EFI_ERROR (Status)) {
-      return Status1;
-    }
-  }
-
-  return Status;
-}
-
 /** Generate a Pci device.
 
   @param [in]       Generator       The SSDT Pci generator.
@@ -865,7 +703,7 @@ GeneratePciDevice (
   }
 
   // Add the template _OSC method.
-  Status = AddOscMethod (PciNode);
+  Status = AddOscMethod (PciInfo, PciNode);
   ASSERT_EFI_ERROR (Status);
   return Status;
 }
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.h b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.h
index 515a3e1785..b302e79786 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.h
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.h
@@ -44,21 +44,6 @@ extern CHAR8  ssdtpcieosctemplate_aml_code[];
 
 #pragma pack(1)
 
-/** Structure used to map integer to an index.
-*/
-typedef struct MappingTable {
-  /// Mapping table.
-  /// Contains the Index <-> integer mapping
-  UINT32    *Table;
-
-  /// Last used index of the Table.
-  /// Bound by MaxIndex.
-  UINT32    LastIndex;
-
-  /// Number of entries in the Table.
-  UINT32    MaxIndex;
-} MAPPING_TABLE;
-
 /** A structure holding the Pcie generator and additional private data.
 */
 typedef struct AcpiPcieGenerator {
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm.inf b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm.inf
index 283b564801..b38a4e9d34 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm.inf
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm.inf
@@ -19,7 +19,6 @@
 [Sources]
   SsdtPcieGenerator.c
   SsdtPcieGenerator.h
-  SsdtPcieOscTemplate.asl
 
 [Packages]
   DynamicTablesPkg/DynamicTablesPkg.dec
@@ -30,3 +29,4 @@
   AcpiHelperLib
   AmlLib
   BaseLib
+  SsdtPcieSupportLib
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieOscTemplate.asl b/DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieOscTemplate.asl
similarity index 100%
rename from DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieOscTemplate.asl
rename to DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieOscTemplate.asl
diff --git a/DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.c b/DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.c
new file mode 100644
index 0000000000..4066653d0b
--- /dev/null
+++ b/DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.c
@@ -0,0 +1,200 @@
+/** @file
+  SSDT Pcie Table Generator.
+
+  Copyright (c) 2021 - 2022, Arm Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Reference(s):
+  - PCI Firmware Specification - Revision 3.0
+  - ACPI 6.4 specification:
+   - s6.2.13 "_PRT (PCI Routing Table)"
+   - s6.1.1 "_ADR (Address)"
+  - linux kernel code
+  - Arm Base Boot Requirements v1.0
+  - Arm Base System Architecture v1.0
+**/
+
+#include <Library/AcpiLib.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Protocol/AcpiTable.h>
+
+// Module specific include files.
+#include <AcpiTableGenerator.h>
+#include <ConfigurationManagerObject.h>
+#include <ConfigurationManagerHelper.h>
+#include <Library/AcpiHelperLib.h>
+#include <Library/TableHelperLib.h>
+#include <Library/AmlLib/AmlLib.h>
+#include <Library/SsdtPcieSupportLib.h>
+#include <Protocol/ConfigurationManagerProtocol.h>
+
+#include "SsdtPcieSupportLibPrivate.h"
+
+/** Generate Pci slots devices.
+
+  PCI Firmware Specification - Revision 3.3,
+  s4.8 "Generic ACPI PCI Slot Description" requests to describe the PCI slot
+  used. It should be possible to enumerate them, but this is additional
+  information.
+
+  @param [in]       PciInfo     Pci device information.
+  @param [in]  MappingTable     The mapping table structure.
+  @param [in, out]  PciNode     Pci node to amend.
+
+  @retval EFI_SUCCESS            Success.
+  @retval EFI_INVALID_PARAMETER  Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCES   Failed to allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+GeneratePciSlots (
+  IN      CONST CM_ARM_PCI_CONFIG_SPACE_INFO  *PciInfo,
+  IN      CONST MAPPING_TABLE                 *MappingTable,
+  IN  OUT       AML_OBJECT_NODE_HANDLE        PciNode
+  )
+{
+  EFI_STATUS              Status;
+  UINT32                  Index;
+  UINT32                  LastIndex;
+  UINT32                  DeviceId;
+  CHAR8                   AslName[AML_NAME_SEG_SIZE + 1];
+  AML_OBJECT_NODE_HANDLE  DeviceNode;
+
+  ASSERT (MappingTable != NULL);
+  ASSERT (PciNode != NULL);
+
+  // Generic device name is "Dxx".
+  CopyMem (AslName, "Dxx_", AML_NAME_SEG_SIZE + 1);
+
+  LastIndex = MappingTable->LastIndex;
+
+  // There are at most 32 devices on a Pci bus.
+  if (LastIndex >= 32) {
+    ASSERT (0);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  for (Index = 0; Index < LastIndex; Index++) {
+    DeviceId                       = MappingTable->Table[Index];
+    AslName[AML_NAME_SEG_SIZE - 3] = AsciiFromHex (DeviceId & 0xF);
+    AslName[AML_NAME_SEG_SIZE - 2] = AsciiFromHex ((DeviceId >> 4) & 0xF);
+
+    // ASL:
+    // Device (Dxx) {
+    //   Name (_ADR, <address value>)
+    // }
+    Status = AmlCodeGenDevice (AslName, PciNode, &DeviceNode);
+    if (EFI_ERROR (Status)) {
+      ASSERT (0);
+      return Status;
+    }
+
+    /* ACPI 6.4 specification, Table 6.2: "ADR Object Address Encodings"
+       High word-Device #, Low word-Function #. (for example, device 3,
+       function 2 is 0x00030002). To refer to all the functions on a device #,
+       use a function number of FFFF).
+    */
+    Status = AmlCodeGenNameInteger (
+               "_ADR",
+               (DeviceId << 16) | 0xFFFF,
+               DeviceNode,
+               NULL
+               );
+    if (EFI_ERROR (Status)) {
+      ASSERT (0);
+      return Status;
+    }
+
+    // _SUN object is not generated as we don't know which slot will be used.
+  }
+
+  return Status;
+}
+
+/** Add an _OSC template method to the PciNode.
+
+  The _OSC method is provided as an AML blob. The blob is
+  parsed and attached at the end of the PciNode list of variable elements.
+
+  @param [in]       PciInfo     Pci device information.
+  @param [in, out]  PciNode     Pci node to amend.
+
+  @retval EFI_SUCCESS             The function completed successfully.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCES    Could not allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AddOscMethod (
+  IN      CONST CM_ARM_PCI_CONFIG_SPACE_INFO  *PciInfo,
+  IN  OUT   AML_OBJECT_NODE_HANDLE            PciNode
+  )
+{
+  EFI_STATUS                   Status;
+  EFI_STATUS                   Status1;
+  EFI_ACPI_DESCRIPTION_HEADER  *SsdtPcieOscTemplate;
+  AML_ROOT_NODE_HANDLE         OscTemplateRoot;
+  AML_OBJECT_NODE_HANDLE       OscNode;
+
+  ASSERT (PciNode != NULL);
+
+  // Parse the Ssdt Pci Osc Template.
+  SsdtPcieOscTemplate = (EFI_ACPI_DESCRIPTION_HEADER *)
+                        ssdtpcieosctemplate_aml_code;
+
+  OscNode         = NULL;
+  OscTemplateRoot = NULL;
+  Status          = AmlParseDefinitionBlock (
+                      SsdtPcieOscTemplate,
+                      &OscTemplateRoot
+                      );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "ERROR: SSDT-PCI-OSC: Failed to parse SSDT PCI OSC Template."
+      " Status = %r\n",
+      Status
+      ));
+    return Status;
+  }
+
+  Status = AmlFindNode (OscTemplateRoot, "\\_OSC", &OscNode);
+  if (EFI_ERROR (Status)) {
+    goto error_handler;
+  }
+
+  Status = AmlDetachNode (OscNode);
+  if (EFI_ERROR (Status)) {
+    goto error_handler;
+  }
+
+  Status = AmlAttachNode (PciNode, OscNode);
+  if (EFI_ERROR (Status)) {
+    // Free the detached node.
+    AmlDeleteTree (OscNode);
+    goto error_handler;
+  }
+
+error_handler:
+  // Cleanup
+  Status1 = AmlDeleteTree (OscTemplateRoot);
+  if (EFI_ERROR (Status1)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "ERROR: SSDT-PCI-OSC: Failed to cleanup AML tree."
+      " Status = %r\n",
+      Status1
+      ));
+    // If Status was success but we failed to delete the AML Tree
+    // return Status1 else return the original error code, i.e. Status.
+    if (!EFI_ERROR (Status)) {
+      return Status1;
+    }
+  }
+
+  return Status;
+}
diff --git a/DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf b/DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
new file mode 100644
index 0000000000..bbdb6ac6c8
--- /dev/null
+++ b/DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
@@ -0,0 +1,30 @@
+## @file
+# Ssdt Serial Port Table Support Library
+#
+#  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION    = 0x0001001B
+  BASE_NAME      = SsdtPcieSupportLib
+  FILE_GUID      = 510451a0-60b2-446c-b6bf-59cbe4a41782
+  VERSION_STRING = 1.0
+  MODULE_TYPE    = DXE_DRIVER
+  LIBRARY_CLASS  = SsdtPcieSupportLib
+
+[Sources]
+  SsdtPcieSupportLib.c
+  SsdtPcieSupportLibPrivate.h
+  SsdtPcieOscTemplate.asl
+
+[Packages]
+  DynamicTablesPkg/DynamicTablesPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  AcpiHelperLib
+  AmlLib
+  BaseLib
diff --git a/DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLibPrivate.h b/DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLibPrivate.h
new file mode 100644
index 0000000000..a5dd1af660
--- /dev/null
+++ b/DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLibPrivate.h
@@ -0,0 +1,25 @@
+/** @file
+  SSDT Pcie Support Library private data.
+
+  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Reference(s):
+  - PCI Firmware Specification - Revision 3.0
+  - ACPI 6.4 specification:
+   - s6.2.13 "_PRT (PCI Routing Table)"
+   - s6.1.1 "_ADR (Address)"
+  - linux kernel code
+  - Arm Base Boot Requirements v1.0
+**/
+
+#ifndef SSDT_PCIE_SUPPORT_LIB_PRIVATE_H_
+#define SSDT_PCIE_SUPPORT_LIB_PRIVATE_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  ssdtpcieosctemplate_aml_code[];
+
+#endif // SSDT_PCIE_SUPPORT_LIB_PRIVATE_H_
-- 
2.25.1

Re: [PATCH v3 0/3] DynamicTablesPkg: Pcie generation updates

[PierreGondois]

Hello Jeff,
Thanks for the answers, for the 3 patches:
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>

Regards,
Pierre


On 7/8/22 22:59, Jeff Brasen wrote:
> 
> Add fixes/features to dynamic PCIe support
> 
>   - Correct issue with translation in generated ACPI tables.
>   - Allow for more than 16 controllers to be generated.
>   - Add support library to allow for override of _OSC and slot info.
> 
> Updates:
>   v3 - Remove segment as UID patch, convert override to library
>   v2 - Add IsPosDecode logic to translation patch
> 
> 
> Jeff Brasen (3):
>    DynamicTablesPkg: AcpiSsdtPcieLibArm: Correct translation value
>    DynamicTablesPkg: AcpiSsdtPcieLibArm: Support UID > 0xF
>    DynamicTablesPkg: AcpiSsdtPcieLibArm: Create support library
> 
>   .../Include/Library/SsdtPcieSupportLib.h      |  73 +++++++
>   .../AcpiSsdtPcieLibArm/SsdtPcieGenerator.c    | 191 ++---------------
>   .../AcpiSsdtPcieLibArm/SsdtPcieGenerator.h    |  17 +-
>   .../Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm.inf |   2 +-
>   .../SsdtPcieOscTemplate.asl                   |   0
>   .../SsdtPcieSupportLib/SsdtPcieSupportLib.c   | 200 ++++++++++++++++++
>   .../SsdtPcieSupportLib/SsdtPcieSupportLib.inf |  30 +++
>   .../SsdtPcieSupportLibPrivate.h               |  25 +++
>   8 files changed, 349 insertions(+), 189 deletions(-)
>   create mode 100644 DynamicTablesPkg/Include/Library/SsdtPcieSupportLib.h
>   rename DynamicTablesPkg/Library/{Acpi/Arm/AcpiSsdtPcieLibArm => Common/SsdtPcieSupportLib}/SsdtPcieOscTemplate.asl (100%)
>   create mode 100644 DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.c
>   create mode 100644 DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
>   create mode 100644 DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLibPrivate.h
> 

Re: [edk2-devel] [PATCH v3 1/3] DynamicTablesPkg: AcpiSsdtPcieLibArm: Correct translation value

[Sami Mujawar]

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

Hi Jeff,

Thank you for this fix.
This patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

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

Re: [edk2-devel] [PATCH v3 2/3] DynamicTablesPkg: AcpiSsdtPcieLibArm: Support UID > 0xF

[Sami Mujawar]

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

Hi Jeff,

Thank you for this patch. These changes look good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

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

Re: [edk2-devel] [PATCH v3 3/3] DynamicTablesPkg: AcpiSsdtPcieLibArm: Create support library

[Sami Mujawar]

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

Hi Jeff,

Thank you for this patch.

I have some minor updates like removing ssdtpcieosctemplate_aml_code[] from SsdtPcieGenerator.h as it has been moved to SsdtPcieSupportLibPrivate.h, and adding SsdtPcieSupportLib to DynamicTables.dsc.inc so that the Package Dsc build succeeds.
Otherwise, these changes look good to me.

I will make the changes locally before pushing.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

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

Re: [edk2-devel] [PATCH v3 0/3] DynamicTablesPkg: Pcie generation updates

[Sami Mujawar]

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

Merged as f0064ac3afa2..19a87683654a

Thanks.

Regards,

Sami Mujawar

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