Hi,
There are some minor fix patches for LOCAL APIC calibration.
[PATCH 1/3] fix debug message of CPU clock speed
[PATCH 2/3] unify PM-Timer messages
[PATCH 3/3] fix typo "ACPI_PM_OVRRUN" -> "ACPI_PM_OVERRUN"
For more details, please refer to the header of each patch.
Thanks,
Yasuaki Ishimatsu
LOCAL APIC is corrected by PM-Timer, when SMI occurred while LOCAL APIC is calibrated.
In this case, LOCAL APIC debug message(Boot with apic=debug) is displayed correctly,
however, CPU clock speed debug message is displayed wrongly .
When SMI occured on my machine, which has 1.6GHz CPU, CPU clock speed is displayed
3622.0205 MHz as follow.
CPU0: Intel(R) Xeon(R) CPU 5110 @ 1.60GHz stepping 06
Using local APIC timer interrupts.
calibrating APIC timer ...
... lapic delta = 3773130
... PM timer delta = 812434
APIC calibration not consistent with PM Timer: 226ms instead of 100ms
APIC delta adjusted to PM-Timer: 1662420 (3773130)
..... delta 1662420
..... mult: 71411249
..... calibration result: 265987
..... CPU clock speed is 3622.0205 MHz. =====> here
..... host bus clock speed is 265.0987 MHz.
This patch fixes to displaying CPU clock speed correctly as follow.
CPU0: Intel(R) Xeon(R) CPU 5110 @ 1.60GHz stepping 06
Using local APIC timer interrupts.
calibrating APIC timer ...
... lapic delta = 3773131
... PM timer delta = 812434
APIC calibration not consistent with PM Timer: 226ms instead of 100ms
APIC delta adjusted to PM-Timer: 1662420 (3773131)
TSC delta adjusted to PM-Timer: 159592409 (362220564)
..... delta 1662420
..... mult: 71411249
..... calibration result: 265987
..... CPU clock speed is 1595.0924 MHz.
..... host bus clock speed is 265.0987 MHz.
Signed-off-by: Yasuaki Ishimatsu <[email protected]>
---
arch/x86/kernel/apic.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
Index: linux-2.6.29-rc2/arch/x86/kernel/apic.c
===================================================================
--- linux-2.6.29-rc2.orig/arch/x86/kernel/apic.c 2009-01-27 11:04:40.000000000 +0900
+++ linux-2.6.29-rc2/arch/x86/kernel/apic.c 2009-01-27 12:29:03.000000000 +0900
@@ -535,7 +535,8 @@ static void __init lapic_cal_handler(str
}
}
-static int __init calibrate_by_pmtimer(long deltapm, long *delta)
+static int __init calibrate_by_pmtimer(long deltapm, long *delta,
+ long *deltatsc)
{
const long pm_100ms = PMTMR_TICKS_PER_SEC / 10;
const long pm_thresh = pm_100ms / 100;
@@ -569,6 +570,15 @@ static int __init calibrate_by_pmtimer(l
pr_info("APIC delta adjusted to PM-Timer: "
"%lu (%ld)\n", (unsigned long)res, *delta);
*delta = (long)res;
+
+ if (cpu_has_tsc) {
+ res = (((u64)(*deltatsc)) * pm_100ms);
+ do_div(res, deltapm);
+ apic_printk(APIC_VERBOSE, "TSC delta adjusted to "
+ "PM-Timer: %lu (%ld)\n",
+ (unsigned long)res, *deltatsc);
+ *deltatsc = (long)res;
+ }
}
return 0;
@@ -579,7 +589,7 @@ static int __init calibrate_APIC_clock(v
struct clock_event_device *levt = &__get_cpu_var(lapic_events);
void (*real_handler)(struct clock_event_device *dev);
unsigned long deltaj;
- long delta;
+ long delta, deltatsc;
int pm_referenced = 0;
local_irq_disable();
@@ -609,9 +619,11 @@ static int __init calibrate_APIC_clock(v
delta = lapic_cal_t1 - lapic_cal_t2;
apic_printk(APIC_VERBOSE, "... lapic delta = %ld\n", delta);
+ deltatsc = (long)(lapic_cal_tsc2 - lapic_cal_tsc1);
+
/* we trust the PM based calibration if possible */
pm_referenced = !calibrate_by_pmtimer(lapic_cal_pm2 - lapic_cal_pm1,
- &delta);
+ &delta, &deltatsc);
/* Calculate the scaled math multiplication factor */
lapic_clockevent.mult = div_sc(delta, TICK_NSEC * LAPIC_CAL_LOOPS,
@@ -629,11 +641,10 @@ static int __init calibrate_APIC_clock(v
calibration_result);
if (cpu_has_tsc) {
- delta = (long)(lapic_cal_tsc2 - lapic_cal_tsc1);
apic_printk(APIC_VERBOSE, "..... CPU clock speed is "
"%ld.%04ld MHz.\n",
- (delta / LAPIC_CAL_LOOPS) / (1000000 / HZ),
- (delta / LAPIC_CAL_LOOPS) % (1000000 / HZ));
+ (deltatsc / LAPIC_CAL_LOOPS) / (1000000 / HZ),
+ (deltatsc / LAPIC_CAL_LOOPS) % (1000000 / HZ));
}
apic_printk(APIC_VERBOSE, "..... host bus clock speed is "
When LOCAL APIC was calibrated, the debug message is displayed as follows.
CPU0: Intel(R) Xeon(R) CPU 5110 @ 1.60GHz stepping 06
Using local APIC timer interrupts.
calibrating APIC timer ...
... lapic delta = 3773131
... PM timer delta = 812434
APIC calibration not consistent with PM Timer: 226ms instead of 100ms
APIC delta adjusted to PM-Timer: 1662420 (3773131)
TSC delta adjusted to PM-Timer: 159592409 (362220564)
..... delta 1662420
..... mult: 71411249
..... calibration result: 265987
..... CPU clock speed is 1595.0924 MHz.
..... host bus clock speed is 265.0987 MHz.
The PM-timer is written in several ways (PM-Timer, PM Timer, and PM timer)
in this message. This patch unifies those messages.
Signed-off-by: Yasuaki Ishimatsu <[email protected]>
---
arch/x86/kernel/apic.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Index: linux-2.6.29-rc2/arch/x86/kernel/apic.c
===================================================================
--- linux-2.6.29-rc2.orig/arch/x86/kernel/apic.c 2009-01-27 12:21:24.000000000 +0900
+++ linux-2.6.29-rc2/arch/x86/kernel/apic.c 2009-01-27 12:24:04.000000000 +0900
@@ -547,7 +547,7 @@ static int __init calibrate_by_pmtimer(l
return -1;
#endif
- apic_printk(APIC_VERBOSE, "... PM timer delta = %ld\n", deltapm);
+ apic_printk(APIC_VERBOSE, "... PM-Timer delta = %ld\n", deltapm);
/* Check, if the PM timer is available */
if (!deltapm)
@@ -557,12 +557,12 @@ static int __init calibrate_by_pmtimer(l
if (deltapm > (pm_100ms - pm_thresh) &&
deltapm < (pm_100ms + pm_thresh)) {
- apic_printk(APIC_VERBOSE, "... PM timer result ok\n");
+ apic_printk(APIC_VERBOSE, "... PM-Timer result ok\n");
} else {
res = (((u64)deltapm) * mult) >> 22;
do_div(res, 1000000);
pr_warning("APIC calibration not consistent "
- "with PM Timer: %ldms instead of 100ms\n",
+ "with PM-Timer: %ldms instead of 100ms\n",
(long)res);
/* Correct the lapic counter value */
res = (((u64)(*delta)) * pm_100ms);
This patch fixes typo.
Signed-off-by: Yasuaki Ishimatsu <[email protected]>
---
arch/x86/kernel/apic.c | 2 +-
arch/x86/kernel/tsc.c | 2 +-
include/linux/acpi_pmtmr.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
Index: linux-2.6.29-rc2/arch/x86/kernel/apic.c
===================================================================
--- linux-2.6.29-rc2.orig/arch/x86/kernel/apic.c 2009-01-27 12:24:04.000000000 +0900
+++ linux-2.6.29-rc2/arch/x86/kernel/apic.c 2009-01-27 12:24:18.000000000 +0900
@@ -528,7 +528,7 @@ static void __init lapic_cal_handler(str
lapic_cal_t2 = tapic;
lapic_cal_tsc2 = tsc;
if (pm < lapic_cal_pm1)
- pm += ACPI_PM_OVRRUN;
+ pm += ACPI_PM_OVERRUN;
lapic_cal_pm2 = pm;
lapic_cal_j2 = jiffies;
break;
Index: linux-2.6.29-rc2/arch/x86/kernel/tsc.c
===================================================================
--- linux-2.6.29-rc2.orig/arch/x86/kernel/tsc.c 2009-01-27 12:21:20.000000000 +0900
+++ linux-2.6.29-rc2/arch/x86/kernel/tsc.c 2009-01-27 12:24:18.000000000 +0900
@@ -161,7 +161,7 @@ static unsigned long calc_pmtimer_ref(u6
return ULONG_MAX;
if (pm2 < pm1)
- pm2 += (u64)ACPI_PM_OVRRUN;
+ pm2 += (u64)ACPI_PM_OVERRUN;
pm2 -= pm1;
tmp = pm2 * 1000000000LL;
do_div(tmp, PMTMR_TICKS_PER_SEC);
Index: linux-2.6.29-rc2/include/linux/acpi_pmtmr.h
===================================================================
--- linux-2.6.29-rc2.orig/include/linux/acpi_pmtmr.h 2009-01-27 12:21:20.000000000 +0900
+++ linux-2.6.29-rc2/include/linux/acpi_pmtmr.h 2009-01-27 12:24:18.000000000 +0900
@@ -10,7 +10,7 @@
#define ACPI_PM_MASK CLOCKSOURCE_MASK(24)
/* Overrun value */
-#define ACPI_PM_OVRRUN (1<<24)
+#define ACPI_PM_OVERRUN (1<<24)
#ifdef CONFIG_X86_PM_TIMER
* Yasuaki Ishimatsu <[email protected]> wrote:
> LOCAL APIC is corrected by PM-Timer, when SMI occurred while LOCAL APIC
> is calibrated. In this case, LOCAL APIC debug message(Boot with
> apic=debug) is displayed correctly, however, CPU clock speed debug
> message is displayed wrongly .
your fix looks good, but there are a few minor style details that need to
be addressed:
> When SMI occured on my machine, which has 1.6GHz CPU, CPU clock speed is
> displayed 3622.0205 MHz as follow.
>
> CPU0: Intel(R) Xeon(R) CPU 5110 @ 1.60GHz stepping 06
> Using local APIC timer interrupts.
> calibrating APIC timer ...
> ... lapic delta = 3773130
> ... PM timer delta = 812434
> APIC calibration not consistent with PM Timer: 226ms instead of 100ms
> APIC delta adjusted to PM-Timer: 1662420 (3773130)
> ..... delta 1662420
> ..... mult: 71411249
> ..... calibration result: 265987
> ..... CPU clock speed is 3622.0205 MHz. =====> here
> ..... host bus clock speed is 265.0987 MHz.
>
> This patch fixes to displaying CPU clock speed correctly as follow.
>
> CPU0: Intel(R) Xeon(R) CPU 5110 @ 1.60GHz stepping 06
> Using local APIC timer interrupts.
> calibrating APIC timer ...
> ... lapic delta = 3773131
> ... PM timer delta = 812434
> APIC calibration not consistent with PM Timer: 226ms instead of 100ms
> APIC delta adjusted to PM-Timer: 1662420 (3773131)
> TSC delta adjusted to PM-Timer: 159592409 (362220564)
> ..... delta 1662420
> ..... mult: 71411249
> ..... calibration result: 265987
> ..... CPU clock speed is 1595.0924 MHz.
> ..... host bus clock speed is 265.0987 MHz.
>
> Signed-off-by: Yasuaki Ishimatsu <[email protected]>
>
> ---
> arch/x86/kernel/apic.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> Index: linux-2.6.29-rc2/arch/x86/kernel/apic.c
> ===================================================================
> --- linux-2.6.29-rc2.orig/arch/x86/kernel/apic.c 2009-01-27 11:04:40.000000000 +0900
> +++ linux-2.6.29-rc2/arch/x86/kernel/apic.c 2009-01-27 12:29:03.000000000 +0900
> @@ -535,7 +535,8 @@ static void __init lapic_cal_handler(str
> }
> }
>
> -static int __init calibrate_by_pmtimer(long deltapm, long *delta)
> +static int __init calibrate_by_pmtimer(long deltapm, long *delta,
> + long *deltatsc)
The cleaner prototype line-width break is:
+ static int __init
+ calibrate_by_pmtimer(long deltapm, long *delta, long *deltatsc)
this function:
> @@ -569,6 +570,15 @@ static int __init calibrate_by_pmtimer(l
> pr_info("APIC delta adjusted to PM-Timer: "
> "%lu (%ld)\n", (unsigned long)res, *delta);
> *delta = (long)res;
> +
> + if (cpu_has_tsc) {
> + res = (((u64)(*deltatsc)) * pm_100ms);
> + do_div(res, deltapm);
> + apic_printk(APIC_VERBOSE, "TSC delta adjusted to "
> + "PM-Timer: %lu (%ld)\n",
> + (unsigned long)res, *deltatsc);
> + *deltatsc = (long)res;
> + }
has too deep nesting in the form of:
if (A) {
...
} else {
...
if (B) {
...
}
}
return 0;
Could you please restructure it to a cleaner control flow, in the form of:
if (A) {
...
return 0;
}
...
if (B) {
...
}
return 0;
?
Thanks,
Ingo
Hi, Ingo
Thank you for your comments.
I apply your comments to the patch ASAP.
Regard,
Yasuaki Ishimatsu
Ingo Molnar wrote:
> * Yasuaki Ishimatsu <[email protected]> wrote:
>
>> LOCAL APIC is corrected by PM-Timer, when SMI occurred while LOCAL APIC
>> is calibrated. In this case, LOCAL APIC debug message(Boot with
>> apic=debug) is displayed correctly, however, CPU clock speed debug
>> message is displayed wrongly .
>
> your fix looks good, but there are a few minor style details that need to
> be addressed:
>
>> When SMI occured on my machine, which has 1.6GHz CPU, CPU clock speed is
>> displayed 3622.0205 MHz as follow.
>>
>> CPU0: Intel(R) Xeon(R) CPU 5110 @ 1.60GHz stepping 06
>> Using local APIC timer interrupts.
>> calibrating APIC timer ...
>> ... lapic delta = 3773130
>> ... PM timer delta = 812434
>> APIC calibration not consistent with PM Timer: 226ms instead of 100ms
>> APIC delta adjusted to PM-Timer: 1662420 (3773130)
>> ..... delta 1662420
>> ..... mult: 71411249
>> ..... calibration result: 265987
>> ..... CPU clock speed is 3622.0205 MHz. =====> here
>> ..... host bus clock speed is 265.0987 MHz.
>>
>> This patch fixes to displaying CPU clock speed correctly as follow.
>>
>> CPU0: Intel(R) Xeon(R) CPU 5110 @ 1.60GHz stepping 06
>> Using local APIC timer interrupts.
>> calibrating APIC timer ...
>> ... lapic delta = 3773131
>> ... PM timer delta = 812434
>> APIC calibration not consistent with PM Timer: 226ms instead of 100ms
>> APIC delta adjusted to PM-Timer: 1662420 (3773131)
>> TSC delta adjusted to PM-Timer: 159592409 (362220564)
>> ..... delta 1662420
>> ..... mult: 71411249
>> ..... calibration result: 265987
>> ..... CPU clock speed is 1595.0924 MHz.
>> ..... host bus clock speed is 265.0987 MHz.
>>
>> Signed-off-by: Yasuaki Ishimatsu <[email protected]>
>>
>> ---
>> arch/x86/kernel/apic.c | 23 +++++++++++++++++------
>> 1 file changed, 17 insertions(+), 6 deletions(-)
>>
>> Index: linux-2.6.29-rc2/arch/x86/kernel/apic.c
>> ===================================================================
>> --- linux-2.6.29-rc2.orig/arch/x86/kernel/apic.c 2009-01-27 11:04:40.000000000 +0900
>> +++ linux-2.6.29-rc2/arch/x86/kernel/apic.c 2009-01-27 12:29:03.000000000 +0900
>> @@ -535,7 +535,8 @@ static void __init lapic_cal_handler(str
>> }
>> }
>>
>> -static int __init calibrate_by_pmtimer(long deltapm, long *delta)
>> +static int __init calibrate_by_pmtimer(long deltapm, long *delta,
>> + long *deltatsc)
>
> The cleaner prototype line-width break is:
>
> + static int __init
> + calibrate_by_pmtimer(long deltapm, long *delta, long *deltatsc)
>
>
> this function:
>
>> @@ -569,6 +570,15 @@ static int __init calibrate_by_pmtimer(l
>> pr_info("APIC delta adjusted to PM-Timer: "
>> "%lu (%ld)\n", (unsigned long)res, *delta);
>> *delta = (long)res;
>> +
>> + if (cpu_has_tsc) {
>> + res = (((u64)(*deltatsc)) * pm_100ms);
>> + do_div(res, deltapm);
>> + apic_printk(APIC_VERBOSE, "TSC delta adjusted to "
>> + "PM-Timer: %lu (%ld)\n",
>> + (unsigned long)res, *deltatsc);
>> + *deltatsc = (long)res;
>> + }
>
> has too deep nesting in the form of:
>
> if (A) {
> ...
> } else {
> ...
> if (B) {
> ...
> }
> }
>
> return 0;
>
>
> Could you please restructure it to a cleaner control flow, in the form of:
>
> if (A) {
> ...
> return 0;
> }
> ...
> if (B) {
> ...
> }
>
> return 0;
>
> ?
>
> Thanks,
>
> Ingo
>