Re: [PATCH v4 17/47] x86/kvm: Mark TSC as reliable when it's constant and nonstop

[David Woodhouse <dwmw2@xxxxxxxxxxxxx>]

On Fri, 29 May 2026 11:57:34 -0700, Sean Christopherson wrote:
> On Fri, May 29, 2026, sashiko-bot@xxxxxxxxxx wrote:
> > > 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?
> 
> Huh, it does indeed.
> 
> > 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?
> 
> Yep, but this is a pre-existing problem that goes all the way back to the 
> original
> commit 7539b174aef4 ("x86: kvmguest: use TSC clocksource if invariant TSC is 
> exposed").
> 
> We could try to fix that, but I'm _very_ strongly inclined to add (yet 
> another)
> patch to simply drop the check_tsc_unstable() since it has always been dead 
> code.

Yeah, kill it with fire.

Reviewed-by: David Woodhouse <dwmw@xxxxxxxxxxxx>

Attachment: smime.p7s
Description: S/MIME cryptographic signature