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

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 <[email protected]>
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?

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().

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

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=26