Re: [edk2-devel] [PATCH] ShellPkg/AcpiView: ERST Parser

["Gao, Zhichao" <zhichao.gao@intel.com>]

Hi,

Sorry, just back from personal affairs. I would review the patch in next week.

Thanks,
Zhichao

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jeshua
> Smith via groups.io
> Sent: Friday, January 6, 2023 12:48 AM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> Subject: Re: [edk2-devel] [PATCH] ShellPkg/AcpiView: ERST Parser
> 
> Resending after the holidays.
> 
> -----Original Message-----
> From: Jeshua Smith
> Sent: Wednesday, December 14, 2022 8:30 AM
> To: devel@edk2.groups.io; Jeshua Smith <jeshuas@nvidia.com>
> Cc: ray.ni@intel.com; zhichao.gao@intel.com
> Subject: RE: [edk2-devel] [PATCH] ShellPkg/AcpiView: ERST Parser
> 
> Is there anything holding this up from getting reviewed?
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jeshua
> Smith via groups.io
> Sent: Wednesday, November 30, 2022 11:31 AM
> To: devel@edk2.groups.io
> Cc: ray.ni@intel.com; zhichao.gao@intel.com; Jeshua Smith
> <jeshuas@nvidia.com>
> Subject: [edk2-devel] [PATCH] ShellPkg/AcpiView: ERST Parser
> 
> External email: Use caution opening links or attachments
> 
> 
> Add a new parser for the Error Record Serialization Table.
> 
> The ERST table describes how an OS can save and retrieve
> 
> hardware error information to and from a persistent store.
> 
> 
> 
> Signed-off-by: Jeshua Smith <jeshuas@nvidia.com>
> 
> ---
> 
>  .../UefiShellAcpiViewCommandLib/AcpiParser.h  |  22 ++
> 
>  .../Parsers/Erst/ErstParser.c                 | 278 ++++++++++++++++++
> 
>  .../UefiShellAcpiViewCommandLib.c             |   2 +
> 
>  .../UefiShellAcpiViewCommandLib.inf           |   2 +
> 
>  4 files changed, 304 insertions(+)
> 
>  create mode 100644
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Erst/ErstParser.c
> 
> 
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> 
> index db8c88f6df..66c992c55c 100644
> 
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> 
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> 
> @@ -1,6 +1,7 @@
> 
>  /** @file
> 
>    Header file for ACPI parser
> 
> 
> 
> +  Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
> 
>    Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.
> 
>    Copyright (c) 2022, AMD Incorporated. All rights reserved.
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -594,6 +595,27 @@ ParseAcpiDsdt (
> 
>    IN UINT8    AcpiTableRevision
> 
>    );
> 
> 
> 
> +/**
> 
> +  This function parses the ACPI ERST table.
> 
> +  When trace is enabled this function parses the ERST 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
> 
> +ParseAcpiErst (
> 
> +  IN BOOLEAN  Trace,
> 
> +  IN UINT8    *Ptr,
> 
> +  IN UINT32   AcpiTableLength,
> 
> +  IN UINT8    AcpiTableRevision
> 
> +  );
> 
> +
> 
>  /**
> 
>    This function parses the ACPI FACS table.
> 
>    When trace is enabled this function parses the FACS table and
> 
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Erst/ErstParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Erst/ErstParser.c
> 
> new file mode 100644
> 
> index 0000000000..e2af0c44b4
> 
> --- /dev/null
> 
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Erst/ErstParser.c
> 
> @@ -0,0 +1,278 @@
> 
> +/** @file
> 
> +  ERST table parser
> 
> +
> 
> +  Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
> 
> +  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
> 
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +
> 
> +  @par Reference(s):
> 
> +    - ACPI 6.5 Specification - August 2022
> 
> +**/
> 
> +
> 
> +#include <IndustryStandard/Acpi.h>
> 
> +#include <Library/UefiLib.h>
> 
> +#include "AcpiParser.h"
> 
> +#include "AcpiTableParser.h"
> 
> +
> 
> +// Local variables
> 
> +STATIC ACPI_DESCRIPTION_HEADER_INFO  AcpiHdrInfo;
> 
> +STATIC UINT32                        *InstructionEntryCount;
> 
> +
> 
> +/**
> 
> +  An array of strings describing the Erst actions
> 
> +**/
> 
> +STATIC CONST CHAR16  *ErstActionTable[] = {
> 
> +  L"BEGIN_WRITE_OPERATION",
> 
> +  L"BEGIN_READ_OPERATION",
> 
> +  L"BEGIN_CLEAR_OPERATION",
> 
> +  L"END_OPERATION",
> 
> +  L"SET_RECORD_OFFSET",
> 
> +  L"EXECUTE_OPERATION",
> 
> +  L"CHECK_BUSY_STATUS",
> 
> +  L"GET_COMMAND_STATUS",
> 
> +  L"GET_RECORD_IDENTIFIER",
> 
> +  L"SET_RECORD_IDENTIFIER",
> 
> +  L"GET_RECORD_COUNT",
> 
> +  L"BEGIN_DUMMY_WRITE_OPERATION",
> 
> +  L"RESERVED",
> 
> +  L"GET_ERROR_LOG_ADDRESS_RANGE",
> 
> +  L"GET_ERROR_LOG_ADDRESS_RANGE_LENGTH",
> 
> +  L"GET_ERROR_LOG_ADDRESS_RANGE_ATTRIBUTES",
> 
> +  L"GET_EXECUTE_OPERATION_TIMINGS"
> 
> +};
> 
> +
> 
> +/**
> 
> +  An array of strings describing the Erst instructions
> 
> +**/
> 
> +STATIC CONST CHAR16  *ErstInstructionTable[] = {
> 
> +  L"READ_REGISTER",
> 
> +  L"READ_REGISTER_VALUE",
> 
> +  L"WRITE_REGISTER",
> 
> +  L"WRITE_REGISTER_VALUE",
> 
> +  L"NOOP",
> 
> +  L"LOAD_VAR1",
> 
> +  L"LOAD_VAR2",
> 
> +  L"STORE_VAR1",
> 
> +  L"ADD",
> 
> +  L"SUBTRACT",
> 
> +  L"ADD_VALUE",
> 
> +  L"SUBTRACT_VALUE",
> 
> +  L"STALL",
> 
> +  L"STALL_WHILE_TRUE",
> 
> +  L"SKIP_NEXT_INSTRUCTION_IF_TRUE",
> 
> +  L"GOTO",
> 
> +  L"SET_SRC_ADDRESS_BASE",
> 
> +  L"SET_DST_ADDRESS_BASE",
> 
> +  L"MOVE_DATA"
> 
> +};
> 
> +
> 
> +/**
> 
> +  Validate Erst action.
> 
> +
> 
> +  @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
> 
> +ValidateErstAction (
> 
> +  IN UINT8  *Ptr,
> 
> +  IN VOID   *Context
> 
> +  )
> 
> +{
> 
> +  if (*Ptr > EFI_ACPI_6_4_ERST_GET_EXECUTE_OPERATION_TIMINGS) {
> 
> +    IncrementErrorCount ();
> 
> +    Print (L"\nError: 0x%02x is not a valid action encoding", *Ptr);
> 
> +  }
> 
> +}
> 
> +
> 
> +/**
> 
> +  Validate Erst instruction.
> 
> +
> 
> +  @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
> 
> +ValidateErstInstruction (
> 
> +  IN UINT8  *Ptr,
> 
> +  IN VOID   *Context
> 
> +  )
> 
> +{
> 
> +  if (*Ptr > EFI_ACPI_6_4_ERST_MOVE_DATA) {
> 
> +    IncrementErrorCount ();
> 
> +    Print (L"\nError: 0x%02x is not a valid instruction encoding", *Ptr);
> 
> +  }
> 
> +}
> 
> +
> 
> +/**
> 
> +  Validate Erst flags.
> 
> +
> 
> +  @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
> 
> +ValidateErstFlags (
> 
> +  IN UINT8  *Ptr,
> 
> +  IN VOID   *Context
> 
> +  )
> 
> +{
> 
> +  if ((*Ptr & 0xfe) != 0) {
> 
> +    IncrementErrorCount ();
> 
> +    Print (L"\nError: Reserved Flag bits not set to 0");
> 
> +  }
> 
> +}
> 
> +
> 
> +/**
> 
> +  Looks up and prints the string corresponding to the index
> 
> +
> 
> +  @param [in] Table      Lookup table
> 
> +  @param [in] Index      Entry to print
> 
> +  @param [in] NumEntries Number of valid entries in the table
> 
> +**/
> 
> +STATIC
> 
> +VOID
> 
> +EFIAPI
> 
> +FormatByte (
> 
> +  IN CONST CHAR16  *Table[],
> 
> +  IN UINT8         Index,
> 
> +  IN UINT8         NumEntries
> 
> +  )
> 
> +{
> 
> +  CONST CHAR16  *String;
> 
> +
> 
> +  if (Index < NumEntries) {
> 
> +    String = Table[Index];
> 
> +  } else {
> 
> +    String = L"**INVALID**";
> 
> +  }
> 
> +
> 
> +  Print (
> 
> +    L"0x%02x (%s)",
> 
> +    Index,
> 
> +    String
> 
> +    );
> 
> +}
> 
> +
> 
> +/**
> 
> +  Prints the Erst Action
> 
> +
> 
> +  @param [in] Format  Optional format string for tracing the data.
> 
> +  @param [in] Ptr     Pointer to the Action byte
> 
> +**/
> 
> +STATIC
> 
> +VOID
> 
> +EFIAPI
> 
> +DumpErstAction (
> 
> +  IN CONST CHAR16  *Format OPTIONAL,
> 
> +  IN UINT8         *Ptr
> 
> +  )
> 
> +{
> 
> +  FormatByte (ErstActionTable, *Ptr, ARRAY_SIZE (ErstActionTable));
> 
> +}
> 
> +
> 
> +/**
> 
> +  Prints the Erst Instruction
> 
> +
> 
> +  @param [in] Format  Optional format string for tracing the data.
> 
> +  @param [in] Ptr     Pointer to the Instruction byte
> 
> +**/
> 
> +STATIC
> 
> +VOID
> 
> +EFIAPI
> 
> +DumpErstInstruction (
> 
> +  IN CONST CHAR16  *Format OPTIONAL,
> 
> +  IN UINT8         *Ptr
> 
> +  )
> 
> +{
> 
> +  FormatByte (ErstInstructionTable, *Ptr, ARRAY_SIZE (ErstInstructionTable));
> 
> +}
> 
> +
> 
> +/**
> 
> +  An ACPI_PARSER array describing the ACPI ERST Table.
> 
> +**/
> 
> +STATIC CONST ACPI_PARSER  ErstParser[] = {
> 
> +  PARSE_ACPI_HEADER (&AcpiHdrInfo),
> 
> +  { L"Serialization Header Size",  4,  36, L"0x%x", NULL, NULL,
> NULL, NULL },
> 
> +  { L"Reserved",                   4,  40, L"0x%x", NULL, NULL,                            NULL,
> NULL },
> 
> +  { L"Instruction Entry Count",    4,  44, L"0x%x", NULL, (VOID
> **)&InstructionEntryCount, NULL, NULL }
> 
> +};
> 
> +
> 
> +/**
> 
> +  An ACPI_PARSER array describing the Serialization Instruction Entry
> structure.
> 
> +**/
> 
> +STATIC CONST ACPI_PARSER  SerializationInstructionEntryParser[] = {
> 
> +  { L"Serialization Action", 1,  0,  L"0x%x",   DumpErstAction,      NULL,
> ValidateErstAction,      NULL },
> 
> +  { L"Instruction",          1,  1,  L"0x%x",   DumpErstInstruction, NULL,
> ValidateErstInstruction, NULL },
> 
> +  { L"Flags",                1,  2,  L"0x%x",   NULL,                NULL, ValidateErstFlags,
> NULL },
> 
> +  { L"Reserved",             1,  3,  L"0x%x",   NULL,                NULL, NULL,
> NULL },
> 
> +  { L"Register Region",      12, 4,  NULL,      DumpGas,             NULL, NULL,
> NULL },
> 
> +  { L"Value",                8,  16, L"0x%llx", NULL,                NULL, NULL,
> NULL },
> 
> +  { L"Mask",                 8,  24, L"0x%llx", NULL,                NULL, NULL,
> NULL }
> 
> +};
> 
> +
> 
> +/**
> 
> +  This function parses the ACPI ERST table.
> 
> +  When trace is enabled this function parses the ERST 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
> 
> +ParseAcpiErst (
> 
> +  IN BOOLEAN  Trace,
> 
> +  IN UINT8    *Ptr,
> 
> +  IN UINT32   AcpiTableLength,
> 
> +  IN UINT8    AcpiTableRevision
> 
> +  )
> 
> +{
> 
> +  UINT32  Offset;
> 
> +
> 
> +  if (!Trace) {
> 
> +    return;
> 
> +  }
> 
> +
> 
> +  Offset = ParseAcpi (
> 
> +             Trace,
> 
> +             0,
> 
> +             "ERST",
> 
> +             Ptr,
> 
> +             AcpiTableLength,
> 
> +             PARSER_PARAMS (ErstParser)
> 
> +             );
> 
> +
> 
> +  if (sizeof
> (EFI_ACPI_6_4_ERST_SERIALIZATION_INSTRUCTION_ENTRY)*(*InstructionE
> ntryCount) != (AcpiTableLength - Offset)) {
> 
> +    IncrementErrorCount ();
> 
> +    Print (
> 
> +      L"ERROR: Invalid InstructionEntryCount. " \
> 
> +      L"Count = %d. Offset = %d. AcpiTableLength = %d.\n",
> 
> +      *InstructionEntryCount,
> 
> +      Offset,
> 
> +      AcpiTableLength
> 
> +      );
> 
> +    return;
> 
> +  }
> 
> +
> 
> +  while (Offset < AcpiTableLength) {
> 
> +    Offset += ParseAcpi (
> 
> +                Trace,
> 
> +                2,
> 
> +                "Serialization Action",
> 
> +                Ptr + Offset,
> 
> +                (AcpiTableLength - Offset),
> 
> +                PARSER_PARAMS (SerializationInstructionEntryParser)
> 
> +                );
> 
> +  }
> 
> +}
> 
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.c
> 
> index 09bdddb56e..d37ad7cacc 100644
> 
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.c
> 
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.c
> 
> @@ -1,6 +1,7 @@
> 
>  /** @file
> 
>    Main file for 'acpiview' Shell command function.
> 
> 
> 
> +  Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
> 
>    Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> 
> @@ -52,6 +53,7 @@ ACPI_TABLE_PARSER  ParserList[] = {
> 
>    { EFI_ACPI_6_2_DEBUG_PORT_2_TABLE_SIGNATURE,
> ParseAcpiDbg2 },
> 
> 
> { EFI_ACPI_6_2_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATUR
> E,
> 
>      ParseAcpiDsdt },
> 
> +  { EFI_ACPI_6_4_ERROR_RECORD_SERIALIZATION_TABLE_SIGNATURE,
> ParseAcpiErst },
> 
>    { EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE,
> ParseAcpiFacs },
> 
>    { EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> ParseAcpiFadt },
> 
>    { EFI_ACPI_6_4_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
> ParseAcpiGtdt },
> 
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.inf
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.inf
> 
> index 63fc5a1281..904fea83de 100644
> 
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.inf
> 
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.inf
> 
> @@ -1,6 +1,7 @@
> 
>  ##  @file
> 
>  # Provides Shell 'acpiview' command functions
> 
>  #
> 
> +# Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
> 
>  # Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.<BR>
> 
>  #
> 
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -31,6 +32,7 @@
> 
>    Parsers/Bgrt/BgrtParser.c
> 
>    Parsers/Dbg2/Dbg2Parser.c
> 
>    Parsers/Dsdt/DsdtParser.c
> 
> +  Parsers/Erst/ErstParser.c
> 
>    Parsers/Facs/FacsParser.c
> 
>    Parsers/Fadt/FadtParser.c
> 
>    Parsers/Gtdt/GtdtParser.c
> 
> --
> 
> 2.25.1
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
>