Re: [PATCH v4 07/47] x86/tdx: Force TSC frequency with CPUID-based info provided by the TDX-Module

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Providing the 'tsc_early_khz=' boot parameter silently bypasses TDX-specific APIC timer and TSC capability initializations, causing a feature regression.
--

commit 8a9e5abad4e72a5ef6c47191b8815b94bbed98c9
Author: Sean Christopherson <seanjc@google.com>

x86/tdx: Force TSC frequency with CPUID-based info provided by the TDX-Module

This commit updates the TSC frequency initialization for TDX guests to use
trustworthy CPUID-based information provided by the TDX-Module instead of
relying on the hypervisor. It also relocates the TSC_RELIABLE capability
setup from tdx_early_init() to a new tdx_tsc_init() routine.

> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 29b6f1ed59ec5..5d79763592201 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c

[ ... ]

> @@ -1123,9 +1124,6 @@ void __init tdx_early_init(void)
>  
>  	setup_force_cpu_cap(X86_FEATURE_TDX_GUEST);
>  
> -	/* TSC is the only reliable clock in TDX guest */
> -	setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> -
>  	cc_vendor = CC_VENDOR_INTEL;

[ ... ]

> 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.

While using tsc_early_khz is an explicit user override, skipping
tdx_tsc_init() completely drops the X86_FEATURE_TSC_RELIABLE capability.
Could this cause the clocksource watchdog to flag the TSC as unstable and
break timekeeping for configurations that rely on this parameter?

>  
>  	if (!determine_cpu_tsc_frequencies(true, known_tsc_khz))
>  		return;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529144435.704127-1-seanjc@google.com?part=7