RE: [PATCH] dma-mapping: benchmark: Extract a common header file for map_benchmark definition
> -Original Message-
> From: tiantao (H)
> Sent: Friday, February 11, 2022 4:15 PM
> To: Song Bao Hua (Barry Song) ; sh...@kernel.org;
> chenxiang (M)
> Cc: iommu@lists.linux-foundation.org; linux-kselft...@vger.kernel.org;
> linux...@openeuler.org
> Subject: [PATCH] dma-mapping: benchmark: Extract a common header file for
> map_benchmark definition
>
> kernel/dma/map_benchmark.c and selftests/dma/dma_map_benchmark.c
> have duplicate map_benchmark definitions, which tends to lead to
> inconsistent changes to map_benchmark on both sides, extract a
> common header file to avoid this problem.
>
> Signed-off-by: Tian Tao
+To: Christoph
Looks like a right cleanup. This will help decrease the maintain
overhead in the future. Other similar selftests tools are also
doing this.
Acked-by: Barry Song
> ---
> kernel/dma/map_benchmark.c| 24 +-
> kernel/dma/map_benchmark.h| 31 +++
> .../testing/selftests/dma/dma_map_benchmark.c | 25 +--
> 3 files changed, 33 insertions(+), 47 deletions(-)
> create mode 100644 kernel/dma/map_benchmark.h
>
> diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
> index 9b9af1bd6be3..c05f4e242991 100644
> --- a/kernel/dma/map_benchmark.c
> +++ b/kernel/dma/map_benchmark.c
> @@ -18,29 +18,7 @@
> #include
> #include
>
> -#define DMA_MAP_BENCHMARK_IOWR('d', 1, struct map_benchmark)
> -#define DMA_MAP_MAX_THREADS 1024
> -#define DMA_MAP_MAX_SECONDS 300
> -#define DMA_MAP_MAX_TRANS_DELAY (10 * NSEC_PER_MSEC)
> -
> -#define DMA_MAP_BIDIRECTIONAL0
> -#define DMA_MAP_TO_DEVICE1
> -#define DMA_MAP_FROM_DEVICE 2
> -
> -struct map_benchmark {
> - __u64 avg_map_100ns; /* average map latency in 100ns */
> - __u64 map_stddev; /* standard deviation of map latency */
> - __u64 avg_unmap_100ns; /* as above */
> - __u64 unmap_stddev;
> - __u32 threads; /* how many threads will do map/unmap in parallel */
> - __u32 seconds; /* how long the test will last */
> - __s32 node; /* which numa node this benchmark will run on */
> - __u32 dma_bits; /* DMA addressing capability */
> - __u32 dma_dir; /* DMA data direction */
> - __u32 dma_trans_ns; /* time for DMA transmission in ns */
> - __u32 granule; /* how many PAGE_SIZE will do map/unmap once a time */
> - __u8 expansion[76]; /* For future use */
> -};
> +#include "map_benchmark.h"
>
> struct map_benchmark_data {
> struct map_benchmark bparam;
> diff --git a/kernel/dma/map_benchmark.h b/kernel/dma/map_benchmark.h
> new file mode 100644
> index ..62674c83bde4
> --- /dev/null
> +++ b/kernel/dma/map_benchmark.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2022 HiSilicon Limited.
> + */
> +
> +#ifndef _KERNEL_DMA_BENCHMARK_H
> +#define _KERNEL_DMA_BENCHMARK_H
> +
> +#define DMA_MAP_BENCHMARK _IOWR('d', 1, struct map_benchmark)
> +#define DMA_MAP_MAX_THREADS 1024
> +#define DMA_MAP_MAX_SECONDS 300
> +#define DMA_MAP_MAX_TRANS_DELAY (10 * NSEC_PER_MSEC)
> +
> +#define DMA_MAP_BIDIRECTIONAL 0
> +#define DMA_MAP_TO_DEVICE 1
> +#define DMA_MAP_FROM_DEVICE 2
> +
> +struct map_benchmark {
> + __u64 avg_map_100ns; /* average map latency in 100ns */
> + __u64 map_stddev; /* standard deviation of map latency */
> + __u64 avg_unmap_100ns; /* as above */
> + __u64 unmap_stddev;
> + __u32 threads; /* how many threads will do map/unmap in parallel */
> + __u32 seconds; /* how long the test will last */
> + __s32 node; /* which numa node this benchmark will run on */
> + __u32 dma_bits; /* DMA addressing capability */
> + __u32 dma_dir; /* DMA data direction */
> + __u32 dma_trans_ns; /* time for DMA transmission in ns */
> + __u32 granule; /* how many PAGE_SIZE will do map/unmap once a time */
> +};
> +#endif /* _KERNEL_DMA_BENCHMARK_H */
> diff --git a/tools/testing/selftests/dma/dma_map_benchmark.c
> b/tools/testing/selftests/dma/dma_map_benchmark.c
> index 485dff51bad2..33bf073071aa 100644
> --- a/tools/testing/selftests/dma/dma_map_benchmark.c
> +++ b/tools/testing/selftests/dma/dma_map_benchmark.c
> @@ -11,39 +11,16 @@
> #include
> #include
> #include
> +#include "../../../../kernel/dma/map_benchmark.h"
>
> #define NSEC_PER_MSEC100L
>
> -#define DMA_MAP_BENCHMARK_IOWR('d', 1, struct map_benchmark)
> -#define DMA_MAP_MAX_THREADS 1024
> -#define DMA_MAP_MAX_SECONDS 300
> -#define DMA_MAP_MAX_TRANS_DELAY (10 * NSEC_PER_MSEC)
> -
> -#define DMA_MAP_BIDIRECTIONAL0
> -#define DMA_MAP_TO_DEVICE1
> -#define DMA_MAP_FROM_DEVICE 2
> -
> static char *directions[] = {
> "BIDIRECTIONAL",
> "TO_DEVICE",
> "FROM_DEVICE",
> };
>
> -struct map_benchmark {
> - __u64 avg_map_100ns; /* average map latency in 100ns */
> - __u64 map_stddev; /* standard deviation of map latency */
> -
[PATCH] dma-mapping: benchmark: Extract a common header file for map_benchmark definition
kernel/dma/map_benchmark.c and selftests/dma/dma_map_benchmark.c
have duplicate map_benchmark definitions, which tends to lead to
inconsistent changes to map_benchmark on both sides, extract a
common header file to avoid this problem.
Signed-off-by: Tian Tao
---
kernel/dma/map_benchmark.c| 24 +-
kernel/dma/map_benchmark.h| 31 +++
.../testing/selftests/dma/dma_map_benchmark.c | 25 +--
3 files changed, 33 insertions(+), 47 deletions(-)
create mode 100644 kernel/dma/map_benchmark.h
diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
index 9b9af1bd6be3..c05f4e242991 100644
--- a/kernel/dma/map_benchmark.c
+++ b/kernel/dma/map_benchmark.c
@@ -18,29 +18,7 @@
#include
#include
-#define DMA_MAP_BENCHMARK _IOWR('d', 1, struct map_benchmark)
-#define DMA_MAP_MAX_THREADS1024
-#define DMA_MAP_MAX_SECONDS300
-#define DMA_MAP_MAX_TRANS_DELAY(10 * NSEC_PER_MSEC)
-
-#define DMA_MAP_BIDIRECTIONAL 0
-#define DMA_MAP_TO_DEVICE 1
-#define DMA_MAP_FROM_DEVICE2
-
-struct map_benchmark {
- __u64 avg_map_100ns; /* average map latency in 100ns */
- __u64 map_stddev; /* standard deviation of map latency */
- __u64 avg_unmap_100ns; /* as above */
- __u64 unmap_stddev;
- __u32 threads; /* how many threads will do map/unmap in parallel */
- __u32 seconds; /* how long the test will last */
- __s32 node; /* which numa node this benchmark will run on */
- __u32 dma_bits; /* DMA addressing capability */
- __u32 dma_dir; /* DMA data direction */
- __u32 dma_trans_ns; /* time for DMA transmission in ns */
- __u32 granule; /* how many PAGE_SIZE will do map/unmap once a time */
- __u8 expansion[76]; /* For future use */
-};
+#include "map_benchmark.h"
struct map_benchmark_data {
struct map_benchmark bparam;
diff --git a/kernel/dma/map_benchmark.h b/kernel/dma/map_benchmark.h
new file mode 100644
index ..62674c83bde4
--- /dev/null
+++ b/kernel/dma/map_benchmark.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2022 HiSilicon Limited.
+ */
+
+#ifndef _KERNEL_DMA_BENCHMARK_H
+#define _KERNEL_DMA_BENCHMARK_H
+
+#define DMA_MAP_BENCHMARK _IOWR('d', 1, struct map_benchmark)
+#define DMA_MAP_MAX_THREADS 1024
+#define DMA_MAP_MAX_SECONDS 300
+#define DMA_MAP_MAX_TRANS_DELAY (10 * NSEC_PER_MSEC)
+
+#define DMA_MAP_BIDIRECTIONAL 0
+#define DMA_MAP_TO_DEVICE 1
+#define DMA_MAP_FROM_DEVICE 2
+
+struct map_benchmark {
+ __u64 avg_map_100ns; /* average map latency in 100ns */
+ __u64 map_stddev; /* standard deviation of map latency */
+ __u64 avg_unmap_100ns; /* as above */
+ __u64 unmap_stddev;
+ __u32 threads; /* how many threads will do map/unmap in parallel */
+ __u32 seconds; /* how long the test will last */
+ __s32 node; /* which numa node this benchmark will run on */
+ __u32 dma_bits; /* DMA addressing capability */
+ __u32 dma_dir; /* DMA data direction */
+ __u32 dma_trans_ns; /* time for DMA transmission in ns */
+ __u32 granule; /* how many PAGE_SIZE will do map/unmap once a time */
+};
+#endif /* _KERNEL_DMA_BENCHMARK_H */
diff --git a/tools/testing/selftests/dma/dma_map_benchmark.c
b/tools/testing/selftests/dma/dma_map_benchmark.c
index 485dff51bad2..33bf073071aa 100644
--- a/tools/testing/selftests/dma/dma_map_benchmark.c
+++ b/tools/testing/selftests/dma/dma_map_benchmark.c
@@ -11,39 +11,16 @@
#include
#include
#include
+#include "../../../../kernel/dma/map_benchmark.h"
#define NSEC_PER_MSEC 100L
-#define DMA_MAP_BENCHMARK _IOWR('d', 1, struct map_benchmark)
-#define DMA_MAP_MAX_THREADS1024
-#define DMA_MAP_MAX_SECONDS 300
-#define DMA_MAP_MAX_TRANS_DELAY(10 * NSEC_PER_MSEC)
-
-#define DMA_MAP_BIDIRECTIONAL 0
-#define DMA_MAP_TO_DEVICE 1
-#define DMA_MAP_FROM_DEVICE2
-
static char *directions[] = {
"BIDIRECTIONAL",
"TO_DEVICE",
"FROM_DEVICE",
};
-struct map_benchmark {
- __u64 avg_map_100ns; /* average map latency in 100ns */
- __u64 map_stddev; /* standard deviation of map latency */
- __u64 avg_unmap_100ns; /* as above */
- __u64 unmap_stddev;
- __u32 threads; /* how many threads will do map/unmap in parallel */
- __u32 seconds; /* how long the test will last */
- __s32 node; /* which numa node this benchmark will run on */
- __u32 dma_bits; /* DMA addressing capability */
- __u32 dma_dir; /* DMA data direction */
- __u32 dma_trans_ns; /* time for DMA transmission in ns */
- __u32 granule; /* how many PAGE_SIZE will do map/unmap once a time */
- __u8 expansion[76]; /* For future use */
-};
-
int main(int argc, char **argv)
{
struct map_benchmark map;
--
2.33.0
___
[PATCH 2/2] iommu/vt-d: Remove unnecessary exported symbol
The exported symbol intel_iommu_gfx_mapped is not used anywhere in the
tree. Remove it to avoid dead code.
Signed-off-by: Lu Baolu
---
include/linux/intel-iommu.h | 1 -
drivers/iommu/intel/iommu.c | 6 --
2 files changed, 7 deletions(-)
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 2f8a4517c929..b8b8be58cb6b 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -789,7 +789,6 @@ extern int iommu_calculate_agaw(struct intel_iommu *iommu);
extern int iommu_calculate_max_sagaw(struct intel_iommu *iommu);
extern int dmar_disabled;
extern int intel_iommu_enabled;
-extern int intel_iommu_gfx_mapped;
#else
static inline int iommu_calculate_agaw(struct intel_iommu *iommu)
{
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 4b06f2f365bd..88a53140c7f1 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -311,9 +311,6 @@ static int iommu_skip_te_disable;
#define IDENTMAP_GFX 2
#define IDENTMAP_AZALIA4
-int intel_iommu_gfx_mapped;
-EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped);
-
static DEFINE_XARRAY_ALLOC(device_domain_array);
/* Convert device source ID into the index of device_domain_array. */
@@ -4070,9 +4067,6 @@ int __init intel_iommu_init(void)
if (list_empty(_satc_units))
pr_info("No SATC found\n");
- if (dmar_map_gfx)
- intel_iommu_gfx_mapped = 1;
-
init_no_remapping_devices();
ret = init_dmars();
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/2] agp/intel: Use per device iommu check
The IOMMU subsystem has already provided an interface to query whether
the IOMMU hardware is enabled for a specific device. This changes the
check from Intel specific intel_iommu_gfx_mapped (globally exported by
the Intel IOMMU driver) to probing the presence of IOMMU on a specific
device using the generic device_iommu_mapped().
This follows commit cca084692394a ("drm/i915: Use per device iommu check")
which converted drm/i915 driver to use device_iommu_mapped().
Signed-off-by: Lu Baolu
---
drivers/char/agp/intel-gtt.c | 17 +++--
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index c53cc9868cd8..9631cbc7002e 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -20,7 +20,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include "agp.h"
@@ -573,18 +573,15 @@ static void intel_gtt_cleanup(void)
*/
static inline int needs_ilk_vtd_wa(void)
{
-#ifdef CONFIG_INTEL_IOMMU
const unsigned short gpu_devid = intel_private.pcidev->device;
- /* Query intel_iommu to see if we need the workaround. Presumably that
-* was loaded first.
+ /*
+* Query iommu subsystem to see if we need the workaround. Presumably
+* that was loaded first.
*/
- if ((gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_D_IG ||
-gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_M_IG) &&
-intel_iommu_gfx_mapped)
- return 1;
-#endif
- return 0;
+ return ((gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_D_IG ||
+gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_M_IG) &&
+ device_iommu_mapped(_private.pcidev->dev));
}
static bool intel_gtt_can_wc(void)
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/2] Replace intel_iommu_gfx_mapped with device_iommu_mapped()
Hi,
This follows commit cca084692394a ("drm/i915: Use per device iommu check")
to convert intel_iommu_gfx_mapped use in agp/intel to device_iommu_mapped().
With this changed, the export intel_iommu_gfx_mapped could be removed.
Best regards,
baolu
Lu Baolu (2):
agp/intel: Use per device iommu check
iommu/vt-d: Remove unnecessary exported symbol
include/linux/intel-iommu.h | 1 -
drivers/char/agp/intel-gtt.c | 17 +++--
drivers/iommu/intel/iommu.c | 6 --
3 files changed, 7 insertions(+), 17 deletions(-)
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/vt-d: Fix list_add double add when enabling VMD and scalable mode
From: Adrian Huang When enabling VMD and IOMMU scalable mode, the following kernel panic call trace/kernel log is shown in Eagle Stream platform (Sapphire Rapids CPU) during booting: pci :59:00.5: Adding to iommu group 42 ... vmd :59:00.5: PCI host bridge to bus 1:80 pci 1:80:01.0: [8086:352a] type 01 class 0x060400 pci 1:80:01.0: reg 0x10: [mem 0x-0x0001 64bit] pci 1:80:01.0: enabling Extended Tags pci 1:80:01.0: PME# supported from D0 D3hot D3cold pci 1:80:01.0: DMAR: Setup RID2PASID failed pci 1:80:01.0: Failed to add to iommu group 42: -16 pci 1:80:03.0: [8086:352b] type 01 class 0x060400 pci 1:80:03.0: reg 0x10: [mem 0x-0x0001 64bit] pci 1:80:03.0: enabling Extended Tags pci 1:80:03.0: PME# supported from D0 D3hot D3cold list_add double add: new=ff4d61160b74b8a0, prev=ff4d611d8e245c10, next=ff4d61160b74b8a0. [ cut here ] kernel BUG at lib/list_debug.c:29! invalid opcode: [#1] PREEMPT SMP NOPTI CPU: 0 PID: 7 Comm: kworker/0:1 Not tainted 5.17.0-rc3+ #7 Hardware name: Lenovo ThinkSystem SR650V3/SB27A86647, BIOS ESE101Y-1.00 01/13/2022 Workqueue: events work_for_cpu_fn RIP: 0010:__list_add_valid.cold+0x26/0x3f Code: 9a 4a ab ff 4c 89 c1 48 c7 c7 40 0c d9 9e e8 b9 b1 fe ff 0f 0b 48 89 f2 4c 89 c1 48 89 fe 48 c7 c7 f0 0c d9 9e e8 a2 b1 fe ff <0f> 0b 48 89 d1 4c 89 c6 4c 89 ca 48 c7 c7 98 0c d9 9e e8 8b b1 fe RSP: :ff5ad434865b3a40 EFLAGS: 00010246 RAX: 0058 RBX: ff4d61160b74b880 RCX: ff4d61255e1fffa8 RDX: RSI: fffe RDI: 9fd34f20 RBP: ff4d611d8e245c00 R08: R09: ff5ad434865b3888 R10: ff5ad434865b3880 R11: ff4d61257fdc6fe8 R12: ff4d61160b74b8a0 R13: ff4d61160b74b8a0 R14: ff4d611d8e245c10 R15: ff4d611d8001ba70 FS: () GS:ff4d611d5ea0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: ff4d611fa1401000 CR3: 000aa0210001 CR4: 00771ef0 DR0: DR1: DR2: DR3: DR6: fffe07f0 DR7: 0400 PKRU: 5554 Call Trace: intel_pasid_alloc_table+0x9c/0x1d0 dmar_insert_one_dev_info+0x423/0x540 ? device_to_iommu+0x12d/0x2f0 intel_iommu_attach_device+0x116/0x290 __iommu_attach_device+0x1a/0x90 iommu_group_add_device+0x190/0x2c0 __iommu_probe_device+0x13e/0x250 iommu_probe_device+0x24/0x150 iommu_bus_notifier+0x69/0x90 blocking_notifier_call_chain+0x5a/0x80 device_add+0x3db/0x7b0 ? arch_memremap_can_ram_remap+0x19/0x50 ? memremap+0x75/0x140 pci_device_add+0x193/0x1d0 pci_scan_single_device+0xb9/0xf0 pci_scan_slot+0x4c/0x110 pci_scan_child_bus_extend+0x3a/0x290 vmd_enable_domain.constprop.0+0x63e/0x820 vmd_probe+0x163/0x190 local_pci_probe+0x42/0x80 work_for_cpu_fn+0x13/0x20 process_one_work+0x1e2/0x3b0 worker_thread+0x1c4/0x3a0 ? rescuer_thread+0x370/0x370 kthread+0xc7/0xf0 ? kthread_complete_and_exit+0x20/0x20 ret_from_fork+0x1f/0x30 Modules linked in: ---[ end trace ]--- ... Kernel panic - not syncing: Fatal exception Kernel Offset: 0x1ca0 from 0x8100 (relocation range: 0x8000-0xbfff) ---[ end Kernel panic - not syncing: Fatal exception ]--- The following 'lspci' output shows devices '1:80:*' are subdevices of the VMD device :59:00.5: $ lspci ... :59:00.5 RAID bus controller: Intel Corporation Volume Management Device NVMe RAID Controller (rev 20) ... 1:80:01.0 PCI bridge: Intel Corporation Device 352a (rev 03) 1:80:03.0 PCI bridge: Intel Corporation Device 352b (rev 03) 1:80:05.0 PCI bridge: Intel Corporation Device 352c (rev 03) 1:80:07.0 PCI bridge: Intel Corporation Device 352d (rev 03) 1:81:00.0 Non-Volatile memory controller: Intel Corporation NVMe Datacenter SSD [3DNAND, Beta Rock Controller] 1:82:00.0 Non-Volatile memory controller: Intel Corporation NVMe Datacenter SSD [3DNAND, Beta Rock Controller] The symptom 'list_add double add' is caused by the following failure message: pci 1:80:01.0: DMAR: Setup RID2PASID failed pci 1:80:01.0: Failed to add to iommu group 42: -16 pci 1:80:03.0: [8086:352b] type 01 class 0x060400 Device 1:80:01.0 is the subdevice of the VMD device :59:00.5, so invoking intel_pasid_alloc_table() gets the pasid_table of the VMD device :59:00.5. Here is call path: intel_pasid_alloc_table pci_for_each_dma_alias get_alias_pasid_table search_pasid_table pci_real_dma_dev() in pci_for_each_dma_alias() gets the real dma device which is the VMD device :59:00.5. However, pte of the VMD device :59:00.5 has been configured during this message "pci :59:00.5: Adding to iommu group 42". So, the status -EBUSY is returned when configuring pasid entry for device 1:80:01.0. It then invokes dmar_remove_one_dev_info() to release 'struct device_domain_info *' from
Re: [PATCH] iommu: Remove trivial ops->capable implementations
On 2/10/22 8:29 PM, Robin Murphy wrote:
Implementing ops->capable to always return false is pointless since it's
the default behaviour anyway. Clean up the unnecessary implementations.
Signed-off-by: Robin Murphy
---
Spinning this out of my bus ops stuff (currently 30 patches and
counting...) since it would be better off alongside Baolu's cleanup
series to avoid conflicts, and I want to depend on those patches for
dev_iommu_ops() anyway.
drivers/iommu/msm_iommu.c | 6 --
drivers/iommu/tegra-gart.c | 6 --
drivers/iommu/tegra-smmu.c | 6 --
3 files changed, 18 deletions(-)
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 06bde6b66732..22061ddbd5df 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -558,11 +558,6 @@ static phys_addr_t msm_iommu_iova_to_phys(struct
iommu_domain *domain,
return ret;
}
-static bool msm_iommu_capable(enum iommu_cap cap)
-{
- return false;
-}
-
static void print_ctx_regs(void __iomem *base, int ctx)
{
unsigned int fsr = GET_FSR(base, ctx);
@@ -672,7 +667,6 @@ irqreturn_t msm_iommu_fault_handler(int irq, void *dev_id)
}
static struct iommu_ops msm_iommu_ops = {
- .capable = msm_iommu_capable,
.domain_alloc = msm_iommu_domain_alloc,
.domain_free = msm_iommu_domain_free,
.attach_dev = msm_iommu_attach_dev,
diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
index 6a358f92c7e5..bbd287d19324 100644
--- a/drivers/iommu/tegra-gart.c
+++ b/drivers/iommu/tegra-gart.c
@@ -238,11 +238,6 @@ static phys_addr_t gart_iommu_iova_to_phys(struct
iommu_domain *domain,
return pte & GART_PAGE_MASK;
}
-static bool gart_iommu_capable(enum iommu_cap cap)
-{
- return false;
-}
-
static struct iommu_device *gart_iommu_probe_device(struct device *dev)
{
if (!dev_iommu_fwspec_get(dev))
@@ -276,7 +271,6 @@ static void gart_iommu_sync(struct iommu_domain *domain,
}
static const struct iommu_ops gart_iommu_ops = {
- .capable= gart_iommu_capable,
.domain_alloc = gart_iommu_domain_alloc,
.domain_free= gart_iommu_domain_free,
.attach_dev = gart_iommu_attach_dev,
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index e900e3c46903..43df44f918a1 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -272,11 +272,6 @@ static void tegra_smmu_free_asid(struct tegra_smmu *smmu,
unsigned int id)
clear_bit(id, smmu->asids);
}
-static bool tegra_smmu_capable(enum iommu_cap cap)
-{
- return false;
-}
-
static struct iommu_domain *tegra_smmu_domain_alloc(unsigned type)
{
struct tegra_smmu_as *as;
@@ -967,7 +962,6 @@ static int tegra_smmu_of_xlate(struct device *dev,
}
static const struct iommu_ops tegra_smmu_ops = {
- .capable = tegra_smmu_capable,
.domain_alloc = tegra_smmu_domain_alloc,
.domain_free = tegra_smmu_domain_free,
.attach_dev = tegra_smmu_attach_dev,
Looks good to me.
Reviewed-by: Lu Baolu
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: Error when running fio against nvme-of rdma target (mlx5 driver)
On 2/9/22 1:41 AM, Chaitanya Kulkarni wrote: > On 2/8/22 6:50 PM, Martin Oliveira wrote: > > Hello, > > > > We have been hitting an error when running IO over our nvme-of setup, using > > the mlx5 driver and we are wondering if anyone has seen anything > > similar/has any suggestions. > > > > Both initiator and target are AMD EPYC 7502 machines connected over RDMA > > using a Mellanox MT28908. Target has 12 NVMe SSDs which are exposed as a > > single NVMe fabrics device, one physical SSD per namespace. > > > > Thanks for reporting this, if you can bisect the problem on your setup > it will help others to help you better. > > -ck Hi Chaitanya, I went back to a kernel as old as 4.15 and the problem was still there, so I don't know of a good commit to start from. I also learned that I can reproduce this with as little as 3 cards and I updated the firmware on the Mellanox cards to the latest version. I'd be happy to try any tests if someone has any suggestions. Thanks, Martin ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit
Hi, Tony, On Thu, Feb 10, 2022 at 10:31:42AM -0800, Fenghua Yu wrote: > > On Thu, Feb 10, 2022 at 09:24:50AM -0800, Luck, Tony wrote: > > On Thu, Feb 10, 2022 at 08:27:50AM -0800, Fenghua Yu wrote: > > > Hi, Jacob, > > > > > > On Wed, Feb 09, 2022 at 07:16:14PM -0800, Jacob Pan wrote: > > > > Hi Fenghua, > > > > > > > > On Mon, 7 Feb 2022 15:02:48 -0800, Fenghua Yu > > > > wrote: > > > > > > > > > @@ -1047,8 +1040,6 @@ struct iommu_sva *intel_svm_bind(struct device > > > > > *dev, struct mm_struct *mm, void } > > > > > > > > > > sva = intel_svm_bind_mm(iommu, dev, mm, flags); > > > > > - if (IS_ERR_OR_NULL(sva)) > > > > > - intel_svm_free_pasid(mm); > > > > If bind fails, the PASID has no IOMMU nor CPU context. It should be > > > > safe to > > > > free here. > > > > > > The PASID can not be freed even if bind fails. The PASID allocated earlier > > > (either in this thread or in another thread) might be populated to other > > > threads already and being used now. > > > > > > Without freeing the PASID on bind failure, the worst case is the PASID > > > might > > > not be used in the process (and will be freed on process exit anyway). > > > > > > This all matches with the PASID life time described in the commit message. > > > > But what does this mean for the user that failed that intel_svm_bind_mm()? > > > > That means the user may have a PASID but there is no PASID entry for the > device. Then ENQCMD cannot be executed successfully. > > > Here's a scenario: > > > > Process sets up to use PASID capable device #1. Everything works, > > so the process has mm->pasid, and the IOMMU has the tables to map > > virtual addresses coming from device #1 using that PASID. > > > > Now the same process asks to start using PASID capable device #2, > > but there is a failure at intel_svm_bind_mm(). > > > > Fenghua is right that we shouldn't free the PASID. It is in use > > by at least one thread of the process to access device #1. > > > > But what happens with device #2? Does the caller of intel_svm_bind() > > do the right thing with the IS_ERR_OR_NULL return value to let the > > user know that device #2 isn't accessible? > > A driver caller of intel_svm_bind() should handle this failure, i.e. fail > the binding and let the user know the failure. > > Even if the driver doesn't do the right thing to handle the failure, > intel_svm_bind() doesn't set up a PASID entry for device #2. > > One example is IDXD driver. User calls open()->IDXD driver idxd_cdev_open() > ->intel_svm_bind()->intel_svm_bind_mm(). idxd_cdev_open() gets failed "sva" > and passes the PTR_ERR(sva) to the user and the user cannot open the device. > Due to the failure, no PASID entry is set up for the device. > > Even if the user ignores the open() failure and tries to run ENQCMD on > device #2, a PASID table fault will be generated due to no PASID entry > for the device and the ENQCMD execution will fail. Plus, the above behavior of handling intel_svm_bind_mm() failure is expected right behavior and the current series doesn't need to be changed. Thanks. -Fenghua ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/5] dt-bindings: reserved-memory: Document memory region specifier
On 2022-02-09 17:31:16 +0100, Thierry Reding wrote:
> On Sun, Feb 06, 2022 at 11:27:00PM +0100, Janne Grunau wrote:
> > On 2021-09-15 17:19:39 +0200, Thierry Reding wrote:
> > > On Tue, Sep 07, 2021 at 07:44:44PM +0200, Thierry Reding wrote:
> > > > On Tue, Sep 07, 2021 at 10:33:24AM -0500, Rob Herring wrote:
> > > > > On Fri, Sep 3, 2021 at 10:36 AM Thierry Reding
> > > > > wrote:
> > > > > >
> > > > > > On Fri, Sep 03, 2021 at 09:36:33AM -0500, Rob Herring wrote:
> > > > > > > On Fri, Sep 3, 2021 at 8:52 AM Thierry Reding
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Fri, Sep 03, 2021 at 08:20:55AM -0500, Rob Herring wrote:
> > > > > > > > >
> > > > > > > > > Couldn't we keep this all in /reserved-memory? Just add an
> > > > > > > > > iova
> > > > > > > > > version of reg. Perhaps abuse 'assigned-address' for this
> > > > > > > > > purpose. The
> > > > > > > > > issue I see would be handling reserved iova areas without a
> > > > > > > > > physical
> > > > > > > > > area. That can be handled with just a iova and no reg. We
> > > > > > > > > already have
> > > > > > > > > a no reg case.
> > > > > > > >
> > > > > > > > I had thought about that initially. One thing I'm worried about
> > > > > > > > is that
> > > > > > > > every child node in /reserved-memory will effectively cause the
> > > > > > > > memory
> > > > > > > > that it described to be reserved. But we don't want that for
> > > > > > > > regions
> > > > > > > > that are "virtual only" (i.e. IOMMU reservations).
> > > > > > >
> > > > > > > By virtual only, you mean no physical mapping, just a region of
> > > > > > > virtual space, right? For that we'd have no 'reg' and therefore no
> > > > > > > (physical) reservation by the OS. It's similar to non-static
> > > > > > > regions.
> > > > > > > You need a specific handler for them. We'd probably want a
> > > > > > > compatible
> > > > > > > as well for these virtual reservations.
> > > > > >
> > > > > > Yeah, these would be purely used for reserving regions in the IOVA
> > > > > > so
> > > > > > that they won't be used by the IOVA allocator. Typically these
> > > > > > would be
> > > > > > used for cases where those addresses have some special meaning.
> > > > > >
> > > > > > Do we want something like:
> > > > > >
> > > > > > compatible = "iommu-reserved";
> > > > > >
> > > > > > for these? Or would that need to be:
> > > > > >
> > > > > > compatible = "linux,iommu-reserved";
> > > > > >
> > > > > > ? There seems to be a mix of vendor-prefix vs. non-vendor-prefix
> > > > > > compatible strings in the reserved-memory DT bindings directory.
> > > > >
> > > > > I would not use 'linux,' here.
> > > > >
> > > > > >
> > > > > > On the other hand, do we actually need the compatible string?
> > > > > > Because we
> > > > > > don't really want to associate much extra information with this
> > > > > > like we
> > > > > > do for example with "shared-dma-pool". The logic to handle this
> > > > > > would
> > > > > > all be within the IOMMU framework. All we really need is for the
> > > > > > standard reservation code to skip nodes that don't have a reg
> > > > > > property
> > > > > > so we don't reserve memory for "virtual-only" allocations.
> > > > >
> > > > > It doesn't hurt to have one and I can imagine we might want to iterate
> > > > > over all the nodes. It's slightly easier and more common to iterate
> > > > > over compatible nodes rather than nodes with some property.
> > > > >
> > > > > > > Are these being global in DT going to be a problem? Presumably we
> > > > > > > have
> > > > > > > a virtual space per IOMMU. We'd know which IOMMU based on a
> > > > > > > device's
> > > > > > > 'iommus' and 'memory-region' properties, but within
> > > > > > > /reserved-memory
> > > > > > > we wouldn't be able to distinguish overlapping addresses from
> > > > > > > separate
> > > > > > > address spaces. Or we could have 2 different IOVAs for 1 physical
> > > > > > > space. That could be solved with something like this:
> > > > > > >
> > > > > > > iommu-addresses = < >;
> > > > > >
> > > > > > The only case that would be problematic would be if we have
> > > > > > overlapping
> > > > > > physical regions, because that will probably trip up the standard
> > > > > > code.
> > > > > >
> > > > > > But this could also be worked around by looking at iommu-addresses.
> > > > > > For
> > > > > > example, if we had something like this:
> > > > > >
> > > > > > reserved-memory {
> > > > > > fb_dc0: fb@8000 {
> > > > > > reg = <0x8000 0x0100>;
> > > > > > iommu-addresses = <0xa000 0x0100>;
> > > > > > };
> > > > > >
> > > > > > fb_dc1: fb@8000 {
> > > > >
> > > > > You can't have 2 nodes with the same name (actually, you can, they
> > > > > just get merged together). Different names with the same unit-address
> > > > > is a dtc warning. I'd
Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit
On Thu, Feb 10, 2022 at 10:49:04AM -0800, Jacob Pan wrote: > > On Wed, 9 Feb 2022 19:16:14 -0800, Jacob Pan > wrote: > > > Hi Fenghua, > > > > On Mon, 7 Feb 2022 15:02:48 -0800, Fenghua Yu > > wrote: > > > > > @@ -1047,8 +1040,6 @@ struct iommu_sva *intel_svm_bind(struct device > > > *dev, struct mm_struct *mm, void } > > > > > > sva = intel_svm_bind_mm(iommu, dev, mm, flags); > > > - if (IS_ERR_OR_NULL(sva)) > > > - intel_svm_free_pasid(mm); > > If bind fails, the PASID has no IOMMU nor CPU context. It should be safe > > to free here. > > > Actually, here we cannot tell if the bind is the first of the mm so I think > this is fine. > > Reviewed-by: Jacob Pan Thank you very much for your review, Jacob! -Fenghua ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit
On Wed, 9 Feb 2022 19:16:14 -0800, Jacob Pan wrote: > Hi Fenghua, > > On Mon, 7 Feb 2022 15:02:48 -0800, Fenghua Yu > wrote: > > > @@ -1047,8 +1040,6 @@ struct iommu_sva *intel_svm_bind(struct device > > *dev, struct mm_struct *mm, void } > > > > sva = intel_svm_bind_mm(iommu, dev, mm, flags); > > - if (IS_ERR_OR_NULL(sva)) > > - intel_svm_free_pasid(mm); > If bind fails, the PASID has no IOMMU nor CPU context. It should be safe > to free here. > Actually, here we cannot tell if the bind is the first of the mm so I think this is fine. Reviewed-by: Jacob Pan > Thanks, > > Jacob Thanks, Jacob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit
Hi, Tony, On Thu, Feb 10, 2022 at 09:24:50AM -0800, Luck, Tony wrote: > On Thu, Feb 10, 2022 at 08:27:50AM -0800, Fenghua Yu wrote: > > Hi, Jacob, > > > > On Wed, Feb 09, 2022 at 07:16:14PM -0800, Jacob Pan wrote: > > > Hi Fenghua, > > > > > > On Mon, 7 Feb 2022 15:02:48 -0800, Fenghua Yu > > > wrote: > > > > > > > @@ -1047,8 +1040,6 @@ struct iommu_sva *intel_svm_bind(struct device > > > > *dev, struct mm_struct *mm, void } > > > > > > > > sva = intel_svm_bind_mm(iommu, dev, mm, flags); > > > > - if (IS_ERR_OR_NULL(sva)) > > > > - intel_svm_free_pasid(mm); > > > If bind fails, the PASID has no IOMMU nor CPU context. It should be safe > > > to > > > free here. > > > > The PASID can not be freed even if bind fails. The PASID allocated earlier > > (either in this thread or in another thread) might be populated to other > > threads already and being used now. > > > > Without freeing the PASID on bind failure, the worst case is the PASID might > > not be used in the process (and will be freed on process exit anyway). > > > > This all matches with the PASID life time described in the commit message. > > But what does this mean for the user that failed that intel_svm_bind_mm()? > That means the user may have a PASID but there is no PASID entry for the device. Then ENQCMD cannot be executed successfully. > Here's a scenario: > > Process sets up to use PASID capable device #1. Everything works, > so the process has mm->pasid, and the IOMMU has the tables to map > virtual addresses coming from device #1 using that PASID. > > Now the same process asks to start using PASID capable device #2, > but there is a failure at intel_svm_bind_mm(). > > Fenghua is right that we shouldn't free the PASID. It is in use > by at least one thread of the process to access device #1. > > But what happens with device #2? Does the caller of intel_svm_bind() > do the right thing with the IS_ERR_OR_NULL return value to let the > user know that device #2 isn't accessible? A driver caller of intel_svm_bind() should handle this failure, i.e. fail the binding and let the user know the failure. Even if the driver doesn't do the right thing to handle the failure, intel_svm_bind() doesn't set up a PASID entry for device #2. One example is IDXD driver. User calls open()->IDXD driver idxd_cdev_open() ->intel_svm_bind()->intel_svm_bind_mm(). idxd_cdev_open() gets failed "sva" and passes the PTR_ERR(sva) to the user and the user cannot open the device. Due to the failure, no PASID entry is set up for the device. Even if the user ignores the open() failure and tries to run ENQCMD on device #2, a PASID table fault will be generated due to no PASID entry for the device and the ENQCMD execution will fail. Thanks. -Fenghua ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit
On Thu, Feb 10, 2022 at 08:27:50AM -0800, Fenghua Yu wrote: > Hi, Jacob, > > On Wed, Feb 09, 2022 at 07:16:14PM -0800, Jacob Pan wrote: > > Hi Fenghua, > > > > On Mon, 7 Feb 2022 15:02:48 -0800, Fenghua Yu wrote: > > > > > @@ -1047,8 +1040,6 @@ struct iommu_sva *intel_svm_bind(struct device > > > *dev, struct mm_struct *mm, void } > > > > > > sva = intel_svm_bind_mm(iommu, dev, mm, flags); > > > - if (IS_ERR_OR_NULL(sva)) > > > - intel_svm_free_pasid(mm); > > If bind fails, the PASID has no IOMMU nor CPU context. It should be safe to > > free here. > > The PASID can not be freed even if bind fails. The PASID allocated earlier > (either in this thread or in another thread) might be populated to other > threads already and being used now. > > Without freeing the PASID on bind failure, the worst case is the PASID might > not be used in the process (and will be freed on process exit anyway). > > This all matches with the PASID life time described in the commit message. But what does this mean for the user that failed that intel_svm_bind_mm()? Here's a scenario: Process sets up to use PASID capable device #1. Everything works, so the process has mm->pasid, and the IOMMU has the tables to map virtual addresses coming from device #1 using that PASID. Now the same process asks to start using PASID capable device #2, but there is a failure at intel_svm_bind_mm(). Fenghua is right that we shouldn't free the PASID. It is in use by at least one thread of the process to access device #1. But what happens with device #2? Does the caller of intel_svm_bind() do the right thing with the IS_ERR_OR_NULL return value to let the user know that device #2 isn't accessible? -Tony ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit
Hi, Jacob, On Wed, Feb 09, 2022 at 07:16:14PM -0800, Jacob Pan wrote: > Hi Fenghua, > > On Mon, 7 Feb 2022 15:02:48 -0800, Fenghua Yu wrote: > > > @@ -1047,8 +1040,6 @@ struct iommu_sva *intel_svm_bind(struct device > > *dev, struct mm_struct *mm, void } > > > > sva = intel_svm_bind_mm(iommu, dev, mm, flags); > > - if (IS_ERR_OR_NULL(sva)) > > - intel_svm_free_pasid(mm); > If bind fails, the PASID has no IOMMU nor CPU context. It should be safe to > free here. The PASID can not be freed even if bind fails. The PASID allocated earlier (either in this thread or in another thread) might be populated to other threads already and being used now. Without freeing the PASID on bind failure, the worst case is the PASID might not be used in the process (and will be freed on process exit anyway). This all matches with the PASID life time described in the commit message. Thanks. -Fenghua ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/amd: Fix I/O page table memory leak
The current logic updates the I/O page table mode for the domain
before calling the logic to free memory used for the page table.
This results in IOMMU page table memory leak, and can be observed
when launching VM w/ pass-through devices.
Fix by freeing the memory used for page table before updating the mode.
Cc: Joerg Roedel
Reported-by: Daniel Jordan
Tested-by: Daniel Jordan
Signed-off-by: Suravee Suthikulpanit
Fixes: e42ba0633064 ("iommu/amd: Restructure code for freeing page table")
Link: https://lore.kernel.org/all/20220118194720.urjgi73b7c3tq...@oracle.com/
---
drivers/iommu/amd/io_pgtable.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/amd/io_pgtable.c b/drivers/iommu/amd/io_pgtable.c
index b1bf4125b0f7..6608d1717574 100644
--- a/drivers/iommu/amd/io_pgtable.c
+++ b/drivers/iommu/amd/io_pgtable.c
@@ -492,18 +492,18 @@ static void v1_free_pgtable(struct io_pgtable *iop)
dom = container_of(pgtable, struct protection_domain, iop);
- /* Update data structure */
- amd_iommu_domain_clr_pt_root(dom);
-
- /* Make changes visible to IOMMUs */
- amd_iommu_domain_update(dom);
-
/* Page-table is not visible to IOMMU anymore, so free it */
BUG_ON(pgtable->mode < PAGE_MODE_NONE ||
pgtable->mode > PAGE_MODE_6_LEVEL);
free_sub_pt(pgtable->root, pgtable->mode, );
+ /* Update data structure */
+ amd_iommu_domain_clr_pt_root(dom);
+
+ /* Make changes visible to IOMMUs */
+ amd_iommu_domain_update(dom);
+
put_pages_list();
}
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/vt-d: Add missing "__init" for rmrr_sanity_check()
rmrr_sanity_check() calls arch_rmrr_sanity_check(), which is marked as
"__init". Add the annotation for rmrr_sanity_check() too. This function is
currently only called from dmar_parse_one_rmrr() which is also "__init".
Signed-off-by: Marco Bonelli
---
drivers/iommu/intel/iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 92fea3fbbb11..7a308fd8d4b9 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3691,7 +3691,7 @@ static void __init init_iommu_pm_ops(void)
static inline void init_iommu_pm_ops(void) {}
#endif /* CONFIG_PM */
-static int rmrr_sanity_check(struct acpi_dmar_reserved_memory *rmrr)
+static int __init rmrr_sanity_check(struct acpi_dmar_reserved_memory *rmrr)
{
if (!IS_ALIGNED(rmrr->base_address, PAGE_SIZE) ||
!IS_ALIGNED(rmrr->end_address + 1, PAGE_SIZE) ||
--
2.30.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: Remove trivial ops->capable implementations
Implementing ops->capable to always return false is pointless since it's
the default behaviour anyway. Clean up the unnecessary implementations.
Signed-off-by: Robin Murphy
---
Spinning this out of my bus ops stuff (currently 30 patches and
counting...) since it would be better off alongside Baolu's cleanup
series to avoid conflicts, and I want to depend on those patches for
dev_iommu_ops() anyway.
drivers/iommu/msm_iommu.c | 6 --
drivers/iommu/tegra-gart.c | 6 --
drivers/iommu/tegra-smmu.c | 6 --
3 files changed, 18 deletions(-)
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 06bde6b66732..22061ddbd5df 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -558,11 +558,6 @@ static phys_addr_t msm_iommu_iova_to_phys(struct
iommu_domain *domain,
return ret;
}
-static bool msm_iommu_capable(enum iommu_cap cap)
-{
- return false;
-}
-
static void print_ctx_regs(void __iomem *base, int ctx)
{
unsigned int fsr = GET_FSR(base, ctx);
@@ -672,7 +667,6 @@ irqreturn_t msm_iommu_fault_handler(int irq, void *dev_id)
}
static struct iommu_ops msm_iommu_ops = {
- .capable = msm_iommu_capable,
.domain_alloc = msm_iommu_domain_alloc,
.domain_free = msm_iommu_domain_free,
.attach_dev = msm_iommu_attach_dev,
diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
index 6a358f92c7e5..bbd287d19324 100644
--- a/drivers/iommu/tegra-gart.c
+++ b/drivers/iommu/tegra-gart.c
@@ -238,11 +238,6 @@ static phys_addr_t gart_iommu_iova_to_phys(struct
iommu_domain *domain,
return pte & GART_PAGE_MASK;
}
-static bool gart_iommu_capable(enum iommu_cap cap)
-{
- return false;
-}
-
static struct iommu_device *gart_iommu_probe_device(struct device *dev)
{
if (!dev_iommu_fwspec_get(dev))
@@ -276,7 +271,6 @@ static void gart_iommu_sync(struct iommu_domain *domain,
}
static const struct iommu_ops gart_iommu_ops = {
- .capable= gart_iommu_capable,
.domain_alloc = gart_iommu_domain_alloc,
.domain_free= gart_iommu_domain_free,
.attach_dev = gart_iommu_attach_dev,
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index e900e3c46903..43df44f918a1 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -272,11 +272,6 @@ static void tegra_smmu_free_asid(struct tegra_smmu *smmu,
unsigned int id)
clear_bit(id, smmu->asids);
}
-static bool tegra_smmu_capable(enum iommu_cap cap)
-{
- return false;
-}
-
static struct iommu_domain *tegra_smmu_domain_alloc(unsigned type)
{
struct tegra_smmu_as *as;
@@ -967,7 +962,6 @@ static int tegra_smmu_of_xlate(struct device *dev,
}
static const struct iommu_ops tegra_smmu_ops = {
- .capable = tegra_smmu_capable,
.domain_alloc = tegra_smmu_domain_alloc,
.domain_free = tegra_smmu_domain_free,
.attach_dev = tegra_smmu_attach_dev,
--
2.28.0.dirty
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: explicitly check for NULL in iommu_dma_get_resv_regions()
On 2022-02-09 14:09, Aleksandr Fedorov wrote:
iommu_dma_get_resv_regions() assumes that iommu_fwspec field for
corresponding device is set which is not always true. Since
iommu_dma_get_resv_regions() seems to be a future-proof generic API
that can be used by any iommu driver, add an explicit check for NULL.
Except it's not a "generic" interface for drivers to call at random,
it's a helper for retrieving common firmware-based information
specifically for drivers already using the fwspec mechanism for common
firmware bindings. If any driver calls this with a device *without* a
valid fwnode, it deserves to crash because it's done something
fundamentally wrong.
I concur that it's not exactly obvious that "non-IOMMU-specific" means
"based on common firmware bindings, thus implying fwspec".
Robin.
Currently it can work by accident since compiler can eliminate
the 'iommu_fwspec' check altogether when CONFIG_ACPI_IORT=n, but
code elimination from optimizations is not reliable.
Signed-off-by: Aleksandr Fedorov
---
A compilation failure has been observed on a gcc-compatible compiler based on
EDG.
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index d85d54f2b549..474b1b7211d7 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -382,10 +382,10 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
*/
void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
{
+ struct iommu_fwspec *iommu_fwspec = dev_iommu_fwspec_get(dev);
- if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode))
+ if (iommu_fwspec && !is_of_node(iommu_fwspec->iommu_fwnode))
iort_iommu_msi_get_resv_regions(dev, list);
-
}
EXPORT_SYMBOL(iommu_dma_get_resv_regions);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
