Hi,
I spotted a problem with the calibration of the local APIC clock. For
the second CPU you sometimes see impossibly high clock rates like
in the following example:
> cpu0: Intel(R) Core(TM) i3-2100 CPU @ 3.10GHz ("GenuineIntel" 686-class) 3.10
> GHz
> cpu0:
> FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,SBF,SSE3,PCLMUL,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,SSE4.2,POPCNT,XSAVE,AVX
> real mem = 2124333056 (2025MB)
> avail mem = 2033119232 (1938MB)
> mainbus0 at root
> bios0 at mainbus0: AT/286+ BIOS, date 10/20/10, SMBIOS rev. 2.7 @ 0xeadc0
> (105 entries)
> bios0: vendor American Megatrends Inc. version "1.1a" date 09/28/2011
> bios0: Supermicro GS0300
> acpi at bios0 function 0x0 not configured
> pcibios at bios0 function 0x1a not configured
> bios0: ROM list: 0xc0000/0x8000
> mainbus0: Intel MP Specification (Version 1.4)
> cpu0 at mainbus0: apid 0 (boot processor)
> cpu0: apic clock running at 249MHz
> cpu1 at mainbus0: apid 2 (application processor)
> cpu1: Intel(R) Core(TM) i3-2100 CPU @ 3.10GHz ("GenuineIntel" 686-class) 7.73
> GHz
The primary cause for this is the wrong speed detected for the APIC
clock. Above it claims to be 249 MHz, when in fact it is 100 MHz.
The main trouble with that error is that now all delay(9) calls are
wrong by the same factor.
In order to calibrate the local APIC timer the i8254 timer is used for
reference. This timer is initialized with "TIMER_FREQ / hz" and then
counts down to one within 1/hz seconds after which it starts another
cycle. So this kinda looks like a saw-tooth.
The code picks an arbitrary value of the counter and stores it in
'starttick'. Then it loops hz times (which should take a total of one
second). The first inner do-while loop waits for the current counter
to become larger than 'starttick', i.e. for a new timer cycle to
start. The second do-while loop waits for the counter to be decreased
to the same value as 'starttick'. Both do-while loops together are
expected to take 1/hz seconds. To visualize this, imagine a time-
diagram with the saw-tooth counter value and a horizontal line at the
height of 'starttick'.
Now if the value of 'starttick' by chance is very large, the code might
miss the small time window where the counter in fact is larger than it.
The chance of a miss is even larger due to the call to i8254_delay()
where we're blind to the counter values. In the extreme case where
starttick is set to the counters initial value (TIMER_FREQ / hz), this
would be an endless loop cause the counter will never carry a value
larger than this. Same problem happens if the value of 'starttick' is
rather small.
In both cases (very large or very small 'starttick') some cycles of the
i8254 counter may be missed and we have to run longer until hz cycles
are detected. Once we leave the for-loop which is expected to run for
one second, the local APIC timer will have made a much larger progress
than expected and the code assumes that it's running faster than fact
it is.
The patch below fixes the problem:
- Neither call i8254_delay() nor lapic_gettick() inside the calibration
loop. Only keep an eye on the i8254 counter. We don't need these calls
and permanently reading the counter reduces the possibility of missing
cycles.
- Because comparison against an arbitrary start value is problematic,
we first wait for the current cycle to finish. Then inside the
calibration loop, we just wait until hz complete cycles have passed.
Gerhard
Index: sys/arch/amd64/amd64/lapic.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
retrieving revision 1.27
diff -u -p -r1.27 lapic.c
--- sys/arch/amd64/amd64/lapic.c 20 Sep 2010 06:33:46 -0000 1.27
+++ sys/arch/amd64/amd64/lapic.c 17 Sep 2012 17:55:48 -0000
@@ -305,6 +305,20 @@ lapic_initclocks(void)
extern int gettick(void); /* XXX put in header file */
extern u_long rtclock_tval; /* XXX put in header file */
+static __inline void
+wait_next_cycle(void)
+{
+ unsigned int tick, tlast;
+
+ tlast = (1 << 16); /* i8254 counter has 16 bits at most */
+ for (;;) {
+ tick = gettick();
+ if (tick > tlast)
+ return;
+ tlast = tick;
+ }
+}
+
/*
* Calibrate the local apic count-down timer (which is running at
* bus-clock speed) vs. the i8254 counter/timer (which is running at
@@ -319,8 +333,7 @@ extern u_long rtclock_tval; /* XXX put i
void
lapic_calibrate_timer(struct cpu_info *ci)
{
- unsigned int starttick, tick1, tick2, endtick;
- unsigned int startapic, apic1, apic2, endapic;
+ unsigned int startapic, endapic;
u_int64_t dtick, dapic, tmp;
long rf = read_rflags();
int i;
@@ -337,27 +350,20 @@ lapic_calibrate_timer(struct cpu_info *c
i82489_writereg(LAPIC_ICR_TIMER, 0x80000000);
disable_intr();
- starttick = gettick();
+
+ /* wait for current cycle to finish */
+ wait_next_cycle();
+
startapic = lapic_gettick();
- for (i = 0; i < hz; i++) {
- i8254_delay(2);
- do {
- tick1 = gettick();
- apic1 = lapic_gettick();
- } while (tick1 < starttick);
- i8254_delay(2);
- do {
- tick2 = gettick();
- apic2 = lapic_gettick();
- } while (tick2 > starttick);
- }
+ /* wait the next hz cycles */
+ for (i = 0; i < hz; i++)
+ wait_next_cycle();
- endtick = gettick();
endapic = lapic_gettick();
write_rflags(rf);
- dtick = hz * rtclock_tval + (starttick-endtick);
+ dtick = hz * rtclock_tval;
dapic = startapic-endapic;
/*
Index: sys/arch/i386/i386/lapic.c
===================================================================
RCS file: /cvs/src/sys/arch/i386/i386/lapic.c,v
retrieving revision 1.31
diff -u -p -r1.31 lapic.c
--- sys/arch/i386/i386/lapic.c 20 Sep 2010 06:33:47 -0000 1.31
+++ sys/arch/i386/i386/lapic.c 17 Sep 2012 17:55:50 -0000
@@ -272,6 +272,20 @@ lapic_initclocks(void)
extern int gettick(void); /* XXX put in header file */
+static __inline void
+wait_next_cycle(void)
+{
+ unsigned int tick, tlast;
+
+ tlast = (1 << 16); /* i8254 counter has 16 bits at most */
+ for (;;) {
+ tick = gettick();
+ if (tick > tlast)
+ return;
+ tlast = tick;
+ }
+}
+
/*
* Calibrate the local apic count-down timer (which is running at
* bus-clock speed) vs. the i8254 counter/timer (which is running at
@@ -286,8 +300,7 @@ extern int gettick(void); /* XXX put in
void
lapic_calibrate_timer(struct cpu_info *ci)
{
- unsigned int starttick, tick1, tick2, endtick;
- unsigned int startapic, apic1, apic2, endapic;
+ unsigned int startapic, endapic;
u_int64_t dtick, dapic, tmp;
int i, ef = read_eflags();
@@ -303,27 +316,20 @@ lapic_calibrate_timer(struct cpu_info *c
i82489_writereg(LAPIC_ICR_TIMER, 0x80000000);
disable_intr();
- starttick = gettick();
+
+ /* wait for current cycle to finish */
+ wait_next_cycle();
+
startapic = lapic_gettick();
- for (i = 0; i < hz; i++) {
- i8254_delay(2);
- do {
- tick1 = gettick();
- apic1 = lapic_gettick();
- } while (tick1 < starttick);
- i8254_delay(2);
- do {
- tick2 = gettick();
- apic2 = lapic_gettick();
- } while (tick2 > starttick);
- }
+ /* wait the next hz cycles */
+ for (i = 0; i < hz; i++)
+ wait_next_cycle();
- endtick = gettick();
endapic = lapic_gettick();
write_eflags(ef);
- dtick = hz * TIMER_DIV(hz) + (starttick-endtick);
+ dtick = hz * TIMER_DIV(hz);
dapic = startapic-endapic;
/*
