[RFC][PATCH] x86_64: Fix collision between pmtimer and pit/hpet timekeeping

[RFC][PATCH] x86_64: Fix collision between pmtimer and pit/hpet timekeeping

[john stultz]

Hello,
	I thought I had caught all the problems when the no-legacy HPET support
landed close to the time that the ACPI PM timer support landed, but
apparently not. :(

On systems that do not support the HPET legacy functions (basically the
IBM x460, but there could be others), in time_init() we accidentally
fall into a PM timer conditional and set the vxtime_hz value to the PM
timer's frequency. We then use this value with the HPET for timekeeping.

This patch (which mimics the behavior in time_init_gtod) corrects the
collision.

Andi, any objections or suggestions for a better way?

thanks
-john

diff --git a/arch/x86_64/kernel/time.c b/arch/x86_64/kernel/time.c
index fdaddc4..fb389d2 100644
--- a/arch/x86_64/kernel/time.c
+++ b/arch/x86_64/kernel/time.c
@@ -913,7 +913,7 @@ void __init time_init(void)
 		cpu_khz = hpet_calibrate_tsc();
 		timename = "HPET";
 #ifdef CONFIG_X86_PM_TIMER
-	} else if (pmtmr_ioport) {
+	} else if (pmtmr_ioport && !vxtime.hpet_address) {
 		vxtime_hz = PM_TIMER_FREQUENCY;
 		timename = "PM";
 		pit_init();

Re: [RFC][PATCH] x86_64: Fix collision between pmtimer and pit/hpet timekeeping

[Andi Kleen]

On Tue, Dec 06, 2005 at 09:00:39PM -0800, john stultz wrote:
> Hello,
> 	I thought I had caught all the problems when the no-legacy HPET support
> landed close to the time that the ACPI PM timer support landed, but
> apparently not. :(
> 
> On systems that do not support the HPET legacy functions (basically the
> IBM x460, but there could be others), in time_init() we accidentally
> fall into a PM timer conditional and set the vxtime_hz value to the PM
> timer's frequency. We then use this value with the HPET for timekeeping.
> 
> This patch (which mimics the behavior in time_init_gtod) corrects the
> collision.
> 
> Andi, any objections or suggestions for a better way?

Ok. I will apply it.

But I never quite got why you fall back to the PIT on these systems
anyways - if LEGSUP is not set it just means that the HPET interrupt
cannot be routed to irq 0, right? It should be quite easy to change
the timer code to accept timer interrupts on other irqs. 

You just need to allocate the other interrupt and possibly coordinate
that with the hpet char driver (or rather move the code for that
from there to time.c). I think implementing that would be a better
solution.

-Andi

Re: [RFC][PATCH] x86_64: Fix collision between pmtimer and pit/hpet timekeeping

[john stultz]

On Wed, 2005-12-07 at 18:53 +0100, Andi Kleen wrote:
> On Tue, Dec 06, 2005 at 09:00:39PM -0800, john stultz wrote:
> > Hello,
> > 	I thought I had caught all the problems when the no-legacy HPET support
> > landed close to the time that the ACPI PM timer support landed, but
> > apparently not. :(
> > 
> > On systems that do not support the HPET legacy functions (basically the
> > IBM x460, but there could be others), in time_init() we accidentally
> > fall into a PM timer conditional and set the vxtime_hz value to the PM
> > timer's frequency. We then use this value with the HPET for timekeeping.
> > 
> > This patch (which mimics the behavior in time_init_gtod) corrects the
> > collision.
> > 
> > Andi, any objections or suggestions for a better way?
> 
> Ok. I will apply it.
> 
> But I never quite got why you fall back to the PIT on these systems
> anyways - if LEGSUP is not set it just means that the HPET interrupt
> cannot be routed to irq 0, right? It should be quite easy to change
> the timer code to accept timer interrupts on other irqs. 
> 
> You just need to allocate the other interrupt and possibly coordinate
> that with the hpet char driver (or rather move the code for that
> from there to time.c). I think implementing that would be a better
> solution.

Indeed that does sound like a decent cleanup. I can't promise anything
in the near future, but its on my list.

Would you then want to move all systems to use the non-legacy HPET
interrupt?

thanks
-john

Re: [RFC][PATCH] x86_64: Fix collision between pmtimer and pit/hpet timekeeping

[Andi Kleen]

On Wed, Dec 07, 2005 at 10:00:30AM -0800, john stultz wrote:
> Would you then want to move all systems to use the non-legacy HPET
> interrupt?

You mean all HPET systems? Yes that might be a good idea unless
someone else knows about problems with this. Interrupt 0 seems
to be shakey on many systems so not using it is probably
a good idea.

-Andi

Re: [RFC][PATCH] x86_64: Fix collision between pmtimer and pit/hpet timekeeping

[Vojtech Pavlik]

On Wed, Dec 07, 2005 at 10:00:30AM -0800, john stultz wrote:
> On Wed, 2005-12-07 at 18:53 +0100, Andi Kleen wrote:
> > On Tue, Dec 06, 2005 at 09:00:39PM -0800, john stultz wrote:
> > > Hello,
> > > 	I thought I had caught all the problems when the no-legacy HPET support
> > > landed close to the time that the ACPI PM timer support landed, but
> > > apparently not. :(
> > > 
> > > On systems that do not support the HPET legacy functions (basically the
> > > IBM x460, but there could be others), in time_init() we accidentally
> > > fall into a PM timer conditional and set the vxtime_hz value to the PM
> > > timer's frequency. We then use this value with the HPET for timekeeping.
> > > 
> > > This patch (which mimics the behavior in time_init_gtod) corrects the
> > > collision.
> > > 
> > > Andi, any objections or suggestions for a better way?
> > 
> > Ok. I will apply it.
> > 
> > But I never quite got why you fall back to the PIT on these systems
> > anyways - if LEGSUP is not set it just means that the HPET interrupt
> > cannot be routed to irq 0, right? It should be quite easy to change
> > the timer code to accept timer interrupts on other irqs. 
> > 
> > You just need to allocate the other interrupt and possibly coordinate
> > that with the hpet char driver (or rather move the code for that
> > from there to time.c). I think implementing that would be a better
> > solution.
> 
> Indeed that does sound like a decent cleanup. I can't promise anything
> in the near future, but its on my list.
> 
> Would you then want to move all systems to use the non-legacy HPET
> interrupt?
 
Yes, that'd be very cool. The problem with doing it is that HPET is
initialized at a very early stage of boot where it isn't at all clear
which APIC pins will be free to take the HPET interrupt(s).

-- 
Vojtech Pavlik
SuSE Labs, SuSE CR