Re: [PATCH v5 26/49] KVM: arm64: gic: Introduce set_pending_state() to irq_ops

[Sascha Bischoff <Sascha.Bischoff@arm.com>]

On Fri, 2026-08-07 at 15:14 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [High] The `set_pending_state` callback is only invoked in
> `kvm_vgic_inject_irq()`, leaving hardware state out of sync when VGIC
> shadow state is modified by other paths.
> --
> 
> --- Patch [26]: [PATCH v5 26/49] KVM: arm64: gic: Introduce
> set_pending_state() to irq_ops ---
> commit 706acf491d97981f9dd498ec21c341cba46f1f47
> Author: Sascha Bischoff <Sascha.Bischoff@arm.com>
> Subject: KVM: arm64: gic: Introduce set_pending_state() to irq_ops
> 
> This commit introduces a set_pending_state() function pointer to
> irq_ops.
> This allows hardware-managed interrupts to mirror KVM's VGIC shadow
> pending
> state into the hardware when the state changes.
> 
> > diff --git a/arch/arm64/kvm/vgic/vgic.c
> > b/arch/arm64/kvm/vgic/vgic.c
> > index 169a8664322d0..023792c28fc5c 100644
> > --- a/arch/arm64/kvm/vgic/vgic.c
> > +++ b/arch/arm64/kvm/vgic/vgic.c
> > @@ -563,6 +563,9 @@ int kvm_vgic_inject_irq(struct kvm *kvm, struct
> > kvm_vcpu *vcpu,
> >  	else
> >  		irq->pending_latch = true;
> >  
> > +	if (irq->ops && irq->ops->set_pending_state)
> > +		WARN_ON_ONCE(!irq->ops->set_pending_state(vcpu,
> > irq));
> > +
> 
> [Severity: High]
> Since the set_pending_state callback is only invoked here in
> kvm_vgic_inject_irq(), will the hardware state become out of sync
> when the
> VGIC shadow state is modified by other paths?

We treat SPIs as "inject and forget". The hardware manages their state,
and determines when they should be presented to the guest. All we do is
to give hardware the most recent pending state. Therefore, we don't
need to do anything in the other paths.

> 
> For example, if a guest accesses the GICD_ICPENDR register, KVM
> updates
> irq->pending_latch in __clear_pending(), but does not appear to
> invoke the
> hardware sync callback:
> 
> arch/arm64/kvm/vgic/vgic-mmio.c:__clear_pending() {
> 		if (irq->hw && !is_user)
> 			vgic_hw_irq_cpending(vcpu, irq);
> 		else
> 			irq->pending_latch = false;
> }
> 
> A similar desync seems possible in __set_pending() when handling
> GICD_ISPENDR writes, as well as in vgic_write_irq_line_level_info()
> and
> kvm_vgic_reset_mapped_irq().

These are not present with GICv5 (as in, this is for a completely
different GIC model), so this is not an issue.

> 
> Could this lead to stuck interrupts or spurious injections for
> hardware-managed interrupts if the hardware remains pending after the
> shadow
> state is cleared?

No.

> 
> >  	vgic_queue_irq_unlock(kvm, irq, flags);
> >  	vgic_put_irq(kvm, irq);
> 

Thanks,
Sascha