Re: [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

["Sami Mujawar" <sami.mujawar@arm.com>]

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>


-----Original Message-----
From: Krzysztof Koch <krzysztof.koch@arm.com> 
Sent: 07 June 2019 09:48 AM
To: devel@edk2.groups.io
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; jaben.carsey@intel.com; ray.ni@intel.com; zhichao.gao@intel.com; Matteo Carlini <Matteo.Carlini@arm.com>; Stephanie Hughes-Fitt <Stephanie.Hughes-Fitt@arm.com>; nd <nd@arm.com>
Subject: [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

The ACPI 6.3 specification introduces a 'SPE overflow Interrupt' field as part of the GICC structure.

Update the MADT parser to decode this field and validate the interrupt ID used.

References:
- ACPI 6.3 Specification - January 2019
- Arm Generic Interrupt Controller Architecture Specification,
  GIC architecture version 3 and version 4, issue E
- Arm Server Base System Architecture 5.0

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

Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/tree/477_acpiview_spe_v2

Notes:
    v2:
    - Add include sandwich in MadtParser.h [Zhichao]
    - Add references to specifications in commit message [Zhichao]

    v1:
    - Decode and validate SPE Overflow Interrupt field [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 86 ++++++++++++++++++--  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h | 40 +++++++++
 2 files changed, 118 insertions(+), 8 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
index a1bf86ade5313f954a77b325c13394cfce133939..59c3df0cc8a080497b517baf36fc63f1e4ab866f 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
+++ er.c
@@ -1,17 +1,21 @@
 /** @file
   MADT table parser
 
-  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
+  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
-    - ACPI 6.2 Specification - Errata A, September 2017
+    - ACPI 6.3 Specification - January 2019
+    - Arm Generic Interrupt Controller Architecture Specification,
+      GIC architecture version 3 and version 4, issue E
+    - Arm Server Base System Architecture 5.0
 **/
 
 #include <IndustryStandard/Acpi.h>
 #include <Library/UefiLib.h>
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
+#include "MadtParser.h"
 
 // Local Variables
 STATIC CONST UINT8* MadtInterruptControllerType; @@ -33,6 +37,21 @@ ValidateGICDSystemVectorBase (
   IN VOID*  Context
   );
 
+/**
+  This function validates the SPE Overflow Interrupt in the GICC.
+
+  @param [in] Ptr     Pointer to the start of the field data.
+  @param [in] Context Pointer to context specific information e.g. this
+                      could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateSpeOverflowInterrupt (
+  IN UINT8* Ptr,
+  IN VOID*  Context
+  );
+
 /**
   An ACPI_PARSER array describing the GICC Interrupt Controller Structure.
 **/
@@ -56,7 +75,9 @@ STATIC CONST ACPI_PARSER GicCParser[] = {
   {L"MPIDR", 8, 68, L"0x%lx", NULL, NULL, NULL, NULL},
   {L"Processor Power Efficiency Class", 1, 76, L"0x%x", NULL, NULL, NULL,
    NULL},
-  {L"Reserved", 3, 77, L"%x %x %x", Dump3Chars, NULL, NULL, NULL}
+  {L"Reserved", 1, 77, L"0x%x", NULL, NULL, NULL, NULL},  {L"SPE 
+ overflow Interrupt", 2, 78, L"0x%x", NULL, NULL,
+    ValidateSpeOverflowInterrupt, NULL}
 };
 
 /**
@@ -160,6 +181,55 @@ ValidateGICDSystemVectorBase (
   }
 }
 
+/**
+  This function validates the SPE Overflow Interrupt in the GICC.
+
+  @param [in] Ptr     Pointer to the start of the field data.
+  @param [in] Context Pointer to context specific information e.g. this
+                      could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateSpeOverflowInterrupt (
+  IN UINT8* Ptr,
+  IN VOID*  Context
+  )
+{
+  UINT16 SpeOverflowInterrupt;
+
+  SpeOverflowInterrupt = *(UINT16*)Ptr;
+
+  // SPE not supported by this processor  if (SpeOverflowInterrupt == 
+ 0) {
+    return;
+  }
+
+  if ((SpeOverflowInterrupt < ARM_PPI_ID_MIN) ||
+      ((SpeOverflowInterrupt > ARM_PPI_ID_MAX) &&
+       (SpeOverflowInterrupt < ARM_PPI_ID_EXTENDED_MIN)) ||
+      (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MAX)) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: SPE Overflow Interrupt ID of %d is not in the allowed PPI ID "
+        L"ranges of %d-%d or %d-%d (for GICv3.1 or later).",
+      SpeOverflowInterrupt,
+      ARM_PPI_ID_MIN,
+      ARM_PPI_ID_MAX,
+      ARM_PPI_ID_EXTENDED_MIN,
+      ARM_PPI_ID_EXTENDED_MAX
+    );
+  } else if (SpeOverflowInterrupt != ARM_PPI_ID_PMBIRQ) {
+    IncrementWarningCount();
+    Print (
+      L"\nWARNING: SPE Overflow Interrupt ID of %d is not compliant with SBSA "
+        L"Level 3 PPI ID assignment: %d.",
+      SpeOverflowInterrupt,
+      ARM_PPI_ID_PMBIRQ
+    );
+  }
+}
+
 /**
   This function parses the ACPI MADT table.
   When trace is enabled this function parses the MADT table and @@ -233,7 +303,7 @@ ParseAcpiMadt (
     }
 
     switch (*MadtInterruptControllerType) {
-      case EFI_ACPI_6_2_GIC: {
+      case EFI_ACPI_6_3_GIC: {
         ParseAcpi (
           TRUE,
           2,
@@ -245,7 +315,7 @@ ParseAcpiMadt (
         break;
       }
 
-      case EFI_ACPI_6_2_GICD: {
+      case EFI_ACPI_6_3_GICD: {
         if (++GICDCount > 1) {
           IncrementErrorCount ();
           Print (
@@ -265,7 +335,7 @@ ParseAcpiMadt (
         break;
       }
 
-      case EFI_ACPI_6_2_GIC_MSI_FRAME: {
+      case EFI_ACPI_6_3_GIC_MSI_FRAME: {
         ParseAcpi (
           TRUE,
           2,
@@ -277,7 +347,7 @@ ParseAcpiMadt (
         break;
       }
 
-      case EFI_ACPI_6_2_GICR: {
+      case EFI_ACPI_6_3_GICR: {
         ParseAcpi (
           TRUE,
           2,
@@ -289,7 +359,7 @@ ParseAcpiMadt (
         break;
       }
 
-      case EFI_ACPI_6_2_GIC_ITS: {
+      case EFI_ACPI_6_3_GIC_ITS: {
         ParseAcpi (
           TRUE,
           2,
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h
new file mode 100644
index 0000000000000000000000000000000000000000..fbbc43e09adbdf9fea302a03a61e6dc179f06a62
--- /dev/null
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
+++ er.h
@@ -0,0 +1,40 @@
+/** @file
+  Header file for MADT table parser
+
+  Copyright (c) 2019, 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 **/
+
+#ifndef MADT_PARSER_H_
+#define MADT_PARSER_H_
+
+///
+/// Level 3 base server system Private Peripheral Inerrupt (PPI) ID 
+assignments ///
+#define ARM_PPI_ID_OVERFLOW_INTERRUPT_FROM_CNTP       30
+#define ARM_PPI_ID_OVERFLOW_INTERRUPT_FROM_CNTPS      29
+#define ARM_PPI_ID_OVERFLOW_INTERRUPT_FROM_CNTHV      28
+#define ARM_PPI_ID_OVERFLOW_INTERRUPT_FROM_CNTV       27
+#define ARM_PPI_ID_OVERFLOW_INTERRUPT_FROM_CNTHP      26
+#define ARM_PPI_ID_GIC_MAINTENANCE_INTERRUPT          25
+#define ARM_PPI_ID_CTIIRQ                             24
+#define ARM_PPI_ID_PERFORMANCE_MONITORS_INTERRUPT     23
+#define ARM_PPI_ID_COMMIRQ                            22
+#define ARM_PPI_ID_PMBIRQ                             21
+#define ARM_PPI_ID_CNTHPS                             20
+#define ARM_PPI_ID_CNTHVS                             19
+
+///
+/// PPI ID allowed ranges
+///
+#define ARM_PPI_ID_MAX              31
+#define ARM_PPI_ID_MIN              16
+#define ARM_PPI_ID_EXTENDED_MAX     1119
+#define ARM_PPI_ID_EXTENDED_MIN     1056
+
+#endif // MADT_PARSER_H_
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'