Re: [PATCH 09/16] vhost-user: enable/disable a single vring

[Kangjie Xu]

在 2022/7/26 12:07, Jason Wang 写道:
> 在 2022/7/18 19:17, Kangjie Xu 写道:
> > Implement the vhost_set_single_vring_enable, which is to enable or
> > disable a single vring.
> >
> > The parameter wait_for_reply is added to help for some cases such as
> > vq reset.
> >
> > Meanwhile, vhost_user_set_vring_enable() is refactored.
> >
> > Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > ---
> >   hw/virtio/vhost-user.c | 55 ++++++++++++++++++++++++++++++++++++------
> >   1 file changed, 48 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index 75b8df21a4..5a80a415f0 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -267,6 +267,8 @@ struct scrub_regions {
> >       int fd_idx;
> >   };
> >   +static int enforce_reply(struct vhost_dev *dev, const VhostUserMsg 
> > *msg);
> > +
> >   static bool ioeventfd_enabled(void)
> >   {
> >       return !kvm_enabled() || kvm_eventfds_enabled();
> > @@ -1198,6 +1200,49 @@ static int vhost_user_set_vring_base(struct 
> > vhost_dev *dev,
> >       return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
> >   }
> >   +
> > +static int vhost_user_set_single_vring_enable(struct vhost_dev *dev,
> > +                                              int index,
> > +                                              int enable,
> > +                                              bool wait_for_reply)
> > +{
> > +    int ret;
> > +
> > +    if (index < dev->vq_index || index >= dev->vq_index + dev->nvqs) {
> > +        return -EINVAL;
> > +    }
> > +
> > +    struct vhost_vring_state state = {
> > +        .index = index,
> > +        .num   = enable,
> > +    };
> > +
> > +    VhostUserMsg msg = {
> > +        .hdr.request = VHOST_USER_SET_VRING_ENABLE,
> > +        .hdr.flags = VHOST_USER_VERSION,
> > +        .payload.state = state,
> > +        .hdr.size = sizeof(msg.payload.state),
> > +    };
> > +
> > +    bool reply_supported = virtio_has_feature(dev->protocol_features,
> > + VHOST_USER_PROTOCOL_F_REPLY_ACK);
> > +
> > +    if (reply_supported && wait_for_reply) {
> > +        msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
> > +    }
>
>
> Do we need to fail if !realy_supported && wait_for_reply?
>
> Thanks
I guess you mean "should we fail if VHOST_USER_PROTOCOL_F_REPLY_ACK 
feature is not supported?".

The implementation here is similar to that in vhost_user_set_vring_addr().

If this feature is not supported, it will call enforce_reply(), then 
call vhost_user_get_features() to get a reply.

Since the messages will be processed sequentailly in DPDK, success of 
enforce_reply() means the previous message VHOST_USER_SET_VRING_ENABLE 
has been processed.

Thanks

> > +
> > +    ret = vhost_user_write(dev, &msg, NULL, 0);
> > +    if (ret < 0) {
> > +        return ret;
> > +    }
> > +
> > +    if (wait_for_reply) {
> > +        return enforce_reply(dev, &msg);
> > +    }
> > +
> > +    return ret;
> > +}
> > +
> >   static int vhost_user_set_vring_enable(struct vhost_dev *dev, int 
> > enable)
> >   {
> >       int i;
> > @@ -1207,13 +1252,8 @@ static int vhost_user_set_vring_enable(struct 
> > vhost_dev *dev, int enable)
> >       }
> >         for (i = 0; i < dev->nvqs; ++i) {
> > -        int ret;
> > -        struct vhost_vring_state state = {
> > -            .index = dev->vq_index + i,
> > -            .num   = enable,
> > -        };
> > -
> > -        ret = vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, 
> > &state);
> > +        int ret = vhost_user_set_single_vring_enable(dev, 
> > dev->vq_index + i,
> > +                                                     enable, false);
> >           if (ret < 0) {
> >               /*
> >                * Restoring the previous state is likely infeasible, 
> > as well as
> > @@ -2627,6 +2667,7 @@ const VhostOps user_ops = {
> >           .vhost_set_owner = vhost_user_set_owner,
> >           .vhost_reset_device = vhost_user_reset_device,
> >           .vhost_get_vq_index = vhost_user_get_vq_index,
> > +        .vhost_set_single_vring_enable = 
> > vhost_user_set_single_vring_enable,
> >           .vhost_set_vring_enable = vhost_user_set_vring_enable,
> >           .vhost_requires_shm_log = vhost_user_requires_shm_log,
> >           .vhost_migration_done = vhost_user_migration_done,