Re: [PATCH v4 26/33] vfio/migration: Multifd device state transfer support - receive init/cleanup

[Cédric Le Goater]

On 1/30/25 11:08, Maciej S. Szmigiero wrote:

From: "Maciej S. Szmigiero" <maciej.szmigi...@oracle.com>

Add support for VFIOMultifd data structure that will contain most of the
receive-side data together with its init/cleanup methods.

Signed-off-by: Maciej S. Szmigiero <maciej.szmigi...@oracle.com>
---
  hw/vfio/migration.c           | 52 +++++++++++++++++++++++++++++++++--
  include/hw/vfio/vfio-common.h |  5 ++++
  2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 3211041939c6..bcdf204d5cf4 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -300,6 +300,9 @@ typedef struct VFIOStateBuffer {
      size_t len;
  } VFIOStateBuffer;

+typedef struct VFIOMultifd {

+} VFIOMultifd;
+
  static void vfio_state_buffer_clear(gpointer data)
  {
      VFIOStateBuffer *lb = data;
@@ -398,6 +401,18 @@ static int vfio_load_device_config_state(QEMUFile *f, void 
*opaque)
      return qemu_file_get_error(f);
  }

+static VFIOMultifd *vfio_multifd_new(void)

+{
+    VFIOMultifd *multifd = g_new(VFIOMultifd, 1);
+
+    return multifd;
+}
+
+static void vfio_multifd_free(VFIOMultifd *multifd)
+{
+    g_free(multifd);
+}
+
  static void vfio_migration_cleanup(VFIODevice *vbasedev)
  {
      VFIOMigration *migration = vbasedev->migration;
@@ -785,14 +800,47 @@ static void vfio_save_state(QEMUFile *f, void *opaque)
  static int vfio_load_setup(QEMUFile *f, void *opaque, Error **errp)
  {
      VFIODevice *vbasedev = opaque;
+    VFIOMigration *migration = vbasedev->migration;
+    int ret;
+
+    /*
+     * Make a copy of this setting at the start in case it is changed
+     * mid-migration.
+     */
+    if (vbasedev->migration_multifd_transfer == ON_OFF_AUTO_AUTO) {
+        migration->multifd_transfer = vfio_multifd_transfer_supported();

Attribute "migration->multifd_transfer" is not necessary. It can be
replaced by a small inline helper testing pointer migration->multifd
and this routine can use a local variable instead.

I don't think the '_transfer' suffix adds much to the understanding.

+    } else {
+        migration->multifd_transfer =
+            vbasedev->migration_multifd_transfer == ON_OFF_AUTO_ON;
+    }
+
+    if (migration->multifd_transfer && !vfio_multifd_transfer_supported()) {
+        error_setg(errp,
+                   "%s: Multifd device transfer requested but unsupported in the 
current config",
+                   vbasedev->name);
+        return -EINVAL;
+    }

The above checks are also introduced in vfio_save_setup(). Please
implement a common routine vfio_multifd_is_enabled() or some other
name.

+    ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_RESUMING,
+                                   migration->device_state, errp);
+    if (ret) {
+        return ret;
+    }
+
+    if (migration->multifd_transfer) {
+        assert(!migration->multifd);
+        migration->multifd = vfio_multifd_new();
+    }

- return vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_RESUMING,

-                                    vbasedev->migration->device_state, errp);
+    return 0;
  }

static int vfio_load_cleanup(void *opaque)

  {
      VFIODevice *vbasedev = opaque;
+    VFIOMigration *migration = vbasedev->migration;
+
+    g_clear_pointer(&migration->multifd, vfio_multifd_free);

please add a vfio_multifd_cleanup() routine.

      vfio_migration_cleanup(vbasedev);
      trace_vfio_load_cleanup(vbasedev->name);
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 153d03745dc7..c0c9c0b1b263 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -61,6 +61,8 @@ typedef struct VFIORegion {
      uint8_t nr; /* cache the region number for debug */
  } VFIORegion;

+typedef struct VFIOMultifd VFIOMultifd;

+
  typedef struct VFIOMigration {
      struct VFIODevice *vbasedev;
      VMChangeStateEntry *vm_state;
@@ -72,6 +74,8 @@ typedef struct VFIOMigration {
      uint64_t mig_flags;
      uint64_t precopy_init_size;
      uint64_t precopy_dirty_size;
+    bool multifd_transfer;
+    VFIOMultifd *multifd;
      bool initial_data_sent;

bool event_save_iterate_started;

@@ -133,6 +137,7 @@ typedef struct VFIODevice {
      bool no_mmap;
      bool ram_block_discard_allowed;
      OnOffAuto enable_migration;
+    OnOffAuto migration_multifd_transfer;

This property should be added at the end of the series, with documentation,
and used in the vfio_multifd_some_name() routine I mentioned above.


Thanks,

C.

      OnOffAuto migration_load_config_after_iter;
      bool migration_events;
      VFIODeviceOps *ops;