[{"id":1765324,"web_url":"http://patchwork.ozlabs.org/comment/1765324/","msgid":"","list_archive_url":null,"date":"2017-09-08T13:04:33","subject":"Re: [PATCH v3 4/5] KVM: arm/arm64: Support VGIC dist pend/active\n\tchanges for mapped IRQs","submitter":{"id":69187,"url":"http://patchwork.ozlabs.org/api/people/69187/","name":"Eric Auger","email":"eric.auger@redhat.com"},"content":"Hi Christoffer,\n\nOn 06/09/2017 14:26, Christoffer Dall wrote:\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.\n> \n> Signed-off-by: Christoffer Dall \n> ---\n> virt/kvm/arm/vgic/vgic-mmio.c | 33 +++++++++++++++++++++++++++++++++\n> virt/kvm/arm/vgic/vgic.c | 7 +++++++\n> virt/kvm/arm/vgic/vgic.h | 1 +\n> 3 files changed, 41 insertions(+)\n> \n> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c\n> index c1e4bdd..00003ae 100644\n> --- a/virt/kvm/arm/vgic/vgic-mmio.c\n> +++ b/virt/kvm/arm/vgic/vgic-mmio.c\n> @@ -131,6 +131,9 @@ 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\tif (irq->hw)\n> +\t\t\tvgic_irq_set_phys_active(irq, true);\n> +\n> \t\tirq->pending_latch = true;\n> \n> \t\tvgic_queue_irq_unlock(vcpu->kvm, irq);\n> @@ -149,6 +152,20 @@ 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> +\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 here. This may lead to taking an\n> +\t\t * additional interrupt on the host, but that should not be a\n> +\t\t * problem as the worst that can happen is an additional vgic\n> +\t\t * injection. We also clear the pending state to maintain\n> +\t\t * proper semantics for edge HW interrupts.\n> +\t\t */\n> +\t\tif (irq->hw) {\n> +\t\t\tvgic_irq_set_phys_pending(irq, false);\n> +\t\t\tvgic_irq_set_phys_active(irq, false);\nI fail in understanding why the active state is reset here. Can you\nprovide an example?\n\nIf the physical dist has an active state can't the virtual IRQ be in\nactive state, in which case you may later on deactivate a phys IRQ which\nis not active?\n\nThanks\n\nEric\n> +\t\t}\n> \n> \t\tirq->pending_latch = false;\n> \n> @@ -214,6 +231,22 @@ 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> +\tif (irq->hw) {\n> +\t\t/*\n> +\t\t * We cannot support setting the physical active state for\n> +\t\t * private interrupts from another CPU than the one running\n> +\t\t * the VCPU which identifies which private interrupt it is\n> +\t\t * trying to modify.\n> +\t\t */\n> +\t\tif (irq->intid < VGIC_NR_PRIVATE_IRQS &&\n> +\t\t irq->target_vcpu != requester_vcpu) {\n> +\t\t\tspin_unlock(&irq->irq_lock);\n> +\t\t\treturn;\n> +\t\t}\n> +\n> +\t\tvgic_irq_set_phys_active(irq, new_active_state);\n> +\t}\n> +\n> \tirq->active = new_active_state;\n> \tif (new_active_state)\n> \t\tvgic_queue_irq_unlock(vcpu->kvm, irq);\n> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c\n> index 8072969..7aec730 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>","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=\"g6QTV7mo\"; dkim-atps=neutral","ext-mx08.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx08.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=eric.auger@redhat.com"],"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 3xpcxT54Wpz9ryk\n\tfor ;\n\tFri, 8 Sep 2017 23:05:17 +1000 (AEST)","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 1dqIy9-0006ME-El; Fri, 08 Sep 2017 13:05:13 +0000","from mx1.redhat.com ([209.132.183.28])\n\tby bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dqIxt-0005zq-Om for linux-arm-kernel@lists.infradead.org;\n\tFri, 08 Sep 2017 13:05:04 +0000","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 77B94C058EA9;\n\tFri, 8 Sep 2017 13:04:36 +0000 (UTC)","from localhost.localdomain (ovpn-117-1.ams2.redhat.com\n\t[10.36.117.1])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id B42BA60A9B;\n\tFri, 8 Sep 2017 13:04:34 +0000 (UTC)"],"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:Date:\n\tMessage-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description\n\t:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=pMZfWJRvAfC8Se71YIXmYKrP7TtQH7JKhBKK8mhGpEo=;\n\tb=g6QTV7moeztmYi\n\tGgn1j1wHGGdxKc2nP7v4Jz74Ic/i4bHfGWTtp8z/8sLBMDwlrWgDztpe60gdocgS6HAzYVgdkad+B\n\tIsrhp91bb2p1idWOglI2dbcF5+7/iR6U2bId09Zm2sNVhwAxB+cqbK3OaIrp+LbxYWfKgArmRakGC\n\tcNIgIjjf6mCrBBgGpYYx0NPAw3XzkdUUGIsupZxpwVQ7h+sMD5oE1gO0Z3c9hE5xUsEPfWiLfxleo\n\tSfLlxm1nktFhqaH59bg6EW0slUu8DCGVjsdP4kGDdCVMX++9GYnwN5mE7a8Qc5f5Fs5l3zw7AkKEs\n\t75NN/wPmAcTJ88QQbycQ==;","DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 77B94C058EA9","Subject":"Re: [PATCH v3 4/5] KVM: arm/arm64: Support VGIC dist pend/active\n\tchanges for mapped IRQs","To":"Christoffer Dall , kvmarm@lists.cs.columbia.edu,\n\tMarc Zyngier ","References":"<20170906122612.18050-1-cdall@linaro.org>\n\t<20170906122612.18050-5-cdall@linaro.org>","From":"Auger Eric ","Message-ID":"","Date":"Fri, 8 Sep 2017 15:04:33 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101\n\tThunderbird/45.4.0","MIME-Version":"1.0","In-Reply-To":"<20170906122612.18050-5-cdall@linaro.org>","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.32]); Fri, 08 Sep 2017 13:04:36 +0000 (UTC)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170908_060458_013332_83824FE5 ","X-CRM114-Status":"GOOD ( 26.82 )","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 [209.132.183.28 listed in list.dnswl.org]\n\t-0.0 RCVD_IN_MSPIKE_H3 RBL: Good reputation (+3)\n\t[209.132.183.28 listed in wl.mailspike.net]\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-0.0 SPF_HELO_PASS SPF: HELO 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.0 RCVD_IN_MSPIKE_WL Mailspike good senders","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":"Andre Przywara , kvm@vger.kernel.org,\n\tlinux-arm-kernel@lists.infradead.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":1765359,"web_url":"http://patchwork.ozlabs.org/comment/1765359/","msgid":"<2e1352d7-fb64-907d-7726-4d53d7af5768@arm.com>","list_archive_url":null,"date":"2017-09-08T13:32:37","subject":"Re: [PATCH v3 4/5] 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 08/09/17 14:04, Auger Eric wrote:\n> Hi Christoffer,\n> \n> On 06/09/2017 14:26, Christoffer Dall wrote:\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.\n>>\n>> Signed-off-by: Christoffer Dall \n>> ---\n>> virt/kvm/arm/vgic/vgic-mmio.c | 33 +++++++++++++++++++++++++++++++++\n>> virt/kvm/arm/vgic/vgic.c | 7 +++++++\n>> virt/kvm/arm/vgic/vgic.h | 1 +\n>> 3 files changed, 41 insertions(+)\n>>\n>> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c\n>> index c1e4bdd..00003ae 100644\n>> --- a/virt/kvm/arm/vgic/vgic-mmio.c\n>> +++ b/virt/kvm/arm/vgic/vgic-mmio.c\n>> @@ -131,6 +131,9 @@ 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\tif (irq->hw)\n>> +\t\t\tvgic_irq_set_phys_active(irq, true);\n>> +\n>> \t\tirq->pending_latch = true;\n>> \n>> \t\tvgic_queue_irq_unlock(vcpu->kvm, irq);\n>> @@ -149,6 +152,20 @@ 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>> +\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 here. This may lead to taking an\n>> +\t\t * additional interrupt on the host, but that should not be a\n>> +\t\t * problem as the worst that can happen is an additional vgic\n>> +\t\t * injection. We also clear the pending state to maintain\n>> +\t\t * proper semantics for edge HW interrupts.\n>> +\t\t */\n>> +\t\tif (irq->hw) {\n>> +\t\t\tvgic_irq_set_phys_pending(irq, false);\n>> +\t\t\tvgic_irq_set_phys_active(irq, false);\n> I fail in understanding why the active state is reset here. Can you\n> provide an example?\n\n\nIf we're clearing the pending state on the virtual side, we need to be\nable to let an incoming physical interrupt fire so that it can be made\npending again. We may have to check that the virtual active state is\nclear though.\n\n> If the physical dist has an active state can't the virtual IRQ be in\n> active state, in which case you may later on deactivate a phys IRQ which\n> is not active?\n\n\nWell, I think we must be able to handle both cases:\n\n- CPENDR write:\n\tclear physical pending\n\tif (virtual active clear)\n\t\tclear physical active\n\n- CACTIVER write:\n\tclear physical active\n\nDoes this work for you?\n\nThanks,\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=\"Ad/6W6As\"; 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 3xpdYc4c9sz9s82\n\tfor ;\n\tFri, 8 Sep 2017 23:33:08 +1000 (AEST)","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 1dqJP5-0002Tz-FF; Fri, 08 Sep 2017 13:33:03 +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 1dqJP2-0002Gw-G5 for linux-arm-kernel@lists.infradead.org;\n\tFri, 08 Sep 2017 13:33:02 +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 42A9580D;\n\tFri, 8 Sep 2017 06:32:40 -0700 (PDT)","from [10.1.206.41] (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\t2FEB43F578; Fri, 8 Sep 2017 06:32:39 -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:Date:\n\tMessage-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description\n\t:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=PVeZ2iNaXo0obe2XVMsmrD2sWeskmmHsisAkqPX8fdo=;\n\tb=Ad/6W6Asw6MNez\n\tAHG69MF+Wh/NvvU+nbtlPlftgvYv2P0eaout/VI4SVuDKLVk67gWzvwj/3/+IKCl/11g0zXJlBsab\n\t04XXV4hrvqh7OuO2jA/gdyHfiYlJac6rGQt4MjknCOiZAfai3wGWNQQJu0b70o5GrkSAJstS65DIE\n\tUGR0wKSICX+/JgUE83CAegB4BPpdQjFLppAE0kf1cOZ8rwluunIYa7KuGC8ubKheOXGKc2BHQW5tl\n\tvMQtHSAcdQRZhxQjx8Z/w3I+/zKYFzdLHWdYKi9pSNnjrf0puCRVTf6zbqEA3l780jhGpTrs/+Smb\n\tOHZZS8L+i2bweYmNrIOA==;","Subject":"Re: [PATCH v3 4/5] KVM: arm/arm64: Support VGIC dist pend/active\n\tchanges for mapped IRQs","To":"Auger Eric , Christoffer Dall , \n\tkvmarm@lists.cs.columbia.edu","References":"<20170906122612.18050-1-cdall@linaro.org>\n\t<20170906122612.18050-5-cdall@linaro.org>\n\t","From":"Marc Zyngier ","Organization":"ARM Ltd","Message-ID":"<2e1352d7-fb64-907d-7726-4d53d7af5768@arm.com>","Date":"Fri, 8 Sep 2017 14:32:37 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-GB","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170908_063300_551774_41BDDBAC ","X-CRM114-Status":"GOOD ( 18.63 )","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":"Andre Przywara , kvm@vger.kernel.org,\n\tlinux-arm-kernel@lists.infradead.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":1765414,"web_url":"http://patchwork.ozlabs.org/comment/1765414/","msgid":"<5579210b-9b62-b33d-ac36-df3cb835c1e7@redhat.com>","list_archive_url":null,"date":"2017-09-08T14:27:47","subject":"Re: [PATCH v3 4/5] KVM: arm/arm64: Support VGIC dist pend/active\n\tchanges for mapped IRQs","submitter":{"id":69187,"url":"http://patchwork.ozlabs.org/api/people/69187/","name":"Eric Auger","email":"eric.auger@redhat.com"},"content":"Hi Marc,\n\nOn 08/09/2017 15:32, Marc Zyngier wrote:\n> On 08/09/17 14:04, Auger Eric wrote:\n>> Hi Christoffer,\n>>\n>> On 06/09/2017 14:26, Christoffer Dall wrote:\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.\n>>>\n>>> Signed-off-by: Christoffer Dall \n>>> ---\n>>> virt/kvm/arm/vgic/vgic-mmio.c | 33 +++++++++++++++++++++++++++++++++\n>>> virt/kvm/arm/vgic/vgic.c | 7 +++++++\n>>> virt/kvm/arm/vgic/vgic.h | 1 +\n>>> 3 files changed, 41 insertions(+)\n>>>\n>>> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c\n>>> index c1e4bdd..00003ae 100644\n>>> --- a/virt/kvm/arm/vgic/vgic-mmio.c\n>>> +++ b/virt/kvm/arm/vgic/vgic-mmio.c\n>>> @@ -131,6 +131,9 @@ 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\tif (irq->hw)\n>>> +\t\t\tvgic_irq_set_phys_active(irq, true);\n>>> +\n>>> \t\tirq->pending_latch = true;\n>>> \n>>> \t\tvgic_queue_irq_unlock(vcpu->kvm, irq);\n>>> @@ -149,6 +152,20 @@ 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>>> +\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 here. This may lead to taking an\n>>> +\t\t * additional interrupt on the host, but that should not be a\n>>> +\t\t * problem as the worst that can happen is an additional vgic\n>>> +\t\t * injection. We also clear the pending state to maintain\n>>> +\t\t * proper semantics for edge HW interrupts.\n>>> +\t\t */\n>>> +\t\tif (irq->hw) {\n>>> +\t\t\tvgic_irq_set_phys_pending(irq, false);\n>>> +\t\t\tvgic_irq_set_phys_active(irq, false);\n>> I fail in understanding why the active state is reset here. Can you\n>> provide an example?\n> \n> \n> If we're clearing the pending state on the virtual side, we need to be\n> able to let an incoming physical interrupt fire so that it can be made\n> pending again. We may have to check that the virtual active state is\n> clear though.\n> \n>> If the physical dist has an active state can't the virtual IRQ be in\n>> active state, in which case you may later on deactivate a phys IRQ which\n>> is not active?\n> \n> \n> Well, I think we must be able to handle both cases:\n> \n> - CPENDR write:\n> \tclear physical pending\n> \tif (virtual active clear)\n> \t\tclear physical active\n> \n> - CACTIVER write:\n> \tclear physical active\n> \n> Does this work for you?\n\nyes it does with that change!\n\nThanks\n\nEric\n\n> \n> Thanks,\n> \n> \tM.\n>","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=\"kXuljdYo\"; dkim-atps=neutral","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=eric.auger@redhat.com"],"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 3xpfnQ3xrpz9sBd\n\tfor ;\n\tSat, 9 Sep 2017 00:28:24 +1000 (AEST)","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 1dqKGa-0003iN-O5; Fri, 08 Sep 2017 14:28:20 +0000","from mx1.redhat.com ([209.132.183.28])\n\tby bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dqKGS-0003dH-OE for linux-arm-kernel@lists.infradead.org;\n\tFri, 08 Sep 2017 14:28:18 +0000","from smtp.corp.redhat.com\n\t(int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 9D6DF80481;\n\tFri, 8 Sep 2017 14:27:51 +0000 (UTC)","from localhost.localdomain (ovpn-117-1.ams2.redhat.com\n\t[10.36.117.1])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id C7A2D600C0;\n\tFri, 8 Sep 2017 14:27:49 +0000 (UTC)"],"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:Date:\n\tMessage-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description\n\t:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=fk/GLKqW7r3jgKt5nHAXWShb1K5gSov+OuOcGrWyxbw=;\n\tb=kXuljdYo1fG+2j\n\tvZlycG4Aj8eJ3oulf55yMSGUVXgUJ1bORXdDNbm1bR7igJ1iA5qDPw3+k4JOjEYbglBlaPAYKf6/V\n\t3hDkvqCWYAHGOWkymXwI85yTuMY9nPmdo47QMgRpRo/Ph2sCkMqq8kKIxWWI/TJCg2EmVyV6UcVrZ\n\tYEuROVHwR4PDYf2ESCOupPSXHpRvEnH49+LgTSUBMQL6my7PzAE//qIhCRNer9GVMoLxa1/2ByJaG\n\tiDRQFICnlVj8/yR9QvzfxkX21r4U2GVlgYwtldDH8TOA+cvnbSnp/KGVMtaGZJ4CI5XUXXxvvOGDI\n\tXzANMwaa4Rsp6zi2MysA==;","DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 9D6DF80481","Subject":"Re: [PATCH v3 4/5] KVM: arm/arm64: Support VGIC dist pend/active\n\tchanges for mapped IRQs","To":"Marc Zyngier , Christoffer Dall ,\n\tkvmarm@lists.cs.columbia.edu","References":"<20170906122612.18050-1-cdall@linaro.org>\n\t<20170906122612.18050-5-cdall@linaro.org>\n\t\n\t<2e1352d7-fb64-907d-7726-4d53d7af5768@arm.com>","From":"Auger Eric ","Message-ID":"<5579210b-9b62-b33d-ac36-df3cb835c1e7@redhat.com>","Date":"Fri, 8 Sep 2017 16:27:47 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101\n\tThunderbird/45.4.0","MIME-Version":"1.0","In-Reply-To":"<2e1352d7-fb64-907d-7726-4d53d7af5768@arm.com>","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.11","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]); Fri, 08 Sep 2017 14:27:51 +0000 (UTC)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170908_072812_865984_C75F17C1 ","X-CRM114-Status":"GOOD ( 23.66 )","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 [209.132.183.28 listed in list.dnswl.org]\n\t-0.0 RCVD_IN_MSPIKE_H3 RBL: Good reputation (+3)\n\t[209.132.183.28 listed in wl.mailspike.net]\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-0.0 SPF_HELO_PASS SPF: HELO 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.0 RCVD_IN_MSPIKE_WL Mailspike good senders","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":"Andre Przywara , kvm@vger.kernel.org,\n\tlinux-arm-kernel@lists.infradead.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":1765483,"web_url":"http://patchwork.ozlabs.org/comment/1765483/","msgid":"","list_archive_url":null,"date":"2017-09-08T16:05:58","subject":"Re: [PATCH v3 4/5] KVM: arm/arm64: Support VGIC dist pend/active\n\tchanges for mapped IRQs","submitter":{"id":71350,"url":"http://patchwork.ozlabs.org/api/people/71350/","name":"Christoffer Dall","email":"cdall@linaro.org"},"content":"On Fri, Sep 8, 2017 at 4:27 PM, Auger Eric wrote:\n> Hi Marc,\n>\n> On 08/09/2017 15:32, Marc Zyngier wrote:\n>> On 08/09/17 14:04, Auger Eric wrote:\n>>> Hi Christoffer,\n>>>\n>>> On 06/09/2017 14:26, Christoffer Dall wrote:\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.\n>>>>\n>>>> Signed-off-by: Christoffer Dall \n>>>> ---\n>>>> virt/kvm/arm/vgic/vgic-mmio.c | 33 +++++++++++++++++++++++++++++++++\n>>>> virt/kvm/arm/vgic/vgic.c | 7 +++++++\n>>>> virt/kvm/arm/vgic/vgic.h | 1 +\n>>>> 3 files changed, 41 insertions(+)\n>>>>\n>>>> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c\n>>>> index c1e4bdd..00003ae 100644\n>>>> --- a/virt/kvm/arm/vgic/vgic-mmio.c\n>>>> +++ b/virt/kvm/arm/vgic/vgic-mmio.c\n>>>> @@ -131,6 +131,9 @@ void vgic_mmio_write_spending(struct kvm_vcpu *vcpu,\n>>>> struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);\n>>>>\n>>>> spin_lock(&irq->irq_lock);\n>>>> + if (irq->hw)\n>>>> + vgic_irq_set_phys_active(irq, true);\n>>>> +\n>>>> irq->pending_latch = true;\n>>>>\n>>>> vgic_queue_irq_unlock(vcpu->kvm, irq);\n>>>> @@ -149,6 +152,20 @@ void vgic_mmio_write_cpending(struct kvm_vcpu *vcpu,\n>>>> struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);\n>>>>\n>>>> spin_lock(&irq->irq_lock);\n>>>> + /*\n>>>> + * We don't want the guest to effectively mask the physical\n>>>> + * interrupt by doing a write to SPENDR followed by a write to\n>>>> + * CPENDR for HW interrupts, so we clear the active state on\n>>>> + * the physical side here. This may lead to taking an\n>>>> + * additional interrupt on the host, but that should not be a\n>>>> + * problem as the worst that can happen is an additional vgic\n>>>> + * injection. We also clear the pending state to maintain\n>>>> + * proper semantics for edge HW interrupts.\n>>>> + */\n>>>> + if (irq->hw) {\n>>>> + vgic_irq_set_phys_pending(irq, false);\n>>>> + vgic_irq_set_phys_active(irq, false);\n>>> I fail in understanding why the active state is reset here. Can you\n>>> provide an example?\n>>\n>>\n>> If we're clearing the pending state on the virtual side, we need to be\n>> able to let an incoming physical interrupt fire so that it can be made\n>> pending again. We may have to check that the virtual active state is\n>> clear though.\n>>\n>>> If the physical dist has an active state can't the virtual IRQ be in\n>>> active state, in which case you may later on deactivate a phys IRQ which\n>>> is not active?\n>>\n>>\n>> Well, I think we must be able to handle both cases:\n>>\n>> - CPENDR write:\n>> clear physical pending\n>> if (virtual active clear)\n>> clear physical active\n>>\n>> - CACTIVER write:\n>> clear physical active\n>>\n>> Does this work for you?\n>\n> yes it does with that change!\n>\nI agree, I should fix that.\n\nBut then I wondered, are we properly handling PPIs for\nsetting/clearing the pending state? Don't we actually need to do\nsomething similar like the active cpu where we check the requester CPU\nand make sure we're not fiddling with some random hardware state when\naccessed by userspace?\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=\"JkhT7MHE\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"UjLPSBya\"; 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 3xphyq268Rz9sQl\n\tfor ;\n\tSat, 9 Sep 2017 02:06:41 +1000 (AEST)","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 1dqLng-0000El-SD; Fri, 08 Sep 2017 16:06:37 +0000","from mail-pf0-x232.google.com ([2607:f8b0:400e:c00::232])\n\tby bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dqLnQ-00009n-Ni for linux-arm-kernel@lists.infradead.org;\n\tFri, 08 Sep 2017 16:06:24 +0000","by mail-pf0-x232.google.com with SMTP id e1so5197291pfk.1\n\tfor ;\n\tFri, 08 Sep 2017 09:06:00 -0700 (PDT)","by 10.100.143.133 with HTTP; Fri, 8 Sep 2017 09:05:58 -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:To:Subject:Message-ID:Date:From:\n\tReferences:In-Reply-To:MIME-Version:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=vSNZjVyEwXpGq7dXAIPLsd4ZHwSgOwozTASrAm54qgc=;\n\tb=JkhT7MHEbdHvUE\n\tp+7+hiH67yU5837Ijo2YM2sKPS7Zh4R31rnilcXq8X/QtRLyaVD9dvMrMwzTduiI35ptsaZ5YXlyV\n\twySTgZHdn3iTp+cnAanznIMy62ehXaurCVW3/zhR1rG3b8xW9BFK3NbwPnRN3HR8UACkf3eSB8x60\n\teL+BoGmIqipDhhK/TE/IVzJjZZE6i265AhAVi3iInrJigElwdiPirINRtVN/7xcjhla2yLabHqDUL\n\txSUctH2X6mzOYJPDnnXC1vr6PZVyUN8+RBxa1JSftGD9uWMKzywA02sqzLl6f7xOVrcFvO8YwJQH1\n\tPDqwsnPRTiYUMn5ixwyg==;","v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=p5mE17ncuo7znimzOf4NyfI40aQoHB/ExrZ4nV2dLcw=;\n\tb=UjLPSByajalo8Suzl7SUFQfZr8Ozjhqrfx4y9RC5QK6lCDHydNCXzsanbAr33l6Iwe\n\tqZ5MRYyPdZ50SGus/BnZUedq7J5gQzXamrPuxgtw6wiSpu4mqzKT6emiQHv0kZyT70lW\n\tWj163fkwl5fyTMr+mXE7RG/WwQbhYRGO42Pgg="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=p5mE17ncuo7znimzOf4NyfI40aQoHB/ExrZ4nV2dLcw=;\n\tb=SSMzb/qDaQux+7cG0lhDlPqWC8KEK6Udpc8YIn49i4xaV1XMMF5eguQ2NF/lZL4ebX\n\t3wzX6Jz/zFLvo72owRiKlgNmKS5ZbR70ivM2jxU50NG82RRszoYKeCdW4jXfsvzvSSCP\n\tyVkTSMTlN85PDZh3dNJe7x0Y6RCW0Iy20IAmMygfRrgGMBUkwsQLEma/hvtx4ASfEctT\n\txQeOBqO8M1OWPRHmB7volreh/VQJq1DA1X2LN+I/bWiNfAVaGQI2yeJqfEfmu2vuQc6r\n\thh7bFsay7cBPD+rCwxk+WBuggbNoXFR7CELdp2/+/Bt0En4jWi4lCpGfnIKqqJ1LMYUw\n\t1B1A==","X-Gm-Message-State":"AHPjjUjZuoGPhi1ng+DzS+VxWrwd4i0jPKFYjScW+bv2mepi0K0y7hgE\n\tBrcArfZeTuk/8NmeEkWKc4y5sEGe3slx2XY=","X-Google-Smtp-Source":"ADKCNb60Ier/OiDYt6qfR4DOwRE9uKOQ0q+o1WSbtmAbzTGLK7ALyMsfNtZltY6OuWDXZQdfaYz6TMQFNk/wpUtyWOc=","X-Received":"by 10.84.231.202 with SMTP id g10mr3940395pln.407.1504886759418; \n\tFri, 08 Sep 2017 09:05:59 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<5579210b-9b62-b33d-ac36-df3cb835c1e7@redhat.com>","References":"<20170906122612.18050-1-cdall@linaro.org>\n\t<20170906122612.18050-5-cdall@linaro.org>\n\t\n\t<2e1352d7-fb64-907d-7726-4d53d7af5768@arm.com>\n\t<5579210b-9b62-b33d-ac36-df3cb835c1e7@redhat.com>","From":"Christoffer Dall ","Date":"Fri, 8 Sep 2017 18:05:58 +0200","Message-ID":"","Subject":"Re: [PATCH v3 4/5] KVM: arm/arm64: Support VGIC dist pend/active\n\tchanges for mapped IRQs","To":"Auger Eric ","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170908_090620_968593_E6BA8CAB ","X-CRM114-Status":"GOOD ( 20.04 )","X-Spam-Score":"-2.0 (--)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-2.0 points)\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 [2607:f8b0:400e:c00:0:0:0:232 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 Message has at least one valid DKIM or DK signature\n\t0.1 DKIM_SIGNED Message has a DKIM or DK signature,\n\tnot necessarily valid\n\t-0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from\n\tauthor's domain","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":"Marc Zyngier ,\n\tAndre Przywara ,\n\t\"kvmarm@lists.cs.columbia.edu\" ,\n\t\"linux-arm-kernel@lists.infradead.org\"\n\t, \n\t\"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"}},{"id":1765499,"web_url":"http://patchwork.ozlabs.org/comment/1765499/","msgid":"","list_archive_url":null,"date":"2017-09-08T16:30:54","subject":"Re: [PATCH v3 4/5] 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 08/09/17 17:05, Christoffer Dall wrote:\n> On Fri, Sep 8, 2017 at 4:27 PM, Auger Eric wrote:\n>> Hi Marc,\n>>\n>> On 08/09/2017 15:32, Marc Zyngier wrote:\n>>> On 08/09/17 14:04, Auger Eric wrote:\n>>>> Hi Christoffer,\n>>>>\n>>>> On 06/09/2017 14:26, Christoffer Dall wrote:\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.\n>>>>>\n>>>>> Signed-off-by: Christoffer Dall \n>>>>> ---\n>>>>> virt/kvm/arm/vgic/vgic-mmio.c | 33 +++++++++++++++++++++++++++++++++\n>>>>> virt/kvm/arm/vgic/vgic.c | 7 +++++++\n>>>>> virt/kvm/arm/vgic/vgic.h | 1 +\n>>>>> 3 files changed, 41 insertions(+)\n>>>>>\n>>>>> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c\n>>>>> index c1e4bdd..00003ae 100644\n>>>>> --- a/virt/kvm/arm/vgic/vgic-mmio.c\n>>>>> +++ b/virt/kvm/arm/vgic/vgic-mmio.c\n>>>>> @@ -131,6 +131,9 @@ void vgic_mmio_write_spending(struct kvm_vcpu *vcpu,\n>>>>> struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);\n>>>>>\n>>>>> spin_lock(&irq->irq_lock);\n>>>>> + if (irq->hw)\n>>>>> + vgic_irq_set_phys_active(irq, true);\n>>>>> +\n>>>>> irq->pending_latch = true;\n>>>>>\n>>>>> vgic_queue_irq_unlock(vcpu->kvm, irq);\n>>>>> @@ -149,6 +152,20 @@ void vgic_mmio_write_cpending(struct kvm_vcpu *vcpu,\n>>>>> struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);\n>>>>>\n>>>>> spin_lock(&irq->irq_lock);\n>>>>> + /*\n>>>>> + * We don't want the guest to effectively mask the physical\n>>>>> + * interrupt by doing a write to SPENDR followed by a write to\n>>>>> + * CPENDR for HW interrupts, so we clear the active state on\n>>>>> + * the physical side here. This may lead to taking an\n>>>>> + * additional interrupt on the host, but that should not be a\n>>>>> + * problem as the worst that can happen is an additional vgic\n>>>>> + * injection. We also clear the pending state to maintain\n>>>>> + * proper semantics for edge HW interrupts.\n>>>>> + */\n>>>>> + if (irq->hw) {\n>>>>> + vgic_irq_set_phys_pending(irq, false);\n>>>>> + vgic_irq_set_phys_active(irq, false);\n>>>> I fail in understanding why the active state is reset here. Can you\n>>>> provide an example?\n>>>\n>>>\n>>> If we're clearing the pending state on the virtual side, we need to be\n>>> able to let an incoming physical interrupt fire so that it can be made\n>>> pending again. We may have to check that the virtual active state is\n>>> clear though.\n>>>\n>>>> If the physical dist has an active state can't the virtual IRQ be in\n>>>> active state, in which case you may later on deactivate a phys IRQ which\n>>>> is not active?\n>>>\n>>>\n>>> Well, I think we must be able to handle both cases:\n>>>\n>>> - CPENDR write:\n>>> clear physical pending\n>>> if (virtual active clear)\n>>> clear physical active\n>>>\n>>> - CACTIVER write:\n>>> clear physical active\n>>>\n>>> Does this work for you?\n>>\n>> yes it does with that change!\n>>\n> I agree, I should fix that.\n> \n> But then I wondered, are we properly handling PPIs for\n> setting/clearing the pending state? Don't we actually need to do\n> something similar like the active cpu where we check the requester CPU\n> and make sure we're not fiddling with some random hardware state when\n> accessed by userspace?\nHmmm. Yeah, that's another can of worm. We should certainly make sure\nthat whatever userspace does is not directly impacting the HW state (and\nonly the guest does).\n\nI think. I'm not quite sure...\n\n\tM. (GIC-ed, again).","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=\"ofmcv6or\"; 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 3xpjWJ08hLz9s76\n\tfor ;\n\tSat, 9 Sep 2017 02:31:24 +1000 (AEST)","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 1dqMBc-0004Ym-Vv; Fri, 08 Sep 2017 16:31:21 +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 1dqMBZ-0004Vq-QG for linux-arm-kernel@lists.infradead.org;\n\tFri, 08 Sep 2017 16:31:19 +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 3618780D;\n\tFri, 8 Sep 2017 09:30:57 -0700 (PDT)","from [10.1.206.41] (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\t223CD3F540; Fri, 8 Sep 2017 09:30:55 -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:Date:\n\tMessage-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description\n\t:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=eeFDUcsuQcRRe7VMr+8PNZdbOSUtpxZapaXXTDqzbIs=;\n\tb=ofmcv6orNiOyRR\n\tw6yunUAPk8EMX0L0imgYvdGHNt0E1Ft8j3d9LO34bD+MEI3FgNwBYRQSMk4tmkxQOdH/jr41jLNDH\n\tD9jxNwfo/LPNc8XceTIFfiO38KfIFVE8x7FxANq10p9qcfkQYHBi0aLWDrJjGzjaFbNIn+lIuQe07\n\t7aeFg0FD5eOje3k2Gr+lhxPfqD3s3F1JwG94VJNYpHis6WJISwDO7hZA/OukhC54VyMwzqO3eIBMX\n\tP9FLfrUVE9/uQuZCt74kcND63qiDGx2qIZLPv8a1uw4/YAr4TK9tGey+svkriU9bPiHSG0wZghtzN\n\tjn2Wie/xWAbVoWI1BRAQ==;","Subject":"Re: [PATCH v3 4/5] KVM: arm/arm64: Support VGIC dist pend/active\n\tchanges for mapped IRQs","To":"Christoffer Dall , Auger Eric ","References":"<20170906122612.18050-1-cdall@linaro.org>\n\t<20170906122612.18050-5-cdall@linaro.org>\n\t\n\t<2e1352d7-fb64-907d-7726-4d53d7af5768@arm.com>\n\t<5579210b-9b62-b33d-ac36-df3cb835c1e7@redhat.com>\n\t","From":"Marc Zyngier ","Organization":"ARM Ltd","Message-ID":"","Date":"Fri, 8 Sep 2017 17:30:54 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-GB","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170908_093117_873439_74E7F11C ","X-CRM114-Status":"GOOD ( 20.19 )","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":"Andre Przywara ,\n\t\"kvmarm@lists.cs.columbia.edu\" ,\n\t\"linux-arm-kernel@lists.infradead.org\"\n\t, \n\t\"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"}}]