* [Qemu-devel] [PATCH V3] vhost_net: don't set backend for the uninitialized virtqueue
@ 2019-04-09 5:58 Jason Wang
2019-04-24 11:58 ` Jason Wang
2019-04-24 13:51 ` Michael S. Tsirkin
0 siblings, 2 replies; 5+ messages in thread
From: Jason Wang @ 2019-04-09 5:58 UTC (permalink / raw)
To: jasowang, mst, qemu-devel; +Cc: yuri.benditovich
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 <jasowang@redhat.com>
---
Changes from V1:
- introduce a generic helper to check if a virtqueue is enabled
---
hw/net/vhost_net.c | 10 ++++++++++
hw/virtio/virtio.c | 5 +++++
include/hw/virtio/virtio.h | 1 +
3 files changed, 16 insertions(+)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index be3cc88370..a6b719035c 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -244,6 +244,11 @@ 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) {
+ if (!virtio_queue_enabled(dev, net->dev.vq_index +
+ file.index)) {
+ /* Queue might not be ready for start */
+ continue;
+ }
r = vhost_net_set_backend(&net->dev, &file);
if (r < 0) {
r = -errno;
@@ -256,6 +261,11 @@ fail:
file.fd = -1;
if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
while (file.index-- > 0) {
+ if (!virtio_queue_enabled(dev, net->dev.vq_index +
+ file.index)) {
+ /* Queue might not be ready for start */
+ continue;
+ }
int r = vhost_net_set_backend(&net->dev, &file);
assert(r >= 0);
}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 2626a895cb..28056a7ef7 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2318,6 +2318,11 @@ hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n)
return vdev->vq[n].vring.desc;
}
+bool virtio_queue_enabled(VirtIODevice *vdev, int n)
+{
+ return virtio_queue_get_desc_addr(vdev, n) != 0;
+}
+
hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n)
{
return vdev->vq[n].vring.avail;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index ce9516236a..7140381e3a 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -282,6 +282,7 @@ typedef struct VirtIORNGConf VirtIORNGConf;
VIRTIO_F_IOMMU_PLATFORM, false)
hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
+bool virtio_queue_enabled(VirtIODevice *vdev, int n);
hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);
hwaddr virtio_queue_get_desc_size(VirtIODevice *vdev, int n);
--
2.19.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH V3] vhost_net: don't set backend for the uninitialized virtqueue
2019-04-09 5:58 [Qemu-devel] [PATCH V3] vhost_net: don't set backend for the uninitialized virtqueue Jason Wang
@ 2019-04-24 11:58 ` Jason Wang
2019-04-24 13:51 ` Michael S. Tsirkin
1 sibling, 0 replies; 5+ messages in thread
From: Jason Wang @ 2019-04-24 11:58 UTC (permalink / raw)
To: mst, qemu-devel; +Cc: yuri.benditovich
On 2019/4/9 下午1:58, 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 <jasowang@redhat.com>
> ---
> Changes from V1:
> - introduce a generic helper to check if a virtqueue is enabled
> ---
Michael, does this look good to you?
Thanks
> hw/net/vhost_net.c | 10 ++++++++++
> hw/virtio/virtio.c | 5 +++++
> include/hw/virtio/virtio.h | 1 +
> 3 files changed, 16 insertions(+)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index be3cc88370..a6b719035c 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -244,6 +244,11 @@ 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) {
> + if (!virtio_queue_enabled(dev, net->dev.vq_index +
> + file.index)) {
> + /* Queue might not be ready for start */
> + continue;
> + }
> r = vhost_net_set_backend(&net->dev, &file);
> if (r < 0) {
> r = -errno;
> @@ -256,6 +261,11 @@ fail:
> file.fd = -1;
> if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> while (file.index-- > 0) {
> + if (!virtio_queue_enabled(dev, net->dev.vq_index +
> + file.index)) {
> + /* Queue might not be ready for start */
> + continue;
> + }
> int r = vhost_net_set_backend(&net->dev, &file);
> assert(r >= 0);
> }
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 2626a895cb..28056a7ef7 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2318,6 +2318,11 @@ hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n)
> return vdev->vq[n].vring.desc;
> }
>
> +bool virtio_queue_enabled(VirtIODevice *vdev, int n)
> +{
> + return virtio_queue_get_desc_addr(vdev, n) != 0;
> +}
> +
> hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n)
> {
> return vdev->vq[n].vring.avail;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index ce9516236a..7140381e3a 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -282,6 +282,7 @@ typedef struct VirtIORNGConf VirtIORNGConf;
> VIRTIO_F_IOMMU_PLATFORM, false)
>
> hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
> +bool virtio_queue_enabled(VirtIODevice *vdev, int n);
> hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
> hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);
> hwaddr virtio_queue_get_desc_size(VirtIODevice *vdev, int n);
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH V3] vhost_net: don't set backend for the uninitialized virtqueue
@ 2019-04-24 13:51 ` Michael S. Tsirkin
0 siblings, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2019-04-24 13:51 UTC (permalink / raw)
To: Jason Wang; +Cc: qemu-devel, yuri.benditovich
On Tue, Apr 09, 2019 at 01:58:50PM +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 <jasowang@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> Changes from V1:
> - introduce a generic helper to check if a virtqueue is enabled
> ---
> hw/net/vhost_net.c | 10 ++++++++++
> hw/virtio/virtio.c | 5 +++++
> include/hw/virtio/virtio.h | 1 +
> 3 files changed, 16 insertions(+)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index be3cc88370..a6b719035c 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -244,6 +244,11 @@ 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) {
> + if (!virtio_queue_enabled(dev, net->dev.vq_index +
> + file.index)) {
> + /* Queue might not be ready for start */
> + continue;
> + }
> r = vhost_net_set_backend(&net->dev, &file);
> if (r < 0) {
> r = -errno;
> @@ -256,6 +261,11 @@ fail:
> file.fd = -1;
> if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> while (file.index-- > 0) {
> + if (!virtio_queue_enabled(dev, net->dev.vq_index +
> + file.index)) {
> + /* Queue might not be ready for start */
> + continue;
> + }
> int r = vhost_net_set_backend(&net->dev, &file);
> assert(r >= 0);
> }
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 2626a895cb..28056a7ef7 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2318,6 +2318,11 @@ hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n)
> return vdev->vq[n].vring.desc;
> }
>
> +bool virtio_queue_enabled(VirtIODevice *vdev, int n)
> +{
> + return virtio_queue_get_desc_addr(vdev, n) != 0;
> +}
> +
> hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n)
> {
> return vdev->vq[n].vring.avail;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index ce9516236a..7140381e3a 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -282,6 +282,7 @@ typedef struct VirtIORNGConf VirtIORNGConf;
> VIRTIO_F_IOMMU_PLATFORM, false)
>
> hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
> +bool virtio_queue_enabled(VirtIODevice *vdev, int n);
> hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
> hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);
> hwaddr virtio_queue_get_desc_size(VirtIODevice *vdev, int n);
> --
> 2.19.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH V3] vhost_net: don't set backend for the uninitialized virtqueue
@ 2019-04-24 13:51 ` Michael S. Tsirkin
0 siblings, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2019-04-24 13:51 UTC (permalink / raw)
To: Jason Wang; +Cc: yuri.benditovich, qemu-devel
On Tue, Apr 09, 2019 at 01:58:50PM +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 <jasowang@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> Changes from V1:
> - introduce a generic helper to check if a virtqueue is enabled
> ---
> hw/net/vhost_net.c | 10 ++++++++++
> hw/virtio/virtio.c | 5 +++++
> include/hw/virtio/virtio.h | 1 +
> 3 files changed, 16 insertions(+)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index be3cc88370..a6b719035c 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -244,6 +244,11 @@ 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) {
> + if (!virtio_queue_enabled(dev, net->dev.vq_index +
> + file.index)) {
> + /* Queue might not be ready for start */
> + continue;
> + }
> r = vhost_net_set_backend(&net->dev, &file);
> if (r < 0) {
> r = -errno;
> @@ -256,6 +261,11 @@ fail:
> file.fd = -1;
> if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> while (file.index-- > 0) {
> + if (!virtio_queue_enabled(dev, net->dev.vq_index +
> + file.index)) {
> + /* Queue might not be ready for start */
> + continue;
> + }
> int r = vhost_net_set_backend(&net->dev, &file);
> assert(r >= 0);
> }
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 2626a895cb..28056a7ef7 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2318,6 +2318,11 @@ hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n)
> return vdev->vq[n].vring.desc;
> }
>
> +bool virtio_queue_enabled(VirtIODevice *vdev, int n)
> +{
> + return virtio_queue_get_desc_addr(vdev, n) != 0;
> +}
> +
> hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n)
> {
> return vdev->vq[n].vring.avail;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index ce9516236a..7140381e3a 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -282,6 +282,7 @@ typedef struct VirtIORNGConf VirtIORNGConf;
> VIRTIO_F_IOMMU_PLATFORM, false)
>
> hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
> +bool virtio_queue_enabled(VirtIODevice *vdev, int n);
> hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
> hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);
> hwaddr virtio_queue_get_desc_size(VirtIODevice *vdev, int n);
> --
> 2.19.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH V3] vhost_net: don't set backend for the uninitialized virtqueue
2019-04-24 13:51 ` Michael S. Tsirkin
(?)
@ 2019-04-25 3:00 ` Jason Wang
-1 siblings, 0 replies; 5+ messages in thread
From: Jason Wang @ 2019-04-25 3:00 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: yuri.benditovich, qemu-devel
On 2019/4/24 下午9:51, Michael S. Tsirkin wrote:
> On Tue, Apr 09, 2019 at 01:58:50PM +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<jasowang@redhat.com>
> Reviewed-by: Michael S. Tsirkin<mst@redhat.com>
>
Applied.
Thanks
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-25 3:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-09 5:58 [Qemu-devel] [PATCH V3] vhost_net: don't set backend for the uninitialized virtqueue Jason Wang
2019-04-24 11:58 ` Jason Wang
2019-04-24 13:51 ` Michael S. Tsirkin
2019-04-24 13:51 ` Michael S. Tsirkin
2019-04-25 3:00 ` Jason Wang
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.