Re: Improve the accuracy of the TSC frequency calibration (Was: Calculate the frequency of the tsc timecounter)

[Adam Steen]

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

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 - 1.78
+++ sys/arch/amd64/amd64/acpi_machdep.c 

Re: Improve the accuracy of the TSC frequency calibration (Was: Calculate the frequency of the tsc timecounter)

[Mike Belopuhov]

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 (Was: Calculate the frequency of the tsc timecounter)

[Ted Unangst]

Adam Steen wrote:
> 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)
> 
> But I looked into using the acpihpet directly but it is never exposed
> outside of acpihpet.c.
> 
> Could someone point me to were if would be appropriate to complete
> this calibration and how to use the acpihpet? (Will it need to be
> exposed like i8254_delay/delay_func/delay in machdep.c and cpu.h)

heh, you're getting into the fun stuff now...

have a look at dev/acpi/files.acpi and compare acpicpu and acpihpet. see the
needs-flag? you need to add that.

that gives you a header "acpihpet.h" that you can include to get NACPIHPET to
optionally compile code if acpihpet is included in the kernel.

grep around under amd64/ for acpicpu.h and NACPICPU for examples.