From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web10.54026.1658324218366148351 for ; Wed, 20 Jul 2022 06:36:58 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: pierre.gondois@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4B6DB13D5; Wed, 20 Jul 2022 06:36:58 -0700 (PDT) Received: from [10.34.100.102] (pierre123.nice.arm.com [10.34.100.102]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 083E93F70D; Wed, 20 Jul 2022 06:36:56 -0700 (PDT) Message-ID: <1019cb29-9da2-0043-f5fc-b5c81fb647d4@arm.com> Date: Wed, 20 Jul 2022 15:36:29 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [edk2-devel] [PATCH v1 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space To: devel@edk2.groups.io, kuqin12@gmail.com, Sami Mujawar Cc: Joe Lopez References: <20220719002254.1891-1-kuqin12@gmail.com> <20220719002254.1891-6-kuqin12@gmail.com> From: "PierreGondois" In-Reply-To: <20220719002254.1891-6-kuqin12@gmail.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Hello Kun, On 7/19/22 02:22, Kun Qin via groups.io wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3998 >=20 > Certain OSes will complain if the ECAM config space is not reserved in > the ACPI namespace. >=20 > This change adds a function to reserve PNP motherboard resources for a > given PCI node. >=20 > Co-authored-by: Joe Lopez > Signed-off-by: Kun Qin > --- > DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerato= r.c | 130 ++++++++++++++++++++ > 1 file changed, 130 insertions(+) >=20 > diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtP= cieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/Ssd= tPcieGenerator.c > index 626e53d70205..d9ed513a2ee3 100644 > --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGene= rator.c > +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGene= rator.c > @@ -772,6 +772,128 @@ error_handler: > return Status; > } > =20 > +/** Generate a Pci Resource Template to hold Address Space Info > + > + @param [in] Generator The SSDT Pci generator. > + @param [in] CfgMgrProtocol Pointer to the Configuration Manag= er > + Protocol interface. > + @param [in] PciInfo Pci device information. > + @param [in, out] PciNode RootNode of the AML tree to popula= te. > + > + @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 *Generat= or, > + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrPr= otocol, > + 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 =3D AmlCodeGenDevice ("RES0", PciNode, &ResNode); > + if (EFI_ERROR (Status)) { > + ASSERT (0); > + return Status; > + } > + > + // ASL: Name (_HID, EISAID ("PNP0C02")) > + Status =3D AmlGetEisaIdFromString ("PNP0C02", &EisaId); /* PNP Mothe= rboard Resources */ > + if (EFI_ERROR (Status)) { > + ASSERT (0); > + return Status; > + } > + > + Status =3D AmlCodeGenNameInteger ("_HID", EisaId, ResNode, NULL); > + if (EFI_ERROR (Status)) { > + ASSERT (0); > + return Status; > + } > + > + // ASL: Name (_CRS, ResourceTemplate () {}) > + Status =3D 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 =3D GetEArmObjCmRef ( > + CfgMgrProtocol, > + PciInfo->AddressMapToken, > + &RefInfo, > + &RefCount > + ); > + if (EFI_ERROR (Status)) { > + ASSERT (0); > + return Status; > + } > + > + for (Index =3D 0; Index < RefCount; Index++) { > + // Get CM_ARM_PCI_ADDRESS_MAP_INFO structures one by one. > + Status =3D GetEArmObjPciAddressMapInfo ( > + CfgMgrProtocol, > + RefInfo[Index].ReferenceToken, > + &AddrMapInfo, > + NULL > + ); > + if (EFI_ERROR (Status)) { > + ASSERT (0); > + return Status; > + } > + > + Translation =3D (AddrMapInfo->CpuAddress !=3D AddrMapInfo->PciAddr= ess); > + > + switch (AddrMapInfo->SpaceCode) { > + case PCI_SS_CONFIG: > + Status =3D 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. > =20 > @param [in] Generator The SSDT Pci generator. > @@ -855,9 +977,17 @@ GeneratePciDevice ( > return Status; > } > =20 > + // Add the PNP Motherboard Resources Device to reserve ECAM space > + Status =3D GeneratePciRes (Generator, CfgMgrProtocol, PciInfo, PciNo= de); > + 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 Sect= ion 4.1.3) must be reserved by declaring a motherboard resource. For mo= st systems, the motherboard resource would appear at the root of the AC= PI namespace (under \_SB) in a node with a _HID of EISAID (PNP0C02), an= d the resources in this case should not be claimed in the root PCI bus= =E2=80=99s _CRS. The resources can optionally be returned in Int15 E820 or EFIGetMemoryMap as reserved memory but must always be reported throu= gh ACPI as a motherboard resource. There are many examples devices describing the configuration space at oth= er 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 S= pace 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 =3D AddOscMethod (PciNode); > ASSERT_EFI_ERROR (Status); > + > return Status; > } > =20