Re: [PATCH v4 10/41] vfio/common: Move giommu_list in base container

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

On 11/2/23 08:12, Zhenzhong Duan wrote:
> From: Eric Auger <eric.auger@redhat.com>
> 
> Move the giommu_list field in the base container and store
> the base container in the VFIOGuestIOMMU.
> 
> No functional change intended.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   include/hw/vfio/vfio-common.h         |  9 ---------
>   include/hw/vfio/vfio-container-base.h |  9 +++++++++
>   hw/vfio/common.c                      | 17 +++++++++++------
>   hw/vfio/container-base.c              |  9 +++++++++
>   hw/vfio/container.c                   |  8 --------
>   5 files changed, 29 insertions(+), 23 deletions(-)


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> 
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 24a26345e5..6be082b8f2 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -95,7 +95,6 @@ typedef struct VFIOContainer {
>       uint64_t max_dirty_bitmap_size;
>       unsigned long pgsizes;
>       unsigned int dma_max_mappings;
> -    QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
>       QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
>       QLIST_HEAD(, VFIOGroup) group_list;
>       QLIST_HEAD(, VFIORamDiscardListener) vrdl_list;
> @@ -104,14 +103,6 @@ typedef struct VFIOContainer {
>       GList *iova_ranges;
>   } VFIOContainer;
>   
> -typedef struct VFIOGuestIOMMU {
> -    VFIOContainer *container;
> -    IOMMUMemoryRegion *iommu_mr;
> -    hwaddr iommu_offset;
> -    IOMMUNotifier n;
> -    QLIST_ENTRY(VFIOGuestIOMMU) giommu_next;
> -} VFIOGuestIOMMU;
> -
>   typedef struct VFIORamDiscardListener {
>       VFIOContainer *container;
>       MemoryRegion *mr;
> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
> index 577f52ccbc..a11aec5755 100644
> --- a/include/hw/vfio/vfio-container-base.h
> +++ b/include/hw/vfio/vfio-container-base.h
> @@ -29,8 +29,17 @@ typedef struct {
>    */
>   typedef struct VFIOContainerBase {
>       const VFIOIOMMUOps *ops;
> +    QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
>   } VFIOContainerBase;
>   
> +typedef struct VFIOGuestIOMMU {
> +    VFIOContainerBase *bcontainer;
> +    IOMMUMemoryRegion *iommu_mr;
> +    hwaddr iommu_offset;
> +    IOMMUNotifier n;
> +    QLIST_ENTRY(VFIOGuestIOMMU) giommu_next;
> +} VFIOGuestIOMMU;
> +
>   int vfio_container_dma_map(VFIOContainerBase *bcontainer,
>                              hwaddr iova, ram_addr_t size,
>                              void *vaddr, bool readonly);
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index e610771888..43580bcc43 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -292,7 +292,7 @@ static bool vfio_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
>   static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
>   {
>       VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
> -    VFIOContainerBase *bcontainer = &giommu->container->bcontainer;
> +    VFIOContainerBase *bcontainer = giommu->bcontainer;
>       hwaddr iova = iotlb->iova + giommu->iommu_offset;
>       void *vaddr;
>       int ret;
> @@ -569,6 +569,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
>                                        MemoryRegionSection *section)
>   {
>       VFIOContainer *container = container_of(listener, VFIOContainer, listener);
> +    VFIOContainerBase *bcontainer = &container->bcontainer;
>       hwaddr iova, end;
>       Int128 llend, llsize;
>       void *vaddr;
> @@ -612,7 +613,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
>           giommu->iommu_mr = iommu_mr;
>           giommu->iommu_offset = section->offset_within_address_space -
>                                  section->offset_within_region;
> -        giommu->container = container;
> +        giommu->bcontainer = bcontainer;
>           llend = int128_add(int128_make64(section->offset_within_region),
>                              section->size);
>           llend = int128_sub(llend, int128_one());
> @@ -647,7 +648,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
>               g_free(giommu);
>               goto fail;
>           }
> -        QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next);
> +        QLIST_INSERT_HEAD(&bcontainer->giommu_list, giommu, giommu_next);
>           memory_region_iommu_replay(giommu->iommu_mr, &giommu->n);
>   
>           return;
> @@ -732,6 +733,7 @@ static void vfio_listener_region_del(MemoryListener *listener,
>                                        MemoryRegionSection *section)
>   {
>       VFIOContainer *container = container_of(listener, VFIOContainer, listener);
> +    VFIOContainerBase *bcontainer = &container->bcontainer;
>       hwaddr iova, end;
>       Int128 llend, llsize;
>       int ret;
> @@ -744,7 +746,7 @@ static void vfio_listener_region_del(MemoryListener *listener,
>       if (memory_region_is_iommu(section->mr)) {
>           VFIOGuestIOMMU *giommu;
>   
> -        QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) {
> +        QLIST_FOREACH(giommu, &bcontainer->giommu_list, giommu_next) {
>               if (MEMORY_REGION(giommu->iommu_mr) == section->mr &&
>                   giommu->n.start == section->offset_within_region) {
>                   memory_region_unregister_iommu_notifier(section->mr,
> @@ -1206,7 +1208,9 @@ static void vfio_iommu_map_dirty_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
>       vfio_giommu_dirty_notifier *gdn = container_of(n,
>                                                   vfio_giommu_dirty_notifier, n);
>       VFIOGuestIOMMU *giommu = gdn->giommu;
> -    VFIOContainer *container = giommu->container;
> +    VFIOContainerBase *bcontainer = giommu->bcontainer;
> +    VFIOContainer *container = container_of(bcontainer, VFIOContainer,
> +                                            bcontainer);
>       hwaddr iova = iotlb->iova + giommu->iommu_offset;
>       ram_addr_t translated_addr;
>       int ret = -EINVAL;
> @@ -1284,12 +1288,13 @@ static int vfio_sync_ram_discard_listener_dirty_bitmap(VFIOContainer *container,
>   static int vfio_sync_dirty_bitmap(VFIOContainer *container,
>                                     MemoryRegionSection *section)
>   {
> +    VFIOContainerBase *bcontainer = &container->bcontainer;
>       ram_addr_t ram_addr;
>   
>       if (memory_region_is_iommu(section->mr)) {
>           VFIOGuestIOMMU *giommu;
>   
> -        QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) {
> +        QLIST_FOREACH(giommu, &bcontainer->giommu_list, giommu_next) {
>               if (MEMORY_REGION(giommu->iommu_mr) == section->mr &&
>                   giommu->n.start == section->offset_within_region) {
>                   Int128 llend;
> diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
> index e929435751..20bcb9669a 100644
> --- a/hw/vfio/container-base.c
> +++ b/hw/vfio/container-base.c
> @@ -34,8 +34,17 @@ int vfio_container_dma_unmap(VFIOContainerBase *bcontainer,
>   void vfio_container_init(VFIOContainerBase *bcontainer, const VFIOIOMMUOps *ops)
>   {
>       bcontainer->ops = ops;
> +    QLIST_INIT(&bcontainer->giommu_list);
>   }
>   
>   void vfio_container_destroy(VFIOContainerBase *bcontainer)
>   {
> +    VFIOGuestIOMMU *giommu, *tmp;
> +
> +    QLIST_FOREACH_SAFE(giommu, &bcontainer->giommu_list, giommu_next, tmp) {
> +        memory_region_unregister_iommu_notifier(
> +                MEMORY_REGION(giommu->iommu_mr), &giommu->n);
> +        QLIST_REMOVE(giommu, giommu_next);
> +        g_free(giommu);
> +    }
>   }
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 32a0251dd1..133d3c8f5c 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -556,7 +556,6 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
>       container->dirty_pages_supported = false;
>       container->dma_max_mappings = 0;
>       container->iova_ranges = NULL;
> -    QLIST_INIT(&container->giommu_list);
>       QLIST_INIT(&container->vrdl_list);
>       bcontainer = &container->bcontainer;
>       vfio_container_init(bcontainer, &vfio_legacy_ops);
> @@ -686,16 +685,9 @@ static void vfio_disconnect_container(VFIOGroup *group)
>   
>       if (QLIST_EMPTY(&container->group_list)) {
>           VFIOAddressSpace *space = container->space;
> -        VFIOGuestIOMMU *giommu, *tmp;
>   
>           QLIST_REMOVE(container, next);
>   
> -        QLIST_FOREACH_SAFE(giommu, &container->giommu_list, giommu_next, tmp) {
> -            memory_region_unregister_iommu_notifier(
> -                    MEMORY_REGION(giommu->iommu_mr), &giommu->n);
> -            QLIST_REMOVE(giommu, giommu_next);
> -            g_free(giommu);
> -        }
>           vfio_container_destroy(bcontainer);
>   
>           trace_vfio_disconnect_container(container->fd);