Hi Eric, On Fri, Mar 13, 2020 at 8:11 PM Auger Eric <eric.au...@redhat.com> wrote: > > Hi Bharat > > On 3/13/20 8:48 AM, Bharat Bhushan wrote: > > iommu-notifier are called when a device is attached > IOMMU notifiers > > or detached to as address-space. > > This is needed for VFIO. > and vhost for detach > > > > Signed-off-by: Bharat Bhushan <bbhush...@marvell.com> > > --- > > hw/virtio/virtio-iommu.c | 47 ++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 47 insertions(+) > > > > diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c > > index e51344a53e..2006f72901 100644 > > --- a/hw/virtio/virtio-iommu.c > > +++ b/hw/virtio/virtio-iommu.c > > @@ -49,6 +49,7 @@ typedef struct VirtIOIOMMUEndpoint { > > uint32_t id; > > VirtIOIOMMUDomain *domain; > > QLIST_ENTRY(VirtIOIOMMUEndpoint) next; > > + VirtIOIOMMU *viommu; > This needs specal care on post-load. When migrating the EPs, only the id > is migrated. On post-load you need to set viommu as it is done for > domain. migration is allowed with vhost.
ok, I have not tried vhost/migration. Below change set viommu when reconstructing endpoint. @@ -984,6 +973,7 @@ static gboolean reconstruct_endpoints(gpointer key, gpointer value, QLIST_FOREACH(iter, &d->endpoint_list, next) { iter->domain = d; + iter->viommu = s; g_tree_insert(s->endpoints, GUINT_TO_POINTER(iter->id), iter); } return false; /* continue the domain traversal */ > > } VirtIOIOMMUEndpoint; > > > > typedef struct VirtIOIOMMUInterval { > > @@ -155,8 +156,44 @@ static void > > virtio_iommu_notify_unmap(IOMMUMemoryRegion *mr, hwaddr iova, > > memory_region_notify_iommu(mr, 0, entry); > > } > > > > +static gboolean virtio_iommu_mapping_unmap(gpointer key, gpointer value, > > + gpointer data) > > +{ > > + VirtIOIOMMUInterval *interval = (VirtIOIOMMUInterval *) key; > > + IOMMUMemoryRegion *mr = (IOMMUMemoryRegion *) data; > > + > > + virtio_iommu_notify_unmap(mr, interval->low, > > + interval->high - interval->low + 1); > > + > > + return false; > > +} > > + > > +static gboolean virtio_iommu_mapping_map(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, mapping->phys_addr, > > + interval->high - interval->low + 1); > > + > > + return false; > > +} > > + > > static void virtio_iommu_detach_endpoint_from_domain(VirtIOIOMMUEndpoint > > *ep) > > { > > + VirtioIOMMUNotifierNode *node; > > + VirtIOIOMMU *s = ep->viommu; > > + VirtIOIOMMUDomain *domain = ep->domain; > > + > > + QLIST_FOREACH(node, &s->notifiers_list, next) { > > + if (ep->id == node->iommu_dev->devfn) { > > + g_tree_foreach(domain->mappings, virtio_iommu_mapping_unmap, > > + &node->iommu_dev->iommu_mr); > I understand this should fo the job for domain removal did not get the comment, are you saying we should do this on domain removal? > > + } > > + } > > + > > if (!ep->domain) { > > return; > > } > > @@ -178,6 +215,7 @@ static VirtIOIOMMUEndpoint > > *virtio_iommu_get_endpoint(VirtIOIOMMU *s, > > } > > ep = g_malloc0(sizeof(*ep)); > > ep->id = ep_id; > > + ep->viommu = s; > > trace_virtio_iommu_get_endpoint(ep_id); > > g_tree_insert(s->endpoints, GUINT_TO_POINTER(ep_id), ep); > > return ep; > > @@ -272,6 +310,7 @@ static int virtio_iommu_attach(VirtIOIOMMU *s, > > { > > uint32_t domain_id = le32_to_cpu(req->domain); > > uint32_t ep_id = le32_to_cpu(req->endpoint); > > + VirtioIOMMUNotifierNode *node; > > VirtIOIOMMUDomain *domain; > > VirtIOIOMMUEndpoint *ep; > > > > @@ -299,6 +338,14 @@ static int virtio_iommu_attach(VirtIOIOMMU *s, > > > > ep->domain = domain; > > > > + /* Replay existing address space mappings on the associated memory > > region */ > maybe use the "domain" terminology here. ok, Thanks -Bharat > > + QLIST_FOREACH(node, &s->notifiers_list, next) { > > + if (ep_id == node->iommu_dev->devfn) { > > + g_tree_foreach(domain->mappings, virtio_iommu_mapping_map, > > + &node->iommu_dev->iommu_mr); > > + } > > + } > > + > > return VIRTIO_IOMMU_S_OK; > > } > > > > > Thanks > > Eric >