Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Medium] The base CPU frequency obtained from CPUID.0x16 is unconditionally 
clamped back to the TSC frequency if it differs by >10%, defeating the purpose 
of the patch.
--

commit 1331572140e49219bdcc6ceabc0370555476adde
Author: Sean Christopherson <[email protected]>

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

This commit reads CPUID.0x16 to fetch the CPU base frequency if valid.
It then sets x86_init.hyper.get_cpu_khz to kvm_get_cpu_khz to avoid
assuming the CPU base frequency matches the TSC frequency.

> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 1cef54e1e7d9e..6c7011ff7bd13 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c

[ ... ]

> @@ -1041,6 +1047,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 new CPU frequency actually be used if it diverges significantly
from the TSC frequency? 

Looking at determine_cpu_tsc_frequencies() in arch/x86/kernel/tsc.c, there
is existing logic that unconditionally clamps the CPU frequency back to the
TSC frequency if they differ by more than 10%:

        else if (abs(cpu_khz - tsc_khz) * 10 > tsc_khz)
                cpu_khz = tsc_khz;

Does this clamping defeat the purpose of this patch for guests running on
hosts where the base CPU frequency and TSC frequency differ by >10%?

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

Reply via email to