[PATCH v2 0/2] KVM: arm64: vgic-its: Fix LPI double mapping and DISCARD

[PATCH v2 0/2] KVM: arm64: vgic-its: Fix LPI double mapping and DISCARD

[Fuad Tabba]

Hi folks,

Changes since v1 [1]:
  - Dropped the second mapping of an LPI at MAPTI, instead of keeping a
    doubly mapped LPI out of GICv4 forwarding. (Marc)
  - Keyed that check on whether an ITE maps the LPI, not on whether its
    vgic_irq exists.
  - Added a patch that makes DISCARD remove the pending state. (Marc)

Two fixes to the ITS emulation. The first stops a guest from mapping
one LPI from two events, which can oops a GICv4 host when the VMM then
routes an assigned device's MSI to the second event. The second makes
DISCARD remove the LPI's pending state, as the spec requires.

Tested on QEMU, which emulates a GICv4 ITS. A local reproducer with two
vCPUs DISCARDs an LPI that's pending in the other vCPU's list register.
On rc3, and with patch 2 but without its halt, the guest still takes
the LPI. With the series applied, it doesn't.

Based on Linux 7.3-rc3 (fd73f4a665989).

Cheers,
/fuad

[1] https://lore.kernel.org/all/20261012084743.2228612-1-fuad.tabba@linux.dev/

Fuad Tabba (2):
  KVM: arm64: vgic-its: Don't map an LPI from more than one event
  KVM: arm64: vgic-its: Remove the pending state of a discarded LPI

 arch/arm64/kvm/vgic/vgic-its.c | 50 +++++++++++++++++++++++++++++-----
 include/kvm/arm_vgic.h         |  1 +
 2 files changed, 44 insertions(+), 7 deletions(-)


base-commit: fd73f4a6659897191fa0d40695fe370925dd3780
-- 
2.39.5

[PATCH v2 1/2] KVM: arm64: vgic-its: Don't map an LPI from more than one event

[Fuad Tabba]

KVM keeps one vgic_irq per LPI. A MAPTI of an LPI whose vgic_irq
already exists reuses it for the new ITE and leaves its target vCPU as
it was. Mapping one LPI from two events is UNPREDICTABLE (IHI 0069H.b,
5.2.10), but nothing stops a guest from 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)

Track in each vgic_irq whether an ITE maps it. Drop a MAPTI of an LPI
that is already mapped, as a failed MAPTI already is. Restoring the ITS
tables keeps the first ITE it reads for each LPI and drops any later
one, so a guest saved with a double mapping still restores. Key the
check on the mapping, not on the vgic_irq. A vgic_irq outlives its ITE
while anything holds a reference to it, such as the vCPU it was last
queued on. A later MAPTI of such an LPI succeeds as before, and now
retargets it to its collection.

Fixes: 196b136498b35 ("KVM: arm/arm64: GICv4: Wire mapping/unmapping of VLPIs in VFIO irq bypass")
Suggested-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 arch/arm64/kvm/vgic/vgic-its.c | 26 ++++++++++++++++++++++++--
 include/kvm/arm_vgic.h         |  1 +
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 9e782a4fea7e5..44b92c01bc727 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -772,6 +772,7 @@ static void its_free_ite(struct kvm *kvm, struct its_ite *ite)
 				its_unmap_vlpi(ite->irq->host_irq);
 
 			irq->hw = false;
+			irq->mapped = false;
 		}
 
 		vgic_put_irq(kvm, ite->irq);
@@ -780,6 +781,23 @@ static void its_free_ite(struct kvm *kvm, struct its_ite *ite)
 	kfree(ite);
 }
 
