[Xen-devel] [PATCH] Avoid race conditions in HPET initialization

[Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>]

Avoid turning on legacy interrupts before hpet_event has been set up.
Particularly, the spinlock can be uninitialised at the point at which
the interrupt first arrives.

Also, fix a memory leak of a cpumask in the unlikely event that
hpet_assign_irq() fails.

Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>
---
 xen/arch/x86/hpet.c |   59 +++++++++++++++++++++++++++------------------------
 1 file changed, 31 insertions(+), 28 deletions(-)

diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
index 3a4f7e8..0dedfb7 100644
--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -387,6 +387,26 @@ static int __init hpet_assign_irq(struct 
hpet_event_channel *ch)
     return 0;
 }
 
+static void __init hpet_init_channel(struct hpet_event_channel *ch)
+{
+    u64 hpet_rate = hpet_setup();
+
+    /*
+     * The period is a femto seconds value. We need to calculate the scaled
+     * math multiplication factor for nanosecond to hpet tick conversion.
+     */
+    ch->mult = div_sc((unsigned long)hpet_rate,
+                                     1000000000ul, 32);
+    ch->shift = 32;
+    ch->next_event = STIME_MAX;
+    spin_lock_init(&ch->lock);
+    ch->event_handler = handle_hpet_broadcast;
+
+    ch->msi.irq = -1;
+    ch->msi.msi_attrib.maskbit = 1;
+    ch->msi.msi_attrib.pos = MSI_TYPE_HPET;
+}
+
 static void __init hpet_fsb_cap_lookup(void)
 {
     u32 id;
@@ -423,11 +443,15 @@ static void __init hpet_fsb_cap_lookup(void)
             break;
         }
 
+        hpet_init_channel(ch);
+
         ch->flags = 0;
         ch->idx = i;
 
         if ( hpet_assign_irq(ch) == 0 )
             num_hpets_used++;
+        else
+            free_cpumask_var(ch->cpumask);
     }
 
     printk(XENLOG_INFO "HPET: %u timers usable for broadcast (%u total)\n",
@@ -553,7 +577,6 @@ void __init hpet_broadcast_init(void)
 {
     u64 hpet_rate = hpet_setup();
     u32 hpet_id, cfg;
-    unsigned int i, n;
 
     if ( hpet_rate == 0 || hpet_broadcast_is_available() )
         return;
@@ -565,7 +588,6 @@ void __init hpet_broadcast_init(void)
     {
         /* Stop HPET legacy interrupts */
         cfg &= ~HPET_CFG_LEGACY;
-        n = num_hpets_used;
     }
     else
     {
@@ -577,11 +599,10 @@ void __init hpet_broadcast_init(void)
             hpet_events = xzalloc(struct hpet_event_channel);
         if ( !hpet_events || !zalloc_cpumask_var(&hpet_events->cpumask) )
             return;
-        hpet_events->msi.irq = -1;
+        hpet_init_channel(hpet_events);
 
         /* Start HPET legacy interrupts */
         cfg |= HPET_CFG_LEGACY;
-        n = 1;
 
         if ( !force_hpet_broadcast )
             pv_rtc_handler = handle_rtc_once;
@@ -589,31 +610,13 @@ void __init hpet_broadcast_init(void)
 
     hpet_write32(cfg, HPET_CFG);
 
-    for ( i = 0; i < n; i++ )
+    if ( cfg & HPET_CFG_LEGACY )
     {
-        if ( i == 0 && (cfg & HPET_CFG_LEGACY) )
-        {
-            /* set HPET T0 as oneshot */
-            cfg = hpet_read32(HPET_Tn_CFG(0));
-            cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC);
-            cfg |= HPET_TN_ENABLE | HPET_TN_32BIT;
-            hpet_write32(cfg, HPET_Tn_CFG(0));
-        }
-
-        /*
-         * The period is a femto seconds value. We need to calculate the scaled
-         * math multiplication factor for nanosecond to hpet tick conversion.
-         */
-        hpet_events[i].mult = div_sc((unsigned long)hpet_rate,
-                                     1000000000ul, 32);
-        hpet_events[i].shift = 32;
-        hpet_events[i].next_event = STIME_MAX;
-        spin_lock_init(&hpet_events[i].lock);
-        wmb();
-        hpet_events[i].event_handler = handle_hpet_broadcast;
-
-        hpet_events[i].msi.msi_attrib.maskbit = 1;
-        hpet_events[i].msi.msi_attrib.pos = MSI_TYPE_HPET;
+        /* set HPET T0 as oneshot */
+        cfg = hpet_read32(HPET_Tn_CFG(0));
+        cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC);
+        cfg |= HPET_TN_ENABLE | HPET_TN_32BIT;
+        hpet_write32(cfg, HPET_Tn_CFG(0));
     }
 
     if ( !num_hpets_used )
-- 
1.7.10.4



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel