| Message ID | 2-v7-de04a3217c48+15055-iommu_all_defdom_jgg@nvidia.com |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | iommu: Make default_domain's mandatory | expand |
On Wed, Aug 23, 2023 at 01:47:16PM -0300, Jason Gunthorpe wrote: > This is used when the iommu driver is taking control of the dma_ops, > currently only on S390 and power spapr. It is designed to preserve the > original ops->detach_dev() semantic that these S390 was built around. > > Provide an opaque domain type and a 'default_domain' ops value that allows > the driver to trivially force any single domain as the default domain. > > Update iommufd selftest to use this instead of set_platform_dma_ops > > Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > drivers/iommu/iommu.c | 13 +++++++++++++ > drivers/iommu/iommufd/selftest.c | 14 +++++--------- > include/linux/iommu.h | 6 ++++++ > 3 files changed, 24 insertions(+), 9 deletions(-) > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index 33bd1107090720..7cedb0640290c8 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -184,6 +184,8 @@ static const char *iommu_domain_type_str(unsigned int t) > case IOMMU_DOMAIN_DMA: > case IOMMU_DOMAIN_DMA_FQ: > return "Translated"; > + case IOMMU_DOMAIN_PLATFORM: > + return "Platform"; > default: > return "Unknown"; > } > @@ -1752,6 +1754,17 @@ iommu_group_alloc_default_domain(struct iommu_group *group, int req_type) > > lockdep_assert_held(&group->mutex); > > + /* > + * Allow legacy drivers to specify the domain that will be the default > + * domain. This should always be either an IDENTITY or PLATFORM domain. > + * Do not use in new drivers. > + */ Would it be worthwhile to mention this in iommu.h for the iommu_ops default_domain? > + if (bus->iommu_ops->default_domain) { > + if (req_type) > + return ERR_PTR(-EINVAL); > + return bus->iommu_ops->default_domain; > + } > + > if (req_type) > return __iommu_group_alloc_default_domain(bus, group, req_type); > > diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c > index d48a202a7c3b81..fb981ba97c4e87 100644 > --- a/drivers/iommu/iommufd/selftest.c > +++ b/drivers/iommu/iommufd/selftest.c > @@ -281,14 +281,6 @@ static bool mock_domain_capable(struct device *dev, enum iommu_cap cap) > return cap == IOMMU_CAP_CACHE_COHERENCY; > } > > -static void mock_domain_set_plaform_dma_ops(struct device *dev) > -{ > - /* > - * mock doesn't setup default domains because we can't hook into the > - * normal probe path > - */ > -} > - > static struct iommu_device mock_iommu_device = { > }; > > @@ -298,12 +290,16 @@ static struct iommu_device *mock_probe_device(struct device *dev) > } > > static const struct iommu_ops mock_ops = { > + /* > + * IOMMU_DOMAIN_BLOCKED cannot be returned from def_domain_type() > + * because it is zero. > + */ > + .default_domain = &mock_blocking_domain, > .owner = THIS_MODULE, > .pgsize_bitmap = MOCK_IO_PAGE_SIZE, > .hw_info = mock_domain_hw_info, > .domain_alloc = mock_domain_alloc, > .capable = mock_domain_capable, > - .set_platform_dma_ops = mock_domain_set_plaform_dma_ops, > .device_group = generic_device_group, > .probe_device = mock_probe_device, > .default_domain_ops = > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > index d0920b2a9f1c0e..48a18b6e07abff 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -64,6 +64,7 @@ struct iommu_domain_geometry { > #define __IOMMU_DOMAIN_DMA_FQ (1U << 3) /* DMA-API uses flush queue */ > > #define __IOMMU_DOMAIN_SVA (1U << 4) /* Shared process address space */ > +#define __IOMMU_DOMAIN_PLATFORM (1U << 5) > > #define IOMMU_DOMAIN_ALLOC_FLAGS ~__IOMMU_DOMAIN_DMA_FQ > /* > @@ -81,6 +82,8 @@ struct iommu_domain_geometry { > * invalidation. > * IOMMU_DOMAIN_SVA - DMA addresses are shared process addresses > * represented by mm_struct's. > + * IOMMU_DOMAIN_PLATFORM - Legacy domain for drivers that do their own > + * dma_api stuff. Do not use in new drivers. > */ > #define IOMMU_DOMAIN_BLOCKED (0U) > #define IOMMU_DOMAIN_IDENTITY (__IOMMU_DOMAIN_PT) > @@ -91,6 +94,7 @@ struct iommu_domain_geometry { > __IOMMU_DOMAIN_DMA_API | \ > __IOMMU_DOMAIN_DMA_FQ) > #define IOMMU_DOMAIN_SVA (__IOMMU_DOMAIN_SVA) > +#define IOMMU_DOMAIN_PLATFORM (__IOMMU_DOMAIN_PLATFORM) > > struct iommu_domain { > unsigned type; > @@ -262,6 +266,7 @@ struct iommu_iotlb_gather { > * @owner: Driver module providing these ops > * @identity_domain: An always available, always attachable identity > * translation. > + * @default_domain: If not NULL this will always be set as the default domain. > */ > struct iommu_ops { > bool (*capable)(struct device *dev, enum iommu_cap); > @@ -297,6 +302,7 @@ struct iommu_ops { > unsigned long pgsize_bitmap; > struct module *owner; > struct iommu_domain *identity_domain; > + struct iommu_domain *default_domain; > }; > > /** > -- > 2.41.0 >
On Thu, Aug 24, 2023 at 06:51:48PM -0700, Jerry Snitselaar wrote: > > + /* > > + * Allow legacy drivers to specify the domain that will be the default > > + * domain. This should always be either an IDENTITY or PLATFORM domain. > > + * Do not use in new drivers. > > + */ > > Would it be worthwhile to mention this in iommu.h for the iommu_ops default_domain? I did this: diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 11d47f9ac9b345..7fa53d28feca87 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -1757,8 +1757,8 @@ iommu_group_alloc_default_domain(struct iommu_group *group, int req_type) /* * Allow legacy drivers to specify the domain that will be the default - * domain. This should always be either an IDENTITY or PLATFORM domain. - * Do not use in new drivers. + * domain. This should always be either an IDENTITY/BLOCKED/PLATFORM + * domain. Do not use in new drivers. */ if (ops->default_domain) { if (req_type) diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 7e9d94a56f473e..6f9e0aacc4431a 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -267,6 +267,8 @@ struct iommu_iotlb_gather { * @blocked_domain: An always available, always attachable blocking * translation. * @default_domain: If not NULL this will always be set as the default domain. + * This should be an IDENTITY/BLOCKED/PLATFORM domain. + * Do not use in new drivers. */ struct iommu_ops { bool (*capable)(struct device *dev, enum iommu_cap); Thanks, Jason
On Fri, Aug 25, 2023 at 02:40:10PM -0300, Jason Gunthorpe wrote: > On Thu, Aug 24, 2023 at 06:51:48PM -0700, Jerry Snitselaar wrote: > > > > + /* > > > + * Allow legacy drivers to specify the domain that will be the default > > > + * domain. This should always be either an IDENTITY or PLATFORM domain. > > > + * Do not use in new drivers. > > > + */ > > > > Would it be worthwhile to mention this in iommu.h for the iommu_ops default_domain? > > I did this: > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index 11d47f9ac9b345..7fa53d28feca87 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -1757,8 +1757,8 @@ iommu_group_alloc_default_domain(struct iommu_group *group, int req_type) > > /* > * Allow legacy drivers to specify the domain that will be the default > - * domain. This should always be either an IDENTITY or PLATFORM domain. > - * Do not use in new drivers. > + * domain. This should always be either an IDENTITY/BLOCKED/PLATFORM > + * domain. Do not use in new drivers. > */ > if (ops->default_domain) { > if (req_type) > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > index 7e9d94a56f473e..6f9e0aacc4431a 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -267,6 +267,8 @@ struct iommu_iotlb_gather { > * @blocked_domain: An always available, always attachable blocking > * translation. > * @default_domain: If not NULL this will always be set as the default domain. > + * This should be an IDENTITY/BLOCKED/PLATFORM domain. > + * Do not use in new drivers. > */ > struct iommu_ops { > bool (*capable)(struct device *dev, enum iommu_cap); > > Thanks, > Jason > For all of 02/24 Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com> > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 33bd1107090720..7cedb0640290c8 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -184,6 +184,8 @@ static const char *iommu_domain_type_str(unsigned int t) case IOMMU_DOMAIN_DMA: case IOMMU_DOMAIN_DMA_FQ: return "Translated"; + case IOMMU_DOMAIN_PLATFORM: + return "Platform"; default: return "Unknown"; } @@ -1752,6 +1754,17 @@ iommu_group_alloc_default_domain(struct iommu_group *group, int req_type) lockdep_assert_held(&group->mutex); + /* + * Allow legacy drivers to specify the domain that will be the default + * domain. This should always be either an IDENTITY or PLATFORM domain. + * Do not use in new drivers. + */ + if (bus->iommu_ops->default_domain) { + if (req_type) + return ERR_PTR(-EINVAL); + return bus->iommu_ops->default_domain; + } + if (req_type) return __iommu_group_alloc_default_domain(bus, group, req_type); diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c index d48a202a7c3b81..fb981ba97c4e87 100644 --- a/drivers/iommu/iommufd/selftest.c +++ b/drivers/iommu/iommufd/selftest.c @@ -281,14 +281,6 @@ static bool mock_domain_capable(struct device *dev, enum iommu_cap cap) return cap == IOMMU_CAP_CACHE_COHERENCY; } -static void mock_domain_set_plaform_dma_ops(struct device *dev) -{ - /* - * mock doesn't setup default domains because we can't hook into the - * normal probe path - */ -} - static struct iommu_device mock_iommu_device = { }; @@ -298,12 +290,16 @@ static struct iommu_device *mock_probe_device(struct device *dev) } static const struct iommu_ops mock_ops = { + /* + * IOMMU_DOMAIN_BLOCKED cannot be returned from def_domain_type() + * because it is zero. + */ + .default_domain = &mock_blocking_domain, .owner = THIS_MODULE, .pgsize_bitmap = MOCK_IO_PAGE_SIZE, .hw_info = mock_domain_hw_info, .domain_alloc = mock_domain_alloc, .capable = mock_domain_capable, - .set_platform_dma_ops = mock_domain_set_plaform_dma_ops, .device_group = generic_device_group, .probe_device = mock_probe_device, .default_domain_ops = diff --git a/include/linux/iommu.h b/include/linux/iommu.h index d0920b2a9f1c0e..48a18b6e07abff 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -64,6 +64,7 @@ struct iommu_domain_geometry { #define __IOMMU_DOMAIN_DMA_FQ (1U << 3) /* DMA-API uses flush queue */ #define __IOMMU_DOMAIN_SVA (1U << 4) /* Shared process address space */ +#define __IOMMU_DOMAIN_PLATFORM (1U << 5) #define IOMMU_DOMAIN_ALLOC_FLAGS ~__IOMMU_DOMAIN_DMA_FQ /* @@ -81,6 +82,8 @@ struct iommu_domain_geometry { * invalidation. * IOMMU_DOMAIN_SVA - DMA addresses are shared process addresses * represented by mm_struct's. + * IOMMU_DOMAIN_PLATFORM - Legacy domain for drivers that do their own + * dma_api stuff. Do not use in new drivers. */ #define IOMMU_DOMAIN_BLOCKED (0U) #define IOMMU_DOMAIN_IDENTITY (__IOMMU_DOMAIN_PT) @@ -91,6 +94,7 @@ struct iommu_domain_geometry { __IOMMU_DOMAIN_DMA_API | \ __IOMMU_DOMAIN_DMA_FQ) #define IOMMU_DOMAIN_SVA (__IOMMU_DOMAIN_SVA) +#define IOMMU_DOMAIN_PLATFORM (__IOMMU_DOMAIN_PLATFORM) struct iommu_domain { unsigned type; @@ -262,6 +266,7 @@ struct iommu_iotlb_gather { * @owner: Driver module providing these ops * @identity_domain: An always available, always attachable identity * translation. + * @default_domain: If not NULL this will always be set as the default domain. */ struct iommu_ops { bool (*capable)(struct device *dev, enum iommu_cap); @@ -297,6 +302,7 @@ struct iommu_ops { unsigned long pgsize_bitmap; struct module *owner; struct iommu_domain *identity_domain; + struct iommu_domain *default_domain; }; /**