[edk2-devel] [PATCH v1 2/6] ShellPkg: acpiview: Make MADT parsing logic data driven

Krzysztof Koch posted 6 patches 9 weeks ago

[edk2-devel] [PATCH v1 2/6] ShellPkg: acpiview: Make MADT parsing logic data driven

Posted by Krzysztof Koch 9 weeks ago
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)'


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

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