Re: [PATCH v3 2/2] timer/linux/x86: override TSC freq if no tsc_known_freq

[Bruce Richardson <bruce.richardson@intel.com>]

On Tue, Oct 01, 2024 at 03:22:51AM +0300, Isaac Boukris wrote:
> If the tsc_known_freq cpu flag is missing, it means the kernel doesn't
> trust it and calculates its own. We should do the same to avoid drift.
> 
> Signed-off-by: Isaac Boukris <iboukris@gmail.com>
> ---
>  lib/eal/common/eal_common_timer.c |  3 +-
>  lib/eal/common/eal_private.h      |  2 +-
>  lib/eal/freebsd/eal_timer.c       |  5 ++-
>  lib/eal/linux/eal_timer.c         | 53 +++++++++++++++++++++++++++++--
>  lib/eal/windows/eal_timer.c       |  5 ++-
>  5 files changed, 60 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/eal/common/eal_common_timer.c b/lib/eal/common/eal_common_timer.c
> index c5c4703f15..e00be0a5c8 100644
> --- a/lib/eal/common/eal_common_timer.c
> +++ b/lib/eal/common/eal_common_timer.c
> @@ -66,8 +66,7 @@ set_tsc_freq(void)
>  	}
>  
>  	freq = get_tsc_freq_arch();
> -	if (!freq)
> -		freq = get_tsc_freq();
> +	freq = get_tsc_freq(freq);
>  	if (!freq)
>  		freq = estimate_tsc_freq();
>  
> diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
> index af09620426..bb315dab04 100644
> --- a/lib/eal/common/eal_private.h
> +++ b/lib/eal/common/eal_private.h
> @@ -374,7 +374,7 @@ void set_tsc_freq(void);
>   *
>   * This function is private to the EAL.
>   */
> -uint64_t get_tsc_freq(void);
> +uint64_t get_tsc_freq(uint64_t arch_hz);
>  
>  /**
>   * Get TSC frequency if the architecture supports.
> diff --git a/lib/eal/freebsd/eal_timer.c b/lib/eal/freebsd/eal_timer.c
> index 3dd70e24ba..5a8aea03e1 100644
> --- a/lib/eal/freebsd/eal_timer.c
> +++ b/lib/eal/freebsd/eal_timer.c
> @@ -26,12 +26,15 @@
>  enum timer_source eal_timer_source = EAL_TIMER_TSC;
>  
>  uint64_t
> -get_tsc_freq(void)
> +get_tsc_freq(uint64_t arch_hz)
>  {
>  	size_t sz;
>  	int tmp;
>  	uint64_t tsc_hz;
>  
> +	if (arch_hz)
> +		return arch_hz;
> +
>  	sz = sizeof(tmp);
>  	tmp = 0;
>  

On FreeBSD I'm not sure this is the best behaviour. On BSD we read the TSC
value from the kernel, which, one assumes, has measured it accurately.
Therefore I'd tend toward just using the kernel value in all cases, maybe
check the arch value (if non-zero) against that and warning if they have
significant divergence. WDYT?

/Bruce