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

From: Marc Zyngier <[email-masked]>
Date: Fri, 05 Jun 2026 08:42:52 +0100

On Fri, 05 Jun 2026 07:00:37 +0100,
Oliver Upton <[email-masked]> wrote:
> 
> On Fri, Jun 05, 2026 at 05:59:15AM +0900, Hyunwoo Kim wrote:
> > vgic_prune_ap_list() drops both ap_list_lock and irq_lock while migrating
> > an interrupt to another vCPU. After reacquiring the locks it only checks
> > that the affinity is unchanged (target_vcpu == vgic_target_oracle(irq))
> > before moving the interrupt, which assumes that an interrupt whose affinity
> > is preserved is still queued on this vCPU's ap_list.
> > 
> > That assumption no longer holds if the interrupt is taken off the ap_list
> > while the locks are dropped. vgic_flush_pending_lpis() removes the
> > interrupt from the list and sets irq->vcpu to NULL, but leaves
> > enabled/pending/target_vcpu untouched. As the interrupt is still enabled
> > and pending, vgic_target_oracle() returns the same target_vcpu, so the
> > affinity check passes and list_del() is run a second time on an entry that
> > has already been removed.
> > 
> > Also check that the interrupt is still assigned to this vCPU
> > (irq->vcpu == vcpu) before moving it.
> > 
> > Fixes: 0919e84c0fc1 ("KVM: arm/arm64: vgic-new: Add IRQ sync/flush framework")
> > Signed-off-by: Hyunwoo Kim <[email-masked]>
> 
> Looking at this and the other VGIC patch you sent (which should've been
> a combined series), are you trying to deal with a vCPU writing to
> another vCPU's redistributor? I.e. vCPU B setting GICR_CTLR.EnableLPIs=0
> behind the back of vCPU A?
> 
> That is extremely relevant information as the off-the-cuff reaction is
> that no race exists. But since the GIC architecture is awesome and
> allows for this sort of insanity, it obviously does....
> 
> Anyway, for LPIs resident on a particular RD, there's zero expectation
> that the pending state is preserved when EnableLPIs=0. So I'd rather
> vgic_flush_pending_lpis() just invalidate the pending state.

Just clearing the pending state introduces a potential problem as we
now have an interrupt that is neither active nor pending on the AP
list. It is not impossible to solve (we now have similar behaviours
with SPI deactivation from another vcpu), but that requires posting a
KVM_REQ_VGIC_PROCESS_UPDATE to the target vcpu.

> Beyond that, I see two other fixes for lifetime issues around the
> vgic_irq in the middle of migration. I'd like to see explicit RCU
> protection around the release && reacquire of the ap_list_lock rather
> than depending on the precondition that IRQs are disabled.

I'm not sure I follow. Are you suggesting turning the AP list into an
RCU protected list?

Thanks,

	M.

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