[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-5.0 v11 06/20] virtio-iommu: Implement attach/detach comm
From: |
Jean-Philippe Brucker |
Subject: |
Re: [PATCH for-5.0 v11 06/20] virtio-iommu: Implement attach/detach command |
Date: |
Tue, 10 Dec 2019 17:41:56 +0100 |
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
- Re: [PATCH for-5.0 v11 06/20] virtio-iommu: Implement attach/detach command,
Jean-Philippe Brucker <=