All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v29 00/17] Add migration support for VFIO devices
@ 2020-10-26  9:36 Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 01/17] vfio: Add function to unmap VFIO region Kirti Wankhede
                   ` (16 more replies)
  0 siblings, 17 replies; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

Hi,

This Patch set adds migration support for VFIO devices in QEMU.

This Patch set include patches as below:
Patch 1-2:
- Few code refactor

Patch 3:
- Added save and restore functions for PCI configuration space. Used
  pci_device_save() and pci_device_load() so that config space cache is saved
  and restored.

Patch 4-9:
- Generic migration functionality for VFIO device.
  * This patch set adds functionality for PCI devices, but can be
    extended to other VFIO devices.
  * Added all the basic functions required for pre-copy, stop-and-copy and
    resume phases of migration.
  * Added state change notifier and from that notifier function, VFIO
    device's state changed is conveyed to VFIO device driver.
  * During save setup phase and resume/load setup phase, migration region
    is queried and is used to read/write VFIO device data.
  * .save_live_pending and .save_live_iterate are implemented to use QEMU's
    functionality of iteration during pre-copy phase.
  * In .save_live_complete_precopy, that is in stop-and-copy phase,
    iteration to read data from VFIO device driver is implemented till pending
    bytes returned by driver are zero.

Patch 10
- Set DIRTY_MEMORY_MIGRATION flag in dirty log mask for migration with vIOMMU
  enabled.

Patch 11:
- Get migration capability from kernel module.

Patch 12-13:
- Add function to start and stop dirty pages tracking.
- Add vfio_listerner_log_sync to mark dirty pages. Dirty pages bitmap is queried
  per container. All pages pinned by vendor driver through vfio_pin_pages
  external API has to be marked as dirty during  migration.
  When there are CPU writes, CPU dirty page tracking can identify dirtied
  pages, but any page pinned by vendor driver can also be written by
  device. As of now there is no device which has hardware support for
  dirty page tracking. So all pages which are pinned by vendor driver
  should be considered as dirty.
  In Qemu, marking pages dirty is only done when device is in stop-and-copy
  phase because if pages are marked dirty during pre-copy phase and content is
  transfered from source to distination, there is no way to know newly dirtied
  pages from the point they were copied earlier until device stops. To avoid
  repeated copy of same content, pinned pages are marked dirty only during
  stop-and-copy phase.

Patch 14:
  When vIOMMU is enabled, used IOMMU notifier to get call back for mapped pages
  on replay during stop-and-copy phase.

Patch 15:
- With vIOMMU, IO virtual address range can get unmapped while in pre-copy
  phase of migration. In that case, unmap ioctl should return pages pinned
  in that range and QEMU should report corresponding guest physical pages
  dirty.

Patch 16:
- Make VFIO PCI device migration capable. If migration region is not provided by
  driver, migration is blocked.

Patch 17:
- Added VFIO device stats to MigrationInfo

Yet TODO:
Since there is no device which has hardware support for system memmory
dirty bitmap tracking, right now there is no other API from vendor driver
to VFIO IOMMU module to report dirty pages. In future, when such hardware
support will be implemented, an API will be required in kernel such that
vendor driver could report dirty pages to VFIO module during migration phases.

Below is the flow of state change for live migration where states in brackets
represent VM state, migration state and VFIO device state as:
    (VM state, MIGRATION_STATUS, VFIO_DEVICE_STATE)

Live migration save path:
        QEMU normal running state
        (RUNNING, _NONE, _RUNNING)
                        |
    migrate_init spawns migration_thread.
    (RUNNING, _SETUP, _RUNNING|_SAVING)
    Migration thread then calls each device's .save_setup()
                        |
    (RUNNING, _ACTIVE, _RUNNING|_SAVING)
    If device is active, get pending bytes by .save_live_pending()
    if pending bytes >= threshold_size,  call save_live_iterate()
    Data of VFIO device for pre-copy phase is copied.
    Iterate till total pending bytes converge and are less than threshold
                        |
    On migration completion, vCPUs stops and calls .save_live_complete_precopy
    for each active device. VFIO device is then transitioned in
     _SAVING state.
    (FINISH_MIGRATE, _DEVICE, _SAVING)
    For VFIO device, iterate in .save_live_complete_precopy until
    pending data is 0.
    (FINISH_MIGRATE, _DEVICE, _STOPPED)
                        |
    (FINISH_MIGRATE, _COMPLETED, _STOPPED)
    Migraton thread schedule cleanup bottom half and exit

Live migration resume path:
    Incomming migration calls .load_setup for each device
    (RESTORE_VM, _ACTIVE, _STOPPED)
                        |
    For each device, .load_state is called for that device section data
    (RESTORE_VM, _ACTIVE, _RESUMING)
                        |
    At the end, called .load_cleanup for each device and vCPUs are started.
                        |
        (RUNNING, _NONE, _RUNNING)

Note that:
- Migration post copy is not supported.

v28 -> 29
- Nit picks.
- Write through PCI_COMMAND register on loading PCI config space as suggested by
  Yan.

v27 -> 28
- Nit picks and minor changes suggested by Alex.

v26 -> 27
- Major change in Patch 3 -PCI config space save and long using VMSTATE_*
- Major change in Patch 14 - Dirty page tracking when vIOMMU is enabled using IOMMU notifier and
  its replay functionality - as suggested by Alex.
- Some Structure changes to keep all migration related members at one place.
- Pulled fix suggested by Zhi Wang <zhi.wang.linux@gmail.com>
  https://www.mail-archive.com/qemu-devel@nongnu.org/msg743722.html
- Add comments where even suggested and required.

v25 -> 26
- Removed emulated_config_bits cache and vdev->pdev.wmask from config space save
  load functions.
- Used VMStateDescription for config space save and load functionality.
- Major fixes from previous version review.
  https://www.mail-archive.com/qemu-devel@nongnu.org/msg714625.html

v23 -> 25
- Updated config space save and load to save config cache, emulated bits cache
  and wmask cache.
- Created idr string as suggested by Dr Dave that includes bus path.
- Updated save and load function to read/write data to mixed regions, mapped or
  trapped.
- When vIOMMU is enabled, created mapped iova range list which also keeps
  translated address. This list is used to mark dirty pages. This reduces
  downtime significantly with vIOMMU enabled than migration patches from
   previous version. 
- Removed get_address_limit() function from v23 patch as this not required now.

v22 -> v23
-- Fixed issue reported by Yan
https://lore.kernel.org/kvm/97977ede-3c5b-c5a5-7858-7eecd7dd531c@nvidia.com/
- Sending this version to test v23 kernel version patches:
https://lore.kernel.org/kvm/1589998088-3250-1-git-send-email-kwankhede@nvidia.com/

v18 -> v22
- Few fixes from v18 review. But not yet fixed all concerns. I'll address those
  concerns in subsequent iterations.
- Sending this version to test v22 kernel version patches:
https://lore.kernel.org/kvm/1589781397-28368-1-git-send-email-kwankhede@nvidia.com/

v16 -> v18
- Nit fixes
- Get migration capability flags from container
- Added VFIO stats to MigrationInfo
- Fixed bug reported by Yan
    https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg00004.html

v9 -> v16
- KABI almost finalised on kernel patches.
- Added support for migration with vIOMMU enabled.

v8 -> v9:
- Split patch set in 2 sets, Kernel and QEMU sets.
- Dirty pages bitmap is queried from IOMMU container rather than from
  vendor driver for per device. Added 2 ioctls to achieve this.

v7 -> v8:
- Updated comments for KABI
- Added BAR address validation check during PCI device's config space load as
  suggested by Dr. David Alan Gilbert.
- Changed vfio_migration_set_state() to set or clear device state flags.
- Some nit fixes.

v6 -> v7:
- Fix build failures.

v5 -> v6:
- Fix build failure.

v4 -> v5:
- Added decriptive comment about the sequence of access of members of structure
  vfio_device_migration_info to be followed based on Alex's suggestion
- Updated get dirty pages sequence.
- As per Cornelia Huck's suggestion, added callbacks to VFIODeviceOps to
  get_object, save_config and load_config.
- Fixed multiple nit picks.
- Tested live migration with multiple vfio device assigned to a VM.

v3 -> v4:
- Added one more bit for _RESUMING flag to be set explicitly.
- data_offset field is read-only for user space application.
- data_size is read for every iteration before reading data from migration, that
  is removed assumption that data will be till end of migration region.
- If vendor driver supports mappable sparsed region, map those region during
  setup state of save/load, similarly unmap those from cleanup routines.
- Handles race condition that causes data corruption in migration region during
  save device state by adding mutex and serialiaing save_buffer and
  get_dirty_pages routines.
- Skip called get_dirty_pages routine for mapped MMIO region of device.
- Added trace events.
- Splitted into multiple functional patches.

v2 -> v3:
- Removed enum of VFIO device states. Defined VFIO device state with 2 bits.
- Re-structured vfio_device_migration_info to keep it minimal and defined action
  on read and write access on its members.

v1 -> v2:
- Defined MIGRATION region type and sub-type which should be used with region
  type capability.
- Re-structured vfio_device_migration_info. This structure will be placed at 0th
  offset of migration region.
- Replaced ioctl with read/write for trapped part of migration region.
- Added both type of access support, trapped or mmapped, for data section of the
  region.
- Moved PCI device functions to pci file.
- Added iteration to get dirty page bitmap until bitmap for all requested pages
  are copied.

Thanks,
Kirti


Kirti Wankhede (17):
  vfio: Add function to unmap VFIO region
  vfio: Add vfio_get_object callback to VFIODeviceOps
  vfio: Add save and load functions for VFIO PCI devices
  vfio: Add migration region initialization and finalize function
  vfio: Add VM state change handler to know state of VM
  vfio: Add migration state change notifier
  vfio: Register SaveVMHandlers for VFIO device
  vfio: Add save state functions to SaveVMHandlers
  vfio: Add load state functions to SaveVMHandlers
  memory: Set DIRTY_MEMORY_MIGRATION when IOMMU is enabled
  vfio: Get migration capability flags for container
  vfio: Add function to start and stop dirty pages tracking
  vfio: Add vfio_listener_log_sync to mark dirty pages
  vfio: Dirty page tracking when vIOMMU is enabled
  vfio: Add ioctl to get dirty pages bitmap during dma unmap
  vfio: Make vfio-pci device migration capable
  qapi: Add VFIO devices migration stats in Migration stats

 hw/vfio/common.c              | 441 +++++++++++++++++++-
 hw/vfio/meson.build           |   1 +
 hw/vfio/migration.c           | 933 ++++++++++++++++++++++++++++++++++++++++++
 hw/vfio/pci.c                 |  87 +++-
 hw/vfio/pci.h                 |   1 -
 hw/vfio/trace-events          |  21 +
 include/hw/vfio/vfio-common.h |  26 ++
 migration/migration.c         |  17 +
 monitor/hmp-cmds.c            |   6 +
 qapi/migration.json           |  17 +
 softmmu/memory.c              |   2 +-
 11 files changed, 1508 insertions(+), 44 deletions(-)
 create mode 100644 hw/vfio/migration.c

-- 
2.7.0



^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v29 01/17] vfio: Add function to unmap VFIO region
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 02/17] vfio: Add vfio_get_object callback to VFIODeviceOps Kirti Wankhede
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

This function will be used for migration region.
Migration region is mmaped when migration starts and will be unmapped when
migration is complete.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Neo Jia <cjia@nvidia.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/vfio/common.c              | 32 ++++++++++++++++++++++++++++----
 hw/vfio/trace-events          |  1 +
 include/hw/vfio/vfio-common.h |  1 +
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 13471ae29436..c6e98b8d61be 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -924,6 +924,18 @@ int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
     return 0;
 }
 
+static void vfio_subregion_unmap(VFIORegion *region, int index)
+{
+    trace_vfio_region_unmap(memory_region_name(&region->mmaps[index].mem),
+                            region->mmaps[index].offset,
+                            region->mmaps[index].offset +
+                            region->mmaps[index].size - 1);
+    memory_region_del_subregion(region->mem, &region->mmaps[index].mem);
+    munmap(region->mmaps[index].mmap, region->mmaps[index].size);
+    object_unparent(OBJECT(&region->mmaps[index].mem));
+    region->mmaps[index].mmap = NULL;
+}
+
 int vfio_region_mmap(VFIORegion *region)
 {
     int i, prot = 0;
@@ -954,10 +966,7 @@ int vfio_region_mmap(VFIORegion *region)
             region->mmaps[i].mmap = NULL;
 
             for (i--; i >= 0; i--) {
-                memory_region_del_subregion(region->mem, &region->mmaps[i].mem);
-                munmap(region->mmaps[i].mmap, region->mmaps[i].size);
-                object_unparent(OBJECT(&region->mmaps[i].mem));
-                region->mmaps[i].mmap = NULL;
+                vfio_subregion_unmap(region, i);
             }
 
             return ret;
@@ -982,6 +991,21 @@ int vfio_region_mmap(VFIORegion *region)
     return 0;
 }
 
+void vfio_region_unmap(VFIORegion *region)
+{
+    int i;
+
+    if (!region->mem) {
+        return;
+    }
+
+    for (i = 0; i < region->nr_mmaps; i++) {
+        if (region->mmaps[i].mmap) {
+            vfio_subregion_unmap(region, i);
+        }
+    }
+}
+
 void vfio_region_exit(VFIORegion *region)
 {
     int i;
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index 93a0bc2522f8..a0c7b49a2ebc 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -113,6 +113,7 @@ vfio_region_mmap(const char *name, unsigned long offset, unsigned long end) "Reg
 vfio_region_exit(const char *name, int index) "Device %s, region %d"
 vfio_region_finalize(const char *name, int index) "Device %s, region %d"
 vfio_region_mmaps_set_enabled(const char *name, bool enabled) "Region %s mmaps enabled: %d"
+vfio_region_unmap(const char *name, unsigned long offset, unsigned long end) "Region %s unmap [0x%lx - 0x%lx]"
 vfio_region_sparse_mmap_header(const char *name, int index, int nr_areas) "Device %s region %d: %d sparse mmap entries"
 vfio_region_sparse_mmap_entry(int i, unsigned long start, unsigned long end) "sparse entry %d [0x%lx - 0x%lx]"
 vfio_get_dev_region(const char *name, int index, uint32_t type, uint32_t subtype) "%s index %d, %08x/%0x8"
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index c78f3ff5593c..dc95f527b583 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -171,6 +171,7 @@ int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
                       int index, const char *name);
 int vfio_region_mmap(VFIORegion *region);
 void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled);
+void vfio_region_unmap(VFIORegion *region);
 void vfio_region_exit(VFIORegion *region);
 void vfio_region_finalize(VFIORegion *region);
 void vfio_reset_handler(void *opaque);
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v29 02/17] vfio: Add vfio_get_object callback to VFIODeviceOps
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 01/17] vfio: Add function to unmap VFIO region Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 03/17] vfio: Add save and load functions for VFIO PCI devices Kirti Wankhede
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

Hook vfio_get_object callback for PCI devices.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Neo Jia <cjia@nvidia.com>
Suggested-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/vfio/pci.c                 | 8 ++++++++
 include/hw/vfio/vfio-common.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 0d83eb0e47bb..bffd5bfe3b78 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2394,10 +2394,18 @@ static void vfio_pci_compute_needs_reset(VFIODevice *vbasedev)
     }
 }
 
+static Object *vfio_pci_get_object(VFIODevice *vbasedev)
+{
+    VFIOPCIDevice *vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev);
+
+    return OBJECT(vdev);
+}
+
 static VFIODeviceOps vfio_pci_ops = {
     .vfio_compute_needs_reset = vfio_pci_compute_needs_reset,
     .vfio_hot_reset_multi = vfio_pci_hot_reset_multi,
     .vfio_eoi = vfio_intx_eoi,
+    .vfio_get_object = vfio_pci_get_object,
 };
 
 int vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index dc95f527b583..fe99c36a693a 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -119,6 +119,7 @@ struct VFIODeviceOps {
     void (*vfio_compute_needs_reset)(VFIODevice *vdev);
     int (*vfio_hot_reset_multi)(VFIODevice *vdev);
     void (*vfio_eoi)(VFIODevice *vdev);
+    Object *(*vfio_get_object)(VFIODevice *vdev);
 };
 
 typedef struct VFIOGroup {
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v29 03/17] vfio: Add save and load functions for VFIO PCI devices
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 01/17] vfio: Add function to unmap VFIO region Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 02/17] vfio: Add vfio_get_object callback to VFIODeviceOps Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 04/17] vfio: Add migration region initialization and finalize function Kirti Wankhede
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

Added functions to save and restore PCI device specific data,
specifically config space of PCI device.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Neo Jia <cjia@nvidia.com>
---
 hw/vfio/pci.c                 | 51 +++++++++++++++++++++++++++++++++++++++++++
 include/hw/vfio/vfio-common.h |  2 ++
 2 files changed, 53 insertions(+)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index bffd5bfe3b78..e27c88be6d85 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -41,6 +41,7 @@
 #include "trace.h"
 #include "qapi/error.h"
 #include "migration/blocker.h"
+#include "migration/qemu-file.h"
 
 #define TYPE_VFIO_PCI_NOHOTPLUG "vfio-pci-nohotplug"
 
@@ -2401,11 +2402,61 @@ static Object *vfio_pci_get_object(VFIODevice *vbasedev)
     return OBJECT(vdev);
 }
 
+static bool vfio_msix_present(void *opaque, int version_id)
+{
+    PCIDevice *pdev = opaque;
+
+    return msix_present(pdev);
+}
+
+const VMStateDescription vmstate_vfio_pci_config = {
+    .name = "VFIOPCIDevice",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_PCI_DEVICE(pdev, VFIOPCIDevice),
+        VMSTATE_MSIX_TEST(pdev, VFIOPCIDevice, vfio_msix_present),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void vfio_pci_save_config(VFIODevice *vbasedev, QEMUFile *f)
+{
+    VFIOPCIDevice *vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev);
+
+    vmstate_save_state(f, &vmstate_vfio_pci_config, vdev, NULL);
+}
+
+static int vfio_pci_load_config(VFIODevice *vbasedev, QEMUFile *f)
+{
+    VFIOPCIDevice *vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev);
+    PCIDevice *pdev = &vdev->pdev;
+    int ret;
+
+    ret = vmstate_load_state(f, &vmstate_vfio_pci_config, vdev, 1);
+    if (ret) {
+        return ret;
+    }
+
+    vfio_pci_write_config(pdev, PCI_COMMAND,
+                          pci_get_word(pdev->config + PCI_COMMAND), 2);
+
+    if (msi_enabled(pdev)) {
+        vfio_msi_enable(vdev);
+    } else if (msix_enabled(pdev)) {
+        vfio_msix_enable(vdev);
+    }
+
+    return ret;
+}
+
 static VFIODeviceOps vfio_pci_ops = {
     .vfio_compute_needs_reset = vfio_pci_compute_needs_reset,
     .vfio_hot_reset_multi = vfio_pci_hot_reset_multi,
     .vfio_eoi = vfio_intx_eoi,
     .vfio_get_object = vfio_pci_get_object,
+    .vfio_save_config = vfio_pci_save_config,
+    .vfio_load_config = vfio_pci_load_config,
 };
 
 int vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index fe99c36a693a..ba6169cd926e 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -120,6 +120,8 @@ struct VFIODeviceOps {
     int (*vfio_hot_reset_multi)(VFIODevice *vdev);
     void (*vfio_eoi)(VFIODevice *vdev);
     Object *(*vfio_get_object)(VFIODevice *vdev);
+    void (*vfio_save_config)(VFIODevice *vdev, QEMUFile *f);
+    int (*vfio_load_config)(VFIODevice *vdev, QEMUFile *f);
 };
 
 typedef struct VFIOGroup {
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v29 04/17] vfio: Add migration region initialization and finalize function
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
                   ` (2 preceding siblings ...)
  2020-10-26  9:36 ` [PATCH v29 03/17] vfio: Add save and load functions for VFIO PCI devices Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2020-10-26 16:23   ` Cornelia Huck
  2020-10-26  9:36 ` [PATCH v29 05/17] vfio: Add VM state change handler to know state of VM Kirti Wankhede
                   ` (12 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

Whether the VFIO device supports migration or not is decided based of
migration region query. If migration region query is successful and migration
region initialization is successful then migration is supported else
migration is blocked.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Neo Jia <cjia@nvidia.com>
Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/vfio/meson.build           |   1 +
 hw/vfio/migration.c           | 122 ++++++++++++++++++++++++++++++++++++++++++
 hw/vfio/trace-events          |   3 ++
 include/hw/vfio/vfio-common.h |   9 ++++
 4 files changed, 135 insertions(+)
 create mode 100644 hw/vfio/migration.c

diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
index 37efa74018bc..da9af297a0c5 100644
--- a/hw/vfio/meson.build
+++ b/hw/vfio/meson.build
@@ -2,6 +2,7 @@ vfio_ss = ss.source_set()
 vfio_ss.add(files(
   'common.c',
   'spapr.c',
+  'migration.c',
 ))
 vfio_ss.add(when: 'CONFIG_VFIO_PCI', if_true: files(
   'display.c',
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
new file mode 100644
index 000000000000..fd7faf423cdc
--- /dev/null
+++ b/hw/vfio/migration.c
@@ -0,0 +1,122 @@
+/*
+ * Migration support for VFIO devices
+ *
+ * Copyright NVIDIA, Inc. 2020
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include <linux/vfio.h>
+
+#include "hw/vfio/vfio-common.h"
+#include "cpu.h"
+#include "migration/migration.h"
+#include "migration/qemu-file.h"
+#include "migration/register.h"
+#include "migration/blocker.h"
+#include "migration/misc.h"
+#include "qapi/error.h"
+#include "exec/ramlist.h"
+#include "exec/ram_addr.h"
+#include "pci.h"
+#include "trace.h"
+
+static void vfio_migration_exit(VFIODevice *vbasedev)
+{
+    VFIOMigration *migration = vbasedev->migration;
+
+    vfio_region_exit(&migration->region);
+    vfio_region_finalize(&migration->region);
+    g_free(vbasedev->migration);
+    vbasedev->migration = NULL;
+}
+
+static int vfio_migration_init(VFIODevice *vbasedev,
+                               struct vfio_region_info *info)
+{
+    int ret;
+    Object *obj;
+
+    if (!vbasedev->ops->vfio_get_object) {
+        return -EINVAL;
+    }
+
+    obj = vbasedev->ops->vfio_get_object(vbasedev);
+    if (!obj) {
+        return -EINVAL;
+    }
+
+    vbasedev->migration = g_new0(VFIOMigration, 1);
+
+    ret = vfio_region_setup(obj, vbasedev, &vbasedev->migration->region,
+                            info->index, "migration");
+    if (ret) {
+        error_report("%s: Failed to setup VFIO migration region %d: %s",
+                     vbasedev->name, info->index, strerror(-ret));
+        goto err;
+    }
+
+    if (!vbasedev->migration->region.size) {
+        error_report("%s: Invalid zero-sized VFIO migration region %d",
+                     vbasedev->name, info->index);
+        ret = -EINVAL;
+        goto err;
+    }
+    return 0;
+
+err:
+    vfio_migration_exit(vbasedev);
+    return ret;
+}
+
+/* ---------------------------------------------------------------------- */
+
+int vfio_migration_probe(VFIODevice *vbasedev, Error **errp)
+{
+    struct vfio_region_info *info = NULL;
+    Error *local_err = NULL;
+    int ret;
+
+    ret = vfio_get_dev_region_info(vbasedev, VFIO_REGION_TYPE_MIGRATION,
+                                   VFIO_REGION_SUBTYPE_MIGRATION, &info);
+    if (ret) {
+        goto add_blocker;
+    }
+
+    ret = vfio_migration_init(vbasedev, info);
+    if (ret) {
+        goto add_blocker;
+    }
+
+    g_free(info);
+    trace_vfio_migration_probe(vbasedev->name, info->index);
+    return 0;
+
+add_blocker:
+    error_setg(&vbasedev->migration_blocker,
+               "VFIO device doesn't support migration");
+    g_free(info);
+
+    ret = migrate_add_blocker(vbasedev->migration_blocker, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        error_free(vbasedev->migration_blocker);
+        vbasedev->migration_blocker = NULL;
+    }
+    return ret;
+}
+
+void vfio_migration_finalize(VFIODevice *vbasedev)
+{
+    if (vbasedev->migration) {
+        vfio_migration_exit(vbasedev);
+    }
+
+    if (vbasedev->migration_blocker) {
+        migrate_del_blocker(vbasedev->migration_blocker);
+        error_free(vbasedev->migration_blocker);
+        vbasedev->migration_blocker = NULL;
+    }
+}
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index a0c7b49a2ebc..9ced5ec6277c 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -145,3 +145,6 @@ vfio_display_edid_link_up(void) ""
 vfio_display_edid_link_down(void) ""
 vfio_display_edid_update(uint32_t prefx, uint32_t prefy) "%ux%u"
 vfio_display_edid_write_error(void) ""
+
+# migration.c
+vfio_migration_probe(const char *name, uint32_t index) " (%s) Region %d"
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index ba6169cd926e..8275c4c68f45 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -57,6 +57,10 @@ typedef struct VFIORegion {
     uint8_t nr; /* cache the region number for debug */
 } VFIORegion;
 
+typedef struct VFIOMigration {
+    VFIORegion region;
+} VFIOMigration;
+
 typedef struct VFIOAddressSpace {
     AddressSpace *as;
     QLIST_HEAD(, VFIOContainer) containers;
@@ -113,6 +117,8 @@ typedef struct VFIODevice {
     unsigned int num_irqs;
     unsigned int num_regions;
     unsigned int flags;
+    VFIOMigration *migration;
+    Error *migration_blocker;
 } VFIODevice;
 
 struct VFIODeviceOps {
@@ -204,4 +210,7 @@ int vfio_spapr_create_window(VFIOContainer *container,
 int vfio_spapr_remove_window(VFIOContainer *container,
                              hwaddr offset_within_address_space);
 
+int vfio_migration_probe(VFIODevice *vbasedev, Error **errp);
+void vfio_migration_finalize(VFIODevice *vbasedev);
+
 #endif /* HW_VFIO_VFIO_COMMON_H */
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v29 05/17] vfio: Add VM state change handler to know state of VM
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
                   ` (3 preceding siblings ...)
  2020-10-26  9:36 ` [PATCH v29 04/17] vfio: Add migration region initialization and finalize function Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2020-10-26 13:00   ` Alex Williamson
  2020-10-26  9:36 ` [PATCH v29 06/17] vfio: Add migration state change notifier Kirti Wankhede
                   ` (11 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

VM state change handler is called on change in VM's state. Based on
VM state, VFIO device state should be changed.
Added read/write helper functions for migration region.
Added function to set device_state.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Neo Jia <cjia@nvidia.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/vfio/migration.c           | 158 ++++++++++++++++++++++++++++++++++++++++++
 hw/vfio/trace-events          |   2 +
 include/hw/vfio/vfio-common.h |   4 ++
 3 files changed, 164 insertions(+)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index fd7faf423cdc..65ce735d667b 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -10,6 +10,7 @@
 #include "qemu/osdep.h"
 #include <linux/vfio.h>
 
+#include "sysemu/runstate.h"
 #include "hw/vfio/vfio-common.h"
 #include "cpu.h"
 #include "migration/migration.h"
@@ -22,6 +23,157 @@
 #include "exec/ram_addr.h"
 #include "pci.h"
 #include "trace.h"
+#include "hw/hw.h"
+
+static inline int vfio_mig_access(VFIODevice *vbasedev, void *val, int count,
+                                  off_t off, bool iswrite)
+{
+    int ret;
+
+    ret = iswrite ? pwrite(vbasedev->fd, val, count, off) :
+                    pread(vbasedev->fd, val, count, off);
+    if (ret < count) {
+        error_report("vfio_mig_%s %d byte %s: failed at offset 0x%lx, err: %s",
+                     iswrite ? "write" : "read", count,
+                     vbasedev->name, off, strerror(errno));
+        return (ret < 0) ? ret : -EINVAL;
+    }
+    return 0;
+}
+
+static int vfio_mig_rw(VFIODevice *vbasedev, __u8 *buf, size_t count,
+                       off_t off, bool iswrite)
+{
+    int ret, done = 0;
+    __u8 *tbuf = buf;
+
+    while (count) {
+        int bytes = 0;
+
+        if (count >= 8 && !(off % 8)) {
+            bytes = 8;
+        } else if (count >= 4 && !(off % 4)) {
+            bytes = 4;
+        } else if (count >= 2 && !(off % 2)) {
+            bytes = 2;
+        } else {
+            bytes = 1;
+        }
+
+        ret = vfio_mig_access(vbasedev, tbuf, bytes, off, iswrite);
+        if (ret) {
+            return ret;
+        }
+
+        count -= bytes;
+        done += bytes;
+        off += bytes;
+        tbuf += bytes;
+    }
+    return done;
+}
+
+#define vfio_mig_read(f, v, c, o)       vfio_mig_rw(f, (__u8 *)v, c, o, false)
+#define vfio_mig_write(f, v, c, o)      vfio_mig_rw(f, (__u8 *)v, c, o, true)
+
+#define VFIO_MIG_STRUCT_OFFSET(f)       \
+                                 offsetof(struct vfio_device_migration_info, f)
+/*
+ * Change the device_state register for device @vbasedev. Bits set in @mask
+ * are preserved, bits set in @value are set, and bits not set in either @mask
+ * or @value are cleared in device_state. If the register cannot be accessed,
+ * the resulting state would be invalid, or the device enters an error state,
+ * an error is returned.
+ */
+
+static int vfio_migration_set_state(VFIODevice *vbasedev, uint32_t mask,
+                                    uint32_t value)
+{
+    VFIOMigration *migration = vbasedev->migration;
+    VFIORegion *region = &migration->region;
+    off_t dev_state_off = region->fd_offset +
+                          VFIO_MIG_STRUCT_OFFSET(device_state);
+    uint32_t device_state;
+    int ret;
+
+    ret = vfio_mig_read(vbasedev, &device_state, sizeof(device_state),
+                        dev_state_off);
+    if (ret < 0) {
+        return ret;
+    }
+
+    device_state = (device_state & mask) | value;
+
+    if (!VFIO_DEVICE_STATE_VALID(device_state)) {
+        return -EINVAL;
+    }
+
+    ret = vfio_mig_write(vbasedev, &device_state, sizeof(device_state),
+                         dev_state_off);
+    if (ret < 0) {
+        int rret;
+
+        rret = vfio_mig_read(vbasedev, &device_state, sizeof(device_state),
+                             dev_state_off);
+
+        if ((rret < 0) || (VFIO_DEVICE_STATE_IS_ERROR(device_state))) {
+            hw_error("%s: Device in error state 0x%x", vbasedev->name,
+                     device_state);
+            return rret ? rret : -EIO;
+        }
+        return ret;
+    }
+
+    migration->device_state = device_state;
+    trace_vfio_migration_set_state(vbasedev->name, device_state);
+    return 0;
+}
+
+static void vfio_vmstate_change(void *opaque, int running, RunState state)
+{
+    VFIODevice *vbasedev = opaque;
+    VFIOMigration *migration = vbasedev->migration;
+    uint32_t value, mask;
+    int ret;
+
+    if ((vbasedev->migration->vm_running == running)) {
+        return;
+    }
+
+    if (running) {
+        /*
+         * Here device state can have one of _SAVING, _RESUMING or _STOP bit.
+         * Transition from _SAVING to _RUNNING can happen if there is migration
+         * failure, in that case clear _SAVING bit.
+         * Transition from _RESUMING to _RUNNING occurs during resuming
+         * phase, in that case clear _RESUMING bit.
+         * In both the above cases, set _RUNNING bit.
+         */
+        mask = ~VFIO_DEVICE_STATE_MASK;
+        value = VFIO_DEVICE_STATE_RUNNING;
+    } else {
+        /*
+         * Here device state could be either _RUNNING or _SAVING|_RUNNING. Reset
+         * _RUNNING bit
+         */
+        mask = ~VFIO_DEVICE_STATE_RUNNING;
+        value = 0;
+    }
+
+    ret = vfio_migration_set_state(vbasedev, mask, value);
+    if (ret) {
+        /*
+         * Migration should be aborted in this case, but vm_state_notify()
+         * currently does not support reporting failures.
+         */
+        error_report("%s: Failed to set device state 0x%x", vbasedev->name,
+                     (migration->device_state & mask) | value);
+        qemu_file_set_error(migrate_get_current()->to_dst_file, ret);
+    }
+    vbasedev->migration->vm_running = running;
+    trace_vfio_vmstate_change(vbasedev->name, running, RunState_str(state),
+            (migration->device_state & mask) | value);
+}
 
 static void vfio_migration_exit(VFIODevice *vbasedev)
 {
@@ -64,6 +216,9 @@ static int vfio_migration_init(VFIODevice *vbasedev,
         ret = -EINVAL;
         goto err;
     }
+
+    migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change,
+                                                           vbasedev);
     return 0;
 
 err:
@@ -111,6 +266,9 @@ add_blocker:
 void vfio_migration_finalize(VFIODevice *vbasedev)
 {
     if (vbasedev->migration) {
+        VFIOMigration *migration = vbasedev->migration;
+
+        qemu_del_vm_change_state_handler(migration->vm_state);
         vfio_migration_exit(vbasedev);
     }
 
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index 9ced5ec6277c..41de81f12f60 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -148,3 +148,5 @@ vfio_display_edid_write_error(void) ""
 
 # migration.c
 vfio_migration_probe(const char *name, uint32_t index) " (%s) Region %d"
+vfio_migration_set_state(const char *name, uint32_t state) " (%s) state %d"
+vfio_vmstate_change(const char *name, int running, const char *reason, uint32_t dev_state) " (%s) running %d reason %s device state %d"
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 8275c4c68f45..9a571f1fb552 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -29,6 +29,7 @@
 #ifdef CONFIG_LINUX
 #include <linux/vfio.h>
 #endif
+#include "sysemu/sysemu.h"
 
 #define VFIO_MSG_PREFIX "vfio %s: "
 
@@ -58,7 +59,10 @@ typedef struct VFIORegion {
 } VFIORegion;
 
 typedef struct VFIOMigration {
+    VMChangeStateEntry *vm_state;
     VFIORegion region;
+    uint32_t device_state;
+    int vm_running;
 } VFIOMigration;
 
 typedef struct VFIOAddressSpace {
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v29 06/17] vfio: Add migration state change notifier
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
                   ` (4 preceding siblings ...)
  2020-10-26  9:36 ` [PATCH v29 05/17] vfio: Add VM state change handler to know state of VM Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 07/17] vfio: Register SaveVMHandlers for VFIO device Kirti Wankhede
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

Added migration state change notifier to get notification on migration state
change. These states are translated to VFIO device state and conveyed to
vendor driver.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Neo Jia <cjia@nvidia.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/vfio/migration.c           | 30 ++++++++++++++++++++++++++++++
 hw/vfio/trace-events          |  1 +
 include/hw/vfio/vfio-common.h |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 65ce735d667b..888a615d39ea 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -175,6 +175,30 @@ static void vfio_vmstate_change(void *opaque, int running, RunState state)
             (migration->device_state & mask) | value);
 }
 
+static void vfio_migration_state_notifier(Notifier *notifier, void *data)
+{
+    MigrationState *s = data;
+    VFIOMigration *migration = container_of(notifier, VFIOMigration,
+                                            migration_state);
+    VFIODevice *vbasedev = migration->vbasedev;
+    int ret;
+
+    trace_vfio_migration_state_notifier(vbasedev->name,
+                                        MigrationStatus_str(s->state));
+
+    switch (s->state) {
+    case MIGRATION_STATUS_CANCELLING:
+    case MIGRATION_STATUS_CANCELLED:
+    case MIGRATION_STATUS_FAILED:
+        ret = vfio_migration_set_state(vbasedev,
+                      ~(VFIO_DEVICE_STATE_SAVING | VFIO_DEVICE_STATE_RESUMING),
+                      VFIO_DEVICE_STATE_RUNNING);
+        if (ret) {
+            error_report("%s: Failed to set state RUNNING", vbasedev->name);
+        }
+    }
+}
+
 static void vfio_migration_exit(VFIODevice *vbasedev)
 {
     VFIOMigration *migration = vbasedev->migration;
@@ -190,6 +214,7 @@ static int vfio_migration_init(VFIODevice *vbasedev,
 {
     int ret;
     Object *obj;
+    VFIOMigration *migration;
 
     if (!vbasedev->ops->vfio_get_object) {
         return -EINVAL;
@@ -217,8 +242,12 @@ static int vfio_migration_init(VFIODevice *vbasedev,
         goto err;
     }
 
+    migration = vbasedev->migration;
+    migration->vbasedev = vbasedev;
     migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change,
                                                            vbasedev);
+    migration->migration_state.notify = vfio_migration_state_notifier;
+    add_migration_state_change_notifier(&migration->migration_state);
     return 0;
 
 err:
@@ -268,6 +297,7 @@ void vfio_migration_finalize(VFIODevice *vbasedev)
     if (vbasedev->migration) {
         VFIOMigration *migration = vbasedev->migration;
 
+        remove_migration_state_change_notifier(&migration->migration_state);
         qemu_del_vm_change_state_handler(migration->vm_state);
         vfio_migration_exit(vbasedev);
     }
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index 41de81f12f60..78d7d83b5ef8 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -150,3 +150,4 @@ vfio_display_edid_write_error(void) ""
 vfio_migration_probe(const char *name, uint32_t index) " (%s) Region %d"
 vfio_migration_set_state(const char *name, uint32_t state) " (%s) state %d"
 vfio_vmstate_change(const char *name, int running, const char *reason, uint32_t dev_state) " (%s) running %d reason %s device state %d"
+vfio_migration_state_notifier(const char *name, const char *state) " (%s) state %s"
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 9a571f1fb552..2bd593ba38bb 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -59,10 +59,12 @@ typedef struct VFIORegion {
 } VFIORegion;
 
 typedef struct VFIOMigration {
+    struct VFIODevice *vbasedev;
     VMChangeStateEntry *vm_state;
     VFIORegion region;
     uint32_t device_state;
     int vm_running;
+    Notifier migration_state;
 } VFIOMigration;
 
 typedef struct VFIOAddressSpace {
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v29 07/17] vfio: Register SaveVMHandlers for VFIO device
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
                   ` (5 preceding siblings ...)
  2020-10-26  9:36 ` [PATCH v29 06/17] vfio: Add migration state change notifier Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 08/17] vfio: Add save state functions to SaveVMHandlers Kirti Wankhede
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

Define flags to be used as delimiter in migration stream for VFIO devices.
Added .save_setup and .save_cleanup functions. Map & unmap migration
region from these functions at source during saving or pre-copy phase.

Set VFIO device state depending on VM's state. During live migration, VM is
running when .save_setup is called, _SAVING | _RUNNING state is set for VFIO
device. During save-restore, VM is paused, _SAVING state is set for VFIO device.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Neo Jia <cjia@nvidia.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
---
 hw/vfio/migration.c  | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/vfio/trace-events |   2 +
 2 files changed, 104 insertions(+)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 888a615d39ea..d3ef9e18f39c 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -8,12 +8,15 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/main-loop.h"
+#include "qemu/cutils.h"
 #include <linux/vfio.h>
 
 #include "sysemu/runstate.h"
 #include "hw/vfio/vfio-common.h"
 #include "cpu.h"
 #include "migration/migration.h"
+#include "migration/vmstate.h"
 #include "migration/qemu-file.h"
 #include "migration/register.h"
 #include "migration/blocker.h"
@@ -25,6 +28,22 @@
 #include "trace.h"
 #include "hw/hw.h"
 
+/*
+ * Flags to be used as unique delimiters for VFIO devices in the migration
+ * stream. These flags are composed as:
+ * 0xffffffff => MSB 32-bit all 1s
+ * 0xef10     => Magic ID, represents emulated (virtual) function IO
+ * 0x0000     => 16-bits reserved for flags
+ *
+ * The beginning of state information is marked by _DEV_CONFIG_STATE,
+ * _DEV_SETUP_STATE, or _DEV_DATA_STATE, respectively. The end of a
+ * certain state information is marked by _END_OF_STATE.
+ */
+#define VFIO_MIG_FLAG_END_OF_STATE      (0xffffffffef100001ULL)
+#define VFIO_MIG_FLAG_DEV_CONFIG_STATE  (0xffffffffef100002ULL)
+#define VFIO_MIG_FLAG_DEV_SETUP_STATE   (0xffffffffef100003ULL)
+#define VFIO_MIG_FLAG_DEV_DATA_STATE    (0xffffffffef100004ULL)
+
 static inline int vfio_mig_access(VFIODevice *vbasedev, void *val, int count,
                                   off_t off, bool iswrite)
 {
@@ -129,6 +148,75 @@ static int vfio_migration_set_state(VFIODevice *vbasedev, uint32_t mask,
     return 0;
 }
 
+static void vfio_migration_cleanup(VFIODevice *vbasedev)
+{
+    VFIOMigration *migration = vbasedev->migration;
+
+    if (migration->region.mmaps) {
+        vfio_region_unmap(&migration->region);
+    }
+}
+
+/* ---------------------------------------------------------------------- */
+
+static int vfio_save_setup(QEMUFile *f, void *opaque)
+{
+    VFIODevice *vbasedev = opaque;
+    VFIOMigration *migration = vbasedev->migration;
+    int ret;
+
+    trace_vfio_save_setup(vbasedev->name);
+
+    qemu_put_be64(f, VFIO_MIG_FLAG_DEV_SETUP_STATE);
+
+    if (migration->region.mmaps) {
+        /*
+         * Calling vfio_region_mmap() from migration thread. Memory API called
+         * from this function require locking the iothread when called from
+         * outside the main loop thread.
+         */
+        qemu_mutex_lock_iothread();
+        ret = vfio_region_mmap(&migration->region);
+        qemu_mutex_unlock_iothread();
+        if (ret) {
+            error_report("%s: Failed to mmap VFIO migration region: %s",
+                         vbasedev->name, strerror(-ret));
+            error_report("%s: Falling back to slow path", vbasedev->name);
+        }
+    }
+
+    ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_MASK,
+                                   VFIO_DEVICE_STATE_SAVING);
+    if (ret) {
+        error_report("%s: Failed to set state SAVING", vbasedev->name);
+        return ret;
+    }
+
+    qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
+
+    ret = qemu_file_get_error(f);
+    if (ret) {
+        return ret;
+    }
+
+    return 0;
+}
+
+static void vfio_save_cleanup(void *opaque)
+{
+    VFIODevice *vbasedev = opaque;
+
+    vfio_migration_cleanup(vbasedev);
+    trace_vfio_save_cleanup(vbasedev->name);
+}
+
+static SaveVMHandlers savevm_vfio_handlers = {
+    .save_setup = vfio_save_setup,
+    .save_cleanup = vfio_save_cleanup,
+};
+
+/* ---------------------------------------------------------------------- */
+
 static void vfio_vmstate_change(void *opaque, int running, RunState state)
 {
     VFIODevice *vbasedev = opaque;
@@ -215,6 +303,8 @@ static int vfio_migration_init(VFIODevice *vbasedev,
     int ret;
     Object *obj;
     VFIOMigration *migration;
+    char id[256] = "";
+    g_autofree char *path = NULL, *oid = NULL;
 
     if (!vbasedev->ops->vfio_get_object) {
         return -EINVAL;
@@ -244,6 +334,18 @@ static int vfio_migration_init(VFIODevice *vbasedev,
 
     migration = vbasedev->migration;
     migration->vbasedev = vbasedev;
+
+    oid = vmstate_if_get_id(VMSTATE_IF(DEVICE(obj)));
+    if (oid) {
+        path = g_strdup_printf("%s/vfio", oid);
+    } else {
+        path = g_strdup("vfio");
+    }
+    strpadcpy(id, sizeof(id), path, '\0');
+
+    register_savevm_live(id, VMSTATE_INSTANCE_ID_ANY, 1, &savevm_vfio_handlers,
+                         vbasedev);
+
     migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change,
                                                            vbasedev);
     migration->migration_state.notify = vfio_migration_state_notifier;
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index 78d7d83b5ef8..f148b5e828c1 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -151,3 +151,5 @@ vfio_migration_probe(const char *name, uint32_t index) " (%s) Region %d"
 vfio_migration_set_state(const char *name, uint32_t state) " (%s) state %d"
 vfio_vmstate_change(const char *name, int running, const char *reason, uint32_t dev_state) " (%s) running %d reason %s device state %d"
 vfio_migration_state_notifier(const char *name, const char *state) " (%s) state %s"
+vfio_save_setup(const char *name) " (%s)"
+vfio_save_cleanup(const char *name) " (%s)"
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v29 08/17] vfio: Add save state functions to SaveVMHandlers
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
                   ` (6 preceding siblings ...)
  2020-10-26  9:36 ` [PATCH v29 07/17] vfio: Register SaveVMHandlers for VFIO device Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 09/17] vfio: Add load " Kirti Wankhede
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

Added .save_live_pending, .save_live_iterate and .save_live_complete_precopy
functions. These functions handles pre-copy and stop-and-copy phase.

In _SAVING|_RUNNING device state or pre-copy phase:
- read pending_bytes. If pending_bytes > 0, go through below steps.
- read data_offset - indicates kernel driver to write data to staging
  buffer.
- read data_size - amount of data in bytes written by vendor driver in
  migration region.
- read data_size bytes of data from data_offset in the migration region.
- Write data packet to file stream as below:
{VFIO_MIG_FLAG_DEV_DATA_STATE, data_size, actual data,
VFIO_MIG_FLAG_END_OF_STATE }

In _SAVING device state or stop-and-copy phase
a. read config space of device and save to migration file stream. This
   doesn't need to be from vendor driver. Any other special config state
   from driver can be saved as data in following iteration.
b. read pending_bytes. If pending_bytes > 0, go through below steps.
c. read data_offset - indicates kernel driver to write data to staging
   buffer.
d. read data_size - amount of data in bytes written by vendor driver in
   migration region.
e. read data_size bytes of data from data_offset in the migration region.
f. Write data packet as below:
   {VFIO_MIG_FLAG_DEV_DATA_STATE, data_size, actual data}
g. iterate through steps b to f while (pending_bytes > 0)
h. Write {VFIO_MIG_FLAG_END_OF_STATE}

When data region is mapped, its user's responsibility to read data from
data_offset of data_size before moving to next steps.

Added fix suggested by Artem Polyakov to reset pending_bytes in
vfio_save_iterate().
Added fix suggested by Zhi Wang to add 0 as data size in migration stream and
add END_OF_STATE delimiter to indicate phase complete.

Suggested-by: Artem Polyakov <artemp@nvidia.com>
Suggested-by: Zhi Wang <zhi.wang.linux@gmail.com>
Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Neo Jia <cjia@nvidia.com>
Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
---
 hw/vfio/migration.c           | 276 ++++++++++++++++++++++++++++++++++++++++++
 hw/vfio/trace-events          |   6 +
 include/hw/vfio/vfio-common.h |   1 +
 3 files changed, 283 insertions(+)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index d3ef9e18f39c..41d568558479 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -148,6 +148,151 @@ static int vfio_migration_set_state(VFIODevice *vbasedev, uint32_t mask,
     return 0;
 }
 
+static void *get_data_section_size(VFIORegion *region, uint64_t data_offset,
+                                   uint64_t data_size, uint64_t *size)
+{
+    void *ptr = NULL;
+    uint64_t limit = 0;
+    int i;
+
+    if (!region->mmaps) {
+        if (size) {
+            *size = MIN(data_size, region->size - data_offset);
+        }
+        return ptr;
+    }
+
+    for (i = 0; i < region->nr_mmaps; i++) {
+        VFIOMmap *map = region->mmaps + i;
+
+        if ((data_offset >= map->offset) &&
+            (data_offset < map->offset + map->size)) {
+
+            /* check if data_offset is within sparse mmap areas */
+            ptr = map->mmap + data_offset - map->offset;
+            if (size) {
+                *size = MIN(data_size, map->offset + map->size - data_offset);
+            }
+            break;
+        } else if ((data_offset < map->offset) &&
+                   (!limit || limit > map->offset)) {
+            /*
+             * data_offset is not within sparse mmap areas, find size of
+             * non-mapped area. Check through all list since region->mmaps list
+             * is not sorted.
+             */
+            limit = map->offset;
+        }
+    }
+
+    if (!ptr && size) {
+        *size = limit ? MIN(data_size, limit - data_offset) : data_size;
+    }
+    return ptr;
+}
+
+static int vfio_save_buffer(QEMUFile *f, VFIODevice *vbasedev, uint64_t *size)
+{
+    VFIOMigration *migration = vbasedev->migration;
+    VFIORegion *region = &migration->region;
+    uint64_t data_offset = 0, data_size = 0, sz;
+    int ret;
+
+    ret = vfio_mig_read(vbasedev, &data_offset, sizeof(data_offset),
+                      region->fd_offset + VFIO_MIG_STRUCT_OFFSET(data_offset));
+    if (ret < 0) {
+        return ret;
+    }
+
+    ret = vfio_mig_read(vbasedev, &data_size, sizeof(data_size),
+                        region->fd_offset + VFIO_MIG_STRUCT_OFFSET(data_size));
+    if (ret < 0) {
+        return ret;
+    }
+
+    trace_vfio_save_buffer(vbasedev->name, data_offset, data_size,
+                           migration->pending_bytes);
+
+    qemu_put_be64(f, data_size);
+    sz = data_size;
+
+    while (sz) {
+        void *buf;
+        uint64_t sec_size;
+        bool buf_allocated = false;
+
+        buf = get_data_section_size(region, data_offset, sz, &sec_size);
+
+        if (!buf) {
+            buf = g_try_malloc(sec_size);
+            if (!buf) {
+                error_report("%s: Error allocating buffer ", __func__);
+                return -ENOMEM;
+            }
+            buf_allocated = true;
+
+            ret = vfio_mig_read(vbasedev, buf, sec_size,
+                                region->fd_offset + data_offset);
+            if (ret < 0) {
+                g_free(buf);
+                return ret;
+            }
+        }
+
+        qemu_put_buffer(f, buf, sec_size);
+
+        if (buf_allocated) {
+            g_free(buf);
+        }
+        sz -= sec_size;
+        data_offset += sec_size;
+    }
+
+    ret = qemu_file_get_error(f);
+
+    if (!ret && size) {
+        *size = data_size;
+    }
+
+    return ret;
+}
+
+static int vfio_update_pending(VFIODevice *vbasedev)
+{
+    VFIOMigration *migration = vbasedev->migration;
+    VFIORegion *region = &migration->region;
+    uint64_t pending_bytes = 0;
+    int ret;
+
+    ret = vfio_mig_read(vbasedev, &pending_bytes, sizeof(pending_bytes),
+                    region->fd_offset + VFIO_MIG_STRUCT_OFFSET(pending_bytes));
+    if (ret < 0) {
+        migration->pending_bytes = 0;
+        return ret;
+    }
+
+    migration->pending_bytes = pending_bytes;
+    trace_vfio_update_pending(vbasedev->name, pending_bytes);
+    return 0;
+}
+
+static int vfio_save_device_config_state(QEMUFile *f, void *opaque)
+{
+    VFIODevice *vbasedev = opaque;
+
+    qemu_put_be64(f, VFIO_MIG_FLAG_DEV_CONFIG_STATE);
+
+    if (vbasedev->ops && vbasedev->ops->vfio_save_config) {
+        vbasedev->ops->vfio_save_config(vbasedev, f);
+    }
+
+    qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
+
+    trace_vfio_save_device_config_state(vbasedev->name);
+
+    return qemu_file_get_error(f);
+}
+
 static void vfio_migration_cleanup(VFIODevice *vbasedev)
 {
     VFIOMigration *migration = vbasedev->migration;
@@ -210,9 +355,140 @@ static void vfio_save_cleanup(void *opaque)
     trace_vfio_save_cleanup(vbasedev->name);
 }
 
+static void vfio_save_pending(QEMUFile *f, void *opaque,
+                              uint64_t threshold_size,
+                              uint64_t *res_precopy_only,
+                              uint64_t *res_compatible,
+                              uint64_t *res_postcopy_only)
+{
+    VFIODevice *vbasedev = opaque;
+    VFIOMigration *migration = vbasedev->migration;
+    int ret;
+
+    ret = vfio_update_pending(vbasedev);
+    if (ret) {
+        return;
+    }
+
+    *res_precopy_only += migration->pending_bytes;
+
+    trace_vfio_save_pending(vbasedev->name, *res_precopy_only,
+                            *res_postcopy_only, *res_compatible);
+}
+
+static int vfio_save_iterate(QEMUFile *f, void *opaque)
+{
+    VFIODevice *vbasedev = opaque;
+    VFIOMigration *migration = vbasedev->migration;
+    uint64_t data_size;
+    int ret;
+
+    qemu_put_be64(f, VFIO_MIG_FLAG_DEV_DATA_STATE);
+
+    if (migration->pending_bytes == 0) {
+        ret = vfio_update_pending(vbasedev);
+        if (ret) {
+            return ret;
+        }
+
+        if (migration->pending_bytes == 0) {
+            qemu_put_be64(f, 0);
+            qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
+            /* indicates data finished, goto complete phase */
+            return 1;
+        }
+    }
+
+    ret = vfio_save_buffer(f, vbasedev, &data_size);
+    if (ret) {
+        error_report("%s: vfio_save_buffer failed %s", vbasedev->name,
+                     strerror(errno));
+        return ret;
+    }
+
+    qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
+
+    ret = qemu_file_get_error(f);
+    if (ret) {
+        return ret;
+    }
+
+    /*
+     * Reset pending_bytes as .save_live_pending is not called during savevm or
+     * snapshot case, in such case vfio_update_pending() at the start of this
+     * function updates pending_bytes.
+     */
+    migration->pending_bytes = 0;
+    trace_vfio_save_iterate(vbasedev->name, data_size);
+    return 0;
+}
+
+static int vfio_save_complete_precopy(QEMUFile *f, void *opaque)
+{
+    VFIODevice *vbasedev = opaque;
+    VFIOMigration *migration = vbasedev->migration;
+    uint64_t data_size;
+    int ret;
+
+    ret = vfio_migration_set_state(vbasedev, ~VFIO_DEVICE_STATE_RUNNING,
+                                   VFIO_DEVICE_STATE_SAVING);
+    if (ret) {
+        error_report("%s: Failed to set state STOP and SAVING",
+                     vbasedev->name);
+        return ret;
+    }
+
+    ret = vfio_save_device_config_state(f, opaque);
+    if (ret) {
+        return ret;
+    }
+
+    ret = vfio_update_pending(vbasedev);
+    if (ret) {
+        return ret;
+    }
+
+    while (migration->pending_bytes > 0) {
+        qemu_put_be64(f, VFIO_MIG_FLAG_DEV_DATA_STATE);
+        ret = vfio_save_buffer(f, vbasedev, &data_size);
+        if (ret < 0) {
+            error_report("%s: Failed to save buffer", vbasedev->name);
+            return ret;
+        }
+
+        if (data_size == 0) {
+            break;
+        }
+
+        ret = vfio_update_pending(vbasedev);
+        if (ret) {
+            return ret;
+        }
+    }
+
+    qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
+
+    ret = qemu_file_get_error(f);
+    if (ret) {
+        return ret;
+    }
+
+    ret = vfio_migration_set_state(vbasedev, ~VFIO_DEVICE_STATE_SAVING, 0);
+    if (ret) {
+        error_report("%s: Failed to set state STOPPED", vbasedev->name);
+        return ret;
+    }
+
+    trace_vfio_save_complete_precopy(vbasedev->name);
+    return ret;
+}
+
 static SaveVMHandlers savevm_vfio_handlers = {
     .save_setup = vfio_save_setup,
     .save_cleanup = vfio_save_cleanup,
+    .save_live_pending = vfio_save_pending,
+    .save_live_iterate = vfio_save_iterate,
+    .save_live_complete_precopy = vfio_save_complete_precopy,
 };
 
 /* ---------------------------------------------------------------------- */
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index f148b5e828c1..9f5712dab1ea 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -153,3 +153,9 @@ vfio_vmstate_change(const char *name, int running, const char *reason, uint32_t
 vfio_migration_state_notifier(const char *name, const char *state) " (%s) state %s"
 vfio_save_setup(const char *name) " (%s)"
 vfio_save_cleanup(const char *name) " (%s)"
+vfio_save_buffer(const char *name, uint64_t data_offset, uint64_t data_size, uint64_t pending) " (%s) Offset 0x%"PRIx64" size 0x%"PRIx64" pending 0x%"PRIx64
+vfio_update_pending(const char *name, uint64_t pending) " (%s) pending 0x%"PRIx64
+vfio_save_device_config_state(const char *name) " (%s)"
+vfio_save_pending(const char *name, uint64_t precopy, uint64_t postcopy, uint64_t compatible) " (%s) precopy 0x%"PRIx64" postcopy 0x%"PRIx64" compatible 0x%"PRIx64
+vfio_save_iterate(const char *name, int data_size) " (%s) data_size %d"
+vfio_save_complete_precopy(const char *name) " (%s)"
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 2bd593ba38bb..f4ebdae013ad 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -65,6 +65,7 @@ typedef struct VFIOMigration {
     uint32_t device_state;
     int vm_running;
     Notifier migration_state;
+    uint64_t pending_bytes;
 } VFIOMigration;
 
 typedef struct VFIOAddressSpace {
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v29 09/17] vfio: Add load state functions to SaveVMHandlers
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
                   ` (7 preceding siblings ...)
  2020-10-26  9:36 ` [PATCH v29 08/17] vfio: Add save state functions to SaveVMHandlers Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 10/17] memory: Set DIRTY_MEMORY_MIGRATION when IOMMU is enabled Kirti Wankhede
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

Sequence  during _RESUMING device state:
While data for this device is available, repeat below steps:
a. read data_offset from where user application should write data.
b. write data of data_size to migration region from data_offset.
c. write data_size which indicates vendor driver that data is written in
   staging buffer.

For user, data is opaque. User should write data in the same order as
received.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Neo Jia <cjia@nvidia.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
---
 hw/vfio/migration.c  | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/vfio/trace-events |   4 ++
 2 files changed, 199 insertions(+)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 41d568558479..6ac72b46a88b 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -257,6 +257,77 @@ static int vfio_save_buffer(QEMUFile *f, VFIODevice *vbasedev, uint64_t *size)
     return ret;
 }
 
+static int vfio_load_buffer(QEMUFile *f, VFIODevice *vbasedev,
+                            uint64_t data_size)
+{
+    VFIORegion *region = &vbasedev->migration->region;
+    uint64_t data_offset = 0, size, report_size;
+    int ret;
+
+    do {
+        ret = vfio_mig_read(vbasedev, &data_offset, sizeof(data_offset),
+                      region->fd_offset + VFIO_MIG_STRUCT_OFFSET(data_offset));
+        if (ret < 0) {
+            return ret;
+        }
+
+        if (data_offset + data_size > region->size) {
+            /*
+             * If data_size is greater than the data section of migration region
+             * then iterate the write buffer operation. This case can occur if
+             * size of migration region at destination is smaller than size of
+             * migration region at source.
+             */
+            report_size = size = region->size - data_offset;
+            data_size -= size;
+        } else {
+            report_size = size = data_size;
+            data_size = 0;
+        }
+
+        trace_vfio_load_state_device_data(vbasedev->name, data_offset, size);
+
+        while (size) {
+            void *buf;
+            uint64_t sec_size;
+            bool buf_alloc = false;
+
+            buf = get_data_section_size(region, data_offset, size, &sec_size);
+
+            if (!buf) {
+                buf = g_try_malloc(sec_size);
+                if (!buf) {
+                    error_report("%s: Error allocating buffer ", __func__);
+                    return -ENOMEM;
+                }
+                buf_alloc = true;
+            }
+
+            qemu_get_buffer(f, buf, sec_size);
+
+            if (buf_alloc) {
+                ret = vfio_mig_write(vbasedev, buf, sec_size,
+                        region->fd_offset + data_offset);
+                g_free(buf);
+
+                if (ret < 0) {
+                    return ret;
+                }
+            }
+            size -= sec_size;
+            data_offset += sec_size;
+        }
+
+        ret = vfio_mig_write(vbasedev, &report_size, sizeof(report_size),
+                        region->fd_offset + VFIO_MIG_STRUCT_OFFSET(data_size));
+        if (ret < 0) {
+            return ret;
+        }
+    } while (data_size);
+
+    return 0;
+}
+
 static int vfio_update_pending(VFIODevice *vbasedev)
 {
     VFIOMigration *migration = vbasedev->migration;
@@ -293,6 +364,33 @@ static int vfio_save_device_config_state(QEMUFile *f, void *opaque)
     return qemu_file_get_error(f);
 }
 
+static int vfio_load_device_config_state(QEMUFile *f, void *opaque)
+{
+    VFIODevice *vbasedev = opaque;
+    uint64_t data;
+
+    if (vbasedev->ops && vbasedev->ops->vfio_load_config) {
+        int ret;
+
+        ret = vbasedev->ops->vfio_load_config(vbasedev, f);
+        if (ret) {
+            error_report("%s: Failed to load device config space",
+                         vbasedev->name);
+            return ret;
+        }
+    }
+
+    data = qemu_get_be64(f);
+    if (data != VFIO_MIG_FLAG_END_OF_STATE) {
+        error_report("%s: Failed loading device config space, "
+                     "end flag incorrect 0x%"PRIx64, vbasedev->name, data);
+        return -EINVAL;
+    }
+
+    trace_vfio_load_device_config_state(vbasedev->name);
+    return qemu_file_get_error(f);
+}
+
 static void vfio_migration_cleanup(VFIODevice *vbasedev)
 {
     VFIOMigration *migration = vbasedev->migration;
@@ -483,12 +581,109 @@ static int vfio_save_complete_precopy(QEMUFile *f, void *opaque)
     return ret;
 }
 
+static int vfio_load_setup(QEMUFile *f, void *opaque)
+{
+    VFIODevice *vbasedev = opaque;
+    VFIOMigration *migration = vbasedev->migration;
+    int ret = 0;
+
+    if (migration->region.mmaps) {
+        ret = vfio_region_mmap(&migration->region);
+        if (ret) {
+            error_report("%s: Failed to mmap VFIO migration region %d: %s",
+                         vbasedev->name, migration->region.nr,
+                         strerror(-ret));
+            error_report("%s: Falling back to slow path", vbasedev->name);
+        }
+    }
+
+    ret = vfio_migration_set_state(vbasedev, ~VFIO_DEVICE_STATE_MASK,
+                                   VFIO_DEVICE_STATE_RESUMING);
+    if (ret) {
+        error_report("%s: Failed to set state RESUMING", vbasedev->name);
+        if (migration->region.mmaps) {
+            vfio_region_unmap(&migration->region);
+        }
+    }
+    return ret;
+}
+
+static int vfio_load_cleanup(void *opaque)
+{
+    VFIODevice *vbasedev = opaque;
+
+    vfio_migration_cleanup(vbasedev);
+    trace_vfio_load_cleanup(vbasedev->name);
+    return 0;
+}
+
+static int vfio_load_state(QEMUFile *f, void *opaque, int version_id)
+{
+    VFIODevice *vbasedev = opaque;
+    int ret = 0;
+    uint64_t data;
+
+    data = qemu_get_be64(f);
+    while (data != VFIO_MIG_FLAG_END_OF_STATE) {
+
+        trace_vfio_load_state(vbasedev->name, data);
+
+        switch (data) {
+        case VFIO_MIG_FLAG_DEV_CONFIG_STATE:
+        {
+            ret = vfio_load_device_config_state(f, opaque);
+            if (ret) {
+                return ret;
+            }
+            break;
+        }
+        case VFIO_MIG_FLAG_DEV_SETUP_STATE:
+        {
+            data = qemu_get_be64(f);
+            if (data == VFIO_MIG_FLAG_END_OF_STATE) {
+                return ret;
+            } else {
+                error_report("%s: SETUP STATE: EOS not found 0x%"PRIx64,
+                             vbasedev->name, data);
+                return -EINVAL;
+            }
+            break;
+        }
+        case VFIO_MIG_FLAG_DEV_DATA_STATE:
+        {
+            uint64_t data_size = qemu_get_be64(f);
+
+            if (data_size) {
+                ret = vfio_load_buffer(f, vbasedev, data_size);
+                if (ret < 0) {
+                    return ret;
+                }
+            }
+            break;
+        }
+        default:
+            error_report("%s: Unknown tag 0x%"PRIx64, vbasedev->name, data);
+            return -EINVAL;
+        }
+
+        data = qemu_get_be64(f);
+        ret = qemu_file_get_error(f);
+        if (ret) {
+            return ret;
+        }
+    }
+    return ret;
+}
+
 static SaveVMHandlers savevm_vfio_handlers = {
     .save_setup = vfio_save_setup,
     .save_cleanup = vfio_save_cleanup,
     .save_live_pending = vfio_save_pending,
     .save_live_iterate = vfio_save_iterate,
     .save_live_complete_precopy = vfio_save_complete_precopy,
+    .load_setup = vfio_load_setup,
+    .load_cleanup = vfio_load_cleanup,
+    .load_state = vfio_load_state,
 };
 
 /* ---------------------------------------------------------------------- */
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index 9f5712dab1ea..a75b5208818c 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -159,3 +159,7 @@ vfio_save_device_config_state(const char *name) " (%s)"
 vfio_save_pending(const char *name, uint64_t precopy, uint64_t postcopy, uint64_t compatible) " (%s) precopy 0x%"PRIx64" postcopy 0x%"PRIx64" compatible 0x%"PRIx64
 vfio_save_iterate(const char *name, int data_size) " (%s) data_size %d"
 vfio_save_complete_precopy(const char *name) " (%s)"
+vfio_load_device_config_state(const char *name) " (%s)"
+vfio_load_state(const char *name, uint64_t data) " (%s) data 0x%"PRIx64
+vfio_load_state_device_data(const char *name, uint64_t data_offset, uint64_t data_size) " (%s) Offset 0x%"PRIx64" size 0x%"PRIx64
+vfio_load_cleanup(const char *name) " (%s)"
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v29 10/17] memory: Set DIRTY_MEMORY_MIGRATION when IOMMU is enabled
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
                   ` (8 preceding siblings ...)
  2020-10-26  9:36 ` [PATCH v29 09/17] vfio: Add load " Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2020-10-26 16:47   ` Alex Williamson
  2020-10-26  9:36 ` [PATCH v29 11/17] vfio: Get migration capability flags for container Kirti Wankhede
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

mr->ram_block is NULL when mr->is_iommu is true, then fr.dirty_log_mask
wasn't set correctly due to which memory listener's log_sync doesn't
get called.
This patch returns log_mask with DIRTY_MEMORY_MIGRATION set when
IOMMU is enabled.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
---
 softmmu/memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/softmmu/memory.c b/softmmu/memory.c
index 403ff3abc99b..94f606e9d9d9 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -1792,7 +1792,7 @@ bool memory_region_is_ram_device(MemoryRegion *mr)
 uint8_t memory_region_get_dirty_log_mask(MemoryRegion *mr)
 {
     uint8_t mask = mr->dirty_log_mask;
-    if (global_dirty_log && mr->ram_block) {
+    if (global_dirty_log && (mr->ram_block || memory_region_is_iommu(mr))) {
         mask |= (1 << DIRTY_MEMORY_MIGRATION);
     }
     return mask;
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v29 11/17] vfio: Get migration capability flags for container
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
                   ` (9 preceding siblings ...)
  2020-10-26  9:36 ` [PATCH v29 10/17] memory: Set DIRTY_MEMORY_MIGRATION when IOMMU is enabled Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 12/17] vfio: Add function to start and stop dirty pages tracking Kirti Wankhede
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, Eric Auger,
	changpeng.liu, eskultet, Shameer Kolothum, Ken.Xue,
	jonathan.davies, pbonzini, dnigam

Added helper functions to get IOMMU info capability chain.
Added function to get migration capability information from that
capability chain for IOMMU container.

Similar change was proposed earlier:
https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg03759.html

Disable migration for devices if IOMMU module doesn't support migration
capability.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Cc: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Cc: Eric Auger <eric.auger@redhat.com>
---
 hw/vfio/common.c              | 90 +++++++++++++++++++++++++++++++++++++++----
 hw/vfio/migration.c           |  7 +++-
 include/hw/vfio/vfio-common.h |  3 ++
 3 files changed, 91 insertions(+), 9 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index c6e98b8d61be..d4959c036dd1 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1228,6 +1228,75 @@ static int vfio_init_container(VFIOContainer *container, int group_fd,
     return 0;
 }
 
+static int vfio_get_iommu_info(VFIOContainer *container,
+                               struct vfio_iommu_type1_info **info)
+{
+
+    size_t argsz = sizeof(struct vfio_iommu_type1_info);
+
+    *info = g_new0(struct vfio_iommu_type1_info, 1);
+again:
+    (*info)->argsz = argsz;
+
+    if (ioctl(container->fd, VFIO_IOMMU_GET_INFO, *info)) {
+        g_free(*info);
+        *info = NULL;
+        return -errno;
+    }
+
+    if (((*info)->argsz > argsz)) {
+        argsz = (*info)->argsz;
+        *info = g_realloc(*info, argsz);
+        goto again;
+    }
+
+    return 0;
+}
+
+static struct vfio_info_cap_header *
+vfio_get_iommu_info_cap(struct vfio_iommu_type1_info *info, uint16_t id)
+{
+    struct vfio_info_cap_header *hdr;
+    void *ptr = info;
+
+    if (!(info->flags & VFIO_IOMMU_INFO_CAPS)) {
+        return NULL;
+    }
+
+    for (hdr = ptr + info->cap_offset; hdr != ptr; hdr = ptr + hdr->next) {
+        if (hdr->id == id) {
+            return hdr;
+        }
+    }
+
+    return NULL;
+}
+
+static void vfio_get_iommu_info_migration(VFIOContainer *container,
+                                         struct vfio_iommu_type1_info *info)
+{
+    struct vfio_info_cap_header *hdr;
+    struct vfio_iommu_type1_info_cap_migration *cap_mig;
+
+    hdr = vfio_get_iommu_info_cap(info, VFIO_IOMMU_TYPE1_INFO_CAP_MIGRATION);
+    if (!hdr) {
+        return;
+    }
+
+    cap_mig = container_of(hdr, struct vfio_iommu_type1_info_cap_migration,
+                            header);
+
+    /*
+     * cpu_physical_memory_set_dirty_lebitmap() expects pages in bitmap of
+     * TARGET_PAGE_SIZE to mark those dirty.
+     */
+    if (cap_mig->pgsize_bitmap & TARGET_PAGE_SIZE) {
+        container->dirty_pages_supported = true;
+        container->max_dirty_bitmap_size = cap_mig->max_dirty_bitmap_size;
+        container->dirty_pgsizes = cap_mig->pgsize_bitmap;
+    }
+}
+
 static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
                                   Error **errp)
 {
@@ -1297,6 +1366,7 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
     container->space = space;
     container->fd = fd;
     container->error = NULL;
+    container->dirty_pages_supported = false;
     QLIST_INIT(&container->giommu_list);
     QLIST_INIT(&container->hostwin_list);
 
@@ -1309,7 +1379,7 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
     case VFIO_TYPE1v2_IOMMU:
     case VFIO_TYPE1_IOMMU:
     {
-        struct vfio_iommu_type1_info info;
+        struct vfio_iommu_type1_info *info;
 
         /*
          * FIXME: This assumes that a Type1 IOMMU can map any 64-bit
@@ -1318,15 +1388,19 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
          * existing Type1 IOMMUs generally support any IOVA we're
          * going to actually try in practice.
          */
-        info.argsz = sizeof(info);
-        ret = ioctl(fd, VFIO_IOMMU_GET_INFO, &info);
-        /* Ignore errors */
-        if (ret || !(info.flags & VFIO_IOMMU_INFO_PGSIZES)) {
+        ret = vfio_get_iommu_info(container, &info);
+
+        if (ret || !(info->flags & VFIO_IOMMU_INFO_PGSIZES)) {
             /* Assume 4k IOVA page size */
-            info.iova_pgsizes = 4096;
+            info->iova_pgsizes = 4096;
         }
-        vfio_host_win_add(container, 0, (hwaddr)-1, info.iova_pgsizes);
-        container->pgsizes = info.iova_pgsizes;
+        vfio_host_win_add(container, 0, (hwaddr)-1, info->iova_pgsizes);
+        container->pgsizes = info->iova_pgsizes;
+
+        if (!ret) {
+            vfio_get_iommu_info_migration(container, info);
+        }
+        g_free(info);
         break;
     }
     case VFIO_SPAPR_TCE_v2_IOMMU:
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 6ac72b46a88b..93f8fe7bd869 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -832,9 +832,14 @@ err:
 
 int vfio_migration_probe(VFIODevice *vbasedev, Error **errp)
 {
+    VFIOContainer *container = vbasedev->group->container;
     struct vfio_region_info *info = NULL;
     Error *local_err = NULL;
-    int ret;
+    int ret = -ENOTSUP;
+
+    if (!container->dirty_pages_supported) {
+        goto add_blocker;
+    }
 
     ret = vfio_get_dev_region_info(vbasedev, VFIO_REGION_TYPE_MIGRATION,
                                    VFIO_REGION_SUBTYPE_MIGRATION, &info);
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index f4ebdae013ad..b1c1b18fd228 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -84,6 +84,9 @@ typedef struct VFIOContainer {
     unsigned iommu_type;
     Error *error;
     bool initialized;
+    bool dirty_pages_supported;
+    uint64_t dirty_pgsizes;
+    uint64_t max_dirty_bitmap_size;
     unsigned long pgsizes;
     QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
     QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v29 12/17] vfio: Add function to start and stop dirty pages tracking
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
                   ` (10 preceding siblings ...)
  2020-10-26  9:36 ` [PATCH v29 11/17] vfio: Get migration capability flags for container Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 13/17] vfio: Add vfio_listener_log_sync to mark dirty pages Kirti Wankhede
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

Call VFIO_IOMMU_DIRTY_PAGES ioctl to start and stop dirty pages tracking
for VFIO devices.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/vfio/migration.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 93f8fe7bd869..ffedbcca179d 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -11,6 +11,7 @@
 #include "qemu/main-loop.h"
 #include "qemu/cutils.h"
 #include <linux/vfio.h>
+#include <sys/ioctl.h>
 
 #include "sysemu/runstate.h"
 #include "hw/vfio/vfio-common.h"
@@ -391,10 +392,40 @@ static int vfio_load_device_config_state(QEMUFile *f, void *opaque)
     return qemu_file_get_error(f);
 }
 
+static int vfio_set_dirty_page_tracking(VFIODevice *vbasedev, bool start)
+{
+    int ret;
+    VFIOMigration *migration = vbasedev->migration;
+    VFIOContainer *container = vbasedev->group->container;
+    struct vfio_iommu_type1_dirty_bitmap dirty = {
+        .argsz = sizeof(dirty),
+    };
+
+    if (start) {
+        if (migration->device_state & VFIO_DEVICE_STATE_SAVING) {
+            dirty.flags = VFIO_IOMMU_DIRTY_PAGES_FLAG_START;
+        } else {
+            return -EINVAL;
+        }
+    } else {
+            dirty.flags = VFIO_IOMMU_DIRTY_PAGES_FLAG_STOP;
+    }
+
+    ret = ioctl(container->fd, VFIO_IOMMU_DIRTY_PAGES, &dirty);
+    if (ret) {
+        error_report("Failed to set dirty tracking flag 0x%x errno: %d",
+                     dirty.flags, errno);
+        return -errno;
+    }
+    return ret;
+}
+
 static void vfio_migration_cleanup(VFIODevice *vbasedev)
 {
     VFIOMigration *migration = vbasedev->migration;
 
+    vfio_set_dirty_page_tracking(vbasedev, false);
+
     if (migration->region.mmaps) {
         vfio_region_unmap(&migration->region);
     }
@@ -435,6 +466,11 @@ static int vfio_save_setup(QEMUFile *f, void *opaque)
         return ret;
     }
 
+    ret = vfio_set_dirty_page_tracking(vbasedev, true);
+    if (ret) {
+        return ret;
+    }
+
     qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
 
     ret = qemu_file_get_error(f);
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v29 13/17] vfio: Add vfio_listener_log_sync to mark dirty pages
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
                   ` (11 preceding siblings ...)
  2020-10-26  9:36 ` [PATCH v29 12/17] vfio: Add function to start and stop dirty pages tracking Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 14/17] vfio: Dirty page tracking when vIOMMU is enabled Kirti Wankhede
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

vfio_listener_log_sync gets list of dirty pages from container using
VFIO_IOMMU_GET_DIRTY_BITMAP ioctl and mark those pages dirty when all
devices are stopped and saving state.
Return early for the RAM block section of mapped MMIO region.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Neo Jia <cjia@nvidia.com>
---
 hw/vfio/common.c     | 116 +++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/vfio/trace-events |   1 +
 2 files changed, 117 insertions(+)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index d4959c036dd1..2634387df948 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -29,6 +29,7 @@
 #include "hw/vfio/vfio.h"
 #include "exec/address-spaces.h"
 #include "exec/memory.h"
+#include "exec/ram_addr.h"
 #include "hw/hw.h"
 #include "qemu/error-report.h"
 #include "qemu/main-loop.h"
@@ -37,6 +38,7 @@
 #include "sysemu/reset.h"
 #include "trace.h"
 #include "qapi/error.h"
+#include "migration/migration.h"
 
 VFIOGroupList vfio_group_list =
     QLIST_HEAD_INITIALIZER(vfio_group_list);
@@ -287,6 +289,39 @@ const MemoryRegionOps vfio_region_ops = {
 };
 
 /*
+ * Device state interfaces
+ */
+
+static bool vfio_devices_all_stopped_and_saving(VFIOContainer *container)
+{
+    VFIOGroup *group;
+    VFIODevice *vbasedev;
+    MigrationState *ms = migrate_get_current();
+
+    if (!migration_is_setup_or_active(ms->state)) {
+        return false;
+    }
+
+    QLIST_FOREACH(group, &container->group_list, container_next) {
+        QLIST_FOREACH(vbasedev, &group->device_list, next) {
+            VFIOMigration *migration = vbasedev->migration;
+
+            if (!migration) {
+                return false;
+            }
+
+            if ((migration->device_state & VFIO_DEVICE_STATE_SAVING) &&
+                !(migration->device_state & VFIO_DEVICE_STATE_RUNNING)) {
+                continue;
+            } else {
+                return false;
+            }
+        }
+    }
+    return true;
+}
+
+/*
  * DMA - Mapping and unmapping for the "type1" IOMMU interface used on x86
  */
 static int vfio_dma_unmap(VFIOContainer *container,
@@ -812,9 +847,90 @@ static void vfio_listener_region_del(MemoryListener *listener,
     }
 }
 
+static int vfio_get_dirty_bitmap(VFIOContainer *container, uint64_t iova,
+                                 uint64_t size, ram_addr_t ram_addr)
+{
+    struct vfio_iommu_type1_dirty_bitmap *dbitmap;
+    struct vfio_iommu_type1_dirty_bitmap_get *range;
+    uint64_t pages;
+    int ret;
+
+    dbitmap = g_malloc0(sizeof(*dbitmap) + sizeof(*range));
+
+    dbitmap->argsz = sizeof(*dbitmap) + sizeof(*range);
+    dbitmap->flags = VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP;
+    range = (struct vfio_iommu_type1_dirty_bitmap_get *)&dbitmap->data;
+    range->iova = iova;
+    range->size = size;
+
+    /*
+     * cpu_physical_memory_set_dirty_lebitmap() expects pages in bitmap of
+     * TARGET_PAGE_SIZE to mark those dirty. Hence set bitmap's pgsize to
+     * TARGET_PAGE_SIZE.
+     */
+    range->bitmap.pgsize = TARGET_PAGE_SIZE;
+
+    pages = TARGET_PAGE_ALIGN(range->size) >> TARGET_PAGE_BITS;
+    range->bitmap.size = ROUND_UP(pages, sizeof(__u64) * BITS_PER_BYTE) /
+                                         BITS_PER_BYTE;
+    range->bitmap.data = g_try_malloc0(range->bitmap.size);
+    if (!range->bitmap.data) {
+        ret = -ENOMEM;
+        goto err_out;
+    }
+
+    ret = ioctl(container->fd, VFIO_IOMMU_DIRTY_PAGES, dbitmap);
+    if (ret) {
+        error_report("Failed to get dirty bitmap for iova: 0x%llx "
+                "size: 0x%llx err: %d",
+                range->iova, range->size, errno);
+        goto err_out;
+    }
+
+    cpu_physical_memory_set_dirty_lebitmap((uint64_t *)range->bitmap.data,
+                                            ram_addr, pages);
+
+    trace_vfio_get_dirty_bitmap(container->fd, range->iova, range->size,
+                                range->bitmap.size, ram_addr);
+err_out:
+    g_free(range->bitmap.data);
+    g_free(dbitmap);
+
+    return ret;
+}
+
+static int vfio_sync_dirty_bitmap(VFIOContainer *container,
+                                  MemoryRegionSection *section)
+{
+    ram_addr_t ram_addr;
+
+    ram_addr = memory_region_get_ram_addr(section->mr) +
+               section->offset_within_region;
+
+    return vfio_get_dirty_bitmap(container,
+                       TARGET_PAGE_ALIGN(section->offset_within_address_space),
+                       int128_get64(section->size), ram_addr);
+}
+
+static void vfio_listerner_log_sync(MemoryListener *listener,
+        MemoryRegionSection *section)
+{
+    VFIOContainer *container = container_of(listener, VFIOContainer, listener);
+
+    if (vfio_listener_skipped_section(section) ||
+        !container->dirty_pages_supported) {
+        return;
+    }
+
+    if (vfio_devices_all_stopped_and_saving(container)) {
+        vfio_sync_dirty_bitmap(container, section);
+    }
+}
+
 static const MemoryListener vfio_memory_listener = {
     .region_add = vfio_listener_region_add,
     .region_del = vfio_listener_region_del,
+    .log_sync = vfio_listerner_log_sync,
 };
 
 static void vfio_listener_release(VFIOContainer *container)
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index a75b5208818c..dd991bd8f265 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -163,3 +163,4 @@ vfio_load_device_config_state(const char *name) " (%s)"
 vfio_load_state(const char *name, uint64_t data) " (%s) data 0x%"PRIx64
 vfio_load_state_device_data(const char *name, uint64_t data_offset, uint64_t data_size) " (%s) Offset 0x%"PRIx64" size 0x%"PRIx64
 vfio_load_cleanup(const char *name) " (%s)"
+vfio_get_dirty_bitmap(int fd, uint64_t iova, uint64_t size, uint64_t bitmap_size, uint64_t start) "container fd=%d, iova=0x%"PRIx64" size= 0x%"PRIx64" bitmap_size=0x%"PRIx64" start=0x%"PRIx64
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v29 14/17] vfio: Dirty page tracking when vIOMMU is enabled
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
                   ` (12 preceding siblings ...)
  2020-10-26  9:36 ` [PATCH v29 13/17] vfio: Add vfio_listener_log_sync to mark dirty pages Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 15/17] vfio: Add ioctl to get dirty pages bitmap during dma unmap Kirti Wankhede
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

When vIOMMU is enabled, register MAP notifier from log_sync when all
devices in container are in stop and copy phase of migration. Call replay
and get dirty pages from notifier callback.

Suggested-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
---
 hw/vfio/common.c     | 88 ++++++++++++++++++++++++++++++++++++++++++++++++----
 hw/vfio/trace-events |  1 +
 2 files changed, 83 insertions(+), 6 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 2634387df948..c0b5b6245a47 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -442,8 +442,8 @@ static bool vfio_listener_skipped_section(MemoryRegionSection *section)
 }
 
 /* Called with rcu_read_lock held.  */
-static bool vfio_get_vaddr(IOMMUTLBEntry *iotlb, void **vaddr,
-                           bool *read_only)
+static bool vfio_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
+                               ram_addr_t *ram_addr, bool *read_only)
 {
     MemoryRegion *mr;
     hwaddr xlat;
@@ -474,8 +474,17 @@ static bool vfio_get_vaddr(IOMMUTLBEntry *iotlb, void **vaddr,
         return false;
     }
 
-    *vaddr = memory_region_get_ram_ptr(mr) + xlat;
-    *read_only = !writable || mr->readonly;
+    if (vaddr) {
+        *vaddr = memory_region_get_ram_ptr(mr) + xlat;
+    }
+
+    if (ram_addr) {
+        *ram_addr = memory_region_get_ram_addr(mr) + xlat;
+    }
+
+    if (read_only) {
+        *read_only = !writable || mr->readonly;
+    }
 
     return true;
 }
@@ -485,7 +494,6 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
     VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
     VFIOContainer *container = giommu->container;
     hwaddr iova = iotlb->iova + giommu->iommu_offset;
-    bool read_only;
     void *vaddr;
     int ret;
 
@@ -501,7 +509,9 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
     rcu_read_lock();
 
     if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
-        if (!vfio_get_vaddr(iotlb, &vaddr, &read_only)) {
+        bool read_only;
+
+        if (!vfio_get_xlat_addr(iotlb, &vaddr, NULL, &read_only)) {
             goto out;
         }
         /*
@@ -899,11 +909,77 @@ err_out:
     return ret;
 }
 
+typedef struct {
+    IOMMUNotifier n;
+    VFIOGuestIOMMU *giommu;
+} vfio_giommu_dirty_notifier;
+
+static void vfio_iommu_map_dirty_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
+{
+    vfio_giommu_dirty_notifier *gdn = container_of(n,
+                                                vfio_giommu_dirty_notifier, n);
+    VFIOGuestIOMMU *giommu = gdn->giommu;
+    VFIOContainer *container = giommu->container;
+    hwaddr iova = iotlb->iova + giommu->iommu_offset;
+    ram_addr_t translated_addr;
+
+    trace_vfio_iommu_map_dirty_notify(iova, iova + iotlb->addr_mask);
+
+    if (iotlb->target_as != &address_space_memory) {
+        error_report("Wrong target AS \"%s\", only system memory is allowed",
+                     iotlb->target_as->name ? iotlb->target_as->name : "none");
+        return;
+    }
+
+    rcu_read_lock();
+    if (vfio_get_xlat_addr(iotlb, NULL, &translated_addr, NULL)) {
+        int ret;
+
+        ret = vfio_get_dirty_bitmap(container, iova, iotlb->addr_mask + 1,
+                                    translated_addr);
+        if (ret) {
+            error_report("vfio_iommu_map_dirty_notify(%p, 0x%"HWADDR_PRIx", "
+                         "0x%"HWADDR_PRIx") = %d (%m)",
+                         container, iova,
+                         iotlb->addr_mask + 1, ret);
+        }
+    }
+    rcu_read_unlock();
+}
+
 static int vfio_sync_dirty_bitmap(VFIOContainer *container,
                                   MemoryRegionSection *section)
 {
     ram_addr_t ram_addr;
 
+    if (memory_region_is_iommu(section->mr)) {
+        VFIOGuestIOMMU *giommu;
+
+        QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) {
+            if (MEMORY_REGION(giommu->iommu) == section->mr &&
+                giommu->n.start == section->offset_within_region) {
+                Int128 llend;
+                vfio_giommu_dirty_notifier gdn = { .giommu = giommu };
+                int idx = memory_region_iommu_attrs_to_index(giommu->iommu,
+                                                       MEMTXATTRS_UNSPECIFIED);
+
+                llend = int128_add(int128_make64(section->offset_within_region),
+                                   section->size);
+                llend = int128_sub(llend, int128_one());
+
+                iommu_notifier_init(&gdn.n,
+                                    vfio_iommu_map_dirty_notify,
+                                    IOMMU_NOTIFIER_MAP,
+                                    section->offset_within_region,
+                                    int128_get64(llend),
+                                    idx);
+                memory_region_iommu_replay(giommu->iommu, &gdn.n);
+                break;
+            }
+        }
+        return 0;
+    }
+
     ram_addr = memory_region_get_ram_addr(section->mr) +
                section->offset_within_region;
 
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index dd991bd8f265..c0e75f24b76d 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -164,3 +164,4 @@ vfio_load_state(const char *name, uint64_t data) " (%s) data 0x%"PRIx64
 vfio_load_state_device_data(const char *name, uint64_t data_offset, uint64_t data_size) " (%s) Offset 0x%"PRIx64" size 0x%"PRIx64
 vfio_load_cleanup(const char *name) " (%s)"
 vfio_get_dirty_bitmap(int fd, uint64_t iova, uint64_t size, uint64_t bitmap_size, uint64_t start) "container fd=%d, iova=0x%"PRIx64" size= 0x%"PRIx64" bitmap_size=0x%"PRIx64" start=0x%"PRIx64
+vfio_iommu_map_dirty_notify(uint64_t iova_start, uint64_t iova_end) "iommu dirty @ 0x%"PRIx64" - 0x%"PRIx64
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v29 15/17] vfio: Add ioctl to get dirty pages bitmap during dma unmap
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
                   ` (13 preceding siblings ...)
  2020-10-26  9:36 ` [PATCH v29 14/17] vfio: Dirty page tracking when vIOMMU is enabled Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 16/17] vfio: Make vfio-pci device migration capable Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 17/17] qapi: Add VFIO devices migration stats in Migration stats Kirti Wankhede
  16 siblings, 0 replies; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

With vIOMMU, IO virtual address range can get unmapped while in pre-copy
phase of migration. In that case, unmap ioctl should return pages pinned
in that range and QEMU should find its correcponding guest physical
addresses and report those dirty.

Suggested-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Neo Jia <cjia@nvidia.com>
---
 hw/vfio/common.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 92 insertions(+), 4 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index c0b5b6245a47..49c68a5253ae 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -321,11 +321,94 @@ static bool vfio_devices_all_stopped_and_saving(VFIOContainer *container)
     return true;
 }
 
+static bool vfio_devices_all_running_and_saving(VFIOContainer *container)
+{
+    VFIOGroup *group;
+    VFIODevice *vbasedev;
+    MigrationState *ms = migrate_get_current();
+
+    if (!migration_is_setup_or_active(ms->state)) {
+        return false;
+    }
+
+    QLIST_FOREACH(group, &container->group_list, container_next) {
+        QLIST_FOREACH(vbasedev, &group->device_list, next) {
+            VFIOMigration *migration = vbasedev->migration;
+
+            if (!migration) {
+                return false;
+            }
+
+            if ((migration->device_state & VFIO_DEVICE_STATE_SAVING) &&
+                (migration->device_state & VFIO_DEVICE_STATE_RUNNING)) {
+                continue;
+            } else {
+                return false;
+            }
+        }
+    }
+    return true;
+}
+
+static int vfio_dma_unmap_bitmap(VFIOContainer *container,
+                                 hwaddr iova, ram_addr_t size,
+                                 IOMMUTLBEntry *iotlb)
+{
+    struct vfio_iommu_type1_dma_unmap *unmap;
+    struct vfio_bitmap *bitmap;
+    uint64_t pages = TARGET_PAGE_ALIGN(size) >> TARGET_PAGE_BITS;
+    int ret;
+
+    unmap = g_malloc0(sizeof(*unmap) + sizeof(*bitmap));
+
+    unmap->argsz = sizeof(*unmap) + sizeof(*bitmap);
+    unmap->iova = iova;
+    unmap->size = size;
+    unmap->flags |= VFIO_DMA_UNMAP_FLAG_GET_DIRTY_BITMAP;
+    bitmap = (struct vfio_bitmap *)&unmap->data;
+
+    /*
+     * cpu_physical_memory_set_dirty_lebitmap() expects pages in bitmap of
+     * TARGET_PAGE_SIZE to mark those dirty. Hence set bitmap_pgsize to
+     * TARGET_PAGE_SIZE.
+     */
+
+    bitmap->pgsize = TARGET_PAGE_SIZE;
+    bitmap->size = ROUND_UP(pages, sizeof(__u64) * BITS_PER_BYTE) /
+                   BITS_PER_BYTE;
+
+    if (bitmap->size > container->max_dirty_bitmap_size) {
+        error_report("UNMAP: Size of bitmap too big 0x%llx", bitmap->size);
+        ret = -E2BIG;
+        goto unmap_exit;
+    }
+
+    bitmap->data = g_try_malloc0(bitmap->size);
+    if (!bitmap->data) {
+        ret = -ENOMEM;
+        goto unmap_exit;
+    }
+
+    ret = ioctl(container->fd, VFIO_IOMMU_UNMAP_DMA, unmap);
+    if (!ret) {
+        cpu_physical_memory_set_dirty_lebitmap((uint64_t *)bitmap->data,
+                iotlb->translated_addr, pages);
+    } else {
+        error_report("VFIO_UNMAP_DMA with DIRTY_BITMAP : %m");
+    }
+
+    g_free(bitmap->data);
+unmap_exit:
+    g_free(unmap);
+    return ret;
+}
+
 /*
  * DMA - Mapping and unmapping for the "type1" IOMMU interface used on x86
  */
 static int vfio_dma_unmap(VFIOContainer *container,
-                          hwaddr iova, ram_addr_t size)
+                          hwaddr iova, ram_addr_t size,
+                          IOMMUTLBEntry *iotlb)
 {
     struct vfio_iommu_type1_dma_unmap unmap = {
         .argsz = sizeof(unmap),
@@ -334,6 +417,11 @@ static int vfio_dma_unmap(VFIOContainer *container,
         .size = size,
     };
 
+    if (iotlb && container->dirty_pages_supported &&
+        vfio_devices_all_running_and_saving(container)) {
+        return vfio_dma_unmap_bitmap(container, iova, size, iotlb);
+    }
+
     while (ioctl(container->fd, VFIO_IOMMU_UNMAP_DMA, &unmap)) {
         /*
          * The type1 backend has an off-by-one bug in the kernel (71a7d3d78e3c
@@ -381,7 +469,7 @@ static int vfio_dma_map(VFIOContainer *container, hwaddr iova,
      * the VGA ROM space.
      */
     if (ioctl(container->fd, VFIO_IOMMU_MAP_DMA, &map) == 0 ||
-        (errno == EBUSY && vfio_dma_unmap(container, iova, size) == 0 &&
+        (errno == EBUSY && vfio_dma_unmap(container, iova, size, NULL) == 0 &&
          ioctl(container->fd, VFIO_IOMMU_MAP_DMA, &map) == 0)) {
         return 0;
     }
@@ -531,7 +619,7 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
                          iotlb->addr_mask + 1, vaddr, ret);
         }
     } else {
-        ret = vfio_dma_unmap(container, iova, iotlb->addr_mask + 1);
+        ret = vfio_dma_unmap(container, iova, iotlb->addr_mask + 1, iotlb);
         if (ret) {
             error_report("vfio_dma_unmap(%p, 0x%"HWADDR_PRIx", "
                          "0x%"HWADDR_PRIx") = %d (%m)",
@@ -834,7 +922,7 @@ static void vfio_listener_region_del(MemoryListener *listener,
     }
 
     if (try_unmap) {
-        ret = vfio_dma_unmap(container, iova, int128_get64(llsize));
+        ret = vfio_dma_unmap(container, iova, int128_get64(llsize), NULL);
         if (ret) {
             error_report("vfio_dma_unmap(%p, 0x%"HWADDR_PRIx", "
                          "0x%"HWADDR_PRIx") = %d (%m)",
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v29 16/17] vfio: Make vfio-pci device migration capable
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
                   ` (14 preceding siblings ...)
  2020-10-26  9:36 ` [PATCH v29 15/17] vfio: Add ioctl to get dirty pages bitmap during dma unmap Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2020-10-26  9:36 ` [PATCH v29 17/17] qapi: Add VFIO devices migration stats in Migration stats Kirti Wankhede
  16 siblings, 0 replies; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

If the device is not a failover primary device, call
vfio_migration_probe() and vfio_migration_finalize() to enable
migration support for those devices that support it respectively to
tear it down again.
Removed migration blocker from VFIO PCI device specific structure and use
migration blocker from generic structure of  VFIO device.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Neo Jia <cjia@nvidia.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/vfio/pci.c | 28 ++++++++--------------------
 hw/vfio/pci.h |  1 -
 2 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index e27c88be6d85..58c0ce8971e3 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2791,17 +2791,6 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
         return;
     }
 
-    if (!pdev->failover_pair_id) {
-        error_setg(&vdev->migration_blocker,
-                "VFIO device doesn't support migration");
-        ret = migrate_add_blocker(vdev->migration_blocker, errp);
-        if (ret) {
-            error_free(vdev->migration_blocker);
-            vdev->migration_blocker = NULL;
-            return;
-        }
-    }
-
     vdev->vbasedev.name = g_path_get_basename(vdev->vbasedev.sysfsdev);
     vdev->vbasedev.ops = &vfio_pci_ops;
     vdev->vbasedev.type = VFIO_DEVICE_TYPE_PCI;
@@ -3069,6 +3058,13 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
         }
     }
 
+    if (!pdev->failover_pair_id) {
+        ret = vfio_migration_probe(&vdev->vbasedev, errp);
+        if (ret) {
+            error_report("%s: Migration disabled", vdev->vbasedev.name);
+        }
+    }
+
     vfio_register_err_notifier(vdev);
     vfio_register_req_notifier(vdev);
     vfio_setup_resetfn_quirk(vdev);
@@ -3083,11 +3079,6 @@ out_teardown:
     vfio_bars_exit(vdev);
 error:
     error_prepend(errp, VFIO_MSG_PREFIX, vdev->vbasedev.name);
-    if (vdev->migration_blocker) {
-        migrate_del_blocker(vdev->migration_blocker);
-        error_free(vdev->migration_blocker);
-        vdev->migration_blocker = NULL;
-    }
 }
 
 static void vfio_instance_finalize(Object *obj)
@@ -3099,10 +3090,6 @@ static void vfio_instance_finalize(Object *obj)
     vfio_bars_finalize(vdev);
     g_free(vdev->emulated_config_bits);
     g_free(vdev->rom);
-    if (vdev->migration_blocker) {
-        migrate_del_blocker(vdev->migration_blocker);
-        error_free(vdev->migration_blocker);
-    }
     /*
      * XXX Leaking igd_opregion is not an oversight, we can't remove the
      * fw_cfg entry therefore leaking this allocation seems like the safest
@@ -3130,6 +3117,7 @@ static void vfio_exitfn(PCIDevice *pdev)
     }
     vfio_teardown_msi(vdev);
     vfio_bars_exit(vdev);
+    vfio_migration_finalize(&vdev->vbasedev);
 }
 
 static void vfio_pci_reset(DeviceState *dev)
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index bce71a9ac93f..1574ef983f8f 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -172,7 +172,6 @@ struct VFIOPCIDevice {
     bool no_vfio_ioeventfd;
     bool enable_ramfb;
     VFIODisplay *dpy;
-    Error *migration_blocker;
     Notifier irqchip_change_notifier;
 };
 
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v29 17/17] qapi: Add VFIO devices migration stats in Migration stats
  2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
                   ` (15 preceding siblings ...)
  2020-10-26  9:36 ` [PATCH v29 16/17] vfio: Make vfio-pci device migration capable Kirti Wankhede
@ 2020-10-26  9:36 ` Kirti Wankhede
  2021-03-15 13:08   ` Thomas Huth
  2021-03-16 13:22   ` Eric Blake
  16 siblings, 2 replies; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26  9:36 UTC (permalink / raw)
  To: alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, Kirti Wankhede, eauger, artemp, yi.l.liu,
	quintela, ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

Added amount of bytes transferred to the VM at destination by all VFIO
devices

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/vfio/common.c              | 19 +++++++++++++++++++
 hw/vfio/migration.c           |  9 +++++++++
 include/hw/vfio/vfio-common.h |  3 +++
 migration/migration.c         | 17 +++++++++++++++++
 monitor/hmp-cmds.c            |  6 ++++++
 qapi/migration.json           | 17 +++++++++++++++++
 6 files changed, 71 insertions(+)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 49c68a5253ae..56f6fee66a55 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -292,6 +292,25 @@ const MemoryRegionOps vfio_region_ops = {
  * Device state interfaces
  */
 
+bool vfio_mig_active(void)
+{
+    VFIOGroup *group;
+    VFIODevice *vbasedev;
+
+    if (QLIST_EMPTY(&vfio_group_list)) {
+        return false;
+    }
+
+    QLIST_FOREACH(group, &vfio_group_list, next) {
+        QLIST_FOREACH(vbasedev, &group->device_list, next) {
+            if (vbasedev->migration_blocker) {
+                return false;
+            }
+        }
+    }
+    return true;
+}
+
 static bool vfio_devices_all_stopped_and_saving(VFIOContainer *container)
 {
     VFIOGroup *group;
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index ffedbcca179d..2d657289c68e 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -45,6 +45,8 @@
 #define VFIO_MIG_FLAG_DEV_SETUP_STATE   (0xffffffffef100003ULL)
 #define VFIO_MIG_FLAG_DEV_DATA_STATE    (0xffffffffef100004ULL)
 
+static int64_t bytes_transferred;
+
 static inline int vfio_mig_access(VFIODevice *vbasedev, void *val, int count,
                                   off_t off, bool iswrite)
 {
@@ -255,6 +257,7 @@ static int vfio_save_buffer(QEMUFile *f, VFIODevice *vbasedev, uint64_t *size)
         *size = data_size;
     }
 
+    bytes_transferred += data_size;
     return ret;
 }
 
@@ -785,6 +788,7 @@ static void vfio_migration_state_notifier(Notifier *notifier, void *data)
     case MIGRATION_STATUS_CANCELLING:
     case MIGRATION_STATUS_CANCELLED:
     case MIGRATION_STATUS_FAILED:
+        bytes_transferred = 0;
         ret = vfio_migration_set_state(vbasedev,
                       ~(VFIO_DEVICE_STATE_SAVING | VFIO_DEVICE_STATE_RESUMING),
                       VFIO_DEVICE_STATE_RUNNING);
@@ -866,6 +870,11 @@ err:
 
 /* ---------------------------------------------------------------------- */
 
+int64_t vfio_mig_bytes_transferred(void)
+{
+    return bytes_transferred;
+}
+
 int vfio_migration_probe(VFIODevice *vbasedev, Error **errp)
 {
     VFIOContainer *container = vbasedev->group->container;
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index b1c1b18fd228..24e299d97425 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -203,6 +203,9 @@ extern const MemoryRegionOps vfio_region_ops;
 typedef QLIST_HEAD(VFIOGroupList, VFIOGroup) VFIOGroupList;
 extern VFIOGroupList vfio_group_list;
 
+bool vfio_mig_active(void);
+int64_t vfio_mig_bytes_transferred(void);
+
 #ifdef CONFIG_LINUX
 int vfio_get_region_info(VFIODevice *vbasedev, int index,
                          struct vfio_region_info **info);
diff --git a/migration/migration.c b/migration/migration.c
index 0575ecb37953..995ccd96a774 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -57,6 +57,10 @@
 #include "qemu/queue.h"
 #include "multifd.h"
 
+#ifdef CONFIG_VFIO
+#include "hw/vfio/vfio-common.h"
+#endif
+
 #define MAX_THROTTLE  (128 << 20)      /* Migration transfer speed throttling */
 
 /* Amount of time to allocate to each "chunk" of bandwidth-throttled
@@ -1002,6 +1006,17 @@ static void populate_disk_info(MigrationInfo *info)
     }
 }
 
+static void populate_vfio_info(MigrationInfo *info)
+{
+#ifdef CONFIG_VFIO
+    if (vfio_mig_active()) {
+        info->has_vfio = true;
+        info->vfio = g_malloc0(sizeof(*info->vfio));
+        info->vfio->transferred = vfio_mig_bytes_transferred();
+    }
+#endif
+}
+
 static void fill_source_migration_info(MigrationInfo *info)
 {
     MigrationState *s = migrate_get_current();
@@ -1026,6 +1041,7 @@ static void fill_source_migration_info(MigrationInfo *info)
         populate_time_info(info, s);
         populate_ram_info(info, s);
         populate_disk_info(info);
+        populate_vfio_info(info);
         break;
     case MIGRATION_STATUS_COLO:
         info->has_status = true;
@@ -1034,6 +1050,7 @@ static void fill_source_migration_info(MigrationInfo *info)
     case MIGRATION_STATUS_COMPLETED:
         populate_time_info(info, s);
         populate_ram_info(info, s);
+        populate_vfio_info(info);
         break;
     case MIGRATION_STATUS_FAILED:
         info->has_status = true;
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 9789f4277f50..56e9bad33d94 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -357,6 +357,12 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
         }
         monitor_printf(mon, "]\n");
     }
+
+    if (info->has_vfio) {
+        monitor_printf(mon, "vfio device transferred: %" PRIu64 " kbytes\n",
+                       info->vfio->transferred >> 10);
+    }
+
     qapi_free_MigrationInfo(info);
 }
 
diff --git a/qapi/migration.json b/qapi/migration.json
index a5da513c9e05..3c7582052725 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -147,6 +147,18 @@
             'active', 'postcopy-active', 'postcopy-paused',
             'postcopy-recover', 'completed', 'failed', 'colo',
             'pre-switchover', 'device', 'wait-unplug' ] }
+##
+# @VfioStats:
+#
+# Detailed VFIO devices migration statistics
+#
+# @transferred: amount of bytes transferred to the target VM by VFIO devices
+#
+# Since: 5.2
+#
+##
+{ 'struct': 'VfioStats',
+  'data': {'transferred': 'int' } }
 
 ##
 # @MigrationInfo:
@@ -208,11 +220,16 @@
 #
 # @socket-address: Only used for tcp, to know what the real port is (Since 4.0)
 #
+# @vfio: @VfioStats containing detailed VFIO devices migration statistics,
+#        only returned if VFIO device is present, migration is supported by all
+#        VFIO devices and status is 'active' or 'completed' (since 5.2)
+#
 # Since: 0.14.0
 ##
 { 'struct': 'MigrationInfo',
   'data': {'*status': 'MigrationStatus', '*ram': 'MigrationStats',
            '*disk': 'MigrationStats',
+           '*vfio': 'VfioStats',
            '*xbzrle-cache': 'XBZRLECacheStats',
            '*total-time': 'int',
            '*expected-downtime': 'int',
-- 
2.7.0



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v29 05/17] vfio: Add VM state change handler to know state of VM
  2020-10-26  9:36 ` [PATCH v29 05/17] vfio: Add VM state change handler to know state of VM Kirti Wankhede
@ 2020-10-26 13:00   ` Alex Williamson
  2020-10-26 13:48     ` Kirti Wankhede
  0 siblings, 1 reply; 28+ messages in thread
From: Alex Williamson @ 2020-10-26 13:00 UTC (permalink / raw)
  To: Kirti Wankhede
  Cc: cohuck, cjia, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, eauger, artemp, yi.l.liu, quintela,
	ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

On Mon, 26 Oct 2020 15:06:15 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> VM state change handler is called on change in VM's state. Based on
> VM state, VFIO device state should be changed.
> Added read/write helper functions for migration region.
> Added function to set device_state.
> 
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Reviewed-by: Neo Jia <cjia@nvidia.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  hw/vfio/migration.c           | 158 ++++++++++++++++++++++++++++++++++++++++++
>  hw/vfio/trace-events          |   2 +
>  include/hw/vfio/vfio-common.h |   4 ++
>  3 files changed, 164 insertions(+)
> 
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index fd7faf423cdc..65ce735d667b 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
[snip]
> @@ -64,6 +216,9 @@ static int vfio_migration_init(VFIODevice *vbasedev,
>          ret = -EINVAL;
>          goto err;
>      }
> +
> +    migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change,
> +                                                           vbasedev);
>      return 0;
>  
>  err:

Fails to build, @migration is not defined.  We could use
vbasedev->migration or pull defining and setting @migration from patch
06.  Thanks,

Alex



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v29 05/17] vfio: Add VM state change handler to know state of VM
  2020-10-26 13:00   ` Alex Williamson
@ 2020-10-26 13:48     ` Kirti Wankhede
  2020-10-26 14:02       ` Alex Williamson
  0 siblings, 1 reply; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26 13:48 UTC (permalink / raw)
  To: Alex Williamson
  Cc: cohuck, cjia, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, eauger, artemp, yi.l.liu, quintela,
	ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam



On 10/26/2020 6:30 PM, Alex Williamson wrote:
> On Mon, 26 Oct 2020 15:06:15 +0530
> Kirti Wankhede <kwankhede@nvidia.com> wrote:
> 
>> VM state change handler is called on change in VM's state. Based on
>> VM state, VFIO device state should be changed.
>> Added read/write helper functions for migration region.
>> Added function to set device_state.
>>
>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
>> Reviewed-by: Neo Jia <cjia@nvidia.com>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>> ---
>>   hw/vfio/migration.c           | 158 ++++++++++++++++++++++++++++++++++++++++++
>>   hw/vfio/trace-events          |   2 +
>>   include/hw/vfio/vfio-common.h |   4 ++
>>   3 files changed, 164 insertions(+)
>>
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index fd7faf423cdc..65ce735d667b 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
> [snip]
>> @@ -64,6 +216,9 @@ static int vfio_migration_init(VFIODevice *vbasedev,
>>           ret = -EINVAL;
>>           goto err;
>>       }
>> +
>> +    migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change,
>> +                                                           vbasedev);
>>       return 0;
>>   
>>   err:
> 
> Fails to build, @migration is not defined.  We could use
> vbasedev->migration or pull defining and setting @migration from patch
> 06.  Thanks,
> 

Pulling and setting migration from patch 06 seems better option.
Should I resend patch 5 & 6 only?

Thanks,
Kirti


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v29 05/17] vfio: Add VM state change handler to know state of VM
  2020-10-26 13:48     ` Kirti Wankhede
@ 2020-10-26 14:02       ` Alex Williamson
  2020-10-26 14:37         ` Kirti Wankhede
  0 siblings, 1 reply; 28+ messages in thread
From: Alex Williamson @ 2020-10-26 14:02 UTC (permalink / raw)
  To: Kirti Wankhede
  Cc: cohuck, cjia, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, eauger, artemp, yi.l.liu, quintela,
	ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

On Mon, 26 Oct 2020 19:18:51 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> On 10/26/2020 6:30 PM, Alex Williamson wrote:
> > On Mon, 26 Oct 2020 15:06:15 +0530
> > Kirti Wankhede <kwankhede@nvidia.com> wrote:
> >   
> >> VM state change handler is called on change in VM's state. Based on
> >> VM state, VFIO device state should be changed.
> >> Added read/write helper functions for migration region.
> >> Added function to set device_state.
> >>
> >> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> >> Reviewed-by: Neo Jia <cjia@nvidia.com>
> >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> >> ---
> >>   hw/vfio/migration.c           | 158 ++++++++++++++++++++++++++++++++++++++++++
> >>   hw/vfio/trace-events          |   2 +
> >>   include/hw/vfio/vfio-common.h |   4 ++
> >>   3 files changed, 164 insertions(+)
> >>
> >> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> >> index fd7faf423cdc..65ce735d667b 100644
> >> --- a/hw/vfio/migration.c
> >> +++ b/hw/vfio/migration.c  
> > [snip]  
> >> @@ -64,6 +216,9 @@ static int vfio_migration_init(VFIODevice *vbasedev,
> >>           ret = -EINVAL;
> >>           goto err;
> >>       }
> >> +
> >> +    migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change,
> >> +                                                           vbasedev);
> >>       return 0;
> >>   
> >>   err:  
> > 
> > Fails to build, @migration is not defined.  We could use
> > vbasedev->migration or pull defining and setting @migration from patch
> > 06.  Thanks,
> >   
> 
> Pulling and setting migration from patch 06 seems better option.
> Should I resend patch 5 & 6 only?

I've resolved this locally as patch 05:

@@ -38,6 +190,7 @@ static int vfio_migration_init(VFIODevice *vbasedev,
 {
     int ret;
     Object *obj;
+    VFIOMigration *migration;
 
     if (!vbasedev->ops->vfio_get_object) {
         return -EINVAL;
@@ -64,6 +217,10 @@ static int vfio_migration_init(VFIODevice *vbasedev,
         ret = -EINVAL;
         goto err;
     }
+
+    migration = vbasedev->migration;
+    migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change,
+                                                           vbasedev);
     return 0;
 
 err:

patch 06:

@@ -219,8 +243,11 @@ static int vfio_migration_init(VFIODevice *vbasedev,
     }
 
     migration = vbasedev->migration;
+    migration->vbasedev = vbasedev;
     migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change,
                                                            vbasedev);
+    migration->migration_state.notify = vfio_migration_state_notifier;
+    add_migration_state_change_notifier(&migration->migration_state);
     return 0;
 
 err:

If you're satisfied with that, no need to resend.  Thanks,

Alex



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v29 05/17] vfio: Add VM state change handler to know state of VM
  2020-10-26 14:02       ` Alex Williamson
@ 2020-10-26 14:37         ` Kirti Wankhede
  0 siblings, 0 replies; 28+ messages in thread
From: Kirti Wankhede @ 2020-10-26 14:37 UTC (permalink / raw)
  To: Alex Williamson
  Cc: cohuck, cjia, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, eauger, artemp, yi.l.liu, quintela,
	ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam



On 10/26/2020 7:32 PM, Alex Williamson wrote:
> On Mon, 26 Oct 2020 19:18:51 +0530
> Kirti Wankhede <kwankhede@nvidia.com> wrote:
> 
>> On 10/26/2020 6:30 PM, Alex Williamson wrote:
>>> On Mon, 26 Oct 2020 15:06:15 +0530
>>> Kirti Wankhede <kwankhede@nvidia.com> wrote:
>>>    
>>>> VM state change handler is called on change in VM's state. Based on
>>>> VM state, VFIO device state should be changed.
>>>> Added read/write helper functions for migration region.
>>>> Added function to set device_state.
>>>>
>>>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
>>>> Reviewed-by: Neo Jia <cjia@nvidia.com>
>>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>>>> ---
>>>>    hw/vfio/migration.c           | 158 ++++++++++++++++++++++++++++++++++++++++++
>>>>    hw/vfio/trace-events          |   2 +
>>>>    include/hw/vfio/vfio-common.h |   4 ++
>>>>    3 files changed, 164 insertions(+)
>>>>
>>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>>>> index fd7faf423cdc..65ce735d667b 100644
>>>> --- a/hw/vfio/migration.c
>>>> +++ b/hw/vfio/migration.c
>>> [snip]
>>>> @@ -64,6 +216,9 @@ static int vfio_migration_init(VFIODevice *vbasedev,
>>>>            ret = -EINVAL;
>>>>            goto err;
>>>>        }
>>>> +
>>>> +    migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change,
>>>> +                                                           vbasedev);
>>>>        return 0;
>>>>    
>>>>    err:
>>>
>>> Fails to build, @migration is not defined.  We could use
>>> vbasedev->migration or pull defining and setting @migration from patch
>>> 06.  Thanks,
>>>    
>>
>> Pulling and setting migration from patch 06 seems better option.
>> Should I resend patch 5 & 6 only?
> 
> I've resolved this locally as patch 05:
> 
> @@ -38,6 +190,7 @@ static int vfio_migration_init(VFIODevice *vbasedev,
>   {
>       int ret;
>       Object *obj;
> +    VFIOMigration *migration;
>   
>       if (!vbasedev->ops->vfio_get_object) {
>           return -EINVAL;
> @@ -64,6 +217,10 @@ static int vfio_migration_init(VFIODevice *vbasedev,
>           ret = -EINVAL;
>           goto err;
>       }
> +
> +    migration = vbasedev->migration;
> +    migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change,
> +                                                           vbasedev);
>       return 0;
>   
>   err:
> 
> patch 06:
> 
> @@ -219,8 +243,11 @@ static int vfio_migration_init(VFIODevice *vbasedev,
>       }
>   
>       migration = vbasedev->migration;
> +    migration->vbasedev = vbasedev;
>       migration->vm_state = qemu_add_vm_change_state_handler(vfio_vmstate_change,
>                                                              vbasedev);
> +    migration->migration_state.notify = vfio_migration_state_notifier;
> +    add_migration_state_change_notifier(&migration->migration_state);
>       return 0;
>   
>   err:
> 
> If you're satisfied with that, no need to resend.  Thanks,
> 

Yes, this is exactly I was going to send.
Thanks for fixing it.

Thanks,
Kirti


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v29 04/17] vfio: Add migration region initialization and finalize function
  2020-10-26  9:36 ` [PATCH v29 04/17] vfio: Add migration region initialization and finalize function Kirti Wankhede
@ 2020-10-26 16:23   ` Cornelia Huck
  0 siblings, 0 replies; 28+ messages in thread
From: Cornelia Huck @ 2020-10-26 16:23 UTC (permalink / raw)
  To: Kirti Wankhede
  Cc: cjia, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, eauger, artemp, yi.l.liu, quintela,
	ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, alex.williamson,
	changpeng.liu, eskultet, Ken.Xue, jonathan.davies, pbonzini,
	dnigam

On Mon, 26 Oct 2020 15:06:14 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> Whether the VFIO device supports migration or not is decided based of
> migration region query. If migration region query is successful and migration
> region initialization is successful then migration is supported else
> migration is blocked.
> 
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Reviewed-by: Neo Jia <cjia@nvidia.com>
> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/vfio/meson.build           |   1 +
>  hw/vfio/migration.c           | 122 ++++++++++++++++++++++++++++++++++++++++++
>  hw/vfio/trace-events          |   3 ++
>  include/hw/vfio/vfio-common.h |   9 ++++
>  4 files changed, 135 insertions(+)
>  create mode 100644 hw/vfio/migration.c

Reviewed-by: Cornelia Huck <cohuck@redhat.com>



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v29 10/17] memory: Set DIRTY_MEMORY_MIGRATION when IOMMU is enabled
  2020-10-26  9:36 ` [PATCH v29 10/17] memory: Set DIRTY_MEMORY_MIGRATION when IOMMU is enabled Kirti Wankhede
@ 2020-10-26 16:47   ` Alex Williamson
  2020-10-26 16:58     ` Paolo Bonzini
  0 siblings, 1 reply; 28+ messages in thread
From: Alex Williamson @ 2020-10-26 16:47 UTC (permalink / raw)
  To: Kirti Wankhede, pbonzini
  Cc: cohuck, cjia, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, eauger, artemp, yi.l.liu, quintela,
	ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, dnigam

Paolo,

Any objection to this change?  Thanks,

Alex

On Mon, 26 Oct 2020 15:06:20 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> mr->ram_block is NULL when mr->is_iommu is true, then fr.dirty_log_mask
> wasn't set correctly due to which memory listener's log_sync doesn't
> get called.
> This patch returns log_mask with DIRTY_MEMORY_MIGRATION set when
> IOMMU is enabled.
> 
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
> ---
>  softmmu/memory.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index 403ff3abc99b..94f606e9d9d9 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -1792,7 +1792,7 @@ bool memory_region_is_ram_device(MemoryRegion *mr)
>  uint8_t memory_region_get_dirty_log_mask(MemoryRegion *mr)
>  {
>      uint8_t mask = mr->dirty_log_mask;
> -    if (global_dirty_log && mr->ram_block) {
> +    if (global_dirty_log && (mr->ram_block || memory_region_is_iommu(mr))) {
>          mask |= (1 << DIRTY_MEMORY_MIGRATION);
>      }
>      return mask;



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v29 10/17] memory: Set DIRTY_MEMORY_MIGRATION when IOMMU is enabled
  2020-10-26 16:47   ` Alex Williamson
@ 2020-10-26 16:58     ` Paolo Bonzini
  0 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2020-10-26 16:58 UTC (permalink / raw)
  To: Alex Williamson, Kirti Wankhede
  Cc: cohuck, cjia, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, eauger, artemp, yi.l.liu, quintela,
	ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, dnigam

On 26/10/20 17:47, Alex Williamson wrote:
> Paolo,
> 
> Any objection to this change?  Thanks,

Not at all.  Sorry I missed this change.

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

> Alex
> 
> On Mon, 26 Oct 2020 15:06:20 +0530
> Kirti Wankhede <kwankhede@nvidia.com> wrote:
> 
>> mr->ram_block is NULL when mr->is_iommu is true, then fr.dirty_log_mask
>> wasn't set correctly due to which memory listener's log_sync doesn't
>> get called.
>> This patch returns log_mask with DIRTY_MEMORY_MIGRATION set when
>> IOMMU is enabled.
>>
>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
>> Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
>> ---
>>  softmmu/memory.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/softmmu/memory.c b/softmmu/memory.c
>> index 403ff3abc99b..94f606e9d9d9 100644
>> --- a/softmmu/memory.c
>> +++ b/softmmu/memory.c
>> @@ -1792,7 +1792,7 @@ bool memory_region_is_ram_device(MemoryRegion *mr)
>>  uint8_t memory_region_get_dirty_log_mask(MemoryRegion *mr)
>>  {
>>      uint8_t mask = mr->dirty_log_mask;
>> -    if (global_dirty_log && mr->ram_block) {
>> +    if (global_dirty_log && (mr->ram_block || memory_region_is_iommu(mr))) {
>>          mask |= (1 << DIRTY_MEMORY_MIGRATION);
>>      }
>>      return mask;
> 



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v29 17/17] qapi: Add VFIO devices migration stats in Migration stats
  2020-10-26  9:36 ` [PATCH v29 17/17] qapi: Add VFIO devices migration stats in Migration stats Kirti Wankhede
@ 2021-03-15 13:08   ` Thomas Huth
  2021-03-16 13:22   ` Eric Blake
  1 sibling, 0 replies; 28+ messages in thread
From: Thomas Huth @ 2021-03-15 13:08 UTC (permalink / raw)
  To: Kirti Wankhede, alex.williamson, cjia, dgilbert, quintela
  Cc: zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst, qemu-devel,
	peterx, eauger, artemp, yi.l.liu, eskultet, ziye.yang, armbru,
	mlevitsk, pasic, felipe, Ken.Xue, mcrossley, kevin.tian,
	yan.y.zhao, changpeng.liu, cohuck, zhi.a.wang, jonathan.davies,
	pbonzini, dnigam

On 26/10/2020 10.36, Kirti Wankhede wrote:
> Added amount of bytes transferred to the VM at destination by all VFIO
> devices
> 
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>   hw/vfio/common.c              | 19 +++++++++++++++++++
>   hw/vfio/migration.c           |  9 +++++++++
>   include/hw/vfio/vfio-common.h |  3 +++
>   migration/migration.c         | 17 +++++++++++++++++
>   monitor/hmp-cmds.c            |  6 ++++++
>   qapi/migration.json           | 17 +++++++++++++++++
>   6 files changed, 71 insertions(+)
[...]
> diff --git a/migration/migration.c b/migration/migration.c
> index 0575ecb37953..995ccd96a774 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -57,6 +57,10 @@
>   #include "qemu/queue.h"
>   #include "multifd.h"
>   
> +#ifdef CONFIG_VFIO
> +#include "hw/vfio/vfio-common.h"
> +#endif
> +
>   #define MAX_THROTTLE  (128 << 20)      /* Migration transfer speed throttling */
>   
>   /* Amount of time to allocate to each "chunk" of bandwidth-throttled
> @@ -1002,6 +1006,17 @@ static void populate_disk_info(MigrationInfo *info)
>       }
>   }
>   
> +static void populate_vfio_info(MigrationInfo *info)
> +{
> +#ifdef CONFIG_VFIO
> +    if (vfio_mig_active()) {
> +        info->has_vfio = true;
> +        info->vfio = g_malloc0(sizeof(*info->vfio));
> +        info->vfio->transferred = vfio_mig_bytes_transferred();
> +    }
> +#endif
> +}

  Hi!

I'm afraid, but this #ifdef CONFIG_VFIO here likely does not work as 
expected: migration/migration.c is common code, i.e. it is compiled only 
once for all targets. But CONFIG_VFIO is a target-specific config switch, so 
this is only defined properly for target specific .c files. So depending on 
which target has been compiled first, the code might be included or not for 
all the other targets, no matter whether they have VFIO or not.
To fix this issue, I think it's likely best to move the function into a new 
file instead and include that via specific_ss.add() in the meson.build file.

  Thomas



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v29 17/17] qapi: Add VFIO devices migration stats in Migration stats
  2020-10-26  9:36 ` [PATCH v29 17/17] qapi: Add VFIO devices migration stats in Migration stats Kirti Wankhede
  2021-03-15 13:08   ` Thomas Huth
@ 2021-03-16 13:22   ` Eric Blake
  2021-03-16 13:24     ` Eric Blake
  1 sibling, 1 reply; 28+ messages in thread
From: Eric Blake @ 2021-03-16 13:22 UTC (permalink / raw)
  To: Kirti Wankhede, alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, eauger, artemp, yi.l.liu, quintela,
	ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

On 10/26/20 4:36 AM, Kirti Wankhede wrote:
> Added amount of bytes transferred to the VM at destination by all VFIO
> devices
> 
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---

> +++ b/qapi/migration.json
> @@ -147,6 +147,18 @@
>              'active', 'postcopy-active', 'postcopy-paused',
>              'postcopy-recover', 'completed', 'failed', 'colo',
>              'pre-switchover', 'device', 'wait-unplug' ] }
> +##
> +# @VfioStats:
> +#
> +# Detailed VFIO devices migration statistics
> +#
> +# @transferred: amount of bytes transferred to the target VM by VFIO devices
> +#
> +# Since: 5.2

5.2 is last year; this should be 6.0 (if this makes it in time for soft
freeze) or even 6.1

> +#
> +##
> +{ 'struct': 'VfioStats',
> +  'data': {'transferred': 'int' } }
>  
>  ##
>  # @MigrationInfo:
> @@ -208,11 +220,16 @@
>  #
>  # @socket-address: Only used for tcp, to know what the real port is (Since 4.0)
>  #
> +# @vfio: @VfioStats containing detailed VFIO devices migration statistics,
> +#        only returned if VFIO device is present, migration is supported by all
> +#        VFIO devices and status is 'active' or 'completed' (since 5.2)

here, too

> +#
>  # Since: 0.14.0
>  ##
>  { 'struct': 'MigrationInfo',
>    'data': {'*status': 'MigrationStatus', '*ram': 'MigrationStats',
>             '*disk': 'MigrationStats',
> +           '*vfio': 'VfioStats',
>             '*xbzrle-cache': 'XBZRLECacheStats',
>             '*total-time': 'int',
>             '*expected-downtime': 'int',
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v29 17/17] qapi: Add VFIO devices migration stats in Migration stats
  2021-03-16 13:22   ` Eric Blake
@ 2021-03-16 13:24     ` Eric Blake
  0 siblings, 0 replies; 28+ messages in thread
From: Eric Blake @ 2021-03-16 13:24 UTC (permalink / raw)
  To: Kirti Wankhede, alex.williamson, cjia
  Cc: cohuck, zhi.wang.linux, aik, Zhengxiao.zx, shuangtai.tst,
	qemu-devel, peterx, eauger, artemp, yi.l.liu, quintela,
	ziye.yang, armbru, mlevitsk, pasic, felipe, zhi.a.wang,
	mcrossley, kevin.tian, yan.y.zhao, dgilbert, changpeng.liu,
	eskultet, Ken.Xue, jonathan.davies, pbonzini, dnigam

On 3/16/21 8:22 AM, Eric Blake wrote:
> On 10/26/20 4:36 AM, Kirti Wankhede wrote:

Hmm, just now realizing this is an old thread...

>> Added amount of bytes transferred to the VM at destination by all VFIO
>> devices
>>
>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> ---
> 
>> +++ b/qapi/migration.json
>> @@ -147,6 +147,18 @@
>>              'active', 'postcopy-active', 'postcopy-paused',
>>              'postcopy-recover', 'completed', 'failed', 'colo',
>>              'pre-switchover', 'device', 'wait-unplug' ] }
>> +##
>> +# @VfioStats:
>> +#
>> +# Detailed VFIO devices migration statistics
>> +#
>> +# @transferred: amount of bytes transferred to the target VM by VFIO devices
>> +#
>> +# Since: 5.2
> 
> 5.2 is last year; this should be 6.0 (if this makes it in time for soft
> freeze) or even 6.1

...but it was revived because of Thomas' notice of a problem with
CONFIG_VFIO with what is already in-tree, rather than something still
awaiting review.  My (untimely) review is thus unnecessary noise; sorry.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2021-03-16 13:26 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-26  9:36 [PATCH v29 00/17] Add migration support for VFIO devices Kirti Wankhede
2020-10-26  9:36 ` [PATCH v29 01/17] vfio: Add function to unmap VFIO region Kirti Wankhede
2020-10-26  9:36 ` [PATCH v29 02/17] vfio: Add vfio_get_object callback to VFIODeviceOps Kirti Wankhede
2020-10-26  9:36 ` [PATCH v29 03/17] vfio: Add save and load functions for VFIO PCI devices Kirti Wankhede
2020-10-26  9:36 ` [PATCH v29 04/17] vfio: Add migration region initialization and finalize function Kirti Wankhede
2020-10-26 16:23   ` Cornelia Huck
2020-10-26  9:36 ` [PATCH v29 05/17] vfio: Add VM state change handler to know state of VM Kirti Wankhede
2020-10-26 13:00   ` Alex Williamson
2020-10-26 13:48     ` Kirti Wankhede
2020-10-26 14:02       ` Alex Williamson
2020-10-26 14:37         ` Kirti Wankhede
2020-10-26  9:36 ` [PATCH v29 06/17] vfio: Add migration state change notifier Kirti Wankhede
2020-10-26  9:36 ` [PATCH v29 07/17] vfio: Register SaveVMHandlers for VFIO device Kirti Wankhede
2020-10-26  9:36 ` [PATCH v29 08/17] vfio: Add save state functions to SaveVMHandlers Kirti Wankhede
2020-10-26  9:36 ` [PATCH v29 09/17] vfio: Add load " Kirti Wankhede
2020-10-26  9:36 ` [PATCH v29 10/17] memory: Set DIRTY_MEMORY_MIGRATION when IOMMU is enabled Kirti Wankhede
2020-10-26 16:47   ` Alex Williamson
2020-10-26 16:58     ` Paolo Bonzini
2020-10-26  9:36 ` [PATCH v29 11/17] vfio: Get migration capability flags for container Kirti Wankhede
2020-10-26  9:36 ` [PATCH v29 12/17] vfio: Add function to start and stop dirty pages tracking Kirti Wankhede
2020-10-26  9:36 ` [PATCH v29 13/17] vfio: Add vfio_listener_log_sync to mark dirty pages Kirti Wankhede
2020-10-26  9:36 ` [PATCH v29 14/17] vfio: Dirty page tracking when vIOMMU is enabled Kirti Wankhede
2020-10-26  9:36 ` [PATCH v29 15/17] vfio: Add ioctl to get dirty pages bitmap during dma unmap Kirti Wankhede
2020-10-26  9:36 ` [PATCH v29 16/17] vfio: Make vfio-pci device migration capable Kirti Wankhede
2020-10-26  9:36 ` [PATCH v29 17/17] qapi: Add VFIO devices migration stats in Migration stats Kirti Wankhede
2021-03-15 13:08   ` Thomas Huth
2021-03-16 13:22   ` Eric Blake
2021-03-16 13:24     ` Eric Blake

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.