3.16.60-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: "Michael S. Tsirkin" mst@redhat.com
commit a7a69ec0d8e4a58be7db88d33cbfa2912807bb2b upstream.
Console driver is out of spec. The spec says: A driver MUST NOT decrement the available idx on a live virtqueue (ie. there is no way to “unexpose” buffers). and it does exactly that by trying to detach unused buffers without doing a device reset first.
Defer detaching the buffers until device unplug.
Of course this means we might get an interrupt for a vq without an attached port now. Handle that by discarding the consumed buffer.
Reported-by: Tiwei Bie tiwei.bie@intel.com Fixes: b3258ff1d6 ("virtio: Decrement avail idx on buffer detach") Signed-off-by: Michael S. Tsirkin mst@redhat.com Signed-off-by: Ben Hutchings ben@decadent.org.uk --- drivers/char/virtio_console.c | 49 +++++++++++++++++------------------ 1 file changed, 24 insertions(+), 25 deletions(-)
--- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1398,7 +1398,6 @@ static int add_port(struct ports_device { char debugfs_name[16]; struct port *port; - struct port_buffer *buf; dev_t devt; unsigned int nr_added_bufs; int err; @@ -1509,8 +1508,6 @@ static int add_port(struct ports_device return 0;
free_inbufs: - while ((buf = virtqueue_detach_unused_buf(port->in_vq))) - free_buf(buf, true); free_device: device_destroy(pdrvdata.class, port->dev->devt); free_cdev: @@ -1535,34 +1532,14 @@ static void remove_port(struct kref *kre
static void remove_port_data(struct port *port) { - struct port_buffer *buf; - spin_lock_irq(&port->inbuf_lock); /* Remove unused data this port might have received. */ discard_port_data(port); spin_unlock_irq(&port->inbuf_lock);
- /* Remove buffers we queued up for the Host to send us data in. */ - do { - spin_lock_irq(&port->inbuf_lock); - buf = virtqueue_detach_unused_buf(port->in_vq); - spin_unlock_irq(&port->inbuf_lock); - if (buf) - free_buf(buf, true); - } while (buf); - spin_lock_irq(&port->outvq_lock); reclaim_consumed_buffers(port); spin_unlock_irq(&port->outvq_lock); - - /* Free pending buffers from the out-queue. */ - do { - spin_lock_irq(&port->outvq_lock); - buf = virtqueue_detach_unused_buf(port->out_vq); - spin_unlock_irq(&port->outvq_lock); - if (buf) - free_buf(buf, true); - } while (buf); }
/* @@ -1783,13 +1760,24 @@ static void control_work_handler(struct spin_unlock(&portdev->c_ivq_lock); }
+static void flush_bufs(struct virtqueue *vq, bool can_sleep) +{ + struct port_buffer *buf; + unsigned int len; + + while ((buf = virtqueue_get_buf(vq, &len))) + free_buf(buf, can_sleep); +} + static void out_intr(struct virtqueue *vq) { struct port *port;
port = find_port_by_vq(vq->vdev->priv, vq); - if (!port) + if (!port) { + flush_bufs(vq, false); return; + }
wake_up_interruptible(&port->waitqueue); } @@ -1800,8 +1788,10 @@ static void in_intr(struct virtqueue *vq unsigned long flags;
port = find_port_by_vq(vq->vdev->priv, vq); - if (!port) + if (!port) { + flush_bufs(vq, false); return; + }
spin_lock_irqsave(&port->inbuf_lock, flags); port->inbuf = get_inbuf(port); @@ -1976,6 +1966,15 @@ static const struct file_operations port
static void remove_vqs(struct ports_device *portdev) { + struct virtqueue *vq; + + virtio_device_for_each_vq(portdev->vdev, vq) { + struct port_buffer *buf; + + flush_bufs(vq, true); + while ((buf = virtqueue_detach_unused_buf(vq))) + free_buf(buf, true); + } portdev->vdev->config->del_vqs(portdev->vdev); kfree(portdev->in_vqs); kfree(portdev->out_vqs);