Re: [PATCH V5 31/38] vfio/iommufd: cpr state

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

On 6/23/2025 6:45 AM, Duan, Zhenzhong wrote:
>> -----Original Message-----
>> From: Steve Sistare <steven.sistare@oracle.com>
>> Subject: [PATCH V5 31/38] vfio/iommufd: cpr state
>>
>> VFIO iommufd devices will need access to ioas_id, devid, and hwpt_id in
>> new QEMU at realize time, so add them to CPR state.  Define CprVFIODevice
>> as the object which holds the state and is serialized to the vmstate file.
>> Define accessors to copy state between VFIODevice and CprVFIODevice.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>> include/hw/vfio/vfio-cpr.h |  3 ++
>> hw/vfio/cpr-iommufd.c      | 96
>> +++++++++++++++++++++++++++++++++++++++++++++-
>> hw/vfio/iommufd.c          |  2 +
>> 3 files changed, 100 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/vfio/vfio-cpr.h b/include/hw/vfio/vfio-cpr.h
>> index 619af07..f88e4ba 100644
>> --- a/include/hw/vfio/vfio-cpr.h
>> +++ b/include/hw/vfio/vfio-cpr.h
>> @@ -33,6 +33,8 @@ typedef struct VFIOContainerCPR {
>> typedef struct VFIODeviceCPR {
>>      Error *mdev_blocker;
>>      Error *id_blocker;
>> +    uint32_t hwpt_id;
>> +    uint32_t ioas_id;
>> } VFIODeviceCPR;
>>
>> bool vfio_legacy_cpr_register_container(struct VFIOContainer *container,
>> @@ -54,6 +56,7 @@ 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);
>> +void vfio_cpr_load_device(struct VFIODevice *vbasedev);
>>
>> int vfio_cpr_group_get_device_fd(int d, const char *name);
>>
>> diff --git a/hw/vfio/cpr-iommufd.c b/hw/vfio/cpr-iommufd.c
>> index 3e78265..2eca8a6 100644
>> --- a/hw/vfio/cpr-iommufd.c
>> +++ b/hw/vfio/cpr-iommufd.c
>> @@ -7,6 +7,7 @@
>> #include "qemu/osdep.h"
>> #include "qapi/error.h"
>> #include "hw/vfio/vfio-cpr.h"
>> +#include "hw/vfio/vfio-device.h"
>> #include "migration/blocker.h"
>> #include "migration/cpr.h"
>> #include "migration/migration.h"
>> @@ -14,7 +15,88 @@
>> #include "system/iommufd.h"
>> #include "vfio-iommufd.h"
>>
>> -const VMStateDescription vmstate_cpr_vfio_devices;  /* TBD in a later patch */
>> +typedef struct CprVFIODevice {
>> +    char *name;
>> +    unsigned int namelen;
>> +    uint32_t ioas_id;
>> +    int devid;
>> +    uint32_t hwpt_id;
>> +    QLIST_ENTRY(CprVFIODevice) next;
>> +} CprVFIODevice;
>> +
>> +static const VMStateDescription vmstate_cpr_vfio_device = {
>> +    .name = "cpr vfio device",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT32(namelen, CprVFIODevice),
>> +        VMSTATE_VBUFFER_ALLOC_UINT32(name, CprVFIODevice, 0, NULL,
>> namelen),
>> +        VMSTATE_INT32(devid, CprVFIODevice),
>> +        VMSTATE_UINT32(ioas_id, CprVFIODevice),
>> +        VMSTATE_UINT32(hwpt_id, CprVFIODevice),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +const VMStateDescription vmstate_cpr_vfio_devices = {
>> +    .name = CPR_STATE "/vfio devices",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .fields = (const VMStateField[]){
>> +        VMSTATE_QLIST_V(vfio_devices, CprState, 1, vmstate_cpr_vfio_device,
>> +                        CprVFIODevice, next),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static void vfio_cpr_save_device(VFIODevice *vbasedev)
>> +{
>> +    CprVFIODevice *elem = g_new0(CprVFIODevice, 1);
>> +
>> +    elem->name = g_strdup(vbasedev->name);
>> +    elem->namelen = strlen(vbasedev->name) + 1;
>> +    elem->ioas_id = vbasedev->cpr.ioas_id;
>> +    elem->devid = vbasedev->devid;
>> +    elem->hwpt_id = vbasedev->cpr.hwpt_id;
>> +    QLIST_INSERT_HEAD(&cpr_state.vfio_devices, elem, next);
>> +}
>> +
>> +static CprVFIODevice *find_device(const char *name)
>> +{
>> +    CprVFIODeviceList *head = &cpr_state.vfio_devices;
>> +    CprVFIODevice *elem;
>> +
>> +    QLIST_FOREACH(elem, head, next) {
>> +        if (!strcmp(elem->name, name)) {
>> +            return elem;
>> +        }
>> +    }
>> +    return NULL;
>> +}
>> +
>> +static void vfio_cpr_delete_device(const char *name)
>> +{
>> +    CprVFIODevice *elem = find_device(name);
>> +
>> +    if (elem) {
>> +        QLIST_REMOVE(elem, next);
>> +        g_free(elem->name);
>> +        g_free(elem);
>> +    }
>> +}
>> +
>> +static bool vfio_cpr_find_device(VFIODevice *vbasedev)
> 
> Better to rename as vfio_cpr_load_device

This is already called by a function named vfio_cpr_load_device.
The usage is the same as cpr_find_fd, so "find" is a consistent name.

>> +{
>> +    CprVFIODevice *elem = find_device(vbasedev->name);
>> +
>> +    if (elem) {
>> +        vbasedev->cpr.ioas_id = elem->ioas_id;
>> +        vbasedev->devid = elem->devid;
>> +        vbasedev->cpr.hwpt_id = elem->hwpt_id;
>> +        return true;
>> +    }
>> +    return false;
>> +}
>>
>> static bool vfio_cpr_supported(IOMMUFDBackend *be, Error **errp)
>> {
>> @@ -79,8 +161,20 @@ void
>> vfio_iommufd_cpr_unregister_container(VFIOIOMMUFDContainer *container)
>>
>> void vfio_iommufd_cpr_register_device(VFIODevice *vbasedev)
>> {
>> +    if (!cpr_is_incoming()) {
>> +        vfio_cpr_save_device(vbasedev);
>> +    }
>> }
>>
>> void vfio_iommufd_cpr_unregister_device(VFIODevice *vbasedev)
>> {
>> +    vfio_cpr_delete_device(vbasedev->name);
>> +}
>> +
>> +void vfio_cpr_load_device(VFIODevice *vbasedev)
>> +{
>> +    if (cpr_is_incoming()) {
>> +        bool ret = vfio_cpr_find_device(vbasedev);
>> +        g_assert(ret);
>> +    }
>> }
>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>> index ff291be..f0d57ea 100644
>> --- a/hw/vfio/iommufd.c
>> +++ b/hw/vfio/iommufd.c
>> @@ -515,6 +515,8 @@ static bool iommufd_cdev_attach(const char *name,
>> VFIODevice *vbasedev,
>>      const VFIOIOMMUClass *iommufd_vioc =
>>
>> VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
>>
>> +    vfio_cpr_load_device(vbasedev);
> 
> This can be open coded.

vfio_cpr_load_device grows in patch "preserve descriptors", so I would
rather keep it closed.

- Steve

>> +
>>      if (vbasedev->fd < 0) {
>>          devfd = iommufd_cdev_getfd(vbasedev->sysfsdev, errp);
>>          if (devfd < 0) {
>> --
>> 1.8.3.1
>