Re: [PATCH v2 2/2] KVM: arm64: vgic-its: Remove the pending state of a discarded LPI

[Fuad Tabba]

Hi Marc,

On Fri, 16 Oct 2026 16:51:37 +0100, Marc Zyngier <maz@xxxxxxxxxx> wrote:
[...]
> > diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
[...]
> > @@ -829,13 +829,27 @@ static int vgic_its_cmd_handle_discard(struct kvm *kvm, struct vgic_its *its,
[...]
> > +                /*
> > +                 * A queued LPI can be pending in a running vCPU's list register,
> > +                 * which the exit path folds back: halt the guest before clearing.
> > +                 */
> > +                scoped_guard(raw_spinlock_irqsave, &irq->irq_lock)
> > +                        queued = irq->vcpu;
>
> Is this comment correct? This is only checking that the LPI is on the
> AP list of a vcpu, not that it is in the LRs.

The comment is correct but incomplete: it should say that only an LPI
on a vCPU's AP list can be in that vCPU's LRs, which is why the check
is enough.

> > +
> > +                if (queued)
> > +                        kvm_arm_halt_guest(kvm);
>
> And should we move this to kvm_pause_vcpu() instead of the big hammer,
> this still works because
>
> - we hold the ITS lock, so no MOVI can be issued on the same ITS
>
> - mapping of the same LPI is already prevented by the previous patch,
>   so no sneaky MOVI generated somewhere else can impact this LPI
>
> Do you agree?

Agreed, with one gap: a MOVI that completed before the DISCARD leaves
the LPI on the old vCPU's AP list until that vCPU's next exit. Pausing
irq->vcpu forces that exit, vgic_prune_ap_list() moves the LPI to the
new target and kicks it, and the target can load it into an LR before
pending_latch is cleared. Your two points keep target_vcpu stable, so
pausing the target as well closes that.

I forced the window with a delay between the pause and the clear: after
a MOVI and then a DISCARD, the new target takes the discarded LPI in
every run with only irq->vcpu paused, and doesn't take it in any run
with the target paused too.

[...]
> If you're OK with the above, I'll make the corresponding changes as I
> apply the patches.

Yes please, with the target paused as well.

Cheers,
/fuad