WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH] passthrough: fix some spinlock issues in vmsi

To: xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-devel] [PATCH] passthrough: fix some spinlock issues in vmsi
From: Kouya Shimura <kouya@xxxxxxxxxxxxxx>
Date: Tue, 10 Mar 2009 13:51:46 +0900
Cc: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>, Qing He <qing.he@xxxxxxxxx>
Delivery-date: Mon, 09 Mar 2009 21:52:16 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Apart from efficiency, I hasten to fix the assertion failure.

- acquire pcidevs_lock before calling pt_irq_xxx_bind_vtd
- allocate msixtbl_entry beforehand
- check return value from domain_spin_lock_irq_desc()
- typo: spin_unlock(&irq_desc->lock) -> spin_unlock_irq(&irq_desc->lock)
- acquire msixtbl_list_lock with irq_disabled

Signed-off-by: Kouya Shimura <kouya@xxxxxxxxxxxxxx>

diff -r d035b66b5b4d xen/arch/x86/domctl.c
--- a/xen/arch/x86/domctl.c     Mon Mar 09 15:01:34 2009 +0000
+++ b/xen/arch/x86/domctl.c     Tue Mar 10 11:31:28 2009 +0900
@@ -764,7 +764,11 @@ long arch_do_domctl(
 
         ret = -ESRCH;
         if ( iommu_enabled )
+        {
+            spin_lock(&pcidevs_lock);
             ret = pt_irq_create_bind_vtd(d, bind);
+            spin_unlock(&pcidevs_lock);
+        }
         if ( ret < 0 )
             gdprintk(XENLOG_ERR, "pt_irq_create_bind failed!\n");
 
@@ -783,7 +787,11 @@ long arch_do_domctl(
             break;
         bind = &(domctl->u.bind_pt_irq);
         if ( iommu_enabled )
+        {
+            spin_lock(&pcidevs_lock);
             ret = pt_irq_destroy_bind_vtd(d, bind);
+            spin_unlock(&pcidevs_lock);
+        }
         if ( ret < 0 )
             gdprintk(XENLOG_ERR, "pt_irq_destroy_bind failed!\n");
         rcu_unlock_domain(d);
diff -r d035b66b5b4d xen/arch/x86/hvm/vmsi.c
--- a/xen/arch/x86/hvm/vmsi.c   Mon Mar 09 15:01:34 2009 +0000
+++ b/xen/arch/x86/hvm/vmsi.c   Tue Mar 10 11:31:28 2009 +0900
@@ -336,16 +336,12 @@ struct hvm_mmio_handler msixtbl_mmio_han
     .write_handler = msixtbl_write
 };
 
-static struct msixtbl_entry *add_msixtbl_entry(struct domain *d,
-                                               struct pci_dev *pdev,
-                                               uint64_t gtable)
+static void add_msixtbl_entry(struct domain *d,
+                              struct pci_dev *pdev,
+                              uint64_t gtable,
+                              struct msixtbl_entry *entry)
 {
-    struct msixtbl_entry *entry;
     u32 len;
-
-    entry = xmalloc(struct msixtbl_entry);
-    if ( !entry )
-        return NULL;
 
     memset(entry, 0, sizeof(struct msixtbl_entry));
         
@@ -359,8 +355,6 @@ static struct msixtbl_entry *add_msixtbl
     entry->gtable = (unsigned long) gtable;
 
     list_add_rcu(&entry->list, &d->arch.hvm_domain.msixtbl_list);
-
-    return entry;
 }
 
 static void free_msixtbl_entry(struct rcu_head *rcu)
@@ -383,12 +377,25 @@ int msixtbl_pt_register(struct domain *d
     irq_desc_t *irq_desc;
     struct msi_desc *msi_desc;
     struct pci_dev *pdev;
-    struct msixtbl_entry *entry;
+    struct msixtbl_entry *entry, *new_entry;
     int r = -EINVAL;
 
     ASSERT(spin_is_locked(&pcidevs_lock));
 
+    /*
+     * xmalloc() with irq_disabled causes the failure of check_lock() 
+     * for xenpool->lock. So we allocate an entry beforehand.
+     */
+    new_entry = xmalloc(struct msixtbl_entry);
+    if ( !new_entry )
+        return -ENOMEM;
+
     irq_desc = domain_spin_lock_irq_desc(d, pirq, NULL);
+    if ( !irq_desc )
+    {
+        xfree(new_entry);
+        return r;
+    }
 
     if ( irq_desc->handler != &pci_msi_type )
         goto out;
@@ -405,12 +412,9 @@ int msixtbl_pt_register(struct domain *d
         if ( pdev == entry->pdev )
             goto found;
 
-    entry = add_msixtbl_entry(d, pdev, gtable);
-    if ( !entry )
-    {
-        spin_unlock(&d->arch.hvm_domain.msixtbl_list_lock);
-        goto out;
-    }
+    entry = new_entry;
+    new_entry = NULL;
+    add_msixtbl_entry(d, pdev, gtable, entry);
 
 found:
     atomic_inc(&entry->refcnt);
@@ -419,8 +423,8 @@ found:
 
 out:
     spin_unlock_irq(&irq_desc->lock);
+    xfree(new_entry);
     return r;
-
 }
 
 void msixtbl_pt_unregister(struct domain *d, int pirq)
@@ -433,6 +437,8 @@ void msixtbl_pt_unregister(struct domain
     ASSERT(spin_is_locked(&pcidevs_lock));
 
     irq_desc = domain_spin_lock_irq_desc(d, pirq, NULL);
+    if ( !irq_desc )
+        return;
 
     if ( irq_desc->handler != &pci_msi_type )
         goto out;
@@ -453,7 +459,7 @@ void msixtbl_pt_unregister(struct domain
 
 
 out:
-    spin_unlock(&irq_desc->lock);
+    spin_unlock_irq(&irq_desc->lock);
     return;
 
 found:
@@ -461,13 +467,16 @@ found:
         del_msixtbl_entry(entry);
 
     spin_unlock(&d->arch.hvm_domain.msixtbl_list_lock);
-    spin_unlock(&irq_desc->lock);
+    spin_unlock_irq(&irq_desc->lock);
 }
 
 void msixtbl_pt_cleanup(struct domain *d, int pirq)
 {
     struct msixtbl_entry *entry, *temp;
+    unsigned long flags;
 
+    /* msixtbl_list_lock must be acquired with irq_disabled for check_lock() */
+    local_irq_save(flags); 
     spin_lock(&d->arch.hvm_domain.msixtbl_list_lock);
 
     list_for_each_entry_safe( entry, temp,
@@ -475,4 +484,5 @@ void msixtbl_pt_cleanup(struct domain *d
         del_msixtbl_entry(entry);
 
     spin_unlock(&d->arch.hvm_domain.msixtbl_list_lock);
+    local_irq_restore(flags);
 }
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [PATCH] passthrough: fix some spinlock issues in vmsi, Kouya Shimura <=