Re: [PATCH v2 17/36] KVM: arm64: gic: Introduce irq_queue and set_pending_state to irq_ops

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Fri, 19 Dec 2025 15:52:41 +0000
Sascha Bischoff <Sascha.Bischoff@arm.com> wrote:

> There are times when the default behaviour of vgic_queue_irq_unlock is

Nice to make function names explicit in a patch description by using
vgic_queue_irq_unlock() style. Some subsystems insist we do that in commit
descriptions. No idea if kvm does though.

> undesirable. This is because some GICs, such a GICv5 which is the main
> driver for this change, handle the majority of the interrupt lifecycle
> in hardware. In this case, there is no need for a per-VCPU AP list as
> the interrupt can be made pending directly. This is done either via
> the ICH_PPI_x_EL2 registers for PPIs, or with the VDPEND system
> instruction for SPIs and LPIs.
> 
> The queue_irq_unlock function is made overridable using a new function
> pointer in struct irq_ops. In kvm_vgic_inject_irq,
> vgic_queue_irq_unlock is overridden if the function pointer is
> non-null.
> 
> Additionally, a new function is added via a function pointer -
> set_pending_state. The intent is for this to be used to directly set
> the pending state in hardware.
> 
> Both of these new irq_ops are unused in this change - it is purely
> providing the infrastructure itself. The subsequent PPI injection
> changes provide a demonstration of their usage.
> 
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>

Trivial stuff only.

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

> ---
>  arch/arm64/kvm/vgic/vgic.c | 11 +++++++++++
>  include/kvm/arm_vgic.h     | 15 +++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
> index d88570bb2f9f0..ac8cb0270e1e4 100644
> --- a/arch/arm64/kvm/vgic/vgic.c
> +++ b/arch/arm64/kvm/vgic/vgic.c
> @@ -404,6 +404,13 @@ bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq,
>  
>  	lockdep_assert_held(&irq->irq_lock);
>  
> +	/*
> +	 * If we have the queue_irq_unlock irq_op, we want to override
> +	 * the default behaviour. Call that, and return early.

I'd say this comment is really just saying what the code clearly does
so provides not benefit. So drop it.

> +	 */
> +	if (irq->ops && irq->ops->queue_irq_unlock)
> +		return irq->ops->queue_irq_unlock(kvm, irq, flags);
> +
>  retry:
>  	vcpu = vgic_target_oracle(irq);
>  	if (irq->vcpu || !vcpu) {
> @@ -547,7 +554,11 @@ 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));
> +
>  	vgic_queue_irq_unlock(kvm, irq, flags);
> +
Reasonable change, for readability but not relevant to this patch, so don't
do it here.

>  	vgic_put_irq(kvm, irq);
>  
>  	return 0;
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h