[PATCH] swiotlb v0.8: seperation of physical/virtual address translation

[PATCH] swiotlb v0.8: seperation of physical/virtual address translation

[Konrad Rzeszutek Wilk]

Fujita-san et al.

Thank you for your continued careful review of these patches! I've attached your
set of patches to this e-mail, I hope that is ok?

Since the last posting [v7] I've done:
 - Minimized the list of exported functions even further.
 - Integrated your set of patches and changed "swiotlb_tlb" to "swiotlb_tbl"
[v6-v7 changes:]
 - Minimized the list exported functions/variable with a prefix of: "swiotbl_tbl".
 - Made the usage of 'int dir' to be 'enum dma_data_direction'.
[v5-v6 changes:]
 - Made the exported functions/variables have the 'swiotlb_bk' prefix to be
   more uniform.
 - dropped the checkpatches/other reworks

.. and the writeup for this set set of patches:

The idea behind this set of patches is to make it possible to have separate
mechanisms for translating virtual to physical or virtual to DMA addresses
on platforms which need an SWIOTLB, and where physical != PCI bus address.

One customers of this is the pv-ops project, which can switch between
different modes of operation depending on the environment it is running in:
bare-metal or virtualized (Xen for now). Another is the Wii DMA implementation
so that the Wii USB controller can work.

On bare-metal SWIOTLB is used when there are no hardware IOMMU. In virtualized
environment it used when PCI pass-through is enabled for the guest. The problems
with PCI pass-through is that the guest's idea of PFN's is not the real thing.
To fix that, there is translation layer for PFN->machine frame number and vice-versa.
To bubble that up to the SWIOTLB layer there are two possible solutions.

One solution has been to wholesale copy the SWIOTLB, stick it in
arch/x86/xen/swiotlb.c and modify the virt_to_phys, phys_to_virt and others
to use the Xen address translation functions. Unfortunately, since the kernel can
run on bare-metal, there would be big code overlap with the real SWIOTLB.
(git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git xen/dom0/swiotlb-new)

Another approach, which this set of patches explores, is to abstract the
address translation and address determination functions away from the
SWIOTLB book-keeping functions. This way the core SWIOTLB library functions
are present in one place, while the address related functions are in
a separate library that can be loaded when running under non-bare-metal platform.

The set of patches is also accessible on:

git://git.kernel.org/pub/scm/linux/kernel/git/konrad/swiotlb-2.6.git swiotlb-0.8

 include/linux/swiotlb.h |   27 ++++++++-
 lib/swiotlb.c           |  140 ++++++++++++++++++++++++++++-------------------
 2 files changed, 108 insertions(+), 59 deletions(-)

[PATCH 1/6] swiotlb: add swiotlb_tbl_map_single library function

[Konrad Rzeszutek Wilk]

From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

swiotlb_tbl_map_single() takes the dma address of iotlb instead of
using swiotlb_virt_to_bus().

[v2: changed swiotlb_tlb to swiotlb_tbl]

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 lib/swiotlb.c |   25 +++++++++++++++++--------
 1 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 5fddf72..cb83d54 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -361,25 +361,22 @@ static void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
 	}
 }
 
-/*
- * Allocates bounce buffer and returns its kernel virtual address.
- */
-static void *
-map_single(struct device *hwdev, phys_addr_t phys, size_t size, int dir)
+void *swiotlb_tbl_map_single(struct device *hwdev, u64 tbl_dma_addr,
+			     phys_addr_t phys, size_t size, int dir)
 {
 	unsigned long flags;
 	char *dma_addr;
 	unsigned int nslots, stride, index, wrap;
 	int i;
-	unsigned long start_dma_addr;
 	unsigned long mask;
 	unsigned long offset_slots;
 	unsigned long max_slots;
 
 	mask = dma_get_seg_boundary(hwdev);
-	start_dma_addr = swiotlb_virt_to_bus(hwdev, io_tlb_start) & mask;
 
-	offset_slots = ALIGN(start_dma_addr, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT;
+	tbl_dma_addr &= mask;
+
+	offset_slots = ALIGN(tbl_dma_addr, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT;
 
 	/*
  	 * Carefully handle integer overflow which can occur when mask == ~0UL.
@@ -468,6 +465,18 @@ found:
 }
 
 /*
+ * Allocates bounce buffer and returns its kernel virtual address.
+ */
+
+static void *
+map_single(struct device *hwdev, phys_addr_t phys, size_t size, int dir)
+{
+	u64 start_dma_addr = swiotlb_virt_to_bus(hwdev, io_tlb_start);
+
+	return swiotlb_tbl_map_single(hwdev, start_dma_addr, phys, size, dir);
+}
+
+/*
  * dma_addr is the kernel virtual address of the bounce buffer to unmap.
  */
 static void
-- 
1.6.2.5

[PATCH 2/6] swiotlb: add the swiotlb initialization function with iotlb memory

[Konrad Rzeszutek Wilk]

From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

This enables the caller to initialize swiotlb with its own iotlb
memory.

[v2: changed ..with_tlb to ..with_tbl]

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 include/linux/swiotlb.h |    1 +
 lib/swiotlb.c           |   48 +++++++++++++++++++++++++++++-----------------
 2 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index febedcf..ab005a5 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -23,6 +23,7 @@ extern int swiotlb_force;
 #define IO_TLB_SHIFT 11
 
 extern void swiotlb_init(int verbose);
+extern void swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose);
 
 extern void
 *swiotlb_alloc_coherent(struct device *hwdev, size_t size,
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index cb83d54..843aa54 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -140,28 +140,14 @@ void swiotlb_print_info(void)
 	       (unsigned long long)pend);
 }
 
-/*
- * Statically reserve bounce buffer space and initialize bounce buffer data
- * structures for the software IO TLB used to implement the DMA API.
- */
-void __init
-swiotlb_init_with_default_size(size_t default_size, int verbose)
+void __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
 {
 	unsigned long i, bytes;
 
-	if (!io_tlb_nslabs) {
-		io_tlb_nslabs = (default_size >> IO_TLB_SHIFT);
-		io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
-	}
+	bytes = nslabs << IO_TLB_SHIFT;
 
-	bytes = io_tlb_nslabs << IO_TLB_SHIFT;
-
-	/*
-	 * Get IO TLB memory from the low pages
-	 */
-	io_tlb_start = alloc_bootmem_low_pages(bytes);
-	if (!io_tlb_start)
-		panic("Cannot allocate SWIOTLB buffer");
+	io_tlb_nslabs = nslabs;
+	io_tlb_start = tlb;
 	io_tlb_end = io_tlb_start + bytes;
 
 	/*
@@ -185,6 +171,32 @@ swiotlb_init_with_default_size(size_t default_size, int verbose)
 		swiotlb_print_info();
 }
 
+/*
+ * Statically reserve bounce buffer space and initialize bounce buffer data
+ * structures for the software IO TLB used to implement the DMA API.
+ */
+void __init
+swiotlb_init_with_default_size(size_t default_size, int verbose)
+{
+	unsigned long bytes;
+
+	if (!io_tlb_nslabs) {
+		io_tlb_nslabs = (default_size >> IO_TLB_SHIFT);
+		io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
+	}
+
+	bytes = io_tlb_nslabs << IO_TLB_SHIFT;
+
+	/*
+	 * Get IO TLB memory from the low pages
+	 */
+	io_tlb_start = alloc_bootmem_low_pages(bytes);
+	if (!io_tlb_start)
+		panic("Cannot allocate SWIOTLB buffer");
+
+	swiotlb_init_with_tbl(io_tlb_start, io_tlb_nslabs, verbose);
+}
+
 void __init
 swiotlb_init(int verbose)
 {
-- 
1.6.2.5

[PATCH 3/6] swiotlb: Make internal bookkeeping functions have 'swiotlb_tbl' prefix.

[Konrad Rzeszutek Wilk]

The functions that operate on io_tlb_list/io_tlb_start/io_tlb_orig_addr
have the prefix 'swiotlb_tbl' now.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 lib/swiotlb.c |   24 +++++++++++++-----------
 1 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 843aa54..1ed01fe 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -61,8 +61,8 @@ enum dma_sync_target {
 int swiotlb_force;
 
 /*
- * Used to do a quick range check in unmap_single and
- * sync_single_*, to see if the memory was in fact allocated by this
+ * Used to do a quick range check in swiotlb_tbl_unmap_single and
+ * swiotlb_tbl_sync_single_*, to see if the memory was in fact allocated by this
  * API.
  */
 static char *io_tlb_start, *io_tlb_end;
@@ -492,7 +492,8 @@ map_single(struct device *hwdev, phys_addr_t phys, size_t size, int dir)
  * dma_addr is the kernel virtual address of the bounce buffer to unmap.
  */
 static void
-do_unmap_single(struct device *hwdev, char *dma_addr, size_t size, int dir)
+swiotlb_tbl_unmap_single(struct device *hwdev, char *dma_addr, size_t size,
+			int dir)
 {
 	unsigned long flags;
 	int i, count, nslots = ALIGN(size, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT;
@@ -532,7 +533,7 @@ do_unmap_single(struct device *hwdev, char *dma_addr, size_t size, int dir)
 }
 
 static void
-sync_single(struct device *hwdev, char *dma_addr, size_t size,
+swiotlb_tbl_sync_single(struct device *hwdev, char *dma_addr, size_t size,
 	    int dir, int target)
 {
 	int index = (dma_addr - io_tlb_start) >> IO_TLB_SHIFT;
@@ -580,8 +581,8 @@ swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 	}
 	if (!ret) {
 		/*
-		 * We are either out of memory or the device can't DMA
-		 * to GFP_DMA memory; fall back on map_single(), which
+		 * We are either out of memory or the device can't DMA to
+		 * GFP_DMA memory; fall back on map_single(), which
 		 * will grab memory from the lowest available address range.
 		 */
 		ret = map_single(hwdev, 0, size, DMA_FROM_DEVICE);
@@ -599,7 +600,7 @@ swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 		       (unsigned long long)dev_addr);
 
 		/* DMA_TO_DEVICE to avoid memcpy in unmap_single */
-		do_unmap_single(hwdev, ret, size, DMA_TO_DEVICE);
+		swiotlb_tbl_unmap_single(hwdev, ret, size, DMA_TO_DEVICE);
 		return NULL;
 	}
 	*dma_handle = dev_addr;
@@ -617,8 +618,8 @@ swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
 	if (!is_swiotlb_buffer(paddr))
 		free_pages((unsigned long)vaddr, get_order(size));
 	else
-		/* DMA_TO_DEVICE to avoid memcpy in unmap_single */
-		do_unmap_single(hwdev, vaddr, size, DMA_TO_DEVICE);
+		/* DMA_TO_DEVICE to avoid memcpy in swiotlb_tbl_unmap_single */
+		swiotlb_tbl_unmap_single(hwdev, vaddr, size, DMA_TO_DEVICE);
 }
 EXPORT_SYMBOL(swiotlb_free_coherent);
 
@@ -708,7 +709,7 @@ static void unmap_single(struct device *hwdev, dma_addr_t dev_addr,
 	BUG_ON(dir == DMA_NONE);
 
 	if (is_swiotlb_buffer(paddr)) {
-		do_unmap_single(hwdev, phys_to_virt(paddr), size, dir);
+		swiotlb_tbl_unmap_single(hwdev, phys_to_virt(paddr), size, dir);
 		return;
 	}
 
@@ -751,7 +752,8 @@ swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr,
 	BUG_ON(dir == DMA_NONE);
 
 	if (is_swiotlb_buffer(paddr)) {
-		sync_single(hwdev, phys_to_virt(paddr), size, dir, target);
+		swiotlb_tbl_sync_single(hwdev, phys_to_virt(paddr), size, dir,
+				       target);
 		return;
 	}
 
-- 
1.6.2.5

[PATCH 4/6] swiotlb: search and replace "int dir" with "enum dma_data_direction dir"

[Konrad Rzeszutek Wilk]

. to catch anybody doing something funky.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 include/linux/swiotlb.h |    4 ++--
 lib/swiotlb.c           |   25 ++++++++++++++-----------
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index ab005a5..f3fc331 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -43,11 +43,11 @@ extern void swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
 
 extern int
 swiotlb_map_sg(struct device *hwdev, struct scatterlist *sg, int nents,
-	       int direction);
+	       enum dma_data_direction direction);
 
 extern void
 swiotlb_unmap_sg(struct device *hwdev, struct scatterlist *sg, int nents,
-		 int direction);
+		 enum dma_data_direction direction);
 
 extern int
 swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl, int nelems,
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 1ed01fe..a1bde96 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -374,7 +374,8 @@ static void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
 }
 
 void *swiotlb_tbl_map_single(struct device *hwdev, u64 tbl_dma_addr,
-			     phys_addr_t phys, size_t size, int dir)
+			     phys_addr_t phys, size_t size,
+			     enum dma_data_direction dir)
 {
 	unsigned long flags;
 	char *dma_addr;
@@ -481,7 +482,8 @@ found:
  */
 
 static void *
-map_single(struct device *hwdev, phys_addr_t phys, size_t size, int dir)
+map_single(struct device *hwdev, phys_addr_t phys, size_t size,
+	   enum dma_data_direction dir)
 {
 	u64 start_dma_addr = swiotlb_virt_to_bus(hwdev, io_tlb_start);
 
@@ -493,7 +495,7 @@ map_single(struct device *hwdev, phys_addr_t phys, size_t size, int dir)
  */
 static void
 swiotlb_tbl_unmap_single(struct device *hwdev, char *dma_addr, size_t size,
-			int dir)
+			enum dma_data_direction dir)
 {
 	unsigned long flags;
 	int i, count, nslots = ALIGN(size, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT;
@@ -534,7 +536,7 @@ swiotlb_tbl_unmap_single(struct device *hwdev, char *dma_addr, size_t size,
 
 static void
 swiotlb_tbl_sync_single(struct device *hwdev, char *dma_addr, size_t size,
-	    int dir, int target)
+	    enum dma_data_direction dir, int target)
 {
 	int index = (dma_addr - io_tlb_start) >> IO_TLB_SHIFT;
 	phys_addr_t phys = io_tlb_orig_addr[index];
@@ -624,7 +626,8 @@ swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
 EXPORT_SYMBOL(swiotlb_free_coherent);
 
 static void
-swiotlb_full(struct device *dev, size_t size, int dir, int do_panic)
+swiotlb_full(struct device *dev, size_t size, enum dma_data_direction dir,
+	     int do_panic)
 {
 	/*
 	 * Ran out of IOMMU space for this operation. This is very bad.
@@ -702,7 +705,7 @@ EXPORT_SYMBOL_GPL(swiotlb_map_page);
  * whatever the device wrote there.
  */
 static void unmap_single(struct device *hwdev, dma_addr_t dev_addr,
-			 size_t size, int dir)
+			 size_t size, enum dma_data_direction dir)
 {
 	phys_addr_t paddr = dma_to_phys(hwdev, dev_addr);
 
@@ -745,7 +748,7 @@ EXPORT_SYMBOL_GPL(swiotlb_unmap_page);
  */
 static void
 swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr,
-		    size_t size, int dir, int target)
+		    size_t size, enum dma_data_direction dir, int target)
 {
 	phys_addr_t paddr = dma_to_phys(hwdev, dev_addr);
 
@@ -785,7 +788,7 @@ EXPORT_SYMBOL(swiotlb_sync_single_for_device);
 static void
 swiotlb_sync_single_range(struct device *hwdev, dma_addr_t dev_addr,
 			  unsigned long offset, size_t size,
-			  int dir, int target)
+			  enum dma_data_direction dir, int target)
 {
 	swiotlb_sync_single(hwdev, dev_addr + offset, size, dir, target);
 }
@@ -863,7 +866,7 @@ EXPORT_SYMBOL(swiotlb_map_sg_attrs);
 
 int
 swiotlb_map_sg(struct device *hwdev, struct scatterlist *sgl, int nelems,
-	       int dir)
+	       enum dma_data_direction dir)
 {
 	return swiotlb_map_sg_attrs(hwdev, sgl, nelems, dir, NULL);
 }
@@ -890,7 +893,7 @@ EXPORT_SYMBOL(swiotlb_unmap_sg_attrs);
 
 void
 swiotlb_unmap_sg(struct device *hwdev, struct scatterlist *sgl, int nelems,
-		 int dir)
+		 enum dma_data_direction dir)
 {
 	return swiotlb_unmap_sg_attrs(hwdev, sgl, nelems, dir, NULL);
 }
@@ -905,7 +908,7 @@ EXPORT_SYMBOL(swiotlb_unmap_sg);
  */
 static void
 swiotlb_sync_sg(struct device *hwdev, struct scatterlist *sgl,
-		int nelems, int dir, int target)
+		int nelems, enum dma_data_direction dir, int target)
 {
 	struct scatterlist *sg;
 	int i;
-- 
1.6.2.5

[PATCH 5/6] swiotlb: Make swiotlb bookkeeping functions visible in the header file.

[Konrad Rzeszutek Wilk]

We put the functions dealing with the operations on
the SWIOTLB buffer in the header and make those functions non-static.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 include/linux/swiotlb.h |   22 ++++++++++++++++++++++
 lib/swiotlb.c           |   28 ++++++++++++----------------
 2 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index f3fc331..dabfd0b 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -25,6 +25,28 @@ extern int swiotlb_force;
 extern void swiotlb_init(int verbose);
 extern void swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose);
 
+/*
+ * Enumeration for sync targets
+ */
+enum dma_sync_target {
+	SYNC_FOR_CPU = 0,
+	SYNC_FOR_DEVICE = 1,
+};
+extern void *swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t phys,
+				    u64 tbl_dma_addr, size_t size,
+				    enum dma_data_direction dir);
+
+extern void swiotlb_tbl_unmap_single(struct device *hwdev, char *dma_addr,
+				     size_t size, enum dma_data_direction dir);
+
+extern void swiotlb_tbl_sync_single(struct device *hwdev, char *dma_addr,
+				    size_t size, enum dma_data_direction dir,
+				    enum dma_sync_target target);
+
+/* Accessory function. */
+extern void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
+			   enum dma_data_direction dir);
+
 extern void
 *swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 			dma_addr_t *dma_handle, gfp_t flags);
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index a1bde96..dc1776c 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -50,14 +50,6 @@
  */
 #define IO_TLB_MIN_SLABS ((1<<20) >> IO_TLB_SHIFT)
 
