linux-kernel - Re: [PATCH] x86/tsc: Use hard-coded crystal clock for Skylake mobile

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 11 May 2020 09:38:34 +0200
From:   Paul Menzel <pmenzel@...gen.mpg.de>
To:     Thomas Gleixner <tglx@...utronix.de>, x86@...nel.org
Cc:     Radoslaw Biernacki <biernacki@...gle.com>,
        Ross Zwisler <zwisler@...gle.com>,
        Daniel Drake <drake@...lessm.com>,
        Andy Lutomirski <luto@...nel.org>,
        Borislav Petkov <bp@...en8.de>,
        "H . Peter Anvin" <hpa@...or.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Len Brown <len.brown@...el.com>, linux@...lessm.com,
        "Rafael J . Wysocki" <rafael.j.wysocki@...el.com>,
        Thorsten Leemhuis <regressions@...mhuis.info>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86/tsc: Use hard-coded crystal clock for Skylake mobile

Dear Thomas,


Thank you for the quick reply.

Am 11.05.20 um 09:17 schrieb Thomas Gleixner:

> Paul Menzel <pmenzel@...gen.mpg.de> writes:
> 
> please send patches to LKML and not offlist.

Sorry about that. From `MAINTAINERS` I thought x86@...nel.org is wanted. 
Other subsystems list LKML explicitly there.

>> From: Radoslaw Biernacki <biernacki@...gle.com>
>>
>> @@ -636,10 +636,24 @@ unsigned long native_calibrate_tsc(void)
>>   	 * Denverton SoCs don't report crystal clock, and also don't support
>>   	 * CPUID.0x16 for the calculation below, so hardcode the 25MHz crystal
>>   	 * clock.
>> +	 * Also estimation code is not reliable and gives 1.5%  difference for
>> +	 * tsc/clock ratio on Skylake mobile. Therefore below is a hardcoded
>> +	 * crystal frequency for Skylake which was removed by upstream commit
>> +	 * "x86/tsc: Use CPUID.0x16 to calculate missing crystal frequency"
>> +	 * This is temporary workaround for bugs:
>> +	 * b/148108096, b/154283905, b/146787525, b/153400677, b/148178929
>> +	 * chromium/1031054
>>   	 */
>> -	if (crystal_khz == 0 &&
>> -			boot_cpu_data.x86_model == INTEL_FAM6_ATOM_GOLDMONT_D)
>> -		crystal_khz = 25000;
>> +	if (crystal_khz == 0) {
>> +		switch (boot_cpu_data.x86_model) {
>> +		case INTEL_FAM6_SKYLAKE_MOBILE:
>> +			crystal_khz = 24000;	/* 24.0 MHz */
>> +			break;
>> +		case INTEL_FAM6_ATOM_GOLDMONT_X:
>> +			crystal_khz = 25000;	/* 25.0 MHz */
>> +			break;
>> +		}
> 
> Aside of being a workaround for Google issues which are probably caused > by broken BIOSes

Even if it was caused by broken firmware, wouldn’t Linux’ no regression 
policy still consider this a regression as user should be able to the 
Linux kernel “no matter what”?

> that patch is broken.
> 
>     INTEL_FAM6_ATOM_GOLDMONT_D != INTEL_FAM6_ATOM_GOLDMONT_X

Good catch. The commit didn’t apply cleanly to the master branch, and I 
missed this.

I’ll wait for Radoslaw to comment before proceeding further with this.


Kind regards,

Paul