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.
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.
> ---
> 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
