Skip to contentSkip to conversation

dma: swiotlb: Centralize default pool policy and sizing

PATCH Superseded Aneesh Kumar K.V (Arm) 3 participants 19 messages Sep 14 – Sep 18

Subsystems (13)

v4 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v4 cover dma: swiotlb: Centralize default pool policy and sizing

Hi,

This series centralizes swiotlb default-pool selection and sizing, and simplifies the initialization flags used to describe pool placement.

The first patch replaces the addressing-limit argument with initialization flags and introduces explicit policies for the different reasons a default pool may be needed. Architectures now report DMA addressing constraints with SWIOTLB_INIT_ADDRESSING_LIMIT, while the core selects the corresponding pool policy.

The second patch moves the reduced sizing used when the pool is needed only for unaligned kmalloc bouncing from arm64 and RISC-V into the swiotlb core.

The third patch centralizes swiotlb sizing for memory-encrypted systems. It moves the existing x86 guest-sizing policy into the core. Host memory encryption retains the normal pool size, and a shared default pool is not selected solely for guest memory encryption when a restricted DMA pool is present.

The fourth patch removes SWIOTLB_ANY. Pool placement is derived directly from SWIOTLB_INIT_ADDRESSING_LIMIT: pools required for limited DMA addressing are allocated below the architecture's low address limit, while other pools may be allocated anywhere in directly mapped memory. Xen continues to request a pool explicitly with SWIOTLB_INIT_REMAP without imposing a low-address constraint.

Changes from v3:
https://lore.kernel.org/all/[email protected]
* Add a new patch, "dma: swiotlb: Add an overridable architecture pool opt-out".
* Clarify the change in default behavior with the removal of SWIOTLB_ANY.

Changes from v2:
https://lore.kernel.org/all/[email protected]
* Rebase to latest tree.
* Drop SWIOTLB_ANY and derive that based on SWIOTLB_INIT_ADDRESSING_LIMIT
* Add SWIOTLB_INIT_REMAP.

v1:
https://lore.kernel.org/all/[email protected]

Cc Robin Murphy <[email protected]>
Cc Marek Szyprowski <[email protected]>
Cc Will Deacon <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Steven Price <[email protected]>
Cc Suzuki K Poulose <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Jiri Pirko <[email protected]>
Cc Jason Gunthorpe <[email protected]>
Cc Mostafa Saleh <[email protected]>
Cc Petr Tesarik <[email protected]>
Cc Alexey Kardashevskiy <[email protected]>
Cc Dan Williams <[email protected]>
Cc Xu Yilun <[email protected]>
Cc Madhavan Srinivasan <[email protected]>
Cc Michael Ellerman <[email protected]>
Cc Nicholas Piggin <[email protected]>
Cc "Christophe Leroy (CS GROUP)" <[email protected]>
Cc Alexander Gordeev <[email protected]>
Cc Gerald Schaefer <[email protected]>
Cc Heiko Carstens <[email protected]>
Cc Vasily Gorbik <[email protected]>
Cc Christian Borntraeger <[email protected]>
Cc Sven Schnelle <[email protected]>
Cc Russell King <[email protected]>
Cc Huacai Chen <[email protected]>
Cc Thomas Bogendoerfer <[email protected]>
Cc Jiaxun Yang <[email protected]>
Cc Paul Walmsley <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Albert Ou <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>

Aneesh Kumar K.V (Arm) (5):
dma: swiotlb: Centralize default pool policy selection
dma: swiotlb: Centralize minimal pool sizing
dma: swiotlb: Centralize memory-encryption pool sizing
dma: swiotlb: Add an overridable architecture pool opt-out
dma: swiotlb: Remove SWIOTLB_ANY

 Documentation/core-api/swiotlb.rst   |   8 +-
 arch/arm/mm/init.c                   |   6 +-
 arch/arm64/mm/init.c                 |  14 +-
 arch/loongarch/kernel/setup.c        |   2 +-
 arch/mips/cavium-octeon/dma-octeon.c |   2 +-
 arch/mips/loongson64/dma.c           |   2 +-
 arch/mips/sibyte/common/dma.c        |   2 +-
 arch/powerpc/kernel/dma-swiotlb.c    |   4 +-
 arch/powerpc/mm/mem.c                |  17 ++-
 arch/powerpc/platforms/pseries/svm.c |  10 --
 arch/powerpc/sysdev/fsl_pci.c        |   1 +
 arch/riscv/mm/init.c                 |  24 +--
 arch/s390/mm/init.c                  |   2 +-
 arch/x86/include/asm/crash_reserve.h |   6 +-
 arch/x86/include/asm/iommu.h         |   2 +
 arch/x86/kernel/amd_gart_64.c        |   1 +
 arch/x86/kernel/pci-dma.c            |  26 +++-
 arch/x86/mm/mem_encrypt.c            |  24 ---
 include/linux/swiotlb.h              |  21 ++-
 kernel/dma/swiotlb.c                 | 220 +++++++++++++++++++++------
 20 files changed, 252 insertions(+), 142 deletions(-)

base-commit: 704340f1cd0dcef829eb62f5b48ae95a2ce17bdf

Signature · 1 line
2.43.0
#1 lore raw

PATCH v4 1/5 dma: swiotlb: Centralize default pool policy selection

The addressing_limited argument to swiotlb_init() describes only one reason for allocating a default SWIOTLB pool. A pool may also be needed for memory encryption, unaligned kmalloc bouncing, or swiotlb=force.

Replace the boolean argument with the SWIOTLB_INIT_ADDRESSING_LIMIT flag and have architectures report their DMA addressing constraints through the initialization flags.

Introduce SWIOTLB pool policies and select the policy in the core before allocating the default pool. This separates the decision to allocate a pool from the policy used to size it, allowing subsequent changes to centralize minimal and confidential-guest sizing.

Set the shared-pool state before memory attributes are updated. Also move the pseries secure-guest setup before swiotlb_init() so its initialization flags are available when the policy is selected.

---
 arch/arm/mm/init.c                   |   6 +-
 arch/arm64/mm/init.c                 |   5 +-
 arch/loongarch/kernel/setup.c        |   2 +-
 arch/mips/cavium-octeon/dma-octeon.c |   2 +-
 arch/mips/loongson64/dma.c           |   2 +-
 arch/mips/sibyte/common/dma.c        |   2 +-
 arch/powerpc/kernel/dma-swiotlb.c    |   4 +-
 arch/powerpc/mm/mem.c                |  15 ++-
 arch/powerpc/platforms/pseries/svm.c |  10 --
 arch/powerpc/sysdev/fsl_pci.c        |   1 +
 arch/riscv/mm/init.c                 |   6 +-
 arch/s390/mm/init.c                  |   2 +-
 arch/x86/include/asm/iommu.h         |   2 +
 arch/x86/kernel/amd_gart_64.c        |   1 +
 arch/x86/kernel/pci-dma.c            |  19 ++--
 include/linux/swiotlb.h              |  12 ++-
 kernel/dma/swiotlb.c                 | 134 +++++++++++++++++++--------
 17 files changed, 156 insertions(+), 69 deletions(-)
arch/arm/mm/init.c +5 -1
index 0cc1bf04686d..aca97a4e5dcd 100644
@@ -223,7 +223,11 @@ static inline void poison_init_mem(void *s, size_t count)
 void __init arch_mm_preinit(void)
 {
 #ifdef CONFIG_ARM_LPAE
-	swiotlb_init(max_pfn > arm_dma_pfn_limit, SWIOTLB_VERBOSE);
+	unsigned int flags = SWIOTLB_VERBOSE;
+
+	if (max_pfn > arm_dma_pfn_limit)
+		flags |= SWIOTLB_INIT_ADDRESSING_LIMIT;
+	swiotlb_init(flags);
 #endif
 
 #ifdef CONFIG_SA1111
arch/arm64/mm/init.c +4 -1
index fbf215ecc7d0..7d51fb5f5a71 100644
@@ -351,7 +351,10 @@ void __init arch_mm_preinit(void)
 		swiotlb_adjust_size(min(swiotlb_size_or_default(), size));
 	}
 
-	swiotlb_init(true, flags);
+	if (max_pfn > PFN_DOWN(arm64_dma_phys_limit))
+		flags |= SWIOTLB_INIT_ADDRESSING_LIMIT;
+
+	swiotlb_init(flags);
 
 	/*
 	 * Check boundaries twice: Some fundamental inconsistencies can be
arch/loongarch/kernel/setup.c +1 -1
index 6fa4a22a58fd..02f52f6b2747 100644
@@ -404,7 +404,7 @@ static void __init arch_mem_init(char **cmdline_p)
 
 	memblock_set_bottom_up(true);
 
-	swiotlb_init(true, SWIOTLB_VERBOSE);
+	swiotlb_init(SWIOTLB_VERBOSE | SWIOTLB_INIT_ADDRESSING_LIMIT);
 
 	dma_contiguous_reserve(PFN_PHYS(max_low_pfn));
 
arch/mips/cavium-octeon/dma-octeon.c +1 -1
index 9fbba6a8fa4c..ca7c37bc070f 100644
@@ -235,5 +235,5 @@ void __init plat_swiotlb_setup(void)
 #endif
 
 	swiotlb_adjust_size(swiotlbsize);
-	swiotlb_init(true, SWIOTLB_VERBOSE);
+	swiotlb_init(SWIOTLB_VERBOSE | SWIOTLB_INIT_ADDRESSING_LIMIT);
 }
arch/mips/loongson64/dma.c +1 -1
index 52801442ea86..5b8056e6c5a8 100644
@@ -25,5 +25,5 @@ phys_addr_t dma_to_phys(struct device *dev, dma_addr_t daddr)
 
 void __init plat_swiotlb_setup(void)
 {
-	swiotlb_init(true, SWIOTLB_VERBOSE);
+	swiotlb_init(SWIOTLB_VERBOSE | SWIOTLB_INIT_ADDRESSING_LIMIT);
 }
arch/mips/sibyte/common/dma.c +1 -1
index c5c2c782aff6..3835fee21489 100644
@@ -10,5 +10,5 @@
 
 void __init plat_swiotlb_setup(void)
 {
-	swiotlb_init(true, SWIOTLB_VERBOSE);
+	swiotlb_init(SWIOTLB_VERBOSE | SWIOTLB_INIT_ADDRESSING_LIMIT);
 }
arch/powerpc/kernel/dma-swiotlb.c +3 -1
index ba256c37bcc0..97fffa46f05a 100644
@@ -14,8 +14,10 @@ unsigned int ppc_swiotlb_flags;
 
 void __init swiotlb_detect_4g(void)
 {
-	if ((memblock_end_of_DRAM() - 1) > 0xffffffff)
+	if ((memblock_end_of_DRAM() - 1) > 0xffffffff) {
 		ppc_swiotlb_enable = 1;
+		ppc_swiotlb_flags |= SWIOTLB_INIT_ADDRESSING_LIMIT;
+	}
 }
 
 static int __init check_swiotlb_enabled(void)
arch/powerpc/mm/mem.c +14 -1
index b617b69452cd..006ed41097e9 100644
@@ -287,6 +287,19 @@ void __init arch_mm_preinit(void)
 	BUILD_BUG_ON(MMU_PAGE_COUNT > 16);
 
 #ifdef CONFIG_SWIOTLB
+	if (is_secure_guest()) {
+
+		/* Don't release the SWIOTLB buffer. */
+		ppc_swiotlb_enable = 1;
+
+		/*
+		 * Since the guest memory is inaccessible to the host,
+		 * devices always need to use the SWIOTLB buffer for DMA
+		 * even if dma_capable() says otherwise.
+		 */
+		ppc_swiotlb_flags |= SWIOTLB_ANY;
+	}
+
 	/*
 	 * Some platforms (e.g. 85xx) limit DMA-able memory way below
 	 * 4G. We force memblock to bottom-up mode to ensure that the
@@ -295,7 +308,7 @@ void __init arch_mm_preinit(void)
 	 * back to to-down.
 	 */
 	memblock_set_bottom_up(true);
-	swiotlb_init(ppc_swiotlb_enable, ppc_swiotlb_flags);
+	swiotlb_init(ppc_swiotlb_flags);
 #endif
 
 	kasan_late_init();
arch/powerpc/platforms/pseries/svm.c +0 -10
index 7a403dbd35ee..4a0be631dc6d 100644
@@ -21,16 +21,6 @@ static int __init init_svm(void)
 	if (!is_secure_guest())
 		return 0;
 
-	/* Don't release the SWIOTLB buffer. */
-	ppc_swiotlb_enable = 1;
-
-	/*
-	 * Since the guest memory is inaccessible to the host, devices always
-	 * need to use the SWIOTLB buffer for DMA even if dma_capable() says
-	 * otherwise.
-	 */
-	ppc_swiotlb_flags |= SWIOTLB_ANY;
-
 	/* Share the SWIOTLB buffer with the host. */
 	swiotlb_update_mem_attributes();
 
arch/powerpc/sysdev/fsl_pci.c +1 -0
index 600f83cea1cd..49264e25108b 100644
@@ -444,6 +444,7 @@ static void setup_pci_atmu(struct pci_controller *hose)
 	if (hose->dma_window_size < mem) {
 #ifdef CONFIG_SWIOTLB
 		ppc_swiotlb_enable = 1;
+		ppc_swiotlb_flags |= SWIOTLB_INIT_ADDRESSING_LIMIT;
 #else
 		pr_err("%pOF: ERROR: Memory size exceeds PCI ATMU ability to "
 			"map - enable CONFIG_SWIOTLB to avoid dma errors.\n",
arch/riscv/mm/init.c +5 -1
index fb37b0b67efe..e0e4c103d603 100644
@@ -190,7 +190,11 @@ void __init arch_mm_preinit(void)
 		swiotlb_flags |= SWIOTLB_ANY;
 	}
 
-	swiotlb_init(swiotlb, swiotlb_flags);
+	if ((max_pfn > PFN_DOWN(dma32_phys_limit)) &&
+	     (memblock_start_of_DRAM() < dma32_phys_limit))
+		swiotlb_flags |= SWIOTLB_INIT_ADDRESSING_LIMIT;
+
+	swiotlb_init(swiotlb_flags);
 
 	print_vm_layout();
 }
arch/s390/mm/init.c +1 -1
index be7e009e7b59..78f507e24ceb 100644
@@ -166,7 +166,7 @@ static void __init pv_init(void)
 	virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
 
 	/* make sure bounce buffers are shared */
-	swiotlb_init(true, SWIOTLB_VERBOSE | SWIOTLB_ANY);
+	swiotlb_init(SWIOTLB_VERBOSE | SWIOTLB_ANY);
 	swiotlb_update_mem_attributes();
 }
 
arch/x86/include/asm/iommu.h +2 -0
index 3be2451e7bc8..22c8190fe34d 100644
@@ -14,8 +14,10 @@ extern bool amd_iommu_snp_en;
 
 #ifdef CONFIG_SWIOTLB
 extern bool x86_swiotlb_enable;
+extern unsigned int x86_swiotlb_flags;
 #else
 #define x86_swiotlb_enable false
+#define x86_swiotlb_flags 0
 #endif
 
 /* 10 seconds */
arch/x86/kernel/amd_gart_64.c +1 -0
index b5f1f031d45b..d0fbc0271e43 100644
@@ -814,6 +814,7 @@ int __init gart_iommu_init(void)
 	dma_ops = &gart_dma_ops;
 	x86_platform.iommu_shutdown = gart_iommu_shutdown;
 	x86_swiotlb_enable = false;
+	x86_swiotlb_flags = 0;
 
 	return 0;
 }
arch/x86/kernel/pci-dma.c +12 -7
index 75cf8f6ae8cd..3c82ebb5b6e0 100644
@@ -39,13 +39,15 @@ int iommu_detected __read_mostly = 0;
 
 #ifdef CONFIG_SWIOTLB
 bool x86_swiotlb_enable;
-static unsigned int x86_swiotlb_flags;
+unsigned int x86_swiotlb_flags;
 
 static void __init pci_swiotlb_detect(void)
 {
 	/* don't initialize swiotlb if iommu=off (no_iommu=1) */
-	if (!no_iommu && max_possible_pfn > MAX_DMA32_PFN)
+	if (!no_iommu && max_possible_pfn > MAX_DMA32_PFN) {
 		x86_swiotlb_enable = true;
+		x86_swiotlb_flags |= SWIOTLB_INIT_ADDRESSING_LIMIT;
+	}
 
 	/*
 	 * Set swiotlb to 1 so that bounce buffers are allocated and used for
@@ -66,7 +68,6 @@ static void __init pci_swiotlb_detect(void)
 static inline void __init pci_swiotlb_detect(void)
 {
 }
-#define x86_swiotlb_flags 0
 #endif /* CONFIG_SWIOTLB */
 
 #ifdef CONFIG_SWIOTLB_XEN
@@ -81,8 +82,10 @@ static void __init pci_xen_swiotlb_init(void)
 	if (!xen_swiotlb_enabled())
 		return;
 	x86_swiotlb_enable = true;
-	x86_swiotlb_flags |= SWIOTLB_ANY;
-	swiotlb_init_remap(true, x86_swiotlb_flags, xen_swiotlb_fixup);
+	/* Xen can use a SWIOTLB pool anywhere in directly mapped memory. */
+	x86_swiotlb_flags &= ~SWIOTLB_INIT_ADDRESSING_LIMIT;
+	x86_swiotlb_flags |= SWIOTLB_INIT_REMAP | SWIOTLB_ANY;
+	swiotlb_init_remap(x86_swiotlb_flags, xen_swiotlb_fixup);
 	dma_ops = &xen_swiotlb_dma_ops;
 	if (IS_ENABLED(CONFIG_PCI))
 		pci_request_acs();
@@ -103,7 +106,7 @@ void __init pci_iommu_alloc(void)
 	gart_iommu_hole_init();
 	amd_iommu_detect();
 	detect_intel_iommu();
-	swiotlb_init(x86_swiotlb_enable, x86_swiotlb_flags);
+	swiotlb_init(x86_swiotlb_flags);
 }
 
 static __init int iommu_setup(char *p)
