On Mon, Aug 28, 2017 at 08:02:35AM +0800, Adam Steen wrote: > 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. > > > > Hi all > > I wanted to post the complicated patch, along with Mike's qaulity > changes for completeness sake, see the diff below. > > I have been running this since Saturday morning with no > problems. > > Cheers > Adam >
This diff works fine on the x230. And although there is no hpet inside vmm/vmd, if you pick tsc anyway, it seems to have less time drift than the 8254/PIT. So, that's good. It's not perfect but it's better. mikeb, do you want to get this in? the diff reads ok to me. -ml > Index: sys/arch/amd64/amd64/acpi_machdep.c > =================================================================== > RCS file: /cvs/src/sys/arch/amd64/amd64/acpi_machdep.c,v > retrieving revision 1.78 > diff -u -p -u -p -r1.78 acpi_machdep.c > --- sys/arch/amd64/amd64/acpi_machdep.c 27 Mar 2017 18:32:53 -0000 > 1.78 > +++ sys/arch/amd64/amd64/acpi_machdep.c 27 Aug 2017 23:33:22 -0000 > @@ -69,6 +69,15 @@ extern int acpi_savecpu(void) __returns_ > > u_int8_t *acpi_scan(struct acpi_mem_map *, paddr_t, size_t); > > +#define RECALIBRATE_MAX_RETRIES 5 > +#define RECALIBRATE_SMI_THRESHOLD 50000 > +#define RECALIBRATE_DELAY_THRESHOLD 20 > + > +struct timecounter *recalibrate_tc; > + > +uint64_t acpi_calibrate_tsc_freq(void); > +int acpi_get_tsc_and_timecount(struct timecounter *, uint64_t *, uint64_t *); > + > int > acpi_map(paddr_t pa, size_t len, struct acpi_mem_map *handle) > { > @@ -438,6 +447,108 @@ acpi_resume_cpu(struct acpi_softc *sc) > /* Re-initialise memory range handling on BSP */ > if (mem_range_softc.mr_op != NULL) > mem_range_softc.mr_op->initAP(&mem_range_softc); > +} > + > +int > +acpi_get_tsc_and_timecount(struct timecounter *tc, uint64_t *tsc, > + uint64_t *count) > +{ > + uint64_t n, tsc1, tsc2; > + int i; > + > + for (i = 0; i < RECALIBRATE_MAX_RETRIES; i++) { > + tsc1 = rdtsc(); > + n = (tc->tc_get_timecount(tc) & tc->tc_counter_mask); > + tsc2 = rdtsc(); > + > + if ((tsc2 - tsc1) < RECALIBRATE_SMI_THRESHOLD) { > + *count = n; > + *tsc = tsc2; > + return (0); > + } > + } > + return 1; > +} > + > +void > +acpi_recalibrate_tsc(struct timecounter *tc) > +{ > + if (!recalibrate_tc || > + recalibrate_tc->tc_quality < tc->tc_quality) { > + recalibrate_tc = tc; > + cpu_adjust_tsc_freq(&acpi_calibrate_tsc_freq); > + } > +} > + > +static inline uint64_t > +acpi_calculate_tsc_freq(uint64_t tsc1, uint64_t tsc2, int usec) > +{ > + uint64_t delta; > + > + delta = (tsc2 - tsc1); > + return delta * 1000000 / usec; > +} > + > +static inline uint64_t > +acpi_calculate_tc_delay(struct timecounter *tc, uint64_t count1, > + uint64_t count2) > +{ > + uint64_t delta; > + > + if (count2 < count1) > + count2 += tc->tc_counter_mask; > + > + delta = (count2 - count1); > + return delta * 1000000 / tc->tc_frequency; > +} > + > +uint64_t > +acpi_calibrate_tsc_freq(void) > +{ > + struct timecounter *tc; > + uint64_t count1, count2, frequency, min_freq, tsc1, tsc2; > + u_long ef; > + int delay_usec, i, err1, err2, usec; > + > + if (recalibrate_tc == NULL) > + return 0; > + > + tc = recalibrate_tc; > + > + /* warmup the timers */ > + for (i =0; i < 3; i++) { > + (void)tc->tc_get_timecount(tc); > + (void)rdtsc(); > + } > + > + min_freq = ULLONG_MAX; > + > + delay_usec = 100000; > + for (i = 0; i < 3; i++) { > + ef = read_rflags(); > + disable_intr(); > + > + err1 = acpi_get_tsc_and_timecount(tc, &tsc1, &count1); > + delay(delay_usec); > + err2 = acpi_get_tsc_and_timecount(tc, &tsc2, &count2); > + > + write_rflags(ef); > + > + if (err1 || err2) > + continue; > + > + usec = acpi_calculate_tc_delay(tc, count1, count2); > + > + if ((usec < (delay_usec - RECALIBRATE_DELAY_THRESHOLD)) || > + (usec > (delay_usec + RECALIBRATE_DELAY_THRESHOLD))) > + continue; > + > + frequency = acpi_calculate_tsc_freq(tsc1, tsc2, usec); > + > + min_freq = ulmin(min_freq, frequency); > + } > + > + return min_freq; > } > > #ifdef MULTIPROCESSOR > 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 27 Aug 2017 23:33:22 -0000 > @@ -63,6 +63,11 @@ struct timecounter tsc_timecounter = { > tsc_get_timecount, NULL, ~0u, 0, "tsc", -1000, NULL > }; > > +u_int64_t amd64_tsc_freq; > +int amd64_tsc_recalibrate; > +u_int64_t (*amd64_tsc_get_freq)(); > + > +int amd64_has_invariant_tsc; > int amd64_has_xcrypt; > #ifdef CRYPTO > int amd64_has_pclmul; > @@ -428,8 +433,9 @@ cpu_tsc_freq_ctr(struct cpu_info *ci) > u_int64_t > cpu_tsc_freq(struct cpu_info *ci) > { > - u_int64_t last_count, count; > + u_int64_t last_count, count, sum; > uint32_t eax, ebx, khz, dummy; > + int i; > > if (!strcmp(cpu_vendor, "GenuineIntel") && > cpuid_level >= 0x15) { > @@ -468,11 +474,25 @@ cpu_tsc_freq(struct cpu_info *ci) > if (count != 0) > return (count); > > - last_count = rdtsc(); > - delay(100000); > - count = rdtsc(); > + if (amd64_tsc_get_freq != NULL && > + (count = (*amd64_tsc_get_freq)()) != 0) { > + if (amd64_has_invariant_tsc) > + tsc_timecounter.tc_quality = 2000; > + return (count); > + } > > - return ((count - last_count) * 10); > + if (ci->ci_flags & CPUF_PRIMARY) > + amd64_tsc_recalibrate = 1; > + > + sum = 0; > + for (i=0; i < 8; i++) { > + last_count = rdtsc(); > + delay(100000); > + count = rdtsc(); > + sum += (count - last_count); > + } > + > + return ((sum >> 3) * 10); > } > > u_int > @@ -482,6 +502,26 @@ tsc_get_timecount(struct timecounter *tc > } > > void > +cpu_adjust_tsc_freq(u_int64_t (*get_frequency)()) > +{ > + u_int64_t freq; > + > + if (amd64_tsc_recalibrate) { > + if ((freq = (*get_frequency)()) == 0) > + return; > + amd64_tsc_freq = freq; > + tsc_timecounter.tc_frequency = freq; > + if (amd64_has_invariant_tsc) > + tsc_timecounter.tc_quality = 2000; > + > + printf("%s: updated TSC frequency %lld\n", > + curcpu()->ci_dev->dv_xname, > + tsc_timecounter.tc_frequency); > + } else > + amd64_tsc_get_freq = get_frequency; > +} > + > +void > identifycpu(struct cpu_info *ci) > { > u_int32_t dummy, val; > @@ -566,9 +606,13 @@ identifycpu(struct cpu_info *ci) > /* Check if it's an invariant TSC */ > if (cpu_apmi_edx & CPUIDEDX_ITSC) > ci->ci_flags |= CPUF_INVAR_TSC; > + > + amd64_has_invariant_tsc = (ci->ci_flags & CPUF_INVAR_TSC) != 0; > } > > ci->ci_tsc_freq = cpu_tsc_freq(ci); > + if (ci->ci_flags & CPUF_PRIMARY) > + amd64_tsc_freq = ci->ci_tsc_freq; > > amd_cpu_cacheinfo(ci); > > Index: sys/arch/amd64/amd64/machdep.c > =================================================================== > RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v > retrieving revision 1.231 > diff -u -p -u -p -r1.231 machdep.c > --- sys/arch/amd64/amd64/machdep.c 12 Jul 2017 06:26:32 -0000 1.231 > +++ sys/arch/amd64/amd64/machdep.c 27 Aug 2017 23:33:22 -0000 > @@ -425,6 +425,8 @@ int > cpu_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp, > size_t newlen, struct proc *p) > { > + extern u_int64_t amd64_tsc_freq; > + extern int amd64_has_invariant_tsc; > extern int amd64_has_xcrypt; > dev_t consdev; > dev_t dev; > @@ -496,6 +498,11 @@ cpu_sysctl(int *name, u_int namelen, voi > pckbc_release_console(); > return (error); > #endif > + case CPU_TSCFREQ: > + return (sysctl_rdquad(oldp, oldlenp, newp, amd64_tsc_freq)); > + case CPU_INVARIANTTSC: > + return (sysctl_rdint(oldp, oldlenp, newp, > + amd64_has_invariant_tsc)); > default: > return (EOPNOTSUPP); > } > 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 27 Aug 2017 23:33:24 -0000 > @@ -427,7 +427,9 @@ void mp_setperf_init(void); > #define CPU_XCRYPT 12 /* supports VIA xcrypt in userland */ > #define CPU_LIDACTION 14 /* action caused by lid close */ > #define CPU_FORCEUKBD 15 /* Force ukbd(4) as console > keyboard */ > -#define CPU_MAXID 16 /* number of valid machdep ids */ > +#define CPU_TSCFREQ 16 /* tsc frequency */ > +#define CPU_INVARIANTTSC 17 /* has invariant tsc */ > +#define CPU_MAXID 18 /* number of valid machdep ids */ > > #define CTL_MACHDEP_NAMES { \ > { 0, 0 }, \ > @@ -446,6 +448,8 @@ void mp_setperf_init(void); > { 0, 0 }, \ > { "lidaction", CTLTYPE_INT }, \ > { "forceukbd", CTLTYPE_INT }, \ > + { "tscfreq", CTLTYPE_QUAD }, \ > + { "invarianttsc", CTLTYPE_INT }, \ > } > > /* > Index: sys/arch/amd64/include/cpuvar.h > =================================================================== > RCS file: /cvs/src/sys/arch/amd64/include/cpuvar.h,v > retrieving revision 1.8 > diff -u -p -u -p -r1.8 cpuvar.h > --- sys/arch/amd64/include/cpuvar.h 28 Jul 2016 21:57:57 -0000 1.8 > +++ sys/arch/amd64/include/cpuvar.h 27 Aug 2017 23:33:24 -0000 > @@ -96,5 +96,6 @@ void x86_ipi_init(int); > void identifycpu(struct cpu_info *); > void cpu_init(struct cpu_info *); > void cpu_init_first(void); > +void cpu_adjust_tsc_freq(uint64_t (*)()); > > #endif > Index: sys/dev/acpi/acpihpet.c > =================================================================== > RCS file: /cvs/src/sys/dev/acpi/acpihpet.c,v > retrieving revision 1.21 > diff -u -p -u -p -r1.21 acpihpet.c > --- sys/dev/acpi/acpihpet.c 6 Oct 2015 20:49:32 -0000 1.21 > +++ sys/dev/acpi/acpihpet.c 27 Aug 2017 23:33:26 -0000 > @@ -264,6 +264,7 @@ acpihpet_attach(struct device *parent, s > hpet_timecounter.tc_priv = sc; > hpet_timecounter.tc_name = sc->sc_dev.dv_xname; > tc_init(&hpet_timecounter); > + acpi_recalibrate_tsc(&hpet_timecounter); > acpihpet_attached++; > } > > Index: sys/dev/acpi/acpitimer.c > =================================================================== > RCS file: /cvs/src/sys/dev/acpi/acpitimer.c,v > retrieving revision 1.11 > diff -u -p -u -p -r1.11 acpitimer.c > --- sys/dev/acpi/acpitimer.c 14 Mar 2015 03:38:46 -0000 1.11 > +++ sys/dev/acpi/acpitimer.c 27 Aug 2017 23:33:26 -0000 > @@ -95,6 +95,7 @@ acpitimerattach(struct device *parent, s > acpi_timecounter.tc_counter_mask = 0xffffffffU; > acpi_timecounter.tc_priv = sc; > acpi_timecounter.tc_name = sc->sc_dev.dv_xname; > + acpi_recalibrate_tsc(&acpi_timecounter); > tc_init(&acpi_timecounter); > } > > Index: sys/dev/acpi/acpivar.h > =================================================================== > RCS file: /cvs/src/sys/dev/acpi/acpivar.h,v > retrieving revision 1.88 > diff -u -p -u -p -r1.88 acpivar.h > --- sys/dev/acpi/acpivar.h 17 Aug 2017 05:16:27 -0000 1.88 > +++ sys/dev/acpi/acpivar.h 27 Aug 2017 23:33:26 -0000 > @@ -25,6 +25,7 @@ > > #include <sys/timeout.h> > #include <sys/rwlock.h> > +#include <sys/timetc.h> > #include <machine/biosvar.h> > > #include "acpipwrres.h" > @@ -324,6 +325,8 @@ void acpi_resume_cpu(struct acpi_softc > void acpi_resume_mp(void); > void acpi_sleep_walk(struct acpi_softc *, int); > > +void acpi_recalibrate_tsc(struct timecounter *); > +uint64_t acpi_calibrate_tsc_freq(); > > #define ACPI_IOREAD 0 > #define ACPI_IOWRITE 1 >