public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "PierreGondois" <pierre.gondois@arm.com>
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Sami Mujawar <sami.mujawar@arm.com>,
	Rebecca Cran <quic_rcran@quicinc.com>,
	Pierre Gondois <pierre.gondois@arm.com>
Subject: [PATCH v5 4/9] DynamicTablesPkg: AcpiSsdtPcieLibArm: Remove link device generation
Date: Tue,  1 Feb 2022 18:22:47 +0100	[thread overview]
Message-ID: <20220201172252.799725-5-Pierre.Gondois@arm.com> (raw)
In-Reply-To: <20220201172252.799725-1-Pierre.Gondois@arm.com>

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, _PRT entries reference link devices. Link devices
then describe interrupts. This allows to dynamically modify
interrupts through _SRS and _PRS objects and to choose exactly the
interrupt type (level/edge triggered, active high/low).
In the second model, interrupt numbder are described in the _PRT entry.
The interrupt type is then assumed by the OS.

The Arm BSA, sE.6 "Legacy interrupts" states that PCI legacy
interrupts must be converted to SPIs, and programmed level-sensitive,
active high. Thus any OS must configure interrupts as such and there
is no need to specify the interrupt type.
Plus it is not possible to dynamically configure PCI interrupts.

Thus remove the link device generation and use the second model
for _PRT.

Suggested-by: Ard Biesheuvel <ardb+tianocore@kernel.org>
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---

Notes:
    v4:
     - New patch. [Ard]

 .../AcpiSsdtPcieLibArm/SsdtPcieGenerator.c    | 239 +-----------------
 .../AcpiSsdtPcieLibArm/SsdtPcieGenerator.h    |  64 +----
 2 files changed, 15 insertions(+), 288 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
index 3e22587d4a25..a34018151f2d 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
 
@@ -12,6 +12,7 @@
    - s6.1.1 "_ADR (Address)"
   - linux kernel code
   - Arm Base Boot Requirements v1.0
+  - Arm Base System Architecture v1.0
 **/
 
 #include <Library/AcpiLib.h>
@@ -279,171 +280,6 @@ GeneratePciDeviceInfo (
   return Status;
 }
 
-/** Generate a Link device.
-
-  The Link device is added at the beginning of the ASL Pci device definition.
-
-  Each Link device represents a Pci legacy interrupt (INTA-...-INTD).
-
-  ASL code:
-  Device (<Link Name>) {
-    Name (_UID, <Uid>])
-    Name (_HID, EISAID ("PNP0C0F"))
-    Name (CRS0, ResourceTemplate () {
-      Interrupt (ResourceProducer, Level, ActiveHigh, Exclusive) { <Irq>] }
-      })
-    Method (_CRS, 0) {
-      Return CRS0
-      })
-    Method (_DIS) { }
-  }
-
-  The list of objects to define is available at:
-  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:
-  "The _PRS (Possible Resource Settings) and _SRS (Set Resource Settings)
-  are not supported."
-
-  @param [in]       Irq         Interrupt controller interrupt.
-  @param [in]       IrqFlags    Interrupt flags.
-  @param [in]       LinkIndex   Legacy Pci interrupt index.
-                                Must be between 0-INTA and 3-INTD.
-  @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
-GenerateLinkDevice (
-  IN      UINT32                  Irq,
-  IN      UINT32                  IrqFlags,
-  IN      UINT32                  LinkIndex,
-  IN  OUT AML_OBJECT_NODE_HANDLE  PciNode
-  )
-{
-  EFI_STATUS              Status;
-  CHAR8                   AslName[AML_NAME_SEG_SIZE + 1];
-  AML_OBJECT_NODE_HANDLE  LinkNode;
-  AML_OBJECT_NODE_HANDLE  CrsNode;
-  UINT32                  EisaId;
-
-  ASSERT (LinkIndex < 4);
-  ASSERT (PciNode != NULL);
-
-  CopyMem (AslName, "LNKx", AML_NAME_SEG_SIZE + 1);
-  AslName[AML_NAME_SEG_SIZE - 1] = 'A' + LinkIndex;
-
-  // ASL: Device (LNKx) {}
-  Status = AmlCodeGenDevice (AslName, NULL, &LinkNode);
-  if (EFI_ERROR (Status)) {
-    ASSERT (0);
-    return Status;
-  }
-
-  Status = AmlAttachNode (PciNode, LinkNode);
-  if (EFI_ERROR (Status)) {
-    ASSERT (0);
-    // Failed to add.
-    AmlDeleteTree ((AML_NODE_HANDLE)LinkNode);
-    return Status;
-  }
-
-  // ASL: Name (_UID, <Uid>)
-  Status = AmlCodeGenNameInteger ("_UID", LinkIndex, LinkNode, NULL);
-  if (EFI_ERROR (Status)) {
-    ASSERT (0);
-    return Status;
-  }
-
-  // ASL: Name (_HID, EISAID ("PNP0C0F"))
-  Status = AmlGetEisaIdFromString ("PNP0C0F", &EisaId);
-  if (EFI_ERROR (Status)) {
-    ASSERT (0);
-    return Status;
-  }
-
-  Status = AmlCodeGenNameInteger ("_HID", EisaId, LinkNode, NULL);
-  if (EFI_ERROR (Status)) {
-    ASSERT (0);
-    return Status;
-  }
-
-  // ASL:
-  // Name (CRS0, ResourceTemplate () {
-  //   Interrupt (ResourceProducer, Level, ActiveHigh, Exclusive) { <Irq> }
-  // })
-  Status = AmlCodeGenNameResourceTemplate ("CRS0", LinkNode, &CrsNode);
-  if (EFI_ERROR (Status)) {
-    ASSERT (0);
-    return Status;
-  }
-
-  Status = AmlCodeGenRdInterrupt (
-             FALSE,
-             (IrqFlags & BIT0) != 0,
-             (IrqFlags & BIT1) != 0,
-             FALSE,
-             &Irq,
-             1,
-             CrsNode,
-             NULL
-             );
-  if (EFI_ERROR (Status)) {
-    ASSERT (0);
-    return Status;
-  }
-
-  // ASL:
-  // Method (_CRS, 0) {
-  //   Return (CRS0)
-  // }
-  Status = AmlCodeGenMethodRetNameString (
-             "_CRS",
-             "CRS0",
-             0,
-             FALSE,
-             0,
-             LinkNode,
-             NULL
-             );
-  if (EFI_ERROR (Status)) {
-    ASSERT (0);
-    return Status;
-  }
-
-  // ASL:Method (_DIS, 1) {}
-  // Not possible to disable interrupts.
-  Status = AmlCodeGenMethodRetNameString (
-             "_DIS",
-             NULL,
-             0,
-             FALSE,
-             0,
-             LinkNode,
-             NULL
-             );
-  if (EFI_ERROR (Status)) {
-    ASSERT (0);
-    return Status;
-  }
-
-  // _STA:
-  // ACPI 6.4, s6.3.7 "_STA (Device Status)":
-  // If a device object describes a device that is not on an enumerable bus
-  // and the device object does not have an _STA object, then OSPM assumes
-  // that the device is present, enabled, shown in the UI, and functioning.
-
-  // _MAT:
-  // Not supported. Mainly used for processors.
-
-  return Status;
-}
-
 /** Generate Pci slots devices.
 
   PCI Firmware Specification - Revision 3.3,
@@ -556,14 +392,10 @@ GeneratePrt (
 {
   EFI_STATUS                     Status;
   INT32                          Index;
-  UINT32                         IrqTableIndex;
   AML_OBJECT_NODE_HANDLE         PrtNode;
-  CHAR8                          AslName[AML_NAME_SEG_SIZE + 1];
   CM_ARM_OBJ_REF                 *RefInfo;
   UINT32                         RefCount;
   CM_ARM_PCI_INTERRUPT_MAP_INFO  *IrqMapInfo;
-  UINT32                         IrqFlags;
-  UINT32                         PrevIrqFlags;
 
   ASSERT (Generator != NULL);
   ASSERT (CfgMgrProtocol != NULL);
@@ -585,13 +417,6 @@ GeneratePrt (
     return Status;
   }
 
-  // Initialized IrqTable.
-  Status = MappingTableInitialize (&Generator->IrqTable, RefCount);
-  if (EFI_ERROR (Status)) {
-    ASSERT (0);
-    return Status;
-  }
-
   // Initialized DeviceTable.
   Status = MappingTableInitialize (&Generator->DeviceTable, RefCount);
   if (EFI_ERROR (Status)) {
@@ -606,8 +431,6 @@ GeneratePrt (
     goto exit_handler;
   }
 
-  CopyMem (AslName, "LNKx", AML_NAME_SEG_SIZE + 1);
-
   for (Index = 0; Index < RefCount; Index++) {
     // Get CM_ARM_PCI_INTERRUPT_MAP_INFO structures one by one.
     Status = GetEArmObjPciInterruptMapInfo (
@@ -621,25 +444,15 @@ GeneratePrt (
       goto exit_handler;
     }
 
-    // Add the interrupt in the IrqTable and get the link name.
-    IrqTableIndex = MappingTableAdd (
-                      &Generator->IrqTable,
-                      IrqMapInfo->IntcInterrupt.Interrupt
-                      );
-    if (IrqTableIndex >= MAX_PCI_LEGACY_INTERRUPT) {
-      ASSERT (0);
-      Status = EFI_INVALID_PARAMETER;
-      goto exit_handler;
-    }
-
-    AslName[AML_NAME_SEG_SIZE - 1] = 'A' + IrqTableIndex;
-
-    // Check that the interrupts flags are identical for all interrupts.
-    PrevIrqFlags = IrqFlags;
-    IrqFlags     = IrqMapInfo->IntcInterrupt.Flags;
-    if ((Index > 0) && (PrevIrqFlags != IrqFlags)) {
-      ASSERT (0);
+    // Check that the interrupts flags are SPIs, level high.
+    // Cf. Arm BSA v1.0, sE.6 "Legacy interrupts"
+    if ((Index > 0)   &&
+        (IrqMapInfo->IntcInterrupt.Interrupt >= 32)   &&
+        (IrqMapInfo->IntcInterrupt.Interrupt < 1020)  &&
+        ((IrqMapInfo->IntcInterrupt.Flags & 0x3) != BIT0))
+    {
       Status = EFI_INVALID_PARAMETER;
+      ASSERT_EFI_ERROR (Status);
       goto exit_handler;
     }
 
@@ -662,8 +475,8 @@ GeneratePrt (
     Status = AmlAddPrtEntry (
                (IrqMapInfo->PciDevice << 16) | 0xFFFF,
                IrqMapInfo->PciInterrupt,
-               AslName,
-               0,
+               NULL,
+               IrqMapInfo->IntcInterrupt.Interrupt,
                PrtNode
                );
     if (EFI_ERROR (Status)) {
@@ -672,24 +485,10 @@ GeneratePrt (
     }
   } // for
 
-  // Generate the LNKx devices now that we know all the interrupts used.
-  for (Index = 0; Index < Generator->IrqTable.LastIndex; Index++) {
-    Status = GenerateLinkDevice (
-               Generator->IrqTable.Table[Index],
-               IrqFlags,
-               Index,
-               PciNode
-               );
-    if (EFI_ERROR (Status)) {
-      ASSERT (0);
-      goto exit_handler;
-    }
-  } // for
-
-  // Attach the _PRT entry now, after the LNKx devices.
+  // Attach the _PRT entry.
   Status = AmlAttachNode (PciNode, PrtNode);
   if (EFI_ERROR (Status)) {
-    ASSERT (0);
+    ASSERT_EFI_ERROR (Status);
     goto exit_handler;
   }
 
@@ -705,7 +504,6 @@ GeneratePrt (
 exit_handler:
   MappingTableFree (&Generator->DeviceTable);
 exit_handler0:
-  MappingTableFree (&Generator->IrqTable);
   if (PrtNode != NULL) {
     AmlDeleteTree (PrtNode);
   }
@@ -1382,15 +1180,6 @@ ACPI_PCI_GENERATOR  SsdtPcieGenerator = {
 
   // Private fields are defined from here.
 
-  // IrqTable
-  {
-    // Table
-    NULL,
-    // LastIndex
-    0,
-    // MaxIndex
-    0
-  },
   // DeviceTable
   {
     // Table
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.h b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.h
index d1e5465e1346..59a0d601a36d 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.h
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.h
@@ -33,12 +33,6 @@
 */
 #define MAX_PCI_ROOT_COMPLEXES_SUPPORTED  16
 
-/** Maximum number of Pci legacy interrupts.
-
-  Currently 4 for INTA-INTB-INTC-INTD.
-*/
-#define MAX_PCI_LEGACY_INTERRUPT  4
-
 // _SB scope of the AML namespace.
 #define SB_SCOPE  "\\_SB_"
 
@@ -73,64 +67,8 @@ typedef struct AcpiPcieGenerator {
 
   // Private fields are defined from here.
 
-  /** A structure used to handle the Address and Interrupt Map referencing.
-
-    A CM_ARM_PCI_CONFIG_SPACE_INFO structure references two CM_ARM_OBJ_REF:
-     - one for the address mapping, referencing
-       CM_ARM_PCI_ADDRESS_MAP_INFO structures.
-     - one for the interrupt mapping, referencing
-       CM_ARM_PCI_INTERRUPT_MAP_INFO structures.
-
-    Example:
-    (Pci0)
-    CM_ARM_PCI_CONFIG_SPACE_INFO
-                |
-                +----------------------------------------
-                |                                       |
-                v                                       v
-          CM_ARM_OBJ_REF                          CM_ARM_OBJ_REF
-      (List of references to                  (List of references to
-         address mappings)                        interrupt mappings)
-                |                                       |
-                v                                       v
-    CM_ARM_PCI_ADDRESS_MAP_INFO[0..N]      CM_ARM_PCI_INTERRUPT_MAP_INFO[0..M]
-    (A list of address mappings)           (A list of interrupt mappings)
-
-    The CM_ARM_PCI_INTERRUPT_MAP_INFO objects cannot be handled individually.
-    Device's Pci legacy interrupts that are mapped to the same CPU interrupt
-    are grouped under a Link device.
-    For instance, the following mapping:
-     - [INTA of device 0] mapped on [GIC irq 168]
-     - [INTB of device 1] mapped on [GIC irq 168]
-    will be represented in an SSDT table as:
-     - [INTA of device 0] mapped on [Link device A]
-     - [INTB of device 1] mapped on [Link device A]
-     - [Link device A] mapped on [GIC irq 168]
-
-    Counting the number of Cpu interrupts used and grouping them in Link
-    devices is done through this IRQ_TABLE.
-
-    ASL code:
-    Scope (_SB) {
-      Device (LNKA) {
-        [...]
-        Name (_PRS, ResourceTemplate () {
-          Interrupt (ResourceProducer, Level, ActiveHigh, Exclusive) { 168 }
-        })
-      }
-
-      Device (PCI0) {
-        Name (_PRT, Package () {
-          Package (0x0FFFF, 0, LNKA, 0)  // INTA of device 0 <-> LNKA
-          Package (0x1FFFF, 1, LNKA, 0)  // INTB of device 1 <-> LNKA
-          })
-        }
-    }
-  */
-  MAPPING_TABLE    IrqTable;
-
   /// Table to map: Index <-> Pci device
-  MAPPING_TABLE    DeviceTable;
+  MAPPING_TABLE           DeviceTable;
 } ACPI_PCI_GENERATOR;
 
 #pragma pack()
-- 
2.25.1


  parent reply	other threads:[~2022-02-01 17:22 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-01 17:22 [PATCH v5 0/9] Add ACPI support for Kvmtool PierreGondois
2022-02-01 17:22 ` [PATCH v5 1/9] DynamicTablesPkg: Print specifier macro for CM_OBJECT_ID PierreGondois
2022-02-01 17:22 ` [PATCH v5 2/9] DynamicTablesPkg: FdtHwInfoParserLib: Parse Pmu info PierreGondois
2022-02-01 17:22 ` [PATCH v5 3/9] DynamicTablesPkg: AmlLib: AmlAddPrtEntry() to handle GSI PierreGondois
2022-02-01 17:22 ` PierreGondois [this message]
2022-02-01 17:22 ` [PATCH v5 5/9] ArmVirtPkg: Add cspell exceptions PierreGondois
2022-02-01 17:22 ` [PATCH v5 6/9] ArmVirtPkg/Kvmtool: Add DSDT ACPI table PierreGondois
2022-02-01 17:22 ` [PATCH v5 7/9] ArmVirtPkg/Kvmtool: Add Configuration Manager PierreGondois
2022-02-01 17:22 ` [PATCH v5 8/9] ArmVirtPkg/Kvmtool: Enable ACPI support PierreGondois
2022-02-01 17:22 ` [PATCH v5 9/9] ArmVirtPkg/Kvmtool: Enable Acpiview PierreGondois
2022-02-01 18:04 ` [PATCH v5 0/9] Add ACPI support for Kvmtool Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220201172252.799725-5-Pierre.Gondois@arm.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox