Hi,
On 29-01-2020 21:57, Thomas Gleixner wrote:
Andy,
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> 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);
