Patchwork [PATCHv7,05/12] iommu/core: add ops->{bound,unbind}_driver()

login
register
mail settings
Submitter Hiroshi Doyu
Date Dec. 12, 2013, 7:57 a.m.
Message ID <1386835033-4701-6-git-send-email-hdoyu@nvidia.com>
Download mbox | patch
Permalink /patch/300559/
State Superseded, archived
Headers show

Comments

Hiroshi Doyu - Dec. 12, 2013, 7:57 a.m.
ops->{bound,unbind}_driver() functions are called at
BUS_NOTIFY_{BOUND,UNBIND}_DRIVER respectively.

This is necessary to control the device population order. IOMMU master
devices depend on an IOMMU device instanciation. IOMMU master devices
can be registered to an IOMMU only after it's successfully
populated. This IOMMU registration is done via
ops->bound_driver(). Currently this population can be deferred if
depending IOMMU device hasn't yet been populated in driver core. This
cannot be done via ops->add_device() since after add_device() device's
population/instanciation can be still deferred via probe().

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
v6:
New for v6.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 drivers/iommu/iommu.c | 13 +++++++++++--
 include/linux/iommu.h |  4 ++++
 2 files changed, 15 insertions(+), 2 deletions(-)
Stephen Warren - Dec. 16, 2013, 6:42 p.m.
On 12/12/2013 12:57 AM, Hiroshi Doyu wrote:
> ops->{bound,unbind}_driver() functions are called at
> BUS_NOTIFY_{BOUND,UNBIND}_DRIVER respectively.

Some explanation re: why the existing {add,remove}_device hooks aren't
enough would be useful. I didn't really get that from the current commit
description. What actions would an IOMMU driver take in these new
callbacks that could not be taken in {add,remove}_device? Surely the
IOMMU could set up any internal data structures, AS, or page tables,
etc. within add_device, rather than deferring to bound_driver, which
presumably happens after the device's probe(), yet the driver might want
to make the HW do bus-master accesses within probe(), or at least map
stuff into the page table...

> This is necessary to control the device population order. IOMMU master
> devices depend on an IOMMU device instanciation. IOMMU master devices

Is an "IOMMU master device" the device that issues transactions on the
bus, which are affected by the IOMMU? The more common term for this is
"bus master"; "IOMMU master device" sounds too similar to the "IOMMU"
itself.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joerg Roedel - Dec. 30, 2013, 1:45 p.m.
On Thu, Dec 12, 2013 at 09:57:06AM +0200, Hiroshi Doyu wrote:
> ops->{bound,unbind}_driver() functions are called at
> BUS_NOTIFY_{BOUND,UNBIND}_DRIVER respectively.
> 
> This is necessary to control the device population order. IOMMU master
> devices depend on an IOMMU device instanciation. IOMMU master devices
> can be registered to an IOMMU only after it's successfully
> populated. This IOMMU registration is done via
> ops->bound_driver(). Currently this population can be deferred if
> depending IOMMU device hasn't yet been populated in driver core. This
> cannot be done via ops->add_device() since after add_device() device's
> population/instanciation can be still deferred via probe().

How about doing dependency checking in the add/remove_device callbacks
instead? When a device is about to be initialized where the IOMMU is not
set up yet, just setup the IOMMU first before initializing the device?


	Joerg


--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index e5555fc..5469d36 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -540,14 +540,23 @@  static int iommu_bus_notifier(struct notifier_block *nb,
 	 * ADD/DEL call into iommu driver ops if provided, which may
 	 * result in ADD/DEL notifiers to group->notifier
 	 */
-	if (action == BUS_NOTIFY_ADD_DEVICE) {
+	switch (action) {
+	case BUS_NOTIFY_ADD_DEVICE:
 		if (ops->add_device)
 			return ops->add_device(dev);
-	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
+	case BUS_NOTIFY_DEL_DEVICE:
 		if (ops->remove_device && dev->iommu_group) {
 			ops->remove_device(dev);
 			return 0;
 		}
+	case BUS_NOTIFY_BOUND_DRIVER:
+		if (ops->bound_driver)
+			ops->bound_driver(dev);
+		break;
+	case BUS_NOTIFY_UNBIND_DRIVER:
+		if (ops->unbind_driver)
+			ops->unbind_driver(dev);
+		break;
 	}
 
 	/*
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index a444c79..a0e92be 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -96,6 +96,8 @@  enum iommu_attr {
  * @domain_has_cap: domain capabilities query
  * @add_device: add device to iommu grouping
  * @remove_device: remove device from iommu grouping
+ * @bound_driver: called at BUS_NOTIFY_BOUND_DRIVER
+ * @unbind_driver: called at BUS_NOTIFY_UNBIND_DRIVER
  * @domain_get_attr: Query domain attributes
  * @domain_set_attr: Change domain attributes
  * @pgsize_bitmap: bitmap of supported page sizes
@@ -114,6 +116,8 @@  struct iommu_ops {
 			      unsigned long cap);
 	int (*add_device)(struct device *dev);
 	void (*remove_device)(struct device *dev);
+	int (*bound_driver)(struct device *dev);
+	void (*unbind_driver)(struct device *dev);
 	int (*device_group)(struct device *dev, unsigned int *groupid);
 	int (*domain_get_attr)(struct iommu_domain *domain,
 			       enum iommu_attr attr, void *data);