[PATCH v2 2/10] x86/boot: add setup_secondary_clock to x86 quirk

[PATCH v2 2/10] x86/boot: add setup_secondary_clock to x86 quirk

[Pan, Jacob jun]

>From 8d2ba963b3f352ee478fda838422425aaed0a97a Mon Sep 17 00:00:00 2001
From: Jacob Pan <jacob.jun.pan@intel.com>
Date: Tue, 9 Jun 2009 17:02:48 -0700
Subject: [PATCH] x86/boot: add setup_secondary_clock to x86 quirk

Some x86 platform such as Intel Moorestown use external timers for per
cpu clockevent devices. This quirk is added such as we can override the
default local APIC timers if such platform is detected.

Should not have impact on other platforms that uses default or platform
specific x86_quirks.

Signed-off-by: Jacob Pan <jacob.jun.pan@intel.com>
---
 arch/x86/include/asm/setup.h |    1 +
 arch/x86/kernel/smpboot.c    |   15 ++++++++++++++-
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index 4093d1e..7bf325a 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -31,6 +31,7 @@ struct x86_quirks {
 	void (*smp_read_mpc_oem)(struct mpc_oemtable *oemtable,
 				unsigned short oemsize);
 	int (*setup_ioapic_ids)(void);
+	void (*setup_secondary_clock)(void);
 };
 
 extern void x86_quirk_intr_init(void);
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 2fecda6..3260159 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -66,6 +66,8 @@
 #include <linux/mc146818rtc.h>
 
 #include <asm/smpboot_hooks.h>
+#include <asm/platform_feature.h>
+#include <asm/apb_timer.h>
 
 #ifdef CONFIG_X86_32
 u8 apicid_2_node[MAX_APICID];
@@ -263,6 +265,17 @@ static void __cpuinit smp_callin(void)
 }
 
 /*
+ * Setup secondary clock
+ */
+notrace static void __cpuinit __setup_secondary_clock(void)
+{
+	if (x86_quirks->setup_secondary_clock)
+		x86_quirks->setup_secondary_clock();
+	else
+		setup_secondary_clock();
+}
+
+/*
  * Activate a secondary processor.
  */
 notrace static void __cpuinit start_secondary(void *unused)
@@ -323,7 +336,7 @@ notrace static void __cpuinit start_secondary(void *unused)
 	/* enable local interrupts */
 	local_irq_enable();
 
-	setup_secondary_clock();
+	__setup_secondary_clock();
 
 	wmb();
 	cpu_idle();
-- 
1.5.6.5

Re: [PATCH v2 2/10] x86/boot: add setup_secondary_clock to x86 quirk

[Thomas Gleixner]

Jacob,

On Thu, 16 Jul 2009, Pan, Jacob jun wrote:

> >From 8d2ba963b3f352ee478fda838422425aaed0a97a Mon Sep 17 00:00:00 2001
> From: Jacob Pan <jacob.jun.pan@intel.com>
> Date: Tue, 9 Jun 2009 17:02:48 -0700
> Subject: [PATCH] x86/boot: add setup_secondary_clock to x86 quirk
> 
> Some x86 platform such as Intel Moorestown use external timers for per
> cpu clockevent devices. This quirk is added such as we can override the
> default local APIC timers if such platform is detected.
> 
> Should not have impact on other platforms that uses default or platform
> specific x86_quirks.

That patch is only half of the story. What about the apic timer setup
of the primary CPU ?

Also we already have paravirt ops overriding the clock setup for boot
and secondary cpus. Your patch adds another level of indirection.
That's just a nightmare.

The correct approach is to clean up the clock setup in general and get
rid of the paravirt indirection. There is no reason why paravirt
guests should not use the same infrastructure as we need for platform
support.

I need to think more about how it should look, but we definitely want
something which is of general use and does not just add extra
conditinal stuff.

Thanks,

	tglx