From e633fc7a1347528c3b4a6bbdeb41f5d63988242c Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Thu, 11 Aug 2016 17:44:05 +0100 Subject: [PATCH 1/4] iommu/io-pgtable-arm-v7s: Fix attributes when splitting blocks Due to the attribute bits being all over the place in the different types of short-descriptor PTEs, when remapping an existing entry, e.g. splitting a section into pages, we take the approach of decomposing the PTE attributes back to the IOMMU API flags to start from scratch. On inspection, though, the existing code seems to have got the read-only bit backwards and ignored the XN bit. How embarrassing... Fortunately the primary user so far, the Mediatek IOMMU, both never splits blocks (because it only serves non-overlapping DMA API calls) and also ignores permissions anyway, but let's put things right before any future users trip up. Cc: Fixes: e5fc9753b1a8 ("iommu/io-pgtable: Add ARMv7 short descriptor support") Signed-off-by: Robin Murphy Signed-off-by: Will Deacon --- drivers/iommu/io-pgtable-arm-v7s.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c index 8c6139986d7d..def8ca1c982d 100644 --- a/drivers/iommu/io-pgtable-arm-v7s.c +++ b/drivers/iommu/io-pgtable-arm-v7s.c @@ -286,12 +286,14 @@ static int arm_v7s_pte_to_prot(arm_v7s_iopte pte, int lvl) int prot = IOMMU_READ; arm_v7s_iopte attr = pte >> ARM_V7S_ATTR_SHIFT(lvl); - if (attr & ARM_V7S_PTE_AP_RDONLY) + if (!(attr & ARM_V7S_PTE_AP_RDONLY)) prot |= IOMMU_WRITE; if ((attr & (ARM_V7S_TEX_MASK << ARM_V7S_TEX_SHIFT)) == 0) prot |= IOMMU_MMIO; else if (pte & ARM_V7S_ATTR_C) prot |= IOMMU_CACHE; + if (pte & ARM_V7S_ATTR_XN(lvl)) + prot |= IOMMU_NOEXEC; return prot; } From aea2037e0d3e23c3be1498feae29f71ca997d9e6 Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Fri, 29 Jul 2016 11:15:37 +0100 Subject: [PATCH 2/4] iommu/arm-smmu: Fix CMDQ error handling In the unlikely event of a global command queue error, the ARM SMMUv3 driver attempts to convert the problematic command into a CMD_SYNC and resume the command queue. Unfortunately, this code is pretty badly broken: 1. It uses the index into the error string table as the CMDQ index, so we probably read the wrong entry out of the queue 2. The arguments to queue_write are the wrong way round, so we end up writing from the queue onto the stack. These happily cancel out, so the kernel is likely to stay alive, but the command queue will probably fault again when we resume. This patch fixes the error handling code to use the correct queue index and write back the CMD_SYNC to the faulting entry. Cc: Fixes: 48ec83bcbcf5 ("iommu/arm-smmu: Add initial driver support for ARM SMMUv3 devices") Reported-by: Diwakar Subraveti Signed-off-by: Will Deacon --- drivers/iommu/arm-smmu-v3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index ce801170d5f2..330623f8e344 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -879,7 +879,7 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu) * We may have concurrent producers, so we need to be careful * not to touch any of the shadow cmdq state. */ - queue_read(cmd, Q_ENT(q, idx), q->ent_dwords); + queue_read(cmd, Q_ENT(q, cons), q->ent_dwords); dev_err(smmu->dev, "skipping command in error state:\n"); for (i = 0; i < ARRAY_SIZE(cmd); ++i) dev_err(smmu->dev, "\t0x%016llx\n", (unsigned long long)cmd[i]); @@ -890,7 +890,7 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu) return; } - queue_write(cmd, Q_ENT(q, idx), q->ent_dwords); + queue_write(Q_ENT(q, cons), cmd, q->ent_dwords); } static void arm_smmu_cmdq_issue_cmd(struct arm_smmu_device *smmu, From 3714ce1d6655098ee69ede632883e5874d67e4ab Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Fri, 5 Aug 2016 19:49:45 +0100 Subject: [PATCH 3/4] iommu/arm-smmu: Disable stalling faults for all endpoints Enabling stalling faults can result in hardware deadlock on poorly designed systems, particularly those with a PCI root complex upstream of the SMMU. Although it's not really Linux's job to save hardware integrators from their own misfortune, it *is* our job to stop userspace (e.g. VFIO clients) from hosing the system for everybody else, even if they might already be required to have elevated privileges. Given that the fault handling code currently executes entirely in IRQ context, there is nothing that can sensibly be done to recover from things like page faults anyway, so let's rip this code out for now and avoid the potential for deadlock. Cc: Fixes: 48ec83bcbcf5 ("iommu/arm-smmu: Add initial driver support for ARM SMMUv3 devices") Reported-by: Matt Evans Signed-off-by: Will Deacon --- drivers/iommu/arm-smmu.c | 34 +++++++--------------------------- 1 file changed, 7 insertions(+), 27 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 4f49fe29f202..2db74ebc3240 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -686,8 +686,7 @@ static struct iommu_gather_ops arm_smmu_gather_ops = { static irqreturn_t arm_smmu_context_fault(int irq, void *dev) { - int flags, ret; - u32 fsr, fsynr, resume; + u32 fsr, fsynr; unsigned long iova; struct iommu_domain *domain = dev; struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); @@ -701,34 +700,15 @@ static irqreturn_t arm_smmu_context_fault(int irq, void *dev) if (!(fsr & FSR_FAULT)) return IRQ_NONE; - if (fsr & FSR_IGN) - dev_err_ratelimited(smmu->dev, - "Unexpected context fault (fsr 0x%x)\n", - fsr); - fsynr = readl_relaxed(cb_base + ARM_SMMU_CB_FSYNR0); - flags = fsynr & FSYNR0_WNR ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ; - iova = readq_relaxed(cb_base + ARM_SMMU_CB_FAR); - if (!report_iommu_fault(domain, smmu->dev, iova, flags)) { - ret = IRQ_HANDLED; - resume = RESUME_RETRY; - } else { - dev_err_ratelimited(smmu->dev, - "Unhandled context fault: iova=0x%08lx, fsynr=0x%x, cb=%d\n", - iova, fsynr, cfg->cbndx); - ret = IRQ_NONE; - resume = RESUME_TERMINATE; - } - /* Clear the faulting FSR */ + dev_err_ratelimited(smmu->dev, + "Unhandled context fault: fsr=0x%x, iova=0x%08lx, fsynr=0x%x, cb=%d\n", + fsr, iova, fsynr, cfg->cbndx); + writel(fsr, cb_base + ARM_SMMU_CB_FSR); - - /* Retry or terminate any stalled transactions */ - if (fsr & FSR_SS) - writel_relaxed(resume, cb_base + ARM_SMMU_CB_RESUME); - - return ret; + return IRQ_HANDLED; } static irqreturn_t arm_smmu_global_fault(int irq, void *dev) @@ -837,7 +817,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain, } /* SCTLR */ - reg = SCTLR_CFCFG | SCTLR_CFIE | SCTLR_CFRE | SCTLR_M | SCTLR_EAE_SBOP; + reg = SCTLR_CFIE | SCTLR_CFRE | SCTLR_M | SCTLR_EAE_SBOP; if (stage1) reg |= SCTLR_S1_ASIDPNE; #ifdef __BIG_ENDIAN From 5bc0a11664e17e9f9551983f5b660bd48b57483c Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Tue, 16 Aug 2016 14:29:16 +0100 Subject: [PATCH 4/4] iommu/arm-smmu: Don't BUG() if we find aborting STEs with disable_bypass The disable_bypass cmdline option changes the SMMUv3 driver to put down faulting stream table entries by default, as opposed to bypassing transactions from unconfigured devices. In this mode of operation, it is entirely expected to see aborting entries in the stream table if and when we come to installing a valid translation, so don't trigger a BUG() as a result of misdiagnosing these entries as stream table corruption. Cc: Fixes: 48ec83bcbcf5 ("iommu/arm-smmu: Add initial driver support for ARM SMMUv3 devices") Tested-by: Robin Murphy Reported-by: Robin Murphy Reviewed-by: Robin Murphy Signed-off-by: Will Deacon --- drivers/iommu/arm-smmu-v3.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index 330623f8e344..641e88761319 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -1034,6 +1034,9 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid, case STRTAB_STE_0_CFG_S2_TRANS: ste_live = true; break; + case STRTAB_STE_0_CFG_ABORT: + if (disable_bypass) + break; default: BUG(); /* STE corruption */ }