Re: [Xen-devel] [PATCH] IOMMU: generalize and correct softirq processing during Dom0 device setup

["Zhang, Xiantao" <xiantao.zhang@xxxxxxxxx>]

Jan, thanks, agree with you here! 
Acked-by: Xiantao Zhang <xiantao.zhang@xxxxxxxxx>

-----Original Message-----
From: Jan Beulich [mailto:JBeulich@xxxxxxxx] 
Sent: Thursday, February 13, 2014 4:58 PM
To: xen-devel
Cc: Zhang, Xiantao
Subject: [PATCH] IOMMU: generalize and correct softirq processing during Dom0 
device setup

c/s 21039:95f5a4ce8f24 ("VT-d: reduce default verbosity") having put a call to 
process_pending_softirqs() in VT-d's domain_context_mapping() was wrong in two 
ways: For one we shouldn't be doing this when setting up a device during DomU 
assignment. And then - I didn't check whether that was the case already back 
then - we shouldn't call that function with the pcidevs_lock (or in fact any 
spin lock) held.

Move the "preemption" into generic code, at once dealing with further actual 
(too much output elsewhere - particularly on systems with very many host bridge 
like devices - having been observed to still cause the watchdog to trigger when 
enabled) and potential (other IOMMU code may also end up being too verbose) 
issues.

Do the "preemption" once per device actually being set up when in verbose mode, 
and once per bus otherwise.

Note that dropping pcidevs_lock around the process_pending_softirqs() 
invocation is specifically not a problem here: We're in an __init function and 
aren't racing with potential additions/removals of PCI devices. Not acquiring 
the lock in setup_dom0_pci_devices() otoh is not an option, as there are too 
many places that assert the lock being held.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -27,6 +27,7 @@
 #include <xen/delay.h>
 #include <xen/keyhandler.h>
 #include <xen/radix-tree.h>
+#include <xen/softirq.h>
 #include <xen/tasklet.h>
 #include <xsm/xsm.h>
 #include <asm/msi.h>
@@ -922,6 +923,20 @@ static int __init _setup_dom0_pci_device
                 printk(XENLOG_WARNING "Dom%d owning %04x:%02x:%02x.%u?\n",
                        pdev->domain->domain_id, pseg->nr, bus,
                        PCI_SLOT(devfn), PCI_FUNC(devfn));
+
+            if ( iommu_verbose )
+            {
+                spin_unlock(&pcidevs_lock);
+                process_pending_softirqs();
+                spin_lock(&pcidevs_lock);
+            }
+        }
+
+        if ( !iommu_verbose )
+        {
+            spin_unlock(&pcidevs_lock);
+            process_pending_softirqs();
+            spin_lock(&pcidevs_lock);
         }
     }
 
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -31,7 +31,6 @@
 #include <xen/pci.h>
 #include <xen/pci_regs.h>
 #include <xen/keyhandler.h>
-#include <xen/softirq.h>
 #include <asm/msi.h>
 #include <asm/irq.h>
 #include <asm/hvm/vmx/vmx.h>
@@ -1484,9 +1483,6 @@ static int domain_context_mapping(
         break;
     }
 
-    if ( iommu_verbose )
-        process_pending_softirqs();
-
     return ret;
 }
 




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel