(One big patch, I'll merge it in properly later).
A reset function solves three problems:
1) It allows us to renegotiate features, eg. if we want to upgrade a
guest driver without rebooting the guest.
2) It gives us a clean way of shutting down virtqueues: after a reset,
we know that the buffers won't be used by the guest, and
3) It helps the guest recover from messed-up drivers.
So we remove the ->shutdown hook, and the only way we now remove
feature bits is via reset.
Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
---
drivers/char/hw_random/virtio-rng.c | 1 -
drivers/lguest/lguest_device.c | 15 ++++++++++++++-
drivers/net/virtio_net.c | 2 --
drivers/virtio/virtio.c | 13 ++++++++++---
drivers/virtio/virtio_pci.c | 10 ++++++++++
drivers/virtio/virtio_ring.c | 11 -----------
include/linux/virtio.h | 5 -----
include/linux/virtio_config.h | 4 ++++
8 files changed, 38 insertions(+), 23 deletions(-)
diff -r f9464b21ed9c drivers/char/hw_random/virtio-rng.c
--- a/drivers/char/hw_random/virtio-rng.c Wed Jan 23 23:52:47 2008 +1100
+++ b/drivers/char/hw_random/virtio-rng.c Thu Jan 24 00:47:05 2008 +1100
@@ -92,7 +92,6 @@ static void virtrng_remove(struct virtio
static void virtrng_remove(struct virtio_device *vdev)
{
hwrng_unregister(&virtio_hwrng);
- vq->vq_ops->shutdown(vq);
vdev->config->del_vq(vq);
}
diff -r f9464b21ed9c drivers/lguest/lguest_device.c
--- a/drivers/lguest/lguest_device.c Wed Jan 23 23:52:47 2008 +1100
+++ b/drivers/lguest/lguest_device.c Thu Jan 24 00:47:05 2008 +1100
@@ -54,7 +54,7 @@ struct lguest_device {
*
* The configuration information for a device consists of one or more
* virtqueues, a feature bitmaks, and some configuration bytes. The
- * configuration bytes don't really matter to us: the Launcher set them up, and
+ * configuration bytes don't really matter to us: the Launcher sets them up, and
* the driver will look at them during setup.
*
* A convenient routine to return the device's virtqueue config array:
@@ -139,7 +139,19 @@ static u8 lg_get_status(struct virtio_de
static void lg_set_status(struct virtio_device *vdev, u8 status)
{
+ BUG_ON(!status);
to_lgdev(vdev)->desc->status = status;
+}
+
+/* To reset the device, we (ab)use the NOTIFY hypercall, with the descriptor
+ * address of the device. The Host will zero the status and all the
+ * features. */
+static void lg_reset(struct virtio_device *vdev)
+{
+ unsigned long offset = (void *)to_lgdev(vdev)->desc - lguest_devices;
+
+ printk("Resetting %i\n", vdev->index);
+ hcall(LHCALL_NOTIFY, (max_pfn<<PAGE_SHIFT) + offset, 0, 0);
}
/*
@@ -279,6 +291,7 @@ static struct virtio_config_ops lguest_c
.set = lg_set,
.get_status = lg_get_status,
.set_status = lg_set_status,
+ .reset = lg_reset,
.find_vq = lg_find_vq,
.del_vq = lg_del_vq,
};
diff -r f9464b21ed9c drivers/net/virtio_net.c
--- a/drivers/net/virtio_net.c Wed Jan 23 23:52:47 2008 +1100
+++ b/drivers/net/virtio_net.c Thu Jan 24 00:47:05 2008 +1100
@@ -415,12 +415,10 @@ static void virtnet_remove(struct virtio
struct sk_buff *skb;
/* Free our skbs in send and recv queues, if any. */
- vi->rvq->vq_ops->shutdown(vi->rvq);
while ((skb = __skb_dequeue(&vi->recv)) != NULL) {
kfree_skb(skb);
vi->num--;
}
- vi->svq->vq_ops->shutdown(vi->svq);
while ((skb = __skb_dequeue(&vi->send)) != NULL)
kfree_skb(skb);
diff -r f9464b21ed9c drivers/virtio/virtio.c
--- a/drivers/virtio/virtio.c Wed Jan 23 23:52:47 2008 +1100
+++ b/drivers/virtio/virtio.c Thu Jan 24 00:47:05 2008 +1100
@@ -104,10 +104,13 @@ static int virtio_dev_remove(struct devi
struct virtio_driver *drv = container_of(dev->dev.driver,
struct virtio_driver, driver);
- dev->config->set_status(dev, dev->config->get_status(dev)
- & ~(VIRTIO_CONFIG_S_DRIVER
- | VIRTIO_CONFIG_S_DRIVER_OK));
+ /* This shuts down all the virtqueues, so their buffers won't
+ * be used. */
+ dev->config->reset(dev);
drv->remove(dev);
+
+ /* Acknowledge the device's existence again, after reset. */
+ add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
return 0;
}
@@ -139,6 +142,10 @@ int register_virtio_device(struct virtio
dev->dev.release = virtio_device_release;
sprintf(dev->dev.bus_id, "%u", dev->index);
+ /* We always start by resetting the device, in case a previous
+ * driver messed it up. This also tests that code path a little. */
+ dev->config->reset(dev);
+
/* Acknowledge that we've seen the device. */
add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
diff -r f9464b21ed9c drivers/virtio/virtio_pci.c
--- a/drivers/virtio/virtio_pci.c Wed Jan 23 23:52:47 2008 +1100
+++ b/drivers/virtio/virtio_pci.c Thu Jan 24 00:47:05 2008 +1100
@@ -148,7 +148,16 @@ static void vp_set_status(struct virtio_
static void vp_set_status(struct virtio_device *vdev, u8 status)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ /* We should never be setting status to 0. */
+ BUG_ON(status == 0);
return iowrite8(status, vp_dev->ioaddr + VIRTIO_PCI_STATUS);
+}
+
+static void vp_reset(struct virtio_device *vdev)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ /* 0 status means a reset. */
+ return iowrite8(0, vp_dev->ioaddr + VIRTIO_PCI_STATUS);
}
/* the notify function used when creating a virt queue */
@@ -291,6 +300,7 @@ static struct virtio_config_ops virtio_p
.set = vp_set,
.get_status = vp_get_status,
.set_status = vp_set_status,
+ .reset = vp_reset,
.find_vq = vp_find_vq,
.del_vq = vp_del_vq,
};
diff -r f9464b21ed9c drivers/virtio/virtio_ring.c
--- a/drivers/virtio/virtio_ring.c Wed Jan 23 23:52:47 2008 +1100
+++ b/drivers/virtio/virtio_ring.c Thu Jan 24 00:47:05 2008 +1100
@@ -173,16 +173,6 @@ static void detach_buf(struct vring_virt
vq->num_free++;
}
-/* FIXME: We need to tell other side about removal, to synchronize. */
-static void vring_shutdown(struct virtqueue *_vq)
-{
- struct vring_virtqueue *vq = to_vvq(_vq);
- unsigned int i;
-
- for (i = 0; i < vq->vring.num; i++)
- detach_buf(vq, i);
-}
-
static inline bool more_used(const struct vring_virtqueue *vq)
{
return vq->last_used_idx != vq->vring.used->idx;
@@ -287,7 +277,6 @@ static struct virtqueue_ops vring_vq_ops
.kick = vring_kick,
.disable_cb = vring_disable_cb,
.enable_cb = vring_enable_cb,
- .shutdown = vring_shutdown,
};
struct virtqueue *vring_new_virtqueue(unsigned int num,
diff -r f9464b21ed9c include/linux/virtio.h
--- a/include/linux/virtio.h Wed Jan 23 23:52:47 2008 +1100
+++ b/include/linux/virtio.h Thu Jan 24 00:47:05 2008 +1100
@@ -45,9 +45,6 @@ struct virtqueue
* vq: the struct virtqueue we're talking about.
* This returns "false" (and doesn't re-enable) if there are pending
* buffers in the queue, to avoid a race.
- * @shutdown: "unadd" all buffers.
- * vq: the struct virtqueue we're talking about.
- * Remove everything from the queue.
*
* Locking rules are straightforward: the driver is responsible for
* locking. No two operations may be invoked simultaneously.
@@ -67,8 +64,6 @@ struct virtqueue_ops {
void (*disable_cb)(struct virtqueue *vq);
bool (*enable_cb)(struct virtqueue *vq);
-
- void (*shutdown)(struct virtqueue *vq);
};
/**
diff -r f9464b21ed9c include/linux/virtio_config.h
--- a/include/linux/virtio_config.h Wed Jan 23 23:52:47 2008 +1100
+++ b/include/linux/virtio_config.h Thu Jan 24 00:47:05 2008 +1100
@@ -43,6 +43,9 @@ struct virtio_device;
* @set_status: write the status byte
* vdev: the virtio_device
* status: the new status byte
+ * @reset: reset the device
+ * vdev: the virtio device
+ * After this, status and feature negotiation must be done again
* @find_vq: find a virtqueue and instantiate it.
* vdev: the virtio_device
* index: the 0-based virtqueue number in case there's more than one.
@@ -59,6 +62,7 @@ struct virtio_config_ops
const void *buf, unsigned len);
u8 (*get_status)(struct virtio_device *vdev);
void (*set_status)(struct virtio_device *vdev, u8 status);
+ void (*reset)(struct virtio_device *vdev);
struct virtqueue *(*find_vq)(struct virtio_device *vdev,
unsigned index,
void (*callback)(struct virtqueue *));
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/virtualization
