* [PATCH v1 1/6] ShellPkg: acpiview: Add interface for data-driven table parsing
2020-05-05 15:45 [PATCH v1 0/6] ShellPkg: acpiview: Refactor ACPI table parsing loops Krzysztof Koch
@ 2020-05-05 15:45 ` Krzysztof Koch
2020-06-11 7:42 ` Gao, Zhichao
2020-05-05 15:46 ` [PATCH v1 2/6] ShellPkg: acpiview: Make MADT parsing logic data driven Krzysztof Koch
` (4 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Koch @ 2020-05-05 15:45 UTC (permalink / raw)
To: devel; +Cc: ray.ni, zhichao.gao, Sami.Mujawar, Laura.Moretta
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)'
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v1 1/6] ShellPkg: acpiview: Add interface for data-driven table parsing
2020-05-05 15:45 ` [PATCH v1 1/6] ShellPkg: acpiview: Add interface for data-driven table parsing Krzysztof Koch
@ 2020-06-11 7:42 ` Gao, Zhichao
2020-06-11 11:49 ` [edk2-devel] " Tomas Pilar (tpilar)
0 siblings, 1 reply; 10+ messages in thread
From: Gao, Zhichao @ 2020-06-11 7:42 UTC (permalink / raw)
To: Krzysztof Koch, devel@edk2.groups.io
Cc: Ni, Ray, Sami.Mujawar@arm.com,
Laura.Moretta@arm.comMatteo.Carlini@arm.com
(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)'
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: Add interface for data-driven table parsing
2020-06-11 7:42 ` Gao, Zhichao
@ 2020-06-11 11:49 ` Tomas Pilar (tpilar)
0 siblings, 0 replies; 10+ messages in thread
From: Tomas Pilar (tpilar) @ 2020-06-11 11:49 UTC (permalink / raw)
To: devel@edk2.groups.io, zhichao.gao@intel.com, Krzysztof Koch
Cc: Ni, Ray, Sami Mujawar,
Laura.Moretta@arm.comMatteo.Carlini@arm.com
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)'
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v1 2/6] ShellPkg: acpiview: Make MADT parsing logic data driven
2020-05-05 15:45 [PATCH v1 0/6] ShellPkg: acpiview: Refactor ACPI table parsing loops Krzysztof Koch
2020-05-05 15:45 ` [PATCH v1 1/6] ShellPkg: acpiview: Add interface for data-driven table parsing Krzysztof Koch
@ 2020-05-05 15:46 ` Krzysztof Koch
2020-05-05 15:46 ` [PATCH v1 3/6] ShellPkg: acpiview: Make SRAT " Krzysztof Koch
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Krzysztof Koch @ 2020-05-05 15:46 UTC (permalink / raw)
To: devel; +Cc: ray.ni, zhichao.gao, Sami.Mujawar, Laura.Moretta
Replace the switch statement in the main parser loop with a table-driven
approach. Use the ParseAcpiStruct () method to resolve how each
Interrupt Controller Structure given should be parsed.
Enumerate all structures found in the Multiple APIC Description Table
(MADT) on a per-type basis. Print the offset from the start of the table
for each structure.
Consolidate all metadata about each Interrupt Controller Structure.
Define an array of ACPI_STRUCT_INFO structures to store the name,
instance count, architecture support and handling information. Use this
array to construct the ACPI_STRUCT_DATABASE for MADT.
Count the number of instances of each Interrupt Controller Structure
type. Optionally report these counts after MADT table parsing is
finished. Validate that Advanced Programmable Interrupt Controller
(APIC) structures are not present on Arm-based platforms.
For Arm-based platforms, make existing GIC Distributor (GICD) instance
count validation code use ACPI_STRUCT_INFO.
References:
- ACPI 6.3 Specification - January 2019, Section 5.2.12
Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---
Notes:
v1:
- Make MADT parsing logic data driven [Krzysztof]
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 217 ++++++++++++--------
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h | 3 +-
2 files changed, 134 insertions(+), 86 deletions(-)
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
index f85d2b36532cfc5db36fe7bef9830cccc64969cc..00898a8853f45de1f813d71fe52920185bc92e2a 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
@@ -15,6 +15,7 @@
#include <Library/UefiLib.h>
#include "AcpiParser.h"
#include "AcpiTableParser.h"
+#include "AcpiView.h"
#include "MadtParser.h"
// Local Variables
@@ -200,6 +201,106 @@ STATIC CONST ACPI_PARSER MadtInterruptControllerHeaderParser[] = {
{L"Reserved", 2, 2, NULL, NULL, NULL, NULL, NULL}
};
+/**
+ Information about each Interrupt Controller Structure type.
+**/
+STATIC ACPI_STRUCT_INFO MadtStructs[] = {
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Processor Local APIC",
+ EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "I/O APIC",
+ EFI_ACPI_6_3_IO_APIC,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Interrupt Source Override",
+ EFI_ACPI_6_3_INTERRUPT_SOURCE_OVERRIDE,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "NMI Source",
+ EFI_ACPI_6_3_NON_MASKABLE_INTERRUPT_SOURCE,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Local APIC NMI",
+ EFI_ACPI_6_3_LOCAL_APIC_NMI,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Local APIC Address Override",
+ EFI_ACPI_6_3_LOCAL_APIC_ADDRESS_OVERRIDE,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "I/O SAPIC",
+ EFI_ACPI_6_3_IO_SAPIC,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Local SAPIC",
+ EFI_ACPI_6_3_LOCAL_SAPIC,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Platform Interrupt Sources",
+ EFI_ACPI_6_3_PLATFORM_INTERRUPT_SOURCES,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Processor Local x2APIC",
+ EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Local x2APIC NMI",
+ EFI_ACPI_6_3_LOCAL_X2APIC_NMI,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GICC",
+ EFI_ACPI_6_3_GIC,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ GicCParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GICD",
+ EFI_ACPI_6_3_GICD,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ GicDParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GIC MSI Frame",
+ EFI_ACPI_6_3_GIC_MSI_FRAME,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ GicMSIFrameParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GICR",
+ EFI_ACPI_6_3_GICR,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ GicRParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GIC ITS",
+ EFI_ACPI_6_3_GIC_ITS,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ GicITSParser
+ )
+};
+
+/**
+ MADT structure database
+**/
+STATIC ACPI_STRUCT_DATABASE MadtDatabase = {
+ "Interrupt Controller Structure",
+ MadtStructs,
+ ARRAY_SIZE (MadtStructs)
+};
+
/**
This function parses the ACPI MADT table.
When trace is enabled this function parses the MADT table and
@@ -231,14 +332,13 @@ ParseAcpiMadt (
{
UINT32 Offset;
UINT8* InterruptContollerPtr;
- UINT32 GICDCount;
-
- GICDCount = 0;
if (!Trace) {
return;
}
+ ResetAcpiStructCounts (&MadtDatabase);
+
Offset = ParseAcpi (
TRUE,
0,
@@ -267,7 +367,8 @@ ParseAcpiMadt (
IncrementErrorCount ();
Print (
L"ERROR: Insufficient remaining table buffer length to read the " \
- L"Interrupt Controller Structure header. Length = %d.\n",
+ L"%a header. Length = %d.\n",
+ MadtDatabase.Name,
AcpiTableLength - Offset
);
return;
@@ -278,8 +379,9 @@ ParseAcpiMadt (
((Offset + (*MadtInterruptControllerLength)) > AcpiTableLength)) {
IncrementErrorCount ();
Print (
- L"ERROR: Invalid Interrupt Controller Structure length. " \
- L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
+ L"ERROR: Invalid %a length. Length = %d. Offset = %d. " \
+ "AcpiTableLength = %d.\n",
+ MadtDatabase.Name,
*MadtInterruptControllerLength,
Offset,
AcpiTableLength
@@ -287,87 +389,32 @@ ParseAcpiMadt (
return;
}
- switch (*MadtInterruptControllerType) {
- case EFI_ACPI_6_3_GIC: {
- ParseAcpi (
- TRUE,
- 2,
- "GICC",
- InterruptContollerPtr,
- *MadtInterruptControllerLength,
- PARSER_PARAMS (GicCParser)
- );
- break;
- }
-
- case EFI_ACPI_6_3_GICD: {
- if (++GICDCount > 1) {
- IncrementErrorCount ();
- Print (
- L"ERROR: Only one GICD must be present,"
- L" GICDCount = %d\n",
- GICDCount
- );
- }
- ParseAcpi (
- TRUE,
- 2,
- "GICD",
- InterruptContollerPtr,
- *MadtInterruptControllerLength,
- PARSER_PARAMS (GicDParser)
- );
- break;
- }
-
- case EFI_ACPI_6_3_GIC_MSI_FRAME: {
- ParseAcpi (
- TRUE,
- 2,
- "GIC MSI Frame",
- InterruptContollerPtr,
- *MadtInterruptControllerLength,
- PARSER_PARAMS (GicMSIFrameParser)
- );
- break;
- }
-
- case EFI_ACPI_6_3_GICR: {
- ParseAcpi (
- TRUE,
- 2,
- "GICR",
- InterruptContollerPtr,
- *MadtInterruptControllerLength,
- PARSER_PARAMS (GicRParser)
- );
- break;
- }
-
- case EFI_ACPI_6_3_GIC_ITS: {
- ParseAcpi (
- TRUE,
- 2,
- "GIC ITS",
- InterruptContollerPtr,
- *MadtInterruptControllerLength,
- PARSER_PARAMS (GicITSParser)
- );
- break;
- }
-
- default: {
- IncrementErrorCount ();
- Print (
- L"ERROR: Unknown Interrupt Controller Structure,"
- L" Type = %d, Length = %d\n",
- *MadtInterruptControllerType,
- *MadtInterruptControllerLength
- );
- }
- } // switch
+ // Parse the Interrupt Controller Structure
+ ParseAcpiStruct (
+ 2,
+ InterruptContollerPtr,
+ &MadtDatabase,
+ Offset,
+ *MadtInterruptControllerType,
+ *MadtInterruptControllerLength,
+ NULL,
+ NULL
+ );
InterruptContollerPtr += *MadtInterruptControllerLength;
Offset += *MadtInterruptControllerLength;
} // while
+
+ // Report and validate Interrupt Controller Structure counts
+ if (GetConsistencyChecking ()) {
+ ValidateAcpiStructCounts (&MadtDatabase);
+
+ if (MadtStructs[EFI_ACPI_6_3_GICD].Count > 1) {
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: Only one %a must be present\n",
+ MadtStructs[EFI_ACPI_6_3_GICD].Name
+ );
+ }
+ }
}
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h
index fbbc43e09adbdf9fea302a03a61e6dc179f06a62..25128081816459106e43ef5c98acd23dc1f910c3 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h
@@ -1,13 +1,14 @@
/** @file
Header file for MADT table parser
- Copyright (c) 2019, ARM Limited. All rights reserved.
+ Copyright (c) 2020, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@par Reference(s):
- Arm Generic Interrupt Controller Architecture Specification,
GIC architecture version 3 and version 4, issue E
- Arm Server Base System Architecture 5.0
+ - ACPI 6.3 Specification - January 2019, Section 5.2.12
**/
#ifndef MADT_PARSER_H_
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v1 3/6] ShellPkg: acpiview: Make SRAT parsing logic data driven
2020-05-05 15:45 [PATCH v1 0/6] ShellPkg: acpiview: Refactor ACPI table parsing loops Krzysztof Koch
2020-05-05 15:45 ` [PATCH v1 1/6] ShellPkg: acpiview: Add interface for data-driven table parsing Krzysztof Koch
2020-05-05 15:46 ` [PATCH v1 2/6] ShellPkg: acpiview: Make MADT parsing logic data driven Krzysztof Koch
@ 2020-05-05 15:46 ` Krzysztof Koch
2020-05-05 15:46 ` [PATCH v1 4/6] ShellPkg: acpiview: Make GTDT " Krzysztof Koch
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Krzysztof Koch @ 2020-05-05 15:46 UTC (permalink / raw)
To: devel; +Cc: ray.ni, zhichao.gao, Sami.Mujawar, Laura.Moretta
Replace the switch statement in the main parser loop with a table-driven
approach. Use the ParseAcpiStruct () method to resolve how each
Static Resource Allocation Structure given should be parsed.
Print the offset of each Static Resource Allocation Structure from the
start of the table.
Consolidate all metadata about each Static Resource Allocation
Structure. Define an array of ACPI_STRUCT_INFO structures to store the
name, instance count, architecture support and handling information.
Use this array to construct the ACPI_STRUCT_DATABASE for the System
Resource Affinity Table (SRAT).
Count the number of instances of each Static Resource Allocation
Structure type. Optionally report these counts after SRAT table parsing
is finished and validate that Advanced Programmable Interrupt Controller
(APIC) structures are not present on Arm-based platforms.
References:
- ACPI 6.3 Specification - January 2019, Section 5.2.16
Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---
Notes:
v1:
- Make SRAT parsing logic data driven [Krzysztof]
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c | 204 ++++++++------------
1 file changed, 77 insertions(+), 127 deletions(-)
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
index 6f66be68cc0bed14811a0432c61a79fd47c54890..8ec233a52861b039979bb8291b3c90193acd86fa 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
@@ -13,6 +13,7 @@
#include <Library/UefiLib.h>
#include "AcpiParser.h"
#include "AcpiTableParser.h"
+#include "AcpiView.h"
// Local Variables
STATIC CONST UINT8* SratRAType;
@@ -330,6 +331,57 @@ STATIC CONST ACPI_PARSER SratX2ApciAffinityParser[] = {
{L"Reserved", 4, 20, L"0x%x", NULL, NULL, NULL, NULL}
};
+/**
+ Information about each Static Resource Allocation Structure type.
+**/
+STATIC ACPI_STRUCT_INFO SratStructs[] = {
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "Processor Local APIC/SAPIC Affinity",
+ EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC_SAPIC_AFFINITY,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64,
+ SratApciSapicAffinityParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "Memory Affinity",
+ EFI_ACPI_6_3_MEMORY_AFFINITY,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64 | ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ SratMemAffinityParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "Processor Local x2APIC Affinity",
+ EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_AFFINITY,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64,
+ SratX2ApciAffinityParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GICC Affinity Structure",
+ EFI_ACPI_6_3_GICC_AFFINITY,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ SratGicCAffinityParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GIC ITS Affinity",
+ EFI_ACPI_6_3_GIC_ITS_AFFINITY,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ SratGicITSAffinityParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "Generic Initiator Affinity",
+ EFI_ACPI_6_3_GENERIC_INITIATOR_AFFINITY,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64 | ARCH_COMPAT_ARM |ARCH_COMPAT_AARCH64,
+ SratGenericInitiatorAffinityParser
+ )
+};
+
+/**
+ SRAT structure database
+**/
+STATIC ACPI_STRUCT_DATABASE SratDatabase = {
+ "Static Resource Allocation Structure",
+ SratStructs,
+ ARRAY_SIZE (SratStructs)
+};
+
/**
This function parses the ACPI SRAT table.
When trace is enabled this function parses the SRAT table and
@@ -357,27 +409,15 @@ ParseAcpiSrat (
IN UINT8 AcpiTableRevision
)
{
- UINT32 Offset;
- UINT8* ResourcePtr;
- UINT32 GicCAffinityIndex;
- UINT32 GicITSAffinityIndex;
- UINT32 GenericInitiatorAffinityIndex;
- UINT32 MemoryAffinityIndex;
- UINT32 ApicSapicAffinityIndex;
- UINT32 X2ApicAffinityIndex;
- CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
-
- GicCAffinityIndex = 0;
- GicITSAffinityIndex = 0;
- GenericInitiatorAffinityIndex = 0;
- MemoryAffinityIndex = 0;
- ApicSapicAffinityIndex = 0;
- X2ApicAffinityIndex = 0;
+ UINT32 Offset;
+ UINT8* ResourcePtr;
if (!Trace) {
return;
}
+ ResetAcpiStructCounts (&SratDatabase);
+
Offset = ParseAcpi (
TRUE,
0,
@@ -406,7 +446,8 @@ ParseAcpiSrat (
IncrementErrorCount ();
Print (
L"ERROR: Insufficient remaining table buffer length to read the " \
- L"Static Resource Allocation structure header. Length = %d.\n",
+ L"%a header. Length = %d.\n",
+ SratDatabase.Name,
AcpiTableLength - Offset
);
return;
@@ -417,8 +458,9 @@ ParseAcpiSrat (
((Offset + (*SratRALength)) > AcpiTableLength)) {
IncrementErrorCount ();
Print (
- L"ERROR: Invalid Static Resource Allocation Structure length. " \
- L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
+ L"ERROR: Invalid %a length. Length = %d. Offset = %d. " \
+ L"AcpiTableLength = %d.\n",
+ SratDatabase.Name,
*SratRALength,
Offset,
AcpiTableLength
@@ -426,116 +468,24 @@ ParseAcpiSrat (
return;
}
- switch (*SratRAType) {
- case EFI_ACPI_6_3_GICC_AFFINITY:
- AsciiSPrint (
- Buffer,
- sizeof (Buffer),
- "GICC Affinity Structure [%d]",
- GicCAffinityIndex++
- );
- ParseAcpi (
- TRUE,
- 2,
- Buffer,
- ResourcePtr,
- *SratRALength,
- PARSER_PARAMS (SratGicCAffinityParser)
- );
- break;
-
- case EFI_ACPI_6_3_GIC_ITS_AFFINITY:
- AsciiSPrint (
- Buffer,
- sizeof (Buffer),
- "GIC ITS Affinity Structure [%d]",
- GicITSAffinityIndex++
- );
- ParseAcpi (
- TRUE,
- 2,
- Buffer,
- ResourcePtr,
- *SratRALength,
- PARSER_PARAMS (SratGicITSAffinityParser)
- );
- break;
-
- case EFI_ACPI_6_3_GENERIC_INITIATOR_AFFINITY:
- AsciiSPrint (
- Buffer,
- sizeof (Buffer),
- "Generic Initiator Affinity Structure [%d]",
- GenericInitiatorAffinityIndex++
- );
- ParseAcpi (
- TRUE,
- 2,
- Buffer,
- ResourcePtr,
- *SratRALength,
- PARSER_PARAMS (SratGenericInitiatorAffinityParser)
- );
- break;
-
- case EFI_ACPI_6_3_MEMORY_AFFINITY:
- AsciiSPrint (
- Buffer,
- sizeof (Buffer),
- "Memory Affinity Structure [%d]",
- MemoryAffinityIndex++
- );
- ParseAcpi (
- TRUE,
- 2,
- Buffer,
- ResourcePtr,
- *SratRALength,
- PARSER_PARAMS (SratMemAffinityParser)
- );
- break;
-
- case EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC_SAPIC_AFFINITY:
- AsciiSPrint (
- Buffer,
- sizeof (Buffer),
- "APIC/SAPIC Affinity Structure [%d]",
- ApicSapicAffinityIndex++
- );
- ParseAcpi (
- TRUE,
- 2,
- Buffer,
- ResourcePtr,
- *SratRALength,
- PARSER_PARAMS (SratApciSapicAffinityParser)
- );
- break;
-
- case EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_AFFINITY:
- AsciiSPrint (
- Buffer,
- sizeof (Buffer),
- "X2APIC Affinity Structure [%d]",
- X2ApicAffinityIndex++
- );
- ParseAcpi (
- TRUE,
- 2,
- Buffer,
- ResourcePtr,
- *SratRALength,
- PARSER_PARAMS (SratX2ApciAffinityParser)
- );
- break;
-
- default:
- IncrementErrorCount ();
- Print (L"ERROR: Unknown SRAT Affinity type = 0x%x\n", *SratRAType);
- break;
- }
+ // Parse the Static Resource Allocation Structure
+ ParseAcpiStruct (
+ 2,
+ ResourcePtr,
+ &SratDatabase,
+ Offset,
+ *SratRAType,
+ *SratRALength,
+ NULL,
+ NULL
+ );
ResourcePtr += (*SratRALength);
Offset += (*SratRALength);
}
+
+ // Report and validate Static Resource Allocation Structure counts
+ if (GetConsistencyChecking ()) {
+ ValidateAcpiStructCounts (&SratDatabase);
+ }
}
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v1 4/6] ShellPkg: acpiview: Make GTDT parsing logic data driven
2020-05-05 15:45 [PATCH v1 0/6] ShellPkg: acpiview: Refactor ACPI table parsing loops Krzysztof Koch
` (2 preceding siblings ...)
2020-05-05 15:46 ` [PATCH v1 3/6] ShellPkg: acpiview: Make SRAT " Krzysztof Koch
@ 2020-05-05 15:46 ` Krzysztof Koch
2020-06-11 7:46 ` Gao, Zhichao
2020-05-05 15:46 ` [PATCH v1 5/6] ShellPkg: acpiview: Make IORT " Krzysztof Koch
2020-05-05 15:46 ` [PATCH v1 6/6] ShellPkg: acpiview: Make PPTT " Krzysztof Koch
5 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Koch @ 2020-05-05 15:46 UTC (permalink / raw)
To: devel; +Cc: ray.ni, zhichao.gao, Sami.Mujawar, Laura.Moretta
Replace the switch statement in the main parser loop with a table-driven
approach. Use the ParseAcpiStruct () method to resolve how each
Platform Timer Structure given should be parsed.
Enumerate all structures found in the Generic Timer Description Table
(GTDT) on a per-type basis. Print the offset from the start of the table
for each structure.
Consolidate all metadata about each Platform Timer Structure.
Define an array of ACPI_STRUCT_INFO structures to store the name,
instance count, architecture support and handling information. Use this
array to construct the ACPI_STRUCT_DATABASE for GTDT.
Count the number of instances of each Platform Timer Structure type.
Optionally report these counts after GTDT table parsing is finished.
Modify DumpGTBlock () funtion signature so that it matches that of
ACPI_STRUCT_PARSER_FUNC. This way, the function can be used in the
ParseAcpiStruct () call.
Remove the definition of the DumpWatchdogTimer (). Its only purpose was
to call ParseAcpi () and now this process is streamlined.
References:
- ACPI 6.3 Specification - January 2019, Section 5.2.24
Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---
Notes:
v1:
- Make GTDT parsing logic data driven [Krzysztof]
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | 123 ++++++++++++--------
1 file changed, 77 insertions(+), 46 deletions(-)
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
index bdd30ff45c61142c071ead63a27babab8998721b..9a9f8fda442081507768b1540f0b9b3c6c254329 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
@@ -9,13 +9,20 @@
**/
#include <IndustryStandard/Acpi.h>
+#include <Library/PrintLib.h>
#include <Library/UefiLib.h>
#include "AcpiParser.h"
#include "AcpiTableParser.h"
+#include "AcpiView.h"
// "The number of GT Block Timers must be less than or equal to 8"
#define GT_BLOCK_TIMER_COUNT_MAX 8
+/**
+ Handler for each Platform Timer Structure type
+**/
+STATIC ACPI_STRUCT_INFO GtdtStructs[];
+
// Local variables
STATIC CONST UINT32* GtdtPlatformTimerCount;
STATIC CONST UINT32* GtdtPlatformTimerOffset;
@@ -167,23 +174,35 @@ STATIC CONST ACPI_PARSER SBSAGenericWatchdogParser[] = {
/**
This function parses the Platform GT Block.
- @param [in] Ptr Pointer to the start of the GT Block data.
- @param [in] Length Length of the GT Block structure.
+ @param [in] Ptr Pointer to the start of structure's buffer.
+ @param [in] Length Length of the buffer.
+ @param [in] OptArg0 First optional argument (Not used).
+ @param [in] OptArg1 Second optional argument (Not used).
**/
STATIC
VOID
DumpGTBlock (
- IN UINT8* Ptr,
- IN UINT16 Length
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0 OPTIONAL,
+ IN CONST VOID* OptArg1 OPTIONAL
)
{
UINT32 Index;
UINT32 Offset;
+ CHAR8 Buffer[80];
+
+ PrintAcpiStructName (
+ GtdtStructs[EFI_ACPI_6_3_GTDT_GT_BLOCK].Name,
+ GtdtStructs[EFI_ACPI_6_3_GTDT_GT_BLOCK].Count,
+ sizeof (Buffer),
+ Buffer
+ );
ParseAcpi (
TRUE,
2,
- "GT Block",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (GtBlockParser)
@@ -195,7 +214,8 @@ DumpGTBlock (
(GtBlockTimerOffset == NULL)) {
IncrementErrorCount ();
Print (
- L"ERROR: Insufficient GT Block Structure length. Length = %d.\n",
+ L"ERROR: Insufficient %a Structure length. Length = %d.\n",
+ GtdtStructs[EFI_ACPI_6_3_GTDT_GT_BLOCK].Name,
Length
);
return;
@@ -206,41 +226,47 @@ DumpGTBlock (
// Parse the specified number of GT Block Timer Structures or the GT Block
// Structure buffer length. Whichever is minimum.
- while ((Index++ < *GtBlockTimerCount) &&
+ while ((Index < *GtBlockTimerCount) &&
(Offset < Length)) {
+ PrintAcpiStructName ("GT Block Timer", Index, sizeof (Buffer), Buffer);
Offset += ParseAcpi (
TRUE,
- 2,
- "GT Block Timer",
+ 4,
+ Buffer,
Ptr + Offset,
Length - Offset,
PARSER_PARAMS (GtBlockTimerParser)
);
+ Index++;
}
}
/**
- This function parses the Platform Watchdog timer.
-
- @param [in] Ptr Pointer to the start of the watchdog timer data.
- @param [in] Length Length of the watchdog timer structure.
+ Information about each Platform Timer Structure type.
**/
-STATIC
-VOID
-DumpWatchdogTimer (
- IN UINT8* Ptr,
- IN UINT16 Length
- )
-{
- ParseAcpi (
- TRUE,
- 2,
+STATIC ACPI_STRUCT_INFO GtdtStructs[] = {
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "GT Block",
+ EFI_ACPI_6_3_GTDT_GT_BLOCK,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpGTBlock
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
"SBSA Generic Watchdog",
- Ptr,
- Length,
- PARSER_PARAMS (SBSAGenericWatchdogParser)
- );
-}
+ EFI_ACPI_6_3_GTDT_SBSA_GENERIC_WATCHDOG,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ SBSAGenericWatchdogParser
+ )
+};
+
+/**
+ GTDT structure database
+**/
+STATIC ACPI_STRUCT_DATABASE GtdtDatabase = {
+ "Platform Timer Structure",
+ GtdtStructs,
+ ARRAY_SIZE (GtdtStructs)
+};
/**
This function parses the ACPI GTDT table.
@@ -275,6 +301,8 @@ ParseAcpiGtdt (
return;
}
+ ResetAcpiStructCounts (&GtdtDatabase);
+
ParseAcpi (
TRUE,
0,
@@ -321,7 +349,8 @@ ParseAcpiGtdt (
IncrementErrorCount ();
Print (
L"ERROR: Insufficient remaining table buffer length to read the " \
- L"Platform Timer Structure header. Length = %d.\n",
+ L"%a header. Length = %d.\n",
+ GtdtDatabase.Name,
AcpiTableLength - Offset
);
return;
@@ -332,8 +361,9 @@ ParseAcpiGtdt (
((Offset + (*PlatformTimerLength)) > AcpiTableLength)) {
IncrementErrorCount ();
Print (
- L"ERROR: Invalid Platform Timer Structure length. " \
- L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
+ L"ERROR: Invalid %a length. Length = %d. Offset = %d. " \
+ L"AcpiTableLength = %d.\n",
+ GtdtDatabase.Name,
*PlatformTimerLength,
Offset,
AcpiTableLength
@@ -341,23 +371,24 @@ ParseAcpiGtdt (
return;
}
- switch (*PlatformTimerType) {
- case EFI_ACPI_6_3_GTDT_GT_BLOCK:
- DumpGTBlock (TimerPtr, *PlatformTimerLength);
- break;
- case EFI_ACPI_6_3_GTDT_SBSA_GENERIC_WATCHDOG:
- DumpWatchdogTimer (TimerPtr, *PlatformTimerLength);
- break;
- default:
- IncrementErrorCount ();
- Print (
- L"ERROR: Invalid Platform Timer Type = %d\n",
- *PlatformTimerType
- );
- break;
- } // switch
+ // Parse the Platform Timer Structure
+ ParseAcpiStruct (
+ 2,
+ TimerPtr,
+ &GtdtDatabase,
+ Offset,
+ *PlatformTimerType,
+ *PlatformTimerLength,
+ NULL,
+ NULL
+ );
TimerPtr += *PlatformTimerLength;
Offset += *PlatformTimerLength;
} // while
+
+ // Report and validate Platform Timer Type Structure counts
+ if (GetConsistencyChecking ()) {
+ ValidateAcpiStructCounts (&GtdtDatabase);
+ }
}
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v1 4/6] ShellPkg: acpiview: Make GTDT parsing logic data driven
2020-05-05 15:46 ` [PATCH v1 4/6] ShellPkg: acpiview: Make GTDT " Krzysztof Koch
@ 2020-06-11 7:46 ` Gao, Zhichao
0 siblings, 0 replies; 10+ messages in thread
From: Gao, Zhichao @ 2020-06-11 7:46 UTC (permalink / raw)
To: Krzysztof Koch, devel@edk2.groups.io
Cc: Ni, Ray, Sami.Mujawar@arm.com,
Laura.Moretta@arm.comMatteo.Carlini@arm.com
> -----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 4/6] ShellPkg: acpiview: Make GTDT parsing logic data driven
>
> Replace the switch statement in the main parser loop with a table-driven
> approach. Use the ParseAcpiStruct () method to resolve how each Platform Timer
> Structure given should be parsed.
>
> Enumerate all structures found in the Generic Timer Description Table
> (GTDT) on a per-type basis. Print the offset from the start of the table for each
> structure.
>
> Consolidate all metadata about each Platform Timer Structure.
> Define an array of ACPI_STRUCT_INFO structures to store the name, instance
> count, architecture support and handling information. Use this array to construct
> the ACPI_STRUCT_DATABASE for GTDT.
>
> Count the number of instances of each Platform Timer Structure type.
> Optionally report these counts after GTDT table parsing is finished.
>
> Modify DumpGTBlock () funtion signature so that it matches that of
> ACPI_STRUCT_PARSER_FUNC. This way, the function can be used in the
> ParseAcpiStruct () call.
>
> Remove the definition of the DumpWatchdogTimer (). Its only purpose was to call
> ParseAcpi () and now this process is streamlined.
>
> References:
> - ACPI 6.3 Specification - January 2019, Section 5.2.24
>
> Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
> ---
>
> Notes:
> v1:
> - Make GTDT parsing logic data driven [Krzysztof]
>
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c |
> 123 ++++++++++++--------
> 1 file changed, 77 insertions(+), 46 deletions(-)
>
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> index
> bdd30ff45c61142c071ead63a27babab8998721b..9a9f8fda442081507768b1540f0
> b9b3c6c254329 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtPars
> +++ er.c
> @@ -9,13 +9,20 @@
> **/
>
> #include <IndustryStandard/Acpi.h>
> +#include <Library/PrintLib.h>
> #include <Library/UefiLib.h>
> #include "AcpiParser.h"
> #include "AcpiTableParser.h"
> +#include "AcpiView.h"
>
> // "The number of GT Block Timers must be less than or equal to 8"
> #define GT_BLOCK_TIMER_COUNT_MAX 8
>
> +/**
> + Handler for each Platform Timer Structure type **/ STATIC
> +ACPI_STRUCT_INFO GtdtStructs[];
It is illegal for most of the compilers to declare an array without size. For VS, it would cause a build error. Two method to solve:
1. define the array without handler field initialization. Update the field at the beginning of the parser function
2. put the required function declaration before the array definition.
Same in the patch #5 & #6.
Thanks,
Zhichao
> +
> // Local variables
> STATIC CONST UINT32* GtdtPlatformTimerCount; STATIC CONST UINT32*
> GtdtPlatformTimerOffset; @@ -167,23 +174,35 @@ STATIC CONST
> ACPI_PARSER SBSAGenericWatchdogParser[] = {
> /**
> This function parses the Platform GT Block.
>
> - @param [in] Ptr Pointer to the start of the GT Block data.
> - @param [in] Length Length of the GT Block structure.
> + @param [in] Ptr Pointer to the start of structure's buffer.
> + @param [in] Length Length of the buffer.
> + @param [in] OptArg0 First optional argument (Not used).
> + @param [in] OptArg1 Second optional argument (Not used).
> **/
> STATIC
> VOID
> DumpGTBlock (
> - IN UINT8* Ptr,
> - IN UINT16 Length
> + IN UINT8* Ptr,
> + IN UINT32 Length,
> + IN CONST VOID* OptArg0 OPTIONAL,
> + IN CONST VOID* OptArg1 OPTIONAL
> )
> {
> UINT32 Index;
> UINT32 Offset;
> + CHAR8 Buffer[80];
> +
> + PrintAcpiStructName (
> + GtdtStructs[EFI_ACPI_6_3_GTDT_GT_BLOCK].Name,
> + GtdtStructs[EFI_ACPI_6_3_GTDT_GT_BLOCK].Count,
> + sizeof (Buffer),
> + Buffer
> + );
>
> ParseAcpi (
> TRUE,
> 2,
> - "GT Block",
> + Buffer,
> Ptr,
> Length,
> PARSER_PARAMS (GtBlockParser)
> @@ -195,7 +214,8 @@ DumpGTBlock (
> (GtBlockTimerOffset == NULL)) {
> IncrementErrorCount ();
> Print (
> - L"ERROR: Insufficient GT Block Structure length. Length = %d.\n",
> + L"ERROR: Insufficient %a Structure length. Length = %d.\n",
> + GtdtStructs[EFI_ACPI_6_3_GTDT_GT_BLOCK].Name,
> Length
> );
> return;
> @@ -206,41 +226,47 @@ DumpGTBlock (
>
> // Parse the specified number of GT Block Timer Structures or the GT Block
> // Structure buffer length. Whichever is minimum.
> - while ((Index++ < *GtBlockTimerCount) &&
> + while ((Index < *GtBlockTimerCount) &&
> (Offset < Length)) {
> + PrintAcpiStructName ("GT Block Timer", Index, sizeof (Buffer),
> + Buffer);
> Offset += ParseAcpi (
> TRUE,
> - 2,
> - "GT Block Timer",
> + 4,
> + Buffer,
> Ptr + Offset,
> Length - Offset,
> PARSER_PARAMS (GtBlockTimerParser)
> );
> + Index++;
> }
> }
>
> /**
> - This function parses the Platform Watchdog timer.
> -
> - @param [in] Ptr Pointer to the start of the watchdog timer data.
> - @param [in] Length Length of the watchdog timer structure.
> + Information about each Platform Timer Structure type.
> **/
> -STATIC
> -VOID
> -DumpWatchdogTimer (
> - IN UINT8* Ptr,
> - IN UINT16 Length
> - )
> -{
> - ParseAcpi (
> - TRUE,
> - 2,
> +STATIC ACPI_STRUCT_INFO GtdtStructs[] = {
> + ADD_ACPI_STRUCT_INFO_FUNC (
> + "GT Block",
> + EFI_ACPI_6_3_GTDT_GT_BLOCK,
> + ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
> + DumpGTBlock
> + ),
> + ADD_ACPI_STRUCT_INFO_ARRAY (
> "SBSA Generic Watchdog",
> - Ptr,
> - Length,
> - PARSER_PARAMS (SBSAGenericWatchdogParser)
> - );
> -}
> + EFI_ACPI_6_3_GTDT_SBSA_GENERIC_WATCHDOG,
> + ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
> + SBSAGenericWatchdogParser
> + )
> +};
> +
> +/**
> + GTDT structure database
> +**/
> +STATIC ACPI_STRUCT_DATABASE GtdtDatabase = {
> + "Platform Timer Structure",
> + GtdtStructs,
> + ARRAY_SIZE (GtdtStructs)
> +};
>
> /**
> This function parses the ACPI GTDT table.
> @@ -275,6 +301,8 @@ ParseAcpiGtdt (
> return;
> }
>
> + ResetAcpiStructCounts (&GtdtDatabase);
> +
> ParseAcpi (
> TRUE,
> 0,
> @@ -321,7 +349,8 @@ ParseAcpiGtdt (
> IncrementErrorCount ();
> Print (
> L"ERROR: Insufficient remaining table buffer length to read the " \
> - L"Platform Timer Structure header. Length = %d.\n",
> + L"%a header. Length = %d.\n",
> + GtdtDatabase.Name,
> AcpiTableLength - Offset
> );
> return;
> @@ -332,8 +361,9 @@ ParseAcpiGtdt (
> ((Offset + (*PlatformTimerLength)) > AcpiTableLength)) {
> IncrementErrorCount ();
> Print (
> - L"ERROR: Invalid Platform Timer Structure length. " \
> - L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
> + L"ERROR: Invalid %a length. Length = %d. Offset = %d. " \
> + L"AcpiTableLength = %d.\n",
> + GtdtDatabase.Name,
> *PlatformTimerLength,
> Offset,
> AcpiTableLength
> @@ -341,23 +371,24 @@ ParseAcpiGtdt (
> return;
> }
>
> - switch (*PlatformTimerType) {
> - case EFI_ACPI_6_3_GTDT_GT_BLOCK:
> - DumpGTBlock (TimerPtr, *PlatformTimerLength);
> - break;
> - case EFI_ACPI_6_3_GTDT_SBSA_GENERIC_WATCHDOG:
> - DumpWatchdogTimer (TimerPtr, *PlatformTimerLength);
> - break;
> - default:
> - IncrementErrorCount ();
> - Print (
> - L"ERROR: Invalid Platform Timer Type = %d\n",
> - *PlatformTimerType
> - );
> - break;
> - } // switch
> + // Parse the Platform Timer Structure
> + ParseAcpiStruct (
> + 2,
> + TimerPtr,
> + &GtdtDatabase,
> + Offset,
> + *PlatformTimerType,
> + *PlatformTimerLength,
> + NULL,
> + NULL
> + );
>
> TimerPtr += *PlatformTimerLength;
> Offset += *PlatformTimerLength;
> } // while
> +
> + // Report and validate Platform Timer Type Structure counts if
> + (GetConsistencyChecking ()) {
> + ValidateAcpiStructCounts (&GtdtDatabase); }
> }
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v1 5/6] ShellPkg: acpiview: Make IORT parsing logic data driven
2020-05-05 15:45 [PATCH v1 0/6] ShellPkg: acpiview: Refactor ACPI table parsing loops Krzysztof Koch
` (3 preceding siblings ...)
2020-05-05 15:46 ` [PATCH v1 4/6] ShellPkg: acpiview: Make GTDT " Krzysztof Koch
@ 2020-05-05 15:46 ` Krzysztof Koch
2020-05-05 15:46 ` [PATCH v1 6/6] ShellPkg: acpiview: Make PPTT " Krzysztof Koch
5 siblings, 0 replies; 10+ messages in thread
From: Krzysztof Koch @ 2020-05-05 15:46 UTC (permalink / raw)
To: devel; +Cc: ray.ni, zhichao.gao, Sami.Mujawar, Laura.Moretta
Replace the switch statement in the main parser loop with a table-driven
approach. Use the ParseAcpiStruct () method to resolve how each IORT
Node given should be parsed.
Enumerate all structures found in the IO Remapping Table (IORT) on a
per-type basis. Replace calls to AsciiSPrint () with the
PrintAcpiStructName () function.
Consolidate all metadata about each IORT Node. Define an array of
ACPI_STRUCT_INFO structures to store the name, instance count,
architecture support and handling information. Use this array to
construct the ACPI_STRUCT_DATABASE for IORT.
Count the number of instances of each IORT Node. Optionally report
these counts after IORT table parsing is finished.
Modify dedicated functions for parsing each IORT Node type such that
their signatures match ACPI_STRUCT_PARSER_FUNC. This way, they can be
used in the ParseAcpiStruct () call.
References:
- IO Remapping Table - Platform Design Document, Issue D, March 2018
Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---
Notes:
v1:
- Make IORT parsing logic data driven [Krzysztof]
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c | 353 +++++++++++++-------
1 file changed, 234 insertions(+), 119 deletions(-)
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
index 9a006a01448b897865cd7cd85651c816933acf05..0c40447b4363f10c7ea5c9eeb283cebeab24243a 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
@@ -9,10 +9,12 @@
**/
#include <IndustryStandard/IoRemappingTable.h>
+#include <Library/DebugLib.h>
#include <Library/PrintLib.h>
#include <Library/UefiLib.h>
#include "AcpiParser.h"
#include "AcpiTableParser.h"
+#include "AcpiView.h"
// Local variables
STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
@@ -32,6 +34,11 @@ STATIC CONST UINT32* PmuInterruptOffset;
STATIC CONST UINT32* ItsCount;
+/**
+ Handler for each IORT Node type
+**/
+STATIC ACPI_STRUCT_INFO IortStructs[];
+
/**
This function validates the ID Mapping array count for the ITS node.
@@ -273,12 +280,7 @@ DumpIortNodeIdMappings (
while ((Index < MappingCount) &&
(Offset < Length)) {
- AsciiSPrint (
- Buffer,
- sizeof (Buffer),
- "ID Mapping [%d]",
- Index
- );
+ PrintAcpiStructName ("ID Mapping", Index, sizeof (Buffer), Buffer);
Offset += ParseAcpi (
TRUE,
4,
@@ -296,27 +298,42 @@ DumpIortNodeIdMappings (
@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
- @param [in] MappingCount The ID Mapping count.
- @param [in] MappingOffset The offset of the ID Mapping array
+ @param [in] OptArg0 The ID Mapping count.
+ @param [in] OptArg1 The offset of the ID Mapping array
from the start of the IORT table.
**/
STATIC
VOID
DumpIortNodeSmmuV1V2 (
- IN UINT8* Ptr,
- IN UINT16 Length,
- IN UINT32 MappingCount,
- IN UINT32 MappingOffset
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0,
+ IN CONST VOID* OptArg1
)
{
UINT32 Index;
UINT32 Offset;
- CHAR8 Buffer[50]; // Used for AsciiName param of ParseAcpi
+ CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
+ UINT32 MappingCount;
+ UINT32 MappingOffset;
+
+ ASSERT (OptArg0 != NULL);
+ ASSERT (OptArg1 != NULL);
+
+ MappingCount = *((UINT32*)OptArg0);
+ MappingOffset = *((UINT32*)OptArg1);
+
+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_SMMUv1v2].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_SMMUv1v2].Count,
+ sizeof (Buffer),
+ Buffer
+ );
ParseAcpi (
TRUE,
2,
- "SMMUv1 or SMMUv2 Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodeSmmuV1V2Parser)
@@ -330,7 +347,8 @@ DumpIortNodeSmmuV1V2 (
(PmuInterruptOffset == NULL)) {
IncrementErrorCount ();
Print (
- L"ERROR: Insufficient SMMUv1/2 node length. Length = %d\n",
+ L"ERROR: Insufficient %a Node length. Length = %d\n",
+ IortStructs[EFI_ACPI_IORT_TYPE_SMMUv1v2].Name,
Length
);
return;
@@ -341,11 +359,11 @@ DumpIortNodeSmmuV1V2 (
while ((Index < *InterruptContextCount) &&
(Offset < Length)) {
- AsciiSPrint (
- Buffer,
+ PrintAcpiStructName (
+ "Context Interrupts Array",
+ Index,
sizeof (Buffer),
- "Context Interrupts Array [%d]",
- Index
+ Buffer
);
Offset += ParseAcpi (
TRUE,
@@ -363,11 +381,11 @@ DumpIortNodeSmmuV1V2 (
while ((Index < *PmuInterruptCount) &&
(Offset < Length)) {
- AsciiSPrint (
- Buffer,
+ PrintAcpiStructName (
+ "PMU Interrupts Array",
+ Index,
sizeof (Buffer),
- "PMU Interrupts Array [%d]",
- Index
+ Buffer
);
Offset += ParseAcpi (
TRUE,
@@ -392,23 +410,40 @@ DumpIortNodeSmmuV1V2 (
@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
- @param [in] MappingCount The ID Mapping count.
- @param [in] MappingOffset The offset of the ID Mapping array
+ @param [in] OptArg0 The ID Mapping count.
+ @param [in] OptArg1 The offset of the ID Mapping array
from the start of the IORT table.
**/
STATIC
VOID
DumpIortNodeSmmuV3 (
- IN UINT8* Ptr,
- IN UINT16 Length,
- IN UINT32 MappingCount,
- IN UINT32 MappingOffset
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0,
+ IN CONST VOID* OptArg1
)
{
+ UINT32 MappingCount;
+ UINT32 MappingOffset;
+ CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
+
+ ASSERT (OptArg0 != NULL);
+ ASSERT (OptArg1 != NULL);
+
+ MappingCount = *((UINT32*)OptArg0);
+ MappingOffset = *((UINT32*)OptArg1);
+
+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_SMMUv3].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_SMMUv3].Count,
+ sizeof (Buffer),
+ Buffer
+ );
+
ParseAcpi (
TRUE,
2,
- "SMMUV3 Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodeSmmuV3Parser)
@@ -424,24 +459,37 @@ DumpIortNodeSmmuV3 (
/**
This function parses the IORT ITS node.
+ ITS nodes have no ID mappings.
+
@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
+ @param [in] OptArg0 First optional argument (Not used).
+ @param [in] OptArg1 Second optional argument (Not used)..
**/
STATIC
VOID
DumpIortNodeIts (
- IN UINT8* Ptr,
- IN UINT16 Length
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0 OPTIONAL,
+ IN CONST VOID* OptArg1 OPTIONAL
)
{
UINT32 Offset;
UINT32 Index;
CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_ITS_GROUP].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_ITS_GROUP].Count,
+ sizeof (Buffer),
+ Buffer
+ );
+
Offset = ParseAcpi (
TRUE,
2,
- "ITS Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodeItsParser)
@@ -452,7 +500,8 @@ DumpIortNodeIts (
if (ItsCount == NULL) {
IncrementErrorCount ();
Print (
- L"ERROR: Insufficient ITS group length. Length = %d.\n",
+ L"ERROR: Insufficient %a Node length. Length = %d.\n",
+ IortStructs[EFI_ACPI_IORT_TYPE_ITS_GROUP].Name,
Length
);
return;
@@ -462,11 +511,11 @@ DumpIortNodeIts (
while ((Index < *ItsCount) &&
(Offset < Length)) {
- AsciiSPrint (
- Buffer,
+ PrintAcpiStructName (
+ "GIC ITS Identifier Array",
+ Index,
sizeof (Buffer),
- "GIC ITS Identifier Array [%d]",
- Index
+ Buffer
);
Offset += ParseAcpi (
TRUE,
@@ -488,25 +537,41 @@ DumpIortNodeIts (
@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
- @param [in] MappingCount The ID Mapping count.
- @param [in] MappingOffset The offset of the ID Mapping array
+ @param [in] OptArg0 The ID Mapping count.
+ @param [in] OptArg1 The offset of the ID Mapping array
from the start of the IORT table.
**/
STATIC
VOID
DumpIortNodeNamedComponent (
- IN UINT8* Ptr,
- IN UINT16 Length,
- IN UINT32 MappingCount,
- IN UINT32 MappingOffset
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0,
+ IN CONST VOID* OptArg1
)
{
UINT32 Offset;
+ UINT32 MappingCount;
+ UINT32 MappingOffset;
+ CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
+
+ ASSERT (OptArg0 != NULL);
+ ASSERT (OptArg1 != NULL);
+
+ MappingCount = *((UINT32*)OptArg0);
+ MappingOffset = *((UINT32*)OptArg1);
+
+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_NAMED_COMP].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_NAMED_COMP].Count,
+ sizeof (Buffer),
+ Buffer
+ );
Offset = ParseAcpi (
TRUE,
2,
- "Named Component Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodeNamedComponentParser)
@@ -534,23 +599,40 @@ DumpIortNodeNamedComponent (
@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
- @param [in] MappingCount The ID Mapping count.
- @param [in] MappingOffset The offset of the ID Mapping array
+ @param [in] OptArg0 The ID Mapping count.
+ @param [in] OptArg1 The offset of the ID Mapping array
from the start of the IORT table.
**/
STATIC
VOID
DumpIortNodeRootComplex (
- IN UINT8* Ptr,
- IN UINT16 Length,
- IN UINT32 MappingCount,
- IN UINT32 MappingOffset
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0,
+ IN CONST VOID* OptArg1
)
{
+ UINT32 MappingCount;
+ UINT32 MappingOffset;
+ CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
+
+ ASSERT (OptArg0 != NULL);
+ ASSERT (OptArg1 != NULL);
+
+ MappingCount = *((UINT32*)OptArg0);
+ MappingOffset = *((UINT32*)OptArg1);
+
+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_ROOT_COMPLEX].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_ROOT_COMPLEX].Count,
+ sizeof (Buffer),
+ Buffer
+ );
+
ParseAcpi (
TRUE,
2,
- "Root Complex Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodeRootComplexParser)
@@ -568,23 +650,40 @@ DumpIortNodeRootComplex (
@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
- @param [in] MappingCount The ID Mapping count.
- @param [in] MappingOffset The offset of the ID Mapping array
+ @param [in] OptArg0 The ID Mapping count.
+ @param [in] OptArg1 The offset of the ID Mapping array
from the start of the IORT table.
**/
STATIC
VOID
DumpIortNodePmcg (
- IN UINT8* Ptr,
- IN UINT16 Length,
- IN UINT32 MappingCount,
- IN UINT32 MappingOffset
-)
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0,
+ IN CONST VOID* OptArg1
+ )
{
+ UINT32 MappingCount;
+ UINT32 MappingOffset;
+ CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
+
+ ASSERT (OptArg0 != NULL);
+ ASSERT (OptArg1 != NULL);
+
+ MappingCount = *((UINT32*)OptArg0);
+ MappingOffset = *((UINT32*)OptArg1);
+
+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_PMCG].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_PMCG].Count,
+ sizeof (Buffer),
+ Buffer
+ );
+
ParseAcpi (
TRUE,
2,
- "PMCG Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodePmcgParser)
@@ -597,6 +696,57 @@ DumpIortNodePmcg (
);
}
+/**
+ Information about each IORT Node type
+**/
+STATIC ACPI_STRUCT_INFO IortStructs[] = {
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "ITS Group",
+ EFI_ACPI_IORT_TYPE_ITS_GROUP,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodeIts
+ ),
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "Named Component",
+ EFI_ACPI_IORT_TYPE_NAMED_COMP,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodeNamedComponent
+ ),
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "Root Complex",
+ EFI_ACPI_IORT_TYPE_ROOT_COMPLEX,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodeRootComplex
+ ),
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "SMMUv1 or SMMUv2",
+ EFI_ACPI_IORT_TYPE_SMMUv1v2,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodeSmmuV1V2
+ ),
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "SMMUv3",
+ EFI_ACPI_IORT_TYPE_SMMUv3,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodeSmmuV3
+ ),
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "PMCG",
+ EFI_ACPI_IORT_TYPE_PMCG,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodePmcg
+ )
+};
+
+/**
+ IORT structure database
+**/
+STATIC ACPI_STRUCT_DATABASE IortDatabase = {
+ "IORT Node",
+ IortStructs,
+ ARRAY_SIZE (IortStructs)
+};
+
/**
This function parses the ACPI IORT table.
When trace is enabled this function parses the IORT table and traces the ACPI fields.
@@ -633,6 +783,8 @@ ParseAcpiIort (
return;
}
+ ResetAcpiStructCounts (&IortDatabase);
+
ParseAcpi (
TRUE,
0,
@@ -666,7 +818,7 @@ ParseAcpiIort (
ParseAcpi (
FALSE,
0,
- "IORT Node Header",
+ NULL,
NodePtr,
AcpiTableLength - Offset,
PARSER_PARAMS (IortNodeHeaderParser)
@@ -681,7 +833,8 @@ ParseAcpiIort (
IncrementErrorCount ();
Print (
L"ERROR: Insufficient remaining table buffer length to read the " \
- L"IORT node header. Length = %d.\n",
+ L"%a header. Length = %d.\n",
+ IortDatabase.Name,
AcpiTableLength - Offset
);
return;
@@ -692,8 +845,9 @@ ParseAcpiIort (
((Offset + (*IortNodeLength)) > AcpiTableLength)) {
IncrementErrorCount ();
Print (
- L"ERROR: Invalid IORT Node length. " \
- L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
+ L"ERROR: Invalid %a length. Length = %d. Offset = %d. " \
+ L"AcpiTableLength = %d.\n",
+ IortDatabase.Name,
*IortNodeLength,
Offset,
AcpiTableLength
@@ -701,63 +855,24 @@ ParseAcpiIort (
return;
}
- PrintFieldName (2, L"* Node Offset *");
- Print (L"0x%x\n", Offset);
-
- switch (*IortNodeType) {
- case EFI_ACPI_IORT_TYPE_ITS_GROUP:
- DumpIortNodeIts (
- NodePtr,
- *IortNodeLength
- );
- break;
- case EFI_ACPI_IORT_TYPE_NAMED_COMP:
- DumpIortNodeNamedComponent (
- NodePtr,
- *IortNodeLength,
- *IortIdMappingCount,
- *IortIdMappingOffset
- );
- break;
- case EFI_ACPI_IORT_TYPE_ROOT_COMPLEX:
- DumpIortNodeRootComplex (
- NodePtr,
- *IortNodeLength,
- *IortIdMappingCount,
- *IortIdMappingOffset
- );
- break;
- case EFI_ACPI_IORT_TYPE_SMMUv1v2:
- DumpIortNodeSmmuV1V2 (
- NodePtr,
- *IortNodeLength,
- *IortIdMappingCount,
- *IortIdMappingOffset
- );
- break;
- case EFI_ACPI_IORT_TYPE_SMMUv3:
- DumpIortNodeSmmuV3 (
- NodePtr,
- *IortNodeLength,
- *IortIdMappingCount,
- *IortIdMappingOffset
- );
- break;
- case EFI_ACPI_IORT_TYPE_PMCG:
- DumpIortNodePmcg (
- NodePtr,
- *IortNodeLength,
- *IortIdMappingCount,
- *IortIdMappingOffset
- );
- break;
-
- default:
- IncrementErrorCount ();
- Print (L"ERROR: Unsupported IORT Node type = %d\n", *IortNodeType);
- } // switch
+ // Parse the IORT Node
+ ParseAcpiStruct (
+ 2,
+ NodePtr,
+ &IortDatabase,
+ Offset,
+ *IortNodeType,
+ *IortNodeLength,
+ IortIdMappingCount,
+ IortIdMappingOffset
+ );
NodePtr += (*IortNodeLength);
Offset += (*IortNodeLength);
} // while
+
+ // Report and validate IORT Node counts
+ if (GetConsistencyChecking ()) {
+ ValidateAcpiStructCounts (&IortDatabase);
+ }
}
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v1 6/6] ShellPkg: acpiview: Make PPTT parsing logic data driven
2020-05-05 15:45 [PATCH v1 0/6] ShellPkg: acpiview: Refactor ACPI table parsing loops Krzysztof Koch
` (4 preceding siblings ...)
2020-05-05 15:46 ` [PATCH v1 5/6] ShellPkg: acpiview: Make IORT " Krzysztof Koch
@ 2020-05-05 15:46 ` Krzysztof Koch
5 siblings, 0 replies; 10+ messages in thread
From: Krzysztof Koch @ 2020-05-05 15:46 UTC (permalink / raw)
To: devel; +Cc: ray.ni, zhichao.gao, Sami.Mujawar, Laura.Moretta
Replace the switch statement in the main parser loop with a table-driven
approach. Use the ParseAcpiStruct () method to resolve how each
Processor Topology Structure given should be parsed.
Enumerate all structures found in the Processor Properties Topology
Table (PPTT) on a per-type basis.
Consolidate all metadata about each Processor Topology Structure.
Define an array of ACPI_STRUCT_INFO structures to store the name,
instance count, architecture support and handling information. Use this
array to construct the ACPI_STRUCT_DATABASE for PPTT.
Count the number of instances of each Processor Topology Structure type.
Optionally report these counts after PPTT table parsing is finished.
Remove the definition of the DumpCacheTypeStructure and DumpIDStructure
functions. Their only purpose was to call ParseAcpi () and now this
process is streamlined.
Make DumpProcessorHierarchyNodeStructure function signature match that
of ACPI_STRUCT_PARSER_FUNC. This way, the function can be called from
ParseAcpiStruct ().
References:
- ACPI 6.3 Specification - January 2019, Section 5.2.29
Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---
Notes:
v1:
- Make PPTT parsing logic data driven [Krzysztof]
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c | 152 ++++++++++----------
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h | 2 +-
2 files changed, 74 insertions(+), 80 deletions(-)
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
index 0db272c16af0ad8824c8da4c88dd409c8550112a..b62f79b52cab989942f84b020e1d737e8ef65439 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
@@ -21,6 +21,11 @@ STATIC CONST UINT8* ProcessorTopologyStructureLength;
STATIC CONST UINT32* NumberOfPrivateResources;
STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
+/**
+ Handler for each Processor Topology Structure
+**/
+STATIC ACPI_STRUCT_INFO PpttStructs[];
+
/**
This function validates the Cache Type Structure (Type 1) 'Number of sets'
field.
@@ -243,22 +248,34 @@ STATIC CONST ACPI_PARSER IdStructureParser[] = {
@param [in] Ptr Pointer to the start of the Processor Hierarchy Node
Structure data.
@param [in] Length Length of the Processor Hierarchy Node Structure.
+ @param [in] OptArg0 First optional argument (Not used).
+ @param [in] OptArg1 Second optional argument (Not used).
**/
STATIC
VOID
DumpProcessorHierarchyNodeStructure (
- IN UINT8* Ptr,
- IN UINT8 Length
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0 OPTIONAL,
+ IN CONST VOID* OptArg1 OPTIONAL
)
{
UINT32 Offset;
UINT32 Index;
CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH];
+ CHAR8 AsciiBuffer[80];
+
+ PrintAcpiStructName (
+ PpttStructs[EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR].Name,
+ PpttStructs[EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR].Count,
+ sizeof (AsciiBuffer),
+ AsciiBuffer
+ );
Offset = ParseAcpi (
TRUE,
2,
- "Processor Hierarchy Node Structure",
+ AsciiBuffer,
Ptr,
Length,
PARSER_PARAMS (ProcessorHierarchyNodeStructureParser)
@@ -269,7 +286,8 @@ DumpProcessorHierarchyNodeStructure (
if (NumberOfPrivateResources == NULL) {
IncrementErrorCount ();
Print (
- L"ERROR: Insufficient Processor Hierarchy Node length. Length = %d.\n",
+ L"ERROR: Insufficient %a Structure length. Length = %d.\n",
+ PpttStructs[EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR].Name,
Length
);
return;
@@ -296,7 +314,7 @@ DumpProcessorHierarchyNodeStructure (
UnicodeSPrint (
Buffer,
sizeof (Buffer),
- L"Private resources [%d]",
+ L"Private resource [%d]",
Index
);
@@ -312,50 +330,37 @@ DumpProcessorHierarchyNodeStructure (
}
/**
- 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.
+ Information about each Processor Topology Structure type.
**/
-STATIC
-VOID
-DumpCacheTypeStructure (
- IN UINT8* Ptr,
- IN UINT8 Length
- )
-{
- ParseAcpi (
- TRUE,
- 2,
- "Cache Type Structure",
- Ptr,
- Length,
- PARSER_PARAMS (CacheTypeStructureParser)
- );
-}
+STATIC ACPI_STRUCT_INFO PpttStructs[] = {
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "Processor",
+ EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64 | ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpProcessorHierarchyNodeStructure
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "Cache",
+ EFI_ACPI_6_3_PPTT_TYPE_CACHE,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64 | ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ CacheTypeStructureParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "ID",
+ EFI_ACPI_6_3_PPTT_TYPE_ID,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64 | ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ IdStructureParser
+ )
+};
/**
- 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.
+ PPTT structure database
**/
-STATIC
-VOID
-DumpIDStructure (
- IN UINT8* Ptr,
- IN UINT8 Length
- )
-{
- ParseAcpi (
- TRUE,
- 2,
- "ID Structure",
- Ptr,
- Length,
- PARSER_PARAMS (IdStructureParser)
- );
-}
+STATIC ACPI_STRUCT_DATABASE PpttDatabase = {
+ "Processor Topology Structure",
+ PpttStructs,
+ ARRAY_SIZE (PpttStructs)
+};
/**
This function parses the ACPI PPTT table.
@@ -390,6 +395,8 @@ ParseAcpiPptt (
return;
}
+ ResetAcpiStructCounts (&PpttDatabase);
+
Offset = ParseAcpi (
TRUE,
0,
@@ -419,7 +426,8 @@ ParseAcpiPptt (
IncrementErrorCount ();
Print (
L"ERROR: Insufficient remaining table buffer length to read the " \
- L"processor topology structure header. Length = %d.\n",
+ L"%a header. Length = %d.\n",
+ PpttDatabase.Name,
AcpiTableLength - Offset
);
return;
@@ -430,8 +438,9 @@ ParseAcpiPptt (
((Offset + (*ProcessorTopologyStructureLength)) > AcpiTableLength)) {
IncrementErrorCount ();
Print (
- L"ERROR: Invalid Processor Topology Structure length. " \
- L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
+ L"ERROR: Invalid %a length. Length = %d. Offset = %d. " \
+ L"AcpiTableLength = %d.\n",
+ PpttDatabase.Name,
*ProcessorTopologyStructureLength,
Offset,
AcpiTableLength
@@ -439,39 +448,24 @@ ParseAcpiPptt (
return;
}
- 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
- );
- }
+ // Parse the Processor Topology Structure
+ ParseAcpiStruct (
+ 2,
+ ProcessorTopologyStructurePtr,
+ &PpttDatabase,
+ Offset,
+ *ProcessorTopologyStructureType,
+ *ProcessorTopologyStructureLength,
+ NULL,
+ NULL
+ );
ProcessorTopologyStructurePtr += *ProcessorTopologyStructureLength;
Offset += *ProcessorTopologyStructureLength;
} // while
+
+ // Report and validate processor topology structure counts
+ if (GetConsistencyChecking ()) {
+ ValidateAcpiStructCounts (&PpttDatabase);
+ }
}
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h
index 2a671203fb0035bbc407ff4bb0ca9960706fa588..a0b0622ba00bda63379b670527145359b738bb7d 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h
@@ -1,7 +1,7 @@
/** @file
Header file for PPTT parser
- Copyright (c) 2019, ARM Limited. All rights reserved.
+ Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@par Reference(s):
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 10+ messages in thread