Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The command line parameter `tsc=unstable` is ignored because `check_tsc_unstable()` is evaluated before the parameter is parsed, incorrectly forcing `X86_FEATURE_TSC_RELIABLE`. --
commit c9259f3d89899a7c421eb7c7311cc995cd768eb4 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, and if the administrator hasn't explicitly marked it unstable. It prioritizes the TSC over kvmclock to avoid the overhead of emulated watchdog clocksources. > diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c > index 909d3e5e5bcd5..4fe9c69bf40b3 100644 > --- a/arch/x86/kernel/kvm.c > +++ b/arch/x86/kernel/kvm.c [ ... ] > @@ -1040,7 +1041,20 @@ static void __init kvm_init_platform(void) [ ... ] > - kvmclock_init(); > + /* > + * 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 > + * TSC reliable, as guaranteed by KVM. Note, the TSC unstable check > + * exists purely to honor the TSC being marked unstable via command > + * line, any runtime detection of an unstable will happen after this. > + */ > + tsc_is_reliable = boot_cpu_has(X86_FEATURE_CONSTANT_TSC) && > + boot_cpu_has(X86_FEATURE_NONSTOP_TSC) && > + !check_tsc_unstable(); [Severity: High] Does this evaluate check_tsc_unstable() too early to catch the command line parameter? It looks like kvm_init_platform() is called from setup_arch(), but the tsc=unstable kernel parameter is parsed via __setup() later during parse_args() in start_kernel(). If check_tsc_unstable() evaluates to 0 here because the parameter hasn't been parsed yet, wouldn't it incorrectly force X86_FEATURE_TSC_RELIABLE and set prefer_tsc to true? This could cause the system to use an unstable TSC for sched_clock(), which might lead to scheduling anomalies if the TSC jumps backwards. > + if (tsc_is_reliable) > + setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE); > + > + kvmclock_init(tsc_is_reliable); > x86_platform.apic_post_init = kvm_apic_init; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=17
