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

[Steven Sistare <steven.sistare@oracle.com>]

On 2/5/2025 12:45 PM, Cédric Le Goater wrote:
> On 1/29/25 15:43, Steve Sistare wrote:
>> Register a vfio iommufd container for CPR.  Add a blocker if the kernel does
>> not support IOMMU_IOAS_CHANGE_PROCESS.
>>
>> 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/cpr-iommufd.c         | 96 +++++++++++++++++++++++++++++++++++++++++++
>>   hw/vfio/iommufd.c             | 12 +++---
>>   hw/vfio/meson.build           |  1 +
>>   include/hw/vfio/vfio-common.h |  6 +++
>>   4 files changed, 110 insertions(+), 5 deletions(-)
>>   create mode 100644 hw/vfio/cpr-iommufd.c
>>
>> diff --git a/hw/vfio/cpr-iommufd.c b/hw/vfio/cpr-iommufd.c
>> new file mode 100644
>> index 0000000..4eb358a
>> --- /dev/null
>> +++ b/hw/vfio/cpr-iommufd.c
>> @@ -0,0 +1,96 @@
>> +/*
>> + * Copyright (c) 2024-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 "qemu/osdep.h"
>> +#include "qapi/error.h"
>> +#include "hw/vfio/vfio-common.h"
>> +#include "migration/blocker.h"
>> +#include "migration/cpr.h"
>> +#include "migration/migration.h"
>> +#include "migration/vmstate.h"
>> +#include "system/iommufd.h"
>> +
>> +static bool vfio_cpr_supported(VFIOIOMMUFDContainer *container, Error **errp)
>> +{
>> +    if (!iommufd_change_process_capable(container->be)) {
>> +        error_setg(errp,
>> +                   "VFIO container does not support IOMMU_IOAS_CHANGE_PROCESS");
>> +        return false;
>> +    }
>> +    return true;
>> +}
>> +
>> +static const VMStateDescription vfio_container_vmstate = {
>> +    .name = "vfio-iommufd-container",
>> +    .version_id = 0,
>> +    .minimum_version_id = 0,
>> +    .needed = cpr_needed_for_reuse,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static const VMStateDescription iommufd_cpr_vmstate = {
>> +    .name = "iommufd",
>> +    .version_id = 0,
>> +    .minimum_version_id = 0,
>> +    .needed = cpr_needed_for_reuse,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +bool vfio_iommufd_cpr_register_container(VFIOIOMMUFDContainer *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);
>> +    vmstate_register(NULL, -1, &iommufd_cpr_vmstate, container->be);
>> +
>> +    return true;
>> +}
>> +
>> +void vfio_iommufd_cpr_unregister_container(VFIOIOMMUFDContainer *container)
>> +{
>> +    VFIOContainerBase *bcontainer = &container->bcontainer;
>> +
>> +    vmstate_unregister(NULL, &iommufd_cpr_vmstate, container->be);
>> +    vmstate_unregister(NULL, &vfio_container_vmstate, container);
>> +    migrate_del_blocker(&container->cpr_blocker);
>> +    vfio_cpr_unregister_container(bcontainer);
>> +}
>> +
>> +static const VMStateDescription vfio_device_vmstate = {
>> +    .name = "vfio-iommufd-device",
>> +    .version_id = 0,
>> +    .minimum_version_id = 0,
>> +    .needed = cpr_needed_for_reuse,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +void vfio_iommufd_cpr_register_device(VFIODevice *vbasedev)
>> +{
>> +    vmstate_register(NULL, -1, &vfio_device_vmstate, vbasedev);
>> +}
>> +
>> +void vfio_iommufd_cpr_unregister_device(VFIODevice *vbasedev)
>> +{
>> +    vmstate_unregister(NULL, &vfio_device_vmstate, vbasedev);
>> +}
>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>> index 8308715..ae78e00 100644
>> --- a/hw/vfio/iommufd.c
>> +++ b/hw/vfio/iommufd.c
>> @@ -592,6 +592,10 @@ static bool iommufd_cdev_attach(const char *name, VFIODevice *vbasedev,
>>       bcontainer->initialized = true;
>> +    if (!vfio_iommufd_cpr_register_container(container, errp)) {
>> +        goto err_listener_register;
>> +    }
>> +
> 
> why this change ?

vfio_iommufd_cpr_register_container() registers empty vmstate handlers in this
patch, but additional fields are added to the vmstate in subsequent patches.

- Steve

>>   found_container:
>>       ret = ioctl(devfd, VFIO_DEVICE_GET_INFO, &dev_info);
>>       if (ret) {
>> @@ -599,10 +603,6 @@ found_container:
>>           goto err_listener_register;
>>       }
>> -    if (!vfio_cpr_register_container(bcontainer, errp)) {
>> -        goto err_listener_register;
>> -    }
>> -
>>       /*
>>        * TODO: examine RAM_BLOCK_DISCARD stuff, should we do group level
>>        * for discarding incompatibility check as well?
>> @@ -619,6 +619,7 @@ found_container:
>>       vbasedev->bcontainer = bcontainer;
>>       QLIST_INSERT_HEAD(&bcontainer->device_list, vbasedev, container_next);
>>       QLIST_INSERT_HEAD(&vfio_device_list, vbasedev, global_next);
>> +    vfio_iommufd_cpr_register_device(vbasedev);
>>       trace_iommufd_cdev_device_info(vbasedev->name, devfd, vbasedev->num_irqs,
>>                                      vbasedev->num_regions, vbasedev->flags);
>> @@ -653,12 +654,13 @@ static void iommufd_cdev_detach(VFIODevice *vbasedev)
>>           iommufd_cdev_ram_block_discard_disable(false);
>>       }
>> -    vfio_cpr_unregister_container(bcontainer);
>> +    vfio_iommufd_cpr_unregister_container(container);
>>       iommufd_cdev_detach_container(vbasedev, container);
>>       iommufd_cdev_container_destroy(container);
>>       vfio_put_address_space(space);
>>       migrate_del_blocker(&vbasedev->cpr_id_blocker);
>> +    vfio_iommufd_cpr_unregister_device(vbasedev);
>>       iommufd_cdev_unbind_and_disconnect(vbasedev);
>>       close(vbasedev->fd);
>>   }
>> diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
>> index 5487815..998adb5 100644
>> --- a/hw/vfio/meson.build
>> +++ b/hw/vfio/meson.build
>> @@ -13,6 +13,7 @@ vfio_ss.add(when: 'CONFIG_IOMMUFD', if_true: files(
>>   vfio_ss.add(when: 'CONFIG_VFIO_PCI', if_true: files(
>>     'cpr.c',
>>     'cpr-legacy.c',
>> +  'cpr-iommufd.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 37e7c26..add44d4 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -113,6 +113,7 @@ typedef struct VFIOIOASHwpt {
>>   typedef struct VFIOIOMMUFDContainer {
>>       VFIOContainerBase bcontainer;
>>       IOMMUFDBackend *be;
>> +    Error *cpr_blocker;
>>       uint32_t ioas_id;
>>       QLIST_HEAD(, VFIOIOASHwpt) hwpt_list;
>>   } VFIOIOMMUFDContainer;
>> @@ -271,6 +272,11 @@ bool vfio_legacy_cpr_register_container(VFIOContainer *container, Error **errp);
>>   void vfio_legacy_cpr_unregister_container(VFIOContainer *container);
>>   bool iommufd_cdev_get_info_iova_range(VFIOIOMMUFDContainer *container,
>>                                         uint32_t ioas_id, Error **errp);
>> +bool vfio_iommufd_cpr_register_container(VFIOIOMMUFDContainer *container,
>> +                                         Error **errp);
>> +void vfio_iommufd_cpr_unregister_container(VFIOIOMMUFDContainer *container);
>> +void vfio_iommufd_cpr_register_device(VFIODevice *vbasedev);
>> +void vfio_iommufd_cpr_unregister_device(VFIODevice *vbasedev);
>>   extern const MemoryRegionOps vfio_region_ops;
>>   typedef QLIST_HEAD(VFIOGroupList, VFIOGroup) VFIOGroupList;
>