summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael S. Tsirkin <mst@redhat.com>2011-11-17 17:41:15 +0200
committerRusty Russell <rusty@rustcorp.com.au>2011-11-24 13:04:48 +1030
commite6af578c5305be693a1bc7f4dc7b51dd82d41425 (patch)
tree7276a130a2d2eddf84660ec2573298e26611a442
parentfe1a7fe2c4456679b3402f04268bdfafca7b127a (diff)
virtio-pci: make reset operation safer
virtio pci device reset actually just does an I/O write, which in PCI is really posted, that is it can complete on CPU before the device has received it. Further, interrupts might have been pending on another CPU, so device callback might get invoked after reset. This conflicts with how drivers use reset, which is typically: reset unregister a callback running after reset completed can race with unregister, potentially leading to use after free bugs. Fix by flushing out the write, and flushing pending interrupts. This assumes that device is never reset from its vq/config callbacks, or in parallel with being added/removed, document this assumption. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
-rw-r--r--drivers/virtio/virtio_pci.c18
-rw-r--r--include/linux/virtio_config.h2
2 files changed, 20 insertions, 0 deletions
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 3d1bf41e8892..03d1984bd363 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -169,11 +169,29 @@ static void vp_set_status(struct virtio_device *vdev, u8 status)
iowrite8(status, vp_dev->ioaddr + VIRTIO_PCI_STATUS);
}
+/* wait for pending irq handlers */
+static void vp_synchronize_vectors(struct virtio_device *vdev)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ int i;
+
+ if (vp_dev->intx_enabled)
+ synchronize_irq(vp_dev->pci_dev->irq);
+
+ for (i = 0; i < vp_dev->msix_vectors; ++i)
+ synchronize_irq(vp_dev->msix_entries[i].vector);
+}
+
static void vp_reset(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
/* 0 status means a reset. */
iowrite8(0, vp_dev->ioaddr + VIRTIO_PCI_STATUS);
+ /* Flush out the status write, and flush in device writes,
+ * including MSi-X interrupts, if any. */
+ ioread8(vp_dev->ioaddr + VIRTIO_PCI_STATUS);
+ /* Flush pending VQ/configuration callbacks. */
+ vp_synchronize_vectors(vdev);
}
/* the notify function used when creating a virt queue */
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index add4790b21fe..e9e72bda1b72 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -85,6 +85,8 @@
* @reset: reset the device
* vdev: the virtio device
* After this, status and feature negotiation must be done again
+ * Device must not be reset from its vq/config callbacks, or in
+ * parallel with being added/removed.
* @find_vqs: find virtqueues and instantiate them.
* vdev: the virtio_device
* nvqs: the number of virtqueues to find