[PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Adrian Hunter]

If it takes longer than 12us to read the PIT counter lsb/msb,
then the error margin will never fall below 500ppm within 50ms,
and Fast TSC calibration will always fail.

This patch detects when that will happen and switches to using
a slightly different algorithm that takes advantage of the PIT's
latch comand.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 arch/x86/kernel/tsc.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 132 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 5054497..0035682 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -553,6 +553,40 @@ static inline int pit_expect_msb(unsigned char val, u64 *tscp, unsigned long *de
 }
 
 /*
+ * High latency version of pit_expect_msb(). Instead of using the read latency
+ * as the error margin, latch the counter and use that latency. The counter
+ * latches at the current value which means there is an addition error margin
+ * of 1 timer tick which is not accounted for here.
+ */
+static inline int pit_expect_msb_hl(unsigned char val, u64 *tscp,
+				    unsigned long *deltap, u16 *cnt)
+{
+	u64 tsc0, tsc1;
+	int count;
+	u8 lsb, msb;
+
+	for (count = 0; count < 50000; count++) {
+		tsc0 = get_cycles();
+		/* Latch counter 2 */
+		outb(0x80, 0x43);
+		tsc1 = get_cycles();
+		lsb = inb(0x42);
+		msb = inb(0x42);
+		if (msb != val)
+			break;
+	}
+	*deltap = tsc1 - tsc0;
+	*tscp = tsc0;
+	*cnt = lsb | ((u16)msb << 8);
+
+	/*
+	 * We require _some_ success, but the quality control
+	 * will be based on the error terms on the TSC values.
+	 */
+	return count > 5;
+}
+
+/*
  * How many MSB values do we want to see? We aim for
  * a maximum error rate of 500ppm (in practice the
  * real error is much smaller), but refuse to spend
@@ -561,6 +595,94 @@ static inline int pit_expect_msb(unsigned char val, u64 *tscp, unsigned long *de
 #define MAX_QUICK_PIT_MS 50
 #define MAX_QUICK_PIT_ITERATIONS (MAX_QUICK_PIT_MS * PIT_TICK_RATE / 1000 / 256)
 
+/*
+ * High latency version of quick_pit_calibrate() that works even if there is a
+ * high latency reading the PIT lsb/msb. This is needed if it takes longer than
+ * 12us to read the lsb/msb because then the error margin will never fall below
+ * 500ppm within 50ms.
+ */
+static unsigned long quick_pit_calibrate_hl(void)
+{
+	int i;
+	u64 tsc, delta;
+	unsigned long d1, d2;
+	u16 cnt0, cnt1, dc;
+
+	/* Re-start at 0xffff */
+	outb(0xff, 0x42);
+	outb(0xff, 0x42);
+
+	/*
+	 * The PIT starts counting at the next edge, so we
+	 * need to delay for a microsecond. The easiest way
+	 * to do that is to just read back the 16-bit counter
+	 * once from the PIT.
+	 */
+	pit_verify_msb(0);
+
+	/*
+	 * Iterate until the error is less than 500 ppm. The error margin due to
+	 * the time to latch the counter is d1 + d2. The counter latches the
+	 * current count which introduces a one tick error at the start and end.
+	 * So the total error is  d1 + d2 + 2 timer ticks. A timer tick is
+	 * approximately the TSC delta divided by the timer delta. So the error
+	 * margin is too high while:
+	 *      d1 + d2 + 2 * (delta / dc) >= delta >> 11
+	 *   => d1 + d2 >= (delta >> 11) - 2 * (delta / dc)
+	 *   => d1 + d2 >= (delta - 4096 * (delta / dc)) / 2048
+	 *   => (d1 + d2) * dc >= (dc * delta - 4096 * delta) / 2048
+	 *   => (d1 + d2) * dc >= (dc - 4096) * delta >> 11
+	*/
+	if (pit_expect_msb_hl(0xff, &tsc, &d1, &cnt0)) {
+		for (i = 1; i <= MAX_QUICK_PIT_ITERATIONS; i++) {
+			if (!pit_expect_msb_hl(0xff - i, &delta, &d2, &cnt1))
+				break;
+
+			dc = cnt0 - cnt1;
+			if (dc < 4096)
+				continue;
+
+			delta -= tsc;
+
+			if ((d1 + d2) * (u64)dc >= (dc - 4096) * delta >> 11)
+				continue;
+
+			/*
+			 * Check the PIT one more time to verify that
+			 * all TSC reads were stable wrt the PIT.
+			 *
+			 * This also guarantees serialization of the
+			 * last cycle read ('d2') in pit_expect_msb.
+			 */
+			if (!pit_verify_msb(0xfe - i))
+				break;
+			goto success;
+		}
+	}
+	pr_info("Fast TSC calibration (with latch) failed\n");
+	return 0;
+
+success:
+	/*
+	 * Ok, if we get here, then we've seen the
+	 * MSB of the PIT decrement 'i' times, and the
+	 * error has shrunk to less than 500 ppm.
+	 *
+	 * As a result, we can depend on there not being
+	 * any odd delays anywhere, and the TSC reads are
+	 * reliable (within the error).
+	 *
+	 * kHz = ticks / time-in-seconds / 1000;
+	 * kHz = ticks / (PIT count / PIT_TICK_RATE) / 1000
+	 * kHz = delta / (dc / PIT_TICK_RATE) / 1000
+	 * kHz = (delta * PIT_TICK_RATE) / (dc * 1000)
+	 */
+	delta *= PIT_TICK_RATE;
+	do_div(delta, dc * 1000);
+	pr_info("Fast TSC calibration (with latch) using PIT\n");
+	return delta;
+}
+
 static unsigned long quick_pit_calibrate(void)
 {
 	int i;
@@ -598,10 +720,19 @@ static unsigned long quick_pit_calibrate(void)
 			if (!pit_expect_msb(0xff-i, &delta, &d2))
 				break;
 
+			delta -= tsc;
+
+			/*
+			 * Extrapolate the error and switch to high-latency
+			 * algorithm if the error will never be below 500 ppm.
+			 */
+			if (i == 1 &&
+			    d1 + d2 >= (delta * MAX_QUICK_PIT_ITERATIONS) >> 11)
+				return quick_pit_calibrate_hl();
+
 			/*
 			 * Iterate until the error is less than 500 ppm
 			 */
-			delta -= tsc;
 			if (d1+d2 >= delta >> 11)
 				continue;
 
-- 
1.9.1

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Adrian Hunter]

On 21/05/15 10:55, Adrian Hunter wrote:
> If it takes longer than 12us to read the PIT counter lsb/msb,
> then the error margin will never fall below 500ppm within 50ms,
> and Fast TSC calibration will always fail.

Hi

This does happen, so it would be nice to have this fix.
Are there any comments?

Regards
Adrian


