iommu/rockchip: Make use of 'struct iommu_device'
diff mbox

Message ID 1490970624-19981-1-git-send-email-joro@8bytes.org
State New
Headers show

Commit Message

Joerg Roedel March 31, 2017, 2:30 p.m. UTC
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(-)

Comments

Heiko Stuebner April 3, 2017, 9:56 a.m. UTC | #1
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
Joerg Roedel April 3, 2017, 11:07 a.m. UTC | #2
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
Heiko Stuebner April 3, 2017, 11:11 a.m. UTC | #3
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
Joerg Roedel April 3, 2017, 11:18 a.m. UTC | #4
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.

Patch
diff mbox

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;
 }