From 86b4d3af601384f977909a5a3959d3cde75e8616 Mon Sep 17 00:00:00 2001 From: Qingqing Zhou Date: Wed, 15 Jun 2022 15:42:43 +0800 Subject: [PATCH 1/2] iommu: Fix compilation issues on sdxbaagha Current codes may don't consider the 32-bit possibility, some compilation errors happen when try to enable smmu on sdxbaagha: 1. arm-smmu-qcom.c: "error: shift count is negative" reports at "FIELD_GET(QTB_OVR_ECATS_OUTFLD0_PA, val)". 2. qcom-iommu-debug-user.c: "error: incompatible pointer types passing" reports at "if (kstrtox_from_user(ubuf, count, 0, &iova))". 3. qcom-io-pgtable-arm.c: "error: implicit declaration of function 'FIELD_GET'" reports at "FIELD_GET(TOP_IGNORED_MASK, table_pte)". 4. qcom-iommu-debug-user.c: "error: implicit declaration of function 'FIELD_GET'" reports at "FIELD_GET(ARM_SMMU_SMR_ID, fwspec->ids[0])". 5. qcom-iommu-debug.c: "error: implicit declaration of function 'FIELD_GET'" reports at "FIELD_GET(ARM_SMMU_SMR_ID, (fwspec->ids[0]))". Change-Id: I38b96e411eaf37d87e48b4c9fe860a94643bc5b7 Signed-off-by: Qingqing Zhou --- drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 3 ++- drivers/iommu/qcom-io-pgtable-arm.c | 1 + drivers/iommu/qcom-iommu-debug-user.c | 8 ++++++++ drivers/iommu/qcom-iommu-debug.c | 3 ++- 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c index 2b25c59fc943..68d0ec6d188a 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2019-2021, The Linux Foundation. All rights reserved. + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -502,7 +503,7 @@ static const struct arm_smmu_impl qcom_smmu_impl = { #define QTB_OVR_ECATS_STATUS_DONE BIT(0) #define QTB_OVR_ECATS_OUTFLD0 0x458 -#define QTB_OVR_ECATS_OUTFLD0_PA GENMASK(63, 12) +#define QTB_OVR_ECATS_OUTFLD0_PA GENMASK_ULL(63, 12) #define QTB_OVR_ECATS_OUTFLD0_FAULT_TYPE GENMASK(5, 4) #define QTB_OVR_ECATS_OUTFLD0_FAULT BIT(0) diff --git a/drivers/iommu/qcom-io-pgtable-arm.c b/drivers/iommu/qcom-io-pgtable-arm.c index a9144d756d6a..916a2b0e715d 100644 --- a/drivers/iommu/qcom-io-pgtable-arm.c +++ b/drivers/iommu/qcom-io-pgtable-arm.c @@ -12,6 +12,7 @@ #define pr_fmt(fmt) "arm-lpae io-pgtable: " fmt #include +#include #include #include #include diff --git a/drivers/iommu/qcom-iommu-debug-user.c b/drivers/iommu/qcom-iommu-debug-user.c index c4dff03d350b..9a06d6276d3c 100644 --- a/drivers/iommu/qcom-iommu-debug-user.c +++ b/drivers/iommu/qcom-iommu-debug-user.c @@ -1,9 +1,11 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved. + * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved. * */ +#include #include #include #include @@ -11,9 +13,15 @@ #include #include "qcom-iommu-debug.h" +#ifdef CONFIG_64BIT #define kstrtoux kstrtou64 #define kstrtox_from_user kstrtoull_from_user #define kstrtosize_t kstrtoul +#else +#define kstrtoux kstrtou32 +#define kstrtox_from_user kstrtouint_from_user +#define kstrtosize_t kstrtouint +#endif static void *test_virt_addr; static DEFINE_MUTEX(test_virt_addr_lock); diff --git a/drivers/iommu/qcom-iommu-debug.c b/drivers/iommu/qcom-iommu-debug.c index 134068ce8e9d..feb51650662b 100644 --- a/drivers/iommu/qcom-iommu-debug.c +++ b/drivers/iommu/qcom-iommu-debug.c @@ -1,11 +1,12 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved. - * + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. */ #define pr_fmt(fmt) "iommu-debug: %s: " fmt, __func__ +#include #include #include #include From 7ff9dff2e6abbb99a3924a52178de9ac2e33b59e Mon Sep 17 00:00:00 2001 From: Sukadev Bhattiprolu Date: Sun, 11 Sep 2022 23:21:25 -0700 Subject: [PATCH 2/2] qcom-iommu-debug: handle unexpected dma_addr When __full_va_sweep(), __rand_va_sweep() and __tlb_stress_sweep(), do their cleanup, they assume that the entire iova range 0..4G was mapped correctly (i.e assigned iova starts at 0 and goes to 4G). But if for some reason the iova starts at a different address (eg: with fastmap, it could start at 0xFFFFF000), we end up with the following warning from iommu_dma_unmap_page() because the iova 0 was never mapped. 898 phys = iommu_iova_to_phys(domain, dma_handle); 899 if (WARN_ON(!phys)) 900 return; Worse, since the cleanup tries to unmap the entire 0..4G range, this warning is repeated a large number of times, making the device unusable. Instead, report an error on the unexpected and unmap the incorrect dma address immediately. And when unmapping the iova range, make sure to only unmap regions that were correctly (when iova == dma_addr) mapped. Change-Id: Ia0ab4fd49ba5021d4866daf69fb9a3fa7e61928d Signed-off-by: Sukadev Bhattiprolu --- drivers/iommu/qcom-iommu-debug-user.c | 57 ++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 6 deletions(-) diff --git a/drivers/iommu/qcom-iommu-debug-user.c b/drivers/iommu/qcom-iommu-debug-user.c index 9a06d6276d3c..c991a2030f80 100644 --- a/drivers/iommu/qcom-iommu-debug-user.c +++ b/drivers/iommu/qcom-iommu-debug-user.c @@ -794,6 +794,7 @@ static int __full_va_sweep(struct device *dev, struct seq_file *s, struct iommu_domain *domain, void *priv) { u64 iova; + int nr_maps = 0; dma_addr_t dma_addr; void *virt; phys_addr_t phys; @@ -826,8 +827,12 @@ static int __full_va_sweep(struct device *dev, struct seq_file *s, dev_err_ratelimited(dev, "Unexpected iova on iter %d (expected: 0x%lx got: 0x%lx)\n", i, expected, (unsigned long)dma_addr); ret = -EINVAL; + if (!dma_mapping_error(dev, dma_addr)) + dma_unmap_single(dev, dma_addr, size, + DMA_TO_DEVICE); goto out; } + nr_maps++; } if (domain) { @@ -868,7 +873,7 @@ static int __full_va_sweep(struct device *dev, struct seq_file *s, } out: - for (iova = 0; iova < max; iova += size) { + for (iova = 0; iova < max && nr_maps--; iova += size) { if (iova == MSI_IOVA_BASE) { iova = MSI_IOVA_BASE + MSI_IOVA_LENGTH - size; continue; @@ -884,7 +889,9 @@ static int __tlb_stress_sweep(struct device *dev, struct seq_file *s, struct iommu_domain *domain, void *unused) { int i, ret = 0; + int nr_maps = 0; u64 iova; + u64 first_iova = 0; const u64 max = SZ_1G * 4ULL - 1; void *virt; phys_addr_t phys; @@ -909,7 +916,13 @@ static int __tlb_stress_sweep(struct device *dev, struct seq_file *s, dev_err_ratelimited(dev, "Failed map on iter %d\n", i); ret = -EINVAL; goto out; + } else if (dma_addr != iova) { + dma_unmap_single(dev, dma_addr, SZ_8K, DMA_TO_DEVICE); + dev_err_ratelimited(dev, "Failed map on iter %d\n", i); + ret = -EINVAL; + goto out; } + nr_maps++; } if (dma_map_single(dev, virt, SZ_4K, DMA_TO_DEVICE) != DMA_MAPPING_ERROR) { @@ -922,6 +935,11 @@ static int __tlb_stress_sweep(struct device *dev, struct seq_file *s, * free up 4K at the very beginning, then leave one 4K mapping, * then free up 8K. This will result in the next 8K map to skip * over the 4K hole and take the 8K one. + * i.e + * 0K..4K Hole + * 4K..8K Map R1 + * 8K..12K Hole + * 12K..4G Map R2 */ dma_unmap_single(dev, 0, SZ_4K, DMA_TO_DEVICE); dma_unmap_single(dev, SZ_8K, SZ_4K, DMA_TO_DEVICE); @@ -934,12 +952,23 @@ static int __tlb_stress_sweep(struct device *dev, struct seq_file *s, dev_err_ratelimited(dev, "Unexpected dma_addr. got: %pa expected: %pa\n", &dma_addr, &expected); + + /* To simplify error handling, unmap the 4K regions (4K..8K + * and 12K..16K) here and the rest (16K..4G) in 8K increments + * in the for loop. + */ + dma_unmap_single(dev, SZ_4K, SZ_4K, DMA_TO_DEVICE); + dma_unmap_single(dev, SZ_8K+SZ_4K, SZ_4K, DMA_TO_DEVICE); + nr_maps -= 2; + first_iova = SZ_8K + SZ_8K; + ret = -EINVAL; goto out; } /* - * now remap 4K. We should get the first 4K chunk that was skipped + * Now we have 0..4K hole and 4K..4G mapped. + * Remap 4K. We should get the first 4K chunk that was skipped * over during the previous 8K map. If we missed a TLB invalidate * at that point this should explode. */ @@ -949,18 +978,27 @@ static int __tlb_stress_sweep(struct device *dev, struct seq_file *s, dev_err_ratelimited(dev, "Unexpected dma_addr. got: %pa expected: %pa\n", &dma_addr, &expected); + /* To simplify error handling, unmap the 4K region (4K..8K) + * here and rest (8K..4G) in 8K increments in the for loop. + */ + dma_unmap_single(dev, SZ_4K, SZ_4K, DMA_TO_DEVICE); + first_iova = SZ_8K; + nr_maps -= 1; ret = -EINVAL; goto out; } + first_iova = 0; + if (dma_map_single(dev, virt, SZ_4K, DMA_TO_DEVICE) != DMA_MAPPING_ERROR) { dev_err_ratelimited(dev, "dma_map_single unexpectedly after remaps (VA should have been exhausted)\n"); ret = -EINVAL; goto out; } +out: /* we're all full again. unmap everything. */ - for (iova = 0; iova < max; iova += SZ_8K) { + for (iova = first_iova; iova < max && nr_maps--; iova += SZ_8K) { if (iova == MSI_IOVA_BASE) { iova = MSI_IOVA_BASE + MSI_IOVA_LENGTH - SZ_8K; continue; @@ -968,7 +1006,6 @@ static int __tlb_stress_sweep(struct device *dev, struct seq_file *s, dma_unmap_single(dev, (dma_addr_t)iova, SZ_8K, DMA_TO_DEVICE); } -out: free_pages((unsigned long)virt, get_order(SZ_8K)); return ret; } @@ -1001,6 +1038,7 @@ static int __rand_va_sweep(struct device *dev, struct seq_file *s, u64 iova; const u64 max = SZ_1G * 4ULL - 1; int i, remapped, unmapped, ret = 0; + int nr_maps = 0; void *virt; dma_addr_t dma_addr, dma_addr2; struct fib_state fib; @@ -1028,7 +1066,14 @@ static int __rand_va_sweep(struct device *dev, struct seq_file *s, dev_err_ratelimited(dev, "Failed map on iter %d\n", i); ret = -EINVAL; goto out; + } else if (dma_addr != iova) { + dma_unmap_single(dev, dma_addr, size, DMA_TO_DEVICE); + dev_err_ratelimited(dev, "Unexpected dma_addr. got: %lx, expected: %lx\n", + (unsigned long)dma_addr, (unsigned long)iova); + ret = -EINVAL; + goto out; } + nr_maps++; } /* now unmap "random" iovas */ @@ -1067,7 +1112,8 @@ static int __rand_va_sweep(struct device *dev, struct seq_file *s, ret = -EINVAL; } - for (iova = 0; iova < max; iova += size) { +out: + for (iova = 0; iova < max && nr_maps--; iova += size) { if (iova == MSI_IOVA_BASE) { iova = MSI_IOVA_BASE + MSI_IOVA_LENGTH - size; continue; @@ -1075,7 +1121,6 @@ static int __rand_va_sweep(struct device *dev, struct seq_file *s, dma_unmap_single(dev, (dma_addr_t)iova, size, DMA_TO_DEVICE); } -out: free_pages((unsigned long)virt, get_order(size)); return ret; }