[Qemu-devel] [RFC V2 0/4] vfio: Introduce Live migration capability to vfio_mdev device

[Qemu-devel] [RFC V2 0/4] vfio: Introduce Live migration capability to vfio_mdev device

[Yulei Zhang]

Summary

This series RFC would like to introduce the live migration capability
to vfio_mdev device. 

As currently vfio_mdev device don't support migration, we introduce 
a new vfio subtype region VFIO_REGION_SUBTYPE_INTEL_IGD_DEVICE_STATE
for Intel vGPU device, during the vfio device initialization, the mdev
device will be set to migratable if the new region exist.  

The intention to add the new region is using it for vfio_mdev device
status save and restore during the migration. The access to this region
will be trapped and forward to the vfio_mdev device driver. And we use 
the first byte in the new region to control the running state of mdev
device.

Meanwhile we add one new ioctl VFIO_IOMMU_GET_DIRTY_BITMAP to help do 
the mdev device dirty page synchronization.

So the vfio_mdev device migration sequence would be
Source VM side:
			start migration
				|
				V
        	 get the cpu state change callback, write to the
        	 subregion's first byte to stop the mdev device
				|
				V
		 quary the dirty page bitmap from iommu container 
		 and add into qemu dirty list for synchronization
				|
				V
		 save the deivce status into Qemufile which is 
                     read from the vfio device subregion

Target VM side:
		   restore the mdev device after get the
		     saved status context from Qemufile
				|
				V
		     get the cpu state change callback
 		     write to subregion's first byte to 
                      start the mdev device to put it in 
                      running status
				|
				V
			finish migration

V1->V2:
Per Alex's suggestion:
1. use device subtype region instead of VFIO PCI fixed region.
2. remove unnecessary ioctl, use the first byte of subregion to 
   control the running state of mdev device.  
3. for dirty page synchronization, implement the interface with
   VFIOContainer instead of vfio pci device.

Yulei Zhang (4):
  vfio: introduce a new VFIO sub region for mdev device migration
    support
  vfio: Add vm status change callback to stop/restart the mdev device
  vfio: Add struct vfio_vmstate_info to introduce put/get callback
    funtion     for vfio device status save/restore
  vifo: introduce new VFIO ioctl VFIO_IOMMU_GET_DIRTY_BITMAP

 hw/vfio/common.c              |  32 +++++++++
 hw/vfio/pci.c                 | 164 +++++++++++++++++++++++++++++++++++++++++-
 hw/vfio/pci.h                 |   1 +
 include/hw/vfio/vfio-common.h |   1 +
 linux-headers/linux/vfio.h    |  26 ++++++-
 5 files changed, 220 insertions(+), 4 deletions(-)

-- 
2.7.4

[Qemu-devel] [RFC V2 1/4] vfio: introduce a new VFIO sub region for mdev device migration support

[Yulei Zhang]

New VFIO sub region VFIO_REGION_SUBTYPE_INTEL_IGD_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              | 13 ++++++++++++-
 hw/vfio/pci.h              |  1 +
 linux-headers/linux/vfio.h |  7 ++++---
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 03a3d01..21a5cef 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
@@ -2792,6 +2793,16 @@ 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_INTEL_IGD_DEVICE_STATE, &device_state)) {
+	memcpy(&vdev->device_state, device_state, sizeof(struct vfio_region_info));
+	g_free(device_state);
+        vfio_pci_vmstate.unmigratable = 0;
+    }
+
     for (i = 0; i < PCI_ROM_SLOT; i++) {
         vfio_bar_quirk_setup(vdev, i);
     }
@@ -2973,7 +2984,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 531cb2e..e2c53bf 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -294,9 +294,10 @@ 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)
+#define VFIO_REGION_SUBTYPE_INTEL_IGD_DEVICE_STATE	(4)
 
 /**
  * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
-- 
2.7.4

[Qemu-devel] [RFC V2 2/4] vfio: Add vm status change callback to stop/restart the mdev device

[Yulei Zhang]

VM status change handler is added to change the vfio pci device
status during the migration, write the demanded device status
to the DEVICE STATUS subregion to 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 <yulei.zhang@intel.com>
---
 hw/vfio/pci.c                 | 19 +++++++++++++++++++
 include/hw/vfio/vfio-common.h |  1 +
 linux-headers/linux/vfio.h    |  3 +++
 3 files changed, 23 insertions(+)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 21a5cef..753da80 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
@@ -2858,6 +2859,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;
 
@@ -2940,6 +2942,23 @@ post_reset:
     vfio_pci_post_reset(vdev);
 }
 
+static void vfio_vm_change_state_handler(void *pv, int running, RunState state)
+{
+    VFIOPCIDevice *vdev = pv;
+    VFIODevice *vbasedev = &vdev->vbasedev;
+    uint8_t dev_state;
+    uint8_t sz = 1;
+
+    dev_state = running ? VFIO_DEVICE_START : VFIO_DEVICE_STOP;
+
+    if (pwrite(vdev->vbasedev.fd, &dev_state, sz, vdev->device_state.offset) != sz) {
+        error_report("vfio: Failed to %s device\n", running ? "start" : "stop");
+        return;
+    }
+
+    vbasedev->device_state = dev_state;
+}
+
 static void vfio_instance_init(Object *obj)
 {
     PCIDevice *pci_dev = PCI_DEVICE(obj);
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index c582de1..c4bab97 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -123,6 +123,7 @@ typedef struct VFIODevice {
     unsigned int num_irqs;
     unsigned int num_regions;
     unsigned int flags;
+    bool device_state;
 } VFIODevice;
 
 struct VFIODeviceOps {
diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index e2c53bf..ae1b953 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -299,6 +299,9 @@ struct vfio_region_info_cap_type {
 #define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG		(3)
 #define VFIO_REGION_SUBTYPE_INTEL_IGD_DEVICE_STATE	(4)
 
+#define VFIO_DEVICE_START	0
+#define VFIO_DEVICE_STOP	1
+
 /**
  * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
  *				    struct vfio_irq_info)
-- 
2.7.4

[Qemu-devel] [RFC V2 3/4] vfio: Add struct vfio_vmstate_info to introduce put/get callback funtion for vfio device status save/restore

[Yulei Zhang]

Introduce vfio_device_put/vfio_device_get funtion for vfio device state
save/restore usage.

For VFIO pci device status migrate, on the source side with
funtion vfio_device_put to save the following states
1. pci configuration space addr0~addr5
2. pci configuration space msi_addr msi_data
3. pci device status fetch from device driver

And on the target side with funtion vfio_device_get to restore
the same states
1. re-setup the pci bar configuration
2. re-setup the pci device msi configuration
3. restore the pci device status

Signed-off-by: Yulei Zhang <yulei.zhang@intel.com>
---
 hw/vfio/pci.c              | 132 +++++++++++++++++++++++++++++++++++++++++++++
 linux-headers/linux/vfio.h |   2 +
 2 files changed, 134 insertions(+)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 753da80..c0fc1d2 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2959,6 +2959,118 @@ static void vfio_vm_change_state_handler(void *pv, int running, RunState state)
     vbasedev->device_state = dev_state;
 }
 
+static int vfio_device_put(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                            QJSON *vmdesc)
+{
+    VFIOPCIDevice *vdev = pv;
+    PCIDevice *pdev = &vdev->pdev;
+    int sz = vdev->device_state.size - VFIO_DEVICE_STATE_OFFSET;
+    uint8_t *buf = NULL;
+    uint32_t msi_cfg, msi_lo, msi_hi, msi_data, bar_cfg, i;
+    bool msi_64bit;
+
+    for (i = 0; i < PCI_ROM_SLOT; i++) {
+        bar_cfg = pci_default_read_config(pdev, PCI_BASE_ADDRESS_0 + i*4, 4);
+        qemu_put_be32(f, bar_cfg);
+    }
+
+    msi_cfg = pci_default_read_config(pdev, pdev->msi_cap + PCI_MSI_FLAGS, 2);
+    msi_64bit = !!(msi_cfg & PCI_MSI_FLAGS_64BIT);
+
+    msi_lo = pci_default_read_config(pdev, pdev->msi_cap + PCI_MSI_ADDRESS_LO, 4);
+    qemu_put_be32(f, msi_lo);
+
+    if (msi_64bit) {
+        msi_hi = pci_default_read_config(pdev, pdev->msi_cap + PCI_MSI_ADDRESS_HI, 4);
+        qemu_put_be32(f, msi_hi);
+    }
+
+    msi_data = pci_default_read_config(pdev,
+                 pdev->msi_cap + (msi_64bit ? PCI_MSI_DATA_64 : PCI_MSI_DATA_32), 2);
+    qemu_put_be32(f, msi_data);
+
+    buf = g_malloc(sz);
+    if (buf == NULL) {
+        error_report("vfio: Failed to allocate memory for migrate\n");
+        goto exit;
+    }
+
+    if (pread(vdev->vbasedev.fd, buf, sz,
+              vdev->device_state.offset + VFIO_DEVICE_STATE_OFFSET) != sz) {
+        error_report("vfio: Failed to read Device State Region\n");
+        goto exit;
+    }
+
+    qemu_put_buffer(f, buf, sz);
+
+exit:
+    if (buf)
+        g_free(buf);
+
+    return 0;
+}
+
+static int vfio_device_get(QEMUFile *f, void *pv, size_t size, VMStateField *field)
+{
+    VFIOPCIDevice *vdev = pv;
+    PCIDevice *pdev = &vdev->pdev;
+    int sz = vdev->device_state.size - VFIO_DEVICE_STATE_OFFSET;
+    uint8_t *buf = NULL;
+    uint32_t ctl, msi_lo, msi_hi, msi_data, bar_cfg, i;
+    bool msi_64bit;
+
+    /* retore pci bar configuration */
+    ctl = pci_default_read_config(pdev, PCI_COMMAND, 2);
+    vfio_pci_write_config(pdev, PCI_COMMAND,
+                          ctl & (!(PCI_COMMAND_IO | PCI_COMMAND_MEMORY)), 2);
+    for (i = 0; i < PCI_ROM_SLOT; i++) {
+        bar_cfg = qemu_get_be32(f);
+        vfio_pci_write_config(pdev, PCI_BASE_ADDRESS_0 + i*4, bar_cfg, 4);
+    }
+    vfio_pci_write_config(pdev, PCI_COMMAND,
+                          ctl | PCI_COMMAND_IO | PCI_COMMAND_MEMORY, 2);
+
+    /* restore msi configuration */
+    ctl = pci_default_read_config(pdev, pdev->msi_cap + PCI_MSI_FLAGS, 2);
+    msi_64bit = !!(ctl & PCI_MSI_FLAGS_64BIT);
+
+    vfio_pci_write_config(&vdev->pdev,
+                          pdev->msi_cap + PCI_MSI_FLAGS,
+                          ctl & (!PCI_MSI_FLAGS_ENABLE), 2);
+
+    msi_lo = qemu_get_be32(f);
+    vfio_pci_write_config(pdev, pdev->msi_cap + PCI_MSI_ADDRESS_LO, msi_lo, 4);
+
+    if (msi_64bit) {
+        msi_hi = qemu_get_be32(f);
+        vfio_pci_write_config(pdev, pdev->msi_cap + PCI_MSI_ADDRESS_HI, msi_hi, 4);
+    }
+    msi_data = qemu_get_be32(f);
+    vfio_pci_write_config(pdev,
+                          pdev->msi_cap + (msi_64bit ? PCI_MSI_DATA_64 : PCI_MSI_DATA_32),
+                          msi_data, 2);
+
+    vfio_pci_write_config(&vdev->pdev, pdev->msi_cap + PCI_MSI_FLAGS,
+                          ctl | PCI_MSI_FLAGS_ENABLE, 2);
+
+    buf = g_malloc(sz);
+    if (buf == NULL) {
+        error_report("vfio: Failed to allocate memory for migrate\n");
+        return -1;
+    }
+
+    qemu_get_buffer(f, buf, sz);
+    if (pwrite(vdev->vbasedev.fd, buf, sz,
+               vdev->device_state.offset + VFIO_DEVICE_STATE_OFFSET) != sz) {
+        error_report("vfio: Failed to write Device State Region\n");
+        return -1;
+    }
+
+    if (buf)
+	g_free(buf);
+    return 0;
+}
+
 static void vfio_instance_init(Object *obj)
 {
     PCIDevice *pci_dev = PCI_DEVICE(obj);
@@ -3003,9 +3115,29 @@ static Property vfio_pci_dev_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static VMStateInfo vfio_vmstate_info = {
+    .name = "vfio-state",
+    .get = vfio_device_get,
+    .put = vfio_device_put,
+};
+
 static VMStateDescription vfio_pci_vmstate = {
     .name = "vfio-pci",
     .unmigratable = 1,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        {
+            .name         = "vfio dev",
+            .version_id   = 0,
+            .field_exists = NULL,
+            .size         = 0,
+            .info         = &vfio_vmstate_info,
+            .flags        = VMS_SINGLE,
+            .offset       = 0,
+         },
+        VMSTATE_END_OF_LIST()
+    },
 };
 
 static void vfio_pci_dev_class_init(ObjectClass *klass, void *data)
diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index ae1b953..dbbe7e1 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -302,6 +302,8 @@ struct vfio_region_info_cap_type {
 #define VFIO_DEVICE_START	0
 #define VFIO_DEVICE_STOP	1
 
+#define VFIO_DEVICE_STATE_OFFSET       1
+
 /**
  * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
  *				    struct vfio_irq_info)
-- 
2.7.4

[Qemu-devel] [RFC V2 4/4] vifo: introduce new VFIO ioctl VFIO_IOMMU_GET_DIRTY_BITMAP

[Yulei Zhang]

New VFIO ioctl VFIO_IOMMU_GET_DIRTY_BITMAP is used to fetch the
bitmap of pinned memory in iommu container, we need copy those
memory to the target during the migration as they are dirtied by
mdev devices.

Signed-off-by: Yulei Zhang <yulei.zhang@intel.com>
---
 hw/vfio/common.c           | 32 ++++++++++++++++++++++++++++++++
 linux-headers/linux/vfio.h | 14 ++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index f3ba9b9..54d43d5 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -35,6 +35,7 @@
 #include "sysemu/kvm.h"
 #include "trace.h"
 #include "qapi/error.h"
+#include "exec/ram_addr.h"
 
 struct vfio_group_head vfio_group_list =
     QLIST_HEAD_INITIALIZER(vfio_group_list);
@@ -603,9 +604,40 @@ static void vfio_listener_region_del(MemoryListener *listener,
     }
 }
 
+static void vfio_log_sync(MemoryListener *listener,
+                          MemoryRegionSection *section)
+{
+    VFIOContainer *container = container_of(listener, VFIOContainer, listener);
+    VFIOGroup *group = QLIST_FIRST(&container->group_list);
+    VFIODevice *vbasedev;
+    QLIST_FOREACH(vbasedev, &group->device_list, next) {
+       if (vbasedev->device_state == VFIO_DEVICE_START)
+		return;
+    }
+
+    struct vfio_iommu_get_dirty_bitmap *d;
+    ram_addr_t size = int128_get64(section->size);
+    unsigned long page_nr = size >> TARGET_PAGE_BITS;
+    unsigned long bitmap_size = (BITS_TO_LONGS(page_nr) + 1) * sizeof(unsigned long);
+    d = g_malloc0(sizeof(*d) + bitmap_size);
+    d->start_addr = section->offset_within_address_space;
+    d->page_nr = page_nr;
+
+    if (ioctl(container->fd, VFIO_IOMMU_GET_DIRTY_BITMAP, d)) {
+        error_report("vfio: Failed to fetch dirty pages for migration\n");
+        goto exit;
+    }
+
+    cpu_physical_memory_set_dirty_lebitmap((unsigned long*)&d->dirty_bitmap, d->start_addr, d->page_nr);
+
+exit:
+    g_free(d);
+}
+
 static const MemoryListener vfio_memory_listener = {
     .region_add = vfio_listener_region_add,
     .region_del = vfio_listener_region_del,
+    .log_sync = vfio_log_sync,
 };
 
 static void vfio_listener_release(VFIOContainer *container)
diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index dbbe7e1..cf3d163 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -553,6 +553,20 @@ struct vfio_iommu_type1_dma_unmap {
 #define VFIO_IOMMU_ENABLE	_IO(VFIO_TYPE, VFIO_BASE + 15)
 #define VFIO_IOMMU_DISABLE	_IO(VFIO_TYPE, VFIO_BASE + 16)
 
+/**
+ * VFIO_IOMMU_GET_DIRTY_BITMAP - _IOW(VFIO_TYPE, VFIO_BASE + 17,
+ *				    struct vfio_iommu_get_dirty_bitmap)
+ *
+ * Return: 0 on success, -errno on failure.
+ */
+struct vfio_iommu_get_dirty_bitmap{
+	__u64	       start_addr;
+	__u64	       page_nr;
+	__u8           dirty_bitmap[];
+};
+
+#define VFIO_IOMMU_GET_DIRTY_BITMAP _IO(VFIO_TYPE, VFIO_BASE + 17)
+
 /* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */
 
 /*
-- 
2.7.4

Re: [Qemu-devel] [RFC V2 0/4] vfio: Introduce Live migration capability to vfio_mdev device

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [RFC V2 0/4] vfio: Introduce Live migration capability to vfio_mdev device
Message-id: 1494316727-15518-1-git-send-email-yulei.zhang@intel.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
e72290dd77 vifo: introduce new VFIO ioctl VFIO_IOMMU_GET_DIRTY_BITMAP
e63ec9785b vfio: Add struct vfio_vmstate_info to introduce put/get callback funtion for vfio device status save/restore
196e200d3c vfio: Add vm status change callback to stop/restart the mdev device
bd9ff9bed2 vfio: introduce a new VFIO sub region for mdev device migration support

=== OUTPUT BEGIN ===
Checking PATCH 1/4: vfio: introduce a new VFIO sub region for mdev device migration support...
WARNING: line over 80 characters
#34: FILE: hw/vfio/pci.c:2822:
+	memcpy(&vdev->device_state, device_state, sizeof(struct vfio_region_info));

ERROR: code indent should never use tabs
#34: FILE: hw/vfio/pci.c:2822:
+^Imemcpy(&vdev->device_state, device_state, sizeof(struct vfio_region_info));$

ERROR: code indent should never use tabs
#35: FILE: hw/vfio/pci.c:2823:
+^Ig_free(device_state);$

ERROR: initializer for struct VMStateDescription should normally be const
#47: FILE: hw/vfio/pci.c:3008:
+static VMStateDescription vfio_pci_vmstate = {

total: 3 errors, 1 warnings, 51 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/4: vfio: Add vm status change callback to stop/restart the mdev device...
WARNING: line over 80 characters
#49: FILE: hw/vfio/pci.c:2975:
+    if (pwrite(vdev->vbasedev.fd, &dev_state, sz, vdev->device_state.offset) != sz) {

ERROR: Error messages should not contain newlines
#50: FILE: hw/vfio/pci.c:2976:
+        error_report("vfio: Failed to %s device\n", running ? "start" : "stop");

total: 1 errors, 1 warnings, 53 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/4: vfio: Add struct vfio_vmstate_info to introduce put/get callback funtion for vfio device status save/restore...
WARNING: line over 80 characters
#33: FILE: hw/vfio/pci.c:2983:
+static int vfio_device_put(QEMUFile *f, void *pv, size_t size, VMStateField *field,

ERROR: spaces required around that '*' (ctx:VxV)
#44: FILE: hw/vfio/pci.c:2994:
+        bar_cfg = pci_default_read_config(pdev, PCI_BASE_ADDRESS_0 + i*4, 4);
                                                                       ^

WARNING: line over 80 characters
#51: FILE: hw/vfio/pci.c:3001:
+    msi_lo = pci_default_read_config(pdev, pdev->msi_cap + PCI_MSI_ADDRESS_LO, 4);

WARNING: line over 80 characters
#55: FILE: hw/vfio/pci.c:3005:
+        msi_hi = pci_default_read_config(pdev, pdev->msi_cap + PCI_MSI_ADDRESS_HI, 4);

WARNING: line over 80 characters
#60: FILE: hw/vfio/pci.c:3010:
+                 pdev->msi_cap + (msi_64bit ? PCI_MSI_DATA_64 : PCI_MSI_DATA_32), 2);

ERROR: Error messages should not contain newlines
#65: FILE: hw/vfio/pci.c:3015:
+        error_report("vfio: Failed to allocate memory for migrate\n");

ERROR: Error messages should not contain newlines
#71: FILE: hw/vfio/pci.c:3021:
+        error_report("vfio: Failed to read Device State Region\n");

ERROR: braces {} are necessary for all arms of this statement
#78: FILE: hw/vfio/pci.c:3028:
+    if (buf)
[...]

ERROR: g_free(NULL) is safe this check is probably not required
#79: FILE: hw/vfio/pci.c:3029:
+    if (buf)
+        g_free(buf);

WARNING: line over 80 characters
#84: FILE: hw/vfio/pci.c:3034:
+static int vfio_device_get(QEMUFile *f, void *pv, size_t size, VMStateField *field)

ERROR: spaces required around that '*' (ctx:VxV)
#99: FILE: hw/vfio/pci.c:3049:
+        vfio_pci_write_config(pdev, PCI_BASE_ADDRESS_0 + i*4, bar_cfg, 4);
                                                           ^

WARNING: line over 80 characters
#117: FILE: hw/vfio/pci.c:3067:
+        vfio_pci_write_config(pdev, pdev->msi_cap + PCI_MSI_ADDRESS_HI, msi_hi, 4);

WARNING: line over 80 characters
#121: FILE: hw/vfio/pci.c:3071:
+                          pdev->msi_cap + (msi_64bit ? PCI_MSI_DATA_64 : PCI_MSI_DATA_32),

ERROR: Error messages should not contain newlines
#129: FILE: hw/vfio/pci.c:3079:
+        error_report("vfio: Failed to allocate memory for migrate\n");

ERROR: Error messages should not contain newlines
#136: FILE: hw/vfio/pci.c:3086:
+        error_report("vfio: Failed to write Device State Region\n");

ERROR: braces {} are necessary for all arms of this statement
#140: FILE: hw/vfio/pci.c:3090:
+    if (buf)
[...]

ERROR: code indent should never use tabs
#141: FILE: hw/vfio/pci.c:3091:
+^Ig_free(buf);$

ERROR: g_free(NULL) is safe this check is probably not required
#141: FILE: hw/vfio/pci.c:3091:
+    if (buf)
+	g_free(buf);

ERROR: initializer for struct VMStateInfo should normally be const
#152: FILE: hw/vfio/pci.c:3139:
+static VMStateInfo vfio_vmstate_info = {

total: 12 errors, 7 warnings, 155 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 4/4: vifo: introduce new VFIO ioctl VFIO_IOMMU_GET_DIRTY_BITMAP...
ERROR: braces {} are necessary for all arms of this statement
#37: FILE: hw/vfio/common.c:635:
+       if (vbasedev->device_state == VFIO_DEVICE_START)
[...]

ERROR: code indent should never use tabs
#38: FILE: hw/vfio/common.c:636:
+^I^Ireturn;$

WARNING: line over 80 characters
#44: FILE: hw/vfio/common.c:642:
+    unsigned long bitmap_size = (BITS_TO_LONGS(page_nr) + 1) * sizeof(unsigned long);

ERROR: Error messages should not contain newlines
#50: FILE: hw/vfio/common.c:648:
+        error_report("vfio: Failed to fetch dirty pages for migration\n");

ERROR: line over 90 characters
#54: FILE: hw/vfio/common.c:652:
+    cpu_physical_memory_set_dirty_lebitmap((unsigned long*)&d->dirty_bitmap, d->start_addr, d->page_nr);

ERROR: "(foo*)" should be "(foo *)"
#54: FILE: hw/vfio/common.c:652:
+    cpu_physical_memory_set_dirty_lebitmap((unsigned long*)&d->dirty_bitmap, d->start_addr, d->page_nr);

total: 5 errors, 1 warnings, 67 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [RFC V2 0/4] vfio: Introduce Live migration capability to vfio_mdev device

[Tian, Kevin]

> From: Zhang, Yulei
> Sent: Tuesday, May 9, 2017 3:59 PM
> 
> Summary
> 
> This series RFC would like to introduce the live migration capability
> to vfio_mdev device.
> 
> As currently vfio_mdev device don't support migration, we introduce
> a new vfio subtype region
> VFIO_REGION_SUBTYPE_INTEL_IGD_DEVICE_STATE
> for Intel vGPU device, during the vfio device initialization, the mdev
> device will be set to migratable if the new region exist.

Looking at your series, there is really nothing specific to vGPU or
even Intel vGPU regarding to device state save/restore...

> 
> The intention to add the new region is using it for vfio_mdev device
> status save and restore during the migration. The access to this region
> will be trapped and forward to the vfio_mdev device driver. And we use
> the first byte in the new region to control the running state of mdev
> device.
> 
> Meanwhile we add one new ioctl VFIO_IOMMU_GET_DIRTY_BITMAP to help
> do
> the mdev device dirty page synchronization.
> 
> So the vfio_mdev device migration sequence would be
> Source VM side:
> 			start migration
> 				|
> 				V
>         	 get the cpu state change callback, write to the
>         	 subregion's first byte to stop the mdev device
> 				|
> 				V
> 		 quary the dirty page bitmap from iommu container
> 		 and add into qemu dirty list for synchronization
> 				|
> 				V
> 		 save the deivce status into Qemufile which is
>                      read from the vfio device subregion
> 
> Target VM side:
> 		   restore the mdev device after get the
> 		     saved status context from Qemufile
> 				|
> 				V
> 		     get the cpu state change callback
>  		     write to subregion's first byte to
>                       start the mdev device to put it in
>                       running status
> 				|
> 				V
> 			finish migration
> 
> V1->V2:
> Per Alex's suggestion:
> 1. use device subtype region instead of VFIO PCI fixed region.
> 2. remove unnecessary ioctl, use the first byte of subregion to
>    control the running state of mdev device.
> 3. for dirty page synchronization, implement the interface with
>    VFIOContainer instead of vfio pci device.
> 
> Yulei Zhang (4):
>   vfio: introduce a new VFIO sub region for mdev device migration
>     support
>   vfio: Add vm status change callback to stop/restart the mdev device
>   vfio: Add struct vfio_vmstate_info to introduce put/get callback
>     funtion     for vfio device status save/restore
>   vifo: introduce new VFIO ioctl VFIO_IOMMU_GET_DIRTY_BITMAP
> 
>  hw/vfio/common.c              |  32 +++++++++
>  hw/vfio/pci.c                 | 164
> +++++++++++++++++++++++++++++++++++++++++-
>  hw/vfio/pci.h                 |   1 +
>  include/hw/vfio/vfio-common.h |   1 +
>  linux-headers/linux/vfio.h    |  26 ++++++-
>  5 files changed, 220 insertions(+), 4 deletions(-)
> 
> --
> 2.7.4

Re: [Qemu-devel] [RFC V2 0/4] vfio: Introduce Live migration capability to vfio_mdev device

[Zhang, Yulei]

> -----Original Message-----
> From: Tian, Kevin
> Sent: Monday, July 31, 2017 2:55 PM
> To: Zhang, Yulei <yulei.zhang@intel.com>; qemu-devel@nongnu.org
> Cc: joonas.lahtinen@linux.intel.com; zhenyuw@linux.intel.com; Zheng, Xiao
> <xiao.zheng@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>;
> alex.williamson@redhat.com
> Subject: RE: [Qemu-devel][RFC V2 0/4] vfio: Introduce Live migration
> capability to vfio_mdev device
> 
> > From: Zhang, Yulei
> > Sent: Tuesday, May 9, 2017 3:59 PM
> >
> > Summary
> >
> > This series RFC would like to introduce the live migration capability
> > to vfio_mdev device.
> >
> > As currently vfio_mdev device don't support migration, we introduce a
> > new vfio subtype region
> VFIO_REGION_SUBTYPE_INTEL_IGD_DEVICE_STATE
> > for Intel vGPU device, during the vfio device initialization, the mdev
> > device will be set to migratable if the new region exist.
> 
> Looking at your series, there is really nothing specific to vGPU or even Intel
> vGPU regarding to device state save/restore...
> 

You are right, we may define a new subtype for it as it is common region for vfio 
device state save/restore.

> >
> > The intention to add the new region is using it for vfio_mdev device
> > status save and restore during the migration. The access to this
> > region will be trapped and forward to the vfio_mdev device driver. And
> > we use the first byte in the new region to control the running state
> > of mdev device.
> >
> > Meanwhile we add one new ioctl VFIO_IOMMU_GET_DIRTY_BITMAP to
> help do
> > the mdev device dirty page synchronization.
> >
> > So the vfio_mdev device migration sequence would be Source VM side:
> > 			start migration
> > 				|
> > 				V
> >         	 get the cpu state change callback, write to the
> >         	 subregion's first byte to stop the mdev device
> > 				|
> > 				V
> > 		 quary the dirty page bitmap from iommu container
> > 		 and add into qemu dirty list for synchronization
> > 				|
> > 				V
> > 		 save the deivce status into Qemufile which is
> >                      read from the vfio device subregion
> >
> > Target VM side:
> > 		   restore the mdev device after get the
> > 		     saved status context from Qemufile
> > 				|
> > 				V
> > 		     get the cpu state change callback
> >  		     write to subregion's first byte to
> >                       start the mdev device to put it in
> >                       running status
> > 				|
> > 				V
> > 			finish migration
> >
> > V1->V2:
> > Per Alex's suggestion:
> > 1. use device subtype region instead of VFIO PCI fixed region.
> > 2. remove unnecessary ioctl, use the first byte of subregion to
> >    control the running state of mdev device.
> > 3. for dirty page synchronization, implement the interface with
> >    VFIOContainer instead of vfio pci device.
> >
> > Yulei Zhang (4):
> >   vfio: introduce a new VFIO sub region for mdev device migration
> >     support
> >   vfio: Add vm status change callback to stop/restart the mdev device
> >   vfio: Add struct vfio_vmstate_info to introduce put/get callback
> >     funtion     for vfio device status save/restore
> >   vifo: introduce new VFIO ioctl VFIO_IOMMU_GET_DIRTY_BITMAP
> >
> >  hw/vfio/common.c              |  32 +++++++++
> >  hw/vfio/pci.c                 | 164
> > +++++++++++++++++++++++++++++++++++++++++-
> >  hw/vfio/pci.h                 |   1 +
> >  include/hw/vfio/vfio-common.h |   1 +
> >  linux-headers/linux/vfio.h    |  26 ++++++-
> >  5 files changed, 220 insertions(+), 4 deletions(-)
> >
> > --
> > 2.7.4