On 19/03/2025 13:24, Joao Martins wrote:
> On 18/03/2025 09:54, Cédric Le Goater wrote:
>> This hides the MemoryListener implementation and makes the code common
>> to both IOMMU backends, legacy and IOMMUFD.
>>
>> Signed-off-by: Cédric Le Goater <c...@redhat.com>
>
> Reviewed-by: Joao Martins <joao.m.mart...@oracle.com>
>
After discussing with Avihai, maybe the routine should be representative on what
it does and not so much on the namespace i.e.
vfio_dirty_tracking_register -> vfio_memory_register
vfio_dirty_tracking_unregister -> vfio_memory_unregister
Given that these have nothing to do with dirty tracking.
In terms of memory listeners the only function relevant in this area is:
vfio_dirty_tracking_init
vfio_dirty_tracking_update
Which have different purpose where we parse the memory ranges to construct 32/64
bit ranges for VF trackers.
>> ---
>> hw/vfio/dirty-tracking.h | 4 ++--
>> hw/vfio/container.c | 11 +++--------
>> hw/vfio/dirty-tracking.c | 21 ++++++++++++++++++++-
>> hw/vfio/iommufd.c | 9 ++-------
>> 4 files changed, 27 insertions(+), 18 deletions(-)
>>
>> diff --git a/hw/vfio/dirty-tracking.h b/hw/vfio/dirty-tracking.h
>> index
>> db9494202a780108ce78b642950bfed78ba3f253..6d717f0e918e47e341114c82ffc2cf520fc7b079
>> 100644
>> --- a/hw/vfio/dirty-tracking.h
>> +++ b/hw/vfio/dirty-tracking.h
>> @@ -9,11 +9,11 @@
>> #ifndef HW_VFIO_DIRTY_TRACKING_H
>> #define HW_VFIO_DIRTY_TRACKING_H
>>
>> -extern const MemoryListener vfio_memory_listener;
>> -
>> bool vfio_dirty_tracking_devices_is_started(const VFIOContainerBase
>> *bcontainer);
>> bool vfio_dirty_tracking_devices_is_supported(const VFIOContainerBase
>> *bcontainer);
>> int vfio_dirty_tracking_query_dirty_bitmap(const VFIOContainerBase
>> *bcontainer, uint64_t iova,
>> uint64_t size, ram_addr_t ram_addr, Error **errp);
>> +bool vfio_dirty_tracking_register(VFIOContainerBase *bcontainer, Error
>> **errp);
>> +void vfio_dirty_tracking_unregister(VFIOContainerBase *bcontainer);
>>
>> #endif /* HW_VFIO_DIRTY_TRACKING_H */
>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>> index
>> 7b3ec798a77052b8cb0b47d3dceaca1428cb50bd..1fcca75caba19353ad3063ae97b20c15f61564e9
>> 100644
>> --- a/hw/vfio/container.c
>> +++ b/hw/vfio/container.c
>> @@ -616,12 +616,7 @@ static bool vfio_container_connect(VFIOGroup *group,
>> AddressSpace *as,
>> group->container = container;
>> QLIST_INSERT_HEAD(&container->group_list, group, container_next);
>>
>> - bcontainer->listener = vfio_memory_listener;
>> - memory_listener_register(&bcontainer->listener, bcontainer->space->as);
>> -
>> - if (bcontainer->error) {
>> - error_propagate_prepend(errp, bcontainer->error,
>> - "memory listener initialization failed: ");
>> + if (!vfio_dirty_tracking_register(bcontainer, errp)) {
>> goto listener_release_exit;
>> }
>>
>> @@ -631,7 +626,7 @@ static bool vfio_container_connect(VFIOGroup *group,
>> AddressSpace *as,
>> listener_release_exit:
>> QLIST_REMOVE(group, container_next);
>> vfio_group_del_kvm_device(group);
>> - memory_listener_unregister(&bcontainer->listener);
>> + vfio_dirty_tracking_unregister(bcontainer);
>> if (vioc->release) {
>> vioc->release(bcontainer);
>> }
>> @@ -669,7 +664,7 @@ static void vfio_container_disconnect(VFIOGroup *group)
>> * group.
>> */
>> if (QLIST_EMPTY(&container->group_list)) {
>> - memory_listener_unregister(&bcontainer->listener);
>> + vfio_dirty_tracking_unregister(bcontainer);
>> if (vioc->release) {
>> vioc->release(bcontainer);
>> }
>> diff --git a/hw/vfio/dirty-tracking.c b/hw/vfio/dirty-tracking.c
>> index
>> 8e47ccbb9aea748e57271508ddcd10e394abf16c..d7827f7b64adf3e2b41fafd59aab71e0b28c1567
>> 100644
>> --- a/hw/vfio/dirty-tracking.c
>> +++ b/hw/vfio/dirty-tracking.c
>> @@ -1267,7 +1267,7 @@ static void vfio_listener_log_sync(MemoryListener
>> *listener,
>> }
>> }
>>
>> -const MemoryListener vfio_memory_listener = {
>> +static const MemoryListener vfio_memory_listener = {
>> .name = "vfio",
>> .region_add = vfio_listener_region_add,
>> .region_del = vfio_listener_region_del,
>> @@ -1275,3 +1275,22 @@ const MemoryListener vfio_memory_listener = {
>> .log_global_stop = vfio_listener_log_global_stop,
>> .log_sync = vfio_listener_log_sync,
>> };
>> +
>> +bool vfio_dirty_tracking_register(VFIOContainerBase *bcontainer, Error
>> **errp)
>> +{
>> + bcontainer->listener = vfio_memory_listener;
>> + memory_listener_register(&bcontainer->listener, bcontainer->space->as);
>> +
>> + if (bcontainer->error) {
>> + error_propagate_prepend(errp, bcontainer->error,
>> + "memory listener initialization failed: ");
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +void vfio_dirty_tracking_unregister(VFIOContainerBase *bcontainer)
>> +{
>> + memory_listener_unregister(&bcontainer->listener);
>> +}
>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>> index
>> 7f354d86cd14270a70dc990860ad5b69f0310719..7737d552f310c54ae2e035198a1a83da8c3199dd
>> 100644
>> --- a/hw/vfio/iommufd.c
>> +++ b/hw/vfio/iommufd.c
>> @@ -411,7 +411,7 @@ static void
>> iommufd_cdev_container_destroy(VFIOIOMMUFDContainer *container)
>> if (!QLIST_EMPTY(&bcontainer->device_list)) {
>> return;
>> }
>> - memory_listener_unregister(&bcontainer->listener);
>> + vfio_dirty_tracking_unregister(bcontainer);
>> iommufd_backend_free_id(container->be, container->ioas_id);
>> object_unref(container);
>> }
>> @@ -563,12 +563,7 @@ static bool iommufd_cdev_attach(const char *name,
>> VFIODevice *vbasedev,
>> bcontainer->pgsizes = qemu_real_host_page_size();
>> }
>>
>> - bcontainer->listener = vfio_memory_listener;
>> - memory_listener_register(&bcontainer->listener, bcontainer->space->as);
>> -
>> - if (bcontainer->error) {
>> - error_propagate_prepend(errp, bcontainer->error,
>> - "memory listener initialization failed: ");
>> + if (!vfio_dirty_tracking_register(bcontainer, errp)) {
>> goto err_listener_register;
>> }
>>
>
>
