diff mbox series

[v6,12/25] iommu/tegra-smmu: Support DMA domains in tegra

Message ID 12-v6-e8114faedade+425-iommu_all_defdom_jgg@nvidia.com (mailing list archive)
State Superseded
Headers show
Series iommu: Make default_domain's mandatory | expand

Commit Message

Jason Gunthorpe Aug. 3, 2023, 12:07 a.m. UTC
All ARM64 iommu drivers should support IOMMU_DOMAIN_DMA to enable
dma-iommu.c.

tegra is blocking dma-iommu usage, and also default_domain's, because it
wants an identity translation. This is needed for some device quirk. The
correct way to do this is to support IDENTITY domains and use
ops->def_domain_type() to return IOMMU_DOMAIN_IDENTITY for only the quirky
devices.

Add support for IOMMU_DOMAIN_DMA and force IOMMU_DOMAIN_IDENTITY mode for
everything so no behavior changes.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/iommu/tegra-smmu.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Baolu Lu Aug. 14, 2023, 5:08 a.m. UTC | #1
On 2023/8/3 8:07, Jason Gunthorpe wrote:
> All ARM64 iommu drivers should support IOMMU_DOMAIN_DMA to enable
> dma-iommu.c.
> 
> tegra is blocking dma-iommu usage, and also default_domain's, because it
> wants an identity translation. This is needed for some device quirk. The
> correct way to do this is to support IDENTITY domains and use
> ops->def_domain_type() to return IOMMU_DOMAIN_IDENTITY for only the quirky
> devices.
> 
> Add support for IOMMU_DOMAIN_DMA and force IOMMU_DOMAIN_IDENTITY mode for
> everything so no behavior changes.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>   drivers/iommu/tegra-smmu.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> index f63f1d4f0bd10f..6cba034905edbf 100644
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> @@ -276,7 +276,7 @@ static struct iommu_domain *tegra_smmu_domain_alloc(unsigned type)
>   {
>   	struct tegra_smmu_as *as;
>   
> -	if (type != IOMMU_DOMAIN_UNMANAGED)
> +	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
>   		return NULL;
>   
>   	as = kzalloc(sizeof(*as), GFP_KERNEL);
> @@ -989,6 +989,12 @@ static int tegra_smmu_def_domain_type(struct device *dev)
>   }
>   
>   static const struct iommu_ops tegra_smmu_ops = {
> +	/*
> +	 * FIXME: For now we want to run all translation in IDENTITY mode,
> +	 * better would be to have a def_domain_type op do this for just the
> +	 * quirky device.
> +	 */
> +	.default_domain = &tegra_smmu_identity_domain,

tegra_smmu_def_domain_type() has already forced the core to use
ops->identity_domain, why do we still need ops->default_domain?

>   	.identity_domain = &tegra_smmu_identity_domain,
>   	.def_domain_type = &tegra_smmu_def_domain_type,
>   	.domain_alloc = tegra_smmu_domain_alloc,

Best regards,
baolu
Jason Gunthorpe Aug. 14, 2023, 3:34 p.m. UTC | #2
On Mon, Aug 14, 2023 at 01:08:39PM +0800, Baolu Lu wrote:

> > @@ -989,6 +989,12 @@ static int tegra_smmu_def_domain_type(struct device *dev)
> >   }
> >   static const struct iommu_ops tegra_smmu_ops = {
> > +	/*
> > +	 * FIXME: For now we want to run all translation in IDENTITY mode,
> > +	 * better would be to have a def_domain_type op do this for just the
> > +	 * quirky device.
> > +	 */
> > +	.default_domain = &tegra_smmu_identity_domain,
> 
> tegra_smmu_def_domain_type() has already forced the core to use
> ops->identity_domain, why do we still need ops->default_domain?

This looks like it is just some cruft from an earlier version that did
not have tegra_smmu_def_domain_type(), I deleted it

Thanks,
Jason
diff mbox series

Patch

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index f63f1d4f0bd10f..6cba034905edbf 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -276,7 +276,7 @@  static struct iommu_domain *tegra_smmu_domain_alloc(unsigned type)
 {
 	struct tegra_smmu_as *as;
 
-	if (type != IOMMU_DOMAIN_UNMANAGED)
+	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
 		return NULL;
 
 	as = kzalloc(sizeof(*as), GFP_KERNEL);
@@ -989,6 +989,12 @@  static int tegra_smmu_def_domain_type(struct device *dev)
 }
 
 static const struct iommu_ops tegra_smmu_ops = {
+	/*
+	 * FIXME: For now we want to run all translation in IDENTITY mode,
+	 * better would be to have a def_domain_type op do this for just the
+	 * quirky device.
+	 */
+	.default_domain = &tegra_smmu_identity_domain,
 	.identity_domain = &tegra_smmu_identity_domain,
 	.def_domain_type = &tegra_smmu_def_domain_type,
 	.domain_alloc = tegra_smmu_domain_alloc,