On Tue, 18 Mar 2025 at 15:26, Cédric Le Goater <c...@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 <c...@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 <p...@fedoraproject.org>
Thank you.
---
- Prasad