> 
> This patch detects when that will happen and switches to using
> a slightly different algorithm that takes advantage of the PIT's
> latch comand.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  arch/x86/kernel/tsc.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 132 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 5054497..0035682 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -553,6 +553,40 @@ static inline int pit_expect_msb(unsigned char val, u64 *tscp, unsigned long *de
>  }
>  
>  /*
> + * High latency version of pit_expect_msb(). Instead of using the read latency
> + * as the error margin, latch the counter and use that latency. The counter
> + * latches at the current value which means there is an addition error margin
> + * of 1 timer tick which is not accounted for here.
> + */
> +static inline int pit_expect_msb_hl(unsigned char val, u64 *tscp,
> +				    unsigned long *deltap, u16 *cnt)
> +{
> +	u64 tsc0, tsc1;
> +	int count;
> +	u8 lsb, msb;
> +
> +	for (count = 0; count < 50000; count++) {
> +		tsc0 = get_cycles();
> +		/* Latch counter 2 */
> +		outb(0x80, 0x43);
> +		tsc1 = get_cycles();
> +		lsb = inb(0x42);
> +		msb = inb(0x42);
> +		if (msb != val)
> +			break;
> +	}
> +	*deltap = tsc1 - tsc0;
> +	*tscp = tsc0;
> +	*cnt = lsb | ((u16)msb << 8);
> +
> +	/*
> +	 * We require _some_ success, but the quality control
> +	 * will be based on the error terms on the TSC values.
> +	 */
> +	return count > 5;
> +}
> +
> +/*
>   * How many MSB values do we want to see? We aim for
>   * a maximum error rate of 500ppm (in practice the
>   * real error is much smaller), but refuse to spend
> @@ -561,6 +595,94 @@ static inline int pit_expect_msb(unsigned char val, u64 *tscp, unsigned long *de
>  #define MAX_QUICK_PIT_MS 50
>  #define MAX_QUICK_PIT_ITERATIONS (MAX_QUICK_PIT_MS * PIT_TICK_RATE / 1000 / 256)
>  
> +/*
> + * High latency version of quick_pit_calibrate() that works even if there is a
> + * high latency reading the PIT lsb/msb. This is needed if it takes longer than
> + * 12us to read the lsb/msb because then the error margin will never fall below
> + * 500ppm within 50ms.
> + */
> +static unsigned long quick_pit_calibrate_hl(void)
> +{
> +	int i;
> +	u64 tsc, delta;
> +	unsigned long d1, d2;
> +	u16 cnt0, cnt1, dc;
> +
> +	/* Re-start at 0xffff */
> +	outb(0xff, 0x42);
> +	outb(0xff, 0x42);
> +
> +	/*
> +	 * The PIT starts counting at the next edge, so we
> +	 * need to delay for a microsecond. The easiest way
> +	 * to do that is to just read back the 16-bit counter
> +	 * once from the PIT.
> +	 */
> +	pit_verify_msb(0);
> +
> +	/*
> +	 * Iterate until the error is less than 500 ppm. The error margin due to
> +	 * the time to latch the counter is d1 + d2. The counter latches the
> +	 * current count which introduces a one tick error at the start and end.
> +	 * So the total error is  d1 + d2 + 2 timer ticks. A timer tick is
> +	 * approximately the TSC delta divided by the timer delta. So the error
> +	 * margin is too high while:
> +	 *      d1 + d2 + 2 * (delta / dc) >= delta >> 11
> +	 *   => d1 + d2 >= (delta >> 11) - 2 * (delta / dc)
> +	 *   => d1 + d2 >= (delta - 4096 * (delta / dc)) / 2048
> +	 *   => (d1 + d2) * dc >= (dc * delta - 4096 * delta) / 2048
> +	 *   => (d1 + d2) * dc >= (dc - 4096) * delta >> 11
> +	*/
> +	if (pit_expect_msb_hl(0xff, &tsc, &d1, &cnt0)) {
> +		for (i = 1; i <= MAX_QUICK_PIT_ITERATIONS; i++) {
> +			if (!pit_expect_msb_hl(0xff - i, &delta, &d2, &cnt1))
> +				break;
> +
> +			dc = cnt0 - cnt1;
> +			if (dc < 4096)
> +				continue;
> +
> +			delta -= tsc;
> +
> +			if ((d1 + d2) * (u64)dc >= (dc - 4096) * delta >> 11)
> +				continue;
> +
> +			/*
> +			 * Check the PIT one more time to verify that
> +			 * all TSC reads were stable wrt the PIT.
> +			 *
> +			 * This also guarantees serialization of the
> +			 * last cycle read ('d2') in pit_expect_msb.
> +			 */
> +			if (!pit_verify_msb(0xfe - i))
> +				break;
> +			goto success;
> +		}
> +	}
> +	pr_info("Fast TSC calibration (with latch) failed\n");
> +	return 0;
> +
> +success:
> +	/*
> +	 * Ok, if we get here, then we've seen the
> +	 * MSB of the PIT decrement 'i' times, and the
> +	 * error has shrunk to less than 500 ppm.
> +	 *
> +	 * As a result, we can depend on there not being
> +	 * any odd delays anywhere, and the TSC reads are
> +	 * reliable (within the error).
> +	 *
> +	 * kHz = ticks / time-in-seconds / 1000;
> +	 * kHz = ticks / (PIT count / PIT_TICK_RATE) / 1000
> +	 * kHz = delta / (dc / PIT_TICK_RATE) / 1000
> +	 * kHz = (delta * PIT_TICK_RATE) / (dc * 1000)
> +	 */
> +	delta *= PIT_TICK_RATE;
> +	do_div(delta, dc * 1000);
> +	pr_info("Fast TSC calibration (with latch) using PIT\n");
> +	return delta;
> +}
> +
>  static unsigned long quick_pit_calibrate(void)
>  {
>  	int i;
> @@ -598,10 +720,19 @@ static unsigned long quick_pit_calibrate(void)
>  			if (!pit_expect_msb(0xff-i, &delta, &d2))
>  				break;
>  
> +			delta -= tsc;
> +
> +			/*
> +			 * Extrapolate the error and switch to high-latency
> +			 * algorithm if the error will never be below 500 ppm.
> +			 */
> +			if (i == 1 &&
> +			    d1 + d2 >= (delta * MAX_QUICK_PIT_ITERATIONS) >> 11)
> +				return quick_pit_calibrate_hl();
> +
>  			/*
>  			 * Iterate until the error is less than 500 ppm
>  			 */
> -			delta -= tsc;
>  			if (d1+d2 >= delta >> 11)
>  				continue;
>  
> 

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Thomas Gleixner]

On Mon, 1 Jun 2015, Adrian Hunter wrote:

> On 21/05/15 10:55, Adrian Hunter wrote:
> > If it takes longer than 12us to read the PIT counter lsb/msb,
> > then the error margin will never fall below 500ppm within 50ms,
> > and Fast TSC calibration will always fail.
> 
> Hi
> 
> This does happen, so it would be nice to have this fix.
> Are there any comments?

Yes. Its on my review list...

Thanks,

	tglx

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Thomas Gleixner]

On Thu, 21 May 2015, Adrian Hunter wrote:

> If it takes longer than 12us to read the PIT counter lsb/msb,
> then the error margin will never fall below 500ppm within 50ms,
> and Fast TSC calibration will always fail.

So finally the legacy PIT emulation takes longer than the 30 years old
hardware implementation. Progress!
 
> This patch detects when that will happen and switches to using
> a slightly different algorithm that takes advantage of the PIT's
> latch comand.

Is there really no smarter way to figure out the TSC frequency on
modern systems?

Thanks,

	tglx

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Andy Lutomirski]

On Tue, Jun 2, 2015 at 12:33 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, 21 May 2015, Adrian Hunter wrote:
>
>> If it takes longer than 12us to read the PIT counter lsb/msb,
>> then the error margin will never fall below 500ppm within 50ms,
>> and Fast TSC calibration will always fail.
>
> So finally the legacy PIT emulation takes longer than the 30 years old
> hardware implementation. Progress!
>
>> This patch detects when that will happen and switches to using
>> a slightly different algorithm that takes advantage of the PIT's
>> latch comand.
>
> Is there really no smarter way to figure out the TSC frequency on
> modern systems?

I just asked Len this question yesterday.  intel_pstate can do it,
although the algorithm is a bit gross.

--Andy

>
> Thanks,
>
>         tglx



-- 
Andy Lutomirski
AMA Capital Management, LLC

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Andi Kleen]

On Tue, Jun 02, 2015 at 12:41:27PM -0700, Andy Lutomirski wrote:
> On Tue, Jun 2, 2015 at 12:33 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Thu, 21 May 2015, Adrian Hunter wrote:
> >
> >> If it takes longer than 12us to read the PIT counter lsb/msb,
> >> then the error margin will never fall below 500ppm within 50ms,
> >> and Fast TSC calibration will always fail.
> >
> > So finally the legacy PIT emulation takes longer than the 30 years old
> > hardware implementation. Progress!
> >
> >> This patch detects when that will happen and switches to using
> >> a slightly different algorithm that takes advantage of the PIT's
> >> latch comand.
> >
> > Is there really no smarter way to figure out the TSC frequency on
> > modern systems?
> 
> I just asked Len this question yesterday.  intel_pstate can do it,
> although the algorithm is a bit gross.

intel_pstate needs to know the model number. If you know the 
model number sure you can do a lot better (e.g. using the 
ref-clock fixed counter or some other methods)

But if you don't you need something else. And at some point
the only thing left over is the PIT.

-Andi

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Thomas Gleixner]

