From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from eu-smtp-delivery-143.mimecast.com (eu-smtp-delivery-143.mimecast.com [146.101.78.143]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 9D1381A1E18 for ; Mon, 8 Aug 2016 03:25:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=fSwcNjAllhNwbaRlU875MPA01GkSJld4UHSKNKbuWRg=; b=NDZadbL3fThjS1PJOBFU90p4+8/tAwzgVcLJGl2nHD+cP5tqNnc7F1wbc8kCMRY4g6gQNTLZQhpG30VN1HhEekUgeABlLQZwbVXgAQ66yNKYKGOCLkbi0e2w5SC4w4H99tSWgba+lzwpAQKwcNHXPEUqvxRVPMp7lyAEHV57TFc= Received: from EUR03-AM5-obe.outbound.protection.outlook.com (mail-am5eur03lp0113.outbound.protection.outlook.com [213.199.154.113]) (Using TLS) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-39-HvHyKQM_NmSR3KulXwQrdA-1; Mon, 08 Aug 2016 11:25:52 +0100 Received: from AM5PR0801MB1955.eurprd08.prod.outlook.com (10.168.157.151) by AM5PR0801MB1761.eurprd08.prod.outlook.com (10.169.247.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.549.15; Mon, 8 Aug 2016 10:25:50 +0000 Received: from AM5PR0801MB1955.eurprd08.prod.outlook.com ([10.168.157.151]) by AM5PR0801MB1955.eurprd08.prod.outlook.com ([10.168.157.151]) with mapi id 15.01.0549.025; Mon, 8 Aug 2016 10:25:50 +0000 From: Alexei Fedorov To: Ard Biesheuvel , Evan Lloyd , "Cohen, Eugene" CC: "edk2-devel@lists.01.org" , Heyi Guo , Leif Lindholm Thread-Topic: [edk2] [PATCH] ArmPkg: Fix double GIC EIOR write per interrupt Thread-Index: AQHR7zq3UpCuqC9RX0qeB/X+2RhBt6A7mYSAgANFQnA= Date: Mon, 8 Aug 2016 10:25:50 +0000 Message-ID: References: <20160805165911.14744-1-evan.lloyd@arm.com> In-Reply-To: Accept-Language: en-GB, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [217.140.96.140] x-ms-office365-filtering-correlation-id: 7058f624-1702-4898-b89e-08d3bf765ea0 x-microsoft-exchange-diagnostics: 1; AM5PR0801MB1761; 20:0cOSpSj6z2p/miyZAET54uzsHS0k/bBFt5S94a1xgu6lebdZF/oRSioqnPaw2DV/rwGD/FB0csPAFeM5WZIde8rQWGRbjXJ4OP8GCjuoHh89IY29QnyDjXrp5Z0MazFJZ8e+AyTytFg/PYRByknplwM7QPblnYHV9SWJEMHTBLU= x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AM5PR0801MB1761; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(180628864354917)(166708455590820)(162533806227266)(17755550239193); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026); SRVR:AM5PR0801MB1761; BCL:0; PCL:0; RULEID:; SRVR:AM5PR0801MB1761; x-forefront-prvs: 00286C0CA6 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(7916002)(199003)(189002)(13464003)(24454002)(40434004)(575784001)(5001770100001)(8666005)(305945005)(101416001)(7846002)(7696003)(7736002)(87936001)(86362001)(68736007)(97736004)(5890100001)(6116002)(105586002)(102836003)(50986999)(3846002)(3660700001)(54356999)(76176999)(9686002)(92566002)(66066001)(19580405001)(74316002)(19580395003)(4326007)(8676002)(189998001)(122556002)(5002640100001)(77096005)(2906002)(15975445007)(106116001)(33656002)(11100500001)(76576001)(2900100001)(586003)(10400500002)(3280700002)(8936002)(2950100001)(81166006)(81156014)(106356001)(7059030)(217873001)(19627235001); DIR:OUT; SFP:1101; SCL:1; SRVR:AM5PR0801MB1761; H:AM5PR0801MB1955.eurprd08.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Aug 2016 10:25:50.2017 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0801MB1761 X-MC-Unique: HvHyKQM_NmSR3KulXwQrdA-1 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: Mon, 08 Aug 2016 10:25:56 -0000 Content-Language: en-US Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable > it does change the contract we have with registered interrupt handlers Looks like it does not: >>From edk2\EmbeddedPkg\Include\Protocol\HardwareInterrupt.h: " Abstraction for hardware based interrupt routine ...The driver implementing this protocol is responsible for clearing the pending interrupt in the interrupt routing hardware. The HARDWARE_INTERRUPT_HANDLER is responsible for clearing interrupt sources from individual devices." -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard = Biesheuvel Sent: 06 August 2016 09:25 To: Evan Lloyd; Cohen, Eugene Cc: edk2-devel@lists.01.org; Heyi Guo; Leif Lindholm Subject: Re: [edk2] [PATCH] ArmPkg: Fix double GIC EIOR write per interrupt (+ 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 inter= rupt. > GicV(2|3)IrqInterruptHandler() calls the Interrupt Handler and then > GicV(2|3)EndOfInterrupt() on exit: > > InterruptHandler =3D gRegisteredInterruptHandlers[GicInterrupt]; > if (InterruptHandler !=3D 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 d= oing 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..34d4be3867647e0fdad7356c949a > f8cd3ede7164 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..983936f3738a74bb5d5e08e01297 > 3df240958a8b 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 IMPORTANT NOTICE: The contents of this email and any attachments are confid= ential and may also be privileged. If you are not the intended recipient, p= lease notify the sender immediately and do not disclose the contents to any= other person, use it for any purpose, or store or copy the information in = any medium. Thank you.