Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756124AbcKJX3p (ORCPT ); Thu, 10 Nov 2016 18:29:45 -0500 Received: from Galois.linutronix.de ([146.0.238.70]:34577 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751057AbcKJX3o (ORCPT ); Thu, 10 Nov 2016 18:29:44 -0500 Date: Fri, 11 Nov 2016 00:26:40 +0100 (CET) From: Thomas Gleixner To: Bin Gao cc: Ingo Molnar , H Peter Anvin , x86@kernel.org, Peter Zijlstra , linux-kernel@vger.kernel.org, Bin Gao Subject: Re: Re: [PATCH 2/2] x86: use KNOWN_FREQ and RELIABLE TSC flags on certain processors/SoCs In-Reply-To: <20161110232001.GB217763@worksta> Message-ID: References: <1478020482-231459-1-git-send-email-bin.gao@intel.com> <1478020482-231459-3-git-send-email-bin.gao@intel.com> <4460FA1017EA3844B646E90DA4E984057E2ECB85@ORSMSX112.amr.corp.intel.com> <20161110232001.GB217763@worksta> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1722 Lines: 45 On Thu, 10 Nov 2016, Bin Gao wrote: > > > @@ -702,6 +702,15 @@ unsigned long native_calibrate_tsc(void) > > > } > > > } > > > > > > + setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ); > > > > I can understand the one below, but this one changes existing behaviour w/o explaining why this is correct and desired. If at all then this wants to be a seperate patch and not just mingled in your goldmont update. > > native_calibrate_tsc() implements determining TSC frequency via CPUID. > The purpose to add X86_FEATURE_TSC_KNOWN_FREQ flag is exactly for this case: > TSC frequency determined via CPUID or MSR are always correct and the whole > calibration should be skipped. Did you actually verify that this is correct and does not introduce NTP issues compared to the long term calibration on such platforms? We've been burnt before and myself and others wasted enough time already debugging that crap. > I will create a seperate patch for this to ensure it's not confusing with > the MSR related change below. Yes please. > > > @@ -100,5 +100,9 @@ unsigned long cpu_khz_from_msr(void) #ifdef > > > CONFIG_X86_LOCAL_APIC > > > lapic_timer_frequency = (freq * 1000) / HZ; #endif > > > + > > > + setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ); > > > + setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE); > > > > Why is this automatically reliable and of known frequency? > > As I said above, TSC frequency determined by CPUID or MSR is always considered > "known" because it is reported by HW. > Regarding the reliable, unfortunately however, there is no a HW way to report > it. We were told by silicon design team it's "reliable". Please add a comment which explains this in great length. Thanks, tglx