Re: [PATCH v6 29/39] KVM: arm64: gic-v5: Enlighten arch timer for GICv5

[Marc Zyngier <maz@kernel.org>]

On Tue, 17 Mar 2026 11:47:29 +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 queue_irq_unlock irq_op is 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 PPI op 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>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> ---
>  arch/arm64/kvm/arch_timer.c     | 110 ++++++++++++++++++++++++++------
>  arch/arm64/kvm/vgic/vgic-init.c |   9 +++
>  arch/arm64/kvm/vgic/vgic-v5.c   |   7 +-
>  include/kvm/arm_arch_timer.h    |  11 +++-
>  include/kvm/arm_vgic.h          |   3 +
>  5 files changed, 115 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> index 53312b88c342d..4575c36cae537 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c
> @@ -56,6 +56,12 @@ static struct irq_ops arch_timer_irq_ops = {
>  	.get_input_level = kvm_arch_timer_get_input_level,
>  };
>  
> +static struct irq_ops arch_timer_irq_ops_vgic_v5 = {
> +	.get_input_level = kvm_arch_timer_get_input_level,
> +	.queue_irq_unlock = vgic_v5_ppi_queue_irq_unlock,
> +	.set_direct_injection = vgic_v5_set_ppi_dvi,
> +};
> +
>  static int nr_timers(struct kvm_vcpu *vcpu)
>  {
>  	if (!vcpu_has_nv(vcpu))
> @@ -177,6 +183,10 @@ void get_timer_map(struct kvm_vcpu *vcpu, struct timer_map *map)
>  		map->emul_ptimer = vcpu_ptimer(vcpu);
>  	}
>  
> +	map->direct_vtimer->direct = true;
> +	if (map->direct_ptimer)
> +		map->direct_ptimer->direct = true;
> +
>  	trace_kvm_get_timer_map(vcpu->vcpu_id, map);
>  }
>  
> @@ -396,7 +406,11 @@ static bool kvm_timer_should_fire(struct arch_timer_context *timer_ctx)
>  
>  int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
>  {
> -	return vcpu_has_wfit_active(vcpu) && wfit_delay_ns(vcpu) == 0;
> +	struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
> +	struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> +
> +	return kvm_timer_should_fire(vtimer) || kvm_timer_should_fire(ptimer) ||
> +	       (vcpu_has_wfit_active(vcpu) && wfit_delay_ns(vcpu) == 0);
>  }
>  
>  /*
> @@ -447,6 +461,10 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,
>  	if (userspace_irqchip(vcpu->kvm))
>  		return;
>  
> +	/* Skip injecting on GICv5 for directly injected (DVI'd) timers */
> +	if (vgic_is_v5(vcpu->kvm) && timer_ctx->direct)
> +		return;
> +
>  	kvm_vgic_inject_irq(vcpu->kvm, vcpu,
>  			    timer_irq(timer_ctx),
>  			    timer_ctx->irq.level,
> @@ -657,6 +675,24 @@ static inline void set_timer_irq_phys_active(struct arch_timer_context *ctx, boo
>  	WARN_ON(r);
>  }
>  
> +/*
> + * On GICv5 we use DVI for the arch timer PPIs. This is restored later
> + * on as part of vgic_load. Therefore, in order to avoid the guest's
> + * interrupt making it to the host we mask it before entering the
> + * guest and unmask it again when we return.
> + */
> +static inline void set_timer_irq_phys_masked(struct arch_timer_context *ctx, bool masked)
> +{
> +	if (masked) {
> +		disable_percpu_irq(ctx->host_timer_irq);
> +	} else {
> +		if (ctx->host_timer_irq == host_vtimer_irq)
> +			enable_percpu_irq(ctx->host_timer_irq, host_vtimer_irq_flags);
> +		else
> +			enable_percpu_irq(ctx->host_timer_irq, host_ptimer_irq_flags);
> +	}
> +}

I think this is missing a trick, which is to reuse the mask/unmask
infrastructure we use for the fruity crap. How about this following
untested hack?

diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
index 600f250753b45..b29bea800e2ab 100644
--- a/arch/arm64/kvm/arch_timer.c
+++ b/arch/arm64/kvm/arch_timer.c
@@ -660,7 +660,7 @@ static inline void set_timer_irq_phys_active(struct arch_timer_context *ctx, boo
 static void kvm_timer_vcpu_load_gic(struct arch_timer_context *ctx)
 {
 	struct kvm_vcpu *vcpu = timer_context_to_vcpu(ctx);
-	bool phys_active = false;
+	bool phys_active = vgic_is_v5(vcpu->kvm);
 
 	/*
 	 * Update the timer output so that it is likely to match the
@@ -934,6 +934,12 @@ void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu)
 
 	if (kvm_vcpu_is_blocking(vcpu))
 		kvm_timer_blocking(vcpu);
+
+	if (vgic_is_v5(vcpu)) {
+		set_timer_irq_phys_active(map.direct_vtimer, false);
+		if (map.direct_ptimer)
+			set_timer_irq_phys_active(map.direct_ptimer, false);
+	}
 }
 
 void kvm_timer_sync_nested(struct kvm_vcpu *vcpu)
@@ -1333,7 +1339,8 @@ static int kvm_irq_init(struct arch_timer_kvm_info *info)
 	host_vtimer_irq = info->virtual_irq;
 	kvm_irq_fixup_flags(host_vtimer_irq, &host_vtimer_irq_flags);
 
-	if (kvm_vgic_global_state.no_hw_deactivation) {
+	if (kvm_vgic_global_state.no_hw_deactivation ||
+	    kvm_vgic_global_state.type == VGIC_V5) {
 		struct fwnode_handle *fwnode;
 		struct irq_data *data;
 
@@ -1351,7 +1358,8 @@ static int kvm_irq_init(struct arch_timer_kvm_info *info)
 			return -ENOMEM;
 		}
 
-		arch_timer_irq_ops.flags |= VGIC_IRQ_SW_RESAMPLE;
+		if (kvm_vgic_global_state.no_hw_deactivation)
+			arch_timer_irq_ops.flags |= VGIC_IRQ_SW_RESAMPLE;
 		WARN_ON(irq_domain_push_irq(domain, host_vtimer_irq,
 					    (void *)TIMER_VTIMER));
 	}

which should avoid adding some new masking stuff.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.