From: Marc Zyngier <maz@kernel.org>
To: Suzuki K Poulose <suzuki.poulose@arm.com>
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 17:46:44 +0100 [thread overview]
Message-ID: <86o6dy5uob.wl-maz@kernel.org> (raw)
In-Reply-To: <20260915160141.3543048-2-suzuki.poulose@arm.com>
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.
>
> 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.
M.
--
Without deviation from the norm, progress is not possible.
next prev parent reply other threads:[~2026-09-15 16:46 UTC|newest]
Thread overview: 29+ 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 [this message]
2026-09-15 17:48 ` Suzuki K Poulose
2026-09-15 21:20 ` Suzuki K Poulose
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
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=86o6dy5uob.wl-maz@kernel.org \
--to=maz@kernel.org \
--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@krenel.org \
--cc=oupton@kernel.org \
--cc=sdonthineni@nvidia.com \
--cc=steven.price@arm.com \
--cc=suzuki.poulose@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®