Messages in this thread Patch in this message | | | Date | Thu, 24 Jul 2008 15:55:02 +0200 | From | Martin Wilck <> | Subject | Re: [PATCH] x86 (64): make calibrate_APIC_clock() SMI-safe |
| |
Cyrill Gorcunov wrote:
> yes, it will issue some effects but it's better then stuck there. > More over in 'case of SMI flood with current patch you don't get > error message printed i think so you better add max iteration > counter so user will see on console (or whatever) that he is got > problems. > - Cyrill -
I disagree. If you have a system that generates SMIs in this extreme frequency, you're better off stuck than running on such an unstable system. The user _will_ see messages on the console if this happens. Note that apparently there are few people who have trouble with this. We did see problems, but never had more than 1 SMI disturbing the calibration procedure.
Anyway, here is another patch that defines max iteration counts. I haven't added a "Signed-off:" line, because I prefer the original version.
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@fujitsu-siemens.com Internet: http://www.fujitsu-siemens.com Company Details: http://www.fujitsu-siemens.com/imprint.html --- arch/x86/kernel/apic_64.c 2008-07-24 15:25:30.000000000 +0200 +++ arch/x86/kernel/apic_64.c.new 2008-07-24 15:26:13.000000000 +0200 @@ -314,11 +314,35 @@ static void setup_APIC_timer(void) #define TICK_COUNT 100000000 +#define MAX_DIFFERENCE 1000UL +#define MAX_ITER 10 +#define MAX_CALIBRATIONS 10 +static inline int +__read_tsc_and_apic(unsigned long *tsc, unsigned *apic) +{ + unsigned long tsc0, tsc1, diff; + int i; + for (i = 0; i < MAX_ITER; i++) { + rdtscll(tsc0); + *apic = apic_read(APIC_TMCCT); + rdtscll(tsc1); + diff = tsc1 - tsc0; + if (diff <= MAX_DIFFERENCE) + break; + } + *tsc = tsc0 + (diff >> 1); + if (i == MAX_ITER) { + printk(KERN_ERR "unable to read TSC abd APIC simultaneously\n"); + return -EIO; + } + return 0; +} + static void __init calibrate_APIC_clock(void) { unsigned apic, apic_start; unsigned long tsc, tsc_start; - int result; + int result, cnt = 0, err_start, err; local_irq_disable(); @@ -329,25 +353,41 @@ static void __init calibrate_APIC_clock( * * No interrupt enable ! */ +smi_occured: __setup_APIC_LVTT(250000000, 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 this takes significantly longer than TICK_COUNT, + * some interruption must have occured - retry. + */ + if (err_start || err || + (tsc - tsc_start) > (TICK_COUNT + TICK_COUNT/1000) || + (apic_start - apic) > (TICK_COUNT + TICK_COUNT/1000)) { + printk(KERN_ERR + "calibrate_APIC_clock: SMI occured? %lx %x\n", + tsc - tsc_start, apic_start - apic); + if (++cnt < MAX_CALIBRATIONS) + goto smi_occured; + else + 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); } | |