Re: [PATCH v10 02/10] virtio-iommu: Store memory region in endpoint struct

[Auger Eric]

Hi Jean,

On 10/8/20 7:15 PM, Jean-Philippe Brucker wrote:
> Store the memory region associated to each endpoint into the endpoint
> structure, to allow efficient memory notification on map/unmap.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Acked-by: Eric Auger <eric.auger@redhat.com>

> ---
> Not super confident about the reconstruct_endpoint() change since I
> haven't tested migration yet. Does it make sense?
It sounds good to me. I tested migration with vhost and this works properly.

Eric
> ---
>  hw/virtio/virtio-iommu.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 543fbbb24fb..33115e82186 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -49,6 +49,7 @@ typedef struct VirtIOIOMMUDomain {
>  typedef struct VirtIOIOMMUEndpoint {
>      uint32_t id;
>      VirtIOIOMMUDomain *domain;
> +    IOMMUMemoryRegion *iommu_mr;
>      QLIST_ENTRY(VirtIOIOMMUEndpoint) next;
>  } VirtIOIOMMUEndpoint;
>  
> @@ -137,16 +138,19 @@ static VirtIOIOMMUEndpoint 
> *virtio_iommu_get_endpoint(VirtIOIOMMU *s,
>                                                        uint32_t ep_id)
>  {
>      VirtIOIOMMUEndpoint *ep;
> +    IOMMUMemoryRegion *mr;
>  
>      ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(ep_id));
>      if (ep) {
>          return ep;
>      }
> -    if (!virtio_iommu_mr(s, ep_id)) {
> +    mr = virtio_iommu_mr(s, ep_id);
> +    if (!mr) {
>          return NULL;
>      }
>      ep = g_malloc0(sizeof(*ep));
>      ep->id = ep_id;
> +    ep->iommu_mr = mr;
>      trace_virtio_iommu_get_endpoint(ep_id);
>      g_tree_insert(s->endpoints, GUINT_TO_POINTER(ep_id), ep);
>      return ep;
> @@ -927,9 +931,14 @@ static gboolean reconstruct_endpoints(gpointer key, 
> gpointer value,
>      VirtIOIOMMU *s = (VirtIOIOMMU *)data;
>      VirtIOIOMMUDomain *d = (VirtIOIOMMUDomain *)value;
>      VirtIOIOMMUEndpoint *iter;
> +    IOMMUMemoryRegion *mr;
>  
>      QLIST_FOREACH(iter, &d->endpoint_list, next) {
> +        mr = virtio_iommu_mr(s, iter->id);
> +        assert(mr);
> +
>          iter->domain = d;
> +        iter->iommu_mr = mr;
>          g_tree_insert(s->endpoints, GUINT_TO_POINTER(iter->id), iter);
>      }
>      return false; /* continue the domain traversal */
>