Re: [PATCH v7 01/24] KVM: arm64: Add a generic clock

[Mostafa Saleh <smostafa@google.com>]

On Wed, Jul 15, 2026 at 02:48:50PM +0100, Vincent Donnefort wrote:
> [...]
> 
> > diff --git a/arch/arm64/kvm/hyp/nvhe/clock.c b/arch/arm64/kvm/hyp/nvhe/clock.c
> > index f3e2619db4e4..43d2cba4f810 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/clock.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/clock.c
> > @@ -8,7 +8,12 @@
> >  
> >  #include <asm/arch_timer.h>
> >  #include <asm/div64.h>
> > +#include <linux/math64.h>
> > +#include <vdso/time64.h>
> >  
> > +static u32 timer_freq;
> > +
> > +#ifdef CONFIG_NVHE_EL2_TRACING
> >  static struct clock_data {
> >  	struct {
> >  		u32 mult;
> > @@ -66,3 +71,27 @@ u64 trace_hyp_clock(void)
> >  
> >  	return (u64)ns + clock->data[bank].epoch_ns;
> >  }
> > +#endif /* CONFIG_NVHE_EL2_TRACING */
> > +
> > +int hyp_clock_init(void)
> > +{
> > +	timer_freq = read_sysreg(cntfrq_el0);
> > +	/*
> > +	 * KVM will not initialize if FW didn't set cntfrq_el0, that is already
> > +	 * part of the boot protocol.
> > +	 */
> > +	if (!timer_freq)
> > +		return -ENODEV;
> > +
> > +	/* Timer freq can't be larger than 1Ghz by spec. */
> > +	if (timer_freq > NSEC_PER_SEC)
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +
> > +/* Return time in ns. */
> > +u64 hyp_clock_ns(void)
> > +{
> > +	return mul_u64_u32_div(__arch_counter_get_cntvct(), NSEC_PER_SEC, timer_freq);
> 
> IIUC, this will overflow the u64 mult very quickly (in few minutes) and also I
> see that we don't need such small nanoseconds accuracy.
> 
> So here we could always fallback to 128-bits mult... or update the epoch from
> time to time. But I have something completely different to propose:
> 

It should not overflow because mul_u64_u32_div() handles this.

> Instead of using a "clock" how about we just modify smmu_wait?
> 
> #define smmu_wait() {
>     static u32 window = arch_timer_get_cntfrq() / ARM_SMMU_EL2_POLL_TIMEOUT_US
>     u64 timeout = __arch_counter_get_cntvct() + window;
>     u64 cur;
> 
>     ...
> 
>     while (!(__cond)) {
>       ...
> 
>       cur = __arch_counter_get_cntvct();
>       if (cur >= timeout)
>           __ret = -ETIMEOUT;
> 
> 
> No risk of overflowing u64
> No init necessary

It believe init will be needed as caching arch_timer_get_cntfrq()
is much better that re-reading everytime. But I can make all of this
contained in the SMMUv3 driver and remove this patch.

Thanks,
Mostafa