[PATCH 00/22 v2] Introduce default domains for iommu groups

[PATCH 00/22 v2] Introduce default domains for iommu groups

[Joerg Roedel]

Hi,

here is the second version of my patch-set to introduce
default domains into the iommu core. This time it has a lot
more patches, mostly because I added a proof of concept
implementation by converting the AMD IOMMU driver to make
use of it.

Converting the first driver to the new concept triggered a
lot of changes and extensions in the patch-set to fit all
the needs of a more complex iommu driver. Converting other
drivers might need further changes, but that is something
for the future.

A major change is that now the default domain has to be
allocated by the code that allocates the iommu group. For
PCI devices this happens in the IOMMU core, but drivers
allocating the group on their own can now implement a policy
that fits their needs (e.g. not allocate one domain per
group but let multiple groups share one domain).

The new core code is changed in a way to stay compatible
with the old behavior. All IOMMU drivers that are not yet
converted to default domains should behave as without this
patch-set.

I tested the patches on AMD systems with IOMMUv1 and
IOMMUv2, did boot-testing and also successfully tested
device assignment. I did the same tests on an Intel VT-d
machine to make sure the changes do not introduce
regressions on unconverted drivers.

If you prefer a git branch for testing, please look here:

	git://git.kernel.org/pub/scm/linux/kernel/git/joro/linux.git iommu-default-domains

Any feedback and further testing welcome!

Thanks,

	Joerg


Joerg Roedel (22):
  iommu: Remove function name from pr_fmt()
  iommu: Add a few printk messages to group handling code
  iommu: Propagate error in add_iommu_group
  iommu: Clean up after a failed bus initialization
  iommu: Call remove_device call-back after driver release
  iommu: Allocate a default domain for iommu groups
  iommu: Limit iommu_attach/detach_device to devices with their own
    group
  iommu: Make sure a device is always attached to a domain
  iommu: Add iommu_get_domain_for_dev function
  iommu: Introduce direct mapped region handling
  iommu: Create direct mappings in default domains
  iommu: Add function to query the default domain of a group
  iommu: Introduce iommu_request_dm_for_dev()
  iommu/amd: Implement dm_region call-backs
  iommu/amd: Use default domain if available for DMA-API
  iommu/amd: Implement add_device and remove_device
  iommu/amd: Support IOMMU_DOMAIN_DMA type allocation
  iommu/amd: Support IOMMU_DOMAIN_IDENTITY type allocation
  iommu/amd: Put IOMMUv2 devices in a direct mapped domain
  iommu/amd: Get rid of device_dma_ops_init()
  iommu/amd: Remove unused fields from struct dma_ops_domain
  iommu/amd: Propagate errors from amd_iommu_init_api

 drivers/iommu/amd_iommu.c       | 568 ++++++++++++----------------------------
 drivers/iommu/amd_iommu_init.c  |  34 +--
 drivers/iommu/amd_iommu_proto.h |   2 +-
 drivers/iommu/amd_iommu_types.h |  11 -
 drivers/iommu/iommu.c           | 369 ++++++++++++++++++++++++--
 include/linux/iommu.h           |  44 ++++
 6 files changed, 562 insertions(+), 466 deletions(-)

-- 
1.9.1

[PATCH 01/22] iommu: Remove function name from pr_fmt()

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Including the function name is only useful for debugging
messages. They don't belong into other messages from the
iommu core.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d4f527e..c31bfd0 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -16,7 +16,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
  */
 
-#define pr_fmt(fmt)    "%s: " fmt, __func__
+#define pr_fmt(fmt)    "iommu: " fmt
 
 #include <linux/device.h>
 #include <linux/kernel.h>
-- 
1.9.1

[PATCH 02/22] iommu: Add a few printk messages to group handling code

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Write a message to the kernel log when a device is added or
removed from a group and add debug messages to group
allocation and release routines.

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

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index c31bfd0..755e488 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -128,6 +128,8 @@ static void iommu_group_release(struct kobject *kobj)
 {
 	struct iommu_group *group = to_iommu_group(kobj);
 
+	pr_debug("Releasing group %d\n", group->id);
+
 	if (group->iommu_data_release)
 		group->iommu_data_release(group->iommu_data);
 
@@ -207,6 +209,8 @@ again:
 	 */
 	kobject_put(&group->kobj);
 
+	pr_debug("Allocated group %d\n", group->id);
+
 	return group;
 }
 EXPORT_SYMBOL_GPL(iommu_group_alloc);
@@ -372,6 +376,9 @@ rename:
 				     IOMMU_GROUP_NOTIFY_ADD_DEVICE, dev);
 
 	trace_add_device_to_group(group->id, dev);
+
+	pr_info("Adding device %s to group %d\n", dev_name(dev), group->id);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(iommu_group_add_device);
@@ -388,6 +395,8 @@ void iommu_group_remove_device(struct device *dev)
 	struct iommu_group *group = dev->iommu_group;
 	struct iommu_device *tmp_device, *device = NULL;
 
+	pr_info("Removing device %s from group %d\n", dev_name(dev), group->id);
+
 	/* Pre-notify listeners that a device is being removed. */
 	blocking_notifier_call_chain(&group->notifier,
 				     IOMMU_GROUP_NOTIFY_DEL_DEVICE, dev);
-- 
1.9.1

[PATCH 03/22] iommu: Propagate error in add_iommu_group

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Make sure any errors reported from the IOMMU drivers get
progapated back to the IOMMU core.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/iommu.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 755e488..9c9336a 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -750,9 +750,7 @@ static int add_iommu_group(struct device *dev, void *data)
 
 	WARN_ON(dev->iommu_group);
 
-	ops->add_device(dev);
-
-	return 0;
+	return ops->add_device(dev);
 }
 
 static int iommu_bus_notifier(struct notifier_block *nb,
-- 
1.9.1

Re: [PATCH 03/22] iommu: Propagate error in add_iommu_group

[Heiko Stübner]

Hi Joerg,


Am Donnerstag, 28. Mai 2015, 18:41:26 schrieb Joerg Roedel:
> From: Joerg Roedel <jroedel@suse.de>
> 
> Make sure any errors reported from the IOMMU drivers get
> progapated back to the IOMMU core.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>

sorry that this series slipped through my inbox without testing somehow. 
Anyway, it seems to have now made its way in the merge-window and everything 
on the Rockchip side still works - except this patch.

> ---
>  drivers/iommu/iommu.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 755e488..9c9336a 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -750,9 +750,7 @@ static int add_iommu_group(struct device *dev, void
> *data)
> 
>  	WARN_ON(dev->iommu_group);
> 
> -	ops->add_device(dev);
> -
> -	return 0;
> +	return ops->add_device(dev);
>  }
> 
>  static int iommu_bus_notifier(struct notifier_block *nb,

The Rockchip iommu uses bus_set_ops to set its iommu-ops for the platform
bus and currently returns -ENODEV if it encounters a platform_devices that
does not have an iommu. As add_iommu_group ignored these returns
till now this worked, but of course starts to fail now.

All two invocations of the add_device callback ignored (or still ignore) the
return value so I've come with the following small patch to fix the breakage
that now exists in the 4.2 tree.

There is probably a better solution possible in the longer term, likely
similar to what Samsung does, but I'm not sure yet how this would work
with our drm device that needs an iommu mapping without having an
iommu (the iommus being attached to the crtc-components).


Heiko


------------- 8< ------------------
From: Heiko Stuebner <heiko@sntech.de>
Date: Sun, 28 Jun 2015 12:50:01 +0200
Subject: [PATCH] iommu/rockchip: ignore non-master devices in rk_iommu_add_device

During init we set the rockchip iommu ops as bus_ops for the platform bus.
While all Rockchip devices implementing an iommu use these same ops, there
are also platform devices without iommu. Until recently bus_set_iommu
ignored errors from the add_device callback and we simply returned -ENODEV
for devices without iommu. But we can also simply ignore devices without
iommu which simply ends up doing the same as before the iommu change that
began checking add_device return values.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Fixes: 19762d7095e6 ("iommu: Propagate error in add_iommu_group")
---
 drivers/iommu/rockchip-iommu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index ebf0adb..63bcf48 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -931,8 +931,9 @@ static int rk_iommu_add_device(struct device *dev)
 	struct iommu_group *group;
 	int ret;
 
+	/* nothing to do if device does not have an iommu */
 	if (!rk_iommu_is_dev_iommu_master(dev))
-		return -ENODEV;
+		return 0;
 
 	group = iommu_group_get(dev);
 	if (!group) {
-- 
2.1.4

Re: [PATCH 03/22] iommu: Propagate error in add_iommu_group

[Joerg Roedel]

Hello Heiko,

On Mon, Jun 29, 2015 at 11:28:40AM +0200, Heiko Stübner wrote:
> The Rockchip iommu uses bus_set_ops to set its iommu-ops for the platform
> bus and currently returns -ENODEV if it encounters a platform_devices that
> does not have an iommu. As add_iommu_group ignored these returns
> till now this worked, but of course starts to fail now.
> 
> All two invocations of the add_device callback ignored (or still ignore) the
> return value so I've come with the following small patch to fix the breakage
> that now exists in the 4.2 tree.
> 
> There is probably a better solution possible in the longer term, likely
> similar to what Samsung does, but I'm not sure yet how this would work
> with our drm device that needs an iommu mapping without having an
> iommu (the iommus being attached to the crtc-components).

Yes, this issue was already reported from the Exynos side. I have a fix
which just ignores -ENODEV as the return value for now. This fixes
Exynos, and should fix Rockchip (and some other IOMMU drivers) too.


	Joerg

Re: [PATCH 03/22] iommu: Propagate error in add_iommu_group

[Joerg Roedel]

On Mon, Jun 29, 2015 at 11:28:40AM +0200, Heiko Stübner wrote:
> The Rockchip iommu uses bus_set_ops to set its iommu-ops for the platform
> bus and currently returns -ENODEV if it encounters a platform_devices that
> does not have an iommu. As add_iommu_group ignored these returns
> till now this worked, but of course starts to fail now.
> 
> All two invocations of the add_device callback ignored (or still ignore) the
> return value so I've come with the following small patch to fix the breakage
> that now exists in the 4.2 tree.
> 
> There is probably a better solution possible in the longer term, likely
> similar to what Samsung does, but I'm not sure yet how this would work
> with our drm device that needs an iommu mapping without having an
> iommu (the iommus being attached to the crtc-components).

Btw, if you want to test the fix too, here is the link:

	http://lists.linuxfoundation.org/pipermail/iommu/2015-June/013508.html


Regards,

	Joerg

Re: [PATCH 03/22] iommu: Propagate error in add_iommu_group

[Heiko Stübner]

Hi Joerg,

Am Montag, 29. Juni 2015, 16:06:37 schrieb Joerg Roedel:
> On Mon, Jun 29, 2015 at 11:28:40AM +0200, Heiko Stübner wrote:
> > The Rockchip iommu uses bus_set_ops to set its iommu-ops for the platform
> > bus and currently returns -ENODEV if it encounters a platform_devices that
> > does not have an iommu. As add_iommu_group ignored these returns
> > till now this worked, but of course starts to fail now.
> > 
> > All two invocations of the add_device callback ignored (or still ignore)
> > the return value so I've come with the following small patch to fix the
> > breakage that now exists in the 4.2 tree.
> > 
> > There is probably a better solution possible in the longer term, likely
> > similar to what Samsung does, but I'm not sure yet how this would work
> > with our drm device that needs an iommu mapping without having an
> > iommu (the iommus being attached to the crtc-components).
> 
> Btw, if you want to test the fix too, here is the link:
> 
> 	http://lists.linuxfoundation.org/pipermail/iommu/2015-June/013508.html

yep, works like a charm and fixes the issue. So if helpful:
Tested-by: Heiko Stuebner <heiko@sntech.de>


Thanks
Heiko

[PATCH 04/22] iommu: Clean up after a failed bus initialization

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Make sure we call the ->remove_device call-back on all
devices already initialized with ->add_device when the bus
initialization fails.

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

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 9c9336a..f0e0a23 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -753,6 +753,17 @@ static int add_iommu_group(struct device *dev, void *data)
 	return ops->add_device(dev);
 }
 
+static int remove_iommu_group(struct device *dev, void *data)
+{
+	struct iommu_callback_data *cb = data;
+	const struct iommu_ops *ops = cb->ops;
+
+	if (ops->remove_device && dev->iommu_group)
+		ops->remove_device(dev);
+
+	return 0;
+}
+
 static int iommu_bus_notifier(struct notifier_block *nb,
 			      unsigned long action, void *data)
 {
@@ -821,19 +832,25 @@ static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops)
 	nb->notifier_call = iommu_bus_notifier;
 
 	err = bus_register_notifier(bus, nb);
-	if (err) {
-		kfree(nb);
-		return err;
-	}
+	if (err)
+		goto out_free;
 
 	err = bus_for_each_dev(bus, NULL, &cb, add_iommu_group);
-	if (err) {
-		bus_unregister_notifier(bus, nb);
-		kfree(nb);
-		return err;
-	}
+	if (err)
+		goto out_err;
+
 
 	return 0;
+
+out_err:
+	/* Clean up */
+	bus_for_each_dev(bus, NULL, &cb, remove_iommu_group);
+	bus_unregister_notifier(bus, nb);
+
+out_free:
+	kfree(nb);
+
+	return err;
 }
 
 /**
-- 
1.9.1

[PATCH 05/22] iommu: Call remove_device call-back after driver release

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Do not remove the device from the IOMMU while the driver is
still attached.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f0e0a23..d69e0ca 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -779,7 +779,7 @@ static int iommu_bus_notifier(struct notifier_block *nb,
 	if (action == BUS_NOTIFY_ADD_DEVICE) {
 		if (ops->add_device)
 			return ops->add_device(dev);
-	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
+	} else if (action == BUS_NOTIFY_REMOVED_DEVICE) {
 		if (ops->remove_device && dev->iommu_group) {
 			ops->remove_device(dev);
 			return 0;
-- 
1.9.1

[PATCH 06/22] iommu: Allocate a default domain for iommu groups

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

The default domain will be used (if supported by the iommu
driver) when the devices in the iommu group are not attached
to any other domain.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/iommu.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d69e0ca..fbfc015 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -51,6 +51,7 @@ struct iommu_group {
 	void (*iommu_data_release)(void *iommu_data);
 	char *name;
 	int id;
+	struct iommu_domain *default_domain;
 };
 
 struct iommu_device {
@@ -75,6 +76,9 @@ struct iommu_group_attribute iommu_group_attr_##_name =		\
 #define to_iommu_group(_kobj)		\
 	container_of(_kobj, struct iommu_group, kobj)
 
+static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
+						 unsigned type);
+
 static ssize_t iommu_group_attr_show(struct kobject *kobj,
 				     struct attribute *__attr, char *buf)
 {
@@ -137,6 +141,9 @@ static void iommu_group_release(struct kobject *kobj)
 	ida_remove(&iommu_group_ida, group->id);
 	mutex_unlock(&iommu_group_mutex);
 
+	if (group->default_domain)
+		iommu_domain_free(group->default_domain);
+
 	kfree(group->name);
 	kfree(group);
 }
@@ -701,7 +708,18 @@ static struct iommu_group *iommu_group_get_for_pci_dev(struct pci_dev *pdev)
 		return group;
 
 	/* No shared group found, allocate new */
