Re: [PATCH v6 24/51] x86/kvm: Get CPU base frequency from CPUID when it's available

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch's intent to decouple CPU base frequency from TSC frequency is silently defeated by core x86 timekeeping logic if the explicitly enumerated frequencies diverge by more than 10%.
--

--- Patch [24]: [PATCH v6 24/51] x86/kvm: Get CPU base frequency from CPUID when it's available ---
commit 950176e6b76f307f2a6acf272049d92426a49766
Author: Sean Christopherson <[email protected]>

x86/kvm: Get CPU base frequency from CPUID when it's available

If CPUID.0x16 is present and valid, use the CPU frequency provided by
CPUID instead of assuming that the virtual CPU runs at the same
frequency as TSC and/or kvmclock.

> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 255a24a99f288..83b9351f28100 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
[ ... ]
> @@ -1043,6 +1049,14 @@ static void __init kvm_init_platform(void)
>  		}
>  	}
>  
> +	/*
> +	 * Prefer CPUID.0x16 over KVM's PV CPUID when possible, as the base CPU
> +	 * frequency isn't necessarily the same as the TSC frequency.
> +	 */
> +	kvm_cpu_khz_cpuid = __cpu_khz_from_cpuid();
> +	if (kvm_cpu_khz_cpuid)
> +		x86_init.hyper.get_cpu_khz = kvm_get_cpu_khz;
> +
[Severity: Medium]
Will this actually decouple the CPU base frequency from the TSC frequency if
they diverge by more than 10%?

Looking at determine_cpu_tsc_frequencies() in arch/x86/kernel/tsc.c, it seems
there is a legacy sanity check that overrides the explicitly enumerated CPU
frequency if it diverges too much from the TSC frequency:

arch/x86/kernel/tsc.c:determine_cpu_tsc_frequencies() {
...
    else if (abs(cpu_khz - tsc_khz) * 10 > tsc_khz)
        cpu_khz = tsc_khz;
...
}

During boot, KVM sets x86_init.hyper.get_cpu_khz to return the CPUID.0x16
frequency. The core x86 tsc_early_init() reads this and passes it to
determine_cpu_tsc_frequencies().

Doesn't this mean the core timekeeping logic will silently overwrite the
explicitly enumerated CPU frequency with the TSC frequency in these cases,
neutralizing the intent of this patch?

>  	/*
>  	 * If the TSC counts at a constant frequency across P/T states and in
>  	 * deep C-states, treat the TSC reliable, as guaranteed by KVM.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=24