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.web08.39366.1634573832566038250 for ; Mon, 18 Oct 2021 09:17:13 -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 35D8F2F; Mon, 18 Oct 2021 09:17:12 -0700 (PDT) Received: from [192.168.1.16] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 861433F694; Mon, 18 Oct 2021 09:17:10 -0700 (PDT) Message-ID: <4f401d55-d08b-ead3-a4fc-24f969009c71@arm.com> Date: Mon, 18 Oct 2021 17:17:09 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 Subject: Re: [edk2-devel] [PATCH v2 8/8] DynamicTablesPkg: IORT generator updates for Rev E.b spec To: devel@edk2.groups.io, sami.mujawar@arm.com Cc: Alexei.Fedorov@arm.com, Matteo.Carlini@arm.com, Ben.Adderson@arm.com, steven.price@arm.com, Lorenzo.Pieralisi@arm.com, nd@arm.com References: <20210617095538.93280-1-sami.mujawar@arm.com> <20210617095538.93280-9-sami.mujawar@arm.com> From: "PierreGondois" In-Reply-To: <20210617095538.93280-9-sami.mujawar@arm.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Hi Sami, With the small modification: Reviewed-by: Pierre Gondois On 6/17/21 10:55, Sami Mujawar via groups.io wrote: > Bugzilla: 3458 - Add support IORT Rev E.b specification updates > (https://bugzilla.tianocore.org/show_bug.cgi?id=3458) > > The IO Remapping Table, Platform Design Document, Revision E.b, > Feb 2021 (https://developer.arm.com/documentation/den0049/) > introduces the following updates, collectively including the > updates and errata fixes to Rev E and Rev E.a: > - increments the IORT table revision to 3. > - updates the node definition to add an 'Identifier' field. > - adds definition of node type 6 - Reserved Memory Range node. > - adds definition for Memory Range Descriptors. > - adds flag to indicate PRI support for root complexes. > - adds flag to indicate if the root complex supports forwarding > of PASID information on translated transactions to the SMMU. > > Therefore, update the IORT generator to: > - increment IORT table revision count to 3. > - populate Identifier filed if revision is greater than 2. > - add support to populate Reserved Memory Range nodes and > the Memory range descriptors. > - add validation to check that the Identifier field is > unique. > > Signed-off-by: Sami Mujawar > --- > > Notes: > v2: > - The macro EFI_ACPI_IO_REMAPPING_TABLE_REVISION was set to [SAMI] > Rev 0 as setting to Rev 3 will break existing platforms. > Therefore, set the max supported IORT generator revision > to 3. > > DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c | 661 ++++++++++++++++++-- > DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.h | 5 +- > 2 files changed, 624 insertions(+), 42 deletions(-) > > diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c > index 9ccf72594db378878d4e3abbafe98e749d9963da..9b687f0165e6136f2f24749d27dee27edaff1b31 100644 > --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c > +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c > @@ -5,8 +5,8 @@ > SPDX-License-Identifier: BSD-2-Clause-Patent > > @par Reference(s): > - - IO Remapping Table, Platform Design Document, > - Document number: ARM DEN 0049D, Issue D, March 2018 > + - IO Remapping Table, Platform Design Document, Revision E.b, Feb 2021 > + (https://developer.arm.com/documentation/den0049/) > > **/ > [snip] > + > // Ids for SMMUv3 node > IdMapArray = (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE*)((UINT8*)SmmuV3Node + > SmmuV3Node->Node.IdReference); > @@ -1417,6 +1616,7 @@ AddSmmuV3Nodes ( > @param [in] This Pointer to the table Generator. > @param [in] CfgMgrProtocol Pointer to the Configuration Manager > Protocol Interface. > + @param [in] AcpiTableInfo Pointer to the ACPI table info structure. > @param [in] Iort Pointer to IORT table structure. > @param [in] NodesStartOffset Offset for the start of the PMCG Nodes. > @param [in] NodeList Pointer to an array of PMCG Node Objects. > @@ -1431,6 +1631,7 @@ EFI_STATUS > AddPmcgNodes ( > IN CONST ACPI_TABLE_GENERATOR * CONST This, > IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST CfgMgrProtocol, > + IN CONST CM_STD_OBJ_ACPI_TABLE_INFO * CONST AcpiTableInfo, > IN CONST EFI_ACPI_6_0_IO_REMAPPING_TABLE * Iort, > IN CONST UINT32 NodesStartOffset, > IN CONST CM_ARM_PMCG_NODE * NodeList, > @@ -1465,12 +1666,18 @@ AddPmcgNodes ( > // Populate the node header > PmcgNode->Node.Type = EFI_ACPI_IORT_TYPE_PMCG; > PmcgNode->Node.Length = (UINT16)NodeLength; > - PmcgNode->Node.Revision = 1; > - PmcgNode->Node.Reserved = EFI_ACPI_RESERVED_DWORD; > PmcgNode->Node.NumIdMappings = NodeList->IdMappingCount; > PmcgNode->Node.IdReference = (NodeList->IdMappingCount == 0) ? > 0 : sizeof (EFI_ACPI_6_0_IO_REMAPPING_PMCG_NODE); > > + if (AcpiTableInfo->AcpiTableRevision < EFI_ACPI_IO_REMAPPING_TABLE_REV3) { > + PmcgNode->Node.Revision = 1; > + PmcgNode->Node.Identifier = EFI_ACPI_RESERVED_DWORD; > + } else { > + PmcgNode->Node.Revision = 2; > + PmcgNode->Node.Identifier = NodeList->Identifier; > + } > + > // PMCG specific data > PmcgNode->Base = NodeList->BaseAddress; > PmcgNode->OverflowInterruptGsiv = NodeList->OverflowInterrupt; > @@ -1494,8 +1701,19 @@ AddPmcgNodes ( > return Status; > } > > - if ((NodeList->IdMappingCount > 0) && > - (NodeList->IdMappingToken != CM_NULL_TOKEN)) { > + if (NodeList->IdMappingCount > 0) { > + if (NodeList->IdMappingToken == CM_NULL_TOKEN) { > + Status = EFI_INVALID_PARAMETER; > + DEBUG (( > + DEBUG_ERROR, > + "ERROR: IORT: Invalid Id Mapping token," > + " Token = 0x%x, Status =%r\n", > + NodeList->IdMappingToken, > + Status > + )); > + return Status; > + } > + As this is not related to the RevE spec update, could it be done in: [PATCH v2 6/8] DynamicTablesPkg: IORT set reference to interrupt array if present This is also done at other places. > // Ids for PMCG node > IdMapArray = (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE*)((UINT8*)PmcgNode + > PmcgNode->Node.IdReference); > @@ -1526,6 +1744,273 @@ AddPmcgNodes ( > return EFI_SUCCESS; > } > [snip] Regards, Pierre