-/*
- * Enumeration for sync targets
- */
-enum dma_sync_target {
-	SYNC_FOR_CPU = 0,
-	SYNC_FOR_DEVICE = 1,
-};
-
 int swiotlb_force;
 
 /*
@@ -335,8 +327,8 @@ static int is_swiotlb_buffer(phys_addr_t paddr)
 /*
  * Bounce: copy the swiotlb buffer back to the original dma location
  */
-static void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
-			   enum dma_data_direction dir)
+void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
+		    enum dma_data_direction dir)
 {
 	unsigned long pfn = PFN_DOWN(phys);
 
@@ -493,7 +485,7 @@ map_single(struct device *hwdev, phys_addr_t phys, size_t size,
 /*
  * dma_addr is the kernel virtual address of the bounce buffer to unmap.
  */
-static void
+void
 swiotlb_tbl_unmap_single(struct device *hwdev, char *dma_addr, size_t size,
 			enum dma_data_direction dir)
 {
@@ -534,9 +526,10 @@ swiotlb_tbl_unmap_single(struct device *hwdev, char *dma_addr, size_t size,
 	spin_unlock_irqrestore(&io_tlb_lock, flags);
 }
 
-static void
+void
 swiotlb_tbl_sync_single(struct device *hwdev, char *dma_addr, size_t size,
-	    enum dma_data_direction dir, int target)
+			enum dma_data_direction dir,
+			enum dma_sync_target target)
 {
 	int index = (dma_addr - io_tlb_start) >> IO_TLB_SHIFT;
 	phys_addr_t phys = io_tlb_orig_addr[index];
@@ -748,7 +741,8 @@ EXPORT_SYMBOL_GPL(swiotlb_unmap_page);
  */
 static void
 swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr,
-		    size_t size, enum dma_data_direction dir, int target)
+		    size_t size, enum dma_data_direction dir,
+		    enum dma_sync_target target)
 {
 	phys_addr_t paddr = dma_to_phys(hwdev, dev_addr);
 
@@ -788,7 +782,8 @@ EXPORT_SYMBOL(swiotlb_sync_single_for_device);
 static void
 swiotlb_sync_single_range(struct device *hwdev, dma_addr_t dev_addr,
 			  unsigned long offset, size_t size,
-			  enum dma_data_direction dir, int target)
+			  enum dma_data_direction dir,
+			  enum dma_sync_target target)
 {
 	swiotlb_sync_single(hwdev, dev_addr + offset, size, dir, target);
 }
@@ -908,7 +903,8 @@ EXPORT_SYMBOL(swiotlb_unmap_sg);
  */
 static void
 swiotlb_sync_sg(struct device *hwdev, struct scatterlist *sgl,
-		int nelems, enum dma_data_direction dir, int target)
+		int nelems, enum dma_data_direction dir,
+		enum dma_sync_target target)
 {
 	struct scatterlist *sg;
 	int i;
-- 
1.6.2.5

[PATCH 6/6] swiotlb: EXPORT_SYMBOL_GPL functions + variables that are defined in the header file.

[Konrad Rzeszutek Wilk]

Make the functions and variables that are now declared in the
swiotlb.h header file visible by the linker.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 lib/swiotlb.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index dc1776c..986742c 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -364,6 +364,7 @@ void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
 			memcpy(phys_to_virt(phys), dma_addr, size);
 	}
 }
