On Fri, Aug 25, 2017 at 12:43:44PM +0200, Mike Belopuhov wrote:
> On Fri, Aug 25, 2017 at 00:40 -0700, Mike Larkin wrote:
> > On Thu, Aug 24, 2017 at 12:39:33PM +0800, Adam Steen wrote:
> > > On Thu, Aug 24, 2017 at 2:35 AM, Mike Larkin <mlar...@azathoth.net> wrote:
> > > > On Wed, Aug 23, 2017 at 09:29:15PM +0800, Adam Steen wrote:
> > > >>
> > > >> Thank you Mike on the feedback on the last patch, please see the diff
> > > >> below, update with your input and style(9)
> > > >>
> > > >> I have continued to use tsc as my timecounter and /var/db/ntpd.driff
> > > >> has stayed under 10.
> > > >>
> > > >> cat /var/db/ntpd.drift
> > > >> 6.665
> > > >>
> > > >> ntpctl -s all
> > > >> 4/4 peers valid, constraint offset -1s, clock synced, stratum 3
> > > >>
> > > >> peer
> > > >> wt tl st next poll offset delay jitter
> > > >> 144.48.166.166 from pool pool.ntp.org
> > > >> 1 10 2 4s 32s -3.159ms 87.723ms 10.389ms
> > > >> 13.55.50.68 from pool pool.ntp.org
> > > >> 1 10 3 11s 32s -3.433ms 86.053ms 18.095ms
> > > >> 14.202.204.182 from pool pool.ntp.org
> > > >> 1 10 1 14s 32s 1.486ms 86.545ms 16.483ms
> > > >> 27.124.125.250 from pool pool.ntp.org
> > > >> * 1 10 2 12s 30s -10.275ms 54.156ms 70.389ms
> > > >>
> > > >> Cheers
> > > >> Adam
> > > >
> > > > IIRC you have an x220, right?
> > > >
> > > > If so, could you try letting the clock run for a bit (while using tsc
> > > > timecounter selection) after apm -L to drop the speed? (make sure
> > > > apm shows that it dropped).
> > > >
> > > > Even though my x230 supposedly has a constant/invar TSC (according to
> > > > cpuid), the TSC drops from 2.5GHz to 1.2GHz when apm -L runs, which
> > > > causes time to run too slowly when tsc is selected there.
> > > >
> > > > -ml
> > > >
> > >
> > > Yes, x220
> > > (bios: LENOVO version "8DET69WW (1.39 )" date 07/18/2013)
> > > (cpu: Intel(R) Core(TM) i5-2520M CPU @ 2.50GHz, 2491.91 MHz)
> > >
> > > I took some measurements to before starting the test.
> > >
> > > note: the laptop has been up for a few days with apm -A set via
> > > rc.conf.local
> > > and sysctl kern.timecounter.hardware as tsc via sysctl.conf and mostly
> > > idle.
> > >
> > > cat /var/db/ntpd.drift
> > > 6.459
> > >
> > > apm -v
> > > Battery state: high, 100% remaining, unknown life estimate
> > > A/C adapter state: connected
> > > Performance adjustment mode: auto (800 MHz)
> > >
> > > 6 hours ago i ran apm -L, verified it was running slowly (800 MHz),
> > > and got the following results
> > >
> > > The clock appears correct (comparing to other computers)
> > >
> > > apm -v
> > > Battery state: high, 100% remaining, unknown life estimate
> > > A/C adapter state: connected
> > > Performance adjustment mode: manual (800 MHz)
> > >
> > > cat /var/db/ntpd.drift
> > > 6.385
> > >
> > > ntpctl -s all
> > > 4/4 peers valid, constraint offset 0s, clock synced, stratum 4
> > >
> > > peer
> > > wt tl st next poll offset delay jitter
> > > 203.23.237.200 from pool pool.ntp.org
> > > 1 10 2 153s 1505s -25.546ms 73.450ms 2.644ms
> > > 203.114.73.24 from pool pool.ntp.org
> > > 1 10 2 253s 1560s -1.042ms 75.133ms 0.752ms
> > > 192.189.54.33 from pool pool.ntp.org
> > > * 1 10 2 204s 1558s 31.644ms 70.910ms 3.388ms
> > > 54.252.165.245 from pool pool.ntp.org
> > > 1 10 2 238s 1518s 0.146ms 73.005ms 2.025ms
> > >
> > > I will leave the laptop in lower power mode over the weekend and see
> > > what happens.
> > >
> >
> > No need, I think you've convinced me that it works :)
>
> But does it actually work on x230 as well? I'm surprised to learn
> that you've observed TSC frequency change on Ivy Bridge. I was
> under impression that everything since at least Sandy Bridge (x220)
> has constant and invariant TSC as advertised.
>
> Adam, I've readjusted and simplified your diff a bit. The biggest
> change is that we can select the reference tc based on it's quality
> so there's no need to have acpitimer and acpihpet specific functions
> and variables.
>
> There's one big thing missing here: increasing the timecounter
> quality so that OS can pick it. Something like this:
>
> https://github.com/mbelop/src/commit/99d6ef3ae95bbd8ea93c27e0425eb65e5a3359a1
>
> I'd say we should try getting this in after 6.3 unlock unless there
> are objections. Further cleanup and testing is welcome of course.
>
>
I'd like to raise another point: as we figured out, the TSC frequency
can be different than the CPU frequency on modern Intel CPUs. The
const+invar TSC can even run faster than the CPU.
For example, my X1 4th Gen currently boots up with the following speeds:
cpu0: Intel(R) Core(TM) i7-6600U CPU @ 2.60GHz, 2808.00 MHz
...
cpu0: Enhanced SpeedStep 2808 MHz: speeds: 2701, 2700, 2600, 2500, 2300, 2100,
1900, 1800, 1600, 1400, 1300, 1100, 800, 700, 600, 400 MHz
The 2.8GHz is actually the TSC speed, the fastest CPU speed is 2.7GHz.
(I didn't figure out why ark.intel.com says max turbo speed is 3.4GHz,
but the 2.7GHz is probably a vendor configuration from Lenovo based on
the Laptop's cooling capabilities).
But the MSR function returns a different value which is supposed to be
the actual CPU speed (of core 0) after going out of power-saving mode.
This caused problems on Skylake because the returned value can be very
different to the TSC - using this value, the kernel failed to
calibrated the TSC timecounter correctly. That's why I added the code
that reads the TSC frequency from CPUID 0x15 and skips the
MSR/RDTSC-based calibration, if CPUID 0x15 is supported and present.
For example, without the CPUID method, I get the MSR-derived speed
(depending on the on-boot power state that is configured in the BIOS):
cpu0: Intel(R) Core(TM) i7-6600U CPU @ 2.60GHz, 2494.75 MHz
But here is my mistake: I actually thought that the MSR function is
"wrong" on Skylake. But it is just a different thing. We have to
differentiate if we want to obtain the TSC or the CPU frequency, but
the CPU frequency might just be derived from the TSC on older CPUs
that don't support the MSR.
I attached a diff that illustrates the problem on the -current code,
but the same could be adjusted for the TSC recalibration diff.
I split the cpu_tsc_freq() function into two functions:
1. cpu_freq() tries to get the CPU frequency from the MSR, falls back
to the TSC via cpu_tsc_freq(). This is used for cpuX dmesg printfs
and for the global speedstep "cpuspeed" variable.
2. cpu_tsc_freq() tries to get the TSC frequency from CPUID 0x15 or
falls back to the less accurate rdtsc() method. This is used for the
(initial) TSC timecounter frequency.
Thoughts?
(I'm not asking for OKs on the diff)
Reyk
Index: sys/arch/amd64/amd64/identcpu.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
retrieving revision 1.87
diff -u -p -u -p -r1.87 identcpu.c
--- sys/arch/amd64/amd64/identcpu.c 20 Jun 2017 05:34:41 -0000 1.87
+++ sys/arch/amd64/amd64/identcpu.c 14 Sep 2017 12:49:47 -0000
@@ -47,8 +47,9 @@
#include <machine/cpufunc.h>
void replacesmap(void);
+u_int64_t cpu_freq(struct cpu_info *);
+u_int64_t cpu_freq_ctr(struct cpu_info *);
u_int64_t cpu_tsc_freq(struct cpu_info *);
-u_int64_t cpu_tsc_freq_ctr(struct cpu_info *);
#if NVMM > 0
void cpu_check_vmm_cap(struct cpu_info *);
#endif /* NVMM > 0 */
@@ -388,12 +389,11 @@ via_update_sensor(void *args)
#endif
u_int64_t
-cpu_tsc_freq_ctr(struct cpu_info *ci)
+cpu_freq_ctr(struct cpu_info *ci)
{
u_int64_t count, last_count, msr;
if ((ci->ci_flags & CPUF_CONST_TSC) == 0 ||
- (ci->ci_flags & CPUF_INVAR_TSC) ||
(cpu_perf_eax & CPUIDEAX_VERID) <= 1 ||
CPUIDEDX_NUM_FC(cpu_perf_edx) <= 1)
return (0);
@@ -426,6 +426,23 @@ cpu_tsc_freq_ctr(struct cpu_info *ci)
}
u_int64_t
+cpu_freq(struct cpu_info *ci)
+{
+ u_int64_t count;
+
+ /*
+ * Try to get the currently running CPU frequency. On modern CPUs,
+ * this does not have to match the TSC frequency.
+ */
+ count = cpu_freq_ctr(ci);
+ if (count != 0)
+ return (count);
+
+ /* Fallback to the TSC frequency which was identical on old CPUs. */
+ return (cpu_tsc_freq(ci));
+}
+
+u_int64_t
cpu_tsc_freq(struct cpu_info *ci)
{
u_int64_t last_count, count;
@@ -464,10 +481,6 @@ cpu_tsc_freq(struct cpu_info *ci)
}
}
- count = cpu_tsc_freq_ctr(ci);
- if (count != 0)
- return (count);
-
last_count = rdtsc();
delay(100000);
count = rdtsc();
@@ -489,6 +502,7 @@ identifycpu(struct cpu_info *ci)
int i;
char *brandstr_from, *brandstr_to;
int skipspace;
+ uint64_t freq;
CPUID(1, ci->ci_signature, val, dummy, ci->ci_feature_flags);
CPUID(0x80000000, ci->ci_pnfeatset, dummy, dummy, dummy);
@@ -568,18 +582,17 @@ identifycpu(struct cpu_info *ci)
ci->ci_flags |= CPUF_INVAR_TSC;
}
- ci->ci_tsc_freq = cpu_tsc_freq(ci);
-
amd_cpu_cacheinfo(ci);
printf("%s: %s", ci->ci_dev->dv_xname, mycpu_model);
- if (ci->ci_tsc_freq != 0)
- printf(", %llu.%02llu MHz", (ci->ci_tsc_freq + 4999) / 1000000,
- ((ci->ci_tsc_freq + 4999) / 10000) % 100);
+ freq = cpu_freq(ci);
+ if (freq != 0)
+ printf(", %llu.%02llu MHz", (freq + 4999) / 1000000,
+ ((freq + 4999) / 10000) % 100);
if (ci->ci_flags & CPUF_PRIMARY) {
- cpuspeed = (ci->ci_tsc_freq + 4999) / 1000000;
+ cpuspeed = (freq + 4999) / 1000000;
cpu_cpuspeed = cpu_amd64speed;
}
@@ -726,10 +739,10 @@ identifycpu(struct cpu_info *ci)
if ((ci->ci_flags & CPUF_PRIMARY) &&
(ci->ci_flags & CPUF_CONST_TSC) &&
(ci->ci_flags & CPUF_INVAR_TSC)) {
- printf("%s: TSC frequency %llu Hz\n",
- ci->ci_dev->dv_xname, ci->ci_tsc_freq);
- tsc_timecounter.tc_frequency = ci->ci_tsc_freq;
+ tsc_timecounter.tc_frequency = cpu_tsc_freq(ci);
tc_init(&tsc_timecounter);
+ printf("%s: TSC frequency %llu Hz\n",
+ ci->ci_dev->dv_xname, tsc_timecounter.tc_frequency);
}
cpu_topology(ci);
Index: sys/arch/amd64/include/cpu.h
===================================================================
RCS file: /cvs/src/sys/arch/amd64/include/cpu.h,v
retrieving revision 1.114
diff -u -p -u -p -r1.114 cpu.h
--- sys/arch/amd64/include/cpu.h 11 Aug 2017 20:19:14 -0000 1.114
+++ sys/arch/amd64/include/cpu.h 14 Sep 2017 12:49:47 -0000
@@ -138,7 +138,6 @@ struct cpu_info {
u_int32_t ci_family;
u_int32_t ci_model;
u_int32_t ci_cflushsz;
- u_int64_t ci_tsc_freq;
int ci_inatomic;
