* [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
[parent not found: <1502317752-8792-2-git-send-email-joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>]
* 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 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
[parent not found: <20170830120902.kqtxh2ls4ob7xpwy-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>]
* 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
[parent not found: <c6ee1ce0-7193-be63-c84d-73795c727d26-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]
* 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: 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
[parent not found: <20170830153034.GK19533-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>]
* 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
[parent not found: <1502317752-8792-1-git-send-email-joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>]
* [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
[parent not found: <1502317752-8792-3-git-send-email-joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>]
* 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
[parent not found: <66711b72-455f-8ec1-e6f7-5946480dde14-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* 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 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 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 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
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.