Re: [PATCH v3 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space


Sami Mujawar
 

Hi Kun,

Thank you for this patch.

These changes look good to me.

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

Regards,

Sami Mujawar

On 31/07/2022 06:37 am, 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@...>
Signed-off-by: Kun Qin <kuqin12@...>
Reviewed-by: Pierre Gondois <pierre.gondois@...>
---

Notes:
v2:
- Only create RES0 after config space checking [Pierre]
v3:
- Updated function names and descriptions [Pierre]
- Moved translation calculation to CONFIG case [Pierre]

DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 171 ++++++++++++++++++++
1 file changed, 171 insertions(+)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
index ceffe2838c03..658a089c8f1f 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
@@ -616,6 +616,169 @@ GeneratePciCrs (
return Status;

}


+/** Generate a RES0 device node to reserve PNP motherboard resources

+ for a given PCI node.

+

+ @param [in] PciNode Parent PCI node handle of the generated

+ resource object.

+ @param [out] CrsNode CRS node of the AML tree to populate.

+

+ @retval EFI_SUCCESS The function completed successfully.

+ @retval EFI_INVALID_PARAMETER Invalid input parameter.

+ @retval EFI_OUT_OF_RESOURCES Could not allocate memory.

+**/

+STATIC

+EFI_STATUS

+EFIAPI

+GenerateMotherboardDevice (

+ IN AML_OBJECT_NODE_HANDLE PciNode,

+ OUT AML_OBJECT_NODE_HANDLE *CrsNode

+ )

+{

+ EFI_STATUS Status;

+ UINT32 EisaId;

+ AML_OBJECT_NODE_HANDLE ResNode;

+

+ if (CrsNode == NULL) {

+ ASSERT (0);

+ return EFI_INVALID_PARAMETER;

+ }

+

+ // ASL: Device (RES0) {}

+ 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;

+ }

+

+ return Status;

+}

+

+/** Reserves ECAM space for PCI config space

+

+ @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

+ReserveEcamSpace (

+ 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;

+ AML_OBJECT_NODE_HANDLE CrsNode;

+ BOOLEAN Translation;

+ UINT32 Index;

+ CM_ARM_OBJ_REF *RefInfo;

+ UINT32 RefCount;

+ CM_ARM_PCI_ADDRESS_MAP_INFO *AddrMapInfo;

+ BOOLEAN IsPosDecode;

+

+ // 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;

+ }

+

+ switch (AddrMapInfo->SpaceCode) {

+ case PCI_SS_CONFIG:

+ Translation = (AddrMapInfo->CpuAddress != AddrMapInfo->PciAddress);

+ if (AddrMapInfo->CpuAddress >= AddrMapInfo->PciAddress) {

+ IsPosDecode = TRUE;

+ } else {

+ IsPosDecode = FALSE;

+ }

+

+ Status = GenerateMotherboardDevice (PciNode, &CrsNode);

+ if (EFI_ERROR (Status)) {

+ ASSERT (0);

+ break;

+ }

+

+ Status = AmlCodeGenRdQWordMemory (

+ FALSE,

+ IsPosDecode,

+ TRUE,

+ TRUE,

+ FALSE, // non-cacheable

+ TRUE,

+ 0,

+ AddrMapInfo->PciAddress,

+ AddrMapInfo->PciAddress + AddrMapInfo->AddressSize - 1,

+ Translation ? AddrMapInfo->CpuAddress - AddrMapInfo->PciAddress : 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.

@@ -702,9 +865,17 @@ GeneratePciDevice (
return Status;

}


+ // Add the PNP Motherboard Resources Device to reserve ECAM space

+ Status = ReserveEcamSpace (Generator, CfgMgrProtocol, PciInfo, PciNode);

+ if (EFI_ERROR (Status)) {

+ ASSERT (0);

+ return Status;

+ }

+

// Add the template _OSC method.

Status = AddOscMethod (PciInfo, PciNode);

ASSERT_EFI_ERROR (Status);

+

return Status;

}

Join devel@edk2.groups.io to automatically receive all group messages.