From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c0c::244; helo=mail-wr0-x244.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wr0-x244.google.com (mail-wr0-x244.google.com [IPv6:2a00:1450:400c:c0c::244]) (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 DEE8C20954BA9 for ; Fri, 16 Mar 2018 04:40:04 -0700 (PDT) Received: by mail-wr0-x244.google.com with SMTP id m12so11337651wrm.13 for ; Fri, 16 Mar 2018 04:46:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=laD8mu9v6xO0Fv70/V1uh2v1Sa4t1UPK1abovJo17YA=; b=Xe2YGbQhXvCJ5jM5c0qZ+sUndNJzfJnWZ1EiZ/p04rZLhoeIDXEqJRU7gYCb5T07bg vc7ZMdFG/6+q0UXqCI+SIFYI5bCkRyrP8xfLTdd2OPgdmPIpDaxk0vHOMV+wqUOGZh8h PE886LTVQDLaE2iqefQDwxxzvMMt8JkNeEbG0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=laD8mu9v6xO0Fv70/V1uh2v1Sa4t1UPK1abovJo17YA=; b=Nz9C/QzJElsYwws8FjFd6apyV14gcciVY2m+u5MVPzEqUa8c6OBh3sxc6R08LEQ74V 5+VqJ5Oc0RXlAxHQM/rE8grlMDjl9mUA+dZhlj70NYyrK0snK0DxXZHc0qBmQhMvQ4Xe Ljd7GYBDWVxCXG4PktHWd3oONb6VxJeNkCffTqDk+Vtxa2T+ucM8VvnLnE96vr6UIo6b /JgYjgVZ2y5ECtLgCtZzOBC2AaK8eiwKVBCt2QHMhNQW3KiH1x4MS9EZcp0y2JKdI3O4 h/XVQevgSQAtrSAeF4qX6n7bXXZN8pEQ+ccMwIPvmYon2LsTSPHWp60pPYzS/GNRPFRi eL+Q== X-Gm-Message-State: AElRT7FR2w99/KnA6fIR2CH9nq149XlD3ONgYrTgXdrHGq890tvBpj20 BmbPINX07HV/EdvBVzuZn//71g== X-Google-Smtp-Source: AG47ELtbJC7eazOScpYvGW1j1F7OUNOQrTlSEDs0HUOq/AgLFKNP/4qfRCAunuEtLOZcBjVqFPv3DQ== X-Received: by 10.223.166.79 with SMTP id k73mr1444872wrc.200.1521200788256; Fri, 16 Mar 2018 04:46:28 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id v22sm4990756wrd.43.2018.03.16.04.46.27 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 16 Mar 2018 04:46:27 -0700 (PDT) Date: Fri, 16 Mar 2018 11:46:25 +0000 From: Leif Lindholm To: Ard Biesheuvel Cc: "edk2-devel@lists.01.org" , Laszlo Ersek , Marc Zyngier , Heyi Guo Message-ID: <20180316114625.zbdwpqjskgpl3iwp@bivouac.eciton.net> References: <20180315102826.10517-1-ard.biesheuvel@linaro.org> <20180315190148.gc74gfdnttvoy3i5@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) 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: Fri, 16 Mar 2018 11:40:05 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Mar 16, 2018 at 11:28:21AM +0000, Ard Biesheuvel wrote: > On 15 March 2018 at 19: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? > > It will affects VMs running new firmware on ancient host kernels (and > v4.2 *is* ancient when it comes to KVM/arm64 and server stuff imo) Oh, it is, but if we have https://bugs.launchpad.net/ubuntu/+source/edk2/+bug/1744754 (Xenial guest), imagining people running a Zesty host isn't that much more of a stretch (it's only EOL 2 months ago). / 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 > >>