public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Krzysztof Koch" <krzysztof.koch@arm.com>
To: <devel@edk2.groups.io>
Cc: <ray.ni@intel.com>, <zhichao.gao@intel.com>,
	<Sami.Mujawar@arm.com>,
	<Laura.Moretta@arm.comMatteo.Carlini@arm.com>, <nd@arm.com>
Subject: [PATCH v1 2/6] ShellPkg: acpiview: Make MADT parsing logic data driven
Date: Tue, 5 May 2020 16:46:00 +0100	[thread overview]
Message-ID: <20200505154604.9848-3-krzysztof.koch@arm.com> (raw)
In-Reply-To: <20200505154604.9848-1-krzysztof.koch@arm.com>

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)'


  parent reply	other threads:[~2020-05-05 15:46 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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-06-11  7:42   ` Gao, Zhichao
2020-06-11 11:49     ` [edk2-devel] " Tomas Pilar (tpilar)
2020-05-05 15:46 ` Krzysztof Koch [this message]
2020-05-05 15:46 ` [PATCH v1 3/6] ShellPkg: acpiview: Make SRAT parsing logic data driven Krzysztof Koch
2020-05-05 15:46 ` [PATCH v1 4/6] ShellPkg: acpiview: Make GTDT " 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200505154604.9848-3-krzysztof.koch@arm.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox