On Wed, Jul 13, 2022 at 7:52 AM Michael S. Tsirkin <m...@redhat.com> wrote: > > On Wed, Jul 06, 2022 at 08:39:48PM +0200, Eugenio Pérez wrote: > > To restore the device in the destination of a live migration we send the > > commands through control virtqueue. For a device to read CVQ it must > > have received DRIVER_OK status bit. > > > > However this open a window where the device could start receiving > > packets in rx queue 0 before it receive the RSS configuration. To avoid > > that, we will not send vring_enable until all configuration is used by > > the device. > > > > As a first step, reverse the DRIVER_OK and SET_VRING_ENABLE steps. > > > > Signed-off-by: Eugenio Pérez <epere...@redhat.com> > > Not a comment on this patch specifically, but generally: > > You should know that lots of existing drivers are buggy and > try to poke at the VQs before DRIVER_OK. We are doing our best > to fix them but it's taking forever. For now it's a good > idea to support such drivers even if they are out of spec.
I think vhost-vdpa should not need to explicitly handle it, since it is started after DRIVER_OK. But I think it's a good idea to perform a fast test. I think those kicks will go to the device's ioeventfd and the specific virtqueue's handle_output callback. > You do that by starting on the first kick in absence of DRIVER_OK. > Further, adding buffers before DRIVER_OK is actually allowed, > as long as you don't kick. > SVQ adds all the buffers after the guest's driver_ok and after set driver_ok to the device. What it does is to send CVQ buffers before enabling the data queues with VHOST_VDPA_SET_VRING_ENABLE. Only CVQ is enabled at this point, but DRIVER_OK has already been sent. Or am I missing something? Thanks! > > > --- > > hw/virtio/vhost-vdpa.c | 22 ++++++++++++++++------ > > 1 file changed, 16 insertions(+), 6 deletions(-) > > > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > > index 66f054a12c..2ee8009594 100644 > > --- a/hw/virtio/vhost-vdpa.c > > +++ b/hw/virtio/vhost-vdpa.c > > @@ -728,13 +728,18 @@ static int vhost_vdpa_get_vq_index(struct vhost_dev > > *dev, int idx) > > return idx; > > } > > > > +/** > > + * Set ready all vring of the device > > + * > > + * @dev: Vhost device > > + */ > > static int vhost_vdpa_set_vring_ready(struct vhost_dev *dev) > > { > > int i; > > trace_vhost_vdpa_set_vring_ready(dev); > > - for (i = 0; i < dev->nvqs; ++i) { > > + for (i = 0; i < dev->vq_index_end; ++i) { > > struct vhost_vring_state state = { > > - .index = dev->vq_index + i, > > + .index = i, > > .num = 1, > > }; > > vhost_vdpa_call(dev, VHOST_VDPA_SET_VRING_ENABLE, &state); > > @@ -1097,7 +1102,6 @@ static int vhost_vdpa_dev_start(struct vhost_dev > > *dev, bool started) > > if (unlikely(!ok)) { > > return -1; > > } > > - vhost_vdpa_set_vring_ready(dev); > > } else { > > ok = vhost_vdpa_svqs_stop(dev); > > if (unlikely(!ok)) { > > @@ -1111,16 +1115,22 @@ static int vhost_vdpa_dev_start(struct vhost_dev > > *dev, bool started) > > } > > > > if (started) { > > + int r; > > + > > memory_listener_register(&v->listener, &address_space_memory); > > - return vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK); > > + r = vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK); > > + if (unlikely(r)) { > > + return r; > > + } > > + vhost_vdpa_set_vring_ready(dev); > > } else { > > vhost_vdpa_reset_device(dev); > > vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE | > > VIRTIO_CONFIG_S_DRIVER); > > memory_listener_unregister(&v->listener); > > - > > - return 0; > > } > > + > > + return 0; > > } > > > > static int vhost_vdpa_set_log_base(struct vhost_dev *dev, uint64_t base, > > -- > > 2.31.1 >