+EXPORT_SYMBOL_GPL(swiotlb_bounce);
 
 void *swiotlb_tbl_map_single(struct device *hwdev, u64 tbl_dma_addr,
 			     phys_addr_t phys, size_t size,
@@ -468,6 +469,7 @@ found:
 
 	return dma_addr;
 }
+EXPORT_SYMBOL_GPL(swiotlb_tbl_map_single);
 
 /*
  * Allocates bounce buffer and returns its kernel virtual address.
@@ -525,6 +527,7 @@ swiotlb_tbl_unmap_single(struct device *hwdev, char *dma_addr, size_t size,
 	}
 	spin_unlock_irqrestore(&io_tlb_lock, flags);
 }
+EXPORT_SYMBOL_GPL(swiotlb_tbl_unmap_single);
 
 void
 swiotlb_tbl_sync_single(struct device *hwdev, char *dma_addr, size_t size,
@@ -553,6 +556,7 @@ swiotlb_tbl_sync_single(struct device *hwdev, char *dma_addr, size_t size,
 		BUG();
 	}
 }
+EXPORT_SYMBOL_GPL(swiotlb_tbl_sync_single);
 
 void *
 swiotlb_alloc_coherent(struct device *hwdev, size_t size,
-- 
1.6.2.5

Re: [PATCH 5/6] swiotlb: Make swiotlb bookkeeping functions visible in the header file.

[Albert Herranz]

Konrad Rzeszutek Wilk wrote:
> We put the functions dealing with the operations on
> the SWIOTLB buffer in the header and make those functions non-static.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  include/linux/swiotlb.h |   22 ++++++++++++++++++++++
>  lib/swiotlb.c           |   28 ++++++++++++----------------
>  2 files changed, 34 insertions(+), 16 deletions(-)
> 
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index f3fc331..dabfd0b 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -25,6 +25,28 @@ extern int swiotlb_force;
>  extern void swiotlb_init(int verbose);
>  extern void swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose);
>  
> +/*
> + * Enumeration for sync targets
> + */
> +enum dma_sync_target {
> +	SYNC_FOR_CPU = 0,
> +	SYNC_FOR_DEVICE = 1,
> +};
> +extern void *swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t phys,
> +				    u64 tbl_dma_addr, size_t size,
> +				    enum dma_data_direction dir);
> +

The phys and tbl_dma_addr arguments in the function prototype are swapped compared to the function definition in patch 1/6.

Cheers,
Albert

Re: [PATCH 5/6] swiotlb: Make swiotlb bookkeeping functions visible in the header file.

[Jeremy Fitzhardinge]

On 05/11/2010 11:28 AM, Albert Herranz wrote:
> Konrad Rzeszutek Wilk wrote:
>   
>> +extern void *swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t phys,
>> +				    u64 tbl_dma_addr, size_t size,
>> +				    enum dma_data_direction dir);
>> +
>>     
> The phys and tbl_dma_addr arguments in the function prototype are swapped compared to the function definition in patch 1/6.
>   

Good catch.  I can see someone beating their head bloody over that...

    J

Re: [PATCH 5/6] swiotlb: Make swiotlb bookkeeping functions visible in the header file.

[Konrad Rzeszutek Wilk]

> > +extern void *swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t phys,
> > +				    u64 tbl_dma_addr, size_t size,
> > +				    enum dma_data_direction dir);
> > +
> 
> The phys and tbl_dma_addr arguments in the function prototype are swapped compared to the function definition in patch 1/6.

Duh! Thanks for spotting that. Here is a repost of this patch (I've 
updated the git tree with the one below):


>From 2d1fddb566f04e14313227bbc5f17b90db295c98 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Mon, 10 May 2010 16:20:57 -0400
Subject: [PATCH 5/6] swiotlb: Make swiotlb bookkeeping functions visible in the header file.

We put the functions dealing with the operations on
the SWIOTLB buffer in the header and make those functions non-static.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 include/linux/swiotlb.h |   22 ++++++++++++++++++++++
 lib/swiotlb.c           |   28 ++++++++++++----------------
 2 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index f3fc331..f7753d8 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -25,6 +25,28 @@ extern int swiotlb_force;
 extern void swiotlb_init(int verbose);
 extern void swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose);
 
