16:30:02 <sommerfeld> noticed NTP having trouble on a box after reboot (clock frequency was off by a lot, despite having been stable before reboot).  I think I see why in the HPET code (and I noticed another glitch while I was in there..)
16:30:21 <sommerfeld> will be filing bugs shortly..
16:34:59 <sommerfeld> first glitch: hpet_read_main_counter_value() loops until the most significant 32 bits read the same twice in a row to work around tearing of the 64-bit value by reads through a 32-bit bus somewhere in the chip.   Except that `counter` isn't declared volatile, and the compiler eliminates the loop.
16:39:32 <sommerfeld> the machine in question I noticed this on runs the HPET at 19.2MHz, which is ~52ns/tick.  the loop that reads the HPET vs the TSC terminates right after the HPET ticks up, but it doesn't *start* at a tick (two consecutive reads that differ), so there will be ~52ns of slop at the start, which is quite a bit when the overall loop runs for only 100us.  
16:41:33 <sommerfeld> calibration might well be more stable if it spun waiting for the HPET to tick at the start.   
16:52:22 <sommerfeld> looks like I rediscovered https://www.illumos.org/issues/14556
16:52:23 <fenix> → BUG 14556: TSC calibration from HPET includes too much error (New)
17:18:28 <jbk> it might be useful to note which HPET you have as well -- apparently not all HPETs are created equal... e.g. on the system I had with both an HPET and TSC, the HPET consistently gave more accurate results than the TSC (the other system didn't have a TSC but the result agreed fairly well with the expected clock rate), but apparently other HPETs are not as great (they they are _supposed_ to at least have a 
17:18:33 <gitomat> [illumos-gate] 16763 dladm simnet functionality should be documented -- William Welliver <william⊙wo>
17:18:34 <jbk> minimum frequency
17:21:37 <jbk> (at least compared to the published frequency -- which intel says in the architecture manuals you should not use, even though intel themselves has contributed code to linux that does exactly what they say you shouldn't)
17:27:49 <sommerfeld> you mean PIT rather than TSC ? (anything we run on should have a TSC)
18:31:26 <jbk> that's not true (and was the reason behind the changes)
18:31:36 <jbk> at least some newer systems don't have a PIT
18:31:48 <jbk> err yeah
18:31:55 <jbk> been a long day already :)
18:31:58 <jbk> don't have a PIT
18:32:21 <jbk> annoying there's no way to tell other than try it and guess it's probably not makingprogress
18:33:06 <jbk> that and my brain is still fried from a excruciating problem over the long weekend
19:59:59 <sommerfeld> okay, threw a bunch of tweaks and instrumentation into the hpet code and it's looking much more accurate (previously NTP found a frequency error of 327 ppm.  now it locked with <1ms offset and a frequency error of ~2 ppm); I'll let it sit for the afternoon ...
20:02:11 <jbk> is the system new(ish) ?
20:03:44 <jbk> I've been meaning to throw up a change that adds using CPUID (when possible)... now that I've found a couple of systems where leaf 15 actually provides the values
20:04:25 <jbk> https://gist.github.com/jasonbking/6f1475e4fe1684042b9fb17c617d3151 is a small program that'll test (just reads /dev/cpu/self/cpuid)
20:11:51 <jbk> oh hrm.. tsoome isn't here..
20:17:23 <richlowe> tsoome_: 
20:17:49 <jbk> oh
20:22:55 <sommerfeld> newish.  "12th Gen Intel(r) Core(tm) i5-1235U"
20:24:40 <sommerfeld> From leaf 0x15:
20:24:40 <sommerfeld> eax: 2 ebx: 130 ecx: 38400000 edx: 0
20:24:40 <sommerfeld> TSC Frequency: 0.35 GHz
20:24:48 <sommerfeld> which is .. not what calibration found
20:25:52 <sommerfeld>  TSC calibrated using HPET; freq is 2496 MHz
20:26:10 <sommerfeld> (or in my patched code:  TSC calibrated using HPET; freq is 2496094662 Hz
20:26:11 <sommerfeld> )
20:35:23 <jbk> hrmm.. that's odd
23:33:46 <sommerfeld> jbk: aha, it's an integer overflow issue.
23:34:14 <sommerfeld> need to widen things to 64 bits first.
23:37:46 <sommerfeld> modified version yields 2496MHz which matches the HPET calibration.
23:52:51 <sommerfeld> 38400000 * 130 = 4992000000 which doesn't fit in a uint32_t