Re: [Qemu-devel] [PATCH V2 RFT] vhost_net: don't set backend for the uninitialized virtqueue

[Jason Wang]

On 2019/3/25 下午8:32, Michael S. Tsirkin wrote:
On Mon, Mar 25, 2019 at 11:56:13AM +0800, Jason Wang wrote:
We used to set backend unconditionally, this won't work for some
guests (e.g windows driver) who may not initialize all virtqueues. For
kernel backend, this will fail since it may try to validate the rings
during setting backend.

Fixing this by simply skipping the backend set when we find desc is
not ready.

Signed-off-by: Jason Wang <jasow...@redhat.com>
---
  hw/net/vhost_net.c | 15 +++++++++++++++
  1 file changed, 15 insertions(+)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index be3cc88370..04fd924d15 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -221,6 +221,7 @@ static int vhost_net_start_one(struct vhost_net *net,
                                 VirtIODevice *dev)
  {
      struct vhost_vring_file file = { };
+    hwaddr a;
      int r;
  
      net->dev.nvqs = 2;
@@ -244,6 +245,13 @@ static int vhost_net_start_one(struct vhost_net *net,
          qemu_set_fd_handler(net->backend, NULL, NULL, NULL);
          file.fd = net->backend;
          for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
+            a = virtio_queue_get_desc_addr(dev,
+                                           net->dev.vq_index +
+                                           file.index);
+            if (a == 0) {
+                /* Queue might not be ready for start */
+                continue;
+            }
              r = vhost_net_set_backend(&net->dev, &file);
              if (r < 0) {
                  r = -errno;
@@ -256,6 +264,13 @@ fail:
      file.fd = -1;
      if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
          while (file.index-- > 0) {
+            a = virtio_queue_get_desc_addr(dev,
+                                           net->dev.vq_index +
+                                           file.index);
+            if (a == 0) {
+                /* Queue might not be ready for start */
+                continue;
+            }
              int r = vhost_net_set_backend(&net->dev, &file);
              assert(r >= 0);
          }

I think we want an API that explicitly says "queue is enabled".
For 0.X it will return !!addr. For 1.X it will return enabled.


For 1.x, desc.addr won't be set until queue_enabled is set through 
virtio_queue_set_vrings(). And it looks to me ccw did something similar 
of  CMD_SET_VQ.

So we're ok actually?

Thanks




At the moment enabled seems to be saved but ignored at least
in case of virtio-pci and I think that's the real bug.


--
2.19.1