Re: [PATCH v18 01/23] KVM: arm64: protected VM: Handle set_one_reg CNTVCT_EL0/CNTPCT_EL0
From: Suzuki K Poulose
Date: Tue Sep 15 2026 - 17:23:54 EST
On 15/09/2026 18:48, Suzuki K Poulose wrote:
On 15/09/2026 17:46, Marc Zyngier wrote:
On Tue, 15 Sep 2026 17:01:19 +0100,
Suzuki K Poulose <suzuki.poulose@xxxxxxx> wrote:
Protected VMs doesn't allow setting offsets for virtual and phyiscal
counters, as the offset is always fixed to 0. The VM ioclt is filtered
out based on the cap. However we don't prevent the userspace from trying
to write to the CNTVCT/CNTPCT registers. This would lead to KVM triggering
a WARN() in timer_set_offset() as the vm_offset pointer is set to NULL.
Fix this by always "fixing" the timer offsets to 0 and marking that the
timer offset is set in the kvm->arch.flags at pKVM init time. The
userspace cannot use the KVM_ARM_SET_COUNTER_OFFSET, as it is blocked for a
protected VM.
A userspace writing to the SYS_CNT*CT would observe success, without
any real effect. This was chosen over preventing the writes to these
registers and returning -EPERM.
With that, we always have a valid vm_offset pointer, remove the checks for
vm_offset == NULL.
Reported by Sashiko here
https://lore.kernel.org/all/20260908164641.416911F00A3A@xxxxxxxxxxxxxxx
Fixes: f7d05ee84a6a ("KVM: arm64: Prevent host from managing timer offsets for protected VMs")
Suggested-by: Marc Zyngier <maz@xxxxxxxxxx>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
---
arch/arm64/kvm/arch_timer.c | 15 +++++----------
arch/arm64/kvm/arm.c | 15 +++++++++++++++
arch/arm64/kvm/hyp/nvhe/pkvm.c | 26 +++++++++++++-------------
include/kvm/arm_arch_timer.h | 3 +--
4 files changed, 34 insertions(+), 25 deletions(-)
diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
index 6ac3321f4c575..dda020da4c9c7 100644
--- a/arch/arm64/kvm/arch_timer.c
+++ b/arch/arm64/kvm/arch_timer.c
@@ -1079,14 +1079,10 @@ static void timer_context_init(struct kvm_vcpu *vcpu, int timerid)
ctxt->timer_id = timerid;
- if (!kvm_vm_is_protected(vcpu->kvm)) {
- if (timerid == TIMER_VTIMER)
- ctxt->offset.vm_offset = &kvm->arch.timer_data.voffset;
- else
- ctxt->offset.vm_offset = &kvm->arch.timer_data.poffset;
- } else {
- ctxt->offset.vm_offset = NULL;
- }
+ if (timerid == TIMER_VTIMER)
+ ctxt->offset.vm_offset = &kvm->arch.timer_data.voffset;
+ else
+ ctxt->offset.vm_offset = &kvm->arch.timer_data.poffset;
hrtimer_setup(&ctxt->hrtimer, kvm_hrtimer_expire, CLOCK_MONOTONIC, HRTIMER_MODE_ABS_HARD);
@@ -1110,8 +1106,7 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu)
timer_context_init(vcpu, i);
/* Synchronize offsets across timers of a VM if not already provided */
- if (!vcpu_is_protected(vcpu) &&
- !test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm- >arch.flags)) {
+ if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm- >arch.flags)) {
timer_set_offset(vcpu_vtimer(vcpu), kvm_phys_timer_read());
timer_set_offset(vcpu_ptimer(vcpu), 0);
}
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 8b080804bc90b..7c88508cac8a1 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -214,6 +214,20 @@ static int kvm_arm_default_max_vcpus(void)
return vgic_present ? kvm_vgic_get_max_vcpus() : KVM_MAX_VCPUS;
}
+/*
+ * Fix the counter offset to 0 for Protected VMs and mark the
+ * offset flag. The user can't set the offset via KVM_ARM_SET_COUNTER_OFFSET.
+ */
+static void kvm_arch_fix_timer_offsets(struct kvm *kvm)
+{
+ if (!kvm_vm_is_protected(kvm))
+ return;
+
+ /* Fix the counter offset to 0 and mark the offset initialised */
+ kvm->arch.timer_data.poffset = kvm->arch.timer_data.voffset = 0;
+ set_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &kvm->arch.flags);
+}
+
/**
* kvm_arch_init_vm - initializes a VM data structure
* @kvm: pointer to the KVM struct
@@ -267,6 +281,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
kvm_vgic_early_init(kvm);
+ kvm_arch_fix_timer_offsets(kvm);
kvm_timer_init_vm(kvm);
This should all be moved to the timer code.
/* The maximum number of VCPUs is limited by the host's GIC model */
diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/ nvhe/pkvm.c
index 459bd9eb7e4bc..e7b38eff63bd1 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -528,19 +528,19 @@ static int init_pkvm_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu,
hyp_vcpu->vcpu.arch.cflags = READ_ONCE(host_vcpu->arch.cflags);
hyp_vcpu->vcpu.arch.mp_state.mp_state = KVM_MP_STATE_STOPPED;
- if (!pkvm_hyp_vcpu_is_protected(hyp_vcpu)) {
- /*
- * Timer offsets are pointing to the untrusted KVM copy,
- * which is pinned in __pkvm_init_vm() for the VM life time.
- * It is worth noting that hyp_vm->host_kvm points to an EL2
- * linear map address and timer_get_offset() will use
- * kern_hyp_va() which is safe as it is idempotent.
- */
- vcpu_vtimer(&hyp_vcpu->vcpu)->offset.vm_offset =
- &hyp_vm->host_kvm->arch.timer_data.voffset;
- vcpu_ptimer(&hyp_vcpu->vcpu)->offset.vm_offset =
- &hyp_vm->host_kvm->arch.timer_data.poffset;
- }
+ /*
+ * Timer offsets are pointing to the untrusted KVM copy,
+ * which is pinned in __pkvm_init_vm() for the VM life time.
+ * It is worth noting that hyp_vm->host_kvm points to an EL2
+ * linear map address and timer_get_offset() will use
+ * kern_hyp_va() which is safe as it is idempotent.
+ * Also for protected VMs the offset is fixed to 0 and is prevented
+ * from changing.
+ */
+ vcpu_vtimer(&hyp_vcpu->vcpu)->offset.vm_offset =
+ &hyp_vm->host_kvm->arch.timer_data.voffset;
+ vcpu_ptimer(&hyp_vcpu->vcpu)->offset.vm_offset =
+ &hyp_vm->host_kvm->arch.timer_data.poffset;
I don't think this is right. Protected guests have no offset, and this
needs to be ensured by the hypervisor. Here, the host can change the
offset any time it wants, and that's not acceptable.
Ah, you're right. :facepalm:
ret = pkvm_vcpu_init_sysregs(hyp_vcpu);
if (ret)
diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
index bc6f2fdd7ad33..4f0aa3bb69f45 100644
--- a/include/kvm/arm_arch_timer.h
+++ b/include/kvm/arm_arch_timer.h
@@ -176,8 +176,7 @@ static inline bool has_cntpoff(void)
if (__ctxt) { \
struct arch_timer_offset *ato = &__ctxt->offset;\
\
- if (ato->vm_offset) \
- off += *KERN_HYP_VA(ato->vm_offset); \
+ off += *KERN_HYP_VA(ato->vm_offset); \
if (ato->vcpu_offset) \
off += *KERN_HYP_VA(ato->vcpu_offset); \
} \
And as you drop the previous hunk, this also needs to be restored to
its original state.
Ack.
Suzuki
This is how it looks like now, reworked.
diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
index 6ac3321f4c575..1c8fac222a643 100644
--- a/arch/arm64/kvm/arch_timer.c
+++ b/arch/arm64/kvm/arch_timer.c
@@ -1110,8 +1110,7 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu)
timer_context_init(vcpu, i);
/* Synchronize offsets across timers of a VM if not already provided */
- if (!vcpu_is_protected(vcpu) &&
- !test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags)) {
+ if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags)) {
timer_set_offset(vcpu_vtimer(vcpu), kvm_phys_timer_read());
timer_set_offset(vcpu_ptimer(vcpu), 0);
}
@@ -1133,6 +1132,15 @@ void kvm_timer_init_vm(struct kvm *kvm)
*/
for (int i = 0; i < NR_KVM_TIMERS; i++)
kvm->arch.timer_data.ppi[i] = get_vgic_ppi(kvm, default_ppi[i]);
+
+ /*
+ * For the protected VMs, fix the counter offset to 0 by marking the
+ * KVM_ARCH_FLAG_VM_COUNTER_OFFSET. This would also prevent userspace
+ * writes to CNTVCT_ELO & CNTPCT_EL0.
+ * The user can't set the offset via vm ioctl KVM_ARM_SET_COUNTER_OFFSET.
+ */
+ if (kvm_vm_is_protected(kvm))
+ set_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &kvm->arch.flags);
}
void kvm_timer_cpu_up(void)