Re: [PATCH] vdpa: set old virtio status at cvq isolation probing end

[Jason Wang]

On Wed, Jul 26, 2023 at 2:21 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> The device already has a virtio status set by vhost_vdpa_init by the
> time vhost_vdpa_probe_cvq_isolation is called. vhost_vdpa_init set
> S_ACKNOWLEDGE and S_DRIVER, so it is invalid to just reset it.
>
> It is invalid to start the device after it, but all devices seems to be
> fine with it.  Fixing qemu so it follows virtio start procedure.
>
> Fixes: 152128d64697 ("vdpa: move CVQ isolation check to net_init_vhost_vdpa")
> Reported-by: Dragos Tatulea <dtatulea@nvidia.com>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  net/vhost-vdpa.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 9795306742..d7e2b714b4 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -1333,6 +1333,8 @@ static int vhost_vdpa_probe_cvq_isolation(int 
> device_fd, uint64_t features,
>  out:
>      status = 0;
>      ioctl(device_fd, VHOST_VDPA_SET_STATUS, &status);
> +    status = VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER;
> +    ioctl(device_fd, VHOST_VDPA_SET_STATUS, &status);

So if we fail after FEATURES_OK, this basically clears that bit. Spec
doesn't say it can or not, I wonder if a reset is better?

Btw, spec requires a read of status after setting FEATURES_OK, this
seems to be missed in the current code.

Thanks

>      return r;
>  }
>
> --
> 2.39.3
>