Re: Improve the accuracy of the TSC frequency calibration - Updated Patch

2017-09-19 Thread Mike Belopuhov
On Tue, Sep 19, 2017 at 22:35 +0200, Mike Belopuhov wrote:
> Keeping all of what Reyk said in mind, here's an updated diff that
> incorporates additional changes:
> 
> 1) doesn't break i386 kernel compilation;
> 
> 2) allows for multiple recalibrations using better timecounter
> sources since acpihpet (and maybe acpitimer) can be attached both
> before and after cpu0;
> 
> 3) factors out the TSC timecounter code into a separate file so
> that it's clear what's related to the timecounter code and what's
> not as I didn't quite like pollution of ACPI bits with unrelated
> stuff;
> 
> 4) cuts down on global variables and provides additional cleanup.
> 
> Same diff as in here: https://github.com/mbelop/src/tree/tsc
> 
> Does this look good to everybody?
> acpitimer and acpihpet parts look a tiny bit gross I guess but
> on overall the tsc.c can be copied to i386 and these ifdefs
> extended to include it.
> 

Forgot to mention.  Since this removes the useless ci_tsc_freq
struct cpu_info member, a "make config" is required to update
cpu_info member offsets table.  Not doing "make clean" at the
same time is on your conscience.



Re: Improve the accuracy of the TSC frequency calibration - Updated Patch

2017-09-19 Thread Mike Belopuhov
On Thu, Sep 14, 2017 at 15:41 +0200, Reyk Floeter wrote:
> 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
> 

Keeping all of what Reyk said in mind, here's an updated diff that
incorporates additional changes:

1) doesn't break i386 kernel compilation;

2) allows for multiple recalibrations using better timecounter
sources since acpihpet (and maybe acpitimer) can be attached both
before and after cpu0;

3) factors out the TSC timecounter code into a separate file so
that it's clear what's related to the timecounter code and what's
not as I didn't quite like pollution of ACPI bits with unrelated
stuff;

4) cuts down on global variables and provides additional cleanup.

Same diff as in here: https://github.com/mbelop/src/tree/tsc

Does this look good to everybody?
acpitimer and acpihpet parts look a tiny bit gross I guess but
on overall the tsc.c can be copied to i386 and these ifdefs
extended to include it.

diff --git sys/arch/amd64/amd64/identcpu.c sys/arch/amd64/amd64/identcpu.c
index a448b885ba7..775e342650d 100644
--- sys/arch/amd64/amd64/identcpu.c
+++ sys/arch/amd64/amd64/identcpu.c
@@ -45,26 +45,20 @@
 
 #include 
 #include 
 
 void   replacesmap(void);
-u_int64_t cpu_tsc_freq(struct cpu_info *);
-u_int64_t cpu_tsc_freq_ctr(struct cpu_info *);
+uint64_t cpu_freq(struct cpu_info *);
+void   tsc_timecounter_init(struct cpu_info *);
 #if NVMM > 0
 void   cpu_check_vmm_cap(struct cpu_info *);
 #endif /* NVMM > 0 */
 
 /* sysctl wants this. */
 char cpu_model[48];
 int cpuspeed;
 
-u_int tsc_get_timecount(struct timecounter *tc);
-
-struct timecounter tsc_timecounter = {
-   tsc_get_timecount, NULL, ~0u, 0, "tsc", -1000, NULL
-};
-
 int amd64_has_xcrypt;
 #ifdef CRYPTO
 int amd64_has_pclmul;
 int amd64_has_aesni;
 #endif
@@ -385,17 +379,16 @@ via_update_sensor(void *args)
ci->ci_sensor.value += 27315;
ci->ci_sensor.flags &= ~SENSOR_FINVALID;
 }
 #endif
 
-u_int64_t
-cpu_tsc_freq_ctr(struct cpu_info *ci)
+uint64_t
+cpu_freq_ctr(struct cpu_info *ci)
 {
-   u_int64_t count, last_count, msr;
+   uint64_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);
 
msr = rdmsr(MSR_PERF_FIXED_CTR_CTRL);
@@ -423,69 +416,30 @@ cpu_tsc_freq_ctr(struct cpu_info *ci)
wrmsr(MSR_PERF_GLOBAL_CTRL, msr);
 
return ((count - last_count) * 10);
 }
 
-u_int64_t
-cpu_tsc_freq(struct cpu_info *ci)
+uint64_t
+cpu_freq(struct cpu_info *ci)
 {
-   u_int64_t 

Re: Improve the accuracy of the TSC frequency calibration - Updated Patch

2017-09-15 Thread Jonathan Gray
On Thu, Sep 14, 2017 at 03:41:25PM +0200, Reyk Floeter 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  
> > > > 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  24s   32s-3.159ms87.723ms10.389ms
> > > > >> 13.55.50.68 from pool pool.ntp.org
> > > > >> 1 10  3   11s   32s-3.433ms86.053ms18.095ms
> > > > >> 14.202.204.182 from pool pool.ntp.org
> > > > >> 1 10  1   14s   32s 1.486ms86.545ms16.483ms
> > > > >> 27.124.125.250 from pool pool.ntp.org
> > > > >>  *  1 10  2   12s   30s   -10.275ms54.156ms70.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.546ms73.450ms 2.644ms
> > > > 203.114.73.24 from pool pool.ntp.org
> > > > 1 10  2  253s 1560s-1.042ms75.133ms 0.752ms
> > > > 192.189.54.33 from pool pool.ntp.org
> > > >  *  1 10  2  204s 1558s31.644ms70.910ms 3.388ms
> > > > 54.252.165.245 from pool pool.ntp.org
> > > > 1 10  2  238s 1518s 0.146ms73.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 

Re: Improve the accuracy of the TSC frequency calibration - Updated Patch

2017-09-14 Thread Reyk Floeter

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  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  24s   32s-3.159ms87.723ms10.389ms
> > > >> 13.55.50.68 from pool pool.ntp.org
> > > >> 1 10  3   11s   32s-3.433ms86.053ms18.095ms
> > > >> 14.202.204.182 from pool pool.ntp.org
> > > >> 1 10  1   14s   32s 1.486ms86.545ms16.483ms
> > > >> 27.124.125.250 from pool pool.ntp.org
> > > >>  *  1 10  2   12s   30s   -10.275ms54.156ms70.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.546ms73.450ms 2.644ms
> > > 203.114.73.24 from pool pool.ntp.org
> > > 1 10  2  253s 1560s-1.042ms75.133ms 0.752ms
> > > 192.189.54.33 from pool pool.ntp.org
> > >  *  1 10  2  204s 1558s31.644ms70.910ms 3.388ms
> > > 54.252.165.245 from pool pool.ntp.org
> > > 1 10  2  238s 1518s 0.146ms73.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 

Re: Improve the accuracy of the TSC frequency calibration - Updated Patch

2017-08-27 Thread Mike Larkin
On Sun, Aug 27, 2017 at 09:45:52PM -0700, Mike Larkin wrote:
> 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  
> > > > > 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  24s   32s-3.159ms87.723ms10.389ms
> > > > > >> 13.55.50.68 from pool pool.ntp.org
> > > > > >> 1 10  3   11s   32s-3.433ms86.053ms18.095ms
> > > > > >> 14.202.204.182 from pool pool.ntp.org
> > > > > >> 1 10  1   14s   32s 1.486ms86.545ms16.483ms
> > > > > >> 27.124.125.250 from pool pool.ntp.org
> > > > > >>  *  1 10  2   12s   30s   -10.275ms54.156ms70.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.546ms73.450ms 2.644ms
> > > > > 203.114.73.24 from pool pool.ntp.org
> > > > > 1 10  2  253s 1560s-1.042ms75.133ms 0.752ms
> > > > > 192.189.54.33 from pool pool.ntp.org
> > > > >  *  1 10  2  204s 1558s31.644ms70.910ms 3.388ms
> > > > > 54.252.165.245 from pool pool.ntp.org
> > > > > 1 10  2  238s 1518s 0.146ms73.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:
> > > 
> > > 

Re: Improve the accuracy of the TSC frequency calibration - Updated Patch

2017-08-27 Thread Mike Larkin
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  
> > > > 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  24s   32s-3.159ms87.723ms10.389ms
> > > > >> 13.55.50.68 from pool pool.ntp.org
> > > > >> 1 10  3   11s   32s-3.433ms86.053ms18.095ms
> > > > >> 14.202.204.182 from pool pool.ntp.org
> > > > >> 1 10  1   14s   32s 1.486ms86.545ms16.483ms
> > > > >> 27.124.125.250 from pool pool.ntp.org
> > > > >>  *  1 10  2   12s   30s   -10.275ms54.156ms70.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.546ms73.450ms 2.644ms
> > > > 203.114.73.24 from pool pool.ntp.org
> > > > 1 10  2  253s 1560s-1.042ms75.133ms 0.752ms
> > > > 192.189.54.33 from pool pool.ntp.org
> > > >  *  1 10  2  204s 1558s31.644ms70.910ms 3.388ms
> > > > 54.252.165.245 from pool pool.ntp.org
> > > > 1 10  2  238s 1518s 0.146ms73.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 

Re: Improve the accuracy of the TSC frequency calibration - Updated Patch

2017-08-27 Thread Adam Steen
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  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  24s   32s-3.159ms87.723ms10.389ms
> > > >> 13.55.50.68 from pool pool.ntp.org
> > > >> 1 10  3   11s   32s-3.433ms86.053ms18.095ms
> > > >> 14.202.204.182 from pool pool.ntp.org
> > > >> 1 10  1   14s   32s 1.486ms86.545ms16.483ms
> > > >> 27.124.125.250 from pool pool.ntp.org
> > > >>  *  1 10  2   12s   30s   -10.275ms54.156ms70.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.546ms73.450ms 2.644ms
> > > 203.114.73.24 from pool pool.ntp.org
> > > 1 10  2  253s 1560s-1.042ms75.133ms 0.752ms
> > > 192.189.54.33 from pool pool.ntp.org
> > >  *  1 10  2  204s 1558s31.644ms70.910ms 3.388ms
> > > 54.252.165.245 from pool pool.ntp.org
> > > 1 10  2  238s 1518s 0.146ms73.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

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
--- 

Re: Improve the accuracy of the TSC frequency calibration - Updated Patch

2017-08-25 Thread Mike Belopuhov
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  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  24s   32s-3.159ms87.723ms10.389ms
> > >> 13.55.50.68 from pool pool.ntp.org
> > >> 1 10  3   11s   32s-3.433ms86.053ms18.095ms
> > >> 14.202.204.182 from pool pool.ntp.org
> > >> 1 10  1   14s   32s 1.486ms86.545ms16.483ms
> > >> 27.124.125.250 from pool pool.ntp.org
> > >>  *  1 10  2   12s   30s   -10.275ms54.156ms70.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.546ms73.450ms 2.644ms
> > 203.114.73.24 from pool pool.ntp.org
> > 1 10  2  253s 1560s-1.042ms75.133ms 0.752ms
> > 192.189.54.33 from pool pool.ntp.org
> >  *  1 10  2  204s 1558s31.644ms70.910ms 3.388ms
> > 54.252.165.245 from pool pool.ntp.org
> > 1 10  2  238s 1518s 0.146ms73.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.


diff --git sys/arch/amd64/amd64/acpi_machdep.c 
sys/arch/amd64/amd64/acpi_machdep.c
index 17d8fb205ef..f632b838ec2 100644
--- sys/arch/amd64/amd64/acpi_machdep.c
+++ sys/arch/amd64/amd64/acpi_machdep.c
@@ -67,10 +67,19 @@ extern int acpi_savecpu(void) __returns_twice;
 #define ACPI_BIOS_RSDP_WINDOW_BASE0xe
 #define ACPI_BIOS_RSDP_WINDOW_SIZE0x2
 
 u_int8_t   *acpi_scan(struct acpi_mem_map *, paddr_t, size_t);
 
+#define RECALIBRATE_MAX_RETRIES5
+#define RECALIBRATE_SMI_THRESHOLD  5
+#define RECALIBRATE_DELAY_THRESHOLD20
+
+struct timecounter *recalibrate_tc;
+
+uint64_t acpi_calibrate_tsc_freq(void);
+int acpi_get_tsc_and_timecount(struct timecounter *, uint64_t *, uint64_t *);
+
 int
 

Re: Improve the accuracy of the TSC frequency calibration - Updated Patch

2017-08-25 Thread Mike Larkin
On Thu, Aug 24, 2017 at 12:39:33PM +0800, Adam Steen wrote:
> On Thu, Aug 24, 2017 at 2:35 AM, Mike Larkin  wrote:
> > On Wed, Aug 23, 2017 at 09:29:15PM +0800, Adam Steen wrote:
> >> On Thu, Aug 17, 2017 at 12:19:28PM +0800, Adam Steen wrote:
> >> > On Tue, Aug 8, 2017 at 10:12 PM, Mike Belopuhov  
> >> > wrote:
> >> > > On Tue, Aug 08, 2017 at 08:18 +0800, Adam Steen wrote:
> >> > >> On Mon, Jul 31, 2017 at 3:58 PM, Mike Belopuhov  
> >> > >> 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.
> >> >
> >> 

Re: Improve the accuracy of the TSC frequency calibration - Updated Patch

2017-08-23 Thread Adam Steen
On Thu, Aug 24, 2017 at 2:35 AM, Mike Larkin  wrote:
> On Wed, Aug 23, 2017 at 09:29:15PM +0800, Adam Steen wrote:
>> On Thu, Aug 17, 2017 at 12:19:28PM +0800, Adam Steen wrote:
>> > On Tue, Aug 8, 2017 at 10:12 PM, Mike Belopuhov  wrote:
>> > > On Tue, Aug 08, 2017 at 08:18 +0800, Adam Steen wrote:
>> > >> On Mon, Jul 31, 2017 at 3:58 PM, Mike Belopuhov  
>> > >> 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=150148792804747=2
>> >
>> > I feel like i don't know enough about the 

Re: Improve the accuracy of the TSC frequency calibration - Updated Patch

2017-08-23 Thread Mike Larkin
On Wed, Aug 23, 2017 at 09:29:15PM +0800, Adam Steen wrote:
> On Thu, Aug 17, 2017 at 12:19:28PM +0800, Adam Steen wrote:
> > On Tue, Aug 8, 2017 at 10:12 PM, Mike Belopuhov  wrote:
> > > On Tue, Aug 08, 2017 at 08:18 +0800, Adam Steen wrote:
> > >> On Mon, Jul 31, 2017 at 3:58 PM, Mike Belopuhov  
> > >> 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=150148792804747=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, 

Re: Improve the accuracy of the TSC frequency calibration - Updated Patch

2017-08-23 Thread Adam Steen
On Thu, Aug 17, 2017 at 12:19:28PM +0800, Adam Steen wrote:
> On Tue, Aug 8, 2017 at 10:12 PM, Mike Belopuhov  wrote:
> > On Tue, Aug 08, 2017 at 08:18 +0800, Adam Steen wrote:
> >> On Mon, Jul 31, 2017 at 3:58 PM, Mike Belopuhov  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=150148792804747=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



Thank you Mike on the feedback on the last patch,