From: Suzuki K Poulose <suzuki.poulose@arm.com>
To: Marc Zyngier <maz@kernel.org>
Cc: kvm@vger.kernel.org, kvmarm@lists.linux.dev, will@kernel.org,
catalin.marinas@arm.com, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, steven.price@arm.com,
aneesh.kumar@kernel.org, oupton@kernel.org, gshan@redhat.com,
joey.gouly@arm.com, tabba@google.com, yuzenghui@huawei.com,
linux-coco@lists.linux.dev, gankulkarni@os.amperecomputing.com,
sdonthineni@nvidia.com, alpergun@google.com,
fj0570is@fujitsu.com, WeiLin.Chang@arm.com,
lpieralisi@kernel.org, enju.kohei@fujitsu.com,
Marc Zyngier <maz@krenel.org>
Subject: Re: [PATCH v18 01/23] KVM: arm64: protected VM: Handle set_one_reg CNTVCT_EL0/CNTPCT_EL0
Date: Tue, 15 Sep 2026 22:20:36 +0100 [thread overview]
Message-ID: <a4e7e6c3-d325-4f35-a17c-e02791cca326@arm.com> (raw)
In-Reply-To: <7c26c860-0543-459e-974b-72c59af261f9@arm.com>
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@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@krenel.org>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@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)
next prev parent reply other threads:[~2026-09-15 21:20 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-15 16:01 [PATCH v18 00/23] KVM: arm64: CCA: Add basic plumbing for Realms Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 01/23] KVM: arm64: protected VM: Handle set_one_reg CNTVCT_EL0/CNTPCT_EL0 Suzuki K Poulose
2026-09-15 16:46 ` Marc Zyngier
2026-09-15 17:48 ` Suzuki K Poulose
2026-09-15 21:20 ` Suzuki K Poulose [this message]
2026-09-16 8:16 ` Marc Zyngier
2026-09-16 8:27 ` Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 02/23] KVM: arm64: Disable Steal time accounting for protected guests Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 03/23] KVM: arm64: Include kvm_emulate.h in kvm/arm_psci.h Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 04/23] KVM: arm64: Avoid including linux/kvm_host.h in kvm_pgtable.h Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 05/23] KVM: arm64: Track the type of VM in kvm_arch Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 06/23] KVM: arm64: Refactor the vcpu_load to allow for VM specific callbacks Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 07/23] KVM: arm64: Add vcpu load/put call backs for flavors Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 08/23] KVM: arm64: Reuse kvm_stage2_unmap_range in kvm_unmap_gfn_range Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 09/23] KVM: arm64: Add VM specific callback for S2 MMU operations Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 10/23] KVM: arm64: Abstract out memory abort handling Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 11/23] KVM: arm64: Use kvm_vm_is_unprotected() for !kvm_vm_is_protected() Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 12/23] KVM: arm64: Widen the scope of "protected" VMs Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 13/23] KVM: arm64: Add a helper for VMs running on hyp that don't trust the host Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 14/23] KVM: arm64: CCA: Add a new mode for supporting Realm guests Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 15/23] KVM: arm64: CCA: Add VCPU load/put for Realms Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 16/23] KVM: arm64: CCA: Add bare minimal S2 operations for Realm Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 17/23] KVM: arm64: CCA: Introduce Realms Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 18/23] KVM: arm64: CCA: Mandate VGIC_V3 for Realms Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 19/23] KVM: arm64: CCA: Support timers in realm RECs Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 20/23] KVM: arm64: CCA: Don't expose unsupported capabilities for realm guests Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 21/23] KVM: arm64: CCA: WARN on injected undef exceptions Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 22/23] KVM: arm64: CCA: Expose SVE VL register before VCPU finalization Suzuki K Poulose
2026-09-15 16:01 ` [PATCH v18 23/23] KVM: arm64: CCA: Control user register access for Realms Suzuki K Poulose
2026-09-16 19:23 ` [PATCH v18 00/23] KVM: arm64: CCA: Add basic plumbing " Mathieu Poirier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a4e7e6c3-d325-4f35-a17c-e02791cca326@arm.com \
--to=suzuki.poulose@arm.com \
--cc=WeiLin.Chang@arm.com \
--cc=alpergun@google.com \
--cc=aneesh.kumar@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=enju.kohei@fujitsu.com \
--cc=fj0570is@fujitsu.com \
--cc=gankulkarni@os.amperecomputing.com \
--cc=gshan@redhat.com \
--cc=joey.gouly@arm.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.linux.dev \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-coco@lists.linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=lpieralisi@kernel.org \
--cc=maz@kernel.org \
--cc=maz@krenel.org \
--cc=oupton@kernel.org \
--cc=sdonthineni@nvidia.com \
--cc=steven.price@arm.com \
--cc=tabba@google.com \
--cc=will@kernel.org \
--cc=yuzenghui@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®