(+ Eugene) On 5 August 2016 at 18:59, <evan.ll...@arm.com> wrote: > From: Alexei <alexei.fedo...@arm.com> > > This commit fixes a bug in the GIC v2 and v3 drivers where the GICC_EOIR > (End Of Interrupt Register) is written twice for a single interrupt. > GicV(2|3)IrqInterruptHandler() calls the Interrupt Handler and then > GicV(2|3)EndOfInterrupt() on exit: > > InterruptHandler = gRegisteredInterruptHandlers[GicInterrupt]; > if (InterruptHandler != NULL) { > // Call the registered interrupt handler. > InterruptHandler (GicInterrupt, SystemContext); > } else { > DEBUG ((EFI_D_ERROR, "Spurious GIC interrupt: 0x%x\n", GicInterrupt)); > } > > GicV2EndOfInterrupt (&gHardwareInterruptV2Protocol, GicInterrupt); > > , although gInterrupt->EndOfInterrupt() has already been called by > InterruptHandler(). > > The fix moves the EndOfInterrupt() call inside the else case for > unregistered/spurious interrupts. This removes a potential race > condition that might have lost interrupts. >
I understand that this solves the problem, but it does change the contract we have with registered interrupt handlers, and we don't know how this may be used out of tree. I know UEFI only supports polling for drivers, but are there any other cases (debug?) where we may break other people's code by doing this? > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Alexei Fedorov <alexei.fedo...@arm.com> > Signed-off-by: Evan Lloyd <evan.ll...@arm.com> > --- > > Code is available at: > https://github.com/EvanLloyd/tianocore/tree/EOIR_v1 > > ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c | 5 ++--- > ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 5 ++--- > 2 files changed, 4 insertions(+), 6 deletions(-) > > diff --git a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c > b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c > index > 036eb5cd6bf6845dd2b03b62c933c1dedaef7251..34d4be3867647e0fdad7356c949af8cd3ede7164 > 100644 > --- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c > +++ b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c > @@ -2,7 +2,7 @@ > > Copyright (c) 2009, Hewlett-Packard Company. All rights reserved.<BR> > Portions copyright (c) 2010, Apple Inc. All rights reserved.<BR> > -Portions copyright (c) 2011-2015, ARM Ltd. All rights reserved.<BR> > +Portions copyright (c) 2011-2016, ARM Ltd. All rights reserved.<BR> > > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD > License > @@ -178,9 +178,8 @@ GicV2IrqInterruptHandler ( > InterruptHandler (GicInterrupt, SystemContext); > } else { > DEBUG ((EFI_D_ERROR, "Spurious GIC interrupt: 0x%x\n", GicInterrupt)); > + GicV2EndOfInterrupt (&gHardwareInterruptV2Protocol, GicInterrupt); > } > - > - GicV2EndOfInterrupt (&gHardwareInterruptV2Protocol, GicInterrupt); > } > > // > diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c > b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c > index > 106c669fcb8777dfaad609c0ce9f5b572727a3ff..983936f3738a74bb5d5e08e012973df240958a8b > 100644 > --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c > +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c > @@ -1,6 +1,6 @@ > /** @file > * > -* Copyright (c) 2011-2015, ARM Limited. All rights reserved. > +* Copyright (c) 2011-2016, ARM Limited. All rights reserved. > * > * This program and the accompanying materials > * are licensed and made available under the terms and conditions of the BSD > License > @@ -169,9 +169,8 @@ GicV3IrqInterruptHandler ( > InterruptHandler (GicInterrupt, SystemContext); > } else { > DEBUG ((EFI_D_ERROR, "Spurious GIC interrupt: 0x%x\n", GicInterrupt)); > + GicV3EndOfInterrupt (&gHardwareInterruptV3Protocol, GicInterrupt); > } > - > - GicV3EndOfInterrupt (&gHardwareInterruptV3Protocol, GicInterrupt); > } > > // > -- > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759") > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel