Re: [Qemu-devel] [PATCH V3 1/4] vfio: introduce a new VFIO subregion for mdev device migration support

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Yulei Zhang (yulei.zhang@intel.com) wrote:
> New VFIO sub region VFIO_REGION_SUBTYPE_DEVICE_STATE is added
> to fetch and restore the status of mdev device vGPU during the
> live migration.
> 
> Signed-off-by: Yulei Zhang <yulei.zhang@intel.com>
> ---
>  hw/vfio/pci.c              | 14 +++++++++++++-
>  hw/vfio/pci.h              |  1 +
>  linux-headers/linux/vfio.h |  9 ++++++---
>  3 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 31e1edf..2fe20e4 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -37,6 +37,7 @@
>  
>  static void vfio_disable_interrupts(VFIOPCIDevice *vdev);
>  static void vfio_mmap_set_enabled(VFIOPCIDevice *vdev, bool enabled);
> +static VMStateDescription vfio_pci_vmstate;
>  
>  /*
>   * Disabling BAR mmaping can be slow, but toggling it around INTx can
> @@ -2813,6 +2814,17 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>          vfio_vga_quirk_setup(vdev);
>      }
>  
> +    struct vfio_region_info *device_state;
> +    /* device state region setup */
> +    if (!vfio_get_dev_region_info(&vdev->vbasedev,
> +                VFIO_REGION_TYPE_PCI_VENDOR_TYPE | PCI_VENDOR_ID_INTEL,
> +                VFIO_REGION_SUBTYPE_DEVICE_STATE, &device_state)) {
> +        memcpy(&vdev->device_state, device_state,
> +               sizeof(struct vfio_region_info));
> +        g_free(device_state);
> +        vfio_pci_vmstate.unmigratable = 0;
> +    }

I don't think we've got any other code that changes the 'unmigratable'
flag on a vmstate.  If you've got a device that you don't know whether
it's migratable until runtime, then you should add a 'migration blocker'
if the device isn't migratable.
See for example the calls to migrate_add_blocker in hw/virtio/vhost.c

Dave

>      for (i = 0; i < PCI_ROM_SLOT; i++) {
>          vfio_bar_quirk_setup(vdev, i);
>      }
> @@ -2994,7 +3006,7 @@ static Property vfio_pci_dev_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -static const VMStateDescription vfio_pci_vmstate = {
> +static VMStateDescription vfio_pci_vmstate = {
>      .name = "vfio-pci",
>      .unmigratable = 1,
>  };
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index a8366bb..6a1d26e 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -116,6 +116,7 @@ typedef struct VFIOPCIDevice {
>      VFIOBAR bars[PCI_NUM_REGIONS - 1]; /* No ROM */
>      VFIOVGA *vga; /* 0xa0000, 0x3b0, 0x3c0 */
>      void *igd_opregion;
> +    struct vfio_region_info device_state;
>      PCIHostDeviceAddress host;
>      EventNotifier err_notifier;
>      EventNotifier req_notifier;
> diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
> index 4e7ab4c..c3b8e4a 100644
> --- a/linux-headers/linux/vfio.h
> +++ b/linux-headers/linux/vfio.h
> @@ -296,9 +296,12 @@ struct vfio_region_info_cap_type {
>  #define VFIO_REGION_TYPE_PCI_VENDOR_MASK	(0xffff)
>  
>  /* 8086 Vendor sub-types */
> -#define VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION	(1)
> -#define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG	(2)
> -#define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG	(3)
> +#define VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION		(1)
> +#define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG		(2)
> +#define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG		(3)
> +
> +/* Mdev sub-type for device state save and restore */
> +#define VFIO_REGION_SUBTYPE_DEVICE_STATE	(4)
>  
>  /**
>   * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
> -- 
> 2.7.4
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK