[PATCH v4 2/2] vhost: Fix last vq queue index of devices with no cvq

Eugenio Pérez posted 2 patches 4 years, 3 months ago
[PATCH v4 2/2] vhost: Fix last vq queue index of devices with no cvq
Posted by Eugenio Pérez 4 years, 3 months ago
The -1 assumes that cvq device model is accounted in data_queue_pairs,
if cvq does not exists, but it's actually the opposite: Devices with
!cvq are ok but devices with cvq does not add the last queue to
data_queue_pairs.

This is not a problem to vhost-net, but it is to vhost-vdpa:
* Devices with cvq gets initialized at last data vq device model, not
at cvq one.
* Devices with !cvq never gets initialized, since last_index is the
first queue of the last device model.

Because of that, the right change in last_index is to actually add the
cvq, not to remove the missing one.

This is not a problem to vhost-net, but it is to vhost-vdpa, which
device model trust to reach the last index to finish starting the
device.

Also, as the previous commit, rename it to index_end.

Tested with vp_vdpa with host's vhost=on and vhost=off, with ctrl_vq=on
and ctrl_vq=off.

Fixes: 049eb15b5fc9 ("vhost: record the last virtqueue index for the virtio device")
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 hw/net/vhost_net.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 29f2c4212f..30379d2ca4 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -326,11 +326,11 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
     VirtIONet *n = VIRTIO_NET(dev);
     int nvhosts = data_queue_pairs + cvq;
     struct vhost_net *net;
-    int r, e, i, last_index = data_queue_pairs * 2;
+    int r, e, i, index_end = data_queue_pairs * 2;
     NetClientState *peer;
 
-    if (!cvq) {
-        last_index -= 1;
+    if (cvq) {
+        index_end += 1;
     }
 
     if (!k->set_guest_notifiers) {
@@ -347,7 +347,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
         }
 
         net = get_vhost_net(peer);
-        vhost_net_set_vq_index(net, i * 2, last_index);
+        vhost_net_set_vq_index(net, i * 2, index_end);
 
         /* Suppress the masking guest notifiers on vhost user
          * because vhost user doesn't interrupt masking/unmasking
-- 
2.27.0


Re: [PATCH v4 2/2] vhost: Fix last vq queue index of devices with no cvq
Posted by Jason Wang 4 years, 3 months ago
On Thu, Nov 4, 2021 at 4:56 PM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> The -1 assumes that cvq device model is accounted in data_queue_pairs,
> if cvq does not exists, but it's actually the opposite: Devices with
> !cvq are ok but devices with cvq does not add the last queue to
> data_queue_pairs.
>
> This is not a problem to vhost-net, but it is to vhost-vdpa:
> * Devices with cvq gets initialized at last data vq device model, not
> at cvq one.
> * Devices with !cvq never gets initialized, since last_index is the
> first queue of the last device model.
>
> Because of that, the right change in last_index is to actually add the
> cvq, not to remove the missing one.
>
> This is not a problem to vhost-net, but it is to vhost-vdpa, which
> device model trust to reach the last index to finish starting the
> device.
>
> Also, as the previous commit, rename it to index_end.
>
> Tested with vp_vdpa with host's vhost=on and vhost=off, with ctrl_vq=on
> and ctrl_vq=off.
>
> Fixes: 049eb15b5fc9 ("vhost: record the last virtqueue index for the virtio device")
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

Acked-by: Jason Wang <jasowang@redhat.com>

> ---
>  hw/net/vhost_net.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 29f2c4212f..30379d2ca4 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -326,11 +326,11 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
>      VirtIONet *n = VIRTIO_NET(dev);
>      int nvhosts = data_queue_pairs + cvq;
>      struct vhost_net *net;
> -    int r, e, i, last_index = data_queue_pairs * 2;
> +    int r, e, i, index_end = data_queue_pairs * 2;
>      NetClientState *peer;
>
> -    if (!cvq) {
> -        last_index -= 1;
> +    if (cvq) {
> +        index_end += 1;
>      }
>
>      if (!k->set_guest_notifiers) {
> @@ -347,7 +347,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
>          }
>
>          net = get_vhost_net(peer);
> -        vhost_net_set_vq_index(net, i * 2, last_index);
> +        vhost_net_set_vq_index(net, i * 2, index_end);
>
>          /* Suppress the masking guest notifiers on vhost user
>           * because vhost user doesn't interrupt masking/unmasking
> --
> 2.27.0
>