+/*
+ * Enumeration for sync targets
+ */
+enum dma_sync_target {
+	SYNC_FOR_CPU = 0,
+	SYNC_FOR_DEVICE = 1,
+};
+extern void *swiotlb_tbl_map_single(struct device *hwdev, u64 tbl_dma_addr,
+				    phys_addr_t phys, size_t size,
+				    enum dma_data_direction dir);
+
+extern void swiotlb_tbl_unmap_single(struct device *hwdev, char *dma_addr,
+				     size_t size, enum dma_data_direction dir);
+
+extern void swiotlb_tbl_sync_single(struct device *hwdev, char *dma_addr,
+				    size_t size, enum dma_data_direction dir,
+				    enum dma_sync_target target);
+
+/* Accessory functions. */
+extern void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
+			   enum dma_data_direction dir);
+
 extern void
 *swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 			dma_addr_t *dma_handle, gfp_t flags);
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index a1bde96..dc1776c 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -50,14 +50,6 @@
  */
 #define IO_TLB_MIN_SLABS ((1<<20) >> IO_TLB_SHIFT)
 
-/*
- * Enumeration for sync targets
- */
-enum dma_sync_target {
-	SYNC_FOR_CPU = 0,
-	SYNC_FOR_DEVICE = 1,
-};
-
 int swiotlb_force;
 
 /*
@@ -335,8 +327,8 @@ static int is_swiotlb_buffer(phys_addr_t paddr)
 /*
  * Bounce: copy the swiotlb buffer back to the original dma location
  */
-static void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
-			   enum dma_data_direction dir)
+void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
+		    enum dma_data_direction dir)
 {
 	unsigned long pfn = PFN_DOWN(phys);
 
@@ -493,7 +485,7 @@ map_single(struct device *hwdev, phys_addr_t phys, size_t size,
 /*
  * dma_addr is the kernel virtual address of the bounce buffer to unmap.
  */
-static void
+void
 swiotlb_tbl_unmap_single(struct device *hwdev, char *dma_addr, size_t size,
 			enum dma_data_direction dir)
 {
@@ -534,9 +526,10 @@ swiotlb_tbl_unmap_single(struct device *hwdev, char *dma_addr, size_t size,
 	spin_unlock_irqrestore(&io_tlb_lock, flags);
 }
 
-static void
+void
 swiotlb_tbl_sync_single(struct device *hwdev, char *dma_addr, size_t size,
-	    enum dma_data_direction dir, int target)
+			enum dma_data_direction dir,
+			enum dma_sync_target target)
 {
 	int index = (dma_addr - io_tlb_start) >> IO_TLB_SHIFT;
 	phys_addr_t phys = io_tlb_orig_addr[index];
@@ -748,7 +741,8 @@ EXPORT_SYMBOL_GPL(swiotlb_unmap_page);
  */
 static void
 swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr,
-		    size_t size, enum dma_data_direction dir, int target)
+		    size_t size, enum dma_data_direction dir,
+		    enum dma_sync_target target)
 {
 	phys_addr_t paddr = dma_to_phys(hwdev, dev_addr);
 
@@ -788,7 +782,8 @@ EXPORT_SYMBOL(swiotlb_sync_single_for_device);
 static void
 swiotlb_sync_single_range(struct device *hwdev, dma_addr_t dev_addr,
 			  unsigned long offset, size_t size,
-			  enum dma_data_direction dir, int target)
+			  enum dma_data_direction dir,
+			  enum dma_sync_target target)
 {
 	swiotlb_sync_single(hwdev, dev_addr + offset, size, dir, target);
 }
@@ -908,7 +903,8 @@ EXPORT_SYMBOL(swiotlb_unmap_sg);
  */
 static void
 swiotlb_sync_sg(struct device *hwdev, struct scatterlist *sgl,
-		int nelems, enum dma_data_direction dir, int target)
+		int nelems, enum dma_data_direction dir,
+		enum dma_sync_target target)
 {
 	struct scatterlist *sg;
 	int i;
-- 
1.6.2.5

Re: [PATCH 5/6] swiotlb: Make swiotlb bookkeeping functions visible in the header file.

[Albert Herranz]

Konrad Rzeszutek Wilk wrote:
>>> +extern void *swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t phys,
>>> +				    u64 tbl_dma_addr, size_t size,
>>> +				    enum dma_data_direction dir);
>>> +
>> The phys and tbl_dma_addr arguments in the function prototype are swapped compared to the function definition in patch 1/6.
> 
> Duh! Thanks for spotting that. Here is a repost of this patch (I've 
> updated the git tree with the one below):
> 

Thanks. That was fast :)

One more thing. Shouldn't be more appropriate to make tbl_dma_addr a dma_addr_t instead of a u64?
For example, in my case I'm currently using the swiotlb code to overcome some DMA limitations in a 32-bit PowerPC platform.
In this scenario the dma_addr_t type is defined either as a u64 for 64-bit PowerPC or as a u32 for 32-bit PowerPC.

Cheers,
Albert

Re: [PATCH 5/6] swiotlb: Make swiotlb bookkeeping functions visible in the header file.

[Konrad Rzeszutek Wilk]

On Tue, May 11, 2010 at 09:01:29PM +0200, Albert Herranz wrote:
> Konrad Rzeszutek Wilk wrote:
> >>> +extern void *swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t phys,
> >>> +				    u64 tbl_dma_addr, size_t size,
> >>> +				    enum dma_data_direction dir);
> >>> +
> >> The phys and tbl_dma_addr arguments in the function prototype are swapped compared to the function definition in patch 1/6.
> > 
> > Duh! Thanks for spotting that. Here is a repost of this patch (I've 
> > updated the git tree with the one below):
> > 
> 
> Thanks. That was fast :)
> 
> One more thing. Shouldn't be more appropriate to make tbl_dma_addr a dma_addr_t instead of a u64?
> For example, in my case I'm currently using the swiotlb code to overcome some DMA limitations in a 32-bit PowerPC platform.
> In this scenario the dma_addr_t type is defined either as a u64 for 64-bit PowerPC or as a u32 for 32-bit PowerPC.

Good point.

So these two patches (one to replace the PATCH 1/6 and the other to
replace this one), would work then:

I've put the whole tree at:
git://git.kernel.org/pub/scm/linux/kernel/git/konrad/swiotlb-2.6.git swiotlb-0.8.1

>From 17ef8495231cef0529a2d9763bc7ce73e8860da2 Mon Sep 17 00:00:00 2001
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Date: Mon, 10 May 2010 15:14:54 -0400
Subject: [PATCH 1/6] swiotlb: add swiotlb_tbl_map_single library function

swiotlb_tbl_map_single() takes the dma address of iotlb instead of
using swiotlb_virt_to_bus().

[v2: changed swiotlb_tlb to swiotlb_tbl]
[v3: changed u64 to dma_addr_t]

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 lib/swiotlb.c |   25 +++++++++++++++++--------
 1 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 5fddf72..78320d7 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -361,25 +361,22 @@ static void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
 	}
 }
 
-/*
- * Allocates bounce buffer and returns its kernel virtual address.
- */
-static void *
-map_single(struct device *hwdev, phys_addr_t phys, size_t size, int dir)
+void *swiotlb_tbl_map_single(struct device *hwdev, dma_addr_t tbl_dma_addr,
+			     phys_addr_t phys, size_t size, int dir)
 {
 	unsigned long flags;
 	char *dma_addr;
 	unsigned int nslots, stride, index, wrap;
 	int i;
-	unsigned long start_dma_addr;
 	unsigned long mask;
 	unsigned long offset_slots;
 	unsigned long max_slots;
 
 	mask = dma_get_seg_boundary(hwdev);
-	start_dma_addr = swiotlb_virt_to_bus(hwdev, io_tlb_start) & mask;
 
-	offset_slots = ALIGN(start_dma_addr, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT;
+	tbl_dma_addr &= mask;
+
+	offset_slots = ALIGN(tbl_dma_addr, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT;
 
 	/*
  	 * Carefully handle integer overflow which can occur when mask == ~0UL.
@@ -468,6 +465,18 @@ found:
 }
 
 /*
+ * Allocates bounce buffer and returns its kernel virtual address.
+ */
+
+static void *
+map_single(struct device *hwdev, phys_addr_t phys, size_t size, int dir)
+{
+	dma_addr_t start_dma_addr = swiotlb_virt_to_bus(hwdev, io_tlb_start);
+
+	return swiotlb_tbl_map_single(hwdev, start_dma_addr, phys, size, dir);
+}
+
+/*
  * dma_addr is the kernel virtual address of the bounce buffer to unmap.
  */
 static void
-- 
1.6.2.5



and:


>From baddb085f29785b3029ec83244b2eda7d23a9d3b Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Mon, 10 May 2010 16:20:57 -0400
Subject: [PATCH 5/6] swiotlb: Make swiotlb bookkeeping functions visible in the header file.

We put the functions dealing with the operations on
the SWIOTLB buffer in the header and make those functions non-static.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 include/linux/swiotlb.h |   22 ++++++++++++++++++++++
 lib/swiotlb.c           |   28 ++++++++++++----------------
 2 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index f3fc331..c40bb2a 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -25,6 +25,28 @@ extern int swiotlb_force;
 extern void swiotlb_init(int verbose);
 extern void swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose);
 
+/*
+ * Enumeration for sync targets
+ */
+enum dma_sync_target {
+	SYNC_FOR_CPU = 0,
+	SYNC_FOR_DEVICE = 1,
+};
+extern void *swiotlb_tbl_map_single(struct device *hwdev, dma_addr_t tbl_dma_addr,
+				    phys_addr_t phys, size_t size,
+				    enum dma_data_direction dir);
+
+extern void swiotlb_tbl_unmap_single(struct device *hwdev, char *dma_addr,
+				     size_t size, enum dma_data_direction dir);
+
+extern void swiotlb_tbl_sync_single(struct device *hwdev, char *dma_addr,
+				    size_t size, enum dma_data_direction dir,
+				    enum dma_sync_target target);
+
+/* Accessory functions. */
+extern void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
+			   enum dma_data_direction dir);
+
 extern void
 *swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 			dma_addr_t *dma_handle, gfp_t flags);
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 7eed4c9..7b4a41a 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -50,14 +50,6 @@
  */
 #define IO_TLB_MIN_SLABS ((1<<20) >> IO_TLB_SHIFT)
 
-/*
- * Enumeration for sync targets
- */
-enum dma_sync_target {
-	SYNC_FOR_CPU = 0,
-	SYNC_FOR_DEVICE = 1,
-};
-
 int swiotlb_force;
 
 /*
@@ -335,8 +327,8 @@ static int is_swiotlb_buffer(phys_addr_t paddr)
 /*
  * Bounce: copy the swiotlb buffer back to the original dma location
  */