On Tue, 2 Jun 2015, Andi Kleen wrote:
> On Tue, Jun 02, 2015 at 12:41:27PM -0700, Andy Lutomirski wrote:
> > On Tue, Jun 2, 2015 at 12:33 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > > On Thu, 21 May 2015, Adrian Hunter wrote:
> > >
> > >> If it takes longer than 12us to read the PIT counter lsb/msb,
> > >> then the error margin will never fall below 500ppm within 50ms,
> > >> and Fast TSC calibration will always fail.
> > >
> > > So finally the legacy PIT emulation takes longer than the 30 years old
> > > hardware implementation. Progress!
> > >
> > >> This patch detects when that will happen and switches to using
> > >> a slightly different algorithm that takes advantage of the PIT's
> > >> latch comand.
> > >
> > > Is there really no smarter way to figure out the TSC frequency on
> > > modern systems?
> > 
> > I just asked Len this question yesterday.  intel_pstate can do it,
> > although the algorithm is a bit gross.

Well, the algorithm for that slow PIT emulation thing is definitely
not from the straight forward kind either.

> intel_pstate needs to know the model number. If you know the 
> model number sure you can do a lot better (e.g. using the 
> ref-clock fixed counter or some other methods)
> 
> But if you don't you need something else. And at some point
> the only thing left over is the PIT.

Right, and if we dont have a way to readout the stuff because the
kernel does not yet have support for that particular model it falls
back to PIT slow path in the worst case.

That's better than having another weird calibration routine which
might be outdated with the next generation of PIT emulation. We've
been there with the HPET deferred writes already. No need to have more
of this nonsense.

It's about time that Intel gets its act together and provides a proper
architected way to figure that out. Though I fear that will require
another 15 years of yelling (IIRC that was the time it took to get a
constant frequency TSC).

Thanks,

	tglx

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Andy Lutomirski]

On Tue, Jun 2, 2015 at 12:58 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Tue, 2 Jun 2015, Andi Kleen wrote:
>> On Tue, Jun 02, 2015 at 12:41:27PM -0700, Andy Lutomirski wrote:
>> > On Tue, Jun 2, 2015 at 12:33 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > > On Thu, 21 May 2015, Adrian Hunter wrote:
>> > >
>> > >> If it takes longer than 12us to read the PIT counter lsb/msb,
>> > >> then the error margin will never fall below 500ppm within 50ms,
>> > >> and Fast TSC calibration will always fail.
>> > >
>> > > So finally the legacy PIT emulation takes longer than the 30 years old
>> > > hardware implementation. Progress!
>> > >
>> > >> This patch detects when that will happen and switches to using
>> > >> a slightly different algorithm that takes advantage of the PIT's
>> > >> latch comand.
>> > >
>> > > Is there really no smarter way to figure out the TSC frequency on
>> > > modern systems?
>> >
>> > I just asked Len this question yesterday.  intel_pstate can do it,
>> > although the algorithm is a bit gross.
>
> Well, the algorithm for that slow PIT emulation thing is definitely
> not from the straight forward kind either.
>
>> intel_pstate needs to know the model number. If you know the
>> model number sure you can do a lot better (e.g. using the
>> ref-clock fixed counter or some other methods)
>>
>> But if you don't you need something else. And at some point
>> the only thing left over is the PIT.
>
> Right, and if we dont have a way to readout the stuff because the
> kernel does not yet have support for that particular model it falls
> back to PIT slow path in the worst case.
>
> That's better than having another weird calibration routine which
> might be outdated with the next generation of PIT emulation. We've
> been there with the HPET deferred writes already. No need to have more
> of this nonsense.
>
> It's about time that Intel gets its act together and provides a proper
> architected way to figure that out. Though I fear that will require
> another 15 years of yelling (IIRC that was the time it took to get a
> constant frequency TSC).

There's the code in tsc_msr.c.  It should be relatively
straightforward to extend it to cover everything that intel_pstate
supports.

But yes, Intel should add an MSR that gives the frequency in Hz.

--Andy

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Andi Kleen]

> There's the code in tsc_msr.c.  It should be relatively
> straightforward to extend it to cover everything that intel_pstate
> supports.

That's a good idea, but we still need an always working fallback when the
model number is not available. So Adrian's patch is needed in any
case.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Thomas Gleixner]

On Tue, 2 Jun 2015, Andi Kleen wrote:

> > There's the code in tsc_msr.c.  It should be relatively
> > straightforward to extend it to cover everything that intel_pstate
> > supports.
> 
> That's a good idea, but we still need an always working fallback when the
> model number is not available. So Adrian's patch is needed in any
> case.
 
Nonsense. The slow calibration is already a working fallback.

Thanks,

	tglx
 

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Andi Kleen]

On Tue, Jun 02, 2015 at 11:03:26PM +0200, Thomas Gleixner wrote:
> 
> 
> On Tue, 2 Jun 2015, Andi Kleen wrote:
> 
> > > There's the code in tsc_msr.c.  It should be relatively
> > > straightforward to extend it to cover everything that intel_pstate
> > > supports.
> > 
> > That's a good idea, but we still need an always working fallback when the
> > model number is not available. So Adrian's patch is needed in any
> > case.
>  
> Nonsense. The slow calibration is already a working fallback.

Please read Adrian's description again. It's not working when the PIT read is
too slow. That is when the new algorithm is needed.

-Andi

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Andy Lutomirski]

On Tue, Jun 2, 2015 at 4:38 PM, Andi Kleen <ak@linux.intel.com> wrote:
> On Tue, Jun 02, 2015 at 11:03:26PM +0200, Thomas Gleixner wrote:
>>
>>
>> On Tue, 2 Jun 2015, Andi Kleen wrote:
>>
>> > > There's the code in tsc_msr.c.  It should be relatively
>> > > straightforward to extend it to cover everything that intel_pstate
>> > > supports.
>> >
>> > That's a good idea, but we still need an always working fallback when the
>> > model number is not available. So Adrian's patch is needed in any
>> > case.
>>
>> Nonsense. The slow calibration is already a working fallback.
>
> Please read Adrian's description again. It's not working when the PIT read is
> too slow. That is when the new algorithm is needed.
>

tglx's suggestion was to use slow calibration as a fallback.

--Andy

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Andi Kleen]

On Tue, Jun 02, 2015 at 05:21:27PM -0700, Andy Lutomirski wrote:
> On Tue, Jun 2, 2015 at 4:38 PM, Andi Kleen <ak@linux.intel.com> wrote:
> > On Tue, Jun 02, 2015 at 11:03:26PM +0200, Thomas Gleixner wrote:
> >>
> >>
> >> On Tue, 2 Jun 2015, Andi Kleen wrote:
> >>
> >> > > There's the code in tsc_msr.c.  It should be relatively
> >> > > straightforward to extend it to cover everything that intel_pstate
> >> > > supports.
> >> >
> >> > That's a good idea, but we still need an always working fallback when the
> >> > model number is not available. So Adrian's patch is needed in any
> >> > case.
> >>
> >> Nonsense. The slow calibration is already a working fallback.
> >
> > Please read Adrian's description again. It's not working when the PIT read is
> > too slow. That is when the new algorithm is needed.
> >
> 
> tglx's suggestion was to use slow calibration as a fallback.

You mean the last fallback we have today?

That one doesn't work if the PIT read is too slow.

And Adrian's patch is fixing that.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Andy Lutomirski]

On Tue, Jun 2, 2015 at 5:39 PM, Andi Kleen <ak@linux.intel.com> wrote:
> On Tue, Jun 02, 2015 at 05:21:27PM -0700, Andy Lutomirski wrote:
>> On Tue, Jun 2, 2015 at 4:38 PM, Andi Kleen <ak@linux.intel.com> wrote:
>> > On Tue, Jun 02, 2015 at 11:03:26PM +0200, Thomas Gleixner wrote:
>> >>
>> >>
>> >> On Tue, 2 Jun 2015, Andi Kleen wrote:
>> >>
>> >> > > There's the code in tsc_msr.c.  It should be relatively
>> >> > > straightforward to extend it to cover everything that intel_pstate
>> >> > > supports.
>> >> >
>> >> > That's a good idea, but we still need an always working fallback when the
>> >> > model number is not available. So Adrian's patch is needed in any
>> >> > case.
>> >>
>> >> Nonsense. The slow calibration is already a working fallback.
>> >
>> > Please read Adrian's description again. It's not working when the PIT read is
>> > too slow. That is when the new algorithm is needed.
>> >
>>
>> tglx's suggestion was to use slow calibration as a fallback.
>
> You mean the last fallback we have today?
>
> That one doesn't work if the PIT read is too slow.
>
> And Adrian's patch is fixing that.

Then the changelog should say that I think.  The current text says
"Fast TSC calibration will always fail", which, to me, suggests that
either the slow calibration will work or that the changelog message
should be changed.

--Andy

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Andi Kleen]

> Then the changelog should say that I think.  The current text says
> "Fast TSC calibration will always fail", which, to me, suggests that
> either the slow calibration will work or that the changelog message
> should be changed.

Ok. No, the slow calibration works I believe.

-Andi

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Adrian Hunter]

On 03/06/15 06:30, Andi Kleen wrote:
>> Then the changelog should say that I think.  The current text says
>> "Fast TSC calibration will always fail", which, to me, suggests that
>> either the slow calibration will work or that the changelog message
>> should be changed.
> 
> Ok. No, the slow calibration works I believe.

Yeah, so the (only?) downside is the 50ms wasted on Fast TSC
calibration.  What about this?


From: Adrian Hunter <adrian.hunter@intel.com>
Date: Wed, 3 Jun 2015 10:39:46 +0300
Subject: [PATCH] x86, tsc: Let high latency PIT fail fast in
 quick_pit_calibrate()

If it takes longer than 12us to read the PIT counter lsb/msb,
then the error margin will never fall below 500ppm within 50ms,
and Fast TSC calibration will always fail.

This patch detects when that will happen and fails fast. Note
the failure message is not printed in that case because:
1. it will always happen on that class of hardware
2. the absence of the message is more informative than its
presence

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 arch/x86/kernel/tsc.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 7114c86af35e..673c4f25021f 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -598,10 +598,19 @@ static unsigned long quick_pit_calibrate(void)
 			if (!pit_expect_msb(0xff-i, &delta, &d2))
 				break;
 
+			delta -= tsc;
+
+			/*
+			 * Extrapolate the error and fail fast if the error will
+			 * never be below 500 ppm.
+			 */
+			if (i == 1 &&
+			    d1 + d2 >= (delta * MAX_QUICK_PIT_ITERATIONS) >> 11)
+				return 0;
+
 			/*
 			 * Iterate until the error is less than 500 ppm
 			 */
-			delta -= tsc;
 			if (d1+d2 >= delta >> 11)
 				continue;
 
-- 
1.9.1

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Linus Torvalds]

On Wed, Jun 3, 2015 at 1:13 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> Yeah, so the (only?) downside is the 50ms wasted on Fast TSC
> calibration.  What about this?

That's certainly simpler.

What platform is this? Do you have access to hw people you can shake
down and ask what clock we can really _trust_?

                    Linus

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Adrian Hunter]

On 03/06/15 16:45, Linus Torvalds wrote:
> On Wed, Jun 3, 2015 at 1:13 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>> Yeah, so the (only?) downside is the 50ms wasted on Fast TSC
>> calibration.  What about this?
> 
> That's certainly simpler.
> 
> What platform is this?

It's a prototype.

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Thomas Gleixner]

On Wed, 3 Jun 2015, Adrian Hunter wrote:

> On 03/06/15 06:30, Andi Kleen wrote:
> >> Then the changelog should say that I think.  The current text says
> >> "Fast TSC calibration will always fail", which, to me, suggests that
> >> either the slow calibration will work or that the changelog message
> >> should be changed.
> > 
> > Ok. No, the slow calibration works I believe.
> 
> Yeah, so the (only?) downside is the 50ms wasted on Fast TSC
> calibration.  What about this?

I'm certainly happy to apply this one. 

Thanks,

	tglx

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Adrian Hunter]

On 03/06/15 19:23, Thomas Gleixner wrote:
> On Wed, 3 Jun 2015, Adrian Hunter wrote:
> 
>> On 03/06/15 06:30, Andi Kleen wrote:
>>>> Then the changelog should say that I think.  The current text says
>>>> "Fast TSC calibration will always fail", which, to me, suggests that
>>>> either the slow calibration will work or that the changelog message
>>>> should be changed.
>>>
>>> Ok. No, the slow calibration works I believe.
>>
>> Yeah, so the (only?) downside is the 50ms wasted on Fast TSC
>> calibration.  What about this?
> 
> I'm certainly happy to apply this one. 

George Spelvin began investigating improving quick_pit_calibrate() but Ingo
anyway suggested this patch as the first, so can this be applied?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Thomas Gleixner]

On Mon, 22 Jun 2015, Adrian Hunter wrote:
> On 03/06/15 19:23, Thomas Gleixner wrote:
> > On Wed, 3 Jun 2015, Adrian Hunter wrote:
> > 
> >> On 03/06/15 06:30, Andi Kleen wrote:
> >>>> Then the changelog should say that I think.  The current text says
> >>>> "Fast TSC calibration will always fail", which, to me, suggests that
> >>>> either the slow calibration will work or that the changelog message
> >>>> should be changed.
> >>>
> >>> Ok. No, the slow calibration works I believe.
> >>
> >> Yeah, so the (only?) downside is the 50ms wasted on Fast TSC
> >> calibration.  What about this?
> > 
> > I'm certainly happy to apply this one. 
> 
> George Spelvin began investigating improving quick_pit_calibrate() but Ingo
> anyway suggested this patch as the first, so can this be applied?

Oh, yes. I simply forgot to pick it up. Thanks for the reminder.

    tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Adrian Hunter]

On 22/06/15 16:14, Thomas Gleixner wrote:
> On Mon, 22 Jun 2015, Adrian Hunter wrote:
>> On 03/06/15 19:23, Thomas Gleixner wrote:
>>> On Wed, 3 Jun 2015, Adrian Hunter wrote:
>>>
>>>> On 03/06/15 06:30, Andi Kleen wrote:
>>>>>> Then the changelog should say that I think.  The current text says
>>>>>> "Fast TSC calibration will always fail", which, to me, suggests that
>>>>>> either the slow calibration will work or that the changelog message
>>>>>> should be changed.
>>>>>
>>>>> Ok. No, the slow calibration works I believe.
>>>>
>>>> Yeah, so the (only?) downside is the 50ms wasted on Fast TSC
>>>> calibration.  What about this?
>>>
>>> I'm certainly happy to apply this one. 
>>
>> George Spelvin began investigating improving quick_pit_calibrate() but Ingo
>> anyway suggested this patch as the first, so can this be applied?
> 
> Oh, yes. I simply forgot to pick it up. Thanks for the reminder.

Sorry to bother you, but I don't still don't see the patch anywhere?

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Thomas Gleixner]

On Mon, 6 Jul 2015, Adrian Hunter wrote:

> On 22/06/15 16:14, Thomas Gleixner wrote:
> > On Mon, 22 Jun 2015, Adrian Hunter wrote:
> >> On 03/06/15 19:23, Thomas Gleixner wrote:
> >>> On Wed, 3 Jun 2015, Adrian Hunter wrote:
> >>>
> >>>> On 03/06/15 06:30, Andi Kleen wrote:
> >>>>>> Then the changelog should say that I think.  The current text says
> >>>>>> "Fast TSC calibration will always fail", which, to me, suggests that
> >>>>>> either the slow calibration will work or that the changelog message
> >>>>>> should be changed.
> >>>>>
> >>>>> Ok. No, the slow calibration works I believe.
> >>>>
> >>>> Yeah, so the (only?) downside is the 50ms wasted on Fast TSC
> >>>> calibration.  What about this?
> >>>
> >>> I'm certainly happy to apply this one. 
> >>
> >> George Spelvin began investigating improving quick_pit_calibrate() but Ingo
> >> anyway suggested this patch as the first, so can this be applied?
> > 
> > Oh, yes. I simply forgot to pick it up. Thanks for the reminder.
> 
> Sorry to bother you, but I don't still don't see the patch anywhere?

That patch seems to be highly merge resistant. It's queued now.

Thanks,

	tglx

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[George Spelvin]

Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 03/06/15 19:23, Thomas Gleixner wrote:
>> I'm certainly happy to apply this one. 
> 
> George Spelvin began investigating improving quick_pit_calibrate() but Ingo
> anyway suggested this patch as the first, so can this be applied?

Acked-by: George Spelvin <linux@horizon.com>

It's currently the first patch in the series I'm developing, but
if someone wants to beat me to it, no objection.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

[tip:x86/urgent] x86/tsc: Let high latency PIT fail fast in quick_pit_calibrate()

[tip-bot for Adrian Hunter]

Commit-ID:  5aac644a9944bea93b4f05ced1883a902a2535f6
Gitweb:     http://git.kernel.org/tip/5aac644a9944bea93b4f05ced1883a902a2535f6
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Wed, 3 Jun 2015 10:39:46 +0300
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 6 Jul 2015 09:41:00 +0200

x86/tsc: Let high latency PIT fail fast in quick_pit_calibrate()

If it takes longer than 12us to read the PIT counter lsb/msb,
then the error margin will never fall below 500ppm within 50ms,
and Fast TSC calibration will always fail.

This patch detects when that will happen and fails fast. Note
the failure message is not printed in that case because:
1. it will always happen on that class of hardware
2. the absence of the message is more informative than its
presence

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Len Brown <lenb@kernel.org>
Cc: Andi Kleen <ak@linux.intel.com>
Link: http://lkml.kernel.org/r/556EB717.9070607@intel.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/tsc.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 5054497..7437b41 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -598,10 +598,19 @@ static unsigned long quick_pit_calibrate(void)
 			if (!pit_expect_msb(0xff-i, &delta, &d2))
 				break;
 
+			delta -= tsc;
+
+			/*
+			 * Extrapolate the error and fail fast if the error will
+			 * never be below 500 ppm.
+			 */
+			if (i == 1 &&
+			    d1 + d2 >= (delta * MAX_QUICK_PIT_ITERATIONS) >> 11)
+				return 0;
+
 			/*
 			 * Iterate until the error is less than 500 ppm
 			 */
-			delta -= tsc;
 			if (d1+d2 >= delta >> 11)
 				continue;
 

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Linus Torvalds]

On Tue, Jun 2, 2015 at 12:33 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Is there really no smarter way to figure out the TSC frequency on
> modern systems?

Sadly, if there is, we have yet to find it.

I don't think the mentioned intel_pstate thing gets it right either.
Yes, it gets some "theoretical frequency", given the pstate and the
scaling factor, but that assumes the base clock (BCLK) is something
trustworthy. That's a big assumption, and I can pretty much guarantee
it's not a valid assumption.

I'm sure that's the *common* case, but how much do you want to bet
that some motherboard makers noticed that they get good press from
good benchmark numbers, and that they can tweak BCLK a bit higher than
the competition? "Oooh, motherboard from Xyz is really good, it's 1%
faster than the competition".

Or all those overclockers? Yes, some of them buy unlocked CPU's and
leave BCLK alone. Or maybe they do both. Or maybe they just buy locked
CPU's and tweak BCLK.

The only *well-defined* clock in a modern PC seems to still remain the
PIT. Yes, it's very sad.  But all the other clocks tend to be
untrustworthy for various reasons, like "frequency is stated in the
ACPI tables", which we know is basically just a fantasy that may be
right *most* of the time, but I can almost guarantee that some BIOS
guys decided that they can get slightly better random benchmark
numbers by "tweaking" the truth a bit. It's the "BIOS version" of
overclocking by increasing BCLK - lie about the PM frequency, so that
any benchmark that times things that way will give better numbers..

But pretty much nobody messes up the PIT. That will change, just
because it's clearly getting to the point where people are just
emulating it, and it's probably not going to be any more trustworthy
than anything else.

So rather than get a new and truly trustworthy reference clock, the
patch makes me suspect we're losing the old one...

              Linus

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Tue, Jun 2, 2015 at 12:33 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> > Is there really no smarter way to figure out the TSC frequency on
> > modern systems?
> 
> Sadly, if there is, we have yet to find it.
> 
> I don't think the mentioned intel_pstate thing gets it right either. Yes, it 
> gets some "theoretical frequency", given the pstate and the scaling factor, but 
> that assumes the base clock (BCLK) is something trustworthy. That's a big 
> assumption, and I can pretty much guarantee it's not a valid assumption.
> 
> I'm sure that's the *common* case, but how much do you want to bet that some 
> motherboard makers noticed that they get good press from good benchmark numbers, 
> and that they can tweak BCLK a bit higher than the competition? "Oooh, 
> motherboard from Xyz is really good, it's 1% faster than the competition".
> 
> Or all those overclockers? Yes, some of them buy unlocked CPU's and leave BCLK 
> alone. Or maybe they do both. Or maybe they just buy locked CPU's and tweak 
> BCLK.
>
> The only *well-defined* clock in a modern PC seems to still remain the PIT. 
> [...]

and the HPET, which is pretty good as well, when available. In fact given that 
it's required to have a frequency of at least 10 MHz, and (unlike the PIT) has a 
pretty wide counter, it could be used for pretty accurate calibration as well that 
runs a lot shorter than PIT calibration.

HPETs have some quirks, and are sometimes emulated (on early hardware, when Window 
did not require HPET and the value of HPET was yet unclear?), but I'd say on 
modern x86 systems it ought to be a pretty good clock for calibration as well.

> [...] Yes, it's very sad.  But all the other clocks tend to be untrustworthy for 
> various reasons, like "frequency is stated in the ACPI tables", which we know is 
> basically just a fantasy that may be right *most* of the time, but I can almost 
> guarantee that some BIOS guys decided that they can get slightly better random 
> benchmark numbers by "tweaking" the truth a bit. It's the "BIOS version" of 
> overclocking by increasing BCLK - lie about the PM frequency, so that any 
> benchmark that times things that way will give better numbers..

So the HPET frequency comes straight from a hardware register, via:

        hpet_period = hpet_readl(HPET_PERIOD);

now you are right that the 'hardware' is in some cases is implemented via SMM 
virtualization - but so is the PIT I suspect. Given that Windows relies on the 
HPET for timekeeping, it might get more attention than the PIT?

> But pretty much nobody messes up the PIT. That will change, just because it's 
> clearly getting to the point where people are just emulating it, and it's 
> probably not going to be any more trustworthy than anything else.

So I think the reason why these tend to be real hardware most of the time is that 
both the PIT and the HPET are supposed to generate interrupts - which is not that 
easy to 'emulate' via SMM: you have to have a real irq-generating clock in the 
hardware _somewhere_, and that comes with counter and all, so not much left to 
emulate after that point.

What I think might happen is to emulate the PIT via HPET, the lower frequency 
clock with a higher frequency clock.

Thanks,

	Ingo

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Linus Torvalds]

On Tue, Jun 2, 2015 at 11:20 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> and the HPET, which is pretty good as well, when available. In fact given that
> it's required to have a frequency of at least 10 MHz, and (unlike the PIT) has a
> pretty wide counter, it could be used for pretty accurate calibration as well that
> runs a lot shorter than PIT calibration.

Yeah, you're probably right that we should look into at least having
the option to use the HPET.

That said, the fact that we can read the period from HPET_PERIOD does
*not* make me trust it all that much. I suspect that register is just
another "filled in by firmware" piece of data.

> Given that Windows relies on the
> HPET for timekeeping, it might get more attention than the PIT?

Does Windows actually do that? Becuase if so, that's just about the
strongest argument for HPET there is - it's likely to work, and the
frequency is likely to be correct.

We've had issues with HPET, but for calibration it might very well be
the right thing to do.

Does anybody know what the base oscillator for HPET tends to be? Also,
some googling shows a vmware paper that is not that impressed with the
HPET.

The good thing about the PIT is that it's just *specified* to be
driven off a real crystal running at a very fixed frequency. There's
no gray areas there. Sure, virtualization can screw it up (but will
likely screw up other higher-resolution clocks even more), and hw
designers can cause problems, but it's been pretty reliable.

(Yeah, the CMOS RTC clock should be too, as George Spelvin points out.
That might be worth looking at too).

                           Linus

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Thomas Gleixner]

