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

[Zhang, Yulei]

> -----Original Message-----
> From: Alex Williamson [mailto:address@hidden
> Sent: Tuesday, June 27, 2017 4:19 AM
> To: Zhang, Yulei <address@hidden>
> Cc: address@hidden; Tian, Kevin <address@hidden>;
> address@hidden; address@hidden; Zheng, Xiao
> <address@hidden>; Wang, Zhi A <address@hidden>
> Subject: Re: [Qemu-devel] [RFC 3/5] vfio: introduce new VFIO ioctl
> VFIO_DEVICE_PCI_STATUS_SET
> 
> 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

Sure, we can remove this ioctl.