On Fri, May 29, 2026, [email protected] wrote:
> > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> > index 2b8f94c3fcc7f..2603f136e29bd 100644
> > --- a/arch/x86/kernel/tsc.c
> > +++ b/arch/x86/kernel/tsc.c
>
> [ ... ]
>
> > @@ -1550,6 +1551,8 @@ void __init tsc_early_init(void)
> > known_tsc_khz = tsc_early_khz;
> > else if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
> > known_tsc_khz = snp_secure_tsc_init();
> > + else if (boot_cpu_has(X86_FEATURE_TDX_GUEST))
> > + known_tsc_khz = tdx_tsc_init();
>
> [Severity: Medium]
> Does this structure accidentally bypass the TDX APIC timer and TSC capability
> initializations if the user provides the tsc_early_khz boot parameter?
>
> Previously, setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE) was executed
> unconditionally in tdx_early_init() for TDX guests. With this change,
> tdx_tsc_init() is skipped entirely when tsc_early_khz is provided due to
> the if/else if chain.
Drat. But this isn't benign. I don't see a better option than biting the
bullet
and ignoring tsc_early_khz for Coco guests, e.g. in the SNP patch do:
if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
known_tsc_khz = snp_secure_tsc_init();
if (!known_tsc_khz)
known_tsc_khz = tsc_early_khz;
else if (tsc_early_khz)
pr_err("Ignoring 'tsc_early_khz' in favor of trusted
firmware\n");
and then this becomes:
if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
known_tsc_khz = snp_secure_tsc_init();
else if (boot_cpu_has(X86_FEATURE_TDX_GUEST))
known_tsc_khz = tdx_tsc_init();
if (!known_tsc_khz)
known_tsc_khz = tsc_early_khz;
else if (tsc_early_khz)
pr_err("Ignoring 'tsc_early_khz' in favor of trusted
firmware\n");
At that point, I think it makes sense to double down and ignore tsc_early_khz if
the hypervisor provides the frequency. It'll yield cleaner code overall, and
will be easy enough to document, e.g. to end up with:
if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
known_tsc_khz = snp_secure_tsc_init();
else if (boot_cpu_has(X86_FEATURE_TDX_GUEST))
known_tsc_khz = tdx_tsc_init();
/*
* If the TSC frequency wasn't provided by trusted firmware, try to get
* it from the hypervisor (which is untrusted when running as a CoCo
guest).
*/
if (!known_tsc_khz && x86_init.hyper.get_tsc_khz)
known_tsc_khz = x86_init.hyper.get_tsc_khz();
/*
* Mark the TSC frequency as known if it was obtained from a hypervisor
* or trusted firmware. Don't mark the frequency as known if the user
* specified the frequency, as the user-provided frequency is intended
* as a "starting point", not a known, guaranteed frequency.
*/
if (known_tsc_khz)
setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
if (!known_tsc_khz)
known_tsc_khz = tsc_early_khz;
else if (tsc_early_khz)
pr_err("Ignoring 'tsc_early_khz' in favor of trusted firmware
or hypervisor\n");
