linux-kernel - Re: [v3] x86/tsc: Unset TSC_KNOWN_FREQ and TSC

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]

Date:   Thu, 30 Jan 2020 09:54:48 +0100
From:   Hans de Goede <hdegoede@...hat.com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     vipul kumar <vipulk0511@...il.com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        linux-kernel@...r.kernel.org, Stable <stable@...r.kernel.org>,
        Srikanth Krishnakar <Srikanth_Krishnakar@...tor.com>,
        Cedric Hombourger <Cedric_Hombourger@...tor.com>,
        x86@...nel.org, Len Brown <len.brown@...el.com>,
        Vipul Kumar <vipul_kumar@...tor.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [v3] x86/tsc: Unset TSC_KNOWN_FREQ and TSC_RELIABLE flags on
 Intel Bay Trail SoC

Hi,

On 29-01-2020 21:57, Thomas Gleixner wrote:
> Andy,
> 
> Andy Shevchenko <andriy.shevchenko@...ux.intel.com> writes:
>> On Wed, Jan 29, 2020 at 04:13:39PM +0100, Thomas Gleixner wrote:
>>> Andy, can you please make sure that people inside Intel who can look
>>> into the secrit documentation confirm what we are aiming for?
>>>
>>> Ideally they should provide the X-tal frequency and the mult/div pair
>>> themself :)
>>
>> So, I don't have access to the CPU core documentation (and may be will not be
>> given), nevertheless I dug a bit to what I have for Cherrytrail. So, the XTAL
>> is 19.2MHz, which becomes 100MHz and 1600MHz by some root PLL, then, the latter
>> two frequencies are being used by another PLL to provide a reference clock (*)
>> to PLL which derives CPU clock.
>>
>> *) According to colleagues of mine it's a fixed rate source.
>>
>> That's all what I have.
> 
> I'm surely not blaming you for this, you're just the messenger.
> 
> Just to make it entirely clear. We are wasting days already due to the
> fact that Intel, who designs, specifies and most importantly sells these
> CPUs is either unable or unwilling to provide accurate information about
> the trivial and essential information to support these CPUs:
> 
>      1) The crystal frequency
> 
>      2) The nominator/denominator pair to calculate the TSC frequency
>         from #1
> 
> The numbers which are in the kernel have been provided by Intel, but
> they are inaccurate as we have proven.
> 
> Sure, we can reverse engineer the exact numbers assumed that we have
> access to all variants of affected devices and enough spare time to
> waste.
> 
> But why should we do that?
> 
> Intel has the exact numbers at their fingertip and is just not providing
> them for whatever reasons (I really don't want to know).
> 
> So instead of wasting our precious time further, I'm going to apply the
> patch below unless Intel comes forth with the information they should
> have provided many years ago.

Thomas, although I fully agree with your sentiment here, especially since
I've been spending pretty much the entirety of my day on this for the last
2 days, I do not think such a patch would be of great service to our end-users...

Between your initial "model the PLL" idea and Andy's provided info I've
come up with a patch which although not pretty I believe addresses this.

I'm running some final tests now and then I will post the patch series
for this upstream.

Regards,

Hans






> 
> Thanks,
> 
>          tglx
> 
> 8<--------------
>   arch/x86/kernel/tsc_msr.c |    9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> --- a/arch/x86/kernel/tsc_msr.c
> +++ b/arch/x86/kernel/tsc_msr.c
> @@ -73,6 +73,13 @@ static const struct x86_cpu_id tsc_msr_c
>   	{}
>   };
>   
> +static char msr_warning[] = \
> +	"The TSC/APIC timer frequency for your CPU is guesswork.\n\n"	\
> +	"It is derived from frequency tables provided by Intel.\n"	\
> +	"These tables are demonstrably inaccurate, but Intel is\n"	\
> +	"either unable or unwilling to provide the correct data.\n"	\
> +	"Please report this to Intel and not on LKML.\n";
> +
>   /*
>    * MSR-based CPU/TSC frequency discovery for certain CPUs.
>    *
> @@ -90,6 +97,8 @@ unsigned long cpu_khz_from_msr(void)
>   	if (!id)
>   		return 0;
>   
> +	WARN_ONCE(1, "%s\n", msr_warning);
> +
>   	freq_desc = (struct freq_desc *)id->driver_data;
>   	if (freq_desc->msr_plat) {
>   		rdmsr(MSR_PLATFORM_INFO, lo, hi);
>