Skip to contentSkip to conversation

x86: tsc: make TSC calibration immune to interrupts

PATCH In review Kasper Pedersen 2 participants 4 messages May 24, 2011 – May 25, 2011 x86

lore
Share & tools
lore

Kasper Pedersen

author
#0 lore raw

PATCH v3 1/1 x86: tsc: make TSC calibration immune to interrupts

When a SMI or plain interrupt occurs during the delayed part of TSC calibration, and the SMI/irq handler is good and fast so that is does not exceed SMI_TRESHOLD, tsc_khz can be a bit off (10-30ppm).

We should keep plain interrupts out of the reading of tsc/reference, so disable interrupts there.

We should not depend on SMIs being longer than 50000 clocks, so, in the refined calibration, always do the 5 tries, and use the best sample we get.

This should work always for any four periodic or rate-limited SMI sources. If we get 5 SMIs with 500ns gaps in a row, behaviour should be as without this patch.

It is safe to use the first value that passes SMI_TRESHOLD for the initial calibration: As long as tsc_khz is above 100MHz, SMI_TRESHOLD represents less than 1% of error.

The 8 additional samples costs us 28 microseconds in startup time.

measurements:
On a 700MHz P3 I see t2-t1=~22000, and 31ppm error. A Core2 is similar: http://n1.taur.dk/tscdeviat.png
(while mostly t2-t1=~1000, in about 1 of 3000 tests I see t2-t1=~20000 for both machines.) vmware ESX4 has t2-t1=~8000 and up.

v2: save ~170usec startup time (Stultz)
v3: also disabling irqs. (Triplett)

Signed-off-by Kasper Pedersen <[email protected]>
---
 arch/x86/kernel/tsc.c |   39 +++++++++++++++++++++++++++------------
 1 files changed, 27 insertions(+), 12 deletions(-)
arch/x86/kernel/tsc.c +27 -12
index ffe5755..9983c03 100644
@@ -117,27 +117,42 @@ static int __init tsc_setup(char *str)
 
 __setup("tsc=", tsc_setup);
 
-#define MAX_RETRIES     5
+#define BESTOF_SAMPLES      5
 #define SMI_TRESHOLD    50000
 
 /*
  * Read TSC and the reference counters. Take care of SMI disturbance
  */
-static u64 tsc_read_refs(u64 *p, int hpet)
+static u64 tsc_read_refs(u64 *p, int hpet, int find_best)
 {
-	u64 t1, t2;
+	u64 t1, t2, tp, best_uncertainty, uncertainty, best_t2;
 	int i;
+	unsigned long flags;
 
-	for (i = 0; i < MAX_RETRIES; i++) {
+	best_uncertainty = SMI_TRESHOLD;
+	best_t2 = 0;
+	for (i = 0; i < BESTOF_SAMPLES; i++) {
+		local_irq_save(flags);
 		t1 = get_cycles();
 		if (hpet)
-			*p = hpet_readl(HPET_COUNTER) & 0xFFFFFFFF;
+			tp = hpet_readl(HPET_COUNTER) & 0xFFFFFFFF;
 		else
-			*p = acpi_pm_read_early();
+			tp = acpi_pm_read_early();
 		t2 = get_cycles();
-		if ((t2 - t1) < SMI_TRESHOLD)
-			return t2;
+		local_irq_restore(flags);
+		uncertainty = t2 - t1;
+		if (uncertainty < best_uncertainty) {
+			best_uncertainty = uncertainty;
+			best_t2 = t2;
+			*p = tp;
+			if (!find_best)
+				break;
+		}
 	}
+	if (best_uncertainty < SMI_TRESHOLD)
+		return best_t2;
+
+	*p = tp;
 	return ULLONG_MAX;
 }
 

@@ -455,9 +470,9 @@ unsigned long native_calibrate_tsc(void)
 		 * read the end value.
 		 */
 		local_irq_save(flags);
-		tsc1 = tsc_read_refs(&ref1, hpet);
+		tsc1 = tsc_read_refs(&ref1, hpet, 0);
 		tsc_pit_khz = pit_calibrate_tsc(latch, ms, loopmin);
-		tsc2 = tsc_read_refs(&ref2, hpet);
+		tsc2 = tsc_read_refs(&ref2, hpet, 0);
 		local_irq_restore(flags);
 
 		/* Pick the lowest PIT TSC calibration so far */

@@ -928,11 +943,11 @@ static void tsc_refine_calibration_work(struct work_struct *work)
 		 */
 		hpet = is_hpet_enabled();
 		schedule_delayed_work(&tsc_irqwork, HZ);
-		tsc_start = tsc_read_refs(&ref_start, hpet);
+		tsc_start = tsc_read_refs(&ref_start, hpet, 1);
 		return;
 	}
 
-	tsc_stop = tsc_read_refs(&ref_stop, hpet);
+	tsc_stop = tsc_read_refs(&ref_stop, hpet, 1);
 
 	/* hpet or pmtimer available ? */
 	if (ref_start == ref_stop)
