ArmPkg/Gic: force GIC driver to run before CPU arch protocol driver

Currently, the GIC driver has a static dependency on the CPU arch protocol
driver, so it can register its IRQ handler at init time. This means there
is a window between dispatch of the CPU driver and dispatch of the GIC
driver where any unexpected GIC state may trigger an interrupt which we
are not set up to handle yet. Note that this is even the case if we enter
UEFI with interrupts disabled at the CPU, given that any TPL manipulation
involving TPL_HIGH_LEVEL will unconditionally enable IRQs at the CPU side
regardless of whether they were enabled to begin with (but only as soon as
the CPU arch protocol is actually installed)

So let's reorder the GIC driver with the CPU driver, and let it run its
initialization that puts the GIC into a known state before enabling
interrupts. Move its installation of its IRQ handler to a protocol notify
callback on the CPU arch protocol so that it runs as soon as it becomes
available.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Tested-by: Marc Zyngier <marc.zyngier@arm.com>

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index 5dbd019e2966573b9f987bc6775ca42ffbffc230..a55b6268ff85ffd7da140be813ec875f7f242c4d 100644 (file)
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -48,6 +48,8 @@
   # Include/Guid/ArmMpCoreInfo.h\r
   gArmMpCoreInfoGuid = { 0xa4ee0728, 0xe5d7, 0x4ac5,  {0xb2, 0x1e, 0x65, 0x8e, 0xd8, 0x57, 0xe8, 0x34} }\r
 \r
   # Include/Guid/ArmMpCoreInfo.h\r
   gArmMpCoreInfoGuid = { 0xa4ee0728, 0xe5d7, 0x4ac5,  {0xb2, 0x1e, 0x65, 0x8e, 0xd8, 0x57, 0xe8, 0x34} }\r
 \r

+  gArmGicDxeFileGuid = { 0xde371f7c, 0xdec4, 0x4d21, { 0xad, 0xf1, 0x59, 0x3a, 0xbc, 0xc1, 0x58, 0x82 } }\r
+\r

 [Ppis]\r
   ## Include/Ppi/ArmMpCoreInfo.h\r
   gArmMpCoreInfoPpiGuid = { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d, 0xab, 0x44, 0xe7, 0x54, 0xa8, 0xfc} }\r
 [Ppis]\r
   ## Include/Ppi/ArmMpCoreInfo.h\r
   gArmMpCoreInfoPpiGuid = { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d, 0xab, 0x44, 0xe7, 0x54, 0xa8, 0xfc} }\r

diff --git a/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c b/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
index bff8d983cf02f7fd1fa6165824cb6556cdea5c6f..e1adcd3bc6d37b15bf385878fa7a714d3822fe78 100644 (file)
--- a/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
+++ b/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
@@ -121,6 +121,44 @@ RegisterInterruptSource (
   }\r
 }\r
 \r
   }\r
 }\r
 \r

