RE: [PATCH v11 05/11] vfio/migration: Block multiple devices migration

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From:
> qemu-devel-bounces+shameerali.kolothum.thodi=huawei.com@nongnu.org
> [mailto:qemu-devel-bounces+shameerali.kolothum.thodi=huawei.com@nong
> nu.org] On Behalf Of Avihai Horon
> Sent: 16 February 2023 14:36
> To: qemu-devel@nongnu.org
> Cc: Alex Williamson <alex.williamson@redhat.com>; Juan Quintela
> <quintela@redhat.com>; Dr. David Alan Gilbert <dgilbert@redhat.com>;
> Michael S. Tsirkin <mst@redhat.com>; Cornelia Huck <cohuck@redhat.com>;
> Paolo Bonzini <pbonzini@redhat.com>; Vladimir Sementsov-Ogievskiy
> <vsementsov@yandex-team.ru>; Cédric Le Goater <clg@redhat.com>; Yishai
> Hadas <yishaih@nvidia.com>; Jason Gunthorpe <jgg@nvidia.com>; Maor
> Gottlieb <maorg@nvidia.com>; Avihai Horon <avihaih@nvidia.com>; Kirti
> Wankhede <kwankhede@nvidia.com>; Tarun Gupta <targupta@nvidia.com>;
> Joao Martins <joao.m.martins@oracle.com>
> Subject: [PATCH v11 05/11] vfio/migration: Block multiple devices migration
> 
> Currently VFIO migration doesn't implement some kind of intermediate
> quiescent state in which P2P DMAs are quiesced before stopping or
> running the device. This can cause problems in multi-device migration
> where the devices are doing P2P DMAs, since the devices are not stopped
> together at the same time.
> 
> Until such support is added, block migration of multiple devices.

Missed this one. Currently this blocks even if the attached devices are not
capable of P2P DMAs. eg; HiSilicon ACC devices. These are integrated end point
devices without any P2P capability between them. Is it Ok to check for
VFIO_MIGRATION_P2P flag and allow if the devices are not supporting that?

I can sent a patch if that’s fine.

Thanks,
Shameer
> 
> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> ---
>  include/hw/vfio/vfio-common.h |  2 ++
>  hw/vfio/common.c              | 53
> +++++++++++++++++++++++++++++++++++
>  hw/vfio/migration.c           |  6 ++++
>  3 files changed, 61 insertions(+)
> 
> diff --git a/include/hw/vfio/vfio-common.h
> b/include/hw/vfio/vfio-common.h
> index e573f5a9f1..56b1683824 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -218,6 +218,8 @@ typedef QLIST_HEAD(VFIOGroupList, VFIOGroup)
> VFIOGroupList;
>  extern VFIOGroupList vfio_group_list;
> 
>  bool vfio_mig_active(void);
> +int vfio_block_multiple_devices_migration(Error **errp);
> +void vfio_unblock_multiple_devices_migration(void);
>  int64_t vfio_mig_bytes_transferred(void);
> 
>  #ifdef CONFIG_LINUX
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 3a35f4afad..fe80ccf914 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -41,6 +41,7 @@
>  #include "qapi/error.h"
>  #include "migration/migration.h"
>  #include "migration/misc.h"
> +#include "migration/blocker.h"
>  #include "sysemu/tpm.h"
> 
>  VFIOGroupList vfio_group_list =
> @@ -337,6 +338,58 @@ bool vfio_mig_active(void)
>      return true;
>  }
> 
> +static Error *multiple_devices_migration_blocker;
> +
> +static unsigned int vfio_migratable_device_num(void)
> +{
> +    VFIOGroup *group;
> +    VFIODevice *vbasedev;
> +    unsigned int device_num = 0;
> +
> +    QLIST_FOREACH(group, &vfio_group_list, next) {
> +        QLIST_FOREACH(vbasedev, &group->device_list, next) {
> +            if (vbasedev->migration) {
> +                device_num++;
> +            }
> +        }
> +    }
> +
> +    return device_num;
> +}
> +
> +int vfio_block_multiple_devices_migration(Error **errp)
> +{
> +    int ret;
> +
> +    if (multiple_devices_migration_blocker ||
> +        vfio_migratable_device_num() <= 1) {
> +        return 0;
> +    }
> +
> +    error_setg(&multiple_devices_migration_blocker,
> +               "Migration is currently not supported with multiple "
> +               "VFIO devices");
> +    ret = migrate_add_blocker(multiple_devices_migration_blocker, errp);
> +    if (ret < 0) {
> +        error_free(multiple_devices_migration_blocker);
> +        multiple_devices_migration_blocker = NULL;
> +    }
> +
> +    return ret;
> +}
> +
> +void vfio_unblock_multiple_devices_migration(void)
> +{
> +    if (!multiple_devices_migration_blocker ||
> +        vfio_migratable_device_num() > 1) {
> +        return;
> +    }
> +
> +    migrate_del_blocker(multiple_devices_migration_blocker);
> +    error_free(multiple_devices_migration_blocker);
> +    multiple_devices_migration_blocker = NULL;
> +}
> +
>  static bool vfio_devices_all_dirty_tracking(VFIOContainer *container)
>  {
>      VFIOGroup *group;
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index e56eef1ee8..8e96999669 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -878,6 +878,11 @@ int vfio_migration_probe(VFIODevice *vbasedev,
> Error **errp)
>          goto add_blocker;
>      }
> 
> +    ret = vfio_block_multiple_devices_migration(errp);
> +    if (ret) {
> +        return ret;
> +    }
> +
>      trace_vfio_migration_probe(vbasedev->name, info->index);
>      g_free(info);
>      return 0;
> @@ -904,6 +909,7 @@ void vfio_migration_finalize(VFIODevice *vbasedev)
>          qemu_del_vm_change_state_handler(migration->vm_state);
>          unregister_savevm(VMSTATE_IF(vbasedev->dev), "vfio",
> vbasedev);
>          vfio_migration_exit(vbasedev);
> +        vfio_unblock_multiple_devices_migration();
>      }
> 
>      if (vbasedev->migration_blocker) {
> --
> 2.26.3
>