Re: [PATCH v2] vhost: always lock all QPs if handlers request it

[Maxime Coquelin <mcoqueli@redhat.com>]

Hi Serghii,

On Tue, Oct 7, 2025 at 5:06 PM Serhii Iliushyk <sil-plv@napatech.com> wrote:
>
> The check for the VIRTIO_DEV_VDPA_CONFIGURED flag is not needed
> since each handler has its own lock_all_qps flag
> to indicate whether it needs all queue pairs.

Hmm, I think this is needed because lock_all_qps is only effective
with the SW datapath.
With vDPA, once the device is configured, we would end-up with
virtqueues modifications
without the HW being stopped or even notified.

> Checking the VIRTIO_DEV_VDPA_CONFIGURED flag caused certain handlers
> (for example, SET_MEM_TABLE for memory hot plug) to run without holding
> the per-vq access_lock and trigger vq_assert_lock failures.

In this specific memory hotplug case, we do the right thing in the handler:

    if (dev->mem) {
        if (dev->flags & VIRTIO_DEV_VDPA_CONFIGURED) {
            struct rte_vdpa_device *vdpa_dev = dev->vdpa_dev;

            if (vdpa_dev && vdpa_dev->ops->dev_close)
                vdpa_dev->ops->dev_close(dev->vid);
            dev->flags &= ~VIRTIO_DEV_VDPA_CONFIGURED;
        }

Maybe we should also lock all the queue pairs once the vDPA is closed.
The problem is to handle the unlock properly.

Maxime

> Signed-off-by: Serhii Iliushyk <sil-plv@napatech.com>
> ---
> v2
> * fix typo in commit message
> ---
>  lib/vhost/vhost_user.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> index 4bfb13fb98..9288da9322 100644
> --- a/lib/vhost/vhost_user.c
> +++ b/lib/vhost/vhost_user.c
> @@ -3171,10 +3171,8 @@ vhost_user_msg_handler(int vid, int fd)
>          * would cause a dead lock.
>          */
>         if (msg_handler != NULL && msg_handler->lock_all_qps) {
> -               if (!(dev->flags & VIRTIO_DEV_VDPA_CONFIGURED)) {
> -                       vhost_user_lock_all_queue_pairs(dev);
> -                       unlock_required = 1;
> -               }
> +               vhost_user_lock_all_queue_pairs(dev);
> +               unlock_required = 1;
>         }
>
>         handled = false;
> --
> 2.47.3
>