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 <[email protected]> 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/[email protected]?part=20
