ArmPkg/CpuDxe: Replace DEPEX on h/w protocol with event notification

Currently, ArmPkg's CpuDxe DEPEXes on the hardware interrupt protocol,
to ensure that it is not dispatched before the GIC driver. This way, the
CpuDxe driver is guaranteed not to enable interrupts on the CPU side
before the GIC driver has had the opportunity to configure the
interrupts on the distribution side.

However, this prevents the GIC driver from using any of the CPU arch
protocol interfaces, such as mapping memory, which it may need to do on
platforms where the GIC MMIO regions are not mapped yet when the driver
is started.

So instead, use a protocol notification on the hardware interrupt
protocol, which is installed by the GIC driver (as well as other
existing interrupt controller drivers for platforms that do not
implement a GIC) after it starts up and deasserts and disables all
incoming interrupts. Manipulate the interrupt state as usual only after
this notification has been received. Before that, keep track of the
caller's intent regarding the interrupt enabled state in a shadow
variable, but do not actually enable interrupt delivery to the CPU just
yet.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

2 files changed, 106 insertions, 4 deletions

diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.c b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
index 191eb1c20b..39e5e9beea 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.c
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
@@ -15,6 +15,59 @@
 

 BOOLEAN  mIsFlushingGCD;

 

+// Shadow state for the CPU interrupt en/disabled bit

+STATIC BOOLEAN  mInterruptsEnabled;

+STATIC VOID     *mHardwareInterruptProtocolNotifyEventRegistration;

+

+/**

+  Mark interrupts as enabled in the shadow variable but don't actually enable them yet.

+**/

+STATIC

+EFI_STATUS

+EFIAPI

+CpuShadowEnableInterrupt (

+  IN EFI_CPU_ARCH_PROTOCOL  *This

+  )

+{

+  mInterruptsEnabled = TRUE;

+

+  return EFI_SUCCESS;

+}

+

+/**

+  Mark interrupts as disabled in the shadow variable.

+**/

+STATIC

+EFI_STATUS

+EFIAPI

+CpuShadowDisableInterrupt (

+  IN EFI_CPU_ARCH_PROTOCOL  *This

+  )

+{

+  mInterruptsEnabled = FALSE;

+

+  return EFI_SUCCESS;

+}

+

+/**

+  Return whether interrupts would be enabled based on the shadow variable.

+**/

+STATIC

+EFI_STATUS

+EFIAPI

+CpuShadowGetInterruptState (

+  IN  EFI_CPU_ARCH_PROTOCOL  *This,

+  OUT BOOLEAN                *State

+  )

+{

+  if (State == NULL) {

+    return EFI_INVALID_PARAMETER;

+  }

+

+  *State = mInterruptsEnabled;

+  return EFI_SUCCESS;

+}

+

 /**

   This function flushes the range of addresses from Start to Start+Length

   from the processor's data cache. If Start is not aligned to a cache line

@@ -216,9 +269,9 @@ IdleLoopEventCallback (
 //

 STATIC EFI_CPU_ARCH_PROTOCOL  mCpu = {

   CpuFlushCpuDataCache,

-  CpuEnableInterrupt,

-  CpuDisableInterrupt,

-  CpuGetInterruptState,

+  CpuShadowEnableInterrupt,

+  CpuShadowDisableInterrupt,

+  CpuShadowGetInterruptState,

   CpuInit,

   CpuRegisterInterruptHandler,

   CpuGetTimerValue,

@@ -307,6 +360,40 @@ RemapUnusedMemoryNx (
   }

 }

 

+STATIC

+VOID

+EFIAPI

+HardwareInterruptProtocolNotify (

+  IN  EFI_EVENT  Event,

+  IN  VOID       *Context

+  )

+{

+  VOID        *Protocol;

+  EFI_STATUS  Status;

+

+  Status = gBS->LocateProtocol (&gHardwareInterruptProtocolGuid, NULL, &Protocol);

+  if (EFI_ERROR (Status)) {

+    return;

+  }

+

+  //

+  // Now that the dedicated driver has taken control of the interrupt

+  // controller, we can allow interrupts to be enabled on the CPU side. So swap

+  // out the function stubs that manipulate the shadow state with the real

+  // ones. Interrupts are still disabled at the CPU so these fields can be set

+  // in any order.

+  //

+  mCpu.EnableInterrupt   = CpuEnableInterrupt;

+  mCpu.DisableInterrupt  = CpuDisableInterrupt;

+  mCpu.GetInterruptState = CpuGetInterruptState;

+

+  if (mInterruptsEnabled) {

+    ArmEnableInterrupts ();

+  }

+

+  gBS->CloseEvent (Event);

+}

+

 EFI_STATUS

 CpuDxeInitialize (

   IN EFI_HANDLE        ImageHandle,

@@ -317,6 +404,7 @@ CpuDxeInitialize (
   EFI_EVENT   IdleLoopEvent;

   EFI_HANDLE  CpuHandle;

 

+  ArmDisableInterrupts ();

   InitializeExceptions ();

 

   InitializeDma (&mCpu);

@@ -372,5 +460,18 @@ CpuDxeInitialize (
                   );

   ASSERT_EFI_ERROR (Status);

 

+  //

+  // Interrupts should only be enabled on the CPU side after the GIC driver has

+  // configured and deasserted all incoming interrupt lines. So keep interrupts

+  // masked until the gHardwareInterruptProtocolGuid protocol appears.

+  //

+  EfiCreateProtocolNotifyEvent (

+    &gHardwareInterruptProtocolGuid,

+    TPL_CALLBACK,

+    HardwareInterruptProtocolNotify,

+    NULL,

+    &mHardwareInterruptProtocolNotifyEventRegistration

+    );

+

   return EFI_SUCCESS;

 }

diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
index 7d8132200e..1d6e2f99e1 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
@@ -56,6 +56,7 @@
 [Protocols]

   gEfiCpuArchProtocolGuid

   gEfiMemoryAttributeProtocolGuid

+  gHardwareInterruptProtocolGuid

 

 [Guids]

   gEfiDebugImageInfoTableGuid

@@ -72,4 +73,4 @@
   gArmTokenSpaceGuid.PcdRemapUnusedMemoryNx

 

 [Depex]

-  gHardwareInterruptProtocolGuid OR gHardwareInterrupt2ProtocolGuid

+  TRUE