Re: [PATCH v3 28/40] KVM: arm64: gic: Introduce set_pending_state() to irq_op

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Hardware pending state desynchronization for GICv5 SPIs and LPIs due to omitted `set_pending_state` / `VDPEND` invocation in guest MMIO and MSI paths.
--

commit ee2f2b77547da18f2f1da7d79905b67a8cbca929
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>
Subject: KVM: arm64: gic: Introduce set_pending_state() to irq_op

This commit introduces a new set_pending_state() function pointer to
irq_ops for the ARM64 KVM VGIC. It calls this function in
kvm_vgic_inject_irq() to allow the backend to mirror the updated VGIC
shadow pending state into hardware for cases where hardware manages the
lifecycle, such as GICv5 SPIs and LPIs.

> diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
> index 8b15938464ab9..ee7fc0c7104e6 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 set_pending_state() is needed to mirror the shadow pending state to
hardware for GICv5 SPIs and LPIs, does it also need to be called in other
paths that update irq->pending_latch?

For example, when a guest writes to ISPENDR/ICPENDR to manually pend or
clear an SPI, the shadow state is updated but the hardware update appears
to be bypassed:

arch/arm64/kvm/vgic/vgic-mmio.c:__set_pending() {
    ...
    irq->pending_latch = true;
    if (irq->hw && !is_user)
        vgic_irq_set_phys_active(irq, true);
    
    vgic_queue_irq_unlock(vcpu->kvm, irq, flags);
    ...
}

Similarly, for LPIs injected via the ITS, the hardware update seems to be
omitted:

arch/arm64/kvm/vgic/vgic-its.c:vgic_its_trigger_msi() {
    ...
    raw_spin_lock_irqsave(&irq->irq_lock, flags);
    irq->pending_latch = true;
    vgic_queue_irq_unlock(kvm, irq, flags);
    ...
}

Could this lead to lost interrupts and potential system hangs if the
hardware remains unaware of the pending state in these paths?

Additionally, do GICv5 LPIs get assigned an irq_ops struct? If not, they
might lack a mechanism to issue the required VDPEND command when triggered
via vgic_its_trigger_msi().

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703154811.3355680-1-sascha.bischoff@arm.com?part=28