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.web11.69583.1656923882992146462 for ; Mon, 04 Jul 2022 01:38:03 -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 D300123A; Mon, 4 Jul 2022 01:38:02 -0700 (PDT) Received: from [10.57.41.108] (unknown [10.57.41.108]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 035033F792; Mon, 4 Jul 2022 01:37:59 -0700 (PDT) Message-ID: Date: Mon, 4 Jul 2022 10:37:36 +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: [PATCH 2/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Allow use of segment number as UID To: Jeff Brasen , "devel@edk2.groups.io" Cc: "Sami.Mujawar@arm.com" , "Alexei.Fedorov@arm.com" References: <7619f090-f973-9a2d-095a-5503e63d10ff@arm.com> From: "PierreGondois" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hello Jeff, I understand that since the _UID value is generated, it is not possible for an external module to guess its value. However, the pcd (and the case you detailed) seem to be really specific. If you need an interface in the AmlLib to locate an AML node based on custom callback, we can help. Would this fit your need ? Also I don't understand how having a known _UID would ease locating the PCI device node. Regards, Pierre On 7/1/22 17:54, Jeff Brasen wrote: > I don't think there are any requirements that this should be mapped this way which is why I added this under a PCD to enable it. We have a use case where it would be helpful to know the ACPI path of the PCIe controllers externally and using the segment number for that would allow us to do this without having to have matching logic that calculates the PCIe UID from the CM object list again. > > Thanks, > Jeff > > >> -----Original Message----- >> From: Pierre Gondois >> Sent: Friday, July 1, 2022 6:42 AM >> To: Jeff Brasen ; devel@edk2.groups.io >> Cc: Sami.Mujawar@arm.com; Alexei.Fedorov@arm.com >> Subject: Re: [PATCH 2/4] DynamicTablesPkg: AcpiSsdtPcieLibArm: Allow use >> of segment number as UID >> >> External email: Use caution opening links or attachments >> >> >> Hello Jeff, >> >> From "PCI Firmware Specification Revision 3.3", s4.1.2. >> "MCFG Table Description", the "PCI Segment Group Number" field of the >> MCFG table must match the value returned by the _SEG object in the >> corresponding object. >> >> I didn't find any constraint about the _UID value. Would it be possible to >> know why this is required ? >> >> Regards, >> Pierre >> >> On 6/30/22 17:48, Jeff Brasen wrote: >>> Add support for selecting to use index or segment number as UID and >> name. >>> This allows the path of the nodes to be well known. >>> >>> Signed-off-by: Jeff Brasen >>> --- >>> DynamicTablesPkg/DynamicTablesPkg.dec | 3 +++ >>> .../AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 19 >> ++++++++++++++++++- >>> .../Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm.inf | 3 +++ >>> 3 files changed, 24 insertions(+), 1 deletion(-) >>> >>> diff --git a/DynamicTablesPkg/DynamicTablesPkg.dec >>> b/DynamicTablesPkg/DynamicTablesPkg.dec >>> index 9b74c5a671..a890a048be 100644 >>> --- a/DynamicTablesPkg/DynamicTablesPkg.dec >>> +++ b/DynamicTablesPkg/DynamicTablesPkg.dec >>> @@ -57,5 +57,8 @@ >>> # Non BSA Compliant 16550 Serial HID >>> >>> >> gEdkiiDynamicTablesPkgTokenSpaceGuid.PcdNonBsaCompliant16550SerialHi >> d| >>> ""|VOID*|0x40000008 >>> >>> + # Use PCI segment numbers as UID >>> + >>> + >> gEdkiiDynamicTablesPkgTokenSpaceGuid.PcdPciUseSegmentAsUid|FALSE|B >> OO >>> + LEAN|0x40000009 >>> + >>> [Guids] >>> gEdkiiDynamicTablesPkgTokenSpaceGuid = { 0xab226e66, 0x31d8, >>> 0x4613, { 0x87, 0x9d, 0xd2, 0xfa, 0xb6, 0x10, 0x26, 0x3c } } diff >>> --git >>> >> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenera >> t >>> or.c >>> >> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenera >> t >>> or.c >>> index f0d15f69a4..85782af380 100644 >>> --- >>> >> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenera >> t >>> or.c >>> +++ >> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGen >>> +++ erator.c >>> @@ -996,6 +996,7 @@ BuildSsdtPciTableEx ( >>> UINTN Index; >>> EFI_ACPI_DESCRIPTION_HEADER **TableList; >>> ACPI_PCI_GENERATOR *Generator; >>> + UINT32 Uid; >>> >>> ASSERT (This != NULL); >>> ASSERT (AcpiTableInfo != NULL); >>> @@ -1051,13 +1052,29 @@ BuildSsdtPciTableEx ( >>> *Table = TableList; >>> >>> for (Index = 0; Index < PciCount; Index++) { >>> + if (PcdGetBool (PcdPciUseSegmentAsUid)) { >>> + Uid = PciInfo[Index].PciSegmentGroupNumber; >>> + if (Uid > MAX_PCI_ROOT_COMPLEXES_SUPPORTED) { >>> + DEBUG (( >>> + DEBUG_ERROR, >>> + "ERROR: SSDT-PCI: Pci root complexes segment number: %d." >>> + " Greater than maximum number of Pci root complexes supported = >> %d.\n", >>> + Uid, >>> + MAX_PCI_ROOT_COMPLEXES_SUPPORTED >>> + )); >>> + return EFI_INVALID_PARAMETER; >>> + } >>> + } else { >>> + Uid = Index; >>> + } >>> + >>> // Build a SSDT table describing the Pci devices. >>> Status = BuildSsdtPciTable ( >>> Generator, >>> CfgMgrProtocol, >>> AcpiTableInfo, >>> &PciInfo[Index], >>> - Index, >>> + Uid, >>> &TableList[Index] >>> ); >>> if (EFI_ERROR (Status)) { >>> diff --git >>> >> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm. >>> inf >>> >> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm. >>> inf >>> index 283b564801..431e32a777 100644 >>> --- >>> >> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieLibArm. >>> inf >>> +++ >> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieLib >>> +++ Arm.inf >>> @@ -30,3 +30,6 @@ >>> AcpiHelperLib >>> AmlLib >>> BaseLib >>> + >>> +[Pcd] >>> + gEdkiiDynamicTablesPkgTokenSpaceGuid.PcdPciUseSegmentAsUid