On Fri, May 29, 2026, [email protected] wrote:
> > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> > index 833eed5c048a9..2b8f94c3fcc7f 100644
> > --- a/arch/x86/kernel/tsc.c
> > +++ b/arch/x86/kernel/tsc.c
> [ ... ]
> > @@ -1537,16 +1538,20 @@ static void __init tsc_enable_sched_clock(void)
> >  
> >  void __init tsc_early_init(void)
> >  {
> > +   unsigned int known_tsc_khz = 0;
> > +
> >     if (!boot_cpu_has(X86_FEATURE_TSC))
> >             return;
> >     /* Don't change UV TSC multi-chassis synchronization */
> >     if (is_early_uv_system())
> >             return;
> >  
> > -   if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
> > -           snp_secure_tsc_init();
> > +   if (tsc_early_khz)
> > +           known_tsc_khz = tsc_early_khz;
> > +   else if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
> > +           known_tsc_khz = snp_secure_tsc_init();
> 
> [Severity: High]
> Does skipping snp_secure_tsc_init() when tsc_early_khz is provided omit
> critical CPU capabilities?
> 
> In the pre-patch code, snp_secure_tsc_init() was called unconditionally to
> set up hardware capabilities before determining frequencies:
> 
> arch/x86/coco/sev/core.c:snp_secure_tsc_init() {
>       ...
>       setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
>       setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
>       ...
> }
> 
> By chaining this into an else-if block, if tsc_early_khz is provided
> (e.g., via the kernel command line), snp_secure_tsc_init() is bypassed.

Which is bizarre and awful, but in practice is effectively what was already
happening, because the kernel would simply never invoke 
x86_platform.calibrate_tsc().

I have no objection to ignoring tsc_early_khz when running as a CoCo guest with
a secure/trusted TSC, but if we want to go that route, I'd like to do it as a
patch on top of everything else, so that it can be easily reverted if it turns
out someone is somehow depending on that combination.

Reply via email to