+STATIC VOID *mCpuArchProtocolNotifyEventRegistration;\r
+\r
+STATIC\r
+VOID\r
+EFIAPI\r
+CpuArchEventProtocolNotify (\r
+  IN  EFI_EVENT       Event,\r
+  IN  VOID            *Context\r
+  )\r
+{\r
+  EFI_CPU_ARCH_PROTOCOL   *Cpu;\r
+  EFI_STATUS              Status;\r
+\r
+  // Get the CPU protocol that this driver requires.\r
+  Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&Cpu);\r
+  if (EFI_ERROR (Status)) {\r
+    return;\r
+  }\r
+\r
+  // Unregister the default exception handler.\r
+  Status = Cpu->RegisterInterruptHandler (Cpu, ARM_ARCH_EXCEPTION_IRQ, NULL);\r
+  if (EFI_ERROR (Status)) {\r
+    DEBUG ((DEBUG_ERROR, "%a: Cpu->RegisterInterruptHandler() - %r\n",\r
+      __FUNCTION__, Status));\r
+    return;\r
+  }\r
+\r
+  // Register to receive interrupts\r
+  Status = Cpu->RegisterInterruptHandler (Cpu, ARM_ARCH_EXCEPTION_IRQ,\r
+                  Context);\r
+  if (EFI_ERROR (Status)) {\r
+    DEBUG ((DEBUG_ERROR, "%a: Cpu->RegisterInterruptHandler() - %r\n",\r
+      __FUNCTION__, Status));\r
+  }\r
+\r
+  gBS->CloseEvent (Event);\r
+}\r
+\r

 EFI_STATUS\r
 InstallAndRegisterInterruptService (\r
   IN EFI_HARDWARE_INTERRUPT_PROTOCOL   *InterruptProtocol,\r
 EFI_STATUS\r
 InstallAndRegisterInterruptService (\r
   IN EFI_HARDWARE_INTERRUPT_PROTOCOL   *InterruptProtocol,\r


@@ -130,7 +168,6 @@ InstallAndRegisterInterruptService (
   )\r
 {\r
   EFI_STATUS               Status;\r
   )\r
 {\r
   EFI_STATUS               Status;\r

-  EFI_CPU_ARCH_PROTOCOL   *Cpu;\r

   CONST UINTN              RihArraySize =\r
     (sizeof(HARDWARE_INTERRUPT_HANDLER) * mGicNumInterrupts);\r
 \r
   CONST UINTN              RihArraySize =\r
     (sizeof(HARDWARE_INTERRUPT_HANDLER) * mGicNumInterrupts);\r
 \r


@@ -152,27 +189,15 @@ InstallAndRegisterInterruptService (
     return Status;\r
   }\r
 \r
     return Status;\r
   }\r
 \r

-  // Get the CPU protocol that this driver requires.\r
-  Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&Cpu);\r
-  if (EFI_ERROR (Status)) {\r
-    return Status;\r
-  }\r
-\r
-  // Unregister the default exception handler.\r
-  Status = Cpu->RegisterInterruptHandler (Cpu, ARM_ARCH_EXCEPTION_IRQ, NULL);\r
-  if (EFI_ERROR (Status)) {\r
-    return Status;\r
-  }\r
-\r
-  // Register to receive interrupts\r
-  Status = Cpu->RegisterInterruptHandler (\r
-                  Cpu,\r
-                  ARM_ARCH_EXCEPTION_IRQ,\r
-                  InterruptHandler\r
-                  );\r
-  if (EFI_ERROR (Status)) {\r
-    return Status;\r
-  }\r
+  //\r
+  // Install the interrupt handler as soon as the CPU arch protocol appears.\r
+  //\r
+  EfiCreateProtocolNotifyEvent (\r
+    &gEfiCpuArchProtocolGuid,\r
+    TPL_CALLBACK,\r
+    CpuArchEventProtocolNotify,\r
+    InterruptHandler,\r
+    &mCpuArchProtocolNotifyEventRegistration);\r

 \r
   // Register for an ExitBootServicesEvent\r
   Status = gBS->CreateEvent (\r
 \r
   // Register for an ExitBootServicesEvent\r
   Status = gBS->CreateEvent (\r

diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.h b/ArmPkg/Drivers/ArmGic/ArmGicDxe.h
index 610ffacc7eb025011f87ad5e6e457a610bd570a9..f6b75d729601422c3db6196ae98f862eb17fb22a 100644 (file)
--- a/ArmPkg/Drivers/ArmGic/ArmGicDxe.h
+++ b/ArmPkg/Drivers/ArmGic/ArmGicDxe.h
@@ -21,6 +21,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/IoLib.h>\r
 #include <Library/MemoryAllocationLib.h>\r
 #include <Library/UefiBootServicesTableLib.h>\r
 #include <Library/IoLib.h>\r
 #include <Library/MemoryAllocationLib.h>\r
 #include <Library/UefiBootServicesTableLib.h>\r

+#include <Library/UefiLib.h>\r

 \r
 #include <Protocol/Cpu.h>\r
 #include <Protocol/HardwareInterrupt.h>\r
 \r
 #include <Protocol/Cpu.h>\r
 #include <Protocol/HardwareInterrupt.h>\r

diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
index d5921533fb68fa32c3e0705b05930700ee81da07..24b02ef30e8317f3664c84e0251f524f51521943 100644 (file)
--- a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
+++ b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
@@ -16,7 +16,7 @@
 [Defines]\r
   INF_VERSION                    = 0x00010005\r
   BASE_NAME                      = ArmGicDxe\r
 [Defines]\r
   INF_VERSION                    = 0x00010005\r
   BASE_NAME                      = ArmGicDxe\r

-  FILE_GUID                      = DE371F7C-DEC4-4D21-ADF1-593ABCC15882\r
+  FILE_GUID                      = DE371F7C-DEC4-4D21-ADF1-593ABCC15882 # gArmGicDxeFileGuid\r

   MODULE_TYPE                    = DXE_DRIVER\r
   VERSION_STRING                 = 1.0\r
 \r
   MODULE_TYPE                    = DXE_DRIVER\r
   VERSION_STRING                 = 1.0\r
 \r


@@ -45,6 +45,7 @@
   UefiDriverEntryPoint\r
   IoLib\r
   PcdLib\r
   UefiDriverEntryPoint\r
   IoLib\r
   PcdLib\r

+  UefiLib\r

 \r
 [Protocols]\r
   gHardwareInterruptProtocolGuid\r
 \r
 [Protocols]\r
   gHardwareInterruptProtocolGuid\r


@@ -58,4 +59,4 @@
   gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy\r
 \r
 [Depex]\r
   gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy\r
 \r
 [Depex]\r

-  gEfiCpuArchProtocolGuid\r
+  TRUE\r

diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
index 0c5017b31e662d8bb7d065142532850700444911..b748f0344537caa703012e04f6c77a063c2bb483 100644 (file)
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
@@ -74,4 +74,4 @@
   gArmTokenSpaceGuid.PcdDebuggerExceptionSupport\r
 \r
 [Depex]\r
   gArmTokenSpaceGuid.PcdDebuggerExceptionSupport\r
 \r
 [Depex]\r

-  TRUE\r
+  AFTER gArmGicDxeFileGuid\r