On Wed, 3 Jun 2015, Linus Torvalds wrote:
> On Tue, Jun 2, 2015 at 11:20 PM, Ingo Molnar <mingo@kernel.org> wrote:
> > Given that Windows relies on the
> > HPET for timekeeping, it might get more attention than the PIT?
> 
> Does Windows actually do that? Becuase if so, that's just about the
> strongest argument for HPET there is - it's likely to work, and the
> frequency is likely to be correct.

At least it used to. Not sure if it still does. 
 
> We've had issues with HPET, but for calibration it might very well be
> the right thing to do.

Right. The issues we had were on the clock events side caused by the
match register delayed writes. I've never seen a bug report about the
frequency being completely wrong, except for crap values which we
filter out. Though, HPET period can be off by more than 500ppm, but I
don't think that matters anymore for timekeeping as we switch to TSC
only after the long term calibration. The quick calibration is just
for getting the TSC frequency roughly correct for udelay.

> Does anybody know what the base oscillator for HPET tends to be? Also,

The most common frequency is 14.318180 MHz.

> some googling shows a vmware paper that is not that impressed with the
> HPET.

Yeah, they complain about period being off by 600ppm and therefor
being useless for timekeeping, but that's not what we want to use it
for.
 
> The good thing about the PIT is that it's just *specified* to be
> driven off a real crystal running at a very fixed frequency. There's
> no gray areas there. Sure, virtualization can screw it up (but will
> likely screw up other higher-resolution clocks even more), and hw
> designers can cause problems, but it's been pretty reliable.

The other known frequency clock is pmtimer. We use hpet and pmtimer in
the slow calibration fallback already.
 
> (Yeah, the CMOS RTC clock should be too, as George Spelvin points out.
> That might be worth looking at too).

Indeed.

Thanks,

	tglx

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Linus Torvalds]

On Wed, Jun 3, 2015 at 9:47 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>
>> Does anybody know what the base oscillator for HPET tends to be? Also,
>
> The most common frequency is 14.318180 MHz.

I was more thinking along the lines of "which actually crystal is
driving it" than the frequency.

That said, that particular frequency does give a clue. That's exactly
12x the PIT frequency. So they presumably share the same basic
crystal, just different PLL's.

Which is a good indication that we should get very similar clock
stability with the HPET as with the PIC.

We might even use that kind of knowledge to decide "ok, let's use the
HPET as the reference clock". IOW, only use the HPET if we find it
listed, _and_ we get the expected frequency from the HPET_PERIOD
register.

Anybody who gives odd HPET_PERIOD values is likely doing somehing
different and possibly questionable.

            Linus

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[H. Peter Anvin]

On 06/03/2015 10:04 AM, Linus Torvalds wrote:
> On Wed, Jun 3, 2015 at 9:47 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>>> Does anybody know what the base oscillator for HPET tends to be? Also,
>>
>> The most common frequency is 14.318180 MHz.
> 
> I was more thinking along the lines of "which actually crystal is
> driving it" than the frequency.
> 
> That said, that particular frequency does give a clue. That's exactly
> 12x the PIT frequency. So they presumably share the same basic
> crystal, just different PLL's.

Here are the gory details as far as I know them:

This is the default time keeping crystal -- it is 4x the NTSC color
burst frequency, which has been used since the original PC.  It is
actually supposed to be 157.5/11 MHz = 14.31818... MHz ±40 Hz, there is
no zero even though it is often seen in docs.

However, the HPET isn't guaranteed to run at this frequency.  The HPET
frequency is advertised via a control register in HPET space (at offset
0x04, 32-bit COUNTER_CLK_PERIOD in fs).  This register is documented as
readonly

The RTC is probably the most reliable reference clock, in part because
32 kHz crystals are generally calibrated and extremely stable.  However,
to get more than 1 Hz frequency out of it you have to enable interrupts
(which gets you to 8192 Hz).  I have heard claims that this is actually
what Windows uses for calibration, but I don't know.

The ACPI PM_TMR is also fed from the 14.31818 MHz clock () with a
divisor of 4 (putting it exactly at the NTSC color burst frequency of
3.579545... MHz ± 10 Hz).  The PM_TMR, if present, is required to run at
this frequency -- there are no provisions in ACPI for specifying the
frequency.

> Which is a good indication that we should get very similar clock
> stability with the HPET as with the PIC.
> 
> We might even use that kind of knowledge to decide "ok, let's use the
> HPET as the reference clock". IOW, only use the HPET if we find it
> listed, _and_ we get the expected frequency from the HPET_PERIOD
> register.
> 
> Anybody who gives odd HPET_PERIOD values is likely doing somehing
> different and possibly questionable.

It would definitely be fishy if PM_TMR is also supported, since PM_TMR
has to run at a fixed frequency.

The HPET frequency register would be 0x429b176 if run off the 14 MHz clock.

However, if someone does something different the PIC/PM_TMR frequency
might very well be synthesized (or worse, approximated!) and the HPET
might be run off the the real BCLK.  This is a legitimate design.
PIC/PM_TMR being synthesized is fine (might introduce some jitter, but
shouldn't affect calibration), approximated is not...

	-hpa

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Ingo Molnar]

* H. Peter Anvin <hpa@zytor.com> wrote:

> [...]
> 
> The RTC is probably the most reliable reference clock, in part because 32 kHz 
> crystals are generally calibrated and extremely stable.  However, to get more 
> than 1 Hz frequency out of it you have to enable interrupts (which gets you to 
> 8192 Hz).  I have heard claims that this is actually what Windows uses for 
> calibration, but I don't know.

So I ran a few tests to determine how reliable the RTC clock is at 8192 Hz, and 
running it during early bootup gives this type of jitter:

[    0.000000] tsc: RTC IRQ     0, at  30566853736, delta:           30, jitter:           30
[    0.000000] tsc: RTC IRQ     1, at  30567333663, delta:       479927, jitter:       479897
[    0.000000] tsc: RTC IRQ     2, at  30567578651, delta:       244988, jitter:      -234939
[    0.000000] tsc: RTC IRQ     3, at  30567825147, delta:       246496, jitter:         1508
[    0.000000] tsc: RTC IRQ     4, at  30568070180, delta:       245033, jitter:        -1463
[    0.000000] tsc: RTC IRQ     5, at  30568315220, delta:       245040, jitter:            7
[    0.000000] tsc: RTC IRQ     6, at  30568561747, delta:       246527, jitter:         1487
[    0.000000] tsc: RTC IRQ     7, at  30568806794, delta:       245047, jitter:        -1480
[    0.000000] tsc: RTC IRQ     8, at  30569051789, delta:       244995, jitter:          -52
[    0.000000] tsc: RTC IRQ     9, at  30569296824, delta:       245035, jitter:           40
[    0.000000] tsc: RTC IRQ    10, at  30569543304, delta:       246480, jitter:         1445
[    0.000000] tsc: RTC IRQ    11, at  30569788346, delta:       245042, jitter:        -1438
[    0.000000] tsc: RTC IRQ    12, at  30570033393, delta:       245047, jitter:            5
[    0.000000] tsc: RTC IRQ    13, at  30570278433, delta:       245040, jitter:           -7
[    0.000000] tsc: RTC IRQ    14, at  30570524894, delta:       246461, jitter:         1421
[    0.000000] tsc: RTC IRQ    15, at  30570769950, delta:       245056, jitter:        -1405
[    0.000000] tsc: RTC IRQ    16, at  30571014993, delta:       245043, jitter:          -13
[    0.000000] tsc: RTC IRQ    17, at  30571260024, delta:       245031, jitter:          -12
[    0.000000] tsc: RTC IRQ    18, at  30571506504, delta:       246480, jitter:         1449
[    0.000000] tsc: RTC IRQ    19, at  30571751560, delta:       245056, jitter:        -1424
[    0.000000] tsc: RTC IRQ    20, at  30571996577, delta:       245017, jitter:          -39
[    0.000000] tsc: RTC IRQ    21, at  30572243072, delta:       246495, jitter:         1478
[    0.000000] tsc: RTC IRQ    22, at  30572488121, delta:       245049, jitter:        -1446
[    0.000000] tsc: RTC IRQ    23, at  30572733125, delta:       245004, jitter:          -45
[    0.000000] tsc: RTC IRQ    24, at  30572978181, delta:       245056, jitter:           52
[    0.000000] tsc: RTC IRQ    25, at  30573224673, delta:       246492, jitter:         1436
[    0.000000] tsc: RTC IRQ    26, at  30573469712, delta:       245039, jitter:        -1453
[    0.000000] tsc: RTC IRQ    27, at  30573714760, delta:       245048, jitter:            9
[    0.000000] tsc: RTC IRQ    28, at  30573959800, delta:       245040, jitter:           -8
[    0.000000] tsc: RTC IRQ    29, at  30574206272, delta:       246472, jitter:         1432
[    0.000000] tsc: RTC IRQ    30, at  30574451320, delta:       245048, jitter:        -1424
[    0.000000] tsc: RTC IRQ    31, at  30574696335, delta:       245015, jitter:          -33
[    0.000000] tsc: RTC IRQ    32, at  30574941390, delta:       245055, jitter:           40
[    0.000000] tsc: RTC IRQ    33, at  30575187872, delta:       246482, jitter:         1427
[    0.000000] tsc: RTC IRQ    34, at  30575432905, delta:       245033, jitter:        -1449
[    0.000000] tsc: RTC IRQ    35, at  30575677944, delta:       245039, jitter:            6
[    0.000000] tsc: RTC IRQ    36, at  30575924434, delta:       246490, jitter:         1451
[    0.000000] tsc: RTC IRQ    37, at  30576169465, delta:       245031, jitter:        -1459
[    0.000000] tsc: RTC IRQ    38, at  30576414504, delta:       245039, jitter:            8
[    0.000000] tsc: RTC IRQ    39, at  30576659542, delta:       245038, jitter:           -1
[    0.000000] tsc: RTC IRQ    40, at  30576906030, delta:       246488, jitter:         1450
[    0.000000] tsc: RTC IRQ    41, at  30577151072, delta:       245042, jitter:        -1446
[    0.000000] tsc: RTC IRQ    42, at  30577396105, delta:       245033, jitter:           -9
[    0.000000] tsc: RTC IRQ    43, at  30577641144, delta:       245039, jitter:            6
[    0.000000] tsc: RTC IRQ    44, at  30577887632, delta:       246488, jitter:         1449
[    0.000000] tsc: RTC IRQ    45, at  30578132665, delta:       245033, jitter:        -1455
[    0.000000] tsc: RTC IRQ    46, at  30578377704, delta:       245039, jitter:            6
[    0.000000] tsc: RTC IRQ    47, at  30578622749, delta:       245045, jitter:            6
[    0.000000] tsc: RTC IRQ    48, at  30578869230, delta:       246481, jitter:         1436
[    0.000000] tsc: RTC IRQ    49, at  30579114272, delta:       245042, jitter:        -1439
[    0.000000] tsc: RTC IRQ    50, at  30581814501, delta:      2700229, jitter:      2455187
[    0.000000] tsc: RTC IRQ    51, at  30582059549, delta:       245048, jitter:     -2455181
[    0.000000] tsc: RTC IRQ    52, at  30582304584, delta:       245035, jitter:          -13
[    0.000000] tsc: RTC IRQ    53, at  30582549631, delta:       245047, jitter:           12
[    0.000000] tsc: RTC IRQ    54, at  30582796117, delta:       246486, jitter:         1439
[    0.000000] tsc: RTC IRQ    55, at  30583041136, delta:       245019, jitter:        -1467
[    0.000000] tsc: RTC IRQ    56, at  30583286184, delta:       245048, jitter:           29
[    0.000000] tsc: RTC IRQ    57, at  30583532682, delta:       246498, jitter:         1450
[    0.000000] tsc: RTC IRQ    58, at  30583777720, delta:       245038, jitter:        -1460
[    0.000000] tsc: RTC IRQ    59, at  30584022745, delta:       245025, jitter:          -13
[    0.000000] tsc: RTC IRQ    60, at  30584267784, delta:       245039, jitter:           14
[    0.000000] tsc: RTC IRQ    61, at  30584514273, delta:       246489, jitter:         1450
[    0.000000] tsc: RTC IRQ    62, at  30584759311, delta:       245038, jitter:        -1451
[    0.000000] tsc: RTC IRQ    63, at  30585004357, delta:       245046, jitter:            8
[    0.000000] tsc: RTC IRQ    64, at  30585249385, delta:       245028, jitter:          -18
[    0.000000] tsc: RTC IRQ    65, at  30585495880, delta:       246495, jitter:         1467
[    0.000000] tsc: RTC IRQ    66, at  30585740908, delta:       245028, jitter:        -1467
[    0.000000] tsc: RTC IRQ    67, at  30585985953, delta:       245045, jitter:           17
[    0.000000] tsc: RTC IRQ    68, at  30586232425, delta:       246472, jitter:         1427
[    0.000000] tsc: RTC IRQ    69, at  30586477464, delta:       245039, jitter:        -1433
[    0.000000] tsc: RTC IRQ    70, at  30586722493, delta:       245029, jitter:          -10
[    0.000000] tsc: RTC IRQ    71, at  30586967516, delta:       245023, jitter:           -6
[    0.000000] tsc: RTC IRQ    72, at  30587213989, delta:       246473, jitter:         1450
[    0.000000] tsc: RTC IRQ    73, at  30587459029, delta:       245040, jitter:        -1433
[    0.000000] tsc: RTC IRQ    74, at  30587704077, delta:       245048, jitter:            8
[    0.000000] tsc: RTC IRQ    75, at  30587949125, delta:       245048, jitter:            0
[    0.000000] tsc: RTC IRQ    76, at  30588195597, delta:       246472, jitter:         1424
[    0.000000] tsc: RTC IRQ    77, at  30588440629, delta:       245032, jitter:        -1440
[    0.000000] tsc: RTC IRQ    78, at  30588685660, delta:       245031, jitter:           -1
[    0.000000] tsc: RTC IRQ    79, at  30588930716, delta:       245056, jitter:           25
[    0.000000] tsc: RTC IRQ    80, at  30589177189, delta:       246473, jitter:         1417
[    0.000000] tsc: RTC IRQ    81, at  30589422229, delta:       245040, jitter:        -1433
[    0.000000] tsc: RTC IRQ    82, at  30589667285, delta:       245056, jitter:           16
[    0.000000] tsc: RTC IRQ    83, at  30589913760, delta:       246475, jitter:         1419
[    0.000000] tsc: RTC IRQ    84, at  30590158798, delta:       245038, jitter:        -1437
[    0.000000] tsc: RTC IRQ    85, at  30590403837, delta:       245039, jitter:            1
[    0.000000] tsc: RTC IRQ    86, at  30590648866, delta:       245029, jitter:          -10
[    0.000000] tsc: RTC IRQ    87, at  30590895346, delta:       246480, jitter:         1451
[    0.000000] tsc: RTC IRQ    88, at  30591140397, delta:       245051, jitter:        -1429
[    0.000000] tsc: RTC IRQ    89, at  30591385446, delta:       245049, jitter:           -2
[    0.000000] tsc: RTC IRQ    90, at  30591630477, delta:       245031, jitter:          -18
[    0.000000] tsc: RTC IRQ    91, at  30591876949, delta:       246472, jitter:         1441
[    0.000000] tsc: RTC IRQ    92, at  30592121998, delta:       245049, jitter:        -1423
[    0.000000] tsc: RTC IRQ    93, at  30592367029, delta:       245031, jitter:          -18
[    0.000000] tsc: RTC IRQ    94, at  30592612075, delta:       245046, jitter:           15
[    0.000000] tsc: RTC IRQ    95, at  30592858555, delta:       246480, jitter:         1434
[    0.000000] tsc: RTC IRQ    96, at  30593103597, delta:       245042, jitter:        -1438
[    0.000000] tsc: RTC IRQ    97, at  30593348630, delta:       245033, jitter:           -9
[    0.000000] tsc: RTC IRQ    98, at  30593595101, delta:       246471, jitter:         1438
[    0.000000] tsc: RTC IRQ    99, at  30593840157, delta:       245056, jitter:        -1415
[    0.000000] tsc: RTC IRQ   100, at  30594085197, delta:       245040, jitter:          -16

The time measurements are in cycles, the TSC on this box runs at 2.010 GHz.

What you can see is that the jitter is pretty stable, with some harmonics in it.

There's outliners that occur at about 80 Hz frequency:

[    0.000000] tsc: RTC IRQ    50, at  30581814501, delta:      2700229, jitter:      2455187
[    0.000000] tsc: RTC IRQ    51, at  30582059549, delta:       245048, jitter:     -2455181

That's too frequent to be the RTC update delay - but its length, 1.3 msecs, comes 
close to that delay - so maybe that is it. We could filter out these outliner 
during calibration.

With that I think we could get down to a jitter of around 10 ppm with just a few 
dozen samples - i.e. collect it all in just 2-3 milliseconds.

So that seems desirable and in would bring our calibration accuracy to the same 
level as our delayed-work based 'refined TSC' calibration method is.

The big practical problem I found is implementational. Boot dependencies are 
horrible:

 - one problem is that the IO-APIC is not enabled yet when we calibrate these 
   values. With vile hacks I was able to enable the RTC during early boot by 
   forcing an early init of the IO-APIC, but I found no robust method.

 - there's also a catch-22 with time init: IO-APIC init relies on time init. I 
   wasn't successful at untangling it.

 - I also tried to activate the RTC IRQ in the i8259 PIC during early boot, so 
   that we can just use it to sample the RTC and then forget about that state, but
   didn't succeed.

all in one, receiving RTC IRQs so early in the bootup was a pretty fragile affair.

 - Alternatively, I also tried a different method: to set up the RTC periodic IRQ 
   during early boot, but not have an IRQ handler, polling RTC_PF in the 
   rtc_cmos_read(RTC_INTR_FLAGS) IRQ status byte.

   Unfortunately when I do this then PIO based RTC accesses can take tens of
   thousands of cycles, and the resulting jitter is pretty bad and hard to filter:

[    0.000000] tsc: RTC edge 57 from  0 to 64, at  29694678517, delta:         246360, jitter:         2456, loops:            7,        35194 cycles/loop
[    0.000000] tsc: RTC edge 58 from  0 to 64, at  29695169485, delta:         490968, jitter:       244608, loops:          118,         4160 cycles/loop
[    0.000000] tsc: RTC edge 59 from  0 to 64, at  29695413981, delta:         244496, jitter:      -246472, loops:            6,        40749 cycles/loop
[    0.000000] tsc: RTC edge 60 from  0 to 64, at  29695660661, delta:         246680, jitter:         2184, loops:            7,        35240 cycles/loop
[    0.000000] tsc: RTC edge 61 from  0 to 64, at  29695904853, delta:         244192, jitter:        -2488, loops:            6,        40698 cycles/loop
[    0.000000] tsc: RTC edge 62 from  0 to 64, at  29696151141, delta:         246288, jitter:         2096, loops:            7,        35184 cycles/loop
[    0.000000] tsc: RTC edge 63 from  0 to 64, at  29696396445, delta:         245304, jitter:         -984, loops:            6,        40884 cycles/loop
[    0.000000] tsc: RTC edge 64 from  0 to 64, at  29696642669, delta:         246224, jitter:          920, loops:            7,        35174 cycles/loop
[    0.000000] tsc: RTC edge 65 from  0 to 64, at  29696887245, delta:         244576, jitter:        -1648, loops:            6,        40762 cycles/loop
[    0.000000] tsc: RTC edge 66 from  0 to 64, at  29697377909, delta:         490664, jitter:       246088, loops:          117,         4193 cycles/loop
[    0.000000] tsc: RTC edge 67 from  0 to 64, at  29697622701, delta:         244792, jitter:      -245872, loops:            6,        40798 cycles/loop
[    0.000000] tsc: RTC edge 68 from  0 to 64, at  29697868773, delta:         246072, jitter:         1280, loops:            7,        35153 cycles/loop
[    0.000000] tsc: RTC edge 69 from  0 to 64, at  29700569301, delta:        2700528, jitter:      2454456, loops:           13,       207732 cycles/loop
[    0.000000] tsc: RTC edge 70 from  0 to 64, at  29700813805, delta:         244504, jitter:     -2456024, loops:            6,        40750 cycles/loop
[    0.000000] tsc: RTC edge 71 from  0 to 64, at  29701060125, delta:         246320, jitter:         1816, loops:            7,        35188 cycles/loop
[    0.000000] tsc: RTC edge 72 from  0 to 64, at  29701550189, delta:         490064, jitter:       243744, loops:          117,         4188 cycles/loop
[    0.000000] tsc: RTC edge 73 from  0 to 64, at  29701796677, delta:         246488, jitter:      -243576, loops:            7,        35212 cycles/loop
[    0.000000] tsc: RTC edge 74 from  0 to 64, at  29702040829, delta:         244152, jitter:        -2336, loops:            6,        40692 cycles/loop
[    0.000000] tsc: RTC edge 75 from  0 to 64, at  29702287597, delta:         246768, jitter:         2616, loops:            7,        35252 cycles/loop
[    0.000000] tsc: RTC edge 76 from  0 to 64, at  29702531741, delta:         244144, jitter:        -2624, loops:            6,        40690 cycles/loop
[    0.000000] tsc: RTC edge 77 from  0 to 64, at  29702778341, delta:         246600, jitter:         2456, loops:            7,        35228 cycles/loop
[    0.000000] tsc: RTC edge 78 from  0 to 64, at  29703022661, delta:         244320, jitter:        -2280, loops:            6,        40720 cycles/loop
[    0.000000] tsc: RTC edge 79 from  0 to 64, at  29703514245, delta:         491584, jitter:       247264, loops:          118,         4165 cycles/loop
[    0.000000] tsc: RTC edge 80 from  0 to 64, at  29703759165, delta:         244920, jitter:      -246664, loops:            6,        40820 cycles/loop
[    0.000000] tsc: RTC edge 81 from  0 to 64, at  29704005397, delta:         246232, jitter:         1312, loops:            7,        35176 cycles/loop
[    0.000000] tsc: RTC edge 82 from  0 to 64, at  29704249589, delta:         244192, jitter:        -2040, loops:            6,        40698 cycles/loop

   and this was on a pretty standard whitebox PC, I'm pessimistic about how this 
   would work on more exotic machines.

So I think we should stay with the PIT fast-calibration method, it's very fast in 
most cases.

Thanks,

	Ingo

Re: [PATCH RFC] x86, tsc: Allow for high latency in quick_pit_calibrate()

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> On Wed, 3 Jun 2015, Linus Torvalds wrote:
> > On Tue, Jun 2, 2015 at 11:20 PM, Ingo Molnar <mingo@kernel.org> wrote:
> > >
> > > Given that Windows relies on the HPET for timekeeping, it might get more 
> > > attention than the PIT?
> > 
> > Does Windows actually do that? Becuase if so, that's just about the strongest 
> > argument for HPET there is - it's likely to work, and the frequency is likely 
> > to be correct.
> 
> At least it used to. Not sure if it still does.

So I googled around a bit, and it's not as clear-cut as I thought initially: it 
appears that if the BIOS enables the HPET then modern Windows (Windows 7 and 
later) will use it, but there are problems and Windows XP (the largest installed 
base) used the TSC+acpipm. (two other lovely clocks)

> > We've had issues with HPET, but for calibration it might very well be the 
> > right thing to do.
> 
> Right. The issues we had were on the clock events side caused by the match 
> register delayed writes. I've never seen a bug report about the frequency being 
> completely wrong, except for crap values which we filter out. Though, HPET 
> period can be off by more than 500ppm, but I don't think that matters anymore 
> for timekeeping as we switch to TSC only after the long term calibration. The 
> quick calibration is just for getting the TSC frequency roughly correct for 
> udelay.

Yes - so the frequency being off due to production variances (or sheer firmware 
incompetence) isn't a big deal in itself: having boot-to-boot jitter during fast 
calibration would be, and that's the big question.

The nice thing about the PIT calibration is that it usually provides very little 
jitter, so that driver delays/etc. are as boot-to-boot identical as possible.

Anyway, the HPET might be worth an attempt - but I'd not be surprised if we 
discovered another rotten apple ...

Thanks,

	Ingo