iommu/ipmmu-vmsa: IPMMU multi-arch update V7
[PATCH v7 01/07] iommu/ipmmu-vmsa: Remove platform data handling
[PATCH v7 02/07] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context
[PATCH v7 03/07] iommu/ipmmu-vmsa: Break out utlb parsing code
[PATCH v7 04/07] iommu/ipmmu-vmsa: Break out domain allocation code
[PATCH v7 05/07] iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops
[PATCH v7 06/07] iommu/ipmmu-vmsa: ARM and ARM64 archdata access
[PATCH v7 07/07] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency
These patches update the IPMMU driver with a couple of changes to support
build on multiple architectures. In the process of doing so the interrupt
code gets reworked, the foundation for supporting multiple contexts are
added and in case of CONFIG_IOMMU_DMA=y (on 64-bit or 32-bit ARM) devices
are grouped together and handled via ->xlate() - thanks Robin! Support for
existing 32-bit ARM SoCs from R-Car Gen2 is kept as-is.
Changes since V6:
- Rebased on top of v4.11-rc1 and the following fast-tracked change:
3b6bb5b iommu/ipmmu-vmsa: Restrict IOMMU Domain Geometry to 32-bit address spac
- Updated patch 5/7 to roll in a few patches from other series
See individual patch for more details
- Build tested on 32-bit and 64-bit ARM
- Run time tested on 64-bit ARM (with additional SoC-specific patches)
Changes since V5:
- Rebased series on top of next-20161019
- Updated patch 5/7 to simplify domain allocation/free code - thanks Joerg!
- Added reviewed-by tag from Joerg for patch 1-4 and 6-7.
Changes since V4:
- Updated patch 3/7 to work on top on the following commit in next-20160920:
b1e2afc iommu/ipmmu-vmsa: Fix wrong error handle of ipmmu_add_device
- Add Kconfig hunk to patch 5/7 to avoid undeclared ipmmu_ops if COMPILE_TEST
- Rebased patch 7/7 to fit on top of new Kconfig bits in 5/7
Changes since V3:
- Updated patch 3/7 to fix hang-on-boot issue on 32-bit ARM - thanks Geert!
- Reworked group parameter handling in patch 3/7 and 5/7.
- Added patch 6/7 to fix build of the driver on s390/tile/um architectures
Changes since V2:
- Got rid of patch 3 from the V2 however patch 1, 2 and 4 are kept.
- V3 patch 3, 4 and 5 come from
[PATCH 00/04] iommu/ipmmu-vmsa: IPMMU CONFIG_IOMMU_DMA update
- Patch 5 has been reworked to include patch 3 of the V1 of this series
Changes since V1:
- Got rid of patch 2 and 3 from initial series
- Updated bitmap code locking and also used lighter bitop functions
- Updated the Kconfig bits to apply on top of ARCH_RENESAS
Signed-off-by: Magnus Damm <[email protected]>
---
Built on top v4.11-rc1:
drivers/iommu/Kconfig | 2
drivers/iommu/ipmmu-vmsa.c | 359 ++++++++++++++++++++++++++++++++++++--------
2 files changed, 299 insertions(+), 62 deletions(-)
From: Magnus Damm <[email protected]>
Introduce an alternative set of iommu_ops suitable for 64-bit ARM
as well as 32-bit ARM when CONFIG_IOMMU_DMA=y. Also adjust the
Kconfig to depend on ARM or IOMMU_DMA. Initialize the device
from ->xlate() when CONFIG_IOMMU_DMA=y.
Signed-off-by: Magnus Damm <[email protected]>
---
Changes since V6:
- Rolled in the following patches from "r8a7795 support V2":
[PATCH v2 04/11] iommu/ipmmu-vmsa: Reuse iommu groups
[PATCH v2 06/11] iommu/ipmmu-vmsa: Teach xlate() to skip disabled iommus
- Moved find_group() implementation to prevent warning on 32-bit ARM
- Rolled in the following patch from "IPMMU slave device whitelist V2":
[PATCH/RFC v2 3/4] iommu/ipmmu-vmsa: Check devices in xlate()
drivers/iommu/Kconfig | 1
drivers/iommu/ipmmu-vmsa.c | 164 +++++++++++++++++++++++++++++++++++++++++---
2 files changed, 157 insertions(+), 8 deletions(-)
--- 0001/drivers/iommu/Kconfig
+++ work/drivers/iommu/Kconfig 2017-03-06 18:42:42.000000000 +0900
@@ -274,6 +274,7 @@ config EXYNOS_IOMMU_DEBUG
config IPMMU_VMSA
bool "Renesas VMSA-compatible IPMMU"
+ depends on ARM || IOMMU_DMA
depends on ARM_LPAE
depends on ARCH_RENESAS || COMPILE_TEST
select IOMMU_API
--- 0009/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2017-03-06 19:22:27.700607110 +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>
@@ -22,8 +23,10 @@
#include <linux/sizes.h>
#include <linux/slab.h>
+#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
#include <asm/dma-iommu.h>
#include <asm/pgalloc.h>
+#endif
#include "io-pgtable.h"
@@ -57,6 +60,8 @@ struct ipmmu_vmsa_archdata {
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);
@@ -522,14 +527,6 @@ static struct iommu_domain *__ipmmu_doma
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);
@@ -572,6 +569,9 @@ static int ipmmu_attach_device(struct io
dev_err(dev, "Can't attach IPMMU %s to domain on IPMMU %s\n",
dev_name(mmu->dev), dev_name(domain->mmu->dev));
ret = -EINVAL;
+ } else {
+ dev_info(dev, "Reusing IPMMU context %u\n",
+ domain->context_id);
}
spin_unlock_irqrestore(&domain->lock, flags);
@@ -708,6 +708,7 @@ static int ipmmu_init_platform_device(st
archdata->mmu = mmu;
archdata->utlbs = utlbs;
archdata->num_utlbs = num_utlbs;
+ archdata->dev = dev;
dev->archdata.iommu = archdata;
return 0;
@@ -716,6 +717,16 @@ error:
return ret;
}
+#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_archdata *archdata;
@@ -823,6 +834,141 @@ static const struct iommu_ops ipmmu_ops
.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
};
+#endif /* !CONFIG_ARM && CONFIG_IOMMU_DMA */
+
+#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;
+
+ 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 ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+ struct iommu_group *group;
+
+ /* The device has been verified in xlate() */
+ if (!archdata)
+ return -ENODEV;
+
+ group = iommu_group_get_for_dev(dev);
+ if (IS_ERR(group))
+ return PTR_ERR(group);
+
+ spin_lock(&ipmmu_slave_devices_lock);
+ list_add(&archdata->list, &ipmmu_slave_devices);
+ spin_unlock(&ipmmu_slave_devices_lock);
+ return 0;
+}
+
+static void ipmmu_remove_device_dma(struct device *dev)
+{
+ struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+
+ spin_lock(&ipmmu_slave_devices_lock);
+ list_del(&archdata->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_archdata *archdata = dev->archdata.iommu;
+ struct ipmmu_vmsa_archdata *sibling_archdata = NULL;
+ bool found = false;
+
+ spin_lock(&ipmmu_slave_devices_lock);
+
+ list_for_each_entry(sibling_archdata, &ipmmu_slave_devices, list) {
+ if (archdata == sibling_archdata)
+ continue;
+ if (sibling_archdata->mmu == archdata->mmu) {
+ found = true;
+ break;
+ }
+ }
+
+ spin_unlock(&ipmmu_slave_devices_lock);
+
+ return found ? sibling_archdata->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);
+
+ 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,
+ .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_find_group_dma,
+ .pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
+ .of_xlate = ipmmu_of_xlate_dma,
+};
+
+#endif /* CONFIG_IOMMU_DMA */
+
/* -----------------------------------------------------------------------------
* Probe/remove and init
*/
@@ -912,7 +1058,9 @@ static int ipmmu_remove(struct platform_
list_del(&mmu->list);
spin_unlock(&ipmmu_devices_lock);
+#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
arm_iommu_release_mapping(mmu->mapping);
+#endif
ipmmu_device_reset(mmu);
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]>
Reviewed-by: Joerg Roedel <[email protected]>
---
Changes since V6:
- None
drivers/iommu/ipmmu-vmsa.c | 5 -----
1 file changed, 5 deletions(-)
--- 0001/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2017-03-06 17:25:49.000000000 +0900
@@ -768,11 +768,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]>
Introduce a bitmap for context handing and convert the
interrupt routine to handle all registered contexts.
At this point the number of contexts are still limited.
Also remove the use of the ARM specific mapping variable
from ipmmu_irq() to allow compile on ARM64.
Signed-off-by: Magnus Damm <[email protected]>
Reviewed-by: Joerg Roedel <[email protected]>
---
Changes since V6:
- None
drivers/iommu/ipmmu-vmsa.c | 76 ++++++++++++++++++++++++++++++++++++++------
1 file changed, 66 insertions(+), 10 deletions(-)
--- 0003/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2017-03-06 18:32:38.350607110 +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,17 @@
#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;
+ spinlock_t lock; /* Protects ctx and domains[] */
+ DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
+ struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
struct dma_iommu_mapping *mapping;
};
@@ -293,9 +299,29 @@ static struct iommu_gather_ops ipmmu_gat
* Domain/Context Management
*/
+static int ipmmu_domain_allocate_context(struct ipmmu_vmsa_device *mmu,
+ struct ipmmu_vmsa_domain *domain)
+{
+ unsigned long flags;
+ int ret;
+
+ spin_lock_irqsave(&mmu->lock, flags);
+
+ ret = find_first_zero_bit(mmu->ctx, IPMMU_CTX_MAX);
+ if (ret != IPMMU_CTX_MAX) {
+ mmu->domains[ret] = domain;
+ set_bit(ret, mmu->ctx);
+ }
+
+ spin_unlock_irqrestore(&mmu->lock, flags);
+
+ return ret;
+}
+
static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
{
u64 ttbr;
+ int ret;
/*
* Allocate the page table operations.
@@ -327,10 +353,15 @@ 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 = ipmmu_domain_allocate_context(domain->mmu, domain);
+ if (ret == IPMMU_CTX_MAX) {
+ free_io_pgtable_ops(domain->iop);
+ return -EBUSY;
+ }
+
+ domain->context_id = ret;
/* TTBR0 */
ttbr = domain->cfg.arm_lpae_s1_cfg.ttbr[0];
@@ -372,6 +403,19 @@ static int ipmmu_domain_init_context(str
return 0;
}
+static void ipmmu_domain_free_context(struct ipmmu_vmsa_device *mmu,
+ unsigned int context_id)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&mmu->lock, flags);
+
+ clear_bit(context_id, mmu->ctx);
+ mmu->domains[context_id] = NULL;
+
+ spin_unlock_irqrestore(&mmu->lock, flags);
+}
+
static void ipmmu_domain_destroy_context(struct ipmmu_vmsa_domain *domain)
{
/*
@@ -382,6 +426,7 @@ static void ipmmu_domain_destroy_context
*/
ipmmu_ctx_write(domain, IMCTR, IMCTR_FLUSH);
ipmmu_tlb_sync(domain);
+ ipmmu_domain_free_context(domain->mmu, domain->context_id);
}
/* -----------------------------------------------------------------------------
@@ -439,16 +484,25 @@ 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;
+ irqreturn_t status = IRQ_NONE;
+ unsigned int i;
+ unsigned long flags;
- if (!mmu->mapping)
- return IRQ_NONE;
+ spin_lock_irqsave(&mmu->lock, flags);
+
+ /*
+ * Check interrupts for all active contexts.
+ */
+ for (i = 0; i < IPMMU_CTX_MAX; i++) {
+ if (!mmu->domains[i])
+ continue;
+ if (ipmmu_domain_irq(mmu->domains[i]) == IRQ_HANDLED)
+ status = IRQ_HANDLED;
+ }
- io_domain = mmu->mapping->domain;
- domain = to_vmsa_domain(io_domain);
+ spin_unlock_irqrestore(&mmu->lock, flags);
- return ipmmu_domain_irq(domain);
+ return status;
}
/* -----------------------------------------------------------------------------
@@ -776,6 +830,8 @@ static int ipmmu_probe(struct platform_d
mmu->dev = &pdev->dev;
mmu->num_utlbs = 32;
+ spin_lock_init(&mmu->lock);
+ 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]>
Not all architectures have an iommu member in their archdata, so
use #ifdefs support build with COMPILE_TEST on any architecture.
Signed-off-by: Magnus Damm <[email protected]>
Reviewed-by: Joerg Roedel <[email protected]>
---
Changes since V6:
- Updated patch to handle newly introduced functions in:
[PATCH v7 05/07] iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops
drivers/iommu/ipmmu-vmsa.c | 43 ++++++++++++++++++++++++++++++-------------
1 file changed, 30 insertions(+), 13 deletions(-)
--- 0010/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2017-03-06 19:26:26.070607110 +0900
@@ -72,6 +72,25 @@ static struct ipmmu_vmsa_domain *to_vmsa
return container_of(dom, struct ipmmu_vmsa_domain, io_domain);
}
+#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
+static struct ipmmu_vmsa_archdata *to_archdata(struct device *dev)
+{
+ return dev->archdata.iommu;
+}
+static void set_archdata(struct device *dev, struct ipmmu_vmsa_archdata *p)
+{
+ dev->archdata.iommu = p;
+}
+#else
+static struct ipmmu_vmsa_archdata *to_archdata(struct device *dev)
+{
+ return NULL;
+}
+static void set_archdata(struct device *dev, struct ipmmu_vmsa_archdata *p)
+{
+}
+#endif
+
#define TLB_LOOP_TIMEOUT 100 /* 100us */
/* -----------------------------------------------------------------------------
@@ -543,7 +562,7 @@ 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_archdata *archdata = to_archdata(dev);
struct ipmmu_vmsa_device *mmu = archdata->mmu;
struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
unsigned long flags;
@@ -588,7 +607,7 @@ 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_archdata *archdata = to_archdata(dev);
struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
unsigned int i;
@@ -709,7 +728,7 @@ static int ipmmu_init_platform_device(st
archdata->utlbs = utlbs;
archdata->num_utlbs = num_utlbs;
archdata->dev = dev;
- dev->archdata.iommu = archdata;
+ set_archdata(dev, archdata);
return 0;
error:
@@ -729,12 +748,11 @@ static struct iommu_domain *ipmmu_domain
static int ipmmu_add_device(struct device *dev)
{
- struct ipmmu_vmsa_archdata *archdata;
struct ipmmu_vmsa_device *mmu = NULL;
struct iommu_group *group;
int ret;
- if (dev->archdata.iommu) {
+ if (to_archdata(dev)) {
dev_warn(dev, "IOMMU driver already assigned to device %s\n",
dev_name(dev));
return -EINVAL;
@@ -770,8 +788,7 @@ static int ipmmu_add_device(struct devic
* - Make the mapping size configurable ? We currently use a 2GB mapping
* at a 1GB offset to ensure that NULL VAs will fault.
*/
- archdata = dev->archdata.iommu;
- mmu = archdata->mmu;
+ mmu = to_archdata(dev)->mmu;
if (!mmu->mapping) {
struct dma_iommu_mapping *mapping;
@@ -799,7 +816,7 @@ error:
if (mmu)
arm_iommu_release_mapping(mmu->mapping);
- dev->archdata.iommu = NULL;
+ set_archdata(dev, NULL);
if (!IS_ERR_OR_NULL(group))
iommu_group_remove_device(dev);
@@ -809,7 +826,7 @@ error:
static void ipmmu_remove_device(struct device *dev)
{
- struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+ struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
arm_iommu_detach_device(dev);
iommu_group_remove_device(dev);
@@ -817,7 +834,7 @@ static void ipmmu_remove_device(struct d
kfree(archdata->utlbs);
kfree(archdata);
- dev->archdata.iommu = NULL;
+ set_archdata(dev, NULL);
}
static const struct iommu_ops ipmmu_ops = {
@@ -874,7 +891,7 @@ static void ipmmu_domain_free_dma(struct
static int ipmmu_add_device_dma(struct device *dev)
{
- struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+ struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
struct iommu_group *group;
/* The device has been verified in xlate() */
@@ -893,7 +910,7 @@ static int ipmmu_add_device_dma(struct d
static void ipmmu_remove_device_dma(struct device *dev)
{
- struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+ struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
spin_lock(&ipmmu_slave_devices_lock);
list_del(&archdata->list);
@@ -904,7 +921,7 @@ static void ipmmu_remove_device_dma(stru
static struct device *ipmmu_find_sibling_device(struct device *dev)
{
- struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+ struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
struct ipmmu_vmsa_archdata *sibling_archdata = NULL;
bool found = false;
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]>
Reviewed-by: Joerg Roedel <[email protected]>
---
Changes since V6:
- None
drivers/iommu/ipmmu-vmsa.c | 58 ++++++++++++++++++++++++++++----------------
1 file changed, 37 insertions(+), 21 deletions(-)
--- 0004/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2017-03-06 18:33:38.600607110 +0900
@@ -649,22 +649,15 @@ static int ipmmu_find_utlbs(struct ipmmu
return 0;
}
-static int ipmmu_add_device(struct device *dev)
+static int ipmmu_init_platform_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;
- if (dev->archdata.iommu) {
- dev_warn(dev, "IOMMU driver already assigned to device %s\n",
- dev_name(dev));
- return -EINVAL;
- }
-
/* Find the master corresponding to the device. */
num_utlbs = of_count_phandle_with_args(dev->of_node, "iommus",
@@ -701,6 +694,36 @@ static int ipmmu_add_device(struct devic
}
}
+ 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;
+}
+
+static int ipmmu_add_device(struct device *dev)
+{
+ struct ipmmu_vmsa_archdata *archdata;
+ struct ipmmu_vmsa_device *mmu = NULL;
+ struct iommu_group *group;
+ int ret;
+
+ if (dev->archdata.iommu) {
+ dev_warn(dev, "IOMMU driver already assigned to device %s\n",
+ dev_name(dev));
+ return -EINVAL;
+ }
+
/* Create a device group and add the device to it. */
group = iommu_group_alloc();
if (IS_ERR(group)) {
@@ -718,16 +741,9 @@ 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);
+ if (ret < 0)
goto error;
- }
-
- archdata->mmu = mmu;
- archdata->utlbs = utlbs;
- archdata->num_utlbs = num_utlbs;
- dev->archdata.iommu = archdata;
/*
* Create the ARM mapping, used by the ARM DMA mapping core to allocate
@@ -738,6 +754,8 @@ static int ipmmu_add_device(struct devic
* - Make the mapping size configurable ? We currently use a 2GB mapping
* at a 1GB offset to ensure that NULL VAs will fault.
*/
+ archdata = dev->archdata.iommu;
+ mmu = archdata->mmu;
if (!mmu->mapping) {
struct dma_iommu_mapping *mapping;
@@ -762,10 +780,8 @@ static int ipmmu_add_device(struct devic
return 0;
error:
- arm_iommu_release_mapping(mmu->mapping);
-
- kfree(dev->archdata.iommu);
- kfree(utlbs);
+ if (mmu)
+ arm_iommu_release_mapping(mmu->mapping);
dev->archdata.iommu = NULL;
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]>
Reviewed-by: Joerg Roedel <[email protected]>
---
Changes since V6:
- None
drivers/iommu/Kconfig | 1 -
1 file changed, 1 deletion(-)
--- 0011/drivers/iommu/Kconfig
+++ work/drivers/iommu/Kconfig 2017-03-06 18:50:49.260607110 +0900
@@ -275,7 +275,6 @@ config EXYNOS_IOMMU_DEBUG
config IPMMU_VMSA
bool "Renesas VMSA-compatible IPMMU"
depends on ARM || IOMMU_DMA
- depends on ARM_LPAE
depends on ARCH_RENESAS || COMPILE_TEST
select IOMMU_API
select IOMMU_IO_PGTABLE_LPAE
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]>
Reviewed-by: Joerg Roedel <[email protected]>
---
Changes since V6:
- None
drivers/iommu/ipmmu-vmsa.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
--- 0006/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2017-03-06 18:34:59.520607110 +0900
@@ -509,13 +509,10 @@ static irqreturn_t ipmmu_irq(int irq, vo
* 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;
@@ -525,6 +522,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);
Hi Magnus,
On Tue, Mar 7, 2017 at 4:17 AM, Magnus Damm <[email protected]> wrote:
> From: Magnus Damm <[email protected]>
>
> Not all architectures have an iommu member in their archdata, so
> use #ifdefs support build with COMPILE_TEST on any architecture.
to support
> Signed-off-by: Magnus Damm <[email protected]>
> Reviewed-by: Joerg Roedel <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
> --- 0010/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c 2017-03-06 19:26:26.070607110 +0900
> @@ -72,6 +72,25 @@ static struct ipmmu_vmsa_domain *to_vmsa
> return container_of(dom, struct ipmmu_vmsa_domain, io_domain);
> }
>
> +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
> +static struct ipmmu_vmsa_archdata *to_archdata(struct device *dev)
> +{
> + return dev->archdata.iommu;
> +}
> +static void set_archdata(struct device *dev, struct ipmmu_vmsa_archdata *p)
> +{
> + dev->archdata.iommu = p;
> +}
> +#else
#else /* compile testing */
> +static struct ipmmu_vmsa_archdata *to_archdata(struct device *dev)
> +{
> + return NULL;
> +}
> +static void set_archdata(struct device *dev, struct ipmmu_vmsa_archdata *p)
> +{
> +}
> +#endif
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
On Tue, Mar 7, 2017 at 4:17 AM, Magnus Damm <[email protected]> wrote:
> 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]>
> Reviewed-by: Joerg Roedel <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
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
On Tue, Mar 7, 2017 at 4:16 AM, Magnus Damm <[email protected]> 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]>
> Reviewed-by: Joerg Roedel <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
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
On 07/03/17 03:17, Magnus Damm wrote:
> From: Magnus Damm <[email protected]>
>
> Not all architectures have an iommu member in their archdata, so
> use #ifdefs support build with COMPILE_TEST on any architecture.
I have a feeling I might be repeating myself, but ipmmu_vmsa_archdata
looks to be trivially convertible to iommu_fwspec, which I strongly
encourage, not least because it would obviate bodges like this.
Robin.
> Signed-off-by: Magnus Damm <[email protected]>
> Reviewed-by: Joerg Roedel <[email protected]>
> ---
>
> Changes since V6:
> - Updated patch to handle newly introduced functions in:
> [PATCH v7 05/07] iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops
>
> drivers/iommu/ipmmu-vmsa.c | 43 ++++++++++++++++++++++++++++++-------------
> 1 file changed, 30 insertions(+), 13 deletions(-)
>
> --- 0010/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c 2017-03-06 19:26:26.070607110 +0900
> @@ -72,6 +72,25 @@ static struct ipmmu_vmsa_domain *to_vmsa
> return container_of(dom, struct ipmmu_vmsa_domain, io_domain);
> }
>
> +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
> +static struct ipmmu_vmsa_archdata *to_archdata(struct device *dev)
> +{
> + return dev->archdata.iommu;
> +}
> +static void set_archdata(struct device *dev, struct ipmmu_vmsa_archdata *p)
> +{
> + dev->archdata.iommu = p;
> +}
> +#else
> +static struct ipmmu_vmsa_archdata *to_archdata(struct device *dev)
> +{
> + return NULL;
> +}
> +static void set_archdata(struct device *dev, struct ipmmu_vmsa_archdata *p)
> +{
> +}
> +#endif
> +
> #define TLB_LOOP_TIMEOUT 100 /* 100us */
>
> /* -----------------------------------------------------------------------------
> @@ -543,7 +562,7 @@ 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_archdata *archdata = to_archdata(dev);
> struct ipmmu_vmsa_device *mmu = archdata->mmu;
> struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
> unsigned long flags;
> @@ -588,7 +607,7 @@ 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_archdata *archdata = to_archdata(dev);
> struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
> unsigned int i;
>
> @@ -709,7 +728,7 @@ static int ipmmu_init_platform_device(st
> archdata->utlbs = utlbs;
> archdata->num_utlbs = num_utlbs;
> archdata->dev = dev;
> - dev->archdata.iommu = archdata;
> + set_archdata(dev, archdata);
> return 0;
>
> error:
> @@ -729,12 +748,11 @@ static struct iommu_domain *ipmmu_domain
>
> static int ipmmu_add_device(struct device *dev)
> {
> - struct ipmmu_vmsa_archdata *archdata;
> struct ipmmu_vmsa_device *mmu = NULL;
> struct iommu_group *group;
> int ret;
>
> - if (dev->archdata.iommu) {
> + if (to_archdata(dev)) {
> dev_warn(dev, "IOMMU driver already assigned to device %s\n",
> dev_name(dev));
> return -EINVAL;
> @@ -770,8 +788,7 @@ static int ipmmu_add_device(struct devic
> * - Make the mapping size configurable ? We currently use a 2GB mapping
> * at a 1GB offset to ensure that NULL VAs will fault.
> */
> - archdata = dev->archdata.iommu;
> - mmu = archdata->mmu;
> + mmu = to_archdata(dev)->mmu;
> if (!mmu->mapping) {
> struct dma_iommu_mapping *mapping;
>
> @@ -799,7 +816,7 @@ error:
> if (mmu)
> arm_iommu_release_mapping(mmu->mapping);
>
> - dev->archdata.iommu = NULL;
> + set_archdata(dev, NULL);
>
> if (!IS_ERR_OR_NULL(group))
> iommu_group_remove_device(dev);
> @@ -809,7 +826,7 @@ error:
>
> static void ipmmu_remove_device(struct device *dev)
> {
> - struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> + struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
>
> arm_iommu_detach_device(dev);
> iommu_group_remove_device(dev);
> @@ -817,7 +834,7 @@ static void ipmmu_remove_device(struct d
> kfree(archdata->utlbs);
> kfree(archdata);
>
> - dev->archdata.iommu = NULL;
> + set_archdata(dev, NULL);
> }
>
> static const struct iommu_ops ipmmu_ops = {
> @@ -874,7 +891,7 @@ static void ipmmu_domain_free_dma(struct
>
> static int ipmmu_add_device_dma(struct device *dev)
> {
> - struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> + struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
> struct iommu_group *group;
>
> /* The device has been verified in xlate() */
> @@ -893,7 +910,7 @@ static int ipmmu_add_device_dma(struct d
>
> static void ipmmu_remove_device_dma(struct device *dev)
> {
> - struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> + struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
>
> spin_lock(&ipmmu_slave_devices_lock);
> list_del(&archdata->list);
> @@ -904,7 +921,7 @@ static void ipmmu_remove_device_dma(stru
>
> static struct device *ipmmu_find_sibling_device(struct device *dev)
> {
> - struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> + struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
> struct ipmmu_vmsa_archdata *sibling_archdata = NULL;
> bool found = false;
>
>
On 07/03/17 03:17, Magnus Damm wrote:
> From: Magnus Damm <[email protected]>
>
> Introduce an alternative set of iommu_ops suitable for 64-bit ARM
> as well as 32-bit ARM when CONFIG_IOMMU_DMA=y. Also adjust the
> Kconfig to depend on ARM or IOMMU_DMA. Initialize the device
> from ->xlate() when CONFIG_IOMMU_DMA=y.
>
> Signed-off-by: Magnus Damm <[email protected]>
> ---
>
> Changes since V6:
> - Rolled in the following patches from "r8a7795 support V2":
> [PATCH v2 04/11] iommu/ipmmu-vmsa: Reuse iommu groups
> [PATCH v2 06/11] iommu/ipmmu-vmsa: Teach xlate() to skip disabled iommus
> - Moved find_group() implementation to prevent warning on 32-bit ARM
> - Rolled in the following patch from "IPMMU slave device whitelist V2":
> [PATCH/RFC v2 3/4] iommu/ipmmu-vmsa: Check devices in xlate()
>
> drivers/iommu/Kconfig | 1
> drivers/iommu/ipmmu-vmsa.c | 164 +++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 157 insertions(+), 8 deletions(-)
>
> --- 0001/drivers/iommu/Kconfig
> +++ work/drivers/iommu/Kconfig 2017-03-06 18:42:42.000000000 +0900
> @@ -274,6 +274,7 @@ config EXYNOS_IOMMU_DEBUG
>
> config IPMMU_VMSA
> bool "Renesas VMSA-compatible IPMMU"
> + depends on ARM || IOMMU_DMA
> depends on ARM_LPAE
> depends on ARCH_RENESAS || COMPILE_TEST
> select IOMMU_API
> --- 0009/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c 2017-03-06 19:22:27.700607110 +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>
> @@ -22,8 +23,10 @@
> #include <linux/sizes.h>
> #include <linux/slab.h>
>
> +#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
> #include <asm/dma-iommu.h>
> #include <asm/pgalloc.h>
> +#endif
>
> #include "io-pgtable.h"
>
> @@ -57,6 +60,8 @@ struct ipmmu_vmsa_archdata {
> 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);
> @@ -522,14 +527,6 @@ static struct iommu_domain *__ipmmu_doma
> 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);
> @@ -572,6 +569,9 @@ static int ipmmu_attach_device(struct io
> dev_err(dev, "Can't attach IPMMU %s to domain on IPMMU %s\n",
> dev_name(mmu->dev), dev_name(domain->mmu->dev));
> ret = -EINVAL;
> + } else {
> + dev_info(dev, "Reusing IPMMU context %u\n",
> + domain->context_id);
Indentation?
> }
>
> spin_unlock_irqrestore(&domain->lock, flags);
> @@ -708,6 +708,7 @@ static int ipmmu_init_platform_device(st
> archdata->mmu = mmu;
> archdata->utlbs = utlbs;
> archdata->num_utlbs = num_utlbs;
> + archdata->dev = dev;
> dev->archdata.iommu = archdata;
> return 0;
>
> @@ -716,6 +717,16 @@ error:
> return ret;
> }
>
> +#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_archdata *archdata;
> @@ -823,6 +834,141 @@ static const struct iommu_ops ipmmu_ops
> .pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
> };
>
> +#endif /* !CONFIG_ARM && CONFIG_IOMMU_DMA */
> +
> +#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;
> +
> + 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;
> +}
I still think it would be tidier to put this logic straight into
__ipmmu_domain_alloc(), and use that directly as the callback for this
case. The ipmmu_domain_alloc() wrapper ensures that IOMMU_DOMAIN_DMA
can't be passed through in the legacy 32-bit case, and the cookie calls
are stubbed for !CONFIG_IOMMU_DMA so there are no build concerns.
> +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;
> + }
> +}
And similarly. The day when 32-bit ARM gets cleaned up to use groups and
default domains properly creeps ever closer (the probe deferral series
looks on-track to finally get in this cycle), so the less fragmentation
and #ifdeffery to untangle at that point the better.
> +static int ipmmu_add_device_dma(struct device *dev)
> +{
> + struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> + struct iommu_group *group;
> +
> + /* The device has been verified in xlate() */
> + if (!archdata)
> + return -ENODEV;
> +
> + group = iommu_group_get_for_dev(dev);
> + if (IS_ERR(group))
> + return PTR_ERR(group);
> +
> + spin_lock(&ipmmu_slave_devices_lock);
> + list_add(&archdata->list, &ipmmu_slave_devices);
> + spin_unlock(&ipmmu_slave_devices_lock);
> + return 0;
> +}
> +
> +static void ipmmu_remove_device_dma(struct device *dev)
> +{
> + struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> +
> + spin_lock(&ipmmu_slave_devices_lock);
> + list_del(&archdata->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_archdata *archdata = dev->archdata.iommu;
> + struct ipmmu_vmsa_archdata *sibling_archdata = NULL;
> + bool found = false;
> +
> + spin_lock(&ipmmu_slave_devices_lock);
> +
> + list_for_each_entry(sibling_archdata, &ipmmu_slave_devices, list) {
> + if (archdata == sibling_archdata)
> + continue;
> + if (sibling_archdata->mmu == archdata->mmu) {
So every master behind the same IPMMU gets put in the same group? In
that case, you don't need any of this machinery - you can simply keep
track of a group per IPMMU instance directly. See mtk_iommu.c for an
example.
> + found = true;
> + break;
> + }
> + }
> +
> + spin_unlock(&ipmmu_slave_devices_lock);
> +
> + return found ? sibling_archdata->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);
> +
> + 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 you're only calling iommu_device_register() from ipmmu_probe(), you
should never see that problem in the first place (because a disabled
node won't even get its platform device created, let alone probe a
driver). I guess this a leftover from when the init_fn once called
of_iommu_set_ops()?
Robin.
> + 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,
> + .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_find_group_dma,
> + .pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
> + .of_xlate = ipmmu_of_xlate_dma,
> +};
> +
> +#endif /* CONFIG_IOMMU_DMA */
> +
> /* -----------------------------------------------------------------------------
> * Probe/remove and init
> */
> @@ -912,7 +1058,9 @@ static int ipmmu_remove(struct platform_
> list_del(&mmu->list);
> spin_unlock(&ipmmu_devices_lock);
>
> +#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
> arm_iommu_release_mapping(mmu->mapping);
> +#endif
>
> ipmmu_device_reset(mmu);
>
>
Hi Robin,
On Wed, Mar 8, 2017 at 9:48 PM, Robin Murphy <[email protected]> wrote:
> On 07/03/17 03:17, Magnus Damm wrote:
>> From: Magnus Damm <[email protected]>
>>
>> Not all architectures have an iommu member in their archdata, so
>> use #ifdefs support build with COMPILE_TEST on any architecture.
>
> I have a feeling I might be repeating myself, but ipmmu_vmsa_archdata
> looks to be trivially convertible to iommu_fwspec, which I strongly
> encourage, not least because it would obviate bodges like this.
Yeah, I think it should be possible to use iommu_fwspec for this
purpose. The question is when to do it. =)
I actually looked into it recently, but then realised that for this to
work then due to code sharing I need to make use of iommu_fwspec on
both 32-bit and 64-bit ARM. So it requires rework of the existing
IPMMU for 32-bit ARM (including hairy legacy CONFIG_IOMMU_DMA=n code).
I was actually thinking of doing some rework of 32-bit ARM IPMMU code
anyway (I suspect iommu_device_* conversion caused breakage) and it
probably has to happen on top of current -next. I would also like to
start reducing burden of forward porting all these patches, and
stirring up the ground does not really help much there...
Cheers,
/ magnus
On 09/03/17 03:44, Magnus Damm wrote:
> Hi Robin,
>
> On Wed, Mar 8, 2017 at 9:48 PM, Robin Murphy <[email protected]> wrote:
>> On 07/03/17 03:17, Magnus Damm wrote:
>>> From: Magnus Damm <[email protected]>
>>>
>>> Not all architectures have an iommu member in their archdata, so
>>> use #ifdefs support build with COMPILE_TEST on any architecture.
>>
>> I have a feeling I might be repeating myself, but ipmmu_vmsa_archdata
>> looks to be trivially convertible to iommu_fwspec, which I strongly
>> encourage, not least because it would obviate bodges like this.
>
> Yeah, I think it should be possible to use iommu_fwspec for this
> purpose. The question is when to do it. =)
I'd actually be inclined to do it *before* any other major changes, as
it would be pretty minimal given the current structure of the driver.
But then I'm free to wilfully ignore the burden of maintaining patch
stacks between both mainline and older trees ;)
> I actually looked into it recently, but then realised that for this to
> work then due to code sharing I need to make use of iommu_fwspec on
> both 32-bit and 64-bit ARM. So it requires rework of the existing
> IPMMU for 32-bit ARM (including hairy legacy CONFIG_IOMMU_DMA=n code).
> I was actually thinking of doing some rework of 32-bit ARM IPMMU code
> anyway (I suspect iommu_device_* conversion caused breakage) and it
> probably has to happen on top of current -next. I would also like to
> start reducing burden of forward porting all these patches, and
> stirring up the ground does not really help much there...
Note that iommu_fwspec can be used pretty much orthogonally to any of
the core DMA ops support, so it shouldn't be as invasive as you might
think. See 84672f192671 ("iommu/mediatek: Convert M4Uv1 to
iommu_fwspec") as an example of an archdata-to-fwspec conversion of a
driver which only supports 32-bit ARM, and notably borrows its master
handling directly from the the IPMMU driver.
Robin.
>
> Cheers,
>
> / magnus
>
On Tue, Mar 07, 2017 at 12:17:33PM +0900, Magnus Damm wrote:
> From: Magnus Damm <[email protected]>
>
> Introduce an alternative set of iommu_ops suitable for 64-bit ARM
> as well as 32-bit ARM when CONFIG_IOMMU_DMA=y. Also adjust the
> Kconfig to depend on ARM or IOMMU_DMA. Initialize the device
> from ->xlate() when CONFIG_IOMMU_DMA=y.
>
> Signed-off-by: Magnus Damm <[email protected]>
> ---
>
> Changes since V6:
> - Rolled in the following patches from "r8a7795 support V2":
> [PATCH v2 04/11] iommu/ipmmu-vmsa: Reuse iommu groups
> [PATCH v2 06/11] iommu/ipmmu-vmsa: Teach xlate() to skip disabled iommus
> - Moved find_group() implementation to prevent warning on 32-bit ARM
> - Rolled in the following patch from "IPMMU slave device whitelist V2":
> [PATCH/RFC v2 3/4] iommu/ipmmu-vmsa: Check devices in xlate()
>
> drivers/iommu/Kconfig | 1
> drivers/iommu/ipmmu-vmsa.c | 164 +++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 157 insertions(+), 8 deletions(-)
Reviewed-by: Joerg Roedel <[email protected]>