Skip to contentSkip to conversation

x86_64: enable clockevents and dynticks

RFC PATCH In review Chris Wright 4 participants 17 messages Mar 31, 2007 – Apr 2, 2007 Char and misc drivers

lore
Share & tools
lore
#0 lore raw

cover x86_64: enable clockevents and dynticks

This series converts x86_64 timers to clockevents drivers and then enables dynticks. There's some minor cleanups along the way. The lapic broadcast mechanism is untested, I'm sure it still needs work, there's still some cruft in lapic_setup_timer.

This is just for comments at this point, now that it's working on my test box in both NO_HZ=n and NO_HZ=n configurations (typically using hpet).

thanks,
-chris
--

#1 lore raw

RFC PATCH 1/5 x86_64: untangle asm/hpet.h from asm/timex.h

When making changes to x86_64 timers, I noticed that touching hpet.h triggered an unreasonably large rebuild. Untangling it from timex.h quiets the extra rebuild quite a bit.

---
 drivers/char/rtc.c         |    2 +-
 include/asm-x86_64/apic.h  |    1 +
 include/asm-x86_64/hpet.h  |    1 -
 include/asm-x86_64/timex.h |    1 -
 4 files changed, 2 insertions(+), 3 deletions(-)
drivers/char/rtc.c +1 -1
@@ -82,7 +82,7 @@
 #include <asm/uaccess.h>
 #include <asm/system.h>
 
-#if defined(__i386__)
+#if defined(__i386__) || defined(__x86_64__)
 #include <asm/hpet.h>
 #endif
 
include/asm-x86_64/timex.h +0 -1
@@ -9,7 +9,6 @@
 #include <asm/8253pit.h>
 #include <asm/msr.h>
 #include <asm/vsyscall.h>
-#include <asm/hpet.h>
 #include <asm/system.h>
 #include <asm/processor.h>
 #include <asm/tsc.h>
include/asm-x86_64/apic.h +1 -0
@@ -84,6 +84,7 @@ extern int APIC_init_uniprocessor (void)
 extern void disable_APIC_timer(void);
 extern void enable_APIC_timer(void);
 extern void clustered_apic_check(void);
+extern int apic_is_clustered_box(void);
 
 extern void setup_APIC_extened_lvt(unsigned char lvt_off, unsigned char vector,
 				   unsigned char msg_type, unsigned char mask);
include/asm-x86_64/hpet.h +0 -1
@@ -55,7 +55,6 @@
 
 extern int is_hpet_enabled(void);
 extern int hpet_rtc_timer_init(void);
-extern int apic_is_clustered_box(void);
 extern int hpet_arch_init(void);
 extern int hpet_timer_stop_set_go(unsigned long tick);
 extern int hpet_reenable(void);

--

#2 lore raw

RFC PATCH 2/5 x86_64: drive set_rtc_mss from standalone timer

In reply to Chris Wright

This is how it's done on i386, and it makes one less bit of code to trim from main_timer_handler() when converting to clockevents.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc john stultz <[email protected]>
Cc Andi Kleen <[email protected]>
---
 arch/x86_64/kernel/time.c |   68 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 52 insertions(+), 16 deletions(-)
arch/x86_64/kernel/time.c +52 -16
@@ -81,8 +81,9 @@ EXPORT_SYMBOL(profile_pc);
  * sheet for details.
  */
 
