Re: [PATCH] x86 (64): make calibrate_APIC_clock() SMI-safe (take3)

[Martin Wilck]

Cyrill Gorcunov wrote:
> Martin, it seems you forgot to attach the patch :) 

I am sorry, here it is. Thanks :)

Martin

--
Martin Wilck
PRIMERGY System Software Engineer
FSC IP ESP DEV 6

Fujitsu Siemens Computers GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn
Germany

Tel:			++49 5251 8 15113
Fax:			++49 5251 8 20209
Email:			mailto:martin.wilck@xxxxxxxxxxxxxxxxxxx
Internet:		http://www.fujitsu-siemens.com
Company Details:	http://www.fujitsu-siemens.com/imprint.html
[PATCH] x86 (64): make calibrate_APIC_clock() SMI-safe (take 3)

Non-maskable asynchronous events (e.g. SMIs) which occur during the APIC 
timer calibration can cause timer miscalibrations, sometimes by large amounts. 
This patch fixes this by making sure that no significant interruption occurs 
between APIC and TSC reads. SMIs may still occur at some stage in the 
calibration loop, causing the loop to last longer than intended. This 
doesn't matter though, as long as the start and end values are both 
taken simultaneously.

Changed wrt take 2: Use max. possible start value for the APIC timer 
to avoid underflow.

Signed-off-by: Martin Wilck <martin.wilck@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Gerhard Wichert <gerhard.wichert@xxxxxxxxxxxxxxxxxxx>

--- arch/x86/kernel/apic_64.c	2008-07-25 15:39:51.000000000 +0200
+++ arch/x86/kernel/apic_64.c.new	2008-07-25 15:55:08.000000000 +0200
@@ -300,6 +300,31 @@ static void setup_APIC_timer(void)
 }
 
 /*
+ * Helper function for calibrate_APIC_clock(): Make sure that
+ * APIC TMCTT and TSC are read at the same time, to reasonable
+ * accuracy. On any sane system, the retry loop won't need more
+ * than a single retry, given that the rdtsc/apic_read/rdtsc
+ * sequence won't take more than a few cycles.
+ */
+
+#define MAX_DIFFERENCE 1000UL
+#define MAX_ITER 10
+static inline int
+__read_tsc_and_apic(unsigned long *tsc, unsigned *apic)
+{
+	unsigned long tsc0, tsc1, diff;
+	int i = 0;
+	do {
+		rdtscll(tsc0);
+		*apic = apic_read(APIC_TMCCT);
+		rdtscll(tsc1);
+		diff = tsc1 - tsc0;
+	} while (diff > MAX_DIFFERENCE && ++i < MAX_ITER); 
+	*tsc = tsc0 + (diff >> 1);
+	return diff > MAX_DIFFERENCE ? -EIO : 0;
+}
+
+/*
  * In this function we calibrate APIC bus clocks to the external
  * timer. Unfortunately we cannot use jiffies and the timer irq
  * to calibrate, since some later bootup code depends on getting
@@ -318,7 +343,7 @@ static void __init calibrate_APIC_clock(
 {
 	unsigned apic, apic_start;
 	unsigned long tsc, tsc_start;
-	int result;
+	int result, err_start, err;
 
 	local_irq_disable();
 
@@ -329,25 +354,27 @@ static void __init calibrate_APIC_clock(
 	 *
 	 * No interrupt enable !
 	 */
-	__setup_APIC_LVTT(250000000, 0, 0);
+	__setup_APIC_LVTT(0xffffffff, 0, 0);
 
-	apic_start = apic_read(APIC_TMCCT);
 #ifdef CONFIG_X86_PM_TIMER
 	if (apic_calibrate_pmtmr && pmtmr_ioport) {
+		apic_start = apic_read(APIC_TMCCT);
 		pmtimer_wait(5000);  /* 5ms wait */
 		apic = apic_read(APIC_TMCCT);
 		result = (apic_start - apic) * 1000L / 5;
 	} else
 #endif
 	{
-		rdtscll(tsc_start);
+		err_start = __read_tsc_and_apic(&tsc_start, &apic_start);
 
 		do {
-			apic = apic_read(APIC_TMCCT);
-			rdtscll(tsc);
+			err = __read_tsc_and_apic(&tsc, &apic);
 		} while ((tsc - tsc_start) < TICK_COUNT &&
 				(apic_start - apic) < TICK_COUNT);
 
+		if (err_start || err)
+			printk(KERN_CRIT "calibrate_APIC_clock: SMI flood - "
+				"the APIC timer calibration may be wrong!\n");
 		result = (apic_start - apic) * 1000L * tsc_khz /
 					(tsc - tsc_start);
 	}