Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Unconditionally forcing X86_FEATURE_TSC_RELIABLE bypasses SMP 
cross-CPU TSC synchronization checks, which will cause time to go backwards for 
guests running on hosts with unsynchronized TSCs.
--

commit fb8fd2f8d18aabaf24f424391748d58e9427bdf7
Author: Sean Christopherson <[email protected]>

x86/kvm: Mark TSC as reliable when it's constant and nonstop

This commit marks the TSC as reliable if KVM enumerates it as constant
and nonstop. This aims to prevent the TSC from occasionally being marked
unstable when the host is under moderate or heavy load.

> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 909d3e5e5bcd5..1cef54e1e7d9e 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
[ ... ]
> @@ -1040,7 +1041,16 @@ static void __init kvm_init_platform(void)
>               }
>       }
>  
> -     kvmclock_init();
> +        /*
> +         * 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.
> +         */
> +     tsc_is_reliable = boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
> +                       boot_cpu_has(X86_FEATURE_NONSTOP_TSC);
> +     if (tsc_is_reliable)
> +             setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);

[Severity: High]
Does setting X86_FEATURE_TSC_RELIABLE here bypass SMP cross-CPU TSC
synchronization checks?

Forcing X86_FEATURE_TSC_RELIABLE based only on CONSTANT_TSC and NONSTOP_TSC,
without verifying if the hypervisor actually guarantees cross-CPU
synchronization (e.g., via KVM_FEATURE_CLOCKSOURCE_STABLE_BIT), sets
tsc_clocksource_reliable = 1.

This bypasses the synchronization check in check_tsc_sync_source():

arch/x86/kernel/tsc_sync.c:check_tsc_sync_source() {
    ...
    if (tsc_store_and_check_tsc_adjust(false) || tsc_clocksource_reliable)
        return;
    ...
}

Could this cause time to go backwards for guests running on hosts with
unsynchronized TSCs when threads migrate between vCPUs?

> +
> +     kvmclock_init(tsc_is_reliable);
>       x86_platform.apic_post_init = kvm_apic_init;
>  
>       /*

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

Reply via email to