-static void set_rtc_mmss(unsigned long nowtime)
+static int set_rtc_mmss(unsigned long nowtime)
 {
+	int retval = 0;
 	int real_seconds, real_minutes, cmos_minutes;
 	unsigned char control, freq_select;
 
@@ -122,6 +123,7 @@ static void set_rtc_mmss(unsigned long n
 	if (abs(real_minutes - cmos_minutes) >= 30) {
 		printk(KERN_WARNING "time.c: can't update CMOS clock "
 		       "from %d to %d\n", cmos_minutes, real_minutes);
+		retval = -1;
 	} else {
 		BIN_TO_BCD(real_seconds);
 		BIN_TO_BCD(real_minutes);
@@ -141,12 +143,60 @@ static void set_rtc_mmss(unsigned long n
 	CMOS_WRITE(freq_select, RTC_FREQ_SELECT);
 
 	spin_unlock(&rtc_lock);
+
+	return retval;
+}
+
+static void sync_cmos_clock(unsigned long dummy);
+
+static DEFINE_TIMER(sync_cmos_timer, sync_cmos_clock, 0, 0);
+
+static void sync_cmos_clock(unsigned long dummy)
+{
+	struct timespec now, next;
+	int fail = 1;
+
+	/*
+	 * If we have an externally synchronized Linux clock, then update
+	 * CMOS clock accordingly every ~11 minutes. Set_rtc_mmss() has to be
+	 * called as close as possible to 500 ms before the new second starts.
+	 * This code is run on a timer.  If the clock is set, that timer
+	 * may not expire at the correct time.  Thus, we adjust...
+	 */
+	if (!ntp_synced())
+		/*
+		 * Not synced, exit, do not restart a timer (if one is
+		 * running, let it run out).
+		 */
+		return;
+
+	getnstimeofday(&now);
+	if (abs(xtime.tv_nsec - 500000000) <= tick_nsec / 2)
+		fail = set_rtc_mmss(now.tv_sec);
+
+	next.tv_nsec = 500000000 - now.tv_nsec;
+	if (next.tv_nsec <= 0)
+		next.tv_nsec += NSEC_PER_SEC;
+
+	if (!fail)
+		next.tv_sec = 659;
+	else
+		next.tv_sec = 0;
+
+	if (next.tv_nsec >= NSEC_PER_SEC) {
+		next.tv_sec++;
+		next.tv_nsec -= NSEC_PER_SEC;
+	}
+	mod_timer(&sync_cmos_timer, jiffies + timespec_to_jiffies(&next));
 }
 
+void notify_arch_cmos_timer(void)
+{
+	mod_timer(&sync_cmos_timer, jiffies + 1);
+}
 
 void main_timer_handler(void)
 {
-	static unsigned long rtc_update = 0;
 /*
  * Here we are in the timer irq handler. We have irqs locally disabled (so we
  * don't need spin_lock_irqsave()) but we don't know if the timer_bh is running
@@ -174,20 +224,6 @@ void main_timer_handler(void)
 	if (!using_apic_timer)
 		smp_local_timer_interrupt();
 
-/*
- * If we have an externally synchronized Linux clock, then update CMOS clock
- * accordingly every ~11 minutes. set_rtc_mmss() will be called in the jiffy
- * closest to exactly 500 ms before the next second. If the update fails, we
- * don't care, as it'll be updated on the next turn, and the problem (time way
- * off) isn't likely to go away much sooner anyway.
- */
-
-	if (ntp_synced() && xtime.tv_sec > rtc_update &&
-		abs(xtime.tv_nsec - 500000000) <= tick_nsec / 2) {
-		set_rtc_mmss(xtime.tv_sec);
-		rtc_update = xtime.tv_sec + 660;
-	}
- 
 	write_sequnlock(&xtime_lock);
 }
 

--

#3 lore raw

RFC PATCH 3/5 x86_64: clockevents drivers

In reply to Chris Wright

Convert lapic, pit and hpet based timers to clockevents. This brings x86_64 in line with i386. And it is needed to enable dynticks.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc john stultz <[email protected]>
Cc Andi Kleen <[email protected]>
---
 arch/x86_64/Kconfig       |    8 +
 arch/x86_64/kernel/apic.c |  123 ++++++++++++++++++++-------
 arch/x86_64/kernel/hpet.c |  168 +++++++++++++++++++++++++++++--------
 arch/x86_64/kernel/time.c |  204 ++++++++++++++++------------------------------
 include/asm-x86_64/hpet.h |    2 
 5 files changed, 307 insertions(+), 198 deletions(-)
arch/x86_64/kernel/time.c +73 -131
@@ -42,10 +42,19 @@
 #include <linux/cpufreq.h>
 #include <linux/hpet.h>
 #include <asm/apic.h>
+#include <linux/clockchips.h>
+#include <asm/delay.h>
 #include <asm/hpet.h>
 
 extern void i8254_timer_resume(void);
 extern int using_apic_timer;
+extern struct clock_event_device pit_clockevent;
+/*
+ * HPET replaces the PIT, when enabled. So we need to know, which of
+ * the two timers is used
+ */
+struct clock_event_device *global_clock_event;
+
 
 static char *timename = NULL;
 
@@ -197,34 +206,7 @@ void notify_arch_cmos_timer(void)
 
 void main_timer_handler(void)
 {
-/*
- * Here we are in the timer irq handler. We have irqs locally disabled (so we
- * don't need spin_lock_irqsave()) but we don't know if the timer_bh is running
- * on the other CPU, so we need a lock. We also need to lock the vsyscall
- * variables, because both do_timer() and us change them -arca+vojtech
- */
-
-	write_seqlock(&xtime_lock);
-
-/*
- * Do the timer stuff.
- */
-
-	do_timer(1);
-#ifndef CONFIG_SMP
-	update_process_times(user_mode(get_irq_regs()));
-#endif
-
-/*
- * In the SMP case we use the local APIC timer interrupt to do the profiling,
- * except when we simulate SMP mode on a uniprocessor system, in that case we
- * have to call the local interrupt handler.
- */
-
-	if (!using_apic_timer)
-		smp_local_timer_interrupt();
-
-	write_sequnlock(&xtime_lock);
+	global_clock_event->event_handler(global_clock_event);
 }
 
 static irqreturn_t timer_interrupt(int irq, void *dev_id)
@@ -237,7 +219,7 @@ static irqreturn_t timer_interrupt(int i
 	return IRQ_HANDLED;
 }
 
-static unsigned long get_cmos_time(void)
+unsigned long read_persistent_clock(void)
 {
 	unsigned int year, mon, day, hour, min, sec;
 	unsigned long flags;
@@ -321,38 +303,80 @@ static unsigned int __init pit_calibrate
 #define PIT_MODE 0x43
 #define PIT_CH0  0x40
 
-static void __init __pit_init(int val, u8 mode)
+static void init_pit_timer(enum clock_event_mode mode,
+			   struct clock_event_device *evt)
 {
 	unsigned long flags;
 
 	spin_lock_irqsave(&i8253_lock, flags);
-	outb_p(mode, PIT_MODE);
-	outb_p(val & 0xff, PIT_CH0);	/* LSB */
-	outb_p(val >> 8, PIT_CH0);	/* MSB */
+
+	switch(mode) {
+	case CLOCK_EVT_MODE_PERIODIC:
+		/* binary, mode 2, LSB/MSB, ch 0 */
+		outb_p(0x34, PIT_MODE);
+		udelay(10);
+		outb_p(LATCH & 0xff , PIT_CH0); /* LSB */
+		outb(LATCH >> 8 , PIT_CH0);     /* MSB */
+		break;
+
+	case CLOCK_EVT_MODE_ONESHOT:
+		/* One shot setup */
+		outb_p(0x38, PIT_MODE);
+		udelay(10);
+		break;
+	case CLOCK_EVT_MODE_SHUTDOWN:
+	case CLOCK_EVT_MODE_UNUSED:
+		outb_p(0x30, PIT_MODE);
+		outb_p(0, PIT_CH0);	/* LSB */
+		outb_p(0, PIT_CH0);	/* MSB */
+		break;
+	}
 	spin_unlock_irqrestore(&i8253_lock, flags);
 }
 
-void __init pit_init(void)
+static int pit_next_event(unsigned long delta, struct clock_event_device *evt)
 {
-	__pit_init(LATCH, 0x34); /* binary, mode 2, LSB/MSB, ch 0 */
+	unsigned long flags;
+
+	spin_lock_irqsave(&i8253_lock, flags);
+	outb_p(delta & 0xff , PIT_CH0); /* LSB */
+	outb(delta >> 8 , PIT_CH0);     /* MSB */
+	spin_unlock_irqrestore(&i8253_lock, flags);
+
+	return 0;
 }
 
-void __init pit_stop_interrupt(void)
+struct clock_event_device pit_clockevent = {
+	.name		= "pit",
+	.features	= CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
+	.set_mode       = init_pit_timer,
+	.set_next_event = pit_next_event,
+	.shift		= 32,
+	.irq		= 0,
+};
+
+void __init stop_timer_interrupt(void)
 {
-	__pit_init(0, 0x30); /* mode 0 */
+	/* XXX this is bogus */
+	clockevents_set_mode(global_clock_event, CLOCK_EVT_MODE_SHUTDOWN);
+	printk(KERN_INFO "timer: %s interrupt stopped.\n", global_clock_event->name);
 }
 
-void __init stop_timer_interrupt(void)
+static void __init setup_pit_timer(void)
 {
-	char *name;
-	if (hpet_address) {
-		name = "HPET";
-		hpet_timer_stop_set_go(0);
-	} else {
-		name = "PIT";
-		pit_stop_interrupt();
-	}
-	printk(KERN_INFO "timer: %s interrupt stopped.\n", name);
+	/*
+	 * Start pit with the boot cpu mask and make it global after the
+	 * IO_APIC has been initialized.
+	 */
+	pit_clockevent.cpumask = cpumask_of_cpu(0);
+
+	pit_clockevent.mult = div_sc(CLOCK_TICK_RATE, NSEC_PER_SEC, 32);
+	pit_clockevent.max_delta_ns =
+		clockevent_delta2ns(0x7FFF, &pit_clockevent);
+	pit_clockevent.min_delta_ns =
+		clockevent_delta2ns(0xF, &pit_clockevent);
+	clockevents_register_device(&pit_clockevent);
+	global_clock_event = &pit_clockevent;
 }
 
 static struct irqaction irq0 = {
@@ -361,16 +385,8 @@ static struct irqaction irq0 = {
 
 void __init time_init(void)
 {
-	if (nohpet)
-		hpet_address = 0;
-	xtime.tv_sec = get_cmos_time();
-	xtime.tv_nsec = 0;
-
-	set_normalized_timespec(&wall_to_monotonic,
-	                        -xtime.tv_sec, -xtime.tv_nsec);
-
-	if (hpet_arch_init())
-		hpet_address = 0;
+	if (!hpet_arch_init())
+		setup_pit_timer();
 
 	if (hpet_use_timer) {
 		/* set tick_nsec to use the proper rate for HPET */
@@ -378,7 +394,6 @@ void __init time_init(void)
 		cpu_khz = hpet_calibrate_tsc();
 		timename = "HPET";
 	} else {
-		pit_init();
 		cpu_khz = pit_calibrate_tsc();
 		timename = "PIT";
 	}
@@ -398,76 +413,3 @@ void __init time_init(void)
 
 	setup_irq(0, &irq0);
 }
-
-
-static long clock_cmos_diff;
-static unsigned long sleep_start;
-
-/*
- * sysfs support for the timer.
- */
-
-static int timer_suspend(struct sys_device *dev, pm_message_t state)
-{
-	/*
-	 * Estimate time zone so that set_time can update the clock
-	 */
-	long cmos_time =  get_cmos_time();
-
-	clock_cmos_diff = -cmos_time;
-	clock_cmos_diff += get_seconds();
-	sleep_start = cmos_time;
-	return 0;
-}
-
-static int timer_resume(struct sys_device *dev)
-{
-	unsigned long flags;
-	unsigned long sec;
-	unsigned long ctime = get_cmos_time();
-	long sleep_length = (ctime - sleep_start) * HZ;
-
-	if (sleep_length < 0) {
-		printk(KERN_WARNING "Time skew detected in timer resume!\n");
-		/* The time after the resume must not be earlier than the time
-		 * before the suspend or some nasty things will happen
-		 */
-		sleep_length = 0;
-		ctime = sleep_start;
-	}
-	if (hpet_address)
-		hpet_reenable();
-	else
-		i8254_timer_resume();
-
-	sec = ctime + clock_cmos_diff;
-	write_seqlock_irqsave(&xtime_lock,flags);
-	xtime.tv_sec = sec;
-	xtime.tv_nsec = 0;
-	jiffies += sleep_length;
-	write_sequnlock_irqrestore(&xtime_lock,flags);
-	touch_softlockup_watchdog();
-	return 0;
-}
-
-static struct sysdev_class timer_sysclass = {
-	.resume = timer_resume,
-	.suspend = timer_suspend,
-	set_kset_name("timer"),
-};
-
-/* XXX this sysfs stuff should probably go elsewhere later -john */
-static struct sys_device device_timer = {
-	.id	= 0,
-	.cls	= &timer_sysclass,
-};
-
-static int time_init_device(void)
-{
-	int error = sysdev_class_register(&timer_sysclass);
-	if (!error)
-		error = sysdev_register(&device_timer);
-	return error;
-}
-
-device_initcall(time_init_device);
arch/x86_64/kernel/hpet.c +133 -35

246 lines not on this page · raw message

arch/x86_64/Kconfig +8 -0
@@ -28,6 +28,14 @@ config GENERIC_TIME
 	bool
 	default y
 
+config GENERIC_CLOCKEVENTS_BROADCAST
+	bool
+	default y
+
+config GENERIC_CLOCKEVENTS
+	bool
+	default y
+
 config GENERIC_TIME_VSYSCALL
 	bool
 	default y
arch/x86_64/kernel/apic.c +93 -30

229 lines not on this page · raw message

include/asm-x86_64/hpet.h +0 -2
@@ -56,8 +56,6 @@
 extern int is_hpet_enabled(void);
 extern int hpet_rtc_timer_init(void);
 extern int hpet_arch_init(void);
-extern int hpet_timer_stop_set_go(unsigned long tick);
-extern int hpet_reenable(void);
 extern unsigned int hpet_calibrate_tsc(void);
 
 extern int hpet_use_timer;

--

#4 lore raw

RFC PATCH 4/5 x86_64: prep idle loop for dynticks

In reply to Chris Wright

Add tick_nohz_{stop,restart}_sched_tick to idle loop in prepartion for turning on dynticks. These are just noops until NO_HZ is enabled in next patch.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc john stultz <[email protected]>
Cc Andi Kleen <[email protected]>
---
 arch/x86_64/kernel/process.c |    3 +++
 1 file changed, 3 insertions(+)
arch/x86_64/kernel/process.c +3 -0
@@ -36,6 +36,7 @@
 #include <linux/random.h>
 #include <linux/notifier.h>
 #include <linux/kprobes.h>
+#include <linux/tick.h>
 
 #include <asm/uaccess.h>
 #include <asm/pgtable.h>
@@ -201,6 +202,7 @@ void cpu_idle (void)
 	current_thread_info()->status |= TS_POLLING;
 	/* endless idle loop with no priority at all */
 	while (1) {
+		tick_nohz_stop_sched_tick();
 		while (!need_resched()) {
 			void (*idle)(void);
 
@@ -227,6 +229,7 @@ void cpu_idle (void)
 			__exit_idle();
 		}
 
+		tick_nohz_restart_sched_tick();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

--

#5 lore raw

RFC PATCH 5/5 x86_64: enable dynticks

In reply to Chris Wright

Everything is in place, so give the CONFIG_NO_HZ option.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc john stultz <[email protected]>
Cc Andi Kleen <[email protected]>
---
 arch/x86_64/Kconfig |    2 ++
 1 file changed, 2 insertions(+)
arch/x86_64/Kconfig +2 -0
@@ -134,6 +134,8 @@ source "init/Kconfig"
 
 menu "Processor type and features"
 
+source "kernel/time/Kconfig"
+
 choice
 	prompt "Subarchitecture Type"
 	default X86_PC

--

#6 lore raw
In reply to Chris Wright
Quoted 8 lines · Chris Wright

This series converts x86_64 timers to clockevents drivers and then enables dynticks. There's some minor cleanups along the way. The lapic broadcast mechanism is untested, I'm sure it still needs work, there's still some cruft in lapic_setup_timer.

This is just for comments at this point, now that it's working on my test box in both NO_HZ=n and NO_HZ=n configurations (typically using hpet).

hm, neat - did you take a look at the x86_64 clockevents code that is in -rt and that has been there for a year or so and has been tested quite extensively?

Ingo

#7 lore raw
Quoted 6 lines · * Ingo Molnar ([email protected]) wrote:
Quoted 5 lines · Chris Wright

This series converts x86_64 timers to clockevents drivers and then enables dynticks. There's some minor cleanups along the way. The lapic broadcast mechanism is untested, I'm sure it still needs work, there's still some cruft in lapic_setup_timer.

Quoted 7 lines
Quoted 3 lines

This is just for comments at this point, now that it's working on my test box in both NO_HZ=n and NO_HZ=n configurations (typically using hpet).

hm, neat - did you take a look at the x86_64 clockevents code that is in -rt and that has been there for a year or so and has been tested quite extensively?

Yes, that's what I started with. It was only partially done, for example, didn't have hpet done yet and didn't work well on my machine (hang on boot). As I dug at it, there was still more room for cleanups, etc. Hence this, which has also been tested somewhat extensively as Jeremy noted from my typo above... Both NO_HZ=n _and_ NO_HZ=n ;-)

thanks,
-chris

#8 lore raw
Quoted 5 lines · Chris Wright
Quoted 3 lines

hm, neat - did you take a look at the x86_64 clockevents code that is in -rt and that has been there for a year or so and has been tested quite extensively?

Yes, that's what I started with. [...]

ok :)

Quoted 5 lines

[...] It was only partially done, for example, didn't have hpet done yet and didn't work well on my machine (hang on boot). As I dug at it, there was still more room for cleanups, etc. Hence this, which has also been tested somewhat extensively as Jeremy noted from my typo above... Both NO_HZ=n _and_ NO_HZ=n ;-)

good :)

Ingo

#9 lore raw
In reply to Chris Wright
Quoted 8 lines · Chris Wright

This series converts x86_64 timers to clockevents drivers and then enables dynticks. There's some minor cleanups along the way. The lapic broadcast mechanism is untested, I'm sure it still needs work, there's still some cruft in lapic_setup_timer.

This is just for comments at this point, now that it's working on my test box in both NO_HZ=n and NO_HZ=n configurations (typically using hpet).

Have you checked, if we could share the code between i386 and x86_64 at least for PIT and HPET. I'm not sure about the local APIC, but I think it might be doable as well.

tglx

#10 lore raw
Quoted 6 lines · * Thomas Gleixner ([email protected]) wrote:
Quoted 5 lines · Chris Wright

This series converts x86_64 timers to clockevents drivers and then enables dynticks. There's some minor cleanups along the way. The lapic broadcast mechanism is untested, I'm sure it still needs work, there's still some cruft in lapic_setup_timer.

Quoted 7 lines
Quoted 3 lines

This is just for comments at this point, now that it's working on my test box in both NO_HZ=n and NO_HZ=n configurations (typically using hpet).

Have you checked, if we could share the code between i386 and x86_64 at least for PIT and HPET. I'm not sure about the local APIC, but I think it might be doable as well.

Yes, that's part of why I did some of the cleanups that way, so that we could merge the two together later.

thanks,
-chris

#11 lore raw
Quoted 11 lines · Chris Wright
Quoted 10 lines · * Thomas Gleixner ([email protected]) wrote:
Quoted 8 lines · Chris Wright

This series converts x86_64 timers to clockevents drivers and then enables dynticks. There's some minor cleanups along the way. The lapic broadcast mechanism is untested, I'm sure it still needs work, there's still some cruft in lapic_setup_timer.

This is just for comments at this point, now that it's working on my test box in both NO_HZ=n and NO_HZ=n configurations (typically using hpet).

Quoted 6 lines
Quoted 3 lines

Have you checked, if we could share the code between i386 and x86_64 at least for PIT and HPET. I'm not sure about the local APIC, but I think it might be doable as well.

Yes, that's part of why I did some of the cleanups that way, so that we could merge the two together later.

I'd prefer to get this done now. If we plug it in as is, the "merge later" probably will never happen.

tglx

#12 lore raw
Quoted 16 lines · * Thomas Gleixner ([email protected]) wrote:
Quoted 15 lines · Chris Wright
Quoted 13 lines · * Thomas Gleixner ([email protected]) wrote:
Quoted 8 lines · Chris Wright

This series converts x86_64 timers to clockevents drivers and then enables dynticks. There's some minor cleanups along the way. The lapic broadcast mechanism is untested, I'm sure it still needs work, there's still some cruft in lapic_setup_timer.

This is just for comments at this point, now that it's working on my test box in both NO_HZ=n and NO_HZ=n configurations (typically using hpet).

Have you checked, if we could share the code between i386 and x86_64 at least for PIT and HPET. I'm not sure about the local APIC, but I think it might be doable as well.

Quoted 5 lines
Quoted 2 lines

Yes, that's part of why I did some of the cleanups that way, so that we could merge the two together later.

I'd prefer to get this done now. If we plug it in as is, the "merge later" probably will never happen.

that's fine, this is just RFC to see if there's obvious broken bits, etc. also, i still need to do some split out of the pit. it and hpet are nearly identical though, so i don't noticing any issues with this.

the part i know is broken is lapic broadcast, so i'd like to fix that up too. trouble is, it's broken on vanilla too, so i'm not 100% sure what i'm debugging yet.

thanks,
-chris

#13 lore raw
Quoted 9 lines · Chris Wright
Quoted 5 lines
Quoted 2 lines

Yes, that's part of why I did some of the cleanups that way, so that we could merge the two together later.

I'd prefer to get this done now. If we plug it in as is, the "merge later" probably will never happen.

that's fine, this is just RFC to see if there's obvious broken bits, etc. also, i still need to do some split out of the pit. it and hpet are nearly identical though, so i don't noticing any issues with this.

Ok.

Quoted 3 lines

the part i know is broken is lapic broadcast, so i'd like to fix that up too. trouble is, it's broken on vanilla too, so i'm not 100% sure what i'm debugging yet.

You need to remove switch_APIC_timer_to_ipi and counterpart so that the clockevents broadcast gets control over broadcasting. This requires a change in drivers/acpi/processor_idle.c as well

commit e9e2cdb412412326c4827fc78ba27f410d837e6e has the basic changes.

tglx

#14 lore raw
Quoted 10 lines · * Thomas Gleixner ([email protected]) wrote:
Quoted 3 lines · Chris Wright

the part i know is broken is lapic broadcast, so i'd like to fix that up too. trouble is, it's broken on vanilla too, so i'm not 100% sure what i'm debugging yet.

You need to remove switch_APIC_timer_to_ipi and counterpart so that the clockevents broadcast gets control over broadcasting. This requires a change in drivers/acpi/processor_idle.c as well

commit e9e2cdb412412326c4827fc78ba27f410d837e6e has the basic changes.

Yeah, I saw that, and while I didn't remove it yet, it shouldn't be getting called from anywhere because of CONFIG_GENERIC_CLOCKEVENTS being set. In fact, acpi_propagate_timer_broadcast() does get called, and all that does it drop me back to hpet because of C3. But, I didn't add DUMMY yet, and I must've missed smth else. Some of that got a little tangled into the nice calibration cleanup, which I didn't want to suck in until I understood what I had broken ;-)

thanks,
-chris

#15 lore raw
In reply to Thomas Gleixner
Quoted 4 lines

Have you checked, if we could share the code between i386 and x86_64 at least for PIT and HPET. I'm not sure about the local APIC, but I think it might be doable as well.

Not for PIT. I don't want all the broken ancient hardware workarounds on x86-64. HPET might be doable, but would need quite some work.

-Andi

#16 lore raw
Quoted 5 lines · Andi Kleen
Quoted 3 lines

Have you checked, if we could share the code between i386 and x86_64 at least for PIT and HPET. I'm not sure about the local APIC, but I think it might be doable as well.

Not for PIT. I don't want all the broken ancient hardware workarounds on x86-64.

Err, what are you talking about ?

The clock events code in arch/i386/kernel/i8253.c does not have any quirks. It's straight forward. The only quirk is in pit_read() and we can simply make it depend on !64bit.

Quoted 1 line

HPET might be doable, but would need quite some work.

Right, but this is better spent than having two implementations of the same thing around.

tglx