Re: [PATCH v1] KVM: arm64: vgic-v4: Don't forward an LPI with no target vCPU

[Marc Zyngier <maz@kernel.org>]

On Mon, 12 Oct 2026 09:47:43 +0100,
Fuad Tabba <fuad.tabba@linux.dev> wrote:
> 
> KVM keeps one vgic_irq per LPI. The MAPTI that creates it sets its
> target vCPU from that MAPTI's collection, and a later MAPTI of the same
> LPI leaves the target alone. Mapping one LPI from two events is
> UNPREDICTABLE (IHI 0069H.b, 5.2.10), but nothing stops a guest doing
> it. If the first MAPTI names a collection the guest never maps and the
> second a mapped one, the LPI translates through the second event with
> a NULL target vCPU.
> 
> When the VMM routes an assigned device's MSI to that event on a host
> with GICv4 enabled, kvm_vgic_v4_set_forwarding() takes the vPE from the
> NULL target vCPU and the host oopses:
> 
>   Unable to handle kernel paging request at virtual address 0000000000001980
>   [...]
>   Call trace:
>    its_build_vmapti_cmd (drivers/irqchip/irq-gic-v3-its.c:943) (P)
>    its_irq_set_vcpu_affinity (drivers/irqchip/irq-gic-v3-its.c:1380)
>    irq_set_vcpu_affinity (kernel/irq/manage.c:670)
>    its_map_vlpi (drivers/irqchip/irq-gic-v4.c:326)
>    kvm_vgic_v4_set_forwarding (arch/arm64/kvm/vgic/vgic-v4.c:485)
>    kvm_arch_irq_bypass_add_producer (arch/arm64/kvm/arm.c:2977)
>    irq_bypass_register_consumer (virt/lib/irqbypass.c:44)
>    kvm_irqfd (arch/arm64/kvm/../../../virt/kvm/eventfd.c:496)
>    kvm_vm_ioctl (arch/arm64/kvm/../../../virt/kvm/kvm_main.c:5270)
> 
> Leave such an LPI to the software path, which copes with a NULL target,
> as kvm_vgic_v4_set_forwarding() already does when the MSI doesn't
> translate.
> 
> Fixes: 196b136498b35 ("KVM: arm/arm64: GICv4: Wire mapping/unmapping of VLPIs in VFIO irq bypass")
> Cc: stable@vger.kernel.org
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
> ---
> Tested on QEMU, which emulates the GICv4 ITS, with a local reproducer
> that sets up the two MAPTIs and forwards a vfio-pci MSI to the LPI. It
> oopses without the fix and passes with it. Not tested on GICv4 hardware.
> 
> Based on v7.3-rc3.
> 
>  arch/arm64/kvm/vgic/vgic-v4.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c
> index ed236f083f0d7..545e55317c863 100644
> --- a/arch/arm64/kvm/vgic/vgic-v4.c
> +++ b/arch/arm64/kvm/vgic/vgic-v4.c
> @@ -466,6 +466,13 @@ int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int virq,
>  	if (irq->hw)
>  		goto out_unlock_irq;
>  
> +	/*
> +	 * Silently exit if the LPI has no target vCPU, as one mapped by
> +	 * multiple ITEs (UNPREDICTABLE) can have.
> +	 */
> +	if (!irq->target_vcpu)
> +		goto out_unlock_irq;
> +
>  	/*
>  	 * Emit the mapping request. If it fails, the ITS probably
>  	 * isn't v4 compatible, so let's silently bail out. Holding
> 

Isn't the core problem that we don't correctly handle the UNPRED
double mapping of the same LPI? I'd rather we attack the issue at the
source instead of papering over the consequences.

I hacked the following, which is obviously untested. Can you give it a
go?

Thanks,

	M.

diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index e5657cb603567..f0a1a879d9ac1 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -73,14 +73,16 @@ static int update_lpi_config(struct kvm *kvm, struct vgic_irq *irq,
  * If this is a "new" LPI, we allocate and initialize a new struct vgic_irq.
  * This function returns a pointer to the _unlocked_ structure.
  */
-static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
-				     struct kvm_vcpu *vcpu)
+static struct vgic_irq *__vgic_add_lpi(struct kvm *kvm, u32 intid,
+				       struct kvm_vcpu *vcpu, bool *created)
 {
 	struct vgic_dist *dist = &kvm->arch.vgic;
 	struct vgic_irq *irq = vgic_get_irq(kvm, intid), *oldirq;
 	unsigned long flags;
 	int ret;
 
+	*created = false;
+
 	/* In this case there is no put, since we keep the reference. */
 	if (irq)
 		return irq;
@@ -133,6 +135,8 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
 
 		if (oldirq && !WARN_ON_ONCE(refcount_read(&oldirq->refcount)))
 			kfree_rcu(oldirq, rcu);
+
+		*created = true;
 	}
 
 	xa_unlock_irqrestore(&dist->lpi_xa, flags);
@@ -160,6 +164,25 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
 	return irq;
 }
 
+static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, int lpi_nr, struct kvm_vcpu *vcpu)
+{
+	struct vgic_irq *irq;
+	bool created;
+
+	irq = __vgic_add_lpi(kvm, lpi_nr, vcpu, &created);
+
+	/*
+	 * Mapping an LPI twice is UNPREDICTIBLE. Drop mapping an
+	 * existing LPI on the floor and convert it into an error.
+	 */
+	if (irq && !IS_ERR(irq) && !created) {
+		vgic_put_irq(kvm, irq);
+		irq = ERR_PTR(-EEXIST);
+	}
+
+	return irq;
+}
+
 /**
  * struct vgic_its_abi - ITS abi ops and settings
  * @cte_esz: collection table entry size

-- 
Without deviation from the norm, progress is not possible.