Subject: Re: [PATCH v7 00/41] KVM: arm64: Introduce vGIC-v5 with PPI support

From: Marc Zyngier <[email-masked]>
Date: Thu, 19 Mar 2026 18:26:17 +0000

On Thu, 19 Mar 2026 15:49:42 +0000,
Sascha Bischoff <[email-masked]> wrote:
> 
> This is v7 of the patch series to add the virtual GICv5 [1] device
> (vgic_v5). Only PPIs are supported by this initial series, and the
> vgic_v5 implementation is restricted to the CPU interface,
> only. Further patch series are to follow in due course, and will add
> support for SPIs, LPIs, the GICv5 IRS, and the GICv5 ITS.

[...]

I have queued this, but it didn't go as planned. QEMU guests end-up
with a NULL pointer dereference, because we are now setting ops pretty
early in the game, while QEMU creates an irqchip very late.

The obvious solution is what you originally had in your v6, which is
to set the ops in kvm_timer_enable(), but to do it over *all* the
timer interrupts in order not to break NV is a fairly subtle way(we
still want to use the .get_input_level() callback on all timers.

I ended up with the following diff, which I folded in the respective
patches, meaning that patch 4 can be dropped altogether.

Thanks,

	M.

diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
index f6d2f0246d057..67b989671b410 100644
--- a/arch/arm64/kvm/arch_timer.c
+++ b/arch/arm64/kvm/arch_timer.c
@@ -1106,8 +1106,6 @@ static void timer_context_init(struct kvm_vcpu *vcpu, int timerid)
 void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu)
 {
 	struct arch_timer_cpu *timer = vcpu_timer(vcpu);
-	struct timer_map map;
-	struct irq_ops *ops;
 
 	for (int i = 0; i < NR_KVM_TIMERS; i++)
 		timer_context_init(vcpu, i);
@@ -1121,15 +1119,6 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu)
 
 	hrtimer_setup(&timer->bg_timer, kvm_bg_timer_expire, CLOCK_MONOTONIC,
 		      HRTIMER_MODE_ABS_HARD);
-
-	get_timer_map(vcpu, &map);
-
-	ops = vgic_is_v5(vcpu->kvm) ? &arch_timer_irq_ops_vgic_v5 :
-				      &arch_timer_irq_ops;
-
-	kvm_vgic_set_irq_ops(vcpu, timer_irq(map.direct_vtimer), ops);
-	if (map.direct_ptimer)
-		kvm_vgic_set_irq_ops(vcpu, timer_irq(map.direct_ptimer), ops);
 }
 
 /*
@@ -1600,6 +1589,7 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
 {
 	struct arch_timer_cpu *timer = vcpu_timer(vcpu);
 	struct timer_map map;
+	struct irq_ops *ops;
 	int ret;
 
 	if (timer->enabled)
@@ -1620,6 +1610,12 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
 
 	get_timer_map(vcpu, &map);
 
+	ops = vgic_is_v5(vcpu->kvm) ? &arch_timer_irq_ops_vgic_v5 :
+				      &arch_timer_irq_ops;
+
+	for (int i = 0; i < nr_timers(vcpu); i++)
+		kvm_vgic_set_irq_ops(vcpu, timer_irq(vcpu_get_timer(vcpu, i)), ops);
+
 	ret = kvm_vgic_map_phys_irq(vcpu,
 				    map.direct_vtimer->host_timer_irq,
 				    timer_irq(map.direct_vtimer));
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index e7d07752143be..36410f7cd2ad3 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -526,6 +526,15 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 
 	vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;
 
+	/* Set up the timer */
+	kvm_timer_vcpu_init(vcpu);
+
+	kvm_pmu_vcpu_init(vcpu);
+
+	kvm_arm_pvtime_vcpu_init(&vcpu->arch);
+
+	vcpu->arch.hw_mmu = &vcpu->kvm->arch.mmu;
+
 	/*
 	 * This vCPU may have been created after mpidr_data was initialized.
 	 * Throw out the pre-computed mappings if that is the case which forces
@@ -537,15 +546,6 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 	if (err)
 		return err;
 
-	/* Set up the timer */
-	kvm_timer_vcpu_init(vcpu);
-
-	kvm_pmu_vcpu_init(vcpu);
-
-	kvm_arm_pvtime_vcpu_init(&vcpu->arch);
-
-	vcpu->arch.hw_mmu = &vcpu->kvm->arch.mmu;
-
 	err = kvm_share_hyp(vcpu, vcpu + 1);
 	if (err)
 		kvm_vgic_vcpu_destroy(vcpu);

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