From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x233.google.com (mail-it0-x233.google.com [IPv6:2607:f8b0:4001:c0b::233]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8D0931A1DEF for ; Sat, 6 Aug 2016 01:24:46 -0700 (PDT) Received: by mail-it0-x233.google.com with SMTP id j124so43865369ith.1 for ; Sat, 06 Aug 2016 01:24:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=f1Y3+ii2SeiCt4feyk6QooJt+CjyaxYbJbKqxJUu8yw=; b=joBHs8TqchC5o9U57uNyktjvrGNDTkpLvotS45N5I/q8X+zQrXTB7ovYqxt73hE/Sg btZUTxrNHdkTUic4PMDbuSIyBzHaeY86xWknoEgukX7Ao+N4KmPUGtHL56u6G+RhV13Z B7FTf+C6sW2kyAd99vJ1Ymn5Ns5UItR5fzdf4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=f1Y3+ii2SeiCt4feyk6QooJt+CjyaxYbJbKqxJUu8yw=; b=nKZ0g/+149QPshSpYE02GZCBfs7ZAjxpkEgeWzcGg1yUyZWJo6gxqfe9aey5oSE/pD ZsrjR2SCzlk5jX5CUFomiBd2MV5CAh2pbSBmx+yBMCrfsTLzTRc2E+4PAuy0TOjyJoYJ Dw2wTYVHibJ6NqgCVTjKMg533I7h0IEzGZxzgPZ47Plb9dF3NVjkLjPqfPeBQRq7u++6 /NrKKgFCyYPtMh2bdPokGzIZclN7/B3WAw8XxJTHyn1M6T2V/nPFDbaj4I9peKxlUTcc tHTkq8vygQ6LX57+1Fu7KZiBk7rHSp4a7IcNBpFjfN8IcDP84VHyYC4VV84VYw+gfzUr rYvw== X-Gm-Message-State: AEkoouvcGPEiRthDK/UMa/FJCgQu0fb8pNexcjerjvpIHepfTY9SjhKKPJBINkvwTmgyvvEr3gLYfzXDgt7ssP86 X-Received: by 10.36.107.211 with SMTP id v202mr8305856itc.51.1470471885760; Sat, 06 Aug 2016 01:24:45 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.204.195 with HTTP; Sat, 6 Aug 2016 01:24:45 -0700 (PDT) In-Reply-To: <20160805165911.14744-1-evan.lloyd@arm.com> References: <20160805165911.14744-1-evan.lloyd@arm.com> From: Ard Biesheuvel Date: Sat, 6 Aug 2016 10:24:45 +0200 Message-ID: To: Evan Lloyd , "Cohen, Eugene" Cc: "edk2-devel@lists.01.org" , Leif Lindholm , Heyi Guo Subject: Re: [PATCH] ArmPkg: Fix double GIC EIOR write per interrupt X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 06 Aug 2016 08:24:46 -0000 Content-Type: text/plain; charset=UTF-8 (+ Eugene) On 5 August 2016 at 18:59, wrote: > From: Alexei > > 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 > Signed-off-by: Evan Lloyd > --- > > 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.
> Portions copyright (c) 2010, Apple Inc. All rights reserved.
> -Portions copyright (c) 2011-2015, ARM Ltd. All rights reserved.
> +Portions copyright (c) 2011-2016, ARM Ltd. All rights reserved.
> > 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") >