From patchwork Tue Jan 30 12:42:04 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 10191787 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 5A6E660375 for ; Tue, 30 Jan 2018 12:43:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4B0EB20174 for ; Tue, 30 Jan 2018 12:43:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3F17D28903; Tue, 30 Jan 2018 12:43:04 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id BBC0420174 for ; Tue, 30 Jan 2018 12:43:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:References: In-Reply-To:Message-Id:Date:Subject:To:From:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=WKaI0RGqiYycENW+PSKcvX5nnKOwiUW00cNQmilZtVg=; b=Jhddlcan63zyQhzVLGuhxC8zWN rwUZYT+4LBuGbzFu8ia3v63OPOVm3jZbulet2Fvdz0ujF6QfHHh0Vu1m4th8ukWJjeZvj6H5iDf2z xfoH3r7lRMxdVQaVQGlGjsgT/LZicBFcIYiVkDVX9Kjh5ugwMd6mB0R+Y9LNU31qq5VHb0/hbEq9H /tsGUUBXQjrADcQhzBR72oaLGy8xWsi2YgYzR+oHVoDwEZVDTL5BSYfzaeBSUp1xQTp0YvQzFsFqL QADqhwMdRtmjf6kADQ9f/4cn+m1pqN9C1TT8s8pVPgnuIA+SSumgKLqgIfPBPYHqsR+CKrddLnLDC Webt9Pxg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.89 #1 (Red Hat Linux)) id 1egVFb-0007oe-OR; Tue, 30 Jan 2018 12:42:59 +0000 Received: from mail-wm0-x244.google.com ([2a00:1450:400c:c09::244]) by bombadil.infradead.org with esmtps (Exim 4.89 #1 (Red Hat Linux)) id 1egVF3-0007GQ-DO for linux-arm-kernel@lists.infradead.org; Tue, 30 Jan 2018 12:42:29 +0000 Received: by mail-wm0-x244.google.com with SMTP id 143so809108wma.5 for ; Tue, 30 Jan 2018 04:42:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=53G4azeJICSZp3US8kQz0q2lBjr99JI7BgNjo04tsCM=; b=JTp1pPSRs3hINNInGdgrJpKdThsBe9jTkKSJvSRAOGNRLym6UnnsH6q8z0YqshalZC Cc+AJt0/6zU9vo0Z34wouG9PJccWGT4QNnjJmghJLZC1pX3Dem1+gW/rRzFqrANY7RQc LLcQEw3AKUGebPTl5RRejwjc3HqX0iAr/QZyY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=53G4azeJICSZp3US8kQz0q2lBjr99JI7BgNjo04tsCM=; b=o88tMDFEBU0kMuG5VQ3mHACdG8ByODs7rFQeIBJCldvV0VdCaXYS5IdWFEZsMpqqD6 klL1nPAsaxnC+r7yUltiUa6YVw3TeJk2LwAf6yd7YEtamCXgeboOratiGRpEP6onm5Jt GSaEf+DxPzXbpuXrJufVEV5rcuvQ/GOJjbbQDz6T+AYHeBZwks4ZM/erbo9XdWK44L7r GkftCZdEWcVPz6EzgVg5Ii9K+tR7rUlICHlWhO2p240JJPSA9JPKk/MQk7aLOqTjl2mH cqFIIOiWWrSxnaCHv4dA7dITDFFcNkvJToZdsoOFVy5gkB4ChS1gF12aFkQzfJMjgUvt qzYA== X-Gm-Message-State: AKwxytcuw72hZZCu5Qrot6B1WzTgMMtoeyF0jr1oWbElUNQcqCCAxWSh y0ZnnjFmZOjqTvaKcGD2WKUzJ3+sZBg= X-Google-Smtp-Source: AH8x224BnJl15o3EEKZKPTwXAbaEv5QypD6LMpvgK4EKEkL6aW6JqXXcNbE9QI92PCQWS1lGwFonYw== X-Received: by 10.80.151.155 with SMTP id e27mr52420830edb.108.1517316133280; Tue, 30 Jan 2018 04:42:13 -0800 (PST) Received: from localhost.localdomain (x50d2404e.cust.hiper.dk. [80.210.64.78]) by smtp.gmail.com with ESMTPSA id z7sm7990225edb.45.2018.01.30.04.42.12 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 30 Jan 2018 04:42:12 -0800 (PST) From: Christoffer Dall To: linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu Subject: [PATCH 1/3] KVM: arm/arm64: Fix incorrect timer_is_pending logic Date: Tue, 30 Jan 2018 13:42:04 +0100 Message-Id: <20180130124206.12262-2-christoffer.dall@linaro.org> X-Mailer: git-send-email 2.14.2 In-Reply-To: <20180130124206.12262-1-christoffer.dall@linaro.org> References: <20180130124206.12262-1-christoffer.dall@linaro.org> X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Marc Zyngier , Christoffer Dall , kvm@vger.kernel.org MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP After the recently introduced support for level-triggered mapped interrupt, I accidentally left the VCPU thread busily going back and forward between the guest and the hypervisor whenever the guest was blocking, because I would always incorrectly report that a timer interrupt was pending. This is because the timer->irq.level field is not valid for mapped interrupts, where we offload the level state to the hardware, and as a result this field is always true. Luckily the problem can be relatively easily solved by not checking the cached signal state of either timer in kvm_timer_should_fire() but instead compute the timer state on the fly, which we do already if the cached signal state wasn't high. In fact, the only reason for checking the cached signal state was a tiny optimization which would only be potentially faster when the polling loop detects a pending timer interrupt, which is quite unlikely. Instead of duplicating the logic from kvm_arch_timer_handler(), we enlighten kvm_timer_should_fire() to report something valid when the timer state is loaded onto the hardware. We can then call this from kvm_arch_timer_handler() as well and avoid the call to __timer_snapshot_state() in kvm_arch_timer_get_input_level(). Reported-by: Tomasz Nowicki Signed-off-by: Christoffer Dall Tested-by: Tomasz Nowicki Reviewed-by: Marc Zyngier --- virt/kvm/arm/arch_timer.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c index cfcd0323deab..63cf828f3c4f 100644 --- a/virt/kvm/arm/arch_timer.c +++ b/virt/kvm/arm/arch_timer.c @@ -97,10 +97,9 @@ static irqreturn_t kvm_arch_timer_handler(int irq, void *dev_id) pr_warn_once("Spurious arch timer IRQ on non-VCPU thread\n"); return IRQ_NONE; } - vtimer = vcpu_vtimer(vcpu); - vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl); - if (kvm_timer_irq_can_fire(vtimer)) + vtimer = vcpu_vtimer(vcpu); + if (kvm_timer_should_fire(vtimer)) kvm_timer_update_irq(vcpu, true, vtimer); if (static_branch_unlikely(&userspace_irqchip_in_use) && @@ -230,6 +229,16 @@ static bool kvm_timer_should_fire(struct arch_timer_context *timer_ctx) { u64 cval, now; + if (timer_ctx->loaded) { + u32 cnt_ctl; + + /* Only the virtual timer can be loaded so far */ + cnt_ctl = read_sysreg_el0(cntv_ctl); + return (cnt_ctl & ARCH_TIMER_CTRL_ENABLE) && + (cnt_ctl & ARCH_TIMER_CTRL_IT_STAT) && + !(cnt_ctl & ARCH_TIMER_CTRL_IT_MASK); + } + if (!kvm_timer_irq_can_fire(timer_ctx)) return false; @@ -244,15 +253,7 @@ bool kvm_timer_is_pending(struct kvm_vcpu *vcpu) struct arch_timer_context *vtimer = vcpu_vtimer(vcpu); struct arch_timer_context *ptimer = vcpu_ptimer(vcpu); - if (vtimer->irq.level || ptimer->irq.level) - return true; - - /* - * When this is called from withing the wait loop of kvm_vcpu_block(), - * the software view of the timer state is up to date (timer->loaded - * is false), and so we can simply check if the timer should fire now. - */ - if (!vtimer->loaded && kvm_timer_should_fire(vtimer)) + if (kvm_timer_should_fire(vtimer)) return true; return kvm_timer_should_fire(ptimer); @@ -270,9 +271,9 @@ void kvm_timer_update_run(struct kvm_vcpu *vcpu) /* Populate the device bitmap with the timer states */ regs->device_irq_level &= ~(KVM_ARM_DEV_EL1_VTIMER | KVM_ARM_DEV_EL1_PTIMER); - if (vtimer->irq.level) + if (kvm_timer_should_fire(vtimer)) regs->device_irq_level |= KVM_ARM_DEV_EL1_VTIMER; - if (ptimer->irq.level) + if (kvm_timer_should_fire(ptimer)) regs->device_irq_level |= KVM_ARM_DEV_EL1_PTIMER; } @@ -507,8 +508,8 @@ bool kvm_timer_should_notify_user(struct kvm_vcpu *vcpu) vlevel = sregs->device_irq_level & KVM_ARM_DEV_EL1_VTIMER; plevel = sregs->device_irq_level & KVM_ARM_DEV_EL1_PTIMER; - return vtimer->irq.level != vlevel || - ptimer->irq.level != plevel; + return kvm_timer_should_fire(vtimer) != vlevel || + kvm_timer_should_fire(ptimer) != plevel; } void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu) @@ -801,9 +802,6 @@ bool kvm_arch_timer_get_input_level(int vintid) else BUG(); /* We only map the vtimer so far */ - if (timer->loaded) - __timer_snapshot_state(timer); - return kvm_timer_should_fire(timer); }