From patchwork Mon Aug 8 11:51:20 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 73448 Delivered-To: patch@linaro.org Received: by 10.140.29.52 with SMTP id a49csp3173097qga; Mon, 8 Aug 2016 04:51:23 -0700 (PDT) X-Received: by 10.66.119.11 with SMTP id kq11mr50755814pab.14.1470657083687; Mon, 08 Aug 2016 04:51:23 -0700 (PDT) Return-Path: Received: from ml01.01.org (ml01.01.org. [2001:19d0:306:5::1]) by mx.google.com with ESMTPS id pw10si36701685pac.54.2016.08.08.04.51.23 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 08 Aug 2016 04:51:23 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 2001:19d0:306:5::1 as permitted sender) client-ip=2001:19d0:306:5::1; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 2001:19d0:306:5::1 as permitted sender) smtp.mailfrom=edk2-devel-bounces@lists.01.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 467061A1E24; Mon, 8 Aug 2016 04:51:23 -0700 (PDT) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mail-it0-x22a.google.com (mail-it0-x22a.google.com [IPv6:2607:f8b0:4001:c0b::22a]) (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 0492B1A1E18 for ; Mon, 8 Aug 2016 04:51:22 -0700 (PDT) Received: by mail-it0-x22a.google.com with SMTP id u186so71464131ita.0 for ; Mon, 08 Aug 2016 04:51:21 -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=utm6CeCI5T5BvMFxcFFDpFw+EfnGwaCxPH1nvLdmQmg=; b=OYoABYM/wfeioVDLGyvHmQWL1yu+p59npjzuCx8vn9NfZDHFyfL0a8n0VdvRcjWHc0 qF3BR5/Og1zRTzzDP6sh1ycCYbto3rQoExc3PVtPltCIUN3jjFt++UipJy4lWf4GMrCc bsJ98KKkGBmDi7r/+U77/OcmrzFCFNlLDofkY= 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=utm6CeCI5T5BvMFxcFFDpFw+EfnGwaCxPH1nvLdmQmg=; b=jZMffiNATFFZHWEvb3kPJrjsTDtSE9e1PzDew3i3kPsYPAnc5TLLzBydrEPhTvTeVG qyFExVrZw3akwuBnAr4ntJ2NfUXZL+5cpO+/vmE59RT4mQX6Xv0xSI+nzYt1oiT5VMy9 RU9bT9MhE43kxyI3yKBZWNYDImeiG+fRmh5x9fgBs27qCYuxRmqF9ZuhsGd12UyrUdoK S04FUBT03c7QyDWSBjPjloH0vvcaj8Qm9Vw6oUL3eB5kXn0MBUGcUppCXgQwCCrl304H 0HXTcda73tHT42HWj1/oV2WUZdgp0Ef+tXTaMBuUvk8R0ZvYh9ShphnPxvZkVbC0CCBS SjXw== X-Gm-Message-State: AEkoouupJaiLkEnScGfYNH5FPJx2ncxayESAGfx3pIs+uNcGTqkxJjPn+e7x+NUvk1oyEwF3+CFUZPisndVKbQM0 X-Received: by 10.36.57.215 with SMTP id l206mr18420893ita.5.1470657081240; Mon, 08 Aug 2016 04:51:21 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.204.195 with HTTP; Mon, 8 Aug 2016 04:51:20 -0700 (PDT) In-Reply-To: References: <20160805165911.14744-1-evan.lloyd@arm.com> From: Ard Biesheuvel Date: Mon, 8 Aug 2016 13:51:20 +0200 Message-ID: To: Alexei Fedorov Subject: Re: [edk2] [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: , Cc: "edk2-devel@lists.01.org" , Heyi Guo , Leif Lindholm , "Cohen, Eugene" Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" On 8 August 2016 at 13:08, Ard Biesheuvel wrote: > On 8 August 2016 at 13:06, Alexei Fedorov wrote: >> Timer's pending interrupt is cleared HARDWARE_INTERRUPT_HANDLER TimerInterruptHandler() in when timer is re-programmed for the next shot. > > Yes, that is the timer side. > >> Does it mean that TimerDxe driver is part EFI_HARDWARE_INTERRUPT_PROTOCOL? >> > > No. The peripheral and the GIC each have their own mask and enable > registers for the interrupt. That is exactly why the comment describes > in detail which part is the responsibility of the GIC, and which is > the responsibility of the peripheral. > ... and actually, looking at TimerInterruptHandler (), I don't see the point of re-enabling the interrupt early, given that 308: OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL); disables interrupts globally, and only re-enables them on line 346, at which point the mTimerNotifyFunction() has already returned. So I propose we simply do mTimerNotifyFunction (mTimerPeriod * mElapsedPeriod); so that if the condition exists that we know will trigger the interrupt immediately as soon as we unmask it, we simply enter the loop again just like we would when taking the [nested] interrupt. @Heyi: any thoughts? -- Ard. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel diff --git a/ArmPkg/Drivers/TimerDxe/TimerDxe.c b/ArmPkg/Drivers/TimerDxe/TimerDxe.c index 1169d426b255..f0fcb05757ac 100644 --- a/ArmPkg/Drivers/TimerDxe/TimerDxe.c +++ b/ArmPkg/Drivers/TimerDxe/TimerDxe.c @@ -308,10 +308,7 @@ TimerInterruptHandler ( OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL); // Check if the timer interrupt is active - if ((ArmGenericTimerGetTimerCtrlReg () ) & ARM_ARCH_TIMER_ISTATUS) { - - // Signal end of interrupt early to help avoid losing subsequent ticks from long duration handlers - gInterrupt->EndOfInterrupt (gInterrupt, Source); + while ((ArmGenericTimerGetTimerCtrlReg () ) & ARM_ARCH_TIMER_ISTATUS) { if (mTimerNotifyFunction) {