Re: [PATCH v4 26/48] KVM: arm64: gic: Introduce set_pending_state() to irq_op

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incomplete state mirroring: hardware pending state desynchronizes from VGIC shadow state on guest MMIO/ITS writes.
- [Medium] Userspace-reachable WARN_ON_ONCE when set_pending_state fails, causing potential Denial of Service.
--

--- Patch [26]: [PATCH v4 26/48] KVM: arm64: gic: Introduce set_pending_state() to irq_op ---
commit f6a8b8f38eb780090a64d051bdb9d419dcf75e26
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>

KVM: arm64: gic: Introduce set_pending_state() to irq_op

This patch introduces a set_pending_state() function pointer to irq_ops.
This allows the VGIC to communicate pending state directly to hardware for
cases where hardware manages parts of the interrupt lifecycle, like GICv5.
It wires this new callback into kvm_vgic_inject_irq().

> The intent is for this to be used after the VGIC shadow
> pending state has changed, allowing the backend to mirror the updated
> state into hardware.
>
> This new function is plumbed into kvm_vgic_inject_irq(), and is only
> called if irq_ops are provided and this function pointer is explicitly
> set.

[Severity: High]
If the intent is to mirror the state after the VGIC shadow pending state
has changed, should this also be wired up for guest MMIO writes and ITS
commands?

For example, when a guest writes to ISPENDR or ICPENDR, the shadow state is
updated in arch/arm64/kvm/vgic/vgic-mmio.c:__set_pending() and
__clear_pending(), but set_pending_state() is not called.

Similarly, in arch/arm64/kvm/vgic/vgic-its.c:vgic_its_cmd_handle_clear(),
the ITS CLEAR command updates the LPI pending state without notifying the
hardware.

Could this omission cause the hardware pending state to become desynchronized
from the VGIC shadow state, leading to missed or spurious interrupts?

> 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: Medium]
Can a guest or unprivileged host-userspace trigger this warning?

Since kvm_vgic_inject_irq() is reachable via userspace injection
(e.g., KVM_IRQ_LINE) or guest actions, if an adversary can create
conditions that cause set_pending_state() to fail, they could trigger
this WARN_ON_ONCE. This would violate the KVM guideline against
user-reachable warnings.

Could this be replaced with a pr_warn_once() or handled by returning
an error code?

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