-	return iommu_group_alloc();
+	group = iommu_group_alloc();
+	if (group) {
+		/*
+		 * Try to allocate a default domain - needs support from the
+		 * IOMMU driver.
+		 */
+		group->default_domain = __iommu_domain_alloc(pdev->dev.bus,
+							     IOMMU_DOMAIN_DMA);
+		group->domain = group->default_domain;
+	}
+
+	return group;
 }
 
 /**
@@ -922,22 +940,28 @@ void iommu_set_fault_handler(struct iommu_domain *domain,
 }
 EXPORT_SYMBOL_GPL(iommu_set_fault_handler);
 
-struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
+static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
+						 unsigned type)
 {
 	struct iommu_domain *domain;
 
 	if (bus == NULL || bus->iommu_ops == NULL)
 		return NULL;
 
-	domain = bus->iommu_ops->domain_alloc(IOMMU_DOMAIN_UNMANAGED);
+	domain = bus->iommu_ops->domain_alloc(type);
 	if (!domain)
 		return NULL;
 
 	domain->ops  = bus->iommu_ops;
-	domain->type = IOMMU_DOMAIN_UNMANAGED;
+	domain->type = type;
 
 	return domain;
 }
+
+struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
+{
+	return __iommu_domain_alloc(bus, IOMMU_DOMAIN_UNMANAGED);
+}
 EXPORT_SYMBOL_GPL(iommu_domain_alloc);
 
 void iommu_domain_free(struct iommu_domain *domain)
-- 
1.9.1

Re: [PATCH 06/22] iommu: Allocate a default domain for iommu groups

[Robin Murphy]

Hi Joerg,

On 28/05/15 17:41, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> The default domain will be used (if supported by the iommu
> driver) when the devices in the iommu group are not attached
> to any other domain.
>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>   drivers/iommu/iommu.c | 32 ++++++++++++++++++++++++++++----
>   1 file changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index d69e0ca..fbfc015 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -51,6 +51,7 @@ struct iommu_group {
>   	void (*iommu_data_release)(void *iommu_data);
>   	char *name;
>   	int id;
> +	struct iommu_domain *default_domain;
>   };
>
>   struct iommu_device {
> @@ -75,6 +76,9 @@ struct iommu_group_attribute iommu_group_attr_##_name =		\
>   #define to_iommu_group(_kobj)		\
>   	container_of(_kobj, struct iommu_group, kobj)
>
> +static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
> +						 unsigned type);
> +
>   static ssize_t iommu_group_attr_show(struct kobject *kobj,
>   				     struct attribute *__attr, char *buf)
>   {
> @@ -137,6 +141,9 @@ static void iommu_group_release(struct kobject *kobj)
>   	ida_remove(&iommu_group_ida, group->id);
>   	mutex_unlock(&iommu_group_mutex);
>
> +	if (group->default_domain)
> +		iommu_domain_free(group->default_domain);
> +
>   	kfree(group->name);
>   	kfree(group);
>   }
> @@ -701,7 +708,18 @@ static struct iommu_group *iommu_group_get_for_pci_dev(struct pci_dev *pdev)
>   		return group;
>
>   	/* No shared group found, allocate new */
> -	return iommu_group_alloc();
> +	group = iommu_group_alloc();
> +	if (group) {
> +		/*
> +		 * Try to allocate a default domain - needs support from the
> +		 * IOMMU driver.
> +		 */
> +		group->default_domain = __iommu_domain_alloc(pdev->dev.bus,
> +							     IOMMU_DOMAIN_DMA);
> +		group->domain = group->default_domain;
> +	}
> +
> +	return group;
>   }
>
>   /**
> @@ -922,22 +940,28 @@ void iommu_set_fault_handler(struct iommu_domain *domain,
>   }
>   EXPORT_SYMBOL_GPL(iommu_set_fault_handler);
>
> -struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
> +static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
> +						 unsigned type)

I'd be happier if we passed the iommu_ops in here...

>   {
>   	struct iommu_domain *domain;
>
>   	if (bus == NULL || bus->iommu_ops == NULL)
>   		return NULL;
>
> -	domain = bus->iommu_ops->domain_alloc(IOMMU_DOMAIN_UNMANAGED);
> +	domain = bus->iommu_ops->domain_alloc(type);
>   	if (!domain)
>   		return NULL;
>
>   	domain->ops  = bus->iommu_ops;
> -	domain->type = IOMMU_DOMAIN_UNMANAGED;
> +	domain->type = type;
>
>   	return domain;
>   }
> +
> +struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
> +{

..having pulled them out of the bus here.

> +	return __iommu_domain_alloc(bus, IOMMU_DOMAIN_UNMANAGED);
> +}
>   EXPORT_SYMBOL_GPL(iommu_domain_alloc);
>
>   void iommu_domain_free(struct iommu_domain *domain)
>

Even if it's only to symbolically inch us closer to having some other 
solution for platform bus devices. The SMMU page size debate is one 
thing, but it's really only a subtle case of the "IOMMU instances" 
problem. Once there are two different IOMMU drivers in the system each 
serving a subset of platform devices, how can we make sure the domain 
allocated here is wrapped in the right driver-private struct? I've had a 
try with making iommu_ops per-instance, but even that doesn't work in 
this case.

Robin.

Re: [PATCH 06/22] iommu: Allocate a default domain for iommu groups

[Robin Murphy]

Hi Joerg,

On 28/05/15 17:41, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> The default domain will be used (if supported by the iommu
> driver) when the devices in the iommu group are not attached
> to any other domain.
>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>   drivers/iommu/iommu.c | 32 ++++++++++++++++++++++++++++----
>   1 file changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index d69e0ca..fbfc015 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -51,6 +51,7 @@ struct iommu_group {
>   	void (*iommu_data_release)(void *iommu_data);
>   	char *name;
>   	int id;
> +	struct iommu_domain *default_domain;
>   };
>
>   struct iommu_device {
> @@ -75,6 +76,9 @@ struct iommu_group_attribute iommu_group_attr_##_name =		\
>   #define to_iommu_group(_kobj)		\
>   	container_of(_kobj, struct iommu_group, kobj)
>
> +static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
> +						 unsigned type);
> +
>   static ssize_t iommu_group_attr_show(struct kobject *kobj,
>   				     struct attribute *__attr, char *buf)
>   {
> @@ -137,6 +141,9 @@ static void iommu_group_release(struct kobject *kobj)
>   	ida_remove(&iommu_group_ida, group->id);
>   	mutex_unlock(&iommu_group_mutex);
>
> +	if (group->default_domain)
> +		iommu_domain_free(group->default_domain);
> +
>   	kfree(group->name);
>   	kfree(group);
>   }
> @@ -701,7 +708,18 @@ static struct iommu_group *iommu_group_get_for_pci_dev(struct pci_dev *pdev)
>   		return group;
>
>   	/* No shared group found, allocate new */
> -	return iommu_group_alloc();
> +	group = iommu_group_alloc();
> +	if (group) {
> +		/*
> +		 * Try to allocate a default domain - needs support from the
> +		 * IOMMU driver.
> +		 */
> +		group->default_domain = __iommu_domain_alloc(pdev->dev.bus,
> +							     IOMMU_DOMAIN_DMA);
> +		group->domain = group->default_domain;
> +	}
> +
> +	return group;
>   }
>
>   /**
> @@ -922,22 +940,28 @@ void iommu_set_fault_handler(struct iommu_domain *domain,
>   }
>   EXPORT_SYMBOL_GPL(iommu_set_fault_handler);
>
> -struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
> +static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
> +						 unsigned type)

I'd be happier if we passed the iommu_ops in here...

>   {
>   	struct iommu_domain *domain;
>
>   	if (bus == NULL || bus->iommu_ops == NULL)
>   		return NULL;
>
> -	domain = bus->iommu_ops->domain_alloc(IOMMU_DOMAIN_UNMANAGED);
> +	domain = bus->iommu_ops->domain_alloc(type);
>   	if (!domain)
>   		return NULL;
>
>   	domain->ops  = bus->iommu_ops;
> -	domain->type = IOMMU_DOMAIN_UNMANAGED;
> +	domain->type = type;
>
>   	return domain;
>   }
> +
> +struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
> +{

..having pulled them out of the bus here.

> +	return __iommu_domain_alloc(bus, IOMMU_DOMAIN_UNMANAGED);
> +}
>   EXPORT_SYMBOL_GPL(iommu_domain_alloc);
>
>   void iommu_domain_free(struct iommu_domain *domain)
>

Even if it's only to symbolically inch us closer to having some other 
solution for platform bus devices. The SMMU page size debate is one 
thing, but it's really only a subtle case of the "IOMMU instances" 
problem. Once there are two different IOMMU drivers in the system each 
serving a subset of platform devices, how can we make sure the domain 
allocated here is wrapped in the right driver-private struct? I've had a 
try with making iommu_ops per-instance, but even that doesn't work in 
this case.

Robin.

[PATCH 07/22] iommu: Limit iommu_attach/detach_device to devices with their own group

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

This patch changes the behavior of the iommu_attach_device
and iommu_detach_device functions. With this change these
functions only work on devices that have their own group.
For all other devices the iommu_group_attach/detach
functions must be used.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/iommu.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 67 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index fbfc015..adeedd2 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -433,6 +433,17 @@ void iommu_group_remove_device(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_group_remove_device);
 
+static int iommu_group_device_count(struct iommu_group *group)
+{
+	struct iommu_device *entry;
+	int ret = 0;
+
+	list_for_each_entry(entry, &group->devices, list)
+		ret++;
+
+	return ret;
+}
+
 /**
  * iommu_group_for_each_dev - iterate over each device in the group
  * @group: the group
@@ -970,7 +981,8 @@ void iommu_domain_free(struct iommu_domain *domain)
 }
 EXPORT_SYMBOL_GPL(iommu_domain_free);
 
-int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
+static int __iommu_attach_device(struct iommu_domain *domain,
+				 struct device *dev)
 {
 	int ret;
 	if (unlikely(domain->ops->attach_dev == NULL))
@@ -981,9 +993,38 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
 		trace_attach_device_to_domain(dev);
 	return ret;
 }
+
+int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
+{
+	struct iommu_group *group;
+	int ret;
+
+	group = iommu_group_get(dev);
+	/* FIXME: Remove this when groups a mandatory for iommu drivers */
+	if (group == NULL)
+		return __iommu_attach_device(domain, dev);
+
+	/*
+	 * We have a group - lock it to make sure the device-count doesn't
+	 * change while we are attaching
+	 */
+	mutex_lock(&group->mutex);
+	ret = -EINVAL;
+	if (iommu_group_device_count(group) != 1)
+		goto out_unlock;
+
+	ret = __iommu_attach_device(domain, dev);
+
+out_unlock:
+	mutex_unlock(&group->mutex);
+	iommu_group_put(group);
+
+	return ret;
+}
 EXPORT_SYMBOL_GPL(iommu_attach_device);
 
-void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
+static void __iommu_detach_device(struct iommu_domain *domain,
+				  struct device *dev)
 {
 	if (unlikely(domain->ops->detach_dev == NULL))
 		return;
@@ -991,6 +1032,28 @@ void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
 	domain->ops->detach_dev(domain, dev);
 	trace_detach_device_from_domain(dev);
 }
+
+void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
+{
+	struct iommu_group *group;
+
+	group = iommu_group_get(dev);
+	/* FIXME: Remove this when groups a mandatory for iommu drivers */
+	if (group == NULL)
+		return __iommu_detach_device(domain, dev);
+
+	mutex_lock(&group->mutex);
+	if (iommu_group_device_count(group) != 1) {
+		WARN_ON(1);
+		goto out_unlock;
+	}
+
+	__iommu_detach_device(domain, dev);
+
+out_unlock:
+	mutex_unlock(&group->mutex);
+	iommu_group_put(group);
+}
 EXPORT_SYMBOL_GPL(iommu_detach_device);
 
 /*
@@ -1007,7 +1070,7 @@ static int iommu_group_do_attach_device(struct device *dev, void *data)
 {
 	struct iommu_domain *domain = data;
 
-	return iommu_attach_device(domain, dev);
+	return __iommu_attach_device(domain, dev);
 }
 
 int iommu_attach_group(struct iommu_domain *domain, struct iommu_group *group)
@@ -1021,7 +1084,7 @@ static int iommu_group_do_detach_device(struct device *dev, void *data)
 {
 	struct iommu_domain *domain = data;
 
-	iommu_detach_device(domain, dev);
+	__iommu_detach_device(domain, dev);
 
 	return 0;
 }
-- 
1.9.1

[PATCH 08/22] iommu: Make sure a device is always attached to a domain

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Make use of the default domain and re-attach a device to it
when it is detached from another domain. Also enforce that a
device has to be in the default domain before it can be
attached to a different domain.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/iommu.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 75 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index adeedd2..7bce522 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -52,6 +52,7 @@ struct iommu_group {
 	char *name;
 	int id;
 	struct iommu_domain *default_domain;
+	struct iommu_domain *domain;
 };
 
 struct iommu_device {
@@ -78,6 +79,12 @@ struct iommu_group_attribute iommu_group_attr_##_name =		\
 
 static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
 						 unsigned type);
+static int __iommu_attach_device(struct iommu_domain *domain,
+				 struct device *dev);
+static int __iommu_attach_group(struct iommu_domain *domain,
+				struct iommu_group *group);
+static void __iommu_detach_group(struct iommu_domain *domain,
+				 struct iommu_group *group);
 
 static ssize_t iommu_group_attr_show(struct kobject *kobj,
 				     struct attribute *__attr, char *buf)
@@ -376,6 +383,8 @@ rename:
 
 	mutex_lock(&group->mutex);
 	list_add_tail(&device->list, &group->devices);
+	if (group->domain)
+		__iommu_attach_device(group->domain, dev);
 	mutex_unlock(&group->mutex);
 
 	/* Notify any listeners about change to group. */
@@ -455,19 +464,30 @@ static int iommu_group_device_count(struct iommu_group *group)
  * The group->mutex is held across callbacks, which will block calls to
  * iommu_group_add/remove_device.
  */
-int iommu_group_for_each_dev(struct iommu_group *group, void *data,
-			     int (*fn)(struct device *, void *))
+static int __iommu_group_for_each_dev(struct iommu_group *group, void *data,
+				      int (*fn)(struct device *, void *))
 {
 	struct iommu_device *device;
 	int ret = 0;
 
-	mutex_lock(&group->mutex);
 	list_for_each_entry(device, &group->devices, list) {
 		ret = fn(device->dev, data);
 		if (ret)
 			break;
 	}
+	return ret;
+}
+
+
+int iommu_group_for_each_dev(struct iommu_group *group, void *data,
+			     int (*fn)(struct device *, void *))
+{
+	int ret;
+
+	mutex_lock(&group->mutex);
+	ret = __iommu_group_for_each_dev(group, data, fn);
 	mutex_unlock(&group->mutex);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(iommu_group_for_each_dev);
@@ -1013,7 +1033,7 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
 	if (iommu_group_device_count(group) != 1)
 		goto out_unlock;
 
-	ret = __iommu_attach_device(domain, dev);
+	ret = __iommu_attach_group(domain, group);
 
 out_unlock:
 	mutex_unlock(&group->mutex);
@@ -1048,7 +1068,7 @@ void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
 		goto out_unlock;
 	}
 
-	__iommu_detach_device(domain, dev);
+	__iommu_detach_group(domain, group);
 
 out_unlock:
 	mutex_unlock(&group->mutex);
@@ -1073,10 +1093,31 @@ static int iommu_group_do_attach_device(struct device *dev, void *data)
 	return __iommu_attach_device(domain, dev);
 }
 
+static int __iommu_attach_group(struct iommu_domain *domain,
+				struct iommu_group *group)
+{
+	int ret;
+
+	if (group->default_domain && group->domain != group->default_domain)
+		return -EBUSY;
+
+	ret = __iommu_group_for_each_dev(group, domain,
+					 iommu_group_do_attach_device);
+	if (ret == 0)
+		group->domain = domain;
+
+	return ret;
+}
+
 int iommu_attach_group(struct iommu_domain *domain, struct iommu_group *group)
 {
-	return iommu_group_for_each_dev(group, domain,
-					iommu_group_do_attach_device);
+	int ret;
+
+	mutex_lock(&group->mutex);
+	ret = __iommu_attach_group(domain, group);
+	mutex_unlock(&group->mutex);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(iommu_attach_group);
 
@@ -1089,9 +1130,35 @@ static int iommu_group_do_detach_device(struct device *dev, void *data)
 	return 0;
 }
 
+static void __iommu_detach_group(struct iommu_domain *domain,
+				 struct iommu_group *group)
+{
+	int ret;
+
+	if (!group->default_domain) {
+		__iommu_group_for_each_dev(group, domain,
+					   iommu_group_do_detach_device);
+		group->domain = NULL;
+		return;
+	}
+
+	if (group->domain == group->default_domain)
+		return;
+
+	/* Detach by re-attaching to the default domain */
+	ret = __iommu_group_for_each_dev(group, group->default_domain,
+					 iommu_group_do_attach_device);
+	if (ret != 0)
+		WARN_ON(1);
+	else
+		group->domain = group->default_domain;
+}
+
 void iommu_detach_group(struct iommu_domain *domain, struct iommu_group *group)
 {
-	iommu_group_for_each_dev(group, domain, iommu_group_do_detach_device);
+	mutex_lock(&group->mutex);
+	__iommu_detach_group(domain, group);
+	mutex_unlock(&group->mutex);
 }
 EXPORT_SYMBOL_GPL(iommu_detach_group);
 
-- 
1.9.1

Re: [PATCH 08/22] iommu: Make sure a device is always attached to a domain

[Robin Murphy]

On 28/05/15 17:41, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> Make use of the default domain and re-attach a device to it
> when it is detached from another domain. Also enforce that a
> device has to be in the default domain before it can be
> attached to a different domain.
>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>   drivers/iommu/iommu.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 75 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index adeedd2..7bce522 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -52,6 +52,7 @@ struct iommu_group {
>   	char *name;
>   	int id;
>   	struct iommu_domain *default_domain;
> +	struct iommu_domain *domain;
>   };
>
>   struct iommu_device {
> @@ -78,6 +79,12 @@ struct iommu_group_attribute iommu_group_attr_##_name =		\
>
>   static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
>   						 unsigned type);
> +static int __iommu_attach_device(struct iommu_domain *domain,
> +				 struct device *dev);
> +static int __iommu_attach_group(struct iommu_domain *domain,
> +				struct iommu_group *group);
> +static void __iommu_detach_group(struct iommu_domain *domain,
> +				 struct iommu_group *group);
>
>   static ssize_t iommu_group_attr_show(struct kobject *kobj,
>   				     struct attribute *__attr, char *buf)
> @@ -376,6 +383,8 @@ rename:
>
>   	mutex_lock(&group->mutex);
>   	list_add_tail(&device->list, &group->devices);
> +	if (group->domain)
> +		__iommu_attach_device(group->domain, dev);
>   	mutex_unlock(&group->mutex);
>
>   	/* Notify any listeners about change to group. */
> @@ -455,19 +464,30 @@ static int iommu_group_device_count(struct iommu_group *group)
>    * The group->mutex is held across callbacks, which will block calls to
>    * iommu_group_add/remove_device.
>    */
> -int iommu_group_for_each_dev(struct iommu_group *group, void *data,
> -			     int (*fn)(struct device *, void *))
> +static int __iommu_group_for_each_dev(struct iommu_group *group, void *data,
> +				      int (*fn)(struct device *, void *))
>   {
>   	struct iommu_device *device;
>   	int ret = 0;
>
> -	mutex_lock(&group->mutex);
>   	list_for_each_entry(device, &group->devices, list) {

Maybe worth using list_for_each_entry_safe so we can reuse this for 
things like group teardown?

>   		ret = fn(device->dev, data);
>   		if (ret)
>   			break;
>   	}
> +	return ret;
> +}
> +
> +
> +int iommu_group_for_each_dev(struct iommu_group *group, void *data,
> +			     int (*fn)(struct device *, void *))
> +{
> +	int ret;
> +
> +	mutex_lock(&group->mutex);
> +	ret = __iommu_group_for_each_dev(group, data, fn);
>   	mutex_unlock(&group->mutex);
> +
>   	return ret;
>   }
>   EXPORT_SYMBOL_GPL(iommu_group_for_each_dev);
> @@ -1013,7 +1033,7 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
>   	if (iommu_group_device_count(group) != 1)
>   		goto out_unlock;
>
> -	ret = __iommu_attach_device(domain, dev);
> +	ret = __iommu_attach_group(domain, group);
>
>   out_unlock:
>   	mutex_unlock(&group->mutex);
> @@ -1048,7 +1068,7 @@ void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
>   		goto out_unlock;
>   	}
>
> -	__iommu_detach_device(domain, dev);
> +	__iommu_detach_group(domain, group);
>
>   out_unlock:
>   	mutex_unlock(&group->mutex);
> @@ -1073,10 +1093,31 @@ static int iommu_group_do_attach_device(struct device *dev, void *data)
>   	return __iommu_attach_device(domain, dev);
>   }
>
> +static int __iommu_attach_group(struct iommu_domain *domain,
> +				struct iommu_group *group)
> +{
> +	int ret;
> +
> +	if (group->default_domain && group->domain != group->default_domain)
> +		return -EBUSY;
> +
> +	ret = __iommu_group_for_each_dev(group, domain,
> +					 iommu_group_do_attach_device);
> +	if (ret == 0)
> +		group->domain = domain;
> +
> +	return ret;
> +}
> +
>   int iommu_attach_group(struct iommu_domain *domain, struct iommu_group *group)
>   {
> -	return iommu_group_for_each_dev(group, domain,
> -					iommu_group_do_attach_device);
> +	int ret;
> +
> +	mutex_lock(&group->mutex);
> +	ret = __iommu_attach_group(domain, group);
> +	mutex_unlock(&group->mutex);
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(iommu_attach_group);
>
> @@ -1089,9 +1130,35 @@ static int iommu_group_do_detach_device(struct device *dev, void *data)
>   	return 0;
>   }
>
> +static void __iommu_detach_group(struct iommu_domain *domain,
> +				 struct iommu_group *group)
> +{
> +	int ret;
> +
> +	if (!group->default_domain) {
> +		__iommu_group_for_each_dev(group, domain,
> +					   iommu_group_do_detach_device);
> +		group->domain = NULL;
> +		return;
> +	}
> +
> +	if (group->domain == group->default_domain)
> +		return;
> +
> +	/* Detach by re-attaching to the default domain */
> +	ret = __iommu_group_for_each_dev(group, group->default_domain,
> +					 iommu_group_do_attach_device);
> +	if (ret != 0)
> +		WARN_ON(1);
> +	else

	if (!(WARN_ON(ret)) ?

Robin.

> +		group->domain = group->default_domain;
> +}
> +
>   void iommu_detach_group(struct iommu_domain *domain, struct iommu_group *group)
>   {
> -	iommu_group_for_each_dev(group, domain, iommu_group_do_detach_device);
> +	mutex_lock(&group->mutex);
> +	__iommu_detach_group(domain, group);
> +	mutex_unlock(&group->mutex);
>   }
>   EXPORT_SYMBOL_GPL(iommu_detach_group);
>
>

[PATCH 09/22] iommu: Add iommu_get_domain_for_dev function

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

This function can be used to request the current domain a
device is attached to.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/iommu.c | 18 ++++++++++++++++++
 include/linux/iommu.h |  6 ++++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 7bce522..a0a38bd 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1076,6 +1076,24 @@ out_unlock:
 }
 EXPORT_SYMBOL_GPL(iommu_detach_device);
 
+struct iommu_domain *iommu_get_domain_for_dev(struct device *dev)
+{
+	struct iommu_domain *domain;
+	struct iommu_group *group;
+
+	group = iommu_group_get(dev);
+	/* FIXME: Remove this when groups a mandatory for iommu drivers */
+	if (group == NULL)
+		return NULL;
+
+	domain = group->domain;
+
+	iommu_group_put(group);
+
+	return domain;
+}
+EXPORT_SYMBOL_GPL(iommu_get_domain_for_dev);
+
 /*
  * IOMMU groups are really the natrual working unit of the IOMMU, but
  * the IOMMU API works on domains and devices.  Bridge that gap by
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 0546b87..683a1c4 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -193,6 +193,7 @@ extern int iommu_attach_device(struct iommu_domain *domain,
 			       struct device *dev);
 extern void iommu_detach_device(struct iommu_domain *domain,
 				struct device *dev);
+extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
 		     phys_addr_t paddr, size_t size, int prot);
 extern size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova,
@@ -332,6 +333,11 @@ static inline void iommu_detach_device(struct iommu_domain *domain,
 {
 }
 
+static inline struct iommu_domain *iommu_get_domain_for_dev(struct device *dev)
+{
+	return NULL;
+}
+
 static inline int iommu_map(struct iommu_domain *domain, unsigned long iova,
 			    phys_addr_t paddr, int gfp_order, int prot)
 {
-- 
1.9.1

[PATCH 10/22] iommu: Introduce direct mapped region handling

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Add two new functions to the IOMMU-API to allow the IOMMU
drivers to export the requirements for direct mapped regions
per device.
This is useful for exporting the information in Intel VT-d's
RMRR entries or AMD-Vi's unity mappings.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/iommu.c | 16 ++++++++++++++++
 include/linux/iommu.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index a0a38bd..6b8d6e7 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1469,3 +1469,19 @@ int iommu_domain_set_attr(struct iommu_domain *domain,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(iommu_domain_set_attr);
+
+void iommu_get_dm_regions(struct device *dev, struct list_head *list)
+{
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
+
+	if (ops && ops->get_dm_regions)
+		ops->get_dm_regions(dev, list);
+}
+
+void iommu_put_dm_regions(struct device *dev, struct list_head *list)
+{
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
+
+	if (ops && ops->put_dm_regions)
+		ops->put_dm_regions(dev, list);
+}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 683a1c4..6894999 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -114,6 +114,20 @@ enum iommu_attr {
 	DOMAIN_ATTR_MAX,
 };
 
+/**
+ * struct iommu_dm_region - descriptor for a direct mapped memory region
+ * @list: Linked list pointers
+ * @start: System physical start address of the region
+ * @length: Length of the region in bytes
+ * @prot: IOMMU Protection flags (READ/WRITE/...)
+ */
+struct iommu_dm_region {
+	struct list_head	list;
+	phys_addr_t		start;
+	size_t			length;
+	int			prot;
+};
+
 #ifdef CONFIG_IOMMU_API
 
 /**
@@ -159,6 +173,10 @@ struct iommu_ops {
 	int (*domain_set_attr)(struct iommu_domain *domain,
 			       enum iommu_attr attr, void *data);
 
+	/* Request/Free a list of direct mapping requirements for a device */
+	void (*get_dm_regions)(struct device *dev, struct list_head *list);
+	void (*put_dm_regions)(struct device *dev, struct list_head *list);
+
 	/* Window handling functions */
 	int (*domain_window_enable)(struct iommu_domain *domain, u32 wnd_nr,
 				    phys_addr_t paddr, u64 size, int prot);
@@ -205,6 +223,9 @@ extern phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t io
 extern void iommu_set_fault_handler(struct iommu_domain *domain,
 			iommu_fault_handler_t handler, void *token);
 
+extern void iommu_get_dm_regions(struct device *dev, struct list_head *list);
+extern void iommu_put_dm_regions(struct device *dev, struct list_head *list);
+
 extern int iommu_attach_group(struct iommu_domain *domain,
 			      struct iommu_group *group);
 extern void iommu_detach_group(struct iommu_domain *domain,
@@ -379,6 +400,16 @@ static inline void iommu_set_fault_handler(struct iommu_domain *domain,
 {
 }
 
+static inline void iommu_get_dm_regions(struct device *dev,
+					struct list_head *list)
+{
+}
+
+static inline void iommu_put_dm_regions(struct device *dev,
+					struct list_head *list)
+{
+}
+
 static inline int iommu_attach_group(struct iommu_domain *domain,
 				     struct iommu_group *group)
 {
-- 
1.9.1

Re: [PATCH 10/22] iommu: Introduce direct mapped region handling

[Will Deacon]

Hi Joerg,

On Thu, May 28, 2015 at 05:41:33PM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> Add two new functions to the IOMMU-API to allow the IOMMU
> drivers to export the requirements for direct mapped regions
> per device.
> This is useful for exporting the information in Intel VT-d's
> RMRR entries or AMD-Vi's unity mappings.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/iommu/iommu.c | 16 ++++++++++++++++
>  include/linux/iommu.h | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index a0a38bd..6b8d6e7 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1469,3 +1469,19 @@ int iommu_domain_set_attr(struct iommu_domain *domain,
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(iommu_domain_set_attr);
> +
> +void iommu_get_dm_regions(struct device *dev, struct list_head *list)
> +{
> +	const struct iommu_ops *ops = dev->bus->iommu_ops;
> +
> +	if (ops && ops->get_dm_regions)
> +		ops->get_dm_regions(dev, list);
> +}
> +
> +void iommu_put_dm_regions(struct device *dev, struct list_head *list)
> +{
> +	const struct iommu_ops *ops = dev->bus->iommu_ops;
> +
> +	if (ops && ops->put_dm_regions)
> +		ops->put_dm_regions(dev, list);
> +}
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 683a1c4..6894999 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -114,6 +114,20 @@ enum iommu_attr {
>  	DOMAIN_ATTR_MAX,
>  };
>  
> +/**
> + * struct iommu_dm_region - descriptor for a direct mapped memory region
> + * @list: Linked list pointers
> + * @start: System physical start address of the region
> + * @length: Length of the region in bytes
> + * @prot: IOMMU Protection flags (READ/WRITE/...)
> + */
> +struct iommu_dm_region {
> +	struct list_head	list;
> +	phys_addr_t		start;
> +	size_t			length;
> +	int			prot;
> +};

I'm slightly puzzled about this. It looks to me like we're asking the
IOMMU driver to construct a description of the system's physical address
space, but this information tends to be known elsewhere for things like
initialising lowmem on the CPU using memblock.

Also, it looks like we just use these regions to create the default
domain using iommu_map calls -- why don't we just have an IOMMU callback
to initialise the default domain instead? That would allow IOMMUs with a
per-master bypass mode to avoid allocating page tables altogether.

Will

Re: [PATCH 10/22] iommu: Introduce direct mapped region handling

[jroedel]

Hi Will,

On Fri, Jun 05, 2015 at 03:17:50PM +0100, Will Deacon wrote:
> On Thu, May 28, 2015 at 05:41:33PM +0100, Joerg Roedel wrote:
> > +/**
> > + * struct iommu_dm_region - descriptor for a direct mapped memory region
> > + * @list: Linked list pointers
> > + * @start: System physical start address of the region
> > + * @length: Length of the region in bytes
> > + * @prot: IOMMU Protection flags (READ/WRITE/...)
> > + */
> > +struct iommu_dm_region {
> > +	struct list_head	list;
> > +	phys_addr_t		start;
> > +	size_t			length;
> > +	int			prot;
> > +};
> 
> I'm slightly puzzled about this. It looks to me like we're asking the
> IOMMU driver to construct a description of the system's physical address
> space, but this information tends to be known elsewhere for things like
> initialising lowmem on the CPU using memblock.

Well, this is not about the general memory layout of the machine, it is
more about the requirements of the firmware. The firmware might have
their own mapping requirements, for example a USB controler that is
handled by the BIOS. Other devices (be2net adapters with special
firmware) might have such requirements too. On x86 these requirements
are described in the IOMMU ACPI tables (RMRR entries on Intel, Unity
mappings on AMD).

> Also, it looks like we just use these regions to create the default
> domain using iommu_map calls -- why don't we just have an IOMMU callback
> to initialise the default domain instead? That would allow IOMMUs with a
> per-master bypass mode to avoid allocating page tables altogether.

In theory yes, but this information is not only needed for the creation
of default domains, but also for a generic DMA-API implementation for
IOMMU drivers. A DMA-API implementation has to mark these address ranges
as reserved in its address allocator, so it is better to export this
information than doing the handling in the iommu drivers.


	Joerg

Re: [PATCH 10/22] iommu: Introduce direct mapped region handling

[Robin Murphy]

On 28/05/15 17:41, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> Add two new functions to the IOMMU-API to allow the IOMMU
> drivers to export the requirements for direct mapped regions
> per device.
> This is useful for exporting the information in Intel VT-d's
> RMRR entries or AMD-Vi's unity mappings.

Whilst I agree with the utility of a common abstraction for the IOMMU 
core to use (I think we could easily turn Marek's proposal for ARM[1] 
into a generic of_iommu_ backend for this), I'm less convinced that it 
needs to be exported as part of the API.

I was envisioning eventually moving the IOVA data inside the 
iommu_domain itself for the managed DMA-API implementation, so that the 
IOMMU core would be responsible for setting it up. Then the core just 
needs to ask the driver about any relevant reservations as it creates a 
domain/attaches a device, and the callers can remain in blissful 
ignorance. Besides, beyond probe time, I don't see many good reasons at 
all for a caller to have a struct device for something, yet not have 
taken control of any DMA the firmware left running.

Is that at odds with your ideas?

Robin.

[1]:http://article.gmane.org/gmane.linux.kernel.samsung-soc/45429

> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>   drivers/iommu/iommu.c | 16 ++++++++++++++++
>   include/linux/iommu.h | 31 +++++++++++++++++++++++++++++++
>   2 files changed, 47 insertions(+)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index a0a38bd..6b8d6e7 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1469,3 +1469,19 @@ int iommu_domain_set_attr(struct iommu_domain *domain,
>   	return ret;
>   }
>   EXPORT_SYMBOL_GPL(iommu_domain_set_attr);
> +
> +void iommu_get_dm_regions(struct device *dev, struct list_head *list)
> +{
> +	const struct iommu_ops *ops = dev->bus->iommu_ops;
> +
> +	if (ops && ops->get_dm_regions)
> +		ops->get_dm_regions(dev, list);
> +}
> +
> +void iommu_put_dm_regions(struct device *dev, struct list_head *list)
> +{
> +	const struct iommu_ops *ops = dev->bus->iommu_ops;
> +
> +	if (ops && ops->put_dm_regions)
> +		ops->put_dm_regions(dev, list);
> +}
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 683a1c4..6894999 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -114,6 +114,20 @@ enum iommu_attr {
>   	DOMAIN_ATTR_MAX,
>   };
>
> +/**
> + * struct iommu_dm_region - descriptor for a direct mapped memory region
> + * @list: Linked list pointers
> + * @start: System physical start address of the region
> + * @length: Length of the region in bytes
> + * @prot: IOMMU Protection flags (READ/WRITE/...)
> + */
> +struct iommu_dm_region {
> +	struct list_head	list;
> +	phys_addr_t		start;
> +	size_t			length;
> +	int			prot;
> +};
> +
>   #ifdef CONFIG_IOMMU_API
>
>   /**
> @@ -159,6 +173,10 @@ struct iommu_ops {
>   	int (*domain_set_attr)(struct iommu_domain *domain,
>   			       enum iommu_attr attr, void *data);
>
> +	/* Request/Free a list of direct mapping requirements for a device */
> +	void (*get_dm_regions)(struct device *dev, struct list_head *list);
> +	void (*put_dm_regions)(struct device *dev, struct list_head *list);
> +
>   	/* Window handling functions */
>   	int (*domain_window_enable)(struct iommu_domain *domain, u32 wnd_nr,
>   				    phys_addr_t paddr, u64 size, int prot);
> @@ -205,6 +223,9 @@ extern phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t io
>   extern void iommu_set_fault_handler(struct iommu_domain *domain,
>   			iommu_fault_handler_t handler, void *token);
>
> +extern void iommu_get_dm_regions(struct device *dev, struct list_head *list);
> +extern void iommu_put_dm_regions(struct device *dev, struct list_head *list);
> +
>   extern int iommu_attach_group(struct iommu_domain *domain,
>   			      struct iommu_group *group);
>   extern void iommu_detach_group(struct iommu_domain *domain,
> @@ -379,6 +400,16 @@ static inline void iommu_set_fault_handler(struct iommu_domain *domain,
>   {
>   }
>
> +static inline void iommu_get_dm_regions(struct device *dev,
> +					struct list_head *list)
> +{
> +}
> +
> +static inline void iommu_put_dm_regions(struct device *dev,
> +					struct list_head *list)
> +{
> +}
> +
>   static inline int iommu_attach_group(struct iommu_domain *domain,
>   				     struct iommu_group *group)
>   {
>

[PATCH 11/22] iommu: Create direct mappings in default domains

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Use the information exported by the IOMMU drivers to create
direct mapped regions in the default domains.

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

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 6b8d6e7..ffad1ea 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -325,6 +325,52 @@ int iommu_group_set_name(struct iommu_group *group, const char *name)
 }
 EXPORT_SYMBOL_GPL(iommu_group_set_name);
 
+static int iommu_group_create_direct_mappings(struct iommu_group *group,
+					      struct device *dev)
+{
+	struct iommu_domain *domain = group->default_domain;
+	struct iommu_dm_region *entry;
+	struct list_head mappings;
+	unsigned long pg_size;
+	int ret = 0;
+
+	if (!domain || domain->type != IOMMU_DOMAIN_DMA)
+		return 0;
+
+	BUG_ON(!domain->ops->pgsize_bitmap);
+
+	pg_size = 1UL << __ffs(domain->ops->pgsize_bitmap);
+	INIT_LIST_HEAD(&mappings);
+
+	iommu_get_dm_regions(dev, &mappings);
+
+	/* We need to consider overlapping regions for different devices */
+	list_for_each_entry(entry, &mappings, list) {
+		dma_addr_t start, end, addr;
+
+		start = ALIGN(entry->start, pg_size);
+		end   = ALIGN(entry->start + entry->length, pg_size);
+
+		for (addr = start; addr < end; addr += pg_size) {
+			phys_addr_t phys_addr;
+
+			phys_addr = iommu_iova_to_phys(domain, addr);
+			if (phys_addr)
+				continue;
+
+			ret = iommu_map(domain, addr, addr, pg_size, entry->prot);
+			if (ret)
+				goto out;
+		}
+
+	}
+
+out:
+	iommu_put_dm_regions(dev, &mappings);
+
+	return ret;
+}
+
 /**
  * iommu_group_add_device - add a device to an iommu group
  * @group: the group into which to add the device (reference should be held)
@@ -381,6 +427,8 @@ rename:
 
 	dev->iommu_group = group;
 
+	iommu_group_create_direct_mappings(group, dev);
+
 	mutex_lock(&group->mutex);
 	list_add_tail(&device->list, &group->devices);
 	if (group->domain)
-- 
1.9.1

[PATCH 12/22] iommu: Add function to query the default domain of a group

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

This will be used to handle unity mappings in the iommu
drivers.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/iommu.c | 5 +++++
 include/linux/iommu.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index ffad1ea..224c6dd 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -837,6 +837,11 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev)
 	return group;
 }
 
+struct iommu_domain *iommu_group_default_domain(struct iommu_group *group)
+{
+	return group->default_domain;
+}
+
 static int add_iommu_group(struct device *dev, void *data)
 {
 	struct iommu_callback_data *cb = data;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 6894999..b944b2b 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -249,6 +249,7 @@ extern int iommu_group_unregister_notifier(struct iommu_group *group,
 					   struct notifier_block *nb);
 extern int iommu_group_id(struct iommu_group *group);
 extern struct iommu_group *iommu_group_get_for_dev(struct device *dev);
+extern struct iommu_domain *iommu_group_default_domain(struct iommu_group *);
 
 extern int iommu_domain_get_attr(struct iommu_domain *domain, enum iommu_attr,
 				 void *data);
-- 
1.9.1

[PATCH 13/22] iommu: Introduce iommu_request_dm_for_dev()

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

This function can be called by an IOMMU driver to request
that a device's default domain is direct mapped.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/iommu.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/iommu.h |  6 ++++++
 2 files changed, 58 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 224c6dd..cf6ea95 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1538,3 +1538,55 @@ void iommu_put_dm_regions(struct device *dev, struct list_head *list)
 	if (ops && ops->put_dm_regions)
 		ops->put_dm_regions(dev, list);
 }
+
+/* Request that a device is direct mapped by the IOMMU */
+int iommu_request_dm_for_dev(struct device *dev)
+{
+	struct iommu_domain *dm_domain;
+	struct iommu_group *group;
+	int ret;
+
+	/* Device must already be in a group before calling this function */
+	group = iommu_group_get_for_dev(dev);
+	if (!group)
+		return -EINVAL;
+
+	mutex_lock(&group->mutex);
+
+	/* Check if the default domain is already direct mapped */
+	ret = 0;
+	if (group->default_domain &&
+	    group->default_domain->type == IOMMU_DOMAIN_IDENTITY)
+		goto out;
+
+	/* Don't change mappings of existing devices */
+	ret = -EBUSY;
+	if (iommu_group_device_count(group) != 1)
+		goto out;
+
+	/* Allocate a direct mapped domain */
+	ret = -ENOMEM;
+	dm_domain = __iommu_domain_alloc(dev->bus, IOMMU_DOMAIN_IDENTITY);
+	if (!dm_domain)
+		goto out;
+
+	/* Attach the device to the domain */
+	ret = __iommu_attach_group(dm_domain, group);
+	if (ret) {
+		iommu_domain_free(dm_domain);
+		goto out;
+	}
+
+	/* Make the direct mapped domain the default for this group */
+	iommu_domain_free(group->default_domain);
+	group->default_domain = dm_domain;
+
+	pr_info("Using direct mapping for device %s\n", dev_name(dev));
+
+	ret = 0;
+out:
+	mutex_unlock(&group->mutex);
+	iommu_group_put(group);
+
+	return ret;
+}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index b944b2b..dc767f7 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -225,6 +225,7 @@ extern void iommu_set_fault_handler(struct iommu_domain *domain,
 
 extern void iommu_get_dm_regions(struct device *dev, struct list_head *list);
 extern void iommu_put_dm_regions(struct device *dev, struct list_head *list);
+extern int iommu_request_dm_for_dev(struct device *dev);
 
 extern int iommu_attach_group(struct iommu_domain *domain,
 			      struct iommu_group *group);
@@ -411,6 +412,11 @@ static inline void iommu_put_dm_regions(struct device *dev,
 {
 }
 
+static inline int iommu_request_dm_for_dev(struct device *dev)
+{
+	return -ENODEV;
+}
+
 static inline int iommu_attach_group(struct iommu_domain *domain,
 				     struct iommu_group *group)
 {
-- 
1.9.1

[PATCH 14/22] iommu/amd: Implement dm_region call-backs

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Add the get_dm_regions and put_dm_regions callbacks to the
iommu_ops of the AMD IOMMU driver.

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

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index e43d489..9da3f0e 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3412,6 +3412,41 @@ static bool amd_iommu_capable(enum iommu_cap cap)
 	return false;
 }
 
+static void amd_iommu_get_dm_regions(struct device *dev,
+				     struct list_head *head)
+{
+	struct unity_map_entry *entry;
+	u16 devid;
+
+	devid = get_device_id(dev);
+
+	list_for_each_entry(entry, &amd_iommu_unity_map, list) {
+		struct iommu_dm_region *region;
+
+		if (devid < entry->devid_start || devid > entry->devid_end)
+			continue;
+
+		region = kzalloc(sizeof(*region), GFP_KERNEL);
+		region->start = entry->address_start;
+		region->length = entry->address_end - entry->address_start;
+		if (entry->prot & IOMMU_PROT_IR)
+			region->prot |= IOMMU_READ;
+		if (entry->prot & IOMMU_PROT_IW)
+			region->prot |= IOMMU_WRITE;
+
+		list_add_tail(&region->list, head);
+	}
+}
+
+static void amd_iommu_put_dm_regions(struct device *dev,
+				     struct list_head *head)
+{
+	struct iommu_dm_region *entry, *next;
+
+	list_for_each_entry_safe(entry, next, head, list)
+		kfree(entry);
+}
+
 static const struct iommu_ops amd_iommu_ops = {
 	.capable = amd_iommu_capable,
 	.domain_alloc = amd_iommu_domain_alloc,
@@ -3422,6 +3457,8 @@ static const struct iommu_ops amd_iommu_ops = {
 	.unmap = amd_iommu_unmap,
 	.map_sg = default_iommu_map_sg,
 	.iova_to_phys = amd_iommu_iova_to_phys,
+	.get_dm_regions = amd_iommu_get_dm_regions,
+	.put_dm_regions = amd_iommu_put_dm_regions,
 	.pgsize_bitmap	= AMD_IOMMU_PGSIZES,
 };
 
-- 
1.9.1

[PATCH 15/22] iommu/amd: Use default domain if available for DMA-API

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

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

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 9da3f0e..dc8e44b 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2495,12 +2495,19 @@ void amd_iommu_init_notifier(void)
 static struct protection_domain *get_domain(struct device *dev)
 {
 	struct protection_domain *domain;
+	struct iommu_domain *io_domain;
 	struct dma_ops_domain *dma_dom;
 	u16 devid = get_device_id(dev);
 
 	if (!check_device(dev))
 		return ERR_PTR(-EINVAL);
 
+	io_domain = iommu_get_domain_for_dev(dev);
+	if (io_domain) {
+		domain = to_pdomain(io_domain);
+		return domain;
+	}
+
 	domain = domain_for_device(dev);
 	if (domain != NULL && !dma_ops_domain(domain))
 		return ERR_PTR(-EBUSY);
-- 
1.9.1

[PATCH 16/22] iommu/amd: Implement add_device and remove_device

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Implement these two iommu-ops call-backs to make use of the
initialization and notifier features of the iommu core.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c      | 210 +++++++++++------------------------------
 drivers/iommu/amd_iommu_init.c |  31 ++----
 2 files changed, 63 insertions(+), 178 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index dc8e44b..44eeca4 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -119,7 +119,7 @@ struct iommu_cmd {
 struct kmem_cache *amd_iommu_irq_cache;
 
 static void update_domain(struct protection_domain *domain);
-static int __init alloc_passthrough_domain(void);
+static int alloc_passthrough_domain(void);
 
 /****************************************************************************
  *
@@ -434,64 +434,15 @@ static void iommu_uninit_device(struct device *dev)
 	/* Unlink from alias, it may change if another device is re-plugged */
 	dev_data->alias_data = NULL;
 
+	/* Remove dma-ops */
+	dev->archdata.dma_ops = NULL;
+
 	/*
 	 * We keep dev_data around for unplugged devices and reuse it when the
 	 * device is re-plugged - not doing so would introduce a ton of races.
 	 */
 }
 
-void __init amd_iommu_uninit_devices(void)
-{
-	struct iommu_dev_data *dev_data, *n;
-	struct pci_dev *pdev = NULL;
-
-	for_each_pci_dev(pdev) {
-
-		if (!check_device(&pdev->dev))
-			continue;
-
-		iommu_uninit_device(&pdev->dev);
-	}
-
-	/* Free all of our dev_data structures */
-	list_for_each_entry_safe(dev_data, n, &dev_data_list, dev_data_list)
-		free_dev_data(dev_data);
-}
-
-int __init amd_iommu_init_devices(void)
-{
-	struct pci_dev *pdev = NULL;
-	int ret = 0;
-
-	for_each_pci_dev(pdev) {
-
-		if (!check_device(&pdev->dev))
-			continue;
-
-		ret = iommu_init_device(&pdev->dev);
-		if (ret == -ENOTSUPP)
-			iommu_ignore_device(&pdev->dev);
-		else if (ret)
-			goto out_free;
-	}
-
-	/*
-	 * Initialize IOMMU groups only after iommu_init_device() has
-	 * had a chance to populate any IVRS defined aliases.
-	 */
-	for_each_pci_dev(pdev) {
-		if (check_device(&pdev->dev))
-			init_iommu_group(&pdev->dev);
-	}
-
-	return 0;
-
-out_free:
-
-	amd_iommu_uninit_devices();
-
-	return ret;
-}
 #ifdef CONFIG_AMD_IOMMU_STATS
 
 /*
@@ -2402,81 +2353,79 @@ static struct protection_domain *domain_for_device(struct device *dev)
 	return dom;
 }
 
-static int device_change_notifier(struct notifier_block *nb,
-				  unsigned long action, void *data)
+static int amd_iommu_add_device(struct device *dev)
 {
 	struct dma_ops_domain *dma_domain;
 	struct protection_domain *domain;
 	struct iommu_dev_data *dev_data;
-	struct device *dev = data;
 	struct amd_iommu *iommu;
 	unsigned long flags;
 	u16 devid;
+	int ret;
 
-	if (!check_device(dev))
+	if (!check_device(dev) || get_dev_data(dev))
 		return 0;
 
-	devid    = get_device_id(dev);
-	iommu    = amd_iommu_rlookup_table[devid];
-	dev_data = get_dev_data(dev);
-
-	switch (action) {
-	case BUS_NOTIFY_ADD_DEVICE:
-
-		iommu_init_device(dev);
-		init_iommu_group(dev);
+	devid = get_device_id(dev);
+	iommu = amd_iommu_rlookup_table[devid];
 
-		/*
-		 * dev_data is still NULL and
-		 * got initialized in iommu_init_device
-		 */
-		dev_data = get_dev_data(dev);
+	ret = iommu_init_device(dev);
+	if (ret == -ENOTSUPP) {
+		iommu_ignore_device(dev);
+		goto out;
+	}
+	init_iommu_group(dev);
 
-		if (iommu_pass_through || dev_data->iommu_v2) {
-			dev_data->passthrough = true;
-			attach_device(dev, pt_domain);
-			break;
-		}
+	dev_data = get_dev_data(dev);
 
-		domain = domain_for_device(dev);
+	if (iommu_pass_through || dev_data->iommu_v2) {
+		/* Make sure passthrough domain is allocated */
+		alloc_passthrough_domain();
+		dev_data->passthrough = true;
+		attach_device(dev, pt_domain);
+		goto out;
+	}
 
-		/* allocate a protection domain if a device is added */
-		dma_domain = find_protection_domain(devid);
-		if (!dma_domain) {
-			dma_domain = dma_ops_domain_alloc();
-			if (!dma_domain)
-				goto out;
-			dma_domain->target_dev = devid;
+	domain = domain_for_device(dev);
 
-			spin_lock_irqsave(&iommu_pd_list_lock, flags);
-			list_add_tail(&dma_domain->list, &iommu_pd_list);
-			spin_unlock_irqrestore(&iommu_pd_list_lock, flags);
-		}
+	/* allocate a protection domain if a device is added */
+	dma_domain = find_protection_domain(devid);
+	if (!dma_domain) {
+		dma_domain = dma_ops_domain_alloc();
+		if (!dma_domain)
+			goto out;
+		dma_domain->target_dev = devid;
 
-		dev->archdata.dma_ops = &amd_iommu_dma_ops;
+		init_unity_mappings_for_device(dma_domain, devid);
 
-		break;
-	case BUS_NOTIFY_REMOVED_DEVICE:
+		spin_lock_irqsave(&iommu_pd_list_lock, flags);
+		list_add_tail(&dma_domain->list, &iommu_pd_list);
+		spin_unlock_irqrestore(&iommu_pd_list_lock, flags);
+	}
 
-		iommu_uninit_device(dev);
+	attach_device(dev, &dma_domain->domain);
 
-	default:
-		goto out;
-	}
+	dev->archdata.dma_ops = &amd_iommu_dma_ops;
 
+out:
 	iommu_completion_wait(iommu);
 
-out:
 	return 0;
 }
 
-static struct notifier_block device_nb = {
-	.notifier_call = device_change_notifier,
-};
-
-void amd_iommu_init_notifier(void)
+static void amd_iommu_remove_device(struct device *dev)
 {
-	bus_register_notifier(&pci_bus_type, &device_nb);
+	struct amd_iommu *iommu;
+	u16 devid;
+
+	if (!check_device(dev))
+		return;
+
+	devid = get_device_id(dev);
+	iommu = amd_iommu_rlookup_table[devid];
+
+	iommu_uninit_device(dev);
+	iommu_completion_wait(iommu);
 }
 
 /*****************************************************************************
@@ -3018,54 +2967,6 @@ static int amd_iommu_dma_supported(struct device *dev, u64 mask)
 	return check_device(dev);
 }
 
-/*
- * The function for pre-allocating protection domains.
- *
- * If the driver core informs the DMA layer if a driver grabs a device
- * we don't need to preallocate the protection domains anymore.
- * For now we have to.
- */
-static void __init prealloc_protection_domains(void)
-{
-	struct iommu_dev_data *dev_data;
-	struct dma_ops_domain *dma_dom;
-	struct pci_dev *dev = NULL;
-	u16 devid;
-
-	for_each_pci_dev(dev) {
-
-		/* Do we handle this device? */
-		if (!check_device(&dev->dev))
-			continue;
-
-		dev_data = get_dev_data(&dev->dev);
-		if (!amd_iommu_force_isolation && dev_data->iommu_v2) {
-			/* Make sure passthrough domain is allocated */
-			alloc_passthrough_domain();
-			dev_data->passthrough = true;
-			attach_device(&dev->dev, pt_domain);
-			pr_info("AMD-Vi: Using passthrough domain for device %s\n",
-				dev_name(&dev->dev));
-		}
-
-		/* Is there already any domain for it? */
-		if (domain_for_device(&dev->dev))
-			continue;
-
-		devid = get_device_id(&dev->dev);
-
-		dma_dom = dma_ops_domain_alloc();
-		if (!dma_dom)
-			continue;
-		init_unity_mappings_for_device(dma_dom, devid);
-		dma_dom->target_dev = devid;
-
-		attach_device(&dev->dev, &dma_dom->domain);
-
-		list_add_tail(&dma_dom->list, &iommu_pd_list);
-	}
-}
-
 static struct dma_map_ops amd_iommu_dma_ops = {
 	.alloc = alloc_coherent,
 	.free = free_coherent,
@@ -3131,11 +3032,6 @@ int __init amd_iommu_init_dma_ops(void)
 			goto free_domains;
 	}
 
-	/*
-	 * Pre-allocate the protection domains for each device.
-	 */
-	prealloc_protection_domains();
-
 	iommu_detected = 1;
 	swiotlb = 0;
 
@@ -3228,7 +3124,7 @@ out_err:
 	return NULL;
 }
 
-static int __init alloc_passthrough_domain(void)
+static int alloc_passthrough_domain(void)
 {
 	if (pt_domain != NULL)
 		return 0;
@@ -3464,6 +3360,8 @@ static const struct iommu_ops amd_iommu_ops = {
 	.unmap = amd_iommu_unmap,
 	.map_sg = default_iommu_map_sg,
 	.iova_to_phys = amd_iommu_iova_to_phys,
+	.add_device = amd_iommu_add_device,
+	.remove_device = amd_iommu_remove_device,
 	.get_dm_regions = amd_iommu_get_dm_regions,
 	.put_dm_regions = amd_iommu_put_dm_regions,
 	.pgsize_bitmap	= AMD_IOMMU_PGSIZES,
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 450ef50..e4a6e40 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -226,6 +226,7 @@ static enum iommu_init_state init_state = IOMMU_START_STATE;
 
 static int amd_iommu_enable_interrupts(void);
 static int __init iommu_go_to_state(enum iommu_init_state state);
+static void init_device_table_dma(void);
 
 static inline void update_last_devid(u16 devid)
 {
@@ -1385,7 +1386,12 @@ static int __init amd_iommu_init_pci(void)
 			break;
 	}
 
-	ret = amd_iommu_init_devices();
+	init_device_table_dma();
+
+	for_each_iommu(iommu)
+		iommu_flush_all_caches(iommu);
+
+	amd_iommu_init_api();
 
 	print_iommu_info();
 
@@ -1825,8 +1831,6 @@ static bool __init check_ioapic_information(void)
 
 static void __init free_dma_resources(void)
 {
-	amd_iommu_uninit_devices();
-
 	free_pages((unsigned long)amd_iommu_pd_alloc_bitmap,
 		   get_order(MAX_DOMAIN_ID/8));
 
@@ -2019,27 +2023,10 @@ static bool detect_ivrs(void)
 
 static int amd_iommu_init_dma(void)
 {
-	struct amd_iommu *iommu;
-	int ret;
-
 	if (iommu_pass_through)
-		ret = amd_iommu_init_passthrough();
+		return amd_iommu_init_passthrough();
 	else
-		ret = amd_iommu_init_dma_ops();
-
-	if (ret)
-		return ret;
-
-	init_device_table_dma();
-
-	for_each_iommu(iommu)
-		iommu_flush_all_caches(iommu);
-
-	amd_iommu_init_api();
-
-	amd_iommu_init_notifier();
-
-	return 0;
+		return amd_iommu_init_dma_ops();
 }
 
 /****************************************************************************
-- 
1.9.1

[PATCH 17/22] iommu/amd: Support IOMMU_DOMAIN_DMA type allocation

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

This enables allocation of DMA-API default domains from the
IOMMU core and switches allocation of domain dma-api domain
to the IOMMU core too.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c       | 311 ++++++++++------------------------------
 drivers/iommu/amd_iommu_types.h |   3 -
 2 files changed, 73 insertions(+), 241 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 44eeca4..06d19e8 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -64,10 +64,6 @@
 
 static DEFINE_RWLOCK(amd_iommu_devtable_lock);
 
-/* A list of preallocated protection domains */
-static LIST_HEAD(iommu_pd_list);
-static DEFINE_SPINLOCK(iommu_pd_list_lock);
-
 /* List of all available dev_data structures */
 static LIST_HEAD(dev_data_list);
 static DEFINE_SPINLOCK(dev_data_list_lock);
@@ -234,31 +230,38 @@ static bool pdev_pri_erratum(struct pci_dev *pdev, u32 erratum)
 }
 
 /*
- * In this function the list of preallocated protection domains is traversed to
- * find the domain for a specific device
+ * This function actually applies the mapping to the page table of the
+ * dma_ops domain.
  */
-static struct dma_ops_domain *find_protection_domain(u16 devid)
+static void alloc_unity_mapping(struct dma_ops_domain *dma_dom,
+				struct unity_map_entry *e)
 {
-	struct dma_ops_domain *entry, *ret = NULL;
-	unsigned long flags;
-	u16 alias = amd_iommu_alias_table[devid];
-
-	if (list_empty(&iommu_pd_list))
-		return NULL;
-
-	spin_lock_irqsave(&iommu_pd_list_lock, flags);
+	u64 addr;
 
-	list_for_each_entry(entry, &iommu_pd_list, list) {
-		if (entry->target_dev == devid ||
-		    entry->target_dev == alias) {
-			ret = entry;
-			break;
-		}
+	for (addr = e->address_start; addr < e->address_end;
+	     addr += PAGE_SIZE) {
+		if (addr < dma_dom->aperture_size)
+			__set_bit(addr >> PAGE_SHIFT,
+				  dma_dom->aperture[0]->bitmap);
 	}
+}
+
+/*
+ * Inits the unity mappings required for a specific device
+ */
+static void init_unity_mappings_for_device(struct device *dev,
+					   struct dma_ops_domain *dma_dom)
+{
+	struct unity_map_entry *e;
+	u16 devid;
 
-	spin_unlock_irqrestore(&iommu_pd_list_lock, flags);
+	devid = get_device_id(dev);
 
-	return ret;
+	list_for_each_entry(e, &amd_iommu_unity_map, list) {
+		if (!(devid >= e->devid_start && devid <= e->devid_end))
+			continue;
+		alloc_unity_mapping(dma_dom, e);
+	}
 }
 
 /*
@@ -290,11 +293,23 @@ static bool check_device(struct device *dev)
 
 static void init_iommu_group(struct device *dev)
 {
+	struct dma_ops_domain *dma_domain;
+	struct iommu_domain *domain;
 	struct iommu_group *group;
 
 	group = iommu_group_get_for_dev(dev);
-	if (!IS_ERR(group))
-		iommu_group_put(group);
+	if (IS_ERR(group))
+		return;
+
+	domain = iommu_group_default_domain(group);
+	if (!domain)
+		goto out;
+
+	dma_domain = to_pdomain(domain)->priv;
+
+	init_unity_mappings_for_device(dev, dma_domain);
+out:
+	iommu_group_put(group);
 }
 
 static int __last_alias(struct pci_dev *pdev, u16 alias, void *data)
@@ -1414,94 +1429,6 @@ static unsigned long iommu_unmap_page(struct protection_domain *dom,
 	return unmapped;
 }
 
-/*
- * This function checks if a specific unity mapping entry is needed for
- * this specific IOMMU.
- */
-static int iommu_for_unity_map(struct amd_iommu *iommu,
-			       struct unity_map_entry *entry)
-{
-	u16 bdf, i;
-
-	for (i = entry->devid_start; i <= entry->devid_end; ++i) {
-		bdf = amd_iommu_alias_table[i];
-		if (amd_iommu_rlookup_table[bdf] == iommu)
-			return 1;
-	}
-
-	return 0;
-}
-
-/*
- * This function actually applies the mapping to the page table of the
- * dma_ops domain.
- */
-static int dma_ops_unity_map(struct dma_ops_domain *dma_dom,
-			     struct unity_map_entry *e)
-{
-	u64 addr;
-	int ret;
-
-	for (addr = e->address_start; addr < e->address_end;
-	     addr += PAGE_SIZE) {
-		ret = iommu_map_page(&dma_dom->domain, addr, addr, e->prot,
-				     PAGE_SIZE);
-		if (ret)
-			return ret;
-		/*
-		 * if unity mapping is in aperture range mark the page
-		 * as allocated in the aperture
-		 */
-		if (addr < dma_dom->aperture_size)
-			__set_bit(addr >> PAGE_SHIFT,
-				  dma_dom->aperture[0]->bitmap);
-	}
-
-	return 0;
-}
-
-/*
- * Init the unity mappings for a specific IOMMU in the system
- *
- * Basically iterates over all unity mapping entries and applies them to
- * the default domain DMA of that IOMMU if necessary.
- */
-static int iommu_init_unity_mappings(struct amd_iommu *iommu)
-{
-	struct unity_map_entry *entry;
-	int ret;
-
-	list_for_each_entry(entry, &amd_iommu_unity_map, list) {
-		if (!iommu_for_unity_map(iommu, entry))
-			continue;
-		ret = dma_ops_unity_map(iommu->default_dom, entry);
-		if (ret)
-			return ret;
-	}
-
-	return 0;
-}
-
-/*
- * Inits the unity mappings required for a specific device
- */
-static int init_unity_mappings_for_device(struct dma_ops_domain *dma_dom,
-					  u16 devid)
-{
-	struct unity_map_entry *e;
-	int ret;
-
-	list_for_each_entry(e, &amd_iommu_unity_map, list) {
-		if (!(devid >= e->devid_start && devid <= e->devid_end))
-			continue;
-		ret = dma_ops_unity_map(dma_dom, e);
-		if (ret)
-			return ret;
-	}
-
-	return 0;
-}
-
 /****************************************************************************
  *
  * The next functions belong to the address allocator for the dma_ops
@@ -2324,42 +2251,9 @@ static void detach_device(struct device *dev)
 	dev_data->ats.enabled = false;
 }
 
-/*
- * Find out the protection domain structure for a given PCI device. This
- * will give us the pointer to the page table root for example.
- */
-static struct protection_domain *domain_for_device(struct device *dev)
-{
-	struct iommu_dev_data *dev_data;
-	struct protection_domain *dom = NULL;
-	unsigned long flags;
-
-	dev_data   = get_dev_data(dev);
-
-	if (dev_data->domain)
-		return dev_data->domain;
-
-	if (dev_data->alias_data != NULL) {
-		struct iommu_dev_data *alias_data = dev_data->alias_data;
-
-		read_lock_irqsave(&amd_iommu_devtable_lock, flags);
-		if (alias_data->domain != NULL) {
-			__attach_device(dev_data, alias_data->domain);
-			dom = alias_data->domain;
-		}
-		read_unlock_irqrestore(&amd_iommu_devtable_lock, flags);
-	}
-
-	return dom;
-}
-
 static int amd_iommu_add_device(struct device *dev)
 {
-	struct dma_ops_domain *dma_domain;
-	struct protection_domain *domain;
-	struct iommu_dev_data *dev_data;
 	struct amd_iommu *iommu;
-	unsigned long flags;
 	u16 devid;
 	int ret;
 
@@ -2376,35 +2270,6 @@ static int amd_iommu_add_device(struct device *dev)
 	}
 	init_iommu_group(dev);
 
-	dev_data = get_dev_data(dev);
-
-	if (iommu_pass_through || dev_data->iommu_v2) {
-		/* Make sure passthrough domain is allocated */
-		alloc_passthrough_domain();
-		dev_data->passthrough = true;
-		attach_device(dev, pt_domain);
-		goto out;
-	}
-
-	domain = domain_for_device(dev);
-
-	/* allocate a protection domain if a device is added */
-	dma_domain = find_protection_domain(devid);
-	if (!dma_domain) {
-		dma_domain = dma_ops_domain_alloc();
-		if (!dma_domain)
-			goto out;
-		dma_domain->target_dev = devid;
-
-		init_unity_mappings_for_device(dma_domain, devid);
-
-		spin_lock_irqsave(&iommu_pd_list_lock, flags);
-		list_add_tail(&dma_domain->list, &iommu_pd_list);
-		spin_unlock_irqrestore(&iommu_pd_list_lock, flags);
-	}
-
-	attach_device(dev, &dma_domain->domain);
-
 	dev->archdata.dma_ops = &amd_iommu_dma_ops;
 
 out:
@@ -2445,34 +2310,19 @@ static struct protection_domain *get_domain(struct device *dev)
 {
 	struct protection_domain *domain;
 	struct iommu_domain *io_domain;
-	struct dma_ops_domain *dma_dom;
-	u16 devid = get_device_id(dev);
 
 	if (!check_device(dev))
 		return ERR_PTR(-EINVAL);
 
 	io_domain = iommu_get_domain_for_dev(dev);
-	if (io_domain) {
-		domain = to_pdomain(io_domain);
-		return domain;
-	}
+	if (!io_domain)
+		return NULL;
 
-	domain = domain_for_device(dev);
-	if (domain != NULL && !dma_ops_domain(domain))
+	domain = to_pdomain(io_domain);
+	if (!dma_ops_domain(domain))
 		return ERR_PTR(-EBUSY);
 
-	if (domain != NULL)
-		return domain;
-
-	/* Device not bound yet - bind it */
-	dma_dom = find_protection_domain(devid);
-	if (!dma_dom)
-		dma_dom = amd_iommu_rlookup_table[devid]->default_dom;
-	attach_device(dev, &dma_dom->domain);
-	DUMP_printk("Using protection domain %d for device %s\n",
-		    dma_dom->domain.id, dev_name(dev));
-
-	return &dma_dom->domain;
+	return domain;
 }
 
 static void update_device_table(struct protection_domain *domain)
@@ -3014,23 +2864,7 @@ void __init amd_iommu_init_api(void)
 
 int __init amd_iommu_init_dma_ops(void)
 {
-	struct amd_iommu *iommu;
-	int ret, unhandled;
-
-	/*
-	 * first allocate a default protection domain for every IOMMU we
-	 * found in the system. Devices not assigned to any other
-	 * protection domain will be assigned to the default one.
-	 */
-	for_each_iommu(iommu) {
-		iommu->default_dom = dma_ops_domain_alloc();
-		if (iommu->default_dom == NULL)
-			return -ENOMEM;
-		iommu->default_dom->domain.flags |= PD_DEFAULT_MASK;
-		ret = iommu_init_unity_mappings(iommu);
-		if (ret)
-			goto free_domains;
-	}
+	int unhandled;
 
 	iommu_detected = 1;
 	swiotlb = 0;
@@ -3050,14 +2884,6 @@ int __init amd_iommu_init_dma_ops(void)
 		pr_info("AMD-Vi: Lazy IO/TLB flushing enabled\n");
 
 	return 0;
-
-free_domains:
-
-	for_each_iommu(iommu) {
-		dma_ops_domain_free(iommu->default_dom);
-	}
-
-	return ret;
 }
 
 /*****************************************************************************
@@ -3142,30 +2968,39 @@ static int alloc_passthrough_domain(void)
 static struct iommu_domain *amd_iommu_domain_alloc(unsigned type)
 {
 	struct protection_domain *pdomain;
+	struct dma_ops_domain *dma_domain;
 
-	/* We only support unmanaged domains for now */
-	if (type != IOMMU_DOMAIN_UNMANAGED)
-		return NULL;
+	switch (type) {
+	case IOMMU_DOMAIN_UNMANAGED:
+		pdomain = protection_domain_alloc();
+		if (!pdomain)
+			return NULL;
 
-	pdomain = protection_domain_alloc();
-	if (!pdomain)
-		goto out_free;
+		pdomain->mode    = PAGE_MODE_3_LEVEL;
+		pdomain->pt_root = (void *)get_zeroed_page(GFP_KERNEL);
+		if (!pdomain->pt_root) {
+			protection_domain_free(pdomain);
+			return NULL;
+		}
 
-	pdomain->mode    = PAGE_MODE_3_LEVEL;
-	pdomain->pt_root = (void *)get_zeroed_page(GFP_KERNEL);
-	if (!pdomain->pt_root)
-		goto out_free;
+		pdomain->domain.geometry.aperture_start = 0;
+		pdomain->domain.geometry.aperture_end   = ~0ULL;
+		pdomain->domain.geometry.force_aperture = true;
 
-	pdomain->domain.geometry.aperture_start = 0;
-	pdomain->domain.geometry.aperture_end   = ~0ULL;
-	pdomain->domain.geometry.force_aperture = true;
+		break;
+	case IOMMU_DOMAIN_DMA:
+		dma_domain = dma_ops_domain_alloc();
+		if (!dma_domain) {
+			pr_err("AMD-Vi: Failed to allocate\n");
+			return NULL;
+		}
+		pdomain = &dma_domain->domain;
+		break;
+	default:
+		return NULL;
+	}
 
 	return &pdomain->domain;
-
-out_free:
-	protection_domain_free(pdomain);
-
-	return NULL;
 }
 
 static void amd_iommu_domain_free(struct iommu_domain *dom)
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 05030e5..fe796cf 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -552,9 +552,6 @@ struct amd_iommu {
 	/* if one, we need to send a completion wait command */
 	bool need_sync;
 
-	/* default dma_ops domain for that IOMMU */
-	struct dma_ops_domain *default_dom;
-
 	/* IOMMU sysfs device */
 	struct device *iommu_dev;
 
-- 
1.9.1

[PATCH 18/22] iommu/amd: Support IOMMU_DOMAIN_IDENTITY type allocation

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Add support to allocate direct mapped domains through the
IOMMU-API.

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

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 06d19e8..33bbb4d 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2996,6 +2996,13 @@ static struct iommu_domain *amd_iommu_domain_alloc(unsigned type)
 		}
 		pdomain = &dma_domain->domain;
 		break;
+	case IOMMU_DOMAIN_IDENTITY:
+		pdomain = protection_domain_alloc();
+		if (!pdomain)
+			return NULL;
+
+		pdomain->mode = PAGE_MODE_NONE;
+		break;
 	default:
 		return NULL;
 	}
-- 
1.9.1

[PATCH 19/22] iommu/amd: Put IOMMUv2 devices in a direct mapped domain

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

A device that might be used for HSA needs to be in a direct
mapped domain so that all DMA-API mappings stay alive when
the IOMMUv2 stack is used.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 33bbb4d..8682786 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2253,6 +2253,8 @@ static void detach_device(struct device *dev)
 
 static int amd_iommu_add_device(struct device *dev)
 {
+	struct iommu_dev_data *dev_data;
+	struct iommu_domain *domain;
 	struct amd_iommu *iommu;
 	u16 devid;
 	int ret;
@@ -2270,7 +2272,18 @@ static int amd_iommu_add_device(struct device *dev)
 	}
 	init_iommu_group(dev);
 
-	dev->archdata.dma_ops = &amd_iommu_dma_ops;
+	dev_data = get_dev_data(dev);
+	if (dev_data && dev_data->iommu_v2)
+		iommu_request_dm_for_dev(dev);
+
+	/* Domains are initialized for this device - have a look what we ended up with */
+	domain = iommu_get_domain_for_dev(dev);
+	if (domain->type == IOMMU_DOMAIN_IDENTITY) {
+		dev_data->passthrough = true;
+		dev->archdata.dma_ops = &nommu_dma_ops;
+	} else {
+		dev->archdata.dma_ops = &amd_iommu_dma_ops;
+	}
 
 out:
 	iommu_completion_wait(iommu);
-- 
1.9.1

[PATCH 20/22] iommu/amd: Get rid of device_dma_ops_init()

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

With device intialization done in the add_device call-back
now there is no reason for this function anymore.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c | 40 +---------------------------------------
 1 file changed, 1 insertion(+), 39 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 8682786..978741b 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2268,6 +2268,7 @@ static int amd_iommu_add_device(struct device *dev)
 	ret = iommu_init_device(dev);
 	if (ret == -ENOTSUPP) {
 		iommu_ignore_device(dev);
+		dev->archdata.dma_ops = &nommu_dma_ops;
 		goto out;
 	}
 	init_iommu_group(dev);
@@ -2840,36 +2841,6 @@ static struct dma_map_ops amd_iommu_dma_ops = {
 	.dma_supported = amd_iommu_dma_supported,
 };
 
-static unsigned device_dma_ops_init(void)
-{
-	struct iommu_dev_data *dev_data;
-	struct pci_dev *pdev = NULL;
-	unsigned unhandled = 0;
-
-	for_each_pci_dev(pdev) {
-		if (!check_device(&pdev->dev)) {
-
-			iommu_ignore_device(&pdev->dev);
-
-			unhandled += 1;
-			continue;
-		}
-
-		dev_data = get_dev_data(&pdev->dev);
-
-		if (!dev_data->passthrough)
-			pdev->dev.archdata.dma_ops = &amd_iommu_dma_ops;
-		else
-			pdev->dev.archdata.dma_ops = &nommu_dma_ops;
-	}
-
-	return unhandled;
-}
-
-/*
- * The function which clues the AMD IOMMU driver into dma_ops.
- */
-
 void __init amd_iommu_init_api(void)
 {
 	bus_set_iommu(&pci_bus_type, &amd_iommu_ops);
@@ -2877,18 +2848,9 @@ void __init amd_iommu_init_api(void)
 
 int __init amd_iommu_init_dma_ops(void)
 {
-	int unhandled;
-
 	iommu_detected = 1;
 	swiotlb = 0;
 
-	/* Make the driver finally visible to the drivers */
-	unhandled = device_dma_ops_init();
-	if (unhandled && max_pfn > MAX_DMA32_PFN) {
-		/* There are unhandled devices - initialize swiotlb for them */
-		swiotlb = 1;
-	}
-
 	amd_iommu_stats_init();
 
 	if (amd_iommu_unmap_flush)
-- 
1.9.1

[PATCH 21/22] iommu/amd: Remove unused fields from struct dma_ops_domain

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

The list_head and target_dev members are not used anymore.
Remove them.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c       | 1 -
 drivers/iommu/amd_iommu_types.h | 8 --------
 2 files changed, 9 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 978741b..6e73fa1 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -1886,7 +1886,6 @@ static struct dma_ops_domain *dma_ops_domain_alloc(void)
 		goto free_dma_dom;
 
 	dma_dom->need_flush = false;
-	dma_dom->target_dev = 0xffff;
 
 	add_domain_to_list(&dma_dom->domain);
 
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index fe796cf..bb56560 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -446,8 +446,6 @@ struct aperture_range {
  * Data container for a dma_ops specific protection domain
  */
 struct dma_ops_domain {
-	struct list_head list;
-
 	/* generic protection domain information */
 	struct protection_domain domain;
 
@@ -462,12 +460,6 @@ struct dma_ops_domain {
 
 	/* This will be set to true when TLB needs to be flushed */
 	bool need_flush;
-
-	/*
-	 * if this is a preallocated domain, keep the device for which it was
-	 * preallocated in this variable
-	 */
-	u16 target_dev;
 };
 
 /*
-- 
1.9.1

[PATCH 22/22] iommu/amd: Propagate errors from amd_iommu_init_api

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

This function can fail. Propagate any errors back to the
initialization state machine.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c       | 4 ++--
 drivers/iommu/amd_iommu_init.c  | 5 +++--
 drivers/iommu/amd_iommu_proto.h | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 6e73fa1..6659b51 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2840,9 +2840,9 @@ static struct dma_map_ops amd_iommu_dma_ops = {
 	.dma_supported = amd_iommu_dma_supported,
 };
 
-void __init amd_iommu_init_api(void)
+int __init amd_iommu_init_api(void)
 {
-	bus_set_iommu(&pci_bus_type, &amd_iommu_ops);
+	return bus_set_iommu(&pci_bus_type, &amd_iommu_ops);
 }
 
 int __init amd_iommu_init_dma_ops(void)
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index e4a6e40..dbac49c 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1391,9 +1391,10 @@ static int __init amd_iommu_init_pci(void)
 	for_each_iommu(iommu)
 		iommu_flush_all_caches(iommu);
 
-	amd_iommu_init_api();
+	ret = amd_iommu_init_api();
 
-	print_iommu_info();
+	if (!ret)
+		print_iommu_info();
 
 	return ret;
 }
diff --git a/drivers/iommu/amd_iommu_proto.h b/drivers/iommu/amd_iommu_proto.h
index 72b0fd4..9ed1c43 100644
--- a/drivers/iommu/amd_iommu_proto.h
+++ b/drivers/iommu/amd_iommu_proto.h
@@ -30,7 +30,7 @@ extern void amd_iommu_reset_cmd_buffer(struct amd_iommu *iommu);
 extern int amd_iommu_init_devices(void);
 extern void amd_iommu_uninit_devices(void);
 extern void amd_iommu_init_notifier(void);
-extern void amd_iommu_init_api(void);
+extern int amd_iommu_init_api(void);
 
 /* Needed for interrupt remapping */
 extern int amd_iommu_prepare(void);
-- 
1.9.1

Re: [PATCH 00/22 v2] Introduce default domains for iommu groups

[Will Deacon]

Hi Joerg,

On Thu, May 28, 2015 at 05:41:23PM +0100, Joerg Roedel wrote:
> here is the second version of my patch-set to introduce
> default domains into the iommu core. This time it has a lot
> more patches, mostly because I added a proof of concept
> implementation by converting the AMD IOMMU driver to make
> use of it.

Most of this looks fine to me, modulo by comments about the dm regions
(which I'm not sure how to implement for ARM).

> Converting the first driver to the new concept triggered a
> lot of changes and extensions in the patch-set to fit all
> the needs of a more complex iommu driver. Converting other
> drivers might need further changes, but that is something
> for the future.
> 
> A major change is that now the default domain has to be
> allocated by the code that allocates the iommu group. For
> PCI devices this happens in the IOMMU core, but drivers
> allocating the group on their own can now implement a policy
> that fits their needs (e.g. not allocate one domain per
> group but let multiple groups share one domain).

Makes sense. I really think we should be moving group allocation out of
the IOMMU drivers and into the bus code, like we already have for PCI.
Once we've got a way to describe groups of platform devices (e.g. in the
device-tree), then we can have the group creation happen automatically
as part of Laurent's of_iommu work.

Will

Re: [PATCH 00/22 v2] Introduce default domains for iommu groups

[jroedel]

Hi Will,

Thanks for having a look!

On Fri, Jun 05, 2015 at 03:22:06PM +0100, Will Deacon wrote:
> 
> Most of this looks fine to me, modulo by comments about the dm regions
> (which I'm not sure how to implement for ARM).

When there are no direct mapping requirements from the firmware on ARM,
you can just return an empty list in these call-backs.

> > A major change is that now the default domain has to be
> > allocated by the code that allocates the iommu group. For
> > PCI devices this happens in the IOMMU core, but drivers
> > allocating the group on their own can now implement a policy
> > that fits their needs (e.g. not allocate one domain per
> > group but let multiple groups share one domain).
> 
> Makes sense. I really think we should be moving group allocation out of
> the IOMMU drivers and into the bus code, like we already have for PCI.
> Once we've got a way to describe groups of platform devices (e.g. in the
> device-tree), then we can have the group creation happen automatically
> as part of Laurent's of_iommu work.

Yes, that makes sense. And PCI is pretty hardcoded into the iommu-groups
implementation right now. This needs probably be more generic too.


	Joerg