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.web09.21245.1657714260048353837 for ; Wed, 13 Jul 2022 05:11:00 -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 124161424; Wed, 13 Jul 2022 05:11:00 -0700 (PDT) Received: from [192.168.1.11] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2B6963F73D; Wed, 13 Jul 2022 05:10:56 -0700 (PDT) Message-ID: <5c75eb38-61ae-b7e3-4e9a-0ff235cab104@arm.com> Date: Wed, 13 Jul 2022 14:10:25 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 From: "PierreGondois" Subject: Re: [PATCH v5 6/8] ShellPkg: Acpiview: IORT parser update for IORT Rev E.d spec To: Sami Mujawar , devel@edk2.groups.io Cc: Alexei.Fedorov@arm.com, steven.price@arm.com, Lorenzo.Pieralisi@arm.com, Matteo.Carlini@arm.com, Akanksha.Jain2@arm.com, Ben.Adderson@arm.com, ray.ni@intel.com, zhichao.gao@intel.com, nd@arm.com References: <20220712143141.18516-1-sami.mujawar@arm.com> <20220712143141.18516-7-sami.mujawar@arm.com> In-Reply-To: <20220712143141.18516-7-sami.mujawar@arm.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi Sami, > + > /** > Helper Macro for populating the IORT Node header in the ACPI_PARSER array. > > @@ -108,15 +160,15 @@ ValidateItsIdArrayReference ( > @param [out] ValidateIdArrayReference Optional pointer to a function for > validating the ID Array reference. > **/ > -#define PARSE_IORT_NODE_HEADER(ValidateIdMappingCount, \ > - ValidateIdArrayReference) \ > - { L"Type", 1, 0, L"%d", NULL, (VOID**)&IortNodeType, NULL, NULL }, \ > - { L"Length", 2, 1, L"%d", NULL, (VOID**)&IortNodeLength, NULL, NULL }, \ > - { L"Revision", 1, 3, L"%d", NULL, NULL, NULL, NULL }, \ > - { L"Reserved", 4, 4, L"0x%x", NULL, NULL, NULL, NULL }, \ > - { L"Number of ID mappings", 4, 8, L"%d", NULL, \ > - (VOID**)&IortIdMappingCount, ValidateIdMappingCount, NULL }, \ > - { L"Reference to ID Array", 4, 12, L"0x%x", NULL, \ > +#define PARSE_IORT_NODE_HEADER(ValidateIdMappingCount, \ > + ValidateIdArrayReference) \ > + { L"Type", 1, 0, L"%d", NULL, (VOID**)&IortNodeType, NULL, NULL }, \ > + { L"Length", 2, 1, L"%d", NULL, (VOID**)&IortNodeLength, NULL, NULL }, \ > + { L"Revision", 1, 3, L"%d", NULL, (VOID**)&IortNodeRevision, NULL, NULL }, \ > + { L"Identifier", 4, 4, L"0x%x", NULL, NULL, NULL, NULL }, \ > + { L"Number of ID mappings", 4, 8, L"%d", NULL, \ > + (VOID**)&IortIdMappingCount, ValidateIdMappingCount, NULL }, \ > + { L"Reference to ID Array", 4, 12, L"0x%x", NULL, \ > (VOID**)&IortIdMappingOffset, ValidateIdArrayReference, NULL } > > /** > @@ -235,11 +287,13 @@ STATIC CONST ACPI_PARSER IortNodeNamedComponentParser[] = { > **/ > STATIC CONST ACPI_PARSER IortNodeRootComplexParser[] = { > PARSE_IORT_NODE_HEADER (NULL, NULL), > - { L"Memory access properties",8, 16, L"0x%lx", NULL, NULL, NULL, NULL }, > - { L"ATS Attribute", 4, 24, L"0x%x", NULL, NULL, NULL, NULL }, > - { L"PCI Segment number", 4, 28, L"0x%x", NULL, NULL, NULL, NULL }, > - { L"Memory access size limit",1, 32, L"0x%x", NULL, NULL, NULL, NULL }, > - { L"Reserved", 3, 33, L"%x %x %x", Dump3Chars, NULL, NULL, NULL } > + { L"Memory access properties",8, 16, L"0x%lx", NULL, NULL, NULL, NULL }, > + { L"ATS Attribute", 4, 24, L"0x%x", NULL, NULL, NULL, NULL }, > + { L"PCI Segment number", 4, 28, L"0x%x", NULL, NULL, NULL, NULL }, > + { L"Memory access size limit",1, 32, L"0x%x", NULL, NULL, NULL, NULL }, > + { L"PASID capabilities", 2, 33, L"0x%x", NULL, NULL, NULL, NULL }, > + { L"Reserved", 1, 35, L"%x", NULL, NULL, NULL, NULL }, > + { L"Flags", 4, 36, L"%x", NULL, NULL, NULL, NULL }, It seems flags are usually printed as L"0x%x" > }; > > /** > @@ -253,6 +307,29 @@ STATIC CONST ACPI_PARSER IortNodePmcgParser[] = { > { L"Page 1 Base Address", 8, 32, L"0x%lx", NULL, NULL, NULL, NULL } > }; > > +/** > + An ACPI_PARSER array describing the IORT RMR node. > +**/ > +STATIC CONST ACPI_PARSER IortNodeRmrParser[] = { > + PARSE_IORT_NODE_HEADER (NULL, NULL), > + { L"Flags", 4, 16, L"0x%x", NULL, NULL, NULL, NULL }, > + { L"Memory Range Desc count", 4, 20, L"%d", NULL, > + (VOID **)&RmrMemDescCount, ValidateRmrMemDescCount,NULL }, > + { L"Memory Range Desc Ref", 4, 24, L"0x%x", NULL, > + (VOID **)&RmrMemDescOffset, NULL, NULL } > +}; > + > +/** > + An ACPI_PARSER array describing the IORT RMR Memory Range Descriptor. > +**/ > +STATIC CONST ACPI_PARSER IortNodeRmrMemRangeDescParser[] = { > + { L"Physical Range offset", 8, 0, L"0x%lx", NULL, NULL, ValidatePhysicalRange, > + NULL }, > + { L"Physical Range length", 8, 8, L"0x%lx", NULL, NULL, ValidatePhysicalRange, > + NULL }, > + { L"Reserved", 4, 16, L"0x%x", NULL, NULL, NULL, NULL} > +}; > + > /** [snip] > /** > This function parses the ACPI IORT table. > - When trace is enabled this function parses the IORT table and traces the ACPI fields. > + When trace is enabled this function parses the IORT table and traces the ACPI > + fields. > > This function also parses the following nodes: > - ITS Group > @@ -618,6 +788,7 @@ DumpIortNodePmcg ( > - SMMUv1/2 > - SMMUv3 > - PMCG > + - RMR > > This function also performs validation of the ACPI table fields. > > @@ -643,6 +814,13 @@ ParseAcpiIort ( > return; > } > > + if (AcpiTableRevision == 4) { > + IncrementErrorCount (); > + Print ( > + L"ERROR: IORT tabe Revision E.c is deprecated and must not be used.\n" > + ); > + } > + I think a macro corresponding to rev E.c should be added in MdePkg so we can identify the deprecated revision easily. Similar comment for the Rmr revision (=2) which is checked in DumpIortNodeRmr(). > ParseAcpi ( > TRUE, > 0, > @@ -765,7 +943,14 @@ ParseAcpiIort ( > *IortIdMappingOffset > ); > break; > - > + case EFI_ACPI_IORT_TYPE_RMR: > + DumpIortNodeRmr ( > + NodePtr, > + *IortNodeLength, > + *IortIdMappingCount, > + *IortIdMappingOffset > + ); > + break; > default: > IncrementErrorCount (); > Print (L"ERROR: Unsupported IORT Node type = %d\n", *IortNodeType);