Re: [PATCH v3 00/37] vfio: Spring cleanup

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

On 4/22/25 18:01, Cédric Le Goater wrote:
> Hello,
> 
> Several large extensions were merged in VFIO recently: migration
> support with dirty tracking, support for different host IOMMU backend
> devices, multifd support, etc. This adds up to the previous
> extensions: vfio-platform, AP, CCW. The result is that VFIO is now a
> subsystem of over +16,000 lines of code :
> 
>    QEMU 2.0  :   3988 total
>                   ...
>    QEMU 10.0 :  16607 total
> 
> Organization is weak, naming inconsistent, the vfio-common.h header
> file and common.c are quite messy. It's time to address the technical
> debt before adding new features.
> 
> This proposal reorganizes some of the VFIO files to isolate features,
> introduces new files and renames services to better reflect the
> namespace they belong to. This is code reshuffling and there are no
> intentional functional changes. If more could be done, please propose !
> Timing seems right.
> 
> 
> I have taken care to preserve all existing copyright notices in the
> file headers. I have added one on behalf of my current employer for
> newly created files. However, original authors may wish to include
> their own notices as well. If so, please respond to the patch, and I
> will update the patch in the next spin or before applying.
> 
> Here is a (short) list requiring your attention :
>    
>    * include/hw/vfio/vfio-migration.h
>    * hw/vfio/vfio-migration-internal.h
>      Kirti Wankhede and Avihai Horon, NVIDIA
>    
>    * hw/vfio/vfio-iommufd.h
>      Joao Martins and Yi Liu, Oracle and Intel
>    
>    * include/hw/vfio/vfio-region.h
>    * hw/vfio/region.c
>      Eric Auger, may be we could reduce the list ?
> 
> I think the remaining new files are correctly covered but I am human,
> so please review and let me know.
> 
> I hope we can merge this when the QEMU 10.1 cycle starts and then
> address the two large series waiting : live update and vfio-user.
> 
> 
> What next in terms of cleanups :
> 
>   - container.c is quite messy
>   - the pci* files need some love too (add vfio- prefix ?)
>   - improve overall documentation, structs and routines documentation
>     would be great
>   - isolate all the low level routines (kvm ioctls) into helpers.c
>     to improve build ?
>   - improve build to reduce the number of files built per target
>     https://lore.kernel.org/qemu-devel/20250308230917.18907-1-philmd@linaro.org
>   - continue the never ending quest of adding 'Error **' parameters,
>     Look for migration_file_set_error, in MemoryListener handlers :
>        vfio_listener_region_add
>        vfio_listener_log_global_stop
>        vfio_listener_log_sync
>     and in callback routines for IOMMU notifiers :
>        vfio_iommu_map_notify
>        vfio_iommu_map_dirty_notify
>     memory_region_iommu_replay() would be a start.
>   - remove vfio-platform (start of QEMU 10.2 cycle ~ September 2025)
> 
> Thanks,
> 
> C.
> 
> 
> Changes in v3:
> 
>   - Fixed commit log typos
>   - Removed hw/vfio/vfio-migration.h include from
>     hw/vfio/migration-multifd.c
>   - Added hw/vfio/vfio-region.h include in hw/vfio/pci.h
>   - Adjusted Copyright in hw/vfio/vfio-cpr.h
>   - Renamed vfio_container_vioc_query_dirty_bitmap() to
>     vfio_container_iommu_query_dirty_bitmap()
>   - Improved hw/vfio/vfio-device.h header description
>   - Fixed code alignment when renaming VFIODevice related services
>   
> Changes in v2:
> 
>   - Dropped vfio_migration_set_error()
>     https://lore.kernel.org/qemu-devel/20250324123315.637827-1-clg@redhat.com/
>   - Dropped R-b trailers on patches which were modified too much
>     (context changes are ok)
>   - Improved commit logs of patches adding new files
>   - Fixed top comment in header files
>   - Used a 'vfio_migration_' prefix instead of 'vfio_mig_'
>   - Made vfio_migration_add_bytes_transferred() internal
>   - Added extra patch for vfio_device_state_is_running/precopy()
>   - Moved vfio_reset_handler() in device.c
>   - Moved "dirty tracking" related services into container-base.c and
>     improved naming
>   - Introduced listener.* files instead of dirty-tracking.*
>   - Introduced vfio_listener_un/register() routines
> 
> 
> Cédric Le Goater (37):
>    vfio: Move vfio_mig_active() into migration.c
>    vfio: Rename vfio_reset_bytes_transferred()
>    vfio: Introduce a new header file for external migration services
>    vfio: Make vfio_un/block_multiple_devices_migration() static
>    vfio: Make vfio_viommu_preset() static
>    vfio: Introduce a new header file for internal migration services
>    vfio: Move vfio_device_state_is_running/precopy() into migration.c
>    vfio: Introduce a new header file for VFIOdisplay declarations
>    vfio: Move VFIOHostDMAWindow definition into spapr.c
>    vfio: Introduce a new header file for VFIOIOMMUFD declarations
>    vfio: Introduce new files for VFIORegion definitions and declarations
>    vfio: Introduce a new header file for VFIOcontainer declarations
>    vfio: Make vfio_group_list static
>    vfio: Move VFIOAddressSpace helpers into container-base.c
>    vfio: Move Host IOMMU type declarations into their respective files
>    vfio: Introduce a new header file for helper services
>    vfio: Move vfio_get_info_dma_avail() into helpers.c
>    vfio: Move vfio_kvm_device_add/del_fd() to helpers.c
>    vfio: Move vfio_get_device_info() to helpers.c
>    vfio: Introduce a new file for VFIODevice definitions
>    vfio: Introduce new files for CPR definitions and declarations
>    vfio: Move vfio_kvm_device_fd() into helpers.c
>    vfio: Move vfio_device_list into device.c
>    vfio: Move vfio_de/attach_device() into device.c
>    vfio: Move vfio_reset_handler() into device.c
>    vfio: Move dirty tracking related services into container-base.c
>    vfio: Make vfio_devices_query_dirty_bitmap() static
>    vfio: Make vfio_container_query_dirty_bitmap() static
>    vfio: Rename vfio_devices_all_dirty_tracking_started()
>    vfio: Rename vfio_devices_all_device_dirty_tracking()
>    vfio: Rename vfio_get_dirty_bitmap()
>    vfio: Introduce new files for VFIO MemoryListener
>    vfio: Rename RAM discard related services
>    vfio: Introduce vfio_listener_un/register() routines
>    vfio: Rename vfio-common.h to vfio-device.h
>    vfio: Rename VFIODevice related services
>    vfio: Rename VFIOContainer related services
> 
>   hw/vfio/migration-multifd.h           |   2 +-
>   hw/vfio/pci.h                         |   4 +-
>   hw/vfio/vfio-cpr.h                    |  15 +
>   hw/vfio/vfio-display.h                |  42 ++
>   hw/vfio/vfio-helpers.h                |  35 ++
>   hw/vfio/vfio-iommufd.h                |  34 ++
>   hw/vfio/vfio-listener.h               |  15 +
>   hw/vfio/vfio-migration-internal.h     |  74 +++
>   include/hw/s390x/vfio-ccw.h           |   2 +-
>   include/hw/vfio/vfio-common.h         | 346 -------------
>   include/hw/vfio/vfio-container-base.h |  12 +-
>   include/hw/vfio/vfio-container.h      |  36 ++
>   include/hw/vfio/vfio-device.h         | 149 ++++++
>   include/hw/vfio/vfio-migration.h      |  16 +
>   include/hw/vfio/vfio-platform.h       |   4 +-
>   include/hw/vfio/vfio-region.h         |  47 ++
>   backends/iommufd.c                    |   2 +-
>   hw/core/sysbus-fdt.c                  |   1 +
>   hw/ppc/spapr_pci_vfio.c               |   6 +-
>   hw/s390x/s390-pci-vfio.c              |   3 +-
>   hw/vfio/ap.c                          |  14 +-
>   hw/vfio/ccw.c                         |  30 +-
>   hw/vfio/container-base.c              | 192 ++++++-
>   hw/vfio/container.c                   | 123 ++---
>   hw/vfio/cpr.c                         |   3 +-
>   hw/vfio/device.c                      | 405 +++++++++++++++
>   hw/vfio/display.c                     |  10 +-
>   hw/vfio/helpers.c                     | 702 +++-----------------------
>   hw/vfio/igd.c                         |  10 +-
>   hw/vfio/iommufd.c                     |  24 +-
>   hw/vfio/{common.c => listener.c}      | 455 +----------------
>   hw/vfio/migration-multifd.c           |   7 +-
>   hw/vfio/migration.c                   | 111 +++-
>   hw/vfio/pci.c                         |  70 +--
>   hw/vfio/platform.c                    |  15 +-
>   hw/vfio/region.c                      | 395 +++++++++++++++
>   hw/vfio/spapr.c                       |  10 +-
>   migration/target.c                    |   8 +-
>   hw/vfio/meson.build                   |  10 +-
>   hw/vfio/trace-events                  |  36 +-
>   40 files changed, 1875 insertions(+), 1600 deletions(-)
>   create mode 100644 hw/vfio/vfio-cpr.h
>   create mode 100644 hw/vfio/vfio-display.h
>   create mode 100644 hw/vfio/vfio-helpers.h
>   create mode 100644 hw/vfio/vfio-iommufd.h
>   create mode 100644 hw/vfio/vfio-listener.h
>   create mode 100644 hw/vfio/vfio-migration-internal.h
>   delete mode 100644 include/hw/vfio/vfio-common.h
>   create mode 100644 include/hw/vfio/vfio-container.h
>   create mode 100644 include/hw/vfio/vfio-device.h
>   create mode 100644 include/hw/vfio/vfio-migration.h
>   create mode 100644 include/hw/vfio/vfio-region.h
>   create mode 100644 hw/vfio/device.c
>   rename hw/vfio/{common.c => listener.c} (76%)
>   create mode 100644 hw/vfio/region.c
> 

Rebased on HEAD and applied to vfio-next.

Thanks,

C.