[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <858qqvwl4w.fsf@amd.com>
Date: Tue, 28 Jan 2025 05:41:19 +0000
From: Nikunj A Dadhania <nikunj@....com>
To: Sean Christopherson <seanjc@...gle.com>
CC: Borislav Petkov <bp@...en8.de>, <linux-kernel@...r.kernel.org>,
<thomas.lendacky@....com>, <x86@...nel.org>, <kvm@...r.kernel.org>,
<mingo@...hat.com>, <tglx@...utronix.de>, <dave.hansen@...ux.intel.com>,
<pgonda@...gle.com>, <pbonzini@...hat.com>, <francescolavra.fl@...il.com>,
Alexey Makhalov <alexey.makhalov@...adcom.com>, Juergen Gross
<jgross@...e.com>, Boris Ostrovsky <boris.ostrovsky@...cle.com>
Subject: Re: [PATCH v16 12/13] x86/tsc: Switch to native sched clock
Sean Christopherson <seanjc@...gle.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
Powered by blists - more mailing lists