[PATCH 0/4] KVM: arm64: vgic: Stop migrating IRQ from vgic_prune_ap_list()

[PATCH 0/4] KVM: arm64: vgic: Stop migrating IRQ from vgic_prune_ap_list()

[Oliver Upton]

The VGIC has been getting a lot of undesired attention lately, as
there's plenty of weird and wonderful ways that the guest can play games
with KVM. My general impression on the whole matter is that modifying AP
lists remotely is a giant mess, with the worst offender being
vgic_prune_ap_list().

This series aims to sidestep the issue by only recalling IRQs from an AP
list if the vCPU has been paused. Keeping the overheads reasonable also
means adding support for pausing a single vCPU instead of the entire
VM, so update the eligible users of kvm_arm_halt_guest() to something
more relaxed while we're at it.

Marc, this series applies on top of your own for LPI malice [1]. You'll
note that I've taken some of it for 7.3 while leaving the rest for you
to take in 7.4, I'm perfectly happy with Patch 3 and 4 getting squashed
into the corresponding bits of your own series if you prefer.

Tested semi-rigorously on an Altra machine and the Orion board, using
both real guests and an LLM-generated selftest specific for migrating
pending LPIs. The latter was way too big to make sense of, I'll try and
post a test if I can it into something tidy.

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

Oliver Upton (4):
  KVM: arm64: Add helpers to halt a vCPU
  KVM: arm64: vgic: Move IRQ migrations out of vgic_prune_ap_list()
  KVM: arm64: vgic-v3: Only pause the targeted vCPU when disabling LPIs
  KVM: arm64: vgic-v3: Pause the source vCPU when processing MOVALL cmd

 arch/arm64/include/asm/kvm_host.h  |   2 +
 arch/arm64/kvm/arm.c               |  18 ++-
 arch/arm64/kvm/vgic/vgic-its.c     |  19 ++-
 arch/arm64/kvm/vgic/vgic-mmio-v2.c |  11 +-
 arch/arm64/kvm/vgic/vgic-mmio-v3.c |  18 ++-
 arch/arm64/kvm/vgic/vgic.c         | 184 +++++++++++++++++------------
 arch/arm64/kvm/vgic/vgic.h         |  24 ++++
 include/kvm/arm_vgic.h             |   1 +
 8 files changed, 164 insertions(+), 113 deletions(-)


base-commit: cfaf3b76669d386912a91103671691692f81c316
-- 
2.47.3

[PATCH 1/4] KVM: arm64: Add helpers to halt a vCPU

[Oliver Upton]

Currently we pause the entire VM when modifying VGIC state that requires
state to be synchronized back from hardware. This is perfectly fine for
heavyweight operations but not ideal for 'simple' things like migrating
an active/pending IRQ.

Add helpers for halting a single vCPU such that they can be used to
manipulate the AP list remotely.

Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/include/asm/kvm_host.h |  2 ++
 arch/arm64/kvm/arm.c              | 18 ++++++++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 66ea2372f987..48f536968a7d 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -1254,6 +1254,8 @@ int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
 
 void kvm_arm_halt_guest(struct kvm *kvm);
 void kvm_arm_resume_guest(struct kvm *kvm);
+void kvm_pause_vcpu(struct kvm_vcpu *vcpu);
+void kvm_resume_vcpu(struct kvm_vcpu *vcpu);
 
 #define vcpu_has_run_once(vcpu)	(!!READ_ONCE((vcpu)->pid))
 
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index bbcaca8b36fb..9cf9b11a6457 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1023,15 +1023,25 @@ void kvm_arm_halt_guest(struct kvm *kvm)
 	kvm_make_all_cpus_request(kvm, KVM_REQ_SLEEP);
 }
 
+void kvm_pause_vcpu(struct kvm_vcpu *vcpu)
+{
+	atomic_inc(&vcpu->arch.pause);
+	kvm_make_request_and_kick(KVM_REQ_SLEEP, vcpu);
+}
+
+void kvm_resume_vcpu(struct kvm_vcpu *vcpu)
+{
+	if (atomic_dec_and_test(&vcpu->arch.pause))
+		__kvm_vcpu_wake_up(vcpu);
+}
+
 void kvm_arm_resume_guest(struct kvm *kvm)
 {
 	unsigned long i;
 	struct kvm_vcpu *vcpu;
 
-	kvm_for_each_vcpu(i, vcpu, kvm) {
-		if (atomic_dec_and_test(&vcpu->arch.pause))
-			__kvm_vcpu_wake_up(vcpu);
-	}
+	kvm_for_each_vcpu(i, vcpu, kvm)
+		kvm_resume_vcpu(vcpu);
 }
 
 static void kvm_vcpu_sleep(struct kvm_vcpu *vcpu)
-- 
2.47.3

[PATCH 2/4] KVM: arm64: vgic: Move IRQ migrations out of vgic_prune_ap_list()

[Oliver Upton]

When an IRQ is migrated between vCPUs, the routing information is updated
synchronously whereas the literal transfer of an active/pending IRQ is
deferred to the next vCPU exit. At least in the case of a MOVI command
this violates the architecture as after a subsequent SYNC command the
effects of the MOVI are said to be globally-visible, including the
transfer of pending state to the new redistributor.

In KVM it is possible to then take an IRQ on the old vCPU after the
migration command retires as there could be a stale LR on the CPU with
pending state. Additionally, it is vulnerable to guest-induced lock
contention as vgic_prune_ap_list() potentially takes *both* vCPU's
ap_list_locks. And yes, this happens before we have the opportunity to
even re-enable IRQs on the CPU.

Align our emulation with the architecture and ensure that active/pending
IRQs are actually on the correct destination vCPU synchronously with the
migration itself. Eliminate the need to nest ap_list_locks in so doing
as the implementation now only needs to inspect a single vCPU's AP list
at a time to detach the IRQ and later requeue it.

Just directly update the IRQ's target if the IRQ is inactive, as there's
no AP list to go modify. Otherwise, halt the owning vCPU and re-attempt
the migration, this time removing it from the AP list and re-queueing it
with vgic_queue_irq_unlock().

Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/vgic/vgic-its.c     |   9 +-
 arch/arm64/kvm/vgic/vgic-mmio-v2.c |  11 +-
 arch/arm64/kvm/vgic/vgic-mmio-v3.c |  10 +-
 arch/arm64/kvm/vgic/vgic.c         | 184 +++++++++++++++++------------
 arch/arm64/kvm/vgic/vgic.h         |  24 ++++
 include/kvm/arm_vgic.h             |   1 +
 6 files changed, 139 insertions(+), 100 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 094eede267e5..585df69d1ffe 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -325,13 +325,10 @@ static int update_affinity(struct vgic_irq *irq,
 	struct its_vlpi_map map;
 	int ret;
 
+	/* Must be called outside of @irq->irq_lock */
+	vgic_update_lpi_affinity(vcpu->kvm, irq, from_vcpu, vcpu);
 	guard(raw_spinlock_irqsave)(&irq->irq_lock);
-	if (from_vcpu && irq->target_vcpu != from_vcpu)
-		return 0;
-
-	irq->target_vcpu = vcpu;
-
-	if (!irq->hw)
+	if (!(irq->target_vcpu == vcpu && irq->hw))
 		return 0;
 
 	ret = its_get_vlpi(irq->host_irq, &map);
diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v2.c b/arch/arm64/kvm/vgic/vgic-mmio-v2.c
index 0643e333db35..ac789c372b7f 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v2.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v2.c
@@ -184,7 +184,6 @@ static void vgic_mmio_write_target(struct kvm_vcpu *vcpu,
 	u32 intid = VGIC_ADDR_TO_INTID(addr, 8);
 	u8 cpu_mask = GENMASK(atomic_read(&vcpu->kvm->online_vcpus) - 1, 0);
 	int i;
-	unsigned long flags;
 
 	/* GICD_ITARGETSR[0-7] are read-only */
 	if (intid < VGIC_NR_PRIVATE_IRQS)
@@ -192,15 +191,9 @@ static void vgic_mmio_write_target(struct kvm_vcpu *vcpu,
 
 	for (i = 0; i < len; i++) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, intid + i);
-		int target;
-
-		raw_spin_lock_irqsave(&irq->irq_lock, flags);
+		int targets = (val >> (i * 8)) & cpu_mask;
 
-		irq->targets = (val >> (i * 8)) & cpu_mask;
-		target = irq->targets ? __ffs(irq->targets) : 0;
-		irq->target_vcpu = kvm_get_vcpu(vcpu->kvm, target);
-
-		raw_spin_unlock_irqrestore(&irq->irq_lock, flags);
+		vgic_v2_update_spi_affinity(vcpu->kvm, irq, targets);
 		vgic_put_irq(vcpu->kvm, irq);
 	}
 }
diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index adefc4296473..e5fd7a0002b5 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -241,7 +241,7 @@ static void vgic_mmio_write_irouter(struct kvm_vcpu *vcpu,
 {
 	int intid = VGIC_ADDR_TO_INTID(addr, 64);
 	struct vgic_irq *irq;
-	unsigned long flags;
+	u64 mpidr;
 
 	/* The upper word is WI for us since we don't implement Aff3. */
 	if (addr & 4)
@@ -252,13 +252,9 @@ static void vgic_mmio_write_irouter(struct kvm_vcpu *vcpu,
 	if (!irq)
 		return;
 
-	raw_spin_lock_irqsave(&irq->irq_lock, flags);
-
 	/* We only care about and preserve Aff0, Aff1 and Aff2. */
-	irq->mpidr = val & GENMASK(23, 0);
-	irq->target_vcpu = kvm_mpidr_to_vcpu(vcpu->kvm, irq->mpidr);
-
-	raw_spin_unlock_irqrestore(&irq->irq_lock, flags);
+	mpidr = val & GENMASK(23, 0);
+	vgic_v3_update_spi_affinity(vcpu->kvm, irq, mpidr);
 	vgic_put_irq(vcpu->kvm, irq);
 }
 
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 5cf5a1ef86cd..e82889033691 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -739,6 +739,98 @@ int kvm_vgic_set_owner(struct kvm_vcpu *vcpu, unsigned int intid, void *owner)
 	return ret;
 }
 
+/*
+ * Updates the affinity of the specified IRQ to the new vCPU (which may be NULL)
+ * and handles the tricky business of moving the IRQ between AP lists if
+ * necessary.
+ */
+void __vgic_update_irq_affinity(struct kvm *kvm, struct vgic_irq *irq,
+				struct kvm_vcpu *old, struct kvm_vcpu *new,
+				u32 data)
+{
+	struct kvm_vcpu *tmp;
+	unsigned long flags;
+	bool pruned = false;
+
+retry:
+	scoped_guard(raw_spinlock_irqsave, &irq->irq_lock) {
+		/*
+		 * We could be filtering migrations by source vCPU (e.g. ITS
+		 * MOVALL command). Bail if the IRQ is affined somewhere we
+		 * don't expect.
+		 */
+		if (old && irq->target_vcpu != old)
+			return;
+
+		/*
+		 * The IRQ we're trying to migrate isn't on any AP list, meaning
+		 * all we need to do is update the target and bail.
+		 */
+		if (!irq->vcpu) {
+			irq->target_vcpu = new;
+
+			/*
+			 * Cheap trick to keep this somewhat generic: demux data
+			 * to the correct field based on whether or not we're
+			 * dealing with GICv2.
+			 */
+			if (vgic_is_v2(kvm))
+				irq->targets = data;
+			else
+				irq->mpidr = data;
+			return;
+		}
+
+		/*
+		 * If not, the IRQ exists on another vCPU. Choose our target
+		 * and set phasers to stun: we need the old vCPU to synchronize
+		 * the LR state back into the AP list. Prepare to drop the
+		 * irq_lock so we may reacquire it behind the ap_list_lock.
+		 */
+		tmp = irq->vcpu;
+	}
+
+	/* Fire! */
+	kvm_pause_vcpu(tmp);
+
+	scoped_guard(raw_spinlock_irqsave, &tmp->arch.vgic_cpu.ap_list_lock) {
+		scoped_guard(raw_spinlock, &irq->irq_lock) {
+			/*
+			 * The IRQ could've been moved to another AP list after
+			 * dropping the irq_lock. Make sure it's where we expect
+			 * it to be, remove from the list and retain the implied
+			 * reference until we queue it on the new vCPU.
+			 */
+			if (irq->vcpu == tmp) {
+				list_del(&irq->ap_list);
+				irq->vcpu = NULL;
+				irq->target_vcpu = new;
+				if (vgic_is_v2(kvm))
+					irq->targets = data;
+				else
+					irq->mpidr = data;
+				pruned = true;
+			}
+		}
+	}
+
+	/*
+	 * Release the stunned vCPU and retry the whole operation if we weren't
+	 * able to move the IRQ off of the old AP list.
+	 */
+	kvm_resume_vcpu(tmp);
+	if (!pruned)
+		goto retry;
+
+	/*
+	 * Finally, queue the IRQ onto the destination AP list (if there is one)
+	 * and drop the retained reference.
+	 */
+	raw_spin_lock_irqsave(&irq->irq_lock, flags);
+	vgic_queue_irq_unlock(kvm, irq, flags);
+	vgic_put_irq(kvm, irq);
+}
+
 /**
  * vgic_prune_ap_list - Remove non-relevant interrupts from the list
  *
@@ -755,102 +847,38 @@ static void vgic_prune_ap_list(struct kvm_vcpu *vcpu)
 
 	DEBUG_SPINLOCK_BUG_ON(!irqs_disabled());
 
-retry:
 	raw_spin_lock(&vgic_cpu->ap_list_lock);
 
 	list_for_each_entry_safe(irq, tmp, &vgic_cpu->ap_list_head, ap_list) {
-		struct kvm_vcpu *target_vcpu, *vcpuA, *vcpuB;
-		bool target_vcpu_needs_kick = false;
-
-		raw_spin_lock(&irq->irq_lock);
+		struct kvm_vcpu *target_vcpu;
 
 		BUG_ON(vcpu != irq->vcpu);
 
-		target_vcpu = vgic_target_oracle(irq);
-
-		if (!target_vcpu) {
-			/*
-			 * We don't need to process this interrupt any
-			 * further, move it off the list.
-			 */
-			list_del(&irq->ap_list);
-			irq->vcpu = NULL;
-			raw_spin_unlock(&irq->irq_lock);
-
-			/*
-			 * This vgic_put_irq call matches the
-			 * vgic_get_irq_ref in vgic_queue_irq_unlock,
-			 * where we added the LPI to the ap_list. As
-			 * we remove the irq from the list, we drop
-			 * also drop the refcount.
-			 */
-			deleted_lpis |= vgic_put_irq_norelease(vcpu->kvm, irq);
-			continue;
-		}
+		raw_spin_lock(&irq->irq_lock);
 
-		if (target_vcpu == vcpu) {
-			/* We're on the right CPU */
+		target_vcpu = vgic_target_oracle(irq);
+		if (target_vcpu) {
+			KVM_BUG_ON(target_vcpu != vcpu, vcpu->kvm);
 			raw_spin_unlock(&irq->irq_lock);
 			continue;
 		}
 
 		/*
-		 * This interrupt looks like it has to be migrated,
-		 * make sure it is kept alive while locks are dropped.
+		 * We don't need to process this interrupt any
+		 * further, move it off the list.
 		 */
-		vgic_get_irq_ref(irq);
-
+		list_del(&irq->ap_list);
+		irq->vcpu = NULL;
 		raw_spin_unlock(&irq->irq_lock);
-		raw_spin_unlock(&vgic_cpu->ap_list_lock);
 
 		/*
-		 * Ensure locking order by always locking the smallest
-		 * ID first.
+		 * This vgic_put_irq call matches the
+		 * vgic_get_irq_ref in vgic_queue_irq_unlock,
+		 * where we added the LPI to the ap_list. As
+		 * we remove the irq from the list, we drop
+		 * also drop the refcount.
 		 */
-		if (vcpu->vcpu_id < target_vcpu->vcpu_id) {
-			vcpuA = vcpu;
-			vcpuB = target_vcpu;
-		} else {
-			vcpuA = target_vcpu;
-			vcpuB = vcpu;
-		}
-
-		raw_spin_lock(&vcpuA->arch.vgic_cpu.ap_list_lock);
-		raw_spin_lock_nested(&vcpuB->arch.vgic_cpu.ap_list_lock,
-				      SINGLE_DEPTH_NESTING);
-		raw_spin_lock(&irq->irq_lock);
-
-		/*
-		 * If the interrupt is still ours and its affinity has
-		 * been preserved, move it around. Otherwise, it means
-		 * things have changed while the interrupt was unlocked
-		 * (it may even have been taken off the list with its
-		 * affinity left untouched), and we need to replay this.
-		 *
-		 * In all cases, we cannot trust the list not to have
-		 * changed, so we restart from the beginning.
-		 */
-		if (irq->vcpu == vcpu && target_vcpu == vgic_target_oracle(irq)) {
-			struct vgic_cpu *new_cpu = &target_vcpu->arch.vgic_cpu;
-
-			list_del(&irq->ap_list);
-			irq->vcpu = target_vcpu;
-			list_add_tail(&irq->ap_list, &new_cpu->ap_list_head);
-			target_vcpu_needs_kick = true;
-		}
-
-		raw_spin_unlock(&irq->irq_lock);
-		raw_spin_unlock(&vcpuB->arch.vgic_cpu.ap_list_lock);
-		raw_spin_unlock(&vcpuA->arch.vgic_cpu.ap_list_lock);
-
 		deleted_lpis |= vgic_put_irq_norelease(vcpu->kvm, irq);
-
-		if (target_vcpu_needs_kick) {
-			kvm_make_request(KVM_REQ_IRQ_PENDING, target_vcpu);
-			kvm_vcpu_kick(target_vcpu);
-		}
-
-		goto retry;
 	}
 
 	/*
diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
index b71d486ae514..18437620e198 100644
--- a/arch/arm64/kvm/vgic/vgic.h
+++ b/arch/arm64/kvm/vgic/vgic.h
@@ -515,4 +515,28 @@ static inline bool vgic_supports_direct_irqs(struct kvm *kvm)
 int vgic_its_debug_init(struct kvm_device *dev);
 void vgic_its_debug_destroy(struct kvm_device *dev);
 
+void __vgic_update_irq_affinity(struct kvm *kvm, struct vgic_irq *irq,
+				struct kvm_vcpu *old, struct kvm_vcpu *new,
+				u32 data);
+
+static inline void vgic_v2_update_spi_affinity(struct kvm *kvm, struct vgic_irq *irq,
+					       u8 targets)
+{
+	int target = targets ? __ffs(targets) : 0;
+
+	__vgic_update_irq_affinity(kvm, irq, NULL, kvm_get_vcpu(kvm, target), targets);
+}
+
+static inline void vgic_v3_update_spi_affinity(struct kvm *kvm, struct vgic_irq *irq,
+					       u32 mpidr)
+{
+	__vgic_update_irq_affinity(kvm, irq, NULL, kvm_mpidr_to_vcpu(kvm, mpidr), mpidr);
+}
+
+static inline void vgic_update_lpi_affinity(struct kvm *kvm, struct vgic_irq *irq,
+					    struct kvm_vcpu *old, struct kvm_vcpu *new)
+{
+	__vgic_update_irq_affinity(kvm, irq, old, new, 0);
+}
+
 #endif
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 086b7578e5f3..e161f2e3a07c 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -123,6 +123,7 @@
 #define vgic_v5_make_lpi(x) (__vgic_v5_set_type(LPI) | vgic_v5_set_hwirq_id(x))
 
 #define __vgic_is_v(k, v) ((k)->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V##v)
+#define vgic_is_v2(k) (__vgic_is_v(k, 2))
 #define vgic_is_v3(k) (__vgic_is_v(k, 3))
 #define vgic_is_v5(k) (__vgic_is_v(k, 5))
 
-- 
2.47.3

[PATCH 3/4] KVM: arm64: vgic-v3: Only pause the targeted vCPU when disabling LPIs

[Oliver Upton]

KVM pauses the VM any time LPIs are disabled at a redistributor, as
retiring pending LPIs was a bit dicey since it involved manipulating the
AP list of another vCPU while it was running. Of course, this is a very
large hammer and we could do slightly better.

Only pause the targeted vCPU when disabling LPIs, which is almost always
going to be done locally unless your guest is up to no good.

Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/vgic/vgic-mmio-v3.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index e5fd7a0002b5..2dd8d83fc93d 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -302,16 +302,16 @@ 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 everybody has written their LRs back to the irq
-		 * structures, and release any reference they would have.
+		 * sure that the target vCPU has written their LRs back to the
+		 * irq structures, and release any reference they would have.
 		 *
 		 * If it hurts, don't do it.
 		 */
-		kvm_arm_halt_guest(vcpu->kvm);
+		kvm_pause_vcpu(vcpu);
 		vgic_flush_pending_lpis(vcpu);
 		vgic_its_invalidate_all_caches(vcpu->kvm);
 		atomic_set_release(&vgic_cpu->ctlr, 0);
-		kvm_arm_resume_guest(vcpu->kvm);
+		kvm_resume_vcpu(vcpu);
 	} else {
 		ctlr = atomic_cmpxchg_acquire(&vgic_cpu->ctlr, 0,
 					      GICR_CTLR_ENABLE_LPIS);
-- 
2.47.3

[PATCH 4/4] KVM: arm64: vgic-v3: Pause the source vCPU when processing MOVALL cmd

[Oliver Upton]

We currently pause the VM to process a MOVALL command to avoid
undesirable contention on the AP list with guest entry/exit. This is no
longer quite as big of an issue, as the migrations of active/pending
IRQs no longer happens in vgic_prune_ap_list().

To cope with this, vgic_update_lpi_affinity() may pause the source vCPU
internally for a pending LPI to remove it from the AP list. Since
keeping the vCPU paused for the duration of the MOVALL command can
reduce the overhead, relax (but preserve) the VM pause to only pause the
source vCPU.

Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 arch/arm64/kvm/vgic/vgic-its.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 585df69d1ffe..4e4970944ee0 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -1382,11 +1382,11 @@ static int vgic_its_cmd_handle_movall(struct kvm *kvm, struct vgic_its *its,
 	/*
 	 * Bulk operations such as MOVALL are a pain, as they can clash
 	 * badly locking-wise with other vcpus entering and exiting the
-	 * guest, should they be affected by it. Stopping the guest is a
-	 * safer bet to ensure uncontended access and ultimately forward
-	 * progress. Yeah...
+	 * guest, should they be affected by it. Stopping the source vCPU
+	 * is a safer bet to ensure uncontended access and ultimately
+	 * forward progress. Yeah...
 	 */
-	kvm_arm_halt_guest(kvm);
+	kvm_pause_vcpu(vcpu1);
 
 	xa_for_each(&dist->lpi_xa, intid, irq) {
 		irq = vgic_get_irq(kvm, intid);
@@ -1400,7 +1400,7 @@ static int vgic_its_cmd_handle_movall(struct kvm *kvm, struct vgic_its *its,
 
 	vgic_its_invalidate_cache(its);
 
-	kvm_arm_resume_guest(kvm);
+	kvm_resume_vcpu(vcpu1);
 
 	return 0;
 }
-- 
2.47.3

Re: [PATCH 0/4] KVM: arm64: vgic: Stop migrating IRQ from vgic_prune_ap_list()

[Fuad Tabba]

Hi Oliver,

On Tue, 29 Sept 2026 at 22:29, Oliver Upton <oupton@kernel.org> wrote:
[...]
> Marc, this series applies on top of your own for LPI malice [1]. You'll
> note that I've taken some of it for 7.3 while leaving the rest for you
> to take in 7.4, I'm perfectly happy with Patch 3 and 4 getting squashed
> into the corresponding bits of your own series if you prefer.

My Tested-by on the applied patches went out with a stray space in the
address, so they didn't get applied.

Sorry about that.

Cheers,
/fuad



> Tested semi-rigorously on an Altra machine and the Orion board, using
> both real guests and an LLM-generated selftest specific for migrating
> pending LPIs. The latter was way too big to make sense of, I'll try and
> post a test if I can it into something tidy.
>
> [1]: https://lore.kernel.org/kvmarm/20260929093548.3598547-1-maz@kernel.org/
>
> Oliver Upton (4):
>   KVM: arm64: Add helpers to halt a vCPU
>   KVM: arm64: vgic: Move IRQ migrations out of vgic_prune_ap_list()
>   KVM: arm64: vgic-v3: Only pause the targeted vCPU when disabling LPIs
>   KVM: arm64: vgic-v3: Pause the source vCPU when processing MOVALL cmd
>
>  arch/arm64/include/asm/kvm_host.h  |   2 +
>  arch/arm64/kvm/arm.c               |  18 ++-
>  arch/arm64/kvm/vgic/vgic-its.c     |  19 ++-
>  arch/arm64/kvm/vgic/vgic-mmio-v2.c |  11 +-
>  arch/arm64/kvm/vgic/vgic-mmio-v3.c |  18 ++-
>  arch/arm64/kvm/vgic/vgic.c         | 184 +++++++++++++++++------------
>  arch/arm64/kvm/vgic/vgic.h         |  24 ++++
>  include/kvm/arm_vgic.h             |   1 +
>  8 files changed, 164 insertions(+), 113 deletions(-)
>
>
> base-commit: cfaf3b76669d386912a91103671691692f81c316
> --
> 2.47.3
>

Re: [PATCH 1/4] KVM: arm64: Add helpers to halt a vCPU

[Fuad Tabba]

Hi Oliver,

On Tue, 29 Sep 2026 22:29:22 +0100, Oliver Upton <oupton@kernel.org> wrote:
[...]
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
[...]
> +void kvm_pause_vcpu(struct kvm_vcpu *vcpu)
> +{
> +        atomic_inc(&vcpu->arch.pause);
> +        kvm_make_request_and_kick(KVM_REQ_SLEEP, vcpu);
> +}

Could this wait like kvm_arm_halt_guest() does? The halt sends an IPI
to any vCPU that isn't OUTSIDE_GUEST_MODE, but this only sends one if
its own cmpxchg takes the vCPU out of IN_GUEST_MODE. After an earlier
kick the vCPU can still have its LRs loaded when this returns. A
scratch test moving a pending LPI back and forth with MOVI hits the
WARN_ON(irq->on_lr) in vgic_v3_compute_lr(), and doesn't once this
waits.

Cheers,
/fuad

Re: [PATCH 2/4] KVM: arm64: vgic: Move IRQ migrations out of vgic_prune_ap_list()

[Fuad Tabba]

Hi Oliver,

On Tue, 29 Sep 2026 22:29:23 +0100, Oliver Upton <oupton@kernel.org> wrote:
[...]
> diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
[...]
> +void __vgic_update_irq_affinity(struct kvm *kvm, struct vgic_irq *irq,
> +                                struct kvm_vcpu *old, struct kvm_vcpu *new,
> +                                u32 data)
[...]
> +        /* Fire! */
> +        kvm_pause_vcpu(tmp);
> +
> +        scoped_guard(raw_spinlock_irqsave, &tmp->arch.vgic_cpu.ap_list_lock) {
> +                scoped_guard(raw_spinlock, &irq->irq_lock) {
> +                        /*
> +                         * The IRQ could've been moved to another AP list after
> +                         * dropping the irq_lock. Make sure it's where we expect
> +                         * it to be, remove from the list and retain the implied
> +                         * reference until we queue it on the new vCPU.
> +                         */
> +                        if (irq->vcpu == tmp) {
> +                                list_del(&irq->ap_list);
> +                                irq->vcpu = NULL;
> +                                irq->target_vcpu = new;
> +                                if (vgic_is_v2(kvm))
> +                                        irq->targets = data;
> +                                else
> +                                        irq->mpidr = data;
> +                                pruned = true;
> +                        }

Could an active IRQ stay on its current vCPU until it's deactivated?
With irq->vcpu cleared, the oracle returns target_vcpu for it, so with
EOImode 0 the old vCPU's EOI finds no LR and the SPI stays active on
the new vCPU. A scratch test moving the IROUTER of an active edge SPI
loses it with this patch, but not on Marc's branch.

Could this also make sure the IRQ isn't still in the old vCPU's LRs? A
vCPU between the vgic flush and the IN_GUEST_MODE store is
OUTSIDE_GUEST_MODE, so even kvm_arm_halt_guest() wouldn't wait for it.

Cheers,
/fuad

Re: [PATCH 2/4] KVM: arm64: vgic: Move IRQ migrations out of vgic_prune_ap_list()

[Oliver Upton]

Hi Fuad,

thanks for the review

On Wed, Sep 30, 2026 at 02:55:04PM +0100, Fuad Tabba wrote:
> Hi Oliver,
> 
> On Tue, 29 Sep 2026 22:29:23 +0100, Oliver Upton <oupton@kernel.org> wrote:
> [...]
> > diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
> [...]
> > +void __vgic_update_irq_affinity(struct kvm *kvm, struct vgic_irq *irq,
> > +                                struct kvm_vcpu *old, struct kvm_vcpu *new,
> > +                                u32 data)
> [...]
> > +        /* Fire! */
> > +        kvm_pause_vcpu(tmp);
> > +
> > +        scoped_guard(raw_spinlock_irqsave, &tmp->arch.vgic_cpu.ap_list_lock) {
> > +                scoped_guard(raw_spinlock, &irq->irq_lock) {
> > +                        /*
> > +                         * The IRQ could've been moved to another AP list after
> > +                         * dropping the irq_lock. Make sure it's where we expect
> > +                         * it to be, remove from the list and retain the implied
> > +                         * reference until we queue it on the new vCPU.
> > +                         */
> > +                        if (irq->vcpu == tmp) {
> > +                                list_del(&irq->ap_list);
> > +                                irq->vcpu = NULL;
> > +                                irq->target_vcpu = new;
> > +                                if (vgic_is_v2(kvm))
> > +                                        irq->targets = data;
> > +                                else
> > +                                        irq->mpidr = data;
> > +                                pruned = true;
> > +                        }
> 
> Could an active IRQ stay on its current vCPU until it's deactivated?
> With irq->vcpu cleared, the oracle returns target_vcpu for it, so with
> EOImode 0 the old vCPU's EOI finds no LR and the SPI stays active on
> the new vCPU. A scratch test moving the IROUTER of an active edge SPI
> loses it with this patch, but not on Marc's branch.

Ugh. Well spotted, of course. Too much time dealing with LPIs :)

Let me have a think about this. Ultimately the goal is to prevent the
guest from queueing up an unbounded amount of work in a context where
we can't schedule, but deactivation still requires some work to be done
locally on the vCPU.

We already have some infrastructure for async processing of the AP list
for EOImode=1, perhaps there's a chance for reusing that here with some
additional guardrails.

> Could this also make sure the IRQ isn't still in the old vCPU's LRs? A
> vCPU between the vgic flush and the IN_GUEST_MODE store is
> OUTSIDE_GUEST_MODE, so even kvm_arm_halt_guest() wouldn't wait for it.

So we set the request on the vCPU, which means we're guaranteed to sync
the LRs and recompute before entering the VM. This wouldn't affect
affinity changes but there's a chance of a stale LR overwriting the
current active/pending state. So yet another bug, ugh.

Thanks,
Oliver