Re: [PATCH v3 14/15] vfio/common: Introduce a global VFIODevice list

[Cédric Le Goater]

On 10/3/23 12:14, Eric Auger wrote:
> From: Zhenzhong Duan <zhenzhong.duan@intel.com>
>
> Some functions iterate over all the VFIODevices. This is currently
> achieved by iterating over all groups/devices. Let's
> introduce a global list of VFIODevices simplifying that scan.

Maybe we should move the qemu_register_reset() when the first device
is added to the list, in vfio_attach_device() ?

Thanks,

C.

> This will also be useful while migrating to IOMMUFD by hiding the
> group specificity.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Suggested-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>   include/hw/vfio/vfio-common.h |  2 ++
>   hw/vfio/common.c              | 45 +++++++++++++++--------------------
>   2 files changed, 21 insertions(+), 26 deletions(-)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index bf12e40667..54905b9dd4 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -131,6 +131,7 @@ typedef struct VFIODeviceOps VFIODeviceOps;
>   typedef struct VFIODevice {
>       QLIST_ENTRY(VFIODevice) next;
>       QLIST_ENTRY(VFIODevice) container_next;
> +    QLIST_ENTRY(VFIODevice) global_next;
>       struct VFIOGroup *group;
>       VFIOContainer *container;
>       char *sysfsdev;
> @@ -232,6 +233,7 @@ int vfio_kvm_device_del_fd(int fd, Error **errp);
>   
>   extern const MemoryRegionOps vfio_region_ops;
>   typedef QLIST_HEAD(VFIOGroupList, VFIOGroup) VFIOGroupList;
> +typedef QLIST_HEAD(VFIODeviceList, VFIODevice) VFIODeviceList;
>   extern VFIOGroupList vfio_group_list;
>   
>   bool vfio_mig_active(void);
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 55f8a113ea..95bc50bcda 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -48,6 +48,8 @@
>   
>   VFIOGroupList vfio_group_list =
>       QLIST_HEAD_INITIALIZER(vfio_group_list);
> +static VFIODeviceList vfio_device_list =
> +    QLIST_HEAD_INITIALIZER(vfio_device_list);
>   static QLIST_HEAD(, VFIOAddressSpace) vfio_address_spaces =
>       QLIST_HEAD_INITIALIZER(vfio_address_spaces);
>   
> @@ -94,18 +96,15 @@ static int vfio_get_dirty_bitmap(VFIOContainer *container, uint64_t iova,
>   
>   bool vfio_mig_active(void)
>   {
> -    VFIOGroup *group;
>       VFIODevice *vbasedev;
>   
> -    if (QLIST_EMPTY(&vfio_group_list)) {
> +    if (QLIST_EMPTY(&vfio_device_list)) {
>           return false;
>       }
>   
> -    QLIST_FOREACH(group, &vfio_group_list, next) {
> -        QLIST_FOREACH(vbasedev, &group->device_list, next) {
> -            if (vbasedev->migration_blocker) {
> -                return false;
> -            }
> +    QLIST_FOREACH(vbasedev, &vfio_device_list, next) {
> +        if (vbasedev->migration_blocker) {
> +            return false;
>           }
>       }
>       return true;
> @@ -120,19 +119,16 @@ static Error *multiple_devices_migration_blocker;
>    */
>   static bool vfio_multiple_devices_migration_is_supported(void)
>   {
> -    VFIOGroup *group;
>       VFIODevice *vbasedev;
>       unsigned int device_num = 0;
>       bool all_support_p2p = true;
>   
> -    QLIST_FOREACH(group, &vfio_group_list, next) {
> -        QLIST_FOREACH(vbasedev, &group->device_list, next) {
> -            if (vbasedev->migration) {
> -                device_num++;
> +    QLIST_FOREACH(vbasedev, &vfio_device_list, next) {
> +        if (vbasedev->migration) {
> +            device_num++;
>   
> -                if (!(vbasedev->migration->mig_flags & VFIO_MIGRATION_P2P)) {
> -                    all_support_p2p = false;
> -                }
> +            if (!(vbasedev->migration->mig_flags & VFIO_MIGRATION_P2P)) {
> +                all_support_p2p = false;
>               }
>           }
>       }
> @@ -1777,22 +1773,17 @@ bool vfio_get_info_dma_avail(struct 
> vfio_iommu_type1_info *info,
>   
>   void vfio_reset_handler(void *opaque)
>   {
> -    VFIOGroup *group;
>       VFIODevice *vbasedev;
>   
> -    QLIST_FOREACH(group, &vfio_group_list, next) {
> -        QLIST_FOREACH(vbasedev, &group->device_list, next) {
> -            if (vbasedev->dev->realized) {
> -                vbasedev->ops->vfio_compute_needs_reset(vbasedev);
> -            }
> +    QLIST_FOREACH(vbasedev, &vfio_device_list, next) {
> +        if (vbasedev->dev->realized) {
> +            vbasedev->ops->vfio_compute_needs_reset(vbasedev);
>           }
>       }
>   
> -    QLIST_FOREACH(group, &vfio_group_list, next) {
> -        QLIST_FOREACH(vbasedev, &group->device_list, next) {
> -            if (vbasedev->dev->realized && vbasedev->needs_reset) {
> -                vbasedev->ops->vfio_hot_reset_multi(vbasedev);
> -            }
> +    QLIST_FOREACH(vbasedev, &vfio_device_list, next) {
> +        if (vbasedev->dev->realized && vbasedev->needs_reset) {
> +            vbasedev->ops->vfio_hot_reset_multi(vbasedev);
>           }
>       }
>   }
> @@ -2657,6 +2648,7 @@ int vfio_attach_device(char *name, VFIODevice *vbasedev,
>       container = group->container;
>       vbasedev->container = container;
>       QLIST_INSERT_HEAD(&container->device_list, vbasedev, container_next);
> +    QLIST_INSERT_HEAD(&vfio_device_list, vbasedev, global_next);
>   
>       return ret;
>   }
> @@ -2669,6 +2661,7 @@ void vfio_detach_device(VFIODevice *vbasedev)
>           return;
>       }
>   
> +    QLIST_REMOVE(vbasedev, global_next);
>       QLIST_REMOVE(vbasedev, container_next);
>       vbasedev->container = NULL;
>       trace_vfio_detach_device(vbasedev->name, group->groupid);