* Eric Auger (eric.au...@redhat.com) wrote: > Hi Jean, > > On 1/27/22 3:29 PM, Jean-Philippe Brucker wrote: > > The driver can create a bypass domain by passing the > > VIRTIO_IOMMU_ATTACH_F_BYPASS flag on the ATTACH request. Bypass domains > > perform slightly better than domains with identity mappings since they > > skip translation. > > > > Signed-off-by: Jean-Philippe Brucker <jean-phili...@linaro.org> > > --- > > hw/virtio/virtio-iommu.c | 32 ++++++++++++++++++++++++++++++-- > > 1 file changed, 30 insertions(+), 2 deletions(-) > > > > diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c > > index ec02029bb6..a112428c65 100644 > > --- a/hw/virtio/virtio-iommu.c > > +++ b/hw/virtio/virtio-iommu.c > > @@ -43,6 +43,7 @@ > > > > typedef struct VirtIOIOMMUDomain { > > uint32_t id; > > + bool bypass; > I am afraid this will break the migration if you don't change > vmstate_domain. > > See static const VMStateDescription vmstate_domain. > Also you need to migrate the new bypass field. > > Logically we should handle this with a vmstate subsection I think to > handle migration of older devices. However I doubt the device has been > used in production environment supporting migration so my guess is we > may skip that burden and just add the missing field. Adding Juan, Dave & > Peter for advices.
I'm not sure about users of this; if no one has used it then yeh; you could bump up the version_id to make it a bit clearer. Dave > Thanks > > Eric > > > GTree *mappings; > > QLIST_HEAD(, VirtIOIOMMUEndpoint) endpoint_list; > > } VirtIOIOMMUDomain; > > @@ -258,12 +259,16 @@ static void virtio_iommu_put_endpoint(gpointer data) > > } > > > > static VirtIOIOMMUDomain *virtio_iommu_get_domain(VirtIOIOMMU *s, > > - uint32_t domain_id) > > + uint32_t domain_id, > > + bool bypass) > > { > > VirtIOIOMMUDomain *domain; > > > > domain = g_tree_lookup(s->domains, GUINT_TO_POINTER(domain_id)); > > if (domain) { > > + if (domain->bypass != bypass) { > > + return NULL; > > + } > > return domain; > > } > > domain = g_malloc0(sizeof(*domain)); > > @@ -271,6 +276,7 @@ static VirtIOIOMMUDomain > > *virtio_iommu_get_domain(VirtIOIOMMU *s, > > domain->mappings = g_tree_new_full((GCompareDataFunc)interval_cmp, > > NULL, (GDestroyNotify)g_free, > > (GDestroyNotify)g_free); > > + domain->bypass = bypass; > > g_tree_insert(s->domains, GUINT_TO_POINTER(domain_id), domain); > > QLIST_INIT(&domain->endpoint_list); > > trace_virtio_iommu_get_domain(domain_id); > > @@ -334,11 +340,16 @@ 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); > > + uint32_t flags = le32_to_cpu(req->flags); > > VirtIOIOMMUDomain *domain; > > VirtIOIOMMUEndpoint *ep; > > > > trace_virtio_iommu_attach(domain_id, ep_id); > > > > + if (flags & ~VIRTIO_IOMMU_ATTACH_F_BYPASS) { > > + return VIRTIO_IOMMU_S_INVAL; > > + } > > + > > ep = virtio_iommu_get_endpoint(s, ep_id); > > if (!ep) { > > return VIRTIO_IOMMU_S_NOENT; > > @@ -356,7 +367,12 @@ static int virtio_iommu_attach(VirtIOIOMMU *s, > > } > > } > > > > - domain = virtio_iommu_get_domain(s, domain_id); > > + domain = virtio_iommu_get_domain(s, domain_id, > > + flags & VIRTIO_IOMMU_ATTACH_F_BYPASS); > > + if (!domain) { > > + /* Incompatible bypass flag */ > > + return VIRTIO_IOMMU_S_INVAL; > > + } > > QLIST_INSERT_HEAD(&domain->endpoint_list, ep, next); > > > > ep->domain = domain; > > @@ -419,6 +435,10 @@ static int virtio_iommu_map(VirtIOIOMMU *s, > > return VIRTIO_IOMMU_S_NOENT; > > } > > > > + if (domain->bypass) { > > + return VIRTIO_IOMMU_S_INVAL; > > + } > > + > > interval = g_malloc0(sizeof(*interval)); > > > > interval->low = virt_start; > > @@ -464,6 +484,11 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s, > > if (!domain) { > > return VIRTIO_IOMMU_S_NOENT; > > } > > + > > + if (domain->bypass) { > > + return VIRTIO_IOMMU_S_INVAL; > > + } > > + > > interval.low = virt_start; > > interval.high = virt_end; > > > > @@ -780,6 +805,9 @@ static IOMMUTLBEntry > > virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr, > > entry.perm = flag; > > } > > goto unlock; > > + } else if (ep->domain->bypass) { > > + entry.perm = flag; > > + goto unlock; > > } > > > > found = g_tree_lookup_extended(ep->domain->mappings, > > (gpointer)(&interval), > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK