* [Qemu-devel] [PATCH-v2 0/3] virtio/vhost: Add checks for uninitialized VQs
@ 2013-04-01 23:58 ` Nicholas A. Bellinger
0 siblings, 0 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2013-04-01 23:58 UTC (permalink / raw
To: target-devel
Cc: kvm-devel, Michael S. Tsirkin, qemu-devel, Nicholas Bellinger,
lf-virt, Anthony Liguori, Stefan Hajnoczi, Paolo Bonzini,
Asias He
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, which is currently
expected to occur during seabios setup of virtio-scsi with in-flight
vhost-scsi-pci device code.
On the vhost side, it also adds virtio_queue_valid() sanity checks in
vhost_virtqueue_[start,stop]() and vhost_verify_ring_mappings() in order
to skip the same uninitialized VQs.
Changes from v1:
- Remove now unnecessary virtio_queue_get_num() calls in virtio-pci.c
- Add virtio_queue_valid() calls in vhost_virtqueue_[start,stop]()
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: Skip uninitialized VQs in vhost_virtqueue_[start,stop]
hw/vhost.c | 12 ++++++++++++
hw/virtio-pci.c | 34 +++++++++++++++-------------------
hw/virtio.c | 5 +++++
hw/virtio.h | 1 +
4 files changed, 33 insertions(+), 19 deletions(-)
--
1.7.2.5
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH-v2 0/3] virtio/vhost: Add checks for uninitialized VQs
@ 2013-04-01 23:58 ` Nicholas A. Bellinger
0 siblings, 0 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2013-04-01 23:58 UTC (permalink / raw
To: target-devel
Cc: kvm-devel, Michael S. Tsirkin, qemu-devel, lf-virt,
Anthony Liguori, Stefan Hajnoczi, Paolo Bonzini
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, which is currently
expected to occur during seabios setup of virtio-scsi with in-flight
vhost-scsi-pci device code.
On the vhost side, it also adds virtio_queue_valid() sanity checks in
vhost_virtqueue_[start,stop]() and vhost_verify_ring_mappings() in order
to skip the same uninitialized VQs.
Changes from v1:
- Remove now unnecessary virtio_queue_get_num() calls in virtio-pci.c
- Add virtio_queue_valid() calls in vhost_virtqueue_[start,stop]()
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: Skip uninitialized VQs in vhost_virtqueue_[start,stop]
hw/vhost.c | 12 ++++++++++++
hw/virtio-pci.c | 34 +++++++++++++++-------------------
hw/virtio.c | 5 +++++
hw/virtio.h | 1 +
4 files changed, 33 insertions(+), 19 deletions(-)
--
1.7.2.5
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH-v2 1/3] virtio: add API to check that ring is setup
2013-04-01 23:58 ` Nicholas A. Bellinger
@ 2013-04-01 23:58 ` Nicholas A. Bellinger
-1 siblings, 0 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2013-04-01 23:58 UTC (permalink / raw
To: target-devel
Cc: kvm-devel, Michael S. Tsirkin, qemu-devel, lf-virt,
Anthony Liguori, Stefan Hajnoczi, Paolo Bonzini, Asias He
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH-v2 1/3] virtio: add API to check that ring is setup
@ 2013-04-01 23:58 ` Nicholas A. Bellinger
0 siblings, 0 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2013-04-01 23:58 UTC (permalink / raw
To: target-devel
Cc: kvm-devel, Michael S. Tsirkin, qemu-devel, lf-virt,
Anthony Liguori, Stefan Hajnoczi, Paolo Bonzini
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH-v2 2/3] virtio-pci: Add virtio_queue_valid checks ahead of virtio_queue_get_num
2013-04-01 23:58 ` Nicholas A. Bellinger
@ 2013-04-01 23:58 ` Nicholas A. Bellinger
-1 siblings, 0 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2013-04-01 23:58 UTC (permalink / raw
To: target-devel
Cc: kvm-devel, Michael S. Tsirkin, qemu-devel, Nicholas Bellinger,
lf-virt, Anthony Liguori, Stefan Hajnoczi, Paolo Bonzini,
Asias He
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.
v2: Drop now unnecessary virtio_queue_get_num calls (mst)
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 | 34 +++++++++++++++-------------------
1 files changed, 15 insertions(+), 19 deletions(-)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 0d67b84..1369d9a 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -211,10 +211,9 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
}
for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
- if (!virtio_queue_get_num(proxy->vdev, n)) {
+ if (!virtio_queue_valid(proxy->vdev, n)) {
continue;
}
-
r = virtio_pci_set_host_notifier_internal(proxy, n, true, true);
if (r < 0) {
goto assign_error;
@@ -225,10 +224,9 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
assign_error:
while (--n >= 0) {
- if (!virtio_queue_get_num(proxy->vdev, n)) {
+ if (!virtio_queue_valid(proxy->vdev, n)) {
continue;
}
-
r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
assert(r >= 0);
}
@@ -246,10 +244,9 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
}
for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
- if (!virtio_queue_get_num(proxy->vdev, n)) {
+ if (!virtio_queue_valid(proxy->vdev, n)) {
continue;
}
-
r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
assert(r >= 0);
}
@@ -546,8 +543,8 @@ 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_get_num(vdev, queue_no)) {
- break;
+ if (!virtio_queue_valid(vdev, queue_no)) {
+ continue;
}
vector = virtio_queue_vector(vdev, queue_no);
if (vector >= msix_nr_vectors_allocated(dev)) {
@@ -593,8 +590,8 @@ 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_get_num(vdev, queue_no)) {
- break;
+ if (!virtio_queue_valid(vdev, queue_no)) {
+ continue;
}
vector = virtio_queue_vector(vdev, queue_no);
if (vector >= msix_nr_vectors_allocated(dev)) {
@@ -665,8 +662,8 @@ 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_get_num(vdev, queue_no)) {
- break;
+ if (!virtio_queue_valid(vdev, queue_no)) {
+ continue;
}
if (virtio_queue_vector(vdev, queue_no) != vector) {
continue;
@@ -695,8 +692,8 @@ 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_get_num(vdev, queue_no)) {
- break;
+ if (!virtio_queue_valid(vdev, queue_no)) {
+ continue;
}
if (virtio_queue_vector(vdev, queue_no) != vector) {
continue;
@@ -717,8 +714,8 @@ 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_get_num(vdev, queue_no)) {
- break;
+ if (!virtio_queue_valid(vdev, queue_no)) {
+ continue;
}
vector = virtio_queue_vector(vdev, queue_no);
if (vector < vector_start || vector >= vector_end ||
@@ -790,10 +787,9 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
}
for (n = 0; n < nvqs; n++) {
- if (!virtio_queue_get_num(vdev, n)) {
- break;
+ if (!virtio_queue_valid(vdev, n)) {
+ continue;
}
-
r = virtio_pci_set_guest_notifier(d, n, assign,
kvm_msi_via_irqfd_enabled());
if (r < 0) {
--
1.7.2.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH-v2 2/3] virtio-pci: Add virtio_queue_valid checks ahead of virtio_queue_get_num
@ 2013-04-01 23:58 ` Nicholas A. Bellinger
0 siblings, 0 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2013-04-01 23:58 UTC (permalink / raw
To: target-devel
Cc: kvm-devel, Michael S. Tsirkin, qemu-devel, lf-virt,
Anthony Liguori, Stefan Hajnoczi, Paolo Bonzini
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.
v2: Drop now unnecessary virtio_queue_get_num calls (mst)
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 | 34 +++++++++++++++-------------------
1 files changed, 15 insertions(+), 19 deletions(-)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 0d67b84..1369d9a 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -211,10 +211,9 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
}
for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
- if (!virtio_queue_get_num(proxy->vdev, n)) {
+ if (!virtio_queue_valid(proxy->vdev, n)) {
continue;
}
-
r = virtio_pci_set_host_notifier_internal(proxy, n, true, true);
if (r < 0) {
goto assign_error;
@@ -225,10 +224,9 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
assign_error:
while (--n >= 0) {
- if (!virtio_queue_get_num(proxy->vdev, n)) {
+ if (!virtio_queue_valid(proxy->vdev, n)) {
continue;
}
-
r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
assert(r >= 0);
}
@@ -246,10 +244,9 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
}
for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
- if (!virtio_queue_get_num(proxy->vdev, n)) {
+ if (!virtio_queue_valid(proxy->vdev, n)) {
continue;
}
-
r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
assert(r >= 0);
}
@@ -546,8 +543,8 @@ 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_get_num(vdev, queue_no)) {
- break;
+ if (!virtio_queue_valid(vdev, queue_no)) {
+ continue;
}
vector = virtio_queue_vector(vdev, queue_no);
if (vector >= msix_nr_vectors_allocated(dev)) {
@@ -593,8 +590,8 @@ 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_get_num(vdev, queue_no)) {
- break;
+ if (!virtio_queue_valid(vdev, queue_no)) {
+ continue;
}
vector = virtio_queue_vector(vdev, queue_no);
if (vector >= msix_nr_vectors_allocated(dev)) {
@@ -665,8 +662,8 @@ 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_get_num(vdev, queue_no)) {
- break;
+ if (!virtio_queue_valid(vdev, queue_no)) {
+ continue;
}
if (virtio_queue_vector(vdev, queue_no) != vector) {
continue;
@@ -695,8 +692,8 @@ 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_get_num(vdev, queue_no)) {
- break;
+ if (!virtio_queue_valid(vdev, queue_no)) {
+ continue;
}
if (virtio_queue_vector(vdev, queue_no) != vector) {
continue;
@@ -717,8 +714,8 @@ 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_get_num(vdev, queue_no)) {
- break;
+ if (!virtio_queue_valid(vdev, queue_no)) {
+ continue;
}
vector = virtio_queue_vector(vdev, queue_no);
if (vector < vector_start || vector >= vector_end ||
@@ -790,10 +787,9 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
}
for (n = 0; n < nvqs; n++) {
- if (!virtio_queue_get_num(vdev, n)) {
- break;
+ if (!virtio_queue_valid(vdev, n)) {
+ continue;
}
-
r = virtio_pci_set_guest_notifier(d, n, assign,
kvm_msi_via_irqfd_enabled());
if (r < 0) {
--
1.7.2.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH-v2 3/3] vhost: Skip uninitialized VQs in vhost_virtqueue_[start, stop]
2013-04-01 23:58 ` Nicholas A. Bellinger
@ 2013-04-01 23:58 ` Nicholas A. Bellinger
-1 siblings, 0 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2013-04-01 23:58 UTC (permalink / raw
To: target-devel
Cc: kvm-devel, Michael S. Tsirkin, qemu-devel, Nicholas Bellinger,
lf-virt, Anthony Liguori, Stefan Hajnoczi, Paolo Bonzini,
Asias He
From: Nicholas Bellinger <nab@linux-iscsi.org>
This patch adds virtio_queue_valid() checks in vhost_virtqueue_start()
and vhost_virtqueue_stop() to avoid uninitialized VQs during vhost-scsi-pci
seabios operation, where we currently expect only the request VQ to have
been initialized before virtio-scsi LLD guest hand-off.
Also, go ahead and skip the same uninitialized VQs during sanity checks
within vhost_verify_ring_mappings() by checking vq->ring_[phys,size]
directly.
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 | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/hw/vhost.c b/hw/vhost.c
index 4d6aee3..832cc89 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;
}
@@ -645,6 +648,10 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
+ if (!virtio_queue_valid(vdev, idx)) {
+ return 0;
+ }
+
vq->num = state.num = virtio_queue_get_num(vdev, idx);
r = ioctl(dev->control, VHOST_SET_VRING_NUM, &state);
if (r) {
@@ -732,6 +739,11 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
};
int r;
assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
+
+ if (!virtio_queue_valid(vdev, idx)) {
+ return;
+ }
+
r = ioctl(dev->control, VHOST_GET_VRING_BASE, &state);
if (r < 0) {
fprintf(stderr, "vhost VQ %d ring restore failed: %d\n", idx, r);
--
1.7.2.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH-v2 3/3] vhost: Skip uninitialized VQs in vhost_virtqueue_[start,stop]
@ 2013-04-01 23:58 ` Nicholas A. Bellinger
0 siblings, 0 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2013-04-01 23:58 UTC (permalink / raw
To: target-devel
Cc: lf-virt, kvm-devel, qemu-devel, Michael S. Tsirkin,
Stefan Hajnoczi, Paolo Bonzini, Asias He, Anthony Liguori,
Nicholas Bellinger
From: Nicholas Bellinger <nab@linux-iscsi.org>
This patch adds virtio_queue_valid() checks in vhost_virtqueue_start()
and vhost_virtqueue_stop() to avoid uninitialized VQs during vhost-scsi-pci
seabios operation, where we currently expect only the request VQ to have
been initialized before virtio-scsi LLD guest hand-off.
Also, go ahead and skip the same uninitialized VQs during sanity checks
within vhost_verify_ring_mappings() by checking vq->ring_[phys,size]
directly.
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 | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/hw/vhost.c b/hw/vhost.c
index 4d6aee3..832cc89 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;
}
@@ -645,6 +648,10 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
+ if (!virtio_queue_valid(vdev, idx)) {
+ return 0;
+ }
+
vq->num = state.num = virtio_queue_get_num(vdev, idx);
r = ioctl(dev->control, VHOST_SET_VRING_NUM, &state);
if (r) {
@@ -732,6 +739,11 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
};
int r;
assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
+
+ if (!virtio_queue_valid(vdev, idx)) {
+ return;
+ }
+
r = ioctl(dev->control, VHOST_GET_VRING_BASE, &state);
if (r < 0) {
fprintf(stderr, "vhost VQ %d ring restore failed: %d\n", idx, r);
--
1.7.2.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH-v2 3/3] vhost: Skip uninitialized VQs in vhost_virtqueue_[start, stop]
2013-04-01 23:58 ` Nicholas A. Bellinger
` (3 preceding siblings ...)
(?)
@ 2013-04-01 23:58 ` Nicholas A. Bellinger
-1 siblings, 0 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2013-04-01 23:58 UTC (permalink / raw
To: target-devel
Cc: kvm-devel, Michael S. Tsirkin, qemu-devel, lf-virt,
Anthony Liguori, Stefan Hajnoczi, Paolo Bonzini
From: Nicholas Bellinger <nab@linux-iscsi.org>
This patch adds virtio_queue_valid() checks in vhost_virtqueue_start()
and vhost_virtqueue_stop() to avoid uninitialized VQs during vhost-scsi-pci
seabios operation, where we currently expect only the request VQ to have
been initialized before virtio-scsi LLD guest hand-off.
Also, go ahead and skip the same uninitialized VQs during sanity checks
within vhost_verify_ring_mappings() by checking vq->ring_[phys,size]
directly.
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 | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/hw/vhost.c b/hw/vhost.c
index 4d6aee3..832cc89 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;
}
@@ -645,6 +648,10 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
+ if (!virtio_queue_valid(vdev, idx)) {
+ return 0;
+ }
+
vq->num = state.num = virtio_queue_get_num(vdev, idx);
r = ioctl(dev->control, VHOST_SET_VRING_NUM, &state);
if (r) {
@@ -732,6 +739,11 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
};
int r;
assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
+
+ if (!virtio_queue_valid(vdev, idx)) {
+ return;
+ }
+
r = ioctl(dev->control, VHOST_GET_VRING_BASE, &state);
if (r < 0) {
fprintf(stderr, "vhost VQ %d ring restore failed: %d\n", idx, r);
--
1.7.2.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH-v2 0/3] virtio/vhost: Add checks for uninitialized VQs
2013-04-01 23:58 ` Nicholas A. Bellinger
@ 2013-04-02 12:01 ` Michael S. Tsirkin
-1 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-04-02 12:01 UTC (permalink / raw
To: Nicholas A. Bellinger
Cc: kvm-devel, qemu-devel, lf-virt, Anthony Liguori, target-devel,
Stefan Hajnoczi, Paolo Bonzini, Asias He
On Mon, Apr 01, 2013 at 11:58:21PM +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, which is currently
> expected to occur during seabios setup of virtio-scsi with in-flight
> vhost-scsi-pci device code.
>
> On the vhost side, it also adds virtio_queue_valid() sanity checks in
> vhost_virtqueue_[start,stop]() and vhost_verify_ring_mappings() in order
> to skip the same uninitialized VQs.
>
> Changes from v1:
> - Remove now unnecessary virtio_queue_get_num() calls in virtio-pci.c
> - Add virtio_queue_valid() calls in vhost_virtqueue_[start,stop]()
>
> Please review.
>
> --nab
Looks reasonable.
Acked-by: Michael S. Tsirkin <mst@redhat.com>
So - does this fix the issues you saw with vhost-scsi?
> 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: Skip uninitialized VQs in vhost_virtqueue_[start,stop]
>
> hw/vhost.c | 12 ++++++++++++
> hw/virtio-pci.c | 34 +++++++++++++++-------------------
> hw/virtio.c | 5 +++++
> hw/virtio.h | 1 +
> 4 files changed, 33 insertions(+), 19 deletions(-)
>
> --
> 1.7.2.5
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH-v2 0/3] virtio/vhost: Add checks for uninitialized VQs
@ 2013-04-02 12:01 ` Michael S. Tsirkin
0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-04-02 12:01 UTC (permalink / raw
To: Nicholas A. Bellinger
Cc: target-devel, lf-virt, kvm-devel, qemu-devel, Stefan Hajnoczi,
Paolo Bonzini, Asias He, Anthony Liguori
On Mon, Apr 01, 2013 at 11:58:21PM +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, which is currently
> expected to occur during seabios setup of virtio-scsi with in-flight
> vhost-scsi-pci device code.
>
> On the vhost side, it also adds virtio_queue_valid() sanity checks in
> vhost_virtqueue_[start,stop]() and vhost_verify_ring_mappings() in order
> to skip the same uninitialized VQs.
>
> Changes from v1:
> - Remove now unnecessary virtio_queue_get_num() calls in virtio-pci.c
> - Add virtio_queue_valid() calls in vhost_virtqueue_[start,stop]()
>
> Please review.
>
> --nab
Looks reasonable.
Acked-by: Michael S. Tsirkin <mst@redhat.com>
So - does this fix the issues you saw with vhost-scsi?
> 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: Skip uninitialized VQs in vhost_virtqueue_[start,stop]
>
> hw/vhost.c | 12 ++++++++++++
> hw/virtio-pci.c | 34 +++++++++++++++-------------------
> hw/virtio.c | 5 +++++
> hw/virtio.h | 1 +
> 4 files changed, 33 insertions(+), 19 deletions(-)
>
> --
> 1.7.2.5
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH-v2 0/3] virtio/vhost: Add checks for uninitialized VQs
2013-04-01 23:58 ` Nicholas A. Bellinger
` (5 preceding siblings ...)
(?)
@ 2013-04-02 12:01 ` Michael S. Tsirkin
-1 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-04-02 12:01 UTC (permalink / raw
To: Nicholas A. Bellinger
Cc: kvm-devel, qemu-devel, lf-virt, Anthony Liguori, target-devel,
Stefan Hajnoczi, Paolo Bonzini
On Mon, Apr 01, 2013 at 11:58:21PM +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, which is currently
> expected to occur during seabios setup of virtio-scsi with in-flight
> vhost-scsi-pci device code.
>
> On the vhost side, it also adds virtio_queue_valid() sanity checks in
> vhost_virtqueue_[start,stop]() and vhost_verify_ring_mappings() in order
> to skip the same uninitialized VQs.
>
> Changes from v1:
> - Remove now unnecessary virtio_queue_get_num() calls in virtio-pci.c
> - Add virtio_queue_valid() calls in vhost_virtqueue_[start,stop]()
>
> Please review.
>
> --nab
Looks reasonable.
Acked-by: Michael S. Tsirkin <mst@redhat.com>
So - does this fix the issues you saw with vhost-scsi?
> 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: Skip uninitialized VQs in vhost_virtqueue_[start,stop]
>
> hw/vhost.c | 12 ++++++++++++
> hw/virtio-pci.c | 34 +++++++++++++++-------------------
> hw/virtio.c | 5 +++++
> hw/virtio.h | 1 +
> 4 files changed, 33 insertions(+), 19 deletions(-)
>
> --
> 1.7.2.5
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH-v2 0/3] virtio/vhost: Add checks for uninitialized VQs
2013-04-02 12:01 ` Michael S. Tsirkin
@ 2013-04-02 23:16 ` Nicholas A. Bellinger
-1 siblings, 0 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2013-04-02 23:16 UTC (permalink / raw
To: Michael S. Tsirkin
Cc: kvm-devel, qemu-devel, lf-virt, Anthony Liguori, target-devel,
Stefan Hajnoczi, Paolo Bonzini, Asias He
On Tue, 2013-04-02 at 15:01 +0300, Michael S. Tsirkin wrote:
> On Mon, Apr 01, 2013 at 11:58:21PM +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, which is currently
> > expected to occur during seabios setup of virtio-scsi with in-flight
> > vhost-scsi-pci device code.
> >
> > On the vhost side, it also adds virtio_queue_valid() sanity checks in
> > vhost_virtqueue_[start,stop]() and vhost_verify_ring_mappings() in order
> > to skip the same uninitialized VQs.
> >
> > Changes from v1:
> > - Remove now unnecessary virtio_queue_get_num() calls in virtio-pci.c
> > - Add virtio_queue_valid() calls in vhost_virtqueue_[start,stop]()
> >
> > Please review.
> >
> > --nab
>
> Looks reasonable.
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
Thanks MST!
Anthony, do you want to pick these up now..? Or shall I include in the
next vhost-scsi-pci PATCH-v3 series..?
--nab
> So - does this fix the issues you saw with vhost-scsi?
>
> > 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: Skip uninitialized VQs in vhost_virtqueue_[start,stop]
> >
> > hw/vhost.c | 12 ++++++++++++
> > hw/virtio-pci.c | 34 +++++++++++++++-------------------
> > hw/virtio.c | 5 +++++
> > hw/virtio.h | 1 +
> > 4 files changed, 33 insertions(+), 19 deletions(-)
> >
> > --
> > 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH-v2 0/3] virtio/vhost: Add checks for uninitialized VQs
@ 2013-04-02 23:16 ` Nicholas A. Bellinger
0 siblings, 0 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2013-04-02 23:16 UTC (permalink / raw
To: Michael S. Tsirkin
Cc: kvm-devel, qemu-devel, lf-virt, Anthony Liguori, target-devel,
Stefan Hajnoczi, Paolo Bonzini
On Tue, 2013-04-02 at 15:01 +0300, Michael S. Tsirkin wrote:
> On Mon, Apr 01, 2013 at 11:58:21PM +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, which is currently
> > expected to occur during seabios setup of virtio-scsi with in-flight
> > vhost-scsi-pci device code.
> >
> > On the vhost side, it also adds virtio_queue_valid() sanity checks in
> > vhost_virtqueue_[start,stop]() and vhost_verify_ring_mappings() in order
> > to skip the same uninitialized VQs.
> >
> > Changes from v1:
> > - Remove now unnecessary virtio_queue_get_num() calls in virtio-pci.c
> > - Add virtio_queue_valid() calls in vhost_virtqueue_[start,stop]()
> >
> > Please review.
> >
> > --nab
>
> Looks reasonable.
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
Thanks MST!
Anthony, do you want to pick these up now..? Or shall I include in the
next vhost-scsi-pci PATCH-v3 series..?
--nab
> So - does this fix the issues you saw with vhost-scsi?
>
> > 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: Skip uninitialized VQs in vhost_virtqueue_[start,stop]
> >
> > hw/vhost.c | 12 ++++++++++++
> > hw/virtio-pci.c | 34 +++++++++++++++-------------------
> > hw/virtio.c | 5 +++++
> > hw/virtio.h | 1 +
> > 4 files changed, 33 insertions(+), 19 deletions(-)
> >
> > --
> > 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
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2013-04-02 23:17 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-01 23:58 [Qemu-devel] [PATCH-v2 0/3] virtio/vhost: Add checks for uninitialized VQs Nicholas A. Bellinger
2013-04-01 23:58 ` Nicholas A. Bellinger
2013-04-01 23:58 ` [Qemu-devel] [PATCH-v2 1/3] virtio: add API to check that ring is setup Nicholas A. Bellinger
2013-04-01 23:58 ` Nicholas A. Bellinger
2013-04-01 23:58 ` [Qemu-devel] [PATCH-v2 2/3] virtio-pci: Add virtio_queue_valid checks ahead of virtio_queue_get_num Nicholas A. Bellinger
2013-04-01 23:58 ` Nicholas A. Bellinger
2013-04-01 23:58 ` [Qemu-devel] [PATCH-v2 3/3] vhost: Skip uninitialized VQs in vhost_virtqueue_[start, stop] Nicholas A. Bellinger
2013-04-01 23:58 ` [PATCH-v2 3/3] vhost: Skip uninitialized VQs in vhost_virtqueue_[start,stop] Nicholas A. Bellinger
2013-04-01 23:58 ` [PATCH-v2 3/3] vhost: Skip uninitialized VQs in vhost_virtqueue_[start, stop] Nicholas A. Bellinger
2013-04-02 12:01 ` [Qemu-devel] [PATCH-v2 0/3] virtio/vhost: Add checks for uninitialized VQs Michael S. Tsirkin
2013-04-02 12:01 ` Michael S. Tsirkin
2013-04-02 23:16 ` [Qemu-devel] " Nicholas A. Bellinger
2013-04-02 23:16 ` Nicholas A. Bellinger
2013-04-02 12:01 ` Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.