diff mbox series

[v28,04/17] vfio: Add migration region initialization and finalize function

Message ID 1603449643-12851-5-git-send-email-kwankhede@nvidia.com (mailing list archive)
State New, archived
Headers show
Series Add migration support for VFIO devices | expand

Commit Message

Kirti Wankhede Oct. 23, 2020, 10:40 a.m. UTC 
Whether the VFIO device supports migration or not is decided based of
migration region query. If migration region query is successful and migration
region initialization is successful then migration is supported else
migration is blocked.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Reviewed-by: Neo Jia <cjia@nvidia.com>
Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/vfio/meson.build           |   1 +
 hw/vfio/migration.c           | 133 ++++++++++++++++++++++++++++++++++++++++++
 hw/vfio/trace-events          |   3 +
 include/hw/vfio/vfio-common.h |   9 +++
 4 files changed, 146 insertions(+)
 create mode 100644 hw/vfio/migration.c

Comments

Cornelia Huck Oct. 23, 2020, 11:24 a.m. UTC | #1 
On Fri, 23 Oct 2020 16:10:30 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> Whether the VFIO device supports migration or not is decided based of
> migration region query. If migration region query is successful and migration
> region initialization is successful then migration is supported else
> migration is blocked.
> 
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Reviewed-by: Neo Jia <cjia@nvidia.com>
> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/vfio/meson.build           |   1 +
>  hw/vfio/migration.c           | 133 ++++++++++++++++++++++++++++++++++++++++++
>  hw/vfio/trace-events          |   3 +
>  include/hw/vfio/vfio-common.h |   9 +++
>  4 files changed, 146 insertions(+)
>  create mode 100644 hw/vfio/migration.c

(...)

> +static int vfio_migration_init(VFIODevice *vbasedev,
> +                               struct vfio_region_info *info)
> +{
> +    int ret;
> +    Object *obj;
> +    VFIOMigration *migration;
> +
> +    if (!vbasedev->ops->vfio_get_object) {
> +        return -EINVAL;
> +    }
> +
> +    obj = vbasedev->ops->vfio_get_object(vbasedev);
> +    if (!obj) {
> +        return -EINVAL;
> +    }
> +
> +    migration = g_new0(VFIOMigration, 1);
> +
> +    ret = vfio_region_setup(obj, vbasedev, &migration->region,
> +                            info->index, "migration");
> +    if (ret) {
> +        error_report("%s: Failed to setup VFIO migration region %d: %s",
> +                     vbasedev->name, info->index, strerror(-ret));
> +        goto err;
> +    }
> +
> +    vbasedev->migration = migration;
> +
> +    if (!migration->region.size) {
> +        error_report("%s: Invalid zero-sized of VFIO migration region %d",

s/of //

> +                     vbasedev->name, info->index);
> +        ret = -EINVAL;
> +        goto err;
> +    }
> +    return 0;
> +
> +err:
> +    vfio_migration_region_exit(vbasedev);
> +    g_free(migration);
> +    vbasedev->migration = NULL;
> +    return ret;
> +}

(...)

> +void vfio_migration_finalize(VFIODevice *vbasedev)
> +{
> +    VFIOMigration *migration = vbasedev->migration;

I don't think you need this variable?

> +
> +    if (migration) {
> +        vfio_migration_region_exit(vbasedev);
> +        g_free(vbasedev->migration);
> +        vbasedev->migration = NULL;
> +    }
> +
> +    if (vbasedev->migration_blocker) {
> +        migrate_del_blocker(vbasedev->migration_blocker);
> +        error_free(vbasedev->migration_blocker);
> +        vbasedev->migration_blocker = NULL;
> +    }
> +}

