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.38896.1634574808588969776 for ; Mon, 18 Oct 2021 09:33:28 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: jeremy.linton@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 179C02F; Mon, 18 Oct 2021 09:33:28 -0700 (PDT) Received: from [192.168.122.166] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B26FB3F694; Mon, 18 Oct 2021 09:33:27 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH v1 3/7] ShellPkg: Update Acpiview PPTT parser to ACPI 6.4 To: devel@edk2.groups.io, christopher.jones@arm.com Cc: michael.d.kinney@intel.com, gaoliming@byosoft.com.cn, zhiguang.liu@intel.com, ray.ni@intel.com, zhichao.gao@intel.com, Alexei.Fedorov@arm.com, Sami.Mujawar@arm.com, nd@arm.com References: <20211018151046.31232-1-christopher.jones@arm.com> <20211018151046.31232-4-christopher.jones@arm.com> From: "Jeremy Linton" Message-ID: Date: Mon, 18 Oct 2021 11:33:05 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <20211018151046.31232-4-christopher.jones@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Hi, On 10/18/21 10:10 AM, Chris Jones via groups.io wrote: > Bugzilla: 3697 (https://bugzilla.tianocore.org/show_bug.cgi?id=3697) > > Update the Acpiview PPTT parser to use Acpi64.h. As part of the changes, > remove support for parsing PPTT type 2 ID structure. > > Signed-off-by: Chris Jones > --- > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c | 61 ++++---------------- > ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c | 2 +- > 2 files changed, 12 insertions(+), 51 deletions(-) > > diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c > index acd2b81bb3258c7322aa10d2c0e0d842d89e358b..bce9edcedde50e53035059e6da57b9449209a674 100644 > --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c > @@ -1,11 +1,11 @@ > /** @file > PPTT table parser > > - Copyright (c) 2019 - 2020, ARM Limited. All rights reserved. > + Copyright (c) 2019 - 2021, ARM Limited. All rights reserved. > SPDX-License-Identifier: BSD-2-Clause-Patent > > @par Reference(s): > - - ACPI 6.3 Specification - January 2019 > + - ACPI 6.4 Specification - January 2021 > - ARM Architecture Reference Manual ARMv8 (D.a) > **/ > > @@ -157,8 +157,8 @@ ValidateCacheAttributes ( > ) > { > // Reference: Advanced Configuration and Power Interface (ACPI) Specification > - // Version 6.2 Errata A, September 2017 > - // Table 5-153: Cache Type Structure > + // Version 6.4, January 2021 > + // Table 5-140: Cache Type Structure > UINT8 Attributes; > Attributes = *(UINT8*)Ptr; > > @@ -222,22 +222,6 @@ STATIC CONST ACPI_PARSER CacheTypeStructureParser[] = { > {L"Line size", 2, 22, L"%d", NULL, NULL, ValidateCacheLineSize, NULL} > }; > > -/** > - An ACPI_PARSER array describing the ID Type Structure - Type 2. > -**/ > -STATIC CONST ACPI_PARSER IdStructureParser[] = { > - {L"Type", 1, 0, L"0x%x", NULL, NULL, NULL, NULL}, > - {L"Length", 1, 1, L"%d", NULL, NULL, NULL, NULL}, > - {L"Reserved", 2, 2, L"0x%x", NULL, NULL, NULL, NULL}, > - > - {L"VENDOR_ID", 4, 4, NULL, Dump4Chars, NULL, NULL, NULL}, > - {L"LEVEL_1_ID", 8, 8, L"0x%x", NULL, NULL, NULL, NULL}, > - {L"LEVEL_2_ID", 8, 16, L"0x%x", NULL, NULL, NULL, NULL}, > - {L"MAJOR_REV", 2, 24, L"0x%x", NULL, NULL, NULL, NULL}, > - {L"MINOR_REV", 2, 26, L"0x%x", NULL, NULL, NULL, NULL}, > - {L"SPIN_REV", 2, 28, L"0x%x", NULL, NULL, NULL, NULL}, > -}; > - Don't people compile shellpkg for out of tree machines? Are we 100% sure that there aren't any machines in the wild with older PPTT's using this structure? Although, this is less of a problem than below.. > /** > This function parses the Processor Hierarchy Node Structure (Type 0). > > @@ -335,29 +319,6 @@ DumpCacheTypeStructure ( > ); > } > > -/** > - This function parses the ID Structure (Type 2). > - > - @param [in] Ptr Pointer to the start of the ID Structure data. > - @param [in] Length Length of the ID Structure. > -**/ > -STATIC > -VOID > -DumpIDStructure ( > - IN UINT8* Ptr, > - IN UINT8 Length > - ) > -{ > - ParseAcpi ( > - TRUE, > - 2, > - "ID Structure", > - Ptr, > - Length, > - PARSER_PARAMS (IdStructureParser) > - ); > -} > - > /** > This function parses the ACPI PPTT table. > When trace is enabled this function parses the PPTT table and > @@ -366,7 +327,6 @@ DumpIDStructure ( > This function parses the following processor topology structures: > - Processor hierarchy node structure (Type 0) > - Cache Type Structure (Type 1) > - - ID structure (Type 2) > > This function also performs validation of the ACPI table fields. > > @@ -444,22 +404,23 @@ ParseAcpiPptt ( > Print (L"0x%x\n", Offset); > > switch (*ProcessorTopologyStructureType) { > - case EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR: > + case EFI_ACPI_6_4_PPTT_TYPE_PROCESSOR: I suspect this breaks every single other edk2-platforms machine in this tree not using the dynamic table generator. AKA all the hardcoded PPTTs used on synquacer/rpi/etc. I suspect this code path should be able to deal with multiple versions of the spec. Thanks, > DumpProcessorHierarchyNodeStructure ( > ProcessorTopologyStructurePtr, > *ProcessorTopologyStructureLength > ); > break; > - case EFI_ACPI_6_2_PPTT_TYPE_CACHE: > + case EFI_ACPI_6_4_PPTT_TYPE_CACHE: > DumpCacheTypeStructure ( > ProcessorTopologyStructurePtr, > *ProcessorTopologyStructureLength > ); > break; > - case EFI_ACPI_6_2_PPTT_TYPE_ID: > - DumpIDStructure ( > - ProcessorTopologyStructurePtr, > - *ProcessorTopologyStructureLength > + case EFI_ACPI_6_3_PPTT_TYPE_ID: > + IncrementErrorCount (); > + Print ( > + L"ERROR: PPTT Type 2 - Processor ID is deprecated and must not be" > + L"used.\n" > ); > break; > default: > diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c > index d725cad14c5d018e2004eb8e33c845aa9c719429..ab9e6c619d70df4f79d782416037d7bef62c92d5 100644 > --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c > @@ -62,7 +62,7 @@ ACPI_TABLE_PARSER ParserList[] = { > ParseAcpiMcfg}, > {EFI_ACPI_6_4_PLATFORM_COMMUNICATIONS_CHANNEL_TABLE_SIGNATURE, > ParseAcpiPcct}, > - {EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE, > + {EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE, > ParseAcpiPptt}, > {RSDP_TABLE_INFO, ParseAcpiRsdp}, > {EFI_ACPI_6_2_SYSTEM_LOCALITY_INFORMATION_TABLE_SIGNATURE, ParseAcpiSlit}, >