* [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers
@ 2023-07-13 15:23 Petr Tesarik
  2023-07-13 15:23 ` [PATCH v4 1/8] swiotlb: make io_tlb_default_mem local to swiotlb.c Petr Tesarik
                   ` (8 more replies)
  0 siblings, 9 replies; 24+ messages in thread
From: Petr Tesarik @ 2023-07-13 15:23 UTC (permalink / raw)
  To: Stefano Stabellini, Russell King, Thomas Bogendoerfer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Petr Tesarik, Jonathan Corbet, Andy Shevchenko,
	Hans de Goede, James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM
  Cc: Roberto Sassu, Kefeng Wang, petr
From: Petr Tesarik <petr.tesarik.ext@huawei.com>
Motivation
==========
The software IO TLB was designed with these assumptions:
1) It would not be used much. Small systems (little RAM) don't need it, and
   big systems (lots of RAM) would have modern DMA controllers and an IOMMU
   chip to handle legacy devices.
2) A small fixed memory area (64 MiB by default) is sufficient to
   handle the few cases which require a bounce buffer.
3) 64 MiB is little enough that it has no impact on the rest of the
   system.
4) Bounce buffers require large contiguous chunks of low memory. Such
   memory is precious and can be allocated only early at boot.
It turns out they are not always true:
1) Embedded systems may have more than 4GiB RAM but no IOMMU and legacy
   32-bit peripheral busses and/or DMA controllers.
2) CoCo VMs use bounce buffers for all I/O but may need substantially more
   than 64 MiB.
3) Embedded developers put as many features as possible into the available
   memory. A few dozen "missing" megabytes may limit what features can be
   implemented.
4) If CMA is available, it can allocate large continuous chunks even after
   the system has run for some time.
Goals
=====
The goal of this work is to start with a small software IO TLB at boot and
expand it later when/if needed.
Design
======
This version of the patch series retains the current slot allocation
algorithm with multiple areas to reduce lock contention, but additional
slots can be added when necessary.
These alternatives have been considered:
- Allocate and free buffers as needed using direct DMA API. This works
  quite well, except in CoCo VMs where each allocation/free requires
  decrypting/encrypting memory, which is a very expensive operation.
- Allocate a very large software IO TLB at boot, but allow to migrate pages
  to/from it (like CMA does). For systems with CMA, this would mean two big
  allocations at boot. Finding the balance between CMA, SWIOTLB and rest of
  available RAM can be challenging. More importantly, there is no clear
  benefit compared to allocating SWIOTLB memory pools from the CMA.
Implementation Constraints
==========================
These constraints have been taken into account:
1) Minimize impact on devices which do not benefit from the change.
2) Minimize the number of memory decryption/encryption operations.
3) Avoid contention on a lock or atomic variable to preserve parallel
   scalability.
Additionally, the software IO TLB code is also used to implement restricted
DMA pools. These pools are restricted to a pre-defined physical memory
region and must not use any other memory. In other words, dynamic
allocation of memory pools must be disabled for restricted DMA pools.
Data Structures
===============
The existing struct io_tlb_mem is the central type for a SWIOTLB allocator,
but it now contains multiple memory pools::
  io_tlb_mem
  +---------+   io_tlb_pool
  | SWIOTLB | 	+-------+   +-------+   +-------+
  |allocator|-->|default|-->|dynamic|-->|dynamic|-->...
  |    	    |  	|memory |   |memory |   |memory |
  +---------+ 	| pool  |   | pool  |   | pool  |
	      	+-------+   +-------+   +-------+
The allocator structure contains global state (such as flags and counters)
and structures needed to schedule new allocations. Each memory pool
contains the actual buffer slots and metadata. The first memory pool in the
list is the default memory pool allocated statically at early boot.
New memory pools are allocated from a kernel worker thread. That's because
bounce buffers are allocated when mapping a DMA buffer, which may happen in
interrupt context where large atomic allocations would probably fail.
Allocation from process context is much more likely to succeed, especially
if it can use CMA.
Nonetheless, the onset of a load spike may fill up the SWIOTLB before the
worker has a chance to run. In that case, try to allocate a small transient
memory pool to accommodate the request. If memory is encrypted and the
device cannot do DMA to encrypted memory, this buffer is allocated from the
coherent atomic DMA memory pool. Reducing the size of SWIOTLB may therefore
require increasing the size of the coherent pool with the "coherent_pool"
command-line parameter.
Performance
===========
All testing compared a vanilla v6.4-rc6 kernel with a fully patched
kernel. The kernel was booted with "swiotlb=force" to allow stress-testing
the software IO TLB on a high-performance device that would otherwise not
need it. CONFIG_DEBUG_FS was set to 'y' to match the configuration of
popular distribution kernels; it is understood that parallel workloads
suffer from contention on the recently added debugfs atomic counters.
These benchmarks were run:
- small: single-threaded I/O of 4 KiB blocks,
- big: single-threaded I/O of 64 KiB blocks,
- 4way: 4-way parallel I/O of 4 KiB blocks.
In all tested cases, the default 64 MiB SWIOTLB would be sufficient (but
wasteful). The "default" pair of columns shows performance impact when
booted with 64 MiB SWIOTLB (i.e. current state). The "growing" pair of
columns shows the impact when booted with a 1 MiB initial SWIOTLB, which
grew to 5 MiB at run time. The "var" column in the tables below is the
coefficient of variance over 5 runs of the test, the "diff" column is the
difference in read-write I/O bandwidth (MiB/s). The very first column is
the coefficient of variance in the results of the base unpatched kernel.
First, on an x86 VM against a QEMU virtio SATA driver backed by a RAM-based
block device on the host:
	base	   default	   growing
	var	var	diff	var	diff
small	1.96%	0.47%	-1.5%	0.52%	-2.2%
big	2.03%	1.35%	+0.9%	2.22%	+2.9%
4way	0.80%	0.45%	-0.7%	1.22%	<0.1%
Second, on a Raspberry Pi4 with 8G RAM and a class 10 A1 microSD card:
	base	   default	   growing
	var	var	diff	var	diff
small	1.09%	1.69%	+0.5%	2.14%	-0.2%
big	0.03%	0.28%	-0.5%	0.03%	-0.1%
4way	5.15%	2.39%	+0.2%	0.66%	<0.1%
Third, on a CoCo VM. This was a bigger system, so I also added a 24-thread
parallel I/O test:
	base	   default	   growing
	var	var	diff	var	diff
small	2.41%	6.02%	+1.1%	10.33%	+6.7%
big	9.20%	2.81%	-0.6%	16.84%	-0.2%
4way	0.86%	2.66%	-0.1%	 2.22%	-4.9%
24way	3.19%	6.19%	+4.4%	 4.08%	-5.9%
Note the increased variance of the CoCo VM, although the host was not
otherwise loaded. These are caused by the first run, which includes the
overhead of allocating additional bounce buffers and sharing them with the
hypervisor. The system was not rebooted between successive runs.
Parallel tests suffer from a reduced number of areas in the dynamically
allocated memory pools. This can be improved by allocating a larger pool
from CMA (not implemented in this series yet).
I have no good explanation for the increase in performance of the
24-thread I/O test with the default (non-growing) memory pool. Although the
difference is within variance, it seems to be real. The average bandwidth
is consistently above that of the unpatched kernel.
To sum it up:
- All workloads benefit from reduced memory footprint.
- No performance regressions have been observed with the default size of
  the software IO TLB.
- Most workloads retain their former performance even if the software IO
  TLB grows at run time.
Changelog
=========
Changes from v3:
- Provide swiotlb_is_allocated() instead of extending swiotlb_is_active().
- Do not grow SWIOTLB if its address has been queried (affects Octeon).
- Do not grow SWIOTLB if a remap function is used (affects Xen PV).
- Use dma_mask instead of coherent_dma_mask.
- Replace complex ternary operators with if-else blocks.
Changes from v2:
- Complete rewrite using dynamically allocated memory pools rather
  than a list of individual buffers
- Depend on other SWIOTLB fixes (already sent)
- Fix Xen and MIPS Octeon builds
Changes from RFC:
- Track dynamic buffers per device instead of per swiotlb
- Use a linked list instead of a maple tree
- Move initialization of swiotlb fields of struct device to a
  helper function
- Rename __lookup_dyn_slot() to lookup_dyn_slot_locked()
- Introduce per-device flag if dynamic buffers are in use
- Add one more user of DMA_ATTR_MAY_SLEEP
- Add kernel-doc comments for new (and some old) code
- Properly escape '*' in dma-attributes.rst
Petr Tesarik (8):
  swiotlb: make io_tlb_default_mem local to swiotlb.c
  swiotlb: add documentation and rename swiotlb_do_find_slots()
  swiotlb: separate memory pool data from other allocator data
  swiotlb: add a flag whether a SWIOTLB is allowed to grow
  swiotlb: if swiotlb is full, fall back to a transient memory pool
  swiotlb: determine potential physical address limit
  swiotlb: allocate a new memory pool when existing pools are full
  swiotlb: search the software IO TLB only if a device makes use of it
 arch/arm/xen/mm.c           |   2 +-
 arch/mips/pci/pci-octeon.c  |   2 +-
 arch/x86/kernel/pci-dma.c   |   2 +-
 drivers/base/core.c         |   4 +-
 drivers/xen/swiotlb-xen.c   |   2 +-
 include/linux/device.h      |   8 +-
 include/linux/dma-mapping.h |   2 +
 include/linux/swiotlb.h     | 111 +++++--
 kernel/dma/direct.c         |   2 +-
 kernel/dma/swiotlb.c        | 605 ++++++++++++++++++++++++++++++++----
 10 files changed, 643 insertions(+), 97 deletions(-)
-- 
2.25.1
^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH v4 1/8] swiotlb: make io_tlb_default_mem local to swiotlb.c
  2023-07-13 15:23 [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers Petr Tesarik
@ 2023-07-13 15:23 ` Petr Tesarik
  2023-07-17  6:06   ` Philippe Mathieu-Daudé
  2023-07-20  6:37   ` Christoph Hellwig
  2023-07-13 15:23 ` [PATCH v4 2/8] swiotlb: add documentation and rename swiotlb_do_find_slots() Petr Tesarik
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 24+ messages in thread
From: Petr Tesarik @ 2023-07-13 15:23 UTC (permalink / raw)
  To: Stefano Stabellini, Russell King, Thomas Bogendoerfer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Petr Tesarik, Jonathan Corbet, Andy Shevchenko,
	Hans de Goede, James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM
  Cc: Roberto Sassu, Kefeng Wang, petr
From: Petr Tesarik <petr.tesarik.ext@huawei.com>
SWIOTLB implementation details should not be exposed to the rest of the
kernel. This will allow to make changes to the implementation without
modifying non-swiotlb code.
To avoid breaking existing users, provide helper functions for the few
required fields.
As a bonus, using a helper function to initialize struct device allows to
get rid of an #ifdef in driver core.
Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
---
 arch/arm/xen/mm.c          |  2 +-
 arch/mips/pci/pci-octeon.c |  2 +-
 arch/x86/kernel/pci-dma.c  |  2 +-
 drivers/base/core.c        |  4 +---
 drivers/xen/swiotlb-xen.c  |  2 +-
 include/linux/swiotlb.h    | 25 +++++++++++++++++++++++-
 kernel/dma/swiotlb.c       | 39 +++++++++++++++++++++++++++++++++++++-
 7 files changed, 67 insertions(+), 9 deletions(-)
diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index 3d826c0b5fee..0f32c14eb786 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -125,7 +125,7 @@ static int __init xen_mm_init(void)
 		return 0;
 
 	/* we can work with the default swiotlb */
-	if (!io_tlb_default_mem.nslabs) {
+	if (!is_swiotlb_allocated()) {
 		rc = swiotlb_init_late(swiotlb_size_or_default(),
 				       xen_swiotlb_gfp(), NULL);
 		if (rc < 0)
diff --git a/arch/mips/pci/pci-octeon.c b/arch/mips/pci/pci-octeon.c
index e457a18cbdc5..c5c4c1f7d5e4 100644
--- a/arch/mips/pci/pci-octeon.c
+++ b/arch/mips/pci/pci-octeon.c
@@ -664,7 +664,7 @@ static int __init octeon_pci_setup(void)
 
 		/* BAR1 movable regions contiguous to cover the swiotlb */
 		octeon_bar1_pci_phys =
-			io_tlb_default_mem.start & ~((1ull << 22) - 1);
+			default_swiotlb_start() & ~((1ull << 22) - 1);
 
 		for (index = 0; index < 32; index++) {
 			union cvmx_pci_bar1_indexx bar1_index;
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index de6be0a3965e..08c6ffc3550f 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -90,7 +90,7 @@ int pci_xen_swiotlb_init_late(void)
 		return 0;
 
 	/* we can work with the default swiotlb */
-	if (!io_tlb_default_mem.nslabs) {
+	if (!is_swiotlb_allocated()) {
 		int rc = swiotlb_init_late(swiotlb_size_or_default(),
 					   GFP_KERNEL, xen_swiotlb_fixup);
 		if (rc < 0)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 3dff5037943e..46d1d78c5beb 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3108,9 +3108,7 @@ void device_initialize(struct device *dev)
     defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
 	dev->dma_coherent = dma_default_coherent;
 #endif
-#ifdef CONFIG_SWIOTLB
-	dev->dma_io_tlb_mem = &io_tlb_default_mem;
-#endif
+	swiotlb_dev_init(dev);
 }
 EXPORT_SYMBOL_GPL(device_initialize);
 
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 67aa74d20162..946bd56f0ac5 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -381,7 +381,7 @@ xen_swiotlb_sync_sg_for_device(struct device *dev, struct scatterlist *sgl,
 static int
 xen_swiotlb_dma_supported(struct device *hwdev, u64 mask)
 {
-	return xen_phys_to_dma(hwdev, io_tlb_default_mem.end - 1) <= mask;
+	return xen_phys_to_dma(hwdev, default_swiotlb_limit()) <= mask;
 }
 
 const struct dma_map_ops xen_swiotlb_dma_ops = {
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 4e52cd5e0bdc..07216af59e93 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -110,7 +110,6 @@ struct io_tlb_mem {
 	atomic_long_t used_hiwater;
 #endif
 };
-extern struct io_tlb_mem io_tlb_default_mem;
 
 static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
 {
@@ -128,13 +127,22 @@ static inline bool is_swiotlb_force_bounce(struct device *dev)
 
 void swiotlb_init(bool addressing_limited, unsigned int flags);
 void __init swiotlb_exit(void);
+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);
 void __init swiotlb_adjust_size(unsigned long size);
+phys_addr_t default_swiotlb_start(void);
+phys_addr_t default_swiotlb_limit(void);
 #else
 static inline void swiotlb_init(bool addressing_limited, unsigned int flags)
 {
 }
+
+static inline void swiotlb_dev_init(struct device *dev)
+{
+}
+
 static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
 {
 	return false;
@@ -151,6 +159,11 @@ static inline size_t swiotlb_max_mapping_size(struct device *dev)
 	return SIZE_MAX;
 }
 
+static inline bool is_swiotlb_allocated(void)
+{
+	return false;
+}
+
 static inline bool is_swiotlb_active(struct device *dev)
 {
 	return false;
@@ -159,6 +172,16 @@ static inline bool is_swiotlb_active(struct device *dev)
 static inline void swiotlb_adjust_size(unsigned long size)
 {
 }
+
+static inline phys_addr_t default_swiotlb_start(void)
+{
+	return 0;
+}
+
+static inline phys_addr_t default_swiotlb_limit(void)
+{
+	return 0;
+}
 #endif /* CONFIG_SWIOTLB */
 
 extern void swiotlb_print_info(void);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 2b83e3ad9dca..873b077d7e37 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -71,7 +71,7 @@ struct io_tlb_slot {
 static bool swiotlb_force_bounce;
 static bool swiotlb_force_disable;
 
-struct io_tlb_mem io_tlb_default_mem;
+static struct io_tlb_mem io_tlb_default_mem;
 
 static unsigned long default_nslabs = IO_TLB_DEFAULT_SIZE >> IO_TLB_SHIFT;
 static unsigned long default_nareas;
@@ -486,6 +486,15 @@ void __init swiotlb_exit(void)
 	memset(mem, 0, sizeof(*mem));
 }
 
+/**
+ * swiotlb_dev_init() - initialize swiotlb fields in &struct device
+ * @dev:	Device to be initialized.
+ */
+void swiotlb_dev_init(struct device *dev)
+{
+	dev->dma_io_tlb_mem = &io_tlb_default_mem;
+}
+
 /*
  * Return the offset into a iotlb slot required to keep the device happy.
  */
@@ -950,6 +959,14 @@ size_t swiotlb_max_mapping_size(struct device *dev)
 	return ((size_t)IO_TLB_SIZE) * IO_TLB_SEGSIZE - min_align;
 }
 
+/**
+ * is_swiotlb_allocated() - check if the default software IO TLB is initialized
+ */
+bool is_swiotlb_allocated(void)
+{
+	return !!io_tlb_default_mem.nslabs;
+}
+
 bool is_swiotlb_active(struct device *dev)
 {
 	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
@@ -958,6 +975,26 @@ bool is_swiotlb_active(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(is_swiotlb_active);
 
+/**
+ * default_swiotlb_start() - get the start of the default SWIOTLB
+ *
+ * Get the lowest physical address used by the default software IO TLB pool.
+ */
+phys_addr_t default_swiotlb_start(void)
+{
+	return io_tlb_default_mem.start;
+}
+
+/**
+ * default_swiotlb_limit() - get the highest address in the default SWIOTLB
+ *
+ * Get the highest physical address used by the default software IO TLB pool.
+ */
+phys_addr_t default_swiotlb_limit(void)
+{
+	return io_tlb_default_mem.end - 1;
+}
+
 #ifdef CONFIG_DEBUG_FS
 
 static int io_tlb_used_get(void *data, u64 *val)
-- 
2.25.1
^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 2/8] swiotlb: add documentation and rename swiotlb_do_find_slots()
  2023-07-13 15:23 [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers Petr Tesarik
  2023-07-13 15:23 ` [PATCH v4 1/8] swiotlb: make io_tlb_default_mem local to swiotlb.c Petr Tesarik
@ 2023-07-13 15:23 ` Petr Tesarik
  2023-07-20  6:38   ` Christoph Hellwig
  2023-07-13 15:23 ` [PATCH v4 3/8] swiotlb: separate memory pool data from other allocator data Petr Tesarik
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Petr Tesarik @ 2023-07-13 15:23 UTC (permalink / raw)
  To: Stefano Stabellini, Russell King, Thomas Bogendoerfer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Petr Tesarik, Jonathan Corbet, Andy Shevchenko,
	Hans de Goede, James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM
  Cc: Roberto Sassu, Kefeng Wang, petr
From: Petr Tesarik <petr.tesarik.ext@huawei.com>
Add some kernel-doc comments and move the existing documentation of struct
io_tlb_slot to its correct location. The latter was forgotten in commit
942a8186eb445 ("swiotlb: move struct io_tlb_slot to swiotlb.c").
Use the opportunity to give swiotlb_do_find_slots() a more descriptive
name, which makes it clear how it differs from swiotlb_find_slots().
Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
---
 include/linux/swiotlb.h | 15 +++++++---
 kernel/dma/swiotlb.c    | 61 +++++++++++++++++++++++++++++++++++++----
 2 files changed, 66 insertions(+), 10 deletions(-)
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 07216af59e93..39313c3a791a 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -76,10 +76,6 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t phys,
  * @nslabs:	The number of IO TLB blocks (in groups of 64) between @start and
  *		@end. For default swiotlb, this is command line adjustable via
  *		setup_io_tlb_npages.
- * @list:	The free list describing the number of free entries available
- *		from each index.
- * @orig_addr:	The original address corresponding to a mapped entry.
- * @alloc_size:	Size of the allocated buffer.
  * @debugfs:	The dentry to debugfs.
  * @late_alloc:	%true if allocated using the page allocator
  * @force_bounce: %true if swiotlb bouncing is forced
@@ -111,6 +107,17 @@ struct io_tlb_mem {
 #endif
 };
 
+/**
+ * is_swiotlb_buffer() - check if a physical address belongs to a swiotlb
+ * @dev:        Device which has mapped the buffer.
+ * @paddr:      Physical address within the DMA buffer.
+ *
+ * Check if @paddr points into a bounce buffer.
+ *
+ * Return:
+ * * %true if @paddr points into a bounce buffer
+ * * %false otherwise
+ */
 static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
 {
 	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 873b077d7e37..01161d040639 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -62,6 +62,13 @@
 
 #define INVALID_PHYS_ADDR (~(phys_addr_t)0)
 
+/**
+ * struct io_tlb_slot - IO TLB slot descriptor
+ * @orig_addr:	The original address corresponding to a mapped entry.
+ * @alloc_size:	Size of the allocated buffer.
+ * @list:	The free list describing the number of free entries available
+ *		from each index.
+ */
 struct io_tlb_slot {
 	phys_addr_t orig_addr;
 	size_t alloc_size;
@@ -632,11 +639,22 @@ static void dec_used(struct io_tlb_mem *mem, unsigned int nslots)
 }
 #endif /* CONFIG_DEBUG_FS */
 
-/*
- * Find a suitable number of IO TLB entries size that will fit this request and
- * allocate a buffer from that IO TLB pool.
+/**
+ * area_find_slots() - search for slots in one IO TLB memory area
+ * @dev:	Device which maps the buffer.
+ * @area_index:	Index of the IO TLB memory area to be searched.
+ * @orig_addr:	Original (non-bounced) IO buffer address.
+ * @alloc_size: Total requested size of the bounce buffer,
+ *		including initial alignment padding.
+ * @alloc_align_mask:	Required alignment of the allocated buffer.
+ *
+ * Find a suitable sequence of IO TLB entries for the request and allocate
+ * a buffer from the given IO TLB memory area.
+ * This function takes care of locking.
+ *
+ * Return: Index of the first allocated slot, or -1 on error.
  */
-static int swiotlb_do_find_slots(struct device *dev, int area_index,
+static int area_find_slots(struct device *dev, int area_index,
 		phys_addr_t orig_addr, size_t alloc_size,
 		unsigned int alloc_align_mask)
 {
@@ -731,6 +749,19 @@ static int swiotlb_do_find_slots(struct device *dev, int area_index,
 	return slot_index;
 }
 
+/**
+ * swiotlb_find_slots() - search for slots in the whole swiotlb
+ * @dev:	Device which maps the buffer.
+ * @orig_addr:	Original (non-bounced) IO buffer address.
+ * @alloc_size: Total requested size of the bounce buffer,
+ *		including initial alignment padding.
+ * @alloc_align_mask:	Required alignment of the allocated buffer.
+ *
+ * Search through the whole software IO TLB to find a sequence of slots that
+ * match the allocation constraints.
+ *
+ * Return: Index of the first allocated slot, or -1 on error.
+ */
 static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
 		size_t alloc_size, unsigned int alloc_align_mask)
 {
@@ -739,8 +770,8 @@ static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
 	int i = start, index;
 
 	do {
-		index = swiotlb_do_find_slots(dev, i, orig_addr, alloc_size,
-					      alloc_align_mask);
+		index = area_find_slots(dev, i, orig_addr, alloc_size,
+					alloc_align_mask);
 		if (index >= 0)
 			return index;
 		if (++i >= mem->nareas)
@@ -752,6 +783,15 @@ static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
 
 #ifdef CONFIG_DEBUG_FS
 
+/**
+ * mem_used() - get number of used slots in an allocator
+ * @mem:	Software IO TLB allocator.
+ *
+ * The result is accurate in this version of the function, because an atomic
+ * counter is available if CONFIG_DEBUG_FS is set.
+ *
+ * Return: Number of used slots.
+ */
 static unsigned long mem_used(struct io_tlb_mem *mem)
 {
 	return atomic_long_read(&mem->total_used);
@@ -759,6 +799,15 @@ static unsigned long mem_used(struct io_tlb_mem *mem)
 
 #else /* !CONFIG_DEBUG_FS */
 
+/**
+ * mem_used() - get number of used slots in an allocator
+ * @mem:	Software IO TLB allocator.
+ *
+ * The result is not accurate, because there is no locking of individual
+ * areas.
+ *
+ * Return: Approximate number of used slots.
+ */
 static unsigned long mem_used(struct io_tlb_mem *mem)
 {
 	int i;
-- 
2.25.1
^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 3/8] swiotlb: separate memory pool data from other allocator data
  2023-07-13 15:23 [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers Petr Tesarik
  2023-07-13 15:23 ` [PATCH v4 1/8] swiotlb: make io_tlb_default_mem local to swiotlb.c Petr Tesarik
  2023-07-13 15:23 ` [PATCH v4 2/8] swiotlb: add documentation and rename swiotlb_do_find_slots() Petr Tesarik
@ 2023-07-13 15:23 ` Petr Tesarik
  2023-07-13 17:53   ` Petr Tesařík
  2023-07-13 15:23 ` [PATCH v4 4/8] swiotlb: add a flag whether a SWIOTLB is allowed to grow Petr Tesarik
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Petr Tesarik @ 2023-07-13 15:23 UTC (permalink / raw)
  To: Stefano Stabellini, Russell King, Thomas Bogendoerfer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Petr Tesarik, Jonathan Corbet, Andy Shevchenko,
	Hans de Goede, James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM
  Cc: Roberto Sassu, Kefeng Wang, petr
From: Petr Tesarik <petr.tesarik.ext@huawei.com>
Carve out memory pool specific fields from struct io_tlb_mem. The original
struct now contains shared data for the whole allocator, while the new
struct io_tlb_pool contains data that is specific to one memory pool of
(potentially) many.
Allocate both structures together for restricted DMA pools to keep the
error cleanup path simple.
Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
---
 include/linux/device.h  |   2 +-
 include/linux/swiotlb.h |  47 +++++++----
 kernel/dma/swiotlb.c    | 181 +++++++++++++++++++++++++---------------
 3 files changed, 147 insertions(+), 83 deletions(-)
diff --git a/include/linux/device.h b/include/linux/device.h
index bbaeabd04b0d..d9754a68ba95 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -625,7 +625,7 @@ struct device_physical_location {
  * @dma_pools:	Dma pools (if dma'ble device).
  * @dma_mem:	Internal for coherent mem override.
  * @cma_area:	Contiguous memory area for dma allocations
- * @dma_io_tlb_mem: Pointer to the swiotlb pool used.  Not for driver use.
+ * @dma_io_tlb_mem: Software IO TLB allocator.  Not for driver use.
  * @archdata:	For arch-specific additions.
  * @of_node:	Associated device tree node.
  * @fwnode:	Associated device node supplied by platform firmware.
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 39313c3a791a..d669e11e2827 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -62,8 +62,7 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t phys,
 #ifdef CONFIG_SWIOTLB
 
 /**
- * struct io_tlb_mem - IO TLB Memory Pool Descriptor
- *
+ * struct io_tlb_pool - IO TLB memory pool descriptor
  * @start:	The start address of the swiotlb memory pool. Used to do a quick
  *		range check to see if the memory was in fact allocated by this
  *		API.
@@ -73,15 +72,36 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t phys,
  * @vaddr:	The vaddr of the swiotlb memory pool. The swiotlb memory pool
  *		may be remapped in the memory encrypted case and store virtual
  *		address for bounce buffer operation.
- * @nslabs:	The number of IO TLB blocks (in groups of 64) between @start and
- *		@end. For default swiotlb, this is command line adjustable via
- *		setup_io_tlb_npages.
+ * @nslabs:	The number of IO TLB slots between @start and @end. For the
+ *		default swiotlb, this can be adjusted with a boot parameter,
+ *		see setup_io_tlb_npages().
+ * @used:	The number of used IO TLB slots.
+ * @late_alloc:	%true if allocated using the page allocator.
+ * @nareas:	Number of areas in the pool.
+ * @area_nslabs: Number of slots in each area.
+ * @areas:	Array of memory area descriptors.
+ * @slots:	Array of slot descriptors.
+ */
+struct io_tlb_pool {
+	phys_addr_t start;
+	phys_addr_t end;
+	void *vaddr;
+	unsigned long nslabs;
+	unsigned long used;
+	bool late_alloc;
+	unsigned int nareas;
+	unsigned int area_nslabs;
+	struct io_tlb_area *areas;
+	struct io_tlb_slot *slots;
+};
+
+/**
+ * struct io_tlb_mem - Software IO TLB allocator
+ * @pool:	IO TLB memory pool descriptor.
+ * @nslabs:	Total number of IO TLB slabs in all pools.
  * @debugfs:	The dentry to debugfs.
- * @late_alloc:	%true if allocated using the page allocator
  * @force_bounce: %true if swiotlb bouncing is forced
  * @for_alloc:  %true if the pool is used for memory allocation
- * @nareas:  The area number in the pool.
- * @area_nslabs: The slot number in the area.
  * @total_used:	The total number of slots in the pool that are currently used
  *		across all areas. Used only for calculating used_hiwater in
  *		debugfs.
@@ -89,18 +109,11 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t phys,
  *		in debugfs.
  */
 struct io_tlb_mem {
-	phys_addr_t start;
-	phys_addr_t end;
-	void *vaddr;
+	struct io_tlb_pool *pool;
 	unsigned long nslabs;
 	struct dentry *debugfs;
-	bool late_alloc;
 	bool force_bounce;
 	bool for_alloc;
-	unsigned int nareas;
-	unsigned int area_nslabs;
-	struct io_tlb_area *areas;
-	struct io_tlb_slot *slots;
 #ifdef CONFIG_DEBUG_FS
 	atomic_long_t total_used;
 	atomic_long_t used_hiwater;
@@ -122,7 +135,7 @@ static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
 {
 	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
 
-	return mem && paddr >= mem->start && paddr < mem->end;
+	return mem && paddr >= mem->pool->start && paddr < mem->pool->end;
 }
 
 static inline bool is_swiotlb_force_bounce(struct device *dev)
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 01161d040639..a80b77de8829 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -78,7 +78,10 @@ struct io_tlb_slot {
 static bool swiotlb_force_bounce;
 static bool swiotlb_force_disable;
 
-static struct io_tlb_mem io_tlb_default_mem;
+static struct io_tlb_pool io_tlb_default_pool;
+static struct io_tlb_mem io_tlb_default_mem = {
+	.pool = &io_tlb_default_pool,
+};
 
 static unsigned long default_nslabs = IO_TLB_DEFAULT_SIZE >> IO_TLB_SHIFT;
 static unsigned long default_nareas;
@@ -209,7 +212,7 @@ void __init swiotlb_adjust_size(unsigned long size)
 
 void swiotlb_print_info(void)
 {
-	struct io_tlb_mem *mem = &io_tlb_default_mem;
+	struct io_tlb_pool *mem = &io_tlb_default_pool;
 
 	if (!mem->nslabs) {
 		pr_warn("No low mem\n");
@@ -238,7 +241,7 @@ static inline unsigned long nr_slots(u64 val)
  */
 void __init swiotlb_update_mem_attributes(void)
 {
-	struct io_tlb_mem *mem = &io_tlb_default_mem;
+	struct io_tlb_pool *mem = &io_tlb_default_pool;
 	unsigned long bytes;
 
 	if (!mem->nslabs || mem->late_alloc)
@@ -247,9 +250,8 @@ void __init swiotlb_update_mem_attributes(void)
 	set_memory_decrypted((unsigned long)mem->vaddr, bytes >> PAGE_SHIFT);
 }
 
-static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start,
-		unsigned long nslabs, unsigned int flags,
-		bool late_alloc, unsigned int nareas)
+static void swiotlb_init_io_tlb_pool(struct io_tlb_pool *mem, phys_addr_t start,
+		unsigned long nslabs, bool late_alloc, unsigned int nareas)
 {
 	void *vaddr = phys_to_virt(start);
 	unsigned long bytes = nslabs << IO_TLB_SHIFT, i;
@@ -261,8 +263,6 @@ static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start,
 	mem->nareas = nareas;
 	mem->area_nslabs = nslabs / mem->nareas;
 
-	mem->force_bounce = swiotlb_force_bounce || (flags & SWIOTLB_FORCE);
-
 	for (i = 0; i < mem->nareas; i++) {
 		spin_lock_init(&mem->areas[i].lock);
 		mem->areas[i].index = 0;
@@ -319,7 +319,7 @@ static void __init *swiotlb_memblock_alloc(unsigned long nslabs,
 void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
 		int (*remap)(void *tlb, unsigned long nslabs))
 {
-	struct io_tlb_mem *mem = &io_tlb_default_mem;
+	struct io_tlb_pool *mem = &io_tlb_default_pool;
 	unsigned long nslabs;
 	unsigned int nareas;
 	size_t alloc_size;
@@ -330,6 +330,9 @@ void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
 	if (swiotlb_force_disable)
 		return;
 
+	io_tlb_default_mem.force_bounce =
+		swiotlb_force_bounce || (flags & SWIOTLB_FORCE);
+
 	if (!default_nareas)
 		swiotlb_adjust_nareas(num_possible_cpus());
 
@@ -363,8 +366,9 @@ void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
 		return;
 	}
 
-	swiotlb_init_io_tlb_mem(mem, __pa(tlb), nslabs, flags, false,
-				default_nareas);
+	swiotlb_init_io_tlb_pool(mem, __pa(tlb), nslabs, false,
+				 default_nareas);
+	io_tlb_default_mem.nslabs = nslabs;
 
 	if (flags & SWIOTLB_VERBOSE)
 		swiotlb_print_info();
@@ -383,7 +387,7 @@ void __init swiotlb_init(bool addressing_limit, unsigned int flags)
 int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 		int (*remap)(void *tlb, unsigned long nslabs))
 {
-	struct io_tlb_mem *mem = &io_tlb_default_mem;
+	struct io_tlb_pool *mem = &io_tlb_default_pool;
 	unsigned long nslabs = ALIGN(size >> IO_TLB_SHIFT, IO_TLB_SEGSIZE);
 	unsigned int nareas;
 	unsigned char *vstart = NULL;
@@ -394,6 +398,8 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 	if (swiotlb_force_disable)
 		return 0;
 
+	io_tlb_default_mem.force_bounce = swiotlb_force_bounce;
+
 	if (!default_nareas)
 		swiotlb_adjust_nareas(num_possible_cpus());
 
@@ -445,8 +451,9 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 
 	set_memory_decrypted((unsigned long)vstart,
 			     (nslabs << IO_TLB_SHIFT) >> PAGE_SHIFT);
-	swiotlb_init_io_tlb_mem(mem, virt_to_phys(vstart), nslabs, 0, true,
-				nareas);
+	swiotlb_init_io_tlb_pool(mem, virt_to_phys(vstart), nslabs, true,
+				 nareas);
+	io_tlb_default_mem.nslabs = nslabs;
 
 	swiotlb_print_info();
 	return 0;
@@ -460,7 +467,7 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 
 void __init swiotlb_exit(void)
 {
-	struct io_tlb_mem *mem = &io_tlb_default_mem;
+	struct io_tlb_pool *mem = &io_tlb_default_pool;
 	unsigned long tbl_vaddr;
 	size_t tbl_size, slots_size;
 	unsigned int area_order;
@@ -516,7 +523,7 @@ static unsigned int swiotlb_align_offset(struct device *dev, u64 addr)
 static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t size,
 			   enum dma_data_direction dir)
 {
-	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+	struct io_tlb_pool *mem = dev->dma_io_tlb_mem->pool;
 	int index = (tlb_addr - mem->start) >> IO_TLB_SHIFT;
 	phys_addr_t orig_addr = mem->slots[index].orig_addr;
 	size_t alloc_size = mem->slots[index].alloc_size;
@@ -598,7 +605,7 @@ static inline unsigned long get_max_slots(unsigned long boundary_mask)
 	return nr_slots(boundary_mask + 1);
 }
 
-static unsigned int wrap_area_index(struct io_tlb_mem *mem, unsigned int index)
+static unsigned int wrap_area_index(struct io_tlb_pool *mem, unsigned int index)
 {
 	if (index >= mem->area_nslabs)
 		return 0;
@@ -642,6 +649,7 @@ static void dec_used(struct io_tlb_mem *mem, unsigned int nslots)
 /**
  * area_find_slots() - search for slots in one IO TLB memory area
  * @dev:	Device which maps the buffer.
+ * @pool:	Memory pool to be searched.
  * @area_index:	Index of the IO TLB memory area to be searched.
  * @orig_addr:	Original (non-bounced) IO buffer address.
  * @alloc_size: Total requested size of the bounce buffer,
@@ -654,15 +662,14 @@ static void dec_used(struct io_tlb_mem *mem, unsigned int nslots)
  *
  * Return: Index of the first allocated slot, or -1 on error.
  */
-static int area_find_slots(struct device *dev, int area_index,
-		phys_addr_t orig_addr, size_t alloc_size,
+static int area_find_slots(struct device *dev, struct io_tlb_pool *pool,
+		int area_index, phys_addr_t orig_addr, size_t alloc_size,
 		unsigned int alloc_align_mask)
 {
-	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
-	struct io_tlb_area *area = mem->areas + area_index;
+	struct io_tlb_area *area = pool->areas + area_index;
 	unsigned long boundary_mask = dma_get_seg_boundary(dev);
 	dma_addr_t tbl_dma_addr =
-		phys_to_dma_unencrypted(dev, mem->start) & boundary_mask;
+		phys_to_dma_unencrypted(dev, pool->start) & boundary_mask;
 	unsigned long max_slots = get_max_slots(boundary_mask);
 	unsigned int iotlb_align_mask =
 		dma_get_min_align_mask(dev) | alloc_align_mask;
@@ -674,7 +681,7 @@ static int area_find_slots(struct device *dev, int area_index,
 	unsigned int slot_index;
 
 	BUG_ON(!nslots);
-	BUG_ON(area_index >= mem->nareas);
+	BUG_ON(area_index >= pool->nareas);
 
 	/*
 	 * For allocations of PAGE_SIZE or larger only look for page aligned
@@ -691,19 +698,19 @@ static int area_find_slots(struct device *dev, int area_index,
 	stride = (iotlb_align_mask >> IO_TLB_SHIFT) + 1;
 
 	spin_lock_irqsave(&area->lock, flags);
-	if (unlikely(nslots > mem->area_nslabs - area->used))
+	if (unlikely(nslots > pool->area_nslabs - area->used))
 		goto not_found;
 
-	slot_base = area_index * mem->area_nslabs;
+	slot_base = area_index * pool->area_nslabs;
 	index = area->index;
 
-	for (slots_checked = 0; slots_checked < mem->area_nslabs; ) {
+	for (slots_checked = 0; slots_checked < pool->area_nslabs; ) {
 		slot_index = slot_base + index;
 
 		if (orig_addr &&
 		    (slot_addr(tbl_dma_addr, slot_index) &
 		     iotlb_align_mask) != (orig_addr & iotlb_align_mask)) {
-			index = wrap_area_index(mem, index + 1);
+			index = wrap_area_index(pool, index + 1);
 			slots_checked++;
 			continue;
 		}
@@ -716,10 +723,10 @@ static int area_find_slots(struct device *dev, int area_index,
 		if (!iommu_is_span_boundary(slot_index, nslots,
 					    nr_slots(tbl_dma_addr),
 					    max_slots)) {
-			if (mem->slots[slot_index].list >= nslots)
+			if (pool->slots[slot_index].list >= nslots)
 				goto found;
 		}
-		index = wrap_area_index(mem, index + stride);
+		index = wrap_area_index(pool, index + stride);
 		slots_checked += stride;
 	}
 
@@ -729,58 +736,79 @@ static int area_find_slots(struct device *dev, int area_index,
 
 found:
 	for (i = slot_index; i < slot_index + nslots; i++) {
-		mem->slots[i].list = 0;
-		mem->slots[i].alloc_size = alloc_size - (offset +
+		pool->slots[i].list = 0;
+		pool->slots[i].alloc_size = alloc_size - (offset +
 				((i - slot_index) << IO_TLB_SHIFT));
 	}
 	for (i = slot_index - 1;
 	     io_tlb_offset(i) != IO_TLB_SEGSIZE - 1 &&
-	     mem->slots[i].list; i--)
-		mem->slots[i].list = ++count;
+	     pool->slots[i].list; i--)
+		pool->slots[i].list = ++count;
 
 	/*
 	 * Update the indices to avoid searching in the next round.
 	 */
-	area->index = wrap_area_index(mem, index + nslots);
+	area->index = wrap_area_index(pool, index + nslots);
 	area->used += nslots;
 	spin_unlock_irqrestore(&area->lock, flags);
 
-	inc_used_and_hiwater(mem, nslots);
+	inc_used_and_hiwater(dev->dma_io_tlb_mem, nslots);
 	return slot_index;
 }
 
 /**
- * swiotlb_find_slots() - search for slots in the whole swiotlb
+ * pool_find_slots() - search for slots in one memory pool
  * @dev:	Device which maps the buffer.
+ * @pool:	Memory pool to be searched.
  * @orig_addr:	Original (non-bounced) IO buffer address.
  * @alloc_size: Total requested size of the bounce buffer,
  *		including initial alignment padding.
  * @alloc_align_mask:	Required alignment of the allocated buffer.
  *
- * Search through the whole software IO TLB to find a sequence of slots that
- * match the allocation constraints.
+ * Search through one memory pool to find a sequence of slots that match the
+ * allocation constraints.
  *
  * Return: Index of the first allocated slot, or -1 on error.
  */
-static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
-		size_t alloc_size, unsigned int alloc_align_mask)
+static int pool_find_slots(struct device *dev, struct io_tlb_pool *pool,
+		phys_addr_t orig_addr, size_t alloc_size,
+		unsigned int alloc_align_mask)
 {
-	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
-	int start = raw_smp_processor_id() & (mem->nareas - 1);
+	int start = raw_smp_processor_id() & (pool->nareas - 1);
 	int i = start, index;
 
 	do {
-		index = area_find_slots(dev, i, orig_addr, alloc_size,
+		index = area_find_slots(dev, pool, i, orig_addr, alloc_size,
 					alloc_align_mask);
 		if (index >= 0)
 			return index;
-		if (++i >= mem->nareas)
+		if (++i >= pool->nareas)
 			i = 0;
 	} while (i != start);
 
 	return -1;
 }
 
+/**
+ * swiotlb_find_slots() - search for slots in the whole swiotlb
+ * @dev:	Device which maps the buffer.
+ * @orig_addr:	Original (non-bounced) IO buffer address.
+ * @alloc_size: Total requested size of the bounce buffer,
+ *		including initial alignment padding.
+ * @alloc_align_mask:	Required alignment of the allocated buffer.
+ *
+ * Search through the whole software IO TLB to find a sequence of slots that
+ * match the allocation constraints.
+ *
+ * Return: Index of the first allocated slot, or -1 on error.
+ */
+static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
+		size_t alloc_size, unsigned int alloc_align_mask)
+{
+	return pool_find_slots(dev, dev->dma_io_tlb_mem->pool, orig_addr,
+			       alloc_size, alloc_align_mask);
+}
+
 #ifdef CONFIG_DEBUG_FS
 
 /**
@@ -799,6 +827,24 @@ static unsigned long mem_used(struct io_tlb_mem *mem)
 
 #else /* !CONFIG_DEBUG_FS */
 
+/**
+ * mem_pool_used() - get number of used slots in a memory pool
+ * @pool:	Software IO TLB memory pool.
+ *
+ * The result is not accurate, see mem_used().
+ *
+ * Return: Approximate number of used slots.
+ */
+static unsigned long mem_pool_used(struct io_tlb_pool *pool)
+{
+	int i;
+	unsigned long used = 0;
+
+	for (i = 0; i < pool->nareas; i++)
+		used += pool->areas[i].used;
+	return used;
+}
+
 /**
  * mem_used() - get number of used slots in an allocator
  * @mem:	Software IO TLB allocator.
@@ -810,12 +856,7 @@ static unsigned long mem_used(struct io_tlb_mem *mem)
  */
 static unsigned long mem_used(struct io_tlb_mem *mem)
 {
-	int i;
-	unsigned long used = 0;
-
-	for (i = 0; i < mem->nareas; i++)
-		used += mem->areas[i].used;
-	return used;
+	return mem_pool_used(mem->pool);
 }
 
 #endif /* CONFIG_DEBUG_FS */
@@ -827,6 +868,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
 {
 	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
 	unsigned int offset = swiotlb_align_offset(dev, orig_addr);
+	struct io_tlb_pool *pool;
 	unsigned int i;
 	int index;
 	phys_addr_t tlb_addr;
@@ -861,9 +903,10 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
 	 * This is needed when we sync the memory.  Then we sync the buffer if
 	 * needed.
 	 */
+	pool = mem->pool;
 	for (i = 0; i < nr_slots(alloc_size + offset); i++)
-		mem->slots[index + i].orig_addr = slot_addr(orig_addr, i);
-	tlb_addr = slot_addr(mem->start, index) + offset;
+		pool->slots[index + i].orig_addr = slot_addr(orig_addr, i);
+	tlb_addr = slot_addr(pool->start, index) + offset;
 	/*
 	 * When dir == DMA_FROM_DEVICE we could omit the copy from the orig
 	 * to the tlb buffer, if we knew for sure the device will
@@ -877,7 +920,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
 
 static void swiotlb_release_slots(struct device *dev, phys_addr_t tlb_addr)
 {
-	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+	struct io_tlb_pool *mem = dev->dma_io_tlb_mem->pool;
 	unsigned long flags;
 	unsigned int offset = swiotlb_align_offset(dev, tlb_addr);
 	int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT;
@@ -921,7 +964,7 @@ static void swiotlb_release_slots(struct device *dev, phys_addr_t tlb_addr)
 	area->used -= nslots;
 	spin_unlock_irqrestore(&area->lock, flags);
 
-	dec_used(mem, nslots);
+	dec_used(dev->dma_io_tlb_mem, nslots);
 }
 
 /*
@@ -1031,7 +1074,7 @@ EXPORT_SYMBOL_GPL(is_swiotlb_active);
  */
 phys_addr_t default_swiotlb_start(void)
 {
-	return io_tlb_default_mem.start;
+	return io_tlb_default_pool.start;
 }
 
 /**
@@ -1041,7 +1084,7 @@ phys_addr_t default_swiotlb_start(void)
  */
 phys_addr_t default_swiotlb_limit(void)
 {
-	return io_tlb_default_mem.end - 1;
+	return io_tlb_default_pool.end - 1;
 }
 
 #ifdef CONFIG_DEBUG_FS
@@ -1117,6 +1160,7 @@ static inline void swiotlb_create_debugfs_files(struct io_tlb_mem *mem,
 struct page *swiotlb_alloc(struct device *dev, size_t size)
 {
 	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+	struct io_tlb_pool *pool;
 	phys_addr_t tlb_addr;
 	int index;
 
@@ -1127,7 +1171,8 @@ struct page *swiotlb_alloc(struct device *dev, size_t size)
 	if (index == -1)
 		return NULL;
 
-	tlb_addr = slot_addr(mem->start, index);
+	pool = mem->pool;
+	tlb_addr = slot_addr(pool->start, index);
 
 	return pfn_to_page(PFN_DOWN(tlb_addr));
 }
@@ -1164,29 +1209,35 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
 	 * to it.
 	 */
 	if (!mem) {
-		mem = kzalloc(sizeof(*mem), GFP_KERNEL);
+		struct io_tlb_pool *pool;
+
+		mem = kzalloc(sizeof(*mem) + sizeof(*pool), GFP_KERNEL);
 		if (!mem)
 			return -ENOMEM;
+		pool = (void *)mem + sizeof(*mem);
 
-		mem->slots = kcalloc(nslabs, sizeof(*mem->slots), GFP_KERNEL);
-		if (!mem->slots) {
+		pool->slots = kcalloc(nslabs, sizeof(*pool->slots), GFP_KERNEL);
+		if (!pool->slots) {
 			kfree(mem);
 			return -ENOMEM;
 		}
 
-		mem->areas = kcalloc(nareas, sizeof(*mem->areas),
+		pool->areas = kcalloc(nareas, sizeof(*pool->areas),
 				GFP_KERNEL);
-		if (!mem->areas) {
-			kfree(mem->slots);
+		if (!pool->areas) {
+			kfree(pool->slots);
 			kfree(mem);
 			return -ENOMEM;
 		}
 
 		set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
 				     rmem->size >> PAGE_SHIFT);
-		swiotlb_init_io_tlb_mem(mem, rmem->base, nslabs, SWIOTLB_FORCE,
-					false, nareas);
+		swiotlb_init_io_tlb_pool(pool, rmem->base, nslabs,
+					 false, nareas);
+		mem->force_bounce = true;
 		mem->for_alloc = true;
+		mem->pool = pool;
+		mem->nslabs = nslabs;
 
 		rmem->priv = mem;
 
-- 
2.25.1
^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 4/8] swiotlb: add a flag whether a SWIOTLB is allowed to grow
  2023-07-13 15:23 [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers Petr Tesarik
                   ` (2 preceding siblings ...)
  2023-07-13 15:23 ` [PATCH v4 3/8] swiotlb: separate memory pool data from other allocator data Petr Tesarik
@ 2023-07-13 15:23 ` Petr Tesarik
  2023-07-13 15:23 ` [PATCH v4 5/8] swiotlb: if swiotlb is full, fall back to a transient memory pool Petr Tesarik
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Petr Tesarik @ 2023-07-13 15:23 UTC (permalink / raw)
  To: Stefano Stabellini, Russell King, Thomas Bogendoerfer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Petr Tesarik, Jonathan Corbet, Andy Shevchenko,
	Hans de Goede, James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM
  Cc: Roberto Sassu, Kefeng Wang, petr
From: Petr Tesarik <petr.tesarik.ext@huawei.com>
Mark the default SWIOTLB as able to grow and restricted DMA pools as
unable.
However, if the address of the default memory pool is explicitly queried,
make the default SWIOTLB also unable to grow. This is currently used to set
up PCI BAR movable regions on some Octeon MIPS boards which may not be able
to use a SWIOTLB pool elsewhere in physical memory. See octeon_pci_setup()
for more details.
If a remap function is specified, it must be also called on any dynamically
allocated pools, but there are some issues:
- The remap function may block, so it should not be called from an atomic
  context.
- There is no corresponding unremap() function if the memory pool is
  freed.
- The only in-tree implementation (xen_swiotlb_fixup) requires that the
  number of slots in the memory pool is a multiple of SWIOTLB_SEGSIZE.
Keep it simple for now and disable growing the SWIOTLB if a remap function
was specified.
Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
---
 include/linux/swiotlb.h | 2 ++
 kernel/dma/swiotlb.c    | 4 ++++
 2 files changed, 6 insertions(+)
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index d669e11e2827..81f8c901e888 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -102,6 +102,7 @@ struct io_tlb_pool {
  * @debugfs:	The dentry to debugfs.
  * @force_bounce: %true if swiotlb bouncing is forced
  * @for_alloc:  %true if the pool is used for memory allocation
+ * @can_grow:	%true if more pools can be allocated dynamically.
  * @total_used:	The total number of slots in the pool that are currently used
  *		across all areas. Used only for calculating used_hiwater in
  *		debugfs.
@@ -114,6 +115,7 @@ struct io_tlb_mem {
 	struct dentry *debugfs;
 	bool force_bounce;
 	bool for_alloc;
+	bool can_grow;
 #ifdef CONFIG_DEBUG_FS
 	atomic_long_t total_used;
 	atomic_long_t used_hiwater;
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index a80b77de8829..16e5b9a82902 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -332,6 +332,7 @@ void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
 
 	io_tlb_default_mem.force_bounce =
 		swiotlb_force_bounce || (flags & SWIOTLB_FORCE);
+	io_tlb_default_mem.can_grow = !remap;
 
 	if (!default_nareas)
 		swiotlb_adjust_nareas(num_possible_cpus());
@@ -399,6 +400,7 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 		return 0;
 
 	io_tlb_default_mem.force_bounce = swiotlb_force_bounce;
+	io_tlb_default_mem.can_grow = !remap;
 
 	if (!default_nareas)
 		swiotlb_adjust_nareas(num_possible_cpus());
@@ -1074,6 +1076,7 @@ EXPORT_SYMBOL_GPL(is_swiotlb_active);
  */
 phys_addr_t default_swiotlb_start(void)
 {
+	io_tlb_default_mem.can_grow = false;
 	return io_tlb_default_pool.start;
 }
 
@@ -1236,6 +1239,7 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
 					 false, nareas);
 		mem->force_bounce = true;
 		mem->for_alloc = true;
+		mem->can_grow = false;
 		mem->pool = pool;
 		mem->nslabs = nslabs;
 
-- 
2.25.1
^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 5/8] swiotlb: if swiotlb is full, fall back to a transient memory pool
  2023-07-13 15:23 [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers Petr Tesarik
                   ` (3 preceding siblings ...)
  2023-07-13 15:23 ` [PATCH v4 4/8] swiotlb: add a flag whether a SWIOTLB is allowed to grow Petr Tesarik
@ 2023-07-13 15:23 ` Petr Tesarik
  2023-07-13 15:23 ` [PATCH v4 6/8] swiotlb: determine potential physical address limit Petr Tesarik
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Petr Tesarik @ 2023-07-13 15:23 UTC (permalink / raw)
  To: Stefano Stabellini, Russell King, Thomas Bogendoerfer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Petr Tesarik, Jonathan Corbet, Andy Shevchenko,
	Hans de Goede, James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM
  Cc: Roberto Sassu, Kefeng Wang, petr
From: Petr Tesarik <petr.tesarik.ext@huawei.com>
Try to allocate a transient memory pool if no suitable slots can be found
and the respective SWIOTLB is allowed to grow. The transient pool is just
enough big for this one bounce buffer. It is inserted into a per-device
list of transient memory pools, and it is freed again when the bounce
buffer is unmapped.
Transient memory pools are kept in an RCU list. A memory barrier is
required after adding a new entry, because any address within a transient
buffer must be immediately recognized as belonging to the SWIOTLB, even if
it is passed to another CPU.
Deletion does not require any synchronization beyond RCU ordering
guarantees. After a buffer is unmapped, its physical addresses may no
longer be passed to the DMA API, so the memory range of the corresponding
stale entry in the RCU list never matches. If the memory range gets
allocated again, then it happens only after a RCU quiescent state.
Since bounce buffers can now be allocated from different pools, add a
parameter to swiotlb_alloc_pool() to let the caller know which memory pool
is used. Add swiotlb_find_pool() to find the memory pool corresponding to
an address. This function is now also used by is_swiotlb_buffer(), because
a simple boundary check is no longer sufficient.
The logic in swiotlb_alloc_tlb() is taken from __dma_direct_alloc_pages(),
simplified and enhanced to use coherent memory pools if needed.
Note that this is not the most efficient way to provide a bounce buffer,
but when a DMA buffer can't be mapped, something may (and will) actually
break. At that point it is better to make an allocation, even if it may be
an expensive operation.
Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
---
 include/linux/device.h      |   4 +
 include/linux/dma-mapping.h |   2 +
 include/linux/swiotlb.h     |  13 +-
 kernel/dma/direct.c         |   2 +-
 kernel/dma/swiotlb.c        | 270 ++++++++++++++++++++++++++++++++++--
 5 files changed, 277 insertions(+), 14 deletions(-)
diff --git a/include/linux/device.h b/include/linux/device.h
index d9754a68ba95..549b0a62455c 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -626,6 +626,8 @@ struct device_physical_location {
  * @dma_mem:	Internal for coherent mem override.
  * @cma_area:	Contiguous memory area for dma allocations
  * @dma_io_tlb_mem: Software IO TLB allocator.  Not for driver use.
+ * @dma_io_tlb_pools:	List of transient swiotlb memory pools.
+ * @dma_io_tlb_lock:	Protects changes to the list of active pools.
  * @archdata:	For arch-specific additions.
  * @of_node:	Associated device tree node.
  * @fwnode:	Associated device node supplied by platform firmware.
@@ -731,6 +733,8 @@ struct device {
 #endif
 #ifdef CONFIG_SWIOTLB
 	struct io_tlb_mem *dma_io_tlb_mem;
+	struct list_head dma_io_tlb_pools;
+	spinlock_t dma_io_tlb_lock;
 #endif
 	/* arch specific additions */
 	struct dev_archdata	archdata;
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index e13050eb9777..f0ccca16a0ac 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -418,6 +418,8 @@ static inline void dma_sync_sgtable_for_device(struct device *dev,
 #define dma_get_sgtable(d, t, v, h, s) dma_get_sgtable_attrs(d, t, v, h, s, 0)
 #define dma_mmap_coherent(d, v, c, h, s) dma_mmap_attrs(d, v, c, h, s, 0)
 
+bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size);
+
 static inline void *dma_alloc_coherent(struct device *dev, size_t size,
 		dma_addr_t *dma_handle, gfp_t gfp)
 {
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 81f8c901e888..b642e7739604 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -63,6 +63,7 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t phys,
 
 /**
  * struct io_tlb_pool - IO TLB memory pool descriptor
+ * @node:	Member of the IO TLB memory pool list.
  * @start:	The start address of the swiotlb memory pool. Used to do a quick
  *		range check to see if the memory was in fact allocated by this
  *		API.
@@ -77,22 +78,27 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t phys,
  *		see setup_io_tlb_npages().
  * @used:	The number of used IO TLB slots.
  * @late_alloc:	%true if allocated using the page allocator.
+ * @transient:  %true if transient memory pool.
  * @nareas:	Number of areas in the pool.
  * @area_nslabs: Number of slots in each area.
  * @areas:	Array of memory area descriptors.
  * @slots:	Array of slot descriptors.
+ * @rcu:	RCU head for swiotlb_dyn_free().
  */
 struct io_tlb_pool {
+	struct list_head node;
 	phys_addr_t start;
 	phys_addr_t end;
 	void *vaddr;
 	unsigned long nslabs;
 	unsigned long used;
 	bool late_alloc;
+	bool transient;
 	unsigned int nareas;
 	unsigned int area_nslabs;
 	struct io_tlb_area *areas;
 	struct io_tlb_slot *slots;
+	struct rcu_head rcu;
 };
 
 /**
@@ -122,6 +128,8 @@ struct io_tlb_mem {
 #endif
 };
 
+struct io_tlb_pool *swiotlb_find_pool(struct device *dev, phys_addr_t paddr);
+
 /**
  * is_swiotlb_buffer() - check if a physical address belongs to a swiotlb
  * @dev:        Device which has mapped the buffer.
@@ -135,9 +143,8 @@ struct io_tlb_mem {
  */
 static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
 {
-	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
-
-	return mem && paddr >= mem->pool->start && paddr < mem->pool->end;
+	return dev->dma_io_tlb_mem &&
+		!!swiotlb_find_pool(dev, paddr);
 }
 
 static inline bool is_swiotlb_force_bounce(struct device *dev)
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index d29cade048db..9596ae1aa0da 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -66,7 +66,7 @@ static gfp_t dma_direct_optimal_gfp_mask(struct device *dev, u64 *phys_limit)
 	return 0;
 }
 
-static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
+bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
 {
 	dma_addr_t dma_addr = phys_to_dma_direct(dev, phys);
 
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 16e5b9a82902..6ec5a81acc2a 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -35,6 +35,7 @@
 #include <linux/memblock.h>
 #include <linux/mm.h>
 #include <linux/pfn.h>
+#include <linux/rculist.h>
 #include <linux/scatterlist.h>
 #include <linux/set_memory.h>
 #include <linux/spinlock.h>
@@ -502,6 +503,159 @@ void __init swiotlb_exit(void)
 	memset(mem, 0, sizeof(*mem));
 }
 
+/**
+ * alloc_dma_pages() - allocate pages to be used for DMA
+ * @gfp:	GFP flags for the allocation.
+ * @bytes:	Size of the buffer.
+ *
+ * Allocate pages from the buddy allocator. If successful, make the allocated
+ * pages decrypted that they can be used for DMA.
+ *
+ * Return: Decrypted pages, or %NULL on failure.
+ */
+static struct page *alloc_dma_pages(gfp_t gfp, size_t bytes)
+{
+	unsigned int order = get_order(bytes);
+	struct page *page;
+	void *vaddr;
+
+	page = alloc_pages(gfp, order);
+	if (!page)
+		return NULL;
+
+	vaddr = page_address(page);
+	if (set_memory_decrypted((unsigned long)vaddr, PFN_UP(bytes)))
+		goto error;
+	return page;
+
+error:
+	__free_pages(page, order);
+	return NULL;
+}
+
+/**
+ * swiotlb_alloc_tlb() - allocate a dynamic IO TLB buffer
+ * @dev:	Device for which a memory pool is allocated.
+ * @bytes:	Size of the buffer.
+ * @phys_limit:	Maximum allowed physical address of the buffer.
+ * @gfp:	GFP flags for the allocation.
+ *
+ * Return: Allocated pages, or %NULL on allocation failure.
+ */
+static struct page *swiotlb_alloc_tlb(struct device *dev, size_t bytes,
+		u64 phys_limit, gfp_t gfp)
+{
+	struct page *page;
+
+	/*
+	 * Allocate from the atomic pools if memory is encrypted and
+	 * the allocation is atomic, because decrypting may block.
+	 */
+	if (!gfpflags_allow_blocking(gfp) && dev && force_dma_unencrypted(dev)) {
+		void *vaddr;
+
+		if (!IS_ENABLED(CONFIG_DMA_COHERENT_POOL))
+			return NULL;
+
+		return dma_alloc_from_pool(dev, bytes, &vaddr, gfp,
+					   dma_coherent_ok);
+	}
+
+	gfp &= ~GFP_ZONEMASK;
+	if (phys_limit <= DMA_BIT_MASK(zone_dma_bits))
+		gfp |= __GFP_DMA;
+	else if (phys_limit <= DMA_BIT_MASK(32))
+		gfp |= __GFP_DMA32;
+
+	while ((page = alloc_dma_pages(gfp, bytes)) &&
+	       page_to_phys(page) + bytes - 1 > phys_limit) {
+		/* allocated, but too high */
+		__free_pages(page, get_order(bytes));
+
+		if (IS_ENABLED(CONFIG_ZONE_DMA32) &&
+		    phys_limit < DMA_BIT_MASK(64) &&
+		    !(gfp & (__GFP_DMA32 | __GFP_DMA)))
+			gfp |= __GFP_DMA32;
+		else if (IS_ENABLED(CONFIG_ZONE_DMA) &&
+			 !(gfp & __GFP_DMA))
+			gfp = (gfp & ~__GFP_DMA32) | __GFP_DMA;
+		else
+			return NULL;
+	}
+
+	return page;
+}
+
+/**
+ * swiotlb_free_tlb() - free a dynamically allocated IO TLB buffer
+ * @vaddr:	Virtual address of the buffer.
+ * @bytes:	Size of the buffer.
+ */
+static void swiotlb_free_tlb(void *vaddr, size_t bytes)
+{
+	if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
+	    dma_free_from_pool(NULL, vaddr, bytes))
+		return;
+
+	/* Intentional leak if pages cannot be encrypted again. */
+	if (!set_memory_encrypted((unsigned long)vaddr, PFN_UP(bytes)))
+		__free_pages(virt_to_page(vaddr), get_order(bytes));
+}
+
+/**
+ * swiotlb_alloc_pool() - allocate a new IO TLB memory pool
+ * @dev:	Device for which a memory pool is allocated.
+ * @nslabs:	Desired number of slabs.
+ * @phys_limit:	Maximum DMA buffer physical address.
+ * @gfp:	GFP flags for the allocations.
+ *
+ * Allocate and initialize a new IO TLB memory pool.
+ *
+ * Return: New memory pool, or %NULL on allocation failure.
+ */
+static struct io_tlb_pool *swiotlb_alloc_pool(struct device *dev,
+		unsigned int nslabs, u64 phys_limit, gfp_t gfp)
+{
+	struct io_tlb_pool *pool;
+	struct page *tlb;
+	size_t pool_size;
+	size_t tlb_size;
+
+	pool_size = sizeof(*pool) + array_size(sizeof(*pool->areas), 1) +
+		array_size(sizeof(*pool->slots), nslabs);
+	pool = kzalloc(pool_size, gfp);
+	if (!pool)
+		goto error;
+	pool->areas = (void *)pool + sizeof(*pool);
+	pool->slots = (void *)pool->areas + sizeof(*pool->areas);
+
+	tlb_size = nslabs << IO_TLB_SHIFT;
+	tlb = swiotlb_alloc_tlb(dev, tlb_size, phys_limit, gfp);
+	if (!tlb)
+		goto error_tlb;
+
+	swiotlb_init_io_tlb_pool(pool, page_to_phys(tlb), nslabs, true, 1);
+	return pool;
+
+error_tlb:
+	kfree(pool);
+error:
+	return NULL;
+}
+
+/**
+ * swiotlb_dyn_free() - RCU callback to free a memory pool
+ * @rcu:	RCU head in the corresponding struct io_tlb_pool.
+ */
+static void swiotlb_dyn_free(struct rcu_head *rcu)
+{
+	struct io_tlb_pool *pool = container_of(rcu, struct io_tlb_pool, rcu);
+	size_t tlb_size = pool->end - pool->start;
+
+	swiotlb_free_tlb(pool->vaddr, tlb_size);
+	kfree(pool);
+}
+
 /**
  * swiotlb_dev_init() - initialize swiotlb fields in &struct device
  * @dev:	Device to be initialized.
@@ -509,6 +663,56 @@ void __init swiotlb_exit(void)
 void swiotlb_dev_init(struct device *dev)
 {
 	dev->dma_io_tlb_mem = &io_tlb_default_mem;
+	INIT_LIST_HEAD(&dev->dma_io_tlb_pools);
+	spin_lock_init(&dev->dma_io_tlb_lock);
+}
+
+/**
+ * swiotlb_find_pool() - find the IO TLB pool for a physical address
+ * @dev:        Device which has mapped the DMA buffer.
+ * @paddr:      Physical address within the DMA buffer.
+ *
+ * Find the IO TLB memory pool descriptor which contains the given physical
+ * address, if any.
+ *
+ * Return: Memory pool which contains @paddr, or %NULL if none.
+ */
+struct io_tlb_pool *swiotlb_find_pool(struct device *dev, phys_addr_t paddr)
+{
+	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+	struct io_tlb_pool *pool = mem->pool;
+
+	if (paddr >= pool->start && paddr < pool->end)
+		return pool;
+
+	/* Pairs with smp_wmb() in swiotlb_find_slots(). */
+	smp_rmb();
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(pool, &dev->dma_io_tlb_pools, node) {
+		if (paddr >= pool->start && paddr < pool->end)
+			goto out;
+	}
+	pool = NULL;
+out:
+	rcu_read_unlock();
+	return pool;
+}
+
+/**
+ * swiotlb_del_pool() - remove an IO TLB pool from a device
+ * @dev:	Owning device.
+ * @pool:	Memory pool to be removed.
+ */
+static void swiotlb_del_pool(struct device *dev, struct io_tlb_pool *pool)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->dma_io_tlb_lock, flags);
+	list_del_rcu(&pool->node);
+	spin_unlock_irqrestore(&dev->dma_io_tlb_lock, flags);
+
+	call_rcu(&pool->rcu, swiotlb_dyn_free);
 }
 
 /*
@@ -525,7 +729,7 @@ static unsigned int swiotlb_align_offset(struct device *dev, u64 addr)
 static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t size,
 			   enum dma_data_direction dir)
 {
-	struct io_tlb_pool *mem = dev->dma_io_tlb_mem->pool;
+	struct io_tlb_pool *mem = swiotlb_find_pool(dev, tlb_addr);
 	int index = (tlb_addr - mem->start) >> IO_TLB_SHIFT;
 	phys_addr_t orig_addr = mem->slots[index].orig_addr;
 	size_t alloc_size = mem->slots[index].alloc_size;
@@ -798,6 +1002,7 @@ static int pool_find_slots(struct device *dev, struct io_tlb_pool *pool,
  * @alloc_size: Total requested size of the bounce buffer,
  *		including initial alignment padding.
  * @alloc_align_mask:	Required alignment of the allocated buffer.
+ * @retpool:	Used memory pool, updated on return.
  *
  * Search through the whole software IO TLB to find a sequence of slots that
  * match the allocation constraints.
@@ -805,10 +1010,49 @@ static int pool_find_slots(struct device *dev, struct io_tlb_pool *pool,
  * Return: Index of the first allocated slot, or -1 on error.
  */
 static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
-		size_t alloc_size, unsigned int alloc_align_mask)
+		size_t alloc_size, unsigned int alloc_align_mask,
+		struct io_tlb_pool **retpool)
 {
-	return pool_find_slots(dev, dev->dma_io_tlb_mem->pool, orig_addr,
-			       alloc_size, alloc_align_mask);
+	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+	struct io_tlb_pool *pool;
+	unsigned long nslabs;
+	unsigned long flags;
+	u64 phys_limit;
+	int index;
+
+	pool = mem->pool;
+	index = pool_find_slots(dev, pool, orig_addr,
+				alloc_size, alloc_align_mask);
+	if (index >= 0)
+		goto found;
+
+	if (!dev->dma_io_tlb_mem->can_grow)
+		return -1;
+
+	nslabs = nr_slots(alloc_size);
+	phys_limit = min_not_zero(*dev->dma_mask, dev->bus_dma_limit);
+	pool = swiotlb_alloc_pool(dev, nslabs, phys_limit,
+				  GFP_NOWAIT | __GFP_NOWARN);
+	if (!pool)
+		return -1;
+
+	index = pool_find_slots(dev, pool, orig_addr,
+				alloc_size, alloc_align_mask);
+	if (index < 0) {
+		swiotlb_dyn_free(&pool->rcu);
+		return -1;
+	}
+
+	pool->transient = true;
+	spin_lock_irqsave(&dev->dma_io_tlb_lock, flags);
+	list_add_rcu(&pool->node, &dev->dma_io_tlb_pools);
+	spin_unlock_irqrestore(&dev->dma_io_tlb_lock, flags);
+
+	/* Pairs with smp_rmb() in swiotlb_find_pool(). */
+	smp_wmb();
+found:
+	*retpool = pool;
+	return index;
 }
 
 #ifdef CONFIG_DEBUG_FS
@@ -891,7 +1135,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
 	}
 
 	index = swiotlb_find_slots(dev, orig_addr,
-				   alloc_size + offset, alloc_align_mask);
+				   alloc_size + offset, alloc_align_mask, &pool);
 	if (index == -1) {
 		if (!(attrs & DMA_ATTR_NO_WARN))
 			dev_warn_ratelimited(dev,
@@ -905,7 +1149,6 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
 	 * This is needed when we sync the memory.  Then we sync the buffer if
 	 * needed.
 	 */
-	pool = mem->pool;
 	for (i = 0; i < nr_slots(alloc_size + offset); i++)
 		pool->slots[index + i].orig_addr = slot_addr(orig_addr, i);
 	tlb_addr = slot_addr(pool->start, index) + offset;
@@ -922,7 +1165,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
 
 static void swiotlb_release_slots(struct device *dev, phys_addr_t tlb_addr)
 {
-	struct io_tlb_pool *mem = dev->dma_io_tlb_mem->pool;
+	struct io_tlb_pool *mem = swiotlb_find_pool(dev, tlb_addr);
 	unsigned long flags;
 	unsigned int offset = swiotlb_align_offset(dev, tlb_addr);
 	int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT;
@@ -976,6 +1219,8 @@ void swiotlb_tbl_unmap_single(struct device *dev, phys_addr_t tlb_addr,
 			      size_t mapping_size, enum dma_data_direction dir,
 			      unsigned long attrs)
 {
+	struct io_tlb_pool *pool;
+
 	/*
 	 * First, sync the memory before unmapping the entry
 	 */
@@ -983,7 +1228,13 @@ void swiotlb_tbl_unmap_single(struct device *dev, phys_addr_t tlb_addr,
 	    (dir == DMA_FROM_DEVICE || dir == DMA_BIDIRECTIONAL))
 		swiotlb_bounce(dev, tlb_addr, mapping_size, DMA_FROM_DEVICE);
 
-	swiotlb_release_slots(dev, tlb_addr);
+	pool = swiotlb_find_pool(dev, tlb_addr);
+	if (pool->transient) {
+		dec_used(dev->dma_io_tlb_mem, pool->nslabs);
+		swiotlb_del_pool(dev, pool);
+	} else {
+		swiotlb_release_slots(dev, tlb_addr);
+	}
 }
 
 void swiotlb_sync_single_for_device(struct device *dev, phys_addr_t tlb_addr,
@@ -1170,11 +1421,10 @@ struct page *swiotlb_alloc(struct device *dev, size_t size)
 	if (!mem)
 		return NULL;
 
-	index = swiotlb_find_slots(dev, 0, size, 0);
+	index = swiotlb_find_slots(dev, 0, size, 0, &pool);
 	if (index == -1)
 		return NULL;
 
-	pool = mem->pool;
 	tlb_addr = slot_addr(pool->start, index);
 
 	return pfn_to_page(PFN_DOWN(tlb_addr));
-- 
2.25.1
^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 6/8] swiotlb: determine potential physical address limit
  2023-07-13 15:23 [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers Petr Tesarik
                   ` (4 preceding siblings ...)
  2023-07-13 15:23 ` [PATCH v4 5/8] swiotlb: if swiotlb is full, fall back to a transient memory pool Petr Tesarik
@ 2023-07-13 15:23 ` Petr Tesarik
  2023-07-13 15:23 ` [PATCH v4 7/8] swiotlb: allocate a new memory pool when existing pools are full Petr Tesarik
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Petr Tesarik @ 2023-07-13 15:23 UTC (permalink / raw)
  To: Stefano Stabellini, Russell King, Thomas Bogendoerfer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Petr Tesarik, Jonathan Corbet, Andy Shevchenko,
	Hans de Goede, James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM
  Cc: Roberto Sassu, Kefeng Wang, petr
From: Petr Tesarik <petr.tesarik.ext@huawei.com>
The value returned by default_swiotlb_limit() should be constant, because
it is used to decide whether DMA can be used. To allow allocating memory
pools on the fly, use the maximum possible physical address rather than the
highest address used by the default pool.
For swiotlb_init_remap(), this is either an arch-specific limit used by
memblock_alloc_low(), or the highest directly mapped physical address if
the initialization flags include SWIOTLB_ANY. For swiotlb_init_late(), the
highest address is determined by the GFP flags.
Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
---
 include/linux/swiotlb.h |  2 ++
 kernel/dma/swiotlb.c    | 12 +++++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index b642e7739604..ff8f5150f4de 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -105,6 +105,7 @@ struct io_tlb_pool {
  * struct io_tlb_mem - Software IO TLB allocator
  * @pool:	IO TLB memory pool descriptor.
  * @nslabs:	Total number of IO TLB slabs in all pools.
+ * @phys_limit:	Maximum allowed physical address.
  * @debugfs:	The dentry to debugfs.
  * @force_bounce: %true if swiotlb bouncing is forced
  * @for_alloc:  %true if the pool is used for memory allocation
@@ -118,6 +119,7 @@ struct io_tlb_pool {
 struct io_tlb_mem {
 	struct io_tlb_pool *pool;
 	unsigned long nslabs;
+	u64 phys_limit;
 	struct dentry *debugfs;
 	bool force_bounce;
 	bool for_alloc;
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 6ec5a81acc2a..d6a05727efc5 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -334,6 +334,10 @@ void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
 	io_tlb_default_mem.force_bounce =
 		swiotlb_force_bounce || (flags & SWIOTLB_FORCE);
 	io_tlb_default_mem.can_grow = !remap;
+	if (flags & SWIOTLB_ANY)
+		io_tlb_default_mem.phys_limit = virt_to_phys(high_memory - 1);
+	else
+		io_tlb_default_mem.phys_limit = ARCH_LOW_ADDRESS_LIMIT;
 
 	if (!default_nareas)
 		swiotlb_adjust_nareas(num_possible_cpus());
@@ -402,6 +406,12 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 
 	io_tlb_default_mem.force_bounce = swiotlb_force_bounce;
 	io_tlb_default_mem.can_grow = !remap;
+	if (IS_ENABLED(CONFIG_ZONE_DMA) && (gfp_mask & __GFP_DMA))
+		io_tlb_default_mem.phys_limit = DMA_BIT_MASK(zone_dma_bits);
+	else if (IS_ENABLED(CONFIG_ZONE_DMA32) && (gfp_mask & __GFP_DMA32))
+		io_tlb_default_mem.phys_limit = DMA_BIT_MASK(32);
+	else
+		io_tlb_default_mem.phys_limit = virt_to_phys(high_memory - 1);
 
 	if (!default_nareas)
 		swiotlb_adjust_nareas(num_possible_cpus());
@@ -1338,7 +1348,7 @@ phys_addr_t default_swiotlb_start(void)
  */
 phys_addr_t default_swiotlb_limit(void)
 {
-	return io_tlb_default_pool.end - 1;
+	return io_tlb_default_mem.phys_limit;
 }
 
 #ifdef CONFIG_DEBUG_FS
-- 
2.25.1
^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 7/8] swiotlb: allocate a new memory pool when existing pools are full
  2023-07-13 15:23 [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers Petr Tesarik
                   ` (5 preceding siblings ...)
  2023-07-13 15:23 ` [PATCH v4 6/8] swiotlb: determine potential physical address limit Petr Tesarik
@ 2023-07-13 15:23 ` Petr Tesarik
  2023-07-13 15:23 ` [PATCH v4 8/8] swiotlb: search the software IO TLB only if a device makes use of it Petr Tesarik
  2023-07-20  6:52 ` [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers Christoph Hellwig
  8 siblings, 0 replies; 24+ messages in thread
From: Petr Tesarik @ 2023-07-13 15:23 UTC (permalink / raw)
  To: Stefano Stabellini, Russell King, Thomas Bogendoerfer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Petr Tesarik, Jonathan Corbet, Andy Shevchenko,
	Hans de Goede, James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM
  Cc: Roberto Sassu, Kefeng Wang, petr
From: Petr Tesarik <petr.tesarik.ext@huawei.com>
When swiotlb_find_slots() cannot find suitable slots, schedule the
allocation of a new memory pool. It is not possible to allocate the pool
immediately, because this code may run in interrupt context, which is not
suitable for large memory allocations. This means that the memory pool will
be available too late for the currently requested mapping, but the stress
on the software IO TLB allocator is likely to continue, and subsequent
allocations will benefit from the additional pool eventually.
Keep all memory pools for an allocator in an RCU list to avoid locking on
the read side. For modifications, add a new spinlock to struct io_tlb_mem.
The spinlock also protects updates to the total number of slabs (nslabs in
struct io_tlb_mem), but not reads of the value. Readers may therefore
encounter a stale value, but this is not an issue:
- swiotlb_tbl_map_single() and is_swiotlb_active() only check for non-zero
  value. This is ensured by the existence of the default memory pool,
  allocated at boot.
- The exact value is used only for non-critical purposes (debugfs, kernel
  messages).
Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
---
 include/linux/swiotlb.h |   9 ++-
 kernel/dma/swiotlb.c    | 134 +++++++++++++++++++++++++++++++---------
 2 files changed, 113 insertions(+), 30 deletions(-)
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index ff8f5150f4de..06fd94de1cd8 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -8,6 +8,7 @@
 #include <linux/types.h>
 #include <linux/limits.h>
 #include <linux/spinlock.h>
+#include <linux/workqueue.h>
 
 struct device;
 struct page;
@@ -103,13 +104,15 @@ struct io_tlb_pool {
 
 /**
  * struct io_tlb_mem - Software IO TLB allocator
- * @pool:	IO TLB memory pool descriptor.
+ * @lock:	Lock to synchronize changes to the list.
+ * @pools:	List of IO TLB memory pool descriptors.
  * @nslabs:	Total number of IO TLB slabs in all pools.
  * @phys_limit:	Maximum allowed physical address.
  * @debugfs:	The dentry to debugfs.
  * @force_bounce: %true if swiotlb bouncing is forced
  * @for_alloc:  %true if the pool is used for memory allocation
  * @can_grow:	%true if more pools can be allocated dynamically.
+ * @dyn_alloc:	Dynamic IO TLB pool allocation work.
  * @total_used:	The total number of slots in the pool that are currently used
  *		across all areas. Used only for calculating used_hiwater in
  *		debugfs.
@@ -117,13 +120,15 @@ struct io_tlb_pool {
  *		in debugfs.
  */
 struct io_tlb_mem {
-	struct io_tlb_pool *pool;
+	spinlock_t lock;
+	struct list_head pools;
 	unsigned long nslabs;
 	u64 phys_limit;
 	struct dentry *debugfs;
 	bool force_bounce;
 	bool for_alloc;
 	bool can_grow;
+	struct work_struct dyn_alloc;
 #ifdef CONFIG_DEBUG_FS
 	atomic_long_t total_used;
 	atomic_long_t used_hiwater;
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index d6a05727efc5..9c66ec2c47dd 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -79,9 +79,14 @@ struct io_tlb_slot {
 static bool swiotlb_force_bounce;
 static bool swiotlb_force_disable;
 
+static void swiotlb_dyn_alloc(struct work_struct *work);
+
 static struct io_tlb_pool io_tlb_default_pool;
-static struct io_tlb_mem io_tlb_default_mem = {
-	.pool = &io_tlb_default_pool,
+struct io_tlb_mem io_tlb_default_mem = {
+	.lock = __SPIN_LOCK_UNLOCKED(io_tlb_default_mem.lock),
+	.pools = LIST_HEAD_INIT(io_tlb_default_mem.pools),
+	.dyn_alloc = __WORK_INITIALIZER(io_tlb_default_mem.dyn_alloc,
+					swiotlb_dyn_alloc),
 };
 
 static unsigned long default_nslabs = IO_TLB_DEFAULT_SIZE >> IO_TLB_SHIFT;
@@ -281,6 +286,19 @@ static void swiotlb_init_io_tlb_pool(struct io_tlb_pool *mem, phys_addr_t start,
 	return;
 }
 
+/**
+ * add_mem_pool() - add a memory pool to the allocator
+ * @mem:	Software IO TLB allocator.
+ * @pool:	Memory pool to be added.
+ */
+static void add_mem_pool(struct io_tlb_mem *mem, struct io_tlb_pool *pool)
+{
+	spin_lock(&mem->lock);
+	list_add_rcu(&pool->node, &mem->pools);
+	mem->nslabs += pool->nslabs;
+	spin_unlock(&mem->lock);
+}
+
 static void __init *swiotlb_memblock_alloc(unsigned long nslabs,
 		unsigned int flags,
 		int (*remap)(void *tlb, unsigned long nslabs))
@@ -374,7 +392,7 @@ void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
 
 	swiotlb_init_io_tlb_pool(mem, __pa(tlb), nslabs, false,
 				 default_nareas);
-	io_tlb_default_mem.nslabs = nslabs;
+	add_mem_pool(&io_tlb_default_mem, mem);
 
 	if (flags & SWIOTLB_VERBOSE)
 		swiotlb_print_info();
@@ -466,7 +484,7 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 			     (nslabs << IO_TLB_SHIFT) >> PAGE_SHIFT);
 	swiotlb_init_io_tlb_pool(mem, virt_to_phys(vstart), nslabs, true,
 				 nareas);
-	io_tlb_default_mem.nslabs = nslabs;
+	add_mem_pool(&io_tlb_default_mem, mem);
 
 	swiotlb_print_info();
 	return 0;
@@ -615,44 +633,83 @@ static void swiotlb_free_tlb(void *vaddr, size_t bytes)
 /**
  * swiotlb_alloc_pool() - allocate a new IO TLB memory pool
  * @dev:	Device for which a memory pool is allocated.
- * @nslabs:	Desired number of slabs.
+ * @minslabs:	Minimum number of slabs.
+ * @nslabs:	Desired (maximum) number of slabs.
+ * @nareas:	Number of areas.
  * @phys_limit:	Maximum DMA buffer physical address.
  * @gfp:	GFP flags for the allocations.
  *
- * Allocate and initialize a new IO TLB memory pool.
+ * Allocate and initialize a new IO TLB memory pool. The actual number of
+ * slabs may be reduced if allocation of @nslabs fails. If even
+ * @minslabs cannot be allocated, this function fails.
  *
  * Return: New memory pool, or %NULL on allocation failure.
  */
 static struct io_tlb_pool *swiotlb_alloc_pool(struct device *dev,
-		unsigned int nslabs, u64 phys_limit, gfp_t gfp)
+		unsigned long minslabs, unsigned long nslabs,
+		unsigned int nareas, u64 phys_limit, gfp_t gfp)
 {
 	struct io_tlb_pool *pool;
+	unsigned int slot_order;
 	struct page *tlb;
 	size_t pool_size;
 	size_t tlb_size;
 
-	pool_size = sizeof(*pool) + array_size(sizeof(*pool->areas), 1) +
-		array_size(sizeof(*pool->slots), nslabs);
+	pool_size = sizeof(*pool) + array_size(sizeof(*pool->areas), nareas);
 	pool = kzalloc(pool_size, gfp);
 	if (!pool)
 		goto error;
 	pool->areas = (void *)pool + sizeof(*pool);
-	pool->slots = (void *)pool->areas + sizeof(*pool->areas);
 
 	tlb_size = nslabs << IO_TLB_SHIFT;
-	tlb = swiotlb_alloc_tlb(dev, tlb_size, phys_limit, gfp);
-	if (!tlb)
-		goto error_tlb;
+	while (!(tlb = swiotlb_alloc_tlb(dev, tlb_size, phys_limit, gfp))) {
+		if (nslabs <= minslabs)
+			goto error_tlb;
+		nslabs = ALIGN(nslabs >> 1, IO_TLB_SEGSIZE);
+		nareas = limit_nareas(nareas, nslabs);
+		tlb_size = nslabs << IO_TLB_SHIFT;
+	}
+
+	slot_order = get_order(array_size(sizeof(*pool->slots), nslabs));
+	pool->slots = (struct io_tlb_slot *)
+		__get_free_pages(gfp, slot_order);
+	if (!pool->slots)
+		goto error_slots;
 
-	swiotlb_init_io_tlb_pool(pool, page_to_phys(tlb), nslabs, true, 1);
+	swiotlb_init_io_tlb_pool(pool, page_to_phys(tlb), nslabs, true, nareas);
 	return pool;
 
+error_slots:
+	swiotlb_free_tlb(page_address(tlb), tlb_size);
 error_tlb:
 	kfree(pool);
 error:
 	return NULL;
 }
 
+/**
+ * swiotlb_dyn_alloc() - dynamic memory pool allocation worker
+ * @work:	Pointer to dyn_alloc in struct io_tlb_mem.
+ */
+static void swiotlb_dyn_alloc(struct work_struct *work)
+{
+	struct io_tlb_mem *mem =
+		container_of(work, struct io_tlb_mem, dyn_alloc);
+	struct io_tlb_pool *pool;
+
+	pool = swiotlb_alloc_pool(NULL, IO_TLB_MIN_SLABS, default_nslabs,
+				  default_nareas, mem->phys_limit, GFP_KERNEL);
+	if (!pool) {
+		pr_warn_ratelimited("Failed to allocate new pool");
+		return;
+	}
+
+	add_mem_pool(mem, pool);
+
+	/* Pairs with smp_rmb() in swiotlb_find_pool(). */
+	smp_wmb();
+}
+
 /**
  * swiotlb_dyn_free() - RCU callback to free a memory pool
  * @rcu:	RCU head in the corresponding struct io_tlb_pool.
@@ -660,8 +717,10 @@ static struct io_tlb_pool *swiotlb_alloc_pool(struct device *dev,
 static void swiotlb_dyn_free(struct rcu_head *rcu)
 {
 	struct io_tlb_pool *pool = container_of(rcu, struct io_tlb_pool, rcu);
+	size_t slots_size = array_size(sizeof(*pool->slots), pool->nslabs);
 	size_t tlb_size = pool->end - pool->start;
 
+	free_pages((unsigned long)pool->slots, get_order(slots_size));
 	swiotlb_free_tlb(pool->vaddr, tlb_size);
 	kfree(pool);
 }
@@ -690,15 +749,19 @@ void swiotlb_dev_init(struct device *dev)
 struct io_tlb_pool *swiotlb_find_pool(struct device *dev, phys_addr_t paddr)
 {
 	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
-	struct io_tlb_pool *pool = mem->pool;
-
-	if (paddr >= pool->start && paddr < pool->end)
-		return pool;
+	struct io_tlb_pool *pool;
 
-	/* Pairs with smp_wmb() in swiotlb_find_slots(). */
+	/* Pairs with smp_wmb() in swiotlb_find_slots() and
+	 * swiotlb_dyn_alloc(), which modify the RCU lists.
+	 */
 	smp_rmb();
 
 	rcu_read_lock();
+	list_for_each_entry_rcu(pool, &mem->pools, node) {
+		if (paddr >= pool->start && paddr < pool->end)
+			goto out;
+	}
+
 	list_for_each_entry_rcu(pool, &dev->dma_io_tlb_pools, node) {
 		if (paddr >= pool->start && paddr < pool->end)
 			goto out;
@@ -1030,18 +1093,25 @@ static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
 	u64 phys_limit;
 	int index;
 
-	pool = mem->pool;
-	index = pool_find_slots(dev, pool, orig_addr,
-				alloc_size, alloc_align_mask);
-	if (index >= 0)
-		goto found;
+	rcu_read_lock();
+	list_for_each_entry_rcu(pool, &mem->pools, node) {
+		index = pool_find_slots(dev, pool, orig_addr,
+					alloc_size, alloc_align_mask);
+		if (index >= 0) {
+			rcu_read_unlock();
+			goto found;
+		}
+	}
+	rcu_read_unlock();
 
 	if (!dev->dma_io_tlb_mem->can_grow)
 		return -1;
 
+	schedule_work(&mem->dyn_alloc);
+
 	nslabs = nr_slots(alloc_size);
 	phys_limit = min_not_zero(*dev->dma_mask, dev->bus_dma_limit);
-	pool = swiotlb_alloc_pool(dev, nslabs, phys_limit,
+	pool = swiotlb_alloc_pool(dev, nslabs, nslabs, 1, phys_limit,
 				  GFP_NOWAIT | __GFP_NOWARN);
 	if (!pool)
 		return -1;
@@ -1112,7 +1182,15 @@ static unsigned long mem_pool_used(struct io_tlb_pool *pool)
  */
 static unsigned long mem_used(struct io_tlb_mem *mem)
 {
-	return mem_pool_used(mem->pool);
+	struct io_tlb_pool *pool;
+	unsigned long used = 0;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(pool, &mem->pools, node)
+		used += mem_pool_used(pool);
+	rcu_read_unlock();
+
+	return used;
 }
 
 #endif /* CONFIG_DEBUG_FS */
@@ -1500,8 +1578,8 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
 		mem->force_bounce = true;
 		mem->for_alloc = true;
 		mem->can_grow = false;
-		mem->pool = pool;
-		mem->nslabs = nslabs;
+		spin_lock_init(&mem->lock);
+		add_mem_pool(mem, pool);
 
 		rmem->priv = mem;
 
-- 
2.25.1
^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v4 8/8] swiotlb: search the software IO TLB only if a device makes use of it
  2023-07-13 15:23 [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers Petr Tesarik
                   ` (6 preceding siblings ...)
  2023-07-13 15:23 ` [PATCH v4 7/8] swiotlb: allocate a new memory pool when existing pools are full Petr Tesarik
@ 2023-07-13 15:23 ` Petr Tesarik
  2023-07-20  6:47   ` Christoph Hellwig
  2023-07-20  6:52 ` [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers Christoph Hellwig
  8 siblings, 1 reply; 24+ messages in thread
From: Petr Tesarik @ 2023-07-13 15:23 UTC (permalink / raw)
  To: Stefano Stabellini, Russell King, Thomas Bogendoerfer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Petr Tesarik, Jonathan Corbet, Andy Shevchenko,
	Hans de Goede, James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM
  Cc: Roberto Sassu, Kefeng Wang, petr
From: Petr Tesarik <petr.tesarik.ext@huawei.com>
Skip searching the software IO TLB if a device has never used it, making
sure these devices are not affected by the introduction of multiple IO TLB
memory pools.
Additional memory barrier is required to ensure that the new value of the
flag is visible to other CPUs after mapping a new bounce buffer. For
efficiency, the flag check should be inlined, and then the memory barrier
must be moved to is_swiotlb_buffer(). However, it can replace the existing
barrier in swiotlb_find_pool(), because all callers use is_swiotlb_buffer()
first to verify that the buffer address belongs to the software IO TLB.
Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
---
 include/linux/device.h  |  2 ++
 include/linux/swiotlb.h |  6 +++++-
 kernel/dma/swiotlb.c    | 14 ++++++--------
 3 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/include/linux/device.h b/include/linux/device.h
index 549b0a62455c..86871d628648 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -628,6 +628,7 @@ struct device_physical_location {
  * @dma_io_tlb_mem: Software IO TLB allocator.  Not for driver use.
  * @dma_io_tlb_pools:	List of transient swiotlb memory pools.
  * @dma_io_tlb_lock:	Protects changes to the list of active pools.
+ * @dma_uses_io_tlb: %true if device has used the software IO TLB.
  * @archdata:	For arch-specific additions.
  * @of_node:	Associated device tree node.
  * @fwnode:	Associated device node supplied by platform firmware.
@@ -735,6 +736,7 @@ struct device {
 	struct io_tlb_mem *dma_io_tlb_mem;
 	struct list_head dma_io_tlb_pools;
 	spinlock_t dma_io_tlb_lock;
+	bool dma_uses_io_tlb;
 #endif
 	/* arch specific additions */
 	struct dev_archdata	archdata;
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 06fd94de1cd8..8069cb62c893 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -150,7 +150,11 @@ struct io_tlb_pool *swiotlb_find_pool(struct device *dev, phys_addr_t paddr);
  */
 static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
 {
-	return dev->dma_io_tlb_mem &&
+	/* Pairs with smp_wmb() in swiotlb_find_slots() and
+	 * swiotlb_dyn_alloc(), which modify the RCU lists.
+	 */
+	smp_rmb();
+	return dev->dma_uses_io_tlb &&
 		!!swiotlb_find_pool(dev, paddr);
 }
 
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 9c66ec2c47dd..854d139ddcb7 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -706,7 +706,7 @@ static void swiotlb_dyn_alloc(struct work_struct *work)
 
 	add_mem_pool(mem, pool);
 
-	/* Pairs with smp_rmb() in swiotlb_find_pool(). */
+	/* Pairs with smp_rmb() in is_swiotlb_buffer(). */
 	smp_wmb();
 }
 
@@ -734,6 +734,7 @@ void swiotlb_dev_init(struct device *dev)
 	dev->dma_io_tlb_mem = &io_tlb_default_mem;
 	INIT_LIST_HEAD(&dev->dma_io_tlb_pools);
 	spin_lock_init(&dev->dma_io_tlb_lock);
+	dev->dma_uses_io_tlb = false;
 }
 
 /**
@@ -751,11 +752,6 @@ struct io_tlb_pool *swiotlb_find_pool(struct device *dev, phys_addr_t paddr)
 	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
 	struct io_tlb_pool *pool;
 
-	/* Pairs with smp_wmb() in swiotlb_find_slots() and
-	 * swiotlb_dyn_alloc(), which modify the RCU lists.
-	 */
-	smp_rmb();
-
 	rcu_read_lock();
 	list_for_each_entry_rcu(pool, &mem->pools, node) {
 		if (paddr >= pool->start && paddr < pool->end)
@@ -1128,9 +1124,11 @@ static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
 	list_add_rcu(&pool->node, &dev->dma_io_tlb_pools);
 	spin_unlock_irqrestore(&dev->dma_io_tlb_lock, flags);
 
-	/* Pairs with smp_rmb() in swiotlb_find_pool(). */
-	smp_wmb();
 found:
+	dev->dma_uses_io_tlb = true;
+	/* Pairs with smp_rmb() in is_swiotlb_buffer() */
+	smp_wmb();
+
 	*retpool = pool;
 	return index;
 }
-- 
2.25.1
^ permalink raw reply related	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 3/8] swiotlb: separate memory pool data from other allocator data
  2023-07-13 15:23 ` [PATCH v4 3/8] swiotlb: separate memory pool data from other allocator data Petr Tesarik
@ 2023-07-13 17:53   ` Petr Tesařík
  0 siblings, 0 replies; 24+ messages in thread
From: Petr Tesařík @ 2023-07-13 17:53 UTC (permalink / raw)
  To: Petr Tesarik
  Cc: Stefano Stabellini, Russell King, Thomas Bogendoerfer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Petr Tesarik, Jonathan Corbet, Andy Shevchenko,
	Hans de Goede, James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM,
	Roberto Sassu, Kefeng Wang
On Thu, 13 Jul 2023 17:23:14 +0200
Petr Tesarik <petrtesarik@huaweicloud.com> wrote:
> From: Petr Tesarik <petr.tesarik.ext@huawei.com>
> 
> Carve out memory pool specific fields from struct io_tlb_mem. The original
> struct now contains shared data for the whole allocator, while the new
> struct io_tlb_pool contains data that is specific to one memory pool of
> (potentially) many.
> 
> Allocate both structures together for restricted DMA pools to keep the
> error cleanup path simple.
> 
> Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
> ---
>  include/linux/device.h  |   2 +-
>  include/linux/swiotlb.h |  47 +++++++----
>  kernel/dma/swiotlb.c    | 181 +++++++++++++++++++++++++---------------
>  3 files changed, 147 insertions(+), 83 deletions(-)
> 
> diff --git a/include/linux/device.h b/include/linux/device.h
> index bbaeabd04b0d..d9754a68ba95 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -625,7 +625,7 @@ struct device_physical_location {
>   * @dma_pools:	Dma pools (if dma'ble device).
>   * @dma_mem:	Internal for coherent mem override.
>   * @cma_area:	Contiguous memory area for dma allocations
> - * @dma_io_tlb_mem: Pointer to the swiotlb pool used.  Not for driver use.
> + * @dma_io_tlb_mem: Software IO TLB allocator.  Not for driver use.
>   * @archdata:	For arch-specific additions.
>   * @of_node:	Associated device tree node.
>   * @fwnode:	Associated device node supplied by platform firmware.
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 39313c3a791a..d669e11e2827 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -62,8 +62,7 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t phys,
>  #ifdef CONFIG_SWIOTLB
>  
>  /**
> - * struct io_tlb_mem - IO TLB Memory Pool Descriptor
> - *
> + * struct io_tlb_pool - IO TLB memory pool descriptor
>   * @start:	The start address of the swiotlb memory pool. Used to do a quick
>   *		range check to see if the memory was in fact allocated by this
>   *		API.
> @@ -73,15 +72,36 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t phys,
>   * @vaddr:	The vaddr of the swiotlb memory pool. The swiotlb memory pool
>   *		may be remapped in the memory encrypted case and store virtual
>   *		address for bounce buffer operation.
> - * @nslabs:	The number of IO TLB blocks (in groups of 64) between @start and
> - *		@end. For default swiotlb, this is command line adjustable via
> - *		setup_io_tlb_npages.
> + * @nslabs:	The number of IO TLB slots between @start and @end. For the
> + *		default swiotlb, this can be adjusted with a boot parameter,
> + *		see setup_io_tlb_npages().
> + * @used:	The number of used IO TLB slots.
> + * @late_alloc:	%true if allocated using the page allocator.
> + * @nareas:	Number of areas in the pool.
> + * @area_nslabs: Number of slots in each area.
> + * @areas:	Array of memory area descriptors.
> + * @slots:	Array of slot descriptors.
> + */
> +struct io_tlb_pool {
> +	phys_addr_t start;
> +	phys_addr_t end;
> +	void *vaddr;
> +	unsigned long nslabs;
> +	unsigned long used;
Oops. This member should not be re-introduced here after I removed it
with commit efa76afdde16...
I'm going to fix this in a v5, but I don't think it's critical enough
to make an immediate resend.
Petr T
^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 1/8] swiotlb: make io_tlb_default_mem local to swiotlb.c
  2023-07-13 15:23 ` [PATCH v4 1/8] swiotlb: make io_tlb_default_mem local to swiotlb.c Petr Tesarik
@ 2023-07-17  6:06   ` Philippe Mathieu-Daudé
  2023-07-17 10:17     ` Petr Tesařík
  2023-07-20  6:37   ` Christoph Hellwig
  1 sibling, 1 reply; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-07-17  6:06 UTC (permalink / raw)
  To: Petr Tesarik, Stefano Stabellini, Russell King,
	Thomas Bogendoerfer, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Petr Tesarik, Jonathan Corbet, Andy Shevchenko,
	Hans de Goede, James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM
  Cc: Roberto Sassu, Kefeng Wang, petr
Hi Petr,
On 13/7/23 17:23, Petr Tesarik wrote:
> From: Petr Tesarik <petr.tesarik.ext@huawei.com>
> 
> SWIOTLB implementation details should not be exposed to the rest of the
> kernel. This will allow to make changes to the implementation without
> modifying non-swiotlb code.
> 
> To avoid breaking existing users, provide helper functions for the few
> required fields.
> 
> As a bonus, using a helper function to initialize struct device allows to
> get rid of an #ifdef in driver core.
> 
> Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
> ---
>   arch/arm/xen/mm.c          |  2 +-
>   arch/mips/pci/pci-octeon.c |  2 +-
>   arch/x86/kernel/pci-dma.c  |  2 +-
>   drivers/base/core.c        |  4 +---
>   drivers/xen/swiotlb-xen.c  |  2 +-
>   include/linux/swiotlb.h    | 25 +++++++++++++++++++++++-
>   kernel/dma/swiotlb.c       | 39 +++++++++++++++++++++++++++++++++++++-
>   7 files changed, 67 insertions(+), 9 deletions(-)
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 4e52cd5e0bdc..07216af59e93 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -110,7 +110,6 @@ struct io_tlb_mem {
>   	atomic_long_t used_hiwater;
>   #endif
>   };
> -extern struct io_tlb_mem io_tlb_default_mem;
>   
>   static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
>   {
> @@ -128,13 +127,22 @@ static inline bool is_swiotlb_force_bounce(struct device *dev)
>   
>   void swiotlb_init(bool addressing_limited, unsigned int flags);
>   void __init swiotlb_exit(void);
> +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);
>   void __init swiotlb_adjust_size(unsigned long size);
> +phys_addr_t default_swiotlb_start(void);
> +phys_addr_t default_swiotlb_limit(void);
Usually we use start/end, base/limit, low[est]/high[est] tuples.
Possibly clearer to rename, regardless:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 2b83e3ad9dca..873b077d7e37 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -958,6 +975,26 @@ bool is_swiotlb_active(struct device *dev)
>   }
>   EXPORT_SYMBOL_GPL(is_swiotlb_active);
>   
> +/**
> + * default_swiotlb_start() - get the start of the default SWIOTLB
> + *
> + * Get the lowest physical address used by the default software IO TLB pool.
> + */
> +phys_addr_t default_swiotlb_start(void)
> +{
> +	return io_tlb_default_mem.start;
> +}
> +
> +/**
> + * default_swiotlb_limit() - get the highest address in the default SWIOTLB
> + *
> + * Get the highest physical address used by the default software IO TLB pool.
(note you describe lowest/highest).
> + */
> +phys_addr_t default_swiotlb_limit(void)
> +{
> +	return io_tlb_default_mem.end - 1;
> +}
> +
>   #ifdef CONFIG_DEBUG_FS
>   
>   static int io_tlb_used_get(void *data, u64 *val)
^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 1/8] swiotlb: make io_tlb_default_mem local to swiotlb.c
  2023-07-17  6:06   ` Philippe Mathieu-Daudé
@ 2023-07-17 10:17     ` Petr Tesařík
  0 siblings, 0 replies; 24+ messages in thread
From: Petr Tesařík @ 2023-07-17 10:17 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Petr Tesarik, Stefano Stabellini, Russell King,
	Thomas Bogendoerfer, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Petr Tesarik, Jonathan Corbet, Andy Shevchenko,
	Hans de Goede, James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM,
	Roberto Sassu, Kefeng Wang
On Mon, 17 Jul 2023 08:06:07 +0200
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> Hi Petr,
> 
> On 13/7/23 17:23, Petr Tesarik wrote:
> > From: Petr Tesarik <petr.tesarik.ext@huawei.com>
> > 
> > SWIOTLB implementation details should not be exposed to the rest of the
> > kernel. This will allow to make changes to the implementation without
> > modifying non-swiotlb code.
> > 
> > To avoid breaking existing users, provide helper functions for the few
> > required fields.
> > 
> > As a bonus, using a helper function to initialize struct device allows to
> > get rid of an #ifdef in driver core.
> > 
> > Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
> > ---
> >   arch/arm/xen/mm.c          |  2 +-
> >   arch/mips/pci/pci-octeon.c |  2 +-
> >   arch/x86/kernel/pci-dma.c  |  2 +-
> >   drivers/base/core.c        |  4 +---
> >   drivers/xen/swiotlb-xen.c  |  2 +-
> >   include/linux/swiotlb.h    | 25 +++++++++++++++++++++++-
> >   kernel/dma/swiotlb.c       | 39 +++++++++++++++++++++++++++++++++++++-
> >   7 files changed, 67 insertions(+), 9 deletions(-)  
> 
> 
> > diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> > index 4e52cd5e0bdc..07216af59e93 100644
> > --- a/include/linux/swiotlb.h
> > +++ b/include/linux/swiotlb.h
> > @@ -110,7 +110,6 @@ struct io_tlb_mem {
> >   	atomic_long_t used_hiwater;
> >   #endif
> >   };
> > -extern struct io_tlb_mem io_tlb_default_mem;
> >   
> >   static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
> >   {
> > @@ -128,13 +127,22 @@ static inline bool is_swiotlb_force_bounce(struct device *dev)
> >   
> >   void swiotlb_init(bool addressing_limited, unsigned int flags);
> >   void __init swiotlb_exit(void);
> > +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);
> >   void __init swiotlb_adjust_size(unsigned long size);
> > +phys_addr_t default_swiotlb_start(void);
> > +phys_addr_t default_swiotlb_limit(void);  
> 
> Usually we use start/end, base/limit, low[est]/high[est] tuples.
I'm no big fan of start/end, because the "end" sometimes means "highest
within range" and sometimes "one past range", being responsible for an
impressive amount of off-by-one errors.
But I agree. When I decided against "end", I should have also replaced
"start" with "base". Well, this patch series will certainly see a v5,
so I'll change it there. Thanks for the suggestion!
Petr T
> Possibly clearer to rename, regardless:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> > diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> > index 2b83e3ad9dca..873b077d7e37 100644
> > --- a/kernel/dma/swiotlb.c
> > +++ b/kernel/dma/swiotlb.c  
> 
> 
> > @@ -958,6 +975,26 @@ bool is_swiotlb_active(struct device *dev)
> >   }
> >   EXPORT_SYMBOL_GPL(is_swiotlb_active);
> >   
> > +/**
> > + * default_swiotlb_start() - get the start of the default SWIOTLB
> > + *
> > + * Get the lowest physical address used by the default software IO TLB pool.
> > + */
> > +phys_addr_t default_swiotlb_start(void)
> > +{
> > +	return io_tlb_default_mem.start;
> > +}
> > +
> > +/**
> > + * default_swiotlb_limit() - get the highest address in the default SWIOTLB
> > + *
> > + * Get the highest physical address used by the default software IO TLB pool.  
> 
> (note you describe lowest/highest).
> 
> > + */
> > +phys_addr_t default_swiotlb_limit(void)
> > +{
> > +	return io_tlb_default_mem.end - 1;
> > +}
> > +
> >   #ifdef CONFIG_DEBUG_FS
> >   
> >   static int io_tlb_used_get(void *data, u64 *val)  
> 
^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 1/8] swiotlb: make io_tlb_default_mem local to swiotlb.c
  2023-07-13 15:23 ` [PATCH v4 1/8] swiotlb: make io_tlb_default_mem local to swiotlb.c Petr Tesarik
  2023-07-17  6:06   ` Philippe Mathieu-Daudé
@ 2023-07-20  6:37   ` Christoph Hellwig
  2023-07-20  7:54     ` Petr Tesařík
  1 sibling, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2023-07-20  6:37 UTC (permalink / raw)
  To: Petr Tesarik
  Cc: Stefano Stabellini, Russell King, Thomas Bogendoerfer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Petr Tesarik, Jonathan Corbet, Andy Shevchenko,
	Hans de Goede, James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM,
	Roberto Sassu, Kefeng Wang, petr
On Thu, Jul 13, 2023 at 05:23:12PM +0200, Petr Tesarik wrote:
> From: Petr Tesarik <petr.tesarik.ext@huawei.com>
> 
> SWIOTLB implementation details should not be exposed to the rest of the
> kernel. This will allow to make changes to the implementation without
> modifying non-swiotlb code.
> 
> To avoid breaking existing users, provide helper functions for the few
> required fields.
> 
> As a bonus, using a helper function to initialize struct device allows to
> get rid of an #ifdef in driver core.
> 
> Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
> ---
>  arch/arm/xen/mm.c          |  2 +-
>  arch/mips/pci/pci-octeon.c |  2 +-
>  arch/x86/kernel/pci-dma.c  |  2 +-
>  drivers/base/core.c        |  4 +---
>  drivers/xen/swiotlb-xen.c  |  2 +-
>  include/linux/swiotlb.h    | 25 +++++++++++++++++++++++-
>  kernel/dma/swiotlb.c       | 39 +++++++++++++++++++++++++++++++++++++-
>  7 files changed, 67 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> index 3d826c0b5fee..0f32c14eb786 100644
> --- a/arch/arm/xen/mm.c
> +++ b/arch/arm/xen/mm.c
> @@ -125,7 +125,7 @@ static int __init xen_mm_init(void)
>  		return 0;
>  
>  	/* we can work with the default swiotlb */
> -	if (!io_tlb_default_mem.nslabs) {
> +	if (!is_swiotlb_allocated()) {
>  		rc = swiotlb_init_late(swiotlb_size_or_default(),
>  				       xen_swiotlb_gfp(), NULL);
>  		if (rc < 0)
I'd much rather move the already initialized check into
swiotlb_init_late, which is a much cleaer interface.
>  	/* we can work with the default swiotlb */
> -	if (!io_tlb_default_mem.nslabs) {
> +	if (!is_swiotlb_allocated()) {
>  		int rc = swiotlb_init_late(swiotlb_size_or_default(),
>  					   GFP_KERNEL, xen_swiotlb_fixup);
>  		if (rc < 0)
.. and would take care of this one as well.
> +bool is_swiotlb_allocated(void)
> +{
> +	return !!io_tlb_default_mem.nslabs;
Nit: no need for the !!, we can rely on the implicit promotion to
bool.  But with the suggestion above the need for this helper
should go away anyway.
^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 2/8] swiotlb: add documentation and rename swiotlb_do_find_slots()
  2023-07-13 15:23 ` [PATCH v4 2/8] swiotlb: add documentation and rename swiotlb_do_find_slots() Petr Tesarik
@ 2023-07-20  6:38   ` Christoph Hellwig
  2023-07-20  7:56     ` Petr Tesařík
  0 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2023-07-20  6:38 UTC (permalink / raw)
  To: Petr Tesarik
  Cc: Stefano Stabellini, Russell King, Thomas Bogendoerfer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Petr Tesarik, Jonathan Corbet, Andy Shevchenko,
	Hans de Goede, James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM,
	Roberto Sassu, Kefeng Wang, petr
On Thu, Jul 13, 2023 at 05:23:13PM +0200, Petr Tesarik wrote:
> From: Petr Tesarik <petr.tesarik.ext@huawei.com>
> 
> Add some kernel-doc comments and move the existing documentation of struct
> io_tlb_slot to its correct location. The latter was forgotten in commit
> 942a8186eb445 ("swiotlb: move struct io_tlb_slot to swiotlb.c").
> 
> Use the opportunity to give swiotlb_do_find_slots() a more descriptive
> name, which makes it clear how it differs from swiotlb_find_slots().
Please keep the swiotlb_ prefix.  Otherwise this looks good to me.
^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 8/8] swiotlb: search the software IO TLB only if a device makes use of it
  2023-07-13 15:23 ` [PATCH v4 8/8] swiotlb: search the software IO TLB only if a device makes use of it Petr Tesarik
@ 2023-07-20  6:47   ` Christoph Hellwig
  2023-07-20  8:02     ` Petr Tesařík
  0 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2023-07-20  6:47 UTC (permalink / raw)
  To: Petr Tesarik
  Cc: Stefano Stabellini, Russell King, Thomas Bogendoerfer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Petr Tesarik, Jonathan Corbet, Andy Shevchenko,
	Hans de Goede, James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM,
	Roberto Sassu, Kefeng Wang, petr
Any reason this can't just do a list_empty_careful on the list
instead of adding yet another field that grows struct device?
^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers
  2023-07-13 15:23 [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers Petr Tesarik
                   ` (7 preceding siblings ...)
  2023-07-13 15:23 ` [PATCH v4 8/8] swiotlb: search the software IO TLB only if a device makes use of it Petr Tesarik
@ 2023-07-20  6:52 ` Christoph Hellwig
  2023-07-20  8:13   ` Petr Tesařík
  8 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2023-07-20  6:52 UTC (permalink / raw)
  To: Petr Tesarik
  Cc: Stefano Stabellini, Russell King, Thomas Bogendoerfer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Petr Tesarik, Jonathan Corbet, Andy Shevchenko,
	Hans de Goede, James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM,
	Roberto Sassu, Kefeng Wang, petr
Just to add a highlevel comment here after I feel like I need a little
more time to review the guts.
I'm still pretty concerned about the extra list that needs to be
consulted in is_swiotlb_buffer, but I can't really think of
anything better.  Maybe an xarray has better cache characteristics,
but that one requires even more allocations in the low-level dma map
path.
One thing I'd like to see for the next version is to make the
new growing code a config option at least for now.  It is a pretty
big change of the existing swiotlb behavior, and I want people to opt
into it conciously.  Maybe we can drop the option again after a few
years once everything has settled.
^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 1/8] swiotlb: make io_tlb_default_mem local to swiotlb.c
  2023-07-20  6:37   ` Christoph Hellwig
@ 2023-07-20  7:54     ` Petr Tesařík
  0 siblings, 0 replies; 24+ messages in thread
From: Petr Tesařík @ 2023-07-20  7:54 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Petr Tesarik, Stefano Stabellini, Russell King,
	Thomas Bogendoerfer, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Marek Szyprowski, Robin Murphy,
	Petr Tesarik, Jonathan Corbet, Andy Shevchenko, Hans de Goede,
	James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM,
	Roberto Sassu, Kefeng Wang
On Thu, 20 Jul 2023 08:37:44 +0200
Christoph Hellwig <hch@lst.de> wrote:
> On Thu, Jul 13, 2023 at 05:23:12PM +0200, Petr Tesarik wrote:
> > From: Petr Tesarik <petr.tesarik.ext@huawei.com>
> > 
> > SWIOTLB implementation details should not be exposed to the rest of the
> > kernel. This will allow to make changes to the implementation without
> > modifying non-swiotlb code.
> > 
> > To avoid breaking existing users, provide helper functions for the few
> > required fields.
> > 
> > As a bonus, using a helper function to initialize struct device allows to
> > get rid of an #ifdef in driver core.
> > 
> > Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
> > ---
> >  arch/arm/xen/mm.c          |  2 +-
> >  arch/mips/pci/pci-octeon.c |  2 +-
> >  arch/x86/kernel/pci-dma.c  |  2 +-
> >  drivers/base/core.c        |  4 +---
> >  drivers/xen/swiotlb-xen.c  |  2 +-
> >  include/linux/swiotlb.h    | 25 +++++++++++++++++++++++-
> >  kernel/dma/swiotlb.c       | 39 +++++++++++++++++++++++++++++++++++++-
> >  7 files changed, 67 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> > index 3d826c0b5fee..0f32c14eb786 100644
> > --- a/arch/arm/xen/mm.c
> > +++ b/arch/arm/xen/mm.c
> > @@ -125,7 +125,7 @@ static int __init xen_mm_init(void)
> >  		return 0;
> >  
> >  	/* we can work with the default swiotlb */
> > -	if (!io_tlb_default_mem.nslabs) {
> > +	if (!is_swiotlb_allocated()) {
> >  		rc = swiotlb_init_late(swiotlb_size_or_default(),
> >  				       xen_swiotlb_gfp(), NULL);
> >  		if (rc < 0)  
> 
> I'd much rather move the already initialized check into
> swiotlb_init_late, which is a much cleaer interface.
> 
> >  	/* we can work with the default swiotlb */
> > -	if (!io_tlb_default_mem.nslabs) {
> > +	if (!is_swiotlb_allocated()) {
> >  		int rc = swiotlb_init_late(swiotlb_size_or_default(),
> >  					   GFP_KERNEL, xen_swiotlb_fixup);
> >  		if (rc < 0)  
> 
> .. and would take care of this one as well.
Oh, you're right! These are the only two places that look at
io_tlb_default_mem.nslabs, and all they need is to avoid double
initialization. Makes perfect sense to move it inside
swiotlb_init_late().
> > +bool is_swiotlb_allocated(void)
> > +{
> > +	return !!io_tlb_default_mem.nslabs;  
> 
> Nit: no need for the !!, we can rely on the implicit promotion to
> bool.  But with the suggestion above the need for this helper
> should go away anyway.
Eh, yes. I initially declared the return type as int and then forgot to
change the return statement. But as you say, the whole function will go
away entirely.
Petr T
^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 2/8] swiotlb: add documentation and rename swiotlb_do_find_slots()
  2023-07-20  6:38   ` Christoph Hellwig
@ 2023-07-20  7:56     ` Petr Tesařík
  2023-07-20  8:01       ` Christoph Hellwig
  0 siblings, 1 reply; 24+ messages in thread
From: Petr Tesařík @ 2023-07-20  7:56 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Petr Tesarik, Stefano Stabellini, Russell King,
	Thomas Bogendoerfer, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Marek Szyprowski, Robin Murphy,
	Petr Tesarik, Jonathan Corbet, Andy Shevchenko, Hans de Goede,
	James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM,
	Roberto Sassu, Kefeng Wang
On Thu, 20 Jul 2023 08:38:19 +0200
Christoph Hellwig <hch@lst.de> wrote:
> On Thu, Jul 13, 2023 at 05:23:13PM +0200, Petr Tesarik wrote:
> > From: Petr Tesarik <petr.tesarik.ext@huawei.com>
> > 
> > Add some kernel-doc comments and move the existing documentation of struct
> > io_tlb_slot to its correct location. The latter was forgotten in commit
> > 942a8186eb445 ("swiotlb: move struct io_tlb_slot to swiotlb.c").
> > 
> > Use the opportunity to give swiotlb_do_find_slots() a more descriptive
> > name, which makes it clear how it differs from swiotlb_find_slots().  
> 
> Please keep the swiotlb_ prefix.  Otherwise this looks good to me.
Will do. Out of curiosity, why does it matter for a static (file-local)
function?
Petr T
^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 2/8] swiotlb: add documentation and rename swiotlb_do_find_slots()
  2023-07-20  7:56     ` Petr Tesařík
@ 2023-07-20  8:01       ` Christoph Hellwig
  2023-07-20  8:14         ` Petr Tesařík
  0 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2023-07-20  8:01 UTC (permalink / raw)
  To: Petr Tesařík
  Cc: Christoph Hellwig, Petr Tesarik, Stefano Stabellini, Russell King,
	Thomas Bogendoerfer, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Marek Szyprowski, Robin Murphy,
	Petr Tesarik, Jonathan Corbet, Andy Shevchenko, Hans de Goede,
	James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM,
	Roberto Sassu, Kefeng Wang
On Thu, Jul 20, 2023 at 09:56:09AM +0200, Petr Tesařík wrote:
> On Thu, 20 Jul 2023 08:38:19 +0200
> Christoph Hellwig <hch@lst.de> wrote:
> 
> > On Thu, Jul 13, 2023 at 05:23:13PM +0200, Petr Tesarik wrote:
> > > From: Petr Tesarik <petr.tesarik.ext@huawei.com>
> > > 
> > > Add some kernel-doc comments and move the existing documentation of struct
> > > io_tlb_slot to its correct location. The latter was forgotten in commit
> > > 942a8186eb445 ("swiotlb: move struct io_tlb_slot to swiotlb.c").
> > > 
> > > Use the opportunity to give swiotlb_do_find_slots() a more descriptive
> > > name, which makes it clear how it differs from swiotlb_find_slots().  
> > 
> > Please keep the swiotlb_ prefix.  Otherwise this looks good to me.
> 
> Will do. Out of curiosity, why does it matter for a static (file-local)
> function?
Because it makes looking at stack traces much easier.
^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 8/8] swiotlb: search the software IO TLB only if a device makes use of it
  2023-07-20  6:47   ` Christoph Hellwig
@ 2023-07-20  8:02     ` Petr Tesařík
  2023-07-20  8:22       ` Christoph Hellwig
  0 siblings, 1 reply; 24+ messages in thread
From: Petr Tesařík @ 2023-07-20  8:02 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Petr Tesarik, Stefano Stabellini, Russell King,
	Thomas Bogendoerfer, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Marek Szyprowski, Robin Murphy,
	Petr Tesarik, Jonathan Corbet, Andy Shevchenko, Hans de Goede,
	James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM,
	Roberto Sassu, Kefeng Wang
On Thu, 20 Jul 2023 08:47:44 +0200
Christoph Hellwig <hch@lst.de> wrote:
> Any reason this can't just do a list_empty_careful on the list
> instead of adding yet another field that grows struct device?
On which list?
The dma_io_tlb_pools list only contains transient pools, but a device
may use bounce buffers from a regular pool.
The dma_io_tlb_mem.pools list will always be non-empty, unless the
system runs without SWIOTLB.
On a system which does have a SWIOTLB, the flag allows to differentiate
between devices that actually use bounce buffers and devices that do
not (e.g. because they do not have any addressing limitations).
Petr T
^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers
  2023-07-20  6:52 ` [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers Christoph Hellwig
@ 2023-07-20  8:13   ` Petr Tesařík
  2023-07-20  8:23     ` Christoph Hellwig
  0 siblings, 1 reply; 24+ messages in thread
From: Petr Tesařík @ 2023-07-20  8:13 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Petr Tesarik, Stefano Stabellini, Russell King,
	Thomas Bogendoerfer, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Marek Szyprowski, Robin Murphy,
	Petr Tesarik, Jonathan Corbet, Andy Shevchenko, Hans de Goede,
	James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM,
	Roberto Sassu, Kefeng Wang
On Thu, 20 Jul 2023 08:52:16 +0200
Christoph Hellwig <hch@lst.de> wrote:
> Just to add a highlevel comment here after I feel like I need a little
> more time to review the guts.
> 
> I'm still pretty concerned about the extra list that needs to be
> consulted in is_swiotlb_buffer, but I can't really think of
> anything better.  Maybe an xarray has better cache characteristics,
> but that one requires even more allocations in the low-level dma map
> path.
> 
> One thing I'd like to see for the next version is to make the
> new growing code a config option at least for now.  It is a pretty
> big change of the existing swiotlb behavior, and I want people to opt
> into it conciously.  Maybe we can drop the option again after a few
> years once everything has settled.
Fine with me. I removed it after all my testing showed no performance
impact as long as the size of the initial SWIOTLB is kept at the
default value (and sufficient for the workload), but it's OK for me if
dynamic SWIOTLB allocations are off by default.
OTOH I'd like to make it a boot-time option rather than build-time
option. Would that be OK for you?
Petr T
^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 2/8] swiotlb: add documentation and rename swiotlb_do_find_slots()
  2023-07-20  8:01       ` Christoph Hellwig
@ 2023-07-20  8:14         ` Petr Tesařík
  0 siblings, 0 replies; 24+ messages in thread
From: Petr Tesařík @ 2023-07-20  8:14 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Petr Tesarik, Stefano Stabellini, Russell King,
	Thomas Bogendoerfer, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Marek Szyprowski, Robin Murphy,
	Petr Tesarik, Jonathan Corbet, Andy Shevchenko, Hans de Goede,
	James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM,
	Roberto Sassu, Kefeng Wang
On Thu, 20 Jul 2023 10:01:10 +0200
Christoph Hellwig <hch@lst.de> wrote:
> On Thu, Jul 20, 2023 at 09:56:09AM +0200, Petr Tesařík wrote:
> > On Thu, 20 Jul 2023 08:38:19 +0200
> > Christoph Hellwig <hch@lst.de> wrote:
> >   
> > > On Thu, Jul 13, 2023 at 05:23:13PM +0200, Petr Tesarik wrote:  
> > > > From: Petr Tesarik <petr.tesarik.ext@huawei.com>
> > > > 
> > > > Add some kernel-doc comments and move the existing documentation of struct
> > > > io_tlb_slot to its correct location. The latter was forgotten in commit
> > > > 942a8186eb445 ("swiotlb: move struct io_tlb_slot to swiotlb.c").
> > > > 
> > > > Use the opportunity to give swiotlb_do_find_slots() a more descriptive
> > > > name, which makes it clear how it differs from swiotlb_find_slots().    
> > > 
> > > Please keep the swiotlb_ prefix.  Otherwise this looks good to me.  
> > 
> > Will do. Out of curiosity, why does it matter for a static (file-local)
> > function?  
> 
> Because it makes looking at stack traces much easier.
Got it. Thanks!
Petr T
^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 8/8] swiotlb: search the software IO TLB only if a device makes use of it
  2023-07-20  8:02     ` Petr Tesařík
@ 2023-07-20  8:22       ` Christoph Hellwig
  0 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2023-07-20  8:22 UTC (permalink / raw)
  To: Petr Tesařík
  Cc: Christoph Hellwig, Petr Tesarik, Stefano Stabellini, Russell King,
	Thomas Bogendoerfer, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Marek Szyprowski, Robin Murphy,
	Petr Tesarik, Jonathan Corbet, Andy Shevchenko, Hans de Goede,
	James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM,
	Roberto Sassu, Kefeng Wang
On Thu, Jul 20, 2023 at 10:02:38AM +0200, Petr Tesařík wrote:
> On Thu, 20 Jul 2023 08:47:44 +0200
> Christoph Hellwig <hch@lst.de> wrote:
> 
> > Any reason this can't just do a list_empty_careful on the list
> > instead of adding yet another field that grows struct device?
> 
> On which list?
dev->dma_io_tlb_mem->pools?
> 
> The dma_io_tlb_pools list only contains transient pools, but a device
> may use bounce buffers from a regular pool.
Oh, true.
> The dma_io_tlb_mem.pools list will always be non-empty, unless the
> system runs without SWIOTLB.
> 
> On a system which does have a SWIOTLB, the flag allows to differentiate
> between devices that actually use bounce buffers and devices that do
> not (e.g. because they do not have any addressing limitations).
Ok.
^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers
  2023-07-20  8:13   ` Petr Tesařík
@ 2023-07-20  8:23     ` Christoph Hellwig
  0 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2023-07-20  8:23 UTC (permalink / raw)
  To: Petr Tesařík
  Cc: Christoph Hellwig, Petr Tesarik, Stefano Stabellini, Russell King,
	Thomas Bogendoerfer, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
	Greg Kroah-Hartman, Rafael J. Wysocki, Juergen Gross,
	Oleksandr Tyshchenko, Marek Szyprowski, Robin Murphy,
	Petr Tesarik, Jonathan Corbet, Andy Shevchenko, Hans de Goede,
	James Seo, James Clark, Kees Cook,
	moderated list:XEN HYPERVISOR ARM, moderated list:ARM PORT,
	open list, open list:MIPS, open list:XEN SWIOTLB SUBSYSTEM,
	Roberto Sassu, Kefeng Wang
On Thu, Jul 20, 2023 at 10:13:20AM +0200, Petr Tesařík wrote:
> Fine with me. I removed it after all my testing showed no performance
> impact as long as the size of the initial SWIOTLB is kept at the
> default value (and sufficient for the workload), but it's OK for me if
> dynamic SWIOTLB allocations are off by default.
> 
> OTOH I'd like to make it a boot-time option rather than build-time
> option. Would that be OK for you?
I'd really like the config option to not even build the code.  But
a boot time option sounds very useful in addition to that.
^ permalink raw reply	[flat|nested] 24+ messages in thread
end of thread, other threads:[~2023-07-20  8:23 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-13 15:23 [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers Petr Tesarik
2023-07-13 15:23 ` [PATCH v4 1/8] swiotlb: make io_tlb_default_mem local to swiotlb.c Petr Tesarik
2023-07-17  6:06   ` Philippe Mathieu-Daudé
2023-07-17 10:17     ` Petr Tesařík
2023-07-20  6:37   ` Christoph Hellwig
2023-07-20  7:54     ` Petr Tesařík
2023-07-13 15:23 ` [PATCH v4 2/8] swiotlb: add documentation and rename swiotlb_do_find_slots() Petr Tesarik
2023-07-20  6:38   ` Christoph Hellwig
2023-07-20  7:56     ` Petr Tesařík
2023-07-20  8:01       ` Christoph Hellwig
2023-07-20  8:14         ` Petr Tesařík
2023-07-13 15:23 ` [PATCH v4 3/8] swiotlb: separate memory pool data from other allocator data Petr Tesarik
2023-07-13 17:53   ` Petr Tesařík
2023-07-13 15:23 ` [PATCH v4 4/8] swiotlb: add a flag whether a SWIOTLB is allowed to grow Petr Tesarik
2023-07-13 15:23 ` [PATCH v4 5/8] swiotlb: if swiotlb is full, fall back to a transient memory pool Petr Tesarik
2023-07-13 15:23 ` [PATCH v4 6/8] swiotlb: determine potential physical address limit Petr Tesarik
2023-07-13 15:23 ` [PATCH v4 7/8] swiotlb: allocate a new memory pool when existing pools are full Petr Tesarik
2023-07-13 15:23 ` [PATCH v4 8/8] swiotlb: search the software IO TLB only if a device makes use of it Petr Tesarik
2023-07-20  6:47   ` Christoph Hellwig
2023-07-20  8:02     ` Petr Tesařík
2023-07-20  8:22       ` Christoph Hellwig
2023-07-20  6:52 ` [PATCH v4 0/8] Allow dynamic allocation of software IO TLB bounce buffers Christoph Hellwig
2023-07-20  8:13   ` Petr Tesařík
2023-07-20  8:23     ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).