[PATCH 0/10 v3] IOMMU: Make iommu_ops per-bus_type

[PATCH 0/10 v3] IOMMU: Make iommu_ops per-bus_type

[Joerg Roedel]

Hi,

here is the third version of the patch-set to make the iommu_ops used in
the iommu-api a bus_type property. This will allow us to move code out
of the iommu drivers into generic code and it simplifies the
implementation of the Alex' device-group property.

It converts all iommu drivers to use the new registration interface and
completly removes the register_iommu interface.

Please let me know of any remaining objections. If there are no I would
be very happy about Acked-By's :)

Regards,

	Joerg

Changes v2->v3:
	* Moved bus_set_iommu() to drivers/iommu/iommu.c to get rid of
	  the CONFIG_IOMMU_API #ifdef around it in bus.c

Changes v1->v2:

        * Added kernel-doc comments to bus_set_iommu function
        * Removed iommu.h include from device.h and added a forward
          declaration for struct iommu_ops

Diffstat:

 arch/ia64/kvm/kvm-ia64.c           |    3 +-
 arch/x86/kvm/x86.c                 |    3 +-
 drivers/iommu/amd_iommu.c          |    2 +-
 drivers/iommu/intel-iommu.c        |    2 +-
 drivers/iommu/iommu.c              |   81 ++++++++++++++++++++++++++++--------
 drivers/iommu/msm_iommu.c          |    2 +-
 drivers/iommu/omap-iommu.c         |    2 +-
 drivers/media/video/omap3isp/isp.c |    2 +-
 include/linux/device.h             |    6 +++
 include/linux/iommu.h              |   21 +++++----
 virt/kvm/iommu.c                   |    4 +-
 11 files changed, 92 insertions(+), 36 deletions(-)

[PATCH 01/10] iommu/core: Define iommu_ops and register_iommu only with CONFIG_IOMMU_API

[Joerg Roedel]

