iommu/ipmmu-vmsa: 32-bit ARM update V2
[PATCH v2 01/05] iommu/ipmmu-vmsa: Use iommu_device_register()/unregister()
[PATCH v2 02/05] iommu/ipmmu-vmsa: Consistent ->of_xlate() handling
[PATCH v2 03/05] iommu/ipmmu-vmsa: Use fwspec on both 32 and 64-bit ARM
[PATCH v2 04/05] iommu/ipmmu-vmsa: Replace local utlb code with fwspec ids
[PATCH v2 05/05] iommu/ipmmu-vmsa: Clean up device tracking
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 <[email protected]> (Patch 2 and 4)
Signed-off-by: Robin Murphy <[email protected]> (Patch 3 and 5)
Signed-off-by: Magnus Damm <[email protected]>
---
Changes since V1:
- Minor changes to patch 1 and 2 - thanks Robin and Geert!
- Added patch 5 to include further clean ups
Developed on top of v4.13-rc1
drivers/iommu/ipmmu-vmsa.c | 198 ++++++++++----------------------------------
1 file changed, 49 insertions(+), 149 deletions(-)
From: Magnus Damm <[email protected]>
Extend the driver to make use of iommu_device_register()/unregister()
functions together with iommu_device_set_ops() and iommu_set_fwnode().
These used to be part of the earlier posted 64-bit ARM (r8a7795) series but
it turns out that these days they are required on 32-bit ARM as well.
Signed-off-by: Magnus Damm <[email protected]>
---
Changes since V1:
- Perform iommu_device_set_ops() and iommu_device_set_fwnode() before
iommu_device_register() - thanks Robin!
drivers/iommu/ipmmu-vmsa.c | 10 ++++++++++
1 file changed, 10 insertions(+)
--- 0001/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2017-07-17 21:01:47.140607110 +0900
@@ -35,6 +35,7 @@
struct ipmmu_vmsa_device {
struct device *dev;
void __iomem *base;
+ struct iommu_device iommu;
struct list_head list;
unsigned int num_utlbs;
@@ -1054,6 +1055,13 @@ static int ipmmu_probe(struct platform_d
ipmmu_device_reset(mmu);
+ iommu_device_set_ops(&mmu->iommu, &ipmmu_ops);
+ iommu_device_set_fwnode(&mmu->iommu, &pdev->dev.of_node->fwnode);
+
+ ret = iommu_device_register(&mmu->iommu);
+ if (ret)
+ return ret;
+
/*
* We can't create the ARM mapping here as it requires the bus to have
* an IOMMU, which only happens when bus_set_iommu() is called in
@@ -1077,6 +1085,8 @@ static int ipmmu_remove(struct platform_
list_del(&mmu->list);
spin_unlock(&ipmmu_devices_lock);
+ iommu_device_unregister(&mmu->iommu);
+
#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
arm_iommu_release_mapping(mmu->mapping);
#endif
From: Magnus Damm <[email protected]>
The 32-bit ARM code gets updated to make use of ->of_xlate() and the
code is shared between 64-bit and 32-bit ARM. The of_device_is_available()
check gets dropped since it is included in of_iommu_xlate().
Suggested-by: Robin Murphy <[email protected]>
Signed-off-by: Magnus Damm <[email protected]>
---
Changes since V1:
- Moved "Initialize once" check to ipmmu_of_xlate() - thanks Geert!
drivers/iommu/ipmmu-vmsa.c | 51 ++++++++++++++------------------------------
1 file changed, 17 insertions(+), 34 deletions(-)
--- 0002/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2017-07-17 21:03:38.340607110 +0900
@@ -734,6 +734,16 @@ error:
return ret;
}
+static int ipmmu_of_xlate(struct device *dev,
+ struct of_phandle_args *spec)
+{
+ /* Initialize once - xlate() will call multiple times */
+ if (to_priv(dev))
+ return 0;
+
+ return ipmmu_init_platform_device(dev);
+}
+
#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
@@ -750,11 +760,11 @@ static int ipmmu_add_device(struct devic
struct iommu_group *group;
int ret;
- if (to_priv(dev)) {
- dev_warn(dev, "IOMMU driver already assigned to device %s\n",
- dev_name(dev));
- return -EINVAL;
- }
+ /*
+ * Only let through devices that have been verified in xlate()
+ */
+ if (!to_priv(dev))
+ return -ENODEV;
/* Create a device group and add the device to it. */
group = iommu_group_alloc();
@@ -773,10 +783,6 @@ static int ipmmu_add_device(struct devic
goto error;
}
- ret = ipmmu_init_platform_device(dev);
- if (ret < 0)
- goto error;
-
/*
* Create the ARM mapping, used by the ARM DMA mapping core to allocate
* VAs. This will allocate a corresponding IOMMU domain.
@@ -817,24 +823,13 @@ error:
if (!IS_ERR_OR_NULL(group))
iommu_group_remove_device(dev);
- kfree(to_priv(dev)->utlbs);
- kfree(to_priv(dev));
- set_priv(dev, NULL);
-
return ret;
}
static void ipmmu_remove_device(struct device *dev)
{
- struct ipmmu_vmsa_iommu_priv *priv = to_priv(dev);
-
arm_iommu_detach_device(dev);
iommu_group_remove_device(dev);
-
- kfree(priv->utlbs);
- kfree(priv);
-
- set_priv(dev, NULL);
}
static const struct iommu_ops ipmmu_ops = {
@@ -849,6 +844,7 @@ static const struct iommu_ops ipmmu_ops
.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 */
@@ -958,19 +954,6 @@ static struct iommu_group *ipmmu_find_gr
return group;
}
-static int ipmmu_of_xlate_dma(struct device *dev,
- struct of_phandle_args *spec)
-{
- /* If the IPMMU device is disabled in DT then return error
- * to make sure the of_iommu code does not install ops
- * even though the iommu device is disabled
- */
- if (!of_device_is_available(spec->np))
- return -ENODEV;
-
- return ipmmu_init_platform_device(dev);
-}
-
static const struct iommu_ops ipmmu_ops = {
.domain_alloc = ipmmu_domain_alloc_dma,
.domain_free = ipmmu_domain_free_dma,
@@ -984,7 +967,7 @@ static const struct iommu_ops ipmmu_ops
.remove_device = ipmmu_remove_device_dma,
.device_group = ipmmu_find_group_dma,
.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
- .of_xlate = ipmmu_of_xlate_dma,
+ .of_xlate = ipmmu_of_xlate,
};
#endif /* CONFIG_IOMMU_DMA */
From: Robin Murphy <[email protected]>
Consolidate the 32-bit and 64-bit code to make use of fwspec instead
of archdata for the 32-bit ARM case.
This is a simplified version of the fwspec handling code from Robin
posted as [PATCH] iommu/ipmmu-vmsa: Convert to iommu_fwspec
Signed-off-by: Robin Murphy <[email protected]>
Signed-off-by: Magnus Damm <[email protected]>
---
Changes since V1:
- Rebased to apply on top of earlier changes in series
drivers/iommu/ipmmu-vmsa.c | 21 +++------------------
1 file changed, 3 insertions(+), 18 deletions(-)
--- 0004/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2017-07-17 21:04:54.850607110 +0900
@@ -73,22 +73,9 @@ static struct ipmmu_vmsa_domain *to_vmsa
return container_of(dom, struct ipmmu_vmsa_domain, io_domain);
}
-
static struct ipmmu_vmsa_iommu_priv *to_priv(struct device *dev)
{
-#if defined(CONFIG_ARM)
- return dev->archdata.iommu;
-#else
- return dev->iommu_fwspec->iommu_priv;
-#endif
-}
-static void set_priv(struct device *dev, struct ipmmu_vmsa_iommu_priv *p)
-{
-#if defined(CONFIG_ARM)
- dev->archdata.iommu = p;
-#else
- dev->iommu_fwspec->iommu_priv = p;
-#endif
+ return dev->iommu_fwspec ? dev->iommu_fwspec->iommu_priv : NULL;
}
#define TLB_LOOP_TIMEOUT 100 /* 100us */
@@ -726,7 +713,7 @@ static int ipmmu_init_platform_device(st
priv->utlbs = utlbs;
priv->num_utlbs = num_utlbs;
priv->dev = dev;
- set_priv(dev, priv);
+ dev->iommu_fwspec->iommu_priv = priv;
return 0;
error:
@@ -887,14 +874,12 @@ static void ipmmu_domain_free_dma(struct
static int ipmmu_add_device_dma(struct device *dev)
{
- struct iommu_fwspec *fwspec = dev->iommu_fwspec;
struct iommu_group *group;
/*
* Only let through devices that have been verified in xlate()
- * We may get called with dev->iommu_fwspec set to NULL.
*/
- if (!fwspec || !fwspec->iommu_priv)
+ if (!to_priv(dev))
return -ENODEV;
group = iommu_group_get_for_dev(dev);
From: Magnus Damm <[email protected]>
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 <[email protected]>
Signed-off-by: Magnus Damm <[email protected]>
---
Changes since V1:
- Rebased to apply on top of earlier changes in series
drivers/iommu/ipmmu-vmsa.c | 104 ++++++++------------------------------------
1 file changed, 19 insertions(+), 85 deletions(-)
--- 0006/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2017-07-17 21:12:11.650607110 +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;
-
- /* 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)
{
+ iommu_fwspec_add_ids(dev, spec->args, 1);
+
/* Initialize once - xlate() will call multiple times */
if (to_priv(dev))
return 0;
- return ipmmu_init_platform_device(dev);
+ return ipmmu_init_platform_device(dev, spec);
}
#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
From: Robin Murphy <[email protected]>
Get rid of now unused device tracking code. Future code should instead
be able to use driver_for_each_device() for this purpose.
This is a simplified version of the following patch from Robin
[PATCH] iommu/ipmmu-vmsa: Clean up group allocation
Signed-off-by: Robin Murphy <[email protected]>
Signed-off-by: Magnus Damm <[email protected]>
---
Change since V1:
- New patch
drivers/iommu/ipmmu-vmsa.c | 12 ------------
1 file changed, 12 deletions(-)
--- 0008/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2017-07-17 21:35:26.690607110 +0900
@@ -37,7 +37,6 @@ struct ipmmu_vmsa_device {
struct device *dev;
void __iomem *base;
struct iommu_device iommu;
- struct list_head list;
unsigned int num_utlbs;
spinlock_t lock; /* Protects ctx and domains[] */
@@ -64,9 +63,6 @@ struct ipmmu_vmsa_iommu_priv {
struct list_head list;
};
-static DEFINE_SPINLOCK(ipmmu_devices_lock);
-static LIST_HEAD(ipmmu_devices);
-
static struct ipmmu_vmsa_domain *to_vmsa_domain(struct iommu_domain *dom)
{
return container_of(dom, struct ipmmu_vmsa_domain, io_domain);
@@ -970,10 +966,6 @@ static int ipmmu_probe(struct platform_d
* ipmmu_init() after the probe function returns.
*/
- spin_lock(&ipmmu_devices_lock);
- list_add(&mmu->list, &ipmmu_devices);
- spin_unlock(&ipmmu_devices_lock);
-
platform_set_drvdata(pdev, mmu);
return 0;
@@ -983,10 +975,6 @@ static int ipmmu_remove(struct platform_
{
struct ipmmu_vmsa_device *mmu = platform_get_drvdata(pdev);
- spin_lock(&ipmmu_devices_lock);
- list_del(&mmu->list);
- spin_unlock(&ipmmu_devices_lock);
-
iommu_device_unregister(&mmu->iommu);
#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
On Mon, Jul 17, 2017 at 10:05:10PM +0900, Magnus Damm wrote:
> --- 0001/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c 2017-07-17 21:01:47.140607110 +0900
> @@ -35,6 +35,7 @@
> struct ipmmu_vmsa_device {
> struct device *dev;
> void __iomem *base;
> + struct iommu_device iommu;
> struct list_head list;
>
> unsigned int num_utlbs;
> @@ -1054,6 +1055,13 @@ static int ipmmu_probe(struct platform_d
>
> ipmmu_device_reset(mmu);
>
> + iommu_device_set_ops(&mmu->iommu, &ipmmu_ops);
> + iommu_device_set_fwnode(&mmu->iommu, &pdev->dev.of_node->fwnode);
> +
> + ret = iommu_device_register(&mmu->iommu);
> + if (ret)
> + return ret;
> +
Looks good so far, why don't you also add the iommu to sysfs with
iommu_device_sysfs_add()?
Joerg