[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 2/2] KVM: arm64: vgic-its: Remove the pending state of a discarded LPI

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] kvm: arm64: vgic-its: TOCTOU race resurrects discarded LPI due to IPI skip
- [High] kvm: arm64: vgic-its: Race on vcpu->arch.pause breaks mutual exclusion
- [Medium] kvm: arm64: vgic-its: Global VM halt on DISCARD causes Denial of Service
--

commit 4bb7971d12d2fa6f1083c9f2ddc383b8d63483fb
Author: Fuad Tabba <fuad.tabba@linux.dev>
KVM: arm64: vgic-its: Remove the pending state of a discarded LPI

This commit ensures the pending state of a queued LPI is cleared when handling
an ITS DISCARD command. It halts the guest before clearing the pending state if
the LPI is queued on a running vCPU.

> diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> --- a/arch/arm64/kvm/vgic/vgic-its.c
> +++ b/arch/arm64/kvm/vgic/vgic-its.c
> @@ -829,12 +829,26 @@ 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;
> +		struct vgic_irq *irq = ite->irq;
> +		bool queued;
> +
> +		vgic_its_invalidate_cache(its);
> +
>  		/*
> -		 * 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.
> +		 * A queued LPI can be pending in a running vCPU's list register,
> +		 * which the exit path folds back: halt the guest before clearing.
>  		 */
> -		vgic_its_invalidate_cache(its);
> +		scoped_guard(raw_spinlock_irqsave, &irq->irq_lock)
> +			queued = irq->vcpu;
> +
> +		if (queued)
> +			kvm_arm_halt_guest(kvm);

[Severity: Medium]
Does this introduce a denial of service if a guest spams DISCARD commands?

Because kvm_arm_halt_guest() forces all vCPUs in the VM to exit, a guest
rapidly issuing DISCARD commands for a single vCPU's LPI could repeatedly
lock up the entire VM and cause host starvation. Could we restrict the
synchronization strictly to the target irq->vcpu where the LPI is queued?

> +
> +		scoped_guard(raw_spinlock_irqsave, &irq->irq_lock)
> +			irq->pending_latch = false;

[Severity: High]
Is there a window here where an exiting vCPU might overwrite the cleared
latch and resurrect the LPI?

If the target vCPU is just exiting when kvm_arm_halt_guest() broadcasts
KVM_REQ_SLEEP, the vCPU sets vcpu->mode = OUTSIDE_GUEST_MODE before folding
its hardware state in kvm_arch_vcpu_ioctl_run().

Because kvm_request_needs_ipi() checks:

virt/kvm/kvm_main.c:kvm_request_needs_ipi() {
    if (req & KVM_REQUEST_WAIT)
        return mode != OUTSIDE_GUEST_MODE;
}

The IPI is skipped and kvm_arm_halt_guest() returns immediately without
waiting for the vCPU to sync. The DISCARD thread then clears
irq->pending_latch = false.

Concurrently, the exiting vCPU executes kvm_vgic_sync_hwstate(), reads the
still-pending state from the hardware LR, and unconditionally overwrites the
cleared latch back to true:

arch/arm64/kvm/vgic/vgic-v3.c:vgic_v3_fold_lr() {
    if (irq->config == VGIC_CONFIG_EDGE && (val & ICH_LR_PENDING_BIT))
        irq->pending_latch = true;
}

Could we coordinate with the vCPU's state folding sequence to ensure the
discard isn't lost?

> +
> +		if (queued)
> +			kvm_arm_resume_guest(kvm);

[Severity: High]
Can concurrent DISCARD commands on different ITS instances break the mutual
exclusion of vcpu->arch.pause?

vgic_its_cmd_handle_discard() executes holding the per-ITS its->its_lock.
If a VM has multiple ITS devices, two DISCARD commands could run concurrently
and both call kvm_arm_halt_guest(), which sets vcpu->arch.pause to true
across all vCPUs.

The first thread to finish will call kvm_arm_resume_guest(), which does:

arch/arm64/kvm/arm.c:kvm_arm_resume_guest() {
    kvm_for_each_vcpu(i, vcpu, kvm) {
        vcpu->arch.pause = false;
        __kvm_vcpu_wake_up(vcpu);
    }
}

This unconditionally clears the pause flag and wakes up all vCPUs while the
second thread is still modifying VGIC state assuming the guest is fully
halted.

>  
>  		its_free_ite(kvm, ite);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013081646.933312-1-fuad.tabba@linux.dev?part=2

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

[Oliver Upton]

On Tue, Oct 13, 2026 at 08:50:51AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] kvm: arm64: vgic-its: TOCTOU race resurrects discarded LPI due to IPI skip
> - [High] kvm: arm64: vgic-its: Race on vcpu->arch.pause breaks mutual exclusion
> - [Medium] kvm: arm64: vgic-its: Global VM halt on DISCARD causes Denial of Service
> --
> 
> commit 4bb7971d12d2fa6f1083c9f2ddc383b8d63483fb
> Author: Fuad Tabba <fuad.tabba@linux.dev>
> KVM: arm64: vgic-its: Remove the pending state of a discarded LPI
> 
> This commit ensures the pending state of a queued LPI is cleared when handling
> an ITS DISCARD command. It halts the guest before clearing the pending state if
> the LPI is queued on a running vCPU.
> 
> > diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> > --- a/arch/arm64/kvm/vgic/vgic-its.c
> > +++ b/arch/arm64/kvm/vgic/vgic-its.c
> > @@ -829,12 +829,26 @@ 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;
> > +		struct vgic_irq *irq = ite->irq;
> > +		bool queued;
> > +
> > +		vgic_its_invalidate_cache(its);
> > +
> >  		/*
> > -		 * 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.
> > +		 * A queued LPI can be pending in a running vCPU's list register,
> > +		 * which the exit path folds back: halt the guest before clearing.
> >  		 */
> > -		vgic_its_invalidate_cache(its);
> > +		scoped_guard(raw_spinlock_irqsave, &irq->irq_lock)
> > +			queued = irq->vcpu;
> > +
> > +		if (queued)
> > +			kvm_arm_halt_guest(kvm);
> 
> [Severity: Medium]
> Does this introduce a denial of service if a guest spams DISCARD commands?
> 
> Because kvm_arm_halt_guest() forces all vCPUs in the VM to exit, a guest
> rapidly issuing DISCARD commands for a single vCPU's LPI could repeatedly
> lock up the entire VM and cause host starvation. Could we restrict the
> synchronization strictly to the target irq->vcpu where the LPI is queued?

A guest has chosen to shoot itself in the foot, why should we care? The
whole point of pausing the VM is to flush state and block the vCPU
threads...

> > +
> > +		scoped_guard(raw_spinlock_irqsave, &irq->irq_lock)
> > +			irq->pending_latch = false;
> 
> [Severity: High]
> Is there a window here where an exiting vCPU might overwrite the cleared
> latch and resurrect the LPI?
> 
> If the target vCPU is just exiting when kvm_arm_halt_guest() broadcasts
> KVM_REQ_SLEEP, the vCPU sets vcpu->mode = OUTSIDE_GUEST_MODE before folding
> its hardware state in kvm_arch_vcpu_ioctl_run().
> 
> Because kvm_request_needs_ipi() checks:
> 
> virt/kvm/kvm_main.c:kvm_request_needs_ipi() {
>     if (req & KVM_REQUEST_WAIT)
>         return mode != OUTSIDE_GUEST_MODE;
> }
> 
> The IPI is skipped and kvm_arm_halt_guest() returns immediately without
> waiting for the vCPU to sync. The DISCARD thread then clears
> irq->pending_latch = false.
> 
> Concurrently, the exiting vCPU executes kvm_vgic_sync_hwstate(), reads the
> still-pending state from the hardware LR, and unconditionally overwrites the
> cleared latch back to true:
> 
> arch/arm64/kvm/vgic/vgic-v3.c:vgic_v3_fold_lr() {
>     if (irq->config == VGIC_CONFIG_EDGE && (val & ICH_LR_PENDING_BIT))
>         irq->pending_latch = true;
> }
> 
> Could we coordinate with the vCPU's state folding sequence to ensure the
> discard isn't lost?

https://lore.kernel.org/kvmarm/20260929093548.3598547-1-maz@kernel.org/

> > +
> > +		if (queued)
> > +			kvm_arm_resume_guest(kvm);
> 
> [Severity: High]
> Can concurrent DISCARD commands on different ITS instances break the mutual
> exclusion of vcpu->arch.pause?
> 
> vgic_its_cmd_handle_discard() executes holding the per-ITS its->its_lock.
> If a VM has multiple ITS devices, two DISCARD commands could run concurrently
> and both call kvm_arm_halt_guest(), which sets vcpu->arch.pause to true
> across all vCPUs.
> 
> The first thread to finish will call kvm_arm_resume_guest(), which does:
> 
> arch/arm64/kvm/arm.c:kvm_arm_resume_guest() {
>     kvm_for_each_vcpu(i, vcpu, kvm) {
>         vcpu->arch.pause = false;
>         __kvm_vcpu_wake_up(vcpu);
>     }
> }
> 
> This unconditionally clears the pause flag and wakes up all vCPUs while the
> second thread is still modifying VGIC state assuming the guest is fully
> halted.
> 

https://lore.kernel.org/kvmarm/20260929093548.3598547-1-maz@kernel.org/

Thanks,
Oliver

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.

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

[Fuad Tabba]

Hi Marc,

On Tue, 13 Oct 2026 12:16:09 +0100, Marc Zyngier <maz@kernel.org> wrote:
[...]
> > 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?

Not for the 9 of 20: that run already had the release. The halt reads
the OUTSIDE_GUEST_MODE the vCPU published at its previous exit, so it
doesn't wait for the early exit that folds the LPI back.

We do still need it for the window after the exit: with the delay
there and the same patches applied, the guest takes it in none of 20
runs, against 13 of 20 on v2.

> > 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.

I'd rather avoid the polling too, but the release alone doesn't close
this one. The vCPU would also need to store IN_GUEST_MODE and check
its requests before kvm_vgic_flush_hwstate(), so that it never loads
its LRs while it's OUTSIDE_GUEST_MODE. The existing check after the
flush would still be needed, for the KVM_REQ_GUEST_HYP_IRQ_PENDING
request kvm_vgic_flush_hwstate() makes in a nested state. With that
and the release, the guest takes it in none of 20 runs with the delay
after the flush, and herd7 finds no LKMM execution that leaves it
pending.

The IN_GUEST_MODE change would also cover vgic_mmio_change_active(),
which depends on the same halt (the comment above
vgic_access_active_prepare()). Would you be OK folding it into your
series, on top of your OUTSIDE_GUEST_MODE changes? Patch 2 then
wouldn't need the on_lr wait.

Cheers,
/fuad

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

[Marc Zyngier]

On Tue, 13 Oct 2026 13:59:22 +0100,
Fuad Tabba <fuad.tabba@linux.dev> wrote:
> 
> Hi Marc,
> 
> On Tue, 13 Oct 2026 12:16:09 +0100, Marc Zyngier <maz@kernel.org> wrote:
> [...]
> > > 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?
> 
> Not for the 9 of 20: that run already had the release. The halt reads
> the OUTSIDE_GUEST_MODE the vCPU published at its previous exit, so it
> doesn't wait for the early exit that folds the LPI back.
> 
> We do still need it for the window after the exit: with the delay
> there and the same patches applied, the guest takes it in none of 20
> runs, against 13 of 20 on v2.

I don't understand what these probabilities represent. Either it works
or it doesn't. And given that I have no idea what you are testing
with, I can't really make head or tail of the above.

> 
> > > 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.
> 
> I'd rather avoid the polling too, but the release alone doesn't close
> this one. The vCPU would also need to store IN_GUEST_MODE and check
> its requests before kvm_vgic_flush_hwstate(), so that it never loads
> its LRs while it's OUTSIDE_GUEST_MODE. The existing check after the
> flush would still be needed, for the KVM_REQ_GUEST_HYP_IRQ_PENDING
> request kvm_vgic_flush_hwstate() makes in a nested state. With that
> and the release, the guest takes it in none of 20 runs with the delay
> after the flush, and herd7 finds no LKMM execution that leaves it
> pending.

If you have a herd litmus test, how about posting it? It would really
avoid a lot of that back and forth where the S/N ratio is pretty poor.

> The IN_GUEST_MODE change would also cover vgic_mmio_change_active(),
> which depends on the same halt (the comment above
> vgic_access_active_prepare()). Would you be OK folding it into your
> series, on top of your OUTSIDE_GUEST_MODE changes? Patch 2 then
> wouldn't need the on_lr wait.

Is the hack below what you are asking for? I can't tell. If it isn't,
please post your own version.

Thanks,

	M.

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 0576c2022ef57..f1449b08531eb 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1321,10 +1321,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 
 		local_irq_disable();
 
