On Tue, Jun 28, 2022 at 3:45 PM Stefano Garzarella <sgarzare@xxxxxxxxxx> wrote: > > On Thu, Jun 23, 2022 at 06:07:37PM +0200, Eugenio Pérez wrote: > >The ioctl adds support for suspending the device from userspace. > > > >This is a must before getting virtqueue indexes (base) for live migration, > >since the device could modify them after userland gets them. There are > >individual ways to perform that action for some devices > >(VHOST_NET_SET_BACKEND, VHOST_VSOCK_SET_RUNNING, ...) but there was no > >way to perform it for any vhost device (and, in particular, vhost-vdpa). > > > >After a successful return of the ioctl call the device must not process > >more virtqueue descriptors. The device can answer to read or writes of > >config fields as if it were not suspended. In particular, writing to > >"queue_enable" with a value of 1 will not make the device start > >processing buffers of the virtqueue. > > > >Signed-off-by: Eugenio Pérez <eperezma@xxxxxxxxxx> > >--- > > drivers/vhost/vdpa.c | 19 +++++++++++++++++++ > > include/uapi/linux/vhost.h | 14 ++++++++++++++ > > 2 files changed, 33 insertions(+) > > > >diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > >index 3d636e192061..7fa671ac4bdf 100644 > >--- a/drivers/vhost/vdpa.c > >+++ b/drivers/vhost/vdpa.c > >@@ -478,6 +478,22 @@ static long vhost_vdpa_get_vqs_count(struct vhost_vdpa *v, u32 __user *argp) > > return 0; > > } > > > >+/* After a successful return of ioctl the device must not process more > >+ * virtqueue descriptors. The device can answer to read or writes of config > >+ * fields as if it were not suspended. In particular, writing to "queue_enable" > >+ * with a value of 1 will not make the device start processing buffers. > >+ */ > >+static long vhost_vdpa_suspend(struct vhost_vdpa *v) > >+{ > >+ struct vdpa_device *vdpa = v->vdpa; > >+ const struct vdpa_config_ops *ops = vdpa->config; > >+ > >+ if (!ops->suspend) > >+ return -EOPNOTSUPP; > >+ > >+ return ops->suspend(vdpa); > >+} > >+ > > static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd, > > void __user *argp) > > { > >@@ -654,6 +670,9 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep, > > case VHOST_VDPA_GET_VQS_COUNT: > > r = vhost_vdpa_get_vqs_count(v, argp); > > break; > >+ case VHOST_VDPA_SUSPEND: > >+ r = vhost_vdpa_suspend(v); > >+ break; > > default: > > r = vhost_dev_ioctl(&v->vdev, cmd, argp); > > if (r == -ENOIOCTLCMD) > >diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h > >index cab645d4a645..6d9f45163155 100644 > >--- a/include/uapi/linux/vhost.h > >+++ b/include/uapi/linux/vhost.h > >@@ -171,4 +171,18 @@ > > #define VHOST_VDPA_SET_GROUP_ASID _IOW(VHOST_VIRTIO, 0x7C, \ > > struct vhost_vring_state) > > > >+/* Suspend or resume a device so it does not process virtqueue requests anymore > >+ * > >+ * After the return of ioctl with suspend != 0, the device must finish any > >+ * pending operations like in flight requests. It must also preserve all the > >+ * necessary state (the virtqueue vring base plus the possible device specific > >+ * states) that is required for restoring in the future. The device must not > >+ * change its configuration after that point. > >+ * > >+ * After the return of ioctl with suspend == 0, the device can continue > >+ * processing buffers as long as typical conditions are met (vq is enabled, > >+ * DRIVER_OK status bit is enabled, etc). > >+ */ > >+#define VHOST_VDPA_SUSPEND _IOW(VHOST_VIRTIO, 0x7D, int) > ^ > IIUC we are not using the argument anymore, so this should be changed in > _IO(VHOST_VIRTIO, 0x7D). > > And we should update a bit the documentation. > Totally right, replacing it for the next version. Thanks! > Thanks, > Stefano >