Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752049AbdLMJIl (ORCPT ); Wed, 13 Dec 2017 04:08:41 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59192 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751853AbdLMJIa (ORCPT ); Wed, 13 Dec 2017 04:08:30 -0500 Subject: Re: [PATCH] KVM: arm/arm64: don't set vtimer->cnt_ctl in kvm_arch_timer_handler To: Marc Zyngier , Jia He , Christoffer Dall , linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu References: <1513148407-2611-1-git-send-email-hejianet@gmail.com> Cc: Jia He , linux-kernel@vger.kernel.org From: Auger Eric Message-ID: Date: Wed, 13 Dec 2017 10:08:25 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Wed, 13 Dec 2017 09:08:29 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2051 Lines: 64 Marc, On 13/12/17 09:56, Marc Zyngier wrote: > Hi Jia, > > On 13/12/17 07:00, Jia He wrote: >> In our Armv8a server (qualcomm Amberwing, non VHE), after applying >> Christoffer's timer optimizing patchset(Optimize arch timer register >> handling), the guest is hang during kernel booting. >> >> The error root cause might be as follows: >> 1. in kvm_arch_timer_handler, it reset vtimer->cnt_ctl with current >> cntv_ctl register value. And then it missed some cases to update timer's >> irq (irq.level) when kvm_timer_irq_can_fire() is false >> 2. It causes kvm_vcpu_check_block return 0 instead of -EINTR >> kvm_vcpu_check_block >> kvm_cpu_has_pending_timer >> kvm_timer_is_pending >> kvm_timer_should_fire >> 3. Thus, the kvm hyp code can not break the loop in kvm_vcpu_block (halt >> poll process) and the guest is hang forever >> >> Fixes: b103cc3f10c0 ("KVM: arm/arm64: Avoid timer save/restore in vcpu entry/exit") >> Signed-off-by: Jia He >> --- >> virt/kvm/arm/arch_timer.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c >> index f9555b1..bb86433 100644 >> --- a/virt/kvm/arm/arch_timer.c >> +++ b/virt/kvm/arm/arch_timer.c >> @@ -100,7 +100,6 @@ static irqreturn_t kvm_arch_timer_handler(int irq, void *dev_id) >> vtimer = vcpu_vtimer(vcpu); >> >> if (!vtimer->irq.level) { >> - vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl); >> if (kvm_timer_irq_can_fire(vtimer)) >> kvm_timer_update_irq(vcpu, true, vtimer); >> } >> > > Which patches are you looking at? The current code in mainline looks > like this: > > vtimer = vcpu_vtimer(vcpu); > > vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl); > if (kvm_timer_irq_can_fire(vtimer)) > kvm_timer_update_irq(vcpu, true, vtimer); > > I'd suggest you use mainline and report if this doesn't work the removal of if (!vtimer->irq.level) test happened in: [PATCH v7 3/8] KVM: arm/arm64: Don't cache the timer IRQ level which is not upstream. Thanks Eric > > Thanks, > > M. >