* [PATCH v4 0/2] vhost: Fix last queue index of devices with no cvq @ 2021-11-04 8:56 Eugenio Pérez 2021-11-04 8:56 ` [PATCH v4 1/2] vhost: Rename last_index to vq_index_end Eugenio Pérez 2021-11-04 8:56 ` [PATCH v4 2/2] vhost: Fix last vq queue index of devices with no cvq Eugenio Pérez 0 siblings, 2 replies; 5+ messages in thread From: Eugenio Pérez @ 2021-11-04 8:56 UTC (permalink / raw) To: qemu-devel; +Cc: Jason Wang, Michael S. Tsirkin, Juan Quintela 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. Tested with vp_vdpa with host's vhost=on and vhost=off, and ctrol_vq on and off. v4: * Rename last_index to index_end v3: * Recover cvq devices. * Rename last_index to last_vq_index v2: * Delete all the conditional code instead of ROUND_DOWN in a deinitely too-bit-tricky way. Eugenio Pérez (2): vhost: Rename last_index to vq_index_end vhost: Fix last vq queue index of devices with no cvq include/hw/virtio/vhost.h | 4 ++-- hw/net/vhost_net.c | 12 ++++++------ hw/virtio/vhost-vdpa.c | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) -- 2.27.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v4 1/2] vhost: Rename last_index to vq_index_end 2021-11-04 8:56 [PATCH v4 0/2] vhost: Fix last queue index of devices with no cvq Eugenio Pérez @ 2021-11-04 8:56 ` Eugenio Pérez 2021-11-06 11:49 ` Juan Quintela 2021-11-04 8:56 ` [PATCH v4 2/2] vhost: Fix last vq queue index of devices with no cvq Eugenio Pérez 1 sibling, 1 reply; 5+ messages in thread From: Eugenio Pérez @ 2021-11-04 8:56 UTC (permalink / raw) To: qemu-devel; +Cc: Jason Wang, Michael S. Tsirkin, Juan Quintela The doc of this field pointed out that last_index is the last vq index. This is misleading, since it's actually one past the end of the vqs. Renaming and modifying comment. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> Acked-by: Jason Wang <jasowang@redhat.com> --- include/hw/virtio/vhost.h | 4 ++-- hw/net/vhost_net.c | 4 ++-- hw/virtio/vhost-vdpa.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h index 3fa0b554ef..58a73e7b7a 100644 --- a/include/hw/virtio/vhost.h +++ b/include/hw/virtio/vhost.h @@ -74,8 +74,8 @@ struct vhost_dev { unsigned int nvqs; /* the first virtqueue which would be used by this vhost dev */ int vq_index; - /* the last vq index for the virtio device (not vhost) */ - int last_index; + /* one past the last vq index for the virtio device (not vhost) */ + int vq_index_end; /* if non-zero, minimum required value for max_queues */ int num_queues; uint64_t features; diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 0d888f29a6..29f2c4212f 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -232,10 +232,10 @@ fail: } static void vhost_net_set_vq_index(struct vhost_net *net, int vq_index, - int last_index) + int vq_index_end) { net->dev.vq_index = vq_index; - net->dev.last_index = last_index; + net->dev.vq_index_end = vq_index_end; } static int vhost_net_start_one(struct vhost_net *net, diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c index 0d8051426c..bcaf00e09f 100644 --- a/hw/virtio/vhost-vdpa.c +++ b/hw/virtio/vhost-vdpa.c @@ -645,7 +645,7 @@ static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started) vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs); } - if (dev->vq_index + dev->nvqs != dev->last_index) { + if (dev->vq_index + dev->nvqs != dev->vq_index_end) { return 0; } -- 2.27.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/2] vhost: Rename last_index to vq_index_end 2021-11-04 8:56 ` [PATCH v4 1/2] vhost: Rename last_index to vq_index_end Eugenio Pérez @ 2021-11-06 11:49 ` Juan Quintela 0 siblings, 0 replies; 5+ messages in thread From: Juan Quintela @ 2021-11-06 11:49 UTC (permalink / raw) To: Eugenio Pérez; +Cc: Jason Wang, qemu-devel, Michael S. Tsirkin Eugenio Pérez <eperezma@redhat.com> wrote: > The doc of this field pointed out that last_index is the last vq index. > This is misleading, since it's actually one past the end of the vqs. > > Renaming and modifying comment. > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v4 2/2] vhost: Fix last vq queue index of devices with no cvq 2021-11-04 8:56 [PATCH v4 0/2] vhost: Fix last queue index of devices with no cvq Eugenio Pérez 2021-11-04 8:56 ` [PATCH v4 1/2] vhost: Rename last_index to vq_index_end Eugenio Pérez @ 2021-11-04 8:56 ` Eugenio Pérez 2021-11-05 4:19 ` Jason Wang 1 sibling, 1 reply; 5+ messages in thread From: Eugenio Pérez @ 2021-11-04 8:56 UTC (permalink / raw) To: qemu-devel; +Cc: Jason Wang, Michael S. Tsirkin, Juan Quintela 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4 2/2] vhost: Fix last vq queue index of devices with no cvq 2021-11-04 8:56 ` [PATCH v4 2/2] vhost: Fix last vq queue index of devices with no cvq Eugenio Pérez @ 2021-11-05 4:19 ` Jason Wang 0 siblings, 0 replies; 5+ messages in thread From: Jason Wang @ 2021-11-05 4:19 UTC (permalink / raw) To: Eugenio Pérez; +Cc: Michael S. Tsirkin, qemu-devel, Juan Quintela 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-11-06 11:50 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-04 8:56 [PATCH v4 0/2] vhost: Fix last queue index of devices with no cvq Eugenio Pérez 2021-11-04 8:56 ` [PATCH v4 1/2] vhost: Rename last_index to vq_index_end Eugenio Pérez 2021-11-06 11:49 ` Juan Quintela 2021-11-04 8:56 ` [PATCH v4 2/2] vhost: Fix last vq queue index of devices with no cvq Eugenio Pérez 2021-11-05 4:19 ` Jason Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).