@@ -149,8 +152,10 @@ static __init int iommu_setup(char *p)
 			return 1;
 		}
 #ifdef CONFIG_SWIOTLB
-		if (!strncmp(p, "soft", 4))
+		if (!strncmp(p, "soft", 4)) {
 			x86_swiotlb_enable = true;
+			x86_swiotlb_flags |= SWIOTLB_INIT_ADDRESSING_LIMIT;
+		}
 #endif
 		if (!strncmp(p, "pt", 2))
 			iommu_set_default_passthrough(true);
include/linux/swiotlb.h +8 -4
index 2a421241b980..d69370c08094 100644
@@ -16,6 +16,10 @@ struct scatterlist;
 
 #define SWIOTLB_VERBOSE	(1 << 0) /* verbose initialization */
 #define SWIOTLB_ANY	(1 << 1) /* allow any memory for the buffer */
+/* Initialize a default-sized pool for devices with limited DMA addressing. */
+#define SWIOTLB_INIT_ADDRESSING_LIMIT	(1 << 2)
+/* Initialize a default-sized pool that requires architecture remapping. */
+#define SWIOTLB_INIT_REMAP		(1 << 3)
 
 /*
  * Maximum allowable number of contiguous slabs to map,
@@ -39,8 +43,8 @@ struct scatterlist;
 #endif
 
 unsigned long swiotlb_size_or_default(void);
-void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
-	int (*remap)(void *tlb, unsigned long nslabs));
+void __init swiotlb_init_remap(unsigned int flags,
+			       int (*remap)(void *tlb, unsigned long nslabs));
 int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 	int (*remap)(void *tlb, unsigned long nslabs));
 extern void __init swiotlb_update_mem_attributes(void);
@@ -183,7 +187,7 @@ static inline bool is_swiotlb_force_bounce(struct device *dev)
 	return mem && mem->force_bounce;
 }
 
-void swiotlb_init(bool addressing_limited, unsigned int flags);
+void swiotlb_init(unsigned int flags);
 void __init swiotlb_exit(void);
 void swiotlb_dev_init(struct device *dev);
 size_t swiotlb_max_mapping_size(struct device *dev);
@@ -193,7 +197,7 @@ void __init swiotlb_adjust_size(unsigned long size);
 phys_addr_t default_swiotlb_base(void);
 phys_addr_t default_swiotlb_limit(void);
 #else
-static inline void swiotlb_init(bool addressing_limited, unsigned int flags)
+static inline void swiotlb_init(unsigned int flags)
 {
 }
 
kernel/dma/swiotlb.c +96 -38

199 lines not on this page · raw message

Signature · 1 line
2.43.0
#2 lore raw

Re: [PATCH v4 1/5] dma: swiotlb: Centralize default pool policy selection

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 13 lines

The addressing_limited argument to swiotlb_init() describes only one reason for allocating a default SWIOTLB pool. A pool may also be needed for memory encryption, unaligned kmalloc bouncing, or swiotlb=force.

Replace the boolean argument with the SWIOTLB_INIT_ADDRESSING_LIMIT flag and have architectures report their DMA addressing constraints through the initialization flags.

Introduce SWIOTLB pool policies and select the policy in the core before allocating the default pool. This separates the decision to allocate a pool from the policy used to size it, allowing subsequent changes to centralize minimal and confidential-guest sizing.

Quoted 5 lines

Set the shared-pool state before memory attributes are updated. Also move the pseries secure-guest setup before swiotlb_init() so its initialization flags are available when the policy is selected.

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
maintainer Reviewed-by on v4 1/5
#3 lore raw

Re: [PATCH v4 1/5] dma: swiotlb: Centralize default pool policy selection

In reply to Aneesh Kumar K.V (Arm)
Quoted 20 lines · Aneesh Kumar K.V (Arm)
arch/x86/include/asm/iommu.h +2 -0
index 3be2451e7bc8..22c8190fe34d 100644
@@ -14,8 +14,10 @@ extern bool amd_iommu_snp_en;
 
 #ifdef CONFIG_SWIOTLB
 extern bool x86_swiotlb_enable;
+extern unsigned int x86_swiotlb_flags;
 #else
 #define x86_swiotlb_enable false
+#define x86_swiotlb_flags 0
 #endif
 
 /* 10 seconds */
arch/x86/kernel/amd_gart_64.c +0 -0
index b5f1f031d45b..d0fbc0271e43 100644

arch/x86/kernel/amd_gart_64.c int __init gart_iommu_init(void) · line 817

Quoted 4 lines
arch/x86/kernel/amd_gart_64.c +1 -0
@@ -814,6 +814,7 @@ int __init gart_iommu_init(void)
 	dma_ops = &gart_dma_ops;
 	x86_platform.iommu_shutdown = gart_iommu_shutdown;
 	x86_swiotlb_enable = false;
+	x86_swiotlb_flags = 0;

I don't think the flags are still read anywhere after this function. gart_iommu_init() is called long after swiotlb_init(). So you can drop this and make x86_swiotlb_flags static in pci-dma.c.

With this:

#4 lore raw

PATCH v4 2/5 dma: swiotlb: Centralize minimal pool sizing

In reply to Aneesh Kumar K.V (Arm)

A default SWIOTLB pool used only for unaligned kmalloc bouncing can be smaller than one required for limited DMA addressing. Arm64 and RISC-V currently implement this sizing independently, using 1 MiB of SWIOTLB space per GiB of RAM.

The SWIOTLB core now identifies this case with SWIOTLB_POOL_MINIMAL. Size that policy in swiotlb_adjust_pool_size() and remove the architecture-specific adjustments.

Explicit swiotlb= sizing remains unchanged because swiotlb_adjust_size() preserves a user-configured size.

NOTE: We lose the RISC-V SWIOTLB_ANY setting in this patch. It will be reinstated in a follow-up patch where we make SWIOTLB_ANY redundant.

---
 arch/arm64/mm/init.c | 11 -----------
 arch/riscv/mm/init.c | 20 --------------------
 kernel/dma/swiotlb.c | 18 ++++++++++++++++--
 3 files changed, 16 insertions(+), 33 deletions(-)
arch/arm64/mm/init.c +0 -11
index 7d51fb5f5a71..95075b5c8207 100644
@@ -340,17 +340,6 @@ void __init arch_mm_preinit(void)
 {
 	unsigned int flags = SWIOTLB_VERBOSE;
 
-	if (max_pfn <= PFN_DOWN(arm64_dma_phys_limit)) {
-		/*
-		 * If no bouncing needed for ZONE_DMA, reduce the swiotlb
-		 * buffer for kmalloc() bouncing to 1MB per 1GB of RAM.
-		 */
-		unsigned long size =
-			DIV_ROUND_UP(memblock_phys_mem_size(), 1024);
-
-		swiotlb_adjust_size(min(swiotlb_size_or_default(), size));
-	}
-
 	if (max_pfn > PFN_DOWN(arm64_dma_phys_limit))
 		flags |= SWIOTLB_INIT_ADDRESSING_LIMIT;
 
arch/riscv/mm/init.c +0 -20
index e0e4c103d603..734712230f35 100644
@@ -165,31 +165,11 @@ static void print_vm_layout(void) { }
 
 void __init arch_mm_preinit(void)
 {
-	bool swiotlb = max_pfn > PFN_DOWN(dma32_phys_limit) &&
-		       memblock_start_of_DRAM() < dma32_phys_limit;
 	unsigned int swiotlb_flags = SWIOTLB_VERBOSE;
 #ifdef CONFIG_FLATMEM
 	BUG_ON(!mem_map);
 #endif /* CONFIG_FLATMEM */
 
-	if (IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) && !swiotlb &&
-	    dma_cache_alignment != 1) {
-		/*
-		 * No 32-bit DMA bouncing needed (either all DRAM is within
-		 * the 32-bit limit, or it all starts above it), but
-		 * kmalloc() buffers whose sizes are not cache-line-aligned
-		 * still require bouncing for non-coherent DMA.  Use
-		 * SWIOTLB_ANY so that the buffer can be allocated from high
-		 * memory when DRAM starts above dma32_phys_limit.  Allocate
-		 * ~1 MB per 1 GB of RAM.
-		 */
-		unsigned long size =
-			DIV_ROUND_UP(memblock_phys_mem_size(), 1024);
-		swiotlb_adjust_size(min(swiotlb_size_or_default(), size));
-		swiotlb = true;
-		swiotlb_flags |= SWIOTLB_ANY;
-	}
-
 	if ((max_pfn > PFN_DOWN(dma32_phys_limit)) &&
 	     (memblock_start_of_DRAM() < dma32_phys_limit))
 		swiotlb_flags |= SWIOTLB_INIT_ADDRESSING_LIMIT;
kernel/dma/swiotlb.c +16 -2
index aa19e81ae544..90e0bfd0715c 100644
@@ -304,6 +304,11 @@ unsigned long swiotlb_size_or_default(void)
 	return default_nslabs << IO_TLB_SHIFT;
 }
 
