Re: Linux 2.6.29-rc6

[Linus Torvalds <torvalds@linux-foundation.org>]

Jesper, here's a patch that actually tries to take teh TSC error really 
into account, and which I suspect will result (on your machine) in failing 
the fast PIT calibration. 

It also has a few extra printk's for debugging, and to see just what the 
values are on your machine.

The idea behind the patch is to just keep track of how big the difference 
was in TSC values between two successive reads of the PIT timer. We only 
really care about the difference when the MSB turns around, and we only 
really care about the two end points. The maximum error in TSC estimation 
will simply be the sum of the differences at those points (d1 and d2).

We can then compare the maximum error with the actual TSC differences 
between those points, and see if the max error is within 500 ppm. That 
_should_ mean that it all works - assuming that the PIT itself is running 
at the correct frequency, of course!

Regardless of whether is succeeds or not, it will print out some debug 
messages, which will be interesting to see.

What's nice about this is that it really should make that whole "yes, it's 
really within 500ppm" assertion have some solid legs to stand on. Rather 
than depend on us being able to read the PIT a certain number of times, we 
can literally give an estimation of the max error.

		Linus

---
 arch/x86/kernel/tsc.c |   41 +++++++++++++++++++++++++++++------------
 1 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 599e581..8e1db42 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -273,17 +273,26 @@ static unsigned long pit_calibrate_tsc(u32 latch, unsigned long ms, int loopmin)
  * use the TSC value at the transitions to calculate a pretty
  * good value for the TSC frequencty.
  */
-static inline int pit_expect_msb(unsigned char val)
+static unsigned long pit_expect_msb(unsigned char val, u64 *tscp, unsigned long *deltap)
 {
-	int count = 0;
+	int count;
+	u64 tsc = 0;
 
 	for (count = 0; count < 50000; count++) {
 		/* Ignore LSB */
 		inb(0x42);
 		if (inb(0x42) != val)
 			break;
+		tsc = get_cycles();
 	}
-	return count > 50;
+	*deltap = get_cycles() - tsc;
+	*tscp = tsc;
+
+	/*
+	 * We require _some_ success, but the quality control
+	 * will be based on the error terms on the TSC values.
+	 */
+	return count > 5;
 }
 
 /*
@@ -297,6 +306,10 @@ static inline int pit_expect_msb(unsigned char val)
 
 static unsigned long quick_pit_calibrate(void)
 {
+	u64 t1, t2;
+	unsigned long d1, d2;
+	unsigned char expect = 0xff;
+
 	/* Set the Gate high, disable speaker */
 	outb((inb(0x61) & ~0x02) | 0x01, 0x61);
 
@@ -315,22 +328,24 @@ static unsigned long quick_pit_calibrate(void)
 	outb(0xff, 0x42);
 	outb(0xff, 0x42);
 
-	if (pit_expect_msb(0xff)) {
+	if (pit_expect_msb(0xff, &t1, &d1)) {
 		int i;
-		u64 t1, t2, delta;
-		unsigned char expect = 0xfe;
+		u64 delta;
 
-		t1 = get_cycles();
+		expect--;
 		for (i = 0; i < QUICK_PIT_ITERATIONS; i++, expect--) {
-			if (!pit_expect_msb(expect))
+			if (!pit_expect_msb(expect, &t2, &d2))
 				goto failed;
 		}
-		t2 = get_cycles();
 
 		/*
-		 * Make sure we can rely on the second TSC timestamp:
+		 * We require the max error on the calibration to be
+		 * within 500 ppm, since that's the limit of ntpd
+		 * drift correction. So the TSC delta must be more
+		 * than 2000x the possible error term (d1+d2).
 		 */
-		if (!pit_expect_msb(expect))
+		delta = t2 - t1;
+		if (d1+d2 > delta >> 11)
 			goto failed;
 
 		/*
@@ -347,12 +362,14 @@ static unsigned long quick_pit_calibrate(void)
 		 * kHz = (t2 - t1) / (QPI * 256 / PIT_TICK_RATE) / 1000
 		 * kHz = ((t2 - t1) * PIT_TICK_RATE) / (QPI * 256 * 1000)
 		 */
-		delta = (t2 - t1)*PIT_TICK_RATE;
+		printk("Fast TSC delta=%lld, error=%lu+%lu=%lu\n", delta, d1, d2, d1+d2);
+		delta *= PIT_TICK_RATE;
 		do_div(delta, QUICK_PIT_ITERATIONS*256*1000);
 		printk("Fast TSC calibration using PIT\n");
 		return delta;
 	}
 failed:
+	printk("Fast TSC calibration failed at %u %llu(%lu) %llu(%lu)\n", expect, t1, d1, t2, d2);
 	return 0;
 }