[Qemu-devel] [PATCH 0/3] virtio/vhost: Add checks for uninitialized VQs

[Qemu-devel] [PATCH 0/3] virtio/vhost: Add checks for uninitialized VQs

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

Hi folks,

This series adds a virtio_queue_valid() for use by virtio-pci code in
order to prevent opreations upon uninitialized VQs, that is currently
expected to occur during seabios setup of virtio-scsi.

This also includes a vhost specific check for uninitialized VQs in
vhost_verify_ring_mappings() to avoid this same case.

Please review.

--nab

Michael S. Tsirkin (1):
  virtio: add API to check that ring is setup

Nicholas Bellinger (2):
  virtio-pci: Add virtio_queue_valid checks ahead of
    virtio_queue_get_num
  vhost: Check+skip uninitialized VQs in vhost_verify_ring_mappings

 hw/vhost.c      |    3 +++
 hw/virtio-pci.c |   27 +++++++++++++++++++++++++++
 hw/virtio.c     |    5 +++++
 hw/virtio.h     |    1 +
 4 files changed, 36 insertions(+), 0 deletions(-)

-- 
1.7.2.5

[Qemu-devel] [PATCH 1/3] virtio: add API to check that ring is setup

[Nicholas A. Bellinger]

From: Michael S. Tsirkin <mst@redhat.com>

virtio scsi makes it legal to only setup a subset of rings.  The only
way to detect the ring is setup seems to be to check whether PA was
written to.  Add API to do this, and teach code to use it instead of
checking hardware queue size.

(nab: use .vring.desc instead of .vring.pa)

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Cc: Asias He <asias@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 hw/virtio.c |    5 +++++
 hw/virtio.h |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/hw/virtio.c b/hw/virtio.c
index 26fbc79..65ba253 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -651,6 +651,11 @@ int virtio_queue_get_num(VirtIODevice *vdev, int n)
     return vdev->vq[n].vring.num;
 }
 
+bool virtio_queue_valid(VirtIODevice *vdev, int n)
+{
+    return vdev->vq[n].vring.num && vdev->vq[n].vring.desc;
+}
+
 int virtio_queue_get_id(VirtQueue *vq)
 {
     VirtIODevice *vdev = vq->vdev;
diff --git a/hw/virtio.h b/hw/virtio.h
index fdbe931..3086798 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -227,6 +227,7 @@ void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data);
 void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr);
 hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n);
 int virtio_queue_get_num(VirtIODevice *vdev, int n);
+bool virtio_queue_valid(VirtIODevice *vdev, int n);
 void virtio_queue_notify(VirtIODevice *vdev, int n);
 uint16_t virtio_queue_vector(VirtIODevice *vdev, int n);
 void virtio_queue_set_vector(VirtIODevice *vdev, int n, uint16_t vector);
-- 
1.7.2.5

[Qemu-devel] [PATCH 2/3] virtio-pci: Add virtio_queue_valid checks ahead of virtio_queue_get_num

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds a number of virtio_queue_valid() checks to virtio-pci
ahead of virtio_queue_get_num() usage in order to skip operation upon
the detection of an uninitialized VQ.

There is one exception in virtio_ioport_read():VIRTIO_PCI_QUEUE_NUM,
where virtio_queue_get_num() may still be called without a valid
vdev->vq[n].vring.desc physical address.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Asias He <asias@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 hw/virtio-pci.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 0d67b84..231ca0c 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -211,6 +211,9 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
     }
 
     for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
+        if (!virtio_queue_valid(proxy->vdev, n)) {
+            continue;
+        }
         if (!virtio_queue_get_num(proxy->vdev, n)) {
             continue;
         }
@@ -225,6 +228,9 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
 
 assign_error:
     while (--n >= 0) {
+        if (!virtio_queue_valid(proxy->vdev, n)) {
+            continue;
+        }
         if (!virtio_queue_get_num(proxy->vdev, n)) {
             continue;
         }
@@ -246,6 +252,9 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
     }
 
     for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
+        if (!virtio_queue_valid(proxy->vdev, n)) {
+            continue;
+        }
         if (!virtio_queue_get_num(proxy->vdev, n)) {
             continue;
         }
@@ -546,6 +555,9 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
     MSIMessage msg;
 
     for (queue_no = 0; queue_no < nvqs; queue_no++) {
+        if (!virtio_queue_valid(vdev, queue_no)) {
+            continue;
+        }
         if (!virtio_queue_get_num(vdev, queue_no)) {
             break;
         }
@@ -593,6 +605,9 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
     int queue_no;
 
     for (queue_no = 0; queue_no < nvqs; queue_no++) {
+        if (!virtio_queue_valid(vdev, queue_no)) {
+            continue;
+        }
         if (!virtio_queue_get_num(vdev, queue_no)) {
             break;
         }
@@ -665,6 +680,9 @@ static int kvm_virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector,
     int ret, queue_no;
 
     for (queue_no = 0; queue_no < proxy->nvqs_with_notifiers; queue_no++) {
+        if (!virtio_queue_valid(vdev, queue_no)) {
+            continue;
+        }
         if (!virtio_queue_get_num(vdev, queue_no)) {
             break;
         }
@@ -695,6 +713,9 @@ static void kvm_virtio_pci_vector_mask(PCIDevice *dev, unsigned vector)
     int queue_no;
 
     for (queue_no = 0; queue_no < proxy->nvqs_with_notifiers; queue_no++) {
+        if (!virtio_queue_valid(vdev, queue_no)) {
+            continue;
+        }
         if (!virtio_queue_get_num(vdev, queue_no)) {
             break;
         }
@@ -717,6 +738,9 @@ static void kvm_virtio_pci_vector_poll(PCIDevice *dev,
     VirtQueue *vq;
 
     for (queue_no = 0; queue_no < proxy->nvqs_with_notifiers; queue_no++) {
+        if (!virtio_queue_valid(vdev, queue_no)) {
+            continue;
+        }
         if (!virtio_queue_get_num(vdev, queue_no)) {
             break;
         }
@@ -790,6 +814,9 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
     }
 
     for (n = 0; n < nvqs; n++) {
+        if (!virtio_queue_valid(vdev, n)) {
+            continue;
+        }
         if (!virtio_queue_get_num(vdev, n)) {
             break;
         }
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH 2/3] virtio-pci: Add virtio_queue_valid checks ahead of virtio_queue_get_num

[Michael S. Tsirkin]

On Fri, Mar 29, 2013 at 04:33:11AM +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> This patch adds a number of virtio_queue_valid() checks to virtio-pci
> ahead of virtio_queue_get_num() usage in order to skip operation upon
> the detection of an uninitialized VQ.
> 
> There is one exception in virtio_ioport_read():VIRTIO_PCI_QUEUE_NUM,
> where virtio_queue_get_num() may still be called without a valid
> vdev->vq[n].vring.desc physical address.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Asias He <asias@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>

Makes sense. Minor nit: virtio_queue_valid calls virtio_queue_get_num
internally, so we can drop it everywhere we know queue is valid.

> ---
>  hw/virtio-pci.c |   27 +++++++++++++++++++++++++++
>  1 files changed, 27 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 0d67b84..231ca0c 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -211,6 +211,9 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
>      }
>  
>      for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
> +        if (!virtio_queue_valid(proxy->vdev, n)) {
> +            continue;
> +        }
>          if (!virtio_queue_get_num(proxy->vdev, n)) {
>              continue;
>          }
> @@ -225,6 +228,9 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
>  
>  assign_error:
>      while (--n >= 0) {
> +        if (!virtio_queue_valid(proxy->vdev, n)) {
> +            continue;
> +        }
>          if (!virtio_queue_get_num(proxy->vdev, n)) {
>              continue;
>          }
> @@ -246,6 +252,9 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
>      }
>  
>      for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
> +        if (!virtio_queue_valid(proxy->vdev, n)) {
> +            continue;
> +        }
>          if (!virtio_queue_get_num(proxy->vdev, n)) {
>              continue;
>          }
> @@ -546,6 +555,9 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
>      MSIMessage msg;
>  
>      for (queue_no = 0; queue_no < nvqs; queue_no++) {
> +        if (!virtio_queue_valid(vdev, queue_no)) {
> +            continue;
> +        }
>          if (!virtio_queue_get_num(vdev, queue_no)) {
>              break;
>          }
> @@ -593,6 +605,9 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
>      int queue_no;
>  
>      for (queue_no = 0; queue_no < nvqs; queue_no++) {
> +        if (!virtio_queue_valid(vdev, queue_no)) {
> +            continue;
> +        }
>          if (!virtio_queue_get_num(vdev, queue_no)) {
>              break;
>          }
> @@ -665,6 +680,9 @@ static int kvm_virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector,
>      int ret, queue_no;
>  
>      for (queue_no = 0; queue_no < proxy->nvqs_with_notifiers; queue_no++) {
> +        if (!virtio_queue_valid(vdev, queue_no)) {
> +            continue;
> +        }
>          if (!virtio_queue_get_num(vdev, queue_no)) {
>              break;
>          }
> @@ -695,6 +713,9 @@ static void kvm_virtio_pci_vector_mask(PCIDevice *dev, unsigned vector)
>      int queue_no;
>  
>      for (queue_no = 0; queue_no < proxy->nvqs_with_notifiers; queue_no++) {
> +        if (!virtio_queue_valid(vdev, queue_no)) {
> +            continue;
> +        }
>          if (!virtio_queue_get_num(vdev, queue_no)) {
>              break;
>          }
> @@ -717,6 +738,9 @@ static void kvm_virtio_pci_vector_poll(PCIDevice *dev,
>      VirtQueue *vq;
>  
>      for (queue_no = 0; queue_no < proxy->nvqs_with_notifiers; queue_no++) {
> +        if (!virtio_queue_valid(vdev, queue_no)) {
> +            continue;
> +        }
>          if (!virtio_queue_get_num(vdev, queue_no)) {
>              break;
>          }
> @@ -790,6 +814,9 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
>      }
>  
>      for (n = 0; n < nvqs; n++) {
> +        if (!virtio_queue_valid(vdev, n)) {
> +            continue;
> +        }
>          if (!virtio_queue_get_num(vdev, n)) {
>              break;
>          }
> -- 
> 1.7.2.5

Re: [Qemu-devel] [PATCH 2/3] virtio-pci: Add virtio_queue_valid checks ahead of virtio_queue_get_num

[Nicholas A. Bellinger]

On Sun, 2013-03-31 at 10:37 +0300, Michael S. Tsirkin wrote:
> On Fri, Mar 29, 2013 at 04:33:11AM +0000, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > 
> > This patch adds a number of virtio_queue_valid() checks to virtio-pci
> > ahead of virtio_queue_get_num() usage in order to skip operation upon
> > the detection of an uninitialized VQ.
> > 
> > There is one exception in virtio_ioport_read():VIRTIO_PCI_QUEUE_NUM,
> > where virtio_queue_get_num() may still be called without a valid
> > vdev->vq[n].vring.desc physical address.
> > 
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Asias He <asias@redhat.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> Makes sense. Minor nit: virtio_queue_valid calls virtio_queue_get_num
> internally, so we can drop it everywhere we know queue is valid.
> 

Yes, of course.  This includes every location in virtio-pci.c below..

Including for patch-v2.

> > ---
> >  hw/virtio-pci.c |   27 +++++++++++++++++++++++++++
> >  1 files changed, 27 insertions(+), 0 deletions(-)
> > 
> > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> > index 0d67b84..231ca0c 100644
> > --- a/hw/virtio-pci.c
> > +++ b/hw/virtio-pci.c
> > @@ -211,6 +211,9 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
> >      }
> >  
> >      for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
> > +        if (!virtio_queue_valid(proxy->vdev, n)) {
> > +            continue;
> > +        }
> >          if (!virtio_queue_get_num(proxy->vdev, n)) {
> >              continue;
> >          }
> > @@ -225,6 +228,9 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
> >  
> >  assign_error:
> >      while (--n >= 0) {
> > +        if (!virtio_queue_valid(proxy->vdev, n)) {
> > +            continue;
> > +        }
> >          if (!virtio_queue_get_num(proxy->vdev, n)) {
> >              continue;
> >          }
> > @@ -246,6 +252,9 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
> >      }
> >  
> >      for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
> > +        if (!virtio_queue_valid(proxy->vdev, n)) {
> > +            continue;
> > +        }
> >          if (!virtio_queue_get_num(proxy->vdev, n)) {
> >              continue;
> >          }
> > @@ -546,6 +555,9 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
> >      MSIMessage msg;
> >  
> >      for (queue_no = 0; queue_no < nvqs; queue_no++) {
> > +        if (!virtio_queue_valid(vdev, queue_no)) {
> > +            continue;
> > +        }
> >          if (!virtio_queue_get_num(vdev, queue_no)) {
> >              break;
> >          }
> > @@ -593,6 +605,9 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
> >      int queue_no;
> >  
> >      for (queue_no = 0; queue_no < nvqs; queue_no++) {
> > +        if (!virtio_queue_valid(vdev, queue_no)) {
> > +            continue;
> > +        }
> >          if (!virtio_queue_get_num(vdev, queue_no)) {
> >              break;
> >          }
> > @@ -665,6 +680,9 @@ static int kvm_virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector,
> >      int ret, queue_no;
> >  
> >      for (queue_no = 0; queue_no < proxy->nvqs_with_notifiers; queue_no++) {
> > +        if (!virtio_queue_valid(vdev, queue_no)) {
> > +            continue;
> > +        }
> >          if (!virtio_queue_get_num(vdev, queue_no)) {
> >              break;
> >          }
> > @@ -695,6 +713,9 @@ static void kvm_virtio_pci_vector_mask(PCIDevice *dev, unsigned vector)
> >      int queue_no;
> >  
> >      for (queue_no = 0; queue_no < proxy->nvqs_with_notifiers; queue_no++) {
> > +        if (!virtio_queue_valid(vdev, queue_no)) {
> > +            continue;
> > +        }
> >          if (!virtio_queue_get_num(vdev, queue_no)) {
> >              break;
> >          }
> > @@ -717,6 +738,9 @@ static void kvm_virtio_pci_vector_poll(PCIDevice *dev,
> >      VirtQueue *vq;
> >  
> >      for (queue_no = 0; queue_no < proxy->nvqs_with_notifiers; queue_no++) {
> > +        if (!virtio_queue_valid(vdev, queue_no)) {
> > +            continue;
> > +        }
> >          if (!virtio_queue_get_num(vdev, queue_no)) {
> >              break;
> >          }
> > @@ -790,6 +814,9 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
> >      }
> >  
> >      for (n = 0; n < nvqs; n++) {
> > +        if (!virtio_queue_valid(vdev, n)) {
> > +            continue;
> > +        }
> >          if (!virtio_queue_get_num(vdev, n)) {
> >              break;
> >          }
> > -- 
> > 1.7.2.5
> --
> To unsubscribe from this list: send the line "unsubscribe target-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Qemu-devel] [PATCH 3/3] vhost: Check+skip uninitialized VQs in vhost_verify_ring_mappings

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

With the virtio_queue_valid() checks in place to skip uninitialized VQs
within virtio-pci code, go ahead and skip the same uninitialized VQs
during vhost_verify_ring_mappings().

Note this patch does not prevent vhost_virtqueue_start() from executing
by checking virtio_queue_valid(), as other logic during seabios ->
virtio-scsi LLD guest hand-off appears to depend upon this execution.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Asias He <asias@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 hw/vhost.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/vhost.c b/hw/vhost.c
index 4d6aee3..3a71aee 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -314,6 +314,9 @@ static int vhost_verify_ring_mappings(struct vhost_dev *dev,
         hwaddr l;
         void *p;
 
+        if (!vq->ring_phys || !vq->ring_size) {
+            continue;
+        }
         if (!ranges_overlap(start_addr, size, vq->ring_phys, vq->ring_size)) {
             continue;
         }
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH 3/3] vhost: Check+skip uninitialized VQs in vhost_verify_ring_mappings

[Michael S. Tsirkin]

On Fri, Mar 29, 2013 at 04:33:12AM +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> With the virtio_queue_valid() checks in place to skip uninitialized VQs
> within virtio-pci code, go ahead and skip the same uninitialized VQs
> during vhost_verify_ring_mappings().
> 
> Note this patch does not prevent vhost_virtqueue_start() from executing
> by checking virtio_queue_valid(), as other logic during seabios ->
> virtio-scsi LLD guest hand-off appears to depend upon this execution.

Weird.
cpu_physical_memory_map only succeeds for PA==0 by chance,
we really should not depend on this.
So the right thing really should be to skip vhost_virtqueue_start IMHO,
maybe add an explicit valid flag in vhost_virtqueue
so vhost_verify_ring_mappings can check it.
What exactly does it do that is needed?

> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Asias He <asias@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>  hw/vhost.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/vhost.c b/hw/vhost.c
> index 4d6aee3..3a71aee 100644
> --- a/hw/vhost.c
> +++ b/hw/vhost.c
> @@ -314,6 +314,9 @@ static int vhost_verify_ring_mappings(struct vhost_dev *dev,
>          hwaddr l;
>          void *p;
>  
> +        if (!vq->ring_phys || !vq->ring_size) {
> +            continue;
> +        }
>          if (!ranges_overlap(start_addr, size, vq->ring_phys, vq->ring_size)) {
>              continue;
>          }
> -- 
> 1.7.2.5

Re: [Qemu-devel] [PATCH 3/3] vhost: Check+skip uninitialized VQs in vhost_verify_ring_mappings

[Nicholas A. Bellinger]

On Sun, 2013-03-31 at 10:45 +0300, Michael S. Tsirkin wrote:
> On Fri, Mar 29, 2013 at 04:33:12AM +0000, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > 
> > With the virtio_queue_valid() checks in place to skip uninitialized VQs
> > within virtio-pci code, go ahead and skip the same uninitialized VQs
> > during vhost_verify_ring_mappings().
> > 
> > Note this patch does not prevent vhost_virtqueue_start() from executing
> > by checking virtio_queue_valid(), as other logic during seabios ->
> > virtio-scsi LLD guest hand-off appears to depend upon this execution.
> 
> Weird.
> cpu_physical_memory_map only succeeds for PA==0 by chance,
> we really should not depend on this.
> So the right thing really should be to skip vhost_virtqueue_start IMHO,
> maybe add an explicit valid flag in vhost_virtqueue
> so vhost_verify_ring_mappings can check it.
> What exactly does it do that is needed?
> 

So the issue with virtio_queue_valid() preventing
vhost_virtqueue_start() execution in the original patch was that
vhost_virtqueue_stop() was missing a matching virtio_queue_valid() call,
which ended up triggering a bad ram pointer during subsequent
cpu_physical_memory_unmap() calls to non-existent virtio queue memory..

With the matching virtio_queue_valid() call in place preventing
vhost_virtqueue_stop() when vhost_virtqueue_start() is skipped for an
uninitialized VQ, a explicit valid flag should not be necessary.

--nab


> > 
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Asias He <asias@redhat.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > ---
> >  hw/vhost.c |    3 +++
> >  1 files changed, 3 insertions(+), 0 deletions(-)
> > 
> > diff --git a/hw/vhost.c b/hw/vhost.c
> > index 4d6aee3..3a71aee 100644
> > --- a/hw/vhost.c
> > +++ b/hw/vhost.c
> > @@ -314,6 +314,9 @@ static int vhost_verify_ring_mappings(struct vhost_dev *dev,
> >          hwaddr l;
> >          void *p;
> >  
> > +        if (!vq->ring_phys || !vq->ring_size) {
> > +            continue;
> > +        }
> >          if (!ranges_overlap(start_addr, size, vq->ring_phys, vq->ring_size)) {
> >              continue;
> >          }
> > -- 
> > 1.7.2.5
> --
> To unsubscribe from this list: send the line "unsubscribe target-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Qemu-devel] [PATCH 0/3] virtio/vhost: Add checks for uninitialized VQs

[Michael S. Tsirkin]

On Fri, Mar 29, 2013 at 04:33:09AM +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> Hi folks,
> 
> This series adds a virtio_queue_valid() for use by virtio-pci code in
> order to prevent opreations upon uninitialized VQs, that is currently
> expected to occur during seabios setup of virtio-scsi.
> 
> This also includes a vhost specific check for uninitialized VQs in
> vhost_verify_ring_mappings() to avoid this same case.
> 
> Please review.
> 
> --nab

Okay, and does this fix the failures in vhost_verify_ring_mappings
that you've observed?

> Michael S. Tsirkin (1):
>   virtio: add API to check that ring is setup
> 
> Nicholas Bellinger (2):
>   virtio-pci: Add virtio_queue_valid checks ahead of
>     virtio_queue_get_num
>   vhost: Check+skip uninitialized VQs in vhost_verify_ring_mappings
> 
>  hw/vhost.c      |    3 +++
>  hw/virtio-pci.c |   27 +++++++++++++++++++++++++++
>  hw/virtio.c     |    5 +++++
>  hw/virtio.h     |    1 +
>  4 files changed, 36 insertions(+), 0 deletions(-)
> 
> -- 
> 1.7.2.5

Re: [Qemu-devel] [PATCH 0/3] virtio/vhost: Add checks for uninitialized VQs

[Nicholas A. Bellinger]

On Sun, 2013-03-31 at 10:46 +0300, Michael S. Tsirkin wrote:
> On Fri, Mar 29, 2013 at 04:33:09AM +0000, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > 
> > Hi folks,
> > 
> > This series adds a virtio_queue_valid() for use by virtio-pci code in
> > order to prevent opreations upon uninitialized VQs, that is currently
> > expected to occur during seabios setup of virtio-scsi.
> > 
> > This also includes a vhost specific check for uninitialized VQs in
> > vhost_verify_ring_mappings() to avoid this same case.
> > 
> > Please review.
> > 
> > --nab
> 
> Okay, and does this fix the failures in vhost_verify_ring_mappings
> that you've observed?
> 

Unfortunately, no.  I've done some more digging and will follow up with
additional details on the original thread shortly..

--nab