Re: [PATCH v4 13/25] virtio_console: enable VQs early
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: 2014-10-20 13:33:27
Also in:
kvm, linux-s390, linux-scsi, lkml, virtualization
On Mon, Oct 20, 2014 at 02:42:23PM +0200, Cornelia Huck wrote:
quoted hunk ↗ jump to hunk
On Mon, 20 Oct 2014 13:07:50 +0100 Thomas Graf [off-list ref] wrote:quoted
On 10/13/14 at 10:50am, Michael S. Tsirkin wrote:quoted
virtio spec requires drivers to set DRIVER_OK before using VQs. This is set automatically after probe returns, virtio console violated this rule by adding inbufs, which causes the VQ to be used directly within probe. To fix, call virtio_device_ready before using VQs. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- drivers/char/virtio_console.c | 2 ++ 1 file changed, 2 insertions(+)diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index b585b47..6ebe8f6 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c@@ -1449,6 +1449,8 @@ static int add_port(struct ports_device *portdev, u32 id) spin_lock_init(&port->outvq_lock); init_waitqueue_head(&port->waitqueue); + virtio_device_ready(portdev->vdev); + /* Fill the in_vq with buffers so the host can send us data. */ nr_added_bufs = fill_queue(port->in_vq, &port->inbuf_lock); if (!nr_added_bufs) {Seems like probe and add_port() now both set VIRTIO_CONFIG_S_DRIVER_OKI think we need to set this in the probe function instead, otherwise we fail for multiqueue (which also wants to use the control queue early). Completely untested patch below; I can send this with proper s-o-b if it helps.diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index bfa6400..cf7a561 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c@@ -1449,8 +1449,6 @@ static int add_port(struct ports_device *portdev, u32 id) spin_lock_init(&port->outvq_lock); init_waitqueue_head(&port->waitqueue); - virtio_device_ready(portdev->vdev); - /* Fill the in_vq with buffers so the host can send us data. */ nr_added_bufs = fill_queue(port->in_vq, &port->inbuf_lock); if (!nr_added_bufs) {@@ -2026,6 +2024,8 @@ static int virtcons_probe(struct virtio_device *vdev) spin_lock_init(&portdev->ports_lock); INIT_LIST_HEAD(&portdev->ports); + virtio_device_ready(portdev->vdev); + if (multiport) { unsigned int nr_added_bufs;
I wanted to set DRIVER_OK as late as possible, to both reduce the chance it can fail after DRIVER_OK and to reduce the risk of introducing a regression since old qemu might only start sending interrupts after DRIVER_OK is set. So I wanted everything completely initialized before DRIVER_OK. You patch makes sense to me since nothing can fail, and we won't get interrupts before we add inbufs. Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Testig will report shortly, pls send with sob line. -- MST