From: "Jeremy Linton" <jeremy.linton@arm.com>
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
Subject: Re: [edk2-devel] [PATCH v1 3/7] ShellPkg: Update Acpiview PPTT parser to ACPI 6.4
Date: Mon, 18 Oct 2021 11:33:05 -0500 [thread overview]
Message-ID: <c5b06ee0-5efb-f7da-9af7-388fa211ae11@arm.com> (raw)
In-Reply-To: <20211018151046.31232-4-christopher.jones@arm.com>
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 <christopher.jones@arm.com>
> ---
> 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},
>
next prev parent reply other threads:[~2021-10-18 16:33 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-18 15:10 [PATCH v1 0/7] Support ACPI 6.4 PPTT changes Chris Jones
2021-10-18 15:10 ` [PATCH v1 1/7] MdePkg: Add missing Cache ID (in)valid define Chris Jones
2021-10-18 15:10 ` [PATCH v1 2/7] MdePkg: Remove PPTT ID type structure Chris Jones
2021-10-29 1:34 ` 回复: " gaoliming
2021-10-29 9:18 ` Chris Jones
2021-10-18 15:10 ` [PATCH v1 3/7] ShellPkg: Update Acpiview PPTT parser to ACPI 6.4 Chris Jones
2021-10-18 16:33 ` Jeremy Linton [this message]
2021-11-03 17:47 ` [edk2-devel] " Chris Jones
2021-10-18 15:10 ` [PATCH v1 4/7] ShellPkg: Add Cache ID to PPTT parser Chris Jones
2021-10-18 15:10 ` [PATCH v1 5/7] DynamicTablesPkg: Remove PPTT ID structure from ACPI 6.4 generator Chris Jones
2021-10-18 15:10 ` [PATCH v1 6/7] DynamicTablesPkg: Update PPTT generator to ACPI 6.4 Chris Jones
2021-10-18 15:10 ` [PATCH v1 7/7] DynamicTablesPkg: Add CacheId to PPTT generator Chris Jones
2021-11-02 1:23 ` 回复: [edk2-devel] [PATCH v1 0/7] Support ACPI 6.4 PPTT changes gaoliming
2021-11-03 16:20 ` Chris Jones
2021-11-04 6:06 ` 回复: " gaoliming
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c5b06ee0-5efb-f7da-9af7-388fa211ae11@arm.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox