[PATCH 0/3] KVM: arm64: vgic: Drop stale LPI pending state

[PATCH 0/3] KVM: arm64: vgic: Drop stale LPI pending state

[Fuad Tabba]

Hi folks,

The GIC just keeps on giving. Three follow-ups to my LPI fixes for
redistributors with LPIs disabled [1], to fix pre-existing bugs Sashiko
found.

The first drops the pending state of every LPI targeting a
redistributor when the guest disables its LPIs, not just of those on
the AP list, so a re-enable with GICR_PENDBASER.PTZ set doesn't bring
any of them back. Linux never sets PTZ.

The second stops the pending-table save from writing to the table of a
redistributor while its LPIs are disabled.

The third stops KVM reading the pending table when mapping an LPI, so
that mapping a discarded LPI's INTID again doesn't make it pending a
second time. A Linux guest can hit that after its VMM pauses and
resumes it. It also drops the copy that patch 2 of [1] takes, which is
why it comes after the second. An LPI mapped after its redistributor
enabled LPIs no longer takes a bit the guest wrote into the table
before the enable. Keeping that needs a per-redistributor record of
the pending state of unmapped INTIDs. I can add it on top if you think
it's worth it.

A pending LPI that MOVI moves onto a redistributor with LPIs disabled,
or that a device signals there through a translation cached before the
move, can still be taken once after a PTZ-set enable. The architecture
lets KVM drop it, but that's pending state moved or raised onto a
redistributor with LPIs disabled, not state the guest gave up. I've
left it alone, but can fix it on top if you think it's worth it.

Reproducers for all three fail on kvmarm/fixes and pass with the
series. I'll fold them into vgic_lpi_stress separately.

Based on kvmarm/fixes (181a701fc14d3), which has [1].

Cheers,
/fuad

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

Fuad Tabba (3):
  KVM: arm64: vgic: Drop the pending state of all LPIs when disabling
    LPIs
  KVM: arm64: vgic-v3: Don't save pending tables with LPIs disabled
  KVM: arm64: vgic-its: Don't read the pending table when mapping an LPI

 .../virt/kvm/devices/arm-vgic-v3.rst          |  1 +
 arch/arm64/kvm/vgic/vgic-its.c                | 33 ++++++++----
 arch/arm64/kvm/vgic/vgic-mmio-v3.c            | 13 ++---
 arch/arm64/kvm/vgic/vgic-v3.c                 | 50 ++-----------------
 arch/arm64/kvm/vgic/vgic.c                    | 16 +++++-
 arch/arm64/kvm/vgic/vgic.h                    |  1 -
 6 files changed, 49 insertions(+), 65 deletions(-)


base-commit: 181a701fc14d3d468c603b5ecfb9f12115c5fb72
-- 
2.47.3

[PATCH 1/3] KVM: arm64: vgic: Drop the pending state of all LPIs when disabling LPIs

[Fuad Tabba]

When the guest clears GICR_CTLR.EnableLPIs, vgic_flush_pending_lpis()
retires the LPIs on the vCPU's AP list. However, an LPI disabled in the
configuration table can be pending without being on that list. An LPI
moved to the vCPU by MOVI can also still be in another vCPU's LRs,
which pausing the vCPU doesn't fold back. These LPIs stay pending, and
the guest takes them even after re-enabling LPIs with
GICR_PENDBASER.PTZ set.

Halt the guest rather than the vCPU, so that no vCPU can fold such an
LPI back from its LRs after the flush. Have the flush clear the pending
state of every LPI targeting the vCPU. A forwarded vLPI's pending state
is in its vPE's pending table, which the flush doesn't clear.

Invalidate the translation cache before the flush. Only an MSI racing
with the disable can then make an LPI pending, or cache its translation
for later ones.

Fixes: 0e4e82f154e38 ("KVM: arm64: vgic-its: Enable ITS emulation as a virtual MSI controller")
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 arch/arm64/kvm/vgic/vgic-mmio-v3.c | 13 +++++++------
 arch/arm64/kvm/vgic/vgic.c         | 16 +++++++++++++++-
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index 02292f8e0b431..93024879ed3b3 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -298,14 +298,15 @@ static void vgic_mmio_write_v3r_ctlr(struct kvm_vcpu *vcpu,
 		/*
 		 * Yes, disabling LPIs is painful, since it can be done from
 		 * a *remote* vcpu! So let's not take any chance, and make
-		 * sure that the target vCPU has written their LRs back to the
+		 * sure that every vCPU has written their LRs back to the
 		 * irq structures, and release any reference they would have.
-		 * Pause it before its LPIs read as disabled, so that the exit's
-		 * prune leaves them for vgic_flush_pending_lpis() to retire.
+		 * Halt them before the LPIs read as disabled, so that none can
+		 * fold an LPI back from its LRs after vgic_flush_pending_lpis()
+		 * has cleared it.
 		 *
 		 * If it hurts, don't do it.
 		 */
-		kvm_pause_vcpu(vcpu);
+		kvm_arm_halt_guest(vcpu->kvm);
 
 		/*
 		 * Don't disable if RWP is set, as there already an
@@ -315,12 +316,12 @@ static void vgic_mmio_write_v3r_ctlr(struct kvm_vcpu *vcpu,
 					      GICR_CTLR_ENABLE_LPIS,
 					      GICR_CTLR_RWP);
 		if (ctlr == GICR_CTLR_ENABLE_LPIS) {
-			vgic_flush_pending_lpis(vcpu);
 			vgic_its_invalidate_all_caches(vcpu->kvm);
+			vgic_flush_pending_lpis(vcpu);
 			atomic_set_release(&vgic_cpu->ctlr, 0);
 		}
 
-		kvm_resume_vcpu(vcpu);
+		kvm_arm_resume_guest(vcpu->kvm);
 	} else {
 		ctlr = atomic_cmpxchg_acquire(&vgic_cpu->ctlr, 0,
 					      GICR_CTLR_ENABLE_LPIS);
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index fe09ada322625..a99570538f104 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -207,9 +207,10 @@ static void vgic_release_deleted_lpis(struct kvm *kvm)
 void vgic_flush_pending_lpis(struct kvm_vcpu *vcpu)
 {
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
 	struct vgic_irq *irq, *tmp;
+	unsigned long intid, flags;
 	bool deleted = false;
-	unsigned long flags;
 
 	raw_spin_lock_irqsave(&vgic_cpu->ap_list_lock, flags);
 
@@ -228,6 +229,19 @@ void vgic_flush_pending_lpis(struct kvm_vcpu *vcpu)
 
 	if (deleted)
 		vgic_release_deleted_lpis(vcpu->kvm);
+
+	/* Disabled LPIs and LPIs MOVI'd here can be pending off this AP list. */
+	xa_for_each(&dist->lpi_xa, intid, irq) {
+		irq = vgic_get_irq(vcpu->kvm, intid);
+		if (!irq)
+			continue;
+
+		raw_spin_lock_irqsave(&irq->irq_lock, flags);
+		if (irq->target_vcpu == vcpu)
+			irq->pending_latch = false;
+		raw_spin_unlock_irqrestore(&irq->irq_lock, flags);
+		vgic_put_irq(vcpu->kvm, irq);
+	}
 }
 
 void vgic_irq_set_phys_pending(struct vgic_irq *irq, bool pending)
-- 
2.47.3

[PATCH 2/3] KVM: arm64: vgic-v3: Don't save pending tables with LPIs disabled

[Fuad Tabba]

KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES writes each LPI's pending state
into its redistributor's pending table, clearing the bit of an LPI that
isn't pending. However, once a redistributor has LPIs disabled and RWP
reads 0, "there are no further accesses by the GIC to the LPI Pending
table" (IHI 0069H.b, 5.1.2) until enabling LPIs reads it. A bit the
guest sets there is cleared by the next save, which QEMU runs whenever
it stops the VM, and the guest never takes the LPI.

Skip the LPIs of a redistributor with LPIs disabled, and document it.
On GICv4.1 this drops the state of a forwarded vLPI there, which the
disable is allowed to lose: "There is no guarantee that clearing
GICR_CTLR.EnableLPIs causes the LPI Pending table to be updated in
memory" (5.1.2).

Fixes: 280771252c1ba ("KVM: arm64: vgic-v3: KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES")
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 Documentation/virt/kvm/devices/arm-vgic-v3.rst | 1 +
 arch/arm64/kvm/vgic/vgic-v3.c                  | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/virt/kvm/devices/arm-vgic-v3.rst b/Documentation/virt/kvm/devices/arm-vgic-v3.rst
index 5395ee66fc324..c878d3f728858 100644
--- a/Documentation/virt/kvm/devices/arm-vgic-v3.rst
+++ b/Documentation/virt/kvm/devices/arm-vgic-v3.rst
@@ -308,6 +308,7 @@ Groups:
       save all LPI pending bits into guest RAM pending tables.
 
       The first kB of the pending table is not altered by this operation.
+      Neither is the pending table of a redistributor with LPIs disabled.
 
   Errors:
 
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 65b77fb35be37..9bbf11ddba92e 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -625,7 +625,9 @@ int vgic_v3_save_pending_tables(struct kvm *kvm)
 			continue;
 
 		vcpu = irq->target_vcpu;
-		if (!vcpu)
+
+		/* While LPIs are disabled, the pending table is the guest's. */
+		if (!vcpu || !vgic_lpis_enabled(vcpu))
 			goto put_irq;
 
 		pendbase = GICR_PENDBASER_ADDRESS(vcpu->arch.vgic_cpu.pendbaser);
-- 
2.47.3

[PATCH 3/3] KVM: arm64: vgic-its: Don't read the pending table when mapping an LPI

[Fuad Tabba]

Mapping an LPI makes it pending if its bit in the target redistributor's
pending table is set. While the redistributor has LPIs enabled, KVM
doesn't keep that bit current. Saving the pending tables, as QEMU does
whenever it stops the VM, sets the bit of each pending LPI, and the bit
stays set once the guest has taken the LPI. Discarding the LPI and
mapping its INTID again makes it pending a second time.

MAPTI (IHI 0069H.b, 5.3.12) doesn't read the table, so stop reading it
when an LPI is created. Read it once the ITS tables are restored
instead, for each redistributor with LPIs enabled, as enabling LPIs
does (5.1.2). The documented restore order restores the redistributors
first.

The copy that commit b4c9f30194a4 ("KVM: arm64: vgic-v3: Don't drop a
pending LPI mapped with LPIs disabled") takes while LPIs are disabled
goes too, now that the save leaves that bit alone.

An LPI mapped after its redistributor enabled LPIs no longer picks up a
bit the table held at the enable. A GIC makes the LPI pending at the
enable, mapped or not (5.1.2). KVM has no pending state for an unmapped
INTID, and a bit the guest wrote is indistinguishable from one a save
left behind. Linux enables LPIs with a zeroed table.

Fixes: 06bd5359549d7 ("KVM: arm64: vgic-its: Read config and pending bit in add_lpi()")
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 arch/arm64/kvm/vgic/vgic-its.c | 33 ++++++++++++++++--------
 arch/arm64/kvm/vgic/vgic-v3.c  | 46 ----------------------------------
 arch/arm64/kvm/vgic/vgic.h     |  1 -
 3 files changed, 23 insertions(+), 57 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 5ce485485a117..ede205e05dc04 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -142,7 +142,7 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
 	 * However we only have those structs for mapped IRQs, so we read in
 	 * the respective config data from memory here upon mapping the LPI.
 	 *
-	 * Should any of these fail, behave as if we couldn't create the LPI
+	 * Should that fail, behave as if we couldn't create the LPI
 	 * by dropping the refcount and returning the error.
 	 */
 	ret = update_lpi_config(kvm, irq, NULL, false);
@@ -151,12 +151,6 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
 		return ERR_PTR(ret);
 	}
 
-	ret = vgic_v3_lpi_sync_pending_status(kvm, irq);
-	if (ret) {
-		vgic_put_irq(kvm, irq);
-		return ERR_PTR(ret);
-	}
-
 	return irq;
 }
 
@@ -2710,6 +2704,9 @@ static int vgic_its_save_tables_v0(struct vgic_its *its)
  */
 static int vgic_its_restore_tables_v0(struct vgic_its *its)
 {
+	struct kvm *kvm = its->dev->kvm;
+	struct kvm_vcpu *vcpu;
+	unsigned long i;
 	int ret;
 
 	ret = vgic_its_restore_collection_table(its);
@@ -2717,9 +2714,25 @@ static int vgic_its_restore_tables_v0(struct vgic_its *its)
 		return ret;
 
 	ret = vgic_its_restore_device_tables(its);
-	if (ret)
-		vgic_its_free_collection_list(its->dev->kvm, its);
-	return ret;
+	if (ret) {
+		vgic_its_free_collection_list(kvm, its);
+		return ret;
+	}
+
+	/* Enabling LPIs read the pending tables before these LPIs existed. */
+	kvm_for_each_vcpu(i, vcpu, kvm) {
+		if (!vgic_lpis_enabled(vcpu))
+			continue;
+
+		ret = its_sync_lpi_pending_table(vcpu);
+		if (ret) {
+			vgic_its_free_device_list(kvm, its);
+			vgic_its_free_collection_list(kvm, its);
+			return ret;
+		}
+	}
+
+	return 0;
 }
 
 static void vgic_its_commit_v0(struct vgic_its *its)
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 9bbf11ddba92e..35e45479dfa42 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -517,52 +517,6 @@ void vcpu_set_ich_hcr(struct kvm_vcpu *vcpu)
 				      ICH_HCR_EL2_TC);
 }
 
-int vgic_v3_lpi_sync_pending_status(struct kvm *kvm, struct vgic_irq *irq)
-{
-	struct kvm_vcpu *vcpu;
-	int byte_offset, bit_nr;
-	gpa_t pendbase, ptr;
-	bool status;
-	u8 val;
-	int ret;
-	unsigned long flags;
-
-retry:
-	vcpu = irq->target_vcpu;
-	if (!vcpu)
-		return 0;
-
-	pendbase = GICR_PENDBASER_ADDRESS(vcpu->arch.vgic_cpu.pendbaser);
-
-	byte_offset = irq->intid / BITS_PER_BYTE;
-	bit_nr = irq->intid % BITS_PER_BYTE;
-	ptr = pendbase + byte_offset;
-
-	ret = kvm_read_guest_lock(kvm, ptr, &val, 1);
-	if (ret)
-		return ret;
-
-	status = val & (1 << bit_nr);
-
-	raw_spin_lock_irqsave(&irq->irq_lock, flags);
-	if (irq->target_vcpu != vcpu) {
-		raw_spin_unlock_irqrestore(&irq->irq_lock, flags);
-		goto retry;
-	}
-	irq->pending_latch = status;
-	vgic_queue_irq_unlock(vcpu->kvm, irq, flags);
-
-	/* Enabling LPIs resets the latch from the table: leave the bit for it */
-	if (status && vgic_lpis_enabled(vcpu)) {
-		/* clear consumed data */
-		val &= ~(1 << bit_nr);
-		ret = vgic_write_guest_lock(kvm, ptr, &val, 1);
-		if (ret)
-			return ret;
-	}
-	return 0;
-}
-
 /*
  * The deactivation of the doorbell interrupt will trigger the
  * unmapping of the associated vPE.
diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
index 71c481ddd98d6..c4407f69f3613 100644
--- a/arch/arm64/kvm/vgic/vgic.h
+++ b/arch/arm64/kvm/vgic/vgic.h
@@ -347,7 +347,6 @@ void vgic_v3_reset(struct kvm_vcpu *vcpu);
 void vgic_v3_enable_cpuif_traps(void);
 int vgic_v3_probe(const struct gic_kvm_info *info);
 int vgic_v3_map_resources(struct kvm *kvm);
-int vgic_v3_lpi_sync_pending_status(struct kvm *kvm, struct vgic_irq *irq);
 int vgic_v3_save_pending_tables(struct kvm *kvm);
 int vgic_v3_set_redist_base(struct kvm *kvm, u32 index, u64 addr, u32 count);
 int vgic_register_redist_iodev(struct kvm_vcpu *vcpu);
-- 
2.47.3

Re: [PATCH 0/3] KVM: arm64: vgic: Drop stale LPI pending state

[Marc Zyngier]

On Thu, 05 Nov 2026 11:00:47 +0000,
Fuad Tabba <fuad.tabba@linux.dev> wrote:
> 
> Hi folks,
> 
> The GIC just keeps on giving. Three follow-ups to my LPI fixes for
> redistributors with LPIs disabled [1], to fix pre-existing bugs Sashiko
> found.

Can you *please* come up with a full series rather than this constant
"here's another set of patches" that address the same thing, one
pothole at a time?

I'm now really tempted to drop all the patches I have queued on this
so far, and wait for the "one true series".

	M.

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

Re: [PATCH 0/3] KVM: arm64: vgic: Drop stale LPI pending state

[Fuad Tabba]

On Thu, 5 Nov 2026 at 15:51, Marc Zyngier <maz@kernel.org> wrote:
>
> On Thu, 05 Nov 2026 11:00:47 +0000,
> Fuad Tabba <fuad.tabba@linux.dev> wrote:
> >
> > Hi folks,
> >
> > The GIC just keeps on giving. Three follow-ups to my LPI fixes for
> > redistributors with LPIs disabled [1], to fix pre-existing bugs Sashiko
> > found.
>
> Can you *please* come up with a full series rather than this constant
> "here's another set of patches" that address the same thing, one
> pothole at a time?
>
> I'm now really tempted to drop all the patches I have queued on this
> so far, and wait for the "one true series".

Sorry about that. Every time I think I'm done with it, Sashiko pokes more holes!

I'll sit on this a bit more, make sure all the gaps are closed,
Sashiko it locally, before I respin.

Cheers,
/fuad

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