linux-kernel - Re: Re: [PATCH 2/2] x86: use KNOWN_FREQ and RELIABLE TSC flags on certain processors/SoCs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.20.1611110012480.3501@nanos>
Date:   Fri, 11 Nov 2016 00:26:40 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Bin Gao <bin.gao@...ux.intel.com>
cc:     Ingo Molnar <mingo@...hat.com>, H Peter Anvin <hpa@...or.com>,
        x86@...nel.org, Peter Zijlstra <peterz@...radead.org>,
        linux-kernel@...r.kernel.org, Bin Gao <bin.gao@...el.com>
Subject: Re: Re: [PATCH 2/2] x86: use KNOWN_FREQ and RELIABLE TSC flags on
 certain processors/SoCs

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