x86: Properly synchronise updates to pirq-to-vector mapping.

Per-domain irq mappings are now protected by d->evtchn_lock and by the
per-vector irq_desc lock.

Signed-off-by: Jan Beulich <jbeulich@novell.com>
Signed-off-by: Keir Fraser <keir.fraser@citrix.com>

11 files changed, 133 insertions, 102 deletions

diff --git a/xen/arch/ia64/xen/irq.c b/xen/arch/ia64/xen/irq.c
index 0f951552eb..156bd8a55b 100644
--- a/xen/arch/ia64/xen/irq.c
+++ b/xen/arch/ia64/xen/irq.c
@@ -459,20 +459,24 @@ int pirq_guest_bind(struct vcpu *v, int irq, int will_share)
     return rc;
 }
 
-void pirq_guest_unbind(struct domain *d, int irq)
+int pirq_guest_unbind(struct domain *d, int irq)
 {
     irq_desc_t         *desc = &irq_desc[irq];
     irq_guest_action_t *action;
     unsigned long       flags;
-    int                 i;
+    int                 i, rc = 0;
 
     spin_lock_irqsave(&desc->lock, flags);
 
     action = (irq_guest_action_t *)desc->action;
 
-    i = 0;
-    while ( action->guest[i] && (action->guest[i] != d) )
-        i++;
+    for ( i = 0; (i < action->nr_guests) && (action->guest[i] != d); i++ )
+        continue;
+    if ( i == action->nr_guests )
+    {
+        rc = -EINVAL;
+        goto out;
+    }
     memmove(&action->guest[i], &action->guest[i+1], IRQ_MAX_GUESTS-i-1);
     action->nr_guests--;
 
@@ -492,7 +496,9 @@ void pirq_guest_unbind(struct domain *d, int irq)
         desc->handler->shutdown(irq);
     }
 
+ out:
     spin_unlock_irqrestore(&desc->lock, flags);    
+    return rc;
 }
 
 void
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 431e27d725..3d322f6da2 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -414,8 +414,6 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
             goto fail;
     }
 
-    spin_lock_init(&d->arch.irq_lock);
-
     if ( is_hvm_domain(d) )
     {
         if ( (rc = hvm_domain_initialise(d)) != 0 )
diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index bd197dccc3..c96ede9e89 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -45,16 +45,6 @@
 int (*ioapic_renumber_irq)(int ioapic, int irq);
 atomic_t irq_mis_count;
 
-int domain_irq_to_vector(struct domain *d, int irq)
-{
-    return d->arch.pirq_vector[irq];
-}
-
-int domain_vector_to_irq(struct domain *d, int vector)
-{
-    return d->arch.vector_pirq[vector];
-}
-
 /* Where if anywhere is the i8259 connect in external int mode */
 static struct { int pin, apic; } ioapic_i8259 = { -1, -1 };
 
@@ -721,7 +711,6 @@ next:
 
 static struct hw_interrupt_type ioapic_level_type;
 static struct hw_interrupt_type ioapic_edge_type;
-struct hw_interrupt_type pci_msi_type;
 
 #define IOAPIC_AUTO	-1
 #define IOAPIC_EDGE	0
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 0d741abe21..e22ccec856 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -277,6 +277,35 @@ static void __do_IRQ_guest(int vector)
     }
 }
 
