On Tue, Jul 26, 2022 at 2:39 PM Kangjie Xu <kangjie...@linux.alibaba.com> wrote: > > > 在 2022/7/26 11:49, Jason Wang 写道: > > > > 在 2022/7/18 19:17, Kangjie Xu 写道: > >> The interface to set enable status for a single vring is lacked in > >> VhostOps, since the vhost_set_vring_enable_op will manipulate all > >> virtqueues in a device. > >> > >> Resetting a single vq will rely on this interface. It requires a > >> reply to indicate that the reset operation is finished, so the > >> parameter, wait_for_reply, is added. > > > > > > The wait reply seems to be a implementation specific thing. Can we > > hide it? > > > > Thanks > > > I do not hide wait_for_reply here because when stopping the queue, a > reply is needed to ensure that the message has been processed and queue > has been disabled.
This needs to be done at vhost-backend level instead of the general vhost code. E.g vhost-kernel or vDPA is using ioctl() which is synchronous. > > When restarting the queue, we do not need a reply, which is the same as > what qemu does in vhost_dev_start(). > > So I add this parameter to distinguish the two cases. > > I think one alternative implementation is to add a interface in > VhostOps: queue_reset(). In this way details can be hidden. What do you > think of this solution? Does it look better? Let me ask it differently, under which case can we call this function with wait_for_reply = false? Thanks > > Thanks > > > > >> > >> Signed-off-by: Kangjie Xu <kangjie...@linux.alibaba.com> > >> Signed-off-by: Xuan Zhuo <xuanz...@linux.alibaba.com> > >> --- > >> include/hw/virtio/vhost-backend.h | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/include/hw/virtio/vhost-backend.h > >> b/include/hw/virtio/vhost-backend.h > >> index eab46d7f0b..7bddd1e9a0 100644 > >> --- a/include/hw/virtio/vhost-backend.h > >> +++ b/include/hw/virtio/vhost-backend.h > >> @@ -81,6 +81,9 @@ typedef int (*vhost_set_backend_cap_op)(struct > >> vhost_dev *dev); > >> typedef int (*vhost_set_owner_op)(struct vhost_dev *dev); > >> typedef int (*vhost_reset_device_op)(struct vhost_dev *dev); > >> typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx); > >> +typedef int (*vhost_set_single_vring_enable_op)(struct vhost_dev *dev, > >> + int index, int enable, > >> + bool wait_for_reply); > >> typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev, > >> int enable); > >> typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev); > >> @@ -155,6 +158,7 @@ typedef struct VhostOps { > >> vhost_set_owner_op vhost_set_owner; > >> vhost_reset_device_op vhost_reset_device; > >> vhost_get_vq_index_op vhost_get_vq_index; > >> + vhost_set_single_vring_enable_op vhost_set_single_vring_enable; > >> vhost_set_vring_enable_op vhost_set_vring_enable; > >> vhost_requires_shm_log_op vhost_requires_shm_log; > >> vhost_migration_done_op vhost_migration_done; >