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

[Vincent Donnefort]

[...]

> 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:

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


> +}
> diff --git a/arch/arm64/kvm/hyp/nvhe/setup.c b/arch/arm64/kvm/hyp/nvhe/setup.c
> index 75b00c323310..970c5cf342f5 100644
> --- a/arch/arm64/kvm/hyp/nvhe/setup.c
> +++ b/arch/arm64/kvm/hyp/nvhe/setup.c
> @@ -10,6 +10,7 @@
>  #include <asm/kvm_pgtable.h>
>  #include <asm/kvm_pkvm.h>
>  
> +#include <nvhe/clock.h>
>  #include <nvhe/early_alloc.h>
>  #include <nvhe/ffa.h>
>  #include <nvhe/gfp.h>
> @@ -320,6 +321,10 @@ void __noreturn __pkvm_init_finalise(void)
>  	if (ret)
>  		goto out;
>  
> +	ret = hyp_clock_init();
> +	if (ret)
> +		goto out;
> +
>  	ret = fix_host_ownership();
>  	if (ret)
>  		goto out;
> -- 
> 2.55.0.141.g00534a21ce-goog
>