From: Ard Biesheuvel Date: Tue, 6 Feb 2018 11:58:12 +0000 (+0000) Subject: ArmPkg/Gic: force GIC driver to run before CPU arch protocol driver X-Git-Tag: edk2-stable201903~2465 X-Git-Url: https://git.proxmox.com/?p=mirror_edk2.git;a=commitdiff_plain;h=61a7b0ec634fa3288f47929ba3ced05ff48de739 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 Reviewed-by: Leif Lindholm Tested-by: Marc Zyngier --- diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec index 5dbd019e29..a55b6268ff 100644 --- a/ArmPkg/ArmPkg.dec +++ b/ArmPkg/ArmPkg.dec @@ -48,6 +48,8 @@ # Include/Guid/ArmMpCoreInfo.h gArmMpCoreInfoGuid = { 0xa4ee0728, 0xe5d7, 0x4ac5, {0xb2, 0x1e, 0x65, 0x8e, 0xd8, 0x57, 0xe8, 0x34} } + gArmGicDxeFileGuid = { 0xde371f7c, 0xdec4, 0x4d21, { 0xad, 0xf1, 0x59, 0x3a, 0xbc, 0xc1, 0x58, 0x82 } } + [Ppis] ## Include/Ppi/ArmMpCoreInfo.h gArmMpCoreInfoPpiGuid = { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d, 0xab, 0x44, 0xe7, 0x54, 0xa8, 0xfc} } diff --git a/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c b/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c index bff8d983cf..e1adcd3bc6 100644 --- a/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c +++ b/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c @@ -121,6 +121,44 @@ RegisterInterruptSource ( } } +STATIC VOID *mCpuArchProtocolNotifyEventRegistration; + +STATIC +VOID +EFIAPI +CpuArchEventProtocolNotify ( + IN EFI_EVENT Event, + IN VOID *Context + ) +{ + EFI_CPU_ARCH_PROTOCOL *Cpu; + EFI_STATUS Status; + + // Get the CPU protocol that this driver requires. + Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&Cpu); + if (EFI_ERROR (Status)) { + return; + } + + // Unregister the default exception handler. + Status = Cpu->RegisterInterruptHandler (Cpu, ARM_ARCH_EXCEPTION_IRQ, NULL); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "%a: Cpu->RegisterInterruptHandler() - %r\n", + __FUNCTION__, Status)); + return; + } + + // Register to receive interrupts + Status = Cpu->RegisterInterruptHandler (Cpu, ARM_ARCH_EXCEPTION_IRQ, + Context); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "%a: Cpu->RegisterInterruptHandler() - %r\n", + __FUNCTION__, Status)); + } + + gBS->CloseEvent (Event); +} + EFI_STATUS InstallAndRegisterInterruptService ( IN EFI_HARDWARE_INTERRUPT_PROTOCOL *InterruptProtocol, @@ -130,7 +168,6 @@ InstallAndRegisterInterruptService ( ) { EFI_STATUS Status; - EFI_CPU_ARCH_PROTOCOL *Cpu; CONST UINTN RihArraySize = (sizeof(HARDWARE_INTERRUPT_HANDLER) * mGicNumInterrupts); @@ -152,27 +189,15 @@ InstallAndRegisterInterruptService ( return Status; } - // Get the CPU protocol that this driver requires. - Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&Cpu); - if (EFI_ERROR (Status)) { - return Status; - } - - // Unregister the default exception handler. - Status = Cpu->RegisterInterruptHandler (Cpu, ARM_ARCH_EXCEPTION_IRQ, NULL); - if (EFI_ERROR (Status)) { - return Status; - } - - // Register to receive interrupts - Status = Cpu->RegisterInterruptHandler ( - Cpu, - ARM_ARCH_EXCEPTION_IRQ, - InterruptHandler - ); - if (EFI_ERROR (Status)) { - return Status; - } + // + // Install the interrupt handler as soon as the CPU arch protocol appears. + // + EfiCreateProtocolNotifyEvent ( + &gEfiCpuArchProtocolGuid, + TPL_CALLBACK, + CpuArchEventProtocolNotify, + InterruptHandler, + &mCpuArchProtocolNotifyEventRegistration); // Register for an ExitBootServicesEvent Status = gBS->CreateEvent ( diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.h b/ArmPkg/Drivers/ArmGic/ArmGicDxe.h index 610ffacc7e..f6b75d7296 100644 --- 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 #include #include +#include #include #include diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf index d5921533fb..24b02ef30e 100644 --- a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf +++ b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf @@ -16,7 +16,7 @@ [Defines] INF_VERSION = 0x00010005 BASE_NAME = ArmGicDxe - FILE_GUID = DE371F7C-DEC4-4D21-ADF1-593ABCC15882 + FILE_GUID = DE371F7C-DEC4-4D21-ADF1-593ABCC15882 # gArmGicDxeFileGuid MODULE_TYPE = DXE_DRIVER VERSION_STRING = 1.0 @@ -45,6 +45,7 @@ UefiDriverEntryPoint IoLib PcdLib + UefiLib [Protocols] gHardwareInterruptProtocolGuid @@ -58,4 +59,4 @@ gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy [Depex] - gEfiCpuArchProtocolGuid + TRUE diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf index 0c5017b31e..b748f03445 100644 --- a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf +++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf @@ -74,4 +74,4 @@ gArmTokenSpaceGuid.PcdDebuggerExceptionSupport [Depex] - TRUE + AFTER gArmGicDxeFileGuid