| Message ID | dc44a2269276e1d0fa6715d4530a51df4e7b781c.1657034828.git.robin.murphy@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iommu: Retire bus_set_iommu() | expand |
> From: Robin Murphy <robin.murphy@arm.com> > Sent: Wednesday, July 6, 2022 1:09 AM > > Clean up the remaining trivial bus_set_iommu() callsites along > with the implementation. Now drivers only have to know and care > about iommu_device instances, phew! > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
On 7/5/22 1:08 PM, Robin Murphy wrote: > Clean up the remaining trivial bus_set_iommu() callsites along > with the implementation. Now drivers only have to know and care > about iommu_device instances, phew! > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> > --- > > v3: Also catch Intel's cheeky open-coded assignment > ... > diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c > index c898bcbbce11..dd957145fb81 100644 > --- a/drivers/iommu/s390-iommu.c > +++ b/drivers/iommu/s390-iommu.c > @@ -385,9 +385,3 @@ static const struct iommu_ops s390_iommu_ops = { > .free = s390_domain_free, > } > }; > - > -static int __init s390_iommu_init(void) > -{ > - return bus_set_iommu(&pci_bus_type, &s390_iommu_ops); > -} > -subsys_initcall(s390_iommu_init); Previously s390_iommu_ops was only being set for pci_bus_type, but with this series it will now also be set for platform_bus_type. To tolerate that, this series needs a change along the lines of: From: Matthew Rosato <mjrosato@linux.ibm.com> Date: Thu, 7 Jul 2022 08:45:44 -0400 Subject: [PATCH] iommu/s390: fail probe for non-pci device s390-iommu only supports pci_bus_type today Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> --- drivers/iommu/s390-iommu.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c index dd957145fb81..762f892b4ec3 100644 --- a/drivers/iommu/s390-iommu.c +++ b/drivers/iommu/s390-iommu.c @@ -185,7 +185,12 @@ static void s390_iommu_detach_device(struct iommu_domain *domain, static struct iommu_device *s390_iommu_probe_device(struct device *dev) { - struct zpci_dev *zdev = to_zpci_dev(dev); + struct zpci_dev *zdev; + + if (!dev_is_pci(dev)) + return ERR_PTR(-ENODEV); + + zdev = to_zpci_dev(dev); return &zdev->iommu_dev; } return &zdev->iommu_dev; }
On 7/7/22 8:49 AM, Matthew Rosato wrote: > On 7/5/22 1:08 PM, Robin Murphy wrote: >> Clean up the remaining trivial bus_set_iommu() callsites along >> with the implementation. Now drivers only have to know and care >> about iommu_device instances, phew! >> >> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >> --- >> >> v3: Also catch Intel's cheeky open-coded assignment >> > > ... > >> diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c >> index c898bcbbce11..dd957145fb81 100644 >> --- a/drivers/iommu/s390-iommu.c >> +++ b/drivers/iommu/s390-iommu.c >> @@ -385,9 +385,3 @@ static const struct iommu_ops s390_iommu_ops = { >> .free = s390_domain_free, >> } >> }; >> - >> -static int __init s390_iommu_init(void) >> -{ >> - return bus_set_iommu(&pci_bus_type, &s390_iommu_ops); >> -} >> -subsys_initcall(s390_iommu_init); > > Previously s390_iommu_ops was only being set for pci_bus_type, but with > this series it will now also be set for platform_bus_type. > > To tolerate that, this series needs a change along the lines of: > ... Sorry, let's try that again without a mangled diff: From: Matthew Rosato <mjrosato@linux.ibm.com> Date: Thu, 7 Jul 2022 08:45:44 -0400 Subject: [PATCH] iommu/s390: fail probe for non-pci device s390-iommu only supports pci_bus_type today Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> --- drivers/iommu/s390-iommu.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c index dd957145fb81..762f892b4ec3 100644 --- a/drivers/iommu/s390-iommu.c +++ b/drivers/iommu/s390-iommu.c @@ -185,7 +185,12 @@ static void s390_iommu_detach_device(struct iommu_domain *domain, static struct iommu_device *s390_iommu_probe_device(struct device *dev) { - struct zpci_dev *zdev = to_zpci_dev(dev); + struct zpci_dev *zdev; + + if (!dev_is_pci(dev)) + return ERR_PTR(-ENODEV); + + zdev = to_zpci_dev(dev); return &zdev->iommu_dev; }
On 2022-07-07 13:54, Matthew Rosato wrote: > On 7/7/22 8:49 AM, Matthew Rosato wrote: >> On 7/5/22 1:08 PM, Robin Murphy wrote: >>> Clean up the remaining trivial bus_set_iommu() callsites along >>> with the implementation. Now drivers only have to know and care >>> about iommu_device instances, phew! >>> >>> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >>> --- >>> >>> v3: Also catch Intel's cheeky open-coded assignment >>> >> >> ... >> >>> diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c >>> index c898bcbbce11..dd957145fb81 100644 >>> --- a/drivers/iommu/s390-iommu.c >>> +++ b/drivers/iommu/s390-iommu.c >>> @@ -385,9 +385,3 @@ static const struct iommu_ops s390_iommu_ops = { >>> .free = s390_domain_free, >>> } >>> }; >>> - >>> -static int __init s390_iommu_init(void) >>> -{ >>> - return bus_set_iommu(&pci_bus_type, &s390_iommu_ops); >>> -} >>> -subsys_initcall(s390_iommu_init); >> >> Previously s390_iommu_ops was only being set for pci_bus_type, but >> with this series it will now also be set for platform_bus_type. Ah, indeed I hadn't got as far as fully appreciating that to_zpci_dev() isn't robust enough on its own. Thanks for the patch, I've pulled it in and will include it in v4. Do I take it that all else works OK with this fixed? Cheers, Robin. >> >> To tolerate that, this series needs a change along the lines of: >> > > ... Sorry, let's try that again without a mangled diff: > > From: Matthew Rosato <mjrosato@linux.ibm.com> > Date: Thu, 7 Jul 2022 08:45:44 -0400 > Subject: [PATCH] iommu/s390: fail probe for non-pci device > > > s390-iommu only supports pci_bus_type today > > > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> > --- > drivers/iommu/s390-iommu.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > > diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c > index dd957145fb81..762f892b4ec3 100644 > --- a/drivers/iommu/s390-iommu.c > +++ b/drivers/iommu/s390-iommu.c > @@ -185,7 +185,12 @@ static void s390_iommu_detach_device(struct > iommu_domain *domain, > > > static struct iommu_device *s390_iommu_probe_device(struct device *dev) > { > - struct zpci_dev *zdev = to_zpci_dev(dev); > + struct zpci_dev *zdev; > + > + if (!dev_is_pci(dev)) > + return ERR_PTR(-ENODEV); > + > + zdev = to_zpci_dev(dev); > > > return &zdev->iommu_dev; > } >
On 7/7/22 10:58 AM, Robin Murphy wrote: > On 2022-07-07 13:54, Matthew Rosato wrote: >> On 7/7/22 8:49 AM, Matthew Rosato wrote: >>> On 7/5/22 1:08 PM, Robin Murphy wrote: >>>> Clean up the remaining trivial bus_set_iommu() callsites along >>>> with the implementation. Now drivers only have to know and care >>>> about iommu_device instances, phew! >>>> >>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >>>> --- >>>> >>>> v3: Also catch Intel's cheeky open-coded assignment >>>> >>> >>> ... >>> >>>> diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c >>>> index c898bcbbce11..dd957145fb81 100644 >>>> --- a/drivers/iommu/s390-iommu.c >>>> +++ b/drivers/iommu/s390-iommu.c >>>> @@ -385,9 +385,3 @@ static const struct iommu_ops s390_iommu_ops = { >>>> .free = s390_domain_free, >>>> } >>>> }; >>>> - >>>> -static int __init s390_iommu_init(void) >>>> -{ >>>> - return bus_set_iommu(&pci_bus_type, &s390_iommu_ops); >>>> -} >>>> -subsys_initcall(s390_iommu_init); >>> >>> Previously s390_iommu_ops was only being set for pci_bus_type, but >>> with this series it will now also be set for platform_bus_type. > > Ah, indeed I hadn't got as far as fully appreciating that to_zpci_dev() > isn't robust enough on its own. Thanks for the patch, I've pulled it in > and will include it in v4. Do I take it that all else works OK with this > fixed? > Yes, with that patch included this looks OK so for s390 you can also take a Tested-by: Matthew Rosato <mjrosato@linux.ibm.com>
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c index 4c077c38fbd6..80af00f468b4 100644 --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -845,8 +845,6 @@ static int qcom_iommu_device_probe(struct platform_device *pdev) goto err_pm_disable; } - bus_set_iommu(&platform_bus_type, &qcom_iommu_ops); - if (qcom_iommu->local_base) { pm_runtime_get_sync(dev); writel_relaxed(0xffffffff, qcom_iommu->local_base + SMMU_INTR_SEL_NS); @@ -864,8 +862,6 @@ static int qcom_iommu_device_remove(struct platform_device *pdev) { struct qcom_iommu_dev *qcom_iommu = platform_get_drvdata(pdev); - bus_set_iommu(&platform_bus_type, NULL); - pm_runtime_force_suspend(&pdev->dev); platform_set_drvdata(pdev, NULL); iommu_device_sysfs_remove(&qcom_iommu->iommu); diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index 94b4589dc67c..09aa90c27723 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -481,11 +481,7 @@ int __init pamu_domain_init(void) if (ret) { iommu_device_sysfs_remove(&pamu_iommu); pr_err("Can't register iommu device\n"); - return ret; } - bus_set_iommu(&platform_bus_type, &fsl_pamu_ops); - bus_set_iommu(&pci_bus_type, &fsl_pamu_ops); - return ret; } diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index 3e02c08802a0..388dd0e22fb9 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -4031,7 +4031,6 @@ static int __init probe_acpi_namespace_devices(void) continue; } - pn->dev->bus->iommu_ops = &intel_iommu_ops; ret = iommu_probe_device(pn->dev); if (ret) break; @@ -4153,7 +4152,6 @@ int __init intel_iommu_init(void) } up_read(&dmar_global_lock); - bus_set_iommu(&pci_bus_type, &intel_iommu_ops); if (si_domain && !hw_pass_through) register_memory_notifier(&intel_iommu_memory_nb); diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index acb7b2ab0b79..c10ecb87fd9c 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -1834,30 +1834,6 @@ static int iommu_bus_init(struct bus_type *bus) return err; } -/** - * bus_set_iommu - set iommu-callbacks for the bus - * @bus: bus. - * @ops: the callbacks provided by the iommu-driver - * - * This function is called by an iommu driver to set the iommu methods - * used for a particular bus. Drivers for devices on that bus can use - * the iommu-api after these ops are registered. - * This special function is needed because IOMMUs are usually devices on - * the bus itself, so the iommu drivers are not initialized when the bus - * is set up. With this function the iommu-driver can set the iommu-ops - * afterwards. - */ -int bus_set_iommu(struct bus_type *bus, const struct iommu_ops *ops) -{ - if (bus->iommu_ops && ops && bus->iommu_ops != ops) - return -EBUSY; - - bus->iommu_ops = ops; - - return 0; -} -EXPORT_SYMBOL_GPL(bus_set_iommu); - bool iommu_present(struct bus_type *bus) { return bus->iommu_ops != NULL; diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c index f09aedfdd462..4a71989406dc 100644 --- a/drivers/iommu/msm_iommu.c +++ b/drivers/iommu/msm_iommu.c @@ -797,8 +797,6 @@ static int msm_iommu_probe(struct platform_device *pdev) goto fail; } - bus_set_iommu(&platform_bus_type, &msm_iommu_ops); - pr_info("device mapped at %p, irq %d with %d ctx banks\n", iommu->base, iommu->irq, iommu->ncb); diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index ab57c4b8fade..a3fc59b814ab 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -1300,8 +1300,6 @@ static int rk_iommu_probe(struct platform_device *pdev) if (!dma_dev) dma_dev = &pdev->dev; - bus_set_iommu(&platform_bus_type, &rk_iommu_ops); - pm_runtime_enable(dev); for (i = 0; i < iommu->num_irq; i++) { diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c index c898bcbbce11..dd957145fb81 100644 --- a/drivers/iommu/s390-iommu.c +++ b/drivers/iommu/s390-iommu.c @@ -385,9 +385,3 @@ static const struct iommu_ops s390_iommu_ops = { .free = s390_domain_free, } }; - -static int __init s390_iommu_init(void) -{ - return bus_set_iommu(&pci_bus_type, &s390_iommu_ops); -} -subsys_initcall(s390_iommu_init); diff --git a/drivers/iommu/sprd-iommu.c b/drivers/iommu/sprd-iommu.c index bd409bab6286..6770e6a72283 100644 --- a/drivers/iommu/sprd-iommu.c +++ b/drivers/iommu/sprd-iommu.c @@ -507,9 +507,6 @@ static int sprd_iommu_probe(struct platform_device *pdev) if (ret) goto remove_sysfs; - if (!iommu_present(&platform_bus_type)) - bus_set_iommu(&platform_bus_type, &sprd_iommu_ops); - ret = sprd_iommu_clk_enable(sdev); if (ret) goto unregister_iommu; @@ -545,8 +542,6 @@ static int sprd_iommu_remove(struct platform_device *pdev) iommu_group_put(sdev->group); sdev->group = NULL; - bus_set_iommu(&platform_bus_type, NULL); - platform_set_drvdata(pdev, NULL); iommu_device_sysfs_remove(&sdev->iommu); iommu_device_unregister(&sdev->iommu); diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c index c54ab477b8fd..e104543b78d9 100644 --- a/drivers/iommu/sun50i-iommu.c +++ b/drivers/iommu/sun50i-iommu.c @@ -968,8 +968,6 @@ static int sun50i_iommu_probe(struct platform_device *pdev) if (ret < 0) goto err_unregister; - bus_set_iommu(&platform_bus_type, &sun50i_iommu_ops); - return 0; err_unregister: diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 5e1afe169549..2cb05ff89f03 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -412,7 +412,6 @@ static inline const struct iommu_ops *dev_iommu_ops(struct device *dev) return dev->iommu->iommu_dev->ops; } -extern int bus_set_iommu(struct bus_type *bus, const struct iommu_ops *ops); extern int bus_iommu_probe(struct bus_type *bus); extern bool iommu_present(struct bus_type *bus); extern bool device_iommu_capable(struct device *dev, enum iommu_cap cap);
Clean up the remaining trivial bus_set_iommu() callsites along with the implementation. Now drivers only have to know and care about iommu_device instances, phew! Signed-off-by: Robin Murphy <robin.murphy@arm.com> --- v3: Also catch Intel's cheeky open-coded assignment drivers/iommu/arm/arm-smmu/qcom_iommu.c | 4 ---- drivers/iommu/fsl_pamu_domain.c | 4 ---- drivers/iommu/intel/iommu.c | 2 -- drivers/iommu/iommu.c | 24 ------------------------ drivers/iommu/msm_iommu.c | 2 -- drivers/iommu/rockchip-iommu.c | 2 -- drivers/iommu/s390-iommu.c | 6 ------ drivers/iommu/sprd-iommu.c | 5 ----- drivers/iommu/sun50i-iommu.c | 2 -- include/linux/iommu.h | 1 - 10 files changed, 52 deletions(-)