[PATCH] iommu/rockchip: Make use of 'struct iommu_device'

[PATCH] iommu/rockchip: Make use of 'struct iommu_device'

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Register hardware IOMMUs seperatly with the iommu-core code
and add a sysfs representation of the iommu topology.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/rockchip-iommu.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 9afcbf7..36d0890 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -90,6 +90,7 @@ struct rk_iommu {
 	void __iomem **bases;
 	int num_mmu;
 	int irq;
+	struct iommu_device iommu;
 	struct list_head node; /* entry in rk_iommu_domain.iommus */
 	struct iommu_domain *domain; /* domain to which iommu is attached */
 };
@@ -1032,6 +1033,7 @@ static int rk_iommu_group_set_iommudata(struct iommu_group *group,
 static int rk_iommu_add_device(struct device *dev)
 {
 	struct iommu_group *group;
+	struct rk_iommu *iommu;
 	int ret;
 
 	if (!rk_iommu_is_dev_iommu_master(dev))
@@ -1054,6 +1056,10 @@ static int rk_iommu_add_device(struct device *dev)
 	if (ret)
 		goto err_remove_device;
 
+	iommu = rk_iommu_from_dev(dev);
+	if (iommu)
+		iommu_device_link(&iommu->iommu, dev);
+
 	iommu_group_put(group);
 
 	return 0;
@@ -1067,9 +1073,15 @@ static int rk_iommu_add_device(struct device *dev)
 
 static void rk_iommu_remove_device(struct device *dev)
 {
+	struct rk_iommu *iommu;
+
 	if (!rk_iommu_is_dev_iommu_master(dev))
 		return;
 
+	iommu = rk_iommu_from_dev(dev);
+	if (iommu)
+		iommu_device_unlink(&iommu->iommu, dev);
+
 	iommu_group_remove_device(dev);
 }
 
@@ -1117,7 +1129,7 @@ static int rk_iommu_probe(struct platform_device *pdev)
 	struct rk_iommu *iommu;
 	struct resource *res;
 	int num_res = pdev->num_resources;
-	int i;
+	int err, i;
 
 	iommu = devm_kzalloc(dev, sizeof(*iommu), GFP_KERNEL);
 	if (!iommu)
@@ -1150,11 +1162,25 @@ static int rk_iommu_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
-	return 0;
+	err = iommu_device_sysfs_add(&iommu->iommu, dev, NULL, dev_name(dev));
+	if (err)
+		return err;
+
+	iommu_device_set_ops(&iommu->iommu, &rk_iommu_ops);
+	err = iommu_device_register(&iommu->iommu);
+
+	return err;
 }
 
 static int rk_iommu_remove(struct platform_device *pdev)
 {
+	struct rk_iommu *iommu = platform_get_drvdata(pdev);
+
+	if (iommu) {
+		iommu_device_sysfs_remove(&iommu->iommu);
+		iommu_device_unregister(&iommu->iommu);
+	}
+
 	return 0;
 }
 
-- 
1.9.1

[PATCH] iommu/rockchip: Make use of 'struct iommu_device'

[Heiko Stübner]

Hi Joerg,

Am Freitag, 31. M?rz 2017, 16:30:24 CEST schrieb Joerg Roedel:
> From: Joerg Roedel <jroedel@suse.de>
> 
> Register hardware IOMMUs seperatly with the iommu-core code
> and add a sysfs representation of the iommu topology.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>

In general works, and I still keep a working iommu-based display :-)
I can also see my two vop iommus under /sys/class/iommu now.

Links in the devices subdirectory do not work though, see below:

> ---
>  drivers/iommu/rockchip-iommu.c | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 9afcbf7..36d0890 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -1054,6 +1056,10 @@ static int rk_iommu_add_device(struct device *dev)
>  	if (ret)
>  		goto err_remove_device;
> 
> +	iommu = rk_iommu_from_dev(dev);
> +	if (iommu)
> +		iommu_device_link(&iommu->iommu, dev);
> +

It looks like the iommu association can very well run before the iommu
actually probes, via the bus_set_iommu and which happens on my veyron-pinky
device). Thus iommu is NULL, as platform_set_drvdata in probe hasn't run yet
and the link is never created.

[    0.034180] iommu: Adding device ff930000.vop to group 0
[    0.034198] rk_iommu_from_dev: found iommu ff930300.iommu for ff930000.vop with data at   (null)
[    0.034209] rk_iommu_add_device: rk_iommu_from_dev returned   (null) for ff930000.vop
[    0.034232] iommu: Adding device ff940000.vop to group 1
[    0.034247] rk_iommu_from_dev: found iommu ff940300.iommu for ff940000.vop with data at   (null)
[    0.034258] rk_iommu_add_device: rk_iommu_from_dev returned   (null) for ff940000.vop
[    0.034342] rk_iommu_probe: set_drvdata for ff930300.iommu at ee2c7a10
[    0.034411] rk_iommu_probe: set_drvdata for ff940300.iommu at ee2c7c10
[    7.326395] rk_iommu_from_dev: found iommu ff930300.iommu for ff930000.vop with data at ee2c7a10


Not sure what the proper solution is though.


Heiko

[PATCH] iommu/rockchip: Make use of 'struct iommu_device'

[Joerg Roedel]

Hey Heiko,

On Mon, Apr 03, 2017 at 11:56:59AM +0200, Heiko St?bner wrote:
> In general works, and I still keep a working iommu-based display :-)
> I can also see my two vop iommus under /sys/class/iommu now.

Great, thanks for testing that patch!

> Links in the devices subdirectory do not work though, see below:

Okay, thats bad, but I think its nothing we should work around right
now. My goal with this work is to have a per-iommu check-device
call-back to bind devices to iommus in the core code.

This will also solve the probe-ordering issue and we are fine again. We
just call check-device then for every device on the bus that has no
iommu already when the iommu is registered and create the links at that
time.



	Joerg

[PATCH] iommu/rockchip: Make use of 'struct iommu_device'

[Heiko Stübner]

Am Montag, 3. April 2017, 13:07:58 CEST schrieb Joerg Roedel:
> Hey Heiko,
> 
> On Mon, Apr 03, 2017 at 11:56:59AM +0200, Heiko St?bner wrote:
> > In general works, and I still keep a working iommu-based display :-)
> > I can also see my two vop iommus under /sys/class/iommu now.
> 
> Great, thanks for testing that patch!
> 
> > Links in the devices subdirectory do not work though, see below:
> Okay, thats bad, but I think its nothing we should work around right
> now. My goal with this work is to have a per-iommu check-device
> call-back to bind devices to iommus in the core code.
> 
> This will also solve the probe-ordering issue and we are fine again. We
> just call check-device then for every device on the bus that has no
> iommu already when the iommu is registered and create the links at that
> time.

ok, so you can at least add my
Tested-by: Heiko Stuebner <heiko@sntech.de>

on the patch


Heiko

[PATCH] iommu/rockchip: Make use of 'struct iommu_device'

[Joerg Roedel]

On Mon, Apr 03, 2017 at 01:11:01PM +0200, Heiko St?bner wrote:
> ok, so you can at least add my
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> 
> on the patch

Added and applied, thanks.