diff mbox series

[v3,14/37] vfio/container: Move vrdl_list, pgsizes and dma_max_mappings to base container

Message ID 20231026103104.1686921-15-zhenzhong.duan@intel.com
State New
Headers show
Series vfio: Adopt iommufd | expand

Commit Message

Zhenzhong Duan Oct. 26, 2023, 10:30 a.m. UTC 
From: Eric Auger <eric.auger@redhat.com>

Move vrdl_list, pgsizes and dma_max_mappings to the base
container object

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>
[ clg: context changes ]
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 include/hw/vfio/vfio-common.h         | 13 -------
 include/hw/vfio/vfio-container-base.h | 13 +++++++
 hw/vfio/common.c                      | 49 ++++++++++++++-------------
 hw/vfio/container-base.c              | 12 +++++++
 hw/vfio/container.c                   | 12 +++----
 hw/vfio/spapr.c                       | 18 +++++++---
 6 files changed, 68 insertions(+), 49 deletions(-)

Comments

Cédric Le Goater Oct. 27, 2023, 3:52 p.m. UTC | #1 
On 10/26/23 12:30, Zhenzhong Duan wrote:
> From: Eric Auger <eric.auger@redhat.com>
> 
> Move vrdl_list, pgsizes and dma_max_mappings to the base
> container object
> 
> 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>
> [ clg: context changes ]
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>   include/hw/vfio/vfio-common.h         | 13 -------
>   include/hw/vfio/vfio-container-base.h | 13 +++++++
>   hw/vfio/common.c                      | 49 ++++++++++++++-------------
>   hw/vfio/container-base.c              | 12 +++++++
>   hw/vfio/container.c                   | 12 +++----
>   hw/vfio/spapr.c                       | 18 +++++++---
>   6 files changed, 68 insertions(+), 49 deletions(-)
> 
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index fb3c7aea8f..65ae2d76cf 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -85,24 +85,11 @@ typedef struct VFIOContainer {
>       bool initialized;
>       uint64_t dirty_pgsizes;
>       uint64_t max_dirty_bitmap_size;
> -    unsigned long pgsizes;
> -    unsigned int dma_max_mappings;
>       QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
>       QLIST_HEAD(, VFIOGroup) group_list;
> -    QLIST_HEAD(, VFIORamDiscardListener) vrdl_list;
>       GList *iova_ranges;
>   } VFIOContainer;
>   
> -typedef struct VFIORamDiscardListener {
> -    VFIOContainer *container;
> -    MemoryRegion *mr;
> -    hwaddr offset_within_address_space;
> -    hwaddr size;
> -    uint64_t granularity;
> -    RamDiscardListener listener;
> -    QLIST_ENTRY(VFIORamDiscardListener) next;
> -} VFIORamDiscardListener;
> -
>   typedef struct VFIOHostDMAWindow {
>       hwaddr min_iova;
>       hwaddr max_iova;
> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
> index f1de1ef120..849c8b34b2 100644
> --- a/include/hw/vfio/vfio-container-base.h
> +++ b/include/hw/vfio/vfio-container-base.h
> @@ -50,8 +50,11 @@ typedef struct VFIOAddressSpace {
>   typedef struct VFIOContainerBase {
>       const VFIOIOMMUOps *ops;
>       VFIOAddressSpace *space;
> +    unsigned long pgsizes;
> +    unsigned int dma_max_mappings;
>       bool dirty_pages_supported;
>       QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
> +    QLIST_HEAD(, VFIORamDiscardListener) vrdl_list;
>       QLIST_ENTRY(VFIOContainerBase) next;
>       QLIST_HEAD(, VFIODevice) device_list;
>   } VFIOContainerBase;
> @@ -64,6 +67,16 @@ typedef struct VFIOGuestIOMMU {
>       QLIST_ENTRY(VFIOGuestIOMMU) giommu_next;
>   } VFIOGuestIOMMU;
>   
> +typedef struct VFIORamDiscardListener {
> +    VFIOContainerBase *bcontainer;
> +    MemoryRegion *mr;
> +    hwaddr offset_within_address_space;
> +    hwaddr size;
> +    uint64_t granularity;
> +    RamDiscardListener listener;
> +    QLIST_ENTRY(VFIORamDiscardListener) next;
> +} VFIORamDiscardListener;
> +
>   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 91411d9844..9b34e7e0f8 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -351,13 +351,13 @@ static void vfio_ram_discard_notify_discard(RamDiscardListener *rdl,
>   {
>       VFIORamDiscardListener *vrdl = container_of(rdl, VFIORamDiscardListener,
>                                                   listener);
> +    VFIOContainerBase *bcontainer = vrdl->bcontainer;
>       const hwaddr size = int128_get64(section->size);
>       const hwaddr iova = section->offset_within_address_space;
>       int ret;
>   
>       /* Unmap with a single call. */
> -    ret = vfio_container_dma_unmap(&vrdl->container->bcontainer,
> -                                   iova, size , NULL);
> +    ret = vfio_container_dma_unmap(bcontainer, iova, size , NULL);
>       if (ret) {
>           error_report("%s: vfio_container_dma_unmap() failed: %s", __func__,
>                        strerror(-ret));
> @@ -369,6 +369,7 @@ static int vfio_ram_discard_notify_populate(RamDiscardListener *rdl,
>   {
>       VFIORamDiscardListener *vrdl = container_of(rdl, VFIORamDiscardListener,
>                                                   listener);
> +    VFIOContainerBase *bcontainer = vrdl->bcontainer;
>       const hwaddr end = section->offset_within_region +
>                          int128_get64(section->size);
>       hwaddr start, next, iova;
> @@ -387,8 +388,8 @@ static int vfio_ram_discard_notify_populate(RamDiscardListener *rdl,
>                  section->offset_within_address_space;
>           vaddr = memory_region_get_ram_ptr(section->mr) + start;
>   
> -        ret = vfio_container_dma_map(&vrdl->container->bcontainer, iova,
> -                                     next - start, vaddr, section->readonly);
> +        ret = vfio_container_dma_map(bcontainer, iova, next - start,
> +                                     vaddr, section->readonly);
>           if (ret) {
>               /* Rollback */
>               vfio_ram_discard_notify_discard(rdl, section);
> @@ -398,7 +399,7 @@ static int vfio_ram_discard_notify_populate(RamDiscardListener *rdl,
>       return 0;
>   }
>   
> -static void vfio_register_ram_discard_listener(VFIOContainer *container,
> +static void vfio_register_ram_discard_listener(VFIOContainerBase *bcontainer,
>                                                  MemoryRegionSection *section)
>   {
>       RamDiscardManager *rdm = memory_region_get_ram_discard_manager(section->mr);
> @@ -411,7 +412,7 @@ static void vfio_register_ram_discard_listener(VFIOContainer *container,
>       g_assert(QEMU_IS_ALIGNED(int128_get64(section->size), TARGET_PAGE_SIZE));
>   
>       vrdl = g_new0(VFIORamDiscardListener, 1);
> -    vrdl->container = container;
> +    vrdl->bcontainer = bcontainer;
>       vrdl->mr = section->mr;
>       vrdl->offset_within_address_space = section->offset_within_address_space;
>       vrdl->size = int128_get64(section->size);
> @@ -419,14 +420,14 @@ static void vfio_register_ram_discard_listener(VFIOContainer *container,
>                                                                   section->mr);
>   
>       g_assert(vrdl->granularity && is_power_of_2(vrdl->granularity));
> -    g_assert(container->pgsizes &&
> -             vrdl->granularity >= 1ULL << ctz64(container->pgsizes));
> +    g_assert(bcontainer->pgsizes &&
> +             vrdl->granularity >= 1ULL << ctz64(bcontainer->pgsizes));
>   
>       ram_discard_listener_init(&vrdl->listener,
>                                 vfio_ram_discard_notify_populate,
>                                 vfio_ram_discard_notify_discard, true);
>       ram_discard_manager_register_listener(rdm, &vrdl->listener, section);
> -    QLIST_INSERT_HEAD(&container->vrdl_list, vrdl, next);
> +    QLIST_INSERT_HEAD(&bcontainer->vrdl_list, vrdl, next);
>   
>       /*
>        * Sanity-check if we have a theoretically problematic setup where we could
> @@ -441,7 +442,7 @@ static void vfio_register_ram_discard_listener(VFIOContainer *container,
>        * number of sections in the address space we could have over time,
>        * also consuming DMA mappings.
>        */
> -    if (container->dma_max_mappings) {
> +    if (bcontainer->dma_max_mappings) {
>           unsigned int vrdl_count = 0, vrdl_mappings = 0, max_memslots = 512;
>   
>   #ifdef CONFIG_KVM
> @@ -450,7 +451,7 @@ static void vfio_register_ram_discard_listener(VFIOContainer *container,
>           }
>   #endif
>   
> -        QLIST_FOREACH(vrdl, &container->vrdl_list, next) {
> +        QLIST_FOREACH(vrdl, &bcontainer->vrdl_list, next) {
>               hwaddr start, end;
>   
>               start = QEMU_ALIGN_DOWN(vrdl->offset_within_address_space,
> @@ -462,23 +463,23 @@ static void vfio_register_ram_discard_listener(VFIOContainer *container,
>           }
>   
>           if (vrdl_mappings + max_memslots - vrdl_count >
> -            container->dma_max_mappings) {
> +            bcontainer->dma_max_mappings) {
>               warn_report("%s: possibly running out of DMA mappings. E.g., try"
>                           " increasing the 'block-size' of virtio-mem devies."
>                           " Maximum possible DMA mappings: %d, Maximum possible"
> -                        " memslots: %d", __func__, container->dma_max_mappings,
> +                        " memslots: %d", __func__, bcontainer->dma_max_mappings,
>                           max_memslots);
>           }
>       }
>   }
>   
> -static void vfio_unregister_ram_discard_listener(VFIOContainer *container,
> +static void vfio_unregister_ram_discard_listener(VFIOContainerBase *bcontainer,
>                                                    MemoryRegionSection *section)
>   {
>       RamDiscardManager *rdm = memory_region_get_ram_discard_manager(section->mr);
>       VFIORamDiscardListener *vrdl = NULL;
>   
> -    QLIST_FOREACH(vrdl, &container->vrdl_list, next) {
> +    QLIST_FOREACH(vrdl, &bcontainer->vrdl_list, next) {
>           if (vrdl->mr == section->mr &&
>               vrdl->offset_within_address_space ==
>               section->offset_within_address_space) {
> @@ -627,7 +628,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
>                               iommu_idx);
>   
>           ret = memory_region_iommu_set_page_size_mask(giommu->iommu_mr,
> -                                                     container->pgsizes,
> +                                                     bcontainer->pgsizes,
>                                                        &err);
>           if (ret) {
>               g_free(giommu);
> @@ -663,7 +664,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
>        * about changes.
>        */
>       if (memory_region_has_ram_discard_manager(section->mr)) {
> -        vfio_register_ram_discard_listener(container, section);
> +        vfio_register_ram_discard_listener(bcontainer, section);
>           return;
>       }
>   
> @@ -782,7 +783,7 @@ static void vfio_listener_region_del(MemoryListener *listener,
>           pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1;
>           try_unmap = !((iova & pgmask) || (int128_get64(llsize) & pgmask));
>       } else if (memory_region_has_ram_discard_manager(section->mr)) {
> -        vfio_unregister_ram_discard_listener(container, section);
> +        vfio_unregister_ram_discard_listener(bcontainer, section);
>           /* Unregistering will trigger an unmap. */
>           try_unmap = false;
>       }
> @@ -1261,17 +1262,17 @@ static int vfio_ram_discard_get_dirty_bitmap(MemoryRegionSection *section,
>        * Sync the whole mapped region (spanning multiple individual mappings)
>        * in one go.
>        */
> -    return vfio_get_dirty_bitmap(&vrdl->container->bcontainer, iova, size,
> -                                 ram_addr);
> +    return vfio_get_dirty_bitmap(vrdl->bcontainer, iova, size, ram_addr);
>   }
>   
> -static int vfio_sync_ram_discard_listener_dirty_bitmap(VFIOContainer *container,
> -                                                   MemoryRegionSection *section)
> +static int
> +vfio_sync_ram_discard_listener_dirty_bitmap(VFIOContainerBase *bcontainer,
> +                                            MemoryRegionSection *section)
>   {
>       RamDiscardManager *rdm = memory_region_get_ram_discard_manager(section->mr);
>       VFIORamDiscardListener *vrdl = NULL;
>   
> -    QLIST_FOREACH(vrdl, &container->vrdl_list, next) {
> +    QLIST_FOREACH(vrdl, &bcontainer->vrdl_list, next) {
>           if (vrdl->mr == section->mr &&
>               vrdl->offset_within_address_space ==
>               section->offset_within_address_space) {
> @@ -1325,7 +1326,7 @@ static int vfio_sync_dirty_bitmap(VFIOContainer *container,
>           }
>           return 0;
>       } else if (memory_region_has_ram_discard_manager(section->mr)) {
> -        return vfio_sync_ram_discard_listener_dirty_bitmap(container, section);
> +        return vfio_sync_ram_discard_listener_dirty_bitmap(bcontainer, section);
>       }
>   
>       ram_addr = memory_region_get_ram_addr(section->mr) +
> diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
> index a7cf517dd2..568f891841 100644
> --- a/hw/vfio/container-base.c
> +++ b/hw/vfio/container-base.c
> @@ -76,15 +76,27 @@ void vfio_container_init(VFIOContainerBase *bcontainer, VFIOAddressSpace *space,
>       bcontainer->ops = ops;
>       bcontainer->space = space;
>       bcontainer->dirty_pages_supported = false;
> +    bcontainer->dma_max_mappings = 0;
>       QLIST_INIT(&bcontainer->giommu_list);
> +    QLIST_INIT(&bcontainer->vrdl_list);
>   }
>   
>   void vfio_container_destroy(VFIOContainerBase *bcontainer)
>   {
> +    VFIORamDiscardListener *vrdl, *vrdl_tmp;
>       VFIOGuestIOMMU *giommu, *tmp;
>   
>       QLIST_REMOVE(bcontainer, next);
>   
> +    QLIST_FOREACH_SAFE(vrdl, &bcontainer->vrdl_list, next, vrdl_tmp) {
> +        RamDiscardManager *rdm;
> +
> +        rdm = memory_region_get_ram_discard_manager(vrdl->mr);
> +        ram_discard_manager_unregister_listener(rdm, &vrdl->listener);
> +        QLIST_REMOVE(vrdl, next);
> +        g_free(vrdl);
> +    }

Where was this done previously ? May be the vrdl list should be handled
separatly from pgsizes and dma_max_mappings.

>       QLIST_FOREACH_SAFE(giommu, &bcontainer->giommu_list, giommu_next, tmp) {
>           memory_region_unregister_iommu_notifier(
>                   MEMORY_REGION(giommu->iommu_mr), &giommu->n);
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 8d5b408e86..0e265ffa67 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -154,7 +154,7 @@ static int vfio_legacy_dma_unmap(VFIOContainerBase *bcontainer, hwaddr iova,
>           if (errno == EINVAL && unmap.size && !(unmap.iova + unmap.size) &&
>               container->iommu_type == VFIO_TYPE1v2_IOMMU) {
>               trace_vfio_legacy_dma_unmap_overflow_workaround();
> -            unmap.size -= 1ULL << ctz64(container->pgsizes);
> +            unmap.size -= 1ULL << ctz64(container->bcontainer.pgsizes);
>               continue;
>           }
>           error_report("VFIO_UNMAP_DMA failed: %s", strerror(errno));
> @@ -559,9 +559,7 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
>       container = g_malloc0(sizeof(*container));
>       container->fd = fd;
>       container->error = NULL;
> -    container->dma_max_mappings = 0;
>       container->iova_ranges = NULL;
> -    QLIST_INIT(&container->vrdl_list);
>       bcontainer = &container->bcontainer;
>       vfio_container_init(bcontainer, space, &vfio_legacy_ops);
>   
> @@ -589,13 +587,13 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
>           }
>   
>           if (info->flags & VFIO_IOMMU_INFO_PGSIZES) {
> -            container->pgsizes = info->iova_pgsizes;
> +            container->bcontainer.pgsizes = info->iova_pgsizes;
>           } else {
> -            container->pgsizes = qemu_real_host_page_size();
> +            container->bcontainer.pgsizes = qemu_real_host_page_size();
>           }
>   
> -        if (!vfio_get_info_dma_avail(info, &container->dma_max_mappings)) {
> -            container->dma_max_mappings = 65535;
> +        if (!vfio_get_info_dma_avail(info, &bcontainer->dma_max_mappings)) {
> +            container->bcontainer.dma_max_mappings = 65535;
>           }
>   
>           vfio_get_info_iova_range(info, container);
> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> index 3495737ab2..dbc4c24052 100644
> --- a/hw/vfio/spapr.c
> +++ b/hw/vfio/spapr.c
> @@ -223,13 +223,13 @@ static int vfio_spapr_create_window(VFIOContainer *container,
>       if (pagesize > rampagesize) {
>           pagesize = rampagesize;
>       }
> -    pgmask = container->pgsizes & (pagesize | (pagesize - 1));
> +    pgmask = container->bcontainer.pgsizes & (pagesize | (pagesize - 1));
>       pagesize = pgmask ? (1ULL << (63 - clz64(pgmask))) : 0;
>       if (!pagesize) {
>           error_report("Host doesn't support page size 0x%"PRIx64
>                        ", the supported mask is 0x%lx",
>                        memory_region_iommu_get_min_page_size(iommu_mr),
> -                     container->pgsizes);
> +                     container->bcontainer.pgsizes);
>           return -EINVAL;
>       }
>   
> @@ -385,7 +385,7 @@ void vfio_container_del_section_window(VFIOContainer *container,
>   
>   bool vfio_spapr_container_init(VFIOContainer *container, Error **errp)
>   {
> -
> +    VFIOContainerBase *bcontainer = &container->bcontainer;
>       struct vfio_iommu_spapr_tce_info info;
>       bool v2 = container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU;
>       int ret, fd = container->fd;
> @@ -424,7 +424,7 @@ bool vfio_spapr_container_init(VFIOContainer *container, Error **errp)
>       }
>   
>       if (v2) {
> -        container->pgsizes = info.ddw.pgsizes;
> +        bcontainer->pgsizes = info.ddw.pgsizes;
>           /*
>            * There is a default window in just created container.
>            * To make region_add/del simpler, we better remove this
> @@ -439,7 +439,7 @@ bool vfio_spapr_container_init(VFIOContainer *container, Error **errp)
>           }
>       } else {
>           /* The default table uses 4K pages */
> -        container->pgsizes = 0x1000;
> +        bcontainer->pgsizes = 0x1000;
>           vfio_host_win_add(container, info.dma32_window_start,
>                             info.dma32_window_start +
>                             info.dma32_window_size - 1,
> @@ -455,7 +455,15 @@ listener_unregister_exit:
>   
>   void vfio_spapr_container_deinit(VFIOContainer *container)
>   {
> +    VFIOHostDMAWindow *hostwin, *next;
> +
>       if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
>           memory_listener_unregister(&container->prereg_listener);
>       }
> +    QLIST_FOREACH_SAFE(hostwin, &container->hostwin_list, hostwin_next,
> +                       next) {
> +        QLIST_REMOVE(hostwin, hostwin_next);
> +        g_free(hostwin);
> +    }
> +
>   }

I am sure this change  belongs to this patch.

Thanks,

C.
Zhenzhong Duan Oct. 30, 2023, 3:14 a.m. UTC | #2 
Hi Cédric,

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Friday, October 27, 2023 11:52 PM
>Subject: Re: [PATCH v3 14/37] vfio/container: Move vrdl_list, pgsizes and
>dma_max_mappings to base container
>
>On 10/26/23 12:30, Zhenzhong Duan wrote:
>> From: Eric Auger <eric.auger@redhat.com>
>>
...

>>   void vfio_container_destroy(VFIOContainerBase *bcontainer)
>>   {
>> +    VFIORamDiscardListener *vrdl, *vrdl_tmp;
>>       VFIOGuestIOMMU *giommu, *tmp;
>>
>>       QLIST_REMOVE(bcontainer, next);
>>
>> +    QLIST_FOREACH_SAFE(vrdl, &bcontainer->vrdl_list, next, vrdl_tmp) {
>> +        RamDiscardManager *rdm;
>> +
>> +        rdm = memory_region_get_ram_discard_manager(vrdl->mr);
>> +        ram_discard_manager_unregister_listener(rdm, &vrdl->listener);
>> +        QLIST_REMOVE(vrdl, next);
>> +        g_free(vrdl);
>> +    }
>
>Where was this done previously ?

Good catch! This should be removed.

>May be the vrdl list should be handled
>separatly from pgsizes and dma_max_mappings.

Good suggestion! Will do.

>
>>       QLIST_FOREACH_SAFE(giommu, &bcontainer->giommu_list, giommu_next,
>tmp) {
>>           memory_region_unregister_iommu_notifier(
>>                   MEMORY_REGION(giommu->iommu_mr), &giommu->n);
>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>> index 8d5b408e86..0e265ffa67 100644
>> --- a/hw/vfio/container.c
>> +++ b/hw/vfio/container.c
>> @@ -154,7 +154,7 @@ static int vfio_legacy_dma_unmap(VFIOContainerBase
>*bcontainer, hwaddr iova,
>>           if (errno == EINVAL && unmap.size && !(unmap.iova + unmap.size) &&
>>               container->iommu_type == VFIO_TYPE1v2_IOMMU) {
>>               trace_vfio_legacy_dma_unmap_overflow_workaround();
>> -            unmap.size -= 1ULL << ctz64(container->pgsizes);
>> +            unmap.size -= 1ULL << ctz64(container->bcontainer.pgsizes);
>>               continue;
>>           }
>>           error_report("VFIO_UNMAP_DMA failed: %s", strerror(errno));
>> @@ -559,9 +559,7 @@ static int vfio_connect_container(VFIOGroup *group,
>AddressSpace *as,
>>       container = g_malloc0(sizeof(*container));
>>       container->fd = fd;
>>       container->error = NULL;
>> -    container->dma_max_mappings = 0;
>>       container->iova_ranges = NULL;
>> -    QLIST_INIT(&container->vrdl_list);
>>       bcontainer = &container->bcontainer;
>>       vfio_container_init(bcontainer, space, &vfio_legacy_ops);
>>
>> @@ -589,13 +587,13 @@ static int vfio_connect_container(VFIOGroup *group,
>AddressSpace *as,
>>           }
>>
>>           if (info->flags & VFIO_IOMMU_INFO_PGSIZES) {
>> -            container->pgsizes = info->iova_pgsizes;
>> +            container->bcontainer.pgsizes = info->iova_pgsizes;
>>           } else {
>> -            container->pgsizes = qemu_real_host_page_size();
>> +            container->bcontainer.pgsizes = qemu_real_host_page_size();
>>           }
>>
>> -        if (!vfio_get_info_dma_avail(info, &container->dma_max_mappings)) {
>> -            container->dma_max_mappings = 65535;
>> +        if (!vfio_get_info_dma_avail(info, &bcontainer->dma_max_mappings)) {
>> +            container->bcontainer.dma_max_mappings = 65535;
>>           }
>>
>>           vfio_get_info_iova_range(info, container);
>> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
>> index 3495737ab2..dbc4c24052 100644
>> --- a/hw/vfio/spapr.c
>> +++ b/hw/vfio/spapr.c
>> @@ -223,13 +223,13 @@ static int vfio_spapr_create_window(VFIOContainer
>*container,
>>       if (pagesize > rampagesize) {
>>           pagesize = rampagesize;
>>       }
>> -    pgmask = container->pgsizes & (pagesize | (pagesize - 1));
>> +    pgmask = container->bcontainer.pgsizes & (pagesize | (pagesize - 1));
>>       pagesize = pgmask ? (1ULL << (63 - clz64(pgmask))) : 0;
>>       if (!pagesize) {
>>           error_report("Host doesn't support page size 0x%"PRIx64
>>                        ", the supported mask is 0x%lx",
>>                        memory_region_iommu_get_min_page_size(iommu_mr),
>> -                     container->pgsizes);
>> +                     container->bcontainer.pgsizes);
>>           return -EINVAL;
>>       }
>>
>> @@ -385,7 +385,7 @@ void
>vfio_container_del_section_window(VFIOContainer *container,
>>
>>   bool vfio_spapr_container_init(VFIOContainer *container, Error **errp)
>>   {
>> -
>> +    VFIOContainerBase *bcontainer = &container->bcontainer;
>>       struct vfio_iommu_spapr_tce_info info;
>>       bool v2 = container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU;
>>       int ret, fd = container->fd;
>> @@ -424,7 +424,7 @@ bool vfio_spapr_container_init(VFIOContainer
>*container, Error **errp)
>>       }
>>
>>       if (v2) {
>> -        container->pgsizes = info.ddw.pgsizes;
>> +        bcontainer->pgsizes = info.ddw.pgsizes;
>>           /*
>>            * There is a default window in just created container.
>>            * To make region_add/del simpler, we better remove this
>> @@ -439,7 +439,7 @@ bool vfio_spapr_container_init(VFIOContainer
>*container, Error **errp)
>>           }
>>       } else {
>>           /* The default table uses 4K pages */
>> -        container->pgsizes = 0x1000;
>> +        bcontainer->pgsizes = 0x1000;
>>           vfio_host_win_add(container, info.dma32_window_start,
>>                             info.dma32_window_start +
>>                             info.dma32_window_size - 1,
>> @@ -455,7 +455,15 @@ listener_unregister_exit:
>>
>>   void vfio_spapr_container_deinit(VFIOContainer *container)
>>   {
>> +    VFIOHostDMAWindow *hostwin, *next;
>> +
>>       if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
>>           memory_listener_unregister(&container->prereg_listener);
>>       }
>> +    QLIST_FOREACH_SAFE(hostwin, &container->hostwin_list, hostwin_next,
>> +                       next) {
>> +        QLIST_REMOVE(hostwin, hostwin_next);
>> +        g_free(hostwin);
>> +    }
>> +
>>   }
>
>I am sure this change  belongs to this patch.

Good catch! Yes, I should move it into below patch.
"vfio/common: Move vfio_host_win_add/del into spapr.c"

Thanks
Zhenzhong
Cédric Le Goater Oct. 31, 2023, 8:26 a.m. UTC | #3 
On 10/30/23 04:14, Duan, Zhenzhong wrote:
> Hi Cédric,
> 
>> -----Original Message-----
>> From: Cédric Le Goater <clg@redhat.com>
>> Sent: Friday, October 27, 2023 11:52 PM
>> Subject: Re: [PATCH v3 14/37] vfio/container: Move vrdl_list, pgsizes and
>> dma_max_mappings to base container
>>
>> On 10/26/23 12:30, Zhenzhong Duan wrote:
>>> From: Eric Auger <eric.auger@redhat.com>
>>>
> ...
> 
>>>    void vfio_container_destroy(VFIOContainerBase *bcontainer)
>>>    {
>>> +    VFIORamDiscardListener *vrdl, *vrdl_tmp;
>>>        VFIOGuestIOMMU *giommu, *tmp;
>>>
>>>        QLIST_REMOVE(bcontainer, next);
>>>
>>> +    QLIST_FOREACH_SAFE(vrdl, &bcontainer->vrdl_list, next, vrdl_tmp) {
>>> +        RamDiscardManager *rdm;
>>> +
>>> +        rdm = memory_region_get_ram_discard_manager(vrdl->mr);
>>> +        ram_discard_manager_unregister_listener(rdm, &vrdl->listener);
>>> +        QLIST_REMOVE(vrdl, next);
>>> +        g_free(vrdl);
>>> +    }
>>
>> Where was this done previously ?
> 
> Good catch! This should be removed.
> 
>> May be the vrdl list should be handled
>> separatly from pgsizes and dma_max_mappings.
> 
> Good suggestion! Will do.
> 
>>
>>>        QLIST_FOREACH_SAFE(giommu, &bcontainer->giommu_list, giommu_next,
>> tmp) {
>>>            memory_region_unregister_iommu_notifier(
>>>                    MEMORY_REGION(giommu->iommu_mr), &giommu->n);
>>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>>> index 8d5b408e86..0e265ffa67 100644
>>> --- a/hw/vfio/container.c
>>> +++ b/hw/vfio/container.c
>>> @@ -154,7 +154,7 @@ static int vfio_legacy_dma_unmap(VFIOContainerBase
>> *bcontainer, hwaddr iova,
>>>            if (errno == EINVAL && unmap.size && !(unmap.iova + unmap.size) &&
>>>                container->iommu_type == VFIO_TYPE1v2_IOMMU) {
>>>                trace_vfio_legacy_dma_unmap_overflow_workaround();
>>> -            unmap.size -= 1ULL << ctz64(container->pgsizes);
>>> +            unmap.size -= 1ULL << ctz64(container->bcontainer.pgsizes);
>>>                continue;
>>>            }
>>>            error_report("VFIO_UNMAP_DMA failed: %s", strerror(errno));
>>> @@ -559,9 +559,7 @@ static int vfio_connect_container(VFIOGroup *group,
>> AddressSpace *as,
>>>        container = g_malloc0(sizeof(*container));
>>>        container->fd = fd;
>>>        container->error = NULL;
>>> -    container->dma_max_mappings = 0;
>>>        container->iova_ranges = NULL;
>>> -    QLIST_INIT(&container->vrdl_list);
>>>        bcontainer = &container->bcontainer;
>>>        vfio_container_init(bcontainer, space, &vfio_legacy_ops);
>>>
>>> @@ -589,13 +587,13 @@ static int vfio_connect_container(VFIOGroup *group,
>> AddressSpace *as,
>>>            }
>>>
>>>            if (info->flags & VFIO_IOMMU_INFO_PGSIZES) {
>>> -            container->pgsizes = info->iova_pgsizes;
>>> +            container->bcontainer.pgsizes = info->iova_pgsizes;
>>>            } else {
>>> -            container->pgsizes = qemu_real_host_page_size();
>>> +            container->bcontainer.pgsizes = qemu_real_host_page_size();
>>>            }
>>>
>>> -        if (!vfio_get_info_dma_avail(info, &container->dma_max_mappings)) {
>>> -            container->dma_max_mappings = 65535;
>>> +        if (!vfio_get_info_dma_avail(info, &bcontainer->dma_max_mappings)) {
>>> +            container->bcontainer.dma_max_mappings = 65535;
>>>            }
>>>
>>>            vfio_get_info_iova_range(info, container);
>>> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
>>> index 3495737ab2..dbc4c24052 100644
>>> --- a/hw/vfio/spapr.c
>>> +++ b/hw/vfio/spapr.c
>>> @@ -223,13 +223,13 @@ static int vfio_spapr_create_window(VFIOContainer
>> *container,
>>>        if (pagesize > rampagesize) {
>>>            pagesize = rampagesize;
>>>        }
>>> -    pgmask = container->pgsizes & (pagesize | (pagesize - 1));
>>> +    pgmask = container->bcontainer.pgsizes & (pagesize | (pagesize - 1));
>>>        pagesize = pgmask ? (1ULL << (63 - clz64(pgmask))) : 0;
>>>        if (!pagesize) {
>>>            error_report("Host doesn't support page size 0x%"PRIx64
>>>                         ", the supported mask is 0x%lx",
>>>                         memory_region_iommu_get_min_page_size(iommu_mr),
>>> -                     container->pgsizes);
>>> +                     container->bcontainer.pgsizes);
>>>            return -EINVAL;
>>>        }
>>>
>>> @@ -385,7 +385,7 @@ void
>> vfio_container_del_section_window(VFIOContainer *container,
>>>
>>>    bool vfio_spapr_container_init(VFIOContainer *container, Error **errp)
>>>    {
>>> -
>>> +    VFIOContainerBase *bcontainer = &container->bcontainer;
>>>        struct vfio_iommu_spapr_tce_info info;
>>>        bool v2 = container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU;
>>>        int ret, fd = container->fd;
>>> @@ -424,7 +424,7 @@ bool vfio_spapr_container_init(VFIOContainer
>> *container, Error **errp)
>>>        }
>>>
>>>        if (v2) {
>>> -        container->pgsizes = info.ddw.pgsizes;
>>> +        bcontainer->pgsizes = info.ddw.pgsizes;
>>>            /*
>>>             * There is a default window in just created container.
>>>             * To make region_add/del simpler, we better remove this
>>> @@ -439,7 +439,7 @@ bool vfio_spapr_container_init(VFIOContainer
>> *container, Error **errp)
>>>            }
>>>        } else {
>>>            /* The default table uses 4K pages */
>>> -        container->pgsizes = 0x1000;
>>> +        bcontainer->pgsizes = 0x1000;
>>>            vfio_host_win_add(container, info.dma32_window_start,
>>>                              info.dma32_window_start +
>>>                              info.dma32_window_size - 1,
>>> @@ -455,7 +455,15 @@ listener_unregister_exit:
>>>
>>>    void vfio_spapr_container_deinit(VFIOContainer *container)
>>>    {
>>> +    VFIOHostDMAWindow *hostwin, *next;
>>> +
>>>        if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
>>>            memory_listener_unregister(&container->prereg_listener);
>>>        }
>>> +    QLIST_FOREACH_SAFE(hostwin, &container->hostwin_list, hostwin_next,
>>> +                       next) {
>>> +        QLIST_REMOVE(hostwin, hostwin_next);
>>> +        g_free(hostwin);
>>> +    }
>>> +
>>>    }
>>
>> I am sure this change  belongs to this patch.
        
"I am not sure" but you read my mind !

> 
> Good catch! Yes, I should move it into below patch.
> "vfio/common: Move vfio_host_win_add/del into spapr.c"

yes.

Thanks,

C.




> 
> Thanks
> Zhenzhong
diff mbox series

Patch

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index fb3c7aea8f..65ae2d76cf 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -85,24 +85,11 @@  typedef struct VFIOContainer {
     bool initialized;
     uint64_t dirty_pgsizes;
     uint64_t max_dirty_bitmap_size;
-    unsigned long pgsizes;
-    unsigned int dma_max_mappings;
     QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
     QLIST_HEAD(, VFIOGroup) group_list;
-    QLIST_HEAD(, VFIORamDiscardListener) vrdl_list;
     GList *iova_ranges;
 } VFIOContainer;
 
-typedef struct VFIORamDiscardListener {
-    VFIOContainer *container;
-    MemoryRegion *mr;
-    hwaddr offset_within_address_space;
-    hwaddr size;
-    uint64_t granularity;
-    RamDiscardListener listener;
-    QLIST_ENTRY(VFIORamDiscardListener) next;
-} VFIORamDiscardListener;
-
 typedef struct VFIOHostDMAWindow {
     hwaddr min_iova;
     hwaddr max_iova;
diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
index f1de1ef120..849c8b34b2 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -50,8 +50,11 @@  typedef struct VFIOAddressSpace {
 typedef struct VFIOContainerBase {
     const VFIOIOMMUOps *ops;
     VFIOAddressSpace *space;
+    unsigned long pgsizes;
+    unsigned int dma_max_mappings;
     bool dirty_pages_supported;
     QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
+    QLIST_HEAD(, VFIORamDiscardListener) vrdl_list;
     QLIST_ENTRY(VFIOContainerBase) next;
     QLIST_HEAD(, VFIODevice) device_list;
 } VFIOContainerBase;
@@ -64,6 +67,16 @@  typedef struct VFIOGuestIOMMU {
     QLIST_ENTRY(VFIOGuestIOMMU) giommu_next;
 } VFIOGuestIOMMU;
 
+typedef struct VFIORamDiscardListener {
+    VFIOContainerBase *bcontainer;
+    MemoryRegion *mr;
+    hwaddr offset_within_address_space;
+    hwaddr size;
+    uint64_t granularity;
+    RamDiscardListener listener;
+    QLIST_ENTRY(VFIORamDiscardListener) next;
+} VFIORamDiscardListener;
+
 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 91411d9844..9b34e7e0f8 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -351,13 +351,13 @@  static void vfio_ram_discard_notify_discard(RamDiscardListener *rdl,
 {
     VFIORamDiscardListener *vrdl = container_of(rdl, VFIORamDiscardListener,
                                                 listener);
+    VFIOContainerBase *bcontainer = vrdl->bcontainer;
     const hwaddr size = int128_get64(section->size);
     const hwaddr iova = section->offset_within_address_space;
     int ret;
 
     /* Unmap with a single call. */
-    ret = vfio_container_dma_unmap(&vrdl->container->bcontainer,
-                                   iova, size , NULL);
+    ret = vfio_container_dma_unmap(bcontainer, iova, size , NULL);
     if (ret) {
         error_report("%s: vfio_container_dma_unmap() failed: %s", __func__,
                      strerror(-ret));
@@ -369,6 +369,7 @@  static int vfio_ram_discard_notify_populate(RamDiscardListener *rdl,
 {
     VFIORamDiscardListener *vrdl = container_of(rdl, VFIORamDiscardListener,
                                                 listener);
+    VFIOContainerBase *bcontainer = vrdl->bcontainer;
     const hwaddr end = section->offset_within_region +
                        int128_get64(section->size);
     hwaddr start, next, iova;
@@ -387,8 +388,8 @@  static int vfio_ram_discard_notify_populate(RamDiscardListener *rdl,
                section->offset_within_address_space;
         vaddr = memory_region_get_ram_ptr(section->mr) + start;
 
-        ret = vfio_container_dma_map(&vrdl->container->bcontainer, iova,
-                                     next - start, vaddr, section->readonly);
+        ret = vfio_container_dma_map(bcontainer, iova, next - start,
+                                     vaddr, section->readonly);
         if (ret) {
             /* Rollback */
             vfio_ram_discard_notify_discard(rdl, section);
@@ -398,7 +399,7 @@  static int vfio_ram_discard_notify_populate(RamDiscardListener *rdl,
     return 0;
 }
 
-static void vfio_register_ram_discard_listener(VFIOContainer *container,
+static void vfio_register_ram_discard_listener(VFIOContainerBase *bcontainer,
                                                MemoryRegionSection *section)
 {
     RamDiscardManager *rdm = memory_region_get_ram_discard_manager(section->mr);
@@ -411,7 +412,7 @@  static void vfio_register_ram_discard_listener(VFIOContainer *container,
     g_assert(QEMU_IS_ALIGNED(int128_get64(section->size), TARGET_PAGE_SIZE));
 
     vrdl = g_new0(VFIORamDiscardListener, 1);
-    vrdl->container = container;
+    vrdl->bcontainer = bcontainer;
     vrdl->mr = section->mr;
     vrdl->offset_within_address_space = section->offset_within_address_space;
     vrdl->size = int128_get64(section->size);
@@ -419,14 +420,14 @@  static void vfio_register_ram_discard_listener(VFIOContainer *container,
                                                                 section->mr);
 
     g_assert(vrdl->granularity && is_power_of_2(vrdl->granularity));
-    g_assert(container->pgsizes &&
-             vrdl->granularity >= 1ULL << ctz64(container->pgsizes));
+    g_assert(bcontainer->pgsizes &&
+             vrdl->granularity >= 1ULL << ctz64(bcontainer->pgsizes));
 
     ram_discard_listener_init(&vrdl->listener,
                               vfio_ram_discard_notify_populate,
                               vfio_ram_discard_notify_discard, true);
     ram_discard_manager_register_listener(rdm, &vrdl->listener, section);
-    QLIST_INSERT_HEAD(&container->vrdl_list, vrdl, next);
+    QLIST_INSERT_HEAD(&bcontainer->vrdl_list, vrdl, next);
 
     /*
      * Sanity-check if we have a theoretically problematic setup where we could
@@ -441,7 +442,7 @@  static void vfio_register_ram_discard_listener(VFIOContainer *container,
      * number of sections in the address space we could have over time,
      * also consuming DMA mappings.
      */
-    if (container->dma_max_mappings) {
+    if (bcontainer->dma_max_mappings) {
         unsigned int vrdl_count = 0, vrdl_mappings = 0, max_memslots = 512;
 
 #ifdef CONFIG_KVM
@@ -450,7 +451,7 @@  static void vfio_register_ram_discard_listener(VFIOContainer *container,
         }
 #endif
 
-        QLIST_FOREACH(vrdl, &container->vrdl_list, next) {
+        QLIST_FOREACH(vrdl, &bcontainer->vrdl_list, next) {
             hwaddr start, end;
 
             start = QEMU_ALIGN_DOWN(vrdl->offset_within_address_space,
@@ -462,23 +463,23 @@  static void vfio_register_ram_discard_listener(VFIOContainer *container,
         }
 
         if (vrdl_mappings + max_memslots - vrdl_count >
-            container->dma_max_mappings) {
+            bcontainer->dma_max_mappings) {
             warn_report("%s: possibly running out of DMA mappings. E.g., try"
                         " increasing the 'block-size' of virtio-mem devies."
                         " Maximum possible DMA mappings: %d, Maximum possible"
-                        " memslots: %d", __func__, container->dma_max_mappings,
+                        " memslots: %d", __func__, bcontainer->dma_max_mappings,
                         max_memslots);
         }
     }
 }
 
-static void vfio_unregister_ram_discard_listener(VFIOContainer *container,
+static void vfio_unregister_ram_discard_listener(VFIOContainerBase *bcontainer,
                                                  MemoryRegionSection *section)
 {
     RamDiscardManager *rdm = memory_region_get_ram_discard_manager(section->mr);
     VFIORamDiscardListener *vrdl = NULL;
 
-    QLIST_FOREACH(vrdl, &container->vrdl_list, next) {
+    QLIST_FOREACH(vrdl, &bcontainer->vrdl_list, next) {
         if (vrdl->mr == section->mr &&
             vrdl->offset_within_address_space ==
             section->offset_within_address_space) {
@@ -627,7 +628,7 @@  static void vfio_listener_region_add(MemoryListener *listener,
                             iommu_idx);
 
         ret = memory_region_iommu_set_page_size_mask(giommu->iommu_mr,
-                                                     container->pgsizes,
+                                                     bcontainer->pgsizes,
                                                      &err);
         if (ret) {
             g_free(giommu);
@@ -663,7 +664,7 @@  static void vfio_listener_region_add(MemoryListener *listener,
      * about changes.
      */
     if (memory_region_has_ram_discard_manager(section->mr)) {
-        vfio_register_ram_discard_listener(container, section);
+        vfio_register_ram_discard_listener(bcontainer, section);
         return;
     }
 
@@ -782,7 +783,7 @@  static void vfio_listener_region_del(MemoryListener *listener,
         pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1;
         try_unmap = !((iova & pgmask) || (int128_get64(llsize) & pgmask));
     } else if (memory_region_has_ram_discard_manager(section->mr)) {
-        vfio_unregister_ram_discard_listener(container, section);
+        vfio_unregister_ram_discard_listener(bcontainer, section);
         /* Unregistering will trigger an unmap. */
         try_unmap = false;
     }
@@ -1261,17 +1262,17 @@  static int vfio_ram_discard_get_dirty_bitmap(MemoryRegionSection *section,
      * Sync the whole mapped region (spanning multiple individual mappings)
      * in one go.
      */
-    return vfio_get_dirty_bitmap(&vrdl->container->bcontainer, iova, size,
-                                 ram_addr);
+    return vfio_get_dirty_bitmap(vrdl->bcontainer, iova, size, ram_addr);
 }
 
-static int vfio_sync_ram_discard_listener_dirty_bitmap(VFIOContainer *container,
-                                                   MemoryRegionSection *section)
+static int
+vfio_sync_ram_discard_listener_dirty_bitmap(VFIOContainerBase *bcontainer,
+                                            MemoryRegionSection *section)
 {
     RamDiscardManager *rdm = memory_region_get_ram_discard_manager(section->mr);
     VFIORamDiscardListener *vrdl = NULL;
 
-    QLIST_FOREACH(vrdl, &container->vrdl_list, next) {
+    QLIST_FOREACH(vrdl, &bcontainer->vrdl_list, next) {
         if (vrdl->mr == section->mr &&
             vrdl->offset_within_address_space ==
             section->offset_within_address_space) {
@@ -1325,7 +1326,7 @@  static int vfio_sync_dirty_bitmap(VFIOContainer *container,
         }
         return 0;
     } else if (memory_region_has_ram_discard_manager(section->mr)) {
-        return vfio_sync_ram_discard_listener_dirty_bitmap(container, section);
+        return vfio_sync_ram_discard_listener_dirty_bitmap(bcontainer, section);
     }
 
     ram_addr = memory_region_get_ram_addr(section->mr) +
diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
index a7cf517dd2..568f891841 100644
--- a/hw/vfio/container-base.c
+++ b/hw/vfio/container-base.c
@@ -76,15 +76,27 @@  void vfio_container_init(VFIOContainerBase *bcontainer, VFIOAddressSpace *space,
     bcontainer->ops = ops;
     bcontainer->space = space;
     bcontainer->dirty_pages_supported = false;
+    bcontainer->dma_max_mappings = 0;
     QLIST_INIT(&bcontainer->giommu_list);
+    QLIST_INIT(&bcontainer->vrdl_list);
 }
 
 void vfio_container_destroy(VFIOContainerBase *bcontainer)
 {
+    VFIORamDiscardListener *vrdl, *vrdl_tmp;
     VFIOGuestIOMMU *giommu, *tmp;
 
     QLIST_REMOVE(bcontainer, next);
 
+    QLIST_FOREACH_SAFE(vrdl, &bcontainer->vrdl_list, next, vrdl_tmp) {
+        RamDiscardManager *rdm;
+
+        rdm = memory_region_get_ram_discard_manager(vrdl->mr);
+        ram_discard_manager_unregister_listener(rdm, &vrdl->listener);
+        QLIST_REMOVE(vrdl, next);
+        g_free(vrdl);
+    }
+
     QLIST_FOREACH_SAFE(giommu, &bcontainer->giommu_list, giommu_next, tmp) {
         memory_region_unregister_iommu_notifier(
                 MEMORY_REGION(giommu->iommu_mr), &giommu->n);
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 8d5b408e86..0e265ffa67 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -154,7 +154,7 @@  static int vfio_legacy_dma_unmap(VFIOContainerBase *bcontainer, hwaddr iova,
         if (errno == EINVAL && unmap.size && !(unmap.iova + unmap.size) &&
             container->iommu_type == VFIO_TYPE1v2_IOMMU) {
             trace_vfio_legacy_dma_unmap_overflow_workaround();
-            unmap.size -= 1ULL << ctz64(container->pgsizes);
+            unmap.size -= 1ULL << ctz64(container->bcontainer.pgsizes);
             continue;
         }
         error_report("VFIO_UNMAP_DMA failed: %s", strerror(errno));
@@ -559,9 +559,7 @@  static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
     container = g_malloc0(sizeof(*container));
     container->fd = fd;
     container->error = NULL;
-    container->dma_max_mappings = 0;
     container->iova_ranges = NULL;
-    QLIST_INIT(&container->vrdl_list);
     bcontainer = &container->bcontainer;
     vfio_container_init(bcontainer, space, &vfio_legacy_ops);
 
@@ -589,13 +587,13 @@  static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
         }
 
         if (info->flags & VFIO_IOMMU_INFO_PGSIZES) {
-            container->pgsizes = info->iova_pgsizes;
+            container->bcontainer.pgsizes = info->iova_pgsizes;
         } else {
-            container->pgsizes = qemu_real_host_page_size();
+            container->bcontainer.pgsizes = qemu_real_host_page_size();
         }
 
-        if (!vfio_get_info_dma_avail(info, &container->dma_max_mappings)) {
-            container->dma_max_mappings = 65535;
+        if (!vfio_get_info_dma_avail(info, &bcontainer->dma_max_mappings)) {
+            container->bcontainer.dma_max_mappings = 65535;
         }
 
         vfio_get_info_iova_range(info, container);
diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
index 3495737ab2..dbc4c24052 100644
--- a/hw/vfio/spapr.c
+++ b/hw/vfio/spapr.c
@@ -223,13 +223,13 @@  static int vfio_spapr_create_window(VFIOContainer *container,
     if (pagesize > rampagesize) {
         pagesize = rampagesize;
     }
-    pgmask = container->pgsizes & (pagesize | (pagesize - 1));
+    pgmask = container->bcontainer.pgsizes & (pagesize | (pagesize - 1));
     pagesize = pgmask ? (1ULL << (63 - clz64(pgmask))) : 0;
     if (!pagesize) {
         error_report("Host doesn't support page size 0x%"PRIx64
                      ", the supported mask is 0x%lx",
                      memory_region_iommu_get_min_page_size(iommu_mr),
-                     container->pgsizes);
+                     container->bcontainer.pgsizes);
         return -EINVAL;
     }
 
@@ -385,7 +385,7 @@  void vfio_container_del_section_window(VFIOContainer *container,
 
 bool vfio_spapr_container_init(VFIOContainer *container, Error **errp)
 {
-
+    VFIOContainerBase *bcontainer = &container->bcontainer;
     struct vfio_iommu_spapr_tce_info info;
     bool v2 = container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU;
     int ret, fd = container->fd;
@@ -424,7 +424,7 @@  bool vfio_spapr_container_init(VFIOContainer *container, Error **errp)
     }
 
     if (v2) {
-        container->pgsizes = info.ddw.pgsizes;
+        bcontainer->pgsizes = info.ddw.pgsizes;
         /*
          * There is a default window in just created container.
          * To make region_add/del simpler, we better remove this
@@ -439,7 +439,7 @@  bool vfio_spapr_container_init(VFIOContainer *container, Error **errp)
         }
     } else {
         /* The default table uses 4K pages */
-        container->pgsizes = 0x1000;
+        bcontainer->pgsizes = 0x1000;
         vfio_host_win_add(container, info.dma32_window_start,
                           info.dma32_window_start +
                           info.dma32_window_size - 1,
@@ -455,7 +455,15 @@  listener_unregister_exit:
 
 void vfio_spapr_container_deinit(VFIOContainer *container)
 {
+    VFIOHostDMAWindow *hostwin, *next;
+
     if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
         memory_listener_unregister(&container->prereg_listener);
     }
+    QLIST_FOREACH_SAFE(hostwin, &container->hostwin_list, hostwin_next,
+                       next) {
+        QLIST_REMOVE(hostwin, hostwin_next);
+        g_free(hostwin);
+    }
+
 }