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

[Marc Zyngier]

On Tue, 13 Oct 2026 09:16:46 +0100,
Fuad Tabba <fuad.tabba@xxxxxxxxx> wrote:
> 
> DISCARD removes the mapping of an event and instructs the Redistributor
> to remove the pending state of its LPI (IHI 0069H.b, 5.3.4). KVM only
> frees the ITE, so an LPI that is pending when the guest discards it
> stays queued on its target vCPU, and the guest still takes it.
> 
> Clear the pending state before freeing the ITE. A queued LPI can be
> pending in a running vCPU's list register, so halt the guest first when
> the LPI is queued, to fold that state back. A forwarded LPI already
> loses its pending state in its_free_ite(), whose unmap from the vPE
> sends a DISCARD to the host ITS.
> 
> Fixes: df9f58fbea9bc ("KVM: arm64: vgic-its: Implement ITS command queue command handlers")
> Suggested-by: Marc Zyngier <maz@xxxxxxxxxx>
> Signed-off-by: Fuad Tabba <fuad.tabba@xxxxxxxxx>
> ---
>  arch/arm64/kvm/vgic/vgic-its.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> index 44b92c01bc727..3b8e0e5b097e0 100644
> --- 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,
>  		struct its_device *device = find_its_device(its, device_id);
>  		int ite_esz = vgic_its_get_abi(its)->ite_esz;
>  		gpa_t gpa = device->itt_addr + ite->event_id * ite_esz;
> -		/*
> -		 * Though the spec talks about removing the pending state, we
> -		 * don't bother here since we clear the ITTE anyway and the
> -		 * pending state is a property of the ITTE struct.
> -		 */
> +		struct vgic_irq *irq = ite->irq;
> +		bool queued;
> +
>  		vgic_its_invalidate_cache(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.

> +
> +		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?

> +
> +		scoped_guard(raw_spinlock_irqsave, &irq->irq_lock)
> +			irq->pending_latch = false;
> +
> +		if (queued)
> +			kvm_arm_resume_guest(kvm);
> +
>  		its_free_ite(kvm, ite);
>  
>  		return vgic_its_write_entry_lock(its, gpa, 0ULL, ite);

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

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.