Re: [PATCH V5 29/38] vfio/iommufd: register container for cpr

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

On 7/2/2025 10:17 AM, Duan, Zhenzhong wrote:
>> -----Original Message-----
>> From: Steve Sistare <steven.sistare@oracle.com>
>> Subject: [PATCH V5 29/38] vfio/iommufd: register container for cpr
>>
>> Register a vfio iommufd container and device for CPR, replacing the generic
>> CPR register call with a more specific iommufd register call.  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>
>> ---
>> include/hw/vfio/vfio-cpr.h | 12 +++++++
>> include/system/iommufd.h   |  1 +
>> backends/iommufd.c         | 10 ++++++
>> hw/vfio/cpr-iommufd.c      | 84
>> ++++++++++++++++++++++++++++++++++++++++++++++
>> hw/vfio/iommufd.c          |  6 ++--
>> hw/vfio/meson.build        |  1 +
>> 6 files changed, 112 insertions(+), 2 deletions(-)
>> create mode 100644 hw/vfio/cpr-iommufd.c
>>
>> diff --git a/include/hw/vfio/vfio-cpr.h b/include/hw/vfio/vfio-cpr.h
>> index 170a116..b9b77ae 100644
>> --- a/include/hw/vfio/vfio-cpr.h
>> +++ b/include/hw/vfio/vfio-cpr.h
>> @@ -15,7 +15,10 @@
>> struct VFIOContainer;
>> struct VFIOContainerBase;
>> struct VFIOGroup;
>> +struct VFIODevice;
>> struct VFIOPCIDevice;
>> +struct VFIOIOMMUFDContainer;
>> +struct IOMMUFDBackend;
>>
>> typedef struct VFIOContainerCPR {
>>      Error *blocker;
>> @@ -43,6 +46,15 @@ bool vfio_cpr_register_container(struct
>> VFIOContainerBase *bcontainer,
>>                                   Error **errp);
>> void vfio_cpr_unregister_container(struct VFIOContainerBase *bcontainer);
>>
>> +bool vfio_iommufd_cpr_register_container(struct VFIOIOMMUFDContainer
>> *container,
>> +                                         Error **errp);
>> +void vfio_iommufd_cpr_unregister_container(
>> +    struct VFIOIOMMUFDContainer *container);
>> +bool vfio_iommufd_cpr_register_iommufd(struct IOMMUFDBackend *be,
>> Error **errp);
>> +void vfio_iommufd_cpr_unregister_iommufd(struct IOMMUFDBackend *be);
>> +void vfio_iommufd_cpr_register_device(struct VFIODevice *vbasedev);
>> +void vfio_iommufd_cpr_unregister_device(struct VFIODevice *vbasedev);
>> +
>> int vfio_cpr_group_get_device_fd(int d, const char *name);
>>
>> bool vfio_cpr_container_match(struct VFIOContainer *container,
>> diff --git a/include/system/iommufd.h b/include/system/iommufd.h
>> index db5f2c7..c9c72ff 100644
>> --- a/include/system/iommufd.h
>> +++ b/include/system/iommufd.h
>> @@ -32,6 +32,7 @@ struct IOMMUFDBackend {
>>      /*< protected >*/
>>      int fd;            /* /dev/iommu file descriptor */
>>      bool owned;        /* is the /dev/iommu opened internally */
>> +    Error *cpr_blocker;/* set if be does not support CPR */
>>      uint32_t users;
>>
>>      /*< public >*/
>> diff --git a/backends/iommufd.c b/backends/iommufd.c
>> index 87f81a0..c554ce5 100644
>> --- a/backends/iommufd.c
>> +++ b/backends/iommufd.c
>> @@ -108,6 +108,13 @@ bool
>> iommufd_backend_connect(IOMMUFDBackend *be, Error **errp)
>>          }
>>          be->fd = fd;
>>      }
>> +    if (!be->users && !vfio_iommufd_cpr_register_iommufd(be, errp)) {
>> +        if (be->owned) {
>> +            close(be->fd);
>> +            be->fd = -1;
>> +        }
>> +        return false;
>> +    }
>>      be->users++;
>>
>>      trace_iommufd_backend_connect(be->fd, be->owned, be->users);
>> @@ -125,6 +132,9 @@ void
>> iommufd_backend_disconnect(IOMMUFDBackend *be)
>>          be->fd = -1;
>>      }
>> out:
>> +    if (!be->users) {
>> +        vfio_iommufd_cpr_unregister_iommufd(be);
>> +    }
>>      trace_iommufd_backend_disconnect(be->fd, be->users);
>> }
>>
>> diff --git a/hw/vfio/cpr-iommufd.c b/hw/vfio/cpr-iommufd.c
>> new file mode 100644
>> index 0000000..60bd7e8
>> --- /dev/null
>> +++ b/hw/vfio/cpr-iommufd.c
>> @@ -0,0 +1,84 @@
>> +/*
>> + * Copyright (c) 2024-2025 Oracle and/or its affiliates.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qapi/error.h"
>> +#include "hw/vfio/vfio-cpr.h"
>> +#include "migration/blocker.h"
>> +#include "migration/cpr.h"
>> +#include "migration/migration.h"
>> +#include "migration/vmstate.h"
>> +#include "system/iommufd.h"
>> +#include "vfio-iommufd.h"
>> +
>> +static bool vfio_cpr_supported(IOMMUFDBackend *be, Error **errp)
>> +{
>> +    if (!iommufd_change_process_capable(be)) {
>> +        if (errp) {
>> +            error_setg(errp, "vfio iommufd backend does not support "
>> +                       "IOMMU_IOAS_CHANGE_PROCESS");
>> +        }
>> +        return false;
>> +    }
>> +    return true;
>> +}
>> +
>> +static const VMStateDescription iommufd_cpr_vmstate = {
>> +    .name = "iommufd",
>> +    .version_id = 0,
>> +    .minimum_version_id = 0,
>> +    .needed = cpr_incoming_needed,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +bool vfio_iommufd_cpr_register_iommufd(IOMMUFDBackend *be, Error
>> **errp)
>> +{
>> +    Error **cpr_blocker = &be->cpr_blocker;
>> +
>> +    if (!vfio_cpr_supported(be, cpr_blocker)) {
>> +        return migrate_add_blocker_modes(cpr_blocker, errp,
>> +
>> MIG_MODE_CPR_TRANSFER, -1) == 0;
>> +    }
> 
> I suspect that blocker is never installed. Because vfio_iommufd_cpr_register_iommufd() is called for first VFIO device and before memory listener is installed. iommufd_change_process_capable() will always return true.

The blocker is installed if the kernel does not support the change process ioctl,
which was added last year.  iommufd_change_process_capable() will return false.

- Steve