Hi Jeff, I have two minor suggestions marked inline as [SAMI]. Other than those this patch looks good to me. With that updated, Reviewed-by: Sami Mujawar Regards, Sami Mujawar On 27/10/2022 03:40 pm, Jeff Brasen via groups.io wrote: > Add a new parser for the Arm Performance Monitoring Unit Table. > > The APMT table describes the properties of PMU support > > implemented by components in an Arm-based system. > > > > Signed-off-by: Jeff Brasen > > --- > > .../UefiShellAcpiViewCommandLib/AcpiParser.h | 21 ++++ > > .../Parsers/Apmt/ApmtParser.c | 105 ++++++++++++++++++ > > .../UefiShellAcpiViewCommandLib.c | 1 + > > .../UefiShellAcpiViewCommandLib.inf | 1 + > > 4 files changed, 128 insertions(+) > > create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Apmt/ApmtParser.c > > > > diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h > > index db8c88f6df..6a1de4e12b 100644 > > --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h > > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h > > @@ -531,6 +531,27 @@ ParseAcpiAest ( > > IN UINT8 AcpiTableRevision > > ); > > > > +/** > > + This function parses the ACPI APMT table. > > + When trace is enabled this function parses the APMT table and > > + traces the ACPI table fields. > > + > > + This function also performs validation of the ACPI table fields. > > + > > + @param [in] Trace If TRUE, trace the ACPI fields. > > + @param [in] Ptr Pointer to the start of the buffer. > > + @param [in] AcpiTableLength Length of the ACPI table. > > + @param [in] AcpiTableRevision Revision of the ACPI table. > > +**/ > > +VOID > > +EFIAPI > > +ParseAcpiApmt ( > > + IN BOOLEAN Trace, > > + IN UINT8 *Ptr, > > + IN UINT32 AcpiTableLength, > > + IN UINT8 AcpiTableRevision > > + ); > > + > > /** > > This function parses the ACPI BGRT table. > > When trace is enabled this function parses the BGRT table and > > diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Apmt/ApmtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Apmt/ApmtParser.c > > new file mode 100644 > > index 0000000000..b036cd12d3 > > --- /dev/null > > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Apmt/ApmtParser.c > > @@ -0,0 +1,105 @@ > > +/** @file > > + APMT table parser > > + > > + Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved. > > + Copyright (c) 2017 - 2018, ARM Limited. All rights reserved. > > + SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > + @par Reference(s): > > + - ACPI 6.2 Specification - Errata A, September 2017 > > +**/ > > + > > +#include > > +#include > > +#include > > +#include "AcpiParser.h" > > +#include "AcpiTableParser.h" > > + > > +// Local variables > > +STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo; > > +STATIC CONST UINT16 *NodeLength; > > + > > +/** > > + An ACPI_PARSER array describing the ACPI APMT Table. > > +**/ > > +STATIC CONST ACPI_PARSER ApmtParser[] = { > > + PARSE_ACPI_HEADER (&AcpiHdrInfo) > > +}; > > + > > +/** > > + An ACPI_PARSER array describing the ACPI Arm PMU Node. > > +**/ > > +STATIC CONST ACPI_PARSER ArmPmuNodeParser[] = { > > + { L"Length", 2, 0, L"0x%x", NULL, (VOID **)&NodeLength, NULL, NULL }, > > + { L"Node flags", 1, 2, L"0x%x", NULL, NULL, NULL, NULL }, > > + { L"Node type", 1, 3, L"0x%x", NULL, NULL, NULL, NULL }, > > + { L"Identifier", 4, 4, L"0x%x", NULL, NULL, NULL, NULL }, > > + { L"Node Instance primary", 8, 8, L"0x%lx", NULL, NULL, NULL, NULL }, > > + { L"Node Instance secondary", 4, 16, L"0x%x", NULL, NULL, NULL, NULL }, > > + { L"Base address 0", 8, 20, L"0x%lx", NULL, NULL, NULL, NULL }, > > + { L"Base address 1", 8, 28, L"0x%lx", NULL, NULL, NULL, NULL }, > > + { L"Overflow interrupt", 4, 36, L"0x%x", NULL, NULL, NULL, NULL }, > > + { L"Reserved1", 4, 40, L"0x%x", NULL, NULL, NULL, NULL }, > > + { L"Overflow interrupt flags", 4, 44, L"0x%x", NULL, NULL, NULL, NULL }, > > + { L"Processor affinity", 4, 48, L"0x%x", NULL, NULL, NULL, NULL }, > > + { L"Implementation ID", 4, 52, L"0x%x", NULL, NULL, NULL, NULL } > > +}; > > + > > +/** > > + This function parses the ACPI APMT table. > > + When trace is enabled this function parses the APMT table and > > + traces the ACPI table fields. > > + > > + This function also performs validation of the ACPI table fields. > > + > > + @param [in] Trace If TRUE, trace the ACPI fields. > > + @param [in] Ptr Pointer to the start of the buffer. > > + @param [in] AcpiTableLength Length of the ACPI table. > > + @param [in] AcpiTableRevision Revision of the ACPI table. > > +**/ > > +VOID > > +EFIAPI > > +ParseAcpiApmt ( > > + IN BOOLEAN Trace, > > + IN UINT8 *Ptr, > > + IN UINT32 AcpiTableLength, > > + IN UINT8 AcpiTableRevision > > + ) > > +{ > > + UINT32 Offset; > > + > > + if (!Trace) { > > + return; > > + } > > + > > + ParseAcpi ( > > + Trace, > > + 0, > > + "APMT", > > + Ptr, > > + AcpiTableLength, > > + PARSER_PARAMS (ApmtParser) > > + ); > > + Offset = sizeof (EFI_ACPI_DESCRIPTION_HEADER); > > + > > + while (Offset < AcpiTableLength) { > > + ParseAcpi ( > > + Trace, > > + 2, > > + "Arm PMU node", > > + Ptr + Offset, > > + (AcpiTableLength - Offset), > > + PARSER_PARAMS (ArmPmuNodeParser) > > + ); > > + if (NodeLength == NULL) { [SAMI] I think it will be good to increment the error count by calling'IncrementErrorCount ()'. > > + Print ( > > + L"ERROR: Insufficient remaining table buffer length to read the " \ > > + L"Node structure. Length = %d.\n", > > + (AcpiTableLength - Offset) > > + ); > > + break; > > + } > > + > > + Offset += *NodeLength; > > + } > > +} > > diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c > > index 09bdddb56e..53f06fc757 100644 > > --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c > > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c > > @@ -48,6 +48,7 @@ STATIC > > CONST > > ACPI_TABLE_PARSER ParserList[] = { > > { EFI_ACPI_6_3_ARM_ERROR_SOURCE_TABLE_SIGNATURE, ParseAcpiAest }, > > + { EFI_ACPI_6_4_ARM_PERFORMANCE_MONITORING_UNIT_TABLE_SIGNATURE, ParseAcpiApmt }, > > { EFI_ACPI_6_2_BOOT_GRAPHICS_RESOURCE_TABLE_SIGNATURE, ParseAcpiBgrt }, > > { EFI_ACPI_6_2_DEBUG_PORT_2_TABLE_SIGNATURE, ParseAcpiDbg2 }, > > { EFI_ACPI_6_2_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE, > > diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf > > index 63fc5a1281..b03ec1a31b 100644 > > --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf > > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf > > @@ -28,6 +28,7 @@ > > AcpiViewConfig.c > > AcpiViewConfig.h > > Parsers/Aest/AestParser.c > > + Parsers/Apmt/ApmtParser.c [SAMI] I think it would be good to update the STR_GET_HELP_ACPIVIEW text in ShellPkg\Library\UefiShellAcpiViewCommandLib\UefiShellAcpiViewCommandLib.uni to add APMT as a supported parser. [/SAMI] > > Parsers/Bgrt/BgrtParser.c > > Parsers/Dbg2/Dbg2Parser.c > > Parsers/Dsdt/DsdtParser.c >