+/* Mapping one LPI from two events is UNPREDICTABLE: the first ITE keeps it. */
+static bool vgic_its_map_lpi(struct kvm *kvm, struct its_ite *ite,
+			     struct vgic_irq *irq, struct kvm_vcpu *vcpu)
+{
+	scoped_guard(raw_spinlock_irqsave, &irq->irq_lock) {
+		if (!irq->mapped) {
+			irq->mapped = true;
+			irq->target_vcpu = vcpu;
+			ite->irq = irq;
+			return true;
+		}
+	}
+
+	vgic_put_irq(kvm, irq);
+	return false;
+}
+
 static u64 its_cmd_mask_field(u64 *its_cmd, int word, int shift, int size)
 {
 	return (le64_to_cpu(its_cmd[word]) >> shift) & (BIT_ULL(size) - 1);
@@ -1089,13 +1107,14 @@ static int vgic_its_cmd_handle_mapi(struct kvm *kvm, struct vgic_its *its,
 		vcpu = collection_to_vcpu(kvm, collection);
 
 	irq = vgic_add_lpi(kvm, lpi_nr, vcpu);
+	if (!IS_ERR(irq) && !vgic_its_map_lpi(kvm, ite, irq, vcpu))
+		irq = ERR_PTR(-EEXIST);
 	if (IS_ERR(irq)) {
 		if (new_coll)
 			vgic_its_free_collection(its, coll_id);
 		its_free_ite(kvm, ite);
 		return PTR_ERR(irq);
 	}
-	ite->irq = irq;
 
 	return 0;
 }
@@ -2195,7 +2214,10 @@ static int vgic_its_restore_ite(struct vgic_its *its, u32 event_id,
 		its_free_ite(kvm, ite);
 		return PTR_ERR(irq);
 	}
-	ite->irq = irq;
+
+	/* Drop an ITE whose LPI another ITE maps, as MAPTI does. */
+	if (!vgic_its_map_lpi(kvm, ite, irq, vcpu))
+		its_free_ite(kvm, ite);
 
 	return offset;
 }
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 086b7578e5f33..5c3a0154becc4 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -256,6 +256,7 @@ struct vgic_irq {
 	bool active:1;
 	bool hw:1;			/* Tied to HW IRQ */
 	bool on_lr:1;			/* Present in a CPU LR */
+	bool mapped:1;			/* LPIs: mapped by an ITE */
 	refcount_t refcount;		/* Used for LPIs */
 	u32 hwintid;			/* HW INTID number */
 	unsigned int host_irq;		/* linux irq corresponding to hwintid */
-- 
2.39.5

[PATCH v2 2/2] KVM: arm64: vgic-its: Remove the pending state of a discarded LPI

[Fuad Tabba]

DISCARD removes the mapping of an event and instructs the Redistributor
to remove the pending state of its LPI (IHI 0069H.b, 5.3.4). KVM only
frees the ITE, so an LPI that is pending when the guest discards it
stays queued on its target vCPU, and the guest still takes it.

Clear the pending state before freeing the ITE. A queued LPI can be
pending in a running vCPU's list register, so halt the guest first when
the LPI is queued, to fold that state back. A forwarded LPI already
loses its pending state in its_free_ite(), whose unmap from the vPE
sends a DISCARD to the host ITS.

Fixes: df9f58fbea9bc ("KVM: arm64: vgic-its: Implement ITS command queue command handlers")
Suggested-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 arch/arm64/kvm/vgic/vgic-its.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 44b92c01bc727..3b8e0e5b097e0 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -829,13 +829,27 @@ static int vgic_its_cmd_handle_discard(struct kvm *kvm, struct vgic_its *its,
 		struct its_device *device = find_its_device(its, device_id);
 		int ite_esz = vgic_its_get_abi(its)->ite_esz;
 		gpa_t gpa = device->itt_addr + ite->event_id * ite_esz;
-		/*
-		 * Though the spec talks about removing the pending state, we
-		 * don't bother here since we clear the ITTE anyway and the
-		 * pending state is a property of the ITTE struct.
-		 */
+		struct vgic_irq *irq = ite->irq;
+		bool queued;
+
 		vgic_its_invalidate_cache(its);
 
+		/*
+		 * A queued LPI can be pending in a running vCPU's list register,
+		 * which the exit path folds back: halt the guest before clearing.
+		 */
+		scoped_guard(raw_spinlock_irqsave, &irq->irq_lock)
+			queued = irq->vcpu;
+
+		if (queued)
+			kvm_arm_halt_guest(kvm);
+
+		scoped_guard(raw_spinlock_irqsave, &irq->irq_lock)
+			irq->pending_latch = false;
+
+		if (queued)
+			kvm_arm_resume_guest(kvm);
+
 		its_free_ite(kvm, ite);
 
 		return vgic_its_write_entry_lock(its, gpa, 0ULL, ite);
-- 
2.39.5

Re: [PATCH v2 0/2] KVM: arm64: vgic-its: Fix LPI double mapping and DISCARD

[Oliver Upton]

On Tue, Oct 13, 2026 at 09:16:44AM +0100, Fuad Tabba wrote:
> Hi folks,
> 
> Changes since v1 [1]:
>   - Dropped the second mapping of an LPI at MAPTI, instead of keeping a
>     doubly mapped LPI out of GICv4 forwarding. (Marc)
>   - Keyed that check on whether an ITE maps the LPI, not on whether its
>     vgic_irq exists.
>   - Added a patch that makes DISCARD remove the pending state. (Marc)
> 
> Two fixes to the ITS emulation. The first stops a guest from mapping
> one LPI from two events, which can oops a GICv4 host when the VMM then
> routes an assigned device's MSI to the second event. The second makes
> DISCARD remove the LPI's pending state, as the spec requires.
> 
> Tested on QEMU, which emulates a GICv4 ITS. A local reproducer with two
> vCPUs DISCARDs an LPI that's pending in the other vCPU's list register.
> On rc3, and with patch 2 but without its halt, the guest still takes
> the LPI. With the series applied, it doesn't.
> 
> Based on Linux 7.3-rc3 (fd73f4a665989).
> 
> Cheers,
> /fuad
> 
> [1] https://lore.kernel.org/all/20261012084743.2228612-1-fuad.tabba@linux.dev/
> 
> Fuad Tabba (2):
>   KVM: arm64: vgic-its: Don't map an LPI from more than one event
>   KVM: arm64: vgic-its: Remove the pending state of a discarded LPI

Looks good. As all of these VGIC fixes depend on reliably synchronizing
hwstate I'll assume Marc will apply this patchset after his own series.

Reviewed-by: Oliver Upton <oupton@kernel.org>

Thanks,
Oliver

Re: [PATCH v2 0/2] KVM: arm64: vgic-its: Fix LPI double mapping and DISCARD

[Marc Zyngier]

On Tue, 13 Oct 2026 10:51:48 +0100,
Oliver Upton <oupton@kernel.org> wrote:
> 
> On Tue, Oct 13, 2026 at 09:16:44AM +0100, Fuad Tabba wrote:
> > Hi folks,
> > 
> > Changes since v1 [1]:
> >   - Dropped the second mapping of an LPI at MAPTI, instead of keeping a
> >     doubly mapped LPI out of GICv4 forwarding. (Marc)
> >   - Keyed that check on whether an ITE maps the LPI, not on whether its
> >     vgic_irq exists.
> >   - Added a patch that makes DISCARD remove the pending state. (Marc)
> > 
> > Two fixes to the ITS emulation. The first stops a guest from mapping
> > one LPI from two events, which can oops a GICv4 host when the VMM then
> > routes an assigned device's MSI to the second event. The second makes
> > DISCARD remove the LPI's pending state, as the spec requires.
> > 
> > Tested on QEMU, which emulates a GICv4 ITS. A local reproducer with two
> > vCPUs DISCARDs an LPI that's pending in the other vCPU's list register.
> > On rc3, and with patch 2 but without its halt, the guest still takes
> > the LPI. With the series applied, it doesn't.
> > 
> > Based on Linux 7.3-rc3 (fd73f4a665989).
> > 
> > Cheers,
> > /fuad
> > 
> > [1] https://lore.kernel.org/all/20261012084743.2228612-1-fuad.tabba@linux.dev/
> > 
> > Fuad Tabba (2):
> >   KVM: arm64: vgic-its: Don't map an LPI from more than one event
> >   KVM: arm64: vgic-its: Remove the pending state of a discarded LPI
> 
> Looks good. As all of these VGIC fixes depend on reliably synchronizing
> hwstate I'll assume Marc will apply this patchset after his own series.

Yeah, I now have a series that has the counting pause, your single
vcpu pause (fixed to DTRT), and the LPI disabling fix. Patch #2 of
this series can probably make use of the single-vcpu stunning.

As for the OUTSIDE_GUEST discussion, I've had a long discussion with
Will where we reversed engineered the synchronisation requirements,
and concluded that there was nothing to fix on that front (the logic
is incredibly subtle and counter-intuitive, but overall sound). This
patch is therefore dropped.

I'll post that today, and we can assemble a series.

Thanks,

	M.

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

Re: [PATCH v2 0/2] KVM: arm64: vgic-its: Fix LPI double mapping and DISCARD

[Fuad Tabba]

Hi Marc,

On Tue, 13 Oct 2026 11:01:44 +0100, Marc Zyngier <maz@kernel.org> wrote:
[...]
> Yeah, I now have a series that has the counting pause, your single
> vcpu pause (fixed to DTRT), and the LPI disabling fix. Patch #2 of
> this series can probably make use of the single-vcpu stunning.

I'll rebase the series on that once it's out, with patch 2 using the
single-vcpu stunning.

> As for the OUTSIDE_GUEST discussion, I've had a long discussion with
> Will where we reversed engineered the synchronisation requirements,
> and concluded that there was nothing to fix on that front (the logic
> is incredibly subtle and counter-intuitive, but overall sound). This
> patch is therefore dropped.

The first [High] issue Sashiko found on patch 2 [1] holds for DISCARD,
because the fold writes back the pending_latch that DISCARD clears.
With a 2ms delay after the exit to widen the window, and the DISCARD
sent at a varying time, the guest in my DISCARD reproducer takes the
discarded LPI in 13 of 20 runs on v2.

A vCPU on its way into the guest does the same: it loads its LRs in
kvm_vgic_flush_hwstate() while it's still OUTSIDE_GUEST_MODE, so
kvm_arm_halt_guest() doesn't wait for it. With the delay after the
flush instead, on v2 plus your 1/7 and 2/7 [2] and both changes from
your reply to 1/7 [3], the guest takes it in 9 of 20 runs.

Once DISCARD waits for irq->on_lr to clear before clearing
pending_latch, the guest takes it in none of 20 runs in either case.
I'll add that to v3, unless the single-vcpu stunning already waits for
it.

Cheers,
/fuad

[1] https://lore.kernel.org/r/sashiko-outbox-169278@kernel.org
[2] https://lore.kernel.org/r/20260929093548.3598547-1-maz@kernel.org
[3] https://lore.kernel.org/r/86a4p0403b.wl-maz@kernel.org

Re: [PATCH v2 0/2] KVM: arm64: vgic-its: Fix LPI double mapping and DISCARD

[Marc Zyngier]

On Tue, 13 Oct 2026 11:50:09 +0100,
Fuad Tabba <fuad.tabba@linux.dev> wrote:
> 
> Hi Marc,
> 
> On Tue, 13 Oct 2026 11:01:44 +0100, Marc Zyngier <maz@kernel.org> wrote:
> [...]
> > Yeah, I now have a series that has the counting pause, your single
> > vcpu pause (fixed to DTRT), and the LPI disabling fix. Patch #2 of
> > this series can probably make use of the single-vcpu stunning.
> 
> I'll rebase the series on that once it's out, with patch 2 using the
> single-vcpu stunning.
> 
> > As for the OUTSIDE_GUEST discussion, I've had a long discussion with
> > Will where we reversed engineered the synchronisation requirements,
> > and concluded that there was nothing to fix on that front (the logic
> > is incredibly subtle and counter-intuitive, but overall sound). This
> > patch is therefore dropped.
> 
> The first [High] issue Sashiko found on patch 2 [1] holds for DISCARD,
> because the fold writes back the pending_latch that DISCARD clears.
> With a 2ms delay after the exit to widen the window, and the DISCARD
> sent at a varying time, the guest in my DISCARD reproducer takes the
> discarded LPI in 13 of 20 runs on v2.
> 
> A vCPU on its way into the guest does the same: it loads its LRs in
> kvm_vgic_flush_hwstate() while it's still OUTSIDE_GUEST_MODE, so
> kvm_arm_halt_guest() doesn't wait for it. With the delay after the
> flush instead, on v2 plus your 1/7 and 2/7 [2] and both changes from
> your reply to 1/7 [3], the guest takes it in 9 of 20 runs.

Does this mean we still need OUTSIDE_GUEST_MODE to be a store-release
past the point where we resync the state?

> Once DISCARD waits for irq->on_lr to clear before clearing
> pending_latch, the guest takes it in none of 20 runs in either case.
> I'll add that to v3, unless the single-vcpu stunning already waits for
> it.

Polling for on_lr feels very dodgy, and I'd like to avoid this. If the
OUTSIDE_GUEST release addresses it, I'd rather do that.

Thanks,

	M.

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