New subject: [v3] x86/tsc: Unset TSC_KNOWN_FREQ and TSC_RELIABLE flags on Intel Bay Trail SoC

22 Jan 2020

Vipul,
vipul kumar vipulk0511@gmail.com writes:
...
On Tue, Jan 21, 2020 at 11:15 PM Thomas Gleixner tglx@linutronix.de wrote:
...
Measurement with the existing code:
   $ echo -n "SystemTime: " ; TZ=UTC date -Iseconds ; echo -n "RTC Time:
" ; TZ=UTC hwclock -r ; echo -n "Uptime: " ; uptime -p
   SystemTime: 2019-12-05T17:18:37+00:00
   RTC Time:   2019-12-05 17:18:07.255341+0000
   Uptime: up 1 day, 7 minutes
This sample shows a difference of 30 seconds after 1 day.
Measurement with this patch:
   SystemTime: 2019-12-11T12:06:19+00:00
   RTC Time:   2019-12-11 12:06:19.083127+0000
   Uptime: up 1 day, 3 minutes
With this patch, no time drift issue is observed. and tsc clocksource
get calibration (tsc: Refined TSC clocksource calibration: 1833.333 MHz)
which is missing
   with the existing implementation.
What's the frequency which is determined from the MSR? Something like
this in dmesg:
tsc: Detected NNN MHz processor
or
       tsc: Detected NNN MHz TSC
Also please apply the debug patch below and provide a _full_ dmesg after
boot.
...
...
...
+config X86_FEATURE_TSC_UNKNOWN_FREQ

bool "Support to skip tsc known frequency flag"


help


  Include support to skip X86_FEATURE_TSC_KNOWN_FREQ flag



  X86_FEATURE_TSC_KNOWN_FREQ flag is causing time-drift on



Valleyview/
...

  Baytrail SoC.


  By selecting this option, user can skip



X86_FEATURE_TSC_KNOWN_FREQ
...

  flag to use refine tsc freq calibration.



This is exactly the same problem as before. How does anyone aside of you
know whether to enable this or not?
Go through the Documentation/kbuild/kconfig-language.rst but didn't

find related to make
    config known to everyone. Could you please point to documentation?
Right. And there is no proper answer to this, which makes it clear that
a config option is not the right tool to use.
...
...
And if someone enables this option then _ALL_ platforms which utilize
cpu_khz_from_msr() are affected. How is that any different from your
previous approach? This works on local kernels where you build for a
specific platform and you know exactly what you're doing, but not for
general consumption. What should a distro do with this option?
TSC frequency is already calculated in cpu_khz_from_msr() function

before setting these flags.
Your mail client does some horrible formatting this zig-zag is
unreadable. I'm reformatting the paragraphs below.
...
This patch return the same calculated TSC frequency but skipping
those two flags. On the basis of these flags, we decide whether we
skip the refined calibration and directly register it as a clocksource
or use refine tsc freq calibration in init_tsc_clocksource()
function. By default this config is disabled and if user wants to use
refine tsc freq calibration() then only user will enable it otherwise
it will go with existing implementations. So, I don't think so it will
break for other ATOM SoC.
It does. I explained most of the following to you in an earlier mail
already. Let me try again.
If X86_FEATURE_TSC_RELIABLE is not set, then the TSC requires a watchdog
clocksource. But some of those SoCs do not have anything else than TSC,
so there is no watchdog available. As a consequence the TSC is not
usable for high resolution timers and NOHZ. That breaks existing systems
whether you like it or not.
If X86_FEATURE_TSC_KNOWN_FREQUENCY is not set, then this delays the
usability of the TSC for high res and NOHZ until the refined calibration
figured out that it can't calibrate. And no, we can't know that it does
not work upfront when the early TSC init happens. Clearing this flag
will not break functionality, but it changes the behaviour on boot-time
optimized systems which can obviously be considered breakage.
So no, having a config knob which might be turned on and turning working
systems into trainwrecks is simply not the way to go.
What can be done is to have a command line option which enforces refined
calibration and that option can turn off X86_FEATURE_TSC_KNOWN_FREQUENCY,
but nothing else.
...
Check the cpu_khz_from_msr() function.
I know that code.
...
In cpu_khz_from_msr() function we are setting
X86_FEATURE_TSC_KNOWN_FREQ and X86_FEATURE_TSC_RELIABLE for all the
SoC's but in native_calibrate_tsc(), we check for vendor == INTEL and
CPUID > 0x15 and then at the end of function, will enable
X86_FEATURE_TSC_RELIABLE for INTEL_FAM6_ATOM_GOLDMONT SoC.
Do we need to set the same flag in two different functions as it will be
set in cpu_khz_from_msr() for all SoCs ?
cpu_khz_from_msr() does not handle INTEL_FAM6_ATOM_GOLDMONT or can you
find that in tsc_msr_cpu_ids[]? Making half informed claims is not
solving anything.
Thanks,
tglx
8<------------------

--- a/arch/x86/kernel/tsc_msr.c
+++ b/arch/x86/kernel/tsc_msr.c
@@ -94,16 +94,20 @@ unsigned long cpu_khz_from_msr(void)
    if (freq_desc->msr_plat) {
    	rdmsr(MSR_PLATFORM_INFO, lo, hi);
    	ratio = (lo >> 8) & 0xff;
+		pr_info("MSR_PINFO: %08x%08x -> %u\n", hi, lo, ratio);
    } else {
    	rdmsr(MSR_IA32_PERF_STATUS, lo, hi);
    	ratio = (hi >> 8) & 0x1f;
+		pr_info("MSR_PSTAT: %08x%08x -> %u\n", hi, lo, ratio);
    }
/* Get FSB FREQ ID */
    rdmsr(MSR_FSB_FREQ, lo, hi);
+	pr_info("MSR_FSBF: %08x%08x\n", hi, lo);
/* Map CPU reference clock freq ID(0-7) to CPU reference clock freq(KHz) */
    freq = freq_desc->freqs[lo & 0x7];
+	pr_info("REF_CLOCK: %08x\n", freq);
/* TSC frequency = maximum resolved freq * maximum resolved bus ratio */
    res = freq * ratio;

    

Re: [v3] x86/tsc: Unset TSC_KNOWN_FREQ and TSC_RELIABLE flags on Intel Bay Trail SoC