On Fri, Apr 21, 2023 at 01:30:48PM +0800, Yajun Wu wrote:
>
> On 4/20/2023 9:07 PM, Stefan Hajnoczi wrote:
> >
> > Setting the VIRTIO Device Status Field to 0 resets the device. The
> > device's state is lost, including the vring configuration.
> >
> > vhost-user.c currently sends SET_STATUS 0 before GET_VRING_BASE. This
> > risks confusion about the lifetime of the vhost-user state (e.g. vring
> > last_avail_idx) across VIRTIO device reset.
> >
> > Eugenio Pérez <epere...@redhat.com> adjusted the order for vhost-vdpa.c
> > in commit c3716f260bff ("vdpa: move vhost reset after get vring base")
> > and in that commit description suggested doing the same for vhost-user
> > in the future.
> >
> > Go ahead and adjust vhost-user.c now. I ran various online code searches
> > to identify vhost-user backends implementing SET_STATUS. It seems only
> > DPDK implements SET_STATUS and Yajun Wu <yaj...@nvidia.com> has
> > confirmed that it is safe to make this change.
> >
> > Cc: Michael S. Tsirkin <m...@redhat.com>
> > Cc: Cindy Lu <l...@redhat.com>
> > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
> > ---
> > hw/virtio/vhost-user.c | 13 ++++++++++++-
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index e5285df4ba..2d40b1b3e7 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -2677,10 +2677,20 @@ static int vhost_user_dev_start(struct vhost_dev
> > *dev, bool started)
> > VIRTIO_CONFIG_S_DRIVER |
> > VIRTIO_CONFIG_S_DRIVER_OK);
> > } else {
> > - return vhost_user_set_status(dev, 0);
> > + return 0;
> > }
> > }
> >
> > +static void vhost_user_reset_status(struct vhost_dev *dev)
> > +{
> > + /* Set device status only for last queue pair */
> > + if (dev->vq_index + dev->nvqs != dev->vq_index_end) {
> > + return;
> > + }
> > +
> > + vhost_user_set_status(dev, 0);
> > +}
> > +
> > const VhostOps user_ops = {
> > .backend_type = VHOST_BACKEND_TYPE_USER,
> > .vhost_backend_init = vhost_user_backend_init,
> > @@ -2716,4 +2726,5 @@ const VhostOps user_ops = {
> > .vhost_get_inflight_fd = vhost_user_get_inflight_fd,
> > .vhost_set_inflight_fd = vhost_user_set_inflight_fd,
> > .vhost_dev_start = vhost_user_dev_start,
> > + .vhost_reset_status = vhost_user_reset_status,
> > };
> > --
> > 2.39.2
> >
> Thank you for this fix.
>
> Can you add protocol feature bit check, just like we do in
> vhost_user_dev_start?
>
> if (!virtio_has_feature(dev->protocol_features,
> VHOST_USER_PROTOCOL_F_STATUS)) {
> return 0;
> }Sure, will fix in v2. Stefan
signature.asc
Description: PGP signature
