On Fri, May 15, 2026, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
> to consider:
> - [Medium] Spurious WARN_ON stack trace on every boot for CoCo (SEV-SNP/TDX) 
> guests under KVM.
> --
> 
> commit e57255e4d076143a67d0e6e44f914aec31aed8c7
> Author: Sean Christopherson <[email protected]>
> 
> x86/tsc: Rejects attempts to override TSC calibration with lesser routine
> 
> This commit adds a sanity check when registering a TSC frequency calibration
> routine to ensure the incoming routine is as robust as the outgoing one,
> rejecting it with a warning if it is not. This prevents replacing a trusted
> calibration source with a less trusted one.
> 
> > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> > index 98bef1d06fa9d..7a261214fa3ef 100644
> > --- a/arch/x86/kernel/tsc.c
> > +++ b/arch/x86/kernel/tsc.c
> > @@ -1319,8 +1319,13 @@ void tsc_register_calibration_routines(unsigned long 
> > (*calibrate_tsc)(void),
> >  
> >     if (properties & TSC_FREQUENCY_KNOWN)
> >             setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
> > +   else if (WARN_ON(boot_cpu_has(X86_FEATURE_TSC_KNOWN_FREQ)))
> > +           return;
> > +
> >     if (properties & TSC_RELIABLE)
> >             setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> > +   else if (WARN_ON(boot_cpu_has(X86_FEATURE_TSC_RELIABLE)))
> > +           return;
> 
> Will this cause a spurious warning stack trace on every boot for CoCo
> (SEV-SNP/TDX) guests under KVM?
> 
> For KVM CoCo guests, CoCo registers first via tsc_early_init(), setting both
> TSC_FREQUENCY_KNOWN and X86_FEATURE_TSC_RELIABLE.
> 
> KVM then registers second via kvmclock_init(). At this point in the patch
> series, KVM only passes TSC_FREQUENCY_KNOWN, so the properties & TSC_RELIABLE
> check is false, and it falls through to this WARN_ON. Since CoCo already set
> the capability, the condition evaluates to true.
> 
> I see this is fixed later in the series by the commit "x86/kvmclock: Mark TSC
> as reliable when it's constant and nonstop", which modifies KVM to pass
> TSC_FREQ_KNOWN_AND_RELIABLE on modern hardware, preventing this warning.
> 
> Could the patch ordering be adjusted to prevent this intermediate regression?

Ah shoot, no.  Well, not entirely.  Because when running as a CoCo guest with a
trusted TSC, the kernel needs to prevent overwriting the TSC calibration, 
*period*.
I.e. changing the ordering will eliminate the unwanted WARN, but it won't fix 
the
underlying goof that the trusted calibration routines are still being 
overwritten
with untrusted routines.

Hrm, the SNP secure TSC code complicates things, but I suspect it's broken.  If
it's indeed broken, then I think we can add a TSC_TRUSTED flag and then do:

        if (WARN_ON_ONCE(!calibrate_tsc))
                return;

        if (properties & TSC_FREQUENCY_KNOWN)
                setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
        else if (WARN_ON(boot_cpu_has(X86_FEATURE_TSC_KNOWN_FREQ)))
                return;

        if (properties & TSC_RELIABLE)
                setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
        else if (WARN_ON(boot_cpu_has(X86_FEATURE_TSC_RELIABLE)))
                return;

        if (!cpu_has_trusted_tsc() || (properties & TSC_TRUSTED))
                x86_platform.calibrate_tsc = calibrate_tsc;

        if (calibrate_cpu)
                x86_platform.calibrate_cpu = calibrate_cpu;


Tom / Nikunj,

Isn't it completely wrong to assume the CPU frequency is the same as the TSC
frequency?  The changelog says the difference "does not apply", but that makes
no sense.

    Use the GUEST_TSC_FREQ MSR to discover the TSC frequency instead of
    relying on kvm-clock based frequency calibration.  Override both CPU and
    TSC frequency calibration callbacks with securetsc_get_tsc_khz(). Since
    the difference between CPU base and TSC frequency does not apply in this
    case, the same callback is being used.

E.g. if the host passed through APERF/MPERF, then the difference most definitely
applies.  If TSC != CPU frequency, then knowingly using bad data is even worse
(far, far worse) than hoping the untrusted host is playing nice.

If the TSC and CPU frequencies are somehow guaranteed to be the same (which I
can't possibly imagine is the case), then the above won't work because we also
want to prevent overriding calibrate_cpu().

Reply via email to