Support late binding of PCI devices to the PCI backend driver in

dom0. This allows you to bind devices to the backend driver *after*
dom0 has booted. You are no longer required to specify the devices to
hide on the kernel command-line. Using the bind/unbind driver
attributes (see /sys/bus/pci/drivers/pciback), you can specify which
devices that the PCI backend will seize. There are three new driver
attributes in that directory:

slots       - lists all of the PCI slots that
              the PCI backend will try to seize
new_slot    - write the name of a slot here
              (in 0000:00:00.0 format) to have the PCI
              Backend seize the device in this slot
remove_slot - write the name of a slot here to have
              the PCI Backend no longer try to seize a
              device in this slot

Note that writing to new_slot/remove_slot does not actually change
whether the PCI Backend is actually bound to the device in that slot or
not. Instead, it tells the PCI backend which slots it should be
interested in. The sysfs attributes "bind" and "unbind" (which are
common to all drivers, not just the PCI Backend) must be used to
actually add or remove a device from the PCI backend driver. Note that
the syntax for specifying a device to bind and unbind is very strict (do
not append a newline).

For Example:

# Add a new slot to the PCI Backend's list
echo -n 0000:01:04.d > /sys/bus/pci/drivers/pciback/new_slot
# Now that the backend is watching for the slot, bind to it
echo -n 0000:01:04.d > /sys/bus/pci/drivers/pciback/bind

# Unbind a PCI network card from its network driver
echo -n 0000:05:02.0 > /sys/bus/pci/drivers/3c905/unbind
# And now bind it to the PCI Backend
echo -n 0000:05:02.0 > /sys/bus/pci/drivers/pciback/new_slot
echo -n 0000:05:02.0 > /sys/bus/pci/drivers/pciback/bind


Unfortunately, Linux makes it possible to remove (unbind) a PCI device
from the PCI backend while that device is attached to a driver domain.
It is also possible to unload the PCI Backend module while a PCI
Frontend is attached. DON'T DO EITHER OF THESE ACTIONS. This patch will
output warnings if you do try and do these. Be aware that while access
to the configuration space of the device has been revoked, the driver
domain can still access the I/O resources of the device as they have not
been revoked (although I *hope* to explore adding support for this
soon). Before unloading the module or unbinding a device, shutdown your
driver domain.

These patches also convert a few function and variable declarations to
static (no sense in polluting the global namespace with local function
names) and rename a few structures in drivers/xen/pciback/pci_stub.c.

Signed-off-by: Ryan Wilson <hap9@epoch.ncsc.mil>

7 files changed, 525 insertions, 145 deletions

diff --git a/linux-2.6-xen-sparse/drivers/xen/pciback/passthrough.c b/linux-2.6-xen-sparse/drivers/xen/pciback/passthrough.c
index e5b7fbbec3..5ee76381cd 100644
--- a/linux-2.6-xen-sparse/drivers/xen/pciback/passthrough.c
+++ b/linux-2.6-xen-sparse/drivers/xen/pciback/passthrough.c
@@ -7,10 +7,13 @@
 
 #include <linux/list.h>
 #include <linux/pci.h>
+#include <linux/spinlock.h>
 #include "pciback.h"
 
 struct passthrough_dev_data {
+	/* Access to dev_list must be protected by lock */
 	struct list_head dev_list;
+	spinlock_t lock;
 };
 
 struct pci_dev *pciback_get_pci_dev(struct pciback_device *pdev,
@@ -19,33 +22,66 @@ struct pci_dev *pciback_get_pci_dev(struct pciback_device *pdev,
 {
 	struct passthrough_dev_data *dev_data = pdev->pci_dev_data;
 	struct pci_dev_entry *dev_entry;
+	struct pci_dev *dev = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev_data->lock, flags);
 
 	list_for_each_entry(dev_entry, &dev_data->dev_list, list) {
 		if (domain == (unsigned int)pci_domain_nr(dev_entry->dev->bus)
 		    && bus == (unsigned int)dev_entry->dev->bus->number
-		    && devfn == dev_entry->dev->devfn)
-			return dev_entry->dev;
+		    && devfn == dev_entry->dev->devfn) {
+			dev = dev_entry->dev;
+			break;
+		}
 	}
 
-	return NULL;
+	spin_unlock_irqrestore(&dev_data->lock, flags);
+
+	return dev;
 }
 
-/* Must hold pciback_device->dev_lock when calling this */
 int pciback_add_pci_dev(struct pciback_device *pdev, struct pci_dev *dev)
 {
 	struct passthrough_dev_data *dev_data = pdev->pci_dev_data;
 	struct pci_dev_entry *dev_entry;
+	unsigned long flags;
 
 	dev_entry = kmalloc(sizeof(*dev_entry), GFP_KERNEL);
 	if (!dev_entry)
 		return -ENOMEM;
 	dev_entry->dev = dev;
 
+	spin_lock_irqsave(&dev_data->lock, flags);
 	list_add_tail(&dev_entry->list, &dev_data->dev_list);
+	spin_unlock_irqrestore(&dev_data->lock, flags);
 
 	return 0;
 }
 
+void pciback_release_pci_dev(struct pciback_device *pdev, struct pci_dev *dev)
+{
+	struct passthrough_dev_data *dev_data = pdev->pci_dev_data;
+	struct pci_dev_entry *dev_entry, *t;
+	struct pci_dev *found_dev = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev_data->lock, flags);
+
+	list_for_each_entry_safe(dev_entry, t, &dev_data->dev_list, list) {
+		if (dev_entry->dev == dev) {
+			list_del(&dev_entry->list);
+			found_dev = dev_entry->dev;
+			kfree(dev_entry);
+		}
+	}
+
+	spin_unlock_irqrestore(&dev_data->lock, flags);
+
+	if (found_dev)
+		pcistub_put_pci_dev(found_dev);
+}
+
 int pciback_init_devices(struct pciback_device *pdev)
 {
 	struct passthrough_dev_data *dev_data;
@@ -54,6 +90,8 @@ int pciback_init_devices(struct pciback_device *pdev)
 	if (!dev_data)
 		return -ENOMEM;
 
+	spin_lock_init(&dev_data->lock);
+
 	INIT_LIST_HEAD(&dev_data->dev_list);
 
 	pdev->pci_dev_data = dev_data;
@@ -71,6 +109,8 @@ int pciback_publish_pci_roots(struct pciback_device *pdev,
 	int found;
 	unsigned int domain, bus;
 
+	spin_lock(&dev_data->lock);
+
 	list_for_each_entry(dev_entry, &dev_data->dev_list, list) {
 		/* Only publish this device as a root if none of its
 		 * parent bridges are exported
@@ -96,10 +136,11 @@ int pciback_publish_pci_roots(struct pciback_device *pdev,
 		}
 	}
 
+	spin_unlock(&dev_data->lock);
+
 	return err;
 }
 
-/* Must hold pciback_device->dev_lock when calling this */
 void pciback_release_devices(struct pciback_device *pdev)
 {
 	struct passthrough_dev_data *dev_data = pdev->pci_dev_data;
diff --git a/linux-2.6-xen-sparse/drivers/xen/pciback/pci_stub.c b/linux-2.6-xen-sparse/drivers/xen/pciback/pci_stub.c
index f3c6f019e0..e0dc110fb7 100644
--- a/linux-2.6-xen-sparse/drivers/xen/pciback/pci_stub.c
+++ b/linux-2.6-xen-sparse/drivers/xen/pciback/pci_stub.c
@@ -7,110 +7,190 @@
 #include <linux/init.h>
 #include <linux/list.h>
 #include <linux/spinlock.h>
+#include <linux/kref.h>
 #include <asm/atomic.h>
 #include "pciback.h"
 
 static char *pci_devs_to_hide = NULL;
 module_param_named(hide, pci_devs_to_hide, charp, 0444);
 
-struct pci_stub_device_id {
+struct pcistub_device_id {
 	struct list_head slot_list;
 	int domain;
 	unsigned char bus;
 	unsigned int devfn;
 };
-LIST_HEAD(pci_stub_device_ids);
+static LIST_HEAD(pcistub_device_ids);
+static DEFINE_SPINLOCK(device_ids_lock);
 
-struct pci_stub_device {
+struct pcistub_device {
+	struct kref kref;
 	struct list_head dev_list;
+	spinlock_t lock;
+
 	struct pci_dev *dev;
-	atomic_t in_use;
+	struct pciback_device *pdev;	/* non-NULL if struct pci_dev is in use */
 };
-/* Access to pci_stub_devices & seized_devices lists and the initialize_devices
- * flag must be locked with pci_stub_devices_lock
+/* Access to pcistub_devices & seized_devices lists and the initialize_devices
+ * flag must be locked with pcistub_devices_lock
  */
-DEFINE_SPINLOCK(pci_stub_devices_lock);
-LIST_HEAD(pci_stub_devices);
+static DEFINE_SPINLOCK(pcistub_devices_lock);
+static LIST_HEAD(pcistub_devices);
 
 /* wait for device_initcall before initializing our devices
  * (see pcistub_init_devices_late)
  */
 static int initialize_devices = 0;
-LIST_HEAD(seized_devices);
+static LIST_HEAD(seized_devices);
 
-static inline struct pci_dev *get_pci_dev(struct pci_stub_device *psdev)
+static struct pcistub_device *pcistub_device_alloc(struct pci_dev *dev)
 {
-	if (atomic_dec_and_test(&psdev->in_use))
-		return psdev->dev;
-	else {
-		atomic_inc(&psdev->in_use);
+	struct pcistub_device *psdev;
+
+	dev_dbg(&dev->dev, "pcistub_device_alloc\n");
+
+	psdev = kzalloc(sizeof(*psdev), GFP_ATOMIC);
+	if (!psdev)
+		return NULL;
+
+	psdev->dev = pci_dev_get(dev);
+	if (!psdev->dev) {
+		kfree(psdev);
 		return NULL;
 	}
+
+	kref_init(&psdev->kref);
+	spin_lock_init(&psdev->lock);
+
+	return psdev;
+}
+
+/* Don't call this directly as it's called by pcistub_device_put */
+static void pcistub_device_release(struct kref *kref)
+{
+	struct pcistub_device *psdev;
+
+	psdev = container_of(kref, struct pcistub_device, kref);
+
+	dev_dbg(&psdev->dev->dev, "pcistub_device_release\n");
+
+	/* Clean-up the device */
+	pciback_reset_device(psdev->dev);
+	pciback_config_free(psdev->dev);
+	kfree(pci_get_drvdata(psdev->dev));
+	pci_set_drvdata(psdev->dev, NULL);
+
+	pci_dev_put(psdev->dev);
+
+	kfree(psdev);
+}
+
+static inline void pcistub_device_get(struct pcistub_device *psdev)
+{
+	kref_get(&psdev->kref);
 }
 
-struct pci_dev *pcistub_get_pci_dev_by_slot(int domain, int bus,
+static inline void pcistub_device_put(struct pcistub_device *psdev)
+{
+	kref_put(&psdev->kref, pcistub_device_release);
+}
+
+static struct pci_dev *pcistub_device_get_pci_dev(struct pciback_device *pdev,
+						  struct pcistub_device *psdev)
+{
+	struct pci_dev *pci_dev = NULL;
+	unsigned long flags;
+
+	pcistub_device_get(psdev);
+
+	spin_lock_irqsave(&psdev->lock, flags);
+	if (!psdev->pdev) {
+		psdev->pdev = pdev;
+		pci_dev = psdev->dev;
+	}
+	spin_unlock_irqrestore(&psdev->lock, flags);
+
+	if (!pci_dev)
+		pcistub_device_put(psdev);
+
+	return pci_dev;
+}
+
+struct pci_dev *pcistub_get_pci_dev_by_slot(struct pciback_device *pdev,
+					    int domain, int bus,
 					    int slot, int func)
 {
-	struct pci_stub_device *psdev;
+	struct pcistub_device *psdev;
 	struct pci_dev *found_dev = NULL;
+	unsigned long flags;
 
-	spin_lock(&pci_stub_devices_lock);
+	spin_lock_irqsave(&pcistub_devices_lock, flags);
 
-	list_for_each_entry(psdev, &pci_stub_devices, dev_list) {
+	list_for_each_entry(psdev, &pcistub_devices, dev_list) {
 		if (psdev->dev != NULL
 		    && domain == pci_domain_nr(psdev->dev->bus)
 		    && bus == psdev->dev->bus->number
 		    && PCI_DEVFN(slot, func) == psdev->dev->devfn) {
-			found_dev = get_pci_dev(psdev);
+			found_dev = pcistub_device_get_pci_dev(pdev, psdev);
 			break;
 		}
 	}
 
-	spin_unlock(&pci_stub_devices_lock);
+	spin_unlock_irqrestore(&pcistub_devices_lock, flags);
 	return found_dev;
 }
 
-struct pci_dev *pcistub_get_pci_dev(struct pci_dev *dev)
+struct pci_dev *pcistub_get_pci_dev(struct pciback_device *pdev,
+				    struct pci_dev *dev)
 {
-	struct pci_stub_device *psdev;
+	struct pcistub_device *psdev;
 	struct pci_dev *found_dev = NULL;
+	unsigned long flags;
 
-	spin_lock(&pci_stub_devices_lock);
+	spin_lock_irqsave(&pcistub_devices_lock, flags);
 
-	list_for_each_entry(psdev, &pci_stub_devices, dev_list) {
+	list_for_each_entry(psdev, &pcistub_devices, dev_list) {
 		if (psdev->dev == dev) {
-			found_dev = get_pci_dev(psdev);
+			found_dev = pcistub_device_get_pci_dev(pdev, psdev);
 			break;
 		}
 	}
 
-	spin_unlock(&pci_stub_devices_lock);
+	spin_unlock_irqrestore(&pcistub_devices_lock, flags);
 	return found_dev;
 }
 
 void pcistub_put_pci_dev(struct pci_dev *dev)
 {
-	struct pci_stub_device *psdev;
+	struct pcistub_device *psdev, *found_psdev = NULL;
+	unsigned long flags;
 
-	spin_lock(&pci_stub_devices_lock);
+	spin_lock_irqsave(&pcistub_devices_lock, flags);
 
-	list_for_each_entry(psdev, &pci_stub_devices, dev_list) {
+	list_for_each_entry(psdev, &pcistub_devices, dev_list) {
 		if (psdev->dev == dev) {
-			/* Cleanup our device
-			 * (so it's ready for the next domain)
-			 */
-			pciback_reset_device(psdev->dev);
-
-			atomic_inc(&psdev->in_use);
+			found_psdev = psdev;
 			break;
 		}
 	}
 
-	spin_unlock(&pci_stub_devices_lock);
+	spin_unlock_irqrestore(&pcistub_devices_lock, flags);
+
+	/* Cleanup our device
+	 * (so it's ready for the next domain)
+	 */
+	pciback_reset_device(found_psdev->dev);
+	pciback_config_reset(found_psdev->dev);
+
+	spin_lock_irqsave(&found_psdev->lock, flags);
+	found_psdev->pdev = NULL;
+	spin_unlock_irqrestore(&found_psdev->lock, flags);
+
+	pcistub_device_put(found_psdev);
 }
 
-static int __devinit pcistub_match(struct pci_dev *dev,
-				   struct pci_stub_device_id *pdev_id)
+static int __devinit pcistub_match_one(struct pci_dev *dev,
+				       struct pcistub_device_id *pdev_id)
 {
 	/* Match the specified device by domain, bus, slot, func and also if
 	 * any of the device's parent bridges match.
@@ -125,23 +205,44 @@ static int __devinit pcistub_match(struct pci_dev *dev,
 	return 0;
 }
 
+static int __devinit pcistub_match(struct pci_dev *dev)
+{
+	struct pcistub_device_id *pdev_id;
+	unsigned long flags;
+	int found = 0;
+
+	spin_lock_irqsave(&device_ids_lock, flags);
+	list_for_each_entry(pdev_id, &pcistub_device_ids, slot_list) {
+		if (pcistub_match_one(dev, pdev_id)) {
+			found = 1;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&device_ids_lock, flags);
+
+	return found;
+}
+
 static int __devinit pcistub_init_device(struct pci_dev *dev)
 {
 	struct pciback_dev_data *dev_data;
 	int err = 0;
 
+	dev_dbg(&dev->dev, "initializing...\n");
+
 	/* The PCI backend is not intended to be a module (or to work with
 	 * removable PCI devices (yet). If it were, pciback_config_free()
 	 * would need to be called somewhere to free the memory allocated
 	 * here and then to call kfree(pci_get_drvdata(psdev->dev)).
 	 */
-	dev_data = kmalloc(sizeof(*dev_data), GFP_KERNEL);
+	dev_data = kmalloc(sizeof(*dev_data), GFP_ATOMIC);
 	if (!dev_data) {
 		err = -ENOMEM;
 		goto out;
 	}
 	pci_set_drvdata(dev, dev_data);
 
+	dev_dbg(&dev->dev, "initializing config\n");
 	err = pciback_config_init(dev);
 	if (err)
 		goto out;
@@ -153,14 +254,15 @@ static int __devinit pcistub_init_device(struct pci_dev *dev)
 	 * This makes the assumption that the device's resources won't
 	 * change after this point (otherwise this code may break!)
 	 */
+	dev_dbg(&dev->dev, "enabling device\n");
 	err = pci_enable_device(dev);
 	if (err)
 		goto config_release;
 
 	/* Now disable the device (this also ensures some private device
 	 * data is setup before we export)
-	 * This calls pciback_config_reset(dev)
 	 */
+	dev_dbg(&dev->dev, "reset device\n");
 	pciback_reset_device(dev);
 
 	return 0;
@@ -182,60 +284,82 @@ static int __devinit pcistub_init_device(struct pci_dev *dev)
  */
 static int __init pcistub_init_devices_late(void)
 {
-	struct pci_stub_device *psdev, *t;
+	struct pcistub_device *psdev;
+	unsigned long flags;
 	int err = 0;
 
-	spin_lock(&pci_stub_devices_lock);
+	pr_debug("pciback: pcistub_init_devices_late\n");
+
+	spin_lock_irqsave(&pcistub_devices_lock, flags);
 
-	list_for_each_entry_safe(psdev, t, &seized_devices, dev_list) {
+	while (!list_empty(&seized_devices)) {
+		psdev = container_of(seized_devices.next,
+				     struct pcistub_device, dev_list);
 		list_del(&psdev->dev_list);
+
+		spin_unlock_irqrestore(&pcistub_devices_lock, flags);
+
 		err = pcistub_init_device(psdev->dev);
 		if (err) {
-			printk(KERN_ERR
-			       "pciback: %s error %d initializing device\n",
-			       pci_name(psdev->dev), err);
+			dev_err(&psdev->dev->dev,
+				"error %d initializing device\n", err);
 			kfree(psdev);
-			continue;
+			psdev = NULL;
 		}
 
-		list_add_tail(&psdev->dev_list, &pci_stub_devices);
+		spin_lock_irqsave(&pcistub_devices_lock, flags);
+
+		if (psdev)
+			list_add_tail(&psdev->dev_list, &pcistub_devices);
 	}
 
 	initialize_devices = 1;
 
-	spin_unlock(&pci_stub_devices_lock);
+	spin_unlock_irqrestore(&pcistub_devices_lock, flags);
 
 	return 0;
 }
 
 static int __devinit pcistub_seize(struct pci_dev *dev)
 {
-	struct pci_stub_device *psdev;
+	struct pcistub_device *psdev;
+	unsigned long flags;
+	int initialize_devices_copy;
 	int err = 0;
 
-	psdev = kmalloc(sizeof(*psdev), GFP_KERNEL);
+	psdev = pcistub_device_alloc(dev);
 	if (!psdev)
 		return -ENOMEM;
 
-	psdev->dev = dev;
-	atomic_set(&psdev->in_use, 1);
+	/* initialize_devices has to be accessed under a spin lock. But since
+	 * it can only change from 0 -> 1, if it's already 1, we don't have to
+	 * worry about it changing. That's why we can take a *copy* of
+	 * initialize_devices and wait till we're outside of the lock to
+	 * check if it's 1 (don't ever check if it's 0 outside of the lock)
+	 */
+	spin_lock_irqsave(&pcistub_devices_lock, flags);
+
+	initialize_devices_copy = initialize_devices;
+
+	if (!initialize_devices_copy) {
+		dev_dbg(&dev->dev, "deferring initialization\n");
+		list_add(&psdev->dev_list, &seized_devices);
+	}
 
-	spin_lock(&pci_stub_devices_lock);
+	spin_unlock_irqrestore(&pcistub_devices_lock, flags);
 
-	if (initialize_devices) {
+	if (initialize_devices_copy) {
+		/* don't want irqs disabled when calling pcistub_init_device */
 		err = pcistub_init_device(psdev->dev);
 		if (err)
 			goto out;
 
-		list_add(&psdev->dev_list, &pci_stub_devices);
-	} else
-		list_add(&psdev->dev_list, &seized_devices);
+		list_add(&psdev->dev_list, &pcistub_devices);
+	}
 
       out:
-	spin_unlock(&pci_stub_devices_lock);
-
 	if (err)
-		kfree(psdev);
+		pcistub_device_put(psdev);
 
 	return err;
 }
@@ -243,47 +367,78 @@ static int __devinit pcistub_seize(struct pci_dev *dev)
 static int __devinit pcistub_probe(struct pci_dev *dev,
 				   const struct pci_device_id *id)
 {
-	struct pci_stub_device_id *pdev_id;
-	struct pci_dev *seized_dev;
 	int err = 0;
 
-	list_for_each_entry(pdev_id, &pci_stub_device_ids, slot_list) {
+	dev_dbg(&dev->dev, "probing...\n");
 
-		if (!pcistub_match(dev, pdev_id))
-			continue;
+	if (pcistub_match(dev)) {
 
 		if (dev->hdr_type != PCI_HEADER_TYPE_NORMAL
 		    && dev->hdr_type != PCI_HEADER_TYPE_BRIDGE) {
-			printk(KERN_ERR
-			       "pciback: %s: can't export pci devices that "
-			       "don't have a normal (0) or bridge (1) "
-			       "header type!\n", pci_name(dev));
-			break;
+			dev_err(&dev->dev, "can't export pci devices that "
+				"don't have a normal (0) or bridge (1) "
+				"header type!\n");
+			err = -ENODEV;
+			goto out;
 		}
 
-		pr_info("pciback: seizing PCI device %s\n", pci_name(dev));
-		seized_dev = pci_dev_get(dev);
+		dev_info(&dev->dev, "seizing device\n");
+		err = pcistub_seize(dev);
+	} else
+		/* Didn't find the device */
+		err = -ENODEV;
 
-		if (seized_dev) {
-			err = pcistub_seize(seized_dev);
-			if (err) {
-				pci_dev_put(dev);
-				goto out;
-			}
+      out:
+	return err;
+}
 
-			/* Success! */
-			goto out;
+static void pcistub_remove(struct pci_dev *dev)
+{
+	struct pcistub_device *psdev, *found_psdev = NULL;
+	unsigned long flags;
+
+	dev_dbg(&dev->dev, "removing\n");
+
+	spin_lock_irqsave(&pcistub_devices_lock, flags);
+
+	list_for_each_entry(psdev, &pcistub_devices, dev_list) {
+		if (psdev->dev == dev) {
+			found_psdev = psdev;
+			break;
 		}
 	}
 
-	/* Didn't find the device */
-	err = -ENODEV;
+	spin_unlock_irqrestore(&pcistub_devices_lock, flags);
+
+	if (found_psdev) {
+		dev_dbg(&dev->dev, "found device to remove - in use? %p\n",
+			found_psdev->pdev);
+
+		if (found_psdev->pdev) {
+			printk(KERN_WARNING "pciback: ****** removing device "
+			       "%s while still in-use! ******\n",
+			       pci_name(found_psdev->dev));
+			printk(KERN_WARNING "pciback: ****** driver domain may "
+			       "still access this device's i/o resources!\n");
+			printk(KERN_WARNING "pciback: ****** shutdown driver "
+			       "domain before binding device\n");
+			printk(KERN_WARNING "pciback: ****** to other drivers "
+			       "or domains\n");
+
+			pciback_release_pci_dev(found_psdev->pdev,
+						found_psdev->dev);
+		}
 
-      out:
-	return err;
+		spin_lock_irqsave(&pcistub_devices_lock, flags);
+		list_del(&found_psdev->dev_list);
+		spin_unlock_irqrestore(&pcistub_devices_lock, flags);
+
+		/* the final put for releasing from the list */
+		pcistub_device_put(found_psdev);
+	}
 }
 
-struct pci_device_id pcistub_ids[] = {
+static struct pci_device_id pcistub_ids[] = {
 	{
 	 .vendor = PCI_ANY_ID,
 	 .device = PCI_ANY_ID,
@@ -298,16 +453,152 @@ struct pci_device_id pcistub_ids[] = {
  * for a normal device. I don't want it to be loaded automatically.
  */
 
-struct pci_driver pciback_pci_driver = {
+static struct pci_driver pciback_pci_driver = {
 	.name = "pciback",
 	.id_table = pcistub_ids,
 	.probe = pcistub_probe,
+	.remove = pcistub_remove,
 };
 
+static inline int str_to_slot(const char *buf, int *domain, int *bus,
+			      int *slot, int *func)
+{
+	int err;
+
+	err = sscanf(buf, " %x:%x:%x.%x", domain, bus, slot, func);
+	if (err == 4)
+		return 0;
+	else if (err < 0)
+		return -EINVAL;
+
+	/* try again without domain */
+	*domain = 0;
+	err = sscanf(buf, " %x:%x.%x", bus, slot, func);
+	if (err == 3)
+		return 0;
+
+	return -EINVAL;
+}
+
+static int pcistub_device_id_add(int domain, int bus, int slot, int func)
+{
+	struct pcistub_device_id *pci_dev_id;
+	unsigned long flags;
+
+	pci_dev_id = kmalloc(sizeof(*pci_dev_id), GFP_KERNEL);
+	if (!pci_dev_id)
+		return -ENOMEM;
+
+	pci_dev_id->domain = domain;
+	pci_dev_id->bus = bus;
+	pci_dev_id->devfn = PCI_DEVFN(slot, func);
+
+	pr_debug("pciback: wants to seize %04x:%02x:%02x.%01x\n",
+		 domain, bus, slot, func);
+
+	spin_lock_irqsave(&device_ids_lock, flags);
+	list_add_tail(&pci_dev_id->slot_list, &pcistub_device_ids);
+	spin_unlock_irqrestore(&device_ids_lock, flags);
+
+	return 0;
+}
+
+static int pcistub_device_id_remove(int domain, int bus, int slot, int func)
+{
+	struct pcistub_device_id *pci_dev_id, *t;
+	int devfn = PCI_DEVFN(slot, func);
+	int err = -ENOENT;
+	unsigned long flags;
+
+	spin_lock_irqsave(&device_ids_lock, flags);
+	list_for_each_entry_safe(pci_dev_id, t, &pcistub_device_ids, slot_list) {
+
+		if (pci_dev_id->domain == domain
+		    && pci_dev_id->bus == bus && pci_dev_id->devfn == devfn) {
+			/* Don't break; here because it's possible the same
+			 * slot could be in the list more than once
+			 */
+			list_del(&pci_dev_id->slot_list);
+			kfree(pci_dev_id);
+
+			err = 0;
+
+			pr_debug("pciback: removed %04x:%02x:%02x.%01x from "
+				 "seize list\n", domain, bus, slot, func);
+		}
+	}
+	spin_unlock_irqrestore(&device_ids_lock, flags);
+
+	return err;
+}
+
+static ssize_t pcistub_slot_add(struct device_driver *drv, const char *buf,
+				size_t count)
+{
+	int domain, bus, slot, func;
+	int err;
+
+	err = str_to_slot(buf, &domain, &bus, &slot, &func);
+	if (err)
+		goto out;
+
+	err = pcistub_device_id_add(domain, bus, slot, func);
+
+      out:
+	if (!err)
+		err = count;
+	return err;
+}
+
+DRIVER_ATTR(new_slot, S_IWUSR, NULL, pcistub_slot_add);
+
+static ssize_t pcistub_slot_remove(struct device_driver *drv, const char *buf,
+				   size_t count)
+{
+	int domain, bus, slot, func;
+	int err;
+
+	err = str_to_slot(buf, &domain, &bus, &slot, &func);
+	if (err)
+		goto out;
+
+	err = pcistub_device_id_remove(domain, bus, slot, func);
+
+      out:
+	if (!err)
+		err = count;
+	return err;
+}
+
+DRIVER_ATTR(remove_slot, S_IWUSR, NULL, pcistub_slot_remove);
+
+static ssize_t pcistub_slot_show(struct device_driver *drv, char *buf)
+{
+	struct pcistub_device_id *pci_dev_id;
+	size_t count = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&device_ids_lock, flags);
+	list_for_each_entry(pci_dev_id, &pcistub_device_ids, slot_list) {
+		if (count >= PAGE_SIZE)
+			break;
+
+		count += scnprintf(buf + count, PAGE_SIZE - count,
+				   "%04x:%02x:%02x.%01x\n",
+				   pci_dev_id->domain, pci_dev_id->bus,
+				   PCI_SLOT(pci_dev_id->devfn),
+				   PCI_FUNC(pci_dev_id->devfn));
+	}
+	spin_unlock_irqrestore(&device_ids_lock, flags);
+
+	return count;
+}
+
+DRIVER_ATTR(slots, S_IRUSR, pcistub_slot_show, NULL);
+
 static int __init pcistub_init(void)
 {
 	int pos = 0;
-	struct pci_stub_device_id *pci_dev_id;
 	int err = 0;
 	int domain, bus, slot, func;
 	int parsed;
@@ -328,34 +619,28 @@ static int __init pcistub_init(void)
 					goto parse_error;
 			}
 
-			pci_dev_id = kmalloc(sizeof(*pci_dev_id), GFP_KERNEL);
-			if (!pci_dev_id) {
-				err = -ENOMEM;
+			err = pcistub_device_id_add(domain, bus, slot, func);
+			if (err)
 				goto out;
-			}
-
-			pci_dev_id->domain = domain;
-			pci_dev_id->bus = bus;
-			pci_dev_id->devfn = PCI_DEVFN(slot, func);
-
-			pr_debug
-			    ("pciback: wants to seize %04x:%02x:%02x.%01x\n",
-			     domain, bus, slot, func);
-
-			list_add_tail(&pci_dev_id->slot_list,
-				      &pci_stub_device_ids);
 
 			/* if parsed<=0, we've reached the end of the string */
 			pos += parsed;
 		} while (parsed > 0 && pci_devs_to_hide[pos]);
-
-		/* If we're the first PCI Device Driver to register, we're the
-		 * first one to get offered PCI devices as they become
-		 * available (and thus we can be the first to grab them)
-		 */
-		pci_register_driver(&pciback_pci_driver);
 	}
 
+	/* If we're the first PCI Device Driver to register, we're the
+	 * first one to get offered PCI devices as they become
+	 * available (and thus we can be the first to grab them)
+	 */
+	err = pci_register_driver(&pciback_pci_driver);
+	if (err < 0)
+		goto out;
+
+	driver_create_file(&pciback_pci_driver.driver, &driver_attr_new_slot);
+	driver_create_file(&pciback_pci_driver.driver,
+			   &driver_attr_remove_slot);
+	driver_create_file(&pciback_pci_driver.driver, &driver_attr_slots);
+
       out:
 	return err;
 
@@ -386,19 +671,22 @@ static int __init pciback_init(void)
 		return err;
 #endif
 
-	if (list_empty(&pci_stub_device_ids))
-		return -ENODEV;
 	pcistub_init_devices_late();
 	pciback_xenbus_register();
 
-	__unsafe(THIS_MODULE);
-
 	return 0;
 }
 
-static void pciback_cleanup(void)
+static void __exit pciback_cleanup(void)
 {
-	BUG();
+	pciback_xenbus_unregister();
+
+	driver_remove_file(&pciback_pci_driver.driver, &driver_attr_new_slot);
+	driver_remove_file(&pciback_pci_driver.driver,
+			   &driver_attr_remove_slot);
+	driver_remove_file(&pciback_pci_driver.driver, &driver_attr_slots);
+
+	pci_unregister_driver(&pciback_pci_driver);
 }
 
 module_init(pciback_init);
diff --git a/linux-2.6-xen-sparse/drivers/xen/pciback/pciback.h b/linux-2.6-xen-sparse/drivers/xen/pciback/pciback.h
index f82ece4a2c..aaa0d75d3a 100644
--- a/linux-2.6-xen-sparse/drivers/xen/pciback/pciback.h
+++ b/linux-2.6-xen-sparse/drivers/xen/pciback/pciback.h
@@ -37,9 +37,11 @@ struct pciback_dev_data {
 };
 
 /* Get/Put PCI Devices that are hidden from the PCI Backend Domain */
-struct pci_dev *pcistub_get_pci_dev_by_slot(int domain, int bus,
+struct pci_dev *pcistub_get_pci_dev_by_slot(struct pciback_device *pdev,
+					    int domain, int bus,
 					    int slot, int func);
-struct pci_dev *pcistub_get_pci_dev(struct pci_dev *dev);
+struct pci_dev *pcistub_get_pci_dev(struct pciback_device *pdev,
+				    struct pci_dev *dev);
 void pcistub_put_pci_dev(struct pci_dev *dev);
 
 /* Ensure a device is turned off or reset */
@@ -57,6 +59,7 @@ int pciback_config_write(struct pci_dev *dev, int offset, int size, u32 value);
 typedef int (*publish_pci_root_cb) (struct pciback_device * pdev,
 				    unsigned int domain, unsigned int bus);
 int pciback_add_pci_dev(struct pciback_device *pdev, struct pci_dev *dev);
+void pciback_release_pci_dev(struct pciback_device *pdev, struct pci_dev *dev);
 struct pci_dev *pciback_get_pci_dev(struct pciback_device *pdev,
 				    unsigned int domain, unsigned int bus,
 				    unsigned int devfn);
@@ -69,6 +72,7 @@ void pciback_release_devices(struct pciback_device *pdev);
 irqreturn_t pciback_handle_event(int irq, void *dev_id, struct pt_regs *regs);
 
 int pciback_xenbus_register(void);
+void pciback_xenbus_unregister(void);
 
 extern int verbose_request;
 #endif
diff --git a/linux-2.6-xen-sparse/drivers/xen/pciback/pciback_ops.c b/linux-2.6-xen-sparse/drivers/xen/pciback/pciback_ops.c
index 55812edd19..48305f549d 100644
--- a/linux-2.6-xen-sparse/drivers/xen/pciback/pciback_ops.c
+++ b/linux-2.6-xen-sparse/drivers/xen/pciback/pciback_ops.c
@@ -12,9 +12,8 @@ int verbose_request = 0;
 module_param(verbose_request, int, 0644);
 
 /* Ensure a device is "turned off" and ready to be exported.
- * This also sets up the device's private data to keep track of what should
- * be in the base address registers (BARs) so that we can keep the
- * client from manipulating them directly.
+ * (Also see pciback_config_reset to ensure virtual configuration space is
+ * ready to be re-exported)
  */
 void pciback_reset_device(struct pci_dev *dev)
 {
diff --git a/linux-2.6-xen-sparse/drivers/xen/pciback/vpci.c b/linux-2.6-xen-sparse/drivers/xen/pciback/vpci.c
index 17d554dcc6..84bc4b9902 100644
--- a/linux-2.6-xen-sparse/drivers/xen/pciback/vpci.c
+++ b/linux-2.6-xen-sparse/drivers/xen/pciback/vpci.c
@@ -8,12 +8,15 @@
 #include <linux/list.h>
 #include <linux/slab.h>
 #include <linux/pci.h>
+#include <linux/spinlock.h>
 #include "pciback.h"
 
 #define PCI_SLOT_MAX 32
 
 struct vpci_dev_data {
+	/* Access to dev_list must be protected by lock */
 	struct list_head dev_list[PCI_SLOT_MAX];
+	spinlock_t lock;
 };
 
 static inline struct list_head *list_first(struct list_head *head)
@@ -25,25 +28,29 @@ struct pci_dev *pciback_get_pci_dev(struct pciback_device *pdev,
 				    unsigned int domain, unsigned int bus,
 				    unsigned int devfn)
 {
-	struct pci_dev_entry *dev_entry;
+	struct pci_dev_entry *entry;
+	struct pci_dev *dev = NULL;
 	struct vpci_dev_data *vpci_dev = pdev->pci_dev_data;
+	unsigned long flags;
 
 	if (domain != 0 || bus != 0)
 		return NULL;
 
 	if (PCI_SLOT(devfn) < PCI_SLOT_MAX) {
-		/* we don't need to lock the list here because once the backend
-		 * is in operation, it won't have any more devices addeded
-		 * (or removed).
-		 */
-		list_for_each_entry(dev_entry,
+		spin_lock_irqsave(&vpci_dev->lock, flags);
+
+		list_for_each_entry(entry,
 				    &vpci_dev->dev_list[PCI_SLOT(devfn)],
 				    list) {
-			if (PCI_FUNC(dev_entry->dev->devfn) == PCI_FUNC(devfn))
-				return dev_entry->dev;
+			if (PCI_FUNC(entry->dev->devfn) == PCI_FUNC(devfn)) {
+				dev = entry->dev;
+				break;
+			}
 		}
+
+		spin_unlock_irqrestore(&vpci_dev->lock, flags);
 	}
-	return NULL;
+	return dev;
 }
 
 static inline int match_slot(struct pci_dev *l, struct pci_dev *r)
@@ -55,12 +62,12 @@ static inline int match_slot(struct pci_dev *l, struct pci_dev *r)
 	return 0;
 }
 
-/* Must hold pciback_device->dev_lock when calling this */
 int pciback_add_pci_dev(struct pciback_device *pdev, struct pci_dev *dev)
 {
 	int err = 0, slot;
 	struct pci_dev_entry *t, *dev_entry;
 	struct vpci_dev_data *vpci_dev = pdev->pci_dev_data;
+	unsigned long flags;
 
 	if ((dev->class >> 24) == PCI_BASE_CLASS_BRIDGE) {
 		err = -EFAULT;
@@ -79,6 +86,8 @@ int pciback_add_pci_dev(struct pciback_device *pdev, struct pci_dev *dev)
 
 	dev_entry->dev = dev;
 
+	spin_lock_irqsave(&vpci_dev->lock, flags);
+
 	/* Keep multi-function devices together on the virtual PCI bus */
 	for (slot = 0; slot < PCI_SLOT_MAX; slot++) {
 		if (!list_empty(&vpci_dev->dev_list[slot])) {
@@ -92,7 +101,7 @@ int pciback_add_pci_dev(struct pciback_device *pdev, struct pci_dev *dev)
 					PCI_FUNC(dev->devfn));
 				list_add_tail(&dev_entry->list,
 					      &vpci_dev->dev_list[slot]);
-				goto out;
+				goto unlock;
 			}
 		}
 	}
@@ -105,7 +114,7 @@ int pciback_add_pci_dev(struct pciback_device *pdev, struct pci_dev *dev)
 			       pci_name(dev), slot);
 			list_add_tail(&dev_entry->list,
 				      &vpci_dev->dev_list[slot]);
-			goto out;
+			goto unlock;
 		}
 	}
 
@@ -113,10 +122,41 @@ int pciback_add_pci_dev(struct pciback_device *pdev, struct pci_dev *dev)
 	xenbus_dev_fatal(pdev->xdev, err,
 			 "No more space on root virtual PCI bus");
 
+      unlock:
+	spin_unlock_irqrestore(&vpci_dev->lock, flags);
       out:
 	return err;
 }
 
+void pciback_release_pci_dev(struct pciback_device *pdev, struct pci_dev *dev)
+{
+	int slot;
+	struct vpci_dev_data *vpci_dev = pdev->pci_dev_data;
+	struct pci_dev *found_dev = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&vpci_dev->lock, flags);
+
+	for (slot = 0; slot < PCI_SLOT_MAX; slot++) {
+		struct pci_dev_entry *e, *tmp;
+		list_for_each_entry_safe(e, tmp, &vpci_dev->dev_list[slot],
+					 list) {
+			if (e->dev == dev) {
+				list_del(&e->list);
+				found_dev = e->dev;
+				kfree(e);
+				goto out;
+			}
+		}
+	}
+
+      out:
+	spin_unlock_irqrestore(&vpci_dev->lock, flags);
+
+	if (found_dev)
+		pcistub_put_pci_dev(found_dev);
+}
+
 int pciback_init_devices(struct pciback_device *pdev)
 {
 	int slot;
@@ -126,6 +166,8 @@ int pciback_init_devices(struct pciback_device *pdev)
 	if (!vpci_dev)
 		return -ENOMEM;
 
+	spin_lock_init(&vpci_dev->lock);
+
 	for (slot = 0; slot < PCI_SLOT_MAX; slot++) {
 		INIT_LIST_HEAD(&vpci_dev->dev_list[slot]);
 	}
@@ -142,7 +184,6 @@ int pciback_publish_pci_roots(struct pciback_device *pdev,
 	return publish_cb(pdev, 0, 0);
 }
 
-/* Must hold pciback_device->dev_lock when calling this */
 void pciback_release_devices(struct pciback_device *pdev)
 {
 	int slot;
diff --git a/linux-2.6-xen-sparse/drivers/xen/pciback/xenbus.c b/linux-2.6-xen-sparse/drivers/xen/pciback/xenbus.c
index 1df5feacdc..f338de1f37 100644
--- a/linux-2.6-xen-sparse/drivers/xen/pciback/xenbus.c
+++ b/linux-2.6-xen-sparse/drivers/xen/pciback/xenbus.c
@@ -12,7 +12,7 @@
 
 #define INVALID_EVTCHN_IRQ  (-1)
 
-struct pciback_device *alloc_pdev(struct xenbus_device *xdev)
+static struct pciback_device *alloc_pdev(struct xenbus_device *xdev)
 {
 	struct pciback_device *pdev;
 
@@ -38,7 +38,7 @@ struct pciback_device *alloc_pdev(struct xenbus_device *xdev)
 	return pdev;
 }
 
-void free_pdev(struct pciback_device *pdev)
+static void free_pdev(struct pciback_device *pdev)
 {
 	if (pdev->be_watching)
 		unregister_xenbus_watch(&pdev->be_watch);
@@ -247,7 +247,7 @@ static int pciback_export_device(struct pciback_device *pdev,
 	dev_dbg(&pdev->xdev->dev, "exporting dom %x bus %x slot %x func %x\n",
 		domain, bus, slot, func);
 
-	dev = pcistub_get_pci_dev_by_slot(domain, bus, slot, func);
+	dev = pcistub_get_pci_dev_by_slot(pdev, domain, bus, slot, func);
 	if (!dev) {
 		err = -EINVAL;
 		xenbus_dev_fatal(pdev->xdev, err,
@@ -434,3 +434,8 @@ int __init pciback_xenbus_register(void)
 {
 	return xenbus_register_backend(&xenbus_pciback_driver);
 }
+
+void __exit pciback_xenbus_unregister(void)
+{
+	xenbus_unregister_driver(&xenbus_pciback_driver);
+}
diff --git a/tools/python/xen/xend/server/pciif.py b/tools/python/xen/xend/server/pciif.py
index 0391e2f467..aa222691f6 100644
--- a/tools/python/xen/xend/server/pciif.py
+++ b/tools/python/xen/xend/server/pciif.py
@@ -118,10 +118,12 @@ class PciController(DevController):
                     "parse it's resources - %s"+str(e))
 
         if dev.driver!='pciback':
-            raise VmError(("pci: PCI Backend does not own device "+
-                    "%s\n"+
-                    "See the pciback.hide kernel "+
-                    "command-line parameter")%(dev.name))
+            raise VmError(("pci: PCI Backend does not own device "+ \
+                    "%s\n"+ \
+                    "See the pciback.hide kernel "+ \
+                    "command-line parameter or\n"+ \
+                    "bind your slot/device to the PCI backend using sysfs" \
+                    )%(dev.name))
 
         for (start, size) in dev.ioports:
             log.debug('pci: enabling ioport 0x%x/0x%x'%(start,size))