Gather all VFIO migration related declarations into "migration.h" to
reduce exposure of VFIO internals in "hw/vfio/vfio-common.h".

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/vfio/migration.h           | 72 +++++++++++++++++++++++++++++++++++
 include/hw/vfio/vfio-common.h | 54 +-------------------------
 hw/vfio/common.c              | 17 +--------
 hw/vfio/iommufd.c             |  1 +
 hw/vfio/migration-multifd.c   |  1 +
 hw/vfio/migration.c           | 17 +++++++++
 hw/vfio/pci.c                 |  1 +
 7 files changed, 94 insertions(+), 69 deletions(-)
 create mode 100644 hw/vfio/migration.h

diff --git a/hw/vfio/migration.h b/hw/vfio/migration.h
new file mode 100644
index 0000000000000000000000000000000000000000..7ad2141d06a7c97f034db908f9ce19fd06f415b9
--- /dev/null
+++ b/hw/vfio/migration.h
@@ -0,0 +1,72 @@
+/*
+ * VFIO migration
+ *
+ * Copyright Red Hat, Inc. 2025
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_VFIO_MIGRATION_H
+#define HW_VFIO_MIGRATION_H
+
+#ifdef CONFIG_LINUX
+#include <linux/vfio.h>
+#endif
+
+#include "qemu/notify.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)
+#define VFIO_MIG_FLAG_DEV_INIT_DATA_SENT (0xffffffffef100005ULL)
+
+typedef struct VFIODevice VFIODevice;
+typedef struct VFIOMultifd VFIOMultifd;
+
+typedef struct VFIOMigration {
+    struct VFIODevice *vbasedev;
+    VMChangeStateEntry *vm_state;
+    NotifierWithReturn migration_state;
+    uint32_t device_state;
+    int data_fd;
+    void *data_buffer;
+    size_t data_buffer_size;
+    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;
+    bool event_precopy_empty_hit;
+} VFIOMigration;
+
+
+bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
+void vfio_migration_exit(VFIODevice *vbasedev);
+bool vfio_device_state_is_running(VFIODevice *vbasedev);
+bool vfio_device_state_is_precopy(VFIODevice *vbasedev);
+int vfio_save_device_config_state(QEMUFile *f, void *opaque, Error **errp);
+int vfio_load_device_config_state(QEMUFile *f, void *opaque);
+
+#ifdef CONFIG_LINUX
+int vfio_migration_set_state(VFIODevice *vbasedev,
+                             enum vfio_device_mig_state new_state,
+                             enum vfio_device_mig_state recover_state,
+                             Error **errp);
+#endif
+
+#endif /* HW_VFIO_MIGRATION_H */
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 0aae88131cffda1a90b8ccd0224387133c0fa83a..799e12d43747addbf444c15052f629b65978322f 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -23,7 +23,6 @@
 
 #include "exec/memory.h"
 #include "qemu/queue.h"
-#include "qemu/notify.h"
 #include "ui/console.h"
 #include "hw/display/ramfb.h"
 #ifdef CONFIG_LINUX
@@ -36,23 +35,6 @@
 
 #define VFIO_MSG_PREFIX "vfio %s: "
 
-/*
- * 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)
-#define VFIO_MIG_FLAG_DEV_INIT_DATA_SENT (0xffffffffef100005ULL)
-
 enum {
     VFIO_DEVICE_TYPE_PCI = 0,
     VFIO_DEVICE_TYPE_PLATFORM = 1,
@@ -78,27 +60,6 @@ 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;
-    NotifierWithReturn migration_state;
-    uint32_t device_state;
-    int data_fd;
-    void *data_buffer;
-    size_t data_buffer_size;
-    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;
-    bool event_precopy_empty_hit;
-} VFIOMigration;
-
 struct VFIOGroup;
 
 typedef struct VFIOContainer {
@@ -136,6 +97,7 @@ typedef struct VFIOIOMMUFDContainer {
 OBJECT_DECLARE_SIMPLE_TYPE(VFIOIOMMUFDContainer, VFIO_IOMMU_IOMMUFD);
 
 typedef struct VFIODeviceOps VFIODeviceOps;
+typedef struct VFIOMigration VFIOMigration;
 
 typedef struct VFIODevice {
     QLIST_ENTRY(VFIODevice) next;
@@ -290,12 +252,6 @@ extern VFIODeviceList vfio_device_list;
 extern const MemoryListener vfio_memory_listener;
 extern int vfio_kvm_device_fd;
 
-bool vfio_device_state_is_running(VFIODevice *vbasedev);
-bool vfio_device_state_is_precopy(VFIODevice *vbasedev);
-
-int vfio_save_device_config_state(QEMUFile *f, void *opaque, Error **errp);
-int vfio_load_device_config_state(QEMUFile *f, void *opaque);
-
 #ifdef CONFIG_LINUX
 int vfio_get_region_info(VFIODevice *vbasedev, int index,
                          struct vfio_region_info **info);
@@ -310,16 +266,8 @@ struct vfio_info_cap_header *
 vfio_get_device_info_cap(struct vfio_device_info *info, uint16_t id);
 struct vfio_info_cap_header *
 vfio_get_cap(void *ptr, uint32_t cap_offset, uint16_t id);
-
-int vfio_migration_set_state(VFIODevice *vbasedev,
-                             enum vfio_device_mig_state new_state,
-                             enum vfio_device_mig_state recover_state,
-                             Error **errp);
 #endif
 
-bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
-void vfio_migration_exit(VFIODevice *vbasedev);
-
 int vfio_bitmap_alloc(VFIOBitmap *vbmap, hwaddr size);
 bool vfio_devices_all_dirty_tracking_started(
     const VFIOContainerBase *bcontainer);
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 0b9b071cd0490867bb6aa4ceb261350ccd6e1125..0e3746eddd1c08e98bf57a59d542e158487d346e 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -44,6 +44,7 @@
 #include "migration/qemu-file.h"
 #include "system/tcg.h"
 #include "system/tpm.h"
+#include "migration.h"
 
 VFIODeviceList vfio_device_list =
     QLIST_HEAD_INITIALIZER(vfio_device_list);
@@ -72,22 +73,6 @@ static void vfio_set_migration_error(int ret)
     }
 }
 
-bool vfio_device_state_is_running(VFIODevice *vbasedev)
-{
-    VFIOMigration *migration = vbasedev->migration;
-
-    return migration->device_state == VFIO_DEVICE_STATE_RUNNING ||
-           migration->device_state == VFIO_DEVICE_STATE_RUNNING_P2P;
-}
-
-bool vfio_device_state_is_precopy(VFIODevice *vbasedev)
-{
-    VFIOMigration *migration = vbasedev->migration;
-
-    return migration->device_state == VFIO_DEVICE_STATE_PRE_COPY ||
-           migration->device_state == VFIO_DEVICE_STATE_PRE_COPY_P2P;
-}
-
 static bool vfio_devices_all_device_dirty_tracking_started(
     const VFIOContainerBase *bcontainer)
 {
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 42c8412bbf50724dddb43f9b19a3aa40c8bc311d..2fb2a01ec6d29dbc284cfd9830c24e78ce560dd0 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -25,6 +25,7 @@
 #include "qemu/cutils.h"
 #include "qemu/chardev_open.h"
 #include "pci.h"
+#include "migration.h"
 
 static int iommufd_cdev_map(const VFIOContainerBase *bcontainer, hwaddr iova,
                             ram_addr_t size, void *vaddr, bool readonly)
diff --git a/hw/vfio/migration-multifd.c b/hw/vfio/migration-multifd.c
index fe84735ec2c7bd085820d25c06be558761fbe0d5..285f9a9a18dd8f0847ac5ac3fdaa304779a2d0db 100644
--- a/hw/vfio/migration-multifd.c
+++ b/hw/vfio/migration-multifd.c
@@ -22,6 +22,7 @@
 #include "io/channel-buffer.h"
 #include "migration/qemu-file.h"
 #include "migration-multifd.h"
+#include "migration.h"
 #include "trace.h"
 
 #define VFIO_DEVICE_STATE_CONFIG_STATE (1)
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index a1ba993ce549fce0d2a9a60ba07d4782c87c0c09..46c4cfecce25ba1146a1d8f2de0d7c51425afe8e 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -31,6 +31,7 @@
 #include "pci.h"
 #include "trace.h"
 #include "hw/hw.h"
+#include "migration.h"
 
 /*
  * This is an arbitrary size based on migration of mlx5 devices, where typically
@@ -1222,3 +1223,19 @@ void vfio_migration_exit(VFIODevice *vbasedev)
 
     migrate_del_blocker(&vbasedev->migration_blocker);
 }
+
+bool vfio_device_state_is_running(VFIODevice *vbasedev)
+{
+    VFIOMigration *migration = vbasedev->migration;
+
+    return migration->device_state == VFIO_DEVICE_STATE_RUNNING ||
+           migration->device_state == VFIO_DEVICE_STATE_RUNNING_P2P;
+}
+
+bool vfio_device_state_is_precopy(VFIODevice *vbasedev)
+{
+    VFIOMigration *migration = vbasedev->migration;
+
+    return migration->device_state == VFIO_DEVICE_STATE_PRE_COPY ||
+           migration->device_state == VFIO_DEVICE_STATE_PRE_COPY_P2P;
+}
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 7f1532fbed9aed2eae2c98f6fd79a9056ff1e84f..3612f6fe7d0864fe3789f4ea221da01ef87d0664 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -44,6 +44,7 @@
 #include "migration/blocker.h"
 #include "migration/qemu-file.h"
 #include "system/iommufd.h"
+#include "migration.h"
 
 #define TYPE_VFIO_PCI_NOHOTPLUG "vfio-pci-nohotplug"
 
-- 
2.48.1

On Tue, Mar 18, 2025 at 10:53:49AM +0100, Cédric Le Goater wrote:

> Gather all VFIO migration related declarations into "migration.h" to
> reduce exposure of VFIO internals in "hw/vfio/vfio-common.h".
> 
> Signed-off-by: Cédric Le Goater <clg@redhat.com>

LGTM modulo the other's comments.

regards
john

On 18/03/2025 11:53, Cédric Le Goater wrote:
> External email: Use caution opening links or attachments
>
>
> Gather all VFIO migration related declarations into "migration.h" to
> reduce exposure of VFIO internals in "hw/vfio/vfio-common.h".

Reviewed-by: Avihai Horon <avihaih@nvidia.com>

Two nits below.

>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>   hw/vfio/migration.h           | 72 +++++++++++++++++++++++++++++++++++
>   include/hw/vfio/vfio-common.h | 54 +-------------------------
>   hw/vfio/common.c              | 17 +--------
>   hw/vfio/iommufd.c             |  1 +
>   hw/vfio/migration-multifd.c   |  1 +
>   hw/vfio/migration.c           | 17 +++++++++
>   hw/vfio/pci.c                 |  1 +
>   7 files changed, 94 insertions(+), 69 deletions(-)
>   create mode 100644 hw/vfio/migration.h
>
> diff --git a/hw/vfio/migration.h b/hw/vfio/migration.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..7ad2141d06a7c97f034db908f9ce19fd06f415b9
> --- /dev/null
> +++ b/hw/vfio/migration.h
> @@ -0,0 +1,72 @@
> +/*
> + * VFIO migration
> + *
> + * Copyright Red Hat, Inc. 2025
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef HW_VFIO_MIGRATION_H
> +#define HW_VFIO_MIGRATION_H
> +
> +#ifdef CONFIG_LINUX
> +#include <linux/vfio.h>
> +#endif
> +
> +#include "qemu/notify.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)
> +#define VFIO_MIG_FLAG_DEV_INIT_DATA_SENT (0xffffffffef100005ULL)
> +
> +typedef struct VFIODevice VFIODevice;
> +typedef struct VFIOMultifd VFIOMultifd;
> +
> +typedef struct VFIOMigration {
> +    struct VFIODevice *vbasedev;
> +    VMChangeStateEntry *vm_state;
> +    NotifierWithReturn migration_state;
> +    uint32_t device_state;
> +    int data_fd;
> +    void *data_buffer;
> +    size_t data_buffer_size;
> +    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;
> +    bool event_precopy_empty_hit;
> +} VFIOMigration;
> +
> +

Extra line break.

> +bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
> +void vfio_migration_exit(VFIODevice *vbasedev);
> +bool vfio_device_state_is_running(VFIODevice *vbasedev);
> +bool vfio_device_state_is_precopy(VFIODevice *vbasedev);
> +int vfio_save_device_config_state(QEMUFile *f, void *opaque, Error **errp);
> +int vfio_load_device_config_state(QEMUFile *f, void *opaque);
> +
> +#ifdef CONFIG_LINUX
> +int vfio_migration_set_state(VFIODevice *vbasedev,
> +                             enum vfio_device_mig_state new_state,
> +                             enum vfio_device_mig_state recover_state,
> +                             Error **errp);
> +#endif
> +
> +#endif /* HW_VFIO_MIGRATION_H */
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 0aae88131cffda1a90b8ccd0224387133c0fa83a..799e12d43747addbf444c15052f629b65978322f 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -23,7 +23,6 @@
>
>   #include "exec/memory.h"
>   #include "qemu/queue.h"
> -#include "qemu/notify.h"
>   #include "ui/console.h"
>   #include "hw/display/ramfb.h"
>   #ifdef CONFIG_LINUX
> @@ -36,23 +35,6 @@
>
>   #define VFIO_MSG_PREFIX "vfio %s: "
>
> -/*
> - * 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)
> -#define VFIO_MIG_FLAG_DEV_INIT_DATA_SENT (0xffffffffef100005ULL)
> -
>   enum {
>       VFIO_DEVICE_TYPE_PCI = 0,
>       VFIO_DEVICE_TYPE_PLATFORM = 1,
> @@ -78,27 +60,6 @@ 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;
> -    NotifierWithReturn migration_state;
> -    uint32_t device_state;
> -    int data_fd;
> -    void *data_buffer;
> -    size_t data_buffer_size;
> -    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;
> -    bool event_precopy_empty_hit;
> -} VFIOMigration;
> -
>   struct VFIOGroup;
>
>   typedef struct VFIOContainer {
> @@ -136,6 +97,7 @@ typedef struct VFIOIOMMUFDContainer {
>   OBJECT_DECLARE_SIMPLE_TYPE(VFIOIOMMUFDContainer, VFIO_IOMMU_IOMMUFD);
>
>   typedef struct VFIODeviceOps VFIODeviceOps;
> +typedef struct VFIOMigration VFIOMigration;
>
>   typedef struct VFIODevice {
>       QLIST_ENTRY(VFIODevice) next;
> @@ -290,12 +252,6 @@ extern VFIODeviceList vfio_device_list;
>   extern const MemoryListener vfio_memory_listener;
>   extern int vfio_kvm_device_fd;
>
> -bool vfio_device_state_is_running(VFIODevice *vbasedev);
> -bool vfio_device_state_is_precopy(VFIODevice *vbasedev);
> -
> -int vfio_save_device_config_state(QEMUFile *f, void *opaque, Error **errp);
> -int vfio_load_device_config_state(QEMUFile *f, void *opaque);
> -
>   #ifdef CONFIG_LINUX
>   int vfio_get_region_info(VFIODevice *vbasedev, int index,
>                            struct vfio_region_info **info);
> @@ -310,16 +266,8 @@ struct vfio_info_cap_header *
>   vfio_get_device_info_cap(struct vfio_device_info *info, uint16_t id);
>   struct vfio_info_cap_header *
>   vfio_get_cap(void *ptr, uint32_t cap_offset, uint16_t id);
> -
> -int vfio_migration_set_state(VFIODevice *vbasedev,
> -                             enum vfio_device_mig_state new_state,
> -                             enum vfio_device_mig_state recover_state,
> -                             Error **errp);
>   #endif
>
> -bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
> -void vfio_migration_exit(VFIODevice *vbasedev);
> -
>   int vfio_bitmap_alloc(VFIOBitmap *vbmap, hwaddr size);
>   bool vfio_devices_all_dirty_tracking_started(
>       const VFIOContainerBase *bcontainer);
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 0b9b071cd0490867bb6aa4ceb261350ccd6e1125..0e3746eddd1c08e98bf57a59d542e158487d346e 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -44,6 +44,7 @@
>   #include "migration/qemu-file.h"
>   #include "system/tcg.h"
>   #include "system/tpm.h"
> +#include "migration.h"
>
>   VFIODeviceList vfio_device_list =
>       QLIST_HEAD_INITIALIZER(vfio_device_list);
> @@ -72,22 +73,6 @@ static void vfio_set_migration_error(int ret)
>       }
>   }
>
> -bool vfio_device_state_is_running(VFIODevice *vbasedev)
> -{
> -    VFIOMigration *migration = vbasedev->migration;
> -
> -    return migration->device_state == VFIO_DEVICE_STATE_RUNNING ||
> -           migration->device_state == VFIO_DEVICE_STATE_RUNNING_P2P;
> -}
> -
> -bool vfio_device_state_is_precopy(VFIODevice *vbasedev)
> -{
> -    VFIOMigration *migration = vbasedev->migration;
> -
> -    return migration->device_state == VFIO_DEVICE_STATE_PRE_COPY ||
> -           migration->device_state == VFIO_DEVICE_STATE_PRE_COPY_P2P;
> -}
> -
>   static bool vfio_devices_all_device_dirty_tracking_started(
>       const VFIOContainerBase *bcontainer)
>   {
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 42c8412bbf50724dddb43f9b19a3aa40c8bc311d..2fb2a01ec6d29dbc284cfd9830c24e78ce560dd0 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -25,6 +25,7 @@
>   #include "qemu/cutils.h"
>   #include "qemu/chardev_open.h"
>   #include "pci.h"
> +#include "migration.h"

I think iommufd.c doesn't need migration.h.

Thanks.

>
>   static int iommufd_cdev_map(const VFIOContainerBase *bcontainer, hwaddr iova,
>                               ram_addr_t size, void *vaddr, bool readonly)
> diff --git a/hw/vfio/migration-multifd.c b/hw/vfio/migration-multifd.c
> index fe84735ec2c7bd085820d25c06be558761fbe0d5..285f9a9a18dd8f0847ac5ac3fdaa304779a2d0db 100644
> --- a/hw/vfio/migration-multifd.c
> +++ b/hw/vfio/migration-multifd.c
> @@ -22,6 +22,7 @@
>   #include "io/channel-buffer.h"
>   #include "migration/qemu-file.h"
>   #include "migration-multifd.h"
> +#include "migration.h"
>   #include "trace.h"
>
>   #define VFIO_DEVICE_STATE_CONFIG_STATE (1)
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index a1ba993ce549fce0d2a9a60ba07d4782c87c0c09..46c4cfecce25ba1146a1d8f2de0d7c51425afe8e 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -31,6 +31,7 @@
>   #include "pci.h"
>   #include "trace.h"
>   #include "hw/hw.h"
> +#include "migration.h"
>
>   /*
>    * This is an arbitrary size based on migration of mlx5 devices, where typically
> @@ -1222,3 +1223,19 @@ void vfio_migration_exit(VFIODevice *vbasedev)
>
>       migrate_del_blocker(&vbasedev->migration_blocker);
>   }
> +
> +bool vfio_device_state_is_running(VFIODevice *vbasedev)
> +{
> +    VFIOMigration *migration = vbasedev->migration;
> +
> +    return migration->device_state == VFIO_DEVICE_STATE_RUNNING ||
> +           migration->device_state == VFIO_DEVICE_STATE_RUNNING_P2P;
> +}
> +
> +bool vfio_device_state_is_precopy(VFIODevice *vbasedev)
> +{
> +    VFIOMigration *migration = vbasedev->migration;
> +
> +    return migration->device_state == VFIO_DEVICE_STATE_PRE_COPY ||
> +           migration->device_state == VFIO_DEVICE_STATE_PRE_COPY_P2P;
> +}
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 7f1532fbed9aed2eae2c98f6fd79a9056ff1e84f..3612f6fe7d0864fe3789f4ea221da01ef87d0664 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -44,6 +44,7 @@
>   #include "migration/blocker.h"
>   #include "migration/qemu-file.h"
>   #include "system/iommufd.h"
> +#include "migration.h"
>
>   #define TYPE_VFIO_PCI_NOHOTPLUG "vfio-pci-nohotplug"
>
> --
> 2.48.1
>

On 3/19/25 15:05, Avihai Horon wrote:
> 
> On 18/03/2025 11:53, Cédric Le Goater wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> Gather all VFIO migration related declarations into "migration.h" to
>> reduce exposure of VFIO internals in "hw/vfio/vfio-common.h".
> 
> Reviewed-by: Avihai Horon <avihaih@nvidia.com>
> 
> Two nits below.


I will add a third. Moving the vfio_device_state_is_* routines could be
done in separate change.



> 
>>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>   hw/vfio/migration.h           | 72 +++++++++++++++++++++++++++++++++++
>>   include/hw/vfio/vfio-common.h | 54 +-------------------------
>>   hw/vfio/common.c              | 17 +--------
>>   hw/vfio/iommufd.c             |  1 +
>>   hw/vfio/migration-multifd.c   |  1 +
>>   hw/vfio/migration.c           | 17 +++++++++
>>   hw/vfio/pci.c                 |  1 +
>>   7 files changed, 94 insertions(+), 69 deletions(-)
>>   create mode 100644 hw/vfio/migration.h
>>
>> diff --git a/hw/vfio/migration.h b/hw/vfio/migration.h
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..7ad2141d06a7c97f034db908f9ce19fd06f415b9
>> --- /dev/null
>> +++ b/hw/vfio/migration.h
>> @@ -0,0 +1,72 @@
>> +/*
>> + * VFIO migration
>> + *
>> + * Copyright Red Hat, Inc. 2025
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#ifndef HW_VFIO_MIGRATION_H
>> +#define HW_VFIO_MIGRATION_H
>> +
>> +#ifdef CONFIG_LINUX
>> +#include <linux/vfio.h>
>> +#endif
>> +
>> +#include "qemu/notify.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)
>> +#define VFIO_MIG_FLAG_DEV_INIT_DATA_SENT (0xffffffffef100005ULL)
>> +
>> +typedef struct VFIODevice VFIODevice;
>> +typedef struct VFIOMultifd VFIOMultifd;
>> +
>> +typedef struct VFIOMigration {
>> +    struct VFIODevice *vbasedev;
>> +    VMChangeStateEntry *vm_state;
>> +    NotifierWithReturn migration_state;
>> +    uint32_t device_state;
>> +    int data_fd;
>> +    void *data_buffer;
>> +    size_t data_buffer_size;
>> +    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;
>> +    bool event_precopy_empty_hit;
>> +} VFIOMigration;
>> +
>> +
> 
> Extra line break.
> 
>> +bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
>> +void vfio_migration_exit(VFIODevice *vbasedev);
>> +bool vfio_device_state_is_running(VFIODevice *vbasedev);
>> +bool vfio_device_state_is_precopy(VFIODevice *vbasedev);
>> +int vfio_save_device_config_state(QEMUFile *f, void *opaque, Error **errp);
>> +int vfio_load_device_config_state(QEMUFile *f, void *opaque);
>> +
>> +#ifdef CONFIG_LINUX
>> +int vfio_migration_set_state(VFIODevice *vbasedev,
>> +                             enum vfio_device_mig_state new_state,
>> +                             enum vfio_device_mig_state recover_state,
>> +                             Error **errp);
>> +#endif
>> +
>> +#endif /* HW_VFIO_MIGRATION_H */
>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>> index 0aae88131cffda1a90b8ccd0224387133c0fa83a..799e12d43747addbf444c15052f629b65978322f 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -23,7 +23,6 @@
>>
>>   #include "exec/memory.h"
>>   #include "qemu/queue.h"
>> -#include "qemu/notify.h"
>>   #include "ui/console.h"
>>   #include "hw/display/ramfb.h"
>>   #ifdef CONFIG_LINUX
>> @@ -36,23 +35,6 @@
>>
>>   #define VFIO_MSG_PREFIX "vfio %s: "
>>
>> -/*
>> - * 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)
>> -#define VFIO_MIG_FLAG_DEV_INIT_DATA_SENT (0xffffffffef100005ULL)
>> -
>>   enum {
>>       VFIO_DEVICE_TYPE_PCI = 0,
>>       VFIO_DEVICE_TYPE_PLATFORM = 1,
>> @@ -78,27 +60,6 @@ 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;
>> -    NotifierWithReturn migration_state;
>> -    uint32_t device_state;
>> -    int data_fd;
>> -    void *data_buffer;
>> -    size_t data_buffer_size;
>> -    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;
>> -    bool event_precopy_empty_hit;
>> -} VFIOMigration;
>> -
>>   struct VFIOGroup;
>>
>>   typedef struct VFIOContainer {
>> @@ -136,6 +97,7 @@ typedef struct VFIOIOMMUFDContainer {
>>   OBJECT_DECLARE_SIMPLE_TYPE(VFIOIOMMUFDContainer, VFIO_IOMMU_IOMMUFD);
>>
>>   typedef struct VFIODeviceOps VFIODeviceOps;
>> +typedef struct VFIOMigration VFIOMigration;
>>
>>   typedef struct VFIODevice {
>>       QLIST_ENTRY(VFIODevice) next;
>> @@ -290,12 +252,6 @@ extern VFIODeviceList vfio_device_list;
>>   extern const MemoryListener vfio_memory_listener;
>>   extern int vfio_kvm_device_fd;
>>
>> -bool vfio_device_state_is_running(VFIODevice *vbasedev);
>> -bool vfio_device_state_is_precopy(VFIODevice *vbasedev);
>> -
>> -int vfio_save_device_config_state(QEMUFile *f, void *opaque, Error **errp);
>> -int vfio_load_device_config_state(QEMUFile *f, void *opaque);
>> -
>>   #ifdef CONFIG_LINUX
>>   int vfio_get_region_info(VFIODevice *vbasedev, int index,
>>                            struct vfio_region_info **info);
>> @@ -310,16 +266,8 @@ struct vfio_info_cap_header *
>>   vfio_get_device_info_cap(struct vfio_device_info *info, uint16_t id);
>>   struct vfio_info_cap_header *
>>   vfio_get_cap(void *ptr, uint32_t cap_offset, uint16_t id);
>> -
>> -int vfio_migration_set_state(VFIODevice *vbasedev,
>> -                             enum vfio_device_mig_state new_state,
>> -                             enum vfio_device_mig_state recover_state,
>> -                             Error **errp);
>>   #endif
>>
>> -bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
>> -void vfio_migration_exit(VFIODevice *vbasedev);
>> -
>>   int vfio_bitmap_alloc(VFIOBitmap *vbmap, hwaddr size);
>>   bool vfio_devices_all_dirty_tracking_started(
>>       const VFIOContainerBase *bcontainer);
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index 0b9b071cd0490867bb6aa4ceb261350ccd6e1125..0e3746eddd1c08e98bf57a59d542e158487d346e 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -44,6 +44,7 @@
>>   #include "migration/qemu-file.h"
>>   #include "system/tcg.h"
>>   #include "system/tpm.h"
>> +#include "migration.h"
>>
>>   VFIODeviceList vfio_device_list =
>>       QLIST_HEAD_INITIALIZER(vfio_device_list);
>> @@ -72,22 +73,6 @@ static void vfio_set_migration_error(int ret)
>>       }
>>   }
>>
>> -bool vfio_device_state_is_running(VFIODevice *vbasedev)
>> -{
>> -    VFIOMigration *migration = vbasedev->migration;
>> -
>> -    return migration->device_state == VFIO_DEVICE_STATE_RUNNING ||
>> -           migration->device_state == VFIO_DEVICE_STATE_RUNNING_P2P;
>> -}
>> -
>> -bool vfio_device_state_is_precopy(VFIODevice *vbasedev)
>> -{
>> -    VFIOMigration *migration = vbasedev->migration;
>> -
>> -    return migration->device_state == VFIO_DEVICE_STATE_PRE_COPY ||
>> -           migration->device_state == VFIO_DEVICE_STATE_PRE_COPY_P2P;
>> -}
>> -
>>   static bool vfio_devices_all_device_dirty_tracking_started(
>>       const VFIOContainerBase *bcontainer)
>>   {
>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>> index 42c8412bbf50724dddb43f9b19a3aa40c8bc311d..2fb2a01ec6d29dbc284cfd9830c24e78ce560dd0 100644
>> --- a/hw/vfio/iommufd.c
>> +++ b/hw/vfio/iommufd.c
>> @@ -25,6 +25,7 @@
>>   #include "qemu/cutils.h"
>>   #include "qemu/chardev_open.h"
>>   #include "pci.h"
>> +#include "migration.h"
> 
> I think iommufd.c doesn't need migration.h.

Indeed !


Thanks,

C.


> 
> Thanks.
> 
>>
>>   static int iommufd_cdev_map(const VFIOContainerBase *bcontainer, hwaddr iova,
>>                               ram_addr_t size, void *vaddr, bool readonly)
>> diff --git a/hw/vfio/migration-multifd.c b/hw/vfio/migration-multifd.c
>> index fe84735ec2c7bd085820d25c06be558761fbe0d5..285f9a9a18dd8f0847ac5ac3fdaa304779a2d0db 100644
>> --- a/hw/vfio/migration-multifd.c
>> +++ b/hw/vfio/migration-multifd.c
>> @@ -22,6 +22,7 @@
>>   #include "io/channel-buffer.h"
>>   #include "migration/qemu-file.h"
>>   #include "migration-multifd.h"
>> +#include "migration.h"
>>   #include "trace.h"
>>
>>   #define VFIO_DEVICE_STATE_CONFIG_STATE (1)
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index a1ba993ce549fce0d2a9a60ba07d4782c87c0c09..46c4cfecce25ba1146a1d8f2de0d7c51425afe8e 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
>> @@ -31,6 +31,7 @@
>>   #include "pci.h"
>>   #include "trace.h"
>>   #include "hw/hw.h"
>> +#include "migration.h"
>>
>>   /*
>>    * This is an arbitrary size based on migration of mlx5 devices, where typically
>> @@ -1222,3 +1223,19 @@ void vfio_migration_exit(VFIODevice *vbasedev)
>>
>>       migrate_del_blocker(&vbasedev->migration_blocker);
>>   }
>> +
>> +bool vfio_device_state_is_running(VFIODevice *vbasedev)
>> +{
>> +    VFIOMigration *migration = vbasedev->migration;
>> +
>> +    return migration->device_state == VFIO_DEVICE_STATE_RUNNING ||
>> +           migration->device_state == VFIO_DEVICE_STATE_RUNNING_P2P;
>> +}
>> +
>> +bool vfio_device_state_is_precopy(VFIODevice *vbasedev)
>> +{
>> +    VFIOMigration *migration = vbasedev->migration;
>> +
>> +    return migration->device_state == VFIO_DEVICE_STATE_PRE_COPY ||
>> +           migration->device_state == VFIO_DEVICE_STATE_PRE_COPY_P2P;
>> +}
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 7f1532fbed9aed2eae2c98f6fd79a9056ff1e84f..3612f6fe7d0864fe3789f4ea221da01ef87d0664 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -44,6 +44,7 @@
>>   #include "migration/blocker.h"
>>   #include "migration/qemu-file.h"
>>   #include "system/iommufd.h"
>> +#include "migration.h"
>>
>>   #define TYPE_VFIO_PCI_NOHOTPLUG "vfio-pci-nohotplug"
>>
>> -- 
>> 2.48.1
>>
> 

On Tue, 18 Mar 2025 at 15:26, Cédric Le Goater <clg@redhat.com> wrote:
> Gather all VFIO migration related declarations into "migration.h" to
> reduce exposure of VFIO internals in "hw/vfio/vfio-common.h".
>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>  hw/vfio/migration.h           | 72 +++++++++++++++++++++++++++++++++++
>  include/hw/vfio/vfio-common.h | 54 +-------------------------
>  hw/vfio/common.c              | 17 +--------
>  hw/vfio/iommufd.c             |  1 +
>  hw/vfio/migration-multifd.c   |  1 +
>  hw/vfio/migration.c           | 17 +++++++++
>  hw/vfio/pci.c                 |  1 +
>  7 files changed, 94 insertions(+), 69 deletions(-)
>  create mode 100644 hw/vfio/migration.h
>
> diff --git a/hw/vfio/migration.h b/hw/vfio/migration.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..7ad2141d06a7c97f034db908f9ce19fd06f415b9
> --- /dev/null
> +++ b/hw/vfio/migration.h
> @@ -0,0 +1,72 @@
> +/*
> + * VFIO migration
> + *
> + * Copyright Red Hat, Inc. 2025
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef HW_VFIO_MIGRATION_H
> +#define HW_VFIO_MIGRATION_H
> +
> +#ifdef CONFIG_LINUX
> +#include <linux/vfio.h>
> +#endif
> +
> +#include "qemu/notify.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)
> +#define VFIO_MIG_FLAG_DEV_INIT_DATA_SENT (0xffffffffef100005ULL)
> +
> +typedef struct VFIODevice VFIODevice;
> +typedef struct VFIOMultifd VFIOMultifd;
> +
> +typedef struct VFIOMigration {
> +    struct VFIODevice *vbasedev;
> +    VMChangeStateEntry *vm_state;
> +    NotifierWithReturn migration_state;
> +    uint32_t device_state;
> +    int data_fd;
> +    void *data_buffer;
> +    size_t data_buffer_size;
> +    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;
> +    bool event_precopy_empty_hit;
> +} VFIOMigration;
> +
> +
> +bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
> +void vfio_migration_exit(VFIODevice *vbasedev);
> +bool vfio_device_state_is_running(VFIODevice *vbasedev);
> +bool vfio_device_state_is_precopy(VFIODevice *vbasedev);
> +int vfio_save_device_config_state(QEMUFile *f, void *opaque, Error **errp);
> +int vfio_load_device_config_state(QEMUFile *f, void *opaque);
> +
> +#ifdef CONFIG_LINUX
> +int vfio_migration_set_state(VFIODevice *vbasedev,
> +                             enum vfio_device_mig_state new_state,
> +                             enum vfio_device_mig_state recover_state,
> +                             Error **errp);
> +#endif
> +
> +#endif /* HW_VFIO_MIGRATION_H */
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 0aae88131cffda1a90b8ccd0224387133c0fa83a..799e12d43747addbf444c15052f629b65978322f 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -23,7 +23,6 @@
>
>  #include "exec/memory.h"
>  #include "qemu/queue.h"
> -#include "qemu/notify.h"
>  #include "ui/console.h"
>  #include "hw/display/ramfb.h"
>  #ifdef CONFIG_LINUX
> @@ -36,23 +35,6 @@
>
>  #define VFIO_MSG_PREFIX "vfio %s: "
>
> -/*
> - * 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)
> -#define VFIO_MIG_FLAG_DEV_INIT_DATA_SENT (0xffffffffef100005ULL)
> -
>  enum {
>      VFIO_DEVICE_TYPE_PCI = 0,
>      VFIO_DEVICE_TYPE_PLATFORM = 1,
> @@ -78,27 +60,6 @@ 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;
> -    NotifierWithReturn migration_state;
> -    uint32_t device_state;
> -    int data_fd;
> -    void *data_buffer;
> -    size_t data_buffer_size;
> -    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;
> -    bool event_precopy_empty_hit;
> -} VFIOMigration;
> -
>  struct VFIOGroup;
>
>  typedef struct VFIOContainer {
> @@ -136,6 +97,7 @@ typedef struct VFIOIOMMUFDContainer {
>  OBJECT_DECLARE_SIMPLE_TYPE(VFIOIOMMUFDContainer, VFIO_IOMMU_IOMMUFD);
>
>  typedef struct VFIODeviceOps VFIODeviceOps;
> +typedef struct VFIOMigration VFIOMigration;
>
>  typedef struct VFIODevice {
>      QLIST_ENTRY(VFIODevice) next;
> @@ -290,12 +252,6 @@ extern VFIODeviceList vfio_device_list;
>  extern const MemoryListener vfio_memory_listener;
>  extern int vfio_kvm_device_fd;
>
> -bool vfio_device_state_is_running(VFIODevice *vbasedev);
> -bool vfio_device_state_is_precopy(VFIODevice *vbasedev);
> -
> -int vfio_save_device_config_state(QEMUFile *f, void *opaque, Error **errp);
> -int vfio_load_device_config_state(QEMUFile *f, void *opaque);
> -
>  #ifdef CONFIG_LINUX
>  int vfio_get_region_info(VFIODevice *vbasedev, int index,
>                           struct vfio_region_info **info);
> @@ -310,16 +266,8 @@ struct vfio_info_cap_header *
>  vfio_get_device_info_cap(struct vfio_device_info *info, uint16_t id);
>  struct vfio_info_cap_header *
>  vfio_get_cap(void *ptr, uint32_t cap_offset, uint16_t id);
> -
> -int vfio_migration_set_state(VFIODevice *vbasedev,
> -                             enum vfio_device_mig_state new_state,
> -                             enum vfio_device_mig_state recover_state,
> -                             Error **errp);
>  #endif
>
> -bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
> -void vfio_migration_exit(VFIODevice *vbasedev);
> -
>  int vfio_bitmap_alloc(VFIOBitmap *vbmap, hwaddr size);
>  bool vfio_devices_all_dirty_tracking_started(
>      const VFIOContainerBase *bcontainer);
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 0b9b071cd0490867bb6aa4ceb261350ccd6e1125..0e3746eddd1c08e98bf57a59d542e158487d346e 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -44,6 +44,7 @@
>  #include "migration/qemu-file.h"
>  #include "system/tcg.h"
>  #include "system/tpm.h"
> +#include "migration.h"
>
>  VFIODeviceList vfio_device_list =
>      QLIST_HEAD_INITIALIZER(vfio_device_list);
> @@ -72,22 +73,6 @@ static void vfio_set_migration_error(int ret)
>      }
>  }
>
> -bool vfio_device_state_is_running(VFIODevice *vbasedev)
> -{
> -    VFIOMigration *migration = vbasedev->migration;
> -
> -    return migration->device_state == VFIO_DEVICE_STATE_RUNNING ||
> -           migration->device_state == VFIO_DEVICE_STATE_RUNNING_P2P;
> -}
> -
> -bool vfio_device_state_is_precopy(VFIODevice *vbasedev)
> -{
> -    VFIOMigration *migration = vbasedev->migration;
> -
> -    return migration->device_state == VFIO_DEVICE_STATE_PRE_COPY ||
> -           migration->device_state == VFIO_DEVICE_STATE_PRE_COPY_P2P;
> -}
> -
>  static bool vfio_devices_all_device_dirty_tracking_started(
>      const VFIOContainerBase *bcontainer)
>  {
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 42c8412bbf50724dddb43f9b19a3aa40c8bc311d..2fb2a01ec6d29dbc284cfd9830c24e78ce560dd0 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -25,6 +25,7 @@
>  #include "qemu/cutils.h"
>  #include "qemu/chardev_open.h"
>  #include "pci.h"
> +#include "migration.h"
>
>  static int iommufd_cdev_map(const VFIOContainerBase *bcontainer, hwaddr iova,
>                              ram_addr_t size, void *vaddr, bool readonly)
> diff --git a/hw/vfio/migration-multifd.c b/hw/vfio/migration-multifd.c
> index fe84735ec2c7bd085820d25c06be558761fbe0d5..285f9a9a18dd8f0847ac5ac3fdaa304779a2d0db 100644
> --- a/hw/vfio/migration-multifd.c
> +++ b/hw/vfio/migration-multifd.c
> @@ -22,6 +22,7 @@
>  #include "io/channel-buffer.h"
>  #include "migration/qemu-file.h"
>  #include "migration-multifd.h"
> +#include "migration.h"
>  #include "trace.h"
>
>  #define VFIO_DEVICE_STATE_CONFIG_STATE (1)
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index a1ba993ce549fce0d2a9a60ba07d4782c87c0c09..46c4cfecce25ba1146a1d8f2de0d7c51425afe8e 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -31,6 +31,7 @@
>  #include "pci.h"
>  #include "trace.h"
>  #include "hw/hw.h"
> +#include "migration.h"
>
>  /*
>   * This is an arbitrary size based on migration of mlx5 devices, where typically
> @@ -1222,3 +1223,19 @@ void vfio_migration_exit(VFIODevice *vbasedev)
>
>      migrate_del_blocker(&vbasedev->migration_blocker);
>  }
> +
> +bool vfio_device_state_is_running(VFIODevice *vbasedev)
> +{
> +    VFIOMigration *migration = vbasedev->migration;
> +
> +    return migration->device_state == VFIO_DEVICE_STATE_RUNNING ||
> +           migration->device_state == VFIO_DEVICE_STATE_RUNNING_P2P;
> +}
> +
> +bool vfio_device_state_is_precopy(VFIODevice *vbasedev)
> +{
> +    VFIOMigration *migration = vbasedev->migration;
> +
> +    return migration->device_state == VFIO_DEVICE_STATE_PRE_COPY ||
> +           migration->device_state == VFIO_DEVICE_STATE_PRE_COPY_P2P;
> +}
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 7f1532fbed9aed2eae2c98f6fd79a9056ff1e84f..3612f6fe7d0864fe3789f4ea221da01ef87d0664 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -44,6 +44,7 @@
>  #include "migration/blocker.h"
>  #include "migration/qemu-file.h"
>  #include "system/iommufd.h"
> +#include "migration.h"
>
>  #define TYPE_VFIO_PCI_NOHOTPLUG "vfio-pci-nohotplug"
>

* Change looks okay. The header name 'migration.h' is rather generic,
vfio-migration.h could be better. Looking at - #include "migration.h"
- it is not clear which migration.h is being included. There is also a
migration/migration.h header. But I'll leave that to you. (just
sharing thoughts)

Reviewed by: Prasad Pandit <pjp@fedoraproject.org>

Thank you.
---
  - Prasad

[ .. ]

> * Change looks okay. The header name 'migration.h' is rather generic,
> vfio-migration.h could be better. Looking at - #include "migration.h"
> - it is not clear which migration.h is being included. There is also a
> migration/migration.h header. But I'll leave that to you. (just
> sharing thoughts)

you are right. I am not satisfied with the header file names.

It has been problematic and I would prefer all header files to have
a "vfio-" prefix. Sadly, the gcc command line is making it difficult,
or I misunderstood some parts.

When using :

   #include "hw/vfio/foo.h"

"hw/vfio/foo.h" could refer to either :

   ./hw/vfio/foo.h

or

   ./include/hw/vfio/foo.h

I find it confusing that we can include files from anywhere in
the directory tree :/

May be call it "vfio-migration-internal.h" ?

or use always : #include "hw/vfio/migration.h" (and remove -I .) ?

Or rename the "include/hw/vfio/vfio-migration.h" file introduced
in patch 3 for external migration services to some other name.

Proposals welcome !

Thanks,

C.

On Wed, 19 Mar 2025 at 22:57, Cédric Le Goater <clg@redhat.com> wrote:
> you are right. I am not satisfied with the header file names.
>
> It has been problematic and I would prefer all header files to have
> a "vfio-" prefix. Sadly, the gcc command line is making it difficult,
> or I misunderstood some parts.
>
> When using :
>
>    #include "hw/vfio/foo.h"
>
> "hw/vfio/foo.h" could refer to either :
>
>    ./hw/vfio/foo.h
>
> or
>
>    ./include/hw/vfio/foo.h
>
> I find it confusing that we can include files from anywhere in
> the directory tree :/

* IIUC, gcc(1) picks the _first_ matching header (name) it finds
across listed (with -I) include directories. When the header name is
quoted ("foo.h"), the search begins from the directory where the
source file resides. In the above example, if the command line was: ..
-I ./hw/vfio/  -I ./include/hw/vfio/  then foo.h from './hw/vfio/'
gets used. If the command line was: -I ./include/hw/vfio/  -I
./hw/vfio/  then foo.h from './include/hw/vfio/' gets used.

> May be call it "vfio-migration-internal.h" ?
>
> or use always : #include "hw/vfio/migration.h" (and remove -I .) ?
>
> Or rename the "include/hw/vfio/vfio-migration.h" file introduced
> in patch 3 for external migration services to some other name.

* It helps to have a unique header (file) name across the project. I
don't see any vfio-migration.h in the current git master.

Thank you.
---
  - Prasad