Xen devel - Mar 2011 - [PATCH 1/4] x86/HPET: fix initialization order

At least the legacy path can enter its interrupt handler callout while
initialization is still in progress - that handler checks whether
->event_handler is non-NULL, and hence all other initialization must
happen before setting this field.

Do the same to the MSI initialization just in case (and to keep the
code in sync).

Signed-off-by: Jan Beulich <jbeulich@novell.com>

--- 2011-03-09.orig/xen/arch/x86/hpet.c
+++ 2011-03-09/xen/arch/x86/hpet.c
@@ -597,9 +597,10 @@ void hpet_broadcast_init(void)
                                          1000000000ul, 32);
             hpet_events[i].shift = 32;
             hpet_events[i].next_event = STIME_MAX;
-            hpet_events[i].event_handler = handle_hpet_broadcast;
             spin_lock_init(&hpet_events[i].lock);
             rwlock_init(&hpet_events[i].cpumask_lock);
+            wmb();
+            hpet_events[i].event_handler = handle_hpet_broadcast;
         }
 
         return;
@@ -630,11 +631,12 @@ void hpet_broadcast_init(void)
     legacy_hpet_event.mult = div_sc((unsigned long)hpet_rate, 1000000000ul,
32);
     legacy_hpet_event.shift = 32;
     legacy_hpet_event.next_event = STIME_MAX;
-    legacy_hpet_event.event_handler = handle_hpet_broadcast;
     legacy_hpet_event.idx = 0;
     legacy_hpet_event.flags = 0;
     spin_lock_init(&legacy_hpet_event.lock);
     rwlock_init(&legacy_hpet_event.cpumask_lock);
+    wmb();
+    legacy_hpet_event.event_handler = handle_hpet_broadcast;
 
     if ( !force_hpet_broadcast )
         pv_rtc_handler = handle_rtc_once;





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jan Beulich wrote on 2011-03-11:
> At least the legacy path can enter its interrupt handler callout while 
> initialization is still in progress - that handler checks whether
> ->event_handler is non-NULL, and hence all other initialization must
> happen before setting this field.
> 
> Do the same to the MSI initialization just in case (and to keep the code in
sync).
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
Acked-by: Wei Gang <gang.wei@intel.com>

The only bogus case for old code should be: one PIT interrupt came to a non-boot
cpu before the boot cpu changes HPET to legacy mode, and this PIT interrupt was
pending until boot cpu initialized the .event_handler with the spin lock &
rw lock uninitialized. But it is still a possible bug. I support make the fix go
into 4.1 and 4.0.

Jimmy

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel