Fix up the way we initialise and set the NMI heartbeat

timers. Also, compund updates to APIC ICR and ICR2 registers
must be done with interrupts disabled to ensure local
atomicity.

Signed-off-by: Keir Fraser <keir@xensource.com>

1 files changed, 23 insertions, 42 deletions

diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
index f8c34a1c27..96c55572cd 100644
--- a/xen/arch/x86/nmi.c
+++ b/xen/arch/x86/nmi.c
@@ -33,10 +33,6 @@
 #include <asm/apic.h>
 
 unsigned int nmi_watchdog = NMI_NONE;
-static spinlock_t   watchdog_lock = SPIN_LOCK_UNLOCKED;
-static unsigned int watchdog_disable_count = 1;
-static unsigned int watchdog_on;
-
 static unsigned int nmi_hz = HZ;
 static unsigned int nmi_perfctr_msr;	/* the MSR to reset in NMI handler */
 static unsigned int nmi_p4_cccr_val;
@@ -314,18 +310,8 @@ static int __pminit setup_p4_watchdog(void)
 
 void __pminit setup_apic_nmi_watchdog(void)
 {
-    int cpu = smp_processor_id();
-
-    if ( nmi_active < 0 )
-	return;
-
-    if ( !nmi_watchdog )
-    {
-	/* Force the watchdog to always be disabled. */
-	watchdog_disable_count++;
-	nmi_active = -1;
-	return;
-    }
+    if (!nmi_watchdog)
+        return;
 
     switch (boot_cpu_data.x86_vendor) {
     case X86_VENDOR_AMD:
@@ -358,45 +344,37 @@ void __pminit setup_apic_nmi_watchdog(void)
 
     lapic_nmi_owner = LAPIC_NMI_WATCHDOG;
     nmi_active = 1;
-
-    init_timer(&nmi_timer[cpu], nmi_timer_fn, NULL, cpu);
 }
 
 static unsigned int
 last_irq_sums [NR_CPUS],
     alert_counter [NR_CPUS];
 
+static atomic_t watchdog_disable_count = ATOMIC_INIT(1);
+
 void watchdog_disable(void)
 {
-    unsigned long flags;
-
-    spin_lock_irqsave(&watchdog_lock, flags);
-
-    if ( watchdog_disable_count++ == 0 )
-        watchdog_on = 0;
-
-    spin_unlock_irqrestore(&watchdog_lock, flags);
+    atomic_inc(&watchdog_disable_count);
 }
 
 void watchdog_enable(void)
 {
-    unsigned int  cpu;
-    unsigned long flags;
+    static unsigned long heartbeat_initialised;
+    unsigned int cpu;
 
-    spin_lock_irqsave(&watchdog_lock, flags);
+    if ( !atomic_dec_and_test(&watchdog_disable_count) ||
+         test_and_set_bit(0, &heartbeat_initialised) )
+        return;
 
-    if ( --watchdog_disable_count == 0 )
+    /*
+     * Activate periodic heartbeats. We cannot do this earlier during 
+     * setup because the timer infrastructure is not available.
+     */
+    for_each_online_cpu ( cpu )
     {
-        watchdog_on = 1;
-        /*
-         * Ensure periodic heartbeats are active. We cannot do this earlier
-         * during setup because the timer infrastructure is not available. 
-         */
-        for_each_online_cpu ( cpu )
-            set_timer(&nmi_timer[cpu], NOW());
+        init_timer(&nmi_timer[cpu], nmi_timer_fn, NULL, cpu);
+        set_timer(&nmi_timer[cpu], NOW());
     }
-
-    spin_unlock_irqrestore(&watchdog_lock, flags);
 }
 
 void nmi_watchdog_tick(struct cpu_user_regs * regs)
@@ -405,7 +383,7 @@ void nmi_watchdog_tick(struct cpu_user_regs * regs)
 
     sum = nmi_timer_ticks[cpu];
 
-    if ( (last_irq_sums[cpu] == sum) && watchdog_on )
+    if ( (last_irq_sums[cpu] == sum) && !atomic_read(&watchdog_disable_count) )
     {
         /*
          * Ayiee, looks like this CPU is stuck ... wait a few IRQs (5 seconds) 
@@ -457,14 +435,17 @@ void nmi_watchdog_tick(struct cpu_user_regs * regs)
  * 8-3 and 8-4 in IA32 Reference Manual Volume 3. We send the IPI to
  * our own APIC ID explicitly which is valid.
  */
-static void do_nmi_trigger(unsigned char key) {
+static void do_nmi_trigger(unsigned char key)
+{
     u32 id = apic_read(APIC_ID);
 
-    printk("triggering NMI on APIC ID %x\n", id);
+    printk("Triggering NMI on APIC ID %x\n", id);
 
+    local_irq_disable();
     apic_wait_icr_idle();
     apic_write_around(APIC_ICR2, SET_APIC_DEST_FIELD(id));
     apic_write_around(APIC_ICR, APIC_DM_NMI | APIC_INT_ASSERT);
+    local_irq_enable();
 }
 
 static __init int register_nmi_trigger(void)