| Message ID | 20220127142940.671333-4-jean-philippe@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG | expand |
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-philippe@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. 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),
* Eric Auger (eric.auger@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-philippe@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), >
Hi Dave, On 1/31/22 2:07 PM, Dr. David Alan Gilbert wrote: > * Eric Auger (eric.auger@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-philippe@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. Thank you for your input. Yes to me it sounds OK to only bump the version_id while adding the new field. Eric > > 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),
On Wed, Feb 02, 2022 at 02:21:37PM +0100, Eric Auger wrote: > >>> 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. > > Thank you for your input. Yes to me it sounds OK to only bump the > version_id while adding the new field. Ok. Just to make sure we're on the same page, this means we don't support migration from new->old or old->new instances, since the migration stream doesn't carry a version ID for the virtio-iommu-device and domain vmstates, as far as I understand. I also believe backward-incompatible changes are fine this time around, though I don't have much visibility in what's being used. Thanks, Jean
* Jean-Philippe Brucker (jean-philippe@linaro.org) wrote: > On Wed, Feb 02, 2022 at 02:21:37PM +0100, Eric Auger wrote: > > >>> 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. > > > > Thank you for your input. Yes to me it sounds OK to only bump the > > version_id while adding the new field. > > Ok. Just to make sure we're on the same page, this means we don't support > migration from new->old or old->new instances, since the migration stream > doesn't carry a version ID for the virtio-iommu-device and domain > vmstates, as far as I understand. I also believe backward-incompatible > changes are fine this time around, though I don't have much visibility in > what's being used. I think the stream only has it for top level devices; I've not dug into this device. Dave > Thanks, > Jean >
Hi Jean, On 2/8/22 2:09 PM, Dr. David Alan Gilbert wrote: > * Jean-Philippe Brucker (jean-philippe@linaro.org) wrote: >> On Wed, Feb 02, 2022 at 02:21:37PM +0100, Eric Auger wrote: >>>>>> 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. >>> Thank you for your input. Yes to me it sounds OK to only bump the >>> version_id while adding the new field. >> Ok. Just to make sure we're on the same page, this means we don't support >> migration from new->old or old->new instances, since the migration stream >> doesn't carry a version ID for the virtio-iommu-device and domain >> vmstates, as far as I understand. I also believe backward-incompatible >> changes are fine this time around, though I don't have much visibility in >> what's being used. > I think the stream only has it for top level devices; I've not dug into > this device. Not sure I get what you meant: vmstate_virtio_iommu has a version_id. Also vmstate_domain has one. Thanks Eric > > Dave > >> Thanks, >> Jean >>
On Tue, Feb 08, 2022 at 02:29:21PM +0100, Eric Auger wrote: > Hi Jean, > > On 2/8/22 2:09 PM, Dr. David Alan Gilbert wrote: > > * Jean-Philippe Brucker (jean-philippe@linaro.org) wrote: > >> On Wed, Feb 02, 2022 at 02:21:37PM +0100, Eric Auger wrote: > >>>>>> 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. > >>> Thank you for your input. Yes to me it sounds OK to only bump the > >>> version_id while adding the new field. > >> Ok. Just to make sure we're on the same page, this means we don't support > >> migration from new->old or old->new instances, since the migration stream > >> doesn't carry a version ID for the virtio-iommu-device and domain > >> vmstates, as far as I understand. I also believe backward-incompatible > >> changes are fine this time around, though I don't have much visibility in > >> what's being used. > > I think the stream only has it for top level devices; I've not dug into > > this device. > Not sure I get what you meant: > > vmstate_virtio_iommu has a version_id. Also vmstate_domain has one. These version numbers are not sent as part of the stream, unless I missed it. On the incoming side, virtio_load() calls vmstate_load_state() for vmstate_virtio_iommu_device and only looks at the internal version number (dc->vmsd->version_id), it doesn't check version numbers from the stream. So if the sender is old and the receiver is new, the stream doesn't have a bypass field but the receiver will assume it does, and will consider the stream corrupted. Since we're not concerned about compatibility for the moment, I think we could just add the bypass field without bumping the version number, the behavior will be the same. Thanks, Jean
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; 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),
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-philippe@linaro.org> --- hw/virtio/virtio-iommu.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-)