This makes it impossible to compile an iommu driver into the
kernel without selecting CONFIG_IOMMU_API.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 include/linux/iommu.h |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 9940319..6470cd8 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -34,6 +34,8 @@ struct iommu_domain {
 #define IOMMU_CAP_CACHE_COHERENCY	0x1
 #define IOMMU_CAP_INTR_REMAP		0x2	/* isolates device intrs */
 
+#ifdef CONFIG_IOMMU_API
+
 struct iommu_ops {
 	int (*domain_init)(struct iommu_domain *domain);
 	void (*domain_destroy)(struct iommu_domain *domain);
@@ -49,8 +51,6 @@ struct iommu_ops {
 			      unsigned long cap);
 };
 
-#ifdef CONFIG_IOMMU_API
-
 extern void register_iommu(struct iommu_ops *ops);
 extern bool iommu_found(void);
 extern struct iommu_domain *iommu_domain_alloc(void);
@@ -70,9 +70,7 @@ extern int iommu_domain_has_cap(struct iommu_domain *domain,
 
 #else /* CONFIG_IOMMU_API */
 
-static inline void register_iommu(struct iommu_ops *ops)
-{
-}
+struct iommu_ops {};
 
 static inline bool iommu_found(void)
 {
-- 
1.7.4.1

[PATCH 02/10] Driver core: Add iommu_ops to bus_type

[Joerg Roedel]

This is the starting point to make the iommu_ops used for
the iommu-api a per-bus-type structure. It is required to
easily implement bus-specific setup in the iommu-layer.
The first user will be the iommu-group attribute in sysfs.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 drivers/iommu/iommu.c  |   31 +++++++++++++++++++++++++++++++
 include/linux/device.h |    6 ++++++
 include/linux/iommu.h  |    2 ++
 3 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 30b0644..3343264 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -34,6 +34,37 @@ void register_iommu(struct iommu_ops *ops)
 	iommu_ops = ops;
 }
 
+static void iommu_bus_init(struct bus_type *bus, struct iommu_ops *ops)
+{
+}
+
+/**
+ * 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, struct iommu_ops *ops)
+{
+	if (bus->iommu_ops != NULL)
+		return -EBUSY;
+
+	bus->iommu_ops = ops;
+
+	/* Do IOMMU specific setup for this bus-type */
+	iommu_bus_init(bus, ops);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(bus_set_iommu);
+
 bool iommu_found(void)
 {
 	return iommu_ops != NULL;
diff --git a/include/linux/device.h b/include/linux/device.h
index c20dfbf..e838e14 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -33,6 +33,7 @@ struct class;
 struct subsys_private;
 struct bus_type;
 struct device_node;
+struct iommu_ops;
 
 struct bus_attribute {
 	struct attribute	attr;
@@ -67,6 +68,9 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *);
  * @resume:	Called to bring a device on this bus out of sleep mode.
  * @pm:		Power management operations of this bus, callback the specific
  *		device driver's pm-ops.
+ * @iommu_ops   IOMMU specific operations for this bus, used to attach IOMMU
+ *              driver implementations to a bus and allow the driver to do
+ *              bus-specific setup
  * @p:		The private data of the driver core, only the driver core can
  *		touch this.
  *
@@ -96,6 +100,8 @@ struct bus_type {
 
 	const struct dev_pm_ops *pm;
 
+	struct iommu_ops *iommu_ops;
+
 	struct subsys_private *p;
 };
 
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 6470cd8..dca83d3 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -25,6 +25,7 @@
 #define IOMMU_WRITE	(2)
 #define IOMMU_CACHE	(4) /* DMA cache coherency */
 
+struct bus_type;
 struct device;
 
 struct iommu_domain {
@@ -52,6 +53,7 @@ struct iommu_ops {
 };
 
 extern void register_iommu(struct iommu_ops *ops);
+extern int bus_set_iommu(struct bus_type *bus, struct iommu_ops *ops);
 extern bool iommu_found(void);
 extern struct iommu_domain *iommu_domain_alloc(void);
 extern void iommu_domain_free(struct iommu_domain *domain);
-- 
1.7.4.1

[PATCH 03/10] iommu/core: Add bus_type parameter to iommu_domain_alloc

[Joerg Roedel]

This is necessary to store a pointer to the bus-specific
iommu_ops in the iommu-domain structure. It will be used
later to call into bus-specific iommu-ops.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 drivers/iommu/iommu.c              |   14 +++++++++++++-
 drivers/media/video/omap3isp/isp.c |    2 +-
 include/linux/iommu.h              |    6 ++++--
 virt/kvm/iommu.c                   |    2 +-
 4 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3343264..46e1c24 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -16,6 +16,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
  */
 
+#include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/bug.h>
 #include <linux/types.h>
@@ -71,15 +72,26 @@ bool iommu_found(void)
 }
 EXPORT_SYMBOL_GPL(iommu_found);
 
-struct iommu_domain *iommu_domain_alloc(void)
+struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
 {
 	struct iommu_domain *domain;
+	struct iommu_ops *ops;
 	int ret;
 
+	if (bus->iommu_ops)
+		ops = bus->iommu_ops;
+	else
+		ops = iommu_ops;
+
+	if (ops == NULL)
+		return NULL;
+
 	domain = kmalloc(sizeof(*domain), GFP_KERNEL);
 	if (!domain)
 		return NULL;
 
+	domain->ops = ops;
+
 	ret = iommu_ops->domain_init(domain);
 	if (ret)
 		goto out_free;
diff --git a/drivers/media/video/omap3isp/isp.c b/drivers/media/video/omap3isp/isp.c
index a4baa61..a7ed985 100644
--- a/drivers/media/video/omap3isp/isp.c
+++ b/drivers/media/video/omap3isp/isp.c
@@ -2141,7 +2141,7 @@ static int isp_probe(struct platform_device *pdev)
 	/* to be removed once iommu migration is complete */
 	isp->iommu = to_iommu(isp->iommu_dev);
 
-	isp->domain = iommu_domain_alloc();
+	isp->domain = iommu_domain_alloc(pdev->dev.bus);
 	if (!isp->domain) {
 		dev_err(isp->dev, "can't alloc iommu domain\n");
 		ret = -ENOMEM;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index dca83d3..c78d068 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -25,10 +25,12 @@
 #define IOMMU_WRITE	(2)
 #define IOMMU_CACHE	(4) /* DMA cache coherency */
 
+struct iommu_ops;
 struct bus_type;
 struct device;
 
 struct iommu_domain {
+	struct iommu_ops *ops;
 	void *priv;
 };
 
@@ -55,7 +57,7 @@ struct iommu_ops {
 extern void register_iommu(struct iommu_ops *ops);
 extern int bus_set_iommu(struct bus_type *bus, struct iommu_ops *ops);
 extern bool iommu_found(void);
-extern struct iommu_domain *iommu_domain_alloc(void);
+extern struct iommu_domain *iommu_domain_alloc(struct bus_type *bus);
 extern void iommu_domain_free(struct iommu_domain *domain);
 extern int iommu_attach_device(struct iommu_domain *domain,
 			       struct device *dev);
@@ -79,7 +81,7 @@ static inline bool iommu_found(void)
 	return false;
 }
 
-static inline struct iommu_domain *iommu_domain_alloc(void)
+static inline struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
 {
 	return NULL;
 }
diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
index 78c80f6..20115b1 100644
--- a/virt/kvm/iommu.c
+++ b/virt/kvm/iommu.c
@@ -233,7 +233,7 @@ int kvm_iommu_map_guest(struct kvm *kvm)
 		return -ENODEV;
 	}
 
-	kvm->arch.iommu_domain = iommu_domain_alloc();
+	kvm->arch.iommu_domain = iommu_domain_alloc(&pci_bus_type);
 	if (!kvm->arch.iommu_domain)
 		return -ENOMEM;
 
-- 
1.7.4.1

[PATCH 04/10] iommu/core: Convert iommu_found to iommu_present

[Joerg Roedel]

With per-bus iommu_ops the iommu_found function needs to
work on a bus_type too. This patch adds a bus_type parameter
to that function and converts all call-places.
The function is also renamed to iommu_present because the
function now checks if an iommu is present for a given bus
and does not check for a global iommu anymore.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/ia64/kvm/kvm-ia64.c |    3 ++-
 arch/x86/kvm/x86.c       |    3 ++-
 drivers/iommu/iommu.c    |    9 ++++++---
 include/linux/iommu.h    |    4 ++--
 virt/kvm/iommu.c         |    2 +-
 5 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 8213efe..43f4c92 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -33,6 +33,7 @@
 #include <linux/uaccess.h>
 #include <linux/iommu.h>
 #include <linux/intel-iommu.h>
+#include <linux/pci.h>
 
 #include <asm/pgtable.h>
 #include <asm/gcc_intrin.h>
@@ -204,7 +205,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 		r = KVM_COALESCED_MMIO_PAGE_OFFSET;
 		break;
 	case KVM_CAP_IOMMU:
-		r = iommu_found();
+		r = iommu_present(&pci_bus_type);
 		break;
 	default:
 		r = 0;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 84a28ea..73c6a42 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -44,6 +44,7 @@
 #include <linux/perf_event.h>
 #include <linux/uaccess.h>
 #include <linux/hash.h>
+#include <linux/pci.h>
 #include <trace/events/kvm.h>
 
 #define CREATE_TRACE_POINTS
@@ -2095,7 +2096,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 		r = 0;
 		break;
 	case KVM_CAP_IOMMU:
-		r = iommu_found();
+		r = iommu_present(&pci_bus_type);
 		break;
 	case KVM_CAP_MCE:
 		r = KVM_MAX_MCE_BANKS;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 46e1c24..ad8ce1a 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -66,11 +66,14 @@ int bus_set_iommu(struct bus_type *bus, struct iommu_ops *ops)
 }
 EXPORT_SYMBOL_GPL(bus_set_iommu);
 
-bool iommu_found(void)
+bool iommu_present(struct bus_type *bus)
 {
-	return iommu_ops != NULL;
+	if (bus->iommu_ops != NULL)
+		return true;
+	else
+		return iommu_ops != NULL;
 }
-EXPORT_SYMBOL_GPL(iommu_found);
+EXPORT_SYMBOL_GPL(iommu_present);
 
 struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
 {
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index c78d068..f56e559 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -56,7 +56,7 @@ struct iommu_ops {
 
 extern void register_iommu(struct iommu_ops *ops);
 extern int bus_set_iommu(struct bus_type *bus, struct iommu_ops *ops);
-extern bool iommu_found(void);
+extern bool iommu_present(struct bus_type *bus);
 extern struct iommu_domain *iommu_domain_alloc(struct bus_type *bus);
 extern void iommu_domain_free(struct iommu_domain *domain);
 extern int iommu_attach_device(struct iommu_domain *domain,
@@ -76,7 +76,7 @@ extern int iommu_domain_has_cap(struct iommu_domain *domain,
 
 struct iommu_ops {};
 
-static inline bool iommu_found(void)
+static inline bool iommu_present(struct bus_type *bus)
 {
 	return false;
 }
diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
index 20115b1..d149940 100644
--- a/virt/kvm/iommu.c
+++ b/virt/kvm/iommu.c
@@ -228,7 +228,7 @@ int kvm_iommu_map_guest(struct kvm *kvm)
 {
 	int r;
 
-	if (!iommu_found()) {
+	if (!iommu_present(&pci_bus_type)) {
 		printk(KERN_ERR "%s: iommu not found\n", __func__);
 		return -ENODEV;
 	}
-- 
1.7.4.1

[PATCH 05/10] iommu/core: Use bus->iommu_ops in the iommu-api

[Joerg Roedel]

Use the per-bus iommu-ops in the functions of the iommu-api
instead of the global iommu_ops.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 drivers/iommu/iommu.c |   34 +++++++++++++++++++++++++++-------
 1 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index ad8ce1a..1575aaa 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -110,34 +110,48 @@ EXPORT_SYMBOL_GPL(iommu_domain_alloc);
 
 void iommu_domain_free(struct iommu_domain *domain)
 {
-	iommu_ops->domain_destroy(domain);
+	if (likely(domain->ops->domain_destroy != NULL))
+		domain->ops->domain_destroy(domain);
+
 	kfree(domain);
 }
 EXPORT_SYMBOL_GPL(iommu_domain_free);
 
 int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
 {
-	return iommu_ops->attach_dev(domain, dev);
+	if (unlikely(domain->ops->attach_dev == NULL))
+		return -ENODEV;
+
+	return domain->ops->attach_dev(domain, dev);
 }
 EXPORT_SYMBOL_GPL(iommu_attach_device);
 
 void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
 {
-	iommu_ops->detach_dev(domain, dev);
+	if (unlikely(domain->ops->detach_dev == NULL))
+		return;
+
+	domain->ops->detach_dev(domain, dev);
 }
 EXPORT_SYMBOL_GPL(iommu_detach_device);
 
 phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain,
 			       unsigned long iova)
 {
-	return iommu_ops->iova_to_phys(domain, iova);
+	if (unlikely(domain->ops->iova_to_phys == NULL))
+		return 0;
+
+	return domain->ops->iova_to_phys(domain, iova);
 }
 EXPORT_SYMBOL_GPL(iommu_iova_to_phys);
 
 int iommu_domain_has_cap(struct iommu_domain *domain,
 			 unsigned long cap)
 {
-	return iommu_ops->domain_has_cap(domain, cap);
+	if (unlikely(domain->ops->domain_has_cap == NULL))
+		return 0;
+
+	return domain->ops->domain_has_cap(domain, cap);
 }
 EXPORT_SYMBOL_GPL(iommu_domain_has_cap);
 
@@ -146,11 +160,14 @@ int iommu_map(struct iommu_domain *domain, unsigned long iova,
 {
 	size_t size;
 
+	if (unlikely(domain->ops->map == NULL))
+		return -ENODEV;
+
 	size         = PAGE_SIZE << gfp_order;
 
 	BUG_ON(!IS_ALIGNED(iova | paddr, size));
 
-	return iommu_ops->map(domain, iova, paddr, gfp_order, prot);
+	return domain->ops->map(domain, iova, paddr, gfp_order, prot);
 }
 EXPORT_SYMBOL_GPL(iommu_map);
 
@@ -158,10 +175,13 @@ int iommu_unmap(struct iommu_domain *domain, unsigned long iova, int gfp_order)
 {
 	size_t size;
 
+	if (unlikely(domain->ops->unmap == NULL))
+		return -ENODEV;
+
 	size         = PAGE_SIZE << gfp_order;
 
 	BUG_ON(!IS_ALIGNED(iova, size));
 
-	return iommu_ops->unmap(domain, iova, gfp_order);
+	return domain->ops->unmap(domain, iova, gfp_order);
 }
 EXPORT_SYMBOL_GPL(iommu_unmap);
-- 
1.7.4.1

[PATCH 06/10] iommu/amd: Use bus_set_iommu instead of register_iommu

[Joerg Roedel]

Convert the AMD IOMMU driver to use the new interface for
publishing the iommu_ops.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 drivers/iommu/amd_iommu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index a14f8dc..57f6f38 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2493,7 +2493,7 @@ static unsigned device_dma_ops_init(void)
 
 void __init amd_iommu_init_api(void)
 {
-	register_iommu(&amd_iommu_ops);
+	bus_set_iommu(&pci_bus_type, &amd_iommu_ops);
 }
 
 int __init amd_iommu_init_dma_ops(void)
-- 
1.7.4.1

[PATCH 07/10] iommu/vt-d: Use bus_set_iommu instead of register_iommu

[Joerg Roedel]

Convert the Intel IOMMU driver to use the new interface for
publishing the iommu_ops.

Cc: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 drivers/iommu/intel-iommu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index c621c98..2d53c3d 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3486,7 +3486,7 @@ int __init intel_iommu_init(void)
 
 	init_iommu_pm_ops();
 
-	register_iommu(&intel_iommu_ops);
+	bus_set_iommu(&pci_bus_type, &intel_iommu_ops);
 
 	bus_register_notifier(&pci_bus_type, &device_nb);
 
-- 
1.7.4.1

[PATCH 08/10] iommu/omap: Use bus_set_iommu instead of register_iommu

[Joerg Roedel]

Convert the OMAP IOMMU driver on ARM to use the new
interface for publishing the iommu_ops.

Cc: Ohad Ben-Cohen <ohad@wizery.com>
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 drivers/iommu/omap-iommu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index bd5f606..16d5b76 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1250,7 +1250,7 @@ static int __init omap_iommu_init(void)
 		return -ENOMEM;
 	iopte_cachep = p;
 
-	register_iommu(&omap_iommu_ops);
+	bus_set_iommu(&platform_bus_type, &omap_iommu_ops);
 
 	return platform_driver_register(&omap_iommu_driver);
 }
-- 
1.7.4.1

[PATCH 09/10] iommu/msm: Use bus_set_iommu instead of register_iommu

[Joerg Roedel]

Convert the MSM IOMMU driver for ARM to use the new
interface for publishing the iommu_ops.

Cc: David Brown <davidb@codeaurora.org>
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 drivers/iommu/msm_iommu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index d1733f6..5865dd2 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -728,7 +728,7 @@ static void __init setup_iommu_tex_classes(void)
 static int __init msm_iommu_init(void)
 {
 	setup_iommu_tex_classes();
-	register_iommu(&msm_iommu_ops);
+	bus_set_iommu(&platform_bus_type, &msm_iommu_ops);
 	return 0;
 }
 
-- 
1.7.4.1

[PATCH 10/10] iommu/core: Remove global iommu_ops and register_iommu

[Joerg Roedel]

With all IOMMU drivers being converted to bus_set_iommu the
global iommu_ops are no longer required. The same is true
for the deprecated register_iommu function.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 drivers/iommu/iommu.c |   27 ++++-----------------------
 include/linux/iommu.h |    1 -
 2 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 1575aaa..64419c8 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -25,16 +25,6 @@
 #include <linux/errno.h>
 #include <linux/iommu.h>
 
-static struct iommu_ops *iommu_ops;
-
-void register_iommu(struct iommu_ops *ops)
-{
-	if (iommu_ops)
-		BUG();
-
-	iommu_ops = ops;
-}
-
 static void iommu_bus_init(struct bus_type *bus, struct iommu_ops *ops)
 {
 }
@@ -68,34 +58,25 @@ EXPORT_SYMBOL_GPL(bus_set_iommu);
 
 bool iommu_present(struct bus_type *bus)
 {
-	if (bus->iommu_ops != NULL)
-		return true;
-	else
-		return iommu_ops != NULL;
+	return bus->iommu_ops != NULL;
 }
 EXPORT_SYMBOL_GPL(iommu_present);
 
 struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
 {
 	struct iommu_domain *domain;
-	struct iommu_ops *ops;
 	int ret;
 
-	if (bus->iommu_ops)
-		ops = bus->iommu_ops;
-	else
-		ops = iommu_ops;
-
-	if (ops == NULL)
+	if (bus == NULL || bus->iommu_ops == NULL)
 		return NULL;
 
 	domain = kmalloc(sizeof(*domain), GFP_KERNEL);
 	if (!domain)
 		return NULL;
 
-	domain->ops = ops;
+	domain->ops = bus->iommu_ops;
 
-	ret = iommu_ops->domain_init(domain);
+	ret = domain->ops->domain_init(domain);
 	if (ret)
 		goto out_free;
 
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index f56e559..609ebf6 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -54,7 +54,6 @@ struct iommu_ops {
 			      unsigned long cap);
 };
 
-extern void register_iommu(struct iommu_ops *ops);
 extern int bus_set_iommu(struct bus_type *bus, struct iommu_ops *ops);
 extern bool iommu_present(struct bus_type *bus);
 extern struct iommu_domain *iommu_domain_alloc(struct bus_type *bus);
-- 
1.7.4.1

Re: [PATCH 09/10] iommu/msm: Use bus_set_iommu instead of register_iommu

[David Brown]

On Fri, Sep 23, 2011 at 05:45:55PM +0200, Joerg Roedel wrote:
> Convert the MSM IOMMU driver for ARM to use the new
> interface for publishing the iommu_ops.
> 
> Cc: David Brown <davidb@codeaurora.org>
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  drivers/iommu/msm_iommu.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
> index d1733f6..5865dd2 100644
> --- a/drivers/iommu/msm_iommu.c
> +++ b/drivers/iommu/msm_iommu.c
> @@ -728,7 +728,7 @@ static void __init setup_iommu_tex_classes(void)
>  static int __init msm_iommu_init(void)
>  {
>  	setup_iommu_tex_classes();
> -	register_iommu(&msm_iommu_ops);
> +	bus_set_iommu(&platform_bus_type, &msm_iommu_ops);
>  	return 0;
>  }

Acked-by: David Brown <davidb@codeaurora.org>

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 08/10] iommu/omap: Use bus_set_iommu instead of register_iommu

[Ohad Ben-Cohen]

On Fri, Sep 23, 2011 at 6:45 PM, Joerg Roedel <joerg.roedel@amd.com> wrote:
> -       register_iommu(&omap_iommu_ops);
> +       bus_set_iommu(&platform_bus_type, &omap_iommu_ops);

I'm not sure it makes a lot of sense to bind the iommu ops to the platform bus.

Unlike the pci bus, the platform bus is not really a hardware bus:
it's some kind of a catch-all pseudo bus which is mostly used to to
describe a collection of autonomous peripherals on an SoC.

As a result, IOMMU users on an SoC might actually not belong to the
platform bus.

More specifically, I'm not sure how this would work out with the
upcoming generic IOMMU-based DMA API, which might be used by those
non-platform-bus users.

That said, the generic DMA API is not here yet, and nothing
immediately breaks for us by doing this change, so if this helps you
guys with the x86/pci iommu drivers than I'm ok with it: we can always
change/fix this later when real issues come up with this model.

Re: [PATCH 08/10] iommu/omap: Use bus_set_iommu instead of register_iommu

[Roedel, Joerg]

On Mon, Sep 26, 2011 at 03:09:58AM -0400, Ohad Ben-Cohen wrote:
> On Fri, Sep 23, 2011 at 6:45 PM, Joerg Roedel <joerg.roedel@amd.com> wrote:
> > -       register_iommu(&omap_iommu_ops);
> > +       bus_set_iommu(&platform_bus_type, &omap_iommu_ops);
> 
> I'm not sure it makes a lot of sense to bind the iommu ops to the platform bus.
> 
> Unlike the pci bus, the platform bus is not really a hardware bus:
> it's some kind of a catch-all pseudo bus which is mostly used to to
> describe a collection of autonomous peripherals on an SoC.
> 
> As a result, IOMMU users on an SoC might actually not belong to the
> platform bus.

That is interesting. What bus do these IOMMU users you talk about belong
to?
Is it a seperate bus that originates from a platform device? in that
case the situation sounds a little bit like with USB on x86. The usb
controlers are PCI devices and DMA handles are allocated using the
pci_dev of the controler and not the usb_device.

Regards,

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH 08/10] iommu/omap: Use bus_set_iommu instead of register_iommu

[Ohad Ben-Cohen]

On Mon, Sep 26, 2011 at 3:22 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
> That is interesting. What bus do these IOMMU users you talk about belong
> to?

E.g., virtio devices (which today originates from a platform device)
or even rpmsg users (which originates from the former virtio devices).

> Is it a seperate bus that originates from a platform device?

Yes.

> in that
> case the situation sounds a little bit like with USB on x86. The usb
> controlers are PCI devices and DMA handles are allocated using the
> pci_dev of the controler and not the usb_device.

Yeah, sounds very similar.

Thanks,
Ohad.

Re: [PATCH 08/10] iommu/omap: Use bus_set_iommu instead of register_iommu

[Roedel, Joerg]

On Mon, Sep 26, 2011 at 08:29:04AM -0400, Ohad Ben-Cohen wrote:
> On Mon, Sep 26, 2011 at 3:22 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
> > That is interesting. What bus do these IOMMU users you talk about belong
> > to?
> 
> E.g., virtio devices (which today originates from a platform device)
> or even rpmsg users (which originates from the former virtio devices).
> 
> > Is it a seperate bus that originates from a platform device?
> 
> Yes.
> 
> > in that
> > case the situation sounds a little bit like with USB on x86. The usb
> > controlers are PCI devices and DMA handles are allocated using the
> > pci_dev of the controler and not the usb_device.
> 
> Yeah, sounds very similar.

The question is, of course, if the IOMMU on the platform device can
identify single devices on the bus that originates from it? That would
be an important difference to the PCI->USB case.

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH 08/10] iommu/omap: Use bus_set_iommu instead of register_iommu

[Ohad Ben-Cohen]

On Tue, Sep 27, 2011 at 2:00 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
> The question is, of course, if the IOMMU on the platform device can
> identify single devices on the bus that originates from it?

We can probably query the ancestors of those devices until we find one
with IOMMU properties.

Re: [PATCH 08/10] iommu/omap: Use bus_set_iommu instead of register_iommu

[KyongHo Cho]

On Tue, Sep 27, 2011 at 8:10 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> On Tue, Sep 27, 2011 at 2:00 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
>> The question is, of course, if the IOMMU on the platform device can
>> identify single devices on the bus that originates from it?
>
> We can probably query the ancestors of those devices until we find one
> with IOMMU properties.

We make a relationship between a platform device and
its dedicated IOMMU (Exynos System MMU) while machine initialization.
The information about the relationship is stored in the platdata of
IOMMU device descriptor
and it is also a platform device.

Exynos platform is in similar situation with OMAP's,
whereas implementation is different.

Re: [PATCH 08/10] iommu/omap: Use bus_set_iommu instead of register_iommu

[Greg KH]

On Fri, Sep 23, 2011 at 05:45:54PM +0200, Joerg Roedel wrote:
> Convert the OMAP IOMMU driver on ARM to use the new
> interface for publishing the iommu_ops.
> 
> Cc: Ohad Ben-Cohen <ohad@wizery.com>
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  drivers/iommu/omap-iommu.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index bd5f606..16d5b76 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -1250,7 +1250,7 @@ static int __init omap_iommu_init(void)
>  		return -ENOMEM;
>  	iopte_cachep = p;
>  
> -	register_iommu(&omap_iommu_ops);
> +	bus_set_iommu(&platform_bus_type, &omap_iommu_ops);

I too worry about this.

It kind of looks like the omap code should not be using the platform bus
code for this, but instead, use their own bus code, as I'm sure that not
all platform devices really work properly with this iommu controller,
right?

thanks,

greg k-h

Re: [PATCH 02/10] Driver core: Add iommu_ops to bus_type

[Greg KH]

On Fri, Sep 23, 2011 at 05:45:48PM +0200, Joerg Roedel wrote:
> This is the starting point to make the iommu_ops used for
> the iommu-api a per-bus-type structure. It is required to
> easily implement bus-specific setup in the iommu-layer.
> The first user will be the iommu-group attribute in sysfs.
> 
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>

Ok, this looks good to me now, thanks for all of the revisions.

Do you want me to take this through my tree, or do you want it to go
through some other one?

thanks,

greg k-h

Re: [PATCH 02/10] Driver core: Add iommu_ops to bus_type

[Joerg Roedel]

Hi Greg,

On Thu, Sep 29, 2011 at 01:05:26PM -0700, Greg KH wrote:
> On Fri, Sep 23, 2011 at 05:45:48PM +0200, Joerg Roedel wrote:
> > This is the starting point to make the iommu_ops used for
> > the iommu-api a per-bus-type structure. It is required to
> > easily implement bus-specific setup in the iommu-layer.
> > The first user will be the iommu-group attribute in sysfs.
> > 
> > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> 
> Ok, this looks good to me now, thanks for all of the revisions.
> 
> Do you want me to take this through my tree, or do you want it to go
> through some other one?

Thanks for all your feedback :) I would prefer to carry this in my tree
so that we can start to modify the IOMMU driver on-top of it. So can I
add your acked-by to this one?

	Joerg

Re: [PATCH 02/10] Driver core: Add iommu_ops to bus_type

[Greg KH]

On Fri, Sep 30, 2011 at 08:24:59AM +0200, Joerg Roedel wrote:
> Hi Greg,
> 
> On Thu, Sep 29, 2011 at 01:05:26PM -0700, Greg KH wrote:
> > On Fri, Sep 23, 2011 at 05:45:48PM +0200, Joerg Roedel wrote:
> > > This is the starting point to make the iommu_ops used for
> > > the iommu-api a per-bus-type structure. It is required to
> > > easily implement bus-specific setup in the iommu-layer.
> > > The first user will be the iommu-group attribute in sysfs.
> > > 
> > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > 
> > Ok, this looks good to me now, thanks for all of the revisions.
> > 
> > Do you want me to take this through my tree, or do you want it to go
> > through some other one?
> 
> Thanks for all your feedback :) I would prefer to carry this in my tree
> so that we can start to modify the IOMMU driver on-top of it. So can I
> add your acked-by to this one?

Yes, please do:
	Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

Re: [PATCH 08/10] iommu/omap: Use bus_set_iommu instead of register_iommu

[Ohad Ben-Cohen]

(sorry for the late response; we had a big holiday here and I was
forced away from my keyboard :)

On Thu, Sep 29, 2011 at 11:04 PM, Greg KH <greg@kroah.com> wrote:
> On Fri, Sep 23, 2011 at 05:45:54PM +0200, Joerg Roedel wrote:
>> -     register_iommu(&omap_iommu_ops);
>> +     bus_set_iommu(&platform_bus_type, &omap_iommu_ops);
>
> I too worry about this.
>
> It kind of looks like the omap code should not be using the platform bus
> code for this, but instead, use their own bus code, as I'm sure that not
> all platform devices really work properly with this iommu controller,
> right?

You're right that not all omap platform devices will work with this
iommu device.

But maybe it's worth explaining the bigger difference here: there is
no omap bus really that makes sense here as much as pci does with x86.

On OMAP each iommu device services a single, and specific, on-chip
device (and not a group of devices that resides on a specific hardware
bus). E.g. on OMAP4 we have an iommu for the DSP, and another one for
the dual Cortex M3.

Those remote processors devices happen to be platform devices (because
there isn't really any relevant hardware bus here), and board code (or
DT) will set their iommu archdata to point at the relevant iommu
devices they're physically attached to. This way their drivers will be
able to configure the appropriate iommu devices as needed.

Binding the iommu ops to the platform bus will not enable other
platform drivers to configure the iommu: no other device will have the
relevant iommu archdata set, and any attempt to erroneously configure
the iommu by an unrelated driver will immediately fail (i.e.
iommu_attach_device() will fail).

In that sense, binding the iommu ops to the platform bus does not
increase the chance of error. I guess it even reduces it, because
today the iommu ops are globally accessible.

I was worried that with this change, sub-devices of the original
platform device, that belong to a different bus (e.g. virtio or
rpmsg), might not be able to access those iommu ops anymore. But I
guess that as long as we set the parent-child device hierarchy
appropriately, that would be a non issue.

Thanks,
Ohad.