Re: [Qemu-devel] [RFC 3/5] vfio: introduce new VFIO ioctl VFIO_DEVICE_PCI_STATUS_SET

[Alex Williamson]

On Tue,  4 Apr 2017 18:27:30 +0800
Yulei Zhang <address@hidden> wrote:

> New VFIO ioctl VFIO_DEVICE_PCI_STATUS_SET is added to change the
> vfio pci device status during the migration, stop the device on
> the source side before fetch its status and start the deivce on
> the target side after restore its status.
> 
> Signed-off-by: Yulei Zhang <address@hidden>
> ---
>  hw/vfio/pci.c              | 17 +++++++++++++++++
>  linux-headers/linux/vfio.h | 15 +++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 7de4eb4..605a473 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -38,6 +38,7 @@
>  static void vfio_disable_interrupts(VFIOPCIDevice *vdev);
>  static void vfio_mmap_set_enabled(VFIOPCIDevice *vdev, bool enabled);
>  static VMStateDescription vfio_pci_vmstate;
> +static void vfio_vm_change_state_handler(void *pv, int running, RunState 
> state);
>  
>  /*
>   * Disabling BAR mmaping can be slow, but toggling it around INTx can
> @@ -2866,6 +2867,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>      vfio_register_err_notifier(vdev);
>      vfio_register_req_notifier(vdev);
>      vfio_setup_resetfn_quirk(vdev);
> +    qemu_add_vm_change_state_handler(vfio_vm_change_state_handler, vdev);
>  
>      return;
>  
> @@ -2948,6 +2950,21 @@ post_reset:
>      vfio_pci_post_reset(vdev);
>  }
>  
> +static void vfio_vm_change_state_handler(void *pv, int running, RunState 
> state)
> +{
> +    VFIOPCIDevice *vdev = pv;
> +    struct vfio_pci_status_set *vfio_status;
> +    int argsz = sizeof(*vfio_status);
> +
> +    vfio_status = g_malloc0(argsz);
> +    vfio_status->argsz = argsz;
> +    vfio_status->flags = running ? VFIO_DEVICE_PCI_START :
> +                               VFIO_DEVICE_PCI_STOP;
> +
> +    ioctl(vdev->vbasedev.fd, VFIO_DEVICE_PCI_STATUS_SET, vfio_status);
> +    g_free(vfio_status);
> +}
> +
>  static int vfio_device_put(QEMUFile *f, void *pv, size_t size, VMStateField 
> *field,
>                              QJSON *vmdesc)
>  {
> diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
> index c87d05c..fa17848 100644
> --- a/linux-headers/linux/vfio.h
> +++ b/linux-headers/linux/vfio.h
> @@ -487,6 +487,21 @@ struct vfio_pci_hot_reset {
>  
>  #define VFIO_DEVICE_PCI_HOT_RESET    _IO(VFIO_TYPE, VFIO_BASE + 13)
>  
> +/**
> + * VFIO_DEVICE_PCI_STATUS_SET - _IOW(VFIO_TYPE, VFIO_BASE + 14,
> + *                               struct vfio_pci_status_set)
> + *
> + * Return: 0 on success, -errno on failure.
> + */
> +struct vfio_pci_status_set{
> +     __u32   argsz;
> +     __u32   flags;
> +#define VFIO_DEVICE_PCI_STOP  (1 << 0)
> +#define VFIO_DEVICE_PCI_START (1 << 1)
> +};
> +
> +#define VFIO_DEVICE_PCI_STATUS_SET   _IO(VFIO_TYPE, VFIO_BASE + 14)
> +
>  /* -------- API for Type1 VFIO IOMMU -------- */
>  
>  /**

Why does this need to be an ioctl?  We could simply define the first
dword of the migration region as the device state and the user could
read and write it.  Thanks,

Alex