[PATCH 1/3] drm/nouveau/tegra: Use iommu_paging_domain_alloc()

[PATCH 1/3] drm/nouveau/tegra: Use iommu_paging_domain_alloc()

[Lu Baolu]

In nvkm_device_tegra_probe_iommu(), a paging domain is allocated for @dev
and attached to it on success. Use iommu_paging_domain_alloc() to make it
explicit.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Link: https://lore.kernel.org/r/20240610085555.88197-7-baolu.lu@linux.intel.com
---
 drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c b/drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c
index 87caa4a72921..763c4c2925f9 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c
@@ -120,8 +120,8 @@ nvkm_device_tegra_probe_iommu(struct nvkm_device_tegra *tdev)
 	mutex_init(&tdev->iommu.mutex);
 
 	if (device_iommu_mapped(dev)) {
-		tdev->iommu.domain = iommu_domain_alloc(&platform_bus_type);
-		if (!tdev->iommu.domain)
+		tdev->iommu.domain = iommu_paging_domain_alloc(dev);
+		if (IS_ERR(tdev->iommu.domain))
 			goto error;
 
 		/*
-- 
2.34.1

[PATCH 2/3] drm/rockchip: Use iommu_paging_domain_alloc()

[Lu Baolu]

Commit <421be3ee36a4> ("drm/rockchip: Refactor IOMMU initialisation") has
refactored rockchip_drm_init_iommu() to pass a device that the domain is
allocated for. Replace iommu_domain_alloc() with
iommu_paging_domain_alloc() to retire the former.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Link: https://lore.kernel.org/r/20240610085555.88197-19-baolu.lu@linux.intel.com
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 44d769d9234d..11e5d10de4d7 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -103,13 +103,17 @@ static int rockchip_drm_init_iommu(struct drm_device *drm_dev)
 	struct rockchip_drm_private *private = drm_dev->dev_private;
 	struct iommu_domain_geometry *geometry;
 	u64 start, end;
+	int ret;
 
 	if (IS_ERR_OR_NULL(private->iommu_dev))
 		return 0;
 
-	private->domain = iommu_domain_alloc(private->iommu_dev->bus);
-	if (!private->domain)
-		return -ENOMEM;
+	private->domain = iommu_paging_domain_alloc(private->iommu_dev);
+	if (IS_ERR(private->domain)) {
+		ret = PTR_ERR(private->domain);
+		private->domain = NULL;
+		return ret;
+	}
 
 	geometry = &private->domain->geometry;
 	start = geometry->aperture_start;
-- 
2.34.1

Re:[PATCH 2/3] drm/rockchip: Use iommu_paging_domain_alloc()

[Andy Yan]

Hi，
  Thanks for your patch

At 2024-08-12 15:10:33, "Lu Baolu" <baolu.lu@linux.intel.com> wrote:
>Commit <421be3ee36a4> ("drm/rockchip: Refactor IOMMU initialisation") has
>refactored rockchip_drm_init_iommu() to pass a device that the domain is
>allocated for. Replace iommu_domain_alloc() with
>iommu_paging_domain_alloc() to retire the former.
>
>Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
>Link: https://lore.kernel.org/r/20240610085555.88197-19-baolu.lu@linux.intel.com
  Acked-by: Andy Yan <andyshrk@163.com>


>---
> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>index 44d769d9234d..11e5d10de4d7 100644
>--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>@@ -103,13 +103,17 @@ static int rockchip_drm_init_iommu(struct drm_device *drm_dev)
> 	struct rockchip_drm_private *private = drm_dev->dev_private;
> 	struct iommu_domain_geometry *geometry;
> 	u64 start, end;
>+	int ret;
> 
> 	if (IS_ERR_OR_NULL(private->iommu_dev))
> 		return 0;
> 
>-	private->domain = iommu_domain_alloc(private->iommu_dev->bus);
>-	if (!private->domain)
>-		return -ENOMEM;
>+	private->domain = iommu_paging_domain_alloc(private->iommu_dev);
>+	if (IS_ERR(private->domain)) {
>+		ret = PTR_ERR(private->domain);
>+		private->domain = NULL;
>+		return ret;
>+	}
> 
> 	geometry = &private->domain->geometry;
> 	start = geometry->aperture_start;
>-- 
>2.34.1
>

[PATCH 3/3] drm/tegra: Remove call to iommu_domain_alloc()

[Lu Baolu]

Commit <17de3f5fdd35> ("iommu: Retire bus ops") removes iommu ops from
the bus structure. The iommu subsystem no longer relies on bus for
operations. So iommu_domain_alloc() interface is no longer relevant.

Normally, iommu_paging_domain_alloc() could be a replacement for
iommu_domain_alloc() if the caller has the right device for IOMMU API
use. Unfortunately, this is not the case for this driver.

Iterate the devices on the platform bus and find a suitable device
whose device DMA is translated by an IOMMU. Then use this device to
allocate an iommu domain. The iommu subsystem prevents domains
allocated by one iommu driver from being attached to devices managed
by any different iommu driver.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Link: https://lore.kernel.org/r/20240610085555.88197-20-baolu.lu@linux.intel.com
---
 drivers/gpu/drm/tegra/drm.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 03d1c76aec2d..ee391f859992 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1133,6 +1133,17 @@ static bool host1x_drm_wants_iommu(struct host1x_device *dev)
 	return domain != NULL;
 }
 
+static int iommu_mapped_device(struct device *dev, void *data)
+{
+	struct device **iommu_dev = data;
+
+	if (!device_iommu_mapped(dev))
+		return 0;
+
+	*iommu_dev = dev;
+	return 1;
+}
+
 static int host1x_drm_probe(struct host1x_device *dev)
 {
 	struct tegra_drm *tegra;
@@ -1149,16 +1160,21 @@ static int host1x_drm_probe(struct host1x_device *dev)
 		goto put;
 	}
 
-	if (host1x_drm_wants_iommu(dev) && iommu_present(&platform_bus_type)) {
-		tegra->domain = iommu_domain_alloc(&platform_bus_type);
-		if (!tegra->domain) {
-			err = -ENOMEM;
-			goto free;
+	if (host1x_drm_wants_iommu(dev)) {
+		struct device *iommu_dev = NULL;
+
+		bus_for_each_dev(&platform_bus_type, NULL, &iommu_dev, iommu_mapped_device);
+		if (iommu_dev) {
+			tegra->domain = iommu_paging_domain_alloc(iommu_dev);
+			if (IS_ERR(tegra->domain)) {
+				err = PTR_ERR(tegra->domain);
+				goto free;
+			}
+
+			err = iova_cache_get();
+			if (err < 0)
+				goto domain;
 		}
-
-		err = iova_cache_get();
-		if (err < 0)
-			goto domain;
 	}
 
 	mutex_init(&tegra->clients_lock);
-- 
2.34.1

Re: [PATCH 3/3] drm/tegra: Remove call to iommu_domain_alloc()

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]

On Mon, Aug 12, 2024 at 03:10:34PM GMT, Lu Baolu wrote:
> Commit <17de3f5fdd35> ("iommu: Retire bus ops") removes iommu ops from
> the bus structure. The iommu subsystem no longer relies on bus for
> operations. So iommu_domain_alloc() interface is no longer relevant.
> 
> Normally, iommu_paging_domain_alloc() could be a replacement for
> iommu_domain_alloc() if the caller has the right device for IOMMU API
> use. Unfortunately, this is not the case for this driver.
> 
> Iterate the devices on the platform bus and find a suitable device
> whose device DMA is translated by an IOMMU. Then use this device to
> allocate an iommu domain. The iommu subsystem prevents domains
> allocated by one iommu driver from being attached to devices managed
> by any different iommu driver.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Link: https://lore.kernel.org/r/20240610085555.88197-20-baolu.lu@linux.intel.com
> ---
>  drivers/gpu/drm/tegra/drm.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)

Actually I think we can just do something like this:

--- >8 ---
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index d9f0728c3afd..d35e411d536b 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1150,7 +1150,7 @@ static int host1x_drm_probe(struct host1x_device *dev)
 	}
 
 	if (host1x_drm_wants_iommu(dev) && iommu_present(&platform_bus_type)) {
-		tegra->domain = iommu_domain_alloc(&platform_bus_type);
+		tegra->domain = iommu_paging_domain_alloc(dev->dev.parent);
 		if (!tegra->domain) {
 			err = -ENOMEM;
 			goto free;
--- >8 ---

That refers to the physical device that the host1x_device virtual device
was instantiated from and is a common parent to all physical devices
that are part of the virtual device.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/3] drm/tegra: Remove call to iommu_domain_alloc()

[Baolu Lu]

On 2024/8/28 23:27, Thierry Reding wrote:
> On Mon, Aug 12, 2024 at 03:10:34PM GMT, Lu Baolu wrote:
>> Commit <17de3f5fdd35> ("iommu: Retire bus ops") removes iommu ops from
>> the bus structure. The iommu subsystem no longer relies on bus for
>> operations. So iommu_domain_alloc() interface is no longer relevant.
>>
>> Normally, iommu_paging_domain_alloc() could be a replacement for
>> iommu_domain_alloc() if the caller has the right device for IOMMU API
>> use. Unfortunately, this is not the case for this driver.
>>
>> Iterate the devices on the platform bus and find a suitable device
>> whose device DMA is translated by an IOMMU. Then use this device to
>> allocate an iommu domain. The iommu subsystem prevents domains
>> allocated by one iommu driver from being attached to devices managed
>> by any different iommu driver.
>>
>> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com>
>> Link:https://lore.kernel.org/r/20240610085555.88197-20-baolu.lu@linux.intel.com
>> ---
>>   drivers/gpu/drm/tegra/drm.c | 34 +++++++++++++++++++++++++---------
>>   1 file changed, 25 insertions(+), 9 deletions(-)
> Actually I think we can just do something like this:
> 
> --- >8 ---
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index d9f0728c3afd..d35e411d536b 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -1150,7 +1150,7 @@ static int host1x_drm_probe(struct host1x_device *dev)
>   	}
>   
>   	if (host1x_drm_wants_iommu(dev) && iommu_present(&platform_bus_type)) {
> -		tegra->domain = iommu_domain_alloc(&platform_bus_type);
> +		tegra->domain = iommu_paging_domain_alloc(dev->dev.parent);
>   		if (!tegra->domain) {
>   			err = -ENOMEM;
>   			goto free;
> --- >8 ---
> 
> That refers to the physical device that the host1x_device virtual device
> was instantiated from and is a common parent to all physical devices
> that are part of the virtual device.

Yes, this is really what we want. I will update the patch series later.

Thanks,
baolu