(...)
Alex Williamson Oct. 23, 2020, 4:52 p.m. UTC | #2 
On Fri, 23 Oct 2020 16:10:30 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> Whether the VFIO device supports migration or not is decided based of
> migration region query. If migration region query is successful and migration
> region initialization is successful then migration is supported else
> migration is blocked.
> 
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Reviewed-by: Neo Jia <cjia@nvidia.com>
> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/vfio/meson.build           |   1 +
>  hw/vfio/migration.c           | 133 ++++++++++++++++++++++++++++++++++++++++++
>  hw/vfio/trace-events          |   3 +
>  include/hw/vfio/vfio-common.h |   9 +++
>  4 files changed, 146 insertions(+)
>  create mode 100644 hw/vfio/migration.c
> 
> diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
> index 37efa74018bc..da9af297a0c5 100644
> --- a/hw/vfio/meson.build
> +++ b/hw/vfio/meson.build
> @@ -2,6 +2,7 @@ vfio_ss = ss.source_set()
>  vfio_ss.add(files(
>    'common.c',
>    'spapr.c',
> +  'migration.c',
>  ))
>  vfio_ss.add(when: 'CONFIG_VFIO_PCI', if_true: files(
>    'display.c',
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> new file mode 100644
> index 000000000000..bbe6e0b7a6cc
> --- /dev/null
> +++ b/hw/vfio/migration.c
> @@ -0,0 +1,133 @@
> +/*
> + * Migration support for VFIO devices
> + *
> + * Copyright NVIDIA, Inc. 2020
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include <linux/vfio.h>
> +
> +#include "hw/vfio/vfio-common.h"
> +#include "cpu.h"
> +#include "migration/migration.h"
> +#include "migration/qemu-file.h"
> +#include "migration/register.h"
> +#include "migration/blocker.h"
> +#include "migration/misc.h"
> +#include "qapi/error.h"
> +#include "exec/ramlist.h"
> +#include "exec/ram_addr.h"
> +#include "pci.h"
> +#include "trace.h"
> +
> +static void vfio_migration_region_exit(VFIODevice *vbasedev)
> +{
> +    VFIOMigration *migration = vbasedev->migration;
> +
> +    if (!migration) {
> +        return;
> +    }
> +
> +    vfio_region_exit(&migration->region);
> +    vfio_region_finalize(&migration->region);

I think it would make sense to also:

g_free(migration);
vbasedev->migration = NULL;

here as well so the callers don't need to.  Not worth a re-spin itself,
maybe a follow-up if there's no other reason for a re-spin.  Thanks,

Alex

> +}
> +
> +static int vfio_migration_init(VFIODevice *vbasedev,
> +                               struct vfio_region_info *info)
> +{
> +    int ret;
> +    Object *obj;
> +    VFIOMigration *migration;
> +
> +    if (!vbasedev->ops->vfio_get_object) {
> +        return -EINVAL;
> +    }
> +
> +    obj = vbasedev->ops->vfio_get_object(vbasedev);
> +    if (!obj) {
> +        return -EINVAL;
> +    }
> +
> +    migration = g_new0(VFIOMigration, 1);
> +
> +    ret = vfio_region_setup(obj, vbasedev, &migration->region,
> +                            info->index, "migration");
> +    if (ret) {
> +        error_report("%s: Failed to setup VFIO migration region %d: %s",
> +                     vbasedev->name, info->index, strerror(-ret));
> +        goto err;
> +    }
> +
> +    vbasedev->migration = migration;
> +
> +    if (!migration->region.size) {
> +        error_report("%s: Invalid zero-sized of VFIO migration region %d",
> +                     vbasedev->name, info->index);
> +        ret = -EINVAL;
> +        goto err;
> +    }
> +    return 0;
> +
> +err:
> +    vfio_migration_region_exit(vbasedev);
> +    g_free(migration);
> +    vbasedev->migration = NULL;
> +    return ret;
> +}
> +
> +/* ---------------------------------------------------------------------- */
> +
> +int vfio_migration_probe(VFIODevice *vbasedev, Error **errp)
> +{
> +    struct vfio_region_info *info = NULL;
> +    Error *local_err = NULL;
> +    int ret;
> +
> +    ret = vfio_get_dev_region_info(vbasedev, VFIO_REGION_TYPE_MIGRATION,
> +                                   VFIO_REGION_SUBTYPE_MIGRATION, &info);
> +    if (ret) {
> +        goto add_blocker;
> +    }
> +
> +    ret = vfio_migration_init(vbasedev, info);
> +    if (ret) {
> +        goto add_blocker;
> +    }
> +
> +    g_free(info);
> +    trace_vfio_migration_probe(vbasedev->name, info->index);
> +    return 0;
> +
> +add_blocker:
> +    error_setg(&vbasedev->migration_blocker,
> +               "VFIO device doesn't support migration");
> +    g_free(info);
> +
> +    ret = migrate_add_blocker(vbasedev->migration_blocker, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        error_free(vbasedev->migration_blocker);
> +        vbasedev->migration_blocker = NULL;
> +    }
> +    return ret;
> +}
> +
> +void vfio_migration_finalize(VFIODevice *vbasedev)
> +{
> +    VFIOMigration *migration = vbasedev->migration;
> +
> +    if (migration) {
> +        vfio_migration_region_exit(vbasedev);
> +        g_free(vbasedev->migration);
> +        vbasedev->migration = NULL;
> +    }
> +
> +    if (vbasedev->migration_blocker) {
> +        migrate_del_blocker(vbasedev->migration_blocker);
> +        error_free(vbasedev->migration_blocker);
> +        vbasedev->migration_blocker = NULL;
> +    }
> +}
> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
> index a0c7b49a2ebc..9ced5ec6277c 100644
> --- a/hw/vfio/trace-events
> +++ b/hw/vfio/trace-events
> @@ -145,3 +145,6 @@ vfio_display_edid_link_up(void) ""
>  vfio_display_edid_link_down(void) ""
>  vfio_display_edid_update(uint32_t prefx, uint32_t prefy) "%ux%u"
>  vfio_display_edid_write_error(void) ""
> +
> +# migration.c
> +vfio_migration_probe(const char *name, uint32_t index) " (%s) Region %d"
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index ba6169cd926e..8275c4c68f45 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -57,6 +57,10 @@ typedef struct VFIORegion {
>      uint8_t nr; /* cache the region number for debug */
>  } VFIORegion;
>  
> +typedef struct VFIOMigration {
> +    VFIORegion region;
> +} VFIOMigration;
> +
>  typedef struct VFIOAddressSpace {
>      AddressSpace *as;
>      QLIST_HEAD(, VFIOContainer) containers;
> @@ -113,6 +117,8 @@ typedef struct VFIODevice {
>      unsigned int num_irqs;
>      unsigned int num_regions;
>      unsigned int flags;
> +    VFIOMigration *migration;
> +    Error *migration_blocker;
>  } VFIODevice;
>  
>  struct VFIODeviceOps {
> @@ -204,4 +210,7 @@ int vfio_spapr_create_window(VFIOContainer *container,
>  int vfio_spapr_remove_window(VFIOContainer *container,
>                               hwaddr offset_within_address_space);
>  
> +int vfio_migration_probe(VFIODevice *vbasedev, Error **errp);
> +void vfio_migration_finalize(VFIODevice *vbasedev);
> +
>  #endif /* HW_VFIO_VFIO_COMMON_H */
Kirti Wankhede Oct. 24, 2020, 9:39 a.m. UTC | #3 
On 10/23/2020 10:22 PM, Alex Williamson wrote:
> On Fri, 23 Oct 2020 16:10:30 +0530
> Kirti Wankhede <kwankhede@nvidia.com> wrote:
> 
>> Whether the VFIO device supports migration or not is decided based of
>> migration region query. If migration region query is successful and migration
>> region initialization is successful then migration is supported else
>> migration is blocked.
>>
>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
>> Reviewed-by: Neo Jia <cjia@nvidia.com>
>> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> ---
>>   hw/vfio/meson.build           |   1 +
>>   hw/vfio/migration.c           | 133 ++++++++++++++++++++++++++++++++++++++++++
>>   hw/vfio/trace-events          |   3 +
>>   include/hw/vfio/vfio-common.h |   9 +++
>>   4 files changed, 146 insertions(+)
>>   create mode 100644 hw/vfio/migration.c
>>
>> diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
>> index 37efa74018bc..da9af297a0c5 100644
>> --- a/hw/vfio/meson.build
>> +++ b/hw/vfio/meson.build
>> @@ -2,6 +2,7 @@ vfio_ss = ss.source_set()
>>   vfio_ss.add(files(
>>     'common.c',
>>     'spapr.c',
>> +  'migration.c',
>>   ))
>>   vfio_ss.add(when: 'CONFIG_VFIO_PCI', if_true: files(
>>     'display.c',
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> new file mode 100644
>> index 000000000000..bbe6e0b7a6cc
>> --- /dev/null
>> +++ b/hw/vfio/migration.c
>> @@ -0,0 +1,133 @@
>> +/*
>> + * Migration support for VFIO devices
>> + *
>> + * Copyright NVIDIA, Inc. 2020
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2. See
>> + * the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include <linux/vfio.h>
>> +
>> +#include "hw/vfio/vfio-common.h"
>> +#include "cpu.h"
>> +#include "migration/migration.h"
>> +#include "migration/qemu-file.h"
>> +#include "migration/register.h"
>> +#include "migration/blocker.h"
>> +#include "migration/misc.h"
>> +#include "qapi/error.h"
>> +#include "exec/ramlist.h"
>> +#include "exec/ram_addr.h"
>> +#include "pci.h"
>> +#include "trace.h"
>> +
>> +static void vfio_migration_region_exit(VFIODevice *vbasedev)
>> +{
>> +    VFIOMigration *migration = vbasedev->migration;
>> +
>> +    if (!migration) {
>> +        return;
>> +    }
>> +
>> +    vfio_region_exit(&migration->region);
>> +    vfio_region_finalize(&migration->region);
> 
> I think it would make sense to also:
> 
> g_free(migration);
> vbasedev->migration = NULL;
> 
> here as well so the callers don't need to. 

No, vfio_migration_init() case, err case is also hit when 
vbasedev->migration is not yet set but local variable migration is not-NULL.

Thanks,
Kirti

> Not worth a re-spin itself,
> maybe a follow-up if there's no other reason for a re-spin.  Thanks,
> 
> Alex
> 
>> +}
>> +
>> +static int vfio_migration_init(VFIODevice *vbasedev,
>> +                               struct vfio_region_info *info)
>> +{
>> +    int ret;
>> +    Object *obj;
>> +    VFIOMigration *migration;
>> +
>> +    if (!vbasedev->ops->vfio_get_object) {
>> +        return -EINVAL;
>> +    }
>> +
>> +    obj = vbasedev->ops->vfio_get_object(vbasedev);
>> +    if (!obj) {
>> +        return -EINVAL;
>> +    }
>> +
>> +    migration = g_new0(VFIOMigration, 1);
>> +
>> +    ret = vfio_region_setup(obj, vbasedev, &migration->region,
>> +                            info->index, "migration");
>> +    if (ret) {
>> +        error_report("%s: Failed to setup VFIO migration region %d: %s",
>> +                     vbasedev->name, info->index, strerror(-ret));
>> +        goto err;
>> +    }
>> +
>> +    vbasedev->migration = migration;
>> +
>> +    if (!migration->region.size) {
>> +        error_report("%s: Invalid zero-sized of VFIO migration region %d",
>> +                     vbasedev->name, info->index);
>> +        ret = -EINVAL;
>> +        goto err;
>> +    }
>> +    return 0;
>> +
>> +err:
>> +    vfio_migration_region_exit(vbasedev);
>> +    g_free(migration);
>> +    vbasedev->migration = NULL;
>> +    return ret;
>> +}
>> +
>> +/* ---------------------------------------------------------------------- */
>> +
>> +int vfio_migration_probe(VFIODevice *vbasedev, Error **errp)
>> +{
>> +    struct vfio_region_info *info = NULL;
>> +    Error *local_err = NULL;
>> +    int ret;
>> +
>> +    ret = vfio_get_dev_region_info(vbasedev, VFIO_REGION_TYPE_MIGRATION,
>> +                                   VFIO_REGION_SUBTYPE_MIGRATION, &info);
>> +    if (ret) {
>> +        goto add_blocker;
>> +    }
>> +
>> +    ret = vfio_migration_init(vbasedev, info);
>> +    if (ret) {
>> +        goto add_blocker;
>> +    }
>> +
>> +    g_free(info);
>> +    trace_vfio_migration_probe(vbasedev->name, info->index);
>> +    return 0;
>> +
>> +add_blocker:
>> +    error_setg(&vbasedev->migration_blocker,
>> +               "VFIO device doesn't support migration");
>> +    g_free(info);
>> +
>> +    ret = migrate_add_blocker(vbasedev->migration_blocker, &local_err);
>> +    if (local_err) {
>> +        error_propagate(errp, local_err);
>> +        error_free(vbasedev->migration_blocker);
>> +        vbasedev->migration_blocker = NULL;
>> +    }
>> +    return ret;
>> +}
>> +
>> +void vfio_migration_finalize(VFIODevice *vbasedev)
>> +{
>> +    VFIOMigration *migration = vbasedev->migration;
>> +
>> +    if (migration) {
>> +        vfio_migration_region_exit(vbasedev);
>> +        g_free(vbasedev->migration);
>> +        vbasedev->migration = NULL;
>> +    }
>> +
>> +    if (vbasedev->migration_blocker) {
>> +        migrate_del_blocker(vbasedev->migration_blocker);
>> +        error_free(vbasedev->migration_blocker);
>> +        vbasedev->migration_blocker = NULL;
>> +    }
>> +}
>> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
>> index a0c7b49a2ebc..9ced5ec6277c 100644
>> --- a/hw/vfio/trace-events
>> +++ b/hw/vfio/trace-events
>> @@ -145,3 +145,6 @@ vfio_display_edid_link_up(void) ""
>>   vfio_display_edid_link_down(void) ""
>>   vfio_display_edid_update(uint32_t prefx, uint32_t prefy) "%ux%u"
>>   vfio_display_edid_write_error(void) ""
>> +
>> +# migration.c
>> +vfio_migration_probe(const char *name, uint32_t index) " (%s) Region %d"
>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>> index ba6169cd926e..8275c4c68f45 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -57,6 +57,10 @@ typedef struct VFIORegion {
>>       uint8_t nr; /* cache the region number for debug */
>>   } VFIORegion;
>>   
>> +typedef struct VFIOMigration {
>> +    VFIORegion region;
>> +} VFIOMigration;
>> +
>>   typedef struct VFIOAddressSpace {
>>       AddressSpace *as;
>>       QLIST_HEAD(, VFIOContainer) containers;
>> @@ -113,6 +117,8 @@ typedef struct VFIODevice {
>>       unsigned int num_irqs;
>>       unsigned int num_regions;
>>       unsigned int flags;
>> +    VFIOMigration *migration;
>> +    Error *migration_blocker;
>>   } VFIODevice;
>>   
>>   struct VFIODeviceOps {
>> @@ -204,4 +210,7 @@ int vfio_spapr_create_window(VFIOContainer *container,
>>   int vfio_spapr_remove_window(VFIOContainer *container,
>>                                hwaddr offset_within_address_space);
>>   
>> +int vfio_migration_probe(VFIODevice *vbasedev, Error **errp);
>> +void vfio_migration_finalize(VFIODevice *vbasedev);
>> +
>>   #endif /* HW_VFIO_VFIO_COMMON_H */
>
Alex Williamson Oct. 24, 2020, 2:21 p.m. UTC | #4 
On Sat, 24 Oct 2020 15:09:14 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> On 10/23/2020 10:22 PM, Alex Williamson wrote:
> > On Fri, 23 Oct 2020 16:10:30 +0530
> > Kirti Wankhede <kwankhede@nvidia.com> wrote:
> >   
> >> Whether the VFIO device supports migration or not is decided based of
> >> migration region query. If migration region query is successful and migration
> >> region initialization is successful then migration is supported else
> >> migration is blocked.
> >>
> >> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> >> Reviewed-by: Neo Jia <cjia@nvidia.com>
> >> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >> ---
> >>   hw/vfio/meson.build           |   1 +
> >>   hw/vfio/migration.c           | 133 ++++++++++++++++++++++++++++++++++++++++++
> >>   hw/vfio/trace-events          |   3 +
> >>   include/hw/vfio/vfio-common.h |   9 +++
> >>   4 files changed, 146 insertions(+)
> >>   create mode 100644 hw/vfio/migration.c
> >>
> >> diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
> >> index 37efa74018bc..da9af297a0c5 100644
> >> --- a/hw/vfio/meson.build
> >> +++ b/hw/vfio/meson.build
> >> @@ -2,6 +2,7 @@ vfio_ss = ss.source_set()
> >>   vfio_ss.add(files(
> >>     'common.c',
> >>     'spapr.c',
> >> +  'migration.c',
> >>   ))
> >>   vfio_ss.add(when: 'CONFIG_VFIO_PCI', if_true: files(
> >>     'display.c',
> >> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> >> new file mode 100644
> >> index 000000000000..bbe6e0b7a6cc
> >> --- /dev/null
> >> +++ b/hw/vfio/migration.c
> >> @@ -0,0 +1,133 @@
> >> +/*
> >> + * Migration support for VFIO devices
> >> + *
> >> + * Copyright NVIDIA, Inc. 2020
> >> + *
> >> + * This work is licensed under the terms of the GNU GPL, version 2. See
> >> + * the COPYING file in the top-level directory.
> >> + */
> >> +
> >> +#include "qemu/osdep.h"
> >> +#include <linux/vfio.h>
> >> +
> >> +#include "hw/vfio/vfio-common.h"
> >> +#include "cpu.h"
> >> +#include "migration/migration.h"
> >> +#include "migration/qemu-file.h"
> >> +#include "migration/register.h"
> >> +#include "migration/blocker.h"
> >> +#include "migration/misc.h"
> >> +#include "qapi/error.h"
> >> +#include "exec/ramlist.h"
> >> +#include "exec/ram_addr.h"
> >> +#include "pci.h"
> >> +#include "trace.h"
> >> +
> >> +static void vfio_migration_region_exit(VFIODevice *vbasedev)
> >> +{
> >> +    VFIOMigration *migration = vbasedev->migration;
> >> +
> >> +    if (!migration) {
> >> +        return;
> >> +    }
> >> +
> >> +    vfio_region_exit(&migration->region);
> >> +    vfio_region_finalize(&migration->region);  
> > 
> > I think it would make sense to also:
> > 
> > g_free(migration);
> > vbasedev->migration = NULL;
> > 
> > here as well so the callers don't need to.   
> 
> No, vfio_migration_init() case, err case is also hit when 
> vbasedev->migration is not yet set but local variable migration is not-NULL.

So why do we even call vfio_migration_region_exit() for that error
case?  It seems that could just g_free(migration); return ret; rather
than goto err.  Thanks,

Alex

> > Not worth a re-spin itself,
> > maybe a follow-up if there's no other reason for a re-spin.  Thanks,
> > 
> > Alex
> >   
> >> +}
> >> +
> >> +static int vfio_migration_init(VFIODevice *vbasedev,
> >> +                               struct vfio_region_info *info)
> >> +{
> >> +    int ret;
> >> +    Object *obj;
> >> +    VFIOMigration *migration;
> >> +
> >> +    if (!vbasedev->ops->vfio_get_object) {
> >> +        return -EINVAL;
> >> +    }
> >> +
> >> +    obj = vbasedev->ops->vfio_get_object(vbasedev);
> >> +    if (!obj) {
> >> +        return -EINVAL;
> >> +    }
> >> +
> >> +    migration = g_new0(VFIOMigration, 1);
> >> +
> >> +    ret = vfio_region_setup(obj, vbasedev, &migration->region,
> >> +                            info->index, "migration");
> >> +    if (ret) {
> >> +        error_report("%s: Failed to setup VFIO migration region %d: %s",
> >> +                     vbasedev->name, info->index, strerror(-ret));
> >> +        goto err;
> >> +    }
> >> +
> >> +    vbasedev->migration = migration;
> >> +
> >> +    if (!migration->region.size) {
> >> +        error_report("%s: Invalid zero-sized of VFIO migration region %d",
> >> +                     vbasedev->name, info->index);
> >> +        ret = -EINVAL;
> >> +        goto err;
> >> +    }
> >> +    return 0;
> >> +
> >> +err:
> >> +    vfio_migration_region_exit(vbasedev);
> >> +    g_free(migration);
> >> +    vbasedev->migration = NULL;
> >> +    return ret;
> >> +}
> >> +
> >> +/* ---------------------------------------------------------------------- */
> >> +
> >> +int vfio_migration_probe(VFIODevice *vbasedev, Error **errp)
> >> +{
> >> +    struct vfio_region_info *info = NULL;
> >> +    Error *local_err = NULL;
> >> +    int ret;
> >> +
> >> +    ret = vfio_get_dev_region_info(vbasedev, VFIO_REGION_TYPE_MIGRATION,
> >> +                                   VFIO_REGION_SUBTYPE_MIGRATION, &info);
> >> +    if (ret) {
> >> +        goto add_blocker;
> >> +    }
> >> +
> >> +    ret = vfio_migration_init(vbasedev, info);
> >> +    if (ret) {
> >> +        goto add_blocker;
> >> +    }
> >> +
> >> +    g_free(info);
> >> +    trace_vfio_migration_probe(vbasedev->name, info->index);
> >> +    return 0;
> >> +
> >> +add_blocker:
> >> +    error_setg(&vbasedev->migration_blocker,
> >> +               "VFIO device doesn't support migration");
> >> +    g_free(info);
> >> +
> >> +    ret = migrate_add_blocker(vbasedev->migration_blocker, &local_err);
> >> +    if (local_err) {
> >> +        error_propagate(errp, local_err);
> >> +        error_free(vbasedev->migration_blocker);
> >> +        vbasedev->migration_blocker = NULL;
> >> +    }
> >> +    return ret;
> >> +}
> >> +
> >> +void vfio_migration_finalize(VFIODevice *vbasedev)
> >> +{
> >> +    VFIOMigration *migration = vbasedev->migration;
> >> +
> >> +    if (migration) {
> >> +        vfio_migration_region_exit(vbasedev);
> >> +        g_free(vbasedev->migration);
> >> +        vbasedev->migration = NULL;
> >> +    }
> >> +
> >> +    if (vbasedev->migration_blocker) {
> >> +        migrate_del_blocker(vbasedev->migration_blocker);
> >> +        error_free(vbasedev->migration_blocker);
> >> +        vbasedev->migration_blocker = NULL;
> >> +    }
> >> +}
> >> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
> >> index a0c7b49a2ebc..9ced5ec6277c 100644
> >> --- a/hw/vfio/trace-events
> >> +++ b/hw/vfio/trace-events
> >> @@ -145,3 +145,6 @@ vfio_display_edid_link_up(void) ""
> >>   vfio_display_edid_link_down(void) ""
> >>   vfio_display_edid_update(uint32_t prefx, uint32_t prefy) "%ux%u"
> >>   vfio_display_edid_write_error(void) ""
> >> +
> >> +# migration.c
> >> +vfio_migration_probe(const char *name, uint32_t index) " (%s) Region %d"
> >> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> >> index ba6169cd926e..8275c4c68f45 100644
> >> --- a/include/hw/vfio/vfio-common.h
> >> +++ b/include/hw/vfio/vfio-common.h
> >> @@ -57,6 +57,10 @@ typedef struct VFIORegion {
> >>       uint8_t nr; /* cache the region number for debug */
> >>   } VFIORegion;
> >>   
> >> +typedef struct VFIOMigration {
> >> +    VFIORegion region;
> >> +} VFIOMigration;
> >> +
> >>   typedef struct VFIOAddressSpace {
> >>       AddressSpace *as;
> >>       QLIST_HEAD(, VFIOContainer) containers;
> >> @@ -113,6 +117,8 @@ typedef struct VFIODevice {
> >>       unsigned int num_irqs;
> >>       unsigned int num_regions;
> >>       unsigned int flags;
> >> +    VFIOMigration *migration;
> >> +    Error *migration_blocker;
> >>   } VFIODevice;
> >>   
> >>   struct VFIODeviceOps {
> >> @@ -204,4 +210,7 @@ int vfio_spapr_create_window(VFIOContainer *container,
> >>   int vfio_spapr_remove_window(VFIOContainer *container,
> >>                                hwaddr offset_within_address_space);
> >>   
> >> +int vfio_migration_probe(VFIODevice *vbasedev, Error **errp);
> >> +void vfio_migration_finalize(VFIODevice *vbasedev);
> >> +
> >>   #endif /* HW_VFIO_VFIO_COMMON_H */  
> >   
>
Kirti Wankhede Oct. 24, 2020, 2:48 p.m. UTC | #5 
On 10/24/2020 7:51 PM, Alex Williamson wrote:
> On Sat, 24 Oct 2020 15:09:14 +0530
> Kirti Wankhede <kwankhede@nvidia.com> wrote:
> 
>> On 10/23/2020 10:22 PM, Alex Williamson wrote:
>>> On Fri, 23 Oct 2020 16:10:30 +0530
>>> Kirti Wankhede <kwankhede@nvidia.com> wrote:
>>>    
>>>> Whether the VFIO device supports migration or not is decided based of
>>>> migration region query. If migration region query is successful and migration
>>>> region initialization is successful then migration is supported else
>>>> migration is blocked.
>>>>
>>>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
>>>> Reviewed-by: Neo Jia <cjia@nvidia.com>
>>>> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>>> ---
>>>>    hw/vfio/meson.build           |   1 +
>>>>    hw/vfio/migration.c           | 133 ++++++++++++++++++++++++++++++++++++++++++
>>>>    hw/vfio/trace-events          |   3 +
>>>>    include/hw/vfio/vfio-common.h |   9 +++
>>>>    4 files changed, 146 insertions(+)
>>>>    create mode 100644 hw/vfio/migration.c
>>>>
>>>> diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
>>>> index 37efa74018bc..da9af297a0c5 100644
>>>> --- a/hw/vfio/meson.build
>>>> +++ b/hw/vfio/meson.build
>>>> @@ -2,6 +2,7 @@ vfio_ss = ss.source_set()
>>>>    vfio_ss.add(files(
>>>>      'common.c',
>>>>      'spapr.c',
>>>> +  'migration.c',
>>>>    ))
>>>>    vfio_ss.add(when: 'CONFIG_VFIO_PCI', if_true: files(
>>>>      'display.c',
>>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>>>> new file mode 100644
>>>> index 000000000000..bbe6e0b7a6cc
>>>> --- /dev/null
>>>> +++ b/hw/vfio/migration.c
>>>> @@ -0,0 +1,133 @@
>>>> +/*
>>>> + * Migration support for VFIO devices
>>>> + *
>>>> + * Copyright NVIDIA, Inc. 2020
>>>> + *
>>>> + * This work is licensed under the terms of the GNU GPL, version 2. See
>>>> + * the COPYING file in the top-level directory.
>>>> + */
>>>> +
>>>> +#include "qemu/osdep.h"
>>>> +#include <linux/vfio.h>
>>>> +
>>>> +#include "hw/vfio/vfio-common.h"
>>>> +#include "cpu.h"
>>>> +#include "migration/migration.h"
>>>> +#include "migration/qemu-file.h"
>>>> +#include "migration/register.h"
>>>> +#include "migration/blocker.h"
>>>> +#include "migration/misc.h"
>>>> +#include "qapi/error.h"
>>>> +#include "exec/ramlist.h"
>>>> +#include "exec/ram_addr.h"
>>>> +#include "pci.h"
>>>> +#include "trace.h"
>>>> +
>>>> +static void vfio_migration_region_exit(VFIODevice *vbasedev)
>>>> +{
>>>> +    VFIOMigration *migration = vbasedev->migration;
>>>> +
>>>> +    if (!migration) {
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    vfio_region_exit(&migration->region);
>>>> +    vfio_region_finalize(&migration->region);
>>>
>>> I think it would make sense to also:
>>>
>>> g_free(migration);
>>> vbasedev->migration = NULL;
>>>
>>> here as well so the callers don't need to.
>>
>> No, vfio_migration_init() case, err case is also hit when
>> vbasedev->migration is not yet set but local variable migration is not-NULL.
> 
> So why do we even call vfio_migration_region_exit() for that error
> case?  It seems that could just g_free(migration); return ret; rather
> than goto err.  Thanks,
> 

Removing temporary local variable, with that above 2 can be moved to 
exit function.

Thanks,
Kirti

> Alex
> 
>>> Not worth a re-spin itself,
>>> maybe a follow-up if there's no other reason for a re-spin.  Thanks,
>>>
>>> Alex
>>>    
>>>> +}
>>>> +
>>>> +static int vfio_migration_init(VFIODevice *vbasedev,
>>>> +                               struct vfio_region_info *info)
>>>> +{
>>>> +    int ret;
>>>> +    Object *obj;
>>>> +    VFIOMigration *migration;
>>>> +
>>>> +    if (!vbasedev->ops->vfio_get_object) {
>>>> +        return -EINVAL;
>>>> +    }
>>>> +
>>>> +    obj = vbasedev->ops->vfio_get_object(vbasedev);
>>>> +    if (!obj) {
>>>> +        return -EINVAL;
>>>> +    }
>>>> +
>>>> +    migration = g_new0(VFIOMigration, 1);
>>>> +
>>>> +    ret = vfio_region_setup(obj, vbasedev, &migration->region,
>>>> +                            info->index, "migration");
>>>> +    if (ret) {
>>>> +        error_report("%s: Failed to setup VFIO migration region %d: %s",
>>>> +                     vbasedev->name, info->index, strerror(-ret));
>>>> +        goto err;
>>>> +    }
>>>> +
>>>> +    vbasedev->migration = migration;
>>>> +
>>>> +    if (!migration->region.size) {
>>>> +        error_report("%s: Invalid zero-sized of VFIO migration region %d",
>>>> +                     vbasedev->name, info->index);
>>>> +        ret = -EINVAL;
>>>> +        goto err;
>>>> +    }
>>>> +    return 0;
>>>> +
>>>> +err:
>>>> +    vfio_migration_region_exit(vbasedev);
>>>> +    g_free(migration);
>>>> +    vbasedev->migration = NULL;
>>>> +    return ret;
>>>> +}
>>>> +
>>>> +/* ---------------------------------------------------------------------- */
>>>> +
>>>> +int vfio_migration_probe(VFIODevice *vbasedev, Error **errp)
>>>> +{
>>>> +    struct vfio_region_info *info = NULL;
>>>> +    Error *local_err = NULL;
>>>> +    int ret;
>>>> +
>>>> +    ret = vfio_get_dev_region_info(vbasedev, VFIO_REGION_TYPE_MIGRATION,
>>>> +                                   VFIO_REGION_SUBTYPE_MIGRATION, &info);
>>>> +    if (ret) {
>>>> +        goto add_blocker;
>>>> +    }
>>>> +
>>>> +    ret = vfio_migration_init(vbasedev, info);
>>>> +    if (ret) {
>>>> +        goto add_blocker;
>>>> +    }
>>>> +
>>>> +    g_free(info);
>>>> +    trace_vfio_migration_probe(vbasedev->name, info->index);
>>>> +    return 0;
>>>> +
>>>> +add_blocker:
>>>> +    error_setg(&vbasedev->migration_blocker,
>>>> +               "VFIO device doesn't support migration");
>>>> +    g_free(info);
>>>> +
>>>> +    ret = migrate_add_blocker(vbasedev->migration_blocker, &local_err);
>>>> +    if (local_err) {
>>>> +        error_propagate(errp, local_err);
>>>> +        error_free(vbasedev->migration_blocker);
>>>> +        vbasedev->migration_blocker = NULL;
>>>> +    }
>>>> +    return ret;
>>>> +}
>>>> +
>>>> +void vfio_migration_finalize(VFIODevice *vbasedev)
>>>> +{
>>>> +    VFIOMigration *migration = vbasedev->migration;
>>>> +
>>>> +    if (migration) {
>>>> +        vfio_migration_region_exit(vbasedev);
>>>> +        g_free(vbasedev->migration);
>>>> +        vbasedev->migration = NULL;
>>>> +    }
>>>> +
>>>> +    if (vbasedev->migration_blocker) {
>>>> +        migrate_del_blocker(vbasedev->migration_blocker);
>>>> +        error_free(vbasedev->migration_blocker);
>>>> +        vbasedev->migration_blocker = NULL;
>>>> +    }
>>>> +}
>>>> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
>>>> index a0c7b49a2ebc..9ced5ec6277c 100644
>>>> --- a/hw/vfio/trace-events
>>>> +++ b/hw/vfio/trace-events
>>>> @@ -145,3 +145,6 @@ vfio_display_edid_link_up(void) ""
>>>>    vfio_display_edid_link_down(void) ""
>>>>    vfio_display_edid_update(uint32_t prefx, uint32_t prefy) "%ux%u"
>>>>    vfio_display_edid_write_error(void) ""
>>>> +
>>>> +# migration.c
>>>> +vfio_migration_probe(const char *name, uint32_t index) " (%s) Region %d"
>>>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>>>> index ba6169cd926e..8275c4c68f45 100644
>>>> --- a/include/hw/vfio/vfio-common.h
>>>> +++ b/include/hw/vfio/vfio-common.h
>>>> @@ -57,6 +57,10 @@ typedef struct VFIORegion {
>>>>        uint8_t nr; /* cache the region number for debug */
>>>>    } VFIORegion;
>>>>    
>>>> +typedef struct VFIOMigration {
>>>> +    VFIORegion region;
>>>> +} VFIOMigration;
>>>> +
>>>>    typedef struct VFIOAddressSpace {
>>>>        AddressSpace *as;
>>>>        QLIST_HEAD(, VFIOContainer) containers;
>>>> @@ -113,6 +117,8 @@ typedef struct VFIODevice {
>>>>        unsigned int num_irqs;
>>>>        unsigned int num_regions;
>>>>        unsigned int flags;
>>>> +    VFIOMigration *migration;
>>>> +    Error *migration_blocker;
>>>>    } VFIODevice;
>>>>    
>>>>    struct VFIODeviceOps {
>>>> @@ -204,4 +210,7 @@ int vfio_spapr_create_window(VFIOContainer *container,
>>>>    int vfio_spapr_remove_window(VFIOContainer *container,
>>>>                                 hwaddr offset_within_address_space);
>>>>    
>>>> +int vfio_migration_probe(VFIODevice *vbasedev, Error **errp);
>>>> +void vfio_migration_finalize(VFIODevice *vbasedev);
>>>> +
>>>>    #endif /* HW_VFIO_VFIO_COMMON_H */
>>>    
>>
>
Kirti Wankhede Oct. 24, 2020, 8:22 p.m. UTC | #6 
On 10/23/2020 4:54 PM, Cornelia Huck wrote:
> On Fri, 23 Oct 2020 16:10:30 +0530
> Kirti Wankhede <kwankhede@nvidia.com> wrote:
> 
>> Whether the VFIO device supports migration or not is decided based of
>> migration region query. If migration region query is successful and migration
>> region initialization is successful then migration is supported else
>> migration is blocked.
>>
>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
>> Reviewed-by: Neo Jia <cjia@nvidia.com>
>> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> ---
>>   hw/vfio/meson.build           |   1 +
>>   hw/vfio/migration.c           | 133 ++++++++++++++++++++++++++++++++++++++++++
>>   hw/vfio/trace-events          |   3 +
>>   include/hw/vfio/vfio-common.h |   9 +++
>>   4 files changed, 146 insertions(+)
>>   create mode 100644 hw/vfio/migration.c
> 
> (...)
> 
>> +static int vfio_migration_init(VFIODevice *vbasedev,
>> +                               struct vfio_region_info *info)
>> +{
>> +    int ret;
>> +    Object *obj;
>> +    VFIOMigration *migration;
>> +
>> +    if (!vbasedev->ops->vfio_get_object) {
>> +        return -EINVAL;
>> +    }
>> +
>> +    obj = vbasedev->ops->vfio_get_object(vbasedev);
>> +    if (!obj) {
>> +        return -EINVAL;
>> +    }
>> +
>> +    migration = g_new0(VFIOMigration, 1);
>> +
>> +    ret = vfio_region_setup(obj, vbasedev, &migration->region,
>> +                            info->index, "migration");
>> +    if (ret) {
>> +        error_report("%s: Failed to setup VFIO migration region %d: %s",
>> +                     vbasedev->name, info->index, strerror(-ret));
>> +        goto err;
>> +    }
>> +
>> +    vbasedev->migration = migration;
>> +
>> +    if (!migration->region.size) {
>> +        error_report("%s: Invalid zero-sized of VFIO migration region %d",
> 
> s/of //
> 
>> +                     vbasedev->name, info->index);
>> +        ret = -EINVAL;
>> +        goto err;
>> +    }
>> +    return 0;
>> +
>> +err:
>> +    vfio_migration_region_exit(vbasedev);
>> +    g_free(migration);
>> +    vbasedev->migration = NULL;
>> +    return ret;
>> +}
> 
> (...)
> 
>> +void vfio_migration_finalize(VFIODevice *vbasedev)
>> +{
>> +    VFIOMigration *migration = vbasedev->migration;
> 
> I don't think you need this variable?
> 

Removing it.

>> +
>> +    if (migration) {
>> +        vfio_migration_region_exit(vbasedev);
>> +        g_free(vbasedev->migration);
>> +        vbasedev->migration = NULL;
>> +    }
>> +
>> +    if (vbasedev->migration_blocker) {
>> +        migrate_del_blocker(vbasedev->migration_blocker);
>> +        error_free(vbasedev->migration_blocker);
>> +        vbasedev->migration_blocker = NULL;
>> +    }
>> +}
> 
> (...)
>
diff mbox series

Patch

diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
index 37efa74018bc..da9af297a0c5 100644
--- a/hw/vfio/meson.build
+++ b/hw/vfio/meson.build
@@ -2,6 +2,7 @@  vfio_ss = ss.source_set()
 vfio_ss.add(files(
   'common.c',
   'spapr.c',
+  'migration.c',
 ))
 vfio_ss.add(when: 'CONFIG_VFIO_PCI', if_true: files(
   'display.c',
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
new file mode 100644
index 000000000000..bbe6e0b7a6cc
--- /dev/null
+++ b/hw/vfio/migration.c
@@ -0,0 +1,133 @@ 
+/*
+ * Migration support for VFIO devices
+ *
+ * Copyright NVIDIA, Inc. 2020
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include <linux/vfio.h>
+
+#include "hw/vfio/vfio-common.h"
+#include "cpu.h"
+#include "migration/migration.h"
+#include "migration/qemu-file.h"
+#include "migration/register.h"
+#include "migration/blocker.h"
+#include "migration/misc.h"
+#include "qapi/error.h"
+#include "exec/ramlist.h"
+#include "exec/ram_addr.h"
+#include "pci.h"
+#include "trace.h"
+
+static void vfio_migration_region_exit(VFIODevice *vbasedev)
+{
+    VFIOMigration *migration = vbasedev->migration;
+
+    if (!migration) {
+        return;
+    }
+
+    vfio_region_exit(&migration->region);
+    vfio_region_finalize(&migration->region);
+}
+
+static int vfio_migration_init(VFIODevice *vbasedev,
+                               struct vfio_region_info *info)
+{
+    int ret;
+    Object *obj;
+    VFIOMigration *migration;
+
+    if (!vbasedev->ops->vfio_get_object) {
+        return -EINVAL;
+    }
+
+    obj = vbasedev->ops->vfio_get_object(vbasedev);
+    if (!obj) {
+        return -EINVAL;
+    }
+
+    migration = g_new0(VFIOMigration, 1);
+
+    ret = vfio_region_setup(obj, vbasedev, &migration->region,
+                            info->index, "migration");
+    if (ret) {
+        error_report("%s: Failed to setup VFIO migration region %d: %s",
+                     vbasedev->name, info->index, strerror(-ret));
+        goto err;
+    }
+
+    vbasedev->migration = migration;
+
+    if (!migration->region.size) {
+        error_report("%s: Invalid zero-sized of VFIO migration region %d",
+                     vbasedev->name, info->index);
+        ret = -EINVAL;
+        goto err;
+    }
+    return 0;
+
+err:
+    vfio_migration_region_exit(vbasedev);
+    g_free(migration);
+    vbasedev->migration = NULL;
+    return ret;
+}
+
+/* ---------------------------------------------------------------------- */
+
+int vfio_migration_probe(VFIODevice *vbasedev, Error **errp)
+{
+    struct vfio_region_info *info = NULL;
+    Error *local_err = NULL;
+    int ret;
+
+    ret = vfio_get_dev_region_info(vbasedev, VFIO_REGION_TYPE_MIGRATION,
+                                   VFIO_REGION_SUBTYPE_MIGRATION, &info);
+    if (ret) {
+        goto add_blocker;
+    }
+
+    ret = vfio_migration_init(vbasedev, info);
+    if (ret) {
+        goto add_blocker;
+    }
+
+    g_free(info);
+    trace_vfio_migration_probe(vbasedev->name, info->index);
+    return 0;
+
+add_blocker:
+    error_setg(&vbasedev->migration_blocker,
+               "VFIO device doesn't support migration");
+    g_free(info);
+
+    ret = migrate_add_blocker(vbasedev->migration_blocker, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        error_free(vbasedev->migration_blocker);
+        vbasedev->migration_blocker = NULL;
+    }
+    return ret;
+}
+
+void vfio_migration_finalize(VFIODevice *vbasedev)
+{
+    VFIOMigration *migration = vbasedev->migration;
+
+    if (migration) {
+        vfio_migration_region_exit(vbasedev);
+        g_free(vbasedev->migration);
+        vbasedev->migration = NULL;
+    }
+
+    if (vbasedev->migration_blocker) {
+        migrate_del_blocker(vbasedev->migration_blocker);
+        error_free(vbasedev->migration_blocker);
+        vbasedev->migration_blocker = NULL;
+    }
+}
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index a0c7b49a2ebc..9ced5ec6277c 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -145,3 +145,6 @@  vfio_display_edid_link_up(void) ""
 vfio_display_edid_link_down(void) ""
 vfio_display_edid_update(uint32_t prefx, uint32_t prefy) "%ux%u"
 vfio_display_edid_write_error(void) ""
+
+# migration.c
+vfio_migration_probe(const char *name, uint32_t index) " (%s) Region %d"
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index ba6169cd926e..8275c4c68f45 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -57,6 +57,10 @@  typedef struct VFIORegion {
     uint8_t nr; /* cache the region number for debug */
 } VFIORegion;
 
+typedef struct VFIOMigration {
+    VFIORegion region;
+} VFIOMigration;
+
 typedef struct VFIOAddressSpace {
     AddressSpace *as;
     QLIST_HEAD(, VFIOContainer) containers;
@@ -113,6 +117,8 @@  typedef struct VFIODevice {
     unsigned int num_irqs;
     unsigned int num_regions;
     unsigned int flags;
+    VFIOMigration *migration;
+    Error *migration_blocker;
 } VFIODevice;
 
 struct VFIODeviceOps {
@@ -204,4 +210,7 @@  int vfio_spapr_create_window(VFIOContainer *container,
 int vfio_spapr_remove_window(VFIOContainer *container,
                              hwaddr offset_within_address_space);
 
+int vfio_migration_probe(VFIODevice *vbasedev, Error **errp);
+void vfio_migration_finalize(VFIODevice *vbasedev);
+
 #endif /* HW_VFIO_VFIO_COMMON_H */