Re: [PATCH V1 04/26] vfio/container: register container for cpr

["Cédric Le Goater" <clg@redhat.com>]

On 1/29/25 15:43, Steve Sistare wrote:
> Register a legacy container for cpr-transfer.  Add a blocker if the kernel
> does not support VFIO_UPDATE_VADDR or VFIO_UNMAP_ALL.
> 
> This is mostly boiler plate.  The fields to to saved and restored are added
> in subsequent patches.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>   hw/vfio/container.c           |  6 ++--
>   hw/vfio/cpr-legacy.c          | 68 +++++++++++++++++++++++++++++++++++++++++++
>   hw/vfio/meson.build           |  3 +-
>   include/hw/vfio/vfio-common.h |  3 ++
>   4 files changed, 76 insertions(+), 4 deletions(-)
>   create mode 100644 hw/vfio/cpr-legacy.c
> 
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 4ebb526..a90ce6c 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -618,7 +618,7 @@ static bool vfio_connect_container(VFIOGroup *group, AddressSpace *as,
>       }
>       bcontainer = &container->bcontainer;
>   
> -    if (!vfio_cpr_register_container(bcontainer, errp)) {
> +    if (!vfio_legacy_cpr_register_container(container, errp)) {
>           goto free_container_exit;
>       }
>   
> @@ -666,7 +666,7 @@ enable_discards_exit:
>       vfio_ram_block_discard_disable(container, false);
>   
>   unregister_container_exit:
> -    vfio_cpr_unregister_container(bcontainer);
> +    vfio_legacy_cpr_unregister_container(container);
>   
>   free_container_exit:
>       object_unref(container);
> @@ -710,7 +710,7 @@ static void vfio_disconnect_container(VFIOGroup *group)
>           VFIOAddressSpace *space = bcontainer->space;
>   
>           trace_vfio_disconnect_container(container->fd);
> -        vfio_cpr_unregister_container(bcontainer);
> +        vfio_legacy_cpr_unregister_container(container);
>           close(container->fd);
>           object_unref(container);
>   
> diff --git a/hw/vfio/cpr-legacy.c b/hw/vfio/cpr-legacy.c
> new file mode 100644
> index 0000000..d3bbc05
> --- /dev/null
> +++ b/hw/vfio/cpr-legacy.c
> @@ -0,0 +1,68 @@
> +/*
> + * Copyright (c) 2021-2025 Oracle and/or its affiliates.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include <sys/ioctl.h>
> +#include "qemu/osdep.h"
> +#include "hw/vfio/vfio-common.h"
> +#include "migration/blocker.h"
> +#include "migration/cpr.h"
> +#include "migration/migration.h"
> +#include "migration/vmstate.h"
> +#include "qapi/error.h"
> +
> +static bool vfio_cpr_supported(VFIOContainer *container, Error **errp)
> +{
> +    if (!ioctl(container->fd, VFIO_CHECK_EXTENSION, VFIO_UPDATE_VADDR)) {
> +        error_setg(errp, "VFIO container does not support VFIO_UPDATE_VADDR");
> +        return false;
> +
> +    } else if (!ioctl(container->fd, VFIO_CHECK_EXTENSION, VFIO_UNMAP_ALL)) {
> +        error_setg(errp, "VFIO container does not support VFIO_UNMAP_ALL");
> +        return false;
> +
> +    } else {
> +        return true;
> +    }
> +}
> +
> +static const VMStateDescription vfio_container_vmstate = {
> +    .name = "vfio-container",
> +    .version_id = 0,
> +    .minimum_version_id = 0,
> +    .needed = cpr_needed_for_reuse,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +bool vfio_legacy_cpr_register_container(VFIOContainer *container, Error **errp)
> +{
> +    VFIOContainerBase *bcontainer = &container->bcontainer;
> +    Error **cpr_blocker = &container->cpr_blocker;
> +
> +    if (!vfio_cpr_register_container(bcontainer, errp)) {
> +        return false;
> +    }
> +
> +    if (!vfio_cpr_supported(container, cpr_blocker)) {
> +        return migrate_add_blocker_modes(cpr_blocker, errp,
> +                                         MIG_MODE_CPR_TRANSFER, -1) == 0;
> +    }
> +
> +    vmstate_register(NULL, -1, &vfio_container_vmstate, container);
> +
> +    return true;
> +}
> +
> +void vfio_legacy_cpr_unregister_container(VFIOContainer *container)
> +{
> +    VFIOContainerBase *bcontainer = &container->bcontainer;
> +
> +    vfio_cpr_unregister_container(bcontainer);
> +    migrate_del_blocker(&container->cpr_blocker);
> +    vmstate_unregister(NULL, &vfio_container_vmstate, container);
> +}
> diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
> index bba776f..5487815 100644
> --- a/hw/vfio/meson.build
> +++ b/hw/vfio/meson.build
> @@ -5,13 +5,14 @@ vfio_ss.add(files(
>     'container-base.c',
>     'container.c',
>     'migration.c',
> -  'cpr.c',
>   ))
>   vfio_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr.c'))
>   vfio_ss.add(when: 'CONFIG_IOMMUFD', if_true: files(
>     'iommufd.c',
>   ))
>   vfio_ss.add(when: 'CONFIG_VFIO_PCI', if_true: files(
> +  'cpr.c',
> +  'cpr-legacy.c',
>     'display.c',
>     'pci-quirks.c',
>     'pci.c',
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 0c60be5..53e554f 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -84,6 +84,7 @@ typedef struct VFIOContainer {
>       VFIOContainerBase bcontainer;
>       int fd; /* /dev/vfio/vfio, empowered by the attached groups */
>       unsigned iommu_type;
> +    Error *cpr_blocker;
>       QLIST_HEAD(, VFIOGroup) group_list;
>   } VFIOContainer;
>   
> @@ -258,6 +259,8 @@ int vfio_kvm_device_del_fd(int fd, Error **errp);
>   
>   bool vfio_cpr_register_container(VFIOContainerBase *bcontainer, Error **errp);
>   void vfio_cpr_unregister_container(VFIOContainerBase *bcontainer);

I think we should now rename the above routines to reflect what they do :
add/remove a notifier.

> +bool vfio_legacy_cpr_register_container(VFIOContainer *container, Error **errp);
> +void vfio_legacy_cpr_unregister_container(VFIOContainer *container);

Thanks,

C.

>   
>   extern const MemoryRegionOps vfio_region_ops;
>   typedef QLIST_HEAD(VFIOGroupList, VFIOGroup) VFIOGroupList;