[edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: Add interface for data-driven table parsing

Krzysztof Koch posted 6 patches 9 weeks ago

[edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: Add interface for data-driven table parsing

Posted by Krzysztof Koch 9 weeks ago
Define and implement an interface to streamline metadata collection and
validation for structures present in each ACPI table.

Most ACPI tables define substructures which constitute the table.
These substructures are identified by their 'Type' field value. The
range of possible 'Type' values is defined on a per-table basis.

For more sophisticated ACPI table validation, additional data about
each structure type needs to be maintained. This patch defines a new
ACPI_STRUCT_INFO structure. It stores additional metadata about a
building block of an ACPI table. ACPI_STRUCT_INFO's are organised into
ACPI_STRUCT_DATABASE's. ACPI_STRUCT_DATABASE is an array of
ACPI_STRUCT_INFO elements which are indexed using structure's type
value.

For example, in the Multiple APIC Description Table (MADT) all
Interrupt Controller Structure types form a single database. In the
database, the GIC CPU Interface (GICC) structure's metadata is the 11th
entry (i.e. Type = 0xB).

ACPI_STRUCT_INFO structure consists of:
- ASCII name of the structure
- ACPI-defined stucture Type
- bitmask defining the validity of the structure for various
  architectures
- instance counter
- a handler for the structure (ACPI_STRUCT_HANDLER)

The bitmask allows detection of structures in a table which
are not compatible with the target platform.

For example, the Multiple APIC Description Table (MADT) contains
Interrupt Controller Structure definitions which apply to either the
Advanced Programmable Interrupt Controller (APIC) model or the Generic
Interrupt Controller (GIC) model. Presence of APIC-related structures
on an Arm-based platform is a bug which is now detected and reported
by acpiview.

This patch adds support for compatibility checks with the Arm
architecture only. However, provisions are made to allow extensions to
other architectures.

ACPI_STRUCT_HANDLER describes how the contents of the structure can
be parsed. The possible options are:
- An ACPI_PARSER array which can be passed to the ParseAcpi() function
- A dedicated function for parsing the structure
  (ACPI_STRUCT_PARSER_FUNC)

If neither of these options is provided, it is assumed that the parsing
logic is not implemented.

ACPI_STRUCT_PARSER_FUNC expects the the first two arguments to be the
pointer to the start of the structure to parse and the length of
structure's buffer. The remaining two optional arguments are context
specific.

This patch adds methods for:
- Resetting the instance count for all structure types in a table.
- Getting the combined instance count for all types in a table.
- Validating the compatibility of a structure with the target arch.
- Printing structure counts for the types which are compatible with
  the target architecture and validating that the non-compatible
  structures are not present in the table.
- Parsing the structure according to the information provided by its
  handle.

Finally, define a helper PrintAcpiStructName () function to streamline
the printing of ACPI structure name together with the structure's
current occurrence count.

References:
- ACPI 6.3, January 2019

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
    v1:
    - Add interface for data-driven table parsing [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 263 ++++++++++++++++++++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h | 234 +++++++++++++++++
 2 files changed, 497 insertions(+)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 3f12a33050a4e4ab3be2187c90ef8dcf0882283d..32566101e2de2eec3ccf44563ee79379404bff62 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -6,6 +6,8 @@
 **/
 
 #include <Uefi.h>
+#include <Library/DebugLib.h>
+#include <Library/PrintLib.h>
 #include <Library/UefiLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include "AcpiParser.h"
@@ -466,6 +468,267 @@ PrintFieldName (
     );
 }
 
+/**
+  Produce a Null-terminated ASCII string with the name and index of an
+  ACPI structure.
+
+  The output string is in the following format: <Name> [<Index>]
+
+  @param [in]  Name           Structure name.
+  @param [in]  Index          Structure index.
+  @param [in]  BufferSize     The size, in bytes, of the output buffer.
+  @param [out] Buffer         Buffer for the output string.
+
+  @return   The number of bytes written to the buffer (not including Null-byte)
+**/
+UINTN
+EFIAPI
+PrintAcpiStructName (
+  IN  CONST CHAR8*  Name,
+  IN        UINT32  Index,
+  IN        UINTN   BufferSize,
+  OUT       CHAR8*  Buffer
+  )
+{
+  ASSERT (Name != NULL);
+  ASSERT (Buffer != NULL);
+
+  return AsciiSPrint (Buffer, BufferSize, "%a [%d]", Name , Index);
+}
+
+/**
+  Set all ACPI structure instance counts to 0.
+
+  @param [in,out] StructDb     ACPI structure database with counts to reset.
+**/
+VOID
+EFIAPI
+ResetAcpiStructCounts (
+  IN OUT ACPI_STRUCT_DATABASE* StructDb
+  )
+{
+  UINT32 Type;
+
+  ASSERT (StructDb != NULL);
+  ASSERT (StructDb->Entries != NULL);
+
+  for (Type = 0; Type < StructDb->EntryCount; Type++) {
+    StructDb->Entries[Type].Count = 0;
+  }
+}
+
+/**
+  Sum all ACPI structure instance counts.
+
+  @param [in] StructDb     ACPI structure database with per-type counts to sum.
+
+  @return   Total number of structure instances recorded in the database.
+**/
+UINT32
+EFIAPI
+SumAcpiStructCounts (
+  IN  CONST ACPI_STRUCT_DATABASE* StructDb
+  )
+{
+  UINT32 Type;
+  UINT32 Total;
+
+  ASSERT (StructDb != NULL);
+  ASSERT (StructDb->Entries != NULL);
+
+  Total = 0;
+
+  for (Type = 0; Type < StructDb->EntryCount; Type++) {
+    Total += StructDb->Entries[Type].Count;
+  }
+
+  return Total;
+}
+
+/**
+  Validate that a structure with a given type value is defined for the given
+  ACPI table and target architecture.
+
+  The target architecture is evaluated from the firmare build parameters.
+
+  @param [in] Type        ACPI-defined structure type.
+  @param [in] StructDb    ACPI structure database with architecture
+                          compatibility info.
+
+  @retval TRUE    Structure is valid.
+  @retval FALSE   Structure is not valid.
+**/
+BOOLEAN
+EFIAPI
+IsAcpiStructTypeValid (
+  IN        UINT32                Type,
+  IN  CONST ACPI_STRUCT_DATABASE* StructDb
+  )
+{
+  UINT32 Compatible;
+
+  ASSERT (StructDb != NULL);
+  ASSERT (StructDb->Entries != NULL);
+
+  if (Type >= StructDb->EntryCount) {
+    return FALSE;
+  }
+
+#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
+  Compatible = StructDb->Entries[Type].CompatArch &
+               (ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64);
+#else
+  Compatible = StructDb->Entries[Type].CompatArch;
+#endif
+
+  return (Compatible != 0);
+}
+
+/**
+  Print the instance count of each structure in an ACPI table that is
+  compatible with the target architecture.
+
+  For structures which are not allowed for the target architecture,
+  validate that their instance counts are 0.
+
+  @param [in] StructDb     ACPI structure database with counts to validate.
+
+  @retval TRUE    All structures are compatible.
+  @retval FALSE   One or more incompatible structures present.
+**/
+BOOLEAN
+EFIAPI
+ValidateAcpiStructCounts (
+  IN  CONST ACPI_STRUCT_DATABASE* StructDb
+  )
+{
+  BOOLEAN   AllValid;
+  UINT32    Type;
+
+  ASSERT (StructDb != NULL);
+  ASSERT (StructDb->Entries != NULL);
+
+  AllValid = TRUE;
+  Print (L"\nTable Breakdown:\n");
+
+  for (Type = 0; Type < StructDb->EntryCount; Type++) {
+    ASSERT (Type == StructDb->Entries[Type].Type);
+
+    if (IsAcpiStructTypeValid (Type, StructDb)) {
+      Print (
+        L"%*a%-*a : %d\n",
+        INSTANCE_COUNT_INDENT,
+        "",
+        OUTPUT_FIELD_COLUMN_WIDTH - INSTANCE_COUNT_INDENT,
+        StructDb->Entries[Type].Name,
+        StructDb->Entries[Type].Count
+        );
+    } else if (StructDb->Entries[Type].Count > 0) {
+      AllValid = FALSE;
+      IncrementErrorCount ();
+      Print (
+        L"ERROR: %a Structure is not valid for the target architecture " \
+          L"(found %d)\n",
+        StructDb->Entries[Type].Name,
+        StructDb->Entries[Type].Count
+        );
+    }
+  }
+
+  return AllValid;
+}
+
+/**
+  Parse the ACPI structure with the type value given according to instructions
+  defined in the ACPI structure database.
+
+  If the input structure type is defined in the database, increment structure's
+  instance count.
+
+  If ACPI_PARSER array is used to parse the input structure, the index of the
+  structure (instance count for the type before update) gets printed alongside
+  the structure name. This helps debugging if there are many instances of the
+  type in a table. For ACPI_STRUCT_PARSER_FUNC, the printing of the index must
+  be implemented separately.
+
+  @param [in]     Indent    Number of spaces to indent the output.
+  @param [in]     Ptr       Ptr to the start of the structure.
+  @param [in,out] StructDb  ACPI structure database with instructions on how
+                            parse every structure type.
+  @param [in]     Offset    Structure offset from the start of the table.
+  @param [in]     Type      ACPI-defined structure type.
+  @param [in]     Length    Length of the structure in bytes.
+  @param [in]     OptArg0   First optional argument to pass to parser function.
+  @param [in]     OptArg1   Second optional argument to pass to parser function.
+
+  @retval TRUE    ACPI structure parsed successfully.
+  @retval FALSE   Undefined structure type or insufficient data to parse.
+**/
+BOOLEAN
+EFIAPI
+ParseAcpiStruct (
+  IN            UINT32                 Indent,
+  IN            UINT8*                 Ptr,
+  IN OUT        ACPI_STRUCT_DATABASE*  StructDb,
+  IN            UINT32                 Offset,
+  IN            UINT32                 Type,
+  IN            UINT32                 Length,
+  IN      CONST VOID*                  OptArg0 OPTIONAL,
+  IN      CONST VOID*                  OptArg1 OPTIONAL
+  )
+{
+  ACPI_STRUCT_PARSER_FUNC ParserFunc;
+  CHAR8                   Buffer[80];
+
+  ASSERT (Ptr != NULL);
+  ASSERT (StructDb != NULL);
+  ASSERT (StructDb->Entries != NULL);
+  ASSERT (StructDb->Name != NULL);
+
+  PrintFieldName (Indent, L"* Offset *");
+  Print (L"0x%x\n", Offset);
+
+  if (Type >= StructDb->EntryCount) {
+    IncrementErrorCount ();
+    Print (L"ERROR: Unknown %a. Type = %d\n", StructDb->Name, Type);
+    return FALSE;
+  }
+
+  if (StructDb->Entries[Type].Handler.ParserFunc != NULL) {
+    ParserFunc = StructDb->Entries[Type].Handler.ParserFunc;
+    ParserFunc (Ptr, Length, OptArg0, OptArg1);
+  } else if (StructDb->Entries[Type].Handler.ParserArray != NULL) {
+    ASSERT (StructDb->Entries[Type].Handler.Elements != 0);
+
+    PrintAcpiStructName (
+      StructDb->Entries[Type].Name,
+      StructDb->Entries[Type].Count,
+      sizeof (Buffer),
+      Buffer
+      );
+
+    ParseAcpi (
+      TRUE,
+      Indent,
+      Buffer,
+      Ptr,
+      Length,
+      StructDb->Entries[Type].Handler.ParserArray,
+      StructDb->Entries[Type].Handler.Elements
+      );
+  } else {
+    StructDb->Entries[Type].Count++;
+    Print (
+      L"ERROR: Parsing of %a Structure is not implemented\n",
+      StructDb->Entries[Type].Name
+      );
+    return FALSE;
+  }
+
+  StructDb->Entries[Type].Count++;
+  return TRUE;
+}
+
 /**
   This function is used to parse an ACPI table buffer.
 
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
index f81ccac7e118378aa185db4b625e5bcd75f78347..70e540b3a76de0ff9ce70bcabed8548063bea0ff 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
@@ -300,6 +300,240 @@ typedef struct AcpiParser {
   VOID*                 Context;
 } ACPI_PARSER;
 
+/**
+  Produce a Null-terminated ASCII string with the name and index of an
+  ACPI structure.
+
+  The output string is in the following format: <Name> [<Index>]
+
+  @param [in]  Name           Structure name.
+  @param [in]  Index          Structure index.
+  @param [in]  BufferSize     The size, in bytes, of the output buffer.
+  @param [out] Buffer         Buffer for the output string.
+
+  @return   The number of bytes written to the buffer (not including Null-byte)
+**/
+UINTN
+EFIAPI
+PrintAcpiStructName (
+  IN  CONST CHAR8*  Name,
+  IN        UINT32  Index,
+  IN        UINTN   BufferSize,
+  OUT       CHAR8*  Buffer
+  );
+
+/**
+  Indentation for printing instance counts for structures in an ACPI table.
+**/
+#define INSTANCE_COUNT_INDENT 2
+
+/**
+  Common signature for functions which parse ACPI structures.
+
+  @param [in] Ptr         Pointer to the start of structure's buffer.
+  @param [in] Length      Length of the buffer.
+  @param [in] OptArg0     First optional argument.
+  @param [in] OptArg1     Second optional argument.
+*/
+typedef VOID (*ACPI_STRUCT_PARSER_FUNC) (
+  IN       UINT8* Ptr,
+  IN       UINT32 Length,
+  IN CONST VOID*  OptArg0 OPTIONAL,
+  IN CONST VOID*  OptArg1 OPTIONAL
+  );
+
+/**
+  Description of how an ACPI structure should be parsed.
+
+  One of ParserFunc or ParserArray should be not NULL. Otherwise, it is
+  assumed that parsing of an ACPI structure is not supported. If both
+  ParserFunc and ParserArray are defined, ParserFunc is used.
+**/
+typedef struct AcpiStructHandler {
+  /// Dedicated function for parsing an ACPI structure
+  ACPI_STRUCT_PARSER_FUNC   ParserFunc;
+  /// Array of instructions on how each structure field should be parsed
+  CONST ACPI_PARSER*        ParserArray;
+  /// Number of elements in ParserArray if ParserArray is defined
+  UINT32                    Elements;
+} ACPI_STRUCT_HANDLER;
+
+/**
+  ACPI structure compatiblity with various architectures.
+
+  Some ACPI tables define structures which are, for example, only valid in
+  the X64 or Arm context. For instance, the Multiple APIC Description Table
+  (MADT) describes both APIC and GIC interrupt models.
+
+  These definitions provide means to describe the belonging of a structure
+  in an ACPI table to a particular architecture. This way, incompatible
+  structures can be detected.
+**/
+#define ARCH_COMPAT_IA32       BIT0
+#define ARCH_COMPAT_X64        BIT1
+#define ARCH_COMPAT_ARM        BIT2
+#define ARCH_COMPAT_AARCH64    BIT3
+#define ARCH_COMPAT_RISCV64    BIT4
+
+/**
+  Information about a structure which constitutes an ACPI table
+**/
+typedef struct AcpiStructInfo {
+  /// ACPI-defined structure Name
+  CONST CHAR8*                Name;
+  /// ACPI-defined structure Type
+  CONST UINT32                Type;
+  /// Architecture(s) for which this structure is valid
+  CONST UINT32                CompatArch;
+  /// Structure's instance count in a table
+  UINT32                      Count;
+  /// Information on how to handle the structure
+  CONST ACPI_STRUCT_HANDLER   Handler;
+} ACPI_STRUCT_INFO;
+
+/**
+  Macro for defining ACPI structure info when an ACPI_PARSER array must
+  be used to parse the structure.
+**/
+#define ADD_ACPI_STRUCT_INFO_ARRAY(Name, Type, Compat, Array)             \
+{                                                                         \
+  Name, Type, Compat, 0, {NULL, Array, ARRAY_SIZE (Array)}                \
+}
+
+/**
+  Macro for defining ACPI structure info when an ACPI_STRUCT_PARSER_FUNC
+  must be used to parse the structure.
+**/
+#define ADD_ACPI_STRUCT_INFO_FUNC(Name, Type, Compat, Func)               \
+{                                                                         \
+  Name, Type, Compat, 0, {Func, NULL, 0}                                  \
+}
+
+/**
+  Macro for defining ACPI structure info when the structure is defined in
+  the ACPI spec but no parsing information is provided.
+**/
+#define ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED(Name, Type, Compat)       \
+{                                                                         \
+  Name, Type, Compat, 0, {NULL, NULL, 0}                                  \
+}
+
+/**
+  Database collating information about every structure type defined by
+  an ACPI table.
+**/
+typedef struct AcpiStructDatabase {
+  /// ACPI-defined name for the structures being described in the database
+  CONST CHAR8*        Name;
+  /// Per-structure-type information. The list must be ordered by the types
+  /// defined for the table. All entries must be unique and there should be
+  /// no gaps.
+  ACPI_STRUCT_INFO*   Entries;
+  /// Total number of unique types defined for the table
+  CONST UINT32        EntryCount;
+} ACPI_STRUCT_DATABASE;
+
+/**
+  Set all ACPI structure instance counts to 0.
+
+  @param [in,out] StructDb     ACPI structure database with counts to reset.
+**/
+VOID
+EFIAPI
+ResetAcpiStructCounts (
+  IN OUT ACPI_STRUCT_DATABASE* StructDb
+  );
+
+/**
+  Sum all ACPI structure instance counts.
+
+  @param [in] StructDb     ACPI structure database with per-type counts to sum.
+
+  @return   Total number of structure instances recorded in the database.
+**/
+UINT32
+EFIAPI
+SumAcpiStructCounts (
+  IN  CONST ACPI_STRUCT_DATABASE* StructDb
+  );
+
+/**
+  Validate that a structure with a given type value is defined for the given
+  ACPI table and target architecture.
+
+  The target architecture is evaluated from the firmare build parameters.
+
+  @param [in] Type        ACPI-defined structure type.
+  @param [in] StructDb    ACPI structure database with architecture
+                          compatibility info.
+
+  @retval TRUE    Structure is valid.
+  @retval FALSE   Structure is not valid.
+**/
+BOOLEAN
+EFIAPI
+IsAcpiStructTypeValid (
+  IN        UINT32                Type,
+  IN  CONST ACPI_STRUCT_DATABASE* StructDb
+  );
+
+/**
+  Print the instance count of each structure in an ACPI table that is
+  compatible with the target architecture.
+
+  For structures which are not allowed for the target architecture,
+  validate that their instance counts are 0.
+
+  @param [in] StructDb     ACPI structure database with counts to validate.
+
+  @retval TRUE    All structures are compatible.
+  @retval FALSE   One or more incompatible structures present.
+**/
+BOOLEAN
+EFIAPI
+ValidateAcpiStructCounts (
+  IN  CONST ACPI_STRUCT_DATABASE* StructDb
+  );
+
+/**
+  Parse the ACPI structure with the type value given according to instructions
+  defined in the ACPI structure database.
+
+  If the input structure type is defined in the database, increment structure's
+  instance count.
+
+  If ACPI_PARSER array is used to parse the input structure, the index of the
+  structure (instance count for the type before update) gets printed alongside
+  the structure name. This helps debugging if there are many instances of the
+  type in a table. For ACPI_STRUCT_PARSER_FUNC, the printing of the index must
+  be implemented separately.
+
+  @param [in]     Indent    Number of spaces to indent the output.
+  @param [in]     Ptr       Ptr to the start of the structure.
+  @param [in,out] StructDb  ACPI structure database with instructions on how
+                            parse every structure type.
+  @param [in]     Offset    Structure offset from the start of the table.
+  @param [in]     Type      ACPI-defined structure type.
+  @param [in]     Length    Length of the structure in bytes.
+  @param [in]     OptArg0   First optional argument to pass to parser function.
+  @param [in]     OptArg1   Second optional argument to pass to parser function.
+
+  @retval TRUE    ACPI structure parsed successfully.
+  @retval FALSE   Undefined structure type or insufficient data to parse.
+**/
+BOOLEAN
+EFIAPI
+ParseAcpiStruct (
+  IN            UINT32                 Indent,
+  IN            UINT8*                 Ptr,
+  IN OUT        ACPI_STRUCT_DATABASE*  StructDb,
+  IN            UINT32                 Offset,
+  IN            UINT32                 Type,
+  IN            UINT32                 Length,
+  IN      CONST VOID*                  OptArg0 OPTIONAL,
+  IN      CONST VOID*                  OptArg1 OPTIONAL
+  );
+
 /**
   A structure used to store the pointers to the members of the
   ACPI description header structure that was parsed.
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#58642): https://edk2.groups.io/g/devel/message/58642
Mute This Topic: https://groups.io/mt/74000905/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: Add interface for data-driven table parsing

Posted by Gao, Zhichao 4 weeks ago
(1) The ASSERT only works with DEBUG build. I think we need add if condition after the ASSERT to return the error code to avoid the null pointer dereference.
(2) It is suggested to use the const (lower-case) instead of CONST. same to static.

Others are fine to me.

Thanks,
Zhichao

> -----Original Message-----
> From: Krzysztof Koch <krzysztof.koch@arm.com>
> Sent: Tuesday, May 5, 2020 11:46 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>;
> Sami.Mujawar@arm.com; Laura.Moretta@arm.comMatteo.Carlini@arm.com;
> nd@arm.com
> Subject: [PATCH v1 1/6] ShellPkg: acpiview: Add interface for data-driven table
> parsing
> 
> Define and implement an interface to streamline metadata collection and
> validation for structures present in each ACPI table.
> 
> Most ACPI tables define substructures which constitute the table.
> These substructures are identified by their 'Type' field value. The range of
> possible 'Type' values is defined on a per-table basis.
> 
> For more sophisticated ACPI table validation, additional data about each
> structure type needs to be maintained. This patch defines a new
> ACPI_STRUCT_INFO structure. It stores additional metadata about a building
> block of an ACPI table. ACPI_STRUCT_INFO's are organised into
> ACPI_STRUCT_DATABASE's. ACPI_STRUCT_DATABASE is an array of
> ACPI_STRUCT_INFO elements which are indexed using structure's type value.
> 
> For example, in the Multiple APIC Description Table (MADT) all Interrupt
> Controller Structure types form a single database. In the database, the GIC CPU
> Interface (GICC) structure's metadata is the 11th entry (i.e. Type = 0xB).
> 
> ACPI_STRUCT_INFO structure consists of:
> - ASCII name of the structure
> - ACPI-defined stucture Type
> - bitmask defining the validity of the structure for various
>   architectures
> - instance counter
> - a handler for the structure (ACPI_STRUCT_HANDLER)
> 
> The bitmask allows detection of structures in a table which are not compatible
> with the target platform.
> 
> For example, the Multiple APIC Description Table (MADT) contains Interrupt
> Controller Structure definitions which apply to either the Advanced
> Programmable Interrupt Controller (APIC) model or the Generic Interrupt
> Controller (GIC) model. Presence of APIC-related structures on an Arm-based
> platform is a bug which is now detected and reported by acpiview.
> 
> This patch adds support for compatibility checks with the Arm architecture only.
> However, provisions are made to allow extensions to other architectures.
> 
> ACPI_STRUCT_HANDLER describes how the contents of the structure can be
> parsed. The possible options are:
> - An ACPI_PARSER array which can be passed to the ParseAcpi() function
> - A dedicated function for parsing the structure
>   (ACPI_STRUCT_PARSER_FUNC)
> 
> If neither of these options is provided, it is assumed that the parsing logic is not
> implemented.
> 
> ACPI_STRUCT_PARSER_FUNC expects the the first two arguments to be the
> pointer to the start of the structure to parse and the length of structure's buffer.
> The remaining two optional arguments are context specific.
> 
> This patch adds methods for:
> - Resetting the instance count for all structure types in a table.
> - Getting the combined instance count for all types in a table.
> - Validating the compatibility of a structure with the target arch.
> - Printing structure counts for the types which are compatible with
>   the target architecture and validating that the non-compatible
>   structures are not present in the table.
> - Parsing the structure according to the information provided by its
>   handle.
> 
> Finally, define a helper PrintAcpiStructName () function to streamline the printing
> of ACPI structure name together with the structure's current occurrence count.
> 
> References:
> - ACPI 6.3, January 2019
> 
> Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
> ---
> 
> Notes:
>     v1:
>     - Add interface for data-driven table parsing [Krzysztof]
> 
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 263
> ++++++++++++++++++++
> ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h | 234
> +++++++++++++++++
>  2 files changed, 497 insertions(+)
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> index
> 3f12a33050a4e4ab3be2187c90ef8dcf0882283d..32566101e2de2eec3ccf44563ee
> 79379404bff62 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> @@ -6,6 +6,8 @@
>  **/
> 
>  #include <Uefi.h>
> +#include <Library/DebugLib.h>
> +#include <Library/PrintLib.h>
>  #include <Library/UefiLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  #include "AcpiParser.h"
> @@ -466,6 +468,267 @@ PrintFieldName (
>      );
>  }
> 
> +/**
> +  Produce a Null-terminated ASCII string with the name and index of an
> +  ACPI structure.
> +
> +  The output string is in the following format: <Name> [<Index>]
> +
> +  @param [in]  Name           Structure name.
> +  @param [in]  Index          Structure index.
> +  @param [in]  BufferSize     The size, in bytes, of the output buffer.
> +  @param [out] Buffer         Buffer for the output string.
> +
> +  @return   The number of bytes written to the buffer (not including Null-byte)
> +**/
> +UINTN
> +EFIAPI
> +PrintAcpiStructName (
> +  IN  CONST CHAR8*  Name,
> +  IN        UINT32  Index,
> +  IN        UINTN   BufferSize,
> +  OUT       CHAR8*  Buffer
> +  )
> +{
> +  ASSERT (Name != NULL);
> +  ASSERT (Buffer != NULL);
> +
> +  return AsciiSPrint (Buffer, BufferSize, "%a [%d]", Name , Index); }
> +
> +/**
> +  Set all ACPI structure instance counts to 0.
> +
> +  @param [in,out] StructDb     ACPI structure database with counts to reset.
> +**/
> +VOID
> +EFIAPI
> +ResetAcpiStructCounts (
> +  IN OUT ACPI_STRUCT_DATABASE* StructDb
> +  )
> +{
> +  UINT32 Type;
> +
> +  ASSERT (StructDb != NULL);
> +  ASSERT (StructDb->Entries != NULL);
> +
> +  for (Type = 0; Type < StructDb->EntryCount; Type++) {
> +    StructDb->Entries[Type].Count = 0;
> +  }
> +}
> +
> +/**
> +  Sum all ACPI structure instance counts.
> +
> +  @param [in] StructDb     ACPI structure database with per-type counts to sum.
> +
> +  @return   Total number of structure instances recorded in the database.
> +**/
> +UINT32
> +EFIAPI
> +SumAcpiStructCounts (
> +  IN  CONST ACPI_STRUCT_DATABASE* StructDb
> +  )
> +{
> +  UINT32 Type;
> +  UINT32 Total;
> +
> +  ASSERT (StructDb != NULL);
> +  ASSERT (StructDb->Entries != NULL);
> +
> +  Total = 0;
> +
> +  for (Type = 0; Type < StructDb->EntryCount; Type++) {
> +    Total += StructDb->Entries[Type].Count;  }
> +
> +  return Total;
> +}
> +
> +/**
> +  Validate that a structure with a given type value is defined for the
> +given
> +  ACPI table and target architecture.
> +
> +  The target architecture is evaluated from the firmare build parameters.
> +
> +  @param [in] Type        ACPI-defined structure type.
> +  @param [in] StructDb    ACPI structure database with architecture
> +                          compatibility info.
> +
> +  @retval TRUE    Structure is valid.
> +  @retval FALSE   Structure is not valid.
> +**/
> +BOOLEAN
> +EFIAPI
> +IsAcpiStructTypeValid (
> +  IN        UINT32                Type,
> +  IN  CONST ACPI_STRUCT_DATABASE* StructDb
> +  )
> +{
> +  UINT32 Compatible;
> +
> +  ASSERT (StructDb != NULL);
> +  ASSERT (StructDb->Entries != NULL);
> +
> +  if (Type >= StructDb->EntryCount) {
> +    return FALSE;
> +  }
> +
> +#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
> +  Compatible = StructDb->Entries[Type].CompatArch &
> +               (ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64); #else
> +  Compatible = StructDb->Entries[Type].CompatArch;
> +#endif
> +
> +  return (Compatible != 0);
> +}
> +
> +/**
> +  Print the instance count of each structure in an ACPI table that is
> +  compatible with the target architecture.
> +
> +  For structures which are not allowed for the target architecture,
> + validate that their instance counts are 0.
> +
> +  @param [in] StructDb     ACPI structure database with counts to validate.
> +
> +  @retval TRUE    All structures are compatible.
> +  @retval FALSE   One or more incompatible structures present.
> +**/
> +BOOLEAN
> +EFIAPI
> +ValidateAcpiStructCounts (
> +  IN  CONST ACPI_STRUCT_DATABASE* StructDb
> +  )
> +{
> +  BOOLEAN   AllValid;
> +  UINT32    Type;
> +
> +  ASSERT (StructDb != NULL);
> +  ASSERT (StructDb->Entries != NULL);
> +
> +  AllValid = TRUE;
> +  Print (L"\nTable Breakdown:\n");
> +
> +  for (Type = 0; Type < StructDb->EntryCount; Type++) {
> +    ASSERT (Type == StructDb->Entries[Type].Type);
> +
> +    if (IsAcpiStructTypeValid (Type, StructDb)) {
> +      Print (
> +        L"%*a%-*a : %d\n",
> +        INSTANCE_COUNT_INDENT,
> +        "",
> +        OUTPUT_FIELD_COLUMN_WIDTH - INSTANCE_COUNT_INDENT,
> +        StructDb->Entries[Type].Name,
> +        StructDb->Entries[Type].Count
> +        );
> +    } else if (StructDb->Entries[Type].Count > 0) {
> +      AllValid = FALSE;
> +      IncrementErrorCount ();
> +      Print (
> +        L"ERROR: %a Structure is not valid for the target architecture " \
> +          L"(found %d)\n",
> +        StructDb->Entries[Type].Name,
> +        StructDb->Entries[Type].Count
> +        );
> +    }
> +  }
> +
> +  return AllValid;
> +}
> +
> +/**
> +  Parse the ACPI structure with the type value given according to
> +instructions
> +  defined in the ACPI structure database.
> +
> +  If the input structure type is defined in the database, increment
> + structure's  instance count.
> +
> +  If ACPI_PARSER array is used to parse the input structure, the index
> + of the  structure (instance count for the type before update) gets
> + printed alongside  the structure name. This helps debugging if there
> + are many instances of the  type in a table. For
> + ACPI_STRUCT_PARSER_FUNC, the printing of the index must  be implemented
> separately.
> +
> +  @param [in]     Indent    Number of spaces to indent the output.
> +  @param [in]     Ptr       Ptr to the start of the structure.
> +  @param [in,out] StructDb  ACPI structure database with instructions on how
> +                            parse every structure type.
> +  @param [in]     Offset    Structure offset from the start of the table.
> +  @param [in]     Type      ACPI-defined structure type.
> +  @param [in]     Length    Length of the structure in bytes.
> +  @param [in]     OptArg0   First optional argument to pass to parser function.
> +  @param [in]     OptArg1   Second optional argument to pass to parser function.
> +
> +  @retval TRUE    ACPI structure parsed successfully.
> +  @retval FALSE   Undefined structure type or insufficient data to parse.
> +**/
> +BOOLEAN
> +EFIAPI
> +ParseAcpiStruct (
> +  IN            UINT32                 Indent,
> +  IN            UINT8*                 Ptr,
> +  IN OUT        ACPI_STRUCT_DATABASE*  StructDb,
> +  IN            UINT32                 Offset,
> +  IN            UINT32                 Type,
> +  IN            UINT32                 Length,
> +  IN      CONST VOID*                  OptArg0 OPTIONAL,
> +  IN      CONST VOID*                  OptArg1 OPTIONAL
> +  )
> +{
> +  ACPI_STRUCT_PARSER_FUNC ParserFunc;
> +  CHAR8                   Buffer[80];
> +
> +  ASSERT (Ptr != NULL);
> +  ASSERT (StructDb != NULL);
> +  ASSERT (StructDb->Entries != NULL);
> +  ASSERT (StructDb->Name != NULL);
> +
> +  PrintFieldName (Indent, L"* Offset *");  Print (L"0x%x\n", Offset);
> +
> +  if (Type >= StructDb->EntryCount) {
> +    IncrementErrorCount ();
> +    Print (L"ERROR: Unknown %a. Type = %d\n", StructDb->Name, Type);
> +    return FALSE;
> +  }
> +
> +  if (StructDb->Entries[Type].Handler.ParserFunc != NULL) {
> +    ParserFunc = StructDb->Entries[Type].Handler.ParserFunc;
> +    ParserFunc (Ptr, Length, OptArg0, OptArg1);  } else if
> + (StructDb->Entries[Type].Handler.ParserArray != NULL) {
> +    ASSERT (StructDb->Entries[Type].Handler.Elements != 0);
> +
> +    PrintAcpiStructName (
> +      StructDb->Entries[Type].Name,
> +      StructDb->Entries[Type].Count,
> +      sizeof (Buffer),
> +      Buffer
> +      );
> +
> +    ParseAcpi (
> +      TRUE,
> +      Indent,
> +      Buffer,
> +      Ptr,
> +      Length,
> +      StructDb->Entries[Type].Handler.ParserArray,
> +      StructDb->Entries[Type].Handler.Elements
> +      );
> +  } else {
> +    StructDb->Entries[Type].Count++;
> +    Print (
> +      L"ERROR: Parsing of %a Structure is not implemented\n",
> +      StructDb->Entries[Type].Name
> +      );
> +    return FALSE;
> +  }
> +
> +  StructDb->Entries[Type].Count++;
> +  return TRUE;
> +}
> +
>  /**
>    This function is used to parse an ACPI table buffer.
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> index
> f81ccac7e118378aa185db4b625e5bcd75f78347..70e540b3a76de0ff9ce70bcabed
> 8548063bea0ff 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> @@ -300,6 +300,240 @@ typedef struct AcpiParser {
>    VOID*                 Context;
>  } ACPI_PARSER;
> 
> +/**
> +  Produce a Null-terminated ASCII string with the name and index of an
> +  ACPI structure.
> +
> +  The output string is in the following format: <Name> [<Index>]
> +
> +  @param [in]  Name           Structure name.
> +  @param [in]  Index          Structure index.
> +  @param [in]  BufferSize     The size, in bytes, of the output buffer.
> +  @param [out] Buffer         Buffer for the output string.
> +
> +  @return   The number of bytes written to the buffer (not including Null-byte)
> +**/
> +UINTN
> +EFIAPI
> +PrintAcpiStructName (
> +  IN  CONST CHAR8*  Name,
> +  IN        UINT32  Index,
> +  IN        UINTN   BufferSize,
> +  OUT       CHAR8*  Buffer
> +  );
> +
> +/**
> +  Indentation for printing instance counts for structures in an ACPI table.
> +**/
> +#define INSTANCE_COUNT_INDENT 2
> +
> +/**
> +  Common signature for functions which parse ACPI structures.
> +
> +  @param [in] Ptr         Pointer to the start of structure's buffer.
> +  @param [in] Length      Length of the buffer.
> +  @param [in] OptArg0     First optional argument.
> +  @param [in] OptArg1     Second optional argument.
> +*/
> +typedef VOID (*ACPI_STRUCT_PARSER_FUNC) (
> +  IN       UINT8* Ptr,
> +  IN       UINT32 Length,
> +  IN CONST VOID*  OptArg0 OPTIONAL,
> +  IN CONST VOID*  OptArg1 OPTIONAL
> +  );
> +
> +/**
> +  Description of how an ACPI structure should be parsed.
> +
> +  One of ParserFunc or ParserArray should be not NULL. Otherwise, it is
> +  assumed that parsing of an ACPI structure is not supported. If both
> +  ParserFunc and ParserArray are defined, ParserFunc is used.
> +**/
> +typedef struct AcpiStructHandler {
> +  /// Dedicated function for parsing an ACPI structure
> +  ACPI_STRUCT_PARSER_FUNC   ParserFunc;
> +  /// Array of instructions on how each structure field should be parsed
> +  CONST ACPI_PARSER*        ParserArray;
> +  /// Number of elements in ParserArray if ParserArray is defined
> +  UINT32                    Elements;
> +} ACPI_STRUCT_HANDLER;
> +
> +/**
> +  ACPI structure compatiblity with various architectures.
> +
> +  Some ACPI tables define structures which are, for example, only valid
> + in  the X64 or Arm context. For instance, the Multiple APIC
> + Description Table
> +  (MADT) describes both APIC and GIC interrupt models.
> +
> +  These definitions provide means to describe the belonging of a
> +structure
> +  in an ACPI table to a particular architecture. This way, incompatible
> +  structures can be detected.
> +**/
> +#define ARCH_COMPAT_IA32       BIT0
> +#define ARCH_COMPAT_X64        BIT1
> +#define ARCH_COMPAT_ARM        BIT2
> +#define ARCH_COMPAT_AARCH64    BIT3
> +#define ARCH_COMPAT_RISCV64    BIT4
> +
> +/**
> +  Information about a structure which constitutes an ACPI table **/
> +typedef struct AcpiStructInfo {
> +  /// ACPI-defined structure Name
> +  CONST CHAR8*                Name;
> +  /// ACPI-defined structure Type
> +  CONST UINT32                Type;
> +  /// Architecture(s) for which this structure is valid
> +  CONST UINT32                CompatArch;
> +  /// Structure's instance count in a table
> +  UINT32                      Count;
> +  /// Information on how to handle the structure
> +  CONST ACPI_STRUCT_HANDLER   Handler;
> +} ACPI_STRUCT_INFO;
> +
> +/**
> +  Macro for defining ACPI structure info when an ACPI_PARSER array must
> +  be used to parse the structure.
> +**/
> +#define ADD_ACPI_STRUCT_INFO_ARRAY(Name, Type, Compat, Array)             \
> +{                                                                         \
> +  Name, Type, Compat, 0, {NULL, Array, ARRAY_SIZE (Array)}                \
> +}
> +
> +/**
> +  Macro for defining ACPI structure info when an
> +ACPI_STRUCT_PARSER_FUNC
> +  must be used to parse the structure.
> +**/
> +#define ADD_ACPI_STRUCT_INFO_FUNC(Name, Type, Compat, Func)               \
> +{                                                                         \
> +  Name, Type, Compat, 0, {Func, NULL, 0}                                  \
> +}
> +
> +/**
> +  Macro for defining ACPI structure info when the structure is defined
> +in
> +  the ACPI spec but no parsing information is provided.
> +**/
> +#define ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED(Name, Type,
> Compat)       \
> +{                                                                         \
> +  Name, Type, Compat, 0, {NULL, NULL, 0}                                  \
> +}
> +
> +/**
> +  Database collating information about every structure type defined by
> +  an ACPI table.
> +**/
> +typedef struct AcpiStructDatabase {
> +  /// ACPI-defined name for the structures being described in the database
> +  CONST CHAR8*        Name;
> +  /// Per-structure-type information. The list must be ordered by the
> +types
> +  /// defined for the table. All entries must be unique and there
> +should be
> +  /// no gaps.
> +  ACPI_STRUCT_INFO*   Entries;
> +  /// Total number of unique types defined for the table
> +  CONST UINT32        EntryCount;
> +} ACPI_STRUCT_DATABASE;
> +
> +/**
> +  Set all ACPI structure instance counts to 0.
> +
> +  @param [in,out] StructDb     ACPI structure database with counts to reset.
> +**/
> +VOID
> +EFIAPI
> +ResetAcpiStructCounts (
> +  IN OUT ACPI_STRUCT_DATABASE* StructDb
> +  );
> +
> +/**
> +  Sum all ACPI structure instance counts.
> +
> +  @param [in] StructDb     ACPI structure database with per-type counts to sum.
> +
> +  @return   Total number of structure instances recorded in the database.
> +**/
> +UINT32
> +EFIAPI
> +SumAcpiStructCounts (
> +  IN  CONST ACPI_STRUCT_DATABASE* StructDb
> +  );
> +
> +/**
> +  Validate that a structure with a given type value is defined for the
> +given
> +  ACPI table and target architecture.
> +
> +  The target architecture is evaluated from the firmare build parameters.
> +
> +  @param [in] Type        ACPI-defined structure type.
> +  @param [in] StructDb    ACPI structure database with architecture
> +                          compatibility info.
> +
> +  @retval TRUE    Structure is valid.
> +  @retval FALSE   Structure is not valid.
> +**/
> +BOOLEAN
> +EFIAPI
> +IsAcpiStructTypeValid (
> +  IN        UINT32                Type,
> +  IN  CONST ACPI_STRUCT_DATABASE* StructDb
> +  );
> +
> +/**
> +  Print the instance count of each structure in an ACPI table that is
> +  compatible with the target architecture.
> +
> +  For structures which are not allowed for the target architecture,
> + validate that their instance counts are 0.
> +
> +  @param [in] StructDb     ACPI structure database with counts to validate.
> +
> +  @retval TRUE    All structures are compatible.
> +  @retval FALSE   One or more incompatible structures present.
> +**/
> +BOOLEAN
> +EFIAPI
> +ValidateAcpiStructCounts (
> +  IN  CONST ACPI_STRUCT_DATABASE* StructDb
> +  );
> +
> +/**
> +  Parse the ACPI structure with the type value given according to
> +instructions
> +  defined in the ACPI structure database.
> +
> +  If the input structure type is defined in the database, increment
> + structure's  instance count.
> +
> +  If ACPI_PARSER array is used to parse the input structure, the index
> + of the  structure (instance count for the type before update) gets
> + printed alongside  the structure name. This helps debugging if there
> + are many instances of the  type in a table. For
> + ACPI_STRUCT_PARSER_FUNC, the printing of the index must  be implemented
> separately.
> +
> +  @param [in]     Indent    Number of spaces to indent the output.
> +  @param [in]     Ptr       Ptr to the start of the structure.
> +  @param [in,out] StructDb  ACPI structure database with instructions on how
> +                            parse every structure type.
> +  @param [in]     Offset    Structure offset from the start of the table.
> +  @param [in]     Type      ACPI-defined structure type.
> +  @param [in]     Length    Length of the structure in bytes.
> +  @param [in]     OptArg0   First optional argument to pass to parser function.
> +  @param [in]     OptArg1   Second optional argument to pass to parser function.
> +
> +  @retval TRUE    ACPI structure parsed successfully.
> +  @retval FALSE   Undefined structure type or insufficient data to parse.
> +**/
> +BOOLEAN
> +EFIAPI
> +ParseAcpiStruct (
> +  IN            UINT32                 Indent,
> +  IN            UINT8*                 Ptr,
> +  IN OUT        ACPI_STRUCT_DATABASE*  StructDb,
> +  IN            UINT32                 Offset,
> +  IN            UINT32                 Type,
> +  IN            UINT32                 Length,
> +  IN      CONST VOID*                  OptArg0 OPTIONAL,
> +  IN      CONST VOID*                  OptArg1 OPTIONAL
> +  );
> +
>  /**
>    A structure used to store the pointers to the members of the
>    ACPI description header structure that was parsed.
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61118): https://edk2.groups.io/g/devel/message/61118
Mute This Topic: https://groups.io/mt/74000905/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: Add interface for data-driven table parsing

Posted by Tomas Pilar (tpilar) 4 weeks ago
Krzysztof has moved on to other pastures, I'll respin the patches accordingly.

Cheers,
Tom

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gao, Zhichao via groups.io
Sent: 11 June 2020 08:43
To: Krzysztof Koch <Krzysztof.Koch@arm.com>; devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Sami Mujawar <Sami.Mujawar@arm.com>; Laura.Moretta@arm.comMatteo.Carlini@arm.com; nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: Add interface for data-driven table parsing

(1) The ASSERT only works with DEBUG build. I think we need add if condition after the ASSERT to return the error code to avoid the null pointer dereference.
(2) It is suggested to use the const (lower-case) instead of CONST. same to static.

Others are fine to me.

Thanks,
Zhichao

> -----Original Message-----
> From: Krzysztof Koch <krzysztof.koch@arm.com>
> Sent: Tuesday, May 5, 2020 11:46 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; 
> Sami.Mujawar@arm.com; Laura.Moretta@arm.comMatteo.Carlini@arm.com;
> nd@arm.com
> Subject: [PATCH v1 1/6] ShellPkg: acpiview: Add interface for 
> data-driven table parsing
> 
> Define and implement an interface to streamline metadata collection 
> and validation for structures present in each ACPI table.
> 
> Most ACPI tables define substructures which constitute the table.
> These substructures are identified by their 'Type' field value. The 
> range of possible 'Type' values is defined on a per-table basis.
> 
> For more sophisticated ACPI table validation, additional data about 
> each structure type needs to be maintained. This patch defines a new 
> ACPI_STRUCT_INFO structure. It stores additional metadata about a 
> building block of an ACPI table. ACPI_STRUCT_INFO's are organised into 
> ACPI_STRUCT_DATABASE's. ACPI_STRUCT_DATABASE is an array of 
> ACPI_STRUCT_INFO elements which are indexed using structure's type value.
> 
> For example, in the Multiple APIC Description Table (MADT) all 
> Interrupt Controller Structure types form a single database. In the 
> database, the GIC CPU Interface (GICC) structure's metadata is the 11th entry (i.e. Type = 0xB).
> 
> ACPI_STRUCT_INFO structure consists of:
> - ASCII name of the structure
> - ACPI-defined stucture Type
> - bitmask defining the validity of the structure for various
>   architectures
> - instance counter
> - a handler for the structure (ACPI_STRUCT_HANDLER)
> 
> The bitmask allows detection of structures in a table which are not 
> compatible with the target platform.
> 
> For example, the Multiple APIC Description Table (MADT) contains 
> Interrupt Controller Structure definitions which apply to either the 
> Advanced Programmable Interrupt Controller (APIC) model or the Generic 
> Interrupt Controller (GIC) model. Presence of APIC-related structures 
> on an Arm-based platform is a bug which is now detected and reported by acpiview.
> 
> This patch adds support for compatibility checks with the Arm architecture only.
> However, provisions are made to allow extensions to other architectures.
> 
> ACPI_STRUCT_HANDLER describes how the contents of the structure can be 
> parsed. The possible options are:
> - An ACPI_PARSER array which can be passed to the ParseAcpi() function
> - A dedicated function for parsing the structure
>   (ACPI_STRUCT_PARSER_FUNC)
> 
> If neither of these options is provided, it is assumed that the 
> parsing logic is not implemented.
> 
> ACPI_STRUCT_PARSER_FUNC expects the the first two arguments to be the 
> pointer to the start of the structure to parse and the length of structure's buffer.
> The remaining two optional arguments are context specific.
> 
> This patch adds methods for:
> - Resetting the instance count for all structure types in a table.
> - Getting the combined instance count for all types in a table.
> - Validating the compatibility of a structure with the target arch.
> - Printing structure counts for the types which are compatible with
>   the target architecture and validating that the non-compatible
>   structures are not present in the table.
> - Parsing the structure according to the information provided by its
>   handle.
> 
> Finally, define a helper PrintAcpiStructName () function to streamline 
> the printing of ACPI structure name together with the structure's current occurrence count.
> 
> References:
> - ACPI 6.3, January 2019
> 
> Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
> ---
> 
> Notes:
>     v1:
>     - Add interface for data-driven table parsing [Krzysztof]
> 
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 263
> ++++++++++++++++++++
> ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h | 234
> +++++++++++++++++
>  2 files changed, 497 insertions(+)
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> index
> 3f12a33050a4e4ab3be2187c90ef8dcf0882283d..32566101e2de2eec3ccf44563ee
> 79379404bff62 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> @@ -6,6 +6,8 @@
>  **/
> 
>  #include <Uefi.h>
> +#include <Library/DebugLib.h>
> +#include <Library/PrintLib.h>
>  #include <Library/UefiLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  #include "AcpiParser.h"
> @@ -466,6 +468,267 @@ PrintFieldName (
>      );
>  }
> 
> +/**
> +  Produce a Null-terminated ASCII string with the name and index of 
> +an
> +  ACPI structure.
> +
> +  The output string is in the following format: <Name> [<Index>]
> +
> +  @param [in]  Name           Structure name.
> +  @param [in]  Index          Structure index.
> +  @param [in]  BufferSize     The size, in bytes, of the output buffer.
> +  @param [out] Buffer         Buffer for the output string.
> +
> +  @return   The number of bytes written to the buffer (not including Null-byte)
> +**/
> +UINTN
> +EFIAPI
> +PrintAcpiStructName (
> +  IN  CONST CHAR8*  Name,
> +  IN        UINT32  Index,
> +  IN        UINTN   BufferSize,
> +  OUT       CHAR8*  Buffer
> +  )
> +{
> +  ASSERT (Name != NULL);
> +  ASSERT (Buffer != NULL);
> +
> +  return AsciiSPrint (Buffer, BufferSize, "%a [%d]", Name , Index); }
> +
> +/**
> +  Set all ACPI structure instance counts to 0.
> +
> +  @param [in,out] StructDb     ACPI structure database with counts to reset.
> +**/
> +VOID
> +EFIAPI
> +ResetAcpiStructCounts (
> +  IN OUT ACPI_STRUCT_DATABASE* StructDb
> +  )
> +{
> +  UINT32 Type;
> +
> +  ASSERT (StructDb != NULL);
> +  ASSERT (StructDb->Entries != NULL);
> +
> +  for (Type = 0; Type < StructDb->EntryCount; Type++) {
> +    StructDb->Entries[Type].Count = 0;
> +  }
> +}
> +
> +/**
> +  Sum all ACPI structure instance counts.
> +
> +  @param [in] StructDb     ACPI structure database with per-type counts to sum.
> +
> +  @return   Total number of structure instances recorded in the database.
> +**/
> +UINT32
> +EFIAPI
> +SumAcpiStructCounts (
> +  IN  CONST ACPI_STRUCT_DATABASE* StructDb
> +  )
> +{
> +  UINT32 Type;
> +  UINT32 Total;
> +
> +  ASSERT (StructDb != NULL);
> +  ASSERT (StructDb->Entries != NULL);
> +
> +  Total = 0;
> +
> +  for (Type = 0; Type < StructDb->EntryCount; Type++) {
> +    Total += StructDb->Entries[Type].Count;  }
> +
> +  return Total;
> +}
> +
> +/**
> +  Validate that a structure with a given type value is defined for 
> +the given
> +  ACPI table and target architecture.
> +
> +  The target architecture is evaluated from the firmare build parameters.
> +
> +  @param [in] Type        ACPI-defined structure type.
> +  @param [in] StructDb    ACPI structure database with architecture
> +                          compatibility info.
> +
> +  @retval TRUE    Structure is valid.
> +  @retval FALSE   Structure is not valid.
> +**/
> +BOOLEAN
> +EFIAPI
> +IsAcpiStructTypeValid (
> +  IN        UINT32                Type,
> +  IN  CONST ACPI_STRUCT_DATABASE* StructDb
> +  )
> +{
> +  UINT32 Compatible;
> +
> +  ASSERT (StructDb != NULL);
> +  ASSERT (StructDb->Entries != NULL);
> +
> +  if (Type >= StructDb->EntryCount) {
> +    return FALSE;
> +  }
> +
> +#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
> +  Compatible = StructDb->Entries[Type].CompatArch &
> +               (ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64); #else
> +  Compatible = StructDb->Entries[Type].CompatArch;
> +#endif
> +
> +  return (Compatible != 0);
> +}
> +
> +/**
> +  Print the instance count of each structure in an ACPI table that is
> +  compatible with the target architecture.
> +
> +  For structures which are not allowed for the target architecture, 
> + validate that their instance counts are 0.
> +
> +  @param [in] StructDb     ACPI structure database with counts to validate.
> +
> +  @retval TRUE    All structures are compatible.
> +  @retval FALSE   One or more incompatible structures present.
> +**/
> +BOOLEAN
> +EFIAPI
> +ValidateAcpiStructCounts (
> +  IN  CONST ACPI_STRUCT_DATABASE* StructDb
> +  )
> +{
> +  BOOLEAN   AllValid;
> +  UINT32    Type;
> +
> +  ASSERT (StructDb != NULL);
> +  ASSERT (StructDb->Entries != NULL);
> +
> +  AllValid = TRUE;
> +  Print (L"\nTable Breakdown:\n");
> +
> +  for (Type = 0; Type < StructDb->EntryCount; Type++) {
> +    ASSERT (Type == StructDb->Entries[Type].Type);
> +
> +    if (IsAcpiStructTypeValid (Type, StructDb)) {
> +      Print (
> +        L"%*a%-*a : %d\n",
> +        INSTANCE_COUNT_INDENT,
> +        "",
> +        OUTPUT_FIELD_COLUMN_WIDTH - INSTANCE_COUNT_INDENT,
> +        StructDb->Entries[Type].Name,
> +        StructDb->Entries[Type].Count
> +        );
> +    } else if (StructDb->Entries[Type].Count > 0) {
> +      AllValid = FALSE;
> +      IncrementErrorCount ();
> +      Print (
> +        L"ERROR: %a Structure is not valid for the target architecture " \
> +          L"(found %d)\n",
> +        StructDb->Entries[Type].Name,
> +        StructDb->Entries[Type].Count
> +        );
> +    }
> +  }
> +
> +  return AllValid;
> +}
> +
> +/**
> +  Parse the ACPI structure with the type value given according to 
> +instructions
> +  defined in the ACPI structure database.
> +
> +  If the input structure type is defined in the database, increment 
> + structure's  instance count.
> +
> +  If ACPI_PARSER array is used to parse the input structure, the 
> + index of the  structure (instance count for the type before update) 
> + gets printed alongside  the structure name. This helps debugging if 
> + there are many instances of the  type in a table. For 
> + ACPI_STRUCT_PARSER_FUNC, the printing of the index must  be 
> + implemented
> separately.
> +
> +  @param [in]     Indent    Number of spaces to indent the output.
> +  @param [in]     Ptr       Ptr to the start of the structure.
> +  @param [in,out] StructDb  ACPI structure database with instructions on how
> +                            parse every structure type.
> +  @param [in]     Offset    Structure offset from the start of the table.
> +  @param [in]     Type      ACPI-defined structure type.
> +  @param [in]     Length    Length of the structure in bytes.
> +  @param [in]     OptArg0   First optional argument to pass to parser function.
> +  @param [in]     OptArg1   Second optional argument to pass to parser function.
> +
> +  @retval TRUE    ACPI structure parsed successfully.
> +  @retval FALSE   Undefined structure type or insufficient data to parse.
> +**/
> +BOOLEAN
> +EFIAPI
> +ParseAcpiStruct (
> +  IN            UINT32                 Indent,
> +  IN            UINT8*                 Ptr,
> +  IN OUT        ACPI_STRUCT_DATABASE*  StructDb,
> +  IN            UINT32                 Offset,
> +  IN            UINT32                 Type,
> +  IN            UINT32                 Length,
> +  IN      CONST VOID*                  OptArg0 OPTIONAL,
> +  IN      CONST VOID*                  OptArg1 OPTIONAL
> +  )
> +{
> +  ACPI_STRUCT_PARSER_FUNC ParserFunc;
> +  CHAR8                   Buffer[80];
> +
> +  ASSERT (Ptr != NULL);
> +  ASSERT (StructDb != NULL);
> +  ASSERT (StructDb->Entries != NULL);  ASSERT (StructDb->Name != 
> + NULL);
> +
> +  PrintFieldName (Indent, L"* Offset *");  Print (L"0x%x\n", Offset);
> +
> +  if (Type >= StructDb->EntryCount) {
> +    IncrementErrorCount ();
> +    Print (L"ERROR: Unknown %a. Type = %d\n", StructDb->Name, Type);
> +    return FALSE;
> +  }
> +
> +  if (StructDb->Entries[Type].Handler.ParserFunc != NULL) {
> +    ParserFunc = StructDb->Entries[Type].Handler.ParserFunc;
> +    ParserFunc (Ptr, Length, OptArg0, OptArg1);  } else if 
> + (StructDb->Entries[Type].Handler.ParserArray != NULL) {
> +    ASSERT (StructDb->Entries[Type].Handler.Elements != 0);
> +
> +    PrintAcpiStructName (
> +      StructDb->Entries[Type].Name,
> +      StructDb->Entries[Type].Count,
> +      sizeof (Buffer),
> +      Buffer
> +      );
> +
> +    ParseAcpi (
> +      TRUE,
> +      Indent,
> +      Buffer,
> +      Ptr,
> +      Length,
> +      StructDb->Entries[Type].Handler.ParserArray,
> +      StructDb->Entries[Type].Handler.Elements
> +      );
> +  } else {
> +    StructDb->Entries[Type].Count++;
> +    Print (
> +      L"ERROR: Parsing of %a Structure is not implemented\n",
> +      StructDb->Entries[Type].Name
> +      );
> +    return FALSE;
> +  }
> +
> +  StructDb->Entries[Type].Count++;
> +  return TRUE;
> +}
> +
>  /**
>    This function is used to parse an ACPI table buffer.
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> index
> f81ccac7e118378aa185db4b625e5bcd75f78347..70e540b3a76de0ff9ce70bcabed
> 8548063bea0ff 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> @@ -300,6 +300,240 @@ typedef struct AcpiParser {
>    VOID*                 Context;
>  } ACPI_PARSER;
> 
> +/**
> +  Produce a Null-terminated ASCII string with the name and index of 
> +an
> +  ACPI structure.
> +
> +  The output string is in the following format: <Name> [<Index>]
> +
> +  @param [in]  Name           Structure name.
> +  @param [in]  Index          Structure index.
> +  @param [in]  BufferSize     The size, in bytes, of the output buffer.
> +  @param [out] Buffer         Buffer for the output string.
> +
> +  @return   The number of bytes written to the buffer (not including Null-byte)
> +**/
> +UINTN
> +EFIAPI
> +PrintAcpiStructName (
> +  IN  CONST CHAR8*  Name,
> +  IN        UINT32  Index,
> +  IN        UINTN   BufferSize,
> +  OUT       CHAR8*  Buffer
> +  );
> +
> +/**
> +  Indentation for printing instance counts for structures in an ACPI table.
> +**/
> +#define INSTANCE_COUNT_INDENT 2
> +
> +/**
> +  Common signature for functions which parse ACPI structures.
> +
> +  @param [in] Ptr         Pointer to the start of structure's buffer.
> +  @param [in] Length      Length of the buffer.
> +  @param [in] OptArg0     First optional argument.
> +  @param [in] OptArg1     Second optional argument.
> +*/
> +typedef VOID (*ACPI_STRUCT_PARSER_FUNC) (
> +  IN       UINT8* Ptr,
> +  IN       UINT32 Length,
> +  IN CONST VOID*  OptArg0 OPTIONAL,
> +  IN CONST VOID*  OptArg1 OPTIONAL
> +  );
> +
> +/**
> +  Description of how an ACPI structure should be parsed.
> +
> +  One of ParserFunc or ParserArray should be not NULL. Otherwise, it 
> +is
> +  assumed that parsing of an ACPI structure is not supported. If both
> +  ParserFunc and ParserArray are defined, ParserFunc is used.
> +**/
> +typedef struct AcpiStructHandler {
> +  /// Dedicated function for parsing an ACPI structure
> +  ACPI_STRUCT_PARSER_FUNC   ParserFunc;
> +  /// Array of instructions on how each structure field should be parsed
> +  CONST ACPI_PARSER*        ParserArray;
> +  /// Number of elements in ParserArray if ParserArray is defined
> +  UINT32                    Elements;
> +} ACPI_STRUCT_HANDLER;
> +
> +/**
> +  ACPI structure compatiblity with various architectures.
> +
> +  Some ACPI tables define structures which are, for example, only 
> + valid in  the X64 or Arm context. For instance, the Multiple APIC 
> + Description Table
> +  (MADT) describes both APIC and GIC interrupt models.
> +
> +  These definitions provide means to describe the belonging of a 
> +structure
> +  in an ACPI table to a particular architecture. This way, 
> +incompatible
> +  structures can be detected.
> +**/
> +#define ARCH_COMPAT_IA32       BIT0
> +#define ARCH_COMPAT_X64        BIT1
> +#define ARCH_COMPAT_ARM        BIT2
> +#define ARCH_COMPAT_AARCH64    BIT3
> +#define ARCH_COMPAT_RISCV64    BIT4
> +
> +/**
> +  Information about a structure which constitutes an ACPI table **/ 
> +typedef struct AcpiStructInfo {
> +  /// ACPI-defined structure Name
> +  CONST CHAR8*                Name;
> +  /// ACPI-defined structure Type
> +  CONST UINT32                Type;
> +  /// Architecture(s) for which this structure is valid
> +  CONST UINT32                CompatArch;
> +  /// Structure's instance count in a table
> +  UINT32                      Count;
> +  /// Information on how to handle the structure
> +  CONST ACPI_STRUCT_HANDLER   Handler;
> +} ACPI_STRUCT_INFO;
> +
> +/**
> +  Macro for defining ACPI structure info when an ACPI_PARSER array 
> +must
> +  be used to parse the structure.
> +**/
> +#define ADD_ACPI_STRUCT_INFO_ARRAY(Name, Type, Compat, Array)             \
> +{                                                                         \
> +  Name, Type, Compat, 0, {NULL, Array, ARRAY_SIZE (Array)}                \
> +}
> +
> +/**
> +  Macro for defining ACPI structure info when an 
> +ACPI_STRUCT_PARSER_FUNC
> +  must be used to parse the structure.
> +**/
> +#define ADD_ACPI_STRUCT_INFO_FUNC(Name, Type, Compat, Func)               \
> +{                                                                         \
> +  Name, Type, Compat, 0, {Func, NULL, 0}                                  \
> +}
> +
> +/**
> +  Macro for defining ACPI structure info when the structure is 
> +defined in
> +  the ACPI spec but no parsing information is provided.
> +**/
> +#define ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED(Name, Type,
> Compat)       \
> +{                                                                         \
> +  Name, Type, Compat, 0, {NULL, NULL, 0}                                  \
> +}
> +
> +/**
> +  Database collating information about every structure type defined 
> +by
> +  an ACPI table.
> +**/
> +typedef struct AcpiStructDatabase {
> +  /// ACPI-defined name for the structures being described in the database
> +  CONST CHAR8*        Name;
> +  /// Per-structure-type information. The list must be ordered by the 
> +types
> +  /// defined for the table. All entries must be unique and there 
> +should be
> +  /// no gaps.
> +  ACPI_STRUCT_INFO*   Entries;
> +  /// Total number of unique types defined for the table
> +  CONST UINT32        EntryCount;
> +} ACPI_STRUCT_DATABASE;
> +
> +/**
> +  Set all ACPI structure instance counts to 0.
> +
> +  @param [in,out] StructDb     ACPI structure database with counts to reset.
> +**/
> +VOID
> +EFIAPI
> +ResetAcpiStructCounts (
> +  IN OUT ACPI_STRUCT_DATABASE* StructDb
> +  );
> +
> +/**
> +  Sum all ACPI structure instance counts.
> +
> +  @param [in] StructDb     ACPI structure database with per-type counts to sum.
> +
> +  @return   Total number of structure instances recorded in the database.
> +**/
> +UINT32
> +EFIAPI
> +SumAcpiStructCounts (
> +  IN  CONST ACPI_STRUCT_DATABASE* StructDb
> +  );
> +
> +/**
> +  Validate that a structure with a given type value is defined for 
> +the given
> +  ACPI table and target architecture.
> +
> +  The target architecture is evaluated from the firmare build parameters.
> +
> +  @param [in] Type        ACPI-defined structure type.
> +  @param [in] StructDb    ACPI structure database with architecture
> +                          compatibility info.
> +
> +  @retval TRUE    Structure is valid.
> +  @retval FALSE   Structure is not valid.
> +**/
> +BOOLEAN
> +EFIAPI
> +IsAcpiStructTypeValid (
> +  IN        UINT32                Type,
> +  IN  CONST ACPI_STRUCT_DATABASE* StructDb
> +  );
> +
> +/**
> +  Print the instance count of each structure in an ACPI table that is
> +  compatible with the target architecture.
> +
> +  For structures which are not allowed for the target architecture, 
> + validate that their instance counts are 0.
> +
> +  @param [in] StructDb     ACPI structure database with counts to validate.
> +
> +  @retval TRUE    All structures are compatible.
> +  @retval FALSE   One or more incompatible structures present.
> +**/
> +BOOLEAN
> +EFIAPI
> +ValidateAcpiStructCounts (
> +  IN  CONST ACPI_STRUCT_DATABASE* StructDb
> +  );
> +
> +/**
> +  Parse the ACPI structure with the type value given according to 
> +instructions
> +  defined in the ACPI structure database.
> +
> +  If the input structure type is defined in the database, increment 
> + structure's  instance count.
> +
> +  If ACPI_PARSER array is used to parse the input structure, the 
> + index of the  structure (instance count for the type before update) 
> + gets printed alongside  the structure name. This helps debugging if 
> + there are many instances of the  type in a table. For 
> + ACPI_STRUCT_PARSER_FUNC, the printing of the index must  be 
> + implemented
> separately.
> +
> +  @param [in]     Indent    Number of spaces to indent the output.
> +  @param [in]     Ptr       Ptr to the start of the structure.
> +  @param [in,out] StructDb  ACPI structure database with instructions on how
> +                            parse every structure type.
> +  @param [in]     Offset    Structure offset from the start of the table.
> +  @param [in]     Type      ACPI-defined structure type.
> +  @param [in]     Length    Length of the structure in bytes.
> +  @param [in]     OptArg0   First optional argument to pass to parser function.
> +  @param [in]     OptArg1   Second optional argument to pass to parser function.
> +
> +  @retval TRUE    ACPI structure parsed successfully.
> +  @retval FALSE   Undefined structure type or insufficient data to parse.
> +**/
> +BOOLEAN
> +EFIAPI
> +ParseAcpiStruct (
> +  IN            UINT32                 Indent,
> +  IN            UINT8*                 Ptr,
> +  IN OUT        ACPI_STRUCT_DATABASE*  StructDb,
> +  IN            UINT32                 Offset,
> +  IN            UINT32                 Type,
> +  IN            UINT32                 Length,
> +  IN      CONST VOID*                  OptArg0 OPTIONAL,
> +  IN      CONST VOID*                  OptArg1 OPTIONAL
> +  );
> +
>  /**
>    A structure used to store the pointers to the members of the
>    ACPI description header structure that was parsed.
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61137): https://edk2.groups.io/g/devel/message/61137
Mute This Topic: https://groups.io/mt/74000905/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-