[rc] gpu: host1x: Do not assume that a NULL domain means no DMA IOMMU

Commit Message

Jason Gunthorpe Feb. 4, 2025, 7:18 p.m. UTC

Previously with tegra-smmu, even with CONFIG_IOMMU_DMA, the default domain
could have been left as NULL. The NULL domain is specially recognized by
host1x_iommu_attach() as meaning it is not the DMA domain and
should be replaced with the special shared domain.

This happened prior to the below commit because tegra-smmu was using the
NULL domain to mean IDENTITY.

Now that the domain is properly labled the test in DRM doesn't see NULL.
Check for IDENTITY as well to enable the special domains.

This is the same issue and basic fix as seen in
commit fae6e669cdc5 ("drm/tegra: Do not assume that a NULL domain means no
DMA IOMMU").

Fixes: c8cc2655cc6c ("iommu/tegra-smmu: Implement an IDENTITY domain")
Reported-by: Diogo Ivo <[e-mail]>
Closes: https://lore.kernel.org/all/[e-mail]/
Tested-by: Diogo Ivo <[e-mail]>
Signed-off-by: Jason Gunthorpe <[e-mail]>
---
 drivers/gpu/host1x/dev.c | 6 ++++++
 1 file changed, 6 insertions(+)


base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b

Comments

Diogo Ivo Feb. 13, 2025, 4:45 p.m. UTC | #1

Hi all,

On 2/4/25 7:18 PM, Jason Gunthorpe wrote:
> Previously with tegra-smmu, even with CONFIG_IOMMU_DMA, the default domain
> could have been left as NULL. The NULL domain is specially recognized by
> host1x_iommu_attach() as meaning it is not the DMA domain and
> should be replaced with the special shared domain.
> 
> This happened prior to the below commit because tegra-smmu was using the
> NULL domain to mean IDENTITY.
> 
> Now that the domain is properly labled the test in DRM doesn't see NULL.
> Check for IDENTITY as well to enable the special domains.
> 
> This is the same issue and basic fix as seen in
> commit fae6e669cdc5 ("drm/tegra: Do not assume that a NULL domain means no
> DMA IOMMU").
> 
> Fixes: c8cc2655cc6c ("iommu/tegra-smmu: Implement an IDENTITY domain")
> Reported-by: Diogo Ivo <[e-mail]>
> Closes: https://lore.kernel.org/all/[e-mail]/
> Tested-by: Diogo Ivo <[e-mail]>
> Signed-off-by: Jason Gunthorpe <[e-mail]>
> ---
>   drivers/gpu/host1x/dev.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
> index 7b1d091f3c090c..5d27662019d28b 100644
> --- a/drivers/gpu/host1x/dev.c
> +++ b/drivers/gpu/host1x/dev.c
> @@ -361,6 +361,10 @@ static bool host1x_wants_iommu(struct host1x *host1x)
>   	return true;
>   }
>   
> +/*
> + * Returns ERR_PTR on failure, NULL if the translation is IDENTITY, otherwise a
> + * valid paging domain.
> + */
>   static struct iommu_domain *host1x_iommu_attach(struct host1x *host)
>   {
>   	struct iommu_domain *domain = iommu_get_domain_for_dev(host->dev);
> @@ -385,6 +389,8 @@ static struct iommu_domain *host1x_iommu_attach(struct host1x *host)
>   	 * Similarly, if host1x is already attached to an IOMMU (via the DMA
>   	 * API), don't try to attach again.
>   	 */
> +	if (domain && domain->type == IOMMU_DOMAIN_IDENTITY)
> +		domain = NULL;
>   	if (!host1x_wants_iommu(host) || domain)
>   		return domain;
>   
> 
> base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b

Any updates on the status of this patch?

Thanks,
Diogo

Patch

diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
index 7b1d091f3c090c..5d27662019d28b 100644
--- a/drivers/gpu/host1x/dev.c
+++ b/drivers/gpu/host1x/dev.c
@@ -361,6 +361,10 @@  static bool host1x_wants_iommu(struct host1x *host1x)
 	return true;
 }
 
+/*
+ * Returns ERR_PTR on failure, NULL if the translation is IDENTITY, otherwise a
+ * valid paging domain.
+ */
 static struct iommu_domain *host1x_iommu_attach(struct host1x *host)
 {
 	struct iommu_domain *domain = iommu_get_domain_for_dev(host->dev);
@@ -385,6 +389,8 @@  static struct iommu_domain *host1x_iommu_attach(struct host1x *host)
 	 * Similarly, if host1x is already attached to an IOMMU (via the DMA
 	 * API), don't try to attach again.
 	 */
+	if (domain && domain->type == IOMMU_DOMAIN_IDENTITY)
+		domain = NULL;
 	if (!host1x_wants_iommu(host) || domain)
 		return domain;