On Fri, Nov 22, 2019 at 07:29:29PM +0100, Eric Auger wrote: > diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c > index 235bde2203..138d5b2a9c 100644 > --- a/hw/virtio/virtio-iommu.c > +++ b/hw/virtio/virtio-iommu.c > @@ -77,11 +77,12 @@ static gint interval_cmp(gconstpointer a, gconstpointer > b, gpointer user_data) > static void virtio_iommu_detach_endpoint_from_domain(viommu_endpoint *ep) > { > QLIST_REMOVE(ep, next); > + g_tree_unref(ep->domain->mappings); > ep->domain = NULL; > } > > -viommu_endpoint *virtio_iommu_get_endpoint(VirtIOIOMMU *s, uint32_t ep_id); > -viommu_endpoint *virtio_iommu_get_endpoint(VirtIOIOMMU *s, uint32_t ep_id) > +static viommu_endpoint *virtio_iommu_get_endpoint(VirtIOIOMMU *s, > + uint32_t ep_id) > { > viommu_endpoint *ep; > > @@ -102,15 +103,14 @@ static void virtio_iommu_put_endpoint(gpointer data) > > if (ep->domain) { > virtio_iommu_detach_endpoint_from_domain(ep); > - g_tree_unref(ep->domain->mappings); > } > > trace_virtio_iommu_put_endpoint(ep->id); > g_free(ep); > } > > -viommu_domain *virtio_iommu_get_domain(VirtIOIOMMU *s, uint32_t domain_id); > -viommu_domain *virtio_iommu_get_domain(VirtIOIOMMU *s, uint32_t domain_id) > +static viommu_domain *virtio_iommu_get_domain(VirtIOIOMMU *s, > + uint32_t domain_id)
Looks like the above change belong to patch 5? > { > viommu_domain *domain; > > @@ -137,7 +137,6 @@ static void virtio_iommu_put_domain(gpointer data) > QLIST_FOREACH_SAFE(iter, &domain->endpoint_list, next, tmp) { > virtio_iommu_detach_endpoint_from_domain(iter); > } > - g_tree_destroy(domain->mappings); When created by virtio_iommu_get_domain(), mappings has one reference. Then for each attach (including the first one) an additional reference is taken, and freed by virtio_iommu_detach_endpoint_from_domain(). So I think there are two problems: * virtio_iommu_put_domain() drops one ref for each endpoint, but we still have one reference to mappings, so they're not freed. We do need this g_tree_destroy() * After detaching all the endpoints, the guest may reuse the domain ID for another domain, but the previous mappings haven't been erased. Not sure how to fix this using the g_tree refs, because dropping all the references will free the internal tree data and it won't be reusable. Thanks, Jean