Re: [PATCH 2/3] x86/tsc: Fix erroneous TSC rate on Skylake Xeon

[Prarit Bhargava <prarit@redhat.com>]

On 12/22/2017 12:27 AM, Len Brown wrote:
> From: Len Brown <len.brown@intel.com>
> 
> Linux-4.9 added INTEL_FAM6_SKYLAKE_X to native_calibrate_tsc():
> 
> commit 6baf3d61821f
> ("x86/tsc: Add additional Intel CPU models to the crystal quirk list")
> 
> There are several problems with doing this.
> 
> The first is that while SKX servers use a 25 MHz crystal,
> SKX workstations (with same model #) use a 24 MHz crystal.
> This results in a -4.0% time drift rate on SKX workstations.
> 
> While SKX servers do have a 25  MHz crystal, but they too have a problem.
> All SKX subject the crystal to an EMI reduction circuit that
> reduces its actual frequency by (approximately) -0.25%.
> This results in -1 second per 10 minute time drift
> as compared to network time.
> 
> This issue can also trigger a timer and power problem,
> on configurations that use the LAPIC timer (versus the TSC deadline timer).
> Clock ticks scheduled with the LAPIC timer arrive a few usec
> before the time they are expected (according to the slow TSC).
> This causes Linux to poll-idle, when it should be in an idle
> power saving state.  The idle and clock code do not graciously
> recover from this error, sometimes resulting in significant polling
> and measurable power impact.
> 
> So stop using native_calibrate_tsc() for INTEL_FAM6_SKYLAKE_X.
> native_calibrate_tsc() will return 0, boot will run with
> tsc_khz = cpu_khz, and the TSC refined calibration will
> update tsc_khz to correct for the difference.
> 
> This patch restores correctness.  Without it, all three of the
> issues above occur.
> 
> Signed-off-by: Len Brown <len.brown@intel.com>
> Cc: Prarit Bhargava <prarit@redhat.com>
> Cc: <stable@vger.kernel.org> # v4.9+
> ---
>  arch/x86/kernel/tsc.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index ce4b71119c36..3bf4df7f52d7 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -602,7 +602,6 @@ unsigned long native_calibrate_tsc(void)
>  		case INTEL_FAM6_KABYLAKE_DESKTOP:
>  			crystal_khz = 24000;	/* 24.0 MHz */
>  			break;
> -		case INTEL_FAM6_SKYLAKE_X:
>  		case INTEL_FAM6_ATOM_DENVERTON:
>  			crystal_khz = 25000;	/* 25.0 MHz */
>  			break;
> 

Sorry, Len I didn't realize you had sent this multiple times.

I lifted this code directly from tools/power/x86/turbostat/turbostat.c:4155
-- is that code incorrect too?

P.