All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] iommu/tegra*: Add support for struct iommu_device
@ 2017-08-09 22:29 ` Joerg Roedel
  0 siblings, 0 replies; 27+ messages in thread
From: Joerg Roedel @ 2017-08-09 22:29 UTC (permalink / raw)
  To: Hiroshi Doyu, Thierry Reding, Jonathan Hunter
  Cc: Robin Murphy, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Joerg Roedel

Hi,

here are two patches to add support for 'struct iommu_device'
to the tegra iommu-drivers. This will make the iommu-core
code aware of the hardware iommus that a driver manages.

It will also add the iommus to sysfs and link them to the
devices managed by them.

The patches apply on-top of Robin's iommu-group patches.

Please review.

Regards,

	Joerg

Joerg Roedel (2):
  iommu/tegra: Add support for struct iommu_device
  iommu/tegra-gart: Add support for struct iommu_device

 drivers/iommu/tegra-gart.c | 26 ++++++++++++++++++++++++++
 drivers/iommu/tegra-smmu.c | 25 +++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 0/2] iommu/tegra*: Add support for struct iommu_device
@ 2017-08-09 22:29 ` Joerg Roedel
  0 siblings, 0 replies; 27+ messages in thread
From: Joerg Roedel @ 2017-08-09 22:29 UTC (permalink / raw)
  To: Hiroshi Doyu, Thierry Reding, Jonathan Hunter
  Cc: Robin Murphy, iommu, linux-tegra, linux-kernel, Joerg Roedel

Hi,

here are two patches to add support for 'struct iommu_device'
to the tegra iommu-drivers. This will make the iommu-core
code aware of the hardware iommus that a driver manages.

It will also add the iommus to sysfs and link them to the
devices managed by them.

The patches apply on-top of Robin's iommu-group patches.

Please review.

Regards,

	Joerg

Joerg Roedel (2):
  iommu/tegra: Add support for struct iommu_device
  iommu/tegra-gart: Add support for struct iommu_device

 drivers/iommu/tegra-gart.c | 26 ++++++++++++++++++++++++++
 drivers/iommu/tegra-smmu.c | 25 +++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 1/2] iommu/tegra: Add support for struct iommu_device
  2017-08-09 22:29 ` Joerg Roedel
  (?)
@ 2017-08-09 22:29 ` Joerg Roedel
       [not found]   ` <1502317752-8792-2-git-send-email-joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
  2017-08-30 11:04     ` Jon Hunter
  -1 siblings, 2 replies; 27+ messages in thread
From: Joerg Roedel @ 2017-08-09 22:29 UTC (permalink / raw)
  To: Hiroshi Doyu, Thierry Reding, Jonathan Hunter
  Cc: Robin Murphy, iommu, linux-tegra, linux-kernel, Joerg Roedel

From: Joerg Roedel <jroedel@suse.de>

Add a struct iommu_device to each tegra-smmu and register it
with the iommu-core. Also link devices added to the driver
to their respective hardware iommus.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/tegra-smmu.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index faa9c1e..2802e12 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -36,6 +36,8 @@ struct tegra_smmu {
 	struct list_head list;
 
 	struct dentry *debugfs;
+
+	struct iommu_device iommu;	/* IOMMU Core code handle */
 };
 
 struct tegra_smmu_as {
@@ -720,6 +722,9 @@ static int tegra_smmu_add_device(struct device *dev)
 			 * first match.
 			 */
 			dev->archdata.iommu = smmu;
+
+			iommu_device_link(&smmu->iommu, dev);
+
 			break;
 		}
 
@@ -737,6 +742,11 @@ static int tegra_smmu_add_device(struct device *dev)
 
 static void tegra_smmu_remove_device(struct device *dev)
 {
+	struct tegra_smmu *smmu = dev->archdata.iommu;
+
+	if (smmu)
+		iommu_device_unlink(&smmu->iommu, dev);
+
 	dev->archdata.iommu = NULL;
 	iommu_group_remove_device(dev);
 }
@@ -943,6 +953,18 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
 	if (err < 0)
 		return ERR_PTR(err);
 
+	err = iommu_device_sysfs_add(&smmu->iommu, dev, NULL, dev_name(dev));
+	if (err)
+		return ERR_PTR(err);
+
+	iommu_device_set_ops(&smmu->iommu, &tegra_smmu_ops);
+
+	err = iommu_device_register(&smmu->iommu);
+	if (err) {
+		iommu_device_sysfs_remove(&smmu->iommu);
+		return ERR_PTR(err);
+	}
+
 	if (IS_ENABLED(CONFIG_DEBUG_FS))
 		tegra_smmu_debugfs_init(smmu);
 
@@ -951,6 +973,9 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
 
 void tegra_smmu_remove(struct tegra_smmu *smmu)
 {
+	iommu_device_unregister(&smmu->iommu);
+	iommu_device_sysfs_remove(&smmu->iommu);
+
 	if (IS_ENABLED(CONFIG_DEBUG_FS))
 		tegra_smmu_debugfs_exit(smmu);
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 2/2] iommu/tegra-gart: Add support for struct iommu_device
  2017-08-09 22:29 ` Joerg Roedel
@ 2017-08-09 22:29     ` Joerg Roedel
  -1 siblings, 0 replies; 27+ messages in thread
From: Joerg Roedel @ 2017-08-09 22:29 UTC (permalink / raw)
  To: Hiroshi Doyu, Thierry Reding, Jonathan Hunter
  Cc: Robin Murphy, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Joerg Roedel

From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>

Add a struct iommu_device to each tegra-gart and register it
with the iommu-core. Also link devices added to the driver
to their respective hardware iommus.

Signed-off-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---
 drivers/iommu/tegra-gart.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
