[PATCH 2/2] x86/apic: check global clockevent in lapic timer setup
Pan, Jacob jun
From: Jacob Pan <jacob....@intel.com>
Date: Thu, 17 Dec 2009 08:07:57 -0800
Subject: [PATCH 2/2] x86/apic: check global clockevent in lapic timer setup
Global clockevent is needed to calibrate local apic timer.
This patch makes sure we have a valid global clockevent prior
to lapic timer setup.
Non-pc x86 mid platform with per cpu platform timer may not
have a global clockevent device.
Signed-off-by: Jacob Pan <jacob....@intel.com>
---
arch/x86/kernel/apic/apic.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index aa57c07..7e7aee1 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -724,6 +724,13 @@ static int __init calibrate_APIC_clock(void)
*/
void __init setup_boot_APIC_clock(void)
{
+ /* global clockevent is needed for calibration */
+ if (!global_clock_event) {
+ apic_printk(APIC_DEBUG,
+ "no global clockevent for calibration\n");
+ return;
+ }
+
/*
* The local apic timer can be disabled via the kernel
* commandline or from the CPU detection code. Register the lapic
--
1.6.5.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Cyrill Gorcunov
Hi Jacob,
Wouldn't be better to operate the same way as in case of "noapictimer"
boot option. I guess the non-pc x86 midplatforms you're mentioning
do not use SMP ever but just to be consistent in code.
Perhaps something like:
void __init setup_boot_APIC_clock(void) {
if (!global_clock_event) {
pr_info("No global clockevent for calibration\n");
disable_apic_timer = 1;
}
if (disable_apic_timer) {
pr_info("Disabling APIC timer\n");
/* No broadcast on UP ! */
if (num_possible_cpus() > 1) {
lapic_clockevent.mult = 1;
setup_APIC_timer();
}
return;
}
...
}
Or I miss something?
-- Cyrill
H. Peter Anvin
>> Wouldn't be better to operate the same way as in case of "noapictimer"
>> boot option. I guess the non-pc x86 midplatforms you're mentioning
>> do not use SMP ever but just to be consistent in code.
>>
> In that case we have a per cpu platform timer without global clockevent.
> so i think we don't want the dummy lapic event. we don't want to use the
> broadcast mechanism as mentioned in the comments before disabling lapic
> timer.
>
> For moorestown, I can use x86_init.timers.setup_percpu_clockev
> abstraction function so that Moorestown platform does not need to call
> setup_boot_APIC_clock() if per cpu platform timer is used. so many IFs :).
>
> But in the case of having constant and always on LAPIC timer, we still do
> not want the dummy lapic clockevent and the broadcast. we will just have
> per cpu always on local apic timers without global clockevent device.
OK, I'm not entirely sure I follow this, and I'm not sure someone trying
to understand the code in five years would, either. I don't really see
the above translating into "we don't have a global clockevent, therefore
we shouldn't initialize (but should still not disable) the local APIC
timer."
-hpa
Pan, Jacob jun
>
>Wouldn't be better to operate the same way as in case of "noapictimer"
>boot option. I guess the non-pc x86 midplatforms you're mentioning
>do not use SMP ever but just to be consistent in code.
>
In that case we have a per cpu platform timer without global clockevent.
so i think we don't want the dummy lapic event. we don't want to use the
broadcast mechanism as mentioned in the comments before disabling lapic
timer.
For moorestown, I can use x86_init.timers.setup_percpu_clockev
abstraction function so that Moorestown platform does not need to call
setup_boot_APIC_clock() if per cpu platform timer is used. so many IFs :).
But in the case of having constant and always on LAPIC timer, we still do
not want the dummy lapic clockevent and the broadcast. we will just have
per cpu always on local apic timers without global clockevent device.
Pan, Jacob jun
[[JPAN]] There is some thing wrong in my logic.
If we have always running lapic timer, and per cpu platform timers, we would
still want to set up the lapic timer without global clockevent, just without
calibration. perhaps use a platform specific setup_percpu_clockev() function.
So i don't think we need this patch at the moment, maybe we only need to do a
sanity check for global clockevent in calibrate_APIC_clock().
Honestly, i don't fully understand how the dummy lapic event device is related
to the broadcast mechanism. can someone explain why we register the dummy
lapic clockevent?
Thomas Gleixner
No, we need to fix the whole lapic timer calibration logic first.
There is no reason why we don't calibrate the lapic at the same time
as we calibrate the TSC.
Another question is why there is no way to read out the lapic clock
frequency from some config registers or wherever the chip designers
decided to hide that. There is no real reason why the lapic timer
calibration needs to be extremly precise.
> Honestly, i don't fully understand how the dummy lapic event device
> is related to the broadcast mechanism. can someone explain why we
> register the dummy lapic clockevent?
The broadcast mechanism is there in the first place to work around the
APIC stops in deeper C-states idiocy.
Then we need to support the disable lapic timer command line option
(even on SMP) so we make use of the existing broadcast mechanism and
register the dummy device to have a per cpu clock event device.
Thanks,
tglx
Pan, Jacob jun
[[JPAN]] that seems to be much more efficient and we can have platform specific
way of calibration too with the x86_init abstraction.
>
>Another question is why there is no way to read out the lapic clock
>frequency from some config registers or wherever the chip designers
>decided to hide that. There is no real reason why the lapic timer
>calibration needs to be extremly precise.
>
[[JPAN]] x86 does have MSR_FSB_FREQ to read bus frequency then the DCR to figure
out LAPIC timer freq. but i guess not all CPU models have that. so having
the abstraction would be a plus for those do have reliable reporting of lapic
freq.
>> Honestly, i don't fully understand how the dummy lapic event device
>> is related to the broadcast mechanism. can someone explain why we
>> register the dummy lapic clockevent?
>
>The broadcast mechanism is there in the first place to work around the
>APIC stops in deeper C-states idiocy.
>
>Then we need to support the disable lapic timer command line option
>(even on SMP) so we make use of the existing broadcast mechanism and
>register the dummy device to have a per cpu clock event device.
>
[[JPAN]] thanks for the explanation. so if we have per cpu timer that is
always-on, and don't have a broadcast timer, then the dummy device would not
be needed, correct?
Cyrill Gorcunov
...
> >
> >No, we need to fix the whole lapic timer calibration logic first.
> >
> >There is no reason why we don't calibrate the lapic at the same time
> >as we calibrate the TSC.
> [[JPAN]] that seems to be much more efficient and we can have platform specific
> way of calibration too with the x86_init abstraction.
Good idea I think!
> >
> >Another question is why there is no way to read out the lapic clock
> >frequency from some config registers or wherever the chip designers
> >decided to hide that. There is no real reason why the lapic timer
> >calibration needs to be extremly precise.
> >
> [[JPAN]] x86 does have MSR_FSB_FREQ to read bus frequency then the DCR to figure
> out LAPIC timer freq. but i guess not all CPU models have that. so having
> the abstraction would be a plus for those do have reliable reporting of lapic
> freq.
IIRC old apics may use independent clock signal too, though I dont think that we
ever switch (espec novadays) to use it due to obsolescense of such chips :)
>
> >> Honestly, i don't fully understand how the dummy lapic event device
> >> is related to the broadcast mechanism. can someone explain why we
> >> register the dummy lapic clockevent?
> >
> >The broadcast mechanism is there in the first place to work around the
> >APIC stops in deeper C-states idiocy.
> >
> >Then we need to support the disable lapic timer command line option
> >(even on SMP) so we make use of the existing broadcast mechanism and
> >register the dummy device to have a per cpu clock event device.
> >
> [[JPAN]] thanks for the explanation. so if we have per cpu timer that is
> always-on, and don't have a broadcast timer, then the dummy device would not
> be needed, correct?
>
>
Hmm... We may be using nmi detector, so I think we still need dummy clockevent
device to send broadcast "time" IPI, or per-cpu timer interrupt handler have
to call the local apic interrupt routine. At least that is how I imagine this
scheme :)
> >Thanks,
> >
> > tglx
>
-- Cyrill