[PATCH 0/4] ipmmu-vmsa cleanup

[PATCH 0/4] ipmmu-vmsa cleanup

[Robin Murphy]

Arnd reports a build warning[1] thanks to me missing ipmmu-vmsa's second
set of ops when converting io-pgtable-arm users to the new iommu_iotlb_*
callbacks. Rather than just treat the symptom with a point fix, this
seemed like a good excuse to clean up the messy #ifdeffery and
duplication in the driver that led to the oversight in the first place.

Build-tested successfully for arm64, ARM, and ARM + CONFIG_IOMMU_DMA.

Robin.

[1]:https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1510423.html

Robin Murphy (4):
  iommu/ipmmu-vmsa: Unify domain alloc/free
  iommu/ipmmu-vmsa: Simplify group allocation
  iommu/ipmmu-vmsa: Clean up struct ipmmu_vmsa_iommu_priv
  iommu/ipmmu-vmsa: Unify ipmmu_ops

 drivers/iommu/ipmmu-vmsa.c | 251 ++++++++++++++-------------------------------
 1 file changed, 78 insertions(+), 173 deletions(-)

-- 
2.13.4.dirty

[PATCH 0/4] ipmmu-vmsa cleanup

[Robin Murphy]

Arnd reports a build warning[1] thanks to me missing ipmmu-vmsa's second
set of ops when converting io-pgtable-arm users to the new iommu_iotlb_*
callbacks. Rather than just treat the symptom with a point fix, this
seemed like a good excuse to clean up the messy #ifdeffery and
duplication in the driver that led to the oversight in the first place.

Build-tested successfully for arm64, ARM, and ARM + CONFIG_IOMMU_DMA.

Robin.

[1]:https://www.mail-archive.com/linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg1510423.html

Robin Murphy (4):
  iommu/ipmmu-vmsa: Unify domain alloc/free
  iommu/ipmmu-vmsa: Simplify group allocation
  iommu/ipmmu-vmsa: Clean up struct ipmmu_vmsa_iommu_priv
  iommu/ipmmu-vmsa: Unify ipmmu_ops

 drivers/iommu/ipmmu-vmsa.c | 251 ++++++++++++++-------------------------------
 1 file changed, 78 insertions(+), 173 deletions(-)

-- 
2.13.4.dirty

[PATCH 0/4] ipmmu-vmsa cleanup

[Robin Murphy]

Arnd reports a build warning[1] thanks to me missing ipmmu-vmsa's second
set of ops when converting io-pgtable-arm users to the new iommu_iotlb_*
callbacks. Rather than just treat the symptom with a point fix, this
seemed like a good excuse to clean up the messy #ifdeffery and
duplication in the driver that led to the oversight in the first place.

Build-tested successfully for arm64, ARM, and ARM + CONFIG_IOMMU_DMA.

Robin.

[1]:https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1510423.html

Robin Murphy (4):
  iommu/ipmmu-vmsa: Unify domain alloc/free
  iommu/ipmmu-vmsa: Simplify group allocation
  iommu/ipmmu-vmsa: Clean up struct ipmmu_vmsa_iommu_priv
  iommu/ipmmu-vmsa: Unify ipmmu_ops

 drivers/iommu/ipmmu-vmsa.c | 251 ++++++++++++++-------------------------------
 1 file changed, 78 insertions(+), 173 deletions(-)

-- 
2.13.4.dirty

[PATCH 1/4] iommu/ipmmu-vmsa: Unify domain alloc/free

[Robin Murphy]

We have two implementations for ipmmu_ops->alloc depending on
CONFIG_IOMMU_DMA, the difference being whether they accept the
IOMMU_DOMAIN_DMA type or not. However, iommu_dma_get_cookie() is
guaranteed to return an error when !CONFIG_IOMMU_DMA, so if
ipmmu_domain_alloc_dma() was actually checking and handling the return
value correctly, it would behave the same as ipmmu_domain_alloc()
anyway.

Similarly for freeing; iommu_put_dma_cookie() is robust by design.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/ipmmu-vmsa.c | 65 +++++++++++++++++-----------------------------
 1 file changed, 24 insertions(+), 41 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 0bef160de7dd..dedb386fa81d 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -528,6 +528,27 @@ static struct iommu_domain *__ipmmu_domain_alloc(unsigned type)
 	return &domain->io_domain;
 }
 
+static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
+{
+	struct iommu_domain *io_domain = NULL;
+
+	switch (type) {
+	case IOMMU_DOMAIN_UNMANAGED:
+		io_domain = __ipmmu_domain_alloc(type);
+		break;
+
+	case IOMMU_DOMAIN_DMA:
+		io_domain = __ipmmu_domain_alloc(type);
+		if (io_domain && iommu_get_dma_cookie(io_domain)) {
+			kfree(io_domain);
+			io_domain = NULL;
+		}
+		break;
+	}
+
+	return io_domain;
+}
+
 static void ipmmu_domain_free(struct iommu_domain *io_domain)
 {
 	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
@@ -536,6 +557,7 @@ static void ipmmu_domain_free(struct iommu_domain *io_domain)
 	 * Free the domain resources. We assume that all devices have already
 	 * been detached.
 	 */
+	iommu_put_dma_cookie(io_domain);
 	ipmmu_domain_destroy_context(domain);
 	free_io_pgtable_ops(domain->iop);
 	kfree(domain);
@@ -671,14 +693,6 @@ static int ipmmu_of_xlate(struct device *dev,
 
 #if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
 
-static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
-{
-	if (type != IOMMU_DOMAIN_UNMANAGED)
-		return NULL;
-
-	return __ipmmu_domain_alloc(type);
-}
-
 static int ipmmu_add_device(struct device *dev)
 {
 	struct ipmmu_vmsa_device *mmu = NULL;
@@ -779,37 +793,6 @@ static const struct iommu_ops ipmmu_ops = {
 static DEFINE_SPINLOCK(ipmmu_slave_devices_lock);
 static LIST_HEAD(ipmmu_slave_devices);
 
-static struct iommu_domain *ipmmu_domain_alloc_dma(unsigned type)
-{
-	struct iommu_domain *io_domain = NULL;
-
-	switch (type) {
-	case IOMMU_DOMAIN_UNMANAGED:
-		io_domain = __ipmmu_domain_alloc(type);
-		break;
-
-	case IOMMU_DOMAIN_DMA:
-		io_domain = __ipmmu_domain_alloc(type);
-		if (io_domain)
-			iommu_get_dma_cookie(io_domain);
-		break;
-	}
-
-	return io_domain;
-}
-
-static void ipmmu_domain_free_dma(struct iommu_domain *io_domain)
-{
-	switch (io_domain->type) {
-	case IOMMU_DOMAIN_DMA:
-		iommu_put_dma_cookie(io_domain);
-		/* fall-through */
-	default:
-		ipmmu_domain_free(io_domain);
-		break;
-	}
-}
-
 static int ipmmu_add_device_dma(struct device *dev)
 {
 	struct iommu_group *group;
@@ -878,8 +861,8 @@ static struct iommu_group *ipmmu_find_group_dma(struct device *dev)
 }
 
 static const struct iommu_ops ipmmu_ops = {
-	.domain_alloc = ipmmu_domain_alloc_dma,
-	.domain_free = ipmmu_domain_free_dma,
+	.domain_alloc = ipmmu_domain_alloc,
+	.domain_free = ipmmu_domain_free,
 	.attach_dev = ipmmu_attach_device,
 	.detach_dev = ipmmu_detach_device,
 	.map = ipmmu_map,
-- 
2.13.4.dirty

[PATCH 1/4] iommu/ipmmu-vmsa: Unify domain alloc/free

[Robin Murphy]

We have two implementations for ipmmu_ops->alloc depending on
CONFIG_IOMMU_DMA, the difference being whether they accept the
IOMMU_DOMAIN_DMA type or not. However, iommu_dma_get_cookie() is
guaranteed to return an error when !CONFIG_IOMMU_DMA, so if
ipmmu_domain_alloc_dma() was actually checking and handling the return
value correctly, it would behave the same as ipmmu_domain_alloc()
anyway.

Similarly for freeing; iommu_put_dma_cookie() is robust by design.

Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
---
 drivers/iommu/ipmmu-vmsa.c | 65 +++++++++++++++++-----------------------------
 1 file changed, 24 insertions(+), 41 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 0bef160de7dd..dedb386fa81d 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -528,6 +528,27 @@ static struct iommu_domain *__ipmmu_domain_alloc(unsigned type)
 	return &domain->io_domain;
 }
 
+static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
+{
+	struct iommu_domain *io_domain = NULL;
+
+	switch (type) {
+	case IOMMU_DOMAIN_UNMANAGED:
+		io_domain = __ipmmu_domain_alloc(type);
+		break;
+
+	case IOMMU_DOMAIN_DMA:
+		io_domain = __ipmmu_domain_alloc(type);
+		if (io_domain && iommu_get_dma_cookie(io_domain)) {
+			kfree(io_domain);
+			io_domain = NULL;
+		}
+		break;
+	}
+
+	return io_domain;
+}
+
 static void ipmmu_domain_free(struct iommu_domain *io_domain)
 {
 	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
@@ -536,6 +557,7 @@ static void ipmmu_domain_free(struct iommu_domain *io_domain)
 	 * Free the domain resources. We assume that all devices have already
 	 * been detached.
 	 */
+	iommu_put_dma_cookie(io_domain);
 	ipmmu_domain_destroy_context(domain);
 	free_io_pgtable_ops(domain->iop);
 	kfree(domain);
@@ -671,14 +693,6 @@ static int ipmmu_of_xlate(struct device *dev,
 
 #if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
 
-static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
-{
-	if (type != IOMMU_DOMAIN_UNMANAGED)
-		return NULL;
-
-	return __ipmmu_domain_alloc(type);
-}
-
 static int ipmmu_add_device(struct device *dev)
 {
 	struct ipmmu_vmsa_device *mmu = NULL;
@@ -779,37 +793,6 @@ static const struct iommu_ops ipmmu_ops = {
 static DEFINE_SPINLOCK(ipmmu_slave_devices_lock);
 static LIST_HEAD(ipmmu_slave_devices);
 
-static struct iommu_domain *ipmmu_domain_alloc_dma(unsigned type)
-{
-	struct iommu_domain *io_domain = NULL;
-
-	switch (type) {
-	case IOMMU_DOMAIN_UNMANAGED:
-		io_domain = __ipmmu_domain_alloc(type);
-		break;
-
-	case IOMMU_DOMAIN_DMA:
-		io_domain = __ipmmu_domain_alloc(type);
-		if (io_domain)
-			iommu_get_dma_cookie(io_domain);
-		break;
-	}
-
-	return io_domain;
-}
-
-static void ipmmu_domain_free_dma(struct iommu_domain *io_domain)
-{
-	switch (io_domain->type) {
-	case IOMMU_DOMAIN_DMA:
-		iommu_put_dma_cookie(io_domain);
-		/* fall-through */
-	default:
-		ipmmu_domain_free(io_domain);
-		break;
-	}
-}
-
 static int ipmmu_add_device_dma(struct device *dev)
 {
 	struct iommu_group *group;
@@ -878,8 +861,8 @@ static struct iommu_group *ipmmu_find_group_dma(struct device *dev)
 }
 
 static const struct iommu_ops ipmmu_ops = {
-	.domain_alloc = ipmmu_domain_alloc_dma,
-	.domain_free = ipmmu_domain_free_dma,
+	.domain_alloc = ipmmu_domain_alloc,
+	.domain_free = ipmmu_domain_free,
 	.attach_dev = ipmmu_attach_device,
 	.detach_dev = ipmmu_detach_device,
 	.map = ipmmu_map,
-- 
2.13.4.dirty

[PATCH 1/4] iommu/ipmmu-vmsa: Unify domain alloc/free

[Robin Murphy]

We have two implementations for ipmmu_ops->alloc depending on
CONFIG_IOMMU_DMA, the difference being whether they accept the
IOMMU_DOMAIN_DMA type or not. However, iommu_dma_get_cookie() is
guaranteed to return an error when !CONFIG_IOMMU_DMA, so if
ipmmu_domain_alloc_dma() was actually checking and handling the return
value correctly, it would behave the same as ipmmu_domain_alloc()
anyway.

Similarly for freeing; iommu_put_dma_cookie() is robust by design.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/ipmmu-vmsa.c | 65 +++++++++++++++++-----------------------------
 1 file changed, 24 insertions(+), 41 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 0bef160de7dd..dedb386fa81d 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -528,6 +528,27 @@ static struct iommu_domain *__ipmmu_domain_alloc(unsigned type)
 	return &domain->io_domain;
 }
 
+static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
+{
+	struct iommu_domain *io_domain = NULL;
+
+	switch (type) {
+	case IOMMU_DOMAIN_UNMANAGED:
+		io_domain = __ipmmu_domain_alloc(type);
+		break;
+
+	case IOMMU_DOMAIN_DMA:
+		io_domain = __ipmmu_domain_alloc(type);
+		if (io_domain && iommu_get_dma_cookie(io_domain)) {
+			kfree(io_domain);
+			io_domain = NULL;
+		}
+		break;
+	}
+
+	return io_domain;
+}
+
 static void ipmmu_domain_free(struct iommu_domain *io_domain)
 {
 	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
@@ -536,6 +557,7 @@ static void ipmmu_domain_free(struct iommu_domain *io_domain)
 	 * Free the domain resources. We assume that all devices have already
 	 * been detached.
 	 */
+	iommu_put_dma_cookie(io_domain);
 	ipmmu_domain_destroy_context(domain);
 	free_io_pgtable_ops(domain->iop);
 	kfree(domain);
@@ -671,14 +693,6 @@ static int ipmmu_of_xlate(struct device *dev,
 
 #if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
 
-static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
-{
-	if (type != IOMMU_DOMAIN_UNMANAGED)
-		return NULL;
-
-	return __ipmmu_domain_alloc(type);
-}
-
 static int ipmmu_add_device(struct device *dev)
 {
 	struct ipmmu_vmsa_device *mmu = NULL;
@@ -779,37 +793,6 @@ static const struct iommu_ops ipmmu_ops = {
 static DEFINE_SPINLOCK(ipmmu_slave_devices_lock);
 static LIST_HEAD(ipmmu_slave_devices);
 
-static struct iommu_domain *ipmmu_domain_alloc_dma(unsigned type)
-{
-	struct iommu_domain *io_domain = NULL;
-
-	switch (type) {
-	case IOMMU_DOMAIN_UNMANAGED:
-		io_domain = __ipmmu_domain_alloc(type);
-		break;
-
-	case IOMMU_DOMAIN_DMA:
-		io_domain = __ipmmu_domain_alloc(type);
-		if (io_domain)
-			iommu_get_dma_cookie(io_domain);
-		break;
-	}
-
-	return io_domain;
-}
-
-static void ipmmu_domain_free_dma(struct iommu_domain *io_domain)
-{
-	switch (io_domain->type) {
-	case IOMMU_DOMAIN_DMA:
-		iommu_put_dma_cookie(io_domain);
-		/* fall-through */
-	default:
-		ipmmu_domain_free(io_domain);
-		break;
-	}
-}
-
 static int ipmmu_add_device_dma(struct device *dev)
 {
 	struct iommu_group *group;
@@ -878,8 +861,8 @@ static struct iommu_group *ipmmu_find_group_dma(struct device *dev)
 }
 
 static const struct iommu_ops ipmmu_ops = {
-	.domain_alloc = ipmmu_domain_alloc_dma,
-	.domain_free = ipmmu_domain_free_dma,
+	.domain_alloc = ipmmu_domain_alloc,
+	.domain_free = ipmmu_domain_free,
 	.attach_dev = ipmmu_attach_device,
 	.detach_dev = ipmmu_detach_device,
 	.map = ipmmu_map,
-- 
2.13.4.dirty

[PATCH 2/4] iommu/ipmmu-vmsa: Simplify group allocation

[Robin Murphy]

We go through quite the merry dance in order to find masters behind the
same IPMMU instance, so that we can ensure they are grouped together.
None of which is really necessary, since the master's private data
already points to the particular IPMMU it is associated with, and that
IPMMU instance data is the perfect place to keep track of a per-instance
group directly.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/ipmmu-vmsa.c | 53 ++++++++--------------------------------------
 1 file changed, 9 insertions(+), 44 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index dedb386fa81d..d8c9bd4dc657 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -43,6 +43,7 @@ struct ipmmu_vmsa_device {
 	DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
 	struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
 
+	struct iommu_group *group;
 	struct dma_iommu_mapping *mapping;
 };
 
@@ -59,8 +60,6 @@ struct ipmmu_vmsa_domain {
 
 struct ipmmu_vmsa_iommu_priv {
 	struct ipmmu_vmsa_device *mmu;
-	struct device *dev;
-	struct list_head list;
 };
 
 static struct ipmmu_vmsa_domain *to_vmsa_domain(struct iommu_domain *dom)
@@ -674,7 +673,6 @@ static int ipmmu_init_platform_device(struct device *dev,
 		return -ENOMEM;
 
 	priv->mmu = platform_get_drvdata(ipmmu_pdev);
-	priv->dev = dev;
 	dev->iommu_fwspec->iommu_priv = priv;
 	return 0;
 }
@@ -790,9 +788,6 @@ static const struct iommu_ops ipmmu_ops = {
 
 #ifdef CONFIG_IOMMU_DMA
 
-static DEFINE_SPINLOCK(ipmmu_slave_devices_lock);
-static LIST_HEAD(ipmmu_slave_devices);
-
 static int ipmmu_add_device_dma(struct device *dev)
 {
 	struct iommu_group *group;
@@ -807,55 +802,25 @@ static int ipmmu_add_device_dma(struct device *dev)
 	if (IS_ERR(group))
 		return PTR_ERR(group);
 
-	spin_lock(&ipmmu_slave_devices_lock);
-	list_add(&to_priv(dev)->list, &ipmmu_slave_devices);
-	spin_unlock(&ipmmu_slave_devices_lock);
 	return 0;
 }
 
 static void ipmmu_remove_device_dma(struct device *dev)
 {
-	struct ipmmu_vmsa_iommu_priv *priv = to_priv(dev);
-
-	spin_lock(&ipmmu_slave_devices_lock);
-	list_del(&priv->list);
-	spin_unlock(&ipmmu_slave_devices_lock);
-
 	iommu_group_remove_device(dev);
 }
 
-static struct device *ipmmu_find_sibling_device(struct device *dev)
+static struct iommu_group *ipmmu_find_group(struct device *dev)
 {
 	struct ipmmu_vmsa_iommu_priv *priv = to_priv(dev);
-	struct ipmmu_vmsa_iommu_priv *sibling_priv = NULL;
-	bool found = false;
-
-	spin_lock(&ipmmu_slave_devices_lock);
-
-	list_for_each_entry(sibling_priv, &ipmmu_slave_devices, list) {
-		if (priv == sibling_priv)
-			continue;
-		if (sibling_priv->mmu == priv->mmu) {
-			found = true;
-			break;
-		}
-	}
-
-	spin_unlock(&ipmmu_slave_devices_lock);
-
-	return found ? sibling_priv->dev : NULL;
-}
-
-static struct iommu_group *ipmmu_find_group_dma(struct device *dev)
-{
 	struct iommu_group *group;
-	struct device *sibling;
 
-	sibling = ipmmu_find_sibling_device(dev);
-	if (sibling)
-		group = iommu_group_get(sibling);
-	if (!sibling || IS_ERR(group))
-		group = generic_device_group(dev);
+	if (priv->mmu->group)
+		return iommu_group_ref_get(priv->mmu->group);
+
+	group = iommu_group_alloc();
+	if (!IS_ERR(group))
+		priv->mmu->group = group;
 
 	return group;
 }
@@ -873,7 +838,7 @@ static const struct iommu_ops ipmmu_ops = {
 	.iova_to_phys = ipmmu_iova_to_phys,
 	.add_device = ipmmu_add_device_dma,
 	.remove_device = ipmmu_remove_device_dma,
-	.device_group = ipmmu_find_group_dma,
+	.device_group = ipmmu_find_group,
 	.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
 	.of_xlate = ipmmu_of_xlate,
 };
-- 
2.13.4.dirty

[PATCH 2/4] iommu/ipmmu-vmsa: Simplify group allocation

[Robin Murphy]

We go through quite the merry dance in order to find masters behind the
same IPMMU instance, so that we can ensure they are grouped together.
None of which is really necessary, since the master's private data
already points to the particular IPMMU it is associated with, and that
IPMMU instance data is the perfect place to keep track of a per-instance
group directly.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/ipmmu-vmsa.c | 53 ++++++++--------------------------------------
 1 file changed, 9 insertions(+), 44 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index dedb386fa81d..d8c9bd4dc657 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -43,6 +43,7 @@ struct ipmmu_vmsa_device {
 	DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
 	struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
 
+	struct iommu_group *group;
 	struct dma_iommu_mapping *mapping;
 };
 
@@ -59,8 +60,6 @@ struct ipmmu_vmsa_domain {
 
 struct ipmmu_vmsa_iommu_priv {
 	struct ipmmu_vmsa_device *mmu;
-	struct device *dev;
-	struct list_head list;
 };
 
 static struct ipmmu_vmsa_domain *to_vmsa_domain(struct iommu_domain *dom)
@@ -674,7 +673,6 @@ static int ipmmu_init_platform_device(struct device *dev,
 		return -ENOMEM;
 
 	priv->mmu = platform_get_drvdata(ipmmu_pdev);
-	priv->dev = dev;
 	dev->iommu_fwspec->iommu_priv = priv;
 	return 0;
 }
@@ -790,9 +788,6 @@ static const struct iommu_ops ipmmu_ops = {
 
 #ifdef CONFIG_IOMMU_DMA
 
-static DEFINE_SPINLOCK(ipmmu_slave_devices_lock);
-static LIST_HEAD(ipmmu_slave_devices);
-
 static int ipmmu_add_device_dma(struct device *dev)
 {
 	struct iommu_group *group;
@@ -807,55 +802,25 @@ static int ipmmu_add_device_dma(struct device *dev)
 	if (IS_ERR(group))
 		return PTR_ERR(group);
 
-	spin_lock(&ipmmu_slave_devices_lock);
-	list_add(&to_priv(dev)->list, &ipmmu_slave_devices);
-	spin_unlock(&ipmmu_slave_devices_lock);
 	return 0;
 }
 
 static void ipmmu_remove_device_dma(struct device *dev)
 {
-	struct ipmmu_vmsa_iommu_priv *priv = to_priv(dev);
-
-	spin_lock(&ipmmu_slave_devices_lock);
-	list_del(&priv->list);
-	spin_unlock(&ipmmu_slave_devices_lock);
-
 	iommu_group_remove_device(dev);
 }
 
-static struct device *ipmmu_find_sibling_device(struct device *dev)
+static struct iommu_group *ipmmu_find_group(struct device *dev)
 {
 	struct ipmmu_vmsa_iommu_priv *priv = to_priv(dev);
-	struct ipmmu_vmsa_iommu_priv *sibling_priv = NULL;
-	bool found = false;
-
-	spin_lock(&ipmmu_slave_devices_lock);
-
-	list_for_each_entry(sibling_priv, &ipmmu_slave_devices, list) {
-		if (priv == sibling_priv)
-			continue;
-		if (sibling_priv->mmu == priv->mmu) {
-			found = true;
-			break;
-		}
-	}
-
-	spin_unlock(&ipmmu_slave_devices_lock);
-
-	return found ? sibling_priv->dev : NULL;
-}
-
-static struct iommu_group *ipmmu_find_group_dma(struct device *dev)
-{
 	struct iommu_group *group;
-	struct device *sibling;
 
-	sibling = ipmmu_find_sibling_device(dev);
-	if (sibling)
-		group = iommu_group_get(sibling);
-	if (!sibling || IS_ERR(group))
-		group = generic_device_group(dev);
+	if (priv->mmu->group)
+		return iommu_group_ref_get(priv->mmu->group);
+
+	group = iommu_group_alloc();
+	if (!IS_ERR(group))
+		priv->mmu->group = group;
 
 	return group;
 }
@@ -873,7 +838,7 @@ static const struct iommu_ops ipmmu_ops = {
 	.iova_to_phys = ipmmu_iova_to_phys,
 	.add_device = ipmmu_add_device_dma,
 	.remove_device = ipmmu_remove_device_dma,
-	.device_group = ipmmu_find_group_dma,
+	.device_group = ipmmu_find_group,
 	.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
 	.of_xlate = ipmmu_of_xlate,
 };
-- 
2.13.4.dirty

[PATCH 3/4] iommu/ipmmu-vmsa: Clean up struct ipmmu_vmsa_iommu_priv

[Robin Murphy]

Now that the IPMMU instance pointer is the only thing remaining in the
private data structure, we no longer need the extra level of indirection
and can simply stash that directlty in the fwspec.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/ipmmu-vmsa.c | 36 ++++++++++++------------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index d8c9bd4dc657..4d69c08145a1 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -58,16 +58,12 @@ struct ipmmu_vmsa_domain {
 	spinlock_t lock;			/* Protects mappings */
 };
 
-struct ipmmu_vmsa_iommu_priv {
-	struct ipmmu_vmsa_device *mmu;
-};
-
 static struct ipmmu_vmsa_domain *to_vmsa_domain(struct iommu_domain *dom)
 {
 	return container_of(dom, struct ipmmu_vmsa_domain, io_domain);
 }
 
-static struct ipmmu_vmsa_iommu_priv *to_priv(struct device *dev)
+static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
 {
 	return dev->iommu_fwspec ? dev->iommu_fwspec->iommu_priv : NULL;
 }
@@ -565,15 +561,14 @@ static void ipmmu_domain_free(struct iommu_domain *io_domain)
 static int ipmmu_attach_device(struct iommu_domain *io_domain,
 			       struct device *dev)
 {
-	struct ipmmu_vmsa_iommu_priv *priv = to_priv(dev);
 	struct iommu_fwspec *fwspec = dev->iommu_fwspec;
-	struct ipmmu_vmsa_device *mmu = priv->mmu;
+	struct ipmmu_vmsa_device *mmu = to_ipmmu(dev);
 	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
 	unsigned long flags;
 	unsigned int i;
 	int ret = 0;
 
-	if (!priv || !priv->mmu) {
+	if (!mmu) {
 		dev_err(dev, "Cannot attach to IPMMU\n");
 		return -ENXIO;
 	}
@@ -662,18 +657,12 @@ static int ipmmu_init_platform_device(struct device *dev,
 				      struct of_phandle_args *args)
 {
 	struct platform_device *ipmmu_pdev;
-	struct ipmmu_vmsa_iommu_priv *priv;
 
 	ipmmu_pdev = of_find_device_by_node(args->np);
 	if (!ipmmu_pdev)
 		return -ENODEV;
 
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
-
-	priv->mmu = platform_get_drvdata(ipmmu_pdev);
-	dev->iommu_fwspec->iommu_priv = priv;
+	dev->iommu_fwspec->iommu_priv = platform_get_drvdata(ipmmu_pdev);
 	return 0;
 }
 
@@ -683,7 +672,7 @@ static int ipmmu_of_xlate(struct device *dev,
 	iommu_fwspec_add_ids(dev, spec->args, 1);
 
 	/* Initialize once - xlate() will call multiple times */
-	if (to_priv(dev))
+	if (to_ipmmu(dev))
 		return 0;
 
 	return ipmmu_init_platform_device(dev, spec);
@@ -693,14 +682,14 @@ static int ipmmu_of_xlate(struct device *dev,
 
 static int ipmmu_add_device(struct device *dev)
 {
-	struct ipmmu_vmsa_device *mmu = NULL;
+	struct ipmmu_vmsa_device *mmu = to_ipmmu(dev);
 	struct iommu_group *group;
 	int ret;
 
 	/*
 	 * Only let through devices that have been verified in xlate()
 	 */
-	if (!to_priv(dev))
+	if (!mmu)
 		return -ENODEV;
 
 	/* Create a device group and add the device to it. */
@@ -729,7 +718,6 @@ static int ipmmu_add_device(struct device *dev)
 	 * - Make the mapping size configurable ? We currently use a 2GB mapping
 	 *   at a 1GB offset to ensure that NULL VAs will fault.
 	 */
-	mmu = to_priv(dev)->mmu;
 	if (!mmu->mapping) {
 		struct dma_iommu_mapping *mapping;
 
@@ -795,7 +783,7 @@ static int ipmmu_add_device_dma(struct device *dev)
 	/*
 	 * Only let through devices that have been verified in xlate()
 	 */
-	if (!to_priv(dev))
+	if (!to_ipmmu(dev))
 		return -ENODEV;
 
 	group = iommu_group_get_for_dev(dev);
@@ -812,15 +800,15 @@ static void ipmmu_remove_device_dma(struct device *dev)
 
 static struct iommu_group *ipmmu_find_group(struct device *dev)
 {
-	struct ipmmu_vmsa_iommu_priv *priv = to_priv(dev);
+	struct ipmmu_vmsa_device *mmu = to_ipmmu(dev);
 	struct iommu_group *group;
 
-	if (priv->mmu->group)
-		return iommu_group_ref_get(priv->mmu->group);
+	if (mmu->group)
+		return iommu_group_ref_get(mmu->group);
 
 	group = iommu_group_alloc();
 	if (!IS_ERR(group))
-		priv->mmu->group = group;
+		mmu->group = group;
 
 	return group;
 }
-- 
2.13.4.dirty

[PATCH 3/4] iommu/ipmmu-vmsa: Clean up struct ipmmu_vmsa_iommu_priv

[Robin Murphy]

Now that the IPMMU instance pointer is the only thing remaining in the
private data structure, we no longer need the extra level of indirection
and can simply stash that directlty in the fwspec.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/ipmmu-vmsa.c | 36 ++++++++++++------------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index d8c9bd4dc657..4d69c08145a1 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -58,16 +58,12 @@ struct ipmmu_vmsa_domain {
 	spinlock_t lock;			/* Protects mappings */
 };
 
-struct ipmmu_vmsa_iommu_priv {
-	struct ipmmu_vmsa_device *mmu;
-};
-
 static struct ipmmu_vmsa_domain *to_vmsa_domain(struct iommu_domain *dom)
 {
 	return container_of(dom, struct ipmmu_vmsa_domain, io_domain);
 }
 
-static struct ipmmu_vmsa_iommu_priv *to_priv(struct device *dev)
+static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
 {
 	return dev->iommu_fwspec ? dev->iommu_fwspec->iommu_priv : NULL;
 }
@@ -565,15 +561,14 @@ static void ipmmu_domain_free(struct iommu_domain *io_domain)
 static int ipmmu_attach_device(struct iommu_domain *io_domain,
 			       struct device *dev)
 {
-	struct ipmmu_vmsa_iommu_priv *priv = to_priv(dev);
 	struct iommu_fwspec *fwspec = dev->iommu_fwspec;
-	struct ipmmu_vmsa_device *mmu = priv->mmu;
+	struct ipmmu_vmsa_device *mmu = to_ipmmu(dev);
 	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
 	unsigned long flags;
 	unsigned int i;
 	int ret = 0;
 
-	if (!priv || !priv->mmu) {
+	if (!mmu) {
 		dev_err(dev, "Cannot attach to IPMMU\n");
 		return -ENXIO;
 	}
@@ -662,18 +657,12 @@ static int ipmmu_init_platform_device(struct device *dev,
 				      struct of_phandle_args *args)
 {
 	struct platform_device *ipmmu_pdev;
-	struct ipmmu_vmsa_iommu_priv *priv;
 
 	ipmmu_pdev = of_find_device_by_node(args->np);
 	if (!ipmmu_pdev)
 		return -ENODEV;
 
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
-
-	priv->mmu = platform_get_drvdata(ipmmu_pdev);
-	dev->iommu_fwspec->iommu_priv = priv;
+	dev->iommu_fwspec->iommu_priv = platform_get_drvdata(ipmmu_pdev);
 	return 0;
 }
 
@@ -683,7 +672,7 @@ static int ipmmu_of_xlate(struct device *dev,
 	iommu_fwspec_add_ids(dev, spec->args, 1);
 
 	/* Initialize once - xlate() will call multiple times */
-	if (to_priv(dev))
+	if (to_ipmmu(dev))
 		return 0;
 
 	return ipmmu_init_platform_device(dev, spec);
@@ -693,14 +682,14 @@ static int ipmmu_of_xlate(struct device *dev,
 
 static int ipmmu_add_device(struct device *dev)
 {
-	struct ipmmu_vmsa_device *mmu = NULL;
+	struct ipmmu_vmsa_device *mmu = to_ipmmu(dev);
 	struct iommu_group *group;
 	int ret;
 
 	/*
 	 * Only let through devices that have been verified in xlate()
 	 */
-	if (!to_priv(dev))
+	if (!mmu)
 		return -ENODEV;
 
 	/* Create a device group and add the device to it. */
@@ -729,7 +718,6 @@ static int ipmmu_add_device(struct device *dev)
 	 * - Make the mapping size configurable ? We currently use a 2GB mapping
 	 *   at a 1GB offset to ensure that NULL VAs will fault.
 	 */
-	mmu = to_priv(dev)->mmu;
 	if (!mmu->mapping) {
 		struct dma_iommu_mapping *mapping;
 
@@ -795,7 +783,7 @@ static int ipmmu_add_device_dma(struct device *dev)
 	/*
 	 * Only let through devices that have been verified in xlate()
 	 */
-	if (!to_priv(dev))
+	if (!to_ipmmu(dev))
 		return -ENODEV;
 
 	group = iommu_group_get_for_dev(dev);
@@ -812,15 +800,15 @@ static void ipmmu_remove_device_dma(struct device *dev)
 
 static struct iommu_group *ipmmu_find_group(struct device *dev)
 {
-	struct ipmmu_vmsa_iommu_priv *priv = to_priv(dev);
+	struct ipmmu_vmsa_device *mmu = to_ipmmu(dev);
 	struct iommu_group *group;
 
-	if (priv->mmu->group)
-		return iommu_group_ref_get(priv->mmu->group);
+	if (mmu->group)
+		return iommu_group_ref_get(mmu->group);
 
 	group = iommu_group_alloc();
 	if (!IS_ERR(group))
-		priv->mmu->group = group;
+		mmu->group = group;
 
 	return group;
 }
-- 
2.13.4.dirty

[PATCH 4/4] iommu/ipmmu-vmsa: Unify ipmmu_ops

[Robin Murphy]

The remaining difference between the ARM-specific and iommu-dma ops is
in the {add,remove}_device implementations, but even those have some
overlap and duplication. By stubbing out the few arm_iommu_*() calls,
we can get rid of the rest of the inline #ifdeffery to both simplify the
code and improve build coverage.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/ipmmu-vmsa.c | 69 +++++++++++++---------------------------------
 1 file changed, 19 insertions(+), 50 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 4d69c08145a1..2005aad09f2f 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -27,6 +27,11 @@
 #if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
 #include <asm/dma-iommu.h>
 #include <asm/pgalloc.h>
+#else
+#define arm_iommu_create_mapping(...)	NULL
+#define arm_iommu_attach_device(...)	-ENODEV
+#define arm_iommu_release_mapping(...)	do {} while (0)
+#define arm_iommu_detach_device(...)	do {} while (0)
 #endif
 
 #include "io-pgtable.h"
@@ -678,26 +683,17 @@ static int ipmmu_of_xlate(struct device *dev,
 	return ipmmu_init_platform_device(dev, spec);
 }
 
-#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
-
-static int ipmmu_add_device(struct device *dev)
+static int ipmmu_init_arm_mapping(struct device *dev)
 {
 	struct ipmmu_vmsa_device *mmu = to_ipmmu(dev);
 	struct iommu_group *group;
 	int ret;
 
-	/*
-	 * Only let through devices that have been verified in xlate()
-	 */
-	if (!mmu)
-		return -ENODEV;
-
 	/* Create a device group and add the device to it. */
 	group = iommu_group_alloc();
 	if (IS_ERR(group)) {
 		dev_err(dev, "Failed to allocate IOMMU group\n");
-		ret = PTR_ERR(group);
-		goto error;
+		return PTR_ERR(group);
 	}
 
 	ret = iommu_group_add_device(group, dev);
@@ -705,8 +701,7 @@ static int ipmmu_add_device(struct device *dev)
 
 	if (ret < 0) {
 		dev_err(dev, "Failed to add device to IPMMU group\n");
-		group = NULL;
-		goto error;
+		return ret;
 	}
 
 	/*
@@ -742,41 +737,14 @@ static int ipmmu_add_device(struct device *dev)
 	return 0;
 
 error:
-	if (mmu)
+	iommu_group_remove_device(dev);
+	if (mmu->mapping)
 		arm_iommu_release_mapping(mmu->mapping);
 
-	if (!IS_ERR_OR_NULL(group))
-		iommu_group_remove_device(dev);
-
 	return ret;
 }
 
-static void ipmmu_remove_device(struct device *dev)
-{
-	arm_iommu_detach_device(dev);
-	iommu_group_remove_device(dev);
-}
-
-static const struct iommu_ops ipmmu_ops = {
-	.domain_alloc = ipmmu_domain_alloc,
-	.domain_free = ipmmu_domain_free,
-	.attach_dev = ipmmu_attach_device,
-	.detach_dev = ipmmu_detach_device,
-	.map = ipmmu_map,
-	.unmap = ipmmu_unmap,
-	.map_sg = default_iommu_map_sg,
-	.iova_to_phys = ipmmu_iova_to_phys,
-	.add_device = ipmmu_add_device,
-	.remove_device = ipmmu_remove_device,
-	.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
-	.of_xlate = ipmmu_of_xlate,
-};
-
-#endif /* !CONFIG_ARM && CONFIG_IOMMU_DMA */
-
-#ifdef CONFIG_IOMMU_DMA
-
-static int ipmmu_add_device_dma(struct device *dev)
+static int ipmmu_add_device(struct device *dev)
 {
 	struct iommu_group *group;
 
@@ -786,15 +754,20 @@ static int ipmmu_add_device_dma(struct device *dev)
 	if (!to_ipmmu(dev))
 		return -ENODEV;
 
+	if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_IOMMU_DMA))
+		return ipmmu_init_arm_mapping(dev);
+
 	group = iommu_group_get_for_dev(dev);
 	if (IS_ERR(group))
 		return PTR_ERR(group);
 
+	iommu_group_put(group);
 	return 0;
 }
 
-static void ipmmu_remove_device_dma(struct device *dev)
+static void ipmmu_remove_device(struct device *dev)
 {
+	arm_iommu_detach_device(dev);
 	iommu_group_remove_device(dev);
 }
 
@@ -824,15 +797,13 @@ static const struct iommu_ops ipmmu_ops = {
 	.iotlb_sync = ipmmu_iotlb_sync,
 	.map_sg = default_iommu_map_sg,
 	.iova_to_phys = ipmmu_iova_to_phys,
-	.add_device = ipmmu_add_device_dma,
-	.remove_device = ipmmu_remove_device_dma,
+	.add_device = ipmmu_add_device,
+	.remove_device = ipmmu_remove_device,
 	.device_group = ipmmu_find_group,
 	.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
 	.of_xlate = ipmmu_of_xlate,
 };
 
-#endif /* CONFIG_IOMMU_DMA */
-
 /* -----------------------------------------------------------------------------
  * Probe/remove and init
  */
@@ -929,9 +900,7 @@ static int ipmmu_remove(struct platform_device *pdev)
 	iommu_device_sysfs_remove(&mmu->iommu);
 	iommu_device_unregister(&mmu->iommu);
 
-#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
 	arm_iommu_release_mapping(mmu->mapping);
-#endif
 
 	ipmmu_device_reset(mmu);
 
-- 
2.13.4.dirty

[PATCH 4/4] iommu/ipmmu-vmsa: Unify ipmmu_ops

[Robin Murphy]

The remaining difference between the ARM-specific and iommu-dma ops is
in the {add,remove}_device implementations, but even those have some
overlap and duplication. By stubbing out the few arm_iommu_*() calls,
we can get rid of the rest of the inline #ifdeffery to both simplify the
code and improve build coverage.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/ipmmu-vmsa.c | 69 +++++++++++++---------------------------------
 1 file changed, 19 insertions(+), 50 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 4d69c08145a1..2005aad09f2f 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -27,6 +27,11 @@
 #if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
 #include <asm/dma-iommu.h>
 #include <asm/pgalloc.h>
+#else
+#define arm_iommu_create_mapping(...)	NULL
+#define arm_iommu_attach_device(...)	-ENODEV
+#define arm_iommu_release_mapping(...)	do {} while (0)
+#define arm_iommu_detach_device(...)	do {} while (0)
 #endif
 
 #include "io-pgtable.h"
@@ -678,26 +683,17 @@ static int ipmmu_of_xlate(struct device *dev,
 	return ipmmu_init_platform_device(dev, spec);
 }
 
-#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
-
-static int ipmmu_add_device(struct device *dev)
+static int ipmmu_init_arm_mapping(struct device *dev)
 {
 	struct ipmmu_vmsa_device *mmu = to_ipmmu(dev);
 	struct iommu_group *group;
 	int ret;
 
-	/*
-	 * Only let through devices that have been verified in xlate()
-	 */
-	if (!mmu)
-		return -ENODEV;
-
 	/* Create a device group and add the device to it. */
 	group = iommu_group_alloc();
 	if (IS_ERR(group)) {
 		dev_err(dev, "Failed to allocate IOMMU group\n");
-		ret = PTR_ERR(group);
-		goto error;
+		return PTR_ERR(group);
 	}
 
 	ret = iommu_group_add_device(group, dev);
@@ -705,8 +701,7 @@ static int ipmmu_add_device(struct device *dev)
 
 	if (ret < 0) {
 		dev_err(dev, "Failed to add device to IPMMU group\n");
-		group = NULL;
-		goto error;
+		return ret;
 	}
 
 	/*
@@ -742,41 +737,14 @@ static int ipmmu_add_device(struct device *dev)
 	return 0;
 
 error:
-	if (mmu)
+	iommu_group_remove_device(dev);
+	if (mmu->mapping)
 		arm_iommu_release_mapping(mmu->mapping);
 
-	if (!IS_ERR_OR_NULL(group))
-		iommu_group_remove_device(dev);
-
 	return ret;
 }
 
-static void ipmmu_remove_device(struct device *dev)
-{
-	arm_iommu_detach_device(dev);
-	iommu_group_remove_device(dev);
-}
-
-static const struct iommu_ops ipmmu_ops = {
-	.domain_alloc = ipmmu_domain_alloc,
-	.domain_free = ipmmu_domain_free,
-	.attach_dev = ipmmu_attach_device,
-	.detach_dev = ipmmu_detach_device,
-	.map = ipmmu_map,
-	.unmap = ipmmu_unmap,
-	.map_sg = default_iommu_map_sg,
-	.iova_to_phys = ipmmu_iova_to_phys,
-	.add_device = ipmmu_add_device,
-	.remove_device = ipmmu_remove_device,
-	.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
-	.of_xlate = ipmmu_of_xlate,
-};
-
-#endif /* !CONFIG_ARM && CONFIG_IOMMU_DMA */
-
-#ifdef CONFIG_IOMMU_DMA
-
-static int ipmmu_add_device_dma(struct device *dev)
+static int ipmmu_add_device(struct device *dev)
 {
 	struct iommu_group *group;
 
@@ -786,15 +754,20 @@ static int ipmmu_add_device_dma(struct device *dev)
 	if (!to_ipmmu(dev))
 		return -ENODEV;
 
+	if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_IOMMU_DMA))
+		return ipmmu_init_arm_mapping(dev);
+
 	group = iommu_group_get_for_dev(dev);
 	if (IS_ERR(group))
 		return PTR_ERR(group);
 
+	iommu_group_put(group);
 	return 0;
 }
 
-static void ipmmu_remove_device_dma(struct device *dev)
+static void ipmmu_remove_device(struct device *dev)
 {
+	arm_iommu_detach_device(dev);
 	iommu_group_remove_device(dev);
 }
 
@@ -824,15 +797,13 @@ static const struct iommu_ops ipmmu_ops = {
 	.iotlb_sync = ipmmu_iotlb_sync,
 	.map_sg = default_iommu_map_sg,
 	.iova_to_phys = ipmmu_iova_to_phys,
-	.add_device = ipmmu_add_device_dma,
-	.remove_device = ipmmu_remove_device_dma,
+	.add_device = ipmmu_add_device,
+	.remove_device = ipmmu_remove_device,
 	.device_group = ipmmu_find_group,
 	.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
 	.of_xlate = ipmmu_of_xlate,
 };
 
-#endif /* CONFIG_IOMMU_DMA */
-
 /* -----------------------------------------------------------------------------
  * Probe/remove and init
  */
@@ -929,9 +900,7 @@ static int ipmmu_remove(struct platform_device *pdev)
 	iommu_device_sysfs_remove(&mmu->iommu);
 	iommu_device_unregister(&mmu->iommu);
 
-#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
 	arm_iommu_release_mapping(mmu->mapping);
-#endif
 
 	ipmmu_device_reset(mmu);
 
-- 
2.13.4.dirty

Re: [PATCH 0/4] ipmmu-vmsa cleanup

[Arnd Bergmann]

On Fri, Oct 13, 2017 at 8:23 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> Arnd reports a build warning[1] thanks to me missing ipmmu-vmsa's second
> set of ops when converting io-pgtable-arm users to the new iommu_iotlb_*
> callbacks. Rather than just treat the symptom with a point fix, this
> seemed like a good excuse to clean up the messy #ifdeffery and
> duplication in the driver that led to the oversight in the first place.
>
> Build-tested successfully for arm64, ARM, and ARM + CONFIG_IOMMU_DMA.

Very nice. I looked over the patches and while I only understood half of
what you do and why it was different to start with, I very much welcome
the end result.

       Arnd

Re: [PATCH 0/4] ipmmu-vmsa cleanup

[Arnd Bergmann]

On Fri, Oct 13, 2017 at 8:23 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
> Arnd reports a build warning[1] thanks to me missing ipmmu-vmsa's second
> set of ops when converting io-pgtable-arm users to the new iommu_iotlb_*
> callbacks. Rather than just treat the symptom with a point fix, this
> seemed like a good excuse to clean up the messy #ifdeffery and
> duplication in the driver that led to the oversight in the first place.
>
> Build-tested successfully for arm64, ARM, and ARM + CONFIG_IOMMU_DMA.

Very nice. I looked over the patches and while I only understood half of
what you do and why it was different to start with, I very much welcome
the end result.

       Arnd

[PATCH 0/4] ipmmu-vmsa cleanup

[Arnd Bergmann]

On Fri, Oct 13, 2017 at 8:23 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> Arnd reports a build warning[1] thanks to me missing ipmmu-vmsa's second
> set of ops when converting io-pgtable-arm users to the new iommu_iotlb_*
> callbacks. Rather than just treat the symptom with a point fix, this
> seemed like a good excuse to clean up the messy #ifdeffery and
> duplication in the driver that led to the oversight in the first place.
>
> Build-tested successfully for arm64, ARM, and ARM + CONFIG_IOMMU_DMA.

Very nice. I looked over the patches and while I only understood half of
what you do and why it was different to start with, I very much welcome
the end result.

       Arnd

Re: [PATCH 0/4] ipmmu-vmsa cleanup

[Alex Williamson]

On Fri, 13 Oct 2017 19:23:38 +0100
Robin Murphy <robin.murphy@arm.com> wrote:

> Arnd reports a build warning[1] thanks to me missing ipmmu-vmsa's second
> set of ops when converting io-pgtable-arm users to the new iommu_iotlb_*
> callbacks. Rather than just treat the symptom with a point fix, this
> seemed like a good excuse to clean up the messy #ifdeffery and
> duplication in the driver that led to the oversight in the first place.
> 
> Build-tested successfully for arm64, ARM, and ARM + CONFIG_IOMMU_DMA.
> 
> Robin.
> 
> [1]:https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1510423.html
> 
> Robin Murphy (4):
>   iommu/ipmmu-vmsa: Unify domain alloc/free
>   iommu/ipmmu-vmsa: Simplify group allocation
>   iommu/ipmmu-vmsa: Clean up struct ipmmu_vmsa_iommu_priv
>   iommu/ipmmu-vmsa: Unify ipmmu_ops
> 
>  drivers/iommu/ipmmu-vmsa.c | 251 ++++++++++++++-------------------------------
>  1 file changed, 78 insertions(+), 173 deletions(-)
> 

Applied to iommu/ipmmu-vmsa for v4.15.  Thanks,

Alex

[PATCH 0/4] ipmmu-vmsa cleanup

[Alex Williamson]

On Fri, 13 Oct 2017 19:23:38 +0100
Robin Murphy <robin.murphy@arm.com> wrote:

> Arnd reports a build warning[1] thanks to me missing ipmmu-vmsa's second
> set of ops when converting io-pgtable-arm users to the new iommu_iotlb_*
> callbacks. Rather than just treat the symptom with a point fix, this
> seemed like a good excuse to clean up the messy #ifdeffery and
> duplication in the driver that led to the oversight in the first place.
> 
> Build-tested successfully for arm64, ARM, and ARM + CONFIG_IOMMU_DMA.
> 
> Robin.
> 
> [1]:https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1510423.html
> 
> Robin Murphy (4):
>   iommu/ipmmu-vmsa: Unify domain alloc/free
>   iommu/ipmmu-vmsa: Simplify group allocation
>   iommu/ipmmu-vmsa: Clean up struct ipmmu_vmsa_iommu_priv
>   iommu/ipmmu-vmsa: Unify ipmmu_ops
> 
>  drivers/iommu/ipmmu-vmsa.c | 251 ++++++++++++++-------------------------------
>  1 file changed, 78 insertions(+), 173 deletions(-)
> 

Applied to iommu/ipmmu-vmsa for v4.15.  Thanks,

Alex