#1 lore raw
Quoted 6 lines

It is safe to use the first value that passes SMI_TRESHOLD for the initial calibration: As long as tsc_khz is above 100MHz, SMI_TRESHOLD represents less than 1% of error.

The 8 additional samples costs us 28 microseconds in startup time.

That's a good reason to avoid the whole conditional thing and just do the best of 5 always.

Quoted 30 lines

/*
* Read TSC and the reference counters. Take care of SMI disturbance */

arch/x86/kernel/tsc.c +13 -7
-static u64 tsc_read_refs(u64 *p, int hpet)
+static u64 tsc_read_refs(u64 *p, int hpet, int find_best)
 {
-	u64 t1, t2;
+	u64 t1, t2, tp, best_uncertainty, uncertainty, best_t2;
 	int i;
+	unsigned long flags;
 
-	for (i = 0; i < MAX_RETRIES; i++) {
+	best_uncertainty = SMI_TRESHOLD;
+	best_t2 = 0;
+	for (i = 0; i < BESTOF_SAMPLES; i++) {
+		local_irq_save(flags);
 		t1 = get_cycles();
 		if (hpet)
-			*p = hpet_readl(HPET_COUNTER) & 0xFFFFFFFF;
+			tp = hpet_readl(HPET_COUNTER) & 0xFFFFFFFF;
 		else
-			*p = acpi_pm_read_early();
+			tp = acpi_pm_read_early();
 		t2 = get_cycles();
-		if ((t2 - t1) < SMI_TRESHOLD)
-			return t2;
+		local_irq_restore(flags);
+		uncertainty = t2 - t1;
+		if (uncertainty < best_uncertainty) {
+			best_uncertainty = uncertainty;

arch/x86/kernel/tsc.c static int __init tsc_setup(char *str) · line 155

Quoted 10 lines
arch/x86/kernel/tsc.c +9 -0
+			best_t2 = t2;
+			*p = tp;
+			if (!find_best)
+				break;
+		}
 	}
+	if (best_uncertainty < SMI_TRESHOLD)
+		return best_t2;
+
+	*p = tp;

The value is completely uninteresting when we return ULLONG_MAX.

Thanks,

tglx

author
#2 lore raw
Quoted 5 lines · Thomas Gleixner
Quoted 2 lines

The 8 additional samples costs us 28 microseconds in startup time.

That's a good reason to avoid the whole conditional thing and just do the best of 5 always.

I do not mind removing it again.
The additional 170us only happen when quick_pit_calibrate fails, and then native_calibrate_tsc burns 30ms extra anyway.

Quoted 5 lines

arch/x86/kernel/tsc.c static int __init tsc_setup(char *str) · line 155

Quoted 2 lines
arch/x86/kernel/tsc.c +2 -0
+
+	*p = tp;

The value is completely uninteresting when we return ULLONG_MAX.

native_calibrate_tsc() depends on *p being written regardless of whether we can get a SMI-free reading:

	/* We don't have an alternative source, disable TSC */
	if (!hpet && !ref1 && !ref2) {
		printk("TSC: No reference (HPET/PMTIMER) available\n");
		return 0;
	}

this works since acpi_pm_read_early() returns 0 when pmtmr_ioport is 0, or CONFIG_X86_PM_TIMER is not set.

Without it we would report "calibration failed" rather than "No reference" or "Using PIT" when there is neither pmtimer nor hpet.

/Kasper Pedersen

#3 lore raw
Quoted 10 lines · Kasper Pedersen
Quoted 5 lines · Thomas Gleixner
Quoted 2 lines

The 8 additional samples costs us 28 microseconds in startup time.

That's a good reason to avoid the whole conditional thing and just do the best of 5 always.

I do not mind removing it again.
The additional 170us only happen when quick_pit_calibrate fails, and then native_calibrate_tsc burns 30ms extra anyway.

Yes, not worth the extra conditional.

Quoted 15 lines
Quoted 5 lines

arch/x86/kernel/tsc.c static int __init tsc_setup(char *str) · line 155

Quoted 2 lines
arch/x86/kernel/tsc.c +2 -0
+
+	*p = tp;

The value is completely uninteresting when we return ULLONG_MAX.

native_calibrate_tsc() depends on *p being written regardless of whether we can get a SMI-free reading:

	/* We don't have an alternative source, disable TSC */
	if (!hpet && !ref1 && !ref2) {
		printk("TSC: No reference (HPET/PMTIMER) available\n");
		return 0;
	}
Quoted 6 lines

this works since acpi_pm_read_early() returns 0 when pmtmr_ioport is 0, or CONFIG_X86_PM_TIMER is not set.

Without it we would report "calibration failed" rather than "No reference" or "Using PIT" when there is neither pmtimer nor hpet.

Fair enough. So a comment might be in order :) That code is fricking complex just to figure out on which frequency that damn thing runs :(

Thanks,

tglx