evtchn: Avoid spurious event-channel notifications across unbind/bind.

Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
Signed-off-by: Huacai Chen <huacai.chen@intel.com>

7 files changed, 41 insertions, 28 deletions

diff --git a/xen/arch/ia64/xen/irq.c b/xen/arch/ia64/xen/irq.c
index c59338f390..b6cf217fb0 100644
--- a/xen/arch/ia64/xen/irq.c
+++ b/xen/arch/ia64/xen/irq.c
@@ -459,7 +459,7 @@ int pirq_guest_bind(struct vcpu *v, int irq, int will_share)
     return rc;
 }
 
-int pirq_guest_unbind(struct domain *d, int irq)
+void pirq_guest_unbind(struct domain *d, int irq)
 {
     irq_desc_t         *desc = &irq_desc[irq];
     irq_guest_action_t *action;
@@ -493,7 +493,6 @@ int pirq_guest_unbind(struct domain *d, int irq)
     }
 
     spin_unlock_irqrestore(&desc->lock, flags);    
-    return 0;
 }
 
 void
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index abd35f11f1..a9fa6ddf76 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -573,7 +573,7 @@ int pirq_guest_bind(struct vcpu *v, int irq, int will_share)
     return rc;
 }
 
-int pirq_guest_unbind(struct domain *d, int irq)
+void pirq_guest_unbind(struct domain *d, int irq)
 {
     unsigned int        vector;
     irq_desc_t         *desc;
@@ -660,7 +660,6 @@ int pirq_guest_unbind(struct domain *d, int irq)
 
  out:
     spin_unlock_irqrestore(&desc->lock, flags);    
-    return 0;
 }
 
 extern void dump_ioapic_irq_info(void);
diff --git a/xen/common/domain.c b/xen/common/domain.c
index c8c7d19bd5..6bf1f49161 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -137,6 +137,8 @@ struct vcpu *alloc_vcpu(
     v->runstate.state = is_idle_vcpu(v) ? RUNSTATE_running : RUNSTATE_offline;
     v->runstate.state_entry_time = NOW();
 
+    spin_lock_init(&v->virq_lock);
+
     if ( !is_idle_domain(d) )
     {
         set_bit(_VPF_down, &v->pause_flags);
diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 84cc455fdb..3e8956e0e7 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -386,14 +386,18 @@ static long __evtchn_close(struct domain *d1, int port1)
         break;
 
     case ECS_PIRQ:
-        if ( (rc = pirq_guest_unbind(d1, chn1->u.pirq)) == 0 )
-            d1->pirq_to_evtchn[chn1->u.pirq] = 0;
+        pirq_guest_unbind(d1, chn1->u.pirq);
+        d1->pirq_to_evtchn[chn1->u.pirq] = 0;
         break;
 
     case ECS_VIRQ:
         for_each_vcpu ( d1, v )
-            if ( v->virq_to_evtchn[chn1->u.virq] == port1 )
-                v->virq_to_evtchn[chn1->u.virq] = 0;
+        {
+            if ( v->virq_to_evtchn[chn1->u.virq] != port1 )
+                continue;
+            v->virq_to_evtchn[chn1->u.virq] = 0;
+            spin_barrier(&v->virq_lock);
+        }
         break;
 
     case ECS_IPI:
@@ -447,6 +451,9 @@ static long __evtchn_close(struct domain *d1, int port1)
         BUG();
     }
 
+    /* Clear pending event to avoid unexpected behavior on re-bind. */
+    clear_bit(port1, &shared_info(d1, evtchn_pending));
+
     /* Reset binding to vcpu0 when the channel is freed. */
     chn1->state          = ECS_FREE;
     chn1->notify_vcpu_id = 0;
@@ -573,37 +580,33 @@ static int evtchn_set_pending(struct vcpu *v, int port)
     return 0;
 }
 
+int guest_enabled_event(struct vcpu *v, int virq)
+{
+    return ((v != NULL) && (v->virq_to_evtchn[virq] != 0));
+}
 
 void send_guest_vcpu_virq(struct vcpu *v, int virq)
 {
+    unsigned long flags;
     int port;
 
     ASSERT(!virq_is_global(virq));
 
+    spin_lock_irqsave(&v->virq_lock, flags);
+
     port = v->virq_to_evtchn[virq];
     if ( unlikely(port == 0) )
-        return;
+        goto out;
 
     evtchn_set_pending(v, port);
-}
-
-int guest_enabled_event(struct vcpu *v, int virq)
-{
-    int port;
-
-    if ( unlikely(v == NULL) )
-        return 0;
 
-    port = v->virq_to_evtchn[virq];
-    if ( port == 0 )
-        return 0;
-
-    /* virq is in use */
-    return 1;
+ out:
+    spin_unlock_irqrestore(&v->virq_lock, flags);
 }
 
 void send_guest_global_virq(struct domain *d, int virq)
 {
+    unsigned long flags;
     int port;
     struct vcpu *v;
     struct evtchn *chn;
@@ -617,20 +620,28 @@ void send_guest_global_virq(struct domain *d, int virq)
     if ( unlikely(v == NULL) )
         return;
 
+    spin_lock_irqsave(&v->virq_lock, flags);
+
     port = v->virq_to_evtchn[virq];
     if ( unlikely(port == 0) )
-        return;
+        goto out;
 
     chn = evtchn_from_port(d, port);
     evtchn_set_pending(d->vcpu[chn->notify_vcpu_id], port);
-}
 
+ out:
+    spin_unlock_irqrestore(&v->virq_lock, flags);
+}
 
 int send_guest_pirq(struct domain *d, int pirq)
 {
     int port = d->pirq_to_evtchn[pirq];
     struct evtchn *chn;
 
+    /*
+     * It should not be possible to race with __evtchn_close():
+     * The caller of this function must synchronise with pirq_guest_unbind().
+     */
     ASSERT(port != 0);
 
     chn = evtchn_from_port(d, port);
diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
index 5b198d8509..bfdbe5c71a 100644
--- a/xen/include/xen/irq.h
+++ b/xen/include/xen/irq.h
@@ -78,7 +78,7 @@ struct vcpu;
 extern int pirq_guest_eoi(struct domain *d, int irq);
 extern int pirq_guest_unmask(struct domain *d);
 extern int pirq_guest_bind(struct vcpu *v, int irq, int will_share);
-extern int pirq_guest_unbind(struct domain *d, int irq);
+extern void pirq_guest_unbind(struct domain *d, int irq);
 
 static inline void set_native_irq_info(int irq, cpumask_t mask)
 {
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 9b05f0d59e..b6a3faabfa 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -137,7 +137,9 @@ struct vcpu
     unsigned long    pause_flags;
     atomic_t         pause_count;
 
+    /* IRQ-safe virq_lock protects against delivering VIRQ to stale evtchn. */
     u16              virq_to_evtchn[NR_VIRQS];
+    spinlock_t       virq_lock;
 
     /* Bitmask of CPUs on which this VCPU may run. */
     cpumask_t        cpu_affinity;
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index fa8c82f8ea..298211cb2a 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -85,8 +85,8 @@ typedef struct { int gcc_is_buggy; } rwlock_t;
 /* Ensure a lock is quiescent between two critical operations. */
 static inline void spin_barrier(spinlock_t *lock)
 {
-    spin_lock(lock);
-    spin_unlock(lock);
+    do { mb(); } while ( spin_is_locked(lock) );
+    mb();
 }
 
 #define DEFINE_SPINLOCK(x) spinlock_t x = SPIN_LOCK_UNLOCKED