From patchwork Wed Jan 31 09:35:05 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 10193697 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 01BB9603EE for ; Wed, 31 Jan 2018 09:40:58 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CC6CF201BC for ; Wed, 31 Jan 2018 09:40:57 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C1276285FC; Wed, 31 Jan 2018 09:40:57 +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 4A384201BC for ; Wed, 31 Jan 2018 09:40:56 +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=iihdcPQyJgL6o8SRMJURPsUFDSzjXLoV1GzuLef8jJQ=; b=CO3ZDy8VnHxGEX15k7CIF+hHyM 9C1pgPbPuMoetx8Z8dZSPXLlSxvORqS/9mV7yNk8Hrr5JbWJzBTxxyyi3WySMhLqMQpua4HKsLm94 wfOpJDlD06jdyjujneqYd2L389ZYYzm+U2lcIuDuxJx9qLDXhFl4Oh41C6WMNHnZPz7CpUr4g8Tq/ BYi9S5IQfEOLKA+eD7Yb57YQWaXOjDx82D2dy2qvKGsgHBYH/lkueGOdXQUsH7IKL1yRKiixQ1yaD koLnXTb0DnpBUVV7GJbaXq6KC1PYWfeSS9aVak/E9QEi3grJ+YuHC9QsQQpWdvpKcL3sgsfERngZt 3CHrkCqg==; 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 1egosu-0003Rc-Fm; Wed, 31 Jan 2018 09:40:52 +0000 Received: from mail-wm0-x243.google.com ([2a00:1450:400c:c09::243]) by bombadil.infradead.org with esmtps (Exim 4.89 #1 (Red Hat Linux)) id 1egoo5-0005sq-6D for linux-arm-kernel@lists.infradead.org; Wed, 31 Jan 2018 09:35:57 +0000 Received: by mail-wm0-x243.google.com with SMTP id 143so6646935wma.5 for ; Wed, 31 Jan 2018 01:35:51 -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=vCfZAsou6ChVC6G6L/JEQ2zzwDH1kQkitmCbU2xBLR4=; b=gCNkhVaL2Hg7pwFnTC1uNEJyEECAKBycPyS9ISdRePsHfQ5tA5U1j+0k11DKluVERO WeNrI0biHos3D7CqCgFhElT1K6R6bwZo51/LkfGUz/r2aHh+9y7y/S3MLM5SZ5+wD2Fi SogGt8guKEtmYjir5P+M/v3IQaFgU9MyW4UJE= 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=vCfZAsou6ChVC6G6L/JEQ2zzwDH1kQkitmCbU2xBLR4=; b=hHL0eNbyZih0tWbDBs+iCxkwcBz6653Q8P254Vc3RVVuqQm+c/SKxVaafe8IlAK9+3 RDzCDP3LtjYuBNz+8VKQVfMLANKoMboVncve0hydMfNXUfTeSOnTRqBozaYje4mF31JO BGtGaO6oPJmyhss8YEC/tmpq/nfA45boCkxxAVa+G6SsXla362YQ6d2vW2P2gXanQnO7 4ENI+WuNqZFgRQYOLnzDVOIKZQA9Vu5r4uHc3PnO1hBRlOgaM40MVnUsL2sya0aVg8sn UqJCtQqxW3Z7s15JVUx22szPyZ/fy/e+ep80+4iSjvHhZAs42CNqldSAhgBteOwLw+a2 eM7g== X-Gm-Message-State: AKwxytcc6wIruiZcWJFRsHtsNYSNKvS7gMHM8iOnQav2hJKfG70SfPGD 97GQqAFgRJjxGwz/HxCksrldnQ== X-Google-Smtp-Source: AH8x227WHZe6Vp9XshM9in/mb7SU063nFbyRy06OPuSiz5ShTVl2cWQ7Almu2tx4PO4vO+FeOsR/yw== X-Received: by 10.80.245.10 with SMTP id t10mr54643575edm.168.1517391350394; Wed, 31 Jan 2018 01:35:50 -0800 (PST) Received: from localhost.localdomain (x50d2404e.cust.hiper.dk. [80.210.64.78]) by smtp.gmail.com with ESMTPSA id z2sm10247837edc.50.2018.01.31.01.35.48 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 31 Jan 2018 01:35:49 -0800 (PST) From: Christoffer Dall To: Paolo Bonzini , =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= Subject: [PULL 26/28] KVM: arm/arm64: Fix incorrect timer_is_pending logic Date: Wed, 31 Jan 2018 10:35:05 +0100 Message-Id: <20180131093507.22219-27-christoffer.dall@linaro.org> X-Mailer: git-send-email 2.14.2 In-Reply-To: <20180131093507.22219-1-christoffer.dall@linaro.org> References: <20180131093507.22219-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 , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, 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 Tested-by: Tomasz Nowicki Reviewed-by: Marc Zyngier Signed-off-by: Christoffer Dall --- 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); }