x86/MSI: fix error handling

__setup_msi_irq() needs to undo what it did before calling
write_msi_msg() in case that returned an error.

map_domain_pirq() needs to get rid of the MSI descriptor it
(implicitly) allocated. The case of a setup_msi_irq() failure on a
non-initial multi-vector-MSI interrupt needs special handling: While
the initial IRQ will get freed by the caller (who also passed it to
us), we need to undo the effect setup_msi_irq() had on it. (As a
benefit from the added call to msi_free_irq() we no longer need to
explicitly call destroy_irq() on the non-initial slots.)

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 29c1b7886c36d4e6aa03a779b2251b829d9689c3
master date: 2015-03-26 11:19:57 +0100

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index f21407240513d2e85f89d42c68a1787bad821c6a..84738e53d3d9e4c248bcb0be9420a9a00681d816 100644 (file)
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1973,6 +1973,8 @@ int map_domain_pirq(
             dprintk(XENLOG_G_ERR, "dom%d: irq %d in use\n",
                     d->domain_id, irq);
             pci_disable_msi(msi_desc);
+            msi_desc->irq = -1;
+            msi_free_irq(msi_desc);
             ret = -EBUSY;
             goto done;
         }
@@ -2027,22 +2029,29 @@ int map_domain_pirq(
         if ( ret )
         {
             spin_unlock_irqrestore(&desc->lock, flags);
+            pci_disable_msi(msi_desc);
+            if ( nr )
+            {
+                ASSERT(msi_desc->irq >= 0);
+                desc = irq_to_desc(msi_desc->irq);
+                spin_lock_irqsave(&desc->lock, flags);
+                desc->handler = &no_irq_type;
+                desc->msi_desc = NULL;
+                spin_unlock_irqrestore(&desc->lock, flags);
+            }
             while ( nr-- )
             {
-                if ( irq >= 0 )
-                {
-                    if ( irq_deny_access(d, irq) )
-                        printk(XENLOG_G_ERR
-                               "dom%d: could not revoke access to IRQ%d (pirq %d)\n",
-                               d->domain_id, irq, pirq);
-                    destroy_irq(irq);
-                }
+                if ( irq >= 0 && irq_deny_access(d, irq) )
+                    printk(XENLOG_G_ERR
+                           "dom%d: could not revoke access to IRQ%d (pirq %d)\n",
+                           d->domain_id, irq, pirq);
                 if ( info )
                     cleanup_domain_irq_pirq(d, irq, info);
                 info = pirq_info(d, pirq + nr);
                 irq = info->arch.irq;
             }
-            pci_disable_msi(msi_desc);
+            msi_desc->irq = -1;
+            msi_free_irq(msi_desc);
             goto done;
         }
 

diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 14d37eceb53a42c9fb9b126b844ecb0bde7b7219..7410d0371312b66fef79bb9140610e99b97a7397 100644 (file)
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -470,6 +470,7 @@ static struct msi_desc *alloc_msi_entry(unsigned int nr)
     while ( nr-- )
     {
         entry[nr].dev = NULL;
+        entry[nr].irq = -1;
         entry[nr].remap_index = -1;
     }
 
@@ -487,11 +488,19 @@ int __setup_msi_irq(struct irq_desc *desc, struct msi_desc *msidesc,
                     hw_irq_controller *handler)
 {
     struct msi_msg msg;
+    int ret;
 
     desc->msi_desc = msidesc;
     desc->handler = handler;
     msi_compose_msg(desc->arch.vector, desc->arch.cpu_mask, &msg);
-    return write_msi_msg(msidesc, &msg);
+    ret = write_msi_msg(msidesc, &msg);
+    if ( unlikely(ret) )
+    {
+        desc->handler = &no_irq_type;
+        desc->msi_desc = NULL;
+    }
+
+    return ret;
 }
 
 int msi_free_irq(struct msi_desc *entry)
@@ -501,7 +510,8 @@ int msi_free_irq(struct msi_desc *entry)
 
     while ( nr-- )
     {
-        destroy_irq(entry[nr].irq);
+        if ( entry[nr].irq >= 0 )
+            destroy_irq(entry[nr].irq);
 
         /* Free the unused IRTE if intr remap enabled */
         if ( iommu_intremap )