On Tue, Oct 14, 2025 at 2:54 PM Maxime Coquelin <[email protected]> wrote: > > Hi Serghii, > > On Tue, Oct 7, 2025 at 5:06 PM Serhii Iliushyk <[email protected]> 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.
Maybe a generic solution would be in vhost_user_msg_handler() to close the vDPA device, clear CONFIGURED flag and lock all queue pairs as we do in vhost_user_set_mem_table? I have no hardware to test, could you have a try if you think it makes sense? Thanks, Maxime > Maxime > > > Signed-off-by: Serhii Iliushyk <[email protected]> > > --- > > 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 > >
