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 <[email protected]> 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/[email protected]?part=7
