[{"id":1783578,"web_url":"http://patchwork.ozlabs.org/comment/1783578/","msgid":"<87sheruxkd.fsf@on-the-bus.cambridge.arm.com>","list_archive_url":null,"date":"2017-10-10T10:06:10","subject":"Re: [PATCH v4 1/7] KVM: arm/arm64: Remove redundant preemptible\n\tchecks","submitter":{"id":7353,"url":"http://patchwork.ozlabs.org/api/people/7353/","name":"Marc Zyngier","email":"marc.zyngier@arm.com"},"content":"On Fri, Sep 15 2017 at 3:19:48 pm BST, Christoffer Dall wrote:\n> From: Christoffer Dall \n>\n> The __this_cpu_read() and __this_cpu_write() functions already implement\n> checks for the required preemption levels when using\n> CONFIG_DEBUG_PREEMPT which gives you nice error messages and such.\n> Therefore there is no need to explicitly check this using a BUG_ON() in\n> the code (which we don't do for other uses of per cpu variables either).\n>\n> Signed-off-by: Christoffer Dall \n> ---\n> virt/kvm/arm/arm.c | 2 --\n> 1 file changed, 2 deletions(-)\n>\n> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c\n> index a39a1e1..04313a2 100644\n> --- a/virt/kvm/arm/arm.c\n> +++ b/virt/kvm/arm/arm.c\n> @@ -69,7 +69,6 @@ static DEFINE_PER_CPU(unsigned char, kvm_arm_hardware_enabled);\n> \n> static void kvm_arm_set_running_vcpu(struct kvm_vcpu *vcpu)\n> {\n> -\tBUG_ON(preemptible());\n> \t__this_cpu_write(kvm_arm_running_vcpu, vcpu);\n> }\n> \n> @@ -79,7 +78,6 @@ static void kvm_arm_set_running_vcpu(struct kvm_vcpu *vcpu)\n> */\n> struct kvm_vcpu *kvm_arm_get_running_vcpu(void)\n> {\n> -\tBUG_ON(preemptible());\n> \treturn __this_cpu_read(kvm_arm_running_vcpu);\n> }\n\nAcked-by: Marc Zyngier \n\n\tM.","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"FZSrZtbi\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yBCSj4kkXz9tYB\n\tfor ;\n\tTue, 10 Oct 2017 21:06:44 +1100 (AEDT)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e1rQv-0000eW-Bp; Tue, 10 Oct 2017 10:06:41 +0000","from foss.arm.com ([217.140.101.70])\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e1rQr-0000ZR-WB for linux-arm-kernel@lists.infradead.org;\n\tTue, 10 Oct 2017 10:06:39 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3DD6280D;\n\tTue, 10 Oct 2017 03:06:17 -0700 (PDT)","from on-the-bus (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\t89DB13F58C; Tue, 10 Oct 2017 03:06:14 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:References\n\t:In-Reply-To:Subject:To:From:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=2W0VHi9QbvaAmEJRU1dPQX0MPRtk1EpCHzd3cQy9RtQ=;\n\tb=FZSrZtbikqvs0K\n\tMzEGyIUMlg3DtMw6KEpeX1/4pCwNs0Lw6W84Xj6YpWCKfvaOpOOvT6HsZAxD1xNDaQFR+S3nEWl+D\n\tCtBOZqWJssUBu6CZmHn6YkNgEmQBY8+psh3x4FxkyU99NCz5y38Q/p/uRNHVY06QKVibgwNUO2IBv\n\tAD67v2gKHN27K8SVbjRStQ4tDsdBVYvdNGbSGzVpluXdj8zMUNXZA3LMG4HKL65hTqMvu29o19b91\n\tbncBJCZJbIUE1CMHiXPyrQVFUQuiUb9kRA5vzgjbIjoZzAbZR/JAnHYme+0QIxBdPSPodkpMP8BKv\n\tnI4oFwxR7Lk6BC5lTYdw==;","From":"Marc Zyngier ","To":"Christoffer Dall ","Subject":"Re: [PATCH v4 1/7] KVM: arm/arm64: Remove redundant preemptible\n\tchecks","In-Reply-To":"<1505513994-77939-2-git-send-email-christoffer.dall@linaro.org>\n\t(Christoffer Dall's message of \"Fri, 15 Sep 2017 15:19:48 -0700\")","Organization":"ARM Ltd","References":"<1505513994-77939-1-git-send-email-christoffer.dall@linaro.org>\n\t<1505513994-77939-2-git-send-email-christoffer.dall@linaro.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)","Date":"Tue, 10 Oct 2017 11:06:10 +0100","Message-ID":"<87sheruxkd.fsf@on-the-bus.cambridge.arm.com>","MIME-Version":"1.0","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20171010_030638_044348_EB7F25A8 ","X-CRM114-Status":"GOOD ( 11.55 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Christoffer Dall , Eric Auger , \n\tkvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org,\n\tkvm@vger.kernel.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1783599,"web_url":"http://patchwork.ozlabs.org/comment/1783599/","msgid":"<87o9pfuwkf.fsf@on-the-bus.cambridge.arm.com>","list_archive_url":null,"date":"2017-10-10T10:27:44","subject":"Re: [PATCH v4 2/7] KVM: arm/arm64: Factor out functionality to get\n\tvgic mmio requester_vcpu","submitter":{"id":7353,"url":"http://patchwork.ozlabs.org/api/people/7353/","name":"Marc Zyngier","email":"marc.zyngier@arm.com"},"content":"On Fri, Sep 15 2017 at 3:19:49 pm BST, Christoffer Dall wrote:\n> From: Christoffer Dall \n>\n> We are about to distinguish between userspace accesses and mmio traps\n> for a number of the mmio handlers. When the requester vcpu is NULL, it\n> mens we are handling a userspace acccess.\n>\n> Factor out the functionality to get the request vcpu into its own\n> function, mostly so we have a common place to document the semantics of\n> the return value.\n>\n> Also take the chance to move the functionality outside of holding a\n> spinlock and instead explicitly disable and enable preemption. This\n> supports PREEMPT_RT kernels as well.\n>\n> Signed-off-by: Christoffer Dall \n> ---\n> virt/kvm/arm/vgic/vgic-mmio.c | 43 +++++++++++++++++++++++++++----------------\n> 1 file changed, 27 insertions(+), 16 deletions(-)\n>\n> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c\n> index c1e4bdd..f3087f6 100644\n> --- a/virt/kvm/arm/vgic/vgic-mmio.c\n> +++ b/virt/kvm/arm/vgic/vgic-mmio.c\n> @@ -120,6 +120,26 @@ unsigned long vgic_mmio_read_pending(struct kvm_vcpu *vcpu,\n> \treturn value;\n> }\n> \n> +/*\n> + * This function will return the VCPU that performed the MMIO access and\n> + * trapped from twithin the VM, and will return NULL if this is a userspace\n> + * access.\n> + *\n> + * We can disable preemption locally around accessing the per-CPU variable\n> + * because even if the current thread is migrated to another CPU, reading the\n> + * per-CPU value later will give us the same value as we update the per-CPU\n> + * variable in the preempt notifier handlers.\n> + */\n> +static struct kvm_vcpu *vgic_get_mmio_requester_vcpu(void)\n> +{\n> +\tstruct kvm_vcpu *vcpu;\n> +\n> +\tpreempt_disable();\n> +\tvcpu = kvm_arm_get_running_vcpu();\n> +\tpreempt_enable();\n> +\treturn vcpu;\n> +}\n> +\n> void vgic_mmio_write_spending(struct kvm_vcpu *vcpu,\n> \t\t\t gpa_t addr, unsigned int len,\n> \t\t\t unsigned long val)\n> @@ -180,23 +200,9 @@ unsigned long vgic_mmio_read_active(struct kvm_vcpu *vcpu,\n> static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,\n> \t\t\t\t bool new_active_state)\n> {\n> -\tstruct kvm_vcpu *requester_vcpu;\n> -\tspin_lock(&irq->irq_lock);\n> +\tstruct kvm_vcpu *requester_vcpu = vgic_get_mmio_requester_vcpu();\n> \n> -\t/*\n> -\t * The vcpu parameter here can mean multiple things depending on how\n> -\t * this function is called; when handling a trap from the kernel it\n> -\t * depends on the GIC version, and these functions are also called as\n> -\t * part of save/restore from userspace.\n> -\t *\n> -\t * Therefore, we have to figure out the requester in a reliable way.\n> -\t *\n> -\t * When accessing VGIC state from user space, the requester_vcpu is\n> -\t * NULL, which is fine, because we guarantee that no VCPUs are running\n> -\t * when accessing VGIC state from user space so irq->vcpu->cpu is\n> -\t * always -1.\n> -\t */\n> -\trequester_vcpu = kvm_arm_get_running_vcpu();\n> +\tspin_lock(&irq->irq_lock);\n> \n> \t/*\n> \t * If this virtual IRQ was written into a list register, we\n> @@ -208,6 +214,11 @@ static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,\n> \t * vgic_change_active_prepare) and still has to sync back this IRQ,\n> \t * so we release and re-acquire the spin_lock to let the other thread\n> \t * sync back the IRQ.\n> +\t *\n> +\t * When accessing VGIC state from user space, requester_vcpu is\n> +\t * NULL, which is fine, because we guarantee that no VCPUs are running\n> +\t * when accessing VGIC state from user space so irq->vcpu->cpu is\n> +\t * always -1.\n> \t */\n> \twhile (irq->vcpu && /* IRQ may have state in an LR somewhere */\n> \t irq->vcpu != requester_vcpu && /* Current thread is not the VCPU thread */\n\nAcked-by: Marc Zyngier \n\n\tM.","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"UWiJqJUm\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yBCxd0fCyz9t2c\n\tfor ;\n\tTue, 10 Oct 2017 21:28:21 +1100 (AEDT)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e1rlo-00062G-Jh; Tue, 10 Oct 2017 10:28:16 +0000","from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]\n\thelo=foss.arm.com)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e1rlk-0005x9-F2 for linux-arm-kernel@lists.infradead.org;\n\tTue, 10 Oct 2017 10:28:14 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 596D01596;\n\tTue, 10 Oct 2017 03:27:52 -0700 (PDT)","from on-the-bus (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\tDB4613F58C; Tue, 10 Oct 2017 03:27:47 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:References\n\t:In-Reply-To:Subject:To:From:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=thMyVOhrX3w8wvQtTb62e8SSsysrP3e+hFgwhQ9dvz4=;\n\tb=UWiJqJUm6v9jDb\n\tjfMGGRlBcv/p5F2THEMtycfSsPYBL5ZIBemYVszwTTNkuLl7rQt84WW+Jw7uUoCBxI7AxiUwEfVbQ\n\tjThSRVi69kiC0oBEx/JIhKzBuN0qAwBevgKHPeSXoUU39cUP2/Zx9fcjmDUMZisk9QdXHo2bVfmf1\n\tnLkZnbuu1WV11A7GdFjXLnXrbxBBtiq0eHrqPC2ldQ3hAgPPntO26x3sCfs+T4sBQYjF5sZKpL820\n\trByVXzfVk0AkWkAfnXoHmyRdOx9dprZc1IMVu/wKuWqAj7Wn8jYj7x52bG7FnQ9+EKOnHKFTKcCCl\n\tE4lmwTrTzdZ5dQnWEMcw==;","From":"Marc Zyngier ","To":"Christoffer Dall ","Subject":"Re: [PATCH v4 2/7] KVM: arm/arm64: Factor out functionality to get\n\tvgic mmio requester_vcpu","In-Reply-To":"<1505513994-77939-3-git-send-email-christoffer.dall@linaro.org>\n\t(Christoffer Dall's message of \"Fri, 15 Sep 2017 15:19:49 -0700\")","Organization":"ARM Ltd","References":"<1505513994-77939-1-git-send-email-christoffer.dall@linaro.org>\n\t<1505513994-77939-3-git-send-email-christoffer.dall@linaro.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)","Date":"Tue, 10 Oct 2017 11:27:44 +0100","Message-ID":"<87o9pfuwkf.fsf@on-the-bus.cambridge.arm.com>","MIME-Version":"1.0","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20171010_032812_611411_5E8E80AE ","X-CRM114-Status":"GOOD ( 20.20 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Christoffer Dall , Eric Auger , \n\tkvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org,\n\tkvm@vger.kernel.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1783609,"web_url":"http://patchwork.ozlabs.org/comment/1783609/","msgid":"<87k203uw01.fsf@on-the-bus.cambridge.arm.com>","list_archive_url":null,"date":"2017-10-10T10:39:58","subject":"Re: [PATCH v4 3/7] KVM: arm/arm64: Don't cache the timer IRQ level","submitter":{"id":7353,"url":"http://patchwork.ozlabs.org/api/people/7353/","name":"Marc Zyngier","email":"marc.zyngier@arm.com"},"content":"On Fri, Sep 15 2017 at 3:19:50 pm BST, Christoffer Dall wrote:\n> From: Christoffer Dall \n>\n> The timer was modeled after a strict idea of modelling an interrupt line\n> level in software, meaning that only transitions in the level needed to\n> be reported to the VGIC. This works well for the timer, because the\n> arch timer code is in complete control of the device and can track the\n> transitions of the line.\n>\n> However, as we are about to support using the HW bit in the VGIC not\n> just for the timer, but also for VFIO which cannot track transitions of\n> the interrupt line, we have to decide on an interface for level\n> triggered mapped interrupts to the GIC, which both the timer and VFIO\n> can use.\n>\n> VFIO only sees an asserting transition of the physical interrupt line,\n> and tells the VGIC when that happens. That means that part of the\n> interrupt flow is offloaded to the hardware.\n>\n> To use the same interface for VFIO devices and the timer, we therefore\n> have to change the timer (we cannot change VFIO because it doesn't know\n> the details of the device it is assigning to a VM).\n>\n> Luckily, changing the timer is simple, we just need to stop 'caching'\n> the line level, but instead let the VGIC know the state of the timer on\n> every entry to the guest, and the VGIC can ignore notifications using\n> its validate mechanism.\n>\n> Signed-off-by: Christoffer Dall \n> ---\n> virt/kvm/arm/arch_timer.c | 14 ++++++++------\n> 1 file changed, 8 insertions(+), 6 deletions(-)\n>\n> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c\n> index 8e89d63..2a5f877 100644\n> --- a/virt/kvm/arm/arch_timer.c\n> +++ b/virt/kvm/arm/arch_timer.c\n> @@ -219,9 +219,10 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,\n> \tint ret;\n> \n> \ttimer_ctx->active_cleared_last = false;\n> +\tif (timer_ctx->irq.level != new_level)\n> +\t\ttrace_kvm_timer_update_irq(vcpu->vcpu_id, timer_ctx->irq.irq,\n> +\t\t\t\t\t new_level);\n> \ttimer_ctx->irq.level = new_level;\n> -\ttrace_kvm_timer_update_irq(vcpu->vcpu_id, timer_ctx->irq.irq,\n> -\t\t\t\t timer_ctx->irq.level);\n> \n> \tif (likely(irqchip_in_kernel(vcpu->kvm))) {\n> \t\tret = kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id,\n> @@ -241,6 +242,7 @@ static void kvm_timer_update_state(struct kvm_vcpu *vcpu)\n> \tstruct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;\n> \tstruct arch_timer_context *vtimer = vcpu_vtimer(vcpu);\n> \tstruct arch_timer_context *ptimer = vcpu_ptimer(vcpu);\n> +\tbool level;\n> \n> \t/*\n> \t * If userspace modified the timer registers via SET_ONE_REG before\n> @@ -251,11 +253,11 @@ static void kvm_timer_update_state(struct kvm_vcpu *vcpu)\n> \tif (unlikely(!timer->enabled))\n> \t\treturn;\n> \n> -\tif (kvm_timer_should_fire(vtimer) != vtimer->irq.level)\n> -\t\tkvm_timer_update_irq(vcpu, !vtimer->irq.level, vtimer);\n> +\tlevel = kvm_timer_should_fire(vtimer);\n> +\tkvm_timer_update_irq(vcpu, level, vtimer);\n> \n> -\tif (kvm_timer_should_fire(ptimer) != ptimer->irq.level)\n> -\t\tkvm_timer_update_irq(vcpu, !ptimer->irq.level, ptimer);\n> +\tlevel = kvm_timer_should_fire(ptimer);\n> +\tkvm_timer_update_irq(vcpu, level, ptimer);\n\nWell, at this stage, you might as well fold the kvm_timer_should_fire()\ninto kvm_timer_update_irq() and from the level parameter. But I suspect\nthis is going to clash badly with your timer series?\n\n> }\n> \n> /* Schedule the background timer for the emulated timer. */\n\nOtherwise:\n\nReviewed-by: Marc Zyngier \n\n\tM.","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"nMp8A638\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yBDF26mYLz9t6m\n\tfor ;\n\tTue, 10 Oct 2017 21:41:42 +1100 (AEDT)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e1ryk-0005PA-P2; Tue, 10 Oct 2017 10:41:38 +0000","from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]\n\thelo=foss.arm.com)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e1rxZ-0004nF-Q3 for linux-arm-kernel@lists.infradead.org;\n\tTue, 10 Oct 2017 10:41:34 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 96A0E80D;\n\tTue, 10 Oct 2017 03:40:05 -0700 (PDT)","from on-the-bus (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\t7002E3F58C; Tue, 10 Oct 2017 03:40:01 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:References\n\t:In-Reply-To:Subject:To:From:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=RT1TCp/Hm2Krzyxq/YeRwe76BD9fzOXkb+H7IzFsxz8=;\n\tb=nMp8A638Fd0jlN\n\ta1rfkhSt3jkQjJkJ6axDhlZA8FIXiOBqLaUWZkgLsFaJXPVo0tEp2Imqpl5k2h6uUvkHs7z2HH9al\n\tayIYMH0I5yfDFoDk5obMSGYB5ecxInsjp7G5CD+CUw0KECbEfX6Fs6V++f6aEVcWreZp0Xn9dJJG4\n\tPcQCaIB+699FxwMpZE6Cpq/1hOGIRwxiPS8fAFXnaF9QJ0QJstLxVpQe0zaqhTpz72PnXBi4UAMfU\n\t+MxBnmKBgi8UnVaz5rRWHSBXVmSDgzy4iLwXVc0+9Na/JPLxme4GGL7zIMMF2M7j1H047IKsO2GVD\n\t5Cv2GFI7J7foxUUO9A/g==;","From":"Marc Zyngier ","To":"Christoffer Dall ","Subject":"Re: [PATCH v4 3/7] KVM: arm/arm64: Don't cache the timer IRQ level","In-Reply-To":"<1505513994-77939-4-git-send-email-christoffer.dall@linaro.org>\n\t(Christoffer Dall's message of \"Fri, 15 Sep 2017 15:19:50 -0700\")","Organization":"ARM Ltd","References":"<1505513994-77939-1-git-send-email-christoffer.dall@linaro.org>\n\t<1505513994-77939-4-git-send-email-christoffer.dall@linaro.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)","Date":"Tue, 10 Oct 2017 11:39:58 +0100","Message-ID":"<87k203uw01.fsf@on-the-bus.cambridge.arm.com>","MIME-Version":"1.0","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20171010_034029_879908_4909664F ","X-CRM114-Status":"GOOD ( 20.11 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Christoffer Dall , Eric Auger , \n\tkvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org,\n\tkvm@vger.kernel.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1783617,"web_url":"http://patchwork.ozlabs.org/comment/1783617/","msgid":"<87fuaruvk0.fsf@on-the-bus.cambridge.arm.com>","list_archive_url":null,"date":"2017-10-10T10:49:35","subject":"Re: [PATCH v4 7/7] KVM: arm/arm64: Support VGIC dist pend/active\n\tchanges for mapped IRQs","submitter":{"id":7353,"url":"http://patchwork.ozlabs.org/api/people/7353/","name":"Marc Zyngier","email":"marc.zyngier@arm.com"},"content":"On Fri, Sep 15 2017 at 3:19:54 pm BST, Christoffer Dall wrote:\n> From: Christoffer Dall \n>\n> For mapped IRQs (with the HW bit set in the LR) we have to follow some\n> rules of the architecture. One of these rules is that VM must not be\n> allowed to deactivate a virtual interrupt with the HW bit set unless the\n> physical interrupt is also active.\n>\n> This works fine when injecting mapped interrupts, because we leave it up\n> to the injector to either set EOImode==1 or manually set the active\n> state of the physical interrupt.\n>\n> However, the guest can set virtual interrupt to be pending or active by\n> writing to the virtual distributor, which could lead to deactivating a\n> virtual interrupt with the HW bit set without the physical interrupt\n> being active.\n>\n> We could set the physical interrupt to active whenever we are about to\n> enter the VM with a HW interrupt either pending or active, but that\n> would be really slow, especially on GICv2. So we take the long way\n> around and do the hard work when needed, which is expected to be\n> extremely rare.\n>\n> When the VM sets the pending state for a HW interrupt on the virtual\n> distributor we set the active state on the physical distributor, because\n> the virtual interrupt can become active and then the guest can\n> deactivate it.\n>\n> When the VM clears the pending state we also clear it on the physical\n> side, because the injector might otherwise raise the interrupt. We also\n> clear the physical active state when the virtual interrupt is not\n> active, since otherwise a SPEND/CPEND sequence from the guest would\n> prevent signaling of future interrupts.\n>\n> Changing the state of mapped interrupts from userspace is not supported,\n> and it's expected that userspace unmaps devices from VFIO before\n> attempting to set the interrupt state, because the interrupt state is\n> driven by hardware. One annoying exception with this is the arch timer,\n> which is mapped directly from the kernel without userspace knowing when\n> that happens. Since we already support saving/restoring the timer IRQ\n> state while it is mapped, and we rely on the timer code to set the\n> physical active state on VCPU entry, we have to preserve this behavior\n> and specifically check if the interrupt is for the timer. Oh well.\n>\n> Signed-off-by: Christoffer Dall \n> ---\n> virt/kvm/arm/vgic/vgic-mmio.c | 79 ++++++++++++++++++++++++++++++++++++++-----\n> virt/kvm/arm/vgic/vgic.c | 7 ++++\n> virt/kvm/arm/vgic/vgic.h | 1 +\n> 3 files changed, 79 insertions(+), 8 deletions(-)\n>\n> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c\n> index f3087f6..3a8c7ae 100644\n> --- a/virt/kvm/arm/vgic/vgic-mmio.c\n> +++ b/virt/kvm/arm/vgic/vgic-mmio.c\n> @@ -16,6 +16,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> \n> #include \"vgic.h\"\n> @@ -140,10 +141,24 @@ static struct kvm_vcpu *vgic_get_mmio_requester_vcpu(void)\n> \treturn vcpu;\n> }\n> \n> +/* Must be called with irq->irq_lock held */\n> +static void vgic_hw_irq_spending(struct kvm_vcpu *vcpu, struct vgic_irq *irq,\n> +\t\t\t\t bool is_uaccess)\n> +{\n> +\tbool is_timer = irq->get_input_level == kvm_arch_timer_get_input_level;\n> +\n> +\tif (!is_uaccess || is_timer)\n> +\t\tirq->pending_latch = true;\n> +\n> +\tif (!is_uaccess)\n> +\t\tvgic_irq_set_phys_active(irq, true);\n> +}\n> +\n> void vgic_mmio_write_spending(struct kvm_vcpu *vcpu,\n> \t\t\t gpa_t addr, unsigned int len,\n> \t\t\t unsigned long val)\n> {\n> +\tbool is_uaccess = !vgic_get_mmio_requester_vcpu();\n> \tu32 intid = VGIC_ADDR_TO_INTID(addr, 1);\n> \tint i;\n> \n> @@ -151,17 +166,47 @@ void vgic_mmio_write_spending(struct kvm_vcpu *vcpu,\n> \t\tstruct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);\n> \n> \t\tspin_lock(&irq->irq_lock);\n> -\t\tirq->pending_latch = true;\n> -\n> +\t\tif (irq->hw)\n> +\t\t\tvgic_hw_irq_spending(vcpu, irq, is_uaccess);\n> +\t\telse\n> +\t\t\tirq->pending_latch = true;\n> \t\tvgic_queue_irq_unlock(vcpu->kvm, irq);\n> \t\tvgic_put_irq(vcpu->kvm, irq);\n> \t}\n> }\n> \n> +/* Must be called with irq->irq_lock held */\n> +static void vgic_hw_irq_cpending(struct kvm_vcpu *vcpu, struct vgic_irq *irq,\n> +\t\t\t\t bool is_uaccess)\n> +{\n> +\tbool is_timer = irq->get_input_level == kvm_arch_timer_get_input_level;\n> +\n> +\tif (!is_uaccess || is_timer)\n> +\t\tirq->pending_latch = false;;\n\nExtra ;\n\n> +\n> +\tif (!is_uaccess) {\n> +\t\t/*\n> +\t\t * We don't want the guest to effectively mask the physical\n> +\t\t * interrupt by doing a write to SPENDR followed by a write to\n> +\t\t * CPENDR for HW interrupts, so we clear the active state on\n> +\t\t * the physical side if the virtual interrupt is not active.\n> +\t\t * This may lead to taking an additional interrupt on the\n> +\t\t * host, but that should not be a problem as the worst that\n> +\t\t * can happen is an additional vgic injection. We also clear\n> +\t\t * the pending state to maintain proper semantics for edge HW\n> +\t\t * interrupts.\n> +\t\t */\n> +\t\tvgic_irq_set_phys_pending(irq, false);\n> +\t\tif (!irq->active)\n> +\t\t\tvgic_irq_set_phys_active(irq, false);\n> +\t}\n> +}\n> +\n> void vgic_mmio_write_cpending(struct kvm_vcpu *vcpu,\n> \t\t\t gpa_t addr, unsigned int len,\n> \t\t\t unsigned long val)\n> {\n> +\tbool is_uaccess = !vgic_get_mmio_requester_vcpu();\n> \tu32 intid = VGIC_ADDR_TO_INTID(addr, 1);\n> \tint i;\n> \n> @@ -169,9 +214,10 @@ void vgic_mmio_write_cpending(struct kvm_vcpu *vcpu,\n> \t\tstruct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);\n> \n> \t\tspin_lock(&irq->irq_lock);\n> -\n> -\t\tirq->pending_latch = false;\n> -\n> +\t\tif (irq->hw)\n> +\t\t\tvgic_hw_irq_cpending(vcpu, irq, is_uaccess);\n> +\t\telse\n> +\t\t\tirq->pending_latch = false;\n> \t\tspin_unlock(&irq->irq_lock);\n> \t\tvgic_put_irq(vcpu->kvm, irq);\n> \t}\n> @@ -197,8 +243,21 @@ unsigned long vgic_mmio_read_active(struct kvm_vcpu *vcpu,\n> \treturn value;\n> }\n> \n> +/* Must be called with irq->irq_lock held */\n> +static void vgic_hw_irq_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,\n> +\t\t\t\t bool active, bool is_uaccess)\n> +{\n> +\tbool is_timer = irq->get_input_level == kvm_arch_timer_get_input_level;\n> +\n> +\tif (!is_uaccess || is_timer)\n> +\t\tirq->active = active;;\n> +\n> +\tif (!is_uaccess)\n> +\t\tvgic_irq_set_phys_active(irq, active);\n> +}\n> +\n> static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,\n> -\t\t\t\t bool new_active_state)\n> +\t\t\t\t bool active)\n> {\n> \tstruct kvm_vcpu *requester_vcpu = vgic_get_mmio_requester_vcpu();\n> \n> @@ -225,8 +284,12 @@ static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,\n> \t irq->vcpu->cpu != -1) /* VCPU thread is running */\n> \t\tcond_resched_lock(&irq->irq_lock);\n> \n> -\tirq->active = new_active_state;\n> -\tif (new_active_state)\n> +\tif (irq->hw)\n> +\t\tvgic_hw_irq_change_active(vcpu, irq, active, !requester_vcpu);\n> +\telse\n> +\t\tirq->active = active;\n> +\n> +\tif (irq->active)\n> \t\tvgic_queue_irq_unlock(vcpu->kvm, irq);\n> \telse\n> \t\tspin_unlock(&irq->irq_lock);\n> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c\n> index af0de3d..0ca2b3d 100644\n> --- a/virt/kvm/arm/vgic/vgic.c\n> +++ b/virt/kvm/arm/vgic/vgic.c\n> @@ -140,6 +140,13 @@ void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq)\n> \tkfree(irq);\n> }\n> \n> +void vgic_irq_set_phys_pending(struct vgic_irq *irq, bool pending)\n> +{\n> +\tWARN_ON(irq_set_irqchip_state(irq->host_irq,\n> +\t\t\t\t IRQCHIP_STATE_PENDING,\n> +\t\t\t\t pending));\n> +}\n> +\n> /* Get the input level of a mapped IRQ directly from the physical GIC */\n> bool vgic_get_phys_line_level(struct vgic_irq *irq)\n> {\n> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h\n> index 7bdcda2..498ee05 100644\n> --- a/virt/kvm/arm/vgic/vgic.h\n> +++ b/virt/kvm/arm/vgic/vgic.h\n> @@ -146,6 +146,7 @@ struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu,\n> \t\t\t u32 intid);\n> void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq);\n> bool vgic_get_phys_line_level(struct vgic_irq *irq);\n> +void vgic_irq_set_phys_pending(struct vgic_irq *irq, bool pending);\n> void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active);\n> bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq);\n> void vgic_kick_vcpus(struct kvm *kvm);\n\nOtherwise:\n\nReviewed-by: Marc Zyngier \n\n\tM.","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"H63H/l44\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yBDSX4336z9tY0\n\tfor ;\n\tTue, 10 Oct 2017 21:51:40 +1100 (AEDT)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e1s8P-0003YS-Vj; Tue, 10 Oct 2017 10:51:38 +0000","from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]\n\thelo=foss.arm.com)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e1s6s-0000mj-Ne for linux-arm-kernel@lists.infradead.org;\n\tTue, 10 Oct 2017 10:50:08 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 83BB280D;\n\tTue, 10 Oct 2017 03:49:42 -0700 (PDT)","from on-the-bus (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\t3256B3F58C; Tue, 10 Oct 2017 03:49:38 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:References\n\t:In-Reply-To:Subject:To:From:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=GfzfX4d1ZEc4LqSPxn1Q8Z5ehD9wJfBVftq5w3G24HQ=;\n\tb=H63H/l44vWIKBT\n\tabvWiW5T0RyF+X+OqPcI3vQKtY+gM9u68ERtpZBNOyJVju+OEQXbZJKBesRxdr33rqeyPu6/p0qRV\n\tAqoyAX2V4OQSCHVVNkJ1QxvycU32KpGZMl5Hw5IlON4MuUEfmgz4ltHQhHUhjJpnV7FpA3431m8sl\n\tNSenHBsBoba8wIJkK/zRKHlGiHSilh6deHi38sNGsR4PfnUcudaZJB0Ba1hea2QlQz/hOfi9ZGEU4\n\tKuGbUEbhRg0q7z3i6QAJ/LH4OzH87TBv6zqUh4IxhmTjYl3IpChFwdZmn9WTLCcF8jaoPJ+JObWYy\n\tkL7IYpWnqu6/y1QuWPZA==;","From":"Marc Zyngier ","To":"Christoffer Dall ","Subject":"Re: [PATCH v4 7/7] KVM: arm/arm64: Support VGIC dist pend/active\n\tchanges for mapped IRQs","In-Reply-To":"<1505513994-77939-8-git-send-email-christoffer.dall@linaro.org>\n\t(Christoffer Dall's message of \"Fri, 15 Sep 2017 15:19:54 -0700\")","Organization":"ARM Ltd","References":"<1505513994-77939-1-git-send-email-christoffer.dall@linaro.org>\n\t<1505513994-77939-8-git-send-email-christoffer.dall@linaro.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)","Date":"Tue, 10 Oct 2017 11:49:35 +0100","Message-ID":"<87fuaruvk0.fsf@on-the-bus.cambridge.arm.com>","MIME-Version":"1.0","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20171010_035002_887848_3AB21627 ","X-CRM114-Status":"GOOD ( 27.71 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Christoffer Dall , Eric Auger , \n\tkvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org,\n\tkvm@vger.kernel.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1790544,"web_url":"http://patchwork.ozlabs.org/comment/1790544/","msgid":"<20171019130548.GR8900@cbox>","list_archive_url":null,"date":"2017-10-19T13:05:48","subject":"Re: [PATCH v4 3/7] KVM: arm/arm64: Don't cache the timer IRQ level","submitter":{"id":71350,"url":"http://patchwork.ozlabs.org/api/people/71350/","name":"Christoffer Dall","email":"cdall@linaro.org"},"content":"On Tue, Oct 10, 2017 at 11:39:58AM +0100, Marc Zyngier wrote:\n> On Fri, Sep 15 2017 at 3:19:50 pm BST, Christoffer Dall wrote:\n> > From: Christoffer Dall \n> >\n> > The timer was modeled after a strict idea of modelling an interrupt line\n> > level in software, meaning that only transitions in the level needed to\n> > be reported to the VGIC. This works well for the timer, because the\n> > arch timer code is in complete control of the device and can track the\n> > transitions of the line.\n> >\n> > However, as we are about to support using the HW bit in the VGIC not\n> > just for the timer, but also for VFIO which cannot track transitions of\n> > the interrupt line, we have to decide on an interface for level\n> > triggered mapped interrupts to the GIC, which both the timer and VFIO\n> > can use.\n> >\n> > VFIO only sees an asserting transition of the physical interrupt line,\n> > and tells the VGIC when that happens. That means that part of the\n> > interrupt flow is offloaded to the hardware.\n> >\n> > To use the same interface for VFIO devices and the timer, we therefore\n> > have to change the timer (we cannot change VFIO because it doesn't know\n> > the details of the device it is assigning to a VM).\n> >\n> > Luckily, changing the timer is simple, we just need to stop 'caching'\n> > the line level, but instead let the VGIC know the state of the timer on\n> > every entry to the guest, and the VGIC can ignore notifications using\n> > its validate mechanism.\n> >\n> > Signed-off-by: Christoffer Dall \n> > ---\n> > virt/kvm/arm/arch_timer.c | 14 ++++++++------\n> > 1 file changed, 8 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c\n> > index 8e89d63..2a5f877 100644\n> > --- a/virt/kvm/arm/arch_timer.c\n> > +++ b/virt/kvm/arm/arch_timer.c\n> > @@ -219,9 +219,10 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,\n> > \tint ret;\n> > \n> > \ttimer_ctx->active_cleared_last = false;\n> > +\tif (timer_ctx->irq.level != new_level)\n> > +\t\ttrace_kvm_timer_update_irq(vcpu->vcpu_id, timer_ctx->irq.irq,\n> > +\t\t\t\t\t new_level);\n> > \ttimer_ctx->irq.level = new_level;\n> > -\ttrace_kvm_timer_update_irq(vcpu->vcpu_id, timer_ctx->irq.irq,\n> > -\t\t\t\t timer_ctx->irq.level);\n> > \n> > \tif (likely(irqchip_in_kernel(vcpu->kvm))) {\n> > \t\tret = kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id,\n> > @@ -241,6 +242,7 @@ static void kvm_timer_update_state(struct kvm_vcpu *vcpu)\n> > \tstruct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;\n> > \tstruct arch_timer_context *vtimer = vcpu_vtimer(vcpu);\n> > \tstruct arch_timer_context *ptimer = vcpu_ptimer(vcpu);\n> > +\tbool level;\n> > \n> > \t/*\n> > \t * If userspace modified the timer registers via SET_ONE_REG before\n> > @@ -251,11 +253,11 @@ static void kvm_timer_update_state(struct kvm_vcpu *vcpu)\n> > \tif (unlikely(!timer->enabled))\n> > \t\treturn;\n> > \n> > -\tif (kvm_timer_should_fire(vtimer) != vtimer->irq.level)\n> > -\t\tkvm_timer_update_irq(vcpu, !vtimer->irq.level, vtimer);\n> > +\tlevel = kvm_timer_should_fire(vtimer);\n> > +\tkvm_timer_update_irq(vcpu, level, vtimer);\n> > \n> > -\tif (kvm_timer_should_fire(ptimer) != ptimer->irq.level)\n> > -\t\tkvm_timer_update_irq(vcpu, !ptimer->irq.level, ptimer);\n> > +\tlevel = kvm_timer_should_fire(ptimer);\n> > +\tkvm_timer_update_irq(vcpu, level, ptimer);\n> \n> Well, at this stage, you might as well fold the kvm_timer_should_fire()\n> into kvm_timer_update_irq() and from the level parameter. But I suspect\n> this is going to clash badly with your timer series?\n> \n\nYes, it's doing extra unnecessary work in the critical path on IRQ\ninjections from the ISR after the timer series.\n\nHow about I rebase this series on the timer series and do a resend of\nthis one, without additional changes, and we have a look at it?\n\n> > }\n> > \n> > /* Schedule the background timer for the emulated timer. */\n> \n> Otherwise:\n> \n> Reviewed-by: Marc Zyngier \n> \nThanks!\n-Christoffer","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org header.b=\"hXmmj6TY\"; \n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=infradead.org header.i=@infradead.org\n\theader.b=\"RoSqJk8C\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"OxMxNgvx\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yHq5J1s0rz9t42\n\tfor ;\n\tFri, 20 Oct 2017 00:09:24 +1100 (AEDT)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e5AZb-00062Z-IY; Thu, 19 Oct 2017 13:09:19 +0000","from casper.infradead.org ([2001:8b0:10b:1236::1])\n\tby bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e5AYy-0005Bf-Md for linux-arm-kernel@bombadil.infradead.org;\n\tThu, 19 Oct 2017 13:08:40 +0000","from mail-wm0-x241.google.com ([2a00:1450:400c:c09::241])\n\tby casper.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e5AWV-00035o-RP for linux-arm-kernel@lists.infradead.org;\n\tThu, 19 Oct 2017 13:06:11 +0000","by mail-wm0-x241.google.com with SMTP id b189so15772230wmd.4\n\tfor ;\n\tThu, 19 Oct 2017 06:05:47 -0700 (PDT)","from localhost (xd93dd96b.cust.hiper.dk. [217.61.217.107])\n\tby smtp.gmail.com with ESMTPSA id\n\td3sm9135058edd.41.2017.10.19.06.05.45\n\t(version=TLS1_2 cipher=AES128-SHA bits=128/128);\n\tThu, 19 Oct 2017 06:05:45 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=0LLwqcRBtuMReGviyhhBi2UkLjcBykfP6MrsasPjTu0=;\n\tb=hXmmj6TYxZtmK8\n\tR42VfPr++n1SIS41fXh74yCCSUHfeqEWBvEk4KlO414pH8+QUOsNjCHLnYyMEVl8OX3hPLUY6Lsir\n\tV3aiUW0r7a78EKMplM33H4FWqcOrc3soVoKuTXReTVoOmNtR1TDPgZnNzdvqFsP1Y9aNrFfIe2Bgx\n\tPAe8SvN7pfY7gkKN7Zc3E3CFX6H5lpHd9OlwpW0KKQgx/34I2Hqn7JozjUbhd3vu6A3juZK1hJviu\n\tNZhkzWglTcwdey5gtL0Z1mfJRMfS9nr5GGjzQ9d2icjjc2f6DJ5LwkiHkn0xQLkf+24S/xveQh4Iw\n\tj/vzAS8VOe5BtsIzjjyg==;","v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=infradead.org; s=casper.20170209;\n\th=In-Reply-To:Content-Type:MIME-Version:\n\tReferences:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To:\n\tContent-Transfer-Encoding:Content-ID:Content-Description:Resent-Date:\n\tResent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id:\n\tList-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive;\n\tbh=zYwt6DPaTJD9vlP0Asa3B/M6U9fcVVsIispvZGfT/so=;\n\tb=RoSqJk8CX3N1NU/LQ0POi64E9\n\ti7l8OdtGEQlACuut1EedUWX3HejV0cl55V6WUNY/bmAQB+9Rtb/81vKIhvhGglBDIds8MiyISjmPR\n\tjrEAXMYI0bVDwow1yqrb8KWxq+MOck8tweIwgeZIFf/45pF/UbeGsk7vvAUIZavIlSN1B0TTXcJjG\n\tQTRE3ibhn8e8qrc25GeIdDjU/Tc2w3rltQNkIzV1AzN6cMtdcdSi/wzECwQS8BERRHoNowZ/gZwFS\n\tW4Du9Nv5MnHhwjCj38qAXnKtHFlrw3LF3ZVuKlcPT4Og/c8ZYdD0IHlqeTE4Bz8OtN6XwIH/CUo9N\n\t9COkmRqtg==;","v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=zYwt6DPaTJD9vlP0Asa3B/M6U9fcVVsIispvZGfT/so=;\n\tb=OxMxNgvxliLWvnwFkhjHJbuyOTzXsbzORmXqq240v70HFoLEgjsmlYbOy/o/v9FmYl\n\twKUb+/Z3eMm/l6JvC5QxDLWFVwfnRRDYuhRKKe3wC8H1H6u5UaPBJO3X2Cg0yS74VagF\n\tfN6MVy9NoXRr2N/AxOyWu7/wvQhb9zCAw/R4A="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=zYwt6DPaTJD9vlP0Asa3B/M6U9fcVVsIispvZGfT/so=;\n\tb=kNv0te8tIaaXGuLxmicZ1FDeL5We8qjJhl8IRAIpeyj2naX/tUV4ceVpKgysiv12RE\n\tz7pA0ur+dchOHeGcU7TVMWtTEAMQnlkd0eqTA3macBwXBlsAwy2sWj2x91uGayN7ccev\n\tdQ4vrUaRPYVVw91Oku3Y6Arp2bS6Qvk2ybq5r7eRQSgP4LnF8f6fQ9H62J8CRIzcrkKb\n\tXCCTrFSqu9q+IhqnbkvjNXM5wGrIjnFfF11pMUwnj+YVi/qy++KeA3VsbQ557sxAiBz1\n\tTz2/vv5TekJS8q3vSYLKaQ5XJzcXQgk7hIjlBcbvnPg2yZCiydiiKRJ34wYo88SDLy8t\n\tv7EA==","X-Gm-Message-State":"AMCzsaVCv3hGhj/W0OS6uf/hqXEP+PB4b1Dq7rKvmywc3sQ3VnKPvpD6\n\tCLIwQiAabSdbgKG1RPT0Tzl0Kw==","X-Google-Smtp-Source":"ABhQp+QyGJmwJD1EQZFsxx6gkH+S62aiPnupvOXzlWIxYGL2s94sL2yDPdK5iBZwKHJlMfJah7Kp4w==","X-Received":"by 10.80.149.171 with SMTP id w40mr2316543eda.102.1508418346305; \n\tThu, 19 Oct 2017 06:05:46 -0700 (PDT)","Date":"Thu, 19 Oct 2017 15:05:48 +0200","From":"Christoffer Dall ","To":"Marc Zyngier ","Subject":"Re: [PATCH v4 3/7] KVM: arm/arm64: Don't cache the timer IRQ level","Message-ID":"<20171019130548.GR8900@cbox>","References":"<1505513994-77939-1-git-send-email-christoffer.dall@linaro.org>\n\t<1505513994-77939-4-git-send-email-christoffer.dall@linaro.org>\n\t<87k203uw01.fsf@on-the-bus.cambridge.arm.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<87k203uw01.fsf@on-the-bus.cambridge.arm.com>","User-Agent":"Mutt/1.5.21 (2010-09-15)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20171019_140607_959766_45444305 ","X-CRM114-Status":"GOOD ( 40.60 )","X-Spam-Score":"-2.0 (--)","X-Spam-Report":"SpamAssassin version 3.4.1 on casper.infradead.org summary:\n\tContent analysis details: (-2.0 points, 5.0 required)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/,\n\tno\n\ttrust [2a00:1450:400c:c09:0:0:0:241 listed in] [list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]\n\t-0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from\n\tauthor's domain\n\t0.1 DKIM_SIGNED Message has a DKIM or DK signature,\n\tnot necessarily valid\n\t-0.1 DKIM_VALID Message has at least one valid DKIM or DK signature","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arm-kernel@lists.infradead.org, Eric Auger ,\n\tkvmarm@lists.cs.columbia.edu,\n\tChristoffer Dall , kvm@vger.kernel.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}}]