-static void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
-			   enum dma_data_direction dir)
+void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
+		    enum dma_data_direction dir)
 {
 	unsigned long pfn = PFN_DOWN(phys);
 
@@ -493,7 +485,7 @@ map_single(struct device *hwdev, phys_addr_t phys, size_t size,
 /*
  * dma_addr is the kernel virtual address of the bounce buffer to unmap.
  */
-static void
+void
 swiotlb_tbl_unmap_single(struct device *hwdev, char *dma_addr, size_t size,
 			enum dma_data_direction dir)
 {
@@ -534,9 +526,10 @@ swiotlb_tbl_unmap_single(struct device *hwdev, char *dma_addr, size_t size,
 	spin_unlock_irqrestore(&io_tlb_lock, flags);
 }
 
-static void
+void
 swiotlb_tbl_sync_single(struct device *hwdev, char *dma_addr, size_t size,
-	    enum dma_data_direction dir, int target)
+			enum dma_data_direction dir,
+			enum dma_sync_target target)
 {
 	int index = (dma_addr - io_tlb_start) >> IO_TLB_SHIFT;
 	phys_addr_t phys = io_tlb_orig_addr[index];
@@ -748,7 +741,8 @@ EXPORT_SYMBOL_GPL(swiotlb_unmap_page);
  */
 static void
 swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr,
-		    size_t size, enum dma_data_direction dir, int target)
+		    size_t size, enum dma_data_direction dir,
+		    enum dma_sync_target target)
 {
 	phys_addr_t paddr = dma_to_phys(hwdev, dev_addr);
 
@@ -788,7 +782,8 @@ EXPORT_SYMBOL(swiotlb_sync_single_for_device);
 static void
 swiotlb_sync_single_range(struct device *hwdev, dma_addr_t dev_addr,
 			  unsigned long offset, size_t size,
-			  enum dma_data_direction dir, int target)
+			  enum dma_data_direction dir,
+			  enum dma_sync_target target)
 {
 	swiotlb_sync_single(hwdev, dev_addr + offset, size, dir, target);
 }
@@ -908,7 +903,8 @@ EXPORT_SYMBOL(swiotlb_unmap_sg);
  */
 static void
 swiotlb_sync_sg(struct device *hwdev, struct scatterlist *sgl,
-		int nelems, enum dma_data_direction dir, int target)
+		int nelems, enum dma_data_direction dir,
+		enum dma_sync_target target)
 {
 	struct scatterlist *sg;
 	int i;
-- 
1.6.2.5

> 
> Cheers,
> Albert
> 
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/iommu

Re: [PATCH 5/6] swiotlb: Make swiotlb bookkeeping functions visible in the header file.

[Albert Herranz]

Konrad Rzeszutek Wilk wrote:
> So these two patches (one to replace the PATCH 1/6 and the other to
> replace this one), would work then:
> 
> I've put the whole tree at:
> git://git.kernel.org/pub/scm/linux/kernel/git/konrad/swiotlb-2.6.git swiotlb-0.8.1
> 

The whole series work fine on the Wii 32-bit PowerPC platform (used to implement the MEM2 DMA facility needed by its EHCI controller).

Tested-by: Albert Herranz <albert_herranz@yahoo.es>

Thanks,
Albert

Re: [PATCH] swiotlb v0.8: seperation of physical/virtual address translation

[FUJITA Tomonori]

On Tue, 11 May 2010 11:39:07 -0400
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:

> git://git.kernel.org/pub/scm/linux/kernel/git/konrad/swiotlb-2.6.git swiotlb-0.8
> 
>  include/linux/swiotlb.h |   27 ++++++++-
>  lib/swiotlb.c           |  140 ++++++++++++++++++++++++++++-------------------
>  2 files changed, 108 insertions(+), 59 deletions(-)

I think that the 5th and 6th should be fold. But the rest (with the
following fixes) looks fine to me. I'm not sure how you send this to
upstream, however, seems that the patchset conflicts with the swiotlb
changes in -mm.

Re: [PATCH 5/6] swiotlb: Make swiotlb bookkeeping functions visible in the header file.

[FUJITA Tomonori]

On Thu, 13 May 2010 07:04:11 +0200
Albert Herranz <albert_herranz@yahoo.es> wrote:

> Konrad Rzeszutek Wilk wrote:
> > So these two patches (one to replace the PATCH 1/6 and the other to
> > replace this one), would work then:
> > 
> > I've put the whole tree at:
> > git://git.kernel.org/pub/scm/linux/kernel/git/konrad/swiotlb-2.6.git swiotlb-0.8.1
> > 
> 
> The whole series work fine on the Wii 32-bit PowerPC platform (used to implement the MEM2 DMA facility needed by its EHCI controller).
> 
> Tested-by: Albert Herranz <albert_herranz@yahoo.es>

I know that you decrease the swiotlb size from 64MB (default) to 1MB,
however, pre-allocating 1MB is too wasteful to Wii?

Wii's EHCI controller connects to storage devices? If not, what you
need is the facility to do bouncing with dynamically allocated memory
such as the network stack, the block layer and
arm/arm/common/dmabounce.c

I think that we should have the common facility to do such.

Re: [PATCH 5/6] swiotlb: Make swiotlb bookkeeping functions visible in the header file.

[Albert Herranz]

Hi,

On 05/18/2010 05:28 AM, FUJITA Tomonori wrote:
>> The whole series work fine on the Wii 32-bit PowerPC platform (used to implement the MEM2 DMA facility needed by its EHCI controller).
>>
>> Tested-by: Albert Herranz <albert_herranz@yahoo.es>
> 
> I know that you decrease the swiotlb size from 64MB (default) to 1MB,
> however, pre-allocating 1MB is too wasteful to Wii?
> 

Every single KB counts on the Wii. It has just 24MB of MEM1 and 64MB of MEM2 (discontiguous memory ranges).
I'm using 1MB for the SWIOTLB for now, but of course that can be further tweaked down.

> Wii's EHCI controller connects to storage devices? If not, what you
> need is the facility to do bouncing with dynamically allocated memory
> such as the network stack, the block layer and
> arm/arm/common/dmabounce.c
> 

The two external USB ports in the Wii are part of an embedded EHCI controller (with its two OHCI companion controllers). You can connect whatever USB device you want to them.

I posted (in the past) a patch series [1] in which I made the dmabounce code in the ARM architecture tree available to other architectures, and used that to implement the needed bouncing infrastructure.
But I was told then by Russell to use swiotlb instead [2].

Either if dmabounce or swiotlb is used, what's needed in the end is a way to allocate coherent buffers from a specific part of memory (MEM2) and to bounce buffers to/from MEM2 as needed.
This is currently solved by using struct dma_map_ops + swiotlb as a bounce buffer implementation, placing the swiotlb area in MEM2.
Coherent memory is allocated from a dedicated pool of MEM2 (currently using the generic per-device dma coherent allocator).

Cheers,
Albert

[1] http://marc.info/?l=linux-usb&m=126736610418835
[2] http://marc.info/?l=linux-usb&m=126736687519788

Re: [PATCH 5/6] swiotlb: Make swiotlb bookkeeping functions visible in the header file.

[FUJITA Tomonori]

On Tue, 18 May 2010 18:52:17 +0200
Albert Herranz <albert_herranz@yahoo.es> wrote:

> Hi,
> 
> On 05/18/2010 05:28 AM, FUJITA Tomonori wrote:
> >> The whole series work fine on the Wii 32-bit PowerPC platform (used to implement the MEM2 DMA facility needed by its EHCI controller).
> >>
> >> Tested-by: Albert Herranz <albert_herranz@yahoo.es>
> > 
> > I know that you decrease the swiotlb size from 64MB (default) to 1MB,
> > however, pre-allocating 1MB is too wasteful to Wii?
> > 
> 
> Every single KB counts on the Wii. It has just 24MB of MEM1 and 64MB of MEM2 (discontiguous memory ranges).
> I'm using 1MB for the SWIOTLB for now, but of course that can be further tweaked down.

You can decrease the swiotlb memory however you can't fix the root
problems of swiotlb:

- it needs pre-allocated memory
- it can't handle the out-of-pre-allocated memory situation.


> > Wii's EHCI controller connects to storage devices? If not, what you
> > need is the facility to do bouncing with dynamically allocated memory
> > such as the network stack, the block layer and
> > arm/arm/common/dmabounce.c
> > 
> 
> The two external USB ports in the Wii are part of an embedded EHCI controller (with its two OHCI companion controllers). You can connect whatever USB device you want to them.

I mean that Wii doesn't boot on the root device on the USB controller,
right?

> I posted (in the past) a patch series [1] in which I made the
> dmabounce code in the ARM architecture tree available to other
> architectures, and used that to implement the needed bouncing
> infrastructure.  But I was told then by Russell to use swiotlb
> instead [2].

I think that we need to modify swiotlb for embedded archs. Otherwise,
I don't think that swiotlb can replace arm's dmabounce.

Ok, I'll implement something like that.

Re: [PATCH 5/6] swiotlb: Make swiotlb bookkeeping functions visible in the header file.

[Albert Herranz]

On 05/19/2010 05:34 AM, FUJITA Tomonori wrote:
>> Every single KB counts on the Wii. It has just 24MB of MEM1 and 64MB of MEM2 (discontiguous memory ranges).
>> I'm using 1MB for the SWIOTLB for now, but of course that can be further tweaked down.
> 
> You can decrease the swiotlb memory however you can't fix the root
> problems of swiotlb:
> 
> - it needs pre-allocated memory
> - it can't handle the out-of-pre-allocated memory situation.
> 

Yes, agreed.
Although having a dedicated pool _may_ be an advantage on a memory constrained system: it can guarantee swiotlb allocations (as long as the pool has been properly sized) even when the system is using up all memory.

> I mean that Wii doesn't boot on the root device on the USB controller,
> right?
> 

The current "bootloaders" boot images from the external SD card.
But Linux itself can have its root device on a USB storage device.

>> I posted (in the past) a patch series [1] in which I made the
>> dmabounce code in the ARM architecture tree available to other
>> architectures, and used that to implement the needed bouncing
>> infrastructure.  But I was told then by Russell to use swiotlb
>> instead [2].
> 
> I think that we need to modify swiotlb for embedded archs. Otherwise,
> I don't think that swiotlb can replace arm's dmabounce.
> 
> Ok, I'll implement something like that.
> 

Thanks. Looking forward to it :)

Cheers,
Albert

Re: [PATCH 5/6] swiotlb: Make swiotlb bookkeeping functions visible in the header file.

[Russell King - ARM Linux]

On Wed, May 19, 2010 at 12:34:18PM +0900, FUJITA Tomonori wrote:
> On Tue, 18 May 2010 18:52:17 +0200
> Albert Herranz <albert_herranz@yahoo.es> wrote:
> > Every single KB counts on the Wii. It has just 24MB of MEM1 and 64MB of MEM2 (discontiguous memory ranges).
> > I'm using 1MB for the SWIOTLB for now, but of course that can be further tweaked down.
> 
> You can decrease the swiotlb memory however you can't fix the root
> problems of swiotlb:
> 
> - it needs pre-allocated memory
> - it can't handle the out-of-pre-allocated memory situation.

That's actually the advantage of swiotlb over dmabounce.  dmabounce's
dynamic allocation results in OOMs under some IO activity patterns.
dmabounce also produces the occasional WARN_ON() from the dma coherent
allocator depending on the driver.

We really need to kill off dmabounce.

Re: [PATCH 5/6] swiotlb: Make swiotlb bookkeeping functions visible in the header file.

[FUJITA Tomonori]

On Wed, 19 May 2010 08:10:57 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Wed, May 19, 2010 at 12:34:18PM +0900, FUJITA Tomonori wrote:
> > On Tue, 18 May 2010 18:52:17 +0200
> > Albert Herranz <albert_herranz@yahoo.es> wrote:
> > > Every single KB counts on the Wii. It has just 24MB of MEM1 and 64MB of MEM2 (discontiguous memory ranges).
> > > I'm using 1MB for the SWIOTLB for now, but of course that can be further tweaked down.
> > 
> > You can decrease the swiotlb memory however you can't fix the root
> > problems of swiotlb:
> > 
> > - it needs pre-allocated memory
> > - it can't handle the out-of-pre-allocated memory situation.
> 
> That's actually the advantage of swiotlb over dmabounce.  dmabounce's
> dynamic allocation results in OOMs under some IO activity patterns.
> dmabounce also produces the occasional WARN_ON() from the dma coherent
> allocator depending on the driver.

However, we can't guarantee that allocation always succeeds in any IO
patters. And the I/O subsystems and drivers can handle the dma
allocation failure nicely. With SCSI, commands that hit the allocation
failure are re-queued. Nothing bad happens. We shouldn't use WARN_ON
in the dma allocation failure path.

For me, the problems are:

- swiotlb could waste too much pre-allocated memory if there are not
  much IO activities.

- Allocation failure could happen even if the system has lots of free
  memory because swiotlb use only pre-allocated memory.

> We really need to kill off dmabounce.

Agreed. What changes to swiotlb are necessary to replace dmabounce?
Needs to use coherent memory (I'm not sure why dmabounce uses coherent
memory to dma_map_single though)? I'll try to take care of them.

Re: [PATCH] swiotlb v0.8: seperation of physical/virtual address translation

[Konrad Rzeszutek Wilk]

On Mon, May 17, 2010 at 06:48:35PM +0900, FUJITA Tomonori wrote:
> On Tue, 11 May 2010 11:39:07 -0400
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> 
> > git://git.kernel.org/pub/scm/linux/kernel/git/konrad/swiotlb-2.6.git swiotlb-0.8
> > 
> >  include/linux/swiotlb.h |   27 ++++++++-
> >  lib/swiotlb.c           |  140 ++++++++++++++++++++++++++++-------------------
> >  2 files changed, 108 insertions(+), 59 deletions(-)
> 
> I think that the 5th and 6th should be fold. But the rest (with the

Will do.
> following fixes) looks fine to me. I'm not sure how you send this to

Thank you for patience and taking the time to review them.
Sorry about answering so late - just got back from vacation.

> upstream, however, seems that the patchset conflicts with the swiotlb
> changes in -mm.

Oh, let me investigate and have a patch-set ready against -mm as well.

Re: [PATCH] swiotlb v0.8: seperation of physical/virtual address translation

[Konrad Rzeszutek Wilk]

> > I think that the 5th and 6th should be fold. But the rest (with the
> 
> Will do.
Done.
> > following fixes) looks fine to me. I'm not sure how you send this to
> 
> Thank you for patience and taking the time to review them.
> Sorry about answering so late - just got back from vacation.
> 
> > upstream, however, seems that the patchset conflicts with the swiotlb
> > changes in -mm.
> 
> Oh, let me investigate and have a patch-set ready against -mm as well.

Fixed it. It was conflicting due to the commits you posted some time ago
for sync_single_range_for_* . I fixed that up and put up all of
those patches along with your Acked-by and Albert's Tested-by on:

 git://git.kernel.org/pub/scm/linux/kernel/git/konrad/swiotlb-2.6.git swiotlb-0.8.2

Tested on baremetal, will soon rebase the Xen ones but I don't
anticipate much trouble.

Was thinking to post here on LKML next week and to akpm.