+static bool __init swiotlb_default_size_changed(void)
+{
+	return default_nslabs != IO_TLB_DEFAULT_SIZE >> IO_TLB_SHIFT;
+}
+
 void __init swiotlb_adjust_size(unsigned long size)
 {
 	/*
@@ -311,7 +316,7 @@ void __init swiotlb_adjust_size(unsigned long size)
 	 * architectures such as those supporting memory encryption to
 	 * adjust/expand SWIOTLB size for their use.
 	 */
-	if (default_nslabs != IO_TLB_DEFAULT_SIZE >> IO_TLB_SHIFT)
+	if (swiotlb_default_size_changed())
 		return;
 
 	default_nslabs = swiotlb_aligned_nslabs(size);
@@ -476,9 +481,18 @@ static bool __init swiotlb_kmalloc_needs_bounce(void)
 static void __init
 swiotlb_adjust_pool_size(enum swiotlb_pool_policy policy)
 {
+	if (swiotlb_default_size_changed())
+		return;
+
 	switch (policy) {
-	case SWIOTLB_POOL_MINIMAL:
+	case SWIOTLB_POOL_MINIMAL: {
+		unsigned long size;
+
+		/* Use 1MB per 1GB of RAM for kmalloc() bouncing. */
+		size = DIV_ROUND_UP(memblock_phys_mem_size(), 1024);
+		swiotlb_adjust_size(min(swiotlb_size_or_default(), size));
 		break;
+	}
 	case SWIOTLB_POOL_CC_GUEST:
 		break;
 	case SWIOTLB_POOL_NONE:
Signature · 1 line
2.43.0
#5 lore raw

Re: [PATCH v4 2/5] dma: swiotlb: Centralize minimal pool sizing

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 9 lines

A default SWIOTLB pool used only for unaligned kmalloc bouncing can be smaller than one required for limited DMA addressing. Arm64 and RISC-V currently implement this sizing independently, using 1 MiB of SWIOTLB space per GiB of RAM.

The SWIOTLB core now identifies this case with SWIOTLB_POOL_MINIMAL. Size that policy in swiotlb_adjust_pool_size() and remove the architecture-specific adjustments.

Quoted 7 lines

Explicit swiotlb= sizing remains unchanged because swiotlb_adjust_size() preserves a user-configured size.

NOTE: We lose the RISC-V SWIOTLB_ANY setting in this patch. It will be reinstated in a follow-up patch where we make SWIOTLB_ANY redundant.

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
maintainer
#6 lore raw

Re: [PATCH v4 2/5] dma: swiotlb: Centralize minimal pool sizing

In reply to Aneesh Kumar K.V (Arm)
Quoted 2 lines · Aneesh Kumar K.V (Arm)

NOTE: We lose the RISC-V SWIOTLB_ANY setting in this patch. It will be reinstated in a follow-up patch where we make SWIOTLB_ANY redundant.

[...]

Quoted 22 lines
arch/riscv/mm/init.c +0 -9
index e0e4c103d603..734712230f35 100644
@@ -165,31 +165,11 @@ static void print_vm_layout(void) { }
 
 void __init arch_mm_preinit(void)
 {
-	bool swiotlb = max_pfn > PFN_DOWN(dma32_phys_limit) &&
-		       memblock_start_of_DRAM() < dma32_phys_limit;
 	unsigned int swiotlb_flags = SWIOTLB_VERBOSE;
 #ifdef CONFIG_FLATMEM
 	BUG_ON(!mem_map);
 #endif /* CONFIG_FLATMEM */
 
-	if (IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) && !swiotlb &&
-	    dma_cache_alignment != 1) {
-		/*
-		 * No 32-bit DMA bouncing needed (either all DRAM is within
-		 * the 32-bit limit, or it all starts above it), but
-		 * kmalloc() buffers whose sizes are not cache-line-aligned
-		 * still require bouncing for non-coherent DMA.  Use

arch/riscv/mm/init.c static void print_vm_layout(void) { } · line 191

Quoted 10 lines
arch/riscv/mm/init.c +0 -10
-		 * SWIOTLB_ANY so that the buffer can be allocated from high
-		 * memory when DRAM starts above dma32_phys_limit.  Allocate
-		 * ~1 MB per 1 GB of RAM.
-		 */
-		unsigned long size =
-			DIV_ROUND_UP(memblock_phys_mem_size(), 1024);
-		swiotlb_adjust_size(min(swiotlb_size_or_default(), size));
-		swiotlb = true;
-		swiotlb_flags |= SWIOTLB_ANY;
-	}

Does it break the bisectability for risc-v? IIUC this undoes commit cfca5a48b03f until patch 5. Basically if there's no low memory and no SWIOTLB_ANY passed, memblock_alloc_low() will fail.

I think we could preserve SWIOTLB_ANY until patch 5. Or it might be simpler to move patches 2 and 3 at the end (I haven't tried).

kernel/dma/swiotlb.c unsigned long swiotlb_size_or_default(void) · line 310

Quoted 12 lines
kernel/dma/swiotlb.c +4 -0
index aa19e81ae544..90e0bfd0715c 100644
@@ -304,6 +304,11 @@ unsigned long swiotlb_size_or_default(void)
 	return default_nslabs << IO_TLB_SHIFT;
 }
 
+static bool __init swiotlb_default_size_changed(void)
+{
+	return default_nslabs != IO_TLB_DEFAULT_SIZE >> IO_TLB_SHIFT;
+}

I mentioned before in passing
(https://lore.kernel.org/all/[email protected]/), if one sets swiotlb= to the default size, it will go unnoticed. I know it's an existing issue but we might as well fix it now with a bool that tells us the swiotlb size was forced/changed.

Signature · 1 line
Catalin
#7 lore raw

Re: [PATCH v4 2/5] dma: swiotlb: Centralize minimal pool sizing

Quoted 34 lines · Catalin Marinas
Quoted 2 lines · Aneesh Kumar K.V (Arm)

NOTE: We lose the RISC-V SWIOTLB_ANY setting in this patch. It will be reinstated in a follow-up patch where we make SWIOTLB_ANY redundant.

[...]

Quoted 28 lines
arch/riscv/mm/init.c +0 -15
index e0e4c103d603..734712230f35 100644
@@ -165,31 +165,11 @@ static void print_vm_layout(void) { }
 
 void __init arch_mm_preinit(void)
 {
-	bool swiotlb = max_pfn > PFN_DOWN(dma32_phys_limit) &&
-		       memblock_start_of_DRAM() < dma32_phys_limit;
 	unsigned int swiotlb_flags = SWIOTLB_VERBOSE;
 #ifdef CONFIG_FLATMEM
 	BUG_ON(!mem_map);
 #endif /* CONFIG_FLATMEM */
 
-	if (IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) && !swiotlb &&
-	    dma_cache_alignment != 1) {
-		/*
-		 * No 32-bit DMA bouncing needed (either all DRAM is within
-		 * the 32-bit limit, or it all starts above it), but
-		 * kmalloc() buffers whose sizes are not cache-line-aligned
-		 * still require bouncing for non-coherent DMA.  Use
-		 * SWIOTLB_ANY so that the buffer can be allocated from high
-		 * memory when DRAM starts above dma32_phys_limit.  Allocate
-		 * ~1 MB per 1 GB of RAM.
-		 */
-		unsigned long size =
-			DIV_ROUND_UP(memblock_phys_mem_size(), 1024);
Quoted 9 lines

arch/riscv/mm/init.c static void print_vm_layout(void) { } · line 191

Quoted 4 lines
arch/riscv/mm/init.c +0 -4
-		swiotlb_adjust_size(min(swiotlb_size_or_default(), size));
-		swiotlb = true;
-		swiotlb_flags |= SWIOTLB_ANY;
-	}

Does it break the bisectability for risc-v? IIUC this undoes commit cfca5a48b03f until patch 5. Basically if there's no low memory and no SWIOTLB_ANY passed, memblock_alloc_low() will fail.

I mentioned this in the commit message. I can add the change below and remove it in patch 5.

modified arch/riscv/mm/init.c
@@ -165,11 +165,27 @@ static void print_vm_layout(void) { }

void __init arch_mm_preinit(void)
{
+ bool swiotlb = max_pfn > PFN_DOWN(dma32_phys_limit) &&
+ memblock_start_of_DRAM() < dma32_phys_limit;
unsigned int swiotlb_flags = SWIOTLB_VERBOSE;
#ifdef CONFIG_FLATMEM
BUG_ON(!mem_map);
#endif /* CONFIG_FLATMEM */

+ if (IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) && !swiotlb &&
+ dma_cache_alignment != 1) {
+ /*
+ * No 32-bit DMA bouncing needed (either all DRAM is within
+ * the 32-bit limit, or it all starts above it), but
+ * kmalloc() buffers whose sizes are not cache-line-aligned
+ * still require bouncing for non-coherent DMA. Use
+ * SWIOTLB_ANY so that the buffer can be allocated from high
+ * memory when DRAM starts above dma32_phys_limit. Allocate
+ * ~1 MB per 1 GB of RAM.
+ */
+ swiotlb_flags |= SWIOTLB_ANY;
+ }
+
if ((max_pfn > PFN_DOWN(dma32_phys_limit)) &&
(memblock_start_of_DRAM() < dma32_phys_limit))
swiotlb_flags |= SWIOTLB_INIT_ADDRESSING_LIMIT;

Quoted 12 lines

I think we could preserve SWIOTLB_ANY until patch 5. Or it might be simpler to move patches 2 and 3 at the end (I haven't tried).

Quoted 8 lines
kernel/dma/swiotlb.c +0 -0
index aa19e81ae544..90e0bfd0715c 100644
@@ -304,6 +304,11 @@ unsigned long swiotlb_size_or_default(void)
 	return default_nslabs << IO_TLB_SHIFT;
 }
 
Quoted 11 lines

kernel/dma/swiotlb.c unsigned long swiotlb_size_or_default(void) · line 310

Quoted 4 lines
kernel/dma/swiotlb.c +4 -0
+static bool __init swiotlb_default_size_changed(void)
+{
+	return default_nslabs != IO_TLB_DEFAULT_SIZE >> IO_TLB_SHIFT;
+}

I mentioned before in passing
(https://lore.kernel.org/all/[email protected]/), if one sets swiotlb= to the default size, it will go unnoticed. I know it's an existing issue but we might as well fix it now with a bool that tells us the swiotlb size was forced/changed.

I will do this as an addtional patch

modified kernel/dma/swiotlb.c
@@ -108,6 +108,7 @@

static unsigned long default_nslabs = IO_TLB_DEFAULT_SIZE >> IO_TLB_SHIFT; static unsigned long default_nareas;
+static bool swiotlb_size_set __initdata;

/**
* struct io_tlb_area - IO TLB memory area descriptor
@@ -270,6 +271,7 @@
/* avoid tail segment of size < IO_TLB_SEGSIZE */ default_nslabs =
ALIGN(simple_strtoul(str, &str, 0), IO_TLB_SEGSIZE);
+ swiotlb_size_set = true;
}
if (*str == ',')
++str;
@@ -307,7 +309,7 @@

static bool __init swiotlb_default_size_changed(void) {
- return default_nslabs != IO_TLB_DEFAULT_SIZE >> IO_TLB_SHIFT;
+ return swiotlb_size_set;
}

void __init swiotlb_adjust_size(unsigned long size)

#8 lore raw

PATCH v4 3/5 dma: swiotlb: Centralize memory-encryption pool sizing

In reply to Aneesh Kumar K.V (Arm)

Memory-encrypted guests use shared or unencrypted memory for DMA and may route all DMA through SWIOTLB. The default pool can therefore be too small for I/O-intensive workloads.

Move the existing x86 guest-sizing policy into the SWIOTLB core. For SWIOTLB_POOL_CC_GUEST, size the pool to 6% of guest memory, clamped between the normal default and 1 GiB. Preserve an explicit swiotlb= size.

Provide swiotlb_adjusted_size() so early users, including the x86 crash kernel reservation, can account for the prospective guest pool size before SWIOTLB initialization. Use the same area-aware alignment for both the prospective and allocated sizes.

Host memory encryption still selects a normal-sized shared pool and does not use the guest-sizing policy.

A restricted DMA pool already provides shared bounce buffers for its devices. Record its presence during reserved-memory initialization and do not select the confidential-guest default-pool policy solely because guest memory encryption is active.

---
 arch/x86/include/asm/crash_reserve.h |  6 ++--
 arch/x86/mm/mem_encrypt.c            | 24 --------------
 include/linux/swiotlb.h              |  6 ++++
 kernel/dma/swiotlb.c                 | 49 +++++++++++++++++++++++++++-
 4 files changed, 58 insertions(+), 27 deletions(-)
arch/x86/include/asm/crash_reserve.h +4 -2
index 7835b2cdff04..c1f5bf5d4182 100644
@@ -18,7 +18,9 @@
  * no good way to detect the paging mode of the target kernel which will be
  * loaded for dumping.
  */
-extern unsigned long swiotlb_size_or_default(void);
+#ifdef CONFIG_X86_64
+unsigned long __init swiotlb_adjusted_size(void);
+#endif
 
 #ifdef CONFIG_X86_32
 # define CRASH_ADDR_LOW_MAX     SZ_512M
@@ -33,7 +35,7 @@ extern unsigned long swiotlb_size_or_default(void);
 static inline unsigned long crash_low_size_default(void)
 {
 #ifdef CONFIG_X86_64
-	return max(swiotlb_size_or_default() + (8UL << 20), 256UL << 20);
+	return max(swiotlb_adjusted_size() + (8UL << 20), 256UL << 20);
 #else
 	return 0;
 #endif
arch/x86/mm/mem_encrypt.c +0 -24
index 95bae74fdab2..912f22ca838f 100644
@@ -101,9 +101,6 @@ void __init mem_encrypt_init(void)
 
 void __init mem_encrypt_setup_arch(void)
 {
-	phys_addr_t total_mem = memblock_phys_mem_size();
-	unsigned long size;
-
 	/*
 	 * Do RMP table fixups after the e820 tables have been setup by
 	 * e820__memory_setup().
@@ -114,27 +111,6 @@ void __init mem_encrypt_setup_arch(void)
 	if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
 		return;
 
-	/*
-	 * For SEV and TDX, all DMA has to occur via shared/unencrypted pages.
-	 * Kernel uses SWIOTLB to make this happen without changing device
-	 * drivers. However, depending on the workload being run, the
-	 * default 64MB of SWIOTLB may not be enough and SWIOTLB may
-	 * run out of buffers for DMA, resulting in I/O errors and/or
-	 * performance degradation especially with high I/O workloads.
-	 *
-	 * Adjust the default size of SWIOTLB using a percentage of guest
-	 * memory for SWIOTLB buffers. Also, as the SWIOTLB bounce buffer
-	 * memory is allocated from low memory, ensure that the adjusted size
-	 * is within the limits of low available memory.
-	 *
-	 * The percentage of guest memory used here for SWIOTLB buffers
-	 * is more of an approximation of the static adjustment which
-	 * 64MB for <1G, and ~128M to 256M for 1G-to-4G, i.e., the 6%
-	 */
-	size = total_mem * 6 / 100;
-	size = clamp_val(size, IO_TLB_DEFAULT_SIZE, SZ_1G);
-	swiotlb_adjust_size(size);
-
 	/* Set restricted memory access for virtio. */
 	virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
 }
include/linux/swiotlb.h +6 -0
index d69370c08094..be3962a33fc6 100644
@@ -193,6 +193,7 @@ void swiotlb_dev_init(struct device *dev);
 size_t swiotlb_max_mapping_size(struct device *dev);
 bool is_swiotlb_allocated(void);
 bool is_swiotlb_active(struct device *dev);
+unsigned long __init swiotlb_adjusted_size(void);
 void __init swiotlb_adjust_size(unsigned long size);
 phys_addr_t default_swiotlb_base(void);
 phys_addr_t default_swiotlb_limit(void);
@@ -232,6 +233,11 @@ static inline bool is_swiotlb_active(struct device *dev)
 	return false;
 }
 
+static inline unsigned long __init swiotlb_adjusted_size(void)
+{
+	return 0;
+}
+
 static inline void swiotlb_adjust_size(unsigned long size)
 {
 }
kernel/dma/swiotlb.c +48 -1

79 lines not on this page · raw message

Signature · 1 line
2.43.0
#9 lore raw

Re: [PATCH v4 3/5] dma: swiotlb: Centralize memory-encryption pool sizing

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 17 lines

Memory-encrypted guests use shared or unencrypted memory for DMA and may route all DMA through SWIOTLB. The default pool can therefore be too small for I/O-intensive workloads.

Move the existing x86 guest-sizing policy into the SWIOTLB core. For SWIOTLB_POOL_CC_GUEST, size the pool to 6% of guest memory, clamped between the normal default and 1 GiB. Preserve an explicit swiotlb= size.

Provide swiotlb_adjusted_size() so early users, including the x86 crash kernel reservation, can account for the prospective guest pool size before SWIOTLB initialization. Use the same area-aware alignment for both the prospective and allocated sizes.

Host memory encryption still selects a normal-sized shared pool and does not use the guest-sizing policy.

Quoted 6 lines

A restricted DMA pool already provides shared bounce buffers for its devices. Record its presence during reserved-memory initialization and do not select the confidential-guest default-pool policy solely because guest memory encryption is active.

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
maintainer Reviewed-by on v4 3/5
#10 lore raw

Re: [PATCH v4 3/5] dma: swiotlb: Centralize memory-encryption pool sizing

In reply to Aneesh Kumar K.V (Arm)
Quoted 20 lines · Aneesh Kumar K.V (Arm)
kernel/dma/swiotlb.c +20 -0
+/**
+ * swiotlb_adjusted_size() - get the prospective adjusted SWIOTLB size
+ *
+ * Return the size that confidential-computing guest sizing would select for
+ * the default pool, without changing the configured SWIOTLB size. An
+ * explicit swiotlb= size is always preserved. An explicit area count is
+ * included in the size calculation. Automatic area sizing is initialized
+ * later from the running kernel's possible CPU map and any resulting size
+ * adjustment is therefore not reflected in the returned size.
+ */
+unsigned long __init swiotlb_adjusted_size(void)
+{
+	unsigned long nslabs, size = swiotlb_size_or_default();
+
+	if (swiotlb_default_size_changed() ||
+	    !cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
+		return size;
+	/*
+	 * For SEV and TDX and CCA, all DMA has to occur via
+	 * shared/unencrypted pages. Kernel uses SWIOTLB to make this

kernel/dma/swiotlb.c static bool __init swiotlb_kmalloc_needs_bounce(void) · line 511

Quoted 10 lines
kernel/dma/swiotlb.c +10 -0
+	 * happen without changing device drivers. However, depending on
+	 * the workload being run, the default 64MB of SWIOTLB may not be
+	 * enough and SWIOTLB may run out of buffers for DMA, resulting in
+	 * I/O errors and/or performance degradation especially with high
+	 * I/O workloads.
+	 *
+	 * Adjust the default size of SWIOTLB using a percentage of guest
+	 * memory for SWIOTLB buffers. Also, as the SWIOTLB bounce buffer
+	 * memory is allocated from low memory, ensure that the adjusted
+	 * size is within the limits of low available memory.

Nit: if SWIOTLB_ANY is passed, do we still allocate only from low memory? I think this comment made sense on x86 but it's not valid for s390, pseries with the changes so far.

Otherwise:

#11 lore raw

Re: [PATCH v4 3/5] dma: swiotlb: Centralize memory-encryption pool sizing

Quoted 27 lines · Catalin Marinas
Quoted 26 lines · Aneesh Kumar K.V (Arm)
kernel/dma/swiotlb.c +26 -0
+/**
+ * swiotlb_adjusted_size() - get the prospective adjusted SWIOTLB size
+ *
+ * Return the size that confidential-computing guest sizing would select for
+ * the default pool, without changing the configured SWIOTLB size. An
+ * explicit swiotlb= size is always preserved. An explicit area count is
+ * included in the size calculation. Automatic area sizing is initialized
+ * later from the running kernel's possible CPU map and any resulting size
+ * adjustment is therefore not reflected in the returned size.
+ */
+unsigned long __init swiotlb_adjusted_size(void)
+{
+	unsigned long nslabs, size = swiotlb_size_or_default();
+
+	if (swiotlb_default_size_changed() ||
+	    !cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
+		return size;
+	/*
+	 * For SEV and TDX and CCA, all DMA has to occur via
+	 * shared/unencrypted pages. Kernel uses SWIOTLB to make this
+	 * happen without changing device drivers. However, depending on
+	 * the workload being run, the default 64MB of SWIOTLB may not be
+	 * enough and SWIOTLB may run out of buffers for DMA, resulting in
+	 * I/O errors and/or performance degradation especially with high
+	 * I/O workloads.
+	 *
Quoted 9 lines

kernel/dma/swiotlb.c static bool __init swiotlb_kmalloc_needs_bounce(void) · line 511

Quoted 4 lines
kernel/dma/swiotlb.c +4 -0
+	 * Adjust the default size of SWIOTLB using a percentage of guest
+	 * memory for SWIOTLB buffers. Also, as the SWIOTLB bounce buffer
+	 * memory is allocated from low memory, ensure that the adjusted
+	 * size is within the limits of low available memory.

Nit: if SWIOTLB_ANY is passed, do we still allocate only from low memory? I think this comment made sense on x86 but it's not valid for s390, pseries with the changes so far.

I'll remove that part of the comment, as the code does not check the adjusted size against the low-memory limit.

Quoted 4 lines

Otherwise:

-aneesh

#12 lore raw

PATCH v4 4/5 dma: swiotlb: Add an overridable architecture pool opt-out

In reply to Aneesh Kumar K.V (Arm)

On x86, iommu=off historically prevented SWIOTLB initialization by not setting SWIOTLB_INIT_ADDRESSING_LIMIT. This overloads that flag: it controls both whether a pool is selected and where the pool is allocated. If another requirement, swiotlb=force selects a pool independently, omitting SWIOTLB_INIT_ADDRESSING_LIMIT may place it above the addressable range of devices which need bounce buffering.

Add SWIOTLB_INIT_FORCE_DISABLE to let an architecture opt out of normal pool initialization without discarding the pool placement constraints.

Unlike swiotlb_force_disable, which is set by swiotlb=noforce and unconditionally prevents pool initialization, the new flag is an architecture default. Explicit requirements such as memory encryption, architecture remapping, or swiotlb=force take precedence over it.

iommu=soft retains SWIOTLB_INIT_ADDRESSING_LIMIT because it requests software bounce buffering for devices whose DMA masks cannot address all system memory. Such devices must be able to reach the bounce pool.

Clear both flags for Xen because Xen explicitly requires a remapped SWIOTLB pool and may allocate it anywhere in directly mapped memory.

With no override, iommu=off disables both hardware IOMMU translation and SWIOTLB bouncing. Direct DMA remains available; mappings outside a device's addressable range fail instead of being bounced.

---
 arch/x86/kernel/pci-dma.c | 11 ++++++++---
 include/linux/swiotlb.h   |  2 ++
 kernel/dma/swiotlb.c      | 10 ++++++++--
 3 files changed, 18 insertions(+), 5 deletions(-)
arch/x86/kernel/pci-dma.c +8 -3
index 3c82ebb5b6e0..cd6dd151d634 100644
@@ -44,9 +44,13 @@ unsigned int x86_swiotlb_flags;
 static void __init pci_swiotlb_detect(void)
 {
 	/* don't initialize swiotlb if iommu=off (no_iommu=1) */
-	if (!no_iommu && max_possible_pfn > MAX_DMA32_PFN) {
-		x86_swiotlb_enable = true;
+	if (no_iommu)
+		x86_swiotlb_flags |= SWIOTLB_INIT_FORCE_DISABLE;
+
+	if (max_possible_pfn > MAX_DMA32_PFN) {
 		x86_swiotlb_flags |= SWIOTLB_INIT_ADDRESSING_LIMIT;
+		if (!no_iommu)
+			x86_swiotlb_enable = true;
 	}
 
 	/*
@@ -83,7 +87,8 @@ static void __init pci_xen_swiotlb_init(void)
 		return;
 	x86_swiotlb_enable = true;
 	/* Xen can use a SWIOTLB pool anywhere in directly mapped memory. */
-	x86_swiotlb_flags &= ~SWIOTLB_INIT_ADDRESSING_LIMIT;
+	x86_swiotlb_flags &= ~(SWIOTLB_INIT_ADDRESSING_LIMIT |
+			       SWIOTLB_INIT_FORCE_DISABLE);
 	x86_swiotlb_flags |= SWIOTLB_INIT_REMAP | SWIOTLB_ANY;
 	swiotlb_init_remap(x86_swiotlb_flags, xen_swiotlb_fixup);
 	dma_ops = &xen_swiotlb_dma_ops;
include/linux/swiotlb.h +2 -0
index be3962a33fc6..ef7a776ee508 100644
@@ -20,6 +20,8 @@ struct scatterlist;
 #define SWIOTLB_INIT_ADDRESSING_LIMIT	(1 << 2)
 /* Initialize a default-sized pool that requires architecture remapping. */
 #define SWIOTLB_INIT_REMAP		(1 << 3)
+/* Do not initialize a pool unless SWIOTLB is explicitly required. */
+#define SWIOTLB_INIT_FORCE_DISABLE	(1 << 4)
 
 /*
  * Maximum allowable number of contiguous slabs to map,
kernel/dma/swiotlb.c +8 -2
index c4611b2c4540..6695da682f96 100644
@@ -563,10 +563,16 @@ swiotlb_select_pool_policy(unsigned int flags)
 	if (flags & SWIOTLB_INIT_REMAP)
 		return SWIOTLB_POOL_DEFAULT;
 
-	if (flags & SWIOTLB_INIT_ADDRESSING_LIMIT)
+	if (swiotlb_force_bounce)
 		return SWIOTLB_POOL_DEFAULT;
 
-	if (swiotlb_force_bounce)
+	/*
+	 * Explicit requirements above override an architecture's default opt-out.
+	 */
+	if (flags & SWIOTLB_INIT_FORCE_DISABLE)
+		return SWIOTLB_POOL_NONE;
+
+	if (flags & SWIOTLB_INIT_ADDRESSING_LIMIT)
 		return SWIOTLB_POOL_DEFAULT;
 
 	if (swiotlb_kmalloc_needs_bounce())
Signature · 1 line
2.43.0
#13 lore raw

Re: [PATCH v4 4/5] dma: swiotlb: Add an overridable architecture pool opt-out

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 19 lines

On x86, iommu=off historically prevented SWIOTLB initialization by not setting SWIOTLB_INIT_ADDRESSING_LIMIT. This overloads that flag: it controls both whether a pool is selected and where the pool is allocated. If another requirement, swiotlb=force selects a pool independently, omitting SWIOTLB_INIT_ADDRESSING_LIMIT may place it above the addressable range of devices which need bounce buffering.

Add SWIOTLB_INIT_FORCE_DISABLE to let an architecture opt out of normal pool initialization without discarding the pool placement constraints.

Unlike swiotlb_force_disable, which is set by swiotlb=noforce and unconditionally prevents pool initialization, the new flag is an architecture default. Explicit requirements such as memory encryption, architecture remapping, or swiotlb=force take precedence over it.

iommu=soft retains SWIOTLB_INIT_ADDRESSING_LIMIT because it requests software bounce buffering for devices whose DMA masks cannot address all system memory. Such devices must be able to reach the bounce pool.

Quoted 8 lines

Clear both flags for Xen because Xen explicitly requires a remapped SWIOTLB pool and may allocate it anywhere in directly mapped memory.

With no override, iommu=off disables both hardware IOMMU translation and SWIOTLB bouncing. Direct DMA remains available; mappings outside a device's addressable range fail instead of being bounced.

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
maintainer Reviewed-by on v4 4/5
#14 lore raw

Re: [PATCH v4 4/5] dma: swiotlb: Add an overridable architecture pool opt-out

In reply to Aneesh Kumar K.V (Arm)

include/linux/swiotlb.h struct scatterlist; · line 24

Quoted 10 lines · Aneesh Kumar K.V (Arm)
include/linux/swiotlb.h +2 -0
index be3962a33fc6..ef7a776ee508 100644
@@ -20,6 +20,8 @@ struct scatterlist;
 #define SWIOTLB_INIT_ADDRESSING_LIMIT	(1 << 2)
 /* Initialize a default-sized pool that requires architecture remapping. */
 #define SWIOTLB_INIT_REMAP		(1 << 3)
+/* Do not initialize a pool unless SWIOTLB is explicitly required. */
+#define SWIOTLB_INIT_FORCE_DISABLE	(1 << 4)

Nit: maybe better as SWIOTLB_INIT_DEFAULT_OFF. "Force" sounds too drastic and it can be overridden on the command line to re-enable.

#15 lore raw

PATCH v4 5/5 dma: swiotlb: Remove SWIOTLB_ANY

In reply to Aneesh Kumar K.V (Arm)

swiotlb pools are currently allocated below the architecture's low address limit by default. SWIOTLB_ANY overrides this and permits allocation anywhere in directly mapped memory.

SWIOTLB_INIT_ADDRESSING_LIMIT now explicitly identifies callers which need a pool that is reachable by devices with limited DMA addressing. Use it as the placement constraint and remove SWIOTLB_ANY.

This changes the default behavior for callers which do not set either flag. Previously, omitting SWIOTLB_ANY placed the default pool below the architecture's low address limit and also constrained dynamically allocated pools to the same range. After this change, omitting SWIOTLB_INIT_ADDRESSING_LIMIT permits both static and dynamic pools to use any directly mapped memory.

---
 Documentation/core-api/swiotlb.rst |  8 ++++----
 arch/powerpc/mm/mem.c              |  6 ++++--
 arch/s390/mm/init.c                |  2 +-
 arch/x86/kernel/pci-dma.c          |  2 +-
 include/linux/swiotlb.h            |  7 +++----
 kernel/dma/swiotlb.c               | 17 ++++++-----------
 6 files changed, 19 insertions(+), 23 deletions(-)
Documentation/core-api/swiotlb.rst +4 -4
index 71b4e4c27eb5..06d9f88fdf7b 100644
@@ -148,10 +148,10 @@ may also be adjusted
 due to other conditions, such as running in a CoCo VM, as described above. If
 CONFIG_SWIOTLB_DYNAMIC is enabled, additional pools may be allocated later in
 the life of the system. Each pool must be a contiguous range of physical
-memory. The default pool is allocated below the 4 GiB physical address line so
-it works for devices that can only address 32-bits of physical memory (unless
-architecture-specific code provides the SWIOTLB_ANY flag). In a CoCo VM, the
-pool memory must be decrypted before swiotlb is used.
+memory. The default pool is allocated below the architecture's low address
+limit when it is needed for devices with limited DMA addressing. Otherwise,
+it may be allocated anywhere in directly mapped memory. In a CoCo VM, the pool
+memory must be decrypted before swiotlb is used.
 
 Each pool is divided into "slots" of size IO_TLB_SIZE, which is 2 KiB with
 current definitions. IO_TLB_SEGSIZE contiguous slots (128 slots) constitute
arch/powerpc/mm/mem.c +4 -2
index 006ed41097e9..1bdea6c03848 100644
@@ -295,9 +295,11 @@ void __init arch_mm_preinit(void)
 		/*
 		 * Since the guest memory is inaccessible to the host,
 		 * devices always need to use the SWIOTLB buffer for DMA
-		 * even if dma_capable() says otherwise.
+		 * even if dma_capable() says otherwise. The hypervisor has
+		 * no addressing limitation, so the buffer may be allocated
+		 * anywhere.
 		 */
-		ppc_swiotlb_flags |= SWIOTLB_ANY;
+		ppc_swiotlb_flags &= ~SWIOTLB_INIT_ADDRESSING_LIMIT;
 	}
 
 	/*
arch/s390/mm/init.c +1 -1
index 78f507e24ceb..ce10292447f1 100644
@@ -166,7 +166,7 @@ static void __init pv_init(void)
 	virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
 
 	/* make sure bounce buffers are shared */
-	swiotlb_init(SWIOTLB_VERBOSE | SWIOTLB_ANY);
+	swiotlb_init(SWIOTLB_VERBOSE);
 	swiotlb_update_mem_attributes();
 }
 
arch/x86/kernel/pci-dma.c +1 -1
index cd6dd151d634..a8070cae8253 100644
@@ -89,7 +89,7 @@ static void __init pci_xen_swiotlb_init(void)
 	/* Xen can use a SWIOTLB pool anywhere in directly mapped memory. */
 	x86_swiotlb_flags &= ~(SWIOTLB_INIT_ADDRESSING_LIMIT |
 			       SWIOTLB_INIT_FORCE_DISABLE);
-	x86_swiotlb_flags |= SWIOTLB_INIT_REMAP | SWIOTLB_ANY;
+	x86_swiotlb_flags |= SWIOTLB_INIT_REMAP;
 	swiotlb_init_remap(x86_swiotlb_flags, xen_swiotlb_fixup);
 	dma_ops = &xen_swiotlb_dma_ops;
 	if (IS_ENABLED(CONFIG_PCI))
include/linux/swiotlb.h +3 -4
index ef7a776ee508..77542f107b3f 100644
@@ -15,13 +15,12 @@ struct page;
 struct scatterlist;
 
 #define SWIOTLB_VERBOSE	(1 << 0) /* verbose initialization */
-#define SWIOTLB_ANY	(1 << 1) /* allow any memory for the buffer */
 /* Initialize a default-sized pool for devices with limited DMA addressing. */
-#define SWIOTLB_INIT_ADDRESSING_LIMIT	(1 << 2)
+#define SWIOTLB_INIT_ADDRESSING_LIMIT	(1 << 1)
 /* Initialize a default-sized pool that requires architecture remapping. */
-#define SWIOTLB_INIT_REMAP		(1 << 3)
+#define SWIOTLB_INIT_REMAP		(1 << 2)
 /* Do not initialize a pool unless SWIOTLB is explicitly required. */
-#define SWIOTLB_INIT_FORCE_DISABLE	(1 << 4)
+#define SWIOTLB_INIT_FORCE_DISABLE	(1 << 3)
 
 /*
  * Maximum allowable number of contiguous slabs to map,
kernel/dma/swiotlb.c +6 -11
index 6695da682f96..f8a77926b8b0 100644
@@ -448,15 +448,10 @@ static void __init *swiotlb_memblock_alloc(unsigned long nslabs,
 	size_t bytes = PAGE_ALIGN(nslabs << IO_TLB_SHIFT);
 	void *tlb;
 
-	/*
-	 * By default allocate the bounce buffer memory from low memory, but
-	 * allow to pick a location everywhere for hypervisors with guest
-	 * memory encryption.
-	 */
-	if (flags & SWIOTLB_ANY)
-		tlb = memblock_alloc(bytes, PAGE_SIZE);
-	else
+	if (flags & SWIOTLB_INIT_ADDRESSING_LIMIT)
 		tlb = memblock_alloc_low(bytes, PAGE_SIZE);
+	else
+		tlb = memblock_alloc(bytes, PAGE_SIZE);
 
 	if (!tlb) {
 		pr_warn("%s: Failed to allocate %zu bytes tlb structure\n",
@@ -604,10 +599,10 @@ void __init swiotlb_init_remap(unsigned int flags,
 #ifdef CONFIG_SWIOTLB_DYNAMIC
 	if (!remap)
 		io_tlb_default_mem.can_grow = true;
-	if (flags & SWIOTLB_ANY)
-		io_tlb_default_mem.phys_limit = virt_to_phys(high_memory - 1);
-	else
+	if (flags & SWIOTLB_INIT_ADDRESSING_LIMIT)
 		io_tlb_default_mem.phys_limit = ARCH_LOW_ADDRESS_LIMIT;
+	else
+		io_tlb_default_mem.phys_limit = virt_to_phys(high_memory - 1);
 #endif
 
 	/* if we have host or guest memory encryption */
Signature · 1 line
2.43.0
#16 lore raw

Re: [PATCH v4 5/5] dma: swiotlb: Remove SWIOTLB_ANY

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Subsystem guideline violation: Use of globally uninitialized high_memory in __init code --

commit 61bbab3b84e605312f21e75e603675cd29154afc
Author: Aneesh Kumar K.V (Arm) <[email protected]>

dma: swiotlb: Remove SWIOTLB_ANY

This patch replaces SWIOTLB_ANY with SWIOTLB_INIT_ADDRESSING_LIMIT to explicitly identify callers that need pools reachable by devices with limited DMA addressing. It changes the default behavior to allow static and dynamic pools anywhere in directly mapped memory when the limit flag is omitted.

[ ... ]

Quoted 4 lines
kernel/dma/swiotlb.c +0 -0
index 6695da682f961..f8a77926b8b09 100644

[ ... ]

kernel/dma/swiotlb.c void __init swiotlb_init_remap(unsigned int flags, · line 605

Quoted 11 lines
kernel/dma/swiotlb.c +3 -3
@@ -604,10 +599,10 @@ void __init swiotlb_init_remap(unsigned int flags,
 #ifdef CONFIG_SWIOTLB_DYNAMIC
 	if (!remap)
 		io_tlb_default_mem.can_grow = true;
-	if (flags & SWIOTLB_ANY)
-		io_tlb_default_mem.phys_limit = virt_to_phys(high_memory - 1);
-	else
+	if (flags & SWIOTLB_INIT_ADDRESSING_LIMIT)
 		io_tlb_default_mem.phys_limit = ARCH_LOW_ADDRESS_LIMIT;
+	else
+		io_tlb_default_mem.phys_limit = virt_to_phys(high_memory - 1);

[Severity: High]
This is a pre-existing issue, but does this evaluation of high_memory in an __init function risk calculating an incorrect physical limit?

The swiotlb_init_remap function in kernel/dma/swiotlb.c evaluates virt_to_phys(high_memory - 1) during early kernel boot. However, the high_memory global is uninitialized and evaluates to zero until free_area_init() runs. Calculating a physical limit using high_memory here can result in a bogus DMA boundary (such as zero or an underflowed limit), which could lead to memory allocation failures or data corruption during DMA mapping.

Could this code use memblock_end_of_DRAM() instead of high_memory during early boot allocations?

Quoted 3 lines
kernel/dma/swiotlb.c +0 -0
 #endif
 
 	/* if we have host or guest memory encryption */
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
#17 lore raw

Re: [PATCH v4 5/5] dma: swiotlb: Remove SWIOTLB_ANY

Quoted 47 lines · [email protected] writes:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Subsystem guideline violation: Use of globally uninitialized high_memory in __init code --

commit 61bbab3b84e605312f21e75e603675cd29154afc
Author: Aneesh Kumar K.V (Arm) <[email protected]>

dma: swiotlb: Remove SWIOTLB_ANY

This patch replaces SWIOTLB_ANY with SWIOTLB_INIT_ADDRESSING_LIMIT to explicitly identify callers that need pools reachable by devices with limited DMA addressing. It changes the default behavior to allow static and dynamic pools anywhere in directly mapped memory when the limit flag is omitted.

[ ... ]

Quoted 4 lines
kernel/dma/swiotlb.c +0 -0
index 6695da682f961..f8a77926b8b09 100644

[ ... ]

kernel/dma/swiotlb.c void __init swiotlb_init_remap(unsigned int flags, · line 605

Quoted 11 lines
kernel/dma/swiotlb.c +3 -3
@@ -604,10 +599,10 @@ void __init swiotlb_init_remap(unsigned int flags,
 #ifdef CONFIG_SWIOTLB_DYNAMIC
 	if (!remap)
 		io_tlb_default_mem.can_grow = true;
-	if (flags & SWIOTLB_ANY)
-		io_tlb_default_mem.phys_limit = virt_to_phys(high_memory - 1);
-	else
+	if (flags & SWIOTLB_INIT_ADDRESSING_LIMIT)
 		io_tlb_default_mem.phys_limit = ARCH_LOW_ADDRESS_LIMIT;
+	else
+		io_tlb_default_mem.phys_limit = virt_to_phys(high_memory - 1);

[Severity: High]
This is a pre-existing issue, but does this evaluation of high_memory in an __init function risk calculating an incorrect physical limit?

The swiotlb_init_remap function in kernel/dma/swiotlb.c evaluates virt_to_phys(high_memory - 1) during early kernel boot. However, the high_memory global is uninitialized and evaluates to zero until free_area_init() runs. Calculating a physical limit using high_memory here can result in a bogus DMA boundary (such as zero or an underflowed limit), which could lead to memory allocation failures or data corruption during DMA mapping.

Quoted 3 lines

Could this code use memblock_end_of_DRAM() instead of high_memory during early boot allocations?

But swiotlb_init() is called after set_high_memory()

start_kernel()
|

  +--> mm_core_init_early()
  |      |
  |      +--> free_area_init()
  |             |
  |             +--> set_high_memory()
  |
  +--> mm_core_init()
         |
         +--> arch_mm_preinit()
                |
                +--> swiotlb_init()

-aneesh

maintainer Reviewed-by on v4 5/5
#18 lore raw

Re: [PATCH v4 5/5] dma: swiotlb: Remove SWIOTLB_ANY

In reply to Aneesh Kumar K.V (Arm)
Quoted 8 lines · Aneesh Kumar K.V (Arm)

swiotlb pools are currently allocated below the architecture's low address limit by default. SWIOTLB_ANY overrides this and permits allocation anywhere in directly mapped memory.

SWIOTLB_INIT_ADDRESSING_LIMIT now explicitly identifies callers which need a pool that is reachable by devices with limited DMA addressing. Use it as the placement constraint and remove SWIOTLB_ANY.

Quoted 8 lines

This changes the default behavior for callers which do not set either flag. Previously, omitting SWIOTLB_ANY placed the default pool below the architecture's low address limit and also constrained dynamically allocated pools to the same range. After this change, omitting SWIOTLB_INIT_ADDRESSING_LIMIT permits both static and dynamic pools to use any directly mapped memory.