Re: [PATCH v4 20/47] x86/kvm: Get CPU base frequency from CPUID when it's available

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The KVM patch attempts to decouple the virtual CPU base frequency from the TSC frequency by explicitly reading CPUID.0x16, but this intent is defeated by the core x86 timekeeping code that forcibly clamps `cpu_khz` to `tsc_khz` if they differ by more than 10%.
--

commit 399b0f26d4487d316fd05edb4e7df98c8da6205f
Author: Sean Christopherson <seanjc@google.com>

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

This commit uses CPUID.0x16 to retrieve the CPU base frequency in KVM instead
of assuming it matches the TSC frequency. It introduces kvm_get_cpu_khz and
hooks it into x86_init.hyper.get_cpu_khz during platform initialization.

> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c

[ ... ]

> @@ -1049,6 +1055,14 @@ static void __init kvm_init_platform(void)
>  #endif
>  	}
>  
> +	/*
> +	 * 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 the CPU frequency actually remain decoupled from the TSC frequency when
they diverge significantly?

During early boot, tsc_early_init() calls this hook and passes the value
down to determine_cpu_tsc_frequencies() in arch/x86/kernel/tsc.c. Inside
determine_cpu_tsc_frequencies(), there is a clamp that forces cpu_khz to
match tsc_khz if they differ by more than 10%:

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

If the CPU base frequency from CPUID.0x16 and the TSC frequency differ by
more than 10%, doesn't this unconditional clamp silently overwrite the newly
populated cpu_khz, reverting to the old behavior that this patch aims to fix?

>          /*
>           * If the TSC counts at a constant frequency across P/T states, counts
>           * in deep C-states, and the TSC hasn't been marked unstable, treat the

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529144435.704127-1-seanjc@google.com?part=20