evtchn: Avoid spurious event-channel notifications across unbind/bind.
authorKeir Fraser <keir.fraser@citrix.com>
Thu, 31 Jul 2008 10:13:30 +0000 (11:13 +0100)
committerKeir Fraser <keir.fraser@citrix.com>
Thu, 31 Jul 2008 10:13:30 +0000 (11:13 +0100)
Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
Signed-off-by: Huacai Chen <huacai.chen@intel.com>
xen/arch/ia64/xen/irq.c
xen/arch/x86/irq.c
xen/common/domain.c
xen/common/event_channel.c
xen/include/xen/irq.h
xen/include/xen/sched.h
xen/include/xen/spinlock.h

index c59338f390b293db3fecbec3524b2a1d7c8fda66..b6cf217fb07837b5477ab69e522bf6c2ed617360 100644 (file)
@@ -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
index abd35f11f19f10cced13f4fb806011c288bb1e45..a9fa6ddf76daeceb629c985e8ceddb0205365326 100644 (file)
@@ -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);
index c8c7d19bd5e146e384a5fc16e57d14ad34a0174d..6bf1f49161c429b9aa257c5862d86f87ef1179c5 100644 (file)
@@ -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);
index 84cc455fdbddf3b4ceb5df30d33708e23d7d2cf2..3e8956e0e74edc0e689a7612bc76e714568fe1ad 100644 (file)
@@ -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);
index 5b198d85090214df1a83741523fc65bb45af8f25..bfdbe5c71a3288696c61f693b1941d5a5a685eea 100644 (file)
@@ -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)
 {
index 9b05f0d59e7e31663fb8319091844cb52d1f6369..b6a3faabfadd87b98e1762daaedc04c604c683fe 100644 (file)
@@ -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;
index fa8c82f8ea4b8044004ca3322a6cc736d378cf9b..298211cb2a754bd6680240e89c824dfc41b67e76 100644 (file)
@@ -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