From: "Maciej S. Szmigiero" <maciej.szmigiero@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.szmigiero@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();
+    }
+
+    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;

On 2/19/25 21:34, Maciej S. Szmigiero wrote:
> From: "Maciej S. Szmigiero" <maciej.szmigiero@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.szmigiero@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;
>

On 26.02.2025 18:46, Cédric Le Goater wrote:
> On 2/19/25 21:34, Maciej S. Szmigiero wrote:
>> From: "Maciej S. Szmigiero" <maciej.szmigiero@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.szmigiero@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.

I have added a "bool alloc_multifd" parameter to
vfio_multifd_transfer_setup() and renamed it to vfio_multifd_setup() for
consistency with vfio_multifd_cleanup().

Unexported vfio_multifd_new() now that it is called only from
vfio_multifd_setup() in the same translation unit.

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

Added vfio_multifd_cleanup() call to vfio_save_cleanup() with a comment
describing that it is currently a NOP.

> Thanks,
> 
> C.

Thanks,
Maciej

On 2/19/25 21:34, Maciej S. Szmigiero wrote:
> From: "Maciej S. Szmigiero" <maciej.szmigiero@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.szmigiero@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();
> +    }
> +
> +    return 0;
> +}
> +
> +static void vfio_multifd_cleanup(VFIODevice *vbasedev)
> +{
> +    VFIOMigration *migration = vbasedev->migration;
> +
> +    g_clear_pointer(&migration->multifd, vfio_multifd_free);
>   }

Please move vfio_multifd_cleanup() to migration-multifd.c.

Thanks,

C.



>   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;
>

On 26.02.2025 18:28, Cédric Le Goater wrote:
> On 2/19/25 21:34, Maciej S. Szmigiero wrote:
>> From: "Maciej S. Szmigiero" <maciej.szmigiero@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.szmigiero@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();
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void vfio_multifd_cleanup(VFIODevice *vbasedev)
>> +{
>> +    VFIOMigration *migration = vbasedev->migration;
>> +
>> +    g_clear_pointer(&migration->multifd, vfio_multifd_free);
>>   }
> 
> Please move vfio_multifd_cleanup() to migration-multifd.c.

Done now.

> Thanks,
> 
> C.
>

Thanks,
Maciej

  

On 2/19/25 21:34, Maciej S. Szmigiero wrote:
> From: "Maciej S. Szmigiero" <maciej.szmigiero@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.szmigiero@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;
> +    }

This check on the consistency of the settings confused me a little. Even if
simple, I would have put it in a separate patch for better understanding.

The rest looks good.


Thanks,

C.



> +    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();
> +    }
> +
> +    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;
>

On 2/26/25 11:14, Cédric Le Goater wrote:
> On 2/19/25 21:34, Maciej S. Szmigiero wrote:
>> From: "Maciej S. Szmigiero" <maciej.szmigiero@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.szmigiero@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;
>> +    }


now that I have reached patch 31 I understand better. I would put the
check above in patch 31 and simply return true for now.

Thanks,

C.


>> +    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;
>> +    }
> 
> This check on the consistency of the settings confused me a little. Even if
> simple, I would have put it in a separate patch for better understanding.
> The rest looks good.
> 
> 
> Thanks,
> 
> C.
> 
> 
> 
>> +    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();
>> +    }
>> +
>> +    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;
>>
> 

On 26.02.2025 18:22, Cédric Le Goater wrote:
> On 2/26/25 11:14, Cédric Le Goater wrote:
>> On 2/19/25 21:34, Maciej S. Szmigiero wrote:
>>> From: "Maciej S. Szmigiero" <maciej.szmigiero@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.szmigiero@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;
>>> +    }
>> This check on the consistency of the settings confused me a little. Even if
>> simple, I would have put it in a separate patch for better understanding.
>> The rest looks good.
> 
> now that I have reached patch 31 I understand better. I would put the
> check above in patch 31 and simply return true for now.
> 

Moved it to that patch then ("Add x-migration-multifd-transfer VFIO property").

> Thanks,
> 
> C.
> 

Thanks,
Maciej