Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1666633 > unrolled thread

[PATCH 00/04] iommu/ipmmu-vmsa: 32-bit ARM update

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/04] iommu/ipmmu-vmsa: 32-bit ARM update Magnus Damm <magnus.damm@gmail.com> - 2017-06-15 12:40 +0200
    [PATCH 04/04] iommu/ipmmu-vmsa: Replace local utlb code with fwspec ids Magnus Damm <magnus.damm@gmail.com> - 2017-06-15 12:40 +0200
      Re: [PATCH 04/04] iommu/ipmmu-vmsa: Replace local utlb code with  fwspec ids Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-16 09:20 +0200
        Re: [PATCH 04/04] iommu/ipmmu-vmsa: Replace local utlb code with  fwspec ids Magnus Damm <magnus.damm@gmail.com> - 2017-06-16 12:20 +0200
    Re: [PATCH 00/04] iommu/ipmmu-vmsa: 32-bit ARM update Robin Murphy <robin.murphy@arm.com> - 2017-06-19 18:30 +0200

#1666633 — [PATCH 00/04] iommu/ipmmu-vmsa: 32-bit ARM update

iommu/ipmmu-vmsa: 32-bit ARM update

[PATCH 01/04] iommu/ipmmu-vmsa: Use iommu_device_register()/unregister()
[PATCH 02/04] iommu/ipmmu-vmsa: Consistent ->of_xlate() handling
[PATCH 03/04] iommu/ipmmu-vmsa: Use fwspec on both 32 and 64-bit ARM
[PATCH 04/04] iommu/ipmmu-vmsa: Replace local utlb code with fwspec ids

This series updates the IPMMU driver to make use of recent IOMMU framework
changes and also improves code sharing in the driver between the 32-bit and
64-bit dma-mapping architecture glue code.

Suggested-by: Robin Murphy <robin.murphy@arm.com> (Patch 2 and 4)
Signed-off-by: Robin Murphy <robin.murphy@arm.com> (Patch 3)
Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

 Developed on renesas-drivers-2017-06-13-v4.12-rc5 and rebased to next-20170614

 drivers/iommu/ipmmu-vmsa.c |  186 +++++++++++---------------------------------
 1 file changed, 49 insertions(+), 137 deletions(-)

[toc] | [next] | [standalone]

#1666634 — [PATCH 04/04] iommu/ipmmu-vmsa: Replace local utlb code with fwspec ids

From: Magnus Damm <damm+renesas@opensource.se>

Now when both 32-bit and 64-bit code inside the driver is using
fwspec it is possible to replace the utlb handling with fwspec ids
that get populated from ->of_xlate().

Suggested-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

 drivers/iommu/ipmmu-vmsa.c |  104 ++++++++------------------------------------
 1 file changed, 19 insertions(+), 85 deletions(-)

--- 0013/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c	2017-06-15 18:32:27.580607110 +0900
@@ -19,6 +19,7 @@
 #include <linux/iommu.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
@@ -59,8 +60,6 @@ struct ipmmu_vmsa_domain {
 
 struct ipmmu_vmsa_iommu_priv {
 	struct ipmmu_vmsa_device *mmu;
-	unsigned int *utlbs;
-	unsigned int num_utlbs;
 	struct device *dev;
 	struct list_head list;
 };
@@ -550,13 +549,14 @@ static int ipmmu_attach_device(struct io
 			       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_domain *domain = to_vmsa_domain(io_domain);
 	unsigned long flags;
 	unsigned int i;
 	int ret = 0;
 
-	if (!mmu) {
+	if (!priv || !priv->mmu) {
 		dev_err(dev, "Cannot attach to IPMMU\n");
 		return -ENXIO;
 	}
@@ -583,8 +583,8 @@ static int ipmmu_attach_device(struct io
 	if (ret < 0)
 		return ret;
 
-	for (i = 0; i < priv->num_utlbs; ++i)
-		ipmmu_utlb_enable(domain, priv->utlbs[i]);
+	for (i = 0; i < fwspec->num_ids; ++i)
+		ipmmu_utlb_enable(domain, fwspec->ids[i]);
 
 	return 0;
 }
@@ -592,12 +592,12 @@ static int ipmmu_attach_device(struct io
 static void ipmmu_detach_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_domain *domain = to_vmsa_domain(io_domain);
 	unsigned int i;
 
-	for (i = 0; i < priv->num_utlbs; ++i)
-		ipmmu_utlb_disable(domain, priv->utlbs[i]);
+	for (i = 0; i < fwspec->num_ids; ++i)
+		ipmmu_utlb_disable(domain, fwspec->ids[i]);
 
 	/*
 	 * TODO: Optimize by disabling the context when no device is attached.
@@ -633,102 +633,36 @@ static phys_addr_t ipmmu_iova_to_phys(st
 	return domain->iop->iova_to_phys(domain->iop, iova);
 }
 
-static int ipmmu_find_utlbs(struct ipmmu_vmsa_device *mmu, struct device *dev,
-			    unsigned int *utlbs, unsigned int num_utlbs)
-{
-	unsigned int i;
-
-	for (i = 0; i < num_utlbs; ++i) {
-		struct of_phandle_args args;
-		int ret;
-
-		ret = of_parse_phandle_with_args(dev->of_node, "iommus",
-						 "#iommu-cells", i, &args);
-		if (ret < 0)
-			return ret;
-
-		of_node_put(args.np);
-
-		if (args.np != mmu->dev->of_node || args.args_count != 1)
-			return -EINVAL;
-
-		utlbs[i] = args.args[0];
-	}
-
-	return 0;
-}
-
-static int ipmmu_init_platform_device(struct device *dev)
+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;
-	struct ipmmu_vmsa_device *mmu;
-	unsigned int *utlbs;
-	unsigned int i;
-	int num_utlbs;
-	int ret = -ENODEV;
 
 	/* Initialize once - xlate() will call multiple times */
 	if (to_priv(dev))
 		return 0;
 
-	/* Find the master corresponding to the device. */
-
-	num_utlbs = of_count_phandle_with_args(dev->of_node, "iommus",
-					       "#iommu-cells");
-	if (num_utlbs < 0)
+	ipmmu_pdev = of_find_device_by_node(args->np);
+	if (!ipmmu_pdev)
 		return -ENODEV;
 
-	utlbs = kcalloc(num_utlbs, sizeof(*utlbs), GFP_KERNEL);
-	if (!utlbs)
-		return -ENOMEM;
-
-	spin_lock(&ipmmu_devices_lock);
-
-	list_for_each_entry(mmu, &ipmmu_devices, list) {
-		ret = ipmmu_find_utlbs(mmu, dev, utlbs, num_utlbs);
-		if (!ret) {
-			/*
-			 * TODO Take a reference to the MMU to protect
-			 * against device removal.
-			 */
-			break;
-		}
-	}
-
-	spin_unlock(&ipmmu_devices_lock);
-
-	if (ret < 0)
-		goto error;
-
-	for (i = 0; i < num_utlbs; ++i) {
-		if (utlbs[i] >= mmu->num_utlbs) {
-			ret = -EINVAL;
-			goto error;
-		}
-	}
-
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv) {
-		ret = -ENOMEM;
-		goto error;
-	}
+	if (!priv)
+		return -ENOMEM;
 
-	priv->mmu = mmu;
-	priv->utlbs = utlbs;
-	priv->num_utlbs = num_utlbs;
+	priv->mmu = platform_get_drvdata(ipmmu_pdev);
 	priv->dev = dev;
 	dev->iommu_fwspec->iommu_priv = priv;
 	return 0;
-
-error:
-	kfree(utlbs);
-	return ret;
 }
 
 static int ipmmu_of_xlate(struct device *dev,
 			  struct of_phandle_args *spec)
 {
-	return ipmmu_init_platform_device(dev);
+	iommu_fwspec_add_ids(dev, spec->args, 1);
+
+	return ipmmu_init_platform_device(dev, spec);
 }
 
 #if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)

[toc] | [prev] | [next] | [standalone]

#1667482 — Re: [PATCH 04/04] iommu/ipmmu-vmsa: Replace local utlb code with fwspec ids

Hi Magnus,

On Thu, Jun 15, 2017 at 12:29 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
> Now when both 32-bit and 64-bit code inside the driver is using
> fwspec it is possible to replace the utlb handling with fwspec ids
> that get populated from ->of_xlate().

Thanks for your patch!

> --- 0013/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c     2017-06-15 18:32:27.580607110 +0900

>  static int ipmmu_of_xlate(struct device *dev,
>                           struct of_phandle_args *spec)
>  {
> -       return ipmmu_init_platform_device(dev);
> +       iommu_fwspec_add_ids(dev, spec->args, 1);

Does it hurt if iommu_fwspec_add_ids() is called multiple times, as this is
done before the "Initialize once - xlate() will call multiple times" check?

> +
> +       return ipmmu_init_platform_device(dev, spec);
>  }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1667613 — Re: [PATCH 04/04] iommu/ipmmu-vmsa: Replace local utlb code with fwspec ids

Hi Geert,

On Fri, Jun 16, 2017 at 4:18 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Magnus,
>
> On Thu, Jun 15, 2017 at 12:29 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> Now when both 32-bit and 64-bit code inside the driver is using
>> fwspec it is possible to replace the utlb handling with fwspec ids
>> that get populated from ->of_xlate().
>
> Thanks for your patch!

Thanks for the feedback!

>> --- 0013/drivers/iommu/ipmmu-vmsa.c
>> +++ work/drivers/iommu/ipmmu-vmsa.c     2017-06-15 18:32:27.580607110 +0900
>
>>  static int ipmmu_of_xlate(struct device *dev,
>>                           struct of_phandle_args *spec)
>>  {
>> -       return ipmmu_init_platform_device(dev);
>> +       iommu_fwspec_add_ids(dev, spec->args, 1);
>
> Does it hurt if iommu_fwspec_add_ids() is called multiple times, as this is
> done before the "Initialize once - xlate() will call multiple times" check?

The function needs to be called several times to populate the ids, so
that the "initialize once" check happens later is intentional and
correct. Perhaps a bit unclear though...

>> +
>> +       return ipmmu_init_platform_device(dev, spec);
>>  }

Cheers,

/ magnus

[toc] | [prev] | [next] | [standalone]

#1669456

Hi Magnus,

On 15/06/17 11:29, Magnus Damm wrote:
> iommu/ipmmu-vmsa: 32-bit ARM update
> 
> [PATCH 01/04] iommu/ipmmu-vmsa: Use iommu_device_register()/unregister()
> [PATCH 02/04] iommu/ipmmu-vmsa: Consistent ->of_xlate() handling
> [PATCH 03/04] iommu/ipmmu-vmsa: Use fwspec on both 32 and 64-bit ARM
> [PATCH 04/04] iommu/ipmmu-vmsa: Replace local utlb code with fwspec ids
> 
> This series updates the IPMMU driver to make use of recent IOMMU framework
> changes and also improves code sharing in the driver between the 32-bit and
> 64-bit dma-mapping architecture glue code.

Nice! I did try rebasing my original patch myself but it quickly got
very messy. I also had the below beforehand as preliminary cleanup for
getting rid of the ipmmu_vmsa_iommu_priv structure altogether (i.e. so
fwspec->priv can just hold the ipmmu_vmsa_device pointer directly) - if
it does actually work as intended, feel free to take it ;)

Thanks,
Robin.

> Suggested-by: Robin Murphy <robin.murphy@arm.com> (Patch 2 and 4)
> Signed-off-by: Robin Murphy <robin.murphy@arm.com> (Patch 3)
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
> 
>  Developed on renesas-drivers-2017-06-13-v4.12-rc5 and rebased to next-20170614
> 
>  drivers/iommu/ipmmu-vmsa.c |  186 +++++++++++---------------------------------
>  1 file changed, 49 insertions(+), 137 deletions(-)
> 

---->8-----
From: Robin Murphy <robin.murphy@arm.com>
Subject: [PATCH] iommu/ipmmu-vmsa: Clean up group allocation

With the new IOMMU group support, 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.

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

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 2a38aa15be17..9d75d7553e20 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -43,6 +43,7 @@ struct ipmmu_vmsa_device {
 	struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];

 	struct dma_iommu_mapping *mapping;
+	struct iommu_group *group;
 };

 struct ipmmu_vmsa_domain {
@@ -60,8 +61,6 @@ struct ipmmu_vmsa_iommu_priv {
 	struct ipmmu_vmsa_device *mmu;
 	unsigned int *utlbs;
 	unsigned int num_utlbs;
-	struct device *dev;
-	struct list_head list;
 };

 static DEFINE_SPINLOCK(ipmmu_devices_lock);
@@ -724,7 +723,6 @@ static int ipmmu_init_platform_device(struct device
*dev)
 	priv->mmu = mmu;
 	priv->utlbs = utlbs;
 	priv->num_utlbs = num_utlbs;
-	priv->dev = dev;
 	set_priv(dev, priv);
 	return 0;

@@ -854,9 +852,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 struct iommu_domain *ipmmu_domain_alloc_dma(unsigned type)
 {
 	struct iommu_domain *io_domain = NULL;
@@ -904,57 +899,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)
-{
-	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;
+	struct ipmmu_vmsa_iommu_priv *priv = to_priv(dev);
+	struct ipmmu_vmsa_device *mmu = priv->mmu;

-	sibling = ipmmu_find_sibling_device(dev);
-	if (sibling)
-		group = iommu_group_get(sibling);
-	if (!sibling || IS_ERR(group))
-		group = generic_device_group(dev);
+	if (mmu->group)
+		return iommu_group_ref_get(mmu->group);

-	return group;
+	mmu->group = generic_device_group(dev);
+
+	return mmu->group;
 }

 static int ipmmu_of_xlate_dma(struct device *dev,

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web