From 24f307d8abf79486dd3c1b645037df7d91602aaa Mon Sep 17 00:00:00 2001 From: Lu Baolu Date: Fri, 24 May 2019 14:30:56 +0800 Subject: iommu: Add missing new line for dma type So that all types are printed in the same format. Fixes: c52c72d3dee81 ("iommu: Add sysfs attribyte for domain type") Signed-off-by: Lu Baolu Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 67ee6623f9b2..c9cfd08673e1 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -341,7 +341,7 @@ static ssize_t iommu_group_show_type(struct iommu_group *group, type = "unmanaged\n"; break; case IOMMU_DOMAIN_DMA: - type = "DMA"; + type = "DMA\n"; break; } } -- cgit v1.2.3-59-g8ed1b From 7560cc3ca7d9d11555f80c830544e463fcdb28b8 Mon Sep 17 00:00:00 2001 From: Dave Jiang Date: Tue, 21 May 2019 15:30:15 +0800 Subject: iommu/vt-d: Fix lock inversion between iommu->lock and device_domain_lock Lockdep debug reported lock inversion related with the iommu code caused by dmar_insert_one_dev_info() grabbing the iommu->lock and the device_domain_lock out of order versus the code path in iommu_flush_dev_iotlb(). Expanding the scope of the iommu->lock and reversing the order of lock acquisition fixes the issue. [ 76.238180] dsa_bus wq0.0: dsa wq wq0.0 disabled [ 76.248706] [ 76.250486] ======================================================== [ 76.257113] WARNING: possible irq lock inversion dependency detected [ 76.263736] 5.1.0-rc5+ #162 Not tainted [ 76.267854] -------------------------------------------------------- [ 76.274485] systemd-journal/521 just changed the state of lock: [ 76.280685] 0000000055b330f5 (device_domain_lock){..-.}, at: iommu_flush_dev_iotlb.part.63+0x29/0x90 [ 76.290099] but this lock took another, SOFTIRQ-unsafe lock in the past: [ 76.297093] (&(&iommu->lock)->rlock){+.+.} [ 76.297094] [ 76.297094] [ 76.297094] and interrupts could create inverse lock ordering between them. [ 76.297094] [ 76.314257] [ 76.314257] other info that might help us debug this: [ 76.321448] Possible interrupt unsafe locking scenario: [ 76.321448] [ 76.328907] CPU0 CPU1 [ 76.333777] ---- ---- [ 76.338642] lock(&(&iommu->lock)->rlock); [ 76.343165] local_irq_disable(); [ 76.349422] lock(device_domain_lock); [ 76.356116] lock(&(&iommu->lock)->rlock); [ 76.363154] [ 76.366134] lock(device_domain_lock); [ 76.370548] [ 76.370548] *** DEADLOCK *** Fixes: 745f2586e78e ("iommu/vt-d: Simplify function get_domain_for_dev()") Signed-off-by: Dave Jiang Reviewed-by: Lu Baolu Signed-off-by: Joerg Roedel --- drivers/iommu/intel-iommu.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index a209199f3af6..91f4912c09c6 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -2512,6 +2512,7 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu, } } + spin_lock(&iommu->lock); spin_lock_irqsave(&device_domain_lock, flags); if (dev) found = find_domain(dev); @@ -2527,17 +2528,16 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu, if (found) { spin_unlock_irqrestore(&device_domain_lock, flags); + spin_unlock(&iommu->lock); free_devinfo_mem(info); /* Caller must free the original domain */ return found; } - spin_lock(&iommu->lock); ret = domain_attach_iommu(domain, iommu); - spin_unlock(&iommu->lock); - if (ret) { spin_unlock_irqrestore(&device_domain_lock, flags); + spin_unlock(&iommu->lock); free_devinfo_mem(info); return NULL; } @@ -2547,6 +2547,7 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu, if (dev) dev->archdata.iommu = info; spin_unlock_irqrestore(&device_domain_lock, flags); + spin_unlock(&iommu->lock); /* PASID table is mandatory for a PCI device in scalable mode. */ if (dev && dev_is_pci(dev) && sm_supported(iommu)) { -- cgit v1.2.3-59-g8ed1b From 66d78ad316b0e1ca5ae19663468554e2c0e31c26 Mon Sep 17 00:00:00 2001 From: Lu Baolu Date: Tue, 21 May 2019 15:30:16 +0800 Subject: iommu/vt-d: Set the right field for Page Walk Snoop Set the page walk snoop to the right bit, otherwise the domain id field will be overlapped. Reported-by: Dave Jiang Fixes: 6f7db75e1c469 ("iommu/vt-d: Add second level page table interface") Signed-off-by: Lu Baolu Signed-off-by: Joerg Roedel --- drivers/iommu/intel-pasid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/iommu/intel-pasid.c b/drivers/iommu/intel-pasid.c index 2fefeafda437..fe51d8af457f 100644 --- a/drivers/iommu/intel-pasid.c +++ b/drivers/iommu/intel-pasid.c @@ -389,7 +389,7 @@ static inline void pasid_set_present(struct pasid_entry *pe) */ static inline void pasid_set_page_snoop(struct pasid_entry *pe, bool value) { - pasid_set_bits(&pe->val[1], 1 << 23, value); + pasid_set_bits(&pe->val[1], 1 << 23, value << 23); } /* -- cgit v1.2.3-59-g8ed1b From 4e4abae311e4b44aaf61f18a826fd7136037f199 Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Mon, 3 Jun 2019 14:15:37 +0200 Subject: iommu/arm-smmu: Avoid constant zero in TLBI writes Apparently, some Qualcomm arm64 platforms which appear to expose their SMMU global register space are still, in fact, using a hypervisor to mediate it by trapping and emulating register accesses. Sadly, some deployed versions of said trapping code have bugs wherein they go horribly wrong for stores using r31 (i.e. XZR/WZR) as the source register. While this can be mitigated for GCC today by tweaking the constraints for the implementation of writel_relaxed(), to avoid any potential arms race with future compilers more aggressively optimising register allocation, the simple way is to just remove all the problematic constant zeros. For the write-only TLB operations, the actual value is irrelevant anyway and any old nearby variable will provide a suitable GPR to encode. The one point at which we really do need a zero to clear a context bank happens before any of the TLB maintenance where crashes have been reported, so is apparently not a problem... :/ Reported-by: AngeloGioacchino Del Regno Tested-by: Marc Gonzalez Signed-off-by: Robin Murphy Signed-off-by: Marc Gonzalez Acked-by: Will Deacon Cc: stable@vger.kernel.org Signed-off-by: Joerg Roedel --- drivers/iommu/arm-smmu.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 5e54cc0a28b3..4f900c663fb8 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -59,6 +59,15 @@ #include "arm-smmu-regs.h" +/* + * Apparently, some Qualcomm arm64 platforms which appear to expose their SMMU + * global register space are still, in fact, using a hypervisor to mediate it + * by trapping and emulating register accesses. Sadly, some deployed versions + * of said trapping code have bugs wherein they go horribly wrong for stores + * using r31 (i.e. XZR/WZR) as the source register. + */ +#define QCOM_DUMMY_VAL -1 + #define ARM_MMU500_ACTLR_CPRE (1 << 1) #define ARM_MMU500_ACR_CACHE_LOCK (1 << 26) @@ -423,7 +432,7 @@ static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu, { unsigned int spin_cnt, delay; - writel_relaxed(0, sync); + writel_relaxed(QCOM_DUMMY_VAL, sync); for (delay = 1; delay < TLB_LOOP_TIMEOUT; delay *= 2) { for (spin_cnt = TLB_SPIN_COUNT; spin_cnt > 0; spin_cnt--) { if (!(readl_relaxed(status) & sTLBGSTATUS_GSACTIVE)) @@ -1763,8 +1772,8 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu) } /* Invalidate the TLB, just in case */ - writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLH); - writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLNSNH); + writel_relaxed(QCOM_DUMMY_VAL, gr0_base + ARM_SMMU_GR0_TLBIALLH); + writel_relaxed(QCOM_DUMMY_VAL, gr0_base + ARM_SMMU_GR0_TLBIALLNSNH); reg = readl_relaxed(ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0); -- cgit v1.2.3-59-g8ed1b