Re: [PATCH v16 12/13] x86/tsc: Switch to native sched clock

Nikunj A Dadhania
2025-01-28T05:41:30Z

Sean Christopherson <seanjc@google.com> writes:

>
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 0864b314c26a..9baffb425386 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -663,7 +663,12 @@ unsigned long native_calibrate_tsc(void)
> unsigned int eax_denominator, ebx_numerator, ecx_hz, edx;
> unsigned int crystal_khz;
>
> - if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
> + /*
> + * Ignore the vendor when running as a VM, if the hypervisor provides
> + * garbage CPUID information then the vendor is also suspect.
> + */
> + if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL &&
> + !boot_cpu_has(X86_FEATURE_HYPERVISOR))
> return 0;
>
> if (boot_cpu_data.cpuid_level < 0x15)
> @@ -713,10 +718,13 @@ unsigned long native_calibrate_tsc(void)
> return 0;
>
> /*
> - * For Atom SoCs TSC is the only reliable clocksource.
> - * Mark TSC reliable so no watchdog on it.
> + * For Atom SoCs TSC is the only reliable clocksource. Similarly, in a
> + * VM, any watchdog is going to be less reliable than the TSC as the
> + * watchdog source will be emulated in software. In both cases, mark
> + * the TSC reliable so that no watchdog runs on it.
> */
> - if (boot_cpu_data.x86_vfm == INTEL_ATOM_GOLDMONT)
> + if (boot_cpu_has(X86_FEATURE_HYPERVISOR) ||
> + boot_cpu_data.x86_vfm == INTEL_ATOM_GOLDMONT)
> setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);

One more point here is for AMD guests, TSC will not be marked reliable
as per the above change, it will only be effective for CPUs supporting
CPUID 15H/16H. Although, the watchdog should be stopped for AMD guests
as well.

Will it make sense to move this before cpuid_level check ?

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index e7abcc4d02c3..2769d1598c0d 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -672,6 +672,14 @@ unsigned long native_calibrate_tsc(void)
!boot_cpu_has(X86_FEATURE_HYPERVISOR))
return 0;

+ /*
+ * In a VM, any watchdog is going to be less reliable than the TSC as
+ * the watchdog source will be emulated in software. Mark the TSC
+ * reliable so that no watchdog runs on it.
+ */
+ if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
+ setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
+
if (boot_cpu_data.cpuid_level < CPUID_LEAF_TSC)
return 0;

@@ -719,13 +727,10 @@ unsigned long native_calibrate_tsc(void)
return 0;

/*
- * For Atom SoCs TSC is the only reliable clocksource. Similarly, in a
- * VM, any watchdog is going to be less reliable than the TSC as the
- * watchdog source will be emulated in software. In both cases, mark
- * the TSC reliable so that no watchdog runs on it.
+ * For Atom SoCs TSC is the only reliable clocksource.
+ * Mark TSC reliable so no watchdog on it.
*/
- if (boot_cpu_has(X86_FEATURE_HYPERVISOR) ||
- boot_cpu_data.x86_vfm == INTEL_ATOM_GOLDMONT)
+ if (boot_cpu_data.x86_vfm == INTEL_ATOM_GOLDMONT)
setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);

#ifdef CONFIG_X86_LOCAL_APIC

Regards
Nikunj