diff options
author | Ard Biesheuvel <ardb@kernel.org> | 2021-08-24 17:09:04 +0200 |
---|---|---|
committer | mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> | 2021-08-27 12:53:36 +0000 |
commit | a82bad9730178a1e3a67c9bfc83412b87a8ad734 (patch) | |
tree | 09593954991113c0d9a3e22193e167c233b2d078 /ArmPkg/Drivers | |
parent | b9af5037b270c4767b275fd5d23b942c422e742f (diff) | |
download | edk2-a82bad9730178a1e3a67c9bfc83412b87a8ad734.zip edk2-a82bad9730178a1e3a67c9bfc83412b87a8ad734.tar.gz edk2-a82bad9730178a1e3a67c9bfc83412b87a8ad734.tar.bz2 |
ArmPkg/GicV3Dxe: Don't signal EOI on arbitrary interrupts
Currently, at ExitBootServices() time, the GICv3 driver signals
End-Of-Interrupt (EOI) on all interrupt lines that are supported by the
interrupt controller. This appears to have been carried over from the
GICv2 version, but has been turned into something that violates the GIC
spec, and may trigger SError exceptions on some implementations.
Marc puts it as follows:
The GIC interrupt state machine is pretty strict. An interrupt can
only be deactivated (with or without prior priority drop) if it has
been acknowledged first. In GIC speak, this means that only the
following sequences are valid:
With EOImode==0:
x = ICC_IAR{0,1}_EL1;
ICC_EOIR{0,1}_EL1 = x;
With EOImode==1:
x = ICC_IAR{0,1}_EL1;
ICC_EOIR{0,1}_EL1 = x;
ICC_DIR_EL1 = x;
Any write to ICC_EOIR{0,1}_EL1 that isn't the direct consequence of
the same value being read from ICC_IAR{0,1}_EL1, and with the correct
nesting, breaks the state machine and leads to unpredictable results
that affects *all* interrupts in the system (most likely, the priority
system is dead). See Figure 4-3 ("Interrupt handling state machine")
in Arm IHI 0069F for a description of the acceptable transitions.
Additionally, on implementations that have ICC_CTLR_EL1.SEIS==1, a
SError may be generated to signal the error. See the various
<quote>
IMPLEMENTATION_DEFINED "SError ....";
</quote>
that are all over the pseudocode contained in the same architecture
spec. Needless to say, this is pretty final for any SW that would do
silly things on such implementations (which do exist).
Given that in our implementation, every signalled interrupt is acked,
handled and EOId in sequence, there is no reason to EOI all interrupts
at ExitBootServices() time in the first place, so let's just drop this
code. This fixes an issue reported by Marc where an SError is triggered
by this code, bringing down the system.
Reported-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Marc Zyngier <maz@kernel.org>
Tested-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Diffstat (limited to 'ArmPkg/Drivers')
-rw-r--r-- | ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 4 |
1 files changed, 0 insertions, 4 deletions
diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c index 85ee4c8..fa515d1 100644 --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c @@ -344,10 +344,6 @@ GicV3ExitBootServicesEvent ( GicV3DisableInterruptSource (&gHardwareInterruptV3Protocol, Index);
}
- for (Index = 0; Index < mGicNumInterrupts; Index++) {
- GicV3EndOfInterrupt (&gHardwareInterruptV3Protocol, Index);
- }
-
// Disable Gic Interface
ArmGicV3DisableInterruptInterface ();
|