Message-ID: <30d49be8-67ad-6f32-37a8-0cdd26f0852e@redhat.com>
Date:   Tue, 28 Jan 2020 19:57:51 +0100
From:   Hans de Goede <hdegoede@...hat.com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Hans de Goede <hdegoede@...hat.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        vipul kumar <vipulk0511@...il.com>
Cc:     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>
Subject: Re: [v3] x86/tsc: Unset TSC_KNOWN_FREQ and TSC_RELIABLE flags on
 Intel Bay Trail SoC

Hi,

On 1/24/20 12:55 PM, Thomas Gleixner wrote:
> Hans,
> 
> Hans de Goede <hdegoede@...hat.com> writes:
>> On 1/24/20 9:35 AM, Thomas Gleixner wrote:
>>> Where does that number come from? Just math?
>>
>> Yes just math, but perhaps the Intel folks can see if they can find some
>> datasheet to back this up ?
> 
> Can you observe the issue on one of the machines in your zoo as well?

Ok, I have been testing this on various devices and I'm pretty sure now
that my initial hunch is correct. The problem is that the accuracy of
the FSB frequency as listed in the Intel docs is not so great:

The "Intel 64 and IA-32 Architectures Software Developer’s Manual Volume 4:
Model-Specific Registers" has the following table for the values from
freq_desc_byt:

    000B: 083.3 MHz
    001B: 100.0 MHz
    010B: 133.3 MHz
    011B: 116.7 MHz
    100B: 080.0 MHz

Notice how for e.g the 83.3 MHz value there are 3 significant digits,
which translates to an accuracy of a 1000 ppm, where as your typical
crystal oscillator is 20 - 100 ppm, so the accuracy of the frequency
format used in the Software Developer’s Manual is not really helpful.

So the 00 part of 83300 which I'm suggesting to replace with 33 in
essence is not specified and when the tsc_msr.c code was written /
Bay Trail support was added the value from the datasheet was simply
padded with zeros.

There is already a hint that that likely is not correct in the values
from the Software Developer’s Manual, we have values ending at 3.3,
but also at 6.7, which to me feels like it is 6.66666666666667 rounded
up and thus the 3.3 likely is 3.33333333333333.

Test 1: Intel(R) Celeron(R) CPU  N2840  @ 2.16GHz"
--------------------------------------------------

As said I've also ran some tests. The first device I have tested is
a HP stream 11 x360 with an "Intel(R) Celeron(R) CPU  N2840  @ 2.16GHz"
(from /proc/cpuinfo) this is the "laptop' version of Bay Trail rather
then the tablet version, so like Vipul's case I can comment out the 2
lines setting the TSC_KNOWN_FREQ and TSC_RELIABLE flags and get
"Refined TSC clocksource calibration". I've also added the changes with
the extra pr_info calls which you requested. Here is the relevant output
from a kernel with the 2 flags commented out + your pr_info changes,
note I changed the REF_CLOCK format from %x to %d as that seems easier
to interpret to me.

[    0.000000] MSR_PINFO: 0000060000001a00 -> 26
[    0.000000] MSR_FSBF: 0000000000000000
[    0.000000] REF_CLOCK: 83000
[    0.000000] tsc: Detected 2165.800 MHz processor
[    3.586805] tsc: Refined TSC clocksource calibration: 2166.666 MHz

And with my suggested change:

[    0.000000] MSR_PINFO: 0000060000001a00 -> 26
[    0.000000] MSR_FSBF: 0000000000000000
[    0.000000] REF_CLOCK: 83333
[    0.000000] tsc: Detected 2166.658 MHz processor
[    3.587326] tsc: Refined TSC clocksource calibration: 2166.667 MHz

Note we are still 0.009 MHz of from the refined calibration, so my
suggestion to really fix this would be to change the freqs part
of struct freq_desc to be in Hz rather then KHz and then calculate
res as:

res = DIV_ROUND_CLOSEST(freq * ratio, 1000); /* res is in KHz */

Which would give us:

[    0.000000] tsc: Detected 2166.667 MHz processor


Test 2: "Intel(R) Atom(TM) CPU  Z3736F @ 1.33GHz"
-------------------------------------------------

Second device tested: HP Pavilion x2 Detachable 10" version
with Bay Trail SoC: "Intel(R) Atom(TM) CPU  Z3736F @ 1.33GHz".

Relevant log messages, unpatched:
[    0.000000] MSR_PINFO: 0000060000001000 -> 16
[    0.000000] MSR_FSBF: 0000000000000000
[    0.000000] REF_CLOCK: 83000
[    0.000000] tsc: Detected 1332.800 MHz processor

Patched:
[    0.000000] MSR_PINFO: 0000060000001000 -> 16
[    0.000000] MSR_FSBF: 0000000000000000
[    0.000000] REF_CLOCK: 83333
[    0.000000] tsc: Detected 1333.328 MHz processor

Now since we do not have another clock source, we do not
know for sure that the 1333.328 MHz is better then the
original 1332.800, but it does seem to be a more logical
value; and from the N2840 @ 2.16GHz string, which runs
at 2166.667 MHz we have learned that the number in the
string is rounded down (at least for Bay Trail devices),
so if the 1332.800 MHz where correct then we would
expect the string to contain 1.32GHz, but it says 1.33GHz


Test 3: "Intel(R) Atom(TM) CPU  Z3775  @ 1.46GHz"
-------------------------------------------------

Third device tested: Asus T200TA" with:
"Intel(R) Atom(TM) CPU  Z3775  @ 1.46GHz"
again this is the tablet version, so only one clocksource
and thus no "Refined TSC clocksource calibration"

[    0.000000] MSR_PINFO: 0000040000000b00 -> 11
[    0.000000] MSR_FSBF: 0000000000000002
[    0.000000] REF_CLOCK: 133300
[    0.000000] tsc: Detected 1466.300 MHz processor

Since we have no other clocksource, we cannot be
sure that this is wrong, unless we compare to say
the RTC using using the commands Vipul used to
test. So I'm leaving this device running for say
12 hours and then I'll check.

I have a hunch that in this case too we need to replace the
00 with 33, so use 133333 as ref-clock, but we will see.

Regards,

Hans

Powered by blists - more mailing lists