[{"id":1763138,"web_url":"http://patchwork.ozlabs.org/comment/1763138/","msgid":"<5971dacd-ba04-1e68-f4c8-359500e79455@redhat.com>","list_archive_url":null,"date":"2017-09-05T09:38:48","subject":"Re: [PATCH v2 3/6] 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 04/09/2017 12:24, 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 <cdall@linaro.org>\n> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>\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..618ed3f 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_get_phys_line_level(irq);\n> +\n> +\t\t\tif (!irq->line_level)\n> +\t\t\t\tvgic_irq_set_phys_active(irq, false);\nI am still unclear wrt ISPENDR case. Assume the guest writes into the\nISPENDR. So now we set the pending state on the phys distributor. The\nSPI is acked by the host. I understand it becomes active (ie. not\npending and active). Then it gets injected into the guest and in the\nabove fold code the LR state is observed GICH_LR_PENDING_BIT. We are\ngoing to read the physical pending state which is: not pending. So the\nline level is low, the latch_pending is not used in mapped case and is\nlow. So to me the vIRQ is missed.\n\nWhat do I miss?\n\nThanks\n\nEric\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..0f72bcb 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_get_phys_line_level(irq);\n> +\n> +\t\t\tif (!irq->line_level)\n> +\t\t\t\tvgic_irq_set_phys_active(irq, false);\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..8072969 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_get_phys_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> +/* Set/Clear the physical active state */\n> +void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active)\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      active));\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..7bdcda2 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_get_phys_line_level(struct vgic_irq *irq);\n> +void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active);\n>  bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq);\n>  void vgic_kick_vcpus(struct kvm *kvm);\n>  \n>","headers":{"Return-Path":"<linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org>","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=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"rsB9UKTl\"; 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 3xmhWG4Tfcz9s75\n\tfor <incoming-imx@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 19:39: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 1dpAKF-0005ge-CH; Tue, 05 Sep 2017 09:39:19 +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 1dpAKA-0005Yt-33 for linux-arm-kernel@lists.infradead.org;\n\tTue, 05 Sep 2017 09:39:16 +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 342AE80491;\n\tTue,  5 Sep 2017 09:38:52 +0000 (UTC)","from localhost.localdomain (ovpn-117-1.ams2.redhat.com\n\t[10.36.117.1])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 63132709E6;\n\tTue,  5 Sep 2017 09:38:50 +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=d6xcMN9BVrB6e7K5ct1jpFTo8Z5dAZuIHHyput7H+8A=;\n\tb=rsB9UKTlTMwVb9\n\tnBLaeo7m650F6JqTuEUN/hdhYXAuRRTn5LGJ0adyGWxGw+doT9gBK/wMvEZETCLhcHoTqtnrSIkWx\n\tWGRZkx0V11p74VwqTXaGdAnVNrP1RFkwoMVx4Q5z42lEHPHZvOHHnhwcqfuy1pDJ/fUPQiEfRYmjL\n\twwI4+u8qtTVYpQYeA0kmPh+6r8qjom6zn97ASL4bIUBAfJhiOpuhe81S1nb8IZoW0oCsLJ9Ezny3m\n\t1ycSjY7bM2JnnQjDrccdWUrSZfoiUi0JllxeMJ6BnYRoqcaKxXbpoPDBwqaBvtibQ7PEYd6zZTD4C\n\tTDdxT/NfMRamcrYHa/Aw==;","DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 342AE80491","Subject":"Re: [PATCH v2 3/6] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","To":"Christoffer Dall <cdall@linaro.org>, kvmarm@lists.cs.columbia.edu,\n\tMarc Zyngier <marc.zyngier@arm.com>","References":"<20170904102456.9025-1-cdall@linaro.org>\n\t<20170904102456.9025-4-cdall@linaro.org>","From":"Auger Eric <eric.auger@redhat.com>","Message-ID":"<5971dacd-ba04-1e68-f4c8-359500e79455@redhat.com>","Date":"Tue, 5 Sep 2017 11:38:48 +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":"<20170904102456.9025-4-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]); Tue, 05 Sep 2017 09:38:52 +0000 (UTC)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170905_023914_186703_8A58219D ","X-CRM114-Status":"GOOD (  35.57  )","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-0.0 RCVD_IN_MSPIKE_H3      RBL: Good reputation (+3)\n\t[209.132.183.28 listed in wl.mailspike.net]\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 SPF_PASS               SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-0.0 SPF_HELO_PASS          SPF: HELO matches SPF record\n\t-1.9 BAYES_00               BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]\n\t-0.0 RCVD_IN_MSPIKE_WL      Mailspike good senders","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":"<http://lists.infradead.org/mailman/options/linux-arm-kernel>,\n\t<mailto:linux-arm-kernel-request@lists.infradead.org?subject=unsubscribe>","List-Archive":"<http://lists.infradead.org/pipermail/linux-arm-kernel/>","List-Post":"<mailto:linux-arm-kernel@lists.infradead.org>","List-Help":"<mailto:linux-arm-kernel-request@lists.infradead.org?subject=help>","List-Subscribe":"<http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>,\n\t<mailto:linux-arm-kernel-request@lists.infradead.org?subject=subscribe>","Cc":"Andre Przywara <andre.przywara@arm.com>,\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\" <linux-arm-kernel-bounces@lists.infradead.org>","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1763182,"web_url":"http://patchwork.ozlabs.org/comment/1763182/","msgid":"<56431c9d-3f7e-f8b2-2d31-6baec4e294da@redhat.com>","list_archive_url":null,"date":"2017-09-05T10:26:14","subject":"Re: [PATCH v2 5/6] KVM: arm/arm64: Rearrange kvm_vgic_[un]map_phys\n\tcode in vgic.c","submitter":{"id":69187,"url":"http://patchwork.ozlabs.org/api/people/69187/","name":"Eric Auger","email":"eric.auger@redhat.com"},"content":"Hi Christoffer,\nOn 04/09/2017 12:24, 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 <cdall@linaro.org>\n> Acked-by: Marc Zyngier <marc.zyngier@arm.com>\n> ---\n>  virt/kvm/arm/vgic/vgic.c | 38 +++++++++++++-------------------------\n>  1 file changed, 13 insertions(+), 25 deletions(-)\n> \n> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c\n> index 7aec730..b704ff5 100644\n> --- a/virt/kvm/arm/vgic/vgic.c\n> +++ b/virt/kvm/arm/vgic/vgic.c\n> @@ -435,12 +435,17 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,\n>  \treturn 0;\n>  }\n>  \n> -/* @irq->irq_lock must be held */\nI chose to hold the lock outside of kvm_vgic_map/unmap_irq because in\nkvm_vgic_set_forwarding(see https://lkml.org/lkml/2017/6/15/278) I was\nalso testing hw and target_vcpu fields. As you pointed out maybe I am\nnot obliged to check them but that was the rationale.\n\nThanks\n\nEric\n> -static int kvm_vgic_map_irq(struct kvm_vcpu *vcpu, struct vgic_irq *irq,\n> -\t\t\t    unsigned int host_irq)\n> +int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,\n> +\t\t\t  u32 vintid)\n>  {\n> +\tstruct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, vintid);\n>  \tstruct irq_desc *desc;\n>  \tstruct irq_data *data;\n> +\tint ret = 0;\n> +\n> +\tBUG_ON(!irq);\n> +\n> +\tspin_lock(&irq->irq_lock);\n>  \n>  \t/*\n>  \t * Find the physical IRQ number corresponding to @host_irq\n> @@ -448,7 +453,8 @@ static int kvm_vgic_map_irq(struct kvm_vcpu *vcpu, struct vgic_irq *irq,\n>  \tdesc = irq_to_desc(host_irq);\n>  \tif (!desc) {\n>  \t\tkvm_err(\"%s: no interrupt descriptor\\n\", __func__);\n> -\t\treturn -EINVAL;\n> +\t\tret = -EINVAL;\n> +\t\tgoto out;\n>  \t}\n>  \tdata = irq_desc_get_irq_data(desc);\n>  \twhile (data->parent_data)\n> @@ -457,29 +463,10 @@ static int kvm_vgic_map_irq(struct kvm_vcpu *vcpu, struct vgic_irq *irq,\n>  \tirq->hw = true;\n>  \tirq->host_irq = host_irq;\n>  \tirq->hwintid = data->hwirq;\n> -\treturn 0;\n> -}\n> -\n> -/* @irq->irq_lock must be held */\n> -static inline void kvm_vgic_unmap_irq(struct vgic_irq *irq)\n> -{\n> -\tirq->hw = false;\n> -\tirq->hwintid = 0;\n> -}\n> -\n> -int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,\n> -\t\t\t  u32 vintid)\n> -{\n> -\tstruct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, vintid);\n> -\tint ret;\n>  \n> -\tBUG_ON(!irq);\n> -\n> -\tspin_lock(&irq->irq_lock);\n> -\tret = kvm_vgic_map_irq(vcpu, irq, host_irq);\n> +out:\n>  \tspin_unlock(&irq->irq_lock);\n>  \tvgic_put_irq(vcpu->kvm, irq);\n> -\n>  \treturn ret;\n>  }\n>  \n> @@ -494,7 +481,8 @@ int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, unsigned int vintid)\n>  \tBUG_ON(!irq);\n>  \n>  \tspin_lock(&irq->irq_lock);\n> -\tkvm_vgic_unmap_irq(irq);\n> +\tirq->hw = false;\n> +\tirq->hwintid = 0;\n>  \tspin_unlock(&irq->irq_lock);\n>  \tvgic_put_irq(vcpu->kvm, irq);\n>  \n>","headers":{"Return-Path":"<linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org>","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=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"gYKT8U6S\"; dkim-atps=neutral","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx10.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 3xmjYz5X5kz9s3w\n\tfor <incoming-imx@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 20:26:47 +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 1dpB48-00032w-DD; Tue, 05 Sep 2017 10:26:44 +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 1dpB42-0002i7-MM for linux-arm-kernel@lists.infradead.org;\n\tTue, 05 Sep 2017 10:26:42 +0000","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 9C8EA5D687;\n\tTue,  5 Sep 2017 10:26:17 +0000 (UTC)","from localhost.localdomain (ovpn-117-1.ams2.redhat.com\n\t[10.36.117.1])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 1F07F901E9;\n\tTue,  5 Sep 2017 10:26:15 +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=oNfGyvJsNCZG7Ttgwg2odQNFXKpDwjmbQnB/HhUzl9M=;\n\tb=gYKT8U6SDM8yQu\n\tysG05yQ3l1VwGLEFbRRDiuTUqoOWvOLT/HZ1UKEpKmuc1wW8KU81m3zASY0ZGCAq8627gth+YU4I4\n\tjrv90TMaft1AGoPhZzlskUwmnn6EGF+UEfC0YPtMl2sPXuEtuyj/q5Su6IYlTOGOLFY+Xo6FiCXJn\n\txvihDDZHA2/8RsN9AydQgQyuvU7eWzEhOPPx+nqJ+qbxCGDRC9HzshKC4t6FtHQoKylO37pDljAi/\n\tBrUhcXwQQ5nH7mPEutfE4HKMbNU2pfpiPIxDD/6IvIJ6307Y5viJmzQ2i1c4fDPcUVCatxQAJrVLc\n\tC548QkYGJ9lKuz1oZN2A==;","DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 9C8EA5D687","Subject":"Re: [PATCH v2 5/6] KVM: arm/arm64: Rearrange kvm_vgic_[un]map_phys\n\tcode in vgic.c","To":"Christoffer Dall <cdall@linaro.org>, kvmarm@lists.cs.columbia.edu,\n\tMarc Zyngier <marc.zyngier@arm.com>","References":"<20170904102456.9025-1-cdall@linaro.org>\n\t<20170904102456.9025-6-cdall@linaro.org>","From":"Auger Eric <eric.auger@redhat.com>","Message-ID":"<56431c9d-3f7e-f8b2-2d31-6baec4e294da@redhat.com>","Date":"Tue, 5 Sep 2017 12:26:14 +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":"<20170904102456.9025-6-cdall@linaro.org>","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.39]); Tue, 05 Sep 2017 10:26:17 +0000 (UTC)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170905_032638_872748_B66E4893 ","X-CRM114-Status":"GOOD (  17.41  )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details:   (-6.9 points)\n\tpts rule name              description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [209.132.183.28 listed in list.dnswl.org]\n\t-0.0 RCVD_IN_MSPIKE_H3      RBL: Good reputation (+3)\n\t[209.132.183.28 listed in wl.mailspike.net]\n\t-0.0 SPF_PASS               SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-0.0 SPF_HELO_PASS          SPF: HELO matches SPF record\n\t-1.9 BAYES_00               BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]\n\t-0.0 RCVD_IN_MSPIKE_WL      Mailspike good senders","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":"<http://lists.infradead.org/mailman/options/linux-arm-kernel>,\n\t<mailto:linux-arm-kernel-request@lists.infradead.org?subject=unsubscribe>","List-Archive":"<http://lists.infradead.org/pipermail/linux-arm-kernel/>","List-Post":"<mailto:linux-arm-kernel@lists.infradead.org>","List-Help":"<mailto:linux-arm-kernel-request@lists.infradead.org?subject=help>","List-Subscribe":"<http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>,\n\t<mailto:linux-arm-kernel-request@lists.infradead.org?subject=subscribe>","Cc":"Andre Przywara <andre.przywara@arm.com>,\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\" <linux-arm-kernel-bounces@lists.infradead.org>","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1763378,"web_url":"http://patchwork.ozlabs.org/comment/1763378/","msgid":"<20170905135702.GO13572@cbox>","list_archive_url":null,"date":"2017-09-05T13:57:02","subject":"Re: [PATCH v2 3/6] 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 Tue, Sep 05, 2017 at 11:38:48AM +0200, Auger Eric wrote:\n> Hi Christoffer,\n> \n> On 04/09/2017 12:24, 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 <cdall@linaro.org>\n> > Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>\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..618ed3f 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_get_phys_line_level(irq);\n> > +\n> > +\t\t\tif (!irq->line_level)\n> > +\t\t\t\tvgic_irq_set_phys_active(irq, false);\n> I am still unclear wrt ISPENDR case. Assume the guest writes into the\n> ISPENDR. So now we set the pending state on the phys distributor. The\n> SPI is acked by the host. I understand it becomes active (ie. not\n> pending and active). Then it gets injected into the guest and in the\n> above fold code the LR state is observed GICH_LR_PENDING_BIT. We are\n> going to read the physical pending state which is: not pending. So the\n> line level is low, the latch_pending is not used in mapped case and is\n> low. So to me the vIRQ is missed.\n> \n> What do I miss?\n\nYou're not missing anything.  It was supposed to set pending first then\nfollowed by active.  I can fix that.\n\nThanks,\n-Christoffer\n\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..0f72bcb 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_get_phys_line_level(irq);\n> > +\n> > +\t\t\tif (!irq->line_level)\n> > +\t\t\t\tvgic_irq_set_phys_active(irq, false);\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..8072969 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_get_phys_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> > +/* Set/Clear the physical active state */\n> > +void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active)\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      active));\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..7bdcda2 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_get_phys_line_level(struct vgic_irq *irq);\n> > +void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active);\n> >  bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq);\n> >  void vgic_kick_vcpus(struct kvm *kvm);\n> >  \n> >","headers":{"Return-Path":"<linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org>","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=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org header.b=\"FaEIP8oK\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"WsL8jAeZ\"; 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 3xmpFg2GHsz9t2R\n\tfor <incoming-imx@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 23:57:59 +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 1dpEMT-0007Va-0O; Tue, 05 Sep 2017 13:57:53 +0000","from mail-wm0-x22a.google.com ([2a00:1450:400c:c09::22a])\n\tby bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dpEM4-0007KV-Qo for linux-arm-kernel@lists.infradead.org;\n\tTue, 05 Sep 2017 13:57:32 +0000","by mail-wm0-x22a.google.com with SMTP id 187so19699557wmn.1\n\tfor <linux-arm-kernel@lists.infradead.org>;\n\tTue, 05 Sep 2017 06:57:07 -0700 (PDT)","from localhost (xd93ddc2d.cust.hiper.dk. [217.61.220.45])\n\tby smtp.gmail.com with ESMTPSA id\n\tz8sm234881edb.32.2017.09.05.06.57.04\n\t(version=TLS1_2 cipher=AES128-SHA bits=128/128);\n\tTue, 05 Sep 2017 06:57:05 -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=5gWVsTyhvYEnDd8UqMqMbEyFkKxh3bsHizILoQUqR3M=;\n\tb=FaEIP8oKDJm1Q/\n\tUY4ZOYE7WA0oSqiAY93aHMYUo0TpbsAjmP4Dc2M7HjG48HtCITdWK4ZQ1EPvAj+kpm1dR+sHHAYmn\n\tHsGVUSTyTyOOPwJmvS1C1Fq0HsXErsCTJbfcZbeMZ0Jbtn47qytSQc8kbzjoaiKi0MtKIFtsgz6QB\n\t0SZLzBB/7Qb0SiUot9oSmxGvxvxw6NLt52lWFy87Mv4vMwKmgwFpFLeP19dVC3c7bHytb0PaI6lmt\n\tXPLpUnUGARrQ/P9hVLIrjl2x/C7+Zm4g18Ai4eFpKUwFUXQ0Px27gZlUuMecZ3GMfH5xJxy9ydgyl\n\tMyQlaU35SpFVW/CURW1A==;","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=YPjrXYfx6dUYGP6sy75F+fi9mqApBwBAzz8ABF+1oyk=;\n\tb=WsL8jAeZMfZzenpglAFPtUkAhnjb0soY4yVKjTmTS/7ytARdHeD/rjM704bFw2IS1d\n\t8RNF5m0s7Bxv4I6D2xrNDS2kYsK2mxn2C/dZh4W0wk9Mx3hKMTN7y+2cu84w/SxXOQNa\n\tGgp8F7OUF0QtnrF4TV4NFesshD8NlO7PTKEFc="],"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=YPjrXYfx6dUYGP6sy75F+fi9mqApBwBAzz8ABF+1oyk=;\n\tb=amS54FOwuanvLXRDwZy+Ghpo16g2za0jB/EXMFKkWraZKEFV1ZU8PAGMj9m+kXRIrG\n\tjcIeOGi6VJVe58nd6a1YNQRW4dsBJzwknUbxwOJ/BNPTW3gXSTYRtfxS7fCPOfCuPQgE\n\tmcAj2YTy3u/GJ8G8OpbC159u5xTHR6960abw4bkzNMieZ33+qinwpdm0ggPmeE2LMLhE\n\tVFu+WQahDDH7IasA70M5T/LDVCXFCtEHxNhruyAVyQfNCZh8viUMLTuDb4XZWarIBqdp\n\tJ4fQsHIcnDNvQXKrpJPMJKvze2dJa65CKcCogoK7Si0Oix1hlqO1UthmVgBBua2w9gAX\n\tDXsg==","X-Gm-Message-State":"AHPjjUirZI7h07vmICxdvaX8Aj8V+MifhTJbgoy3WPMp2qlEVy8+6D6s\n\tdA2gbLTQkXA8RzNR","X-Google-Smtp-Source":"ADKCNb4mAvEFiTxe03LNVLl6R6NgwBZ25fjGTz2Ia4im0w7YZaUDfsvaQbeuGIMB4+ggnhzirxa5VA==","X-Received":"by 10.80.179.110 with SMTP id r43mr3466804edd.180.1504619825956; \n\tTue, 05 Sep 2017 06:57:05 -0700 (PDT)","Date":"Tue, 5 Sep 2017 15:57:02 +0200","From":"Christoffer Dall <cdall@linaro.org>","To":"Auger Eric <eric.auger@redhat.com>","Subject":"Re: [PATCH v2 3/6] KVM: arm/arm64: vgic: Support level-triggered\n\tmapped interrupts","Message-ID":"<20170905135702.GO13572@cbox>","References":"<20170904102456.9025-1-cdall@linaro.org>\n\t<20170904102456.9025-4-cdall@linaro.org>\n\t<5971dacd-ba04-1e68-f4c8-359500e79455@redhat.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<5971dacd-ba04-1e68-f4c8-359500e79455@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-20170905_065729_179264_A02DEDF5 ","X-CRM114-Status":"GOOD (  39.87  )","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:22a 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":"<http://lists.infradead.org/mailman/options/linux-arm-kernel>,\n\t<mailto:linux-arm-kernel-request@lists.infradead.org?subject=unsubscribe>","List-Archive":"<http://lists.infradead.org/pipermail/linux-arm-kernel/>","List-Post":"<mailto:linux-arm-kernel@lists.infradead.org>","List-Help":"<mailto:linux-arm-kernel-request@lists.infradead.org?subject=help>","List-Subscribe":"<http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>,\n\t<mailto:linux-arm-kernel-request@lists.infradead.org?subject=subscribe>","Cc":"Marc Zyngier <marc.zyngier@arm.com>,\n\tAndre Przywara <andre.przywara@arm.com>, kvmarm@lists.cs.columbia.edu,\n\tkvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" <linux-arm-kernel-bounces@lists.infradead.org>","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1763383,"web_url":"http://patchwork.ozlabs.org/comment/1763383/","msgid":"<20170905140024.GP13572@cbox>","list_archive_url":null,"date":"2017-09-05T14:00:24","subject":"Re: [PATCH v2 5/6] KVM: arm/arm64: Rearrange kvm_vgic_[un]map_phys\n\tcode in vgic.c","submitter":{"id":71350,"url":"http://patchwork.ozlabs.org/api/people/71350/","name":"Christoffer Dall","email":"cdall@linaro.org"},"content":"On Tue, Sep 05, 2017 at 12:26:14PM +0200, Auger Eric wrote:\n> Hi Christoffer,\n> On 04/09/2017 12:24, 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 <cdall@linaro.org>\n> > Acked-by: Marc Zyngier <marc.zyngier@arm.com>\n> > ---\n> >  virt/kvm/arm/vgic/vgic.c | 38 +++++++++++++-------------------------\n> >  1 file changed, 13 insertions(+), 25 deletions(-)\n> > \n> > diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c\n> > index 7aec730..b704ff5 100644\n> > --- a/virt/kvm/arm/vgic/vgic.c\n> > +++ b/virt/kvm/arm/vgic/vgic.c\n> > @@ -435,12 +435,17 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,\n> >  \treturn 0;\n> >  }\n> >  \n> > -/* @irq->irq_lock must be held */\n> I chose to hold the lock outside of kvm_vgic_map/unmap_irq because in\n> kvm_vgic_set_forwarding(see https://lkml.org/lkml/2017/6/15/278) I was\n> also testing hw and target_vcpu fields. As you pointed out maybe I am\n> not obliged to check them but that was the rationale.\n> \n\nAh ok, I see, you want to reuse this bit of code and the caller will\nalready be holding the spin-lock?\n\nI can rework it then to pass the callback in kvm_vgic_map_irq.  Would\nthat fit better with your subsequent patches?\n\nThanks,\n-Christoffer\n\n> > -static int kvm_vgic_map_irq(struct kvm_vcpu *vcpu, struct vgic_irq *irq,\n> > -\t\t\t    unsigned int host_irq)\n> > +int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,\n> > +\t\t\t  u32 vintid)\n> >  {\n> > +\tstruct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, vintid);\n> >  \tstruct irq_desc *desc;\n> >  \tstruct irq_data *data;\n> > +\tint ret = 0;\n> > +\n> > +\tBUG_ON(!irq);\n> > +\n> > +\tspin_lock(&irq->irq_lock);\n> >  \n> >  \t/*\n> >  \t * Find the physical IRQ number corresponding to @host_irq\n> > @@ -448,7 +453,8 @@ static int kvm_vgic_map_irq(struct kvm_vcpu *vcpu, struct vgic_irq *irq,\n> >  \tdesc = irq_to_desc(host_irq);\n> >  \tif (!desc) {\n> >  \t\tkvm_err(\"%s: no interrupt descriptor\\n\", __func__);\n> > -\t\treturn -EINVAL;\n> > +\t\tret = -EINVAL;\n> > +\t\tgoto out;\n> >  \t}\n> >  \tdata = irq_desc_get_irq_data(desc);\n> >  \twhile (data->parent_data)\n> > @@ -457,29 +463,10 @@ static int kvm_vgic_map_irq(struct kvm_vcpu *vcpu, struct vgic_irq *irq,\n> >  \tirq->hw = true;\n> >  \tirq->host_irq = host_irq;\n> >  \tirq->hwintid = data->hwirq;\n> > -\treturn 0;\n> > -}\n> > -\n> > -/* @irq->irq_lock must be held */\n> > -static inline void kvm_vgic_unmap_irq(struct vgic_irq *irq)\n> > -{\n> > -\tirq->hw = false;\n> > -\tirq->hwintid = 0;\n> > -}\n> > -\n> > -int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,\n> > -\t\t\t  u32 vintid)\n> > -{\n> > -\tstruct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, vintid);\n> > -\tint ret;\n> >  \n> > -\tBUG_ON(!irq);\n> > -\n> > -\tspin_lock(&irq->irq_lock);\n> > -\tret = kvm_vgic_map_irq(vcpu, irq, host_irq);\n> > +out:\n> >  \tspin_unlock(&irq->irq_lock);\n> >  \tvgic_put_irq(vcpu->kvm, irq);\n> > -\n> >  \treturn ret;\n> >  }\n> >  \n> > @@ -494,7 +481,8 @@ int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, unsigned int vintid)\n> >  \tBUG_ON(!irq);\n> >  \n> >  \tspin_lock(&irq->irq_lock);\n> > -\tkvm_vgic_unmap_irq(irq);\n> > +\tirq->hw = false;\n> > +\tirq->hwintid = 0;\n> >  \tspin_unlock(&irq->irq_lock);\n> >  \tvgic_put_irq(vcpu->kvm, irq);\n> >  \n> >","headers":{"Return-Path":"<linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org>","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=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org header.b=\"imXPc9mn\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"dtVqEVOA\"; 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 3xmpK54QNxz9t2R\n\tfor <incoming-imx@patchwork.ozlabs.org>;\n\tWed,  6 Sep 2017 00:00:57 +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 1dpEPO-0001jm-1q; Tue, 05 Sep 2017 14:00:54 +0000","from mail-wm0-x22d.google.com ([2a00:1450:400c:c09::22d])\n\tby bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dpEPK-0001eQ-7F for linux-arm-kernel@lists.infradead.org;\n\tTue, 05 Sep 2017 14:00:52 +0000","by mail-wm0-x22d.google.com with SMTP id 187so19738439wmn.1\n\tfor <linux-arm-kernel@lists.infradead.org>;\n\tTue, 05 Sep 2017 07:00:29 -0700 (PDT)","from localhost (xd93ddc2d.cust.hiper.dk. [217.61.220.45])\n\tby smtp.gmail.com with ESMTPSA id\n\te12sm242807edm.85.2017.09.05.07.00.26\n\t(version=TLS1_2 cipher=AES128-SHA bits=128/128);\n\tTue, 05 Sep 2017 07:00: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=T4XV4Pu/7ZBrOE2dARmFS3qn6zuEQ6oxzA3iJIeOP8A=;\n\tb=imXPc9mnUZ+ikQ\n\tbiWp42UBzu4iYpjC+o8dcvtAOwqaDuR2tZ3ybZ4jlEp/RbHWYpg91cS4krgrKjxjutMhLGVdFZ5AG\n\tHYwn8j8t6zRTome9hrF4TG37Wbe/vrHsSVYmHaglP2nsHvt9URawQG5kTla2QiNMFgpqdqnSNv+YP\n\tk3P1hq5CG6o3/dTD9FKGFHiL9M9g1GXue3Do8V77c7G9ZDSpq7bMxaV5bu7VlwxPpJ0zHK2iZuKCg\n\tIVM24uU739A76plEkShBlfMey69GpN/3o0RF7vb1LjliQZzXYjtkFap4tTIPSkrU7iJADKfhBBG4H\n\ty46TOc+qWQJswZZ22XHA==;","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=TqFjP3aaGF606FXBs3aQAfrkAL55g68phTQvxTrhEqw=;\n\tb=dtVqEVOAI6NJ44uPKxV2oMhYPez3/enKdNUrSVlBbivmsLr7dU0ffAEe0FprCQxYhN\n\tWKnsRBr5EcKLV9ewVB6jdMLXtpzUhKfx1copjqR5mMPSGy1orwBQDgKZd7SOpGvxcny5\n\t3RMGiHX1Ct9ueCRDWHfadL/nfIEl1s/b2HdDQ="],"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=TqFjP3aaGF606FXBs3aQAfrkAL55g68phTQvxTrhEqw=;\n\tb=n0yaTjHnQ+b3P/JcikRfEyeQd25OoQH2IaMI2kys7CoqXgMCdXPOJz9TNQ6jXPUtVH\n\tyuVQ6QmdmV9/lr1SyVM23/gBxl9JUafoOO+hviIOcM1X4fBW7z+PliknyCi+Bwkb2gEU\n\tSV4N5+4YbXBLD2/+KwbytPW4ulqXrE/A+ifhLOpfQwP+r4Oo9F3sBLxZEAaWyWhjFCmX\n\tcjH33VzmWVzVIuoMWIS98E+rkf07WydV7svUiR88oG3Qlcc4Oh+u0nymwAe6hChSMA+5\n\t3iFup9r9IS61lTAY3nGVaTqaSysVHbyr/dW0gZIie32CFDbzuVQcONtI+wmcpGFCy322\n\tq/wQ==","X-Gm-Message-State":"AHPjjUjkEVf7tPPz3+CQS/Oj8Dz+TMvKDlbPZdJ9JdZtpBTWycHa52Qr\n\tcOg1jgWLugOc3MPGbRoROg==","X-Google-Smtp-Source":"ADKCNb7LM8Bp+IBO7oH0Qd/fD7fcb6W7YNd9qWYT4uyeomKIxBEEdf39oCMqkqQwcGObgCzV+d0A4g==","X-Received":"by 10.80.144.182 with SMTP id c51mr3364387eda.68.1504620027846; \n\tTue, 05 Sep 2017 07:00:27 -0700 (PDT)","Date":"Tue, 5 Sep 2017 16:00:24 +0200","From":"Christoffer Dall <cdall@linaro.org>","To":"Auger Eric <eric.auger@redhat.com>","Subject":"Re: [PATCH v2 5/6] KVM: arm/arm64: Rearrange kvm_vgic_[un]map_phys\n\tcode in vgic.c","Message-ID":"<20170905140024.GP13572@cbox>","References":"<20170904102456.9025-1-cdall@linaro.org>\n\t<20170904102456.9025-6-cdall@linaro.org>\n\t<56431c9d-3f7e-f8b2-2d31-6baec4e294da@redhat.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<56431c9d-3f7e-f8b2-2d31-6baec4e294da@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-20170905_070050_420274_96BF5BB9 ","X-CRM114-Status":"GOOD (  20.16  )","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:22d 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":"<http://lists.infradead.org/mailman/options/linux-arm-kernel>,\n\t<mailto:linux-arm-kernel-request@lists.infradead.org?subject=unsubscribe>","List-Archive":"<http://lists.infradead.org/pipermail/linux-arm-kernel/>","List-Post":"<mailto:linux-arm-kernel@lists.infradead.org>","List-Help":"<mailto:linux-arm-kernel-request@lists.infradead.org?subject=help>","List-Subscribe":"<http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>,\n\t<mailto:linux-arm-kernel-request@lists.infradead.org?subject=subscribe>","Cc":"Marc Zyngier <marc.zyngier@arm.com>,\n\tAndre Przywara <andre.przywara@arm.com>, kvmarm@lists.cs.columbia.edu,\n\tkvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" <linux-arm-kernel-bounces@lists.infradead.org>","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1763417,"web_url":"http://patchwork.ozlabs.org/comment/1763417/","msgid":"<2e5123d1-2440-45a1-651c-2813ffce42fd@redhat.com>","list_archive_url":null,"date":"2017-09-05T14:49:26","subject":"Re: [PATCH v2 5/6] KVM: arm/arm64: Rearrange kvm_vgic_[un]map_phys\n\tcode in vgic.c","submitter":{"id":69187,"url":"http://patchwork.ozlabs.org/api/people/69187/","name":"Eric Auger","email":"eric.auger@redhat.com"},"content":"Hi Christoffer,\nOn 05/09/2017 16:00, Christoffer Dall wrote:\n> On Tue, Sep 05, 2017 at 12:26:14PM +0200, Auger Eric wrote:\n>> Hi Christoffer,\n>> On 04/09/2017 12:24, 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 <cdall@linaro.org>\n>>> Acked-by: Marc Zyngier <marc.zyngier@arm.com>\n>>> ---\n>>>  virt/kvm/arm/vgic/vgic.c | 38 +++++++++++++-------------------------\n>>>  1 file changed, 13 insertions(+), 25 deletions(-)\n>>>\n>>> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c\n>>> index 7aec730..b704ff5 100644\n>>> --- a/virt/kvm/arm/vgic/vgic.c\n>>> +++ b/virt/kvm/arm/vgic/vgic.c\n>>> @@ -435,12 +435,17 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,\n>>>  \treturn 0;\n>>>  }\n>>>  \n>>> -/* @irq->irq_lock must be held */\n>> I chose to hold the lock outside of kvm_vgic_map/unmap_irq because in\n>> kvm_vgic_set_forwarding(see https://lkml.org/lkml/2017/6/15/278) I was\n>> also testing hw and target_vcpu fields. As you pointed out maybe I am\n>> not obliged to check them but that was the rationale.\n>>\n> \n> Ah ok, I see, you want to reuse this bit of code and the caller will\n> already be holding the spin-lock?\n> \n> I can rework it then to pass the callback in kvm_vgic_map_irq.  Would\n> that fit better with your subsequent patches?\nYes it would.\n\nThanks\n\nEric\n> \n> Thanks,\n> -Christoffer\n> \n>>> -static int kvm_vgic_map_irq(struct kvm_vcpu *vcpu, struct vgic_irq *irq,\n>>> -\t\t\t    unsigned int host_irq)\n>>> +int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,\n>>> +\t\t\t  u32 vintid)\n>>>  {\n>>> +\tstruct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, vintid);\n>>>  \tstruct irq_desc *desc;\n>>>  \tstruct irq_data *data;\n>>> +\tint ret = 0;\n>>> +\n>>> +\tBUG_ON(!irq);\n>>> +\n>>> +\tspin_lock(&irq->irq_lock);\n>>>  \n>>>  \t/*\n>>>  \t * Find the physical IRQ number corresponding to @host_irq\n>>> @@ -448,7 +453,8 @@ static int kvm_vgic_map_irq(struct kvm_vcpu *vcpu, struct vgic_irq *irq,\n>>>  \tdesc = irq_to_desc(host_irq);\n>>>  \tif (!desc) {\n>>>  \t\tkvm_err(\"%s: no interrupt descriptor\\n\", __func__);\n>>> -\t\treturn -EINVAL;\n>>> +\t\tret = -EINVAL;\n>>> +\t\tgoto out;\n>>>  \t}\n>>>  \tdata = irq_desc_get_irq_data(desc);\n>>>  \twhile (data->parent_data)\n>>> @@ -457,29 +463,10 @@ static int kvm_vgic_map_irq(struct kvm_vcpu *vcpu, struct vgic_irq *irq,\n>>>  \tirq->hw = true;\n>>>  \tirq->host_irq = host_irq;\n>>>  \tirq->hwintid = data->hwirq;\n>>> -\treturn 0;\n>>> -}\n>>> -\n>>> -/* @irq->irq_lock must be held */\n>>> -static inline void kvm_vgic_unmap_irq(struct vgic_irq *irq)\n>>> -{\n>>> -\tirq->hw = false;\n>>> -\tirq->hwintid = 0;\n>>> -}\n>>> -\n>>> -int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,\n>>> -\t\t\t  u32 vintid)\n>>> -{\n>>> -\tstruct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, vintid);\n>>> -\tint ret;\n>>>  \n>>> -\tBUG_ON(!irq);\n>>> -\n>>> -\tspin_lock(&irq->irq_lock);\n>>> -\tret = kvm_vgic_map_irq(vcpu, irq, host_irq);\n>>> +out:\n>>>  \tspin_unlock(&irq->irq_lock);\n>>>  \tvgic_put_irq(vcpu->kvm, irq);\n>>> -\n>>>  \treturn ret;\n>>>  }\n>>>  \n>>> @@ -494,7 +481,8 @@ int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, unsigned int vintid)\n>>>  \tBUG_ON(!irq);\n>>>  \n>>>  \tspin_lock(&irq->irq_lock);\n>>> -\tkvm_vgic_unmap_irq(irq);\n>>> +\tirq->hw = false;\n>>> +\tirq->hwintid = 0;\n>>>  \tspin_unlock(&irq->irq_lock);\n>>>  \tvgic_put_irq(vcpu->kvm, irq);\n>>>  \n>>>","headers":{"Return-Path":"<linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org>","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=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"Qn66S2Wi\"; dkim-atps=neutral","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx09.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 3xmqPh5RDwz9t2v\n\tfor <incoming-imx@patchwork.ozlabs.org>;\n\tWed,  6 Sep 2017 00:50: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 1dpFAq-0005dV-Mn; Tue, 05 Sep 2017 14:49:56 +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 1dpFAl-0005JW-4d for linux-arm-kernel@lists.infradead.org;\n\tTue, 05 Sep 2017 14:49:53 +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 98FC165D12;\n\tTue,  5 Sep 2017 14:49:29 +0000 (UTC)","from localhost.localdomain (ovpn-117-1.ams2.redhat.com\n\t[10.36.117.1])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 181888E36D;\n\tTue,  5 Sep 2017 14:49:27 +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=ikGkzCK+a6aL0kJISRt5mo3cfEQdGg875kzac1aUcWA=;\n\tb=Qn66S2Wi+IWw2j\n\t2bmEKdj1gZLnQIiottmBpma8hQQErulPsc4aLWDuNVR7g1vLZlacQ7qqCCwBfAtTLaoT6Qc3rmXXd\n\tXK13yUpjD45cIqZI+NpNtdKZvruklM3uSKWUV9iav3xA2scRxMdJjoPvPO8i5oWgg+iYuKhZCxxw7\n\tl9s/r6ggz6bfIXZScNRrDO7EuceiH0TNcPKywfvWLXcnqfYuprvBsr2OCLcD4khITxB+oEMVwpZ00\n\tW9l07Bc7eRUPeYhKT9iJH53OHjvhcmYOLT+9v+yS9jUEYQXB8XiIVfj16NraB6jxytYC2iJ7QEGHH\n\tw8VQ3parPDD1BTZpPHEg==;","DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 98FC165D12","Subject":"Re: [PATCH v2 5/6] KVM: arm/arm64: Rearrange kvm_vgic_[un]map_phys\n\tcode in vgic.c","To":"Christoffer Dall <cdall@linaro.org>","References":"<20170904102456.9025-1-cdall@linaro.org>\n\t<20170904102456.9025-6-cdall@linaro.org>\n\t<56431c9d-3f7e-f8b2-2d31-6baec4e294da@redhat.com>\n\t<20170905140024.GP13572@cbox>","From":"Auger Eric <eric.auger@redhat.com>","Message-ID":"<2e5123d1-2440-45a1-651c-2813ffce42fd@redhat.com>","Date":"Tue, 5 Sep 2017 16:49:26 +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":"<20170905140024.GP13572@cbox>","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.38]); Tue, 05 Sep 2017 14:49:29 +0000 (UTC)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170905_074951_288551_46AE86BC ","X-CRM114-Status":"GOOD (  19.20  )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details:   (-6.9 points)\n\tpts rule name              description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [209.132.183.28 listed in list.dnswl.org]\n\t-0.0 RCVD_IN_MSPIKE_H3      RBL: Good reputation (+3)\n\t[209.132.183.28 listed in wl.mailspike.net]\n\t-0.0 SPF_PASS               SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-0.0 SPF_HELO_PASS          SPF: HELO matches SPF record\n\t-1.9 BAYES_00               BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]\n\t-0.0 RCVD_IN_MSPIKE_WL      Mailspike good senders","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":"<http://lists.infradead.org/mailman/options/linux-arm-kernel>,\n\t<mailto:linux-arm-kernel-request@lists.infradead.org?subject=unsubscribe>","List-Archive":"<http://lists.infradead.org/pipermail/linux-arm-kernel/>","List-Post":"<mailto:linux-arm-kernel@lists.infradead.org>","List-Help":"<mailto:linux-arm-kernel-request@lists.infradead.org?subject=help>","List-Subscribe":"<http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>,\n\t<mailto:linux-arm-kernel-request@lists.infradead.org?subject=subscribe>","Cc":"Marc Zyngier <marc.zyngier@arm.com>,\n\tAndre Przywara <andre.przywara@arm.com>, kvmarm@lists.cs.columbia.edu,\n\tkvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" <linux-arm-kernel-bounces@lists.infradead.org>","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}}]