Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 23 Apr 2019 11:08:49 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Daniel Drake <drake@endlessm.com>
Cc:     tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, hpa@zytor.com,
        x86@kernel.org, linux-kernel@vger.kernel.org, len.brown@intel.com,
        rafael.j.wysocki@intel.com, linux@endlessm.com
Subject: Re: [PATCH 1/2] x86/tsc: use CPUID.0x16 to calculate missing crystal
 frequency
Message-ID: <20190423090849.GH11158@hirez.programming.kicks-ass.net>
References: <20190422101526.28415-1-drake@endlessm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190422101526.28415-1-drake@endlessm.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Apr 22, 2019 at 06:15:25PM +0800, Daniel Drake wrote:
> Additionally, crystal clock frequency for platforms that were missing
> from the list (e.g. SKYLAKE_X) will now be provided.

Well, SKX isn't exactly 'missing'; it would be very good if we can
confirm the new code is still correct under the below mentioned
conditions.

---

commit b511203093489eb1829cb4de86e8214752205ac6
Author: Len Brown <len.brown@intel.com>
Date:   Fri Dec 22 00:27:55 2017 -0500

    x86/tsc: Fix erroneous TSC rate on Skylake Xeon
    
    The INTEL_FAM6_SKYLAKE_X hardcoded crystal_khz value of 25MHZ is
    problematic:
    
     - SKX workstations (with same model # as server variants) use a 24 MHz
       crystal.  This results in a -4.0% time drift rate on SKX workstations.
    
     - SKX servers 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.
    
    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.
    
    [ tglx: Sanitized change log ]
    
    Fixes: 6baf3d61821f ("x86/tsc: Add additional Intel CPU models to the crystal quirk list")
    Signed-off-by: Len Brown <len.brown@intel.com>
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Cc: peterz@infradead.org
    Cc: Prarit Bhargava <prarit@redhat.com>
    Cc: stable@vger.kernel.org
    Link: https://lkml.kernel.org/r/ff6dcea166e8ff8f2f6a03c17beab2cb436aa779.1513920414.git.len.brown@intel.com

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;