Re: [PATCH v5 25/36] vfio/migration: Multifd device state transfer support - receive init/cleanup

[Cédric Le Goater]

On 2/19/25 21:34, 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-multifd.c   | 33 +++++++++++++++++++++++++++++++++
  hw/vfio/migration-multifd.h   |  8 ++++++++
  hw/vfio/migration.c           | 29 +++++++++++++++++++++++++++--
  include/hw/vfio/vfio-common.h |  3 +++
  4 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/migration-multifd.c b/hw/vfio/migration-multifd.c
index 7328ad8e925c..c2defc0efef0 100644
--- a/hw/vfio/migration-multifd.c
+++ b/hw/vfio/migration-multifd.c
@@ -41,6 +41,9 @@ typedef struct VFIOStateBuffer {
      size_t len;
  } VFIOStateBuffer;

+typedef struct VFIOMultifd {

+} VFIOMultifd;
+
  static void vfio_state_buffer_clear(gpointer data)
  {
      VFIOStateBuffer *lb = data;
@@ -84,8 +87,38 @@ static VFIOStateBuffer 
*vfio_state_buffers_at(VFIOStateBuffers *bufs, guint idx)
      return &g_array_index(bufs->array, VFIOStateBuffer, idx);
  }

+VFIOMultifd *vfio_multifd_new(void)

+{
+    VFIOMultifd *multifd = g_new(VFIOMultifd, 1);
+
+    return multifd;
+}
+
+void vfio_multifd_free(VFIOMultifd *multifd)
+{
+    g_free(multifd);
+}
+
  bool vfio_multifd_transfer_supported(void)
  {
      return multifd_device_state_supported() &&
          migrate_send_switchover_start();
  }
+
+bool vfio_multifd_transfer_enabled(VFIODevice *vbasedev)
+{
+    return false;
+}
+
+bool vfio_multifd_transfer_setup(VFIODevice *vbasedev, Error **errp)
+{
+    if (vfio_multifd_transfer_enabled(vbasedev) &&
+        !vfio_multifd_transfer_supported()) {
+        error_setg(errp,
+                   "%s: Multifd device transfer requested but unsupported in the 
current config",
+                   vbasedev->name);
+        return false;
+    }
+
+    return true;
+}
diff --git a/hw/vfio/migration-multifd.h b/hw/vfio/migration-multifd.h
index 8fe004c1da81..1eefba3b2eed 100644
--- a/hw/vfio/migration-multifd.h
+++ b/hw/vfio/migration-multifd.h
@@ -12,6 +12,14 @@

#include "hw/vfio/vfio-common.h" +typedef struct VFIOMultifd VFIOMultifd;

+
+VFIOMultifd *vfio_multifd_new(void);
+void vfio_multifd_free(VFIOMultifd *multifd);
+
  bool vfio_multifd_transfer_supported(void);
+bool vfio_multifd_transfer_enabled(VFIODevice *vbasedev);
+
+bool vfio_multifd_transfer_setup(VFIODevice *vbasedev, Error **errp);

#endif

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 7b79be6ad293..4311de763885 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -674,15 +674,40 @@ 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;
+
+    if (!vfio_multifd_transfer_setup(vbasedev, errp)) {
+        return -EINVAL;
+    }
+
+    ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_RESUMING,
+                                   migration->device_state, errp);
+    if (ret) {
+        return ret;
+    }

- return vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_RESUMING,

-                                    vbasedev->migration->device_state, errp);
+    if (vfio_multifd_transfer_enabled(vbasedev)) {
+        assert(!migration->multifd);
+        migration->multifd = vfio_multifd_new();

When called from vfio_load_setup(), I think vfio_multifd_transfer_setup()
should allocate migration->multifd at the same time. It would simplify
the setup to one step. Maybe we could add a bool parameter ? because,
IIRC, you didn't like the idea of allocating it always, that is in
vfio_save_setup() too.

For symmetry, could vfio_save_cleanup() call vfio_multifd_cleanup() too ?
a setup implies a cleanup.

Thanks,

C.

+    }
+
+    return 0;
+}
+
+static void vfio_multifd_cleanup(VFIODevice *vbasedev)
+{
+    VFIOMigration *migration = vbasedev->migration;
+
+    g_clear_pointer(&migration->multifd, vfio_multifd_free);
  }

static int vfio_load_cleanup(void *opaque)

  {
      VFIODevice *vbasedev = opaque;

+ vfio_multifd_cleanup(vbasedev);

+
      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 64ee3b1a2547..ab110198bd6b 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -78,6 +78,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;
@@ -89,6 +91,7 @@ typedef struct VFIOMigration {
      uint64_t mig_flags;
      uint64_t precopy_init_size;
      uint64_t precopy_dirty_size;
+    VFIOMultifd *multifd;
      bool initial_data_sent;

bool event_save_iterate_started;