[PATCH v18 01/23] KVM: arm64: protected VM: Handle set_one_reg CNTVCT_EL0/CNTPCT_EL0
Suzuki K Poulose
suzuki.poulose at arm.com
Tue Sep 15 14:20:36 PDT 2026
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 at arm.com> 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@smtp.kernel.org
>>>
>>> Fixes: f7d05ee84a6a ("KVM: arm64: Prevent host from managing timer
>>> offsets for protected VMs")
>>> Suggested-by: Marc Zyngier <maz at krenel.org>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose at arm.com>
>>> ---
>>> 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)
More information about the linux-arm-kernel
mailing list