| Message ID | 5-v1-21cc72fcfb22+a7a-iommu_all_defdom_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iommu: Make default_domain's mandatory | expand |
On 2023-05-01 19:02, Jason Gunthorpe wrote: > Even though dma-iommu.c and CONFIG_ARM_DMA_USE_IOMMU do approximately the > same stuff, the way they relate to the IOMMU core is quiet different. > > dma-iommu.c expects the core code to setup an UNMANAGED domain (of type > IOMMU_DOMAIN_DMA) and then configures itself to use that domain. This > becomes the default_domain for the group. > > ARM_DMA_USE_IOMMU does not use the default_domain, instead it directly > allocates an UNMANAGED domain and operates it just like an external > driver. In this case group->default_domain is NULL. > > Allow iommu drivers to specify a global static identity_domain and, if > present, automatically use this domain as the default_domain when in > ARM_DMA_USE_IOMMU mode. > > This allows drivers that implemented default_domain == NULL as an IDENTITY > translation to trivially get a properly labeled non-NULL default_domain on > ARM32 configs. > > With this arrangment when ARM_DMA_USE_IOMMU wants to disconnect from the > device the normal detach_domain flow will restore the IDENTITY domain as > the default domain. Overall this makes attach_dev() of the IDENTITY domain > called in the same places as detach_dev(). > > This effectively migrates these drivers to default_domain mode. For > drivers that support ARM64 they will gain support for the IDENTITY > translation mode for the dma_api and behave in a uniform way. > > Drivers use this by setting ops->identity_domain to a static singleton > iommu_domain that implements the identity attach. If the core detects > ARM_DMA_USE_IOMMU mode then it automatically attaches the IDENTITY domain > during probe. > > If the driver does not want to support dma_api with translation then it > always sets default_domain to the identity domain and even if IOMMU_DMA is > turned on it will not allow it to be used. Could we not retain the use of .def_domain_type for this? I think if we can avoid the IOMMU_DOMAIN_PLATFORM thing altogether than that's the more appealing option. > This allows removing the set_platform_dma_ops() from every remaining > driver. > > Add the core support and convert rockchip to use it. > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > drivers/iommu/iommu.c | 13 +++++++++++++ > drivers/iommu/rockchip-iommu.c | 19 +------------------ > include/linux/iommu.h | 3 +++ > 3 files changed, 17 insertions(+), 18 deletions(-) > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index ba7f38630665b5..8b9af774de68f1 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -1654,6 +1654,16 @@ iommu_group_alloc_default_domain(struct iommu_group *group, int req_type) > if (req_type) > return __iommu_group_alloc_default_domain(bus, group, req_type); > > + /* > + * ARM32 drivers supporting CONFIG_ARM_DMA_USE_IOMMU can declare an > + * identity_domain and it becomes their default domain. Later on > + * ARM_DMA_USE_IOMMU will install its UNMANAGED domain. > + */ > + if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU) && For the sake of reasoning, I think just use CONFIG_ARM for this. Otherwise we may still end up with potential corner cases of default_domain == NULL that we'd rather not have to accommodate. > + bus->iommu_ops->identity_domain) > + return __iommu_group_alloc_default_domain( > + bus, group, IOMMU_DOMAIN_IDENTITY); Why not simply return the identity_domain that we now know exists? It would seem even more logical, however, to put this ARM workaround in iommu_get_default_domain_type() and keep the actual allocation path clean. Do we strictly need to check for identity_domain up front? (Note that as it stands this narrowly misses out on un-breaking arm-smmu for 32-bit) Thanks, Robin. > + > /* The driver gave no guidance on what type to use, try the default */ > dom = __iommu_group_alloc_default_domain(bus, group, iommu_def_domain_type); > if (dom) > @@ -1923,6 +1933,9 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus, > if (bus == NULL || bus->iommu_ops == NULL) > return NULL; > > + if (type == IOMMU_DOMAIN_IDENTITY && bus->iommu_ops->identity_domain) > + return bus->iommu_ops->identity_domain; > + > domain = bus->iommu_ops->domain_alloc(type); > if (!domain) > return NULL; > diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c > index ea5a3088bb7e8a..9e1296a856ac4c 100644 > --- a/drivers/iommu/rockchip-iommu.c > +++ b/drivers/iommu/rockchip-iommu.c > @@ -1017,13 +1017,8 @@ static int rk_iommu_identity_attach(struct iommu_domain *identity_domain, > return 0; > } > > -static void rk_iommu_identity_free(struct iommu_domain *domain) > -{ > -} > - > static struct iommu_domain_ops rk_identity_ops = { > .attach_dev = rk_iommu_identity_attach, > - .free = rk_iommu_identity_free, > }; > > static struct iommu_domain rk_identity_domain = { > @@ -1031,13 +1026,6 @@ static struct iommu_domain rk_identity_domain = { > .ops = &rk_identity_ops, > }; > > -#ifdef CONFIG_ARM > -static void rk_iommu_set_platform_dma(struct device *dev) > -{ > - WARN_ON(rk_iommu_identity_attach(&rk_identity_domain, dev)); > -} > -#endif > - > static int rk_iommu_attach_device(struct iommu_domain *domain, > struct device *dev) > { > @@ -1087,9 +1075,6 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type) > { > struct rk_iommu_domain *rk_domain; > > - if (type == IOMMU_DOMAIN_IDENTITY) > - return &rk_identity_domain; > - > if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA) > return NULL; > > @@ -1214,13 +1199,11 @@ static int rk_iommu_of_xlate(struct device *dev, > } > > static const struct iommu_ops rk_iommu_ops = { > + .identity_domain = &rk_identity_domain, > .domain_alloc = rk_iommu_domain_alloc, > .probe_device = rk_iommu_probe_device, > .release_device = rk_iommu_release_device, > .device_group = rk_iommu_device_group, > -#ifdef CONFIG_ARM > - .set_platform_dma_ops = rk_iommu_set_platform_dma, > -#endif > .pgsize_bitmap = RK_IOMMU_PGSIZE_BITMAP, > .of_xlate = rk_iommu_of_xlate, > .default_domain_ops = &(const struct iommu_domain_ops) { > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > index ddcad3597c177b..427490b5736d40 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -253,6 +253,8 @@ struct iommu_iotlb_gather { > * @pgsize_bitmap: bitmap of all possible supported page sizes > * @owner: Driver module providing these ops > * @default_domain: If not NULL this will always be set as the default domain. > + * @identity_domain: An always available, always attachable identity > + * translation. > */ > struct iommu_ops { > bool (*capable)(struct device *dev, enum iommu_cap); > @@ -287,6 +289,7 @@ struct iommu_ops { > unsigned long pgsize_bitmap; > struct module *owner; > struct iommu_domain *default_domain; > + struct iommu_domain *identity_domain; > }; > > /**
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index ba7f38630665b5..8b9af774de68f1 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -1654,6 +1654,16 @@ iommu_group_alloc_default_domain(struct iommu_group *group, int req_type) if (req_type) return __iommu_group_alloc_default_domain(bus, group, req_type); + /* + * ARM32 drivers supporting CONFIG_ARM_DMA_USE_IOMMU can declare an + * identity_domain and it becomes their default domain. Later on + * ARM_DMA_USE_IOMMU will install its UNMANAGED domain. + */ + if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU) && + bus->iommu_ops->identity_domain) + return __iommu_group_alloc_default_domain( + bus, group, IOMMU_DOMAIN_IDENTITY); + /* The driver gave no guidance on what type to use, try the default */ dom = __iommu_group_alloc_default_domain(bus, group, iommu_def_domain_type); if (dom) @@ -1923,6 +1933,9 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus, if (bus == NULL || bus->iommu_ops == NULL) return NULL; + if (type == IOMMU_DOMAIN_IDENTITY && bus->iommu_ops->identity_domain) + return bus->iommu_ops->identity_domain; + domain = bus->iommu_ops->domain_alloc(type); if (!domain) return NULL; diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index ea5a3088bb7e8a..9e1296a856ac4c 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -1017,13 +1017,8 @@ static int rk_iommu_identity_attach(struct iommu_domain *identity_domain, return 0; } -static void rk_iommu_identity_free(struct iommu_domain *domain) -{ -} - static struct iommu_domain_ops rk_identity_ops = { .attach_dev = rk_iommu_identity_attach, - .free = rk_iommu_identity_free, }; static struct iommu_domain rk_identity_domain = { @@ -1031,13 +1026,6 @@ static struct iommu_domain rk_identity_domain = { .ops = &rk_identity_ops, }; -#ifdef CONFIG_ARM -static void rk_iommu_set_platform_dma(struct device *dev) -{ - WARN_ON(rk_iommu_identity_attach(&rk_identity_domain, dev)); -} -#endif - static int rk_iommu_attach_device(struct iommu_domain *domain, struct device *dev) { @@ -1087,9 +1075,6 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type) { struct rk_iommu_domain *rk_domain; - if (type == IOMMU_DOMAIN_IDENTITY) - return &rk_identity_domain; - if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA) return NULL; @@ -1214,13 +1199,11 @@ static int rk_iommu_of_xlate(struct device *dev, } static const struct iommu_ops rk_iommu_ops = { + .identity_domain = &rk_identity_domain, .domain_alloc = rk_iommu_domain_alloc, .probe_device = rk_iommu_probe_device, .release_device = rk_iommu_release_device, .device_group = rk_iommu_device_group, -#ifdef CONFIG_ARM - .set_platform_dma_ops = rk_iommu_set_platform_dma, -#endif .pgsize_bitmap = RK_IOMMU_PGSIZE_BITMAP, .of_xlate = rk_iommu_of_xlate, .default_domain_ops = &(const struct iommu_domain_ops) { diff --git a/include/linux/iommu.h b/include/linux/iommu.h index ddcad3597c177b..427490b5736d40 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -253,6 +253,8 @@ struct iommu_iotlb_gather { * @pgsize_bitmap: bitmap of all possible supported page sizes * @owner: Driver module providing these ops * @default_domain: If not NULL this will always be set as the default domain. + * @identity_domain: An always available, always attachable identity + * translation. */ struct iommu_ops { bool (*capable)(struct device *dev, enum iommu_cap); @@ -287,6 +289,7 @@ struct iommu_ops { unsigned long pgsize_bitmap; struct module *owner; struct iommu_domain *default_domain; + struct iommu_domain *identity_domain; }; /**
Even though dma-iommu.c and CONFIG_ARM_DMA_USE_IOMMU do approximately the same stuff, the way they relate to the IOMMU core is quiet different. dma-iommu.c expects the core code to setup an UNMANAGED domain (of type IOMMU_DOMAIN_DMA) and then configures itself to use that domain. This becomes the default_domain for the group. ARM_DMA_USE_IOMMU does not use the default_domain, instead it directly allocates an UNMANAGED domain and operates it just like an external driver. In this case group->default_domain is NULL. Allow iommu drivers to specify a global static identity_domain and, if present, automatically use this domain as the default_domain when in ARM_DMA_USE_IOMMU mode. This allows drivers that implemented default_domain == NULL as an IDENTITY translation to trivially get a properly labeled non-NULL default_domain on ARM32 configs. With this arrangment when ARM_DMA_USE_IOMMU wants to disconnect from the device the normal detach_domain flow will restore the IDENTITY domain as the default domain. Overall this makes attach_dev() of the IDENTITY domain called in the same places as detach_dev(). This effectively migrates these drivers to default_domain mode. For drivers that support ARM64 they will gain support for the IDENTITY translation mode for the dma_api and behave in a uniform way. Drivers use this by setting ops->identity_domain to a static singleton iommu_domain that implements the identity attach. If the core detects ARM_DMA_USE_IOMMU mode then it automatically attaches the IDENTITY domain during probe. If the driver does not want to support dma_api with translation then it always sets default_domain to the identity domain and even if IOMMU_DMA is turned on it will not allow it to be used. This allows removing the set_platform_dma_ops() from every remaining driver. Add the core support and convert rockchip to use it. Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- drivers/iommu/iommu.c | 13 +++++++++++++ drivers/iommu/rockchip-iommu.c | 19 +------------------ include/linux/iommu.h | 3 +++ 3 files changed, 17 insertions(+), 18 deletions(-)