Kernel KVM virtualization development
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: sashiko-reviews@lists.linux.dev
Cc: kvm@vger.kernel.org, linux-hyperv@vger.kernel.org
Subject: Re: [PATCH v5 06/51] x86/sev: Don't override CPU frequency calibration for SNP's Secure TSC
Date: Wed, 1 Jul 2026 13:09:44 -0700	[thread overview]
Message-ID: <akV0CHxODhM4Q2zv@google.com> (raw)
In-Reply-To: <20260701195337.2E1F01F000E9@smtp.kernel.org>

On Wed, Jul 01, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The patch removes the `calibrate_cpu` override to allow the CPU base frequency to differ from the TSC frequency, but fails to account for `determine_cpu_tsc_frequencies()` which will forcefully clobber `cpu_khz` if it diverges by more than 10%.
> --
> 
> commit 6d3b9e81661ead5f25434666266aec6514dadaa8
> Author: Sean Christopherson <seanjc@google.com>
> 
> x86/sev: Don't override CPU frequency calibration for SNP's Secure TSC
> 
> This commit removes the override of the kernel's CPU frequency calibration
> routine when registering SNP's Secure TSC. It aims to decouple the CPU base
> frequency from the TSC frequency, as exposing APERF/MPERF requires an
> accurate CPU frequency.
> 
> > diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> > --- a/arch/x86/coco/sev/core.c
> > +++ b/arch/x86/coco/sev/core.c
> > @@ -2046,7 +2046,6 @@ void __init snp_secure_tsc_init(void)
> >  
> >  	snp_tsc_freq_khz = SNP_SCALE_TSC_FREQ(tsc_freq_mhz * 1000, secrets->tsc_factor);
> >  
> > -	x86_platform.calibrate_cpu = securetsc_get_tsc_khz;
> 
> [Severity: Medium]
> Will this removal be sufficient to decouple cpu_khz from tsc_khz if they
> diverge significantly?

No, but IMO it's a step in the right direction.

> Looking at determine_cpu_tsc_frequencies() in arch/x86/kernel/tsc.c, there
> is a generic sanity check that enforces a 10% limit on the difference
> between the two frequencies:
> 
> arch/x86/kernel/tsc.c:determine_cpu_tsc_frequencies() {
>     ...
>     else if (abs(cpu_khz - tsc_khz) * 10 > tsc_khz)
>         cpu_khz = tsc_khz;
>     ...
> }
> 
> If the true host CPU frequency diverges from the guest TSC frequency by more
> than 10%, won't this logic forcefully overwrite the correct cpu_khz back to
> tsc_khz, unintentionally undoing the fix for the APERF/MPERF calculation?

Probably, but this series is already boiling a pretty large lake, I think the
ocean can be left for a future cleanup.

  reply	other threads:[~2026-07-01 20:09 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-01 19:31 [PATCH v5 00/51] x86: Try to wrangle PV clocks vs. TSC Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 01/51] x86/apic: Provide helpers to set local APIC timer period in hz and khz Sean Christopherson
2026-07-01 19:46   ` sashiko-bot
2026-07-01 20:05     ` Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 02/51] x86/apic: Add CONFIG_X86_LOCAL_APIC=n stubs for apic_set_timer_period_{,k}hz() Sean Christopherson
2026-07-01 19:48   ` sashiko-bot
2026-07-01 19:31 ` [PATCH v5 03/51] x86/tsc: Ensure that TSC recalibration doesn't run if TSC frequency is known Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 04/51] x86/tsc: Restrict recalibrate_cpu_khz() export to p4-clockmod and powernow-k7 Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 05/51] x86/sev: Mark TSC as reliable when configuring Secure TSC Sean Christopherson
2026-07-01 19:56   ` sashiko-bot
2026-07-01 20:07     ` Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 06/51] x86/sev: Don't override CPU frequency calibration for SNP's " Sean Christopherson
2026-07-01 19:53   ` sashiko-bot
2026-07-01 20:09     ` Sean Christopherson [this message]
2026-07-01 19:31 ` [PATCH v5 07/51] x86/sev: Move check for SNP Secure TSC support to tsc_early_init() Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 08/51] x86/sev: Shove SNP's secure/trusted TSC frequency directly into "calibration" Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 09/51] x86/tsc: Add a standalone helper for getting TSC info from CPUID.0x15 Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 10/51] x86/tdx: Force TSC frequency with CPUID-based info provided by the TDX-Module Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 11/51] x86/tsc: Add dedicated hypervisor hooks for getting known TSC/CPU frequencies Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 12/51] x86/acrn: Register TSC/CPU frequency callbacks iff frequency is actually in CPUID Sean Christopherson
2026-07-01 19:52   ` sashiko-bot
2026-07-01 20:10     ` Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 13/51] x86/acrn: Mark TSC frequency as known when using ACRN for calibration Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 14/51] x86/tsc: Consolidate forcing of X86_FEATURE_TSC_KNOWN_FREQ for PV code Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 15/51] x86/tsc: Kill off x86_platform_ops.calibrate_{cpu,tsc}() hooks Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 16/51] x86/tsc: Rename pit_hpet_ptimer_calibrate_cpu() => native_calibrate_cpu_late() Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 17/51] x86/tsc: Fold native_calibrate_cpu() into recalibrate_cpu_khz() Sean Christopherson
2026-07-01 19:50   ` sashiko-bot
2026-07-01 19:31 ` [PATCH v5 18/51] x86/kvmclock: Rename kvm_get_tsc_khz() to kvmclock_get_tsc_khz() Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 19/51] x86/kvmclock: Drop dead check on TSC being unstable during kvmclock_init() Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 20/51] KVM: x86: Officially define CPUID 0x40000010 as PV Timing Info (TSC and Bus) Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 21/51] x86/kvm: Obtain TSC frequency from PV CPUID if present Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 22/51] x86/kvm: Mark TSC as reliable when it's constant and nonstop Sean Christopherson
2026-07-01 20:03   ` sashiko-bot
2026-07-01 20:13     ` Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 23/51] x86/tsc: Add standalone helper for getting CPU frequency from CPUID Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 24/51] x86/kvm: Get CPU base frequency from CPUID when it's available Sean Christopherson
2026-07-01 19:54   ` sashiko-bot
2026-07-01 19:31 ` [PATCH v5 25/51] clocksource: hyper-v: Register sched_clock save/restore iff it's necessary Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 26/51] clocksource: hyper-v: Drop wrappers to sched_clock save/restore helpers Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 27/51] clocksource: hyper-v: Don't save/restore TSC offset when using HV sched_clock Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 28/51] x86/kvmclock: Setup kvmclock for secondary CPUs iff CONFIG_SMP=y Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 29/51] x86/kvm: Don't disable kvmclock on BSP in syscore_suspend() Sean Christopherson
2026-07-01 20:03   ` sashiko-bot
2026-07-01 20:43     ` Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 30/51] x86/paravirt: Remove unnecessary PARAVIRT=n stub for paravirt_set_sched_clock() Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 31/51] x86/paravirt: Move handling of unstable PV clocks into paravirt_set_sched_clock() Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 32/51] x86/kvmclock: Move sched_clock save/restore helpers up in kvmclock.c Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 33/51] x86/xen/time: NOP-ify x86_platform's sched_clock save/restore hooks Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 34/51] x86/vmware: NOP-ify save/restore hooks when using VMware's sched_clock Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 35/51] x86/tsc: WARN if TSC sched_clock save/restore used with PV sched_clock Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 36/51] x86/paravirt: Pass sched_clock save/restore helpers during registration Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 37/51] x86/kvmclock: Move kvm_sched_clock_init() down in kvmclock.c Sean Christopherson
2026-07-01 19:31 ` [PATCH v5 38/51] x86/xen/time: Mark xen_setup_vsyscall_time_info() as __init Sean Christopherson
2026-07-01 19:32 ` [PATCH v5 39/51] x86/pvclock: Mark setup helpers and related various as __init/__ro_after_init Sean Christopherson
2026-07-01 19:32 ` [PATCH v5 40/51] x86/pvclock: WARN if pvclock's valid_flags are overwritten Sean Christopherson
2026-07-01 19:32 ` [PATCH v5 41/51] x86/kvmclock: Refactor handling of PVCLOCK_TSC_STABLE_BIT during kvmclock_init() Sean Christopherson
2026-07-01 19:32 ` [PATCH v5 42/51] timekeeping: Resume clocksources before reading persistent clock Sean Christopherson
2026-07-01 19:32 ` [PATCH v5 43/51] x86/kvmclock: Hook clocksource.suspend/resume when kvmclock isn't sched_clock Sean Christopherson
2026-07-01 19:32 ` [PATCH v5 44/51] x86/kvmclock: WARN if wall clock is read while kvmclock is suspended Sean Christopherson
2026-07-01 19:32 ` [PATCH v5 45/51] x86/paravirt: Mark __paravirt_set_sched_clock() as __init Sean Christopherson
2026-07-01 19:32 ` [PATCH v5 46/51] x86/paravirt: Plumb a return code into __paravirt_set_sched_clock() Sean Christopherson
2026-07-01 19:32 ` [PATCH v5 47/51] x86/paravirt: Don't use a PV sched_clock in CoCo guests with trusted TSC Sean Christopherson
2026-07-01 19:32 ` [PATCH v5 48/51] x86/kvmclock: Use TSC for sched_clock if it's constant and non-stop Sean Christopherson
2026-07-01 19:32 ` [PATCH v5 49/51] x86/kvmclock: Plumb in AP-online and BSP-resume to kvmlock, for documentation Sean Christopherson
2026-07-01 19:32 ` [PATCH v5 50/51] x86/paravirt: Move using_native_sched_clock() stub into timer.h Sean Christopherson
2026-07-01 19:32 ` [PATCH v5 51/51] x86/kvm: Get local APIC bus frequency from PV CPUID Timing Info Sean Christopherson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=akV0CHxODhM4Q2zv@google.com \
    --to=seanjc@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox