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

[Hyunwoo Kim <imv4bel@gmail.com>]

On Sun, Jun 14, 2026 at 04:16:44PM +0100, Marc Zyngier wrote:
> 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?

I confirmed your diff fixes the issue. Could you submit this 
patch? Feel free to add Tested-by: Hyunwoo Kim <imv4bel@gmail.com>


Best regards,
Hyunwoo Kim