Re: [PATCH v19 01/20] KVM: arm64: protected VM: Handle user writes to CNTVCT_EL0/CNTPCT_EL0

[Suzuki K Poulose]

On 22/09/2026 20:25, Jonathan Cameron wrote:
> On Sun, 20 Sep 2026 22:28:26 +0100
> Suzuki K Poulose <suzuki.poulose@xxxxxxx> wrote:
>
> Hi Suzuki,
>
> > Protected VMs doesn't allow setting offsets for virtual and phyiscal
>
> physical
>
> > counters, as the offset is always fixed to 0. The VM ioclt is filtered
>
> ioctl
>
> > 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 KVM init time for protected
> > VMs. A userspace writing to the CNT*CT_EL0 would observe success, without
> > any real effect. This was chosen over preventing the writes to these
> > registers and returning -EPERM.
>
> Why?  I don't mind the decision but telling us what was chosen is something
> we can see in the code - patch description should give us the stuff we
> can't see.
>
> One comment on the comment below.
> > Reported by Sashiko
> >
> > Link: 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>
> > ---
> > Changes since v18:
> >   - Retain NULL vm_offset for protected VMs to avoid host tampering with the
> >     offset.
> >   - Moved the flag setting into kvm_timer_init_vm(), where it should have been
> >     in the first place
> > ---
> >   arch/arm64/kvm/arch_timer.c | 12 ++++++++++--
> >   1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> > index 6ac3321f4c575..226cd5a495c8b 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]);
> > +
> > +	/*
> > +	 * Protected VMs don't allow any offset being set from userspace,
> > +	 * either set via writes to the counters or using the dedicated
> This sentence confused me.  Second clause isn't obviously the ways that
> are being blocked. Maybe shorten to:
>
> 	Protected VMs don't allow the offset to be set from userspace,
> 	whether via writes to the counters or the dedicated ioctl.

LLM suggests:


   * Protected VMs don't allow userspace to set counter offsets,
   * either via counter register writes or the dedicated ioctl.

Have updated the comment.

Cheers
Suzuki

> > +	 * ioctl. Pretend the offset has already been set and rely on the
> > +	 * default offset being 0.
> > +	 */
> > +	if (kvm_vm_is_protected(kvm))
> > +		set_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &kvm->arch.flags);
> >   }
> >   
> >   void kvm_timer_cpu_up(void)