Re: [PATCH v3 40/41] x86/tsc: Add standalone helper for getting CPU frequency from CPUID

[Paolo Bonzini <pbonzini@xxxxxxxxxx>]

On 5/15/26 21:19, Sean Christopherson wrote:
> Extract the guts of cpu_khz_from_cpuid() to a standalone helper that
> doesn't restrict the usage to Intel CPUs.  This will allow sharing the
> core logic with kvmclock, as (a) CPUID.0x16 may be enumerated alongside
> kvmclock, and (b) KVM generally doesn't restrict CPUID based on vendor.
Even for native there's no real reason to restrict to Intel, I think. 
native_calibrate_tsc() only limits itself because historically (prior to 
commit 604dc9170f24, "x86/tsc: Use CPUID.0x16 to calculate missing 
crystal frequency", 2019-05-09) it used a hardcoded table of crystal 
frequencies.
Of course paranoia applies, but for virtualization, if the leaf exists 
there is no reason not to trust it.
Thanks,

Paolo

> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
>   arch/x86/include/asm/tsc.h |  1 +
>   arch/x86/kernel/tsc.c      | 37 +++++++++++++++++++++++--------------
>   2 files changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
> index f458be688512..c145f5707b52 100644
> --- a/arch/x86/include/asm/tsc.h
> +++ b/arch/x86/include/asm/tsc.h
> @@ -91,6 +91,7 @@ struct cpuid_tsc_info {
>   };
>   extern int cpuid_get_tsc_info(struct cpuid_tsc_info *info);
>   extern int cpuid_get_tsc_freq(struct cpuid_tsc_info *info);
> +extern int cpuid_get_cpu_freq(unsigned int *cpu_khz);
>   
>   extern void tsc_early_init(void);
>   extern void tsc_init(void);
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 1b569954ae5e..745fa2052c74 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -719,6 +719,24 @@ int cpuid_get_tsc_freq(struct cpuid_tsc_info *info)
>         return 0;
>   }
>   
> +int cpuid_get_cpu_freq(unsigned int *cpu_khz)
> +{
> +       unsigned int eax_base_mhz, ebx, ecx, edx;
> +
> +       *cpu_khz = 0;
> +
> +       if (boot_cpu_data.cpuid_level < CPUID_LEAF_FREQ)
> +               return -ENOENT;
> +
> +       cpuid(CPUID_LEAF_FREQ, &eax_base_mhz, &ebx, &ecx, &edx);
> +
> +       if (!eax_base_mhz)
> +               return -ENOENT;
> +
> +       *cpu_khz = eax_base_mhz * 1000;
> +       return 0;
> +}
> +
>   /**
>    * native_calibrate_tsc - determine TSC frequency
>    * Determine TSC frequency via CPUID, else return 0.
> @@ -754,13 +772,8 @@ unsigned long native_calibrate_tsc(void)
>          * clock, but we can easily calculate it to a high degree of accuracy
>          * by considering the crystal ratio and the CPU speed.
>          */
> -       if (!info.crystal_khz && boot_cpu_data.cpuid_level >= CPUID_LEAF_FREQ) {
> -               unsigned int eax_base_mhz, ebx, ecx, edx;
> -
> -               cpuid(CPUID_LEAF_FREQ, &eax_base_mhz, &ebx, &ecx, &edx);
> -               info.crystal_khz = eax_base_mhz * 1000 *
> -                       info.denominator / info.numerator;
> -       }
> +       if (!info.crystal_khz && !cpuid_get_cpu_freq(&cpu_khz))
> +               info.crystal_khz = cpu_khz * info.denominator / info.numerator;
>   
>   	if (!info.crystal_khz)
>                 return 0;
> @@ -787,19 +800,15 @@ unsigned long native_calibrate_tsc(void)
>   
>   static unsigned long cpu_khz_from_cpuid(void)
>   {
> -       unsigned int eax_base_mhz, ebx_max_mhz, ecx_bus_mhz, edx;
> +       unsigned int cpu_khz;
>   
>   	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
>                 return 0;
>   
> -	if (boot_cpu_data.cpuid_level < CPUID_LEAF_FREQ)
> +       if (cpuid_get_cpu_freq(&cpu_khz))
>                 return 0;
>   
> -	eax_base_mhz = ebx_max_mhz = ecx_bus_mhz = edx = 0;
> -
> -       cpuid(CPUID_LEAF_FREQ, &eax_base_mhz, &ebx_max_mhz, &ecx_bus_mhz, &edx);
> -
> -       return eax_base_mhz * 1000;
> +       return cpu_khz;
>   }
>   
>   /*