[PATCH v1 1/1] ShellPkg: Validate that the Boot CPU is present in MADT

["Joey Gouly" <joey.gouly@arm.com>]

The ACPI 6.3 Specification, January 2019, section 5.2.12.14 states that
the firmware must convey each processor’s GIC information to the OS using
the GICC structure.

If a GICC structure for the boot CPU is missing some standards-based
operating system may crash with an error code. It may be difficult to
diagnose the reason for the crash as the error code may be too generic
and mean firmware error.

Therefore add validation to the MADT table parser to check that a GICC is
present for the boot CPU.

Signed-off-by: Joey Gouly <joey.gouly@arm.com>
---
The changes can be seen at https://github.com/jgouly/edk2/tree/1474_validate_boot_cpu_mpidr_v1

 ShellPkg/ShellPkg.dsc                                                        |  3 ++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf |  7 ++-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c       | 54 +++++++++++++++++++-
 ShellPkg/ShellPkg.ci.yaml                                                    |  3 +-
 4 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/ShellPkg/ShellPkg.dsc b/ShellPkg/ShellPkg.dsc
index c42bc9464a0f7be111ee3086a664506c8288928c..661cc8b02b0971280c3649e0f29e109305bcc776 100644
--- a/ShellPkg/ShellPkg.dsc
+++ b/ShellPkg/ShellPkg.dsc
@@ -71,6 +71,9 @@ [LibraryClasses.ARM,LibraryClasses.AARCH64]
   # Add support for GCC stack protector
   NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
 
+  # Add support for reading MPIDR
+  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
+
 [PcdsFixedAtBuild]
   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
   gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|16000
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
index 91459f9ec632635ee453c5ef46f67445cd9eee0c..6f9e77eed8f8c5f12ecf6b44c494da2e6aacda27 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
@@ -1,7 +1,7 @@
 ##  @file
 # Provides Shell 'acpiview' command functions
 #
-# Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.<BR>
+# Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -57,6 +57,9 @@ [Packages]
   MdePkg/MdePkg.dec
   ShellPkg/ShellPkg.dec
 
+[Packages.ARM, Packages.AARCH64]
+  ArmPkg/ArmPkg.dec
+
 [LibraryClasses]
   BaseLib
   BaseMemoryLib
@@ -72,6 +75,8 @@ [LibraryClasses]
   UefiLib
   UefiRuntimeServicesTableLib
 
+[LibraryClasses.ARM, LibraryClasses.AARCH64]
+  ArmLib
 
 [FixedPcd]
   gEfiShellPkgTokenSpaceGuid.PcdShellProfileMask ## CONSUMES
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
index 15aa2392b60cee9e3843c7c560b0ab84e0be4174..be013ce5c06541d12dcd594eb0f5c1820708923e 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
@@ -1,7 +1,7 @@
 /** @file
   MADT table parser
 
-  Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
+  Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
@@ -13,6 +13,9 @@
 
 #include <IndustryStandard/Acpi.h>
 #include <Library/UefiLib.h>
+#if defined(MDE_CPU_ARM) || defined(MDE_CPU_AARCH64)
+#include <Library/ArmLib.h>
+#endif
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
 #include "AcpiViewConfig.h"
@@ -23,6 +26,11 @@ STATIC CONST UINT8* MadtInterruptControllerType;
 STATIC CONST UINT8* MadtInterruptControllerLength;
 STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
 
+#if defined(MDE_CPU_ARM) || defined(MDE_CPU_AARCH64)
+STATIC UINT64 BootMpidr;
+STATIC BOOLEAN HasBootCpuGicc = FALSE;
+#endif
+
 /**
   This function validates the System Vector Base in the GICD.
 
@@ -95,6 +103,33 @@ ValidateSpeOverflowInterrupt (
   }
 }
 
+/**
+  This function validates that the GICC structure contains an entry for
+  the Boot CPU.
+
+  @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
+ValidateBootMpidr (
+  IN UINT8* Ptr,
+  IN VOID*  Context
+  )
+{
+#if defined(MDE_CPU_ARM) || defined(MDE_CPU_AARCH64)
+  UINT64 CurrentMpidr;
+
+  CurrentMpidr = *(UINT64*)Ptr;
+
+  if (CurrentMpidr == BootMpidr) {
+    HasBootCpuGicc = TRUE;
+  }
+#endif
+}
+
 /**
   An ACPI_PARSER array describing the GICC Interrupt Controller Structure.
 **/
@@ -115,7 +150,7 @@ STATIC CONST ACPI_PARSER GicCParser[] = {
   {L"GICH", 8, 48, L"0x%lx", NULL, NULL, NULL, NULL},
   {L"VGIC Maintenance interrupt", 4, 56, L"0x%x", NULL, NULL, NULL, NULL},
   {L"GICR Base Address", 8, 60, L"0x%lx", NULL, NULL, NULL, NULL},
-  {L"MPIDR", 8, 68, L"0x%lx", NULL, NULL, NULL, NULL},
+  {L"MPIDR", 8, 68, L"0x%lx", NULL, NULL, ValidateBootMpidr, NULL},
   {L"Processor Power Efficiency Class", 1, 76, L"0x%x", NULL, NULL, NULL,
    NULL},
   {L"Reserved", 1, 77, L"0x%x", NULL, NULL, NULL, NULL},
@@ -234,6 +269,11 @@ ParseAcpiMadt (
   UINT8* InterruptContollerPtr;
   UINT32 GICDCount;
 
+#if defined(MDE_CPU_ARM) || defined(MDE_CPU_AARCH64)
+  BootMpidr = ArmReadMpidr () &
+              (ARM_CORE_AFF0 | ARM_CORE_AFF1 | ARM_CORE_AFF2 | ARM_CORE_AFF3);
+#endif
+
   GICDCount = 0;
 
   if (!Trace) {
@@ -371,4 +411,14 @@ ParseAcpiMadt (
     InterruptContollerPtr += *MadtInterruptControllerLength;
     Offset += *MadtInterruptControllerLength;
   } // while
+
+#if defined(MDE_CPU_ARM) || defined(MDE_CPU_AARCH64)
+  if (!HasBootCpuGicc) {
+    IncrementErrorCount ();
+    Print (
+      L"ERROR: No GICC present for Boot CPU (MPIDR: 0x%lx)",
+      BootMpidr
+      );
+  }
+#endif
 }
diff --git a/ShellPkg/ShellPkg.ci.yaml b/ShellPkg/ShellPkg.ci.yaml
index 30894d44bc3ae9a2a9796146c5bcdc62d4ce9801..2833febb296f3d3b1fa0c48268955bb574d1dbee 100644
--- a/ShellPkg/ShellPkg.ci.yaml
+++ b/ShellPkg/ShellPkg.ci.yaml
@@ -31,7 +31,8 @@
             "MdePkg/MdePkg.dec",
             "MdeModulePkg/MdeModulePkg.dec",
             "ShellPkg/ShellPkg.dec",
-            "NetworkPkg/NetworkPkg.dec"
+            "NetworkPkg/NetworkPkg.dec",
+            "ArmPkg/ArmPkg.dec"
         ],
         # For host based unit tests
         "AcceptableDependencies-HOST_APPLICATION":[],
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")