-		kvm_vgic_flush_hwstate(vcpu);
-
-		kvm_pmu_update_vcpu_events(vcpu);
-
 		/*
 		 * Ensure we set mode to IN_GUEST_MODE after we disable
 		 * interrupts and before the final VCPU requests check.
@@ -1333,14 +1329,18 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 		 */
 		smp_store_mb(vcpu->mode, IN_GUEST_MODE);
 
+		kvm_vgic_flush_hwstate(vcpu);
+
+		kvm_pmu_update_vcpu_events(vcpu);
+
 		if (ret <= 0 || kvm_vcpu_exit_request(vcpu, &ret)) {
-			vcpu->mode = OUTSIDE_GUEST_MODE;
 			isb(); /* Ensure work in x_flush_hwstate is committed */
 			if (kvm_vcpu_has_pmu(vcpu))
 				kvm_pmu_sync_hwstate(vcpu);
 			if (unlikely(!irqchip_in_kernel(vcpu->kvm)))
 				kvm_timer_sync_user(vcpu);
 			kvm_vgic_sync_hwstate(vcpu);
+			smp_store_release(&vcpu->mode, OUTSIDE_GUEST_MODE);
 			local_irq_enable();
 			preempt_enable();
 			continue;
@@ -1356,7 +1356,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 
 		ret = kvm_arm_vcpu_enter_exit(vcpu);
 
-		vcpu->mode = OUTSIDE_GUEST_MODE;
 		vcpu->stat.exits++;
 		/*
 		 * Back from guest
@@ -1390,6 +1389,12 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 
 		kvm_arch_vcpu_ctxsync_fp(vcpu);
 
+		/*
+		 * All the state has been synchronised, let advertise
+		 * we're outside of the guest.
+		 */
+		smp_store_release(&vcpu->mode, OUTSIDE_GUEST_MODE);
+
 		/*
 		 * We must ensure that any pending interrupts are taken before
 		 * we exit guest timing so that timer ticks are accounted as

-- 
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 14:25:09 +0100, Marc Zyngier <maz@kernel.org> wrote:
[...]
> > We do still need it for the window after the exit: with the delay
> > there and the same patches applied, the guest takes it in none of 20
> > runs, against 13 of 20 on v2.
>
> I don't understand what these probabilities represent. Either it works
> or it doesn't. And given that I have no idea what you are testing
> with, I can't really make head or tail of the above.

They're the number of runs, out of 20, in which the guest took the
discarded LPI. My reproducer has vCPU 0 issue DISCARD for an LPI pending
in vCPU 1's LR, at a varying time, with a debug mdelay() widening the
window, so only some runs hit it. I've since replaced the delay with
debug hooks that force the race, so now it either works or it doesn't.

[...]
> If you have a herd litmus test, how about posting it? It would really
> avoid a lot of that back and forth where the S/N ratio is pretty poor.

Sure, here are two, run with herd7 7.58 and the LKMM from v7.3-rc3. The
first models your diff and comes out Sometimes. The second adds the
request check before the flush and comes out Never.

C discard+halt

(*
 * P0 is the vCPU in kvm_arch_vcpu_ioctl_run() with your diff, with a
 * pending LPI queued on it: check_vcpu_requests(), the IN_GUEST_MODE
 * store, the flush moving pending_latch into an LR, the fold (on the
 * early exit, or after a guest run that leaves the LPI pending), and
 * the release of OUTSIDE_GUEST_MODE.
 *
 * P1 is DISCARD: kvm_arm_halt_guest() making KVM_REQ_SLEEP and reading
 * the mode in kvm_vcpu_exiting_guest_mode(), then clearing
 * pending_latch. A halt that reads IN or EXITING (1, 2) waits for the
 * IPI, which the vCPU takes after its fold, so only one that reads
 * OUTSIDE (0) clears here.
 *)

{
	pending_latch = 1;
}

P0(int *mode, int *requests, int *pending_latch, spinlock_t *irq_lock)
{
	int r0;
	int r1 = 0;

	r0 = READ_ONCE(*requests);
	if (r0 == 0) {
		smp_store_mb(*mode, 1);

		spin_lock(irq_lock);
		r1 = READ_ONCE(*pending_latch);
		if (r1 == 1)
			WRITE_ONCE(*pending_latch, 0);
		spin_unlock(irq_lock);

		spin_lock(irq_lock);
		if (r1 == 1)
			WRITE_ONCE(*pending_latch, 1);
		spin_unlock(irq_lock);

		smp_store_release(mode, 0);
	}
}

P1(int *mode, int *requests, int *pending_latch, spinlock_t *irq_lock)
{
	int r3;

	smp_wmb();
	WRITE_ONCE(*requests, 1);
	smp_mb__before_atomic();
	r3 = cmpxchg(mode, 1, 2);
	if (r3 == 0) {
		spin_lock(irq_lock);
		WRITE_ONCE(*pending_latch, 0);
		spin_unlock(irq_lock);
	}
}

exists (0:r1=1 /\ 1:r3=0 /\ pending_latch=1)

C discard+halt+check

(*
 * As discard+halt, with the request check between the IN_GUEST_MODE
 * store and the flush.
 *)

{
	pending_latch = 1;
}

P0(int *mode, int *requests, int *pending_latch, spinlock_t *irq_lock)
{
	int r0;
	int r1 = 0;
	int r2;

	r0 = READ_ONCE(*requests);
	if (r0 == 0) {
		smp_store_mb(*mode, 1);

		r2 = READ_ONCE(*requests);
		if (r2 == 0) {
			spin_lock(irq_lock);
			r1 = READ_ONCE(*pending_latch);
			if (r1 == 1)
				WRITE_ONCE(*pending_latch, 0);
			spin_unlock(irq_lock);

			spin_lock(irq_lock);
			if (r1 == 1)
				WRITE_ONCE(*pending_latch, 1);
			spin_unlock(irq_lock);
		}

		smp_store_release(mode, 0);
	}
}

P1(int *mode, int *requests, int *pending_latch, spinlock_t *irq_lock)
{
	int r3;

	smp_wmb();
	WRITE_ONCE(*requests, 1);
	smp_mb__before_atomic();
	r3 = cmpxchg(mode, 1, 2);
	if (r3 == 0) {
		spin_lock(irq_lock);
		WRITE_ONCE(*pending_latch, 0);
		spin_unlock(irq_lock);
	}
}

exists (0:r1=1 /\ 1:r3=0 /\ pending_latch=1)

[...]
> Is the hack below what you are asking for? I can't tell. If it isn't,
> please post your own version.

Almost: it's missing a request check between the IN_GUEST_MODE store and
the flush. Without it, a halt that reads OUTSIDE_GUEST_MODE just before
the store doesn't wait for the vCPU, which then loads the LPI into an
LR, takes the early exit on KVM_REQ_SLEEP and folds the LPI back after
DISCARD has cleared it. That's the outcome in the first litmus test's
exists clause. With the debug hooks forcing it on QEMU, the guest takes
the discarded LPI in every run with your diff, and in none with this on
top:

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index f1449b0..896bc76 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1329,11 +1329,29 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 		 */
 		smp_store_mb(vcpu->mode, IN_GUEST_MODE);
 
+		/*
+		 * Don't load the LRs with a request pending:
+		 * kvm_arm_halt_guest() doesn't wait for a vCPU it reads
+		 * as OUTSIDE_GUEST_MODE.
+		 */
+		if (ret <= 0 || kvm_vcpu_exit_request(vcpu, &ret)) {
+			isb(); /* Ensure work in x_flush_hwstate is committed */
+			if (kvm_vcpu_has_pmu(vcpu))
+				kvm_pmu_sync_hwstate(vcpu);
+			if (unlikely(!irqchip_in_kernel(vcpu->kvm)))
+				kvm_timer_sync_user(vcpu);
+			smp_store_release(&vcpu->mode, OUTSIDE_GUEST_MODE);
+			local_irq_enable();
+			preempt_enable();
+			continue;
+		}
+
 		kvm_vgic_flush_hwstate(vcpu);
 
 		kvm_pmu_update_vcpu_events(vcpu);
 
-		if (ret <= 0 || kvm_vcpu_exit_request(vcpu, &ret)) {
+		/* kvm_vgic_flush_hwstate() can make a request when nested */
+		if (kvm_vcpu_exit_request(vcpu, &ret)) {
 			isb(); /* Ensure work in x_flush_hwstate is committed */
 			if (kvm_vcpu_has_pmu(vcpu))
 				kvm_pmu_sync_hwstate(vcpu);

Cheers,
/fuad