Re: [PATCH v3 26/36] KVM: arm64: gic-v5: Bump arch timer for GICv5
From: Jonathan Cameron <jonathan.cameron@huawei.com>
Date: 2026-01-12 16:28:21
Also in:
kvm, kvmarm
On Fri, 9 Jan 2026 17:04:47 +0000 Sascha Bischoff [off-list ref] 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 <redacted>
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>
quoted hunk ↗ jump to hunk
--- 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
quoted hunk ↗ jump to hunk
@@ -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!
quoted hunk ↗ jump to hunk
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; }