Re: [PATCH] KVM: arm64: vgic: Check the interrupt is still ours before migrating it

[Marc Zyngier <maz@kernel.org>]

On Fri, 12 Jun 2026 03:22:35 +0100,
Hyunwoo Kim <imv4bel@gmail.com> wrote:
> 
> On Wed, Jun 10, 2026 at 05:00:25PM +0100, Marc Zyngier wrote:
> > It's rather unclear to me what the semantics of this are.
> > 
> > If vcpu-a decides to nuke the LPIs of vcpu-b and the LPI had in the
> > meantime been migrated to vcpu-c, but obviously not observed by vcpu-c
> > yet as the LPI is still on vcpu-b's AP-list, then I don't see the
> > point in keeping this state.
> > 
> > Am I missing something obvious?
> 
> I looked a bit more into Oliver's review, the one suggesting that pending 
> be cleared only for resident LPIs while the ones being migrated are left 
> in place.
> 
> What the leave preserves is the pending edge of a single LPI whose target 
> is already vcpu-c but which is still on vcpu-b's ap_list. This edge is
> always lost when we just clear it, but for a device that fires again a 
> later INT reaches vcpu-c through the oracle, so it is mostly harmless.

Not completely harmless. When the guest writes EnableLPIs==0, it
accepts the lost of any pending bit that could be stored. These won't
be regenerated, unless the device signals a new event that maps to the
same LPIs. But again, this is the guest's own decision, and I don't
see a reason to prevent it from shooting itself in the foot.

> The
> exception is a software LPI that never fires again(irq->hw == false): 
> that edge is then lost with no way to recover it, because 
> its_sync_lpi_pending_table only re-syncs the LPIs whose target_vcpu matches, 
> and the disable path does no pending writeback. I am not entirely sure about 
> this part, though.

I don't think this is a problem, as the architecture doesn't guarantee
the state of the pending table after turning EnableLPIs off. There's
even a note recommending to move the interrupts to another RD before
doing that.

> 
> Since this does not look like the common case, if it does not need to be 
> covered I will send v2 keeping only the pending clear and the ref hold in 
> vgic_prune_ap_list(). What do you think?

So that it is entirely unambiguous, my suggestion is to have this:

diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 5a4768d8cd4f3..70a161383e5a6 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -203,6 +203,7 @@ void vgic_flush_pending_lpis(struct kvm_vcpu *vcpu)
 	list_for_each_entry_safe(irq, tmp, &vgic_cpu->ap_list_head, ap_list) {
 		if (irq_is_lpi(vcpu->kvm, irq->intid)) {
 			raw_spin_lock(&irq->irq_lock);
+			irq->pending_latch = false;
 			list_del(&irq->ap_list);
 			irq->vcpu = NULL;
 			raw_spin_unlock(&irq->irq_lock);
@@ -792,7 +793,11 @@ static void vgic_prune_ap_list(struct kvm_vcpu *vcpu)
 			continue;
 		}
 
-		/* This interrupt looks like it has to be migrated. */
+		/*
+		 * This interrupt looks like it has to be migrated,
+		 * make sure it is kept alive while locks are dropped.
+		 */
+		vgic_get_irq_ref(irq);
 
 		raw_spin_unlock(&irq->irq_lock);
 		raw_spin_unlock(&vgic_cpu->ap_list_lock);
@@ -836,6 +841,8 @@ static void vgic_prune_ap_list(struct kvm_vcpu *vcpu)
 		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);

Could you please give it a go with whatever reproducer you have?

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.