Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Bypassing `snp_secure_tsc_init()` when `tsc_early_khz` is provided omits critical SEV-SNP CPU capabilities, breaking TSC stability and security. --
commit 8e7d363bd0d5d42b37ec02afb4108ddb034a9988 Author: Sean Christopherson <[email protected]> x86/sev: Shove SNP's secure/trusted TSC frequency directly into "calibration" This commit modifies early TSC initialization to directly pass the secure TSC frequency obtained from SNP firmware to determine_cpu_tsc_frequencies(). It introduces logic to prioritize tsc_early_khz over other sources to maintain existing behavior. > 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. Without X86_FEATURE_TSC_RELIABLE, the TSC clocksource watchdog remains active. In a Confidential VM, an untrusted hypervisor could manipulate legacy external timers (HPET/PIT) to trigger the watchdog, falsely marking the TSC as unstable and forcing a fallback to untrusted time sources. > - if (!determine_cpu_tsc_frequencies(true)) > + if (!determine_cpu_tsc_frequencies(true, known_tsc_khz)) > return; > tsc_enable_sched_clock(); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
