Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Medium] The patch removes the `calibrate_cpu` override to allow the CPU base 
frequency to differ from the TSC frequency, but fails to account for 
`determine_cpu_tsc_frequencies()` which will forcefully clobber `cpu_khz` if it 
diverges by more than 10%.
--

commit 6d3b9e81661ead5f25434666266aec6514dadaa8
Author: Sean Christopherson <[email protected]>

x86/sev: Don't override CPU frequency calibration for SNP's Secure TSC

This commit removes the override of the kernel's CPU frequency calibration
routine when registering SNP's Secure TSC. It aims to decouple the CPU base
frequency from the TSC frequency, as exposing APERF/MPERF requires an
accurate CPU frequency.

> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -2046,7 +2046,6 @@ void __init snp_secure_tsc_init(void)
>  
>       snp_tsc_freq_khz = SNP_SCALE_TSC_FREQ(tsc_freq_mhz * 1000, 
> secrets->tsc_factor);
>  
> -     x86_platform.calibrate_cpu = securetsc_get_tsc_khz;

[Severity: Medium]
Will this removal be sufficient to decouple cpu_khz from tsc_khz if they
diverge significantly?

Looking at determine_cpu_tsc_frequencies() in arch/x86/kernel/tsc.c, there
is a generic sanity check that enforces a 10% limit on the difference
between the two frequencies:

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 true host CPU frequency diverges from the guest TSC frequency by more
than 10%, won't this logic forcefully overwrite the correct cpu_khz back to
tsc_khz, unintentionally undoing the fix for the APERF/MPERF calculation?

>       x86_platform.calibrate_tsc = securetsc_get_tsc_khz;
>  
>       early_memunmap(mem, PAGE_SIZE);

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

Reply via email to