[PATCH 08/13] KVM: arm64: Move CNT*CT_EL0 userspace accessors to generic infrastructure

[Joey Gouly]

On Tue, Sep 30, 2025 at 01:05:05PM +0100, Marc Zyngier wrote:
> On Tue, 30 Sep 2025 11:45:52 +0100,
> Joey Gouly <joey.gouly at arm.com> wrote:
> > 
> > Observation below
> > 
> >   |
> >   v
> > 
> > On Mon, Sep 29, 2025 at 05:04:52PM +0100, Marc Zyngier wrote:
> > > Moving the counter registers is a bit more involved than for the control
> > > and comparator (there is no shadow data for the counter), but still
> > > pretty manageable.
> > > 
> > > Signed-off-by: Marc Zyngier <maz at kernel.org>
> > > ---
> > >  arch/arm64/kvm/guest.c    |  7 -------
> > >  arch/arm64/kvm/sys_regs.c | 34 +++++++++++++++++++++++++++++++---
> > >  2 files changed, 31 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> > > index c23ec9be4ce27..138e5e2dc10c8 100644
> > > --- a/arch/arm64/kvm/guest.c
> > > +++ b/arch/arm64/kvm/guest.c
> > > @@ -592,19 +592,12 @@ static unsigned long num_core_regs(const struct kvm_vcpu *vcpu)
> > >  }
> > >  
> > >  static const u64 timer_reg_list[] = {
> > > -	KVM_REG_ARM_TIMER_CNT,
> > > -	KVM_REG_ARM_PTIMER_CNT,
> > >  };
> > >  
> > >  #define NUM_TIMER_REGS ARRAY_SIZE(timer_reg_list)
> > >  
> > >  static bool is_timer_reg(u64 index)
> > >  {
> > > -	switch (index) {
> > > -	case KVM_REG_ARM_TIMER_CNT:
> > > -	case KVM_REG_ARM_PTIMER_CNT:
> > > -		return true;
> > > -	}
> > >  	return false;
> > >  }
> > >  
> > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > > index 68e88d5c0dfb5..e67eb39ddc118 100644
> > > --- a/arch/arm64/kvm/sys_regs.c
> > > +++ b/arch/arm64/kvm/sys_regs.c
> > > @@ -1605,12 +1605,38 @@ static int arch_timer_set_user(struct kvm_vcpu *vcpu,
> > >  	case SYS_CNTHP_CTL_EL2:
> > >  		val &= ~ARCH_TIMER_CTRL_IT_STAT;
> > >  		break;
> > > +	case SYS_CNTVCT_EL0:
> > > +		if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags))
> > > +			timer_set_offset(vcpu_vtimer(vcpu), kvm_phys_timer_read() - val);
> > > +		return 0;
> > > +	case SYS_CNTPCT_EL0:
> > > +		if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags))
> > > +			timer_set_offset(vcpu_ptimer(vcpu), kvm_phys_timer_read() - val);
> > > +		return 0;
> > >  	}
> > >  
> > >  	__vcpu_assign_sys_reg(vcpu, rd->reg, val);
> > >  	return 0;
> > >  }
> > >  
> > > +static int arch_timer_get_user(struct kvm_vcpu *vcpu,
> > > +			       const struct sys_reg_desc *rd,
> > > +			       u64 *val)
> > > +{
> > > +	switch (reg_to_encoding(rd)) {
> > > +	case SYS_CNTVCT_EL0:
> > > +		*val = kvm_phys_timer_read() - timer_get_offset(vcpu_vtimer(vcpu));
> > > +		break;
> > > +	case SYS_CNTPCT_EL0:
> > > +		*val = kvm_phys_timer_read() - timer_get_offset(vcpu_ptimer(vcpu));
> > > +		break;
> > > +	default:
> > > +		*val = __vcpu_sys_reg(vcpu, rd->reg);
> > 
> > Unsure if this is actually an issue but for the _CTL registers, via
> > access_arch_timer() (kvm_arm_timer_read_sysreg() -> .. -> read_timer_ctl()),
> > the ARCH_TIMER_CTRL_IT_STAT bit will be set if the timer expired, but that's
> > not done here.
> 
> Indeed, but I don't think this really matters, at least not for
> save/restore. We always clear the ISTATUS bit on restore, and
> snapshoting CTL is always a racy process.

Makes sense, so:

Reviewed-by: Joey Gouly <joey.gouly at arm.com>

Thanks,
Joey

> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.