Re: [PATCH v10 04/10] virtio-iommu: Call memory notifiers in attach/detach

[Auger Eric <eric.auger@redhat.com>]

Hi jean,

On 10/8/20 7:15 PM, Jean-Philippe Brucker wrote:
> From: Bharat Bhushan <bbhushan2@marvell.com>
> 
> Call the memory notifiers when attaching an endpoint to a domain, to
> replay existing mappings, and when detaching the endpoint, to remove all
> mappings.
> 
> Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> v10: Remove notifiers_list, rename callbacks
> ---
>  hw/virtio/virtio-iommu.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index fcdf3a819f8..7e6e3cf5200 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -168,11 +168,39 @@ static void virtio_iommu_notify_unmap(IOMMUMemoryRegion *mr, hwaddr virt_start,
>      memory_region_notify_iommu(mr, 0, entry);
>  }
>  
> +static gboolean virtio_iommu_notify_unmap_cb(gpointer key, gpointer value,
> +                                             gpointer data)
> +{
> +    VirtIOIOMMUInterval *interval = (VirtIOIOMMUInterval *) key;
> +    IOMMUMemoryRegion *mr = (IOMMUMemoryRegion *) data;
> +
> +    virtio_iommu_notify_unmap(mr, interval->low, interval->high);
> +
> +    return false;
> +}
> +
> +static gboolean virtio_iommu_notify_map_cb(gpointer key, gpointer value,
> +                                           gpointer data)
> +{
> +    VirtIOIOMMUMapping *mapping = (VirtIOIOMMUMapping *) value;
> +    VirtIOIOMMUInterval *interval = (VirtIOIOMMUInterval *) key;
> +    IOMMUMemoryRegion *mr = (IOMMUMemoryRegion *) data;
> +
> +    virtio_iommu_notify_map(mr, interval->low, interval->high,
> +                            mapping->phys_addr);
Here also I think we should apply the mapping->flags.
> +
> +    return false;
> +}
> +
>  static void virtio_iommu_detach_endpoint_from_domain(VirtIOIOMMUEndpoint *ep)
>  {
> +    VirtIOIOMMUDomain *domain = ep->domain;
> +
>      if (!ep->domain) {
>          return;
>      }
> +    g_tree_foreach(domain->mappings, virtio_iommu_notify_unmap_cb,
> +                   ep->iommu_mr);
>      QLIST_REMOVE(ep, next);
>      ep->domain = NULL;
>  }
> @@ -315,6 +343,10 @@ static int virtio_iommu_attach(VirtIOIOMMU *s,
>  
>      ep->domain = domain;
>  
> +    /* Replay domain mappings on the associated memory region */
> +    g_tree_foreach(domain->mappings, virtio_iommu_notify_map_cb,
> +                   ep->iommu_mr);
> +
>      return VIRTIO_IOMMU_S_OK;
>  }
>  
> 
Thanks

Eric