iommu/ipmmu-vmsa: IPMMU CONFIG_IOMMU_DMA update
[PATCH 01/04] iommu/ipmmu-vmsa: 32-bit ARM may have CONFIG_IOMMU_DMA=y
[PATCH 02/04] iommu/ipmmu-vmsa: Break out utlb parsing code
[PATCH 03/04] iommu/ipmmu-vmsa: Break out domain allocation code
[PATCH 04/04] iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops
This series updates the IPMMU driver with CONFIG_IOMMU_DMA=y support
which is present in mainline for 64-bit ARM while experimental support
has been posted for 32-bit ARM thanks to Marek Szyprowski:
[RFC 0/3] Unify IOMMU-based DMA-mapping code for ARM and ARM64
Most of the patches in this series based on code earlier included in
the series below but has been reworked to also fit on 32-bit ARM:
[PATCH/RFC 00/10] iommu/ipmmu-vmsa: Experimental r8a7795 support
Signed-off-by: Magnus Damm <[email protected]>
---
Built on top of next-20160315
Depends on [PATCH v2 00/04] iommu/ipmmu-vmsa: IPMMU multi-arch update V2
drivers/iommu/ipmmu-vmsa.c | 238 ++++++++++++++++++++++++++++++++------------
1 file changed, 174 insertions(+), 64 deletions(-)
From: Magnus Damm <[email protected]>
Instead of assuming that CONFIG_ARM=y also means CONFIG_IOMMU_DMA=n,
convert the #ifdefs to take CONFIG_IOMMU_DMA into consideration
so 32-bit ARM can make use of CONFIG_IOMMU_DMA=y as well once those
bits are in place.
Signed-off-by: Magnus Damm <[email protected]>
---
drivers/iommu/ipmmu-vmsa.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
--- 0007/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-03-15 21:05:40.590513000 +0900
@@ -22,7 +22,7 @@
#include <linux/sizes.h>
#include <linux/slab.h>
-#ifdef CONFIG_ARM
+#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
#include <asm/dma-iommu.h>
#include <asm/pgalloc.h>
#endif
@@ -40,7 +40,7 @@ struct ipmmu_vmsa_device {
DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
-#ifdef CONFIG_ARM
+#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
struct dma_iommu_mapping *mapping;
#endif
};
@@ -619,7 +619,7 @@ static int ipmmu_find_utlbs(struct ipmmu
return 0;
}
-#ifdef CONFIG_ARM
+#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
static int ipmmu_map_attach(struct device *dev, struct ipmmu_vmsa_device *mmu)
{
int ret;
From: Magnus Damm <[email protected]>
Break out the utlb parsing code and dev_data allocation into a
separate function. This is preparation for future code sharing.
Signed-off-by: Magnus Damm <[email protected]>
---
drivers/iommu/ipmmu-vmsa.c | 125 ++++++++++++++++++++++++--------------------
1 file changed, 70 insertions(+), 55 deletions(-)
--- 0008/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-03-15 21:08:20.180513000 +0900
@@ -619,6 +619,69 @@ static int ipmmu_find_utlbs(struct ipmmu
return 0;
}
+static int ipmmu_init_platform_device(struct device *dev,
+ struct iommu_group *group)
+{
+ struct ipmmu_vmsa_archdata *archdata;
+ 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)
+ 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;
+ }
+ }
+
+ archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
+ if (!archdata) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ archdata->mmu = mmu;
+ archdata->utlbs = utlbs;
+ archdata->num_utlbs = num_utlbs;
+ dev->archdata.iommu = archdata;
+ return 0;
+
+error:
+ kfree(utlbs);
+ return ret;
+}
+
#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
static int ipmmu_map_attach(struct device *dev, struct ipmmu_vmsa_device *mmu)
{
@@ -675,13 +738,8 @@ static inline void ipmmu_release_mapping
static int ipmmu_add_device(struct device *dev)
{
- struct ipmmu_vmsa_archdata *archdata;
- struct ipmmu_vmsa_device *mmu;
- struct iommu_group *group = NULL;
- unsigned int *utlbs;
- unsigned int i;
- int num_utlbs;
- int ret = -ENODEV;
+ struct iommu_group *group;
+ int ret;
if (dev->archdata.iommu) {
dev_warn(dev, "IOMMU driver already assigned to device %s\n",
@@ -689,42 +747,6 @@ static int ipmmu_add_device(struct devic
return -EINVAL;
}
- /* Find the master corresponding to the device. */
-
- num_utlbs = of_count_phandle_with_args(dev->of_node, "iommus",
- "#iommu-cells");
- if (num_utlbs < 0)
- 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)
- return -ENODEV;
-
- for (i = 0; i < num_utlbs; ++i) {
- if (utlbs[i] >= mmu->num_utlbs) {
- ret = -EINVAL;
- goto error;
- }
- }
-
/* Create a device group and add the device to it. */
group = iommu_group_alloc();
if (IS_ERR(group)) {
@@ -742,27 +764,20 @@ static int ipmmu_add_device(struct devic
goto error;
}
- archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
- if (!archdata) {
- ret = -ENOMEM;
+ ret = ipmmu_init_platform_device(dev, group);
+ if (ret < 0) {
+ dev_err(dev, "Failed to init platform device\n");
goto error;
}
- archdata->mmu = mmu;
- archdata->utlbs = utlbs;
- archdata->num_utlbs = num_utlbs;
- dev->archdata.iommu = archdata;
-
- ret = ipmmu_map_attach(dev, mmu);
+ ret = ipmmu_map_attach(dev, ((struct ipmmu_vmsa_archdata *)
+ dev->archdata.iommu)->mmu);
if (ret < 0)
goto error;
return 0;
error:
- kfree(dev->archdata.iommu);
- kfree(utlbs);
-
dev->archdata.iommu = NULL;
if (!IS_ERR_OR_NULL(group))
From: Magnus Damm <[email protected]>
Break out the domain allocation code into a separate function.
This is preparation for future code sharing.
Signed-off-by: Magnus Damm <[email protected]>
---
drivers/iommu/ipmmu-vmsa.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
--- 0014/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-14 19:23:22.210513000 +0900
@@ -457,13 +457,10 @@ static irqreturn_t ipmmu_domain_irq(int
* IOMMU Operations
*/
-static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
+static struct iommu_domain *__ipmmu_domain_alloc(unsigned type)
{
struct ipmmu_vmsa_domain *domain;
- if (type != IOMMU_DOMAIN_UNMANAGED)
- return NULL;
-
domain = kzalloc(sizeof(*domain), GFP_KERNEL);
if (!domain)
return NULL;
@@ -473,6 +470,14 @@ static struct iommu_domain *ipmmu_domain
return &domain->io_domain;
}
+static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
+{
+ if (type != IOMMU_DOMAIN_UNMANAGED)
+ return NULL;
+
+ return __ipmmu_domain_alloc(type);
+}
+
static void ipmmu_domain_free(struct iommu_domain *io_domain)
{
struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
From: Magnus Damm <[email protected]>
Introduce a new set of iommu_ops suitable for 64-bit ARM
as well as 32-bit ARM when CONFIG_IOMMU_DMA=y. The ->of_xlate()
callback is needed by the code exported by of_iommu.h and
it is wrapped in #ifdefs to also compile of x86_64.
Signed-off-by: Magnus Damm <[email protected]>
---
drivers/iommu/ipmmu-vmsa.c | 94 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 92 insertions(+), 2 deletions(-)
--- 0011/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-03-16 01:35:06.990513000 +0900
@@ -10,6 +10,7 @@
#include <linux/bitmap.h>
#include <linux/delay.h>
+#include <linux/dma-iommu.h>
#include <linux/dma-mapping.h>
#include <linux/err.h>
#include <linux/export.h>
@@ -804,7 +805,7 @@ static void ipmmu_remove_device(struct d
dev->archdata.iommu = NULL;
}
-static const struct iommu_ops ipmmu_ops = {
+static const struct iommu_ops __maybe_unused ipmmu_ops = {
.domain_alloc = ipmmu_domain_alloc,
.domain_free = ipmmu_domain_free,
.attach_dev = ipmmu_attach_device,
@@ -818,6 +819,92 @@ static const struct iommu_ops ipmmu_ops
.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
};
+static struct iommu_domain *ipmmu_domain_alloc_dma(unsigned type)
+{
+ struct iommu_domain *io_domain;
+
+ if (type != IOMMU_DOMAIN_DMA)
+ return NULL;
+
+ io_domain = __ipmmu_domain_alloc(type);
+ if (io_domain)
+ iommu_get_dma_cookie(io_domain);
+
+ return io_domain;
+}
+
+static void ipmmu_domain_free_dma(struct iommu_domain *io_domain)
+{
+ iommu_put_dma_cookie(io_domain);
+ ipmmu_domain_free(io_domain);
+}
+
+static int ipmmu_add_device_dma(struct device *dev)
+{
+ struct iommu_group *group;
+
+ /* only accept devices with iommus property */
+ if (of_count_phandle_with_args(dev->of_node, "iommus",
+ "#iommu-cells") < 0)
+ return -ENODEV;
+
+ group = iommu_group_get_for_dev(dev);
+ if (IS_ERR(group))
+ return PTR_ERR(group);
+
+ return 0;
+}
+
+static void ipmmu_remove_device_dma(struct device *dev)
+{
+ iommu_group_remove_device(dev);
+}
+
+static struct iommu_group *ipmmu_device_group_dma(struct device *dev)
+{
+ struct iommu_group *group;
+ int ret;
+
+ group = generic_device_group(dev);
+ if (IS_ERR(group))
+ return group;
+
+ ret = ipmmu_init_platform_device(dev, group);
+ if (ret) {
+ iommu_group_put(group);
+ group = ERR_PTR(ret);
+ }
+
+ return group;
+}
+
+#ifdef CONFIG_OF_IOMMU
+static int ipmmu_of_xlate_dma(struct device *dev,
+ struct of_phandle_args *spec)
+{
+ /* dummy callback to satisfy of_iommu_configure() */
+ return 0;
+}
+#endif
+
+static const struct iommu_ops __maybe_unused ipmmu_ops_dma = {
+ .domain_alloc = ipmmu_domain_alloc_dma,
+ .domain_free = ipmmu_domain_free_dma,
+ .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_dma,
+ .remove_device = ipmmu_remove_device_dma,
+ .device_group = ipmmu_device_group_dma,
+ .pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
+#ifdef CONFIG_OF_IOMMU
+ .of_xlate = ipmmu_of_xlate_dma,
+#endif
+};
+
/* -----------------------------------------------------------------------------
* Probe/remove and init
*/
@@ -929,14 +1016,17 @@ static struct platform_driver ipmmu_driv
static int __init ipmmu_init(void)
{
+ const struct iommu_ops *ops;
int ret;
ret = platform_driver_register(&ipmmu_driver);
if (ret < 0)
return ret;
+ ops = IS_ENABLED(CONFIG_IOMMU_DMA) ? &ipmmu_ops_dma : &ipmmu_ops;
+
if (!iommu_present(&platform_bus_type))
- bus_set_iommu(&platform_bus_type, &ipmmu_ops);
+ bus_set_iommu(&platform_bus_type, ops);
return 0;
}
Hi Magnus,
Thank you for the patch.
On Wednesday 16 March 2016 02:05:10 Magnus Damm wrote:
> From: Magnus Damm <[email protected]>
>
> Introduce a new set of iommu_ops suitable for 64-bit ARM
> as well as 32-bit ARM when CONFIG_IOMMU_DMA=y. The ->of_xlate()
> callback is needed by the code exported by of_iommu.h and
> it is wrapped in #ifdefs to also compile of x86_64.
>
> Signed-off-by: Magnus Damm <[email protected]>
Nice work, thanks a lot. With the two comments below addressed,
Reviewed-by: Laurent Pinchart <[email protected]>
> ---
>
> drivers/iommu/ipmmu-vmsa.c | 94 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 92 insertions(+), 2 deletions(-)
>
> --- 0011/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c 2016-03-16 01:35:06.990513000 +0900
> @@ -10,6 +10,7 @@
>
> #include <linux/bitmap.h>
> #include <linux/delay.h>
> +#include <linux/dma-iommu.h>
> #include <linux/dma-mapping.h>
> #include <linux/err.h>
> #include <linux/export.h>
> @@ -804,7 +805,7 @@ static void ipmmu_remove_device(struct d
> dev->archdata.iommu = NULL;
> }
>
> -static const struct iommu_ops ipmmu_ops = {
> +static const struct iommu_ops __maybe_unused ipmmu_ops = {
> .domain_alloc = ipmmu_domain_alloc,
> .domain_free = ipmmu_domain_free,
> .attach_dev = ipmmu_attach_device,
> @@ -818,6 +819,92 @@ static const struct iommu_ops ipmmu_ops
> .pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
> };
>
> +static struct iommu_domain *ipmmu_domain_alloc_dma(unsigned type)
> +{
> + struct iommu_domain *io_domain;
> +
> + if (type != IOMMU_DOMAIN_DMA)
> + return NULL;
> +
> + io_domain = __ipmmu_domain_alloc(type);
> + if (io_domain)
> + iommu_get_dma_cookie(io_domain);
> +
> + return io_domain;
> +}
> +
> +static void ipmmu_domain_free_dma(struct iommu_domain *io_domain)
> +{
> + iommu_put_dma_cookie(io_domain);
> + ipmmu_domain_free(io_domain);
> +}
> +
> +static int ipmmu_add_device_dma(struct device *dev)
> +{
> + struct iommu_group *group;
> +
> + /* only accept devices with iommus property */
Nitpicking, the driver capitalizes the first letter of the sentence in
comments and includes a period at the end. Could you please follow the same
style for consistency ?
> + if (of_count_phandle_with_args(dev->of_node, "iommus",
> + "#iommu-cells") < 0)
> + return -ENODEV;
Given that the iommu_group_get_for_dev() call will call the .device_group()
operation that ends up calling ipmmu_init_platform_device(), do we need this
check here ?
> + group = iommu_group_get_for_dev(dev);
> + if (IS_ERR(group))
> + return PTR_ERR(group);
> +
> + return 0;
> +}
> +
> +static void ipmmu_remove_device_dma(struct device *dev)
> +{
> + iommu_group_remove_device(dev);
> +}
> +
> +static struct iommu_group *ipmmu_device_group_dma(struct device *dev)
> +{
> + struct iommu_group *group;
> + int ret;
> +
> + group = generic_device_group(dev);
> + if (IS_ERR(group))
> + return group;
> +
> + ret = ipmmu_init_platform_device(dev, group);
> + if (ret) {
> + iommu_group_put(group);
> + group = ERR_PTR(ret);
> + }
> +
> + return group;
> +}
> +
> +#ifdef CONFIG_OF_IOMMU
> +static int ipmmu_of_xlate_dma(struct device *dev,
> + struct of_phandle_args *spec)
> +{
> + /* dummy callback to satisfy of_iommu_configure() */
> + return 0;
> +}
> +#endif
> +
> +static const struct iommu_ops __maybe_unused ipmmu_ops_dma = {
> + .domain_alloc = ipmmu_domain_alloc_dma,
> + .domain_free = ipmmu_domain_free_dma,
> + .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_dma,
> + .remove_device = ipmmu_remove_device_dma,
> + .device_group = ipmmu_device_group_dma,
> + .pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
> +#ifdef CONFIG_OF_IOMMU
> + .of_xlate = ipmmu_of_xlate_dma,
> +#endif
> +};
> +
> /*
> ---------------------------------------------------------------------------
> -- * Probe/remove and init
> */
> @@ -929,14 +1016,17 @@ static struct platform_driver ipmmu_driv
>
> static int __init ipmmu_init(void)
> {
> + const struct iommu_ops *ops;
> int ret;
>
> ret = platform_driver_register(&ipmmu_driver);
> if (ret < 0)
> return ret;
>
> + ops = IS_ENABLED(CONFIG_IOMMU_DMA) ? &ipmmu_ops_dma : &ipmmu_ops;
> +
> if (!iommu_present(&platform_bus_type))
> - bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> + bus_set_iommu(&platform_bus_type, ops);
>
> return 0;
> }
--
Regards,
Laurent Pinchart
Hi Magnus,
Thank you for the patches.
On Wednesday 16 March 2016 02:04:31 Magnus Damm wrote:
> iommu/ipmmu-vmsa: IPMMU CONFIG_IOMMU_DMA update
>
> [PATCH 01/04] iommu/ipmmu-vmsa: 32-bit ARM may have CONFIG_IOMMU_DMA=y
> [PATCH 02/04] iommu/ipmmu-vmsa: Break out utlb parsing code
> [PATCH 03/04] iommu/ipmmu-vmsa: Break out domain allocation code
> [PATCH 04/04] iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops
>
> This series updates the IPMMU driver with CONFIG_IOMMU_DMA=y support
> which is present in mainline for 64-bit ARM while experimental support
> has been posted for 32-bit ARM thanks to Marek Szyprowski:
>
> [RFC 0/3] Unify IOMMU-based DMA-mapping code for ARM and ARM64
>
> Most of the patches in this series based on code earlier included in
> the series below but has been reworked to also fit on 32-bit ARM:
>
> [PATCH/RFC 00/10] iommu/ipmmu-vmsa: Experimental r8a7795 support
>
> Signed-off-by: Magnus Damm <[email protected]>
Great ! Thanks a lot for the work, I'm really happy to see this being
addressed.
For patches 1/4 to 3/4,
Reviewed-by: Laurent Pinchart <[email protected]>
For patch 4/4, please see my comments in the reply to the patch.
> ---
>
> Built on top of next-20160315
> Depends on [PATCH v2 00/04] iommu/ipmmu-vmsa: IPMMU multi-arch update V2
>
> drivers/iommu/ipmmu-vmsa.c | 238 ++++++++++++++++++++++++++++++----------
> 1 file changed, 174 insertions(+), 64 deletions(-)
--
Regards,
Laurent Pinchart
On 15/03/16 17:05, Magnus Damm wrote:
> From: Magnus Damm <[email protected]>
>
> Introduce a new set of iommu_ops suitable for 64-bit ARM
> as well as 32-bit ARM when CONFIG_IOMMU_DMA=y. The ->of_xlate()
> callback is needed by the code exported by of_iommu.h and
> it is wrapped in #ifdefs to also compile of x86_64.
>
> Signed-off-by: Magnus Damm <[email protected]>
> ---
>
> drivers/iommu/ipmmu-vmsa.c | 94 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 92 insertions(+), 2 deletions(-)
>
> --- 0011/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c 2016-03-16 01:35:06.990513000 +0900
> @@ -10,6 +10,7 @@
>
> #include <linux/bitmap.h>
> #include <linux/delay.h>
> +#include <linux/dma-iommu.h>
> #include <linux/dma-mapping.h>
> #include <linux/err.h>
> #include <linux/export.h>
> @@ -804,7 +805,7 @@ static void ipmmu_remove_device(struct d
> dev->archdata.iommu = NULL;
> }
>
> -static const struct iommu_ops ipmmu_ops = {
> +static const struct iommu_ops __maybe_unused ipmmu_ops = {
> .domain_alloc = ipmmu_domain_alloc,
> .domain_free = ipmmu_domain_free,
> .attach_dev = ipmmu_attach_device,
> @@ -818,6 +819,92 @@ static const struct iommu_ops ipmmu_ops
> .pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
> };
>
> +static struct iommu_domain *ipmmu_domain_alloc_dma(unsigned type)
> +{
> + struct iommu_domain *io_domain;
> +
> + if (type != IOMMU_DOMAIN_DMA)
> + return NULL;
> +
> + io_domain = __ipmmu_domain_alloc(type);
> + if (io_domain)
> + iommu_get_dma_cookie(io_domain);
> +
> + return io_domain;
> +}
> +
> +static void ipmmu_domain_free_dma(struct iommu_domain *io_domain)
> +{
> + iommu_put_dma_cookie(io_domain);
> + ipmmu_domain_free(io_domain);
> +}
> +
> +static int ipmmu_add_device_dma(struct device *dev)
> +{
> + struct iommu_group *group;
> +
> + /* only accept devices with iommus property */
> + if (of_count_phandle_with_args(dev->of_node, "iommus",
> + "#iommu-cells") < 0)
> + return -ENODEV;
It seems a bit weird to deliberately ignore the information of_xlate
gives you, only to go off poking the DT to dredge it up manually later.
Why not use of_xlate to create the archdata, then simply check for it
here? (Ideally that should also be common with ARM, but I think you
still get called in the wrong order there)
> +
> + group = iommu_group_get_for_dev(dev);
> + if (IS_ERR(group))
> + return PTR_ERR(group);
> +
> + return 0;
> +}
> +
> +static void ipmmu_remove_device_dma(struct device *dev)
> +{
> + iommu_group_remove_device(dev);
> +}
> +
> +static struct iommu_group *ipmmu_device_group_dma(struct device *dev)
> +{
> + struct iommu_group *group;
> + int ret;
> +
> + group = generic_device_group(dev);
> + if (IS_ERR(group))
> + return group;
> +
> + ret = ipmmu_init_platform_device(dev, group);
> + if (ret) {
> + iommu_group_put(group);
> + group = ERR_PTR(ret);
> + }
> +
> + return group;
> +}
> +
> +#ifdef CONFIG_OF_IOMMU
> +static int ipmmu_of_xlate_dma(struct device *dev,
> + struct of_phandle_args *spec)
> +{
> + /* dummy callback to satisfy of_iommu_configure() */
> + return 0;
> +}
> +#endif
> +
> +static const struct iommu_ops __maybe_unused ipmmu_ops_dma = {
> + .domain_alloc = ipmmu_domain_alloc_dma,
> + .domain_free = ipmmu_domain_free_dma,
> + .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_dma,
> + .remove_device = ipmmu_remove_device_dma,
> + .device_group = ipmmu_device_group_dma,
> + .pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
> +#ifdef CONFIG_OF_IOMMU
> + .of_xlate = ipmmu_of_xlate_dma,
> +#endif
Yup, I definitely think Arnd is right about eradicating these #ifdefs[1].
Robin.
[1]:http://thread.gmane.org/gmane.linux.kernel/2177390
> +};
> +
> /* -----------------------------------------------------------------------------
> * Probe/remove and init
> */
> @@ -929,14 +1016,17 @@ static struct platform_driver ipmmu_driv
>
> static int __init ipmmu_init(void)
> {
> + const struct iommu_ops *ops;
> int ret;
>
> ret = platform_driver_register(&ipmmu_driver);
> if (ret < 0)
> return ret;
>
> + ops = IS_ENABLED(CONFIG_IOMMU_DMA) ? &ipmmu_ops_dma : &ipmmu_ops;
> +
> if (!iommu_present(&platform_bus_type))
> - bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> + bus_set_iommu(&platform_bus_type, ops);
>
> return 0;
> }
> _______________________________________________
> iommu mailing list
> [email protected]
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>
On Wed, Mar 16, 2016 at 02:04:31AM +0900, Magnus Damm wrote:
> iommu/ipmmu-vmsa: IPMMU CONFIG_IOMMU_DMA update
>
> [PATCH 01/04] iommu/ipmmu-vmsa: 32-bit ARM may have CONFIG_IOMMU_DMA=y
> [PATCH 02/04] iommu/ipmmu-vmsa: Break out utlb parsing code
> [PATCH 03/04] iommu/ipmmu-vmsa: Break out domain allocation code
> [PATCH 04/04] iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops
These patches do not apply to v4.6-rc2. Can you please rebase them,
address the comments and add the Reviewed-by's?
Joerg