On Tue, Aug 8, 2017 at 10:12 PM, Mike Belopuhov <m...@belopuhov.com> wrote: > On Tue, Aug 08, 2017 at 08:18 +0800, Adam Steen wrote: >> On Mon, Jul 31, 2017 at 3:58 PM, Mike Belopuhov <m...@belopuhov.com> wrote: >> > On Mon, Jul 31, 2017 at 09:48 +0800, Adam Steen wrote: >> >> Ted Unangst wrote: >> >> > we don't currently export this info, but we could add some sysctls. >> >> > there's >> >> > some cpufeatures stuff there, but generally stuff isn't exported until >> >> > somebody finds a use for it... it shouldn't be too hard to add >> >> > something to >> >> > amd64/machdep.c sysctl if you're interested. >> >> >> >> I am interested, as i need the info, i will look into it and hopefully >> >> come back with a patch. >> > >> > This is a bad idea because TSC as the time source is only usable >> > by OpenBSD on Skylake and Kaby Lake CPUs since they encode the TSC >> > frequency in the CPUID. All older CPUs have their TSCs measured >> > against the PIT. Currently the measurement done by the kernel isn't >> > very precise and if TSC is selected as a timecounter, the machine >> > would be gaining time on a pace that cannot be corrected by our NTP >> > daemon. (IIRC, about an hour a day on my Haswell running with NTP). >> > >> > To be able to use TSC as a timecounter source on OpenBSD or Solo5 >> > you'd have to improve the in-kernel measurement of the TSC frequency >> > first. I've tried to perform 10 measurements and take an average and >> > it does improve accuracy, however I believe we need to poach another >> > bit from Linux and re-calibrate TSC via HPET: >> > >> > >> > http://elixir.free-electrons.com/linux/v4.12.4/source/arch/x86/kernel/tsc.c#L409 >> > >> > I think this is the most sane thing we can do. Here's a complete >> > procedure that Linux kernel undertakes: >> > >> > >> > http://elixir.free-electrons.com/linux/v4.12.4/source/arch/x86/kernel/tsc.c#L751 >> > >> > Regards, >> > Mike >> >> Hi Mike/All >> >> I would like to improve the accuracy of TSC frequency calibration as >> Mike B. describes above. >> >> I initially thought the calibration would take place at line 470 of >> amd64/identcpu.c >> (https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/amd64/amd64/identcpu.c?annotate=1.87) >> > > Indeed, it cannot happen there simply because you don't know at > that point whether or not HPET actually exists. > >> But I looked into using the acpihpet directly but it is never exposed >> outside of acpihpet.c. >> > > And it shouldn't be. > >> Could someone point me to were if would be appropriate to complete >> this calibration and how to use the acpihpet? > > The way I envision this is a multi-step approach: > > 1) TSC frequency is approximated with the PIT (possibly performing > multiple measurements and averaging them out; also keep in mind that > doing it 8 times means you can shift the sum right by 3 instead of > using actual integer division). This is what should happen around > the line 470 of identcpu.c > > 2) A function can be provided by identcpu.c to further adjust the > TSC frequency once acpitimer(4) (this is a PM timer) and acpihpet(4) > (or any other timer for that matter) are attached. > > 3) Once acpitimer(4) or acpihpet(4) or any other timecounter source > are attached and are verified to be operating correctly, they can > perform TSC re-calibration and update the TSC frequency with their > measurements. The idea here is that the function (or functions) that > facilitate this must abstract enough logic so that you don't have to > duplicate it in the acpitimer or acpihpet themselves. > >> (Will it need to be >> exposed like i8254_delay/delay_func/delay in machdep.c and cpu.h) >> > > No it won't. > >> Lastly should the calibration be done using both delay(i8254 pit) and >> hpet timers similar to Linux described above or just using the hpet? >> > > Well, that's what I was arguing for. As I said in my initial mail > on misc (not quoted here), the TSC must be calibrated using separate > known clocks sources.
Hi Mike Please see the below diff to improve the accuracy of the TSC frequency. It is model after the linux calibration you linked to earlier. https://marc.info/?l=openbsd-misc&m=150148792804747&w=2 I feel like i don't know enough about the kernel internals, the consistency of the results across reboots are not as close as i would have liked, i feel the call to do the actual calibration should be later in the boot cycle, when things have calmed down a little, but couldn't figure out the best way of doing this. please bear with me i haven't been programming c for long, but the only way to get things done is to do it your self. Cheers Adam 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 17 Aug 2017 04:07:25 -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_timecounter; +u_int64_t acpi_calculate_timecounter_delay(struct timecounter *, u_int64_t, u_int64_t); +u_int64_t acpi_calculate_tsc_frequency(u_int64_t, u_int64_t, int); +u_int64_t acpi_calibrate_tsc_frequency(); +int acpi_get_tsc_and_timecount(struct timecounter *, u_int64_t *, u_int64_t *); + int acpi_map(paddr_t pa, size_t len, struct acpi_mem_map *handle) { @@ -438,6 +447,105 @@ 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, u_int64_t *tsc, u_int64_t *count) +{ + u_int64_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; +} + +u_int64_t +acpi_calculate_timecounter_delay(struct timecounter *tc, u_int64_t count1, u_int64_t count2) +{ + u_int64_t delta; + + if(count2 < count1) + count2 += tc->tc_counter_mask; + + delta = (count2 - count1); + return delta * 1000000 / tc->tc_frequency; +} + +u_int64_t +acpi_calculate_tsc_frequency(u_int64_t tsc1, u_int64_t tsc2, int usec) +{ + u_int64_t delta; + + delta = (tsc2 - tsc1); + return delta * 1000000 / usec; +} + +void +acpi_recalibrate_timecounter_init(struct timecounter *tc) +{ + recalibrate_timecounter = tc; + cpu_adjust_tsc_frequency(&acpi_calibrate_tsc_frequency); +} + +u_int64_t +acpi_calibrate_tsc_frequency() +{ + u_int64_t count1, count2, frequency, min_freq, tsc1, tsc2; + u_long ef; + int delay_usec, i, err1, err2, usec; + + struct timecounter *tc = recalibrate_timecounter; + + // 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_timecounter_delay(tc, count1, count2); + + if(usec < (delay_usec - RECALIBRATE_DELAY_THRESHOLD)) + continue; + + if(usec > (delay_usec + RECALIBRATE_DELAY_THRESHOLD)) + continue; + + frequency = acpi_calculate_tsc_frequency(tsc1, tsc2, usec); + + min_freq = ulmin(min_freq, frequency); + } + + if(min_freq == 0) { + printf("Failed to recalibrate timecounter \"tsc\" with timecounter \"%s\"\n", + tc->tc_name); + } + 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 17 Aug 2017 04:07:25 -0000 @@ -63,6 +63,10 @@ struct timecounter tsc_timecounter = { tsc_get_timecount, NULL, ~0u, 0, "tsc", -1000, NULL }; +u_int64_t (*amd_tsc_get_frequency)(); +u_int64_t amd64_tsc_freq; +int amd64_tsc_requires_calibration; +int amd64_has_invariant_tsc; int amd64_has_xcrypt; #ifdef CRYPTO int amd64_has_pclmul; @@ -428,8 +432,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 +473,21 @@ cpu_tsc_freq(struct cpu_info *ci) if (count != 0) return (count); - last_count = rdtsc(); - delay(100000); - count = rdtsc(); + if(amd_tsc_get_frequency != NULL) + return (*amd_tsc_get_frequency)(); - return ((count - last_count) * 10); + if (ci->ci_flags & CPUF_PRIMARY) + amd64_tsc_requires_calibration = 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 +497,20 @@ tsc_get_timecount(struct timecounter *tc } void +cpu_adjust_tsc_frequency(u_int64_t (*get_frequency)()) +{ + if(amd64_tsc_requires_calibration) { + amd64_tsc_freq = (*get_frequency)(); + tsc_timecounter.tc_frequency = amd64_tsc_freq; + + printf("Timecounter \"%s\" recalibrated with frequency %llu Hz\n", + tsc_timecounter.tc_name, (unsigned long)tsc_timecounter.tc_frequency); + } else { + amd_tsc_get_frequency = get_frequency; + } +} + +void identifycpu(struct cpu_info *ci) { u_int32_t dummy, val; @@ -566,9 +595,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 17 Aug 2017 04:07:25 -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,10 @@ 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.113 diff -u -p -u -p -r1.113 cpu.h --- sys/arch/amd64/include/cpu.h 12 Jul 2017 06:26:32 -0000 1.113 +++ sys/arch/amd64/include/cpu.h 17 Aug 2017 04:07:25 -0000 @@ -429,7 +429,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 }, \ @@ -448,6 +450,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 17 Aug 2017 04:07:25 -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_frequency(u_int64_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 17 Aug 2017 04:07:26 -0000 @@ -265,6 +265,7 @@ acpihpet_attach(struct device *parent, s hpet_timecounter.tc_name = sc->sc_dev.dv_xname; tc_init(&hpet_timecounter); acpihpet_attached++; + acpi_recalibrate_timecounter_init(&hpet_timecounter); } u_int 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 17 Aug 2017 04:07:26 -0000 @@ -25,6 +25,8 @@ #include <dev/acpi/acpireg.h> #include <dev/acpi/acpivar.h> +#include "acpihpet.h" + int acpitimermatch(struct device *, void *, void *); void acpitimerattach(struct device *, struct device *, void *); @@ -96,6 +98,9 @@ acpitimerattach(struct device *parent, s acpi_timecounter.tc_priv = sc; acpi_timecounter.tc_name = sc->sc_dev.dv_xname; tc_init(&acpi_timecounter); +#if NACPIHPET == 0 + acpi_recalibrate_timecounter_init(&acpi_timecounter); +#endif } Index: sys/dev/acpi/acpivar.h =================================================================== RCS file: /cvs/src/sys/dev/acpi/acpivar.h,v retrieving revision 1.87 diff -u -p -u -p -r1.87 acpivar.h --- sys/dev/acpi/acpivar.h 8 Apr 2017 01:20:10 -0000 1.87 +++ sys/dev/acpi/acpivar.h 17 Aug 2017 04:07: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_timecounter_init(struct timecounter *); +u_int64_t acpi_calibrate_tsc_frequency(); #define ACPI_IOREAD 0 #define ACPI_IOWRITE 1 Index: sys/dev/acpi/files.acpi =================================================================== RCS file: /cvs/src/sys/dev/acpi/files.acpi,v retrieving revision 1.37 diff -u -p -u -p -r1.37 files.acpi --- sys/dev/acpi/files.acpi 22 Feb 2017 16:39:56 -0000 1.37 +++ sys/dev/acpi/files.acpi 17 Aug 2017 04:07:26 -0000 @@ -39,7 +39,7 @@ file dev/acpi/acpicpu.c acpicpu needs-f # High Precision Event Timer device acpihpet attach acpihpet at acpi -file dev/acpi/acpihpet.c acpihpet +file dev/acpi/acpihpet.c acpihpet needs-flag # Embedded Controller device acpiec