+/*
+ * Retrieve Xen irq-descriptor corresponding to a domain-specific irq.
+ * The descriptor is returned locked. This function is safe against changes
+ * to the per-domain irq-to-vector mapping.
+ */
+static irq_desc_t *domain_spin_lock_irq_desc(
+    struct domain *d, int irq, unsigned long *pflags)
+{
+    unsigned int vector;
+    unsigned long flags;
+    irq_desc_t *desc;
+
+    for ( ; ; )
+    {
+        vector = domain_irq_to_vector(d, irq);
+        if ( vector <= 0 )
+            return NULL;
+        desc = &irq_desc[vector];
+        spin_lock_irqsave(&desc->lock, flags);
+        if ( vector == domain_irq_to_vector(d, irq) )
+            break;
+        spin_unlock_irqrestore(&desc->lock, flags);
+    }
+
+    if ( pflags != NULL )
+        *pflags = flags;
+    return desc;
+}
+
 /* Flush all ready EOIs from the top of this CPU's pending-EOI stack. */
 static void flush_ready_eoi(void *unused)
 {
@@ -342,11 +371,13 @@ static void __pirq_guest_eoi(struct domain *d, int irq)
     cpumask_t           cpu_eoi_map;
     int                 vector;
 
-    vector = domain_irq_to_vector(d, irq);
-    desc   = &irq_desc[vector];
-    action = (irq_guest_action_t *)desc->action;
+    ASSERT(local_irq_is_enabled());
+    desc = domain_spin_lock_irq_desc(d, irq, NULL);
+    if ( desc == NULL )
+        return;
 
-    spin_lock_irq(&desc->lock);
+    action = (irq_guest_action_t *)desc->action;
+    vector = desc - irq_desc;
 
     ASSERT(!test_bit(irq, d->pirq_mask) ||
            (action->ack_type != ACKTYPE_NONE));
@@ -418,7 +449,7 @@ int pirq_acktype(struct domain *d, int irq)
     unsigned int vector;
 
     vector = domain_irq_to_vector(d, irq);
-    if ( vector == 0 )
+    if ( vector <= 0 )
         return ACKTYPE_NONE;
 
     desc = &irq_desc[vector];
@@ -447,13 +478,6 @@ int pirq_acktype(struct domain *d, int irq)
     if ( !strcmp(desc->handler->typename, "XT-PIC") )
         return ACKTYPE_UNMASK;
 
-    if ( strstr(desc->handler->typename, "MPIC") )
-    {
-        if ( desc->status & IRQ_LEVEL )
-            return (desc->status & IRQ_PER_CPU) ? ACKTYPE_EOI : ACKTYPE_UNMASK;
-        return ACKTYPE_NONE; /* edge-triggered => no final EOI */
-    }
-
     printk("Unknown PIC type '%s' for IRQ %d\n", desc->handler->typename, irq);
     BUG();
 
@@ -462,21 +486,18 @@ int pirq_acktype(struct domain *d, int irq)
 
 int pirq_shared(struct domain *d, int irq)
 {
-    unsigned int        vector;
     irq_desc_t         *desc;
     irq_guest_action_t *action;
     unsigned long       flags;
     int                 shared;
 
-    vector = domain_irq_to_vector(d, irq);
-    if ( vector == 0 )
+    desc = domain_spin_lock_irq_desc(d, irq, &flags);
+    if ( desc == NULL )
         return 0;
 
-    desc = &irq_desc[vector];
-
-    spin_lock_irqsave(&desc->lock, flags);
     action = (irq_guest_action_t *)desc->action;
     shared = ((desc->status & IRQ_GUEST) && (action->nr_guests > 1));
+
     spin_unlock_irqrestore(&desc->lock, flags);
 
     return shared;
@@ -491,16 +512,15 @@ int pirq_guest_bind(struct vcpu *v, int irq, int will_share)
     int                 rc = 0;
     cpumask_t           cpumask = CPU_MASK_NONE;
 
+    WARN_ON(!spin_is_locked(&v->domain->evtchn_lock));
+
  retry:
-    vector = domain_irq_to_vector(v->domain, irq);
-    if ( vector == 0 )
+    desc = domain_spin_lock_irq_desc(v->domain, irq, &flags);
+    if ( desc == NULL )
         return -EINVAL;
 
-    desc = &irq_desc[vector];
-
-    spin_lock_irqsave(&desc->lock, flags);
-
     action = (irq_guest_action_t *)desc->action;
+    vector = desc - irq_desc;
 
     if ( !(desc->status & IRQ_GUEST) )
     {
@@ -575,26 +595,39 @@ int pirq_guest_bind(struct vcpu *v, int irq, int will_share)
     return rc;
 }
 
-void pirq_guest_unbind(struct domain *d, int irq)
+int pirq_guest_unbind(struct domain *d, int irq)
 {
-    unsigned int        vector;
+    int                 vector;
     irq_desc_t         *desc;
     irq_guest_action_t *action;
     cpumask_t           cpu_eoi_map;
     unsigned long       flags;
-    int                 i;
+    int                 i, rc = 0;
 
-    vector = domain_irq_to_vector(d, irq);
-    desc = &irq_desc[vector];
-    BUG_ON(vector == 0);
+    WARN_ON(!spin_is_locked(&d->evtchn_lock));
 
-    spin_lock_irqsave(&desc->lock, flags);
+    desc = domain_spin_lock_irq_desc(d, irq, &flags);
+    if ( unlikely(desc == NULL) )
+    {
+        if ( (vector = -domain_irq_to_vector(d, irq)) == 0 )
+            return -EINVAL;
+        BUG_ON(vector <= 0);
+        desc = &irq_desc[vector];
+        spin_lock_irqsave(&desc->lock, flags);
+        d->arch.pirq_vector[irq] = d->arch.vector_pirq[vector] = 0;
+        goto out;
+    }
 
     action = (irq_guest_action_t *)desc->action;
+    vector = desc - irq_desc;
 
-    i = 0;
-    while ( action->guest[i] && (action->guest[i] != d) )
-        i++;
+    for ( i = 0; (i < action->nr_guests) && (action->guest[i] != d); i++ )
+        continue;
+    if ( i == action->nr_guests )
+    {
+        rc = -EINVAL;
+        goto out;
+    }
     memmove(&action->guest[i], &action->guest[i+1], IRQ_MAX_GUESTS-i-1);
     action->nr_guests--;
 
@@ -661,7 +694,8 @@ void pirq_guest_unbind(struct domain *d, int irq)
     desc->handler->shutdown(vector);
 
  out:
-    spin_unlock_irqrestore(&desc->lock, flags);    
+    spin_unlock_irqrestore(&desc->lock, flags);
+    return rc;
 }
 
 extern void dump_ioapic_irq_info(void);
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index cf5339e8e5..5a87e30037 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -727,7 +727,6 @@ void pci_disable_msi(int vector)
         __pci_disable_msix(vector);
 }
 
-extern struct hw_interrupt_type pci_msi_type;
 static void msi_free_vectors(struct pci_dev* dev)
 {
     struct msi_desc *entry, *tmp;
diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index eff6438368..18e6b26279 100644
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -26,17 +26,11 @@ int
 ioapic_guest_write(
     unsigned long physbase, unsigned int reg, u32 pval);
 
-
-extern struct hw_interrupt_type pci_msi_type;
-
 static int get_free_pirq(struct domain *d, int type, int index)
 {
     int i;
 
-    if ( d == NULL )
-        return -EINVAL;
-
-    ASSERT(spin_is_locked(&d->arch.irq_lock));
+    ASSERT(spin_is_locked(&d->evtchn_lock));
 
     if ( type == MAP_PIRQ_TYPE_GSI )
     {
@@ -64,11 +58,10 @@ static int map_domain_pirq(struct domain *d, int pirq, int vector,
     int ret = 0;
     int old_vector, old_pirq;
     struct msi_info msi;
+    irq_desc_t *desc;
+    unsigned long flags;
 
-    if ( d == NULL )
-        return -EINVAL;
-
-    ASSERT(spin_is_locked(&d->arch.irq_lock));
+    ASSERT(spin_is_locked(&d->evtchn_lock));
 
     if ( !IS_PRIV(current->domain) )
         return -EPERM;
@@ -88,8 +81,7 @@ static int map_domain_pirq(struct domain *d, int pirq, int vector,
     {
         dprintk(XENLOG_G_ERR, "dom%d: pirq %d or vector %d already mapped\n",
                 d->domain_id, pirq, vector);
-        ret = -EINVAL;
-        goto done;
+        return -EINVAL;
     }
 
     ret = irq_permit_access(d, pirq);
@@ -97,17 +89,14 @@ static int map_domain_pirq(struct domain *d, int pirq, int vector,
     {
         dprintk(XENLOG_G_ERR, "dom%d: could not permit access to irq %d\n",
                 d->domain_id, pirq);
-        goto done;
+        return ret;
     }
 
+    desc = &irq_desc[vector];
+    spin_lock_irqsave(&desc->lock, flags);
+
     if ( map && MAP_PIRQ_TYPE_MSI == map->type )
     {
-        irq_desc_t         *desc;
-        unsigned long flags;
-
-        desc = &irq_desc[vector];
-
-        spin_lock_irqsave(&desc->lock, flags);
         if ( desc->handler != &no_irq_type )
             dprintk(XENLOG_G_ERR, "dom%d: vector %d in use\n",
                     d->domain_id, vector);
@@ -120,8 +109,6 @@ static int map_domain_pirq(struct domain *d, int pirq, int vector,
         msi.vector = vector;
 
         ret = pci_enable_msi(&msi);
-
-        spin_unlock_irqrestore(&desc->lock, flags);
         if ( ret )
             goto done;
     }
@@ -130,6 +117,7 @@ static int map_domain_pirq(struct domain *d, int pirq, int vector,
     d->arch.vector_pirq[vector] = pirq;
 
 done:
+    spin_unlock_irqrestore(&desc->lock, flags);
     return ret;
 }
 
@@ -139,18 +127,18 @@ static int unmap_domain_pirq(struct domain *d, int pirq)
     unsigned long flags;
     irq_desc_t *desc;
     int vector, ret = 0;
+    bool_t forced_unbind;
 
-    if ( d == NULL || pirq < 0 || pirq >= NR_PIRQS )
+    if ( (pirq < 0) || (pirq >= NR_PIRQS) )
         return -EINVAL;
 
     if ( !IS_PRIV(current->domain) )
         return -EINVAL;
 
-    ASSERT(spin_is_locked(&d->arch.irq_lock));
+    ASSERT(spin_is_locked(&d->evtchn_lock));
 
     vector = d->arch.pirq_vector[pirq];
-
-    if ( !vector )
+    if ( vector <= 0 )
     {
         dprintk(XENLOG_G_ERR, "dom%d: pirq %d not mapped\n",
                 d->domain_id, pirq);
@@ -158,20 +146,34 @@ static int unmap_domain_pirq(struct domain *d, int pirq)
         goto done;
     }
 
+    forced_unbind = (pirq_guest_unbind(d, pirq) == 0);
+    if ( forced_unbind )
+        dprintk(XENLOG_G_WARNING, "dom%d: forcing unbind of pirq %d\n",
+                d->domain_id, pirq);
+
     desc = &irq_desc[vector];
     spin_lock_irqsave(&desc->lock, flags);
+
+    BUG_ON(vector != d->arch.pirq_vector[pirq]);
+
     if ( desc->msi_desc )
         pci_disable_msi(vector);
 
     if ( desc->handler == &pci_msi_type )
+        desc->handler = &no_irq_type;
+
+    if ( !forced_unbind )
     {
-        /* MSI is not shared, so should be released already */
-        BUG_ON(desc->status & IRQ_GUEST);
-        irq_desc[vector].handler = &no_irq_type;
+        d->arch.pirq_vector[pirq] = 0;
+        d->arch.vector_pirq[vector] = 0;
+    }
+    else
+    {
+        d->arch.pirq_vector[pirq] = -vector;
+        d->arch.vector_pirq[vector] = -pirq;
     }
-    spin_unlock_irqrestore(&desc->lock, flags);
 
-    d->arch.pirq_vector[pirq] = d->arch.vector_pirq[vector] = 0;
+    spin_unlock_irqrestore(&desc->lock, flags);
 
     ret = irq_deny_access(d, pirq);
     if ( ret )
@@ -186,7 +188,6 @@ static int physdev_map_pirq(struct physdev_map_pirq *map)
 {
     struct domain *d;
     int vector, pirq, ret = 0;
-    unsigned long flags;
 
     if ( !IS_PRIV(current->domain) )
         return -EPERM;
@@ -243,8 +244,8 @@ static int physdev_map_pirq(struct physdev_map_pirq *map)
             goto free_domain;
     }
 
-    spin_lock_irqsave(&d->arch.irq_lock, flags);
-    if ( map->pirq == -1 )
+    spin_lock(&d->evtchn_lock);
+    if ( map->pirq < 0 )
     {
         if ( d->arch.vector_pirq[vector] )
         {
@@ -252,6 +253,11 @@ static int physdev_map_pirq(struct physdev_map_pirq *map)
                     d->domain_id, map->index, map->pirq,
                     d->arch.vector_pirq[vector]);
             pirq = d->arch.vector_pirq[vector];
+            if ( pirq < 0 )
+            {
+                ret = -EBUSY;
+                goto done;
+            }
         }
         else
         {
@@ -284,7 +290,7 @@ static int physdev_map_pirq(struct physdev_map_pirq *map)
     if ( !ret )
         map->pirq = pirq;
 done:
-    spin_unlock_irqrestore(&d->arch.irq_lock, flags);
+    spin_unlock(&d->evtchn_lock);
 free_domain:
     rcu_unlock_domain(d);
     return ret;
@@ -293,7 +299,6 @@ free_domain:
 static int physdev_unmap_pirq(struct physdev_unmap_pirq *unmap)
 {
     struct domain *d;
-    unsigned long flags;
     int ret;
 
     if ( !IS_PRIV(current->domain) )
@@ -307,9 +312,9 @@ static int physdev_unmap_pirq(struct physdev_unmap_pirq *unmap)
     if ( d == NULL )
         return -ESRCH;
 
-    spin_lock_irqsave(&d->arch.irq_lock, flags);
+    spin_lock(&d->evtchn_lock);
     ret = unmap_domain_pirq(d, unmap->pirq);
-    spin_unlock_irqrestore(&d->arch.irq_lock, flags);
+    spin_unlock(&d->evtchn_lock);
 
     rcu_unlock_domain(d);
 
@@ -416,7 +421,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
 
     case PHYSDEVOP_alloc_irq_vector: {
         struct physdev_irq irq_op;
-        unsigned long flags;
 
         ret = -EFAULT;
         if ( copy_from_guest(&irq_op, arg, 1) != 0 )
@@ -437,9 +441,9 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
 
         irq_op.vector = assign_irq_vector(irq);
 
-        spin_lock_irqsave(&dom0->arch.irq_lock, flags);
+        spin_lock(&dom0->evtchn_lock);
         ret = map_domain_pirq(dom0, irq_op.irq, irq_op.vector, NULL);
-        spin_unlock_irqrestore(&dom0->arch.irq_lock, flags);
+        spin_unlock(&dom0->evtchn_lock);
 
         if ( copy_to_guest(arg, &irq_op, 1) != 0 )
             ret = -EFAULT;
diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 006d5eca3a..f7a0e87bc7 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -387,7 +387,8 @@ static long __evtchn_close(struct domain *d1, int port1)
         break;
 
     case ECS_PIRQ:
-        pirq_guest_unbind(d1, chn1->u.pirq);
+        if ( pirq_guest_unbind(d1, chn1->u.pirq) != 0 )
+            BUG();
         d1->pirq_to_evtchn[chn1->u.pirq] = 0;
         break;
 
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index e49bedbfe7..08c3c7765b 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -235,7 +235,7 @@ struct arch_domain
     /* Shadow translated domain: P2M mapping */
     pagetable_t phys_table;
 
-    spinlock_t irq_lock;
+    /* NB. protected by d->evtchn_lock and by irq_desc[vector].lock */
     int vector_pirq[NR_VECTORS];
     int pirq_vector[NR_PIRQS];
 
diff --git a/xen/include/asm-x86/irq.h b/xen/include/asm-x86/irq.h
index 8858b4eb95..692823d0b1 100644
--- a/xen/include/asm-x86/irq.h
+++ b/xen/include/asm-x86/irq.h
@@ -52,6 +52,7 @@ extern atomic_t irq_mis_count;
 int pirq_acktype(struct domain *d, int irq);
 int pirq_shared(struct domain *d , int irq);
 
-extern int domain_irq_to_vector(struct domain *d, int irq);
-extern int domain_vector_to_irq(struct domain *d, int vector);
+#define domain_irq_to_vector(d, irq) ((d)->arch.pirq_vector[(irq)])
+#define domain_vector_to_irq(d, vec) ((d)->arch.vector_pirq[(vec)])
+
 #endif /* _ASM_HW_IRQ_H */
diff --git a/xen/include/asm-x86/msi.h b/xen/include/asm-x86/msi.h
index 6aaffc90d5..a72f348e53 100644
--- a/xen/include/asm-x86/msi.h
+++ b/xen/include/asm-x86/msi.h
@@ -106,7 +106,7 @@ struct msi_desc {
  */
 #define NR_HP_RESERVED_VECTORS 	20
 
-extern int vector_irq[NR_VECTORS];
+extern struct hw_interrupt_type pci_msi_type;
 
 /*
  * MSI-X Address Register
diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
index bfdbe5c71a..7f0acb52dd 100644
--- a/xen/include/xen/irq.h
+++ b/xen/include/xen/irq.h
@@ -22,7 +22,6 @@ struct irqaction
 #define IRQ_PENDING	4	/* IRQ pending - replay on enable */
 #define IRQ_REPLAY	8	/* IRQ has been replayed but not acked yet */
 #define IRQ_GUEST       16      /* IRQ is handled by guest OS(es) */
-#define IRQ_LEVEL       64      /* IRQ level triggered */
 #define IRQ_PER_CPU     256     /* IRQ is per CPU */
 
 /*
@@ -78,7 +77,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 void pirq_guest_unbind(struct domain *d, int irq);
+extern int pirq_guest_unbind(struct domain *d, int irq);
 
 static inline void set_native_irq_info(int irq, cpumask_t mask)
 {