index 29bafc6..b62f790 100644
--- a/drivers/iommu/tegra-gart.c
+++ b/drivers/iommu/tegra-gart.c
@@ -61,6 +61,8 @@ struct gart_device {
 	struct list_head	client;
 	spinlock_t		client_lock;	/* for client list */
 	struct device		*dev;
+
+	struct iommu_device	iommu;		/* IOMMU Core handle */
 };
 
 struct gart_domain {
@@ -342,12 +344,16 @@ static int gart_iommu_add_device(struct device *dev)
 		return PTR_ERR(group);
 
 	iommu_group_put(group);
+
+	iommu_device_link(&gart_handle->iommu, dev);
+
 	return 0;
 }
 
 static void gart_iommu_remove_device(struct device *dev)
 {
 	iommu_group_remove_device(dev);
+	iommu_device_unlink(&gart_handle->iommu, dev);
 }
 
 static const struct iommu_ops gart_iommu_ops = {
@@ -397,6 +403,7 @@ static int tegra_gart_probe(struct platform_device *pdev)
 	struct resource *res, *res_remap;
 	void __iomem *gart_regs;
 	struct device *dev = &pdev->dev;
+	int ret;
 
 	if (gart_handle)
 		return -EIO;
@@ -423,6 +430,22 @@ static int tegra_gart_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
+	ret = iommu_device_sysfs_add(&gart->iommu, &pdev->dev, NULL,
+				     dev_name(&pdev->dev));
+	if (ret) {
+		dev_err(dev, "Failed to register IOMMU in sysfs\n");
+		return ret;
+	}
+
+	iommu_device_set_ops(&gart->iommu, &gart_iommu_ops);
+
+	ret = iommu_device_register(&gart->iommu);
+	if (ret) {
+		dev_err(dev, "Failed to register IOMMU\n");
+		iommu_device_sysfs_remove(&gart->iommu);
+		return ret;
+	}
+
 	gart->dev = &pdev->dev;
 	spin_lock_init(&gart->pte_lock);
 	spin_lock_init(&gart->client_lock);
@@ -449,6 +472,9 @@ static int tegra_gart_remove(struct platform_device *pdev)
 {
 	struct gart_device *gart = platform_get_drvdata(pdev);
 
+	iommu_device_unregister(&gart->iommu);
+	iommu_device_sysfs_remove(&gart->iommu);
+
 	writel(0, gart->regs + GART_CONFIG);
 	if (gart->savedata)
 		vfree(gart->savedata);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 2/2] iommu/tegra-gart: Add support for struct iommu_device
@ 2017-08-09 22:29     ` Joerg Roedel
  0 siblings, 0 replies; 27+ messages in thread
From: Joerg Roedel @ 2017-08-09 22:29 UTC (permalink / raw)
  To: Hiroshi Doyu, Thierry Reding, Jonathan Hunter
  Cc: Robin Murphy, iommu, linux-tegra, linux-kernel, Joerg Roedel

From: Joerg Roedel <jroedel@suse.de>

Add a struct iommu_device to each tegra-gart and register it
with the iommu-core. Also link devices added to the driver
to their respective hardware iommus.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/tegra-gart.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
index 29bafc6..b62f790 100644
--- a/drivers/iommu/tegra-gart.c
+++ b/drivers/iommu/tegra-gart.c
@@ -61,6 +61,8 @@ struct gart_device {
 	struct list_head	client;
 	spinlock_t		client_lock;	/* for client list */
 	struct device		*dev;
+
+	struct iommu_device	iommu;		/* IOMMU Core handle */
 };
 
 struct gart_domain {
@@ -342,12 +344,16 @@ static int gart_iommu_add_device(struct device *dev)
 		return PTR_ERR(group);
 
 	iommu_group_put(group);
+
+	iommu_device_link(&gart_handle->iommu, dev);
+
 	return 0;
 }
 
 static void gart_iommu_remove_device(struct device *dev)
 {
 	iommu_group_remove_device(dev);
+	iommu_device_unlink(&gart_handle->iommu, dev);
 }
 
 static const struct iommu_ops gart_iommu_ops = {
@@ -397,6 +403,7 @@ static int tegra_gart_probe(struct platform_device *pdev)
 	struct resource *res, *res_remap;
 	void __iomem *gart_regs;
 	struct device *dev = &pdev->dev;
+	int ret;
 
 	if (gart_handle)
 		return -EIO;
@@ -423,6 +430,22 @@ static int tegra_gart_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
+	ret = iommu_device_sysfs_add(&gart->iommu, &pdev->dev, NULL,
+				     dev_name(&pdev->dev));
+	if (ret) {
+		dev_err(dev, "Failed to register IOMMU in sysfs\n");
+		return ret;
+	}
+
+	iommu_device_set_ops(&gart->iommu, &gart_iommu_ops);
+
+	ret = iommu_device_register(&gart->iommu);
+	if (ret) {
+		dev_err(dev, "Failed to register IOMMU\n");
+		iommu_device_sysfs_remove(&gart->iommu);
+		return ret;
+	}
+
 	gart->dev = &pdev->dev;
 	spin_lock_init(&gart->pte_lock);
 	spin_lock_init(&gart->client_lock);
@@ -449,6 +472,9 @@ static int tegra_gart_remove(struct platform_device *pdev)
 {
 	struct gart_device *gart = platform_get_drvdata(pdev);
 
+	iommu_device_unregister(&gart->iommu);
+	iommu_device_sysfs_remove(&gart->iommu);
+
 	writel(0, gart->regs + GART_CONFIG);
 	if (gart->savedata)
 		vfree(gart->savedata);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH 2/2] iommu/tegra-gart: Add support for struct iommu_device
  2017-08-09 22:29     ` Joerg Roedel
@ 2017-08-16 22:21         ` Dmitry Osipenko
  -1 siblings, 0 replies; 27+ messages in thread
From: Dmitry Osipenko @ 2017-08-16 22:21 UTC (permalink / raw)
  To: Joerg Roedel, Hiroshi Doyu, Thierry Reding, Jonathan Hunter
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Joerg Roedel,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Hello Joerg,

On 10.08.2017 01:29, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> 
> Add a struct iommu_device to each tegra-gart and register it
> with the iommu-core. Also link devices added to the driver
> to their respective hardware iommus.
> 
> Signed-off-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> ---
>  drivers/iommu/tegra-gart.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 

Reviewed-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Tested-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
> index 29bafc6..b62f790 100644
> --- a/drivers/iommu/tegra-gart.c
> +++ b/drivers/iommu/tegra-gart.c

[snip]

> @@ -449,6 +472,9 @@ static int tegra_gart_remove(struct platform_device *pdev)
>  {

BTW, GART's driver can't be build as a module, so this function is pretty much a
dead code. Probably worth considering its removal.

>  	struct gart_device *gart = platform_get_drvdata(pdev);
>  
> +	iommu_device_unregister(&gart->iommu);
> +	iommu_device_sysfs_remove(&gart->iommu);
> +
>  	writel(0, gart->regs + GART_CONFIG);
>  	if (gart->savedata)
>  		vfree(gart->savedata);
> 


-- 
Dmitry

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 2/2] iommu/tegra-gart: Add support for struct iommu_device
@ 2017-08-16 22:21         ` Dmitry Osipenko
  0 siblings, 0 replies; 27+ messages in thread
From: Dmitry Osipenko @ 2017-08-16 22:21 UTC (permalink / raw)
  To: Joerg Roedel, Hiroshi Doyu, Thierry Reding, Jonathan Hunter
  Cc: Robin Murphy, iommu, linux-tegra, linux-kernel, Joerg Roedel

Hello Joerg,

On 10.08.2017 01:29, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> Add a struct iommu_device to each tegra-gart and register it
> with the iommu-core. Also link devices added to the driver
> to their respective hardware iommus.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/iommu/tegra-gart.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
Tested-by: Dmitry Osipenko <digetx@gmail.com>

> diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
> index 29bafc6..b62f790 100644
> --- a/drivers/iommu/tegra-gart.c
> +++ b/drivers/iommu/tegra-gart.c

[snip]

> @@ -449,6 +472,9 @@ static int tegra_gart_remove(struct platform_device *pdev)
>  {

BTW, GART's driver can't be build as a module, so this function is pretty much a
dead code. Probably worth considering its removal.

>  	struct gart_device *gart = platform_get_drvdata(pdev);
>  
> +	iommu_device_unregister(&gart->iommu);
> +	iommu_device_sysfs_remove(&gart->iommu);
> +
>  	writel(0, gart->regs + GART_CONFIG);
>  	if (gart->savedata)
>  		vfree(gart->savedata);
> 


-- 
Dmitry

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 2/2] iommu/tegra-gart: Add support for struct iommu_device
  2017-08-16 22:21         ` Dmitry Osipenko
@ 2017-08-17 13:52             ` Thierry Reding
  -1 siblings, 0 replies; 27+ messages in thread
From: Thierry Reding @ 2017-08-17 13:52 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Joerg Roedel, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	Jonathan Hunter,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA, Hiroshi Doyu


[-- Attachment #1.1: Type: text/plain, Size: 1857 bytes --]

On Thu, Aug 17, 2017 at 01:21:52AM +0300, Dmitry Osipenko wrote:
> Hello Joerg,
> 
> On 10.08.2017 01:29, Joerg Roedel wrote:
> > From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> > 
> > Add a struct iommu_device to each tegra-gart and register it
> > with the iommu-core. Also link devices added to the driver
> > to their respective hardware iommus.
> > 
> > Signed-off-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> > ---
> >  drivers/iommu/tegra-gart.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> 
> Reviewed-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Tested-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> > diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
> > index 29bafc6..b62f790 100644
> > --- a/drivers/iommu/tegra-gart.c
> > +++ b/drivers/iommu/tegra-gart.c
> 
> [snip]
> 
> > @@ -449,6 +472,9 @@ static int tegra_gart_remove(struct platform_device *pdev)
> >  {
> 
> BTW, GART's driver can't be build as a module, so this function is pretty much a
> dead code. Probably worth considering its removal.

Technically you can unbind the driver via sysfs, in which case this
function would still be called. That said, all sorts of things will
probably start to crash when you do that. I'd like to think that we
will eventually be able to deal with this sanely (there's some work
in progress to establish stronger links between devices, so that we
can sanely deal with this kind of dependency), so I think it's okay
to keep this around in case we ever get there.

I don't have any objections to making the driver unloadable if that
is what everyone else prefers, though. In that case, however, there
are more steps involved than just removing the ->remove() callback.

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 2/2] iommu/tegra-gart: Add support for struct iommu_device
@ 2017-08-17 13:52             ` Thierry Reding
  0 siblings, 0 replies; 27+ messages in thread
From: Thierry Reding @ 2017-08-17 13:52 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Joerg Roedel, Hiroshi Doyu, Jonathan Hunter, Robin Murphy, iommu,
	linux-tegra, linux-kernel, Joerg Roedel

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

On Thu, Aug 17, 2017 at 01:21:52AM +0300, Dmitry Osipenko wrote:
> Hello Joerg,
> 
> On 10.08.2017 01:29, Joerg Roedel wrote:
> > From: Joerg Roedel <jroedel@suse.de>
> > 
> > Add a struct iommu_device to each tegra-gart and register it
> > with the iommu-core. Also link devices added to the driver
> > to their respective hardware iommus.
> > 
> > Signed-off-by: Joerg Roedel <jroedel@suse.de>
> > ---
> >  drivers/iommu/tegra-gart.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> 
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
> Tested-by: Dmitry Osipenko <digetx@gmail.com>
> 
> > diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
> > index 29bafc6..b62f790 100644
> > --- a/drivers/iommu/tegra-gart.c
> > +++ b/drivers/iommu/tegra-gart.c
> 
> [snip]
> 
> > @@ -449,6 +472,9 @@ static int tegra_gart_remove(struct platform_device *pdev)
> >  {
> 
> BTW, GART's driver can't be build as a module, so this function is pretty much a
> dead code. Probably worth considering its removal.

Technically you can unbind the driver via sysfs, in which case this
function would still be called. That said, all sorts of things will
probably start to crash when you do that. I'd like to think that we
will eventually be able to deal with this sanely (there's some work
in progress to establish stronger links between devices, so that we
can sanely deal with this kind of dependency), so I think it's okay
to keep this around in case we ever get there.

I don't have any objections to making the driver unloadable if that
is what everyone else prefers, though. In that case, however, there
are more steps involved than just removing the ->remove() callback.

Thierry

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 0/2] iommu/tegra*: Add support for struct iommu_device
  2017-08-09 22:29 ` Joerg Roedel
@ 2017-08-17 13:58     ` Thierry Reding
  -1 siblings, 0 replies; 27+ messages in thread
From: Thierry Reding @ 2017-08-17 13:58 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Hiroshi Doyu, Jonathan Hunter, Robin Murphy,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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

On Thu, Aug 10, 2017 at 12:29:10AM +0200, Joerg Roedel wrote:
> Hi,
> 
> here are two patches to add support for 'struct iommu_device'
> to the tegra iommu-drivers. This will make the iommu-core
> code aware of the hardware iommus that a driver manages.
> 
> It will also add the iommus to sysfs and link them to the
> devices managed by them.
> 
> The patches apply on-top of Robin's iommu-group patches.
> 
> Please review.
> 
> Regards,
> 
> 	Joerg
> 
> Joerg Roedel (2):
>   iommu/tegra: Add support for struct iommu_device
>   iommu/tegra-gart: Add support for struct iommu_device
> 
>  drivers/iommu/tegra-gart.c | 26 ++++++++++++++++++++++++++
>  drivers/iommu/tegra-smmu.c | 25 +++++++++++++++++++++++++
>  2 files changed, 51 insertions(+)

The series:

Acked-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 0/2] iommu/tegra*: Add support for struct iommu_device
@ 2017-08-17 13:58     ` Thierry Reding
  0 siblings, 0 replies; 27+ messages in thread
From: Thierry Reding @ 2017-08-17 13:58 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Hiroshi Doyu, Jonathan Hunter, Robin Murphy, iommu, linux-tegra,
	linux-kernel

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

On Thu, Aug 10, 2017 at 12:29:10AM +0200, Joerg Roedel wrote:
> Hi,
> 
> here are two patches to add support for 'struct iommu_device'
> to the tegra iommu-drivers. This will make the iommu-core
> code aware of the hardware iommus that a driver manages.
> 
> It will also add the iommus to sysfs and link them to the
> devices managed by them.
> 
> The patches apply on-top of Robin's iommu-group patches.
> 
> Please review.
> 
> Regards,
> 
> 	Joerg
> 
> Joerg Roedel (2):
>   iommu/tegra: Add support for struct iommu_device
>   iommu/tegra-gart: Add support for struct iommu_device
> 
>  drivers/iommu/tegra-gart.c | 26 ++++++++++++++++++++++++++
>  drivers/iommu/tegra-smmu.c | 25 +++++++++++++++++++++++++
>  2 files changed, 51 insertions(+)

The series:

Acked-by: Thierry Reding <treding@nvidia.com>

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 2/2] iommu/tegra-gart: Add support for struct iommu_device
  2017-08-17 13:52             ` Thierry Reding
  (?)
@ 2017-08-17 17:17             ` Dmitry Osipenko
  -1 siblings, 0 replies; 27+ messages in thread
From: Dmitry Osipenko @ 2017-08-17 17:17 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Joerg Roedel, Hiroshi Doyu, Jonathan Hunter, Robin Murphy, iommu,
	linux-tegra, linux-kernel, Joerg Roedel

On 17.08.2017 16:52, Thierry Reding wrote:
> On Thu, Aug 17, 2017 at 01:21:52AM +0300, Dmitry Osipenko wrote:
>> Hello Joerg,
>>
>> On 10.08.2017 01:29, Joerg Roedel wrote:
>>> From: Joerg Roedel <jroedel@suse.de>
>>>
>>> Add a struct iommu_device to each tegra-gart and register it
>>> with the iommu-core. Also link devices added to the driver
>>> to their respective hardware iommus.
>>>
>>> Signed-off-by: Joerg Roedel <jroedel@suse.de>
>>> ---
>>>  drivers/iommu/tegra-gart.c | 26 ++++++++++++++++++++++++++
>>>  1 file changed, 26 insertions(+)
>>>
>>
>> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
>> Tested-by: Dmitry Osipenko <digetx@gmail.com>
>>
>>> diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
>>> index 29bafc6..b62f790 100644
>>> --- a/drivers/iommu/tegra-gart.c
>>> +++ b/drivers/iommu/tegra-gart.c
>>
>> [snip]
>>
>>> @@ -449,6 +472,9 @@ static int tegra_gart_remove(struct platform_device *pdev)
>>>  {
>>
>> BTW, GART's driver can't be build as a module, so this function is pretty much a
>> dead code. Probably worth considering its removal.
> 
> Technically you can unbind the driver via sysfs, in which case this
> function would still be called. That said, all sorts of things will
> probably start to crash when you do that. I'd like to think that we
> will eventually be able to deal with this sanely (there's some work
> in progress to establish stronger links between devices, so that we
> can sanely deal with this kind of dependency), so I think it's okay
> to keep this around in case we ever get there.
> 

Good point! I tried to unbind and with this patch kernel crashes immediately:

[  193.968506] kernel BUG at mm/slab.c:2816!
[  193.968912] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
[  193.969466] Modules linked in:
[  193.969822] CPU: 1 PID: 1177 Comm: bash Not tainted
4.13.0-rc5-next-20170816-00067-gd320d19b76f8 #593
[  193.970653] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[  193.971313] task: ee31e380 task.stack: ee394000
[  193.971771] PC is at ___cache_free+0x374/0x47c
[  193.972261] LR is at 0x1
[  193.972539] pc : [<c02776ec>]    lr : [<00000001>]    psr: 200b0093
[  193.973112] sp : ee395dd0  ip : 00000009  fp : 00000000
[  193.973603] r10: 00000480  r9 : 2e844000  r8 : c1814d8c
[  193.974097] r7 : 005e4c40  r6 : c0f91fc0  r5 : ef262480  r4 : ef0003c0
[  193.974694] r3 : ef262400  r2 : ef262000  r1 : 00000400  r0 : 00000004

[snip]

[  193.991288] [<c02776ec>] (___cache_free) from [<c0278230>] (kfree+0xbc/0x260)
[  193.991978] [<c0278230>] (kfree) from [<c058897c>] (device_release+0x2c/0x90)
[  193.992732] [<c058897c>] (device_release) from [<c0ab6f14>]
(kobject_put+0x8c/0xe8)
[  193.993474] [<c0ab6f14>] (kobject_put) from [<c0527868>]
(tegra_gart_remove+0x1c/0x58)
[  193.994320] [<c0527868>] (tegra_gart_remove) from [<c058f770>]
(platform_drv_remove+0x24/0x3c)
[  193.995121] [<c058f770>] (platform_drv_remove) from [<c058db30>]
(device_release_driver_internal+0x15c/0x204)
[  193.996033] [<c058db30>] (device_release_driver_internal) from [<c058c2fc>]
(unbind_store+0x7c/0xfc)
[  193.996890] [<c058c2fc>] (unbind_store) from [<c02f8078>]
(kernfs_fop_write+0x104/0x208)
[  193.997661] [<c02f8078>] (kernfs_fop_write) from [<c027f6ec>]
(__vfs_write+0x1c/0x128)
[  193.998445] [<c027f6ec>] (__vfs_write) from [<c02810b8>] (vfs_write+0xa4/0x168)
[  194.017668] [<c02810b8>] (vfs_write) from [<c0281f0c>] (SyS_write+0x3c/0x90)
[  194.038493] [<c0281f0c>] (SyS_write) from [<c0107ce0>]
(ret_fast_syscall+0x0/0x1c)
[  194.057182] Code: e3a03000 ebfff54e eaffffe2 e7f001f2 (e7f001f2)

Without this patch, it crashes too after unbinding but not immediately. Either
way unbinding isn't useful for this device.

> I don't have any objections to making the driver unloadable if that
> is what everyone else prefers, though. In that case, however, there
> are more steps involved than just removing the ->remove() callback.
> 
Indeed!

-- 
Dmitry

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/2] iommu/tegra: Add support for struct iommu_device
  2017-08-09 22:29 ` [PATCH 1/2] iommu/tegra: " Joerg Roedel
@ 2017-08-18 12:47       ` Thierry Reding
  2017-08-30 11:04     ` Jon Hunter
  1 sibling, 0 replies; 27+ messages in thread
From: Thierry Reding @ 2017-08-18 12:47 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Hiroshi Doyu, Jonathan Hunter, Robin Murphy,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Joerg Roedel

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

On Thu, Aug 10, 2017 at 12:29:11AM +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> 
> Add a struct iommu_device to each tegra-smmu and register it
> with the iommu-core. Also link devices added to the driver
> to their respective hardware iommus.
> 
> Signed-off-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> ---
>  drivers/iommu/tegra-smmu.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)

Acked-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/2] iommu/tegra: Add support for struct iommu_device
@ 2017-08-18 12:47       ` Thierry Reding
  0 siblings, 0 replies; 27+ messages in thread
From: Thierry Reding @ 2017-08-18 12:47 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Hiroshi Doyu, Jonathan Hunter, Robin Murphy, iommu, linux-tegra,
	linux-kernel, Joerg Roedel

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

On Thu, Aug 10, 2017 at 12:29:11AM +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> Add a struct iommu_device to each tegra-smmu and register it
> with the iommu-core. Also link devices added to the driver
> to their respective hardware iommus.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/iommu/tegra-smmu.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)

Acked-by: Thierry Reding <treding@nvidia.com>

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 2/2] iommu/tegra-gart: Add support for struct iommu_device
  2017-08-09 22:29     ` Joerg Roedel
  (?)
  (?)
@ 2017-08-18 12:47     ` Thierry Reding
  -1 siblings, 0 replies; 27+ messages in thread
From: Thierry Reding @ 2017-08-18 12:47 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Hiroshi Doyu, Jonathan Hunter, Robin Murphy, iommu, linux-tegra,
	linux-kernel, Joerg Roedel

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

On Thu, Aug 10, 2017 at 12:29:12AM +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> Add a struct iommu_device to each tegra-gart and register it
> with the iommu-core. Also link devices added to the driver
> to their respective hardware iommus.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/iommu/tegra-gart.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)

Acked-by: Thierry Reding <treding@nvidia.com>

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/2] iommu/tegra: Add support for struct iommu_device
  2017-08-09 22:29 ` [PATCH 1/2] iommu/tegra: " Joerg Roedel
@ 2017-08-30 11:04     ` Jon Hunter
  2017-08-30 11:04     ` Jon Hunter
  1 sibling, 0 replies; 27+ messages in thread
From: Jon Hunter @ 2017-08-30 11:04 UTC (permalink / raw)
  To: Joerg Roedel, Hiroshi Doyu, Thierry Reding
  Cc: Robin Murphy, iommu, linux-tegra, linux-kernel, Joerg Roedel


On 09/08/17 23:29, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> Add a struct iommu_device to each tegra-smmu and register it
> with the iommu-core. Also link devices added to the driver
> to their respective hardware iommus.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/iommu/tegra-smmu.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> index faa9c1e..2802e12 100644
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> @@ -36,6 +36,8 @@ struct tegra_smmu {
>  	struct list_head list;
>  
>  	struct dentry *debugfs;
> +
> +	struct iommu_device iommu;	/* IOMMU Core code handle */
>  };
>  
>  struct tegra_smmu_as {
> @@ -720,6 +722,9 @@ static int tegra_smmu_add_device(struct device *dev)
>  			 * first match.
>  			 */
>  			dev->archdata.iommu = smmu;
> +
> +			iommu_device_link(&smmu->iommu, dev);
> +
>  			break;
>  		}
>  
> @@ -737,6 +742,11 @@ static int tegra_smmu_add_device(struct device *dev)
>  
>  static void tegra_smmu_remove_device(struct device *dev)
>  {
> +	struct tegra_smmu *smmu = dev->archdata.iommu;
> +
> +	if (smmu)
> +		iommu_device_unlink(&smmu->iommu, dev);
> +
>  	dev->archdata.iommu = NULL;
>  	iommu_group_remove_device(dev);
>  }
> @@ -943,6 +953,18 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
>  	if (err < 0)
>  		return ERR_PTR(err);
>  
> +	err = iommu_device_sysfs_add(&smmu->iommu, dev, NULL, dev_name(dev));
> +	if (err)
> +		return ERR_PTR(err);
> +
> +	iommu_device_set_ops(&smmu->iommu, &tegra_smmu_ops);
> +
> +	err = iommu_device_register(&smmu->iommu);
> +	if (err) {
> +		iommu_device_sysfs_remove(&smmu->iommu);
> +		return ERR_PTR(err);
> +	}
> +
>  	if (IS_ENABLED(CONFIG_DEBUG_FS))
>  		tegra_smmu_debugfs_init(smmu);
>  
> @@ -951,6 +973,9 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
>  
>  void tegra_smmu_remove(struct tegra_smmu *smmu)
>  {
> +	iommu_device_unregister(&smmu->iommu);
> +	iommu_device_sysfs_remove(&smmu->iommu);
> +
>  	if (IS_ENABLED(CONFIG_DEBUG_FS))
>  		tegra_smmu_debugfs_exit(smmu);
>  }
> 

With next-20170829 I am seeing several Tegra boards crashing [0][1] on
boot in tegra_smmu_probe() and the bisect is point to this commit. Looks
like there maybe a sequence problem between calls to bus_set_iommu() and
iommu_device_add_sysfs() which results in a NULL pointer dereference.

You can see the full crash log here [1].

Cheers
Jon

[0] https://nvtb.github.io//linux-next/
[1]
https://nvtb.github.io//linux-next/test_next-20170829/20170829024534/boot/tegra124-jetson-tk1/tegra124-jetson-tk1/tegra_defconfig_log.txt

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/2] iommu/tegra: Add support for struct iommu_device
@ 2017-08-30 11:04     ` Jon Hunter
  0 siblings, 0 replies; 27+ messages in thread
From: Jon Hunter @ 2017-08-30 11:04 UTC (permalink / raw)
  To: Joerg Roedel, Hiroshi Doyu, Thierry Reding
  Cc: Robin Murphy, iommu, linux-tegra, linux-kernel, Joerg Roedel


On 09/08/17 23:29, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> Add a struct iommu_device to each tegra-smmu and register it
> with the iommu-core. Also link devices added to the driver
> to their respective hardware iommus.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/iommu/tegra-smmu.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> index faa9c1e..2802e12 100644
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> @@ -36,6 +36,8 @@ struct tegra_smmu {
>  	struct list_head list;
>  
>  	struct dentry *debugfs;
> +
> +	struct iommu_device iommu;	/* IOMMU Core code handle */
>  };
>  
>  struct tegra_smmu_as {
> @@ -720,6 +722,9 @@ static int tegra_smmu_add_device(struct device *dev)
>  			 * first match.
>  			 */
>  			dev->archdata.iommu = smmu;
> +
> +			iommu_device_link(&smmu->iommu, dev);
> +
>  			break;
>  		}
>  
> @@ -737,6 +742,11 @@ static int tegra_smmu_add_device(struct device *dev)
>  
>  static void tegra_smmu_remove_device(struct device *dev)
>  {
> +	struct tegra_smmu *smmu = dev->archdata.iommu;
> +
> +	if (smmu)
> +		iommu_device_unlink(&smmu->iommu, dev);
> +
>  	dev->archdata.iommu = NULL;
>  	iommu_group_remove_device(dev);
>  }
> @@ -943,6 +953,18 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
>  	if (err < 0)
>  		return ERR_PTR(err);
>  
> +	err = iommu_device_sysfs_add(&smmu->iommu, dev, NULL, dev_name(dev));
> +	if (err)
> +		return ERR_PTR(err);
> +
> +	iommu_device_set_ops(&smmu->iommu, &tegra_smmu_ops);
> +
> +	err = iommu_device_register(&smmu->iommu);
> +	if (err) {
> +		iommu_device_sysfs_remove(&smmu->iommu);
> +		return ERR_PTR(err);
> +	}
> +
>  	if (IS_ENABLED(CONFIG_DEBUG_FS))
>  		tegra_smmu_debugfs_init(smmu);
>  
> @@ -951,6 +973,9 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
>  
>  void tegra_smmu_remove(struct tegra_smmu *smmu)
>  {
> +	iommu_device_unregister(&smmu->iommu);
> +	iommu_device_sysfs_remove(&smmu->iommu);
> +
>  	if (IS_ENABLED(CONFIG_DEBUG_FS))
>  		tegra_smmu_debugfs_exit(smmu);
>  }
> 

With next-20170829 I am seeing several Tegra boards crashing [0][1] on
boot in tegra_smmu_probe() and the bisect is point to this commit. Looks
like there maybe a sequence problem between calls to bus_set_iommu() and
iommu_device_add_sysfs() which results in a NULL pointer dereference.

You can see the full crash log here [1].

Cheers
Jon

[0] https://nvtb.github.io//linux-next/
[1]
https://nvtb.github.io//linux-next/test_next-20170829/20170829024534/boot/tegra124-jetson-tk1/tegra124-jetson-tk1/tegra_defconfig_log.txt

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/2] iommu/tegra: Add support for struct iommu_device
  2017-08-30 11:04     ` Jon Hunter
  (?)
@ 2017-08-30 12:09     ` Joerg Roedel
       [not found]       ` <20170830120902.kqtxh2ls4ob7xpwy-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
  -1 siblings, 1 reply; 27+ messages in thread
From: Joerg Roedel @ 2017-08-30 12:09 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Hiroshi Doyu, Thierry Reding, Robin Murphy, iommu, linux-tegra,
	linux-kernel, Joerg Roedel

Hi Jon,

On Wed, Aug 30, 2017 at 12:04:38PM +0100, Jon Hunter wrote:
> With next-20170829 I am seeing several Tegra boards crashing [0][1] on
> boot in tegra_smmu_probe() and the bisect is point to this commit. Looks
> like there maybe a sequence problem between calls to bus_set_iommu() and
> iommu_device_add_sysfs() which results in a NULL pointer dereference.

Thanks for the report. Can you please test whether the patch below
fixes the problem?

Thanks,

	Joerg

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 2802e12e6a54..48ffbe95a663 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -949,10 +949,6 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
 
 	tegra_smmu_ahb_enable();
 
-	err = bus_set_iommu(&platform_bus_type, &tegra_smmu_ops);
-	if (err < 0)
-		return ERR_PTR(err);
-
 	err = iommu_device_sysfs_add(&smmu->iommu, dev, NULL, dev_name(dev));
 	if (err)
 		return ERR_PTR(err);
@@ -965,6 +961,10 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
 		return ERR_PTR(err);
 	}
 
+	err = bus_set_iommu(&platform_bus_type, &tegra_smmu_ops);
+	if (err < 0)
+		return ERR_PTR(err);
+
 	if (IS_ENABLED(CONFIG_DEBUG_FS))
 		tegra_smmu_debugfs_init(smmu);
 

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/2] iommu/tegra: Add support for struct iommu_device
  2017-08-30 12:09     ` Joerg Roedel
@ 2017-08-30 14:22           ` Jon Hunter
  0 siblings, 0 replies; 27+ messages in thread
From: Jon Hunter @ 2017-08-30 14:22 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Hiroshi Doyu, Thierry Reding, Robin Murphy,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Joerg Roedel

Hi Joerg,

On 30/08/17 13:09, Joerg Roedel wrote:
> Hi Jon,
> 
> On Wed, Aug 30, 2017 at 12:04:38PM +0100, Jon Hunter wrote:
>> With next-20170829 I am seeing several Tegra boards crashing [0][1] on
>> boot in tegra_smmu_probe() and the bisect is point to this commit. Looks
>> like there maybe a sequence problem between calls to bus_set_iommu() and
>> iommu_device_add_sysfs() which results in a NULL pointer dereference.
> 
> Thanks for the report. Can you please test whether the patch below
> fixes the problem?
> 
> Thanks,
> 
> 	Joerg
> 
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> index 2802e12e6a54..48ffbe95a663 100644
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> @@ -949,10 +949,6 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
>  
>  	tegra_smmu_ahb_enable();
>  
> -	err = bus_set_iommu(&platform_bus_type, &tegra_smmu_ops);
> -	if (err < 0)
> -		return ERR_PTR(err);
> -
>  	err = iommu_device_sysfs_add(&smmu->iommu, dev, NULL, dev_name(dev));
>  	if (err)
>  		return ERR_PTR(err);
> @@ -965,6 +961,10 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
>  		return ERR_PTR(err);
>  	}
>  
> +	err = bus_set_iommu(&platform_bus_type, &tegra_smmu_ops);
> +	if (err < 0)
> +		return ERR_PTR(err);
> +

Yes I can confirm that this fixes the crash. I assume that you will fix
the error path for bus_set_iommu() above as I believe now it needs to
call iommu_device_sysfs_remove().

Cheers!
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/2] iommu/tegra: Add support for struct iommu_device
@ 2017-08-30 14:22           ` Jon Hunter
  0 siblings, 0 replies; 27+ messages in thread
From: Jon Hunter @ 2017-08-30 14:22 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Hiroshi Doyu, Thierry Reding, Robin Murphy, iommu, linux-tegra,
	linux-kernel, Joerg Roedel

Hi Joerg,

On 30/08/17 13:09, Joerg Roedel wrote:
> Hi Jon,
> 
> On Wed, Aug 30, 2017 at 12:04:38PM +0100, Jon Hunter wrote:
>> With next-20170829 I am seeing several Tegra boards crashing [0][1] on
>> boot in tegra_smmu_probe() and the bisect is point to this commit. Looks
>> like there maybe a sequence problem between calls to bus_set_iommu() and
>> iommu_device_add_sysfs() which results in a NULL pointer dereference.
> 
> Thanks for the report. Can you please test whether the patch below
> fixes the problem?
> 
> Thanks,
> 
> 	Joerg
> 
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> index 2802e12e6a54..48ffbe95a663 100644
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> @@ -949,10 +949,6 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
>  
>  	tegra_smmu_ahb_enable();
>  
> -	err = bus_set_iommu(&platform_bus_type, &tegra_smmu_ops);
> -	if (err < 0)
> -		return ERR_PTR(err);
> -
>  	err = iommu_device_sysfs_add(&smmu->iommu, dev, NULL, dev_name(dev));
>  	if (err)
>  		return ERR_PTR(err);
> @@ -965,6 +961,10 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
>  		return ERR_PTR(err);
>  	}
>  
> +	err = bus_set_iommu(&platform_bus_type, &tegra_smmu_ops);
> +	if (err < 0)
> +		return ERR_PTR(err);
> +

Yes I can confirm that this fixes the crash. I assume that you will fix
the error path for bus_set_iommu() above as I believe now it needs to
call iommu_device_sysfs_remove().

Cheers!
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/2] iommu/tegra: Add support for struct iommu_device
  2017-08-30 14:22           ` Jon Hunter
  (?)
@ 2017-08-30 15:30               ` Joerg Roedel
  -1 siblings, 0 replies; 27+ messages in thread
From: Joerg Roedel @ 2017-08-30 15:30 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Joerg Roedel, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Thierry Reding, linux-tegra-u79uwXL29TY76Z2rM5mHXA, Hiroshi Doyu

Hi Jon,

On Wed, Aug 30, 2017 at 03:22:05PM +0100, Jon Hunter wrote:
> Yes I can confirm that this fixes the crash. I assume that you will fix
> the error path for bus_set_iommu() above as I believe now it needs to
> call iommu_device_sysfs_remove().

Thanks for testing the patch. I updated the error-path and put the
commit below into the iommu-tree:

From 96302d89a03524e04d46ec82c6730881bb755923 Mon Sep 17 00:00:00 2001
From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
Date: Wed, 30 Aug 2017 15:06:43 +0200
Subject: [PATCH] arm/tegra: Call bus_set_iommu() after iommu_device_register()

The bus_set_iommu() function will call the add_device()
call-back which needs the iommu to be registered.

Reported-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Fixes: 0b480e447006 ('iommu/tegra: Add support for struct iommu_device')
Signed-off-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---
 drivers/iommu/tegra-smmu.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 2802e12e6a54..3b6449e2cbf1 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -949,10 +949,6 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
 
 	tegra_smmu_ahb_enable();
 
-	err = bus_set_iommu(&platform_bus_type, &tegra_smmu_ops);
-	if (err < 0)
-		return ERR_PTR(err);
-
 	err = iommu_device_sysfs_add(&smmu->iommu, dev, NULL, dev_name(dev));
 	if (err)
 		return ERR_PTR(err);
@@ -965,6 +961,13 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
 		return ERR_PTR(err);
 	}
 
+	err = bus_set_iommu(&platform_bus_type, &tegra_smmu_ops);
+	if (err < 0) {
+		iommu_device_unregister(&smmu->iommu);
+		iommu_device_sysfs_remove(&smmu->iommu);
+		return ERR_PTR(err);
+	}
+
 	if (IS_ENABLED(CONFIG_DEBUG_FS))
 		tegra_smmu_debugfs_init(smmu);
 
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/2] iommu/tegra: Add support for struct iommu_device
@ 2017-08-30 15:30               ` Joerg Roedel
  0 siblings, 0 replies; 27+ messages in thread
From: Joerg Roedel @ 2017-08-30 15:30 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Hiroshi Doyu, Thierry Reding, Robin Murphy, iommu, linux-tegra,
	linux-kernel, Joerg Roedel

Hi Jon,

On Wed, Aug 30, 2017 at 03:22:05PM +0100, Jon Hunter wrote:
> Yes I can confirm that this fixes the crash. I assume that you will fix
> the error path for bus_set_iommu() above as I believe now it needs to
> call iommu_device_sysfs_remove().

Thanks for testing the patch. I updated the error-path and put the
commit below into the iommu-tree:

>From 96302d89a03524e04d46ec82c6730881bb755923 Mon Sep 17 00:00:00 2001
From: Joerg Roedel <jroedel@suse.de>
Date: Wed, 30 Aug 2017 15:06:43 +0200
Subject: [PATCH] arm/tegra: Call bus_set_iommu() after iommu_device_register()

The bus_set_iommu() function will call the add_device()
call-back which needs the iommu to be registered.

Reported-by: Jon Hunter <jonathanh@nvidia.com>
Fixes: 0b480e447006 ('iommu/tegra: Add support for struct iommu_device')
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/tegra-smmu.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 2802e12e6a54..3b6449e2cbf1 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -949,10 +949,6 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
 
 	tegra_smmu_ahb_enable();
 
-	err = bus_set_iommu(&platform_bus_type, &tegra_smmu_ops);
-	if (err < 0)
-		return ERR_PTR(err);
-
 	err = iommu_device_sysfs_add(&smmu->iommu, dev, NULL, dev_name(dev));
 	if (err)
 		return ERR_PTR(err);
@@ -965,6 +961,13 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
 		return ERR_PTR(err);
 	}
 
+	err = bus_set_iommu(&platform_bus_type, &tegra_smmu_ops);
+	if (err < 0) {
+		iommu_device_unregister(&smmu->iommu);
+		iommu_device_sysfs_remove(&smmu->iommu);
+		return ERR_PTR(err);
+	}
+
 	if (IS_ENABLED(CONFIG_DEBUG_FS))
 		tegra_smmu_debugfs_init(smmu);
 
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/2] iommu/tegra: Add support for struct iommu_device
@ 2017-08-30 15:30               ` Joerg Roedel
  0 siblings, 0 replies; 27+ messages in thread
From: Joerg Roedel @ 2017-08-30 15:30 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Joerg Roedel, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Thierry Reding, linux-tegra-u79uwXL29TY76Z2rM5mHXA, Hiroshi Doyu

Hi Jon,

On Wed, Aug 30, 2017 at 03:22:05PM +0100, Jon Hunter wrote:
> Yes I can confirm that this fixes the crash. I assume that you will fix
> the error path for bus_set_iommu() above as I believe now it needs to
> call iommu_device_sysfs_remove().

Thanks for testing the patch. I updated the error-path and put the
commit below into the iommu-tree:

>From 96302d89a03524e04d46ec82c6730881bb755923 Mon Sep 17 00:00:00 2001
From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
Date: Wed, 30 Aug 2017 15:06:43 +0200
Subject: [PATCH] arm/tegra: Call bus_set_iommu() after iommu_device_register()

The bus_set_iommu() function will call the add_device()
call-back which needs the iommu to be registered.

Reported-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Fixes: 0b480e447006 ('iommu/tegra: Add support for struct iommu_device')
Signed-off-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---
 drivers/iommu/tegra-smmu.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 2802e12e6a54..3b6449e2cbf1 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -949,10 +949,6 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
 
 	tegra_smmu_ahb_enable();
 
-	err = bus_set_iommu(&platform_bus_type, &tegra_smmu_ops);
-	if (err < 0)
-		return ERR_PTR(err);
-
 	err = iommu_device_sysfs_add(&smmu->iommu, dev, NULL, dev_name(dev));
 	if (err)
 		return ERR_PTR(err);
@@ -965,6 +961,13 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
 		return ERR_PTR(err);
 	}
 
+	err = bus_set_iommu(&platform_bus_type, &tegra_smmu_ops);
+	if (err < 0) {
+		iommu_device_unregister(&smmu->iommu);
+		iommu_device_sysfs_remove(&smmu->iommu);
+		return ERR_PTR(err);
+	}
+
 	if (IS_ENABLED(CONFIG_DEBUG_FS))
 		tegra_smmu_debugfs_init(smmu);
 
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/2] iommu/tegra: Add support for struct iommu_device
  2017-08-30 15:30               ` Joerg Roedel
@ 2017-08-30 16:53                   ` Jon Hunter
  -1 siblings, 0 replies; 27+ messages in thread
From: Jon Hunter @ 2017-08-30 16:53 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Hiroshi Doyu, Thierry Reding, Robin Murphy,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Joerg Roedel


On 30/08/17 16:30, Joerg Roedel wrote:
> Hi Jon,
> 
> On Wed, Aug 30, 2017 at 03:22:05PM +0100, Jon Hunter wrote:
>> Yes I can confirm that this fixes the crash. I assume that you will fix
>> the error path for bus_set_iommu() above as I believe now it needs to
>> call iommu_device_sysfs_remove().
> 
> Thanks for testing the patch. I updated the error-path and put the
> commit below into the iommu-tree:
Great! Thanks, Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/2] iommu/tegra: Add support for struct iommu_device
@ 2017-08-30 16:53                   ` Jon Hunter
  0 siblings, 0 replies; 27+ messages in thread
From: Jon Hunter @ 2017-08-30 16:53 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Hiroshi Doyu, Thierry Reding, Robin Murphy, iommu, linux-tegra,
	linux-kernel, Joerg Roedel


On 30/08/17 16:30, Joerg Roedel wrote:
> Hi Jon,
> 
> On Wed, Aug 30, 2017 at 03:22:05PM +0100, Jon Hunter wrote:
>> Yes I can confirm that this fixes the crash. I assume that you will fix
>> the error path for bus_set_iommu() above as I believe now it needs to
>> call iommu_device_sysfs_remove().
> 
> Thanks for testing the patch. I updated the error-path and put the
> commit below into the iommu-tree:
Great! Thanks, Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/2] iommu/tegra: Add support for struct iommu_device
  2017-08-30 15:30               ` Joerg Roedel
@ 2017-08-31 10:23                   ` Jon Hunter
  -1 siblings, 0 replies; 27+ messages in thread
From: Jon Hunter @ 2017-08-31 10:23 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Thierry Reding, Robin Murphy,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Joerg Roedel

Hi Joerg,

On 30/08/17 16:30, Joerg Roedel wrote:
> Hi Jon,
> 
> On Wed, Aug 30, 2017 at 03:22:05PM +0100, Jon Hunter wrote:
>> Yes I can confirm that this fixes the crash. I assume that you will fix
>> the error path for bus_set_iommu() above as I believe now it needs to
>> call iommu_device_sysfs_remove().
> 
> Thanks for testing the patch. I updated the error-path and put the
> commit below into the iommu-tree:
Today's -next is still failing and I don't see this fix in your public
tree yet [0]. Can you push this out?

Cheers
Jon

[0] https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/

-- 
nvpublic

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/2] iommu/tegra: Add support for struct iommu_device
@ 2017-08-31 10:23                   ` Jon Hunter
  0 siblings, 0 replies; 27+ messages in thread
From: Jon Hunter @ 2017-08-31 10:23 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Thierry Reding, Robin Murphy, iommu, linux-tegra, linux-kernel,
	Joerg Roedel

Hi Joerg,

On 30/08/17 16:30, Joerg Roedel wrote:
> Hi Jon,
> 
> On Wed, Aug 30, 2017 at 03:22:05PM +0100, Jon Hunter wrote:
>> Yes I can confirm that this fixes the crash. I assume that you will fix
>> the error path for bus_set_iommu() above as I believe now it needs to
>> call iommu_device_sysfs_remove().
> 
> Thanks for testing the patch. I updated the error-path and put the
> commit below into the iommu-tree:
Today's -next is still failing and I don't see this fix in your public
tree yet [0]. Can you push this out?

Cheers
Jon

[0] https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/

-- 
nvpublic

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2017-08-31 10:24 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-09 22:29 [PATCH 0/2] iommu/tegra*: Add support for struct iommu_device Joerg Roedel
2017-08-09 22:29 ` Joerg Roedel
2017-08-09 22:29 ` [PATCH 1/2] iommu/tegra: " Joerg Roedel
     [not found]   ` <1502317752-8792-2-git-send-email-joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2017-08-18 12:47     ` Thierry Reding
2017-08-18 12:47       ` Thierry Reding
2017-08-30 11:04   ` Jon Hunter
2017-08-30 11:04     ` Jon Hunter
2017-08-30 12:09     ` Joerg Roedel
     [not found]       ` <20170830120902.kqtxh2ls4ob7xpwy-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2017-08-30 14:22         ` Jon Hunter
2017-08-30 14:22           ` Jon Hunter
     [not found]           ` <c6ee1ce0-7193-be63-c84d-73795c727d26-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2017-08-30 15:30             ` Joerg Roedel
2017-08-30 15:30               ` Joerg Roedel
2017-08-30 15:30               ` Joerg Roedel
     [not found]               ` <20170830153034.GK19533-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2017-08-30 16:53                 ` Jon Hunter
2017-08-30 16:53                   ` Jon Hunter
2017-08-31 10:23                 ` Jon Hunter
2017-08-31 10:23                   ` Jon Hunter
     [not found] ` <1502317752-8792-1-git-send-email-joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2017-08-09 22:29   ` [PATCH 2/2] iommu/tegra-gart: " Joerg Roedel
2017-08-09 22:29     ` Joerg Roedel
     [not found]     ` <1502317752-8792-3-git-send-email-joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2017-08-16 22:21       ` Dmitry Osipenko
2017-08-16 22:21         ` Dmitry Osipenko
     [not found]         ` <66711b72-455f-8ec1-e6f7-5946480dde14-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-08-17 13:52           ` Thierry Reding
2017-08-17 13:52             ` Thierry Reding
2017-08-17 17:17             ` Dmitry Osipenko
2017-08-18 12:47     ` Thierry Reding
2017-08-17 13:58   ` [PATCH 0/2] iommu/tegra*: " Thierry Reding
2017-08-17 13:58     ` Thierry Reding

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.