[{"id":1759952,"web_url":"http://patchwork.ozlabs.org/comment/1759952/","msgid":"<428bb30a-4d3d-d983-e1d1-8f160da846c5@redhat.com>","list_archive_url":null,"date":"2017-08-30T08:19:38","subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","submitter":{"id":69187,"url":"http://patchwork.ozlabs.org/api/people/69187/","name":"Eric Auger","email":"eric.auger@redhat.com"},"content":"Hi Christoffer,\n\nOn 29/08/2017 11:39, Christoffer Dall wrote:\n> Level-triggered mapped IRQs are special because we only observe rising\n> edges as input to the VGIC, and we don't set the EOI flag and therefore\n> are not told when the level goes down, so that we can re-queue a new\n> interrupt when the level goes up.\n> \n> One way to solve this problem is to side-step the logic of the VGIC and\n> special case the validation in the injection path, but it has the\n> unfortunate drawback of having to peak into the physical GIC state\n> whenever we want to know if the interrupt is pending on the virtual\n> distributor.\n> \n> Instead, we can maintain the current semantics of a level triggered\n> interrupt by sort of treating it as an edge-triggered interrupt,\n> following from the fact that we only observe an asserting edge. This\n> requires us to be a bit careful when populating the LRs and when folding\n> the state back in though:\n> \n> * We lower the line level when populating the LR, so that when\n> subsequently observing an asserting edge, the VGIC will do the right\n> thing.\n> \n> * If the guest never acked the interrupt while running (for example if\n> it had masked interrupts at the CPU level while running), we have\n> to preserve the pending state of the LR and move it back to the\n> line_level field of the struct irq when folding LR state.\n> \n> If the guest never acked the interrupt while running, but changed the\n> device state and lowered the line (again with interrupts masked) then\n> we need to observe this change in the line_level.\n> \n> Both of the above situations are solved by sampling the physical line\n> and set the line level when folding the LR back.\n> \n> * Finally, if the guest never acked the interrupt while running and\n> sampling the line reveals that the device state has changed and the\n> line has been lowered, we must clear the physical active state, since\n> we will otherwise never be told when the interrupt becomes asserted\n> again.\n> \n> This has the added benefit of making the timer optimization patches\n> (https://lists.cs.columbia.edu/pipermail/kvmarm/2017-July/026343.html) a\n> bit simpler, because the timer code doesn't have to clear the active\n> state on the sync anymore. It also potentially improves the performance\n> of the timer implementation because the GIC knows the state or the LR\n> and only needs to clear the\n> active state when the pending bit in the LR is still set, where the\n> timer has to always clear it when returning from running the guest with\n> an injected timer interrupt.\n> \n> Signed-off-by: Christoffer Dall \n> ---\n> virt/kvm/arm/vgic/vgic-v2.c | 29 +++++++++++++++++++++++++++++\n> virt/kvm/arm/vgic/vgic-v3.c | 29 +++++++++++++++++++++++++++++\n> virt/kvm/arm/vgic/vgic.c | 23 +++++++++++++++++++++++\n> virt/kvm/arm/vgic/vgic.h | 7 +++++++\n> 4 files changed, 88 insertions(+)\n> \n> diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c\n> index e4187e5..f7c5cb5 100644\n> --- a/virt/kvm/arm/vgic/vgic-v2.c\n> +++ b/virt/kvm/arm/vgic/vgic-v2.c\n> @@ -104,6 +104,26 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)\n> \t\t\t\tirq->pending_latch = false;\n> \t\t}\n> \n> +\t\t/*\n> +\t\t * Level-triggered mapped IRQs are special because we only\n> +\t\t * observe rising edges as input to the VGIC.\n> +\t\t *\n> +\t\t * If the guest never acked the interrupt we have to sample\n> +\t\t * the physical line and set the line level, because the\n> +\t\t * device state could have changed or we simply need to\n> +\t\t * process the still pending interrupt later.\n> +\t\t *\n> +\t\t * If this causes us to lower the level, we have to also clear\n> +\t\t * the physical active state, since we will otherwise never be\n> +\t\t * told when the interrupt becomes asserted again.\n> +\t\t */\n> +\t\tif (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT)) {\n> +\t\t\tirq->line_level = vgic_irq_line_level(irq);\n> +\n> +\t\t\tif (!irq->line_level)\n> +\t\t\t\tvgic_irq_clear_phys_active(irq);\n> +\t\t}\n> +\n> \t\tspin_unlock(&irq->irq_lock);\n> \t\tvgic_put_irq(vcpu->kvm, irq);\n> \t}\n> @@ -161,6 +181,15 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)\n> \t\t\tval |= GICH_LR_EOI;\n> \t}\n> \n> +\t/*\n> +\t * Level-triggered mapped IRQs are special because we only observe\n> +\t * rising edges as input to the VGIC. We therefore lower the line\n> +\t * level here, so that we can take new virtual IRQs. See\n> +\t * vgic_v2_fold_lr_state for more info.\n> +\t */\n> +\tif (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT))\n> +\t\tirq->line_level = false;\n> +\n> \t/* The GICv2 LR only holds five bits of priority. */\n> \tval |= (irq->priority >> 3) << GICH_LR_PRIORITY_SHIFT;\n> \n> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c\n> index 96ea597..e377036 100644\n> --- a/virt/kvm/arm/vgic/vgic-v3.c\n> +++ b/virt/kvm/arm/vgic/vgic-v3.c\n> @@ -94,6 +94,26 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)\n> \t\t\t\tirq->pending_latch = false;\n> \t\t}\n> \n> +\t\t/*\n> +\t\t * Level-triggered mapped IRQs are special because we only\n> +\t\t * observe rising edges as input to the VGIC.\n> +\t\t *\n> +\t\t * If the guest never acked the interrupt we have to sample\n> +\t\t * the physical line and set the line level, because the\n> +\t\t * device state could have changed or we simply need to\n> +\t\t * process the still pending interrupt later.\n> +\t\t *\n> +\t\t * If this causes us to lower the level, we have to also clear\n> +\t\t * the physical active state, since we will otherwise never be\n> +\t\t * told when the interrupt becomes asserted again.\n> +\t\t */\n> +\t\tif (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) {\n> +\t\t\tirq->line_level = vgic_irq_line_level(irq);\n\nI don't see anything related to the GICD_ISPENDING/ICPENDING. In the\nlast thread, we said we were obliged to set the pending bit on the\nphysical distributor to avoid the guest deactivating a non active\nphysical IRQ. If we still intend to do so, with that addition, aren't we\nlikely to deactivate the physical IRQ before the guest?\n\nThanks\n\nEric\n> +\n> +\t\t\tif (!irq->line_level)\n> +\t\t\t\tvgic_irq_clear_phys_active(irq);\n> +\t\t}\n> +\n> \t\tspin_unlock(&irq->irq_lock);\n> \t\tvgic_put_irq(vcpu->kvm, irq);\n> \t}\n> @@ -144,6 +164,15 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)\n> \t}\n> \n> \t/*\n> +\t * Level-triggered mapped IRQs are special because we only observe\n> +\t * rising edges as input to the VGIC. We therefore lower the line\n> +\t * level here, so that we can take new virtual IRQs. See\n> +\t * vgic_v3_fold_lr_state for more info.\n> +\t */\n> +\tif (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT))\n> +\t\tirq->line_level = false;\n> +\n> +\t/*\n> \t * We currently only support Group1 interrupts, which is a\n> \t * known defect. This needs to be addressed at some point.\n> \t */\n> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c\n> index 9d557efd..2691a0a 100644\n> --- a/virt/kvm/arm/vgic/vgic.c\n> +++ b/virt/kvm/arm/vgic/vgic.c\n> @@ -140,6 +140,29 @@ void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq)\n> \tkfree(irq);\n> }\n> \n> +/* Get the input level of a mapped IRQ directly from the physical GIC */\n> +bool vgic_irq_line_level(struct vgic_irq *irq)\n> +{\n> +\tbool line_level;\n> +\n> +\tBUG_ON(!irq->hw);\n> +\n> +\tWARN_ON(irq_get_irqchip_state(irq->host_irq,\n> +\t\t\t\t IRQCHIP_STATE_PENDING,\n> +\t\t\t\t &line_level));\n> +\treturn line_level;\n> +}\n> +\n> +/* Clear the physical active state */\n> +void vgic_irq_clear_phys_active(struct vgic_irq *irq)\n> +{\n> +\n> +\tBUG_ON(!irq->hw);\n> +\tWARN_ON(irq_set_irqchip_state(irq->host_irq,\n> +\t\t\t\t IRQCHIP_STATE_ACTIVE,\n> +\t\t\t\t false));\n> +}\n> +\n> /**\n> * kvm_vgic_target_oracle - compute the target vcpu for an irq\n> *\n> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h\n> index bba7fa2..22f106d 100644\n> --- a/virt/kvm/arm/vgic/vgic.h\n> +++ b/virt/kvm/arm/vgic/vgic.h\n> @@ -104,6 +104,11 @@ static inline bool irq_is_pending(struct vgic_irq *irq)\n> \t\treturn irq->pending_latch || irq->line_level;\n> }\n> \n> +static inline bool vgic_irq_is_mapped_level(struct vgic_irq *irq)\n> +{\n> +\treturn irq->config == VGIC_CONFIG_LEVEL && irq->hw;\n> +}\n> +\n> /*\n> * This struct provides an intermediate representation of the fields contained\n> * in the GICH_VMCR and ICH_VMCR registers, such that code exporting the GIC\n> @@ -140,6 +145,8 @@ vgic_get_mmio_region(struct kvm_vcpu *vcpu, struct vgic_io_device *iodev,\n> 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_irq_line_level(struct vgic_irq *irq);\n> +void vgic_irq_clear_phys_active(struct vgic_irq *irq);\n> bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq);\n> void vgic_kick_vcpus(struct kvm *kvm);\n> \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=\"HxKpVl86\"; 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 3xhz2z5CrJz9s9Y\n\tfor ;\n\tWed, 30 Aug 2017 18:20:26 +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 1dmyEM-0003I2-SP; Wed, 30 Aug 2017 08:20:10 +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 1dmyEG-0002JS-D5 for linux-arm-kernel@lists.infradead.org;\n\tWed, 30 Aug 2017 08:20:08 +0000","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\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 E9AEA80473;\n\tWed, 30 Aug 2017 08:19:42 +0000 (UTC)","from localhost.localdomain (ovpn-116-92.ams2.redhat.com\n\t[10.36.116.92])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id CF8DF96547;\n\tWed, 30 Aug 2017 08:19:39 +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=Oi5dYC/osTWUDAQvCWTdqMBcODleHjcSxaFeIVPf4iU=;\n\tb=HxKpVl86lTZisE\n\tCouqtidrJyTSfLMqM1HTBJ86LXqwBpPRoe9z/DkuhPqY55liLOaY5ayF2kanCSKSNWmIDc5pp4sR/\n\turWxaDUUxH8eE1Vk2CP3qT40D5KTRCYspzRGLePj3f4S7Qob28mGCISAB5GPUHV8ss6uSmk4iv9yN\n\t/fAIjx816+wXLom2setNlM+FiChX/xLX1s4N9TQEH1aqAeHYWRhYUtUwEC4SdKbtXa0GsK8aL2UZ1\n\t0s1ttShuzBbXWSn0djxZ0rK313n78yXpJdyb6O2cbuo5ssMpWY+m08b/JpVuPo9bYySUtEt78a9Ml\n\tOegrYLDA9LqroiBKAXKw==;","DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com E9AEA80473","Subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","To":"Christoffer Dall , kvmarm@lists.cs.columbia.edu,\n\tMarc Zyngier ","References":"<20170829093902.15379-1-cdall@linaro.org>\n\t<20170829093902.15379-3-cdall@linaro.org>","From":"Auger Eric ","Message-ID":"<428bb30a-4d3d-d983-e1d1-8f160da846c5@redhat.com>","Date":"Wed, 30 Aug 2017 10:19:38 +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":"<20170829093902.15379-3-cdall@linaro.org>","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]); Wed, 30 Aug 2017 08:19:43 +0000 (UTC)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170830_012004_545806_F0E724CF ","X-CRM114-Status":"GOOD ( 33.51 )","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 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":1759987,"web_url":"http://patchwork.ozlabs.org/comment/1759987/","msgid":"<20170830092026.GC24522@cbox>","list_archive_url":null,"date":"2017-08-30T09:20:26","subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","submitter":{"id":71350,"url":"http://patchwork.ozlabs.org/api/people/71350/","name":"Christoffer Dall","email":"cdall@linaro.org"},"content":"On Wed, Aug 30, 2017 at 10:19:38AM +0200, Auger Eric wrote:\n> Hi Christoffer,\n> \n> On 29/08/2017 11:39, Christoffer Dall wrote:\n> > Level-triggered mapped IRQs are special because we only observe rising\n> > edges as input to the VGIC, and we don't set the EOI flag and therefore\n> > are not told when the level goes down, so that we can re-queue a new\n> > interrupt when the level goes up.\n> > \n> > One way to solve this problem is to side-step the logic of the VGIC and\n> > special case the validation in the injection path, but it has the\n> > unfortunate drawback of having to peak into the physical GIC state\n> > whenever we want to know if the interrupt is pending on the virtual\n> > distributor.\n> > \n> > Instead, we can maintain the current semantics of a level triggered\n> > interrupt by sort of treating it as an edge-triggered interrupt,\n> > following from the fact that we only observe an asserting edge. This\n> > requires us to be a bit careful when populating the LRs and when folding\n> > the state back in though:\n> > \n> > * We lower the line level when populating the LR, so that when\n> > subsequently observing an asserting edge, the VGIC will do the right\n> > thing.\n> > \n> > * If the guest never acked the interrupt while running (for example if\n> > it had masked interrupts at the CPU level while running), we have\n> > to preserve the pending state of the LR and move it back to the\n> > line_level field of the struct irq when folding LR state.\n> > \n> > If the guest never acked the interrupt while running, but changed the\n> > device state and lowered the line (again with interrupts masked) then\n> > we need to observe this change in the line_level.\n> > \n> > Both of the above situations are solved by sampling the physical line\n> > and set the line level when folding the LR back.\n> > \n> > * Finally, if the guest never acked the interrupt while running and\n> > sampling the line reveals that the device state has changed and the\n> > line has been lowered, we must clear the physical active state, since\n> > we will otherwise never be told when the interrupt becomes asserted\n> > again.\n> > \n> > This has the added benefit of making the timer optimization patches\n> > (https://lists.cs.columbia.edu/pipermail/kvmarm/2017-July/026343.html) a\n> > bit simpler, because the timer code doesn't have to clear the active\n> > state on the sync anymore. It also potentially improves the performance\n> > of the timer implementation because the GIC knows the state or the LR\n> > and only needs to clear the\n> > active state when the pending bit in the LR is still set, where the\n> > timer has to always clear it when returning from running the guest with\n> > an injected timer interrupt.\n> > \n> > Signed-off-by: Christoffer Dall \n> > ---\n> > virt/kvm/arm/vgic/vgic-v2.c | 29 +++++++++++++++++++++++++++++\n> > virt/kvm/arm/vgic/vgic-v3.c | 29 +++++++++++++++++++++++++++++\n> > virt/kvm/arm/vgic/vgic.c | 23 +++++++++++++++++++++++\n> > virt/kvm/arm/vgic/vgic.h | 7 +++++++\n> > 4 files changed, 88 insertions(+)\n> > \n> > diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c\n> > index e4187e5..f7c5cb5 100644\n> > --- a/virt/kvm/arm/vgic/vgic-v2.c\n> > +++ b/virt/kvm/arm/vgic/vgic-v2.c\n> > @@ -104,6 +104,26 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)\n> > \t\t\t\tirq->pending_latch = false;\n> > \t\t}\n> > \n> > +\t\t/*\n> > +\t\t * Level-triggered mapped IRQs are special because we only\n> > +\t\t * observe rising edges as input to the VGIC.\n> > +\t\t *\n> > +\t\t * If the guest never acked the interrupt we have to sample\n> > +\t\t * the physical line and set the line level, because the\n> > +\t\t * device state could have changed or we simply need to\n> > +\t\t * process the still pending interrupt later.\n> > +\t\t *\n> > +\t\t * If this causes us to lower the level, we have to also clear\n> > +\t\t * the physical active state, since we will otherwise never be\n> > +\t\t * told when the interrupt becomes asserted again.\n> > +\t\t */\n> > +\t\tif (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT)) {\n> > +\t\t\tirq->line_level = vgic_irq_line_level(irq);\n> > +\n> > +\t\t\tif (!irq->line_level)\n> > +\t\t\t\tvgic_irq_clear_phys_active(irq);\n> > +\t\t}\n> > +\n> > \t\tspin_unlock(&irq->irq_lock);\n> > \t\tvgic_put_irq(vcpu->kvm, irq);\n> > \t}\n> > @@ -161,6 +181,15 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)\n> > \t\t\tval |= GICH_LR_EOI;\n> > \t}\n> > \n> > +\t/*\n> > +\t * Level-triggered mapped IRQs are special because we only observe\n> > +\t * rising edges as input to the VGIC. We therefore lower the line\n> > +\t * level here, so that we can take new virtual IRQs. See\n> > +\t * vgic_v2_fold_lr_state for more info.\n> > +\t */\n> > +\tif (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT))\n> > +\t\tirq->line_level = false;\n> > +\n> > \t/* The GICv2 LR only holds five bits of priority. */\n> > \tval |= (irq->priority >> 3) << GICH_LR_PRIORITY_SHIFT;\n> > \n> > diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c\n> > index 96ea597..e377036 100644\n> > --- a/virt/kvm/arm/vgic/vgic-v3.c\n> > +++ b/virt/kvm/arm/vgic/vgic-v3.c\n> > @@ -94,6 +94,26 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)\n> > \t\t\t\tirq->pending_latch = false;\n> > \t\t}\n> > \n> > +\t\t/*\n> > +\t\t * Level-triggered mapped IRQs are special because we only\n> > +\t\t * observe rising edges as input to the VGIC.\n> > +\t\t *\n> > +\t\t * If the guest never acked the interrupt we have to sample\n> > +\t\t * the physical line and set the line level, because the\n> > +\t\t * device state could have changed or we simply need to\n> > +\t\t * process the still pending interrupt later.\n> > +\t\t *\n> > +\t\t * If this causes us to lower the level, we have to also clear\n> > +\t\t * the physical active state, since we will otherwise never be\n> > +\t\t * told when the interrupt becomes asserted again.\n> > +\t\t */\n> > +\t\tif (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) {\n> > +\t\t\tirq->line_level = vgic_irq_line_level(irq);\n> \n> I don't see anything related to the GICD_ISPENDING/ICPENDING.\n\nvgic_irq_line_level() reads GICD_ISPEND. Not sure what you mean?\n\n> In the\n> last thread, we said we were obliged to set the pending bit on the\n> physical distributor to avoid the guest deactivating a non active\n> physical IRQ.\n\nDid we? I don't remember this, and this doesn't make sense to me. The\nonly constraint I think we have is that when setting the HW bit in the\nLR, the interrupt must be active on the physical side, so that a\ndeactivate by the guest is also a deactivate on the physical side.\n\n> If we still intend to do so, with that addition, aren't we\n> likely to deactivate the physical IRQ before the guest?\n\nDo you mean if it's pending+active in the LR? That can never happen for\na mapped (HW bit set) interrupt.\n\nSo this particular code sequence means that\n (val & ICH_LR_STATE) == ICH_LR_PENDING_BIT\n\nand we never clear the physical active state while the virtual interrupt\nis active.\n\nThere is an interesting point in terms of handling guest accesses to the\nvirtual distributor GICD_ISPENDR, GICD_ICPENDR, GICD_ISACTIVER, and\nGICD_ICACTIVER where we must also propogate those changes to the\nphysical side.\n\nWas this what you meant?\n\nThanks,\n-Christoffer\n\n\n> > +\n> > +\t\t\tif (!irq->line_level)\n> > +\t\t\t\tvgic_irq_clear_phys_active(irq);\n> > +\t\t}\n> > +\n> > \t\tspin_unlock(&irq->irq_lock);\n> > \t\tvgic_put_irq(vcpu->kvm, irq);\n> > \t}\n> > @@ -144,6 +164,15 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)\n> > \t}\n> > \n> > \t/*\n> > +\t * Level-triggered mapped IRQs are special because we only observe\n> > +\t * rising edges as input to the VGIC. We therefore lower the line\n> > +\t * level here, so that we can take new virtual IRQs. See\n> > +\t * vgic_v3_fold_lr_state for more info.\n> > +\t */\n> > +\tif (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT))\n> > +\t\tirq->line_level = false;\n> > +\n> > +\t/*\n> > \t * We currently only support Group1 interrupts, which is a\n> > \t * known defect. This needs to be addressed at some point.\n> > \t */\n> > diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c\n> > index 9d557efd..2691a0a 100644\n> > --- a/virt/kvm/arm/vgic/vgic.c\n> > +++ b/virt/kvm/arm/vgic/vgic.c\n> > @@ -140,6 +140,29 @@ void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq)\n> > \tkfree(irq);\n> > }\n> > \n> > +/* Get the input level of a mapped IRQ directly from the physical GIC */\n> > +bool vgic_irq_line_level(struct vgic_irq *irq)\n> > +{\n> > +\tbool line_level;\n> > +\n> > +\tBUG_ON(!irq->hw);\n> > +\n> > +\tWARN_ON(irq_get_irqchip_state(irq->host_irq,\n> > +\t\t\t\t IRQCHIP_STATE_PENDING,\n> > +\t\t\t\t &line_level));\n> > +\treturn line_level;\n> > +}\n> > +\n> > +/* Clear the physical active state */\n> > +void vgic_irq_clear_phys_active(struct vgic_irq *irq)\n> > +{\n> > +\n> > +\tBUG_ON(!irq->hw);\n> > +\tWARN_ON(irq_set_irqchip_state(irq->host_irq,\n> > +\t\t\t\t IRQCHIP_STATE_ACTIVE,\n> > +\t\t\t\t false));\n> > +}\n> > +\n> > /**\n> > * kvm_vgic_target_oracle - compute the target vcpu for an irq\n> > *\n> > diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h\n> > index bba7fa2..22f106d 100644\n> > --- a/virt/kvm/arm/vgic/vgic.h\n> > +++ b/virt/kvm/arm/vgic/vgic.h\n> > @@ -104,6 +104,11 @@ static inline bool irq_is_pending(struct vgic_irq *irq)\n> > \t\treturn irq->pending_latch || irq->line_level;\n> > }\n> > \n> > +static inline bool vgic_irq_is_mapped_level(struct vgic_irq *irq)\n> > +{\n> > +\treturn irq->config == VGIC_CONFIG_LEVEL && irq->hw;\n> > +}\n> > +\n> > /*\n> > * This struct provides an intermediate representation of the fields contained\n> > * in the GICH_VMCR and ICH_VMCR registers, such that code exporting the GIC\n> > @@ -140,6 +145,8 @@ vgic_get_mmio_region(struct kvm_vcpu *vcpu, struct vgic_io_device *iodev,\n> > 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_irq_line_level(struct vgic_irq *irq);\n> > +void vgic_irq_clear_phys_active(struct vgic_irq *irq);\n> > bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq);\n> > void vgic_kick_vcpus(struct kvm *kvm);\n> > \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 header.b=\"AmRYJvLs\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"KXCrX+GO\"; 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 3xj0Nt5xjtz9t3J\n\tfor ;\n\tWed, 30 Aug 2017 19:21:02 +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 1dmzBC-0003I7-VC; Wed, 30 Aug 2017 09:20:58 +0000","from mail-wm0-x22e.google.com ([2a00:1450:400c:c09::22e])\n\tby bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dmzB3-00034i-81 for linux-arm-kernel@lists.infradead.org;\n\tWed, 30 Aug 2017 09:20:56 +0000","by mail-wm0-x22e.google.com with SMTP id u126so5945720wmg.1\n\tfor ;\n\tWed, 30 Aug 2017 02:20:28 -0700 (PDT)","from localhost (xd93ddc2d.cust.hiper.dk. [217.61.220.45])\n\tby smtp.gmail.com with ESMTPSA id\n\tp31sm2327380edp.93.2017.08.30.02.20.26\n\t(version=TLS1_2 cipher=AES128-SHA bits=128/128);\n\tWed, 30 Aug 2017 02:20:26 -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=wng0WZ5pq+ovvp4tlBSU3ksmLEya3onXxzpBtm9bRAg=;\n\tb=AmRYJvLswubgCK\n\tvwRGgWcLDaIr2xrL+YEqWCuyszmtK+qB7zi/b8evYn+qPKVZFJkjwv7PkM8ThzVTiZjJr8igwlfmK\n\tBk8dFVKE0gYF3GC1kTfqKx/pBeBIAfA6v0hQd5W8skB8f8N625K7ylSoj7m93V3iQSbJ8rAtXcE0i\n\tQuus9wqnvoPnx6wgxz7qtMDC3TFB4y+k/pWYXHvdIyBQU+Vi2FVzPqWbHfySN7NpdEJVUPrBzubdF\n\tFG9UG/oBmW2JpZeVR3DeOM7hLEHS1/3k8i2gMsRker0scmj1rzb9eofyONEIi37dlycH1DYDzfVYW\n\tn2ocQCivdT01XsVfkBtA==;","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=EdQMAzXKnfRDIzfrf/evoZc1DiX18eKq3nEN2CrgHGE=;\n\tb=KXCrX+GO05LYuzQCvhteLI8XUYzjbhC2CzcRdsZNeOSx0WCqCMPgPGB1yul39HBEHU\n\t6be/iYrkudY74wSKGf2oM4SXcTvvzeMF8pQj0kk+9W0MsPVP3FI1uqswnc/LkTGecqBy\n\tmrOGCJIokgZ2/nkf0CibgCJwOREOkEgh2IGrI="],"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=EdQMAzXKnfRDIzfrf/evoZc1DiX18eKq3nEN2CrgHGE=;\n\tb=HJq2cS8s+QN1a5VQIUO8CbJj7hj1Xm9AGPD0QWpaFqwmYUOUueCw0n6CQNIjglbYAf\n\ti+z6Jhib2Xf5cLiZeIbvsVDsVcoYvdXEKBcwAcw8VXUHVhzEgcX1J5tFE4yx+iLi0kE+\n\tnRuHuG8U70ZO3Mqjq/zfkb2m2FrbOBkVkG5/BfyPFtOzgYP/pjO5QoYmOvJEVTwqC6r2\n\tijODEv0Of+ypnbHyB059nfNm+ToSSi7CMUQEy4oQXyfyIf8xd5KGMzp4oD35tl1YdELR\n\t+mKSXh+Ggy2uwwzZh5iDlnpfi2jfaWqCe/7g7/f4WMJxBnKu1ByWHGDSMvClSw4362Xo\n\tRwbg==","X-Gm-Message-State":"AHYfb5jyJnQI65PznB/bW088PLUP2hWOGWoEZXejcfGqMecM327UUXtf\n\t8kxJqDQDUcjM1Q6dtSQl9w==","X-Received":"by 10.80.213.140 with SMTP id v12mr200077edi.89.1504084827044;\n\tWed, 30 Aug 2017 02:20:27 -0700 (PDT)","Date":"Wed, 30 Aug 2017 11:20:26 +0200","From":"Christoffer Dall ","To":"Auger Eric ","Subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","Message-ID":"<20170830092026.GC24522@cbox>","References":"<20170829093902.15379-1-cdall@linaro.org>\n\t<20170829093902.15379-3-cdall@linaro.org>\n\t<428bb30a-4d3d-d983-e1d1-8f160da846c5@redhat.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<428bb30a-4d3d-d983-e1d1-8f160da846c5@redhat.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-20170830_022050_365970_56D9457F ","X-CRM114-Status":"GOOD ( 40.02 )","X-Spam-Score":"-2.7 (--)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-2.7 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/,\n\tlow\n\ttrust [2a00:1450:400c:c09:0:0:0:22e 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 , kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org, 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":1760019,"web_url":"http://patchwork.ozlabs.org/comment/1760019/","msgid":"","list_archive_url":null,"date":"2017-08-30T10:13:32","subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","submitter":{"id":69187,"url":"http://patchwork.ozlabs.org/api/people/69187/","name":"Eric Auger","email":"eric.auger@redhat.com"},"content":"Hi Christoffer,\n\nOn 30/08/2017 11:20, Christoffer Dall wrote:\n> On Wed, Aug 30, 2017 at 10:19:38AM +0200, Auger Eric wrote:\n>> Hi Christoffer,\n>>\n>> On 29/08/2017 11:39, Christoffer Dall wrote:\n>>> Level-triggered mapped IRQs are special because we only observe rising\n>>> edges as input to the VGIC, and we don't set the EOI flag and therefore\n>>> are not told when the level goes down, so that we can re-queue a new\n>>> interrupt when the level goes up.\n>>>\n>>> One way to solve this problem is to side-step the logic of the VGIC and\n>>> special case the validation in the injection path, but it has the\n>>> unfortunate drawback of having to peak into the physical GIC state\n>>> whenever we want to know if the interrupt is pending on the virtual\n>>> distributor.\n>>>\n>>> Instead, we can maintain the current semantics of a level triggered\n>>> interrupt by sort of treating it as an edge-triggered interrupt,\n>>> following from the fact that we only observe an asserting edge. This\n>>> requires us to be a bit careful when populating the LRs and when folding\n>>> the state back in though:\n>>>\n>>> * We lower the line level when populating the LR, so that when\n>>> subsequently observing an asserting edge, the VGIC will do the right\n>>> thing.\n>>>\n>>> * If the guest never acked the interrupt while running (for example if\n>>> it had masked interrupts at the CPU level while running), we have\n>>> to preserve the pending state of the LR and move it back to the\n>>> line_level field of the struct irq when folding LR state.\n>>>\n>>> If the guest never acked the interrupt while running, but changed the\n>>> device state and lowered the line (again with interrupts masked) then\n>>> we need to observe this change in the line_level.\n>>>\n>>> Both of the above situations are solved by sampling the physical line\n>>> and set the line level when folding the LR back.\n>>>\n>>> * Finally, if the guest never acked the interrupt while running and\n>>> sampling the line reveals that the device state has changed and the\n>>> line has been lowered, we must clear the physical active state, since\n>>> we will otherwise never be told when the interrupt becomes asserted\n>>> again.\n>>>\n>>> This has the added benefit of making the timer optimization patches\n>>> (https://lists.cs.columbia.edu/pipermail/kvmarm/2017-July/026343.html) a\n>>> bit simpler, because the timer code doesn't have to clear the active\n>>> state on the sync anymore. It also potentially improves the performance\n>>> of the timer implementation because the GIC knows the state or the LR\n>>> and only needs to clear the\n>>> active state when the pending bit in the LR is still set, where the\n>>> timer has to always clear it when returning from running the guest with\n>>> an injected timer interrupt.\n>>>\n>>> Signed-off-by: Christoffer Dall \n>>> ---\n>>> virt/kvm/arm/vgic/vgic-v2.c | 29 +++++++++++++++++++++++++++++\n>>> virt/kvm/arm/vgic/vgic-v3.c | 29 +++++++++++++++++++++++++++++\n>>> virt/kvm/arm/vgic/vgic.c | 23 +++++++++++++++++++++++\n>>> virt/kvm/arm/vgic/vgic.h | 7 +++++++\n>>> 4 files changed, 88 insertions(+)\n>>>\n>>> diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c\n>>> index e4187e5..f7c5cb5 100644\n>>> --- a/virt/kvm/arm/vgic/vgic-v2.c\n>>> +++ b/virt/kvm/arm/vgic/vgic-v2.c\n>>> @@ -104,6 +104,26 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)\n>>> \t\t\t\tirq->pending_latch = false;\n>>> \t\t}\n>>> \n>>> +\t\t/*\n>>> +\t\t * Level-triggered mapped IRQs are special because we only\n>>> +\t\t * observe rising edges as input to the VGIC.\n>>> +\t\t *\n>>> +\t\t * If the guest never acked the interrupt we have to sample\n>>> +\t\t * the physical line and set the line level, because the\n>>> +\t\t * device state could have changed or we simply need to\n>>> +\t\t * process the still pending interrupt later.\n>>> +\t\t *\n>>> +\t\t * If this causes us to lower the level, we have to also clear\n>>> +\t\t * the physical active state, since we will otherwise never be\n>>> +\t\t * told when the interrupt becomes asserted again.\n>>> +\t\t */\n>>> +\t\tif (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT)) {\n>>> +\t\t\tirq->line_level = vgic_irq_line_level(irq);\n>>> +\n>>> +\t\t\tif (!irq->line_level)\n>>> +\t\t\t\tvgic_irq_clear_phys_active(irq);\n>>> +\t\t}\n>>> +\n>>> \t\tspin_unlock(&irq->irq_lock);\n>>> \t\tvgic_put_irq(vcpu->kvm, irq);\n>>> \t}\n>>> @@ -161,6 +181,15 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)\n>>> \t\t\tval |= GICH_LR_EOI;\n>>> \t}\n>>> \n>>> +\t/*\n>>> +\t * Level-triggered mapped IRQs are special because we only observe\n>>> +\t * rising edges as input to the VGIC. We therefore lower the line\n>>> +\t * level here, so that we can take new virtual IRQs. See\n>>> +\t * vgic_v2_fold_lr_state for more info.\n>>> +\t */\n>>> +\tif (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT))\n>>> +\t\tirq->line_level = false;\n>>> +\n>>> \t/* The GICv2 LR only holds five bits of priority. */\n>>> \tval |= (irq->priority >> 3) << GICH_LR_PRIORITY_SHIFT;\n>>> \n>>> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c\n>>> index 96ea597..e377036 100644\n>>> --- a/virt/kvm/arm/vgic/vgic-v3.c\n>>> +++ b/virt/kvm/arm/vgic/vgic-v3.c\n>>> @@ -94,6 +94,26 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)\n>>> \t\t\t\tirq->pending_latch = false;\n>>> \t\t}\n>>> \n>>> +\t\t/*\n>>> +\t\t * Level-triggered mapped IRQs are special because we only\n>>> +\t\t * observe rising edges as input to the VGIC.\n>>> +\t\t *\n>>> +\t\t * If the guest never acked the interrupt we have to sample\n>>> +\t\t * the physical line and set the line level, because the\n>>> +\t\t * device state could have changed or we simply need to\n>>> +\t\t * process the still pending interrupt later.\n>>> +\t\t *\n>>> +\t\t * If this causes us to lower the level, we have to also clear\n>>> +\t\t * the physical active state, since we will otherwise never be\n>>> +\t\t * told when the interrupt becomes asserted again.\n>>> +\t\t */\n>>> +\t\tif (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) {\n>>> +\t\t\tirq->line_level = vgic_irq_line_level(irq);\n>>\n>> I don't see anything related to the GICD_ISPENDING/ICPENDING.\n> \n> vgic_irq_line_level() reads GICD_ISPEND. Not sure what you mean?\n> \n>> In the\n>> last thread, we said we were obliged to set the pending bit on the\n>> physical distributor to avoid the guest deactivating a non active\n>> physical IRQ.\n> \n> Did we? I don't remember this, and this doesn't make sense to me. The\n> only constraint I think we have is that when setting the HW bit in the\n> LR, the interrupt must be active on the physical side, so that a\n> deactivate by the guest is also a deactivate on the physical side.\n\nThat what I understood from this thread:\nhttps://lkml.org/lkml/2017/7/25/534\n\nAt the moment the LR HW bit is set whatever the source of the IRQ I\nthink, HW driven or ISPENDRn driven (we only test irq->hw)\n\n\n> \n>> If we still intend to do so, with that addition, aren't we\n>> likely to deactivate the physical IRQ before the guest?\n> \n> Do you mean if it's pending+active in the LR? That can never happen for\n> a mapped (HW bit set) interrupt.\n> \n> So this particular code sequence means that\n> (val & ICH_LR_STATE) == ICH_LR_PENDING_BIT\n> \n> and we never clear the physical active state while the virtual interrupt\n> is active.\n\nNo my scenario was:\nIRQ originates from a ISPENDRn. I Thought we set the physical\ndistributor state accordingly. pIRQ is active. vIRQ is pending. On fold\nwe detect the line is low and we deactivate the pIRQ. pending_latch is\nstill true. The vIRQ will be processed but its physical IRQ has been\ndeactivated.\n\n> There is an interesting point in terms of handling guest accesses to the\n> virtual distributor GICD_ISPENDR, GICD_ICPENDR, GICD_ISACTIVER, and\n> GICD_ICACTIVER where we must also propogate those changes to the\n> physical side.\n> \n> Was this what you meant?\nyes I think we also need to address this to get the whole picture.\n\nThanks\n\nEric\n> \n> Thanks,\n> -Christoffer\n> \n> \n>>> +\n>>> +\t\t\tif (!irq->line_level)\n>>> +\t\t\t\tvgic_irq_clear_phys_active(irq);\n>>> +\t\t}\n>>> +\n>>> \t\tspin_unlock(&irq->irq_lock);\n>>> \t\tvgic_put_irq(vcpu->kvm, irq);\n>>> \t}\n>>> @@ -144,6 +164,15 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)\n>>> \t}\n>>> \n>>> \t/*\n>>> +\t * Level-triggered mapped IRQs are special because we only observe\n>>> +\t * rising edges as input to the VGIC. We therefore lower the line\n>>> +\t * level here, so that we can take new virtual IRQs. See\n>>> +\t * vgic_v3_fold_lr_state for more info.\n>>> +\t */\n>>> +\tif (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT))\n>>> +\t\tirq->line_level = false;\n>>> +\n>>> +\t/*\n>>> \t * We currently only support Group1 interrupts, which is a\n>>> \t * known defect. This needs to be addressed at some point.\n>>> \t */\n>>> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c\n>>> index 9d557efd..2691a0a 100644\n>>> --- a/virt/kvm/arm/vgic/vgic.c\n>>> +++ b/virt/kvm/arm/vgic/vgic.c\n>>> @@ -140,6 +140,29 @@ void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq)\n>>> \tkfree(irq);\n>>> }\n>>> \n>>> +/* Get the input level of a mapped IRQ directly from the physical GIC */\n>>> +bool vgic_irq_line_level(struct vgic_irq *irq)\n>>> +{\n>>> +\tbool line_level;\n>>> +\n>>> +\tBUG_ON(!irq->hw);\n>>> +\n>>> +\tWARN_ON(irq_get_irqchip_state(irq->host_irq,\n>>> +\t\t\t\t IRQCHIP_STATE_PENDING,\n>>> +\t\t\t\t &line_level));\n>>> +\treturn line_level;\n>>> +}\n>>> +\n>>> +/* Clear the physical active state */\n>>> +void vgic_irq_clear_phys_active(struct vgic_irq *irq)\n>>> +{\n>>> +\n>>> +\tBUG_ON(!irq->hw);\n>>> +\tWARN_ON(irq_set_irqchip_state(irq->host_irq,\n>>> +\t\t\t\t IRQCHIP_STATE_ACTIVE,\n>>> +\t\t\t\t false));\n>>> +}\n>>> +\n>>> /**\n>>> * kvm_vgic_target_oracle - compute the target vcpu for an irq\n>>> *\n>>> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h\n>>> index bba7fa2..22f106d 100644\n>>> --- a/virt/kvm/arm/vgic/vgic.h\n>>> +++ b/virt/kvm/arm/vgic/vgic.h\n>>> @@ -104,6 +104,11 @@ static inline bool irq_is_pending(struct vgic_irq *irq)\n>>> \t\treturn irq->pending_latch || irq->line_level;\n>>> }\n>>> \n>>> +static inline bool vgic_irq_is_mapped_level(struct vgic_irq *irq)\n>>> +{\n>>> +\treturn irq->config == VGIC_CONFIG_LEVEL && irq->hw;\n>>> +}\n>>> +\n>>> /*\n>>> * This struct provides an intermediate representation of the fields contained\n>>> * in the GICH_VMCR and ICH_VMCR registers, such that code exporting the GIC\n>>> @@ -140,6 +145,8 @@ vgic_get_mmio_region(struct kvm_vcpu *vcpu, struct vgic_io_device *iodev,\n>>> 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_irq_line_level(struct vgic_irq *irq);\n>>> +void vgic_irq_clear_phys_active(struct vgic_irq *irq);\n>>> bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq);\n>>> void vgic_kick_vcpus(struct kvm *kvm);\n>>> \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=\"mY8EsmQs\"; dkim-atps=neutral","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx03.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 3xj1Z75WDgz9t0F\n\tfor ;\n\tWed, 30 Aug 2017 20:14:07 +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 1dn00Y-0000vb-Jn; Wed, 30 Aug 2017 10:14:02 +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 1dn00T-0000qc-Aj for linux-arm-kernel@lists.infradead.org;\n\tWed, 30 Aug 2017 10:13:59 +0000","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\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 67151804E3;\n\tWed, 30 Aug 2017 10:13:36 +0000 (UTC)","from localhost.localdomain (ovpn-116-92.ams2.redhat.com\n\t[10.36.116.92])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 6272317125;\n\tWed, 30 Aug 2017 10:13: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=Kyi3SvNQJ8eQ3EVFLOm5npUpo8ZsGiOIux3voFQ7xf4=;\n\tb=mY8EsmQsp7LEZw\n\td746mmVD4vNhTs7Yeiyk/VRggy/7VHKUPz+WJ8bgwrWxC+4iSTik8mufpT/xRLbvXcp/HyglkWn57\n\tGOFkIQ6UwjoZxN759DQtnA3PKv4VGVYt3Sk84PEOE+Pl5mo0zpSJYz8qDp/3E3a+jID//FlLoSRZR\n\t53wzWRbJI/YDoW5ZMapWb+Lb9YAyO+QW88E+yLeoYOFByk0OZIYpZzOJNIRDHX6DkWTbq7qEFrkCx\n\tZ07ikUEsWF+nbWCdUVs5dlYjo27ivIJ1i8oWrEtt7AMVyF4MxrQaaWrrNLUx8y+D+pcYwvQspMT3O\n\tL+vo54fwZfPC0mTGNnbA==;","DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 67151804E3","Subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","To":"Christoffer Dall ","References":"<20170829093902.15379-1-cdall@linaro.org>\n\t<20170829093902.15379-3-cdall@linaro.org>\n\t<428bb30a-4d3d-d983-e1d1-8f160da846c5@redhat.com>\n\t<20170830092026.GC24522@cbox>","From":"Auger Eric ","Message-ID":"","Date":"Wed, 30 Aug 2017 12:13:32 +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":"<20170830092026.GC24522@cbox>","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.27]); Wed, 30 Aug 2017 10:13:36 +0000 (UTC)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170830_031357_425755_682E04BE ","X-CRM114-Status":"GOOD ( 33.38 )","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 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":"Marc Zyngier ,\n\tAndre Przywara , kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org, 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":1760093,"web_url":"http://patchwork.ozlabs.org/comment/1760093/","msgid":"<20170830120302.GH24522@cbox>","list_archive_url":null,"date":"2017-08-30T12:03:02","subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","submitter":{"id":71350,"url":"http://patchwork.ozlabs.org/api/people/71350/","name":"Christoffer Dall","email":"cdall@linaro.org"},"content":"On Wed, Aug 30, 2017 at 12:13:32PM +0200, Auger Eric wrote:\n> Hi Christoffer,\n> \n> On 30/08/2017 11:20, Christoffer Dall wrote:\n> > On Wed, Aug 30, 2017 at 10:19:38AM +0200, Auger Eric wrote:\n> >> Hi Christoffer,\n> >>\n> >> On 29/08/2017 11:39, Christoffer Dall wrote:\n> >>> Level-triggered mapped IRQs are special because we only observe rising\n> >>> edges as input to the VGIC, and we don't set the EOI flag and therefore\n> >>> are not told when the level goes down, so that we can re-queue a new\n> >>> interrupt when the level goes up.\n> >>>\n> >>> One way to solve this problem is to side-step the logic of the VGIC and\n> >>> special case the validation in the injection path, but it has the\n> >>> unfortunate drawback of having to peak into the physical GIC state\n> >>> whenever we want to know if the interrupt is pending on the virtual\n> >>> distributor.\n> >>>\n> >>> Instead, we can maintain the current semantics of a level triggered\n> >>> interrupt by sort of treating it as an edge-triggered interrupt,\n> >>> following from the fact that we only observe an asserting edge. This\n> >>> requires us to be a bit careful when populating the LRs and when folding\n> >>> the state back in though:\n> >>>\n> >>> * We lower the line level when populating the LR, so that when\n> >>> subsequently observing an asserting edge, the VGIC will do the right\n> >>> thing.\n> >>>\n> >>> * If the guest never acked the interrupt while running (for example if\n> >>> it had masked interrupts at the CPU level while running), we have\n> >>> to preserve the pending state of the LR and move it back to the\n> >>> line_level field of the struct irq when folding LR state.\n> >>>\n> >>> If the guest never acked the interrupt while running, but changed the\n> >>> device state and lowered the line (again with interrupts masked) then\n> >>> we need to observe this change in the line_level.\n> >>>\n> >>> Both of the above situations are solved by sampling the physical line\n> >>> and set the line level when folding the LR back.\n> >>>\n> >>> * Finally, if the guest never acked the interrupt while running and\n> >>> sampling the line reveals that the device state has changed and the\n> >>> line has been lowered, we must clear the physical active state, since\n> >>> we will otherwise never be told when the interrupt becomes asserted\n> >>> again.\n> >>>\n> >>> This has the added benefit of making the timer optimization patches\n> >>> (https://lists.cs.columbia.edu/pipermail/kvmarm/2017-July/026343.html) a\n> >>> bit simpler, because the timer code doesn't have to clear the active\n> >>> state on the sync anymore. It also potentially improves the performance\n> >>> of the timer implementation because the GIC knows the state or the LR\n> >>> and only needs to clear the\n> >>> active state when the pending bit in the LR is still set, where the\n> >>> timer has to always clear it when returning from running the guest with\n> >>> an injected timer interrupt.\n> >>>\n> >>> Signed-off-by: Christoffer Dall \n> >>> ---\n> >>> virt/kvm/arm/vgic/vgic-v2.c | 29 +++++++++++++++++++++++++++++\n> >>> virt/kvm/arm/vgic/vgic-v3.c | 29 +++++++++++++++++++++++++++++\n> >>> virt/kvm/arm/vgic/vgic.c | 23 +++++++++++++++++++++++\n> >>> virt/kvm/arm/vgic/vgic.h | 7 +++++++\n> >>> 4 files changed, 88 insertions(+)\n> >>>\n> >>> diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c\n> >>> index e4187e5..f7c5cb5 100644\n> >>> --- a/virt/kvm/arm/vgic/vgic-v2.c\n> >>> +++ b/virt/kvm/arm/vgic/vgic-v2.c\n> >>> @@ -104,6 +104,26 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)\n> >>> \t\t\t\tirq->pending_latch = false;\n> >>> \t\t}\n> >>> \n> >>> +\t\t/*\n> >>> +\t\t * Level-triggered mapped IRQs are special because we only\n> >>> +\t\t * observe rising edges as input to the VGIC.\n> >>> +\t\t *\n> >>> +\t\t * If the guest never acked the interrupt we have to sample\n> >>> +\t\t * the physical line and set the line level, because the\n> >>> +\t\t * device state could have changed or we simply need to\n> >>> +\t\t * process the still pending interrupt later.\n> >>> +\t\t *\n> >>> +\t\t * If this causes us to lower the level, we have to also clear\n> >>> +\t\t * the physical active state, since we will otherwise never be\n> >>> +\t\t * told when the interrupt becomes asserted again.\n> >>> +\t\t */\n> >>> +\t\tif (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT)) {\n> >>> +\t\t\tirq->line_level = vgic_irq_line_level(irq);\n> >>> +\n> >>> +\t\t\tif (!irq->line_level)\n> >>> +\t\t\t\tvgic_irq_clear_phys_active(irq);\n> >>> +\t\t}\n> >>> +\n> >>> \t\tspin_unlock(&irq->irq_lock);\n> >>> \t\tvgic_put_irq(vcpu->kvm, irq);\n> >>> \t}\n> >>> @@ -161,6 +181,15 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)\n> >>> \t\t\tval |= GICH_LR_EOI;\n> >>> \t}\n> >>> \n> >>> +\t/*\n> >>> +\t * Level-triggered mapped IRQs are special because we only observe\n> >>> +\t * rising edges as input to the VGIC. We therefore lower the line\n> >>> +\t * level here, so that we can take new virtual IRQs. See\n> >>> +\t * vgic_v2_fold_lr_state for more info.\n> >>> +\t */\n> >>> +\tif (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT))\n> >>> +\t\tirq->line_level = false;\n> >>> +\n> >>> \t/* The GICv2 LR only holds five bits of priority. */\n> >>> \tval |= (irq->priority >> 3) << GICH_LR_PRIORITY_SHIFT;\n> >>> \n> >>> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c\n> >>> index 96ea597..e377036 100644\n> >>> --- a/virt/kvm/arm/vgic/vgic-v3.c\n> >>> +++ b/virt/kvm/arm/vgic/vgic-v3.c\n> >>> @@ -94,6 +94,26 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)\n> >>> \t\t\t\tirq->pending_latch = false;\n> >>> \t\t}\n> >>> \n> >>> +\t\t/*\n> >>> +\t\t * Level-triggered mapped IRQs are special because we only\n> >>> +\t\t * observe rising edges as input to the VGIC.\n> >>> +\t\t *\n> >>> +\t\t * If the guest never acked the interrupt we have to sample\n> >>> +\t\t * the physical line and set the line level, because the\n> >>> +\t\t * device state could have changed or we simply need to\n> >>> +\t\t * process the still pending interrupt later.\n> >>> +\t\t *\n> >>> +\t\t * If this causes us to lower the level, we have to also clear\n> >>> +\t\t * the physical active state, since we will otherwise never be\n> >>> +\t\t * told when the interrupt becomes asserted again.\n> >>> +\t\t */\n> >>> +\t\tif (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) {\n> >>> +\t\t\tirq->line_level = vgic_irq_line_level(irq);\n> >>\n> >> I don't see anything related to the GICD_ISPENDING/ICPENDING.\n> > \n> > vgic_irq_line_level() reads GICD_ISPEND. Not sure what you mean?\n> > \n> >> In the\n> >> last thread, we said we were obliged to set the pending bit on the\n> >> physical distributor to avoid the guest deactivating a non active\n> >> physical IRQ.\n> > \n> > Did we? I don't remember this, and this doesn't make sense to me. The\n> > only constraint I think we have is that when setting the HW bit in the\n> > LR, the interrupt must be active on the physical side, so that a\n> > deactivate by the guest is also a deactivate on the physical side.\n> \n> That what I understood from this thread:\n> https://lkml.org/lkml/2017/7/25/534\n\nRight, but that was specific to how we handle the guest writing to\nPEND/ACT on the virtual distributor.\n\n> \n> At the moment the LR HW bit is set whatever the source of the IRQ I\n> think, HW driven or ISPENDRn driven (we only test irq->hw)\n> \n\nAh, I didn't actually consider that we could special-case the\npending_latch for a HW interrupt. I suppose we should actually consider\nthat if someone tries to map an edge-triggered SPI. Let me have a look\nat that.\n\n> \n> > \n> >> If we still intend to do so, with that addition, aren't we\n> >> likely to deactivate the physical IRQ before the guest?\n> > \n> > Do you mean if it's pending+active in the LR? That can never happen for\n> > a mapped (HW bit set) interrupt.\n> > \n> > So this particular code sequence means that\n> > (val & ICH_LR_STATE) == ICH_LR_PENDING_BIT\n> > \n> > and we never clear the physical active state while the virtual interrupt\n> > is active.\n> \n> No my scenario was:\n> IRQ originates from a ISPENDRn.\n\nvirtual IRQ from a write to the VGIC ISPENDRn, correct?\n\n> I Thought we set the physical\n> distributor state accordingly. pIRQ is active. vIRQ is pending. On fold\n> we detect the line is low and we deactivate the pIRQ. pending_latch is\n> still true. The vIRQ will be processed but its physical IRQ has been\n> deactivated.\n> \n\nAs long as we properly handle pending_latch and guest writes to the VGIC\nPEND/ACT registers, I don't see a problem with this code.\n\n> > There is an interesting point in terms of handling guest accesses to the\n> > virtual distributor GICD_ISPENDR, GICD_ICPENDR, GICD_ISACTIVER, and\n> > GICD_ICACTIVER where we must also propogate those changes to the\n> > physical side.\n> > \n> > Was this what you meant?\n> yes I think we also need to address this to get the whole picture.\n> \n\nYes, I'll respin.\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=\"KWcgruBU\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"evAxlNF6\"; 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 3xj40f4rNSz9t1t\n\tfor ;\n\tWed, 30 Aug 2017 22:03:46 +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 1dn1ig-0000Lu-FV; Wed, 30 Aug 2017 12:03:42 +0000","from mail-wm0-x233.google.com ([2a00:1450:400c:c09::233])\n\tby bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dn1iW-000094-Ba for linux-arm-kernel@lists.infradead.org;\n\tWed, 30 Aug 2017 12:03:39 +0000","by mail-wm0-x233.google.com with SMTP id u26so9139805wma.0\n\tfor ;\n\tWed, 30 Aug 2017 05:03:11 -0700 (PDT)","from localhost (xd93ddc2d.cust.hiper.dk. [217.61.220.45])\n\tby smtp.gmail.com with ESMTPSA id\n\tc26sm4236249eda.74.2017.08.30.05.03.02\n\t(version=TLS1_2 cipher=AES128-SHA bits=128/128);\n\tWed, 30 Aug 2017 05:03:03 -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=mfn/hPQ5CijQBAXx7eFMzYeHiZWr7JxmJ3vroFpTUKw=;\n\tb=KWcgruBUKGcdYr\n\tfZiyafXUPEaglOIr8AhRdj3M1h3L1CHbqYh6BjnHIFHg4fMHghR1NLJ+Y81NsBsSa0N9UrWvngXly\n\tEoLmV5Ih8ofB39P7adTY7PwDGl+6rpEA/98KZJZ87k7yLP+CXWJ2Q/U7IER67q5A/A2ySaxILbu5p\n\t48o7h07tbfigHJYb3QSreTZo365emBkNbFfkrpZ4IEQFMUy3JRzGvCz7fZmfJ8fFhSRKcYF20nL9g\n\tW8KvVa8TiQGiU9n+AEkfooFXAHGo4Fgp8w/SwTYCfXEQNQSlM4zpxCVuCuGq1RO19sWkWDWwuSbAY\n\tt/Hm8kZ1v6MbrffpOa/Q==;","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=h3WA02okKHX5XugrjHVhJ6P2RnOIlKmU039aCp7EKfs=;\n\tb=evAxlNF6p7qzz4pzXy+EZPRbL28LswQtfQnNdBW9wYAMWLbJ19pAK6/xaeGNZ5fz73\n\t5VZNFSG0eh1kpd0nge5Syw080o9+GTk/Vz0f8jOd3cvaAosvY8eb1CW6WT2UvfWXH0v/\n\taiAGwqig9XZshtpt0pm28ja7KftQw+jisqgvs="],"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=h3WA02okKHX5XugrjHVhJ6P2RnOIlKmU039aCp7EKfs=;\n\tb=CMz7Nk2XEe5qO24wlii5/JpLnP0i671sOGoZ+D4XAxQ3mNgF9Vii6k8a0oI2JzA473\n\t91aceWcNHK6tOdk1SIsJikpgp4/fniPZ11uC55UbVGylyaU+WzNPMd94QVw6n8idtYVM\n\tc4A3DFPgieP26V3a43BJkC8C7kdjpFjjEsGYzuQbMI/9dd0Aw0qZFk+9S6Em+13aFTFb\n\tKBycBkGABP+3xvSdednl3C6i+Z/jMwCBuirNhN5PEjnyxnGh0gIk51fj4I0AiN7NzeMm\n\tWsWB4S0vy6wA08egrwCnEd+DWb+NY6FTPFS5YSNAD5on0o2LBvGzPJg+YL7VQMGLVRij\n\tMg1w==","X-Gm-Message-State":"AHYfb5j6+8Z1QUgp38WdWJdlM4zcZdSL92DN0TTxWMAt7CSPIAo6Jtwk\n\tbzjZNhJzS6iKMgs6ngApuA==","X-Received":"by 10.80.181.38 with SMTP id y35mr250057edd.304.1504094583724;\n\tWed, 30 Aug 2017 05:03:03 -0700 (PDT)","Date":"Wed, 30 Aug 2017 14:03:02 +0200","From":"Christoffer Dall ","To":"Auger Eric ","Subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","Message-ID":"<20170830120302.GH24522@cbox>","References":"<20170829093902.15379-1-cdall@linaro.org>\n\t<20170829093902.15379-3-cdall@linaro.org>\n\t<428bb30a-4d3d-d983-e1d1-8f160da846c5@redhat.com>\n\t<20170830092026.GC24522@cbox>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","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-20170830_050332_744262_5863CFB5 ","X-CRM114-Status":"GOOD ( 42.63 )","X-Spam-Score":"-2.7 (--)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-2.7 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/,\n\tlow\n\ttrust [2a00:1450:400c:c09:0:0:0:233 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 , kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org, 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":1760131,"web_url":"http://patchwork.ozlabs.org/comment/1760131/","msgid":"<993d1d14-c6e9-ca52-4009-1ee55ece55a8@redhat.com>","list_archive_url":null,"date":"2017-08-30T12:57:37","subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","submitter":{"id":69187,"url":"http://patchwork.ozlabs.org/api/people/69187/","name":"Eric Auger","email":"eric.auger@redhat.com"},"content":"Hi Christoffer,\n\nOn 30/08/2017 14:03, Christoffer Dall wrote:\n> On Wed, Aug 30, 2017 at 12:13:32PM +0200, Auger Eric wrote:\n>> Hi Christoffer,\n>>\n>> On 30/08/2017 11:20, Christoffer Dall wrote:\n>>> On Wed, Aug 30, 2017 at 10:19:38AM +0200, Auger Eric wrote:\n>>>> Hi Christoffer,\n>>>>\n>>>> On 29/08/2017 11:39, Christoffer Dall wrote:\n>>>>> Level-triggered mapped IRQs are special because we only observe rising\n>>>>> edges as input to the VGIC, and we don't set the EOI flag and therefore\n>>>>> are not told when the level goes down, so that we can re-queue a new\n>>>>> interrupt when the level goes up.\n>>>>>\n>>>>> One way to solve this problem is to side-step the logic of the VGIC and\n>>>>> special case the validation in the injection path, but it has the\n>>>>> unfortunate drawback of having to peak into the physical GIC state\n>>>>> whenever we want to know if the interrupt is pending on the virtual\n>>>>> distributor.\n>>>>>\n>>>>> Instead, we can maintain the current semantics of a level triggered\n>>>>> interrupt by sort of treating it as an edge-triggered interrupt,\n>>>>> following from the fact that we only observe an asserting edge. This\n>>>>> requires us to be a bit careful when populating the LRs and when folding\n>>>>> the state back in though:\n>>>>>\n>>>>> * We lower the line level when populating the LR, so that when\n>>>>> subsequently observing an asserting edge, the VGIC will do the right\n>>>>> thing.\n>>>>>\n>>>>> * If the guest never acked the interrupt while running (for example if\n>>>>> it had masked interrupts at the CPU level while running), we have\n>>>>> to preserve the pending state of the LR and move it back to the\n>>>>> line_level field of the struct irq when folding LR state.\n>>>>>\n>>>>> If the guest never acked the interrupt while running, but changed the\n>>>>> device state and lowered the line (again with interrupts masked) then\n>>>>> we need to observe this change in the line_level.\n>>>>>\n>>>>> Both of the above situations are solved by sampling the physical line\n>>>>> and set the line level when folding the LR back.\n>>>>>\n>>>>> * Finally, if the guest never acked the interrupt while running and\n>>>>> sampling the line reveals that the device state has changed and the\n>>>>> line has been lowered, we must clear the physical active state, since\n>>>>> we will otherwise never be told when the interrupt becomes asserted\n>>>>> again.\n>>>>>\n>>>>> This has the added benefit of making the timer optimization patches\n>>>>> (https://lists.cs.columbia.edu/pipermail/kvmarm/2017-July/026343.html) a\n>>>>> bit simpler, because the timer code doesn't have to clear the active\n>>>>> state on the sync anymore. It also potentially improves the performance\n>>>>> of the timer implementation because the GIC knows the state or the LR\n>>>>> and only needs to clear the\n>>>>> active state when the pending bit in the LR is still set, where the\n>>>>> timer has to always clear it when returning from running the guest with\n>>>>> an injected timer interrupt.\n>>>>>\n>>>>> Signed-off-by: Christoffer Dall \n>>>>> ---\n>>>>> virt/kvm/arm/vgic/vgic-v2.c | 29 +++++++++++++++++++++++++++++\n>>>>> virt/kvm/arm/vgic/vgic-v3.c | 29 +++++++++++++++++++++++++++++\n>>>>> virt/kvm/arm/vgic/vgic.c | 23 +++++++++++++++++++++++\n>>>>> virt/kvm/arm/vgic/vgic.h | 7 +++++++\n>>>>> 4 files changed, 88 insertions(+)\n>>>>>\n>>>>> diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c\n>>>>> index e4187e5..f7c5cb5 100644\n>>>>> --- a/virt/kvm/arm/vgic/vgic-v2.c\n>>>>> +++ b/virt/kvm/arm/vgic/vgic-v2.c\n>>>>> @@ -104,6 +104,26 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)\n>>>>> \t\t\t\tirq->pending_latch = false;\n>>>>> \t\t}\n>>>>> \n>>>>> +\t\t/*\n>>>>> +\t\t * Level-triggered mapped IRQs are special because we only\n>>>>> +\t\t * observe rising edges as input to the VGIC.\n>>>>> +\t\t *\n>>>>> +\t\t * If the guest never acked the interrupt we have to sample\n>>>>> +\t\t * the physical line and set the line level, because the\n>>>>> +\t\t * device state could have changed or we simply need to\n>>>>> +\t\t * process the still pending interrupt later.\n>>>>> +\t\t *\n>>>>> +\t\t * If this causes us to lower the level, we have to also clear\n>>>>> +\t\t * the physical active state, since we will otherwise never be\n>>>>> +\t\t * told when the interrupt becomes asserted again.\n>>>>> +\t\t */\n>>>>> +\t\tif (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT)) {\n>>>>> +\t\t\tirq->line_level = vgic_irq_line_level(irq);\n>>>>> +\n>>>>> +\t\t\tif (!irq->line_level)\n>>>>> +\t\t\t\tvgic_irq_clear_phys_active(irq);\n>>>>> +\t\t}\n>>>>> +\n>>>>> \t\tspin_unlock(&irq->irq_lock);\n>>>>> \t\tvgic_put_irq(vcpu->kvm, irq);\n>>>>> \t}\n>>>>> @@ -161,6 +181,15 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)\n>>>>> \t\t\tval |= GICH_LR_EOI;\n>>>>> \t}\n>>>>> \n>>>>> +\t/*\n>>>>> +\t * Level-triggered mapped IRQs are special because we only observe\n>>>>> +\t * rising edges as input to the VGIC. We therefore lower the line\n>>>>> +\t * level here, so that we can take new virtual IRQs. See\n>>>>> +\t * vgic_v2_fold_lr_state for more info.\n>>>>> +\t */\n>>>>> +\tif (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT))\n>>>>> +\t\tirq->line_level = false;\n>>>>> +\n>>>>> \t/* The GICv2 LR only holds five bits of priority. */\n>>>>> \tval |= (irq->priority >> 3) << GICH_LR_PRIORITY_SHIFT;\n>>>>> \n>>>>> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c\n>>>>> index 96ea597..e377036 100644\n>>>>> --- a/virt/kvm/arm/vgic/vgic-v3.c\n>>>>> +++ b/virt/kvm/arm/vgic/vgic-v3.c\n>>>>> @@ -94,6 +94,26 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)\n>>>>> \t\t\t\tirq->pending_latch = false;\n>>>>> \t\t}\n>>>>> \n>>>>> +\t\t/*\n>>>>> +\t\t * Level-triggered mapped IRQs are special because we only\n>>>>> +\t\t * observe rising edges as input to the VGIC.\n>>>>> +\t\t *\n>>>>> +\t\t * If the guest never acked the interrupt we have to sample\n>>>>> +\t\t * the physical line and set the line level, because the\n>>>>> +\t\t * device state could have changed or we simply need to\n>>>>> +\t\t * process the still pending interrupt later.\n>>>>> +\t\t *\n>>>>> +\t\t * If this causes us to lower the level, we have to also clear\n>>>>> +\t\t * the physical active state, since we will otherwise never be\n>>>>> +\t\t * told when the interrupt becomes asserted again.\n>>>>> +\t\t */\n>>>>> +\t\tif (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) {\n>>>>> +\t\t\tirq->line_level = vgic_irq_line_level(irq);\n>>>>\n>>>> I don't see anything related to the GICD_ISPENDING/ICPENDING.\n>>>\n>>> vgic_irq_line_level() reads GICD_ISPEND. Not sure what you mean?\n>>>\n>>>> In the\n>>>> last thread, we said we were obliged to set the pending bit on the\n>>>> physical distributor to avoid the guest deactivating a non active\n>>>> physical IRQ.\n>>>\n>>> Did we? I don't remember this, and this doesn't make sense to me. The\n>>> only constraint I think we have is that when setting the HW bit in the\n>>> LR, the interrupt must be active on the physical side, so that a\n>>> deactivate by the guest is also a deactivate on the physical side.\n>>\n>> That what I understood from this thread:\n>> https://lkml.org/lkml/2017/7/25/534\n> \n> Right, but that was specific to how we handle the guest writing to\n> PEND/ACT on the virtual distributor.\n> \n>>\n>> At the moment the LR HW bit is set whatever the source of the IRQ I\n>> think, HW driven or ISPENDRn driven (we only test irq->hw)\n>>\n> \n> Ah, I didn't actually consider that we could special-case the\n> pending_latch for a HW interrupt. I suppose we should actually consider\n> that if someone tries to map an edge-triggered SPI. Let me have a look\n> at that.\n> \n>>\n>>>\n>>>> If we still intend to do so, with that addition, aren't we\n>>>> likely to deactivate the physical IRQ before the guest?\n>>>\n>>> Do you mean if it's pending+active in the LR? That can never happen for\n>>> a mapped (HW bit set) interrupt.\n>>>\n>>> So this particular code sequence means that\n>>> (val & ICH_LR_STATE) == ICH_LR_PENDING_BIT\n>>>\n>>> and we never clear the physical active state while the virtual interrupt\n>>> is active.\n>>\n>> No my scenario was:\n>> IRQ originates from a ISPENDRn.\n> \n> virtual IRQ from a write to the VGIC ISPENDRn, correct?\ncorrect.\n> \n>> I Thought we set the physical\n>> distributor state accordingly. pIRQ is active. vIRQ is pending. On fold\n>> we detect the line is low and we deactivate the pIRQ. pending_latch is\n>> still true. The vIRQ will be processed but its physical IRQ has been\n>> deactivated.\n>>\n> \n> As long as we properly handle pending_latch and guest writes to the VGIC\n> PEND/ACT registers, I don't see a problem with this code.\nto me this depends if the LR HW bit is set.\n> \n>>> There is an interesting point in terms of handling guest accesses to the\n>>> virtual distributor GICD_ISPENDR, GICD_ICPENDR, GICD_ISACTIVER, and\n>>> GICD_ICACTIVER where we must also propogate those changes to the\n>>> physical side.\n>>>\n>>> Was this what you meant?\n>> yes I think we also need to address this to get the whole picture.\n>>\n> \n> Yes, I'll respin.\nOK\n\nThanks\n\nEric\n> \n> Thanks,\n> -Christoffer\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=\"gm0fhPts\"; 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 3xj5Dp1XL7z9sNn\n\tfor ;\n\tWed, 30 Aug 2017 22:59:22 +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 1dn2aR-0004Fo-E7; Wed, 30 Aug 2017 12:59:15 +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 1dn2ZF-0003WI-W7 for linux-arm-kernel@lists.infradead.org;\n\tWed, 30 Aug 2017 12:58:09 +0000","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\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 52F107EA89;\n\tWed, 30 Aug 2017 12:57:41 +0000 (UTC)","from localhost.localdomain (ovpn-116-92.ams2.redhat.com\n\t[10.36.116.92])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 7540697BB5;\n\tWed, 30 Aug 2017 12:57:39 +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=vngj8VoI4uoa/0vM6yeGE44WYLOw6ahHN1DqM8YpONM=;\n\tb=gm0fhPtsD7CjA/\n\tv3ZgtfjXVV745/+o6x0HZLQL++6rFFf/JNuk4v7hLe3utvPu1u2WBEoNTpYNc78MuCRfJsCnRN9NE\n\t5Pe0Fr6gewdc0yteeBKQbq7+eP7oJobSndXFWRgY8M+SJGVHw/Brk818EAhVtjBT1hwcvjAh7sqGx\n\tTI0JHmHpe1VdXEXLlSq5fpNPMg1ueCYasTFaoMiIt3V3aFctOyUBL4WsH9CJr/bnjac3rl8XEgC69\n\tpVqLN0/c8/3Ijz1NuyPEMgy4P6Pi5pmlEmkIsVt+zGClqrUkq199yh1625GU7XZ+qt2CpVZwl0bjA\n\tM3smT8Jef8sPyw+zA5NQ==;","DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 52F107EA89","Subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","To":"Christoffer Dall ","References":"<20170829093902.15379-1-cdall@linaro.org>\n\t<20170829093902.15379-3-cdall@linaro.org>\n\t<428bb30a-4d3d-d983-e1d1-8f160da846c5@redhat.com>\n\t<20170830092026.GC24522@cbox>\n\t\n\t<20170830120302.GH24522@cbox>","From":"Auger Eric ","Message-ID":"<993d1d14-c6e9-ca52-4009-1ee55ece55a8@redhat.com>","Date":"Wed, 30 Aug 2017 14:57:37 +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":"<20170830120302.GH24522@cbox>","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]); Wed, 30 Aug 2017 12:57:41 +0000 (UTC)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170830_055802_825939_4D88B411 ","X-CRM114-Status":"GOOD ( 33.14 )","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 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":"Marc Zyngier ,\n\tAndre Przywara , kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org, 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":1762104,"web_url":"http://patchwork.ozlabs.org/comment/1762104/","msgid":"<86efrpv0c6.fsf@arm.com>","list_archive_url":null,"date":"2017-09-02T10:52:57","subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","submitter":{"id":7353,"url":"http://patchwork.ozlabs.org/api/people/7353/","name":"Marc Zyngier","email":"marc.zyngier@arm.com"},"content":"On Tue, Aug 29 2017 at 11:39:00 am BST, Christoffer Dall wrote:\n> Level-triggered mapped IRQs are special because we only observe rising\n> edges as input to the VGIC, and we don't set the EOI flag and therefore\n> are not told when the level goes down, so that we can re-queue a new\n> interrupt when the level goes up.\n>\n> One way to solve this problem is to side-step the logic of the VGIC and\n> special case the validation in the injection path, but it has the\n> unfortunate drawback of having to peak into the physical GIC state\n> whenever we want to know if the interrupt is pending on the virtual\n> distributor.\n>\n> Instead, we can maintain the current semantics of a level triggered\n> interrupt by sort of treating it as an edge-triggered interrupt,\n> following from the fact that we only observe an asserting edge. This\n\nWould it be worth mentioning that this is a consequence of offloading\npart of the flow handling to the HW?\n\n> requires us to be a bit careful when populating the LRs and when folding\n> the state back in though:\n>\n> * We lower the line level when populating the LR, so that when\n> subsequently observing an asserting edge, the VGIC will do the right\n> thing.\n>\n> * If the guest never acked the interrupt while running (for example if\n> it had masked interrupts at the CPU level while running), we have\n> to preserve the pending state of the LR and move it back to the\n> line_level field of the struct irq when folding LR state.\n>\n> If the guest never acked the interrupt while running, but changed the\n> device state and lowered the line (again with interrupts masked) then\n> we need to observe this change in the line_level.\n>\n> Both of the above situations are solved by sampling the physical line\n> and set the line level when folding the LR back.\n>\n> * Finally, if the guest never acked the interrupt while running and\n> sampling the line reveals that the device state has changed and the\n> line has been lowered, we must clear the physical active state, since\n> we will otherwise never be told when the interrupt becomes asserted\n> again.\n>\n> This has the added benefit of making the timer optimization patches\n> (https://lists.cs.columbia.edu/pipermail/kvmarm/2017-July/026343.html) a\n> bit simpler, because the timer code doesn't have to clear the active\n> state on the sync anymore. It also potentially improves the performance\n> of the timer implementation because the GIC knows the state or the LR\n> and only needs to clear the\n> active state when the pending bit in the LR is still set, where the\n> timer has to always clear it when returning from running the guest with\n> an injected timer interrupt.\n>\n> Signed-off-by: Christoffer Dall \n> ---\n> virt/kvm/arm/vgic/vgic-v2.c | 29 +++++++++++++++++++++++++++++\n> virt/kvm/arm/vgic/vgic-v3.c | 29 +++++++++++++++++++++++++++++\n> virt/kvm/arm/vgic/vgic.c | 23 +++++++++++++++++++++++\n> virt/kvm/arm/vgic/vgic.h | 7 +++++++\n> 4 files changed, 88 insertions(+)\n>\n> diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c\n> index e4187e5..f7c5cb5 100644\n> --- a/virt/kvm/arm/vgic/vgic-v2.c\n> +++ b/virt/kvm/arm/vgic/vgic-v2.c\n> @@ -104,6 +104,26 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)\n> \t\t\t\tirq->pending_latch = false;\n> \t\t}\n> \n> +\t\t/*\n> +\t\t * Level-triggered mapped IRQs are special because we only\n> +\t\t * observe rising edges as input to the VGIC.\n> +\t\t *\n> +\t\t * If the guest never acked the interrupt we have to sample\n> +\t\t * the physical line and set the line level, because the\n> +\t\t * device state could have changed or we simply need to\n> +\t\t * process the still pending interrupt later.\n> +\t\t *\n> +\t\t * If this causes us to lower the level, we have to also clear\n> +\t\t * the physical active state, since we will otherwise never be\n> +\t\t * told when the interrupt becomes asserted again.\n> +\t\t */\n> +\t\tif (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT)) {\n> +\t\t\tirq->line_level = vgic_irq_line_level(irq);\n> +\n> +\t\t\tif (!irq->line_level)\n> +\t\t\t\tvgic_irq_clear_phys_active(irq);\n> +\t\t}\n> +\n> \t\tspin_unlock(&irq->irq_lock);\n> \t\tvgic_put_irq(vcpu->kvm, irq);\n> \t}\n> @@ -161,6 +181,15 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)\n> \t\t\tval |= GICH_LR_EOI;\n> \t}\n> \n> +\t/*\n> +\t * Level-triggered mapped IRQs are special because we only observe\n> +\t * rising edges as input to the VGIC. We therefore lower the line\n> +\t * level here, so that we can take new virtual IRQs. See\n> +\t * vgic_v2_fold_lr_state for more info.\n> +\t */\n> +\tif (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT))\n> +\t\tirq->line_level = false;\n> +\n> \t/* The GICv2 LR only holds five bits of priority. */\n> \tval |= (irq->priority >> 3) << GICH_LR_PRIORITY_SHIFT;\n> \n> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c\n> index 96ea597..e377036 100644\n> --- a/virt/kvm/arm/vgic/vgic-v3.c\n> +++ b/virt/kvm/arm/vgic/vgic-v3.c\n> @@ -94,6 +94,26 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)\n> \t\t\t\tirq->pending_latch = false;\n> \t\t}\n> \n> +\t\t/*\n> +\t\t * Level-triggered mapped IRQs are special because we only\n> +\t\t * observe rising edges as input to the VGIC.\n> +\t\t *\n> +\t\t * If the guest never acked the interrupt we have to sample\n> +\t\t * the physical line and set the line level, because the\n> +\t\t * device state could have changed or we simply need to\n> +\t\t * process the still pending interrupt later.\n> +\t\t *\n> +\t\t * If this causes us to lower the level, we have to also clear\n> +\t\t * the physical active state, since we will otherwise never be\n> +\t\t * told when the interrupt becomes asserted again.\n> +\t\t */\n> +\t\tif (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) {\n> +\t\t\tirq->line_level = vgic_irq_line_level(irq);\n> +\n> +\t\t\tif (!irq->line_level)\n> +\t\t\t\tvgic_irq_clear_phys_active(irq);\n> +\t\t}\n> +\n> \t\tspin_unlock(&irq->irq_lock);\n> \t\tvgic_put_irq(vcpu->kvm, irq);\n> \t}\n> @@ -144,6 +164,15 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)\n> \t}\n> \n> \t/*\n> +\t * Level-triggered mapped IRQs are special because we only observe\n> +\t * rising edges as input to the VGIC. We therefore lower the line\n> +\t * level here, so that we can take new virtual IRQs. See\n> +\t * vgic_v3_fold_lr_state for more info.\n> +\t */\n> +\tif (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT))\n> +\t\tirq->line_level = false;\n> +\n> +\t/*\n> \t * We currently only support Group1 interrupts, which is a\n> \t * known defect. This needs to be addressed at some point.\n> \t */\n> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c\n> index 9d557efd..2691a0a 100644\n> --- a/virt/kvm/arm/vgic/vgic.c\n> +++ b/virt/kvm/arm/vgic/vgic.c\n> @@ -140,6 +140,29 @@ void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq)\n> \tkfree(irq);\n> }\n> \n> +/* Get the input level of a mapped IRQ directly from the physical GIC */\n> +bool vgic_irq_line_level(struct vgic_irq *irq)\n\nnit: it would make the above clearer if this was named something like\nvgic_irq_get_phys_line_level(), as the current name is pretty ambiguous\nabout which side of the GIC we're looking at.\n\n> +{\n> +\tbool line_level;\n> +\n> +\tBUG_ON(!irq->hw);\n> +\n> +\tWARN_ON(irq_get_irqchip_state(irq->host_irq,\n> +\t\t\t\t IRQCHIP_STATE_PENDING,\n> +\t\t\t\t &line_level));\n> +\treturn line_level;\n> +}\n> +\n> +/* Clear the physical active state */\n> +void vgic_irq_clear_phys_active(struct vgic_irq *irq)\n> +{\n> +\n> +\tBUG_ON(!irq->hw);\n> +\tWARN_ON(irq_set_irqchip_state(irq->host_irq,\n> +\t\t\t\t IRQCHIP_STATE_ACTIVE,\n> +\t\t\t\t false));\n> +}\n> +\n> /**\n> * kvm_vgic_target_oracle - compute the target vcpu for an irq\n> *\n> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h\n> index bba7fa2..22f106d 100644\n> --- a/virt/kvm/arm/vgic/vgic.h\n> +++ b/virt/kvm/arm/vgic/vgic.h\n> @@ -104,6 +104,11 @@ static inline bool irq_is_pending(struct vgic_irq *irq)\n> \t\treturn irq->pending_latch || irq->line_level;\n> }\n> \n> +static inline bool vgic_irq_is_mapped_level(struct vgic_irq *irq)\n> +{\n> +\treturn irq->config == VGIC_CONFIG_LEVEL && irq->hw;\n> +}\n> +\n> /*\n> * This struct provides an intermediate representation of the fields contained\n> * in the GICH_VMCR and ICH_VMCR registers, such that code exporting the GIC\n> @@ -140,6 +145,8 @@ vgic_get_mmio_region(struct kvm_vcpu *vcpu, struct vgic_io_device *iodev,\n> 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_irq_line_level(struct vgic_irq *irq);\n> +void vgic_irq_clear_phys_active(struct vgic_irq *irq);\n> bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq);\n> void vgic_kick_vcpus(struct kvm *kvm);\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=\"kRp0Y1vU\"; 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 3xktJC3bsjz9sP1\n\tfor ;\n\tSat, 2 Sep 2017 20:53:31 +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 1do63L-0007vf-MR; Sat, 02 Sep 2017 10:53:27 +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 1do63H-0007uA-QI for linux-arm-kernel@lists.infradead.org;\n\tSat, 02 Sep 2017 10:53:25 +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 0E71380D;\n\tSat, 2 Sep 2017 03:53:03 -0700 (PDT)","from zomby-woof (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\t169B43F58F; Sat, 2 Sep 2017 03:53:00 -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=e5Dqhdp/3CuyDJeS+tYIJLWtVWmufgFyicpLzSqYltQ=;\n\tb=kRp0Y1vUNg+hyJ\n\t38ijjc+COX17Tz7pfXMLeFa+6Ivl/3TRmgH0tVxdKB/cD9eHkfvcNBMxPzfaN3Bb6LQ69kqOYldvS\n\tvZDn8+QUOHR2Cyx/9++vE6+ImRkOIw9BXa0WE3lxJb9KPKbNkM99B3CjjAsPw4R56pOqW73kB6kP9\n\tmRBlpNEUqtfKYGBvXeZCT5lXvaltVeq5qcf1D4uudNy5yIfT4SWdNMBbh8zNsV5QQU6TdqGImwg0h\n\tOaxb7rVk3+K1VhBiE9mQ59Fp6OguprNpa+8jGPwPdWgedxYLBjqTgne/sEi4mQYU1YXX8MsddckD1\n\tsqEB4YDB4+nGGWq+tYTA==;","From":"Marc Zyngier ","To":"Christoffer Dall ","Subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","In-Reply-To":"<20170829093902.15379-3-cdall@linaro.org> (Christoffer Dall's\n\tmessage of \"Tue, 29 Aug 2017 11:39:00 +0200\")","Organization":"ARM Ltd","References":"<20170829093902.15379-1-cdall@linaro.org>\n\t<20170829093902.15379-3-cdall@linaro.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)","Date":"Sat, 02 Sep 2017 11:52:57 +0100","Message-ID":"<86efrpv0c6.fsf@arm.com>","MIME-Version":"1.0","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170902_035323_868302_CECDD2BF ","X-CRM114-Status":"GOOD ( 29.00 )","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":"Eric Auger , kvm@vger.kernel.org,\n\tkvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org,\n\tAndre Przywara ","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":1762106,"web_url":"http://patchwork.ozlabs.org/comment/1762106/","msgid":"<867exhuztl.fsf@arm.com>","list_archive_url":null,"date":"2017-09-02T11:04:06","subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","submitter":{"id":7353,"url":"http://patchwork.ozlabs.org/api/people/7353/","name":"Marc Zyngier","email":"marc.zyngier@arm.com"},"content":"On Tue, Aug 29 2017 at 11:39:00 am BST, Christoffer Dall wrote:\n> Level-triggered mapped IRQs are special because we only observe rising\n> edges as input to the VGIC, and we don't set the EOI flag and therefore\n> are not told when the level goes down, so that we can re-queue a new\n> interrupt when the level goes up.\n>\n> One way to solve this problem is to side-step the logic of the VGIC and\n> special case the validation in the injection path, but it has the\n> unfortunate drawback of having to peak into the physical GIC state\n> whenever we want to know if the interrupt is pending on the virtual\n> distributor.\n>\n> Instead, we can maintain the current semantics of a level triggered\n> interrupt by sort of treating it as an edge-triggered interrupt,\n> following from the fact that we only observe an asserting edge. This\n> requires us to be a bit careful when populating the LRs and when folding\n> the state back in though:\n>\n> * We lower the line level when populating the LR, so that when\n> subsequently observing an asserting edge, the VGIC will do the right\n> thing.\n>\n> * If the guest never acked the interrupt while running (for example if\n> it had masked interrupts at the CPU level while running), we have\n> to preserve the pending state of the LR and move it back to the\n> line_level field of the struct irq when folding LR state.\n>\n> If the guest never acked the interrupt while running, but changed the\n> device state and lowered the line (again with interrupts masked) then\n> we need to observe this change in the line_level.\n>\n> Both of the above situations are solved by sampling the physical line\n> and set the line level when folding the LR back.\n>\n> * Finally, if the guest never acked the interrupt while running and\n> sampling the line reveals that the device state has changed and the\n> line has been lowered, we must clear the physical active state, since\n> we will otherwise never be told when the interrupt becomes asserted\n> again.\n>\n> This has the added benefit of making the timer optimization patches\n> (https://lists.cs.columbia.edu/pipermail/kvmarm/2017-July/026343.html) a\n> bit simpler, because the timer code doesn't have to clear the active\n> state on the sync anymore. It also potentially improves the performance\n> of the timer implementation because the GIC knows the state or the LR\n> and only needs to clear the\n> active state when the pending bit in the LR is still set, where the\n> timer has to always clear it when returning from running the guest with\n> an injected timer interrupt.\n>\n> Signed-off-by: Christoffer Dall \n> ---\n> virt/kvm/arm/vgic/vgic-v2.c | 29 +++++++++++++++++++++++++++++\n> virt/kvm/arm/vgic/vgic-v3.c | 29 +++++++++++++++++++++++++++++\n> virt/kvm/arm/vgic/vgic.c | 23 +++++++++++++++++++++++\n> virt/kvm/arm/vgic/vgic.h | 7 +++++++\n> 4 files changed, 88 insertions(+)\n>\n> diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c\n> index e4187e5..f7c5cb5 100644\n> --- a/virt/kvm/arm/vgic/vgic-v2.c\n> +++ b/virt/kvm/arm/vgic/vgic-v2.c\n> @@ -104,6 +104,26 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)\n> \t\t\t\tirq->pending_latch = false;\n> \t\t}\n> \n> +\t\t/*\n> +\t\t * Level-triggered mapped IRQs are special because we only\n> +\t\t * observe rising edges as input to the VGIC.\n> +\t\t *\n> +\t\t * If the guest never acked the interrupt we have to sample\n> +\t\t * the physical line and set the line level, because the\n> +\t\t * device state could have changed or we simply need to\n> +\t\t * process the still pending interrupt later.\n> +\t\t *\n> +\t\t * If this causes us to lower the level, we have to also clear\n> +\t\t * the physical active state, since we will otherwise never be\n> +\t\t * told when the interrupt becomes asserted again.\n> +\t\t */\n> +\t\tif (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT)) {\n\nI've just had a worrying though. Here, we're looking at the guest's view\nof the trigger. But shouldn't that be the *host's*? We're assuming that\nthe two should match, and while they certainly do for the timer, this is\nnot something that can be enforced for SPIs.\n\nWhat do you think?\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=\"cRFxvFnP\"; 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 3xktY506cDz9s3T\n\tfor ;\n\tSat, 2 Sep 2017 21:04: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 1do6E7-0005Fi-Ky; Sat, 02 Sep 2017 11:04:35 +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 1do6E4-0005EQ-E7 for linux-arm-kernel@lists.infradead.org;\n\tSat, 02 Sep 2017 11:04: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 7B71480D;\n\tSat, 2 Sep 2017 04:04:11 -0700 (PDT)","from zomby-woof (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\tD827D3F540; Sat, 2 Sep 2017 04:04:09 -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=6TTN2MkAuzojK8dUEmQYRmPp1QH7jrQpbG69JBnCL5Y=;\n\tb=cRFxvFnPnYU4I4\n\tbwNC9uUF7Mtom42YvUKfV7ez3WLZRNjb8tiO9gzNIJ+ZPIIcsrjdDXAroNrqUQJ0nOtQxxEhSU8AE\n\taRCkFrVq0z+HqR3+q39gJTO2HNcJbz5YOjuXYSb23VoSc05zV4Hq7OdaeyvUITYD6EKSf2rAIZYQs\n\tjMAhZ7RqbiWWPT7r7LGQIm9uNe2HxwH2bW8+T2rSa+la17kIdsYNALKpGYpYoLZRE0eCezr8UDZW3\n\tJHTyP05kWFEpGFUwjcYpvoAFOt46uNigvUnaoFhDHJMe/IepNe5BozMUydDSjo3VNtVV3tiYCag4D\n\t8883XOCLExVuoSc+usYw==;","From":"Marc Zyngier ","To":"Christoffer Dall ","Subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","In-Reply-To":"<20170829093902.15379-3-cdall@linaro.org> (Christoffer Dall's\n\tmessage of \"Tue, 29 Aug 2017 11:39:00 +0200\")","Organization":"ARM Ltd","References":"<20170829093902.15379-1-cdall@linaro.org>\n\t<20170829093902.15379-3-cdall@linaro.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)","Date":"Sat, 02 Sep 2017 12:04:06 +0100","Message-ID":"<867exhuztl.fsf@arm.com>","MIME-Version":"1.0","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170902_040432_493018_0B84B7AF ","X-CRM114-Status":"GOOD ( 24.94 )","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":"Eric Auger , kvm@vger.kernel.org,\n\tkvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org,\n\tAndre Przywara ","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":1762108,"web_url":"http://patchwork.ozlabs.org/comment/1762108/","msgid":"<863785uzdx.fsf@arm.com>","list_archive_url":null,"date":"2017-09-02T11:13:30","subject":"Re: [RFC PATCH 1/4] KVM: arm/arm64: vgic: restructure\n\tkvm_vgic_(un)map_phys_irq","submitter":{"id":7353,"url":"http://patchwork.ozlabs.org/api/people/7353/","name":"Marc Zyngier","email":"marc.zyngier@arm.com"},"content":"On Tue, Aug 29 2017 at 11:38:59 am BST, Christoffer Dall wrote:\n> From: Eric Auger \n>\n> We want to reuse the core of the map/unmap functions for IRQ forwarding.\n> Let's move the computation of the hwirq in kvm_vgic_map_phys_irq and\n> pass the linux IRQ as parameter.\n>\n> The host_irq is added to struct vgic_irq because it is needed in later\n> patches which manipulate the physical GIC state to support forwarded\n> IRQs.\n>\n> We introduce kvm_vgic_map/unmap_irq which take a struct vgic_irq handle\n> as a parameter.\n>\n> Signed-off-by: Eric Auger \n> Signed-off-by: Christoffer Dall \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=\"cTedNbed\"; 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 3xktm06RR1z9s4q\n\tfor ;\n\tSat, 2 Sep 2017 21:14:04 +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 1do6NE-0000mu-Kt; Sat, 02 Sep 2017 11:14:00 +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 1do6NB-0000lg-5r for linux-arm-kernel@lists.infradead.org;\n\tSat, 02 Sep 2017 11:13:58 +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 AA55980D;\n\tSat, 2 Sep 2017 04:13:36 -0700 (PDT)","from zomby-woof (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\t1422E3F540; Sat, 2 Sep 2017 04:13:34 -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=9BQANY6CR4iRwnzLXWUFI22dvz0uryGQMyl/g1gv3Xo=;\n\tb=cTedNbedi03JI4\n\tQNV8rMdwlzNY5YrkKEiVRmShPbOHPk6Mt4fqpJ03M5rTzml07yeV1pUV+ZKnrWz6hEkhcXmooUiUI\n\tKjPm5bH25b066fn9CjZ/ZgHpTbwZFuJ+FGWqSKEprdB5tvICVwpjs8GQrG/xhN7K47hlJG4t88KLK\n\t5nbIWTnlNfEFGKEZ3B7PoPcz1qdOR54GVnDH0oCSF132Z1R99hrT0WA77rlHM2+CCnXzNHz66EjBr\n\t4HVhKoT/z8hV6mMm8VAI+FZ6fpu+F/GqLmKj9zNiU6Ttj6C5NEvmn4nECFLCBCThZZTLq2uxgh7Op\n\t4wR+yE1ibyhUBxtvaWEw==;","From":"Marc Zyngier ","To":"Christoffer Dall ","Subject":"Re: [RFC PATCH 1/4] KVM: arm/arm64: vgic: restructure\n\tkvm_vgic_(un)map_phys_irq","In-Reply-To":"<20170829093902.15379-2-cdall@linaro.org> (Christoffer Dall's\n\tmessage of \"Tue, 29 Aug 2017 11:38:59 +0200\")","Organization":"ARM Ltd","References":"<20170829093902.15379-1-cdall@linaro.org>\n\t<20170829093902.15379-2-cdall@linaro.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)","Date":"Sat, 02 Sep 2017 12:13:30 +0100","Message-ID":"<863785uzdx.fsf@arm.com>","MIME-Version":"1.0","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170902_041357_230434_208BF0A8 ","X-CRM114-Status":"UNSURE ( 8.37 )","X-CRM114-Notice":"Please train this message.","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":"Eric Auger , kvm@vger.kernel.org,\n\tkvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org,\n\tAndre Przywara ","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":1762109,"web_url":"http://patchwork.ozlabs.org/comment/1762109/","msgid":"<86y3pxtkrv.fsf@arm.com>","list_archive_url":null,"date":"2017-09-02T11:14:28","subject":"Re: [RFC PATCH 3/4] KVM: arm/arm64: Rearrange kvm_vgic_[un]map_phys\n\tcode in vgic.c","submitter":{"id":7353,"url":"http://patchwork.ozlabs.org/api/people/7353/","name":"Marc Zyngier","email":"marc.zyngier@arm.com"},"content":"On Tue, Aug 29 2017 at 11:39:01 am BST, Christoffer Dall wrote:\n> The small indirection of a static function made the locking very obvious\n> but becomes pretty ugly as we start passing function pointer around.\n> Let's inline these two functions first to make the following patch more\n> readable.\n>\n> Signed-off-by: Christoffer Dall \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=\"bk4ofZtA\"; 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 3xktn06S0cz9s4q\n\tfor ;\n\tSat, 2 Sep 2017 21:15:00 +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 1do6O9-00017T-FY; Sat, 02 Sep 2017 11:14:57 +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 1do6O5-00010E-QL for linux-arm-kernel@lists.infradead.org;\n\tSat, 02 Sep 2017 11:14:55 +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 24A1680D;\n\tSat, 2 Sep 2017 04:14:33 -0700 (PDT)","from zomby-woof (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\t840783F540; Sat, 2 Sep 2017 04:14:31 -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=4f9xAQnJUu6yT0YKfObAYtnnbDzprK392vrmCIjgxts=;\n\tb=bk4ofZtAfrmDDJ\n\ti8wIdYQds37om3SwebnlpDxMBHMflENKH9Wb9Q6/NJmRu1M8b/Ev704/7lMqxp//lV7egHa9O1WJD\n\tWHsA7CwvWznOj68xiMppeYxYlpWlVPveV/dWiFHUCw9hfw7Fr9L646dec+3pqZooWTZO1XtfJ2mXk\n\tIEvvt5xQYZUJODcsRf4Q3yBgzYEIm3nq8RL9wnatsDgij2982p9Inh25MsC4inJYQixDvrrhTmIWe\n\tGnVYa/RgYypKHChuARo+JWoB329BxvSoDGSq2WF9pCJ/xpzjuuq8KT5mFszSd6lgLhxGkwjTFh1Ko\n\tTV3OJc74nA0mgCn76tsA==;","From":"Marc Zyngier ","To":"Christoffer Dall ","Subject":"Re: [RFC PATCH 3/4] KVM: arm/arm64: Rearrange kvm_vgic_[un]map_phys\n\tcode in vgic.c","In-Reply-To":"<20170829093902.15379-4-cdall@linaro.org> (Christoffer Dall's\n\tmessage of \"Tue, 29 Aug 2017 11:39:01 +0200\")","Organization":"ARM Ltd","References":"<20170829093902.15379-1-cdall@linaro.org>\n\t<20170829093902.15379-4-cdall@linaro.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)","Date":"Sat, 02 Sep 2017 12:14:28 +0100","Message-ID":"<86y3pxtkrv.fsf@arm.com>","MIME-Version":"1.0","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170902_041453_872049_FFFF4552 ","X-CRM114-Status":"UNSURE ( 5.70 )","X-CRM114-Notice":"Please train this message.","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":"Eric Auger , kvm@vger.kernel.org,\n\tkvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org,\n\tAndre Przywara ","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":1762110,"web_url":"http://patchwork.ozlabs.org/comment/1762110/","msgid":"<86tw0ltkhp.fsf@arm.com>","list_archive_url":null,"date":"2017-09-02T11:20:34","subject":"Re: [RFC PATCH 4/4] KVM: arm/arm64: Provide a vgic interrupt line\n\tlevel sample function","submitter":{"id":7353,"url":"http://patchwork.ozlabs.org/api/people/7353/","name":"Marc Zyngier","email":"marc.zyngier@arm.com"},"content":"On Tue, Aug 29 2017 at 11:39:02 am BST, Christoffer Dall wrote:\n> The GIC sometimes need to sample the physical line of a mapped\n> interrupt. As we know this to be notoriously slow, provide a callback\n> function for devices (such as the timer) which can do this much faster\n> than talking to the distributor, for example by comparing a few\n> in-memory values. Fall back to the good old method of poking the\n> physical GIC if no callback is provided.\n>\n> Signed-off-by: Christoffer Dall \n> ---\n> include/kvm/arm_vgic.h | 13 ++++++++++++-\n> virt/kvm/arm/arch_timer.c | 16 +++++++++++++++-\n> virt/kvm/arm/vgic/vgic.c | 7 ++++++-\n> 3 files changed, 33 insertions(+), 3 deletions(-)\n>\n> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h\n> index 53f631b..a52990b 100644\n> --- a/include/kvm/arm_vgic.h\n> +++ b/include/kvm/arm_vgic.h\n> @@ -125,6 +125,17 @@ struct vgic_irq {\n> \tu8 priority;\n> \tenum vgic_irq_config config;\t/* Level or edge */\n> \n> +\t/*\n> +\t * Callback function pointer to in-kernel devices that can tell us the\n> +\t * state of the input level of mapped level-triggered IRQ faster than\n> +\t * peaking into the physical GIC.\n> +\t *\n> +\t * Always called in non-preemptible section and the functions can use\n> +\t * kvm_arm_get_running_vcpu() to get the vcpu pointer for private\n> +\t * IRQs.\n> +\t */\n> +\tbool (*get_input_level)(int vintid);\n> +\n> \tvoid *owner;\t\t\t/* Opaque pointer to reserve an interrupt\n> \t\t\t\t\t for in-kernel devices. */\n> };\n> @@ -309,7 +320,7 @@ void kvm_vgic_init_cpu_hardware(void);\n> int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,\n> \t\t\tbool level, void *owner);\n> int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,\n> -\t\t\t u32 vintid);\n> +\t\t\t u32 vintid, bool (*callback)(int vindid));\n> int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, unsigned int vintid);\n> bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, unsigned int vintid);\n> \n> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c\n> index b24e2f7..82169ef 100644\n> --- a/virt/kvm/arm/arch_timer.c\n> +++ b/virt/kvm/arm/arch_timer.c\n> @@ -643,6 +643,19 @@ static bool timer_irqs_are_valid(struct kvm_vcpu *vcpu)\n> \treturn true;\n> }\n> \n> +static bool timer_get_input_level(int vintid)\n> +{\n> +\tstruct kvm_vcpu *vcpu = kvm_arm_get_running_vcpu();\n> +\tstruct arch_timer_context *timer;\n> +\n> +\tif (vintid == vcpu_vtimer(vcpu)->irq.irq)\n> +\t\ttimer = vcpu_vtimer(vcpu);\n> +\telse\n> +\t\tBUG(); /* We only map the vtimer so far */\n> +\n> +\treturn kvm_timer_should_fire(timer);\n> +}\n> +\n> int kvm_timer_enable(struct kvm_vcpu *vcpu)\n> {\n> \tstruct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;\n> @@ -664,7 +677,8 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)\n> \t\treturn -EINVAL;\n> \t}\n> \n> -\tret = kvm_vgic_map_phys_irq(vcpu, host_vtimer_irq, vtimer->irq.irq);\n> +\tret = kvm_vgic_map_phys_irq(vcpu, host_vtimer_irq, vtimer->irq.irq,\n> +\t\t\t\t &timer_get_input_level);\n\nnit: no need for a & here.\n\n> \tif (ret)\n> \t\treturn ret;\n> \n> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c\n> index e3ce2fa..0466c10 100644\n> --- a/virt/kvm/arm/vgic/vgic.c\n> +++ b/virt/kvm/arm/vgic/vgic.c\n> @@ -147,6 +147,9 @@ bool vgic_irq_line_level(struct vgic_irq *irq)\n> \n> \tBUG_ON(!irq->hw);\n> \n> +\tif (irq->get_input_level)\n> +\t\treturn irq->get_input_level(irq->intid);\n> +\n> \tWARN_ON(irq_get_irqchip_state(irq->host_irq,\n> \t\t\t\t IRQCHIP_STATE_PENDING,\n> \t\t\t\t &line_level));\n> @@ -429,7 +432,7 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,\n> }\n> \n> int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,\n> -\t\t\t u32 vintid)\n> +\t\t\t u32 vintid, bool (*callback)(int vindid))\n\nnit #2: \"callback\" is a very non-descriptive name for a callback... ;-)\nHow about calling it \"get_input_level\", matching the vgic_irq field?\n\n> {\n> \tstruct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, vintid);\n> \tstruct irq_desc *desc;\n> @@ -456,6 +459,7 @@ int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,\n> \tirq->hw = true;\n> \tirq->host_irq = host_irq;\n> \tirq->hwintid = data->hwirq;\n> +\tirq->get_input_level = callback;\n> \n> out:\n> \tspin_unlock(&irq->irq_lock);\n> @@ -476,6 +480,7 @@ int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, unsigned int vintid)\n> \tspin_lock(&irq->irq_lock);\n> \tirq->hw = false;\n> \tirq->hwintid = 0;\n> +\tirq->get_input_level = NULL;\n> \tspin_unlock(&irq->irq_lock);\n> \tvgic_put_irq(vcpu->kvm, irq);\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=\"iH0pbhkI\"; 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 3xktw05gw8z9s76\n\tfor ;\n\tSat, 2 Sep 2017 21:21:04 +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 1do6U2-0005hE-1t; Sat, 02 Sep 2017 11:21:02 +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 1do6Ty-0005fv-K3 for linux-arm-kernel@lists.infradead.org;\n\tSat, 02 Sep 2017 11:21:00 +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 4F8EE80D;\n\tSat, 2 Sep 2017 04:20:38 -0700 (PDT)","from zomby-woof (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\tA9FF73F540; Sat, 2 Sep 2017 04:20:36 -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=5fknUkyM1BUDnHSlz3Jh2BlX/FvoAy8LKtqsCMK1onw=;\n\tb=iH0pbhkIbJ+7VM\n\tFl17JnuJBBuYs18LyR6szpxuvdv+aFEL49/sDuKFDDEDhXYHvF1qPdrIy0n0OBqGJDV0wsgALDfD1\n\tyQ6lcFyOuhJJ9cQlk40Leh42mdfXLr9yT6XdFlwxgpRsqu9xVFSYv4PIFNh4DmfYuPq0J7SIq4v0K\n\taAYPLrZH1C9mSS9JXKux7X+9FWUmzlePyp6uQ+Vktq29r8hf/yU5Uqegx+gvc5Bnj+qpw6Nw7uEqs\n\tEwfTFrb1IkCDsRiuP+k/3/pSNd5D+2tf60Ou9cZCrsKoFlofuByWLrxvFpLFCQxgzpB9YKNbt6FXT\n\thAFApyH9gnIwbFhAeEdA==;","From":"Marc Zyngier ","To":"Christoffer Dall ","Subject":"Re: [RFC PATCH 4/4] KVM: arm/arm64: Provide a vgic interrupt line\n\tlevel sample function","In-Reply-To":"<20170829093902.15379-5-cdall@linaro.org> (Christoffer Dall's\n\tmessage of \"Tue, 29 Aug 2017 11:39:02 +0200\")","Organization":"ARM Ltd","References":"<20170829093902.15379-1-cdall@linaro.org>\n\t<20170829093902.15379-5-cdall@linaro.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)","Date":"Sat, 02 Sep 2017 12:20:34 +0100","Message-ID":"<86tw0ltkhp.fsf@arm.com>","MIME-Version":"1.0","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170902_042058_676887_14F5F0EE ","X-CRM114-Status":"GOOD ( 17.94 )","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":"Eric Auger , kvm@vger.kernel.org,\n\tkvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org,\n\tAndre Przywara ","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":1762182,"web_url":"http://patchwork.ozlabs.org/comment/1762182/","msgid":"<20170902202339.GG13572@cbox>","list_archive_url":null,"date":"2017-09-02T20:23:39","subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","submitter":{"id":71350,"url":"http://patchwork.ozlabs.org/api/people/71350/","name":"Christoffer Dall","email":"cdall@linaro.org"},"content":"On Sat, Sep 02, 2017 at 12:04:06PM +0100, Marc Zyngier wrote:\n> On Tue, Aug 29 2017 at 11:39:00 am BST, Christoffer Dall wrote:\n> > Level-triggered mapped IRQs are special because we only observe rising\n> > edges as input to the VGIC, and we don't set the EOI flag and therefore\n> > are not told when the level goes down, so that we can re-queue a new\n> > interrupt when the level goes up.\n> >\n> > One way to solve this problem is to side-step the logic of the VGIC and\n> > special case the validation in the injection path, but it has the\n> > unfortunate drawback of having to peak into the physical GIC state\n> > whenever we want to know if the interrupt is pending on the virtual\n> > distributor.\n> >\n> > Instead, we can maintain the current semantics of a level triggered\n> > interrupt by sort of treating it as an edge-triggered interrupt,\n> > following from the fact that we only observe an asserting edge. This\n> > requires us to be a bit careful when populating the LRs and when folding\n> > the state back in though:\n> >\n> > * We lower the line level when populating the LR, so that when\n> > subsequently observing an asserting edge, the VGIC will do the right\n> > thing.\n> >\n> > * If the guest never acked the interrupt while running (for example if\n> > it had masked interrupts at the CPU level while running), we have\n> > to preserve the pending state of the LR and move it back to the\n> > line_level field of the struct irq when folding LR state.\n> >\n> > If the guest never acked the interrupt while running, but changed the\n> > device state and lowered the line (again with interrupts masked) then\n> > we need to observe this change in the line_level.\n> >\n> > Both of the above situations are solved by sampling the physical line\n> > and set the line level when folding the LR back.\n> >\n> > * Finally, if the guest never acked the interrupt while running and\n> > sampling the line reveals that the device state has changed and the\n> > line has been lowered, we must clear the physical active state, since\n> > we will otherwise never be told when the interrupt becomes asserted\n> > again.\n> >\n> > This has the added benefit of making the timer optimization patches\n> > (https://lists.cs.columbia.edu/pipermail/kvmarm/2017-July/026343.html) a\n> > bit simpler, because the timer code doesn't have to clear the active\n> > state on the sync anymore. It also potentially improves the performance\n> > of the timer implementation because the GIC knows the state or the LR\n> > and only needs to clear the\n> > active state when the pending bit in the LR is still set, where the\n> > timer has to always clear it when returning from running the guest with\n> > an injected timer interrupt.\n> >\n> > Signed-off-by: Christoffer Dall \n> > ---\n> > virt/kvm/arm/vgic/vgic-v2.c | 29 +++++++++++++++++++++++++++++\n> > virt/kvm/arm/vgic/vgic-v3.c | 29 +++++++++++++++++++++++++++++\n> > virt/kvm/arm/vgic/vgic.c | 23 +++++++++++++++++++++++\n> > virt/kvm/arm/vgic/vgic.h | 7 +++++++\n> > 4 files changed, 88 insertions(+)\n> >\n> > diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c\n> > index e4187e5..f7c5cb5 100644\n> > --- a/virt/kvm/arm/vgic/vgic-v2.c\n> > +++ b/virt/kvm/arm/vgic/vgic-v2.c\n> > @@ -104,6 +104,26 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)\n> > \t\t\t\tirq->pending_latch = false;\n> > \t\t}\n> > \n> > +\t\t/*\n> > +\t\t * Level-triggered mapped IRQs are special because we only\n> > +\t\t * observe rising edges as input to the VGIC.\n> > +\t\t *\n> > +\t\t * If the guest never acked the interrupt we have to sample\n> > +\t\t * the physical line and set the line level, because the\n> > +\t\t * device state could have changed or we simply need to\n> > +\t\t * process the still pending interrupt later.\n> > +\t\t *\n> > +\t\t * If this causes us to lower the level, we have to also clear\n> > +\t\t * the physical active state, since we will otherwise never be\n> > +\t\t * told when the interrupt becomes asserted again.\n> > +\t\t */\n> > +\t\tif (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT)) {\n> \n> I've just had a worrying though. Here, we're looking at the guest's view\n> of the trigger. But shouldn't that be the *host's*? We're assuming that\n> the two should match, and while they certainly do for the timer, this is\n> not something that can be enforced for SPIs.\n> \n> What do you think?\n> \nThis is a tricky one. I think that in general, if the guest\nmisconfigures the level/edge of a virtual interrupt, it is only setting\nitself up for trouble, which doesn't interfere with the host\nfunctionality, so we don't have to worry about that. What we do have to\nworry is about is that the guest can't interfere with the host in\nuninteded ways by playing with the interrupts. Let's consider the two\nscenarios:\n\nPhys = level, Virt = Edge\n-------------------------\nThese patches don't change anything from today's behavior. We may loose\ninterrupts for the VM, tough.\n\nPhys = edge, Virt = Level\n-------------------------\nIn this case we will read the physical input level on return from the\nVM. That changes the virtual line_level, no harm done. If we see the\ninput line to the VGIC (or physial pending state) is asserted, we will\ndeactivate a physical interrup which means, if it's actually pending,\nthe driver (VFIO or arch timer, typically) will take another\ninterrupt, no harm done.\n\nSo I think we're fine.\n\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=\"e7EYrm6F\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"D6zOxzFL\"; 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 3xl6yj0z1Qz9sRW\n\tfor ;\n\tSun, 3 Sep 2017 06:24:13 +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 1doExd-0002In-Fb; Sat, 02 Sep 2017 20:24:09 +0000","from mail-wm0-x22b.google.com ([2a00:1450:400c:c09::22b])\n\tby bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1doExZ-0001tG-BS for linux-arm-kernel@lists.infradead.org;\n\tSat, 02 Sep 2017 20:24:07 +0000","by mail-wm0-x22b.google.com with SMTP id i145so2515538wmf.1\n\tfor ;\n\tSat, 02 Sep 2017 13:23:44 -0700 (PDT)","from localhost (xd93ddc2d.cust.hiper.dk. [217.61.220.45])\n\tby smtp.gmail.com with ESMTPSA id\n\te12sm1479452edj.72.2017.09.02.13.23.41\n\t(version=TLS1_2 cipher=AES128-SHA bits=128/128);\n\tSat, 02 Sep 2017 13:23:41 -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=c6LiAGnkJIovMHT0Nix9In5XIq/3rvUkVElrJHcSsl8=;\n\tb=e7EYrm6FxY41DU\n\tfHdcH2RWU/NccQR1V0uBW/VpQNssRbSpMfQ2TsuFbUGphlexfFJ7t4+LkPZoaBXrGo0q/J3ATw35j\n\tS2fwKUPjbjLeXRiVeSnnCYLphuopKm4rRaZ9EakEIAgYzp+t9aA+z7lmQoKdxQa3V+UtMYZx85DaI\n\tbMgRiiBpWrLm+CjFyxwgOgLbFEAXnCtU/wDXvSvZBQ5NtakLbfrbJuFlTjByPzIDjmSpbOEuDLfXs\n\ty75QxPtdvz5QXjRjmw8TOhBM3GMngpbuJecfw25e13Fyd70bP4UIhWDEOuqmIHC7L/EDfHTCjdbGU\n\tUS+UAZWfCoyovmo4ecmg==;","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=ZU/Dd765t6OO1H+YBFuuj8+ooQwFZDMYJFVPqrBJG88=;\n\tb=D6zOxzFLXGKuWzuGgKm4+6Vo2AwoxDSt3BF43kBMLaCNkOdeSIoRKLugHabCCMJCMT\n\tN6X210gxzggGoyisJN+5hE0oWtzwtrFywqayHcQzGkIwYVxDcjw8C1B8yItUb1HDdk2l\n\tE/VIU/pDgH/z4JdukAMUlVxnTPbVlR8ykAV9c="],"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=ZU/Dd765t6OO1H+YBFuuj8+ooQwFZDMYJFVPqrBJG88=;\n\tb=sqqUQuMWEII27V49EV4jbUNm6SnN2uvLki4REqFUOh/aYnY5krXiHFaEgieNuow4XD\n\t7RN5AZZ0x5BEJLdWKnUpcs5pcUFPD3jjaKzkgXUhiv1W2oE1OtsiDaHaU99FCqPjugTU\n\tYR9WZPPCEtBzKB/QtWEahZYlmxpkeErHjxBD79igk6/z9QSrfFdO5fumo2Cm2kO3n+81\n\tnwkvjV3nParnVNVn20Te1UK0cZ2Ar0YSsvWSkNYhQjIkehwN9T+0Zv9QPbmFCsiGa2co\n\tLnyasLwoAXXNOp5Jtau+6vyGnHNf67Q52qyIIYpw2o6e43wxJs3ssOQQ/KmvpqF7Efit\n\tLozA==","X-Gm-Message-State":"AHPjjUgAqkEBfiuYgDWcuJM2Huv+Zrfb9ST6TK2vZ3zsWVJC89MiODtW\n\tUStPLJcaeOoriW6x","X-Google-Smtp-Source":"ADKCNb5P83GWKXiivj+sNSrCtby0W0hNa3CzenBOHJIMiS+C47GRPZhyapPbioHGFZ6nL31AczbReQ==","X-Received":"by 10.80.169.193 with SMTP id n59mr4897000edc.244.1504383822849; \n\tSat, 02 Sep 2017 13:23:42 -0700 (PDT)","Date":"Sat, 2 Sep 2017 22:23:39 +0200","From":"Christoffer Dall ","To":"Marc Zyngier ","Subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","Message-ID":"<20170902202339.GG13572@cbox>","References":"<20170829093902.15379-1-cdall@linaro.org>\n\t<20170829093902.15379-3-cdall@linaro.org> <867exhuztl.fsf@arm.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<867exhuztl.fsf@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-20170902_132405_702281_E0916BB6 ","X-CRM114-Status":"GOOD ( 36.96 )","X-Spam-Score":"-2.7 (--)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-2.7 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/,\n\tlow\n\ttrust [2a00:1450:400c:c09:0:0:0:22b 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":"Eric Auger , kvm@vger.kernel.org,\n\tkvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org,\n\tAndre Przywara ","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":1762184,"web_url":"http://patchwork.ozlabs.org/comment/1762184/","msgid":"<20170902203753.GH13572@cbox>","list_archive_url":null,"date":"2017-09-02T20:37:53","subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","submitter":{"id":71350,"url":"http://patchwork.ozlabs.org/api/people/71350/","name":"Christoffer Dall","email":"cdall@linaro.org"},"content":"On Sat, Sep 02, 2017 at 11:52:57AM +0100, Marc Zyngier wrote:\n> On Tue, Aug 29 2017 at 11:39:00 am BST, Christoffer Dall wrote:\n> > Level-triggered mapped IRQs are special because we only observe rising\n> > edges as input to the VGIC, and we don't set the EOI flag and therefore\n> > are not told when the level goes down, so that we can re-queue a new\n> > interrupt when the level goes up.\n> >\n> > One way to solve this problem is to side-step the logic of the VGIC and\n> > special case the validation in the injection path, but it has the\n> > unfortunate drawback of having to peak into the physical GIC state\n> > whenever we want to know if the interrupt is pending on the virtual\n> > distributor.\n> >\n> > Instead, we can maintain the current semantics of a level triggered\n> > interrupt by sort of treating it as an edge-triggered interrupt,\n> > following from the fact that we only observe an asserting edge. This\n> \n> Would it be worth mentioning that this is a consequence of offloading\n> part of the flow handling to the HW?\n> \n\nPossibly, but I'm not really sure what that actually means. Can you\nexplain a little more, and I'll be happy to add the text?\n\n> > requires us to be a bit careful when populating the LRs and when folding\n> > the state back in though:\n> >\n> > * We lower the line level when populating the LR, so that when\n> > subsequently observing an asserting edge, the VGIC will do the right\n> > thing.\n> >\n> > * If the guest never acked the interrupt while running (for example if\n> > it had masked interrupts at the CPU level while running), we have\n> > to preserve the pending state of the LR and move it back to the\n> > line_level field of the struct irq when folding LR state.\n> >\n> > If the guest never acked the interrupt while running, but changed the\n> > device state and lowered the line (again with interrupts masked) then\n> > we need to observe this change in the line_level.\n> >\n> > Both of the above situations are solved by sampling the physical line\n> > and set the line level when folding the LR back.\n> >\n> > * Finally, if the guest never acked the interrupt while running and\n> > sampling the line reveals that the device state has changed and the\n> > line has been lowered, we must clear the physical active state, since\n> > we will otherwise never be told when the interrupt becomes asserted\n> > again.\n> >\n> > This has the added benefit of making the timer optimization patches\n> > (https://lists.cs.columbia.edu/pipermail/kvmarm/2017-July/026343.html) a\n> > bit simpler, because the timer code doesn't have to clear the active\n> > state on the sync anymore. It also potentially improves the performance\n> > of the timer implementation because the GIC knows the state or the LR\n> > and only needs to clear the\n> > active state when the pending bit in the LR is still set, where the\n> > timer has to always clear it when returning from running the guest with\n> > an injected timer interrupt.\n> >\n> > Signed-off-by: Christoffer Dall \n> > ---\n> > virt/kvm/arm/vgic/vgic-v2.c | 29 +++++++++++++++++++++++++++++\n> > virt/kvm/arm/vgic/vgic-v3.c | 29 +++++++++++++++++++++++++++++\n> > virt/kvm/arm/vgic/vgic.c | 23 +++++++++++++++++++++++\n> > virt/kvm/arm/vgic/vgic.h | 7 +++++++\n> > 4 files changed, 88 insertions(+)\n> >\n> > diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c\n> > index e4187e5..f7c5cb5 100644\n> > --- a/virt/kvm/arm/vgic/vgic-v2.c\n> > +++ b/virt/kvm/arm/vgic/vgic-v2.c\n> > @@ -104,6 +104,26 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)\n> > \t\t\t\tirq->pending_latch = false;\n> > \t\t}\n> > \n> > +\t\t/*\n> > +\t\t * Level-triggered mapped IRQs are special because we only\n> > +\t\t * observe rising edges as input to the VGIC.\n> > +\t\t *\n> > +\t\t * If the guest never acked the interrupt we have to sample\n> > +\t\t * the physical line and set the line level, because the\n> > +\t\t * device state could have changed or we simply need to\n> > +\t\t * process the still pending interrupt later.\n> > +\t\t *\n> > +\t\t * If this causes us to lower the level, we have to also clear\n> > +\t\t * the physical active state, since we will otherwise never be\n> > +\t\t * told when the interrupt becomes asserted again.\n> > +\t\t */\n> > +\t\tif (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT)) {\n> > +\t\t\tirq->line_level = vgic_irq_line_level(irq);\n> > +\n> > +\t\t\tif (!irq->line_level)\n> > +\t\t\t\tvgic_irq_clear_phys_active(irq);\n> > +\t\t}\n> > +\n> > \t\tspin_unlock(&irq->irq_lock);\n> > \t\tvgic_put_irq(vcpu->kvm, irq);\n> > \t}\n> > @@ -161,6 +181,15 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)\n> > \t\t\tval |= GICH_LR_EOI;\n> > \t}\n> > \n> > +\t/*\n> > +\t * Level-triggered mapped IRQs are special because we only observe\n> > +\t * rising edges as input to the VGIC. We therefore lower the line\n> > +\t * level here, so that we can take new virtual IRQs. See\n> > +\t * vgic_v2_fold_lr_state for more info.\n> > +\t */\n> > +\tif (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT))\n> > +\t\tirq->line_level = false;\n> > +\n> > \t/* The GICv2 LR only holds five bits of priority. */\n> > \tval |= (irq->priority >> 3) << GICH_LR_PRIORITY_SHIFT;\n> > \n> > diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c\n> > index 96ea597..e377036 100644\n> > --- a/virt/kvm/arm/vgic/vgic-v3.c\n> > +++ b/virt/kvm/arm/vgic/vgic-v3.c\n> > @@ -94,6 +94,26 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)\n> > \t\t\t\tirq->pending_latch = false;\n> > \t\t}\n> > \n> > +\t\t/*\n> > +\t\t * Level-triggered mapped IRQs are special because we only\n> > +\t\t * observe rising edges as input to the VGIC.\n> > +\t\t *\n> > +\t\t * If the guest never acked the interrupt we have to sample\n> > +\t\t * the physical line and set the line level, because the\n> > +\t\t * device state could have changed or we simply need to\n> > +\t\t * process the still pending interrupt later.\n> > +\t\t *\n> > +\t\t * If this causes us to lower the level, we have to also clear\n> > +\t\t * the physical active state, since we will otherwise never be\n> > +\t\t * told when the interrupt becomes asserted again.\n> > +\t\t */\n> > +\t\tif (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) {\n> > +\t\t\tirq->line_level = vgic_irq_line_level(irq);\n> > +\n> > +\t\t\tif (!irq->line_level)\n> > +\t\t\t\tvgic_irq_clear_phys_active(irq);\n> > +\t\t}\n> > +\n> > \t\tspin_unlock(&irq->irq_lock);\n> > \t\tvgic_put_irq(vcpu->kvm, irq);\n> > \t}\n> > @@ -144,6 +164,15 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)\n> > \t}\n> > \n> > \t/*\n> > +\t * Level-triggered mapped IRQs are special because we only observe\n> > +\t * rising edges as input to the VGIC. We therefore lower the line\n> > +\t * level here, so that we can take new virtual IRQs. See\n> > +\t * vgic_v3_fold_lr_state for more info.\n> > +\t */\n> > +\tif (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT))\n> > +\t\tirq->line_level = false;\n> > +\n> > +\t/*\n> > \t * We currently only support Group1 interrupts, which is a\n> > \t * known defect. This needs to be addressed at some point.\n> > \t */\n> > diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c\n> > index 9d557efd..2691a0a 100644\n> > --- a/virt/kvm/arm/vgic/vgic.c\n> > +++ b/virt/kvm/arm/vgic/vgic.c\n> > @@ -140,6 +140,29 @@ void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq)\n> > \tkfree(irq);\n> > }\n> > \n> > +/* Get the input level of a mapped IRQ directly from the physical GIC */\n> > +bool vgic_irq_line_level(struct vgic_irq *irq)\n> \n> nit: it would make the above clearer if this was named something like\n> vgic_irq_get_phys_line_level(), as the current name is pretty ambiguous\n> about which side of the GIC we're looking at.\n> \nYes, absolutely.\n\n> > +{\n> > +\tbool line_level;\n> > +\n> > +\tBUG_ON(!irq->hw);\n> > +\n> > +\tWARN_ON(irq_get_irqchip_state(irq->host_irq,\n> > +\t\t\t\t IRQCHIP_STATE_PENDING,\n> > +\t\t\t\t &line_level));\n> > +\treturn line_level;\n> > +}\n> > +\n> > +/* Clear the physical active state */\n> > +void vgic_irq_clear_phys_active(struct vgic_irq *irq)\n> > +{\n> > +\n> > +\tBUG_ON(!irq->hw);\n> > +\tWARN_ON(irq_set_irqchip_state(irq->host_irq,\n> > +\t\t\t\t IRQCHIP_STATE_ACTIVE,\n> > +\t\t\t\t false));\n> > +}\n> > +\n> > /**\n> > * kvm_vgic_target_oracle - compute the target vcpu for an irq\n> > *\n> > diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h\n> > index bba7fa2..22f106d 100644\n> > --- a/virt/kvm/arm/vgic/vgic.h\n> > +++ b/virt/kvm/arm/vgic/vgic.h\n> > @@ -104,6 +104,11 @@ static inline bool irq_is_pending(struct vgic_irq *irq)\n> > \t\treturn irq->pending_latch || irq->line_level;\n> > }\n> > \n> > +static inline bool vgic_irq_is_mapped_level(struct vgic_irq *irq)\n> > +{\n> > +\treturn irq->config == VGIC_CONFIG_LEVEL && irq->hw;\n> > +}\n> > +\n> > /*\n> > * This struct provides an intermediate representation of the fields contained\n> > * in the GICH_VMCR and ICH_VMCR registers, such that code exporting the GIC\n> > @@ -140,6 +145,8 @@ vgic_get_mmio_region(struct kvm_vcpu *vcpu, struct vgic_io_device *iodev,\n> > 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_irq_line_level(struct vgic_irq *irq);\n> > +void vgic_irq_clear_phys_active(struct vgic_irq *irq);\n> > bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq);\n> > void vgic_kick_vcpus(struct kvm *kvm);\n> \n> Reviewed-by: Marc Zyngier \n> \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=\"Kc1ihZ0u\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"gY/j4NuF\"; 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 3xl7HB5KrTz9sRW\n\tfor ;\n\tSun, 3 Sep 2017 06:38:30 +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 1doFBT-0000I6-I4; Sat, 02 Sep 2017 20:38:27 +0000","from mail-wm0-x22f.google.com ([2a00:1450:400c:c09::22f])\n\tby bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1doFBN-0000G5-3L for linux-arm-kernel@lists.infradead.org;\n\tSat, 02 Sep 2017 20:38:25 +0000","by mail-wm0-x22f.google.com with SMTP id 187so17741843wmn.1\n\tfor ;\n\tSat, 02 Sep 2017 13:37:58 -0700 (PDT)","from localhost (xd93ddc2d.cust.hiper.dk. [217.61.220.45])\n\tby smtp.gmail.com with ESMTPSA id\n\tq16sm948288eda.87.2017.09.02.13.37.55\n\t(version=TLS1_2 cipher=AES128-SHA bits=128/128);\n\tSat, 02 Sep 2017 13:37: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: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=UHln/0KJcolDirwLpZ+c5kszxDV5fZbtawsRLfWxpq0=;\n\tb=Kc1ihZ0uA64FiG\n\tlMpWjm+6QYNQb0jIom5YKTxWvjKyU6uTSYmnWvMThLFL50DvOv+hSjCuCw3sQLF6fnqi6Vw/NGLK+\n\tRzx8Dfvs+S0N8oaynx9R8MQgvXDLY/z/7kZcR0Xg2LEfVbemw3+W288TQYkXJv6ngsiPFnIE5K2oS\n\tmyuJPDsb7yN/MqwO474VeMORikmIej/ohECa9674eG8kJprE0sWamZAEcYmEUOI3d9wITI6lAJK5O\n\tzqwo4YLFsEk6DeK6CLXHK6wxyyjEY6mLFe03vhR7dG4V7wuIfYy1xoTyMCZSohxoB8ZYK3BtND2Ng\n\tFJ+DiR2rpqdrHk5w4W1A==;","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=UHOeD+Jqyn/ZrhiV0v54YWlWPhd0H/Tj8WH7p15udI4=;\n\tb=gY/j4NuFLmxs7mPkxtqWqVLH4Or2SDU6NoC4CHyUnFHm67ftyL0LlCLKfCvMcWgM7H\n\tuTp9cOKOeUdbvyAJMrEVrZ71xj+sSsT9bBJ9XLttKMazZvrwJfizNmv3Pg0aQ6JP8jK2\n\t8eu8rHEKs/GWV4EeD7qdbtYsi30kKxuXqYjIc="],"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=UHOeD+Jqyn/ZrhiV0v54YWlWPhd0H/Tj8WH7p15udI4=;\n\tb=tCTvomlO8r5Jh4KGiC/3/02V0RIxYIMCR3x1Zet8Rl86bnEv2+3isGNoicB5LtxJq2\n\tMaqIpwGKn3zafRxC5I8n2z1NGgFHLg7WCOsal9jFfl3RKeVpJlAwHcGl/NljP6zi09be\n\tD4PZ7OqDV0lE3JQQKl7kUNQWmnymjeBIKdOP3Vt9oDhfUxNQZ6sr7/oRLseWFe8tSjNN\n\t1MQAADnKO9YlkxY+9ywN63mXSUjQPZ4TnaeI/L82HQ0q36oLuOtJeeUPuegPxHsbt7gL\n\th+Qmkm9T0V+go2SP+aTLQ5x3AK6sHeGEmouy5EaAN7ihoU8SJKsak3Q8QJekBJ7mf3HD\n\tWRbg==","X-Gm-Message-State":"AHPjjUitZ7EVhwLnlfLJiFA0yl9FDrMn1mhwTMxx72iqxocxBQkFPmbX\n\tLOk2VR/PoKglTKCA","X-Google-Smtp-Source":"ADKCNb6VxIT5AbyBpOq+dTrwXNepJthcp17JRSjqwbfZ33T71zgwpdJ3KzjisoNU/WTI19Zvzc/Rtw==","X-Received":"by 10.80.161.69 with SMTP id 63mr4935967edj.181.1504384676454;\n\tSat, 02 Sep 2017 13:37:56 -0700 (PDT)","Date":"Sat, 2 Sep 2017 22:37:53 +0200","From":"Christoffer Dall ","To":"Marc Zyngier ","Subject":"Re: [RFC PATCH 2/4] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","Message-ID":"<20170902203753.GH13572@cbox>","References":"<20170829093902.15379-1-cdall@linaro.org>\n\t<20170829093902.15379-3-cdall@linaro.org> <86efrpv0c6.fsf@arm.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<86efrpv0c6.fsf@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-20170902_133822_836605_BEB15E91 ","X-CRM114-Status":"GOOD ( 37.77 )","X-Spam-Score":"-2.7 (--)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-2.7 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/,\n\tlow\n\ttrust [2a00:1450:400c:c09:0:0:0:22f 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":"Eric Auger , kvm@vger.kernel.org,\n\tkvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org,\n\tAndre Przywara ","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":1762185,"web_url":"http://patchwork.ozlabs.org/comment/1762185/","msgid":"<20170902204152.GI13572@cbox>","list_archive_url":null,"date":"2017-09-02T20:41:52","subject":"Re: [RFC PATCH 4/4] KVM: arm/arm64: Provide a vgic interrupt line\n\tlevel sample function","submitter":{"id":71350,"url":"http://patchwork.ozlabs.org/api/people/71350/","name":"Christoffer Dall","email":"cdall@linaro.org"},"content":"On Sat, Sep 02, 2017 at 12:20:34PM +0100, Marc Zyngier wrote:\n> On Tue, Aug 29 2017 at 11:39:02 am BST, Christoffer Dall wrote:\n> > The GIC sometimes need to sample the physical line of a mapped\n> > interrupt. As we know this to be notoriously slow, provide a callback\n> > function for devices (such as the timer) which can do this much faster\n> > than talking to the distributor, for example by comparing a few\n> > in-memory values. Fall back to the good old method of poking the\n> > physical GIC if no callback is provided.\n> >\n> > Signed-off-by: Christoffer Dall \n> > ---\n> > include/kvm/arm_vgic.h | 13 ++++++++++++-\n> > virt/kvm/arm/arch_timer.c | 16 +++++++++++++++-\n> > virt/kvm/arm/vgic/vgic.c | 7 ++++++-\n> > 3 files changed, 33 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h\n> > index 53f631b..a52990b 100644\n> > --- a/include/kvm/arm_vgic.h\n> > +++ b/include/kvm/arm_vgic.h\n> > @@ -125,6 +125,17 @@ struct vgic_irq {\n> > \tu8 priority;\n> > \tenum vgic_irq_config config;\t/* Level or edge */\n> > \n> > +\t/*\n> > +\t * Callback function pointer to in-kernel devices that can tell us the\n> > +\t * state of the input level of mapped level-triggered IRQ faster than\n> > +\t * peaking into the physical GIC.\n> > +\t *\n> > +\t * Always called in non-preemptible section and the functions can use\n> > +\t * kvm_arm_get_running_vcpu() to get the vcpu pointer for private\n> > +\t * IRQs.\n> > +\t */\n> > +\tbool (*get_input_level)(int vintid);\n> > +\n> > \tvoid *owner;\t\t\t/* Opaque pointer to reserve an interrupt\n> > \t\t\t\t\t for in-kernel devices. */\n> > };\n> > @@ -309,7 +320,7 @@ void kvm_vgic_init_cpu_hardware(void);\n> > int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,\n> > \t\t\tbool level, void *owner);\n> > int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,\n> > -\t\t\t u32 vintid);\n> > +\t\t\t u32 vintid, bool (*callback)(int vindid));\n> > int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, unsigned int vintid);\n> > bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, unsigned int vintid);\n> > \n> > diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c\n> > index b24e2f7..82169ef 100644\n> > --- a/virt/kvm/arm/arch_timer.c\n> > +++ b/virt/kvm/arm/arch_timer.c\n> > @@ -643,6 +643,19 @@ static bool timer_irqs_are_valid(struct kvm_vcpu *vcpu)\n> > \treturn true;\n> > }\n> > \n> > +static bool timer_get_input_level(int vintid)\n> > +{\n> > +\tstruct kvm_vcpu *vcpu = kvm_arm_get_running_vcpu();\n> > +\tstruct arch_timer_context *timer;\n> > +\n> > +\tif (vintid == vcpu_vtimer(vcpu)->irq.irq)\n> > +\t\ttimer = vcpu_vtimer(vcpu);\n> > +\telse\n> > +\t\tBUG(); /* We only map the vtimer so far */\n> > +\n> > +\treturn kvm_timer_should_fire(timer);\n> > +}\n> > +\n> > int kvm_timer_enable(struct kvm_vcpu *vcpu)\n> > {\n> > \tstruct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;\n> > @@ -664,7 +677,8 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)\n> > \t\treturn -EINVAL;\n> > \t}\n> > \n> > -\tret = kvm_vgic_map_phys_irq(vcpu, host_vtimer_irq, vtimer->irq.irq);\n> > +\tret = kvm_vgic_map_phys_irq(vcpu, host_vtimer_irq, vtimer->irq.irq,\n> > +\t\t\t\t &timer_get_input_level);\n> \n> nit: no need for a & here.\n> \n> > \tif (ret)\n> > \t\treturn ret;\n> > \n> > diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c\n> > index e3ce2fa..0466c10 100644\n> > --- a/virt/kvm/arm/vgic/vgic.c\n> > +++ b/virt/kvm/arm/vgic/vgic.c\n> > @@ -147,6 +147,9 @@ bool vgic_irq_line_level(struct vgic_irq *irq)\n> > \n> > \tBUG_ON(!irq->hw);\n> > \n> > +\tif (irq->get_input_level)\n> > +\t\treturn irq->get_input_level(irq->intid);\n> > +\n> > \tWARN_ON(irq_get_irqchip_state(irq->host_irq,\n> > \t\t\t\t IRQCHIP_STATE_PENDING,\n> > \t\t\t\t &line_level));\n> > @@ -429,7 +432,7 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,\n> > }\n> > \n> > int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,\n> > -\t\t\t u32 vintid)\n> > +\t\t\t u32 vintid, bool (*callback)(int vindid))\n> \n> nit #2: \"callback\" is a very non-descriptive name for a callback... ;-)\n> How about calling it \"get_input_level\", matching the vgic_irq field?\n\nYeah, totally.\n\n> \n> > {\n> > \tstruct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, vintid);\n> > \tstruct irq_desc *desc;\n> > @@ -456,6 +459,7 @@ int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,\n> > \tirq->hw = true;\n> > \tirq->host_irq = host_irq;\n> > \tirq->hwintid = data->hwirq;\n> > +\tirq->get_input_level = callback;\n> > \n> > out:\n> > \tspin_unlock(&irq->irq_lock);\n> > @@ -476,6 +480,7 @@ int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, unsigned int vintid)\n> > \tspin_lock(&irq->irq_lock);\n> > \tirq->hw = false;\n> > \tirq->hwintid = 0;\n> > +\tirq->get_input_level = NULL;\n> > \tspin_unlock(&irq->irq_lock);\n> > \tvgic_put_irq(vcpu->kvm, irq);\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=\"lOsG4Xsg\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"cIk6tpvb\"; 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 3xl7Mj1qGmz9sRW\n\tfor ;\n\tSun, 3 Sep 2017 06:42:25 +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 1doFFF-0003N4-CW; Sat, 02 Sep 2017 20:42:21 +0000","from mail-wm0-x231.google.com ([2a00:1450:400c:c09::231])\n\tby bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1doFFC-00034D-BV for linux-arm-kernel@lists.infradead.org;\n\tSat, 02 Sep 2017 20:42:20 +0000","by mail-wm0-x231.google.com with SMTP id v2so16877229wmf.0\n\tfor ;\n\tSat, 02 Sep 2017 13:41:56 -0700 (PDT)","from localhost (xd93ddc2d.cust.hiper.dk. [217.61.220.45])\n\tby smtp.gmail.com with ESMTPSA id\n\th13sm1686664edb.64.2017.09.02.13.41.54\n\t(version=TLS1_2 cipher=AES128-SHA bits=128/128);\n\tSat, 02 Sep 2017 13:41:54 -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=BjBgbral1SYOuze3N0QFXDG0qPwq5ZCu5uRZ3orP4bs=;\n\tb=lOsG4XsgF94qPY\n\thRkxV0dKpbJjB36av/CxOaAu4tnVEMERu+JnaU2yfwqylmkUwLbBWQchnLDPrgzATItl5Rb7S0h7t\n\twMnqXnBqaKlcwPgkPoh911P8JwVWETenAEyMIQPqFPTavcnxn1qCClHOoI9lErVeV6rhY/N2L9tv7\n\tg76Yw97cmPpM6oOnR/DHQMa1iRTC2Mpxyo9QhTMLhR4PKlxZoWCsefFkNGIMZMLMEAuREbnJQ4GDQ\n\t6sfNXLc2wYyaoHJ6ZYu/QA4i/f17O9eQffw680w77TlVDzc6JW3Jl/Robv6iyf7P3xXIKs0xWMA8b\n\twbDmvob14/Bx+A4+mE7g==;","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=aUHWVmNkDCOqgXKTWabB1khKSef0Z7CToMh+rpL7ovE=;\n\tb=cIk6tpvb/ewUHgYFbJILS8aOC3P7B0fRXSf7cQ+hReWPYZcYM5tK9yUWdR7gVYDvME\n\t2YM5AYIKKp4B6F3+BGPrYLeLewIxiy0jsLfbF+0tU1HyvkyOJj4boMKdOIfB9swkICYz\n\tdIPDneBrv9NkpEXW8s9/8ZThGUfz2UFte7Xmw="],"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=aUHWVmNkDCOqgXKTWabB1khKSef0Z7CToMh+rpL7ovE=;\n\tb=paizpc8ZCkuShh2hilQw4TzuHklKyndQF1IL1gyeu30TRKzB/nzjXvAWgyjiTUsHI2\n\tNNyENob6jzkVOEu4WJqnO9zWJKKm49kBBG8m88Wl9PbgxsvKNz6WWpOL20/dyDu9gFoq\n\tY/PINuhgm4HAuYU8IXSrZgwJOkSMcTpqBH4EVu5fCdQd2bNnSNOLL06vj0BDG0CNf2M+\n\trJvMc1EBfnkw5JCHroqIu2S0zKqLCkGlycJo7K4+SsOJrB6ZkC9AhlJTy0bwPi1FA9V2\n\ttIMQXVKr9HNyWR7z0Ngnufc39oW5fATTMO3L11RPVgGXH73Q471RhSekBRyrAJ/pbgfF\n\txMyQ==","X-Gm-Message-State":"AHPjjUh7vVquHztHtcDUwrh2S4InnlbDEXXg5ZkrELylRNOnpV4F8J5s\n\tbvTmcbHoISXsXovfcI1YSQ==","X-Google-Smtp-Source":"ADKCNb72d109dUMuSg+EC6GTWxArQVykbTw1GBL4QfA3SYgO/uBdG5/ppxXuQlaVn0SHy5un2Azbxw==","X-Received":"by 10.80.183.170 with SMTP id h39mr5107819ede.149.1504384915259; \n\tSat, 02 Sep 2017 13:41:55 -0700 (PDT)","Date":"Sat, 2 Sep 2017 22:41:52 +0200","From":"Christoffer Dall ","To":"Marc Zyngier ","Subject":"Re: [RFC PATCH 4/4] KVM: arm/arm64: Provide a vgic interrupt line\n\tlevel sample function","Message-ID":"<20170902204152.GI13572@cbox>","References":"<20170829093902.15379-1-cdall@linaro.org>\n\t<20170829093902.15379-5-cdall@linaro.org> <86tw0ltkhp.fsf@arm.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<86tw0ltkhp.fsf@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-20170902_134218_557408_5CC85383 ","X-CRM114-Status":"GOOD ( 23.69 )","X-Spam-Score":"-2.7 (--)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-2.7 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/,\n\tlow\n\ttrust [2a00:1450:400c:c09:0:0:0:231 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":"Eric Auger , kvm@vger.kernel.org,\n\tkvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org,\n\tAndre Przywara ","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"}}]