Re: [PATCH v8 0/3] Implement vdpasim suspend operation

Re: [PATCH v8 0/3] Implement vdpasim suspend operation

[Michael S. Tsirkin]

On Thu, Aug 11, 2022 at 03:53:50PM +0200, Eugenio Pérez wrote:
> Implement suspend operation for vdpa_sim devices, so vhost-vdpa will offer
> that backend feature and userspace can effectively suspend the device.
> 
> 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 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 virtqueue buffers.
> 
> In the future, we will provide features similar to
> VHOST_USER_GET_INFLIGHT_FD so the device can save pending operations.
> 
> Applied on top of vhost branch.
> 
> Comments are welcome.
> 
> v8:
> * v7 but incremental from vhost instead of isolated.

Now I'm lost. incremental to what? Does the vhost branch now
have the correct bits?

> v7:
> * Remove ioctl leftover argument and update doc accordingly.
> 
> v6:
> * Remove the resume operation, making the ioctl simpler. We can always add
>   another ioctl for VM_STOP/VM_RESUME operation later.
> * s/stop/suspend/ to differentiate more from reset.
> * Clarify scope of the suspend operation.
> 
> v5:
> * s/not stop/resume/ in doc.
> 
> v4:
> * Replace VHOST_STOP to VHOST_VDPA_STOP in vhost ioctl switch case too.
> 
> v3:
> * s/VHOST_STOP/VHOST_VDPA_STOP/
> * Add documentation and requirements of the ioctl above its definition.
> 
> v2:
> * Replace raw _F_STOP with BIT_ULL(_F_STOP).
> * Fix obtaining of stop ioctl arg (it was not obtained but written).
> * Add stop to vdpa_sim_blk.
> 
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git
> 
> Eugenio Pérez (3):
>   vdpa: delete unreachable branch on vdpasim_suspend
>   vdpa: Remove wrong doc of VHOST_VDPA_SUSPEND ioctl
>   vhost: Remove invalid parameter of VHOST_VDPA_SUSPEND ioctl
> 
>  drivers/vdpa/vdpa_sim/vdpa_sim.c |  7 -------
>  include/linux/vdpa.h             |  2 +-
>  include/uapi/linux/vhost.h       | 17 ++++++-----------
>  3 files changed, 7 insertions(+), 19 deletions(-)
> 
> -- 
> 2.31.1
> 

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v8 1/3] vdpa: delete unreachable branch on vdpasim_suspend

[Jason Wang]

On Thu, Aug 11, 2022 at 9:54 PM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> It was a leftover from previous versions.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

Acked-by: Jason Wang <jasowang@redhat.com>

> ---
>  drivers/vdpa/vdpa_sim/vdpa_sim.c | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index 213883487f9b..79a50edf8998 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -509,16 +509,9 @@ static int vdpasim_reset(struct vdpa_device *vdpa)
>  static int vdpasim_suspend(struct vdpa_device *vdpa)
>  {
>         struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
> -       int i;
>
>         spin_lock(&vdpasim->lock);
>         vdpasim->running = false;
> -       if (vdpasim->running) {
> -               /* Check for missed buffers */
> -               for (i = 0; i < vdpasim->dev_attr.nvqs; ++i)
> -                       vdpasim_kick_vq(vdpa, i);
> -
> -       }
>         spin_unlock(&vdpasim->lock);
>
>         return 0;
> --
> 2.31.1
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v8 2/3] vdpa: Remove wrong doc of VHOST_VDPA_SUSPEND ioctl

[Jason Wang]

On Thu, Aug 11, 2022 at 9:54 PM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> It was a leftover from previous versions.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

Acked-by: Jason Wang <jasowang@redhat.com>

> ---
>  include/linux/vdpa.h       |  2 +-
>  include/uapi/linux/vhost.h | 15 +++++----------
>  2 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index d282f464d2f1..6c4e6ea7f7eb 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -218,7 +218,7 @@ struct vdpa_map_file {
>   * @reset:                     Reset device
>   *                             @vdev: vdpa device
>   *                             Returns integer: success (0) or error (< 0)
> - * @suspend:                   Suspend or resume the device (optional)
> + * @suspend:                   Suspend the device (optional)
>   *                             @vdev: vdpa device
>   *                             Returns integer: success (0) or error (< 0)
>   * @get_config_size:           Get the size of the configuration space includes
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index 6d9f45163155..89fcb2afe472 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -171,17 +171,12 @@
>  #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
> +/* Suspend 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).
> + * After the return of ioctl the device must finish any pending operations. 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.
>   */
>  #define VHOST_VDPA_SUSPEND             _IOW(VHOST_VIRTIO, 0x7D, int)
>
> --
> 2.31.1
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v8 3/3] vhost: Remove invalid parameter of VHOST_VDPA_SUSPEND ioctl

[Jason Wang]

On Thu, Aug 11, 2022 at 9:54 PM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> It was a leftover from previous versions.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

Acked-by: Jason Wang <jasowang@redhat.com>

> ---
> Note that I'm not sure this removal is valid. The ioctl is not in master
> branch by the send date of this patch, but there are commits on vhost
> branch that do have it.
> ---
>  include/uapi/linux/vhost.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index 89fcb2afe472..768ec46a88bf 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -178,6 +178,6 @@
>   * the possible device specific states) that is required for restoring in the
>   * future. The device must not change its configuration after that point.
>   */
> -#define VHOST_VDPA_SUSPEND             _IOW(VHOST_VIRTIO, 0x7D, int)
> +#define VHOST_VDPA_SUSPEND             _IO(VHOST_VIRTIO, 0x7D)
>
>  #endif
> --
> 2.31.1
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization