Re: [PATCH 5/11] Fix PIT override bug for paravirt

[Zachary Amsden <zach@vmware.com>]

[-- Attachment #1: Type: text/plain, Size: 611 bytes --]

Andi Kleen wrote:
> On Mon, Feb 05, 2007 at 07:53:02PM -0800, Zachary Amsden wrote:
>   
>> The time initialization changed for i386 when some code moved into time_init.
>> This made it no longer possible to override the PIT / HPET, which broke
>> paravirt guests.
>>     
>
> Looks still fragile. Can this be done cleaner? 
>
> In particularly some comments would be good, but clearer code 
> logic would be preferred.
>
> -Andi
>   

Well, the logic is still overly complicated, but I axed the ifdef and 
commented it.  It is difficult to work around the number of different 
compile and boot sequences here.

[-- Attachment #2: pit-override --]
[-- Type: text/plain, Size: 2091 bytes --]

diff -r 31c3528be5d2 arch/i386/kernel/paravirt.c
--- a/arch/i386/kernel/paravirt.c	Fri Feb 02 16:29:46 2007 -0800
+++ b/arch/i386/kernel/paravirt.c	Tue Feb 06 16:16:54 2007 -0800
@@ -497,7 +497,7 @@ struct paravirt_ops paravirt_ops = {
 	.memory_setup = machine_specific_memory_setup,
 	.get_wallclock = native_get_wallclock,
 	.set_wallclock = native_set_wallclock,
-	.time_init = time_init_hook,
+	.time_init = native_time_init,
 	.init_IRQ = native_init_IRQ,
 
 	.cpuid = native_cpuid,
diff -r 31c3528be5d2 arch/i386/kernel/time.c
--- a/arch/i386/kernel/time.c	Fri Feb 02 16:29:46 2007 -0800
+++ b/arch/i386/kernel/time.c	Tue Feb 06 16:27:06 2007 -0800
@@ -266,10 +266,24 @@ static void __init hpet_time_init(void)
 {
 	if (!hpet_enable())
 		setup_pit_timer();
+	time_init_hook();
+}
+
+void __init native_time_init(void)
+{
+	late_time_init = hpet_time_init;
+}
+
+/*
+ * This is called directly from init code; we must delay timer setup in the
+ * HPET case as we can't make the decision to turn on HPET this early in the
+ * boot process.  So we call do_time_init(), which either calls
+ * native_time_init above or a paravirt-op which implements an alternative
+ * timer.  This may seem like overkill, but is necessary for the rather large
+ * number of timer combinations.  But remember, this is all dealing with time,
+ * and with time, too much is never enough.
+ */
+void __init time_init(void)
+{
 	do_time_init();
 }
-
-void __init time_init(void)
-{
-	late_time_init = hpet_time_init;
-}
diff -r 31c3528be5d2 include/asm-i386/time.h
--- a/include/asm-i386/time.h	Fri Feb 02 16:29:46 2007 -0800
+++ b/include/asm-i386/time.h	Tue Feb 06 16:21:05 2007 -0800
@@ -28,13 +28,15 @@ static inline int native_set_wallclock(u
 	return retval;
 }
 
+extern void native_time_init(void);
+
 #ifdef CONFIG_PARAVIRT
 #include <asm/paravirt.h>
 #else /* !CONFIG_PARAVIRT */
 
 #define get_wallclock() native_get_wallclock()
 #define set_wallclock(x) native_set_wallclock(x)
-#define do_time_init() time_init_hook()
+#define do_time_init() native_time_init()
 
 #endif /* CONFIG_PARAVIRT */