Re: [PATCH v3 26/36] KVM: arm64: gic-v5: Bump arch timer for GICv5

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

On Fri, 9 Jan 2026 17:04:47 +0000
Sascha Bischoff <Sascha.Bischoff@arm.com> wrote:

> Now that GICv5 has arrived, the arch timer requires some TLC to
> address some of the key differences introduced with GICv5.
> 
> For PPIs on GICv5, the set_pending_state and queue_irq_unlock irq_ops
> are used as AP lists are not required at all for GICv5. The arch timer
> also introduces an irq_op - get_input_level. Extend the
> arch-timer-provided irq_ops to include the two PPI ops for vgic_v5
> guests.
> 
> When possible, DVI (Direct Virtual Interrupt) is set for PPIs when
> using a vgic_v5, which directly inject the pending state into the
> guest. This means that the host never sees the interrupt for the guest
> for these interrupts. This has three impacts.
> 
> * First of all, the kvm_cpu_has_pending_timer check is updated to
>   explicitly check if the timers are expected to fire.
> 
> * Secondly, for mapped timers (which use DVI) they must be masked on
>   the host prior to entering a GICv5 guest, and unmasked on the return
>   path. This is handled in set_timer_irq_phys_masked.
> 
> * Thirdly, it makes zero sense to attempt to inject state for a DVI'd
>   interrupt. Track which timers are direct, and skip the call to
>   kvm_vgic_inject_irq() for these.
> 
> The final, but rather important, change is that the architected PPIs
> for the timers are made mandatory for a GICv5 guest. Attempts to set
> them to anything else are actively rejected. Once a vgic_v5 is
> initialised, the arch timer PPIs are also explicitly reinitialised to
> ensure the correct GICv5-compatible PPIs are used - this also adds in
> the GICv5 PPI type to the intid.
> 
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
Trivial comment below, but either way I'm fine with it. If you do
split the patch, than both can have tag, if not then this one can have it.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> ---
>  arch/arm64/kvm/arch_timer.c     | 117 +++++++++++++++++++++++++-------
>  arch/arm64/kvm/vgic/vgic-init.c |   9 +++
>  arch/arm64/kvm/vgic/vgic-v5.c   |   8 +--
>  include/kvm/arm_arch_timer.h    |  11 ++-
>  include/kvm/arm_vgic.h          |   4 ++
>  5 files changed, 119 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> index 6f033f6644219..2f30d69dbf1ac 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c

> @@ -1601,12 +1665,11 @@ int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
>  	if (!(irq_is_ppi(vcpu->kvm, irq)))
>  		return -EINVAL;
>  
> -	mutex_lock(&vcpu->kvm->arch.config_lock);
> +	guard(mutex)(&vcpu->kvm->arch.config_lock);

In ideal world, separate patch for guard() introduction, but if the maintainers
are happy I don't care that much!

>  
>  	if (test_bit(KVM_ARCH_FLAG_TIMER_PPIS_IMMUTABLE,
>  		     &vcpu->kvm->arch.flags)) {
> -		ret = -EBUSY;
> -		goto out;
> +		return -EBUSY;
>  	}
>  
>  	switch (attr->attr) {
> @@ -1623,10 +1686,16 @@ int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
>  		idx = TIMER_HPTIMER;
>  		break;
>  	default:
> -		ret = -ENXIO;
> -		goto out;
> +		return -ENXIO;
>  	}
>  
> +	/*
> +	 * The PPIs for the Arch Timers are architecturally defined for
> +	 * GICv5. Reject anything that changes them from the specified value.
> +	 */
> +	if (vgic_is_v5(vcpu->kvm) && vcpu->kvm->arch.timer_data.ppi[idx] != irq)
> +		return -EINVAL;
> +
>  	/*
>  	 * We cannot validate the IRQ unicity before we run, so take it at
>  	 * face value. The verdict will be given on first vcpu run, for each
> @@ -1634,8 +1703,6 @@ int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
>  	 */
>  	vcpu->kvm->arch.timer_data.ppi[idx] = irq;
>  
> -out:
> -	mutex_unlock(&vcpu->kvm->arch.config_lock);
>  	return ret;
>  }