iommu/ipmmu-vmsa: IPMMU multi-arch update
[PATCH 01/06] iommu/ipmmu-vmsa: Remove platform data handling
[PATCH 02/06] iommu/ipmmu-vmsa: Convert to dev_data
[PATCH 03/06] iommu/ipmmu-vmsa: Break out utlb control function
[PATCH 04/06] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context
[PATCH 05/06] iommu/ipmmu-vmsa: Break out 32-bit ARM mapping code
[PATCH 06/06] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency
These patches update the IPMMU driver with a few minor changes
to support multiple architectures. With these patches applied
the driver is known to compile without issues on 32-bit ARM,
64-bit ARM and x86_64.
With the series applied the r8a7790 IPMMU still gets invoked
when the modetest utility is used to display a test pattern
on the VGA port driven by the DU device.
Signed-off-by: Magnus Damm <[email protected]>
---
Developed on top of renesas-drivers-2015-12-08-v4.4-rc4
drivers/iommu/Kconfig | 1
drivers/iommu/ipmmu-vmsa.c | 224 +++++++++++++++++++++++++++-----------------
2 files changed, 139 insertions(+), 86 deletions(-)
From: Magnus Damm <[email protected]>
The IPMMU driver is using DT these days, and platform data is no longer
used by the driver. Remove unused code.
Signed-off-by: Magnus Damm <[email protected]>
---
drivers/iommu/ipmmu-vmsa.c | 5 -----
1 file changed, 5 deletions(-)
--- 0001/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-15 11:35:00.490513000 +0900
@@ -766,11 +766,6 @@ static int ipmmu_probe(struct platform_d
int irq;
int ret;
- if (!IS_ENABLED(CONFIG_OF) && !pdev->dev.platform_data) {
- dev_err(&pdev->dev, "missing platform data\n");
- return -EINVAL;
- }
-
mmu = devm_kzalloc(&pdev->dev, sizeof(*mmu), GFP_KERNEL);
if (!mmu) {
dev_err(&pdev->dev, "cannot allocate device data\n");
From: Magnus Damm <[email protected]>
Rename ipmmu_vmsa_archdata to ipmmu_vmsa_dev_data to avoid
confusion when using the driver on multiple architectures.
The data now stored in ipmmu_vmsa_dev_data is used to point
various on-chip devices to the actual IPMMU instances.
Signed-off-by: Magnus Damm <[email protected]>
---
drivers/iommu/ipmmu-vmsa.c | 58 +++++++++++++++++++++++++++-----------------
1 file changed, 36 insertions(+), 22 deletions(-)
--- 0003/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-15 13:16:27.160513000 +0900
@@ -47,7 +47,7 @@ struct ipmmu_vmsa_domain {
spinlock_t lock; /* Protects mappings */
};
-struct ipmmu_vmsa_archdata {
+struct ipmmu_vmsa_dev_data {
struct ipmmu_vmsa_device *mmu;
unsigned int *utlbs;
unsigned int num_utlbs;
@@ -182,6 +182,20 @@ static struct ipmmu_vmsa_domain *to_vmsa
#define IMUASID_ASID0_SHIFT 0
/* -----------------------------------------------------------------------------
+ * Consumer device side private data handling
+ */
+
+static struct ipmmu_vmsa_dev_data *get_dev_data(struct device *dev)
+{
+ return dev->archdata.iommu;
+}
+
+static void set_dev_data(struct device *dev, struct ipmmu_vmsa_dev_data *data)
+{
+ dev->archdata.iommu = data;
+}
+
+/* -----------------------------------------------------------------------------
* Read/Write Access
*/
@@ -485,8 +499,8 @@ static void ipmmu_domain_free(struct iom
static int ipmmu_attach_device(struct iommu_domain *io_domain,
struct device *dev)
{
- struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
- struct ipmmu_vmsa_device *mmu = archdata->mmu;
+ struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
+ struct ipmmu_vmsa_device *mmu = dev_data->mmu;
struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
unsigned long flags;
unsigned int i;
@@ -518,8 +532,8 @@ static int ipmmu_attach_device(struct io
if (ret < 0)
return ret;
- for (i = 0; i < archdata->num_utlbs; ++i)
- ipmmu_utlb_enable(domain, archdata->utlbs[i]);
+ for (i = 0; i < dev_data->num_utlbs; ++i)
+ ipmmu_utlb_enable(domain, dev_data->utlbs[i]);
return 0;
}
@@ -527,12 +541,12 @@ static int ipmmu_attach_device(struct io
static void ipmmu_detach_device(struct iommu_domain *io_domain,
struct device *dev)
{
- struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+ struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
unsigned int i;
- for (i = 0; i < archdata->num_utlbs; ++i)
- ipmmu_utlb_disable(domain, archdata->utlbs[i]);
+ for (i = 0; i < dev_data->num_utlbs; ++i)
+ ipmmu_utlb_disable(domain, dev_data->utlbs[i]);
/*
* TODO: Optimize by disabling the context when no device is attached.
@@ -595,7 +609,7 @@ static int ipmmu_find_utlbs(struct ipmmu
static int ipmmu_add_device(struct device *dev)
{
- struct ipmmu_vmsa_archdata *archdata;
+ struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
struct ipmmu_vmsa_device *mmu;
struct iommu_group *group = NULL;
unsigned int *utlbs;
@@ -603,7 +617,7 @@ static int ipmmu_add_device(struct devic
int num_utlbs;
int ret = -ENODEV;
- if (dev->archdata.iommu) {
+ if (dev_data) {
dev_warn(dev, "IOMMU driver already assigned to device %s\n",
dev_name(dev));
return -EINVAL;
@@ -662,16 +676,16 @@ static int ipmmu_add_device(struct devic
goto error;
}
- archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
- if (!archdata) {
+ dev_data = kzalloc(sizeof(*dev_data), GFP_KERNEL);
+ if (!dev_data) {
ret = -ENOMEM;
goto error;
}
- archdata->mmu = mmu;
- archdata->utlbs = utlbs;
- archdata->num_utlbs = num_utlbs;
- dev->archdata.iommu = archdata;
+ dev_data->mmu = mmu;
+ dev_data->utlbs = utlbs;
+ dev_data->num_utlbs = num_utlbs;
+ set_dev_data(dev, dev_data);
/*
* Create the ARM mapping, used by the ARM DMA mapping core to allocate
@@ -708,10 +722,10 @@ static int ipmmu_add_device(struct devic
error:
arm_iommu_release_mapping(mmu->mapping);
- kfree(dev->archdata.iommu);
+ kfree(dev_data);
kfree(utlbs);
- dev->archdata.iommu = NULL;
+ set_dev_data(dev, NULL);
if (!IS_ERR_OR_NULL(group))
iommu_group_remove_device(dev);
@@ -721,15 +735,15 @@ error:
static void ipmmu_remove_device(struct device *dev)
{
- struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+ struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
arm_iommu_detach_device(dev);
iommu_group_remove_device(dev);
- kfree(archdata->utlbs);
- kfree(archdata);
+ kfree(dev_data->utlbs);
+ kfree(dev_data);
- dev->archdata.iommu = NULL;
+ set_dev_data(dev, NULL);
}
static const struct iommu_ops ipmmu_ops = {
From: Magnus Damm <[email protected]>
Introduce the function ipmmu_utlb_ctrl() that shares code
to enable or disable utlbs.
Signed-off-by: Magnus Damm <[email protected]>
---
drivers/iommu/ipmmu-vmsa.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
--- 0004/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-15 13:17:40.580513000 +0900
@@ -279,9 +279,18 @@ static void ipmmu_utlb_enable(struct ipm
static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain,
unsigned int utlb)
{
- struct ipmmu_vmsa_device *mmu = domain->mmu;
+ ipmmu_write(domain->mmu, IMUCTR(utlb), 0);
+}
+
+static void ipmmu_utlb_ctrl(struct ipmmu_vmsa_domain *domain,
+ void (*fn)(struct ipmmu_vmsa_domain *,
+ unsigned int utlb), struct device *dev)
+{
+ struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
+ unsigned int i;
- ipmmu_write(mmu, IMUCTR(utlb), 0);
+ for (i = 0; i < dev_data->num_utlbs; ++i)
+ fn(domain, dev_data->utlbs[i]);
}
static void ipmmu_tlb_flush_all(void *cookie)
@@ -503,7 +512,6 @@ static int ipmmu_attach_device(struct io
struct ipmmu_vmsa_device *mmu = dev_data->mmu;
struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
unsigned long flags;
- unsigned int i;
int ret = 0;
if (!mmu) {
@@ -529,24 +537,16 @@ static int ipmmu_attach_device(struct io
spin_unlock_irqrestore(&domain->lock, flags);
- if (ret < 0)
- return ret;
+ if (!ret)
+ ipmmu_utlb_ctrl(domain, ipmmu_utlb_enable, dev);
- for (i = 0; i < dev_data->num_utlbs; ++i)
- ipmmu_utlb_enable(domain, dev_data->utlbs[i]);
-
- return 0;
+ return ret;
}
static void ipmmu_detach_device(struct iommu_domain *io_domain,
struct device *dev)
{
- struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
- struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
- unsigned int i;
-
- for (i = 0; i < dev_data->num_utlbs; ++i)
- ipmmu_utlb_disable(domain, dev_data->utlbs[i]);
+ ipmmu_utlb_ctrl(to_vmsa_domain(io_domain), ipmmu_utlb_disable, dev);
/*
* TODO: Optimize by disabling the context when no device is attached.
From: Magnus Damm <[email protected]>
Introduce a bitmap for context handing and convert the
interrupt routine to go handle all registered contexts.
At this point the number of contexts are still limited.
The purpose of this patch is to remove the use of the
ARM specific mapping variable from ipmmu_irq().
Signed-off-by: Magnus Damm <[email protected]>
---
drivers/iommu/ipmmu-vmsa.c | 37 ++++++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 11 deletions(-)
--- 0007/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-15 13:14:35.540513000 +0900
@@ -8,6 +8,7 @@
* the Free Software Foundation; version 2 of the License.
*/
+#include <linux/bitmap.h>
#include <linux/delay.h>
#include <linux/dma-mapping.h>
#include <linux/err.h>
@@ -26,12 +27,16 @@
#include "io-pgtable.h"
+#define IPMMU_CTX_MAX 1
+
struct ipmmu_vmsa_device {
struct device *dev;
void __iomem *base;
struct list_head list;
unsigned int num_utlbs;
+ DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
+ struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
struct dma_iommu_mapping *mapping;
};
@@ -319,6 +324,7 @@ static struct iommu_gather_ops ipmmu_gat
static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
{
phys_addr_t ttbr;
+ int ret;
/*
* Allocate the page table operations.
@@ -348,10 +354,16 @@ static int ipmmu_domain_init_context(str
return -EINVAL;
/*
- * TODO: When adding support for multiple contexts, find an unused
- * context.
+ * Find an unused context.
*/
- domain->context_id = 0;
+ ret = bitmap_find_free_region(domain->mmu->ctx, IPMMU_CTX_MAX, 0);
+ if (ret < 0) {
+ free_io_pgtable_ops(domain->iop);
+ return ret;
+ }
+
+ domain->context_id = ret;
+ domain->mmu->domains[ret] = domain;
/* TTBR0 */
ttbr = domain->cfg.arm_lpae_s1_cfg.ttbr[0];
@@ -395,6 +407,8 @@ static int ipmmu_domain_init_context(str
static void ipmmu_domain_destroy_context(struct ipmmu_vmsa_domain *domain)
{
+ bitmap_release_region(domain->mmu->ctx, domain->context_id, 0);
+
/*
* Disable the context. Flush the TLB as required when modifying the
* context registers.
@@ -460,16 +474,16 @@ static irqreturn_t ipmmu_domain_irq(stru
static irqreturn_t ipmmu_irq(int irq, void *dev)
{
struct ipmmu_vmsa_device *mmu = dev;
- struct iommu_domain *io_domain;
- struct ipmmu_vmsa_domain *domain;
-
- if (!mmu->mapping)
- return IRQ_NONE;
+ irqreturn_t status = IRQ_NONE;
+ unsigned int k;
- io_domain = mmu->mapping->domain;
- domain = to_vmsa_domain(io_domain);
+ /* Check interrupts for all active contexts */
+ for (k = find_first_bit(mmu->ctx, IPMMU_CTX_MAX);
+ k < IPMMU_CTX_MAX && status == IRQ_NONE;
+ k = find_next_bit(mmu->ctx, IPMMU_CTX_MAX, k))
+ status = ipmmu_domain_irq(mmu->domains[k]);
- return ipmmu_domain_irq(domain);
+ return status;
}
/* -----------------------------------------------------------------------------
@@ -788,6 +802,7 @@ static int ipmmu_probe(struct platform_d
mmu->dev = &pdev->dev;
mmu->num_utlbs = 32;
+ bitmap_zero(mmu->ctx, IPMMU_CTX_MAX);
/* Map I/O memory and request IRQ. */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
From: Magnus Damm <[email protected]>
Make the driver compile on more than just 32-bit ARM
by breaking out and wrapping ARM specific functions
in #ifdefs. Needed to be able to use the driver on
other architectures.
Signed-off-by: Magnus Damm <[email protected]>
---
drivers/iommu/ipmmu-vmsa.c | 94 +++++++++++++++++++++++++++++---------------
1 file changed, 62 insertions(+), 32 deletions(-)
--- 0007/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-15 13:20:26.580513000 +0900
@@ -22,8 +22,10 @@
#include <linux/sizes.h>
#include <linux/slab.h>
+#ifdef CONFIG_ARM
#include <asm/dma-iommu.h>
#include <asm/pgalloc.h>
+#endif
#include "io-pgtable.h"
@@ -38,7 +40,9 @@ struct ipmmu_vmsa_device {
DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
+#ifdef CONFIG_ARM
struct dma_iommu_mapping *mapping;
+#endif
};
struct ipmmu_vmsa_domain {
@@ -621,6 +625,60 @@ static int ipmmu_find_utlbs(struct ipmmu
return 0;
}
+#ifdef CONFIG_ARM
+static int ipmmu_map_attach(struct device *dev, struct ipmmu_vmsa_device *mmu)
+{
+ int ret;
+
+ /*
+ * Create the ARM mapping, used by the ARM DMA mapping core to allocate
+ * VAs. This will allocate a corresponding IOMMU domain.
+ *
+ * TODO:
+ * - Create one mapping per context (TLB).
+ * - Make the mapping size configurable ? We currently use a 2GB mapping
+ * at a 1GB offset to ensure that NULL VAs will fault.
+ */
+ if (!mmu->mapping) {
+ struct dma_iommu_mapping *mapping;
+
+ mapping = arm_iommu_create_mapping(&platform_bus_type,
+ SZ_1G, SZ_2G);
+ if (IS_ERR(mapping)) {
+ dev_err(mmu->dev, "failed to create ARM IOMMU mapping\n");
+ return PTR_ERR(mapping);
+ }
+
+ mmu->mapping = mapping;
+ }
+
+ /* Attach the ARM VA mapping to the device. */
+ ret = arm_iommu_attach_device(dev, mmu->mapping);
+ if (ret < 0) {
+ dev_err(dev, "Failed to attach device to VA mapping\n");
+ arm_iommu_release_mapping(mmu->mapping);
+ }
+
+ return ret;
+}
+static inline void ipmmu_detach(struct device *dev)
+{
+ arm_iommu_detach_device(dev);
+}
+static inline void ipmmu_release_mapping(struct ipmmu_vmsa_device *mmu)
+{
+ arm_iommu_release_mapping(mmu->mapping);
+}
+#else
+static inline int ipmmu_map_attach(struct device *dev,
+ struct ipmmu_vmsa_device *mmu)
+{
+ return 0;
+}
+static inline void ipmmu_detach(struct device *dev) {}
+static inline void ipmmu_release_mapping(struct ipmmu_vmsa_device *mmu) {}
+#endif
+
static int ipmmu_add_device(struct device *dev)
{
struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
@@ -701,41 +759,13 @@ static int ipmmu_add_device(struct devic
dev_data->num_utlbs = num_utlbs;
set_dev_data(dev, dev_data);
- /*
- * Create the ARM mapping, used by the ARM DMA mapping core to allocate
- * VAs. This will allocate a corresponding IOMMU domain.
- *
- * TODO:
- * - Create one mapping per context (TLB).
- * - Make the mapping size configurable ? We currently use a 2GB mapping
- * at a 1GB offset to ensure that NULL VAs will fault.
- */
- if (!mmu->mapping) {
- struct dma_iommu_mapping *mapping;
-
- mapping = arm_iommu_create_mapping(&platform_bus_type,
- SZ_1G, SZ_2G);
- if (IS_ERR(mapping)) {
- dev_err(mmu->dev, "failed to create ARM IOMMU mapping\n");
- ret = PTR_ERR(mapping);
- goto error;
- }
-
- mmu->mapping = mapping;
- }
-
- /* Attach the ARM VA mapping to the device. */
- ret = arm_iommu_attach_device(dev, mmu->mapping);
- if (ret < 0) {
- dev_err(dev, "Failed to attach device to VA mapping\n");
+ ret = ipmmu_map_attach(dev, mmu);
+ if (ret < 0)
goto error;
- }
return 0;
error:
- arm_iommu_release_mapping(mmu->mapping);
-
kfree(dev_data);
kfree(utlbs);
@@ -751,7 +781,7 @@ static void ipmmu_remove_device(struct d
{
struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
- arm_iommu_detach_device(dev);
+ ipmmu_detach(dev);
iommu_group_remove_device(dev);
kfree(dev_data->utlbs);
@@ -862,7 +892,7 @@ static int ipmmu_remove(struct platform_
list_del(&mmu->list);
spin_unlock(&ipmmu_devices_lock);
- arm_iommu_release_mapping(mmu->mapping);
+ ipmmu_release_mapping(mmu);
ipmmu_device_reset(mmu);
From: Magnus Damm <[email protected]>
Neither the ARM page table code enabled by IOMMU_IO_PGTABLE_LPAE
nor the IPMMU_VMSA driver actually depends on ARM_LPAE, so get
rid of the dependency.
Tested with ipmmu-vmsa on r8a7794 ALT and a kernel config using:
# CONFIG_ARM_LPAE is not set
Signed-off-by: Magnus Damm <[email protected]>
---
This time the result also compiles on x86. Need to be
applied as last patch in the following series:
[PATCH 00/06] iommu/ipmmu-vmsa: IPMMU multi-arch update
drivers/iommu/Kconfig | 1 -
1 file changed, 1 deletion(-)
--- 0001/drivers/iommu/Kconfig
+++ work/drivers/iommu/Kconfig 2015-10-18 14:58:09.080513000 +0900
@@ -324,7 +324,6 @@ config SHMOBILE_IOMMU_L1SIZE
config IPMMU_VMSA
bool "Renesas VMSA-compatible IPMMU"
- depends on ARM_LPAE
depends on ARCH_SHMOBILE || COMPILE_TEST
select IOMMU_API
select IOMMU_IO_PGTABLE_LPAE
Hi Magnus,
On Tue, Dec 15, 2015 at 1:02 PM, Magnus Damm <[email protected]> wrote:
> --- 0004/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-15 13:17:40.580513000 +0900
> @@ -279,9 +279,18 @@ static void ipmmu_utlb_enable(struct ipm
> static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain,
> unsigned int utlb)
> {
> - struct ipmmu_vmsa_device *mmu = domain->mmu;
> + ipmmu_write(domain->mmu, IMUCTR(utlb), 0);
> +}
> +
> +static void ipmmu_utlb_ctrl(struct ipmmu_vmsa_domain *domain,
> + void (*fn)(struct ipmmu_vmsa_domain *,
> + unsigned int utlb), struct device *dev)
> +{
> + struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
> + unsigned int i;
>
> - ipmmu_write(mmu, IMUCTR(utlb), 0);
> + for (i = 0; i < dev_data->num_utlbs; ++i)
> + fn(domain, dev_data->utlbs[i]);
> }
Unless you have further plans with the "fn" parameter, I would simply pass
a bool enable/disable flag instead of a function pointer.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
Hi Magnus,
Thank you for the patch.
On Tuesday 15 December 2015 21:02:21 Magnus Damm wrote:
> From: Magnus Damm <[email protected]>
>
> The IPMMU driver is using DT these days, and platform data is no longer
> used by the driver. Remove unused code.
>
> Signed-off-by: Magnus Damm <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
> ---
>
> drivers/iommu/ipmmu-vmsa.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> --- 0001/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-15 11:35:00.490513000 +0900
> @@ -766,11 +766,6 @@ static int ipmmu_probe(struct platform_d
> int irq;
> int ret;
>
> - if (!IS_ENABLED(CONFIG_OF) && !pdev->dev.platform_data) {
> - dev_err(&pdev->dev, "missing platform data\n");
> - return -EINVAL;
> - }
> -
> mmu = devm_kzalloc(&pdev->dev, sizeof(*mmu), GFP_KERNEL);
> if (!mmu) {
> dev_err(&pdev->dev, "cannot allocate device data\n");
--
Regards,
Laurent Pinchart
Hi Magnus,
Thank you for the patch.
On Tuesday 15 December 2015 21:02:30 Magnus Damm wrote:
> From: Magnus Damm <[email protected]>
>
> Rename ipmmu_vmsa_archdata to ipmmu_vmsa_dev_data to avoid
> confusion when using the driver on multiple architectures.
I'm sorry but this patches makes the driver more confusing to me. archdata is
a well established concept for IOMMU on ARM, I don't see how hiding help would
help.
> The data now stored in ipmmu_vmsa_dev_data is used to point
> various on-chip devices to the actual IPMMU instances.
>
> Signed-off-by: Magnus Damm <[email protected]>
> ---
>
> drivers/iommu/ipmmu-vmsa.c | 58 +++++++++++++++++++++++++----------------
> 1 file changed, 36 insertions(+), 22 deletions(-)
>
> --- 0003/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-15 13:16:27.160513000 +0900
> @@ -47,7 +47,7 @@ struct ipmmu_vmsa_domain {
> spinlock_t lock; /* Protects mappings */
> };
>
> -struct ipmmu_vmsa_archdata {
> +struct ipmmu_vmsa_dev_data {
> struct ipmmu_vmsa_device *mmu;
> unsigned int *utlbs;
> unsigned int num_utlbs;
> @@ -182,6 +182,20 @@ static struct ipmmu_vmsa_domain *to_vmsa
> #define IMUASID_ASID0_SHIFT 0
>
> /* ------------------------------------------------------------------------
> + * Consumer device side private data handling
> + */
> +
> +static struct ipmmu_vmsa_dev_data *get_dev_data(struct device *dev)
> +{
> + return dev->archdata.iommu;
> +}
> +
> +static void set_dev_data(struct device *dev, struct ipmmu_vmsa_dev_data
> *data)
> +{
> + dev->archdata.iommu = data;
> +}
> +
> +/* ------------------------------------------------------------------------
> * Read/Write Access
> */
>
> @@ -485,8 +499,8 @@ static void ipmmu_domain_free(struct iom
> static int ipmmu_attach_device(struct iommu_domain *io_domain,
> struct device *dev)
> {
> - struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> - struct ipmmu_vmsa_device *mmu = archdata->mmu;
> + struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
> + struct ipmmu_vmsa_device *mmu = dev_data->mmu;
> struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
> unsigned long flags;
> unsigned int i;
> @@ -518,8 +532,8 @@ static int ipmmu_attach_device(struct io
> if (ret < 0)
> return ret;
>
> - for (i = 0; i < archdata->num_utlbs; ++i)
> - ipmmu_utlb_enable(domain, archdata->utlbs[i]);
> + for (i = 0; i < dev_data->num_utlbs; ++i)
> + ipmmu_utlb_enable(domain, dev_data->utlbs[i]);
>
> return 0;
> }
> @@ -527,12 +541,12 @@ static int ipmmu_attach_device(struct io
> static void ipmmu_detach_device(struct iommu_domain *io_domain,
> struct device *dev)
> {
> - struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> + struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
> struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
> unsigned int i;
>
> - for (i = 0; i < archdata->num_utlbs; ++i)
> - ipmmu_utlb_disable(domain, archdata->utlbs[i]);
> + for (i = 0; i < dev_data->num_utlbs; ++i)
> + ipmmu_utlb_disable(domain, dev_data->utlbs[i]);
>
> /*
> * TODO: Optimize by disabling the context when no device is attached.
> @@ -595,7 +609,7 @@ static int ipmmu_find_utlbs(struct ipmmu
>
> static int ipmmu_add_device(struct device *dev)
> {
> - struct ipmmu_vmsa_archdata *archdata;
> + struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
> struct ipmmu_vmsa_device *mmu;
> struct iommu_group *group = NULL;
> unsigned int *utlbs;
> @@ -603,7 +617,7 @@ static int ipmmu_add_device(struct devic
> int num_utlbs;
> int ret = -ENODEV;
>
> - if (dev->archdata.iommu) {
> + if (dev_data) {
> dev_warn(dev, "IOMMU driver already assigned to device %s\n",
> dev_name(dev));
> return -EINVAL;
> @@ -662,16 +676,16 @@ static int ipmmu_add_device(struct devic
> goto error;
> }
>
> - archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
> - if (!archdata) {
> + dev_data = kzalloc(sizeof(*dev_data), GFP_KERNEL);
> + if (!dev_data) {
> ret = -ENOMEM;
> goto error;
> }
>
> - archdata->mmu = mmu;
> - archdata->utlbs = utlbs;
> - archdata->num_utlbs = num_utlbs;
> - dev->archdata.iommu = archdata;
> + dev_data->mmu = mmu;
> + dev_data->utlbs = utlbs;
> + dev_data->num_utlbs = num_utlbs;
> + set_dev_data(dev, dev_data);
>
> /*
> * Create the ARM mapping, used by the ARM DMA mapping core to allocate
> @@ -708,10 +722,10 @@ static int ipmmu_add_device(struct devic
> error:
> arm_iommu_release_mapping(mmu->mapping);
>
> - kfree(dev->archdata.iommu);
> + kfree(dev_data);
> kfree(utlbs);
>
> - dev->archdata.iommu = NULL;
> + set_dev_data(dev, NULL);
>
> if (!IS_ERR_OR_NULL(group))
> iommu_group_remove_device(dev);
> @@ -721,15 +735,15 @@ error:
>
> static void ipmmu_remove_device(struct device *dev)
> {
> - struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> + struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
>
> arm_iommu_detach_device(dev);
> iommu_group_remove_device(dev);
>
> - kfree(archdata->utlbs);
> - kfree(archdata);
> + kfree(dev_data->utlbs);
> + kfree(dev_data);
>
> - dev->archdata.iommu = NULL;
> + set_dev_data(dev, NULL);
> }
>
> static const struct iommu_ops ipmmu_ops = {
--
Regards,
Laurent Pinchart
Hi Magnus,
On Tuesday 15 December 2015 16:48:42 Geert Uytterhoeven wrote:
> On Tue, Dec 15, 2015 at 1:02 PM, Magnus Damm <[email protected]> wrote:
> > --- 0004/drivers/iommu/ipmmu-vmsa.c
> > +++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-15 13:17:40.580513000
> > +0900
> > @@ -279,9 +279,18 @@ static void ipmmu_utlb_enable(struct ipm
> > static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain,
> > unsigned int utlb)
> > {
> > - struct ipmmu_vmsa_device *mmu = domain->mmu;
> > + ipmmu_write(domain->mmu, IMUCTR(utlb), 0);
> > +}
> > +
> > +static void ipmmu_utlb_ctrl(struct ipmmu_vmsa_domain *domain,
> > + void (*fn)(struct ipmmu_vmsa_domain *,
> > + unsigned int utlb), struct device
> > *dev)
> > +{
> > + struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
> > + unsigned int i;
> >
> > - ipmmu_write(mmu, IMUCTR(utlb), 0);
> > + for (i = 0; i < dev_data->num_utlbs; ++i)
> > + fn(domain, dev_data->utlbs[i]);
> >
> > }
>
> Unless you have further plans with the "fn" parameter, I would simply pass
> a bool enable/disable flag instead of a function pointer.
I agree with Geert. What's your plan here ?
--
Regards,
Laurent Pinchart
Hi Magnus,
Thank you for the patch.
On Tuesday 15 December 2015 21:02:49 Magnus Damm wrote:
> From: Magnus Damm <[email protected]>
>
> Introduce a bitmap for context handing and convert the
> interrupt routine to go handle all registered contexts.
>
> At this point the number of contexts are still limited.
That's all nice, but without seeing support for multiple contexts it's hard to
tell if the implementation is correct for multiple context purpose.
> The purpose of this patch is to remove the use of the
> ARM specific mapping variable from ipmmu_irq().
Why do you want to do that ?
> Signed-off-by: Magnus Damm <[email protected]>
> ---
>
> drivers/iommu/ipmmu-vmsa.c | 37 ++++++++++++++++++++++++++-----------
> 1 file changed, 26 insertions(+), 11 deletions(-)
>
> --- 0007/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-15 13:14:35.540513000 +0900
> @@ -8,6 +8,7 @@
> * the Free Software Foundation; version 2 of the License.
> */
>
> +#include <linux/bitmap.h>
> #include <linux/delay.h>
> #include <linux/dma-mapping.h>
> #include <linux/err.h>
> @@ -26,12 +27,16 @@
>
> #include "io-pgtable.h"
>
> +#define IPMMU_CTX_MAX 1
> +
> struct ipmmu_vmsa_device {
> struct device *dev;
> void __iomem *base;
> struct list_head list;
>
> unsigned int num_utlbs;
> + DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
We have up to 4 context on Gen2 and 8 on Gen3, a bitmap might be slightly
overkill.
> + struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
>
> struct dma_iommu_mapping *mapping;
> };
> @@ -319,6 +324,7 @@ static struct iommu_gather_ops ipmmu_gat
> static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
> {
> phys_addr_t ttbr;
> + int ret;
>
> /*
> * Allocate the page table operations.
> @@ -348,10 +354,16 @@ static int ipmmu_domain_init_context(str
> return -EINVAL;
>
> /*
> - * TODO: When adding support for multiple contexts, find an unused
> - * context.
> + * Find an unused context.
We need to support multiple devices per context or we will very soon run out
of contexts. How to pick a proper context is a topic that needs to be
researched, I believe IOMMU groups might come into play.
> */
> - domain->context_id = 0;
> + ret = bitmap_find_free_region(domain->mmu->ctx, IPMMU_CTX_MAX, 0);
> + if (ret < 0) {
> + free_io_pgtable_ops(domain->iop);
> + return ret;
> + }
> +
> + domain->context_id = ret;
> + domain->mmu->domains[ret] = domain;
This requires locking to protect against races with the interrupt handler.
>
> /* TTBR0 */
> ttbr = domain->cfg.arm_lpae_s1_cfg.ttbr[0];
> @@ -395,6 +407,8 @@ static int ipmmu_domain_init_context(str
>
> static void ipmmu_domain_destroy_context(struct ipmmu_vmsa_domain *domain)
> {
> + bitmap_release_region(domain->mmu->ctx, domain->context_id, 0);
> +
> /*
> * Disable the context. Flush the TLB as required when modifying the
> * context registers.
> @@ -460,16 +474,16 @@ static irqreturn_t ipmmu_domain_irq(stru
> static irqreturn_t ipmmu_irq(int irq, void *dev)
> {
> struct ipmmu_vmsa_device *mmu = dev;
> - struct iommu_domain *io_domain;
> - struct ipmmu_vmsa_domain *domain;
> -
> - if (!mmu->mapping)
> - return IRQ_NONE;
> + irqreturn_t status = IRQ_NONE;
> + unsigned int k;
i is a perfectly fine loop counter :-)
> - io_domain = mmu->mapping->domain;
> - domain = to_vmsa_domain(io_domain);
> + /* Check interrupts for all active contexts */
> + for (k = find_first_bit(mmu->ctx, IPMMU_CTX_MAX);
> + k < IPMMU_CTX_MAX && status == IRQ_NONE;
> + k = find_next_bit(mmu->ctx, IPMMU_CTX_MAX, k))
You can just loop over mmu->domains and skip NULL entries.
> + status = ipmmu_domain_irq(mmu->domains[k]);
Only the status of the last domain is taken into account.
> - return ipmmu_domain_irq(domain);
> + return status;
> }
>
> /* ------------------------------------------------------------------------
> @@ -788,6 +802,7 @@ static int ipmmu_probe(struct platform_d
>
> mmu->dev = &pdev->dev;
> mmu->num_utlbs = 32;
> + bitmap_zero(mmu->ctx, IPMMU_CTX_MAX);
>
> /* Map I/O memory and request IRQ. */
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
--
Regards,
Laurent Pinchart
Hi Magnus,
Thank you for the patch.
On Tuesday 15 December 2015 21:02:58 Magnus Damm wrote:
> From: Magnus Damm <[email protected]>
>
> Make the driver compile on more than just 32-bit ARM
> by breaking out and wrapping ARM specific functions
> in #ifdefs. Needed to be able to use the driver on
> other architectures.
The only other architecture where we need to use the driver is ARM64, and I'm
not sure this is the right fix to correctly support ARM and ARM64. I believe
we should instead improve the ARM implementation to get rid of
arm_iommu_create_mapping() and arm_iommu_attach_device() the same way that
ARM64 did.
> Signed-off-by: Magnus Damm <[email protected]>
> ---
>
> drivers/iommu/ipmmu-vmsa.c | 94 ++++++++++++++++++++++++++---------------
> 1 file changed, 62 insertions(+), 32 deletions(-)
>
> --- 0007/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-15 13:20:26.580513000 +0900
> @@ -22,8 +22,10 @@
> #include <linux/sizes.h>
> #include <linux/slab.h>
>
> +#ifdef CONFIG_ARM
> #include <asm/dma-iommu.h>
> #include <asm/pgalloc.h>
> +#endif
>
> #include "io-pgtable.h"
>
> @@ -38,7 +40,9 @@ struct ipmmu_vmsa_device {
> DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
> struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
>
> +#ifdef CONFIG_ARM
> struct dma_iommu_mapping *mapping;
> +#endif
> };
>
> struct ipmmu_vmsa_domain {
> @@ -621,6 +625,60 @@ static int ipmmu_find_utlbs(struct ipmmu
> return 0;
> }
>
> +#ifdef CONFIG_ARM
> +static int ipmmu_map_attach(struct device *dev, struct ipmmu_vmsa_device
> *mmu)
> +{
> + int ret;
> +
> + /*
> + * Create the ARM mapping, used by the ARM DMA mapping core to allocate
> + * VAs. This will allocate a corresponding IOMMU domain.
> + *
> + * TODO:
> + * - Create one mapping per context (TLB).
> + * - Make the mapping size configurable ? We currently use a 2GB mapping
> + * at a 1GB offset to ensure that NULL VAs will fault.
> + */
> + if (!mmu->mapping) {
> + struct dma_iommu_mapping *mapping;
> +
> + mapping = arm_iommu_create_mapping(&platform_bus_type,
> + SZ_1G, SZ_2G);
> + if (IS_ERR(mapping)) {
> + dev_err(mmu->dev, "failed to create ARM IOMMU mapping\n");
> + return PTR_ERR(mapping);
> + }
> +
> + mmu->mapping = mapping;
> + }
> +
> + /* Attach the ARM VA mapping to the device. */
> + ret = arm_iommu_attach_device(dev, mmu->mapping);
> + if (ret < 0) {
> + dev_err(dev, "Failed to attach device to VA mapping\n");
> + arm_iommu_release_mapping(mmu->mapping);
> + }
> +
> + return ret;
> +}
> +static inline void ipmmu_detach(struct device *dev)
> +{
> + arm_iommu_detach_device(dev);
> +}
> +static inline void ipmmu_release_mapping(struct ipmmu_vmsa_device *mmu)
> +{
> + arm_iommu_release_mapping(mmu->mapping);
> +}
> +#else
> +static inline int ipmmu_map_attach(struct device *dev,
> + struct ipmmu_vmsa_device *mmu)
> +{
> + return 0;
> +}
> +static inline void ipmmu_detach(struct device *dev) {}
> +static inline void ipmmu_release_mapping(struct ipmmu_vmsa_device *mmu) {}
> +#endif
> +
> static int ipmmu_add_device(struct device *dev)
> {
> struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
> @@ -701,41 +759,13 @@ static int ipmmu_add_device(struct devic
> dev_data->num_utlbs = num_utlbs;
> set_dev_data(dev, dev_data);
>
> - /*
> - * Create the ARM mapping, used by the ARM DMA mapping core to allocate
> - * VAs. This will allocate a corresponding IOMMU domain.
> - *
> - * TODO:
> - * - Create one mapping per context (TLB).
> - * - Make the mapping size configurable ? We currently use a 2GB mapping
> - * at a 1GB offset to ensure that NULL VAs will fault.
> - */
> - if (!mmu->mapping) {
> - struct dma_iommu_mapping *mapping;
> -
> - mapping = arm_iommu_create_mapping(&platform_bus_type,
> - SZ_1G, SZ_2G);
> - if (IS_ERR(mapping)) {
> - dev_err(mmu->dev, "failed to create ARM IOMMU mapping\n");
> - ret = PTR_ERR(mapping);
> - goto error;
> - }
> -
> - mmu->mapping = mapping;
> - }
> -
> - /* Attach the ARM VA mapping to the device. */
> - ret = arm_iommu_attach_device(dev, mmu->mapping);
> - if (ret < 0) {
> - dev_err(dev, "Failed to attach device to VA mapping\n");
> + ret = ipmmu_map_attach(dev, mmu);
> + if (ret < 0)
> goto error;
> - }
>
> return 0;
>
> error:
> - arm_iommu_release_mapping(mmu->mapping);
> -
> kfree(dev_data);
> kfree(utlbs);
>
> @@ -751,7 +781,7 @@ static void ipmmu_remove_device(struct d
> {
> struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
>
> - arm_iommu_detach_device(dev);
> + ipmmu_detach(dev);
> iommu_group_remove_device(dev);
>
> kfree(dev_data->utlbs);
> @@ -862,7 +892,7 @@ static int ipmmu_remove(struct platform_
> list_del(&mmu->list);
> spin_unlock(&ipmmu_devices_lock);
>
> - arm_iommu_release_mapping(mmu->mapping);
> + ipmmu_release_mapping(mmu);
>
> ipmmu_device_reset(mmu);
--
Regards,
Laurent Pinchart
Hi Magnus,
Thank you for the patch.
On Tuesday 15 December 2015 21:03:08 Magnus Damm wrote:
> From: Magnus Damm <[email protected]>
>
> Neither the ARM page table code enabled by IOMMU_IO_PGTABLE_LPAE
> nor the IPMMU_VMSA driver actually depends on ARM_LPAE, so get
> rid of the dependency.
>
> Tested with ipmmu-vmsa on r8a7794 ALT and a kernel config using:
> # CONFIG_ARM_LPAE is not set
>
> Signed-off-by: Magnus Damm <[email protected]>
Acked-by: Laurent Pinchart <[email protected]>
> ---
>
> This time the result also compiles on x86. Need to be
> applied as last patch in the following series:
> [PATCH 00/06] iommu/ipmmu-vmsa: IPMMU multi-arch update
>
> drivers/iommu/Kconfig | 1 -
> 1 file changed, 1 deletion(-)
>
> --- 0001/drivers/iommu/Kconfig
> +++ work/drivers/iommu/Kconfig 2015-10-18 14:58:09.080513000 +0900
> @@ -324,7 +324,6 @@ config SHMOBILE_IOMMU_L1SIZE
>
> config IPMMU_VMSA
> bool "Renesas VMSA-compatible IPMMU"
> - depends on ARM_LPAE
> depends on ARCH_SHMOBILE || COMPILE_TEST
> select IOMMU_API
> select IOMMU_IO_PGTABLE_LPAE
--
Regards,
Laurent Pinchart
Hi Laurent,
On Tue, Dec 29, 2015 at 9:14 AM, Laurent Pinchart
<[email protected]> wrote:
> Hi Magnus,
>
> Thank you for the patch.
Thanks for your feedback!
> On Tuesday 15 December 2015 21:02:49 Magnus Damm wrote:
>> From: Magnus Damm <[email protected]>
>>
>> Introduce a bitmap for context handing and convert the
>> interrupt routine to go handle all registered contexts.
>>
>> At this point the number of contexts are still limited.
>
> That's all nice, but without seeing support for multiple contexts it's hard to
> tell if the implementation is correct for multiple context purpose.
>
>> The purpose of this patch is to remove the use of the
>> ARM specific mapping variable from ipmmu_irq().
>
> Why do you want to do that ?
The purpose of this series is to be able to use the IPMMU driver on
other architectures than 32-bit ARM. The main goal is to use the
driver on 64-bit ARM where the mapping variable does not exist.
>> Signed-off-by: Magnus Damm <[email protected]>
>> ---
>>
>> drivers/iommu/ipmmu-vmsa.c | 37 ++++++++++++++++++++++++++-----------
>> 1 file changed, 26 insertions(+), 11 deletions(-)
>>
>> --- 0007/drivers/iommu/ipmmu-vmsa.c
>> +++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-15 13:14:35.540513000 +0900
>> @@ -8,6 +8,7 @@
>> * the Free Software Foundation; version 2 of the License.
>> */
>>
>> +#include <linux/bitmap.h>
>> #include <linux/delay.h>
>> #include <linux/dma-mapping.h>
>> #include <linux/err.h>
>> @@ -26,12 +27,16 @@
>>
>> #include "io-pgtable.h"
>>
>> +#define IPMMU_CTX_MAX 1
>> +
>> struct ipmmu_vmsa_device {
>> struct device *dev;
>> void __iomem *base;
>> struct list_head list;
>>
>> unsigned int num_utlbs;
>> + DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
>
> We have up to 4 context on Gen2 and 8 on Gen3, a bitmap might be slightly
> overkill.
Maybe so, but I'd rather use something standard than rolling my own.
Can you think of a better data structure?
>> + struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
>>
>> struct dma_iommu_mapping *mapping;
>> };
>> @@ -319,6 +324,7 @@ static struct iommu_gather_ops ipmmu_gat
>> static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
>> {
>> phys_addr_t ttbr;
>> + int ret;
>>
>> /*
>> * Allocate the page table operations.
>> @@ -348,10 +354,16 @@ static int ipmmu_domain_init_context(str
>> return -EINVAL;
>>
>> /*
>> - * TODO: When adding support for multiple contexts, find an unused
>> - * context.
>> + * Find an unused context.
>
> We need to support multiple devices per context or we will very soon run out
> of contexts. How to pick a proper context is a topic that needs to be
> researched, I believe IOMMU groups might come into play.
The experimental 64-bit ARM patches for this driver (on top of this
series) that I've posted makes use of IOMMU groups.
>> */
>> - domain->context_id = 0;
>> + ret = bitmap_find_free_region(domain->mmu->ctx, IPMMU_CTX_MAX, 0);
>> + if (ret < 0) {
>> + free_io_pgtable_ops(domain->iop);
>> + return ret;
>> + }
>> +
>> + domain->context_id = ret;
>> + domain->mmu->domains[ret] = domain;
>
> This requires locking to protect against races with the interrupt handler.
Hm, it seems that I mistakenly assumed that bitmap_find_free_region()
was built on top of atomic set_bit() and managed the bitmap in an
atomic way. I believe you are correct that locking is needed. Will
fix.
>>
>> /* TTBR0 */
>> ttbr = domain->cfg.arm_lpae_s1_cfg.ttbr[0];
>> @@ -395,6 +407,8 @@ static int ipmmu_domain_init_context(str
>>
>> static void ipmmu_domain_destroy_context(struct ipmmu_vmsa_domain *domain)
>> {
>> + bitmap_release_region(domain->mmu->ctx, domain->context_id, 0);
>> +
>> /*
>> * Disable the context. Flush the TLB as required when modifying the
>> * context registers.
>> @@ -460,16 +474,16 @@ static irqreturn_t ipmmu_domain_irq(stru
>> static irqreturn_t ipmmu_irq(int irq, void *dev)
>> {
>> struct ipmmu_vmsa_device *mmu = dev;
>> - struct iommu_domain *io_domain;
>> - struct ipmmu_vmsa_domain *domain;
>> -
>> - if (!mmu->mapping)
>> - return IRQ_NONE;
>> + irqreturn_t status = IRQ_NONE;
>> + unsigned int k;
>
> i is a perfectly fine loop counter :-)
>
>> - io_domain = mmu->mapping->domain;
>> - domain = to_vmsa_domain(io_domain);
>> + /* Check interrupts for all active contexts */
>> + for (k = find_first_bit(mmu->ctx, IPMMU_CTX_MAX);
>> + k < IPMMU_CTX_MAX && status == IRQ_NONE;
>> + k = find_next_bit(mmu->ctx, IPMMU_CTX_MAX, k))
>
> You can just loop over mmu->domains and skip NULL entries.
You are right, that may be easier!
>> + status = ipmmu_domain_irq(mmu->domains[k]);
>
> Only the status of the last domain is taken into account.
Will fix, thanks!
/ magnus