[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 10:48:59 PDT 2026
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
>
> M.
>
More information about the linux-arm-kernel
mailing list