* [PATCH] ArmPkg/Gic: force GIC driver to run before CPU arch protocol driver
@ 2018-02-06 12:04 Ard Biesheuvel
2018-02-06 13:01 ` Leif Lindholm
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2018-02-06 12:04 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: marc.zyngier, Ard Biesheuvel
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>
---
This fixes an issue observed with GICv3 guests running under KVM.
ArmPkg/ArmPkg.dec | 2 +
ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c | 69 +++++++++++++-------
ArmPkg/Drivers/ArmGic/ArmGicDxe.h | 1 +
ArmPkg/Drivers/ArmGic/ArmGicDxe.inf | 5 +-
ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 2 +-
5 files changed, 54 insertions(+), 25 deletions(-)
diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index 5dbd019e2966..a55b6268ff85 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -48,6 +48,8 @@ [Guids.common]
# 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 bff8d983cf02..e1adcd3bc6d3 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 610ffacc7eb0..f6b75d729601 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 <Library/IoLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
#include <Protocol/Cpu.h>
#include <Protocol/HardwareInterrupt.h>
diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
index d5921533fb68..24b02ef30e83 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 @@ [LibraryClasses]
UefiDriverEntryPoint
IoLib
PcdLib
+ UefiLib
[Protocols]
gHardwareInterruptProtocolGuid
@@ -58,4 +59,4 @@ [Pcd.common]
gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy
[Depex]
- gEfiCpuArchProtocolGuid
+ TRUE
diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
index d068e06803ed..cda549922e9c 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
@@ -76,4 +76,4 @@ [FeaturePcd.common]
gArmTokenSpaceGuid.PcdDebuggerExceptionSupport
[Depex]
- TRUE
+ AFTER gArmGicDxeFileGuid
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPkg/Gic: force GIC driver to run before CPU arch protocol driver
2018-02-06 12:04 [PATCH] ArmPkg/Gic: force GIC driver to run before CPU arch protocol driver Ard Biesheuvel
@ 2018-02-06 13:01 ` Leif Lindholm
2018-02-06 14:55 ` Marc Zyngier
2018-03-21 13:15 ` Shannon Zhao
2 siblings, 0 replies; 5+ messages in thread
From: Leif Lindholm @ 2018-02-06 13:01 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, marc.zyngier
On Tue, Feb 06, 2018 at 12:04:16PM +0000, Ard Biesheuvel wrote:
> 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>
> ---
>
> This fixes an issue observed with GICv3 guests running under KVM.
>
> ArmPkg/ArmPkg.dec | 2 +
> ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c | 69 +++++++++++++-------
> ArmPkg/Drivers/ArmGic/ArmGicDxe.h | 1 +
> ArmPkg/Drivers/ArmGic/ArmGicDxe.inf | 5 +-
> ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 2 +-
> 5 files changed, 54 insertions(+), 25 deletions(-)
>
> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> index 5dbd019e2966..a55b6268ff85 100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -48,6 +48,8 @@ [Guids.common]
> # 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 bff8d983cf02..e1adcd3bc6d3 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 610ffacc7eb0..f6b75d729601 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 <Library/IoLib.h>
> #include <Library/MemoryAllocationLib.h>
> #include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
>
> #include <Protocol/Cpu.h>
> #include <Protocol/HardwareInterrupt.h>
> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
> index d5921533fb68..24b02ef30e83 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 @@ [LibraryClasses]
> UefiDriverEntryPoint
> IoLib
> PcdLib
> + UefiLib
>
> [Protocols]
> gHardwareInterruptProtocolGuid
> @@ -58,4 +59,4 @@ [Pcd.common]
> gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy
>
> [Depex]
> - gEfiCpuArchProtocolGuid
> + TRUE
> diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
> index d068e06803ed..cda549922e9c 100644
> --- a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
> +++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
> @@ -76,4 +76,4 @@ [FeaturePcd.common]
> gArmTokenSpaceGuid.PcdDebuggerExceptionSupport
>
> [Depex]
> - TRUE
> + AFTER gArmGicDxeFileGuid
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPkg/Gic: force GIC driver to run before CPU arch protocol driver
2018-02-06 12:04 [PATCH] ArmPkg/Gic: force GIC driver to run before CPU arch protocol driver Ard Biesheuvel
2018-02-06 13:01 ` Leif Lindholm
@ 2018-02-06 14:55 ` Marc Zyngier
2018-02-06 19:01 ` Ard Biesheuvel
2018-03-21 13:15 ` Shannon Zhao
2 siblings, 1 reply; 5+ messages in thread
From: Marc Zyngier @ 2018-02-06 14:55 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel, leif.lindholm
On 06/02/18 12:04, Ard Biesheuvel wrote:
> 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>
> ---
>
> This fixes an issue observed with GICv3 guests running under KVM.
This fixes the problem I was seeing, so here's my:
Tested-by: Marc Zyngier <marc.zyngier@arm.com>
Thanks a lot Ard!
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPkg/Gic: force GIC driver to run before CPU arch protocol driver
2018-02-06 14:55 ` Marc Zyngier
@ 2018-02-06 19:01 ` Ard Biesheuvel
0 siblings, 0 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2018-02-06 19:01 UTC (permalink / raw)
To: Marc Zyngier; +Cc: edk2-devel@lists.01.org, Leif Lindholm
On 6 February 2018 at 14:55, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 06/02/18 12:04, Ard Biesheuvel wrote:
>> 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>
>> ---
>>
>> This fixes an issue observed with GICv3 guests running under KVM.
>
> This fixes the problem I was seeing, so here's my:
>
> Tested-by: Marc Zyngier <marc.zyngier@arm.com>
>
> Thanks a lot Ard!
>
Pushed as 61a7b0ec634fa3288f47929ba3ced05ff48de739
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPkg/Gic: force GIC driver to run before CPU arch protocol driver
2018-02-06 12:04 [PATCH] ArmPkg/Gic: force GIC driver to run before CPU arch protocol driver Ard Biesheuvel
2018-02-06 13:01 ` Leif Lindholm
2018-02-06 14:55 ` Marc Zyngier
@ 2018-03-21 13:15 ` Shannon Zhao
2 siblings, 0 replies; 5+ messages in thread
From: Shannon Zhao @ 2018-03-21 13:15 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel, leif.lindholm
Hi Ard,
On 2018/2/6 20:04, Ard Biesheuvel wrote:
> 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>
> ---
>
> This fixes an issue observed with GICv3 guests running under KVM.
I'm backporting this patch to the branch UDK2017 with three extra patches.
3ba3528 ArmPkg/Gic: force GIC driver to run before CPU arch protocol driver
c397d52 ArmPkg/ArmGicDxe: Expose HardwareInterrupt2 protocol
9a73ffd EmbeddedPkg: Introduce HardwareInterrupt2 protocol
29d33ba ArmPkg: Tidy GIC code before changes.
But when I start a VM using the new edk2 binary, it throws below error.
ASSERT_EFI_ERROR (Status = Device Error)
ASSERT [Shell]
/root/rpmbuild/BUILD/edk2-2.7.0/Build/ArmVirtQemu-AARCH64/RELEASE_GCC49/AARCH64/ShellPkg/Application/Shell/Shell/DEBUG/AutoGen.c(885):
!EFI_ERROR (Status)
Synchronous Exception at 0x000000023866A1F0
X0 0x00000000000000FF X1 0x000000000000000A X2 0x00000000000000AB
X3 0x000000023F2A2B80
X4 0x0000000000000001 X5 0x0000000000000001 X6 0x0000000000000000
X7 0x0000000000000002
X8 0x0000000000000000 X9 0x0000000700000000 X10 0x0000000238B60000
X11 0x000000023AD86FFF
X12 0x0000000000000000 X13 0x0000000000000001 X14 0x0000000000000000
X15 0x0000000000000000
X16 0x0000000000000000 X17 0x0000000000672E50 X18 0x0000000000672E50
X19 0x000000023B2ECE98
X20 0x000000023BFF0018 X21 0x8000000000000007 X22 0x000000023F2CB088
X23 0x0000000000000000
X24 0x000000023B2ED438 X25 0x000000023B2ED440 X26 0x000000023F2CA3D8
X27 0x0000000000000095
X28 0x000000004007E010 FP 0x0000000000000000 LR 0x000000023861E844
V0 0x0000000000000000 0000000000000000 V1 0x63702F6666666666
6666666666666666
V2 0x7363732F312C3140 6567646972622D69 V3 0x0000000000000000
0000000000000000
V4 0x0000000000100000 0000000000000000 V5 0x4010040140100401
4010040140100401
V6 0x0010000000000000 0010000000000000 V7 0x0000000000000000
0000000000000000
V8 0x0000000000000000 0000000000000000 V9 0x0000000000000000
0000000000000000
V10 0x0000000000000000 0000000000000000 V11 0x0000000000000000
0000000000000000
V12 0x0000000000000000 0000000000000000 V13 0x0000000000000000
0000000000000000
V14 0x0000000000000000 0000000000000000 V15 0x0000000000000000
0000000000000000
V16 0x0000000000000000 0000000000000000 V17 0x0000000000000000
0000000000000000
V18 0x0000000000000000 0000000000000000 V19 0x0000000000000000
0000000000000000
V20 0x0000000000000000 0000000000000000 V21 0x0000000000000000
0000000000000000
V22 0x0000000000000000 0000000000000000 V23 0x0000000000000000
0000000000000000
V24 0x0000000000000000 0000000000000000 V25 0x0000000000000000
0000000000000000
V26 0x0000000000000000 0000000000000000 V27 0x0000000000000000
0000000000000000
V28 0x0000000000000000 0000000000000000 V29 0x0000000000000000
0000000000000000
V30 0x0000000000000000 0000000000000000 V31 0x0000000000000000
0000000000000000
SP 0x000000023F2A2B70 ELR 0x000000023866A1F0 SPSR 0x60000305 FPSR
0x00000000
ESR 0x5600DBDB FAR 0x1DE7EC7EDBADC0DE
ESR : EC 0x15 IL 0x1 ISS 0x0000DBDB
SVC executed in AArch64
Stack dump:
000023F2A2A70: 000000023861E820 000000023BFF0018 000000023F2A2B70
000000023F2A2B70
000023F2A2A90: 000000023F2A2B40 FFFFFF80FFFFFFD8 000000023F2A2B70
000000023F2A2B70
000023F2A2AB0: 000000023F2A2B40 FFFFFF80FFFFFFD8 0000000000000000
0000000000000000
000023F2A2AD0: 6666666666666666 63702F6666666666 6567646972622D69
7363732F312C3140
000023F2A2AF0: 0000000000000000 0000000000000000 0000000000000000
0000000000100000
000023F2A2B10: 4010040140100401 4010040140100401 0010000000000000
0010000000000000
000023F2A2B30: 000000023F2A2C2A 000000023866A74C 000000023B2ECE98
000000023BFF0018
000023F2A2B50: 8000000000000007 000000023F2CB088 0000000000000000
000000023861E834
> 000023F2A2B70: 000000023860B4A4 0010000000000000 5B20545245535341
2F205D6C6C656853
000023F2A2B90: 6D70722F746F6F72 55422F646C697562 326B64652F444C49
422F302E372E322D
000023F2A2BB0: 6D72412F646C6975 756D655174726956 343648435241412D
455341454C45522F
000023F2A2BD0: 412F39344343475F 532F343648435241 2F676B506C6C6568
746163696C707041
000023F2A2BF0: 6C6568532F6E6F69 2F6C6C6568532F6C 75412F4755424544
28632E6E65476F74
000023F2A2C10: 4521203A29353838 524F5252455F4946 7375746174532820
40100401000A0D29
000023F2A2C30: 0010000000000000 0010000000000000 0000000000000000
0000000000000000
000023F2A2C50: 8000000000000007 46987762DD9E7534 AA25A61785F5148C
000000023F2A2C6B
Thanks,
--
Shannon
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-03-21 13:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-06 12:04 [PATCH] ArmPkg/Gic: force GIC driver to run before CPU arch protocol driver Ard Biesheuvel
2018-02-06 13:01 ` Leif Lindholm
2018-02-06 14:55 ` Marc Zyngier
2018-02-06 19:01 ` Ard Biesheuvel
2018-03-21 13:15 ` Shannon Zhao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox