From d346180e70b91b3d5a1ae7e5603e65593d4622bc Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Tue, 26 Jan 2016 18:06:34 +0000 Subject: iommu/arm-smmu: Treat all device transactions as unprivileged The IOMMU API has no concept of privilege so assumes all devices and mappings are equal, and indeed most non-CPU master devices on an AMBA interconnect make little use of the attribute bits on the bus thus by default perform unprivileged data accesses. Some devices, however, believe themselves more equal than others, such as programmable DMA controllers whose 'master' thread issues bus transactions marked as privileged instruction fetches, while the data accesses of its channel threads (under the control of Linux, at least) are marked as unprivileged. This poses a problem for implementing the DMA API on an IOMMU conforming to ARM VMSAv8, under which a page that is unprivileged-writeable is also implicitly privileged-execute-never. Given that, there is no one set of attributes with which iommu_map() can implement, say, dma_alloc_coherent() that will allow every possible type of access without something running into unexecepted permission faults. Fortunately the SMMU architecture provides a means to mitigate such issues by overriding the incoming attributes of a transaction; make use of that to strip the privileged/unprivileged status off incoming transactions, leaving just the instruction/data dichotomy which the IOMMU API does at least understand; Four states good, two states better. Signed-off-by: Robin Murphy Signed-off-by: Will Deacon --- drivers/iommu/arm-smmu.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 59ee4b8a3236..1f9093d76d93 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -167,6 +167,9 @@ #define S2CR_TYPE_BYPASS (1 << S2CR_TYPE_SHIFT) #define S2CR_TYPE_FAULT (2 << S2CR_TYPE_SHIFT) +#define S2CR_PRIVCFG_SHIFT 24 +#define S2CR_PRIVCFG_UNPRIV (2 << S2CR_PRIVCFG_SHIFT) + /* Context bank attribute registers */ #define ARM_SMMU_GR1_CBAR(n) (0x0 + ((n) << 2)) #define CBAR_VMID_SHIFT 0 @@ -1083,7 +1086,7 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain, u32 idx, s2cr; idx = cfg->smrs ? cfg->smrs[i].idx : cfg->streamids[i]; - s2cr = S2CR_TYPE_TRANS | + s2cr = S2CR_TYPE_TRANS | S2CR_PRIVCFG_UNPRIV | (smmu_domain->cfg.cbndx << S2CR_CBNDX_SHIFT); writel_relaxed(s2cr, gr0_base + ARM_SMMU_GR0_S2CR(idx)); } -- cgit From 9adb95949a343dac53b1cd81dc973b5f815c88d4 Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Tue, 26 Jan 2016 18:06:36 +0000 Subject: iommu/arm-smmu: Support DMA-API domains With DMA mapping ops provided by the iommu-dma code, only a minimal contribution from the IOMMU driver is needed to create a suitable DMA-API domain for them to use. Implement this for the ARM SMMUs. Signed-off-by: Robin Murphy Signed-off-by: Will Deacon --- drivers/iommu/arm-smmu-v3.c | 10 +++++++++- drivers/iommu/arm-smmu.c | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index 20875341c865..f003b3ab646b 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -21,6 +21,7 @@ */ #include +#include #include #include #include @@ -1396,7 +1397,7 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type) { struct arm_smmu_domain *smmu_domain; - if (type != IOMMU_DOMAIN_UNMANAGED) + if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA) return NULL; /* @@ -1408,6 +1409,12 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type) if (!smmu_domain) return NULL; + if (type == IOMMU_DOMAIN_DMA && + iommu_get_dma_cookie(&smmu_domain->domain)) { + kfree(smmu_domain); + return NULL; + } + mutex_init(&smmu_domain->init_mutex); spin_lock_init(&smmu_domain->pgtbl_lock); return &smmu_domain->domain; @@ -1436,6 +1443,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain) struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); struct arm_smmu_device *smmu = smmu_domain->smmu; + iommu_put_dma_cookie(domain); free_io_pgtable_ops(smmu_domain->pgtbl_ops); /* Free the CD and ASID, if we allocated them */ diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 1f9093d76d93..e8e7bcc4540c 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -29,6 +29,7 @@ #define pr_fmt(fmt) "arm-smmu: " fmt #include +#include #include #include #include @@ -966,7 +967,7 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type) { struct arm_smmu_domain *smmu_domain; - if (type != IOMMU_DOMAIN_UNMANAGED) + if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA) return NULL; /* * Allocate the domain and initialise some of its data structures. @@ -977,6 +978,12 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type) if (!smmu_domain) return NULL; + if (type == IOMMU_DOMAIN_DMA && + iommu_get_dma_cookie(&smmu_domain->domain)) { + kfree(smmu_domain); + return NULL; + } + mutex_init(&smmu_domain->init_mutex); spin_lock_init(&smmu_domain->pgtbl_lock); @@ -991,6 +998,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain) * Free the domain resources. We assume that all devices have * already been detached. */ + iommu_put_dma_cookie(domain); arm_smmu_destroy_domain_context(domain); kfree(smmu_domain); } -- cgit From 45bb966d3dc7c6d4f0bd7673b5fe51deebcaf70b Mon Sep 17 00:00:00 2001 From: Anup Patel Date: Wed, 27 Jan 2016 10:51:16 +0530 Subject: of: iommu: Increment DT node refcount in of_iommu_set_ops() We are saving pointer to iommu DT node in of_iommu_set_ops() hence we should increment DT node ref count. Reviewed-by: Ray Jui Reviewed-by: Scott Branden Reviewed-by: Robin Murphy Signed-off-by: Anup Patel Signed-off-by: Will Deacon --- drivers/iommu/of_iommu.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index 60ba238090d9..5fea665af99d 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -110,6 +110,7 @@ void of_iommu_set_ops(struct device_node *np, struct iommu_ops *ops) if (WARN_ON(!iommu)) return; + of_node_get(np); INIT_LIST_HEAD(&iommu->list); iommu->np = np; iommu->ops = ops; -- cgit From 25a1c96cd22c949b50d6f0269c23e1c5cb55e5a5 Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Wed, 10 Feb 2016 14:25:33 +0000 Subject: iommu/arm-smmu: Allow disabling unmatched stream bypass Borrow the disable_bypass parameter from the SMMUv3 driver as a handy debugging/security feature so that unmatched stream IDs (i.e. devices not attached to an IOMMU domain) may be configured to fault. Rather than introduce unsightly inconsistency, or repeat the existing unnecessary use of module_param_named(), fix that as well in passing. Signed-off-by: Robin Murphy Signed-off-by: Will Deacon --- drivers/iommu/arm-smmu.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) (limited to 'drivers') diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index e8e7bcc4540c..7012531abe62 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -261,9 +261,13 @@ #define FSYNR0_WNR (1 << 4) static int force_stage; -module_param_named(force_stage, force_stage, int, S_IRUGO); +module_param(force_stage, int, S_IRUGO); MODULE_PARM_DESC(force_stage, "Force SMMU mappings to be installed at a particular stage of translation. A value of '1' or '2' forces the corresponding stage. All other values are ignored (i.e. no stage is forced). Note that selecting a specific stage will disable support for nested translation."); +static bool disable_bypass; +module_param(disable_bypass, bool, S_IRUGO); +MODULE_PARM_DESC(disable_bypass, + "Disable bypass streams such that incoming transactions from devices that are not attached to an iommu domain will report an abort back to the device and will not be allowed to pass through the SMMU."); enum arm_smmu_arch_version { ARM_SMMU_V1 = 1, @@ -1119,9 +1123,9 @@ static void arm_smmu_domain_remove_master(struct arm_smmu_domain *smmu_domain, */ for (i = 0; i < cfg->num_streamids; ++i) { u32 idx = cfg->smrs ? cfg->smrs[i].idx : cfg->streamids[i]; + u32 reg = disable_bypass ? S2CR_TYPE_FAULT : S2CR_TYPE_BYPASS; - writel_relaxed(S2CR_TYPE_BYPASS, - gr0_base + ARM_SMMU_GR0_S2CR(idx)); + writel_relaxed(reg, gr0_base + ARM_SMMU_GR0_S2CR(idx)); } arm_smmu_master_free_smrs(smmu, cfg); @@ -1484,11 +1488,11 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu) reg = readl_relaxed(ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sGFSR); writel(reg, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sGFSR); - /* Mark all SMRn as invalid and all S2CRn as bypass */ + /* Mark all SMRn as invalid and all S2CRn as bypass unless overridden */ + reg = disable_bypass ? S2CR_TYPE_FAULT : S2CR_TYPE_BYPASS; for (i = 0; i < smmu->num_mapping_groups; ++i) { writel_relaxed(0, gr0_base + ARM_SMMU_GR0_SMR(i)); - writel_relaxed(S2CR_TYPE_BYPASS, - gr0_base + ARM_SMMU_GR0_S2CR(i)); + writel_relaxed(reg, gr0_base + ARM_SMMU_GR0_S2CR(i)); } /* Make sure all context banks are disabled and clear CB_FSR */ @@ -1510,8 +1514,12 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu) /* Disable TLB broadcasting. */ reg |= (sCR0_VMIDPNE | sCR0_PTM); - /* Enable client access, but bypass when no mapping is found */ - reg &= ~(sCR0_CLIENTPD | sCR0_USFCFG); + /* Enable client access, handling unmatched streams as appropriate */ + reg &= ~sCR0_CLIENTPD; + if (disable_bypass) + reg |= sCR0_USFCFG; + else + reg &= ~sCR0_USFCFG; /* Disable forced broadcasting */ reg &= ~sCR0_FB; -- cgit From bc7f2ce0a7b54ba7703f81995fe434f0926424d2 Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Wed, 17 Feb 2016 17:41:57 +0000 Subject: iommu/arm-smmu: Don't fail device attach if already attached to a domain The ARM SMMU attach_dev implementations returns -EEXIST if the device being attached is already attached to a domain. This doesn't play nicely with the default domain, resulting in splats such as: WARNING: at drivers/iommu/iommu.c:1257 Modules linked in: CPU: 3 PID: 1939 Comm: virtio-net-tx Tainted: G S 4.5.0-rc4+ #1 Hardware name: FVP Base (DT) task: ffffffc87a9d0000 ti: ffffffc07a278000 task.ti: ffffffc07a278000 PC is at __iommu_detach_group+0x68/0xe8 LR is at __iommu_detach_group+0x48/0xe8 This patch fixes the problem by forcefully detaching the device from its old domain, if present, when attaching to a new one. The unused ->detach_dev callback is also removed the iommu_ops structures. Signed-off-by: Will Deacon --- drivers/iommu/arm-smmu-v3.c | 33 ++++++++++++--------------------- drivers/iommu/arm-smmu.c | 33 ++++++++++++++------------------- 2 files changed, 26 insertions(+), 40 deletions(-) (limited to 'drivers') diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index f003b3ab646b..d96d45ddf500 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -1638,6 +1638,17 @@ static int arm_smmu_install_ste_for_group(struct arm_smmu_group *smmu_group) return 0; } +static void arm_smmu_detach_dev(struct device *dev) +{ + struct arm_smmu_group *smmu_group = arm_smmu_group_get(dev); + + smmu_group->ste.bypass = true; + if (IS_ERR_VALUE(arm_smmu_install_ste_for_group(smmu_group))) + dev_warn(dev, "failed to install bypass STE\n"); + + smmu_group->domain = NULL; +} + static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) { int ret = 0; @@ -1650,7 +1661,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) /* Already attached to a different domain? */ if (smmu_group->domain && smmu_group->domain != smmu_domain) - return -EEXIST; + arm_smmu_detach_dev(dev); smmu = smmu_group->smmu; mutex_lock(&smmu_domain->init_mutex); @@ -1687,25 +1698,6 @@ out_unlock: return ret; } -static void arm_smmu_detach_dev(struct iommu_domain *domain, struct device *dev) -{ - struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); - struct arm_smmu_group *smmu_group = arm_smmu_group_get(dev); - - BUG_ON(!smmu_domain); - BUG_ON(!smmu_group); - - mutex_lock(&smmu_domain->init_mutex); - BUG_ON(smmu_group->domain != smmu_domain); - - smmu_group->ste.bypass = true; - if (IS_ERR_VALUE(arm_smmu_install_ste_for_group(smmu_group))) - dev_warn(dev, "failed to install bypass STE\n"); - - smmu_group->domain = NULL; - mutex_unlock(&smmu_domain->init_mutex); -} - static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova, phys_addr_t paddr, size_t size, int prot) { @@ -1943,7 +1935,6 @@ static struct iommu_ops arm_smmu_ops = { .domain_alloc = arm_smmu_domain_alloc, .domain_free = arm_smmu_domain_free, .attach_dev = arm_smmu_attach_dev, - .detach_dev = arm_smmu_detach_dev, .map = arm_smmu_map, .unmap = arm_smmu_unmap, .iova_to_phys = arm_smmu_iova_to_phys, diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 7012531abe62..f21332a15040 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1131,6 +1131,16 @@ static void arm_smmu_domain_remove_master(struct arm_smmu_domain *smmu_domain, arm_smmu_master_free_smrs(smmu, cfg); } +static void arm_smmu_detach_dev(struct device *dev, + struct arm_smmu_master_cfg *cfg) +{ + struct iommu_domain *domain = dev->archdata.iommu; + struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); + + dev->archdata.iommu = NULL; + arm_smmu_domain_remove_master(smmu_domain, cfg); +} + static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) { int ret; @@ -1144,11 +1154,6 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) return -ENXIO; } - if (dev->archdata.iommu) { - dev_err(dev, "already attached to IOMMU domain\n"); - return -EEXIST; - } - /* Ensure that the domain is finalised */ ret = arm_smmu_init_domain_context(domain, smmu); if (IS_ERR_VALUE(ret)) @@ -1170,25 +1175,16 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) if (!cfg) return -ENODEV; + /* Detach the dev from its current domain */ + if (dev->archdata.iommu) + arm_smmu_detach_dev(dev, cfg); + ret = arm_smmu_domain_add_master(smmu_domain, cfg); if (!ret) dev->archdata.iommu = domain; return ret; } -static void arm_smmu_detach_dev(struct iommu_domain *domain, struct device *dev) -{ - struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); - struct arm_smmu_master_cfg *cfg; - - cfg = find_smmu_master_cfg(dev); - if (!cfg) - return; - - dev->archdata.iommu = NULL; - arm_smmu_domain_remove_master(smmu_domain, cfg); -} - static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova, phys_addr_t paddr, size_t size, int prot) { @@ -1464,7 +1460,6 @@ static struct iommu_ops arm_smmu_ops = { .domain_alloc = arm_smmu_domain_alloc, .domain_free = arm_smmu_domain_free, .attach_dev = arm_smmu_attach_dev, - .detach_dev = arm_smmu_detach_dev, .map = arm_smmu_map, .unmap = arm_smmu_unmap, .map_sg = default_iommu_map_sg, -- cgit From cbf8277ef4562075eb4e3932ccd60cc11ee9454c Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Thu, 18 Feb 2016 12:05:57 +0000 Subject: iommu/arm-smmu: Treat IOMMU_DOMAIN_DMA as bypass for now Until all upstream devices have their DMA ops swizzled to point at the SMMU, we need to treat the IOMMU_DOMAIN_DMA domain as bypass to avoid putting devices into an empty address space when detaching from VFIO. Signed-off-by: Will Deacon --- drivers/iommu/arm-smmu-v3.c | 7 ++++++- drivers/iommu/arm-smmu.c | 7 +++++++ 2 files changed, 13 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index d96d45ddf500..4ff73ff64e49 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -1687,7 +1687,12 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) goto out_unlock; smmu_group->domain = smmu_domain; - smmu_group->ste.bypass = false; + + /* + * FIXME: This should always be "false" once we have IOMMU-backed + * DMA ops for all devices behind the SMMU. + */ + smmu_group->ste.bypass = domain->type == IOMMU_DOMAIN_DMA; ret = arm_smmu_install_ste_for_group(smmu_group); if (IS_ERR_VALUE(ret)) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index f21332a15040..2409e3bd3df2 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1094,6 +1094,13 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain, if (ret) return ret == -EEXIST ? 0 : ret; + /* + * FIXME: This won't be needed once we have IOMMU-backed DMA ops + * for all devices behind the SMMU. + */ + if (smmu_domain->domain.type == IOMMU_DOMAIN_DMA) + return 0; + for (i = 0; i < cfg->num_streamids; ++i) { u32 idx, s2cr; -- cgit