Re: [PATCH v1 1/1] ShellPkg/UefiShellAcpiViewCommandLib: Add support for PPTT

["Carsey, Jaben" <jaben.carsey@intel.com>]

Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

Note: I am not pushing this to allow proper time for other to review/comment. The code looks good to me.

Jaben

> -----Original Message-----
> From: Krzysztof Koch [mailto:krzysztof.koch@arm.com]
> Sent: Tuesday, January 22, 2019 9:06 AM
> To: edk2-devel@lists.01.org
> Cc: leif.lindholm@linaro.org; Matteo.Carlini@arm.com; Stephanie.Hughes-
> Fitt@arm.com; Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ray
> <ray.ni@intel.com>; sami.mujawar@arm.com; nd@arm.com
> Subject: [PATCH v1 1/1] ShellPkg/UefiShellAcpiViewCommandLib: Add
> support for PPTT
> Importance: High
> 
> Added the acpiview parser for the PPTT table.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
> ---
> The changes can be seen at:
> https://github.com/KrzysztofKoch1/edk2/tree/woa_390_pptt_acpiview_v1
> 
> Notes:
>     v1:
>     - add PPTT parser to acpiview                               [Krzysztof]
> 
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h                    |  23
> +-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
> | 362 ++++++++++++++++++++
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman
> dLib.c   |   4 +-
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman
> dLib.inf |   3 +-
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman
> dLib.uni |   3 +-
>  5 files changed, 391 insertions(+), 4 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> index
> ecf7dae9038a4ebcb3e3764964f0c16ca3ef51f6..a42450c1431be343870dabb4e
> 03f64ed1cf78afc 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> @@ -1,7 +1,7 @@
>  /** @file
>    Header file for ACPI parser
> 
> -  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
> +  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD
> License
>    which accompanies this distribution.  The full text of the license may be
> found at
> @@ -655,6 +655,27 @@ ParseAcpiMcfg (
>    IN UINT8   AcpiTableRevision
>    );
> 
> +/**
> +  This function parses the ACPI PPTT table.
> +  When trace is enabled this function parses the PPTT 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
> +ParseAcpiPptt (
> +  IN BOOLEAN Trace,
> +  IN UINT8*  Ptr,
> +  IN UINT32  AcpiTableLength,
> +  IN UINT8   AcpiTableRevision
> +  );
> +
>  /**
>    This function parses the ACPI RSDP table.
> 
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.
> c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.
> c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..d97ddf8e925d30917f888bf33
> e2c1346cd330663
> --- /dev/null
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.
> c
> @@ -0,0 +1,362 @@
> +/** @file
> +  PPTT table parser
> +
> +  Copyright (c) 2019, ARM Limited. All rights reserved.
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD
> License
> +  which accompanies this distribution.  The full text of the license may be
> found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> +
> +  @par Reference(s):
> +    - ACPI 6.2 Specification - Errata A, September 2017
> +**/
> +
> +#include <Library/PrintLib.h>
> +#include <Library/UefiLib.h>
> +#include "AcpiParser.h"
> +
> +// Local variables
> +STATIC CONST UINT8*  ProcessorTopologyStructureType;
> +STATIC CONST UINT8*  ProcessorTopologyStructureLength;
> +STATIC CONST UINT32* NumberOfPrivateResources;
> +STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
> +
> +/**
> +  An ACPI_PARSER array describing the ACPI PPTT Table.
> +**/
> +STATIC CONST ACPI_PARSER PpttParser[] = {
> +  PARSE_ACPI_HEADER (&AcpiHdrInfo)
> +};
> +
> +/**
> +  This function validates the Cache Type Structure (Type 1) Line size field.
> +
> +  @param [in] Ptr     Pointer to the start of the field data.
> +  @param [in] Context Pointer to context specific information e.g. this
> +                      could be a pointer to the ACPI table header.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ValidateCacheLineSize (
> +  IN UINT8* Ptr,
> +  IN VOID*  Context
> +  )
> +{
> +#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
> +  // Reference: ARM Architecture Reference Manual ARMv8 (D.a)
> +  // Section D12.2.25: CCSIDR_EL1, Current Cache Size ID Register
> +  //   LineSize, bits [2:0]
> +  //     (Log2(Number of bytes in cache line)) - 4.
> +
> +  UINT16 LineSize;
> +  LineSize = *(UINT16*)Ptr;
> +
> +  if ((LineSize < 16) || (LineSize > 2048)) {
> +    IncrementErrorCount ();
> +    Print (
> +      L"\nERROR: The cache line size must be between 16 and 2048 bytes"
> +        L" on ARM Platforms."
> +      );
> +    return;
> +  }
> +
> +  if ((LineSize & (LineSize - 1)) != 0) {
> +    IncrementErrorCount ();
> +    Print (L"\nERROR: The cache line size is not a power of 2.");
> +  }
> +#endif
> +}
> +
> +/**
> +  This function validates the Cache Type Structure (Type 1) Attributes field.
> +
> +  @param [in] Ptr     Pointer to the start of the field data.
> +  @param [in] Context Pointer to context specific information e.g. this
> +                      could be a pointer to the ACPI table header.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ValidateCacheAttributes (
> +  IN UINT8* Ptr,
> +  IN VOID*  Context
> +  )
> +{
> +#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
> +  // Reference: Advanced Configuration and Power Interface (ACPI)
> Specification
> +  //            Version 6.2 Errata A, September 2017
> +  // Table 5-153: Cache Type Structure
> +
> +  UINT8 Attributes;
> +  Attributes = *(UINT8*)Ptr;
> +
> +  if ((Attributes & 0xE0) != 0) {
> +    IncrementErrorCount ();
> +    Print (
> +      L"\nERROR: Attributes bits [7:5] are reserved and must be zero.",
> +      Attributes
> +      );
> +    return;
> +  }
> +#endif
> +}
> +
> +/**
> +  An ACPI_PARSER array describing the processor topology structure
> header.
> +**/
> +STATIC CONST ACPI_PARSER ProcessorTopologyStructureHeaderParser[] =
> {
> +  {L"Type", 1, 0, NULL, NULL, (VOID**)&ProcessorTopologyStructureType,
> +   NULL, NULL},
> +  {L"Length", 1, 1, NULL, NULL,
> (VOID**)&ProcessorTopologyStructureLength,
> +   NULL, NULL},
> +  {L"Reserved", 2, 2, NULL, NULL, NULL, NULL, NULL}
> +};
> +
> +/**
> +  An ACPI_PARSER array describing the Processor Hierarchy Node Structure -
> Type 0.
> +**/
> +STATIC CONST ACPI_PARSER ProcessorHierarchyNodeStructureParser[] = {
> +  {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"Flags", 4, 4, L"0x%x", NULL, NULL, NULL, NULL},
> +  {L"Parent", 4, 8, L"0x%x", NULL, NULL, NULL, NULL},
> +  {L"ACPI Processor ID", 4, 12, L"0x%x", NULL, NULL, NULL, NULL},
> +  {L"Number of private resources", 4, 16, L"%d", NULL,
> +   (VOID**)&NumberOfPrivateResources, NULL, NULL}
> +};
> +
> +/**
> +  An ACPI_PARSER array describing the Cache Type Structure - Type 1.
> +**/
> +STATIC CONST ACPI_PARSER CacheTypeStructureParser[] = {
> +  {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"Flags", 4, 4, L"0x%x", NULL, NULL, NULL, NULL},
> +  {L"Next Level of Cache", 4, 8, L"0x%x", NULL, NULL, NULL, NULL},
> +  {L"Size", 4, 12, L"0x%x", NULL, NULL, NULL, NULL},
> +  {L"Number of sets", 4, 16, L"%d", NULL, NULL, NULL, NULL},
> +  {L"Associativity", 1, 20, L"%d", NULL, NULL, NULL, NULL},
> +  {L"Attributes", 1, 21, L"0x%x", NULL, NULL, ValidateCacheAttributes,
> NULL},
> +  {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, L"0x%x", NULL, 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},
> +};
> +
> +/**
> +  This function parses the Processor Hierarchy Node Structure (Type 0).
> +
> +  @param [in] Ptr     Pointer to the start of the Processor Hierarchy Node
> +                      Structure data.
> +  @param [in] Length  Length of the Processor Hierarchy Node Structure.
> +**/
> +STATIC
> +VOID
> +DumpProcessorHierarchyNodeStructure (
> +  IN UINT8* Ptr,
> +  IN UINT8  Length
> +  )
> +{
> +  UINT8  Offset;
> +  UINT8* PrivateResourcePtr;
> +  UINT32 Index;
> +  CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH];
> +
> +  Offset = ParseAcpi (
> +             TRUE,
> +             2,
> +             "Processor Hierarchy Node Structure",
> +             Ptr,
> +             Length,
> +             PARSER_PARAMS (ProcessorHierarchyNodeStructureParser)
> +             );
> +
> +  PrivateResourcePtr = Ptr + Offset;
> +  Index = 0;
> +  while (Index < *NumberOfPrivateResources) {
> +    UnicodeSPrint (
> +      Buffer,
> +      sizeof (Buffer),
> +      L"Private resources [%d]",
> +      Index
> +      );
> +
> +    PrintFieldName (4, Buffer);
> +    Print (
> +      L"0x%x\n",
> +      *((UINT32*) PrivateResourcePtr)
> +      );
> +
> +    PrivateResourcePtr += sizeof(UINT32);
> +    Index++;
> +  }
> +}
> +
> +/**
> +  This function parses the Cache Type Structure (Type 1).
> +
> +  @param [in] Ptr     Pointer to the start of the Cache Type Structure data.
> +  @param [in] Length  Length of the Cache Type Structure.
> +**/
> +STATIC
> +VOID
> +DumpCacheTypeStructure (
> +  IN UINT8* Ptr,
> +  IN UINT8  Length
> +  )
> +{
> +  ParseAcpi (
> +    TRUE,
> +    2,
> +    "Cache Type Structure",
> +    Ptr,
> +    Length,
> +    PARSER_PARAMS (CacheTypeStructureParser)
> +    );
> +}
> +
> +/**
> +  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
> +  traces the ACPI table fields.
> +
> +  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.
> +
> +  @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
> +ParseAcpiPptt (
> +  IN BOOLEAN Trace,
> +  IN UINT8*  Ptr,
> +  IN UINT32  AcpiTableLength,
> +  IN UINT8   AcpiTableRevision
> +  )
> +{
> +  UINT32 Offset;
> +  UINT8* ProcessorTopologyStructurePtr;
> +
> +  if (!Trace) {
> +    return;
> +  }
> +
> +  Offset = ParseAcpi (
> +             TRUE,
> +             0,
> +             "PPTT",
> +             Ptr,
> +             AcpiTableLength,
> +             PARSER_PARAMS (PpttParser)
> +             );
> +  ProcessorTopologyStructurePtr = Ptr + Offset;
> +
> +  while (Offset < AcpiTableLength) {
> +    // Parse Processor Hierarchy Node Structure to obtain Type and Length.
> +    ParseAcpi (
> +      FALSE,
> +      0,
> +      NULL,
> +      ProcessorTopologyStructurePtr,
> +      4,  // Length of the processor topology structure header is 4 bytes
> +      PARSER_PARAMS (ProcessorTopologyStructureHeaderParser)
> +      );
> +
> +    if ((Offset + (*ProcessorTopologyStructureLength)) > AcpiTableLength) {
> +      IncrementErrorCount ();
> +      Print (
> +        L"ERROR: Invalid processor topology structure length:"
> +          L" Type = %d, Length = %d\n",
> +        *ProcessorTopologyStructureType,
> +        *ProcessorTopologyStructureLength
> +        );
> +      break;
> +    }
> +
> +    PrintFieldName (2, L"* Structure Offset *");
> +    Print (L"0x%x\n", Offset);
> +
> +    switch (*ProcessorTopologyStructureType) {
> +      case EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR:
> +        DumpProcessorHierarchyNodeStructure (
> +          ProcessorTopologyStructurePtr,
> +          *ProcessorTopologyStructureLength
> +          );
> +        break;
> +      case EFI_ACPI_6_2_PPTT_TYPE_CACHE:
> +        DumpCacheTypeStructure (
> +          ProcessorTopologyStructurePtr,
> +          *ProcessorTopologyStructureLength
> +          );
> +        break;
> +      case EFI_ACPI_6_2_PPTT_TYPE_ID:
> +        DumpIDStructure (
> +          ProcessorTopologyStructurePtr,
> +          *ProcessorTopologyStructureLength
> +          );
> +        break;
> +      default:
> +        IncrementErrorCount ();
> +        Print (
> +          L"ERROR: Unknown processor topology structure:"
> +            L" Type = %d, Length = %d\n",
> +          *ProcessorTopologyStructureType,
> +          *ProcessorTopologyStructureLength
> +          );
> +    }
> +
> +    ProcessorTopologyStructurePtr += *ProcessorTopologyStructureLength;
> +    Offset += *ProcessorTopologyStructureLength;
> +  } // while
> +}
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.c
> index
> c6eb7087cfdde2db6a0298b508c751110a061677..2f908bfbd3381f2ca2cfd066dc
> ed1ee22f116b46 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Main file for 'acpiview' Shell command function.
> 
> -  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.<BR>
> +  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.<BR>
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD
> License
>    which accompanies this distribution.  The full text of the license may be
> found at
> @@ -41,6 +41,8 @@ ACPI_TABLE_PARSER ParserList[] = {
>    {EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
> ParseAcpiMadt},
> 
> {EFI_ACPI_6_2_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE
> _BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE,
>     ParseAcpiMcfg},
> +
> {EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_S
> IGNATURE,
> +   ParseAcpiPptt},
>    {RSDP_TABLE_INFO, ParseAcpiRsdp},
>    {EFI_ACPI_6_2_SYSTEM_LOCALITY_INFORMATION_TABLE_SIGNATURE,
> ParseAcpiSlit},
> 
> {EFI_ACPI_6_2_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
> ParseAcpiSpcr},
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.inf
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.inf
> index
> 6df8c08b855241ed4704b606bb1fd61a0a59464a..db06e7a0667c787b183322e87
> 51ab3f923f01ea4 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.inf
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.inf
> @@ -1,7 +1,7 @@
>  ##  @file
>  # Provides Shell 'acpiview' command functions
>  #
> -# Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.<BR>
> +# Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.<BR>
>  #
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the
> BSD License
> @@ -41,6 +41,7 @@ [Sources.common]
>    Parsers/Iort/IortParser.c
>    Parsers/Madt/MadtParser.c
>    Parsers/Mcfg/McfgParser.c
> +  Parsers/Pptt/PpttParser.c
>    Parsers/Rsdp/RsdpParser.c
>    Parsers/Slit/SlitParser.c
>    Parsers/Spcr/SpcrParser.c
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.uni
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.uni
> index
> 1c4d66e1ff507054d4aa42bd47bfae8824c218e6..0762eeba533f01e6e4656084e
> c108dea8bd7c58b 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.uni
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.uni
> @@ -1,6 +1,6 @@
>  // /**
>  //
> -// Copyright (c) 2016 - 2017, ARM Limited. All rights reserved.<BR>
> +// Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.<BR>
>  // This program and the accompanying materials
>  // are licensed and made available under the terms and conditions of the
> BSD License
>  // which accompanies this distribution. The full text of the license may be
> found at
> @@ -82,6 +82,7 @@
>  "       GTDT  - Generic Timer Description Table\r\n"
>  "       IORT  - IO Remapping Table\r\n"
>  "       MCFG  - Memory Mapped Config Space Base Address Description
> Table\r\n"
> +"       PPTT  - Processor Properties Topology Table\r\n"
>  "       RSDP  - Root System Description Pointer\r\n"
>  "       SLIT  - System Locality Information Table\r\n"
>  "       SPCR  - Serial Port Console Redirection Table\r\n"
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'