Commit 6e5aa7ef authored by Rusty Russell's avatar Rusty Russell

virtio: reset function

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 host, 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.

We leave it to the driver to do the reset before it deletes queues:
the balloon driver, for example, needs to chat to the host in its
remove function.
Signed-off-by: default avatarRusty Russell <rusty@rustcorp.com.au>
parent b3369c1f
...@@ -193,6 +193,13 @@ static void *_convert(struct iovec *iov, size_t size, size_t align, ...@@ -193,6 +193,13 @@ static void *_convert(struct iovec *iov, size_t size, size_t align,
#define le32_to_cpu(v32) (v32) #define le32_to_cpu(v32) (v32)
#define le64_to_cpu(v64) (v64) #define le64_to_cpu(v64) (v64)
/* The device virtqueue descriptors are followed by feature bitmasks. */
static u8 *get_feature_bits(struct device *dev)
{
return (u8 *)(dev->desc + 1)
+ dev->desc->num_vq * sizeof(struct lguest_vqconfig);
}
/*L:100 The Launcher code itself takes us out into userspace, that scary place /*L:100 The Launcher code itself takes us out into userspace, that scary place
* where pointers run wild and free! Unfortunately, like most userspace * where pointers run wild and free! Unfortunately, like most userspace
* programs, it's quite boring (which is why everyone likes to hack on the * programs, it's quite boring (which is why everyone likes to hack on the
...@@ -914,21 +921,58 @@ static void enable_fd(int fd, struct virtqueue *vq) ...@@ -914,21 +921,58 @@ static void enable_fd(int fd, struct virtqueue *vq)
write(waker_fd, &vq->dev->fd, sizeof(vq->dev->fd)); write(waker_fd, &vq->dev->fd, sizeof(vq->dev->fd));
} }
/* Resetting a device is fairly easy. */
static void reset_device(struct device *dev)
{
struct virtqueue *vq;
verbose("Resetting device %s\n", dev->name);
/* Clear the status. */
dev->desc->status = 0;
/* Clear any features they've acked. */
memset(get_feature_bits(dev) + dev->desc->feature_len, 0,
dev->desc->feature_len);
/* Zero out the virtqueues. */
for (vq = dev->vq; vq; vq = vq->next) {
memset(vq->vring.desc, 0,
vring_size(vq->config.num, getpagesize()));
vq->last_avail_idx = 0;
}
}
/* This is the generic routine we call when the Guest uses LHCALL_NOTIFY. */ /* This is the generic routine we call when the Guest uses LHCALL_NOTIFY. */
static void handle_output(int fd, unsigned long addr) static void handle_output(int fd, unsigned long addr)
{ {
struct device *i; struct device *i;
struct virtqueue *vq; struct virtqueue *vq;
/* Check each virtqueue. */ /* Check each device and virtqueue. */
for (i = devices.dev; i; i = i->next) { for (i = devices.dev; i; i = i->next) {
/* Notifications to device descriptors reset the device. */
if (from_guest_phys(addr) == i->desc) {
reset_device(i);
return;
}
/* Notifications to virtqueues mean output has occurred. */
for (vq = i->vq; vq; vq = vq->next) { for (vq = i->vq; vq; vq = vq->next) {
if (vq->config.pfn == addr/getpagesize()) { if (vq->config.pfn != addr/getpagesize())
verbose("Output to %s\n", vq->dev->name); continue;
if (vq->handle_output)
vq->handle_output(fd, vq); /* Guest should acknowledge (and set features!) before
* using the device. */
if (i->desc->status == 0) {
warnx("%s gave early output", i->name);
return; return;
} }
if (strcmp(vq->dev->name, "console") != 0)
verbose("Output to %s\n", vq->dev->name);
if (vq->handle_output)
vq->handle_output(fd, vq);
return;
} }
} }
...@@ -1074,10 +1118,11 @@ static void add_virtqueue(struct device *dev, unsigned int num_descs, ...@@ -1074,10 +1118,11 @@ static void add_virtqueue(struct device *dev, unsigned int num_descs,
vq->vring.used->flags = VRING_USED_F_NO_NOTIFY; vq->vring.used->flags = VRING_USED_F_NO_NOTIFY;
} }
/* The virtqueue descriptors are followed by feature bytes. */ /* The first half of the feature bitmask is for us to advertise features. The
* second half if for the Guest to accept features. */
static void add_feature(struct device *dev, unsigned bit) static void add_feature(struct device *dev, unsigned bit)
{ {
u8 *features; u8 *features = get_feature_bits(dev);
/* We can't extend the feature bits once we've added config bytes */ /* We can't extend the feature bits once we've added config bytes */
if (dev->desc->feature_len <= bit / CHAR_BIT) { if (dev->desc->feature_len <= bit / CHAR_BIT) {
...@@ -1085,9 +1130,6 @@ static void add_feature(struct device *dev, unsigned bit) ...@@ -1085,9 +1130,6 @@ static void add_feature(struct device *dev, unsigned bit)
dev->desc->feature_len = (bit / CHAR_BIT) + 1; dev->desc->feature_len = (bit / CHAR_BIT) + 1;
} }
features = (u8 *)(dev->desc + 1)
+ dev->desc->num_vq * sizeof(struct lguest_vqconfig);
features[bit / CHAR_BIT] |= (1 << (bit % CHAR_BIT)); features[bit / CHAR_BIT] |= (1 << (bit % CHAR_BIT));
} }
......
...@@ -264,12 +264,16 @@ static void virtblk_remove(struct virtio_device *vdev) ...@@ -264,12 +264,16 @@ static void virtblk_remove(struct virtio_device *vdev)
struct virtio_blk *vblk = vdev->priv; struct virtio_blk *vblk = vdev->priv;
int major = vblk->disk->major; int major = vblk->disk->major;
/* Nothing should be pending. */
BUG_ON(!list_empty(&vblk->reqs)); BUG_ON(!list_empty(&vblk->reqs));
/* Stop all the virtqueues. */
vdev->config->reset(vdev);
blk_cleanup_queue(vblk->disk->queue); blk_cleanup_queue(vblk->disk->queue);
put_disk(vblk->disk); put_disk(vblk->disk);
unregister_blkdev(major, "virtblk"); unregister_blkdev(major, "virtblk");
mempool_destroy(vblk->pool); mempool_destroy(vblk->pool);
/* There should be nothing in the queue now, so no need to shutdown */
vdev->config->del_vq(vblk->vq); vdev->config->del_vq(vblk->vq);
kfree(vblk); kfree(vblk);
} }
......
...@@ -54,7 +54,7 @@ struct lguest_device { ...@@ -54,7 +54,7 @@ struct lguest_device {
* *
* The configuration information for a device consists of one or more * The configuration information for a device consists of one or more
* virtqueues, a feature bitmaks, and some configuration bytes. The * 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. * the driver will look at them during setup.
* *
* A convenient routine to return the device's virtqueue config array: * A convenient routine to return the device's virtqueue config array:
...@@ -139,9 +139,20 @@ static u8 lg_get_status(struct virtio_device *vdev) ...@@ -139,9 +139,20 @@ static u8 lg_get_status(struct virtio_device *vdev)
static void lg_set_status(struct virtio_device *vdev, u8 status) static void lg_set_status(struct virtio_device *vdev, u8 status)
{ {
BUG_ON(!status);
to_lgdev(vdev)->desc->status = 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;
hcall(LHCALL_NOTIFY, (max_pfn<<PAGE_SHIFT) + offset, 0, 0);
}
/* /*
* Virtqueues * Virtqueues
* *
...@@ -279,6 +290,7 @@ static struct virtio_config_ops lguest_config_ops = { ...@@ -279,6 +290,7 @@ static struct virtio_config_ops lguest_config_ops = {
.set = lg_set, .set = lg_set,
.get_status = lg_get_status, .get_status = lg_get_status,
.set_status = lg_set_status, .set_status = lg_set_status,
.reset = lg_reset,
.find_vq = lg_find_vq, .find_vq = lg_find_vq,
.del_vq = lg_del_vq, .del_vq = lg_del_vq,
}; };
......
...@@ -390,13 +390,14 @@ static void virtnet_remove(struct virtio_device *vdev) ...@@ -390,13 +390,14 @@ static void virtnet_remove(struct virtio_device *vdev)
struct virtnet_info *vi = vdev->priv; struct virtnet_info *vi = vdev->priv;
struct sk_buff *skb; struct sk_buff *skb;
/* Stop all the virtqueues. */
vdev->config->reset(vdev);
/* Free our skbs in send and recv queues, if any. */ /* Free our skbs in send and recv queues, if any. */
vi->rvq->vq_ops->shutdown(vi->rvq);
while ((skb = __skb_dequeue(&vi->recv)) != NULL) { while ((skb = __skb_dequeue(&vi->recv)) != NULL) {
kfree_skb(skb); kfree_skb(skb);
vi->num--; vi->num--;
} }
vi->svq->vq_ops->shutdown(vi->svq);
while ((skb = __skb_dequeue(&vi->send)) != NULL) while ((skb = __skb_dequeue(&vi->send)) != NULL)
kfree_skb(skb); kfree_skb(skb);
......
...@@ -102,9 +102,13 @@ static int virtio_dev_remove(struct device *_d) ...@@ -102,9 +102,13 @@ static int virtio_dev_remove(struct device *_d)
struct virtio_driver *drv = container_of(dev->dev.driver, struct virtio_driver *drv = container_of(dev->dev.driver,
struct virtio_driver, driver); struct virtio_driver, driver);
dev->config->set_status(dev, dev->config->get_status(dev)
& ~VIRTIO_CONFIG_S_DRIVER);
drv->remove(dev); drv->remove(dev);
/* Driver should have reset device. */
BUG_ON(dev->config->get_status(dev));
/* Acknowledge the device's existence again. */
add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
return 0; return 0;
} }
...@@ -130,6 +134,10 @@ int register_virtio_device(struct virtio_device *dev) ...@@ -130,6 +134,10 @@ int register_virtio_device(struct virtio_device *dev)
dev->dev.bus = &virtio_bus; dev->dev.bus = &virtio_bus;
sprintf(dev->dev.bus_id, "%u", dev->index); 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. */ /* Acknowledge that we've seen the device. */
add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE); add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
......
...@@ -173,16 +173,6 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) ...@@ -173,16 +173,6 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head)
vq->num_free++; 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) static inline bool more_used(const struct vring_virtqueue *vq)
{ {
return vq->last_used_idx != vq->vring.used->idx; return vq->last_used_idx != vq->vring.used->idx;
...@@ -278,7 +268,6 @@ static struct virtqueue_ops vring_vq_ops = { ...@@ -278,7 +268,6 @@ static struct virtqueue_ops vring_vq_ops = {
.kick = vring_kick, .kick = vring_kick,
.disable_cb = vring_disable_cb, .disable_cb = vring_disable_cb,
.enable_cb = vring_enable_cb, .enable_cb = vring_enable_cb,
.shutdown = vring_shutdown,
}; };
struct virtqueue *vring_new_virtqueue(unsigned int num, struct virtqueue *vring_new_virtqueue(unsigned int num,
......
...@@ -45,9 +45,6 @@ struct virtqueue ...@@ -45,9 +45,6 @@ struct virtqueue
* vq: the struct virtqueue we're talking about. * vq: the struct virtqueue we're talking about.
* This returns "false" (and doesn't re-enable) if there are pending * This returns "false" (and doesn't re-enable) if there are pending
* buffers in the queue, to avoid a race. * 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 rules are straightforward: the driver is responsible for
* locking. No two operations may be invoked simultaneously. * locking. No two operations may be invoked simultaneously.
...@@ -67,8 +64,6 @@ struct virtqueue_ops { ...@@ -67,8 +64,6 @@ struct virtqueue_ops {
void (*disable_cb)(struct virtqueue *vq); void (*disable_cb)(struct virtqueue *vq);
bool (*enable_cb)(struct virtqueue *vq); bool (*enable_cb)(struct virtqueue *vq);
void (*shutdown)(struct virtqueue *vq);
}; };
/** /**
......
...@@ -43,6 +43,9 @@ struct virtio_device; ...@@ -43,6 +43,9 @@ struct virtio_device;
* @set_status: write the status byte * @set_status: write the status byte
* vdev: the virtio_device * vdev: the virtio_device
* status: the new status byte * 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. * @find_vq: find a virtqueue and instantiate it.
* vdev: the virtio_device * vdev: the virtio_device
* index: the 0-based virtqueue number in case there's more than one. * index: the 0-based virtqueue number in case there's more than one.
...@@ -59,6 +62,7 @@ struct virtio_config_ops ...@@ -59,6 +62,7 @@ struct virtio_config_ops
const void *buf, unsigned len); const void *buf, unsigned len);
u8 (*get_status)(struct virtio_device *vdev); u8 (*get_status)(struct virtio_device *vdev);
void (*set_status)(struct virtio_device *vdev, u8 status); void (*set_status)(struct virtio_device *vdev, u8 status);
void (*reset)(struct virtio_device *vdev);
struct virtqueue *(*find_vq)(struct virtio_device *vdev, struct virtqueue *(*find_vq)(struct virtio_device *vdev,
unsigned index, unsigned index,
void (*callback)(struct virtqueue *)); void (*callback)(struct virtqueue *));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment