REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3998

Certain OSes will complain if the ECAM config space is not reserved in
the ACPI namespace.

This change adds a function to reserve PNP motherboard resources for a
given PCI node.

Co-authored-by: Joe Lopez <joelopez@microsoft.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
 DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 130 ++++++++++++++++++++
 1 file changed, 130 insertions(+)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
index 626e53d70205..d9ed513a2ee3 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
@@ -772,6 +772,128 @@ error_handler:
   return Status;
 }
 
+/** Generate a Pci Resource Template to hold Address Space Info
+
+  @param [in]       Generator       The SSDT Pci generator.
+  @param [in]       CfgMgrProtocol  Pointer to the Configuration Manager
+                                    Protocol interface.
+  @param [in]       PciInfo         Pci device information.
+  @param [in, out]  PciNode         RootNode of the AML tree to populate.
+
+  @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
+GeneratePciRes (
+  IN            ACPI_PCI_GENERATOR                            *Generator,
+  IN      CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
+  IN      CONST CM_ARM_PCI_CONFIG_SPACE_INFO                  *PciInfo,
+  IN  OUT       AML_OBJECT_NODE_HANDLE                        PciNode
+  )
+{
+  EFI_STATUS                   Status;
+  UINT32                       EisaId;
+  AML_OBJECT_NODE_HANDLE       ResNode;
+  AML_OBJECT_NODE_HANDLE       CrsNode;
+  BOOLEAN                      Translation;
+  UINT32                       Index;
+  CM_ARM_OBJ_REF               *RefInfo;
+  UINT32                       RefCount;
+  CM_ARM_PCI_ADDRESS_MAP_INFO  *AddrMapInfo;
+
+  // ASL: Device (PCIx) {}
+  Status = AmlCodeGenDevice ("RES0", PciNode, &ResNode);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
+  // ASL: Name (_HID, EISAID ("PNP0C02"))
+  Status = AmlGetEisaIdFromString ("PNP0C02", &EisaId); /* PNP Motherboard Resources */
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
+  Status = AmlCodeGenNameInteger ("_HID", EisaId, ResNode, NULL);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
+  // ASL: Name (_CRS, ResourceTemplate () {})
+  Status = AmlCodeGenNameResourceTemplate ("_CRS", ResNode, &CrsNode);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
+  // Get the array of CM_ARM_OBJ_REF referencing the
+  // CM_ARM_PCI_ADDRESS_MAP_INFO objects.
+  Status = GetEArmObjCmRef (
+             CfgMgrProtocol,
+             PciInfo->AddressMapToken,
+             &RefInfo,
+             &RefCount
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
+  for (Index = 0; Index < RefCount; Index++) {
+    // Get CM_ARM_PCI_ADDRESS_MAP_INFO structures one by one.
+    Status = GetEArmObjPciAddressMapInfo (
+               CfgMgrProtocol,
+               RefInfo[Index].ReferenceToken,
+               &AddrMapInfo,
+               NULL
+               );
+    if (EFI_ERROR (Status)) {
+      ASSERT (0);
+      return Status;
+    }
+
+    Translation = (AddrMapInfo->CpuAddress != AddrMapInfo->PciAddress);
+
+    switch (AddrMapInfo->SpaceCode) {
+      case PCI_SS_CONFIG:
+        Status = AmlCodeGenRdQWordMemory (
+                   FALSE,
+                   TRUE,
+                   TRUE,
+                   TRUE,
+                   FALSE, // non-cacheable
+                   TRUE,
+                   0,
+                   AddrMapInfo->PciAddress,
+                   AddrMapInfo->PciAddress + AddrMapInfo->AddressSize - 1,
+                   Translation ? AddrMapInfo->CpuAddress : 0,
+                   AddrMapInfo->AddressSize,
+                   0,
+                   NULL,
+                   0,
+                   TRUE,
+                   CrsNode,
+                   NULL
+                   );
+        break;
+      default:
+        break;
+    } // switch
+
+    if (EFI_ERROR (Status)) {
+      ASSERT (0);
+      return Status;
+    }
+  }
+
+  return Status;
+}
+
 /** Generate a Pci device.
 
   @param [in]       Generator       The SSDT Pci generator.
@@ -855,9 +977,17 @@ GeneratePciDevice (
     return Status;
   }
 
+  // Add the PNP Motherboard Resources Device to reserve ECAM space
+  Status = GeneratePciRes (Generator, CfgMgrProtocol, PciInfo, PciNode);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
   // Add the template _OSC method.
   Status = AddOscMethod (PciNode);
   ASSERT_EFI_ERROR (Status);
+
   return Status;
 }
 
-- 
2.36.0.windows.1



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

Hello Kun,

On 7/19/22 02:22, Kun Qin via groups.io wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3998
> 
> Certain OSes will complain if the ECAM config space is not reserved in
> the ACPI namespace.
> 
> This change adds a function to reserve PNP motherboard resources for a
> given PCI node.
> 
> Co-authored-by: Joe Lopez <joelopez@microsoft.com>
> Signed-off-by: Kun Qin <kuqin12@gmail.com>
> ---
>   DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 130 ++++++++++++++++++++
>   1 file changed, 130 insertions(+)
> 
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> index 626e53d70205..d9ed513a2ee3 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> @@ -772,6 +772,128 @@ error_handler:
>     return Status;
>   }
>   
> +/** Generate a Pci Resource Template to hold Address Space Info
> +
> +  @param [in]       Generator       The SSDT Pci generator.
> +  @param [in]       CfgMgrProtocol  Pointer to the Configuration Manager
> +                                    Protocol interface.
> +  @param [in]       PciInfo         Pci device information.
> +  @param [in, out]  PciNode         RootNode of the AML tree to populate.
> +
> +  @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
> +GeneratePciRes (
> +  IN            ACPI_PCI_GENERATOR                            *Generator,
> +  IN      CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
> +  IN      CONST CM_ARM_PCI_CONFIG_SPACE_INFO                  *PciInfo,
> +  IN  OUT       AML_OBJECT_NODE_HANDLE                        PciNode
> +  )
> +{
> +  EFI_STATUS                   Status;
> +  UINT32                       EisaId;
> +  AML_OBJECT_NODE_HANDLE       ResNode;
> +  AML_OBJECT_NODE_HANDLE       CrsNode;
> +  BOOLEAN                      Translation;
> +  UINT32                       Index;
> +  CM_ARM_OBJ_REF               *RefInfo;
> +  UINT32                       RefCount;
> +  CM_ARM_PCI_ADDRESS_MAP_INFO  *AddrMapInfo;
> +
> +  // ASL: Device (PCIx) {}
> +  Status = AmlCodeGenDevice ("RES0", PciNode, &ResNode);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +    return Status;
> +  }
> +
> +  // ASL: Name (_HID, EISAID ("PNP0C02"))
> +  Status = AmlGetEisaIdFromString ("PNP0C02", &EisaId); /* PNP Motherboard Resources */
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +    return Status;
> +  }
> +
> +  Status = AmlCodeGenNameInteger ("_HID", EisaId, ResNode, NULL);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +    return Status;
> +  }
> +
> +  // ASL: Name (_CRS, ResourceTemplate () {})
> +  Status = AmlCodeGenNameResourceTemplate ("_CRS", ResNode, &CrsNode);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +    return Status;
> +  }
> +
> +  // Get the array of CM_ARM_OBJ_REF referencing the
> +  // CM_ARM_PCI_ADDRESS_MAP_INFO objects.
> +  Status = GetEArmObjCmRef (
> +             CfgMgrProtocol,
> +             PciInfo->AddressMapToken,
> +             &RefInfo,
> +             &RefCount
> +             );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +    return Status;
> +  }
> +
> +  for (Index = 0; Index < RefCount; Index++) {
> +    // Get CM_ARM_PCI_ADDRESS_MAP_INFO structures one by one.
> +    Status = GetEArmObjPciAddressMapInfo (
> +               CfgMgrProtocol,
> +               RefInfo[Index].ReferenceToken,
> +               &AddrMapInfo,
> +               NULL
> +               );
> +    if (EFI_ERROR (Status)) {
> +      ASSERT (0);
> +      return Status;
> +    }
> +
> +    Translation = (AddrMapInfo->CpuAddress != AddrMapInfo->PciAddress);
> +
> +    switch (AddrMapInfo->SpaceCode) {
> +      case PCI_SS_CONFIG:
> +        Status = AmlCodeGenRdQWordMemory (
> +                   FALSE,
> +                   TRUE,
> +                   TRUE,
> +                   TRUE,
> +                   FALSE, // non-cacheable
> +                   TRUE,
> +                   0,
> +                   AddrMapInfo->PciAddress,
> +                   AddrMapInfo->PciAddress + AddrMapInfo->AddressSize - 1,
> +                   Translation ? AddrMapInfo->CpuAddress : 0,
> +                   AddrMapInfo->AddressSize,
> +                   0,
> +                   NULL,
> +                   0,
> +                   TRUE,
> +                   CrsNode,
> +                   NULL
> +                   );
> +        break;
> +      default:
> +        break;
> +    } // switch
> +
> +    if (EFI_ERROR (Status)) {
> +      ASSERT (0);
> +      return Status;
> +    }
> +  }
> +
> +  return Status;
> +}
> +
>   /** Generate a Pci device.
>   
>     @param [in]       Generator       The SSDT Pci generator.
> @@ -855,9 +977,17 @@ GeneratePciDevice (
>       return Status;
>     }
>   
> +  // Add the PNP Motherboard Resources Device to reserve ECAM space
> +  Status = GeneratePciRes (Generator, CfgMgrProtocol, PciInfo, PciNode);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +    return Status;
> +  }
> +

(Just a remark for Sami)
It seems the RES0 device will be generated under each PCI device:
\_SB.PCIX.RES0
So there would be multiple devices with the PNP0C02 Eisaid.

PCI Firmware 3.2, sec 4.1.2:
     If the operating system does not natively comprehend reserving the
     MMCFG region, the MMCFG region must be reserved by firmware.  The
     address range reported in the MCFG table or by _CBA method (see Section
     4.1.3) must be reserved by declaring a motherboard resource.  For most
     systems, the motherboard resource would appear at the root of the ACPI
     namespace (under \_SB) in a node with a _HID of EISAID (PNP0C02), and
     the resources in this case should not be claimed in the root PCI bus’s
     _CRS.  The resources can optionally be returned in Int15 E820 or
     EFIGetMemoryMap as reserved memory but must always be reported through
     ACPI as a motherboard resource.

There are many examples devices describing the configuration space at other places
than under \_SB, so it should be ok to place it here.

----

Also it seems that a RES0 device will be generated even when no Address Space Info
is available. I think it should be checked that there is a configuration space
to describe first.

Regards,
Pierre

>     // Add the template _OSC method.
>     Status = AddOscMethod (PciNode);
>     ASSERT_EFI_ERROR (Status);
> +
>     return Status;
>   }
>   


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