From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E4FA6223FCF51 for ; Thu, 15 Mar 2018 12:41:39 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9AE3A406E971; Thu, 15 Mar 2018 19:48:03 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-121-233.rdu2.redhat.com [10.10.121.233]) by smtp.corp.redhat.com (Postfix) with ESMTP id 479901227721; Thu, 15 Mar 2018 19:48:02 +0000 (UTC) To: Leif Lindholm , Ard Biesheuvel Cc: edk2-devel@lists.01.org, marc.zyngier@arm.com, heyi.guo@linaro.org References: <20180315102826.10517-1-ard.biesheuvel@linaro.org> <20180315190148.gc74gfdnttvoy3i5@bivouac.eciton.net> From: Laszlo Ersek Message-ID: <9a1184f2-dede-eba8-1dc7-026bfb3bde01@redhat.com> Date: Thu, 15 Mar 2018 20:47:58 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180315190148.gc74gfdnttvoy3i5@bivouac.eciton.net> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Thu, 15 Mar 2018 19:48:03 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Thu, 15 Mar 2018 19:48:03 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH] ArmPkg/TimerDxe: remove workaround for KVM timer handling X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 15 Mar 2018 19:41:40 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 03/15/18 20:01, Leif Lindholm wrote: > On Thu, Mar 15, 2018 at 10:28:26AM +0000, Ard Biesheuvel wrote: >> When we first ported EDK2 to KVM/arm, we implemented a workaround for >> the quirky timer handling on the KVM side. This has been fixed in >> Linux commit f120cd6533d2 ("KVM: arm/arm64: timer: Allow the timer to >> control the active state") dated 23 June 2014, which was incorporated >> into Linux release 4.3. >> >> So almost 4 years later, it should be safe to drop this workaround on >> the EDK2 side. >> >> This reverts commit b1a633434ddc. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel > > I'm happy with this, with Marc's Ack. > Reviewed-by: Leif Lindholm > > However, if this can affect old kernels running in vms, could you ping > cross-distro@lists.linaro.org as well, so it doesn't catch anyone by > surprise? If I understand correctly, the patch could negatively affect old kernels that run on *hosts*. (Discounting nested virtualization, which I'm perfectly willing to ignore for this discussion.) Emailing the cross-distro list shouldn't hurt in any case. >>From my side (since I was CC'd): Acked-by: Laszlo Ersek Thanks! Laszlo > > / > Leif > >> --- >> ArmPkg/Drivers/TimerDxe/TimerDxe.c | 1 - >> ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.c | 10 ---------- >> 2 files changed, 11 deletions(-) >> >> diff --git a/ArmPkg/Drivers/TimerDxe/TimerDxe.c b/ArmPkg/Drivers/TimerDxe/TimerDxe.c >> index a3202fa056f3..bd616d2efc73 100644 >> --- a/ArmPkg/Drivers/TimerDxe/TimerDxe.c >> +++ b/ArmPkg/Drivers/TimerDxe/TimerDxe.c >> @@ -337,7 +337,6 @@ TimerInterruptHandler ( >> >> // Set next compare value >> ArmGenericTimerSetCompareVal (CompareValue); >> - ArmGenericTimerEnableTimer (); >> ArmInstructionSynchronizationBarrier (); >> } >> >> diff --git a/ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.c b/ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.c >> index 69a4ceb62db6..c941895a3574 100644 >> --- a/ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.c >> +++ b/ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.c >> @@ -26,16 +26,6 @@ ArmGenericTimerEnableTimer ( >> >> TimerCtrlReg = ArmReadCntvCtl (); >> TimerCtrlReg |= ARM_ARCH_TIMER_ENABLE; >> - >> - // >> - // When running under KVM, we need to unmask the interrupt on the timer side >> - // as KVM will mask it when servicing the interrupt at the hypervisor level >> - // and delivering the virtual timer interrupt to the guest. Otherwise, the >> - // interrupt will fire again, trapping into the hypervisor again, etc. etc. >> - // This is scheduled to be fixed on the KVM side, but there is no harm in >> - // leaving this in once KVM gets fixed. >> - // >> - TimerCtrlReg &= ~ARM_ARCH_TIMER_IMASK; >> ArmWriteCntvCtl (TimerCtrlReg); >> } >> >> -- >> 2.15.1 >>