[PATCH V3 0/7] swiotlb: changes for powerpc/highmem

[PATCH V3 0/7] swiotlb: changes for powerpc/highmem

[Kumar Gala]

This is a repost of Becky's patches from the v2 version.  The only change
is to incorporate feedback on "Create swiotlb_unmap_single" an to add
Ack by FUJITA Tomonori for patches that have been reviewed.

- k

[PATCH 1/7] swiotlb: comment corrections (no code changes)

[Kumar Gala]

From: Becky Bruce <beckyb@kernel.crashing.org>

swiotlb_map/unmap_single are now swiotlb_map/unmap_page;
trivially change all the comments to reference new names.
Also, there were some comments that should have been
referring to just plain old map_single, not swiotlb_map_single;
fix those as well. Also change a use of the word "pointer", when
what is referred to is actually a dma/physical address.

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
Acked-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 lib/swiotlb.c |   24 +++++++++++-------------
 1 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 2b0b5a7..170cf56 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -60,8 +60,8 @@ enum dma_sync_target {
 int swiotlb_force;
 
 /*
- * Used to do a quick range check in swiotlb_unmap_single and
- * swiotlb_sync_single_*, to see if the memory was in fact allocated by this
+ * Used to do a quick range check in unmap_single and
+ * sync_single_*, to see if the memory was in fact allocated by this
  * API.
  */
 static char *io_tlb_start, *io_tlb_end;
@@ -560,7 +560,6 @@ swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 				   size)) {
 		/*
 		 * The allocated memory isn't reachable by the device.
-		 * Fall back on swiotlb_map_single().
 		 */
 		free_pages((unsigned long) ret, order);
 		ret = NULL;
@@ -568,9 +567,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
-		 * swiotlb_map_single(), which will grab memory from
-		 * the lowest available address range.
+		 * 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);
 		if (!ret)
@@ -634,7 +632,7 @@ swiotlb_full(struct device *dev, size_t size, int dir, int do_panic)
  * physical address to use is returned.
  *
  * Once the device is given the dma address, the device owns this memory until
- * either swiotlb_unmap_single or swiotlb_dma_sync_single is performed.
+ * either swiotlb_unmap_page or swiotlb_dma_sync_single is performed.
  */
 dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 			    unsigned long offset, size_t size,
@@ -648,7 +646,7 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 
 	BUG_ON(dir == DMA_NONE);
 	/*
-	 * If the pointer passed in happens to be in the device's DMA window,
+	 * If the address happens to be in the device's DMA window,
 	 * we can safely return the device addr and not worry about bounce
 	 * buffering it.
 	 */
@@ -679,7 +677,7 @@ EXPORT_SYMBOL_GPL(swiotlb_map_page);
 
 /*
  * Unmap a single streaming mode DMA translation.  The dma_addr and size must
- * match what was provided for in a previous swiotlb_map_single call.  All
+ * match what was provided for in a previous swiotlb_map_page call.  All
  * other usages are undefined.
  *
  * After this call, reads by the cpu to the buffer are guaranteed to see
@@ -703,7 +701,7 @@ EXPORT_SYMBOL_GPL(swiotlb_unmap_page);
  * Make physical memory consistent for a single streaming mode DMA translation
  * after a transfer.
  *
- * If you perform a swiotlb_map_single() but wish to interrogate the buffer
+ * If you perform a swiotlb_map_page() but wish to interrogate the buffer
  * using the cpu, yet do not wish to teardown the dma mapping, you must
  * call this function before doing so.  At the next point you give the dma
  * address back to the card, you must first perform a
@@ -777,7 +775,7 @@ EXPORT_SYMBOL_GPL(swiotlb_sync_single_range_for_device);
 
 /*
  * Map a set of buffers described by scatterlist in streaming mode for DMA.
- * This is the scatter-gather version of the above swiotlb_map_single
+ * This is the scatter-gather version of the above swiotlb_map_page
  * interface.  Here the scatter gather list elements are each tagged with the
  * appropriate dma address and length.  They are obtained via
  * sg_dma_{address,length}(SG).
@@ -788,7 +786,7 @@ EXPORT_SYMBOL_GPL(swiotlb_sync_single_range_for_device);
  *       The routine returns the number of addr/length pairs actually
  *       used, at most nents.
  *
- * Device ownership issues as mentioned above for swiotlb_map_single are the
+ * Device ownership issues as mentioned above for swiotlb_map_page are the
  * same here.
  */
 int
@@ -836,7 +834,7 @@ EXPORT_SYMBOL(swiotlb_map_sg);
 
 /*
  * Unmap a set of streaming mode DMA translations.  Again, cpu read rules
- * concerning calls here are the same as for swiotlb_unmap_single() above.
+ * concerning calls here are the same as for swiotlb_unmap_page() above.
  */
 void
 swiotlb_unmap_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
-- 
1.6.0.6

[PATCH 2/7] swiotlb: fix compile warning

[Kumar Gala]

From: Becky Bruce <beckyb@kernel.crashing.org>

Squash a build warning seen on 32-bit powerpc caused by calling min()
with 2 different types. Use min_t() instead

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
Acked-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 lib/swiotlb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 170cf56..4fd6a76 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -341,7 +341,7 @@ static void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
 		unsigned long flags;
 
 		while (size) {
-			sz = min(PAGE_SIZE - offset, size);
+			sz = min_t(size_t, PAGE_SIZE - offset, size);
 
 			local_irq_save(flags);
 			buffer = kmap_atomic(pfn_to_page(pfn),
-- 
1.6.0.6

[PATCH 3/7] swiotlb: map_page fix for highmem systems

[Kumar Gala]

From: Becky Bruce <beckyb@kernel.crashing.org>

The current code calls virt_to_phys() on address that might
be in highmem, which is bad.  This wasn't needed, anyway, because
we already have the physical address we need. Get rid of the
now-unused virtual address as well.

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
Acked-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 lib/swiotlb.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 4fd6a76..e8a47c8 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -640,7 +640,6 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 			    struct dma_attrs *attrs)
 {
 	phys_addr_t phys = page_to_phys(page) + offset;
-	void *ptr = page_address(page) + offset;
 	dma_addr_t dev_addr = swiotlb_phys_to_bus(dev, phys);
 	void *map;
 
@@ -651,7 +650,7 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 	 * buffering it.
 	 */
 	if (!address_needs_mapping(dev, dev_addr, size) &&
-	    !range_needs_mapping(virt_to_phys(ptr), size))
+	    !range_needs_mapping(phys, size))
 		return dev_addr;
 
 	/*
-- 
1.6.0.6

[PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[Kumar Gala]

From: Becky Bruce <beckyb@kernel.crashing.org>

Some architectures require additional checking to determine
if a device can dma to an address and need to provide their
own address_needs_mapping..

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
Acked-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 lib/swiotlb.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index e8a47c8..d81afab 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -145,6 +145,12 @@ static void *swiotlb_bus_to_virt(dma_addr_t address)
 	return phys_to_virt(swiotlb_bus_to_phys(address));
 }
 
+int __weak swiotlb_arch_address_needs_mapping(struct device *hwdev,
+					       dma_addr_t addr, size_t size)
+{
+	return !is_buffer_dma_capable(dma_get_mask(hwdev), addr, size);
+}
+
 int __weak swiotlb_arch_range_needs_mapping(phys_addr_t paddr, size_t size)
 {
 	return 0;
@@ -309,10 +315,10 @@ cleanup1:
 	return -ENOMEM;
 }
 
-static int
+static inline int
 address_needs_mapping(struct device *hwdev, dma_addr_t addr, size_t size)
 {
-	return !is_buffer_dma_capable(dma_get_mask(hwdev), addr, size);
+	return swiotlb_arch_address_needs_mapping(hwdev, addr, size);
 }
 
 static inline int range_needs_mapping(phys_addr_t paddr, size_t size)
-- 
1.6.0.6

[PATCH 5/7] swiotlb: Rename unmap_single to do_unmap_single

[Kumar Gala]

From: Becky Bruce <beckyb@kernel.crashing.org>

Previously, swiotlb_unmap_page and swiotlb_unmap_sg were
duplicating very similar code.  Refactor that code into a
new unmap_single and unmap_single use do_unmap_single.

Note that the swiotlb_unmap_sg code was previously doing
a complicated comparison to determine if an addresses needed
to be unmapped where a simple is_swiotlb_buffer() call
would have sufficed.

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 lib/swiotlb.c |   42 +++++++++++++++++++++++++-----------------
 1 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index d81afab..2bde54a 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -482,7 +482,7 @@ found:
  * dma_addr is the kernel virtual address of the bounce buffer to unmap.
  */
 static void
-unmap_single(struct device *hwdev, char *dma_addr, size_t size, int dir)
+do_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;
@@ -591,7 +591,7 @@ swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 		       (unsigned long long)dev_addr);
 
 		/* DMA_TO_DEVICE to avoid memcpy in unmap_single */
-		unmap_single(hwdev, ret, size, DMA_TO_DEVICE);
+		do_unmap_single(hwdev, ret, size, DMA_TO_DEVICE);
 		return NULL;
 	}
 	*dma_handle = dev_addr;
@@ -608,7 +608,7 @@ swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
 		free_pages((unsigned long) vaddr, get_order(size));
 	else
 		/* DMA_TO_DEVICE to avoid memcpy in unmap_single */
-		unmap_single(hwdev, vaddr, size, DMA_TO_DEVICE);
+		do_unmap_single(hwdev, vaddr, size, DMA_TO_DEVICE);
 }
 EXPORT_SYMBOL(swiotlb_free_coherent);
 
@@ -688,17 +688,29 @@ EXPORT_SYMBOL_GPL(swiotlb_map_page);
  * After this call, reads by the cpu to the buffer are guaranteed to see
  * whatever the device wrote there.
  */
-void swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
-			size_t size, enum dma_data_direction dir,
-			struct dma_attrs *attrs)
+static void unmap_single(struct device *hwdev, dma_addr_t dev_addr,
+			 size_t size, int dir)
 {
 	char *dma_addr = swiotlb_bus_to_virt(dev_addr);
 
 	BUG_ON(dir == DMA_NONE);
-	if (is_swiotlb_buffer(dma_addr))
-		unmap_single(hwdev, dma_addr, size, dir);
-	else if (dir == DMA_FROM_DEVICE)
-		dma_mark_clean(dma_addr, size);
+
+	if (is_swiotlb_buffer(dma_addr)) {
+		do_unmap_single(hwdev, dma_addr, size, dir);
+		return;
+	}
+
+	if (dir != DMA_FROM_DEVICE)
+		return;
+
+	dma_mark_clean(dma_addr, size);
+}
+
+void swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
+			size_t size, enum dma_data_direction dir,
+			struct dma_attrs *attrs)
+{
+	unmap_single(hwdev, dev_addr, size, dir);
 }
 EXPORT_SYMBOL_GPL(swiotlb_unmap_page);
 
@@ -850,13 +862,9 @@ swiotlb_unmap_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
 
 	BUG_ON(dir == DMA_NONE);
 
-	for_each_sg(sgl, sg, nelems, i) {
-		if (sg->dma_address != swiotlb_phys_to_bus(hwdev, sg_phys(sg)))
-			unmap_single(hwdev, swiotlb_bus_to_virt(sg->dma_address),
-				     sg->dma_length, dir);
-		else if (dir == DMA_FROM_DEVICE)
-			dma_mark_clean(swiotlb_bus_to_virt(sg->dma_address), sg->dma_length);
-	}
+	for_each_sg(sgl, sg, nelems, i)
+		unmap_single(hwdev, sg->dma_address, sg->dma_length, dir);
+
 }
 EXPORT_SYMBOL(swiotlb_unmap_sg_attrs);
 
-- 
1.6.0.6

[PATCH 6/7] swiotlb: Use swiotlb_sync_single instead of duplicating code

[Kumar Gala]

From: Becky Bruce <beckyb@kernel.crashing.org>

Right now both swiotlb_sync_single_range and swiotlb_sync_sg
were duplicating the code in swiotlb_sync_single.  Just call it
instead.  Also rearrange the sync_single code for readability.

Note that the swiotlb_sync_sg code was previously doing
a complicated comparison to determine if an addresses needed
to be unmapped where a simple is_swiotlb_buffer() call
would have sufficed.

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
Acked-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 lib/swiotlb.c |   30 ++++++++++++------------------
 1 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 2bde54a..d912f06 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -731,10 +731,16 @@ swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr,
 	char *dma_addr = swiotlb_bus_to_virt(dev_addr);
 
 	BUG_ON(dir == DMA_NONE);
-	if (is_swiotlb_buffer(dma_addr))
+
+	if (is_swiotlb_buffer(dma_addr)) {
 		sync_single(hwdev, dma_addr, size, dir, target);
-	else if (dir == DMA_FROM_DEVICE)
-		dma_mark_clean(dma_addr, size);
+		return;
+	}
+
+	if (dir != DMA_FROM_DEVICE)
+		return;
+
+	dma_mark_clean(dma_addr, size);
 }
 
 void
@@ -761,13 +767,7 @@ swiotlb_sync_single_range(struct device *hwdev, dma_addr_t dev_addr,
 			  unsigned long offset, size_t size,
 			  int dir, int target)
 {
-	char *dma_addr = swiotlb_bus_to_virt(dev_addr) + offset;
-
-	BUG_ON(dir == DMA_NONE);
-	if (is_swiotlb_buffer(dma_addr))
-		sync_single(hwdev, dma_addr, size, dir, target);
-	else if (dir == DMA_FROM_DEVICE)
-		dma_mark_clean(dma_addr, size);
+	swiotlb_sync_single(hwdev, dev_addr + offset, size, dir, target);
 }
 
 void
@@ -890,15 +890,9 @@ swiotlb_sync_sg(struct device *hwdev, struct scatterlist *sgl,
 	struct scatterlist *sg;
 	int i;
 
-	BUG_ON(dir == DMA_NONE);
-
-	for_each_sg(sgl, sg, nelems, i) {
-		if (sg->dma_address != swiotlb_phys_to_bus(hwdev, sg_phys(sg)))
-			sync_single(hwdev, swiotlb_bus_to_virt(sg->dma_address),
+	for_each_sg(sgl, sg, nelems, i)
+		swiotlb_sync_single(hwdev, sg->dma_address,
 				    sg->dma_length, dir, target);
-		else if (dir == DMA_FROM_DEVICE)
-			dma_mark_clean(swiotlb_bus_to_virt(sg->dma_address), sg->dma_length);
-	}
 }
 
 void
-- 
1.6.0.6

[PATCH 7/7] swiotlb: Change swiotlb_bus_to[phys,virt] prototypes

[Kumar Gala]

From: Becky Bruce <beckyb@kernel.crashing.org>

Add a hwdev argument that is needed on some architectures
in order to access a per-device offset that is taken into
account when producing a physical address (also needed to
get from bus address to virtual address because the physical
address is an intermediate step).

Also make swiotlb_bus_to_virt weak so architectures can
override it.

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
Acked-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/x86/kernel/pci-swiotlb.c |    2 +-
 include/linux/swiotlb.h       |    3 ++-
 lib/swiotlb.c                 |   10 +++++-----
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
index 34f12e9..887388a 100644
--- a/arch/x86/kernel/pci-swiotlb.c
+++ b/arch/x86/kernel/pci-swiotlb.c
@@ -28,7 +28,7 @@ dma_addr_t swiotlb_phys_to_bus(struct device *hwdev, phys_addr_t paddr)
 	return paddr;
 }
 
-phys_addr_t swiotlb_bus_to_phys(dma_addr_t baddr)
+phys_addr_t swiotlb_bus_to_phys(struct device *hwdev, dma_addr_t baddr)
 {
 	return baddr;
 }
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index ac9ff54..cb1a663 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -29,7 +29,8 @@ extern void *swiotlb_alloc(unsigned order, unsigned long nslabs);
 
 extern dma_addr_t swiotlb_phys_to_bus(struct device *hwdev,
 				      phys_addr_t address);
-extern phys_addr_t swiotlb_bus_to_phys(dma_addr_t address);
+extern phys_addr_t swiotlb_bus_to_phys(struct device *hwdev,
+				       dma_addr_t address);
 
 extern int swiotlb_arch_range_needs_mapping(phys_addr_t paddr, size_t size);
 
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index d912f06..bffe6d7 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -129,7 +129,7 @@ dma_addr_t __weak swiotlb_phys_to_bus(struct device *hwdev, phys_addr_t paddr)
 	return paddr;
 }
 
-phys_addr_t __weak swiotlb_bus_to_phys(dma_addr_t baddr)
+phys_addr_t __weak swiotlb_bus_to_phys(struct device *hwdev, dma_addr_t baddr)
 {
 	return baddr;
 }
@@ -140,9 +140,9 @@ static dma_addr_t swiotlb_virt_to_bus(struct device *hwdev,
 	return swiotlb_phys_to_bus(hwdev, virt_to_phys(address));
 }
 
-static void *swiotlb_bus_to_virt(dma_addr_t address)
+void * __weak swiotlb_bus_to_virt(struct device *hwdev, dma_addr_t address)
 {
-	return phys_to_virt(swiotlb_bus_to_phys(address));
+	return phys_to_virt(swiotlb_bus_to_phys(hwdev, address));
 }
 
 int __weak swiotlb_arch_address_needs_mapping(struct device *hwdev,
@@ -691,7 +691,7 @@ EXPORT_SYMBOL_GPL(swiotlb_map_page);
 static void unmap_single(struct device *hwdev, dma_addr_t dev_addr,
 			 size_t size, int dir)
 {
-	char *dma_addr = swiotlb_bus_to_virt(dev_addr);
+	char *dma_addr = swiotlb_bus_to_virt(hwdev, dev_addr);
 
 	BUG_ON(dir == DMA_NONE);
 
@@ -728,7 +728,7 @@ static void
 swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr,
 		    size_t size, int dir, int target)
 {
-	char *dma_addr = swiotlb_bus_to_virt(dev_addr);
+	char *dma_addr = swiotlb_bus_to_virt(hwdev, dev_addr);
 
 	BUG_ON(dir == DMA_NONE);
 
-- 
1.6.0.6

[tip:core/iommu] swiotlb: change swiotlb_bus_to[phys,virt] prototypes

[Becky Bruce]

Commit-ID:  42d7c5e353cef9062129b0de3ec9ddf10567b9ca
Gitweb:     http://git.kernel.org/tip/42d7c5e353cef9062129b0de3ec9ddf10567b9ca
Author:     Becky Bruce <beckyb@kernel.crashing.org>
AuthorDate: Wed, 8 Apr 2009 09:09:21 -0500
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 8 Apr 2009 16:18:38 +0200

swiotlb: change swiotlb_bus_to[phys,virt] prototypes

Add a hwdev argument that is needed on some architectures
in order to access a per-device offset that is taken into
account when producing a physical address (also needed to
get from bus address to virtual address because the physical
address is an intermediate step).

Also make swiotlb_bus_to_virt weak so architectures can
override it.

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
Acked-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Cc: jeremy@goop.org
Cc: ian.campbell@citrix.com
LKML-Reference: <1239199761-22886-8-git-send-email-galak@kernel.crashing.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 arch/x86/kernel/pci-swiotlb.c |    2 +-
 include/linux/swiotlb.h       |    3 ++-
 lib/swiotlb.c                 |   10 +++++-----
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
index 34f12e9..887388a 100644
--- a/arch/x86/kernel/pci-swiotlb.c
+++ b/arch/x86/kernel/pci-swiotlb.c
@@ -28,7 +28,7 @@ dma_addr_t swiotlb_phys_to_bus(struct device *hwdev, phys_addr_t paddr)
 	return paddr;
 }
 
-phys_addr_t swiotlb_bus_to_phys(dma_addr_t baddr)
+phys_addr_t swiotlb_bus_to_phys(struct device *hwdev, dma_addr_t baddr)
 {
 	return baddr;
 }
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index ac9ff54..cb1a663 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -29,7 +29,8 @@ extern void *swiotlb_alloc(unsigned order, unsigned long nslabs);
 
 extern dma_addr_t swiotlb_phys_to_bus(struct device *hwdev,
 				      phys_addr_t address);
-extern phys_addr_t swiotlb_bus_to_phys(dma_addr_t address);
+extern phys_addr_t swiotlb_bus_to_phys(struct device *hwdev,
+				       dma_addr_t address);
 
 extern int swiotlb_arch_range_needs_mapping(phys_addr_t paddr, size_t size);
 
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index d912f06..bffe6d7 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -129,7 +129,7 @@ dma_addr_t __weak swiotlb_phys_to_bus(struct device *hwdev, phys_addr_t paddr)
 	return paddr;
 }
 
-phys_addr_t __weak swiotlb_bus_to_phys(dma_addr_t baddr)
+phys_addr_t __weak swiotlb_bus_to_phys(struct device *hwdev, dma_addr_t baddr)
 {
 	return baddr;
 }
@@ -140,9 +140,9 @@ static dma_addr_t swiotlb_virt_to_bus(struct device *hwdev,
 	return swiotlb_phys_to_bus(hwdev, virt_to_phys(address));
 }
 
-static void *swiotlb_bus_to_virt(dma_addr_t address)
+void * __weak swiotlb_bus_to_virt(struct device *hwdev, dma_addr_t address)
 {
-	return phys_to_virt(swiotlb_bus_to_phys(address));
+	return phys_to_virt(swiotlb_bus_to_phys(hwdev, address));
 }
 
 int __weak swiotlb_arch_address_needs_mapping(struct device *hwdev,
@@ -691,7 +691,7 @@ EXPORT_SYMBOL_GPL(swiotlb_map_page);
 static void unmap_single(struct device *hwdev, dma_addr_t dev_addr,
 			 size_t size, int dir)
 {
-	char *dma_addr = swiotlb_bus_to_virt(dev_addr);
+	char *dma_addr = swiotlb_bus_to_virt(hwdev, dev_addr);
 
 	BUG_ON(dir == DMA_NONE);
 
@@ -728,7 +728,7 @@ static void
 swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr,
 		    size_t size, int dir, int target)
 {
-	char *dma_addr = swiotlb_bus_to_virt(dev_addr);
+	char *dma_addr = swiotlb_bus_to_virt(hwdev, dev_addr);
 
 	BUG_ON(dir == DMA_NONE);
 

[tip:core/iommu] swiotlb: use swiotlb_sync_single instead of duplicating code

[Becky Bruce]

Commit-ID:  380d687833aee098c4a2c3b35beaefe1c1f48d01
Gitweb:     http://git.kernel.org/tip/380d687833aee098c4a2c3b35beaefe1c1f48d01
Author:     Becky Bruce <beckyb@kernel.crashing.org>
AuthorDate: Wed, 8 Apr 2009 09:09:20 -0500
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 8 Apr 2009 16:18:37 +0200

swiotlb: use swiotlb_sync_single instead of duplicating code

Right now both swiotlb_sync_single_range and swiotlb_sync_sg
were duplicating the code in swiotlb_sync_single.  Just call it
instead.  Also rearrange the sync_single code for readability.

Note that the swiotlb_sync_sg code was previously doing
a complicated comparison to determine if an addresses needed
to be unmapped where a simple is_swiotlb_buffer() call
would have sufficed.

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
Acked-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Cc: jeremy@goop.org
Cc: ian.campbell@citrix.com
LKML-Reference: <1239199761-22886-7-git-send-email-galak@kernel.crashing.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 lib/swiotlb.c |   30 ++++++++++++------------------
 1 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 2bde54a..d912f06 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -731,10 +731,16 @@ swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr,
 	char *dma_addr = swiotlb_bus_to_virt(dev_addr);
 
 	BUG_ON(dir == DMA_NONE);
-	if (is_swiotlb_buffer(dma_addr))
+
+	if (is_swiotlb_buffer(dma_addr)) {
 		sync_single(hwdev, dma_addr, size, dir, target);
-	else if (dir == DMA_FROM_DEVICE)
-		dma_mark_clean(dma_addr, size);
+		return;
+	}
+
+	if (dir != DMA_FROM_DEVICE)
+		return;
+
+	dma_mark_clean(dma_addr, size);
 }
 
 void
@@ -761,13 +767,7 @@ swiotlb_sync_single_range(struct device *hwdev, dma_addr_t dev_addr,
 			  unsigned long offset, size_t size,
 			  int dir, int target)
 {
-	char *dma_addr = swiotlb_bus_to_virt(dev_addr) + offset;
-
-	BUG_ON(dir == DMA_NONE);
-	if (is_swiotlb_buffer(dma_addr))
-		sync_single(hwdev, dma_addr, size, dir, target);
-	else if (dir == DMA_FROM_DEVICE)
-		dma_mark_clean(dma_addr, size);
+	swiotlb_sync_single(hwdev, dev_addr + offset, size, dir, target);
 }
 
 void
@@ -890,15 +890,9 @@ swiotlb_sync_sg(struct device *hwdev, struct scatterlist *sgl,
 	struct scatterlist *sg;
 	int i;
 
-	BUG_ON(dir == DMA_NONE);
-
-	for_each_sg(sgl, sg, nelems, i) {
-		if (sg->dma_address != swiotlb_phys_to_bus(hwdev, sg_phys(sg)))
-			sync_single(hwdev, swiotlb_bus_to_virt(sg->dma_address),
+	for_each_sg(sgl, sg, nelems, i)
+		swiotlb_sync_single(hwdev, sg->dma_address,
 				    sg->dma_length, dir, target);
-		else if (dir == DMA_FROM_DEVICE)
-			dma_mark_clean(swiotlb_bus_to_virt(sg->dma_address), sg->dma_length);
-	}
 }
 
 void

[tip:core/iommu] swiotlb: rename unmap_single to do_unmap_single

[Becky Bruce]

Commit-ID:  7fcebbd2d984eac3fdd6da2f4453e7c43d32de89
Gitweb:     http://git.kernel.org/tip/7fcebbd2d984eac3fdd6da2f4453e7c43d32de89
Author:     Becky Bruce <beckyb@kernel.crashing.org>
AuthorDate: Wed, 8 Apr 2009 09:09:19 -0500
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 8 Apr 2009 16:18:37 +0200

swiotlb: rename unmap_single to do_unmap_single

Previously, swiotlb_unmap_page and swiotlb_unmap_sg were
duplicating very similar code.  Refactor that code into a
new unmap_single and unmap_single use do_unmap_single.

Note that the swiotlb_unmap_sg code was previously doing
a complicated comparison to determine if an addresses needed
to be unmapped where a simple is_swiotlb_buffer() call
would have sufficed.

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Cc: jeremy@goop.org
Cc: ian.campbell@citrix.com
LKML-Reference: <1239199761-22886-6-git-send-email-galak@kernel.crashing.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 lib/swiotlb.c |   42 +++++++++++++++++++++++++-----------------
 1 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index d81afab..2bde54a 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -482,7 +482,7 @@ found:
  * dma_addr is the kernel virtual address of the bounce buffer to unmap.
  */
 static void
-unmap_single(struct device *hwdev, char *dma_addr, size_t size, int dir)
+do_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;
@@ -591,7 +591,7 @@ swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 		       (unsigned long long)dev_addr);
 
 		/* DMA_TO_DEVICE to avoid memcpy in unmap_single */
-		unmap_single(hwdev, ret, size, DMA_TO_DEVICE);
+		do_unmap_single(hwdev, ret, size, DMA_TO_DEVICE);
 		return NULL;
 	}
 	*dma_handle = dev_addr;
@@ -608,7 +608,7 @@ swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
 		free_pages((unsigned long) vaddr, get_order(size));
 	else
 		/* DMA_TO_DEVICE to avoid memcpy in unmap_single */
-		unmap_single(hwdev, vaddr, size, DMA_TO_DEVICE);
+		do_unmap_single(hwdev, vaddr, size, DMA_TO_DEVICE);
 }
 EXPORT_SYMBOL(swiotlb_free_coherent);
 
@@ -688,17 +688,29 @@ EXPORT_SYMBOL_GPL(swiotlb_map_page);
  * After this call, reads by the cpu to the buffer are guaranteed to see
  * whatever the device wrote there.
  */
-void swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
-			size_t size, enum dma_data_direction dir,
-			struct dma_attrs *attrs)
+static void unmap_single(struct device *hwdev, dma_addr_t dev_addr,
+			 size_t size, int dir)
 {
 	char *dma_addr = swiotlb_bus_to_virt(dev_addr);
 
 	BUG_ON(dir == DMA_NONE);
-	if (is_swiotlb_buffer(dma_addr))
-		unmap_single(hwdev, dma_addr, size, dir);
-	else if (dir == DMA_FROM_DEVICE)
-		dma_mark_clean(dma_addr, size);
+
+	if (is_swiotlb_buffer(dma_addr)) {
+		do_unmap_single(hwdev, dma_addr, size, dir);
+		return;
+	}
+
+	if (dir != DMA_FROM_DEVICE)
+		return;
+
+	dma_mark_clean(dma_addr, size);
+}
+
+void swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
+			size_t size, enum dma_data_direction dir,
+			struct dma_attrs *attrs)
+{
+	unmap_single(hwdev, dev_addr, size, dir);
 }
 EXPORT_SYMBOL_GPL(swiotlb_unmap_page);
 
@@ -850,13 +862,9 @@ swiotlb_unmap_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
 
 	BUG_ON(dir == DMA_NONE);
 
-	for_each_sg(sgl, sg, nelems, i) {
-		if (sg->dma_address != swiotlb_phys_to_bus(hwdev, sg_phys(sg)))
-			unmap_single(hwdev, swiotlb_bus_to_virt(sg->dma_address),
-				     sg->dma_length, dir);
-		else if (dir == DMA_FROM_DEVICE)
-			dma_mark_clean(swiotlb_bus_to_virt(sg->dma_address), sg->dma_length);
-	}
+	for_each_sg(sgl, sg, nelems, i)
+		unmap_single(hwdev, sg->dma_address, sg->dma_length, dir);
+
 }
 EXPORT_SYMBOL(swiotlb_unmap_sg_attrs);
 

[tip:core/iommu] swiotlb: allow arch override of address_needs_mapping

[Becky Bruce]

Commit-ID:  ef5722f698bde01cfec2b98fff733a48663ebf55
Gitweb:     http://git.kernel.org/tip/ef5722f698bde01cfec2b98fff733a48663ebf55
Author:     Becky Bruce <beckyb@kernel.crashing.org>
AuthorDate: Wed, 8 Apr 2009 09:09:18 -0500
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 8 Apr 2009 16:18:36 +0200

swiotlb: allow arch override of address_needs_mapping

Some architectures require additional checking to determine
if a device can dma to an address and need to provide their
own address_needs_mapping..

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
Acked-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Cc: jeremy@goop.org
Cc: ian.campbell@citrix.com
LKML-Reference: <1239199761-22886-5-git-send-email-galak@kernel.crashing.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 lib/swiotlb.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index e8a47c8..d81afab 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -145,6 +145,12 @@ static void *swiotlb_bus_to_virt(dma_addr_t address)
 	return phys_to_virt(swiotlb_bus_to_phys(address));
 }
 
+int __weak swiotlb_arch_address_needs_mapping(struct device *hwdev,
+					       dma_addr_t addr, size_t size)
+{
+	return !is_buffer_dma_capable(dma_get_mask(hwdev), addr, size);
+}
+
 int __weak swiotlb_arch_range_needs_mapping(phys_addr_t paddr, size_t size)
 {
 	return 0;
@@ -309,10 +315,10 @@ cleanup1:
 	return -ENOMEM;
 }
 
-static int
+static inline int
 address_needs_mapping(struct device *hwdev, dma_addr_t addr, size_t size)
 {
-	return !is_buffer_dma_capable(dma_get_mask(hwdev), addr, size);
+	return swiotlb_arch_address_needs_mapping(hwdev, addr, size);
 }
 
 static inline int range_needs_mapping(phys_addr_t paddr, size_t size)

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[Christoph Hellwig]

On Wed, Apr 08, 2009 at 09:09:18AM -0500, Kumar Gala wrote:
> From: Becky Bruce <beckyb@kernel.crashing.org>
> 
> Some architectures require additional checking to determine
> if a device can dma to an address and need to provide their
> own address_needs_mapping..

Shouldn't we just move it completely to the arch?  I think that ia64 and
x86 currently use the same one is more of an accident.

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[Kumar Gala]

On Apr 8, 2009, at 3:38 PM, Christoph Hellwig wrote:

> On Wed, Apr 08, 2009 at 09:09:18AM -0500, Kumar Gala wrote:
>> From: Becky Bruce <beckyb@kernel.crashing.org>
>>
>> Some architectures require additional checking to determine
>> if a device can dma to an address and need to provide their
>> own address_needs_mapping..
>
> Shouldn't we just move it completely to the arch?  I think that ia64  
> and
> x86 currently use the same one is more of an accident.

It seems like the swiotlb code uses __weak for a number of things:

lib/swiotlb.c:void * __weak __init swiotlb_alloc_boot(size_t size,  
unsigned long nslabs)
lib/swiotlb.c:void * __weak swiotlb_alloc(unsigned order, unsigned  
long nslabs)
lib/swiotlb.c:dma_addr_t __weak swiotlb_phys_to_bus(struct device  
*hwdev, phys_addr_t paddr)
lib/swiotlb.c:phys_addr_t __weak swiotlb_bus_to_phys(struct device  
*hwdev, dma_addr_t baddr)
lib/swiotlb.c:void * __weak swiotlb_bus_to_virt(struct device *hwdev,  
dma_addr_t address)
lib/swiotlb.c:int __weak swiotlb_arch_address_needs_mapping(struct  
device *hwdev,
lib/swiotlb.c:int __weak swiotlb_arch_range_needs_mapping(phys_addr_t  
paddr, size_t size)

instead of #ifndef HAVE_ARCH_<FOO>.  Not sure if there is a historical  
reason for that.

- k

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[FUJITA Tomonori]

On Wed, 8 Apr 2009 15:56:32 -0500
Kumar Gala <galak@kernel.crashing.org> wrote:

> 
> On Apr 8, 2009, at 3:38 PM, Christoph Hellwig wrote:
> 
> > On Wed, Apr 08, 2009 at 09:09:18AM -0500, Kumar Gala wrote:
> >> From: Becky Bruce <beckyb@kernel.crashing.org>
> >>
> >> Some architectures require additional checking to determine
> >> if a device can dma to an address and need to provide their
> >> own address_needs_mapping..
> >
> > Shouldn't we just move it completely to the arch?  I think that ia64  
> > and
> > x86 currently use the same one is more of an accident.
> 
> It seems like the swiotlb code uses __weak for a number of things:
> 
> lib/swiotlb.c:void * __weak __init swiotlb_alloc_boot(size_t size,  
> unsigned long nslabs)
> lib/swiotlb.c:void * __weak swiotlb_alloc(unsigned order, unsigned  
> long nslabs)
> lib/swiotlb.c:dma_addr_t __weak swiotlb_phys_to_bus(struct device  
> *hwdev, phys_addr_t paddr)
> lib/swiotlb.c:phys_addr_t __weak swiotlb_bus_to_phys(struct device  
> *hwdev, dma_addr_t baddr)
> lib/swiotlb.c:void * __weak swiotlb_bus_to_virt(struct device *hwdev,  
> dma_addr_t address)
> lib/swiotlb.c:int __weak swiotlb_arch_address_needs_mapping(struct  
> device *hwdev,
> lib/swiotlb.c:int __weak swiotlb_arch_range_needs_mapping(phys_addr_t  
> paddr, size_t size)
> 
> instead of #ifndef HAVE_ARCH_<FOO>.  Not sure if there is a historical  
> reason for that.

ia64 and x86_64 use swiotlb but neither need this function. And
neither need any above __weak. They were added for dom0 support.
Yeah, swiotlb is much cleaner and better if we don't add dom0 support.

About this patch, I think that we could do better. What we need to do
is allowing each architectures to have is_buffer_dma_capable().

I'm doing the dma_mapping_ops unification. I think that adding
something like is_buffer_dma_capable to dma_map_ops struct is
cleaner. Then we don't need this __weak function. But this patch is
fine by me for now.

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[Jeremy Fitzhardinge]

FUJITA Tomonori wrote:
> On Wed, 8 Apr 2009 15:56:32 -0500
> Kumar Gala <galak@kernel.crashing.org> wrote:
>
>   
>> On Apr 8, 2009, at 3:38 PM, Christoph Hellwig wrote:
>>
>>     
>>> On Wed, Apr 08, 2009 at 09:09:18AM -0500, Kumar Gala wrote:
>>>       
>>>> From: Becky Bruce <beckyb@kernel.crashing.org>
>>>>
>>>> Some architectures require additional checking to determine
>>>> if a device can dma to an address and need to provide their
>>>> own address_needs_mapping..
>>>>         
>>> Shouldn't we just move it completely to the arch?  I think that ia64  
>>> and
>>> x86 currently use the same one is more of an accident.
>>>       
>> It seems like the swiotlb code uses __weak for a number of things:
>>
>> lib/swiotlb.c:void * __weak __init swiotlb_alloc_boot(size_t size,  
>> unsigned long nslabs)
>> lib/swiotlb.c:void * __weak swiotlb_alloc(unsigned order, unsigned  
>> long nslabs)
>> lib/swiotlb.c:dma_addr_t __weak swiotlb_phys_to_bus(struct device  
>> *hwdev, phys_addr_t paddr)
>> lib/swiotlb.c:phys_addr_t __weak swiotlb_bus_to_phys(struct device  
>> *hwdev, dma_addr_t baddr)
>> lib/swiotlb.c:void * __weak swiotlb_bus_to_virt(struct device *hwdev,  
>> dma_addr_t address)
>> lib/swiotlb.c:int __weak swiotlb_arch_address_needs_mapping(struct  
>> device *hwdev,
>> lib/swiotlb.c:int __weak swiotlb_arch_range_needs_mapping(phys_addr_t  
>> paddr, size_t size)
>>
>> instead of #ifndef HAVE_ARCH_<FOO>.  Not sure if there is a historical  
>> reason for that.
>>     
>
> ia64 and x86_64 use swiotlb but neither need this function. And
> neither need any above __weak. They were added for dom0 support.
> Yeah, swiotlb is much cleaner and better if we don't add dom0 support.
>   

Some architectures need non-trivial bus<->phys conversion routines, etc, 
so either we can require it that all architectures wishing to use 
swiotlb define these functions, or have weak default functions that can 
be overridden by architectures where necessary.

This isn't a specific Xen dom0 requirement, except that enabling it in 
the config will override these functions (but now in a Xen-only file, 
rather than affecting the normal x86 pci-swiotlb.c).

    J

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[FUJITA Tomonori]

On Wed, 08 Apr 2009 14:55:55 -0700
Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> FUJITA Tomonori wrote:
> > On Wed, 8 Apr 2009 15:56:32 -0500
> > Kumar Gala <galak@kernel.crashing.org> wrote:
> >
> >   
> >> On Apr 8, 2009, at 3:38 PM, Christoph Hellwig wrote:
> >>
> >>     
> >>> On Wed, Apr 08, 2009 at 09:09:18AM -0500, Kumar Gala wrote:
> >>>       
> >>>> From: Becky Bruce <beckyb@kernel.crashing.org>
> >>>>
> >>>> Some architectures require additional checking to determine
> >>>> if a device can dma to an address and need to provide their
> >>>> own address_needs_mapping..
> >>>>         
> >>> Shouldn't we just move it completely to the arch?  I think that ia64  
> >>> and
> >>> x86 currently use the same one is more of an accident.
> >>>       
> >> It seems like the swiotlb code uses __weak for a number of things:
> >>
> >> lib/swiotlb.c:void * __weak __init swiotlb_alloc_boot(size_t size,  
> >> unsigned long nslabs)
> >> lib/swiotlb.c:void * __weak swiotlb_alloc(unsigned order, unsigned  
> >> long nslabs)
> >> lib/swiotlb.c:dma_addr_t __weak swiotlb_phys_to_bus(struct device  
> >> *hwdev, phys_addr_t paddr)
> >> lib/swiotlb.c:phys_addr_t __weak swiotlb_bus_to_phys(struct device  
> >> *hwdev, dma_addr_t baddr)
> >> lib/swiotlb.c:void * __weak swiotlb_bus_to_virt(struct device *hwdev,  
> >> dma_addr_t address)
> >> lib/swiotlb.c:int __weak swiotlb_arch_address_needs_mapping(struct  
> >> device *hwdev,
> >> lib/swiotlb.c:int __weak swiotlb_arch_range_needs_mapping(phys_addr_t  
> >> paddr, size_t size)
> >>
> >> instead of #ifndef HAVE_ARCH_<FOO>.  Not sure if there is a historical  
> >> reason for that.
> >>     
> >
> > ia64 and x86_64 use swiotlb but neither need this function. And
> > neither need any above __weak. They were added for dom0 support.
> > Yeah, swiotlb is much cleaner and better if we don't add dom0 support.
> >   
> 
> Some architectures need non-trivial bus<->phys conversion routines, etc, 

Only Xen needs such conversion for swiotlb.


> so either we can require it that all architectures wishing to use 
> swiotlb define these functions, or have weak default functions that can 
> be overridden by architectures where necessary.

Can you give an example? I don't think IA64, X86_64 or POWER (which
will use swiotlb) need any __weak functions. If you say other archs
could use swiotlb, please tell me how they need these __weak.


> This isn't a specific Xen dom0 requirement, except that enabling it in 

Yes, it is.


> the config will override these functions (but now in a Xen-only file, 
> rather than affecting the normal x86 pci-swiotlb.c).

And again, x86' pci-swiotlb is much cleaner without dom0 support.

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[Jeremy Fitzhardinge]

FUJITA Tomonori wrote:
>> Some architectures need non-trivial bus<->phys conversion routines, etc, 
>>     
>
> Only Xen needs such conversion for swiotlb.
>   

Becky's patches of last week also added __weak annotations to 
swiotlb_bus_to_virt, virt_to_bus and bus_to_phys; added the hwdev 
parameter to swiotlb_bus_to_phys; and added a weak 
swiotlb_arch_address_needs_mapping.  I assume that was needed because 
powerpc needs non-trivial implementations for those functions.

>> so either we can require it that all architectures wishing to use 
>> swiotlb define these functions, or have weak default functions that can 
>> be overridden by architectures where necessary.
>>     
>
> Can you give an example? I don't think IA64, X86_64 or POWER (which
> will use swiotlb) need any __weak functions. If you say other archs
> could use swiotlb, please tell me how they need these __weak.
>   

As I said, Becky's patches added hooks in many of the places we added 
them for Xen.  I assume that's because powerpc needs them; I have not 
seen the arch/powerpc side of those changes.

    J

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[FUJITA Tomonori]

On Wed, 08 Apr 2009 15:36:58 -0700
Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> FUJITA Tomonori wrote:
> >> Some architectures need non-trivial bus<->phys conversion routines, etc, 
> >>     
> >
> > Only Xen needs such conversion for swiotlb.
> >   
> 
> Becky's patches of last week also added __weak annotations to 
> swiotlb_bus_to_virt, virt_to_bus and bus_to_phys; added the hwdev 
> parameter to swiotlb_bus_to_phys; and added a weak 
> swiotlb_arch_address_needs_mapping.  I assume that was needed because 
> powerpc needs non-trivial implementations for those functions.

Hmm, what she added are wrappers of virt_to_bus and bus_to_virt. We
can remove these and directly use virt_to_bus and bus_to_virt.

About __weak address_needs_mapping function, as I said, removing it
and using dma_map_ops is a proper solution.


> >> so either we can require it that all architectures wishing to use 
> >> swiotlb define these functions, or have weak default functions that can 
> >> be overridden by architectures where necessary.
> >>     
> >
> > Can you give an example? I don't think IA64, X86_64 or POWER (which
> > will use swiotlb) need any __weak functions. If you say other archs
> > could use swiotlb, please tell me how they need these __weak.
> >   
> 
> As I said, Becky's patches added hooks in many of the places we added 
> them for Xen.  I assume that's because powerpc needs them; I have not 
> seen the arch/powerpc side of those changes.
> 
>     J
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[Jeremy Fitzhardinge]

FUJITA Tomonori wrote:
>> Becky's patches of last week also added __weak annotations to 
>> swiotlb_bus_to_virt, virt_to_bus and bus_to_phys; added the hwdev 
>> parameter to swiotlb_bus_to_phys; and added a weak 
>> swiotlb_arch_address_needs_mapping.  I assume that was needed because 
>> powerpc needs non-trivial implementations for those functions.
>>     
>
> Hmm, what she added are wrappers of virt_to_bus and bus_to_virt. We
> can remove these and directly use virt_to_bus and bus_to_virt.
>   

In general those interfaces are deprecated.  Are we un-deprecating 
them?  Or do you mean adding virt<->bus to dma_ops?

> About __weak address_needs_mapping function, as I said, removing it
> and using dma_map_ops is a proper solution.
>   

Fine.  Could swiotlb_alloc() just call dma_alloc_coherent() too?

    J

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[FUJITA Tomonori]

On Wed, 08 Apr 2009 16:16:17 -0700
Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> FUJITA Tomonori wrote:
> >> Becky's patches of last week also added __weak annotations to 
> >> swiotlb_bus_to_virt, virt_to_bus and bus_to_phys; added the hwdev 
> >> parameter to swiotlb_bus_to_phys; and added a weak 
> >> swiotlb_arch_address_needs_mapping.  I assume that was needed because 
> >> powerpc needs non-trivial implementations for those functions.
> >>     
> >
> > Hmm, what she added are wrappers of virt_to_bus and bus_to_virt. We
> > can remove these and directly use virt_to_bus and bus_to_virt.
> >   
> 
> In general those interfaces are deprecated.  Are we un-deprecating 
> them?  Or do you mean adding virt<->bus to dma_ops?

Hmm, these interfaces are wrong for drivers surely because they can't
handle dma mapping properly. However, they are exactly what swiotlb
needs (swiotlb doesn't need to care about dma mapping). Until 2.6.28,
swiotlb has used them. They are with IA64, X86_64 and PPC_32, I think.


> > About __weak address_needs_mapping function, as I said, removing it
> > and using dma_map_ops is a proper solution.
> >   
> 
> Fine.  Could swiotlb_alloc() just call dma_alloc_coherent() too?

I'm not sure what you mean. And I don't think ppc wants swiotlb_alloc.

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[Jeremy Fitzhardinge]

FUJITA Tomonori wrote:
> On Wed, 08 Apr 2009 16:16:17 -0700
> Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>
>   
>> FUJITA Tomonori wrote:
>>     
>>>> Becky's patches of last week also added __weak annotations to 
>>>> swiotlb_bus_to_virt, virt_to_bus and bus_to_phys; added the hwdev 
>>>> parameter to swiotlb_bus_to_phys; and added a weak 
>>>> swiotlb_arch_address_needs_mapping.  I assume that was needed because 
>>>> powerpc needs non-trivial implementations for those functions.
>>>>     
>>>>         
>>> Hmm, what she added are wrappers of virt_to_bus and bus_to_virt. We
>>> can remove these and directly use virt_to_bus and bus_to_virt.
>>>   
>>>       
>> In general those interfaces are deprecated.  Are we un-deprecating 
>> them?  Or do you mean adding virt<->bus to dma_ops?
>>     
>
> Hmm, these interfaces are wrong for drivers surely because they can't
> handle dma mapping properly. However, they are exactly what swiotlb
> needs (swiotlb doesn't need to care about dma mapping).

It needs to care about the mapping from phys to bus.  On x86 they're 
identical, but on powerpc there can be at least an offset between them.

>  Until 2.6.28,
> swiotlb has used them. They are with IA64, X86_64 and PPC_32, I think.
>   

Well, Becky's patches also added the hwdev argument to them, so 
presumably the powerpc implementation needs that (different 
devices/buses have differing views of physical memory, I guess).

> I'm not sure what you mean. And I don't think ppc wants swiotlb_alloc.
>   

No, its something we need for Xen.  I was thinking that swiotlb could 
allocate its memory with dma_alloc_coherent(NULL, size, ...).  That 
would allocate via x86_fallback_device, which would not have the right 
behaviour (it would set GFP_DMA, for a start), and would end up hitting 
the uninitialized dma_ops.  So the idea doesn't really work; it would 
need swiotlb to define another placeholder  device who's alloc_coherent 
operation could be overridden, and it all gets pretty ugly.

As an aside, I'm also wondering why there's a distinction between 
swiotlb_alloc() and swiotlb_alloc_boot().  The latter allocates from 
bootmem, but I don't see what's wrong with allocating from slab a little 
bit later, once it has been initialized.  The comment mentions something 
about allocating ISA DMA memory, but the code doesn't make any attempt 
to allocate the buffer below 16MB (its generally much larger than 16MB 
anyway).

    J

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[Kumar Gala]

On Apr 8, 2009, at 7:09 PM, Jeremy Fitzhardinge wrote:

> FUJITA Tomonori wrote:
>> On Wed, 08 Apr 2009 16:16:17 -0700
>> Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>>
>>
>>> FUJITA Tomonori wrote:
>>>
>>>>> Becky's patches of last week also added __weak annotations to  
>>>>> swiotlb_bus_to_virt, virt_to_bus and bus_to_phys; added the  
>>>>> hwdev parameter to swiotlb_bus_to_phys; and added a weak  
>>>>> swiotlb_arch_address_needs_mapping.  I assume that was needed  
>>>>> because powerpc needs non-trivial implementations for those  
>>>>> functions.
>>>>>
>>>> Hmm, what she added are wrappers of virt_to_bus and bus_to_virt. We
>>>> can remove these and directly use virt_to_bus and bus_to_virt.
>>>>
>>> In general those interfaces are deprecated.  Are we un-deprecating  
>>> them?  Or do you mean adding virt<->bus to dma_ops?
>>>
>>
>> Hmm, these interfaces are wrong for drivers surely because they can't
>> handle dma mapping properly. However, they are exactly what swiotlb
>> needs (swiotlb doesn't need to care about dma mapping).
>
> It needs to care about the mapping from phys to bus.  On x86 they're  
> identical, but on powerpc there can be at least an offset between  
> them.
>
>> Until 2.6.28,
>> swiotlb has used them. They are with IA64, X86_64 and PPC_32, I  
>> think.
>>
>
> Well, Becky's patches also added the hwdev argument to them, so  
> presumably the powerpc implementation needs that (different devices/ 
> buses have differing views of physical memory, I guess).

On powerpc we need the hwdev because things vary based on bus.  For  
our SoC chips we don't need any mapping between phys & bus.  However  
something like PCI does have a mapping (a simple offset).

- k

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[FUJITA Tomonori]

On Wed, 08 Apr 2009 17:09:00 -0700
Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> FUJITA Tomonori wrote:
> > On Wed, 08 Apr 2009 16:16:17 -0700
> > Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> >
> >   
> >> FUJITA Tomonori wrote:
> >>     
> >>>> Becky's patches of last week also added __weak annotations to 
> >>>> swiotlb_bus_to_virt, virt_to_bus and bus_to_phys; added the hwdev 
> >>>> parameter to swiotlb_bus_to_phys; and added a weak 
> >>>> swiotlb_arch_address_needs_mapping.  I assume that was needed because 
> >>>> powerpc needs non-trivial implementations for those functions.
> >>>>     
> >>>>         
> >>> Hmm, what she added are wrappers of virt_to_bus and bus_to_virt. We
> >>> can remove these and directly use virt_to_bus and bus_to_virt.
> >>>   
> >>>       
> >> In general those interfaces are deprecated.  Are we un-deprecating 
> >> them?  Or do you mean adding virt<->bus to dma_ops?
> >>     
> >
> > Hmm, these interfaces are wrong for drivers surely because they can't
> > handle dma mapping properly. However, they are exactly what swiotlb
> > needs (swiotlb doesn't need to care about dma mapping).
> 
> It needs to care about the mapping from phys to bus.  On x86 they're 
> identical, but on powerpc there can be at least an offset between them.
> 
> >  Until 2.6.28,
> > swiotlb has used them. They are with IA64, X86_64 and PPC_32, I think.
> >   
> 
> Well, Becky's patches also added the hwdev argument to them, so 
> presumably the powerpc implementation needs that (different 
> devices/buses have differing views of physical memory, I guess).

Until I see the ppc specific swiotlb patchset, I'm not sure but I
think that we can remove phys_to_bus in swiotlb.

Even if we need phys_to_bus, we can remove the rest of __weak tricks
for only dom0. And we can make phys_to_bus arch-specific. Then we
don't need any __weak tricks in swiotlb (and x86's swiotlb). dom0
support adds many hacks to swiotlb.


> > I'm not sure what you mean. And I don't think ppc wants swiotlb_alloc.
> >   
> 
> No, its something we need for Xen.  I was thinking that swiotlb could 
> allocate its memory with dma_alloc_coherent(NULL, size, ...).  That 
> would allocate via x86_fallback_device, which would not have the right 
> behaviour (it would set GFP_DMA, for a start), and would end up hitting 
> the uninitialized dma_ops.  So the idea doesn't really work; it would 
> need swiotlb to define another placeholder  device who's alloc_coherent 
> operation could be overridden, and it all gets pretty ugly.

It doesn't work. And swiotlb's alloc_coherent needs to be
arch-specific because the page allocator can't handle dma mask.


> As an aside, I'm also wondering why there's a distinction between 
> swiotlb_alloc() and swiotlb_alloc_boot().  The latter allocates from 
> bootmem, but I don't see what's wrong with allocating from slab a little 
> bit later, once it has been initialized.  The comment mentions something 
> about allocating ISA DMA memory, but the code doesn't make any attempt 
> to allocate the buffer below 16MB (its generally much larger than 16MB 
> anyway).

Yeah, ISA DMA comment is misleading. swiotlb can't handle it. And it
doesn't need to handle it because the block layer can thanks to
the bouncing (the network layer does the similar, I think).

As you said, we could remove the latter though I'm not sure.

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[Jeremy Fitzhardinge]

FUJITA Tomonori wrote:
>> Well, Becky's patches also added the hwdev argument to them, so 
>> presumably the powerpc implementation needs that (different 
>> devices/buses have differing views of physical memory, I guess).
>>     
>
> Until I see the ppc specific swiotlb patchset, I'm not sure but I
> think that we can remove phys_to_bus in swiotlb.
>   

Kumar's comment was: "For our SoC chips we don't need any mapping 
between phys & bus.  However something like PCI does have a mapping (a 
simple offset)."

Kumar, could a single system have different phys<->bus mappings on a 
single system, or could it differ from device to device (or bus to bus)?

> Even if we need phys_to_bus, we can remove the rest of __weak tricks
> for only dom0. And we can make phys_to_bus arch-specific. Then we
> don't need any __weak tricks in swiotlb (and x86's swiotlb). dom0
> support adds many hacks to swiotlb.
>   

Well, we'd still need a way to do hook the swiotlb_alloc(_boot) 
allocation.  At the moment its effectively arch-specific because x86 
only uses swiotlb_alloc_boot(), and ia64 only uses swiotlb_alloc().  One 
option would be to simply make that function arch-defined, which would 
remove the need for any kind of override mechanism in lib/swiotlb; that 
would match the handling of phys_to_bus.  And its more appealing if we 
manage to drop swiotlb_alloc_boot, so there's only a single function for 
the arches to worry about.

> Yeah, ISA DMA comment is misleading. swiotlb can't handle it. And it
> doesn't need to handle it because the block layer can thanks to
> the bouncing (the network layer does the similar, I think).
>
> As you said, we could remove the latter though I'm not sure.
>   

It would take a bit of rearranging the x86 swiotlb/iommu init sequence, 
but I don't think it would be too complex.  I'll look into it.

    J

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[FUJITA Tomonori]

On Thu, 09 Apr 2009 12:19:19 -0700
Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> FUJITA Tomonori wrote:
> >> Well, Becky's patches also added the hwdev argument to them, so 
> >> presumably the powerpc implementation needs that (different 
> >> devices/buses have differing views of physical memory, I guess).
> >>     
> >
> > Until I see the ppc specific swiotlb patchset, I'm not sure but I
> > think that we can remove phys_to_bus in swiotlb.
> >   
> 
> Kumar's comment was: "For our SoC chips we don't need any mapping 
> between phys & bus.  However something like PCI does have a mapping (a 
> simple offset)."

I meant that swiotlb doesn't need to use phys_to_bus stuff. But I as
said, I'm not sure until I see the ppc swiotlb code.


> Kumar, could a single system have different phys<->bus mappings on a 
> single system, or could it differ from device to device (or bus to bus)?
> 
> > Even if we need phys_to_bus, we can remove the rest of __weak tricks
> > for only dom0. And we can make phys_to_bus arch-specific. Then we
> > don't need any __weak tricks in swiotlb (and x86's swiotlb). dom0
> > support adds many hacks to swiotlb.
> >   
> 
> Well, we'd still need a way to do hook the swiotlb_alloc(_boot) 
> allocation.  At the moment its effectively arch-specific because x86 
> only uses swiotlb_alloc_boot(), and ia64 only uses swiotlb_alloc().  One 
> option would be to simply make that function arch-defined, which would 
> remove the need for any kind of override mechanism in lib/swiotlb; that 
> would match the handling of phys_to_bus.  And its more appealing if we 
> manage to drop swiotlb_alloc_boot, so there's only a single function for 
> the arches to worry about.
> 
> > Yeah, ISA DMA comment is misleading. swiotlb can't handle it. And it
> > doesn't need to handle it because the block layer can thanks to
> > the bouncing (the network layer does the similar, I think).
> >
> > As you said, we could remove the latter though I'm not sure.
> >   
> 
> It would take a bit of rearranging the x86 swiotlb/iommu init sequence, 
> but I don't think it would be too complex.  I'll look into it.

Unfortunately, not that simple. IA64_64 and x86 share the iommu init
sequence. So you need to look at both. I put some cleanups on it in
30-rc1. But I need to do more cleanups.

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[FUJITA Tomonori]

On Thu, 09 Apr 2009 12:19:19 -0700
Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> FUJITA Tomonori wrote:
> >> Well, Becky's patches also added the hwdev argument to them, so 
> >> presumably the powerpc implementation needs that (different 
> >> devices/buses have differing views of physical memory, I guess).
> >>     
> >
> > Until I see the ppc specific swiotlb patchset, I'm not sure but I
> > think that we can remove phys_to_bus in swiotlb.
> >   
> 
> Kumar's comment was: "For our SoC chips we don't need any mapping 
> between phys & bus.  However something like PCI does have a mapping (a 
> simple offset)."
> 
> Kumar, could a single system have different phys<->bus mappings on a 
> single system, or could it differ from device to device (or bus to bus)?
> 
> > Even if we need phys_to_bus, we can remove the rest of __weak tricks
> > for only dom0. And we can make phys_to_bus arch-specific. Then we
> > don't need any __weak tricks in swiotlb (and x86's swiotlb). dom0
> > support adds many hacks to swiotlb.
> >   
> 
> Well, we'd still need a way to do hook the swiotlb_alloc(_boot) 
> allocation.  At the moment its effectively arch-specific because x86 
> only uses swiotlb_alloc_boot(), and ia64 only uses swiotlb_alloc().  One 

Hmm, I think that ia64 use swiotlb_alloc_boot too.

ia64 uses swiotlb in two ways; using only swiotlb, using swiotlb and
hw iommu. The latter is similar what x86 Calgary does.

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[Jeremy Fitzhardinge]

FUJITA Tomonori wrote:
> Hmm, I think that ia64 use swiotlb_alloc_boot too.
>
> ia64 uses swiotlb in two ways; using only swiotlb, using swiotlb and
> hw iommu. The latter is similar what x86 Calgary does.
>   

Yes, I see.  It also calls swiotlb_init() -> 
swiotlb_init_with_default_size(), though I got lost in the 
machvec/platform setup, so I couldn't quite work out where 
platform_dma_init() gets called from.

But I couldn't see any x86 references to 
swiotlb_late_init_with_default_size().

    J

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[Kumar Gala]

On Apr 8, 2009, at 6:01 PM, FUJITA Tomonori wrote:

>> Becky's patches of last week also added __weak annotations to
>> swiotlb_bus_to_virt, virt_to_bus and bus_to_phys; added the hwdev
>> parameter to swiotlb_bus_to_phys; and added a weak
>> swiotlb_arch_address_needs_mapping.  I assume that was needed because
>> powerpc needs non-trivial implementations for those functions.
>
> Hmm, what she added are wrappers of virt_to_bus and bus_to_virt. We
> can remove these and directly use virt_to_bus and bus_to_virt.
>
> About __weak address_needs_mapping function, as I said, removing it
> and using dma_map_ops is a proper solution.

Is this something you are looking at doing in the .31 timeframe?

I'm looking at the fact that we need to switch over to using struct  
dma_map_ops on ppc.  (I'm guessing this might be the patches you  
mentioned the other day).  If so did you add set_dma_mask() to the  
generic dma_map_ops?

- k

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[FUJITA Tomonori]

On Wed, 8 Apr 2009 23:59:18 -0500
Kumar Gala <galak@kernel.crashing.org> wrote:

> 
> On Apr 8, 2009, at 6:01 PM, FUJITA Tomonori wrote:
> 
> >> Becky's patches of last week also added __weak annotations to
> >> swiotlb_bus_to_virt, virt_to_bus and bus_to_phys; added the hwdev
> >> parameter to swiotlb_bus_to_phys; and added a weak
> >> swiotlb_arch_address_needs_mapping.  I assume that was needed because
> >> powerpc needs non-trivial implementations for those functions.
> >
> > Hmm, what she added are wrappers of virt_to_bus and bus_to_virt. We
> > can remove these and directly use virt_to_bus and bus_to_virt.
> >
> > About __weak address_needs_mapping function, as I said, removing it
> > and using dma_map_ops is a proper solution.
> 
> Is this something you are looking at doing in the .31 timeframe?
> 
> I'm looking at the fact that we need to switch over to using struct  
> dma_map_ops on ppc.  (I'm guessing this might be the patches you  
> mentioned the other day).  If so did you add set_dma_mask() to the  
> generic dma_map_ops?

Yeah, I'll send patches to convert ppc to use dma_map_ops. In .31
timeframe, I plan to:

- add a generic dma-mapping.h and convert ia64, x86, and ppc to use it

- clean up swiotlb.

- try to convert archs supporting multiple dma ops to use dma_map_ops

- rewrite ia64 and x86 dma ops initialization


BTW, the ppc specific swiotlb patchset is available?

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[Kumar Gala]

On Apr 9, 2009, at 1:50 PM, FUJITA Tomonori wrote:

> On Wed, 8 Apr 2009 23:59:18 -0500
> Kumar Gala <galak@kernel.crashing.org> wrote:
>
>>
>> On Apr 8, 2009, at 6:01 PM, FUJITA Tomonori wrote:
>>
>>>> Becky's patches of last week also added __weak annotations to
>>>> swiotlb_bus_to_virt, virt_to_bus and bus_to_phys; added the hwdev
>>>> parameter to swiotlb_bus_to_phys; and added a weak
>>>> swiotlb_arch_address_needs_mapping.  I assume that was needed  
>>>> because
>>>> powerpc needs non-trivial implementations for those functions.
>>>
>>> Hmm, what she added are wrappers of virt_to_bus and bus_to_virt. We
>>> can remove these and directly use virt_to_bus and bus_to_virt.
>>>
>>> About __weak address_needs_mapping function, as I said, removing it
>>> and using dma_map_ops is a proper solution.
>>
>> Is this something you are looking at doing in the .31 timeframe?
>>
>> I'm looking at the fact that we need to switch over to using struct
>> dma_map_ops on ppc.  (I'm guessing this might be the patches you
>> mentioned the other day).  If so did you add set_dma_mask() to the
>> generic dma_map_ops?
>
> Yeah, I'll send patches to convert ppc to use dma_map_ops. In .31
> timeframe, I plan to:
>
> - add a generic dma-mapping.h and convert ia64, x86, and ppc to use it
>
> - clean up swiotlb.
>
> - try to convert archs supporting multiple dma ops to use dma_map_ops
>
> - rewrite ia64 and x86 dma ops initialization
>
>
> BTW, the ppc specific swiotlb patchset is available?

Still cleaning it up, but I'll post a WIP patch so people have a sense  
of what the changes look like.

- k

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[Kumar Gala]

Here's the WIP patch for the PPC changes to give some sense of what we
need out of swiotlb for PPC.

- k

---
 arch/powerpc/include/asm/dma-mapping.h     |   18 ++++
 arch/powerpc/include/asm/scatterlist.h     |    6 +-
 arch/powerpc/include/asm/swiotlb.h         |   15 +++
 arch/powerpc/kernel/Makefile               |    1 +
 arch/powerpc/kernel/dma-swiotlb.c          |  154 ++++++++++++++++++++++++++++
 arch/powerpc/kernel/dma.c                  |    2 +-
 arch/powerpc/kernel/pci-common.c           |    2 +
 arch/powerpc/kernel/setup_32.c             |    4 +
 10 files changed, 211 insertions(+), 7 deletions(-)
 create mode 100644 arch/powerpc/include/asm/swiotlb.h
 create mode 100644 arch/powerpc/kernel/dma-swiotlb.c

diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
index c69f2b5..db6edfe 100644
--- a/arch/powerpc/include/asm/dma-mapping.h
+++ b/arch/powerpc/include/asm/dma-mapping.h
@@ -16,8 +16,18 @@
 #include <linux/dma-attrs.h>
 #include <asm/io.h>

+#ifdef CONFIG_SWIOTLB
+#include <asm/swiotlb.h>
+#endif
+
 #define DMA_ERROR_CODE		(~(dma_addr_t)0x0)

+/* Some dma direct funcs must be visible for use in other dma_ops */
+extern void *dma_direct_alloc_coherent(struct device *dev, size_t size,
+				       dma_addr_t *dma_handle, gfp_t flag);
+extern void dma_direct_free_coherent(struct device *dev, size_t size,
+				     void *vaddr, dma_addr_t dma_handle);
+
 #ifdef CONFIG_NOT_COHERENT_CACHE
 /*
  * DMA-consistent mapping functions for PowerPCs that don't support
@@ -112,11 +122,19 @@ static inline struct dma_mapping_ops *get_dma_ops(struct device *dev)
 	if (unlikely(dev == NULL))
 		return NULL;

+	/* BGILL - temp code to look for problems */
+	if(dev->archdata.dma_ops == NULL)
+		printk(KERN_EMERG "dev %s has null dma_ops\n", dev->bus->name);
+
 	return dev->archdata.dma_ops;
 }

 static inline void set_dma_ops(struct device *dev, struct dma_mapping_ops *ops)
 {
+	printk(KERN_EMERG "Setting dma_ops to %s\n",
+	       (ops == &dma_direct_ops) ? "dma_direct_ops" :
+	       ((ops == &swiotlb_dma_ops) ? "swiotlb_dma_ops" : "unknown"));
+
 	dev->archdata.dma_ops = ops;
 }

diff --git a/arch/powerpc/include/asm/scatterlist.h b/arch/powerpc/include/asm/scatterlist.h
index fcf7d55..912bf59 100644
--- a/arch/powerpc/include/asm/scatterlist.h
+++ b/arch/powerpc/include/asm/scatterlist.h
@@ -21,7 +21,7 @@ struct scatterlist {
 	unsigned int offset;
 	unsigned int length;

-	/* For TCE support */
+	/* For TCE or SWIOTLB support */
 	dma_addr_t dma_address;
 	u32 dma_length;
 };
@@ -34,11 +34,7 @@ struct scatterlist {
  * is 0.
  */
 #define sg_dma_address(sg)	((sg)->dma_address)
-#ifdef __powerpc64__
 #define sg_dma_len(sg)		((sg)->dma_length)
-#else
-#define sg_dma_len(sg)		((sg)->length)
-#endif

 #ifdef __powerpc64__
 #define ISA_DMA_THRESHOLD	(~0UL)
diff --git a/arch/powerpc/include/asm/swiotlb.h b/arch/powerpc/include/asm/swiotlb.h
new file mode 100644
index 0000000..cbfce6c
--- /dev/null
+++ b/arch/powerpc/include/asm/swiotlb.h
@@ -0,0 +1,15 @@
+#ifndef __ASM_SWIOTLB_H
+#define __ASM_SWIOTLB_H
+
+#include <linux/swiotlb.h>
+
+extern int swiotlb_force;
+extern int swiotlb;
+extern struct dma_mapping_ops swiotlb_dma_ops;
+
+int swiotlb_arch_address_needs_mapping(struct device *, dma_addr_t,
+				       size_t size);
+
+static inline void dma_mark_clean(void *addr, size_t size) {}
+
+#endif /* __ASM_SWIOTLB_H */
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 71901fb..34c0a95 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_SMP)		+= smp.o
 obj-$(CONFIG_KPROBES)		+= kprobes.o
 obj-$(CONFIG_PPC_UDBG_16550)	+= legacy_serial.o udbg_16550.o
 obj-$(CONFIG_STACKTRACE)	+= stacktrace.o
+obj-$(CONFIG_SWIOTLB)		+= dma-swiotlb.o

 pci64-$(CONFIG_PPC64)		+= pci_dn.o isa-bridge.o
 obj-$(CONFIG_PCI)		+= pci_$(CONFIG_WORD_SIZE).o $(pci64-y) \
diff --git a/arch/powerpc/kernel/dma-swiotlb.c b/arch/powerpc/kernel/dma-swiotlb.c
new file mode 100644
index 0000000..639f3bc
--- /dev/null
+++ b/arch/powerpc/kernel/dma-swiotlb.c
@@ -0,0 +1,154 @@
+/*
+ * Copyright (C) 2009 Becky Bruce, Freescale Semiconductor
+ *
+ * swiotlb dma ops and functions required by the swiotlb code.
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/pfn.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#include <asm/machdep.h>
+#include <asm/swiotlb.h>
+#include <asm/dma.h>
+#include <asm/abs_addr.h>
+
+int swiotlb __read_mostly;
+
+unsigned long get_dma_direct_offset(struct device *dev);
+
+void *swiotlb_bus_to_virt(struct device *hwdev, dma_addr_t addr)
+{
+	unsigned long pfn = PFN_DOWN(swiotlb_bus_to_phys(hwdev, addr));
+	void *pageaddr = page_address(pfn_to_page(pfn));
+
+	if(pageaddr != NULL)
+		return pageaddr + (addr % PAGE_SIZE);
+	return NULL;
+}
+
+#if 0 /* BGILL - don't need */
+/* This can only be called on pages with a kernel mapping */
+dma_addr_t swiotlb_virt_to_bus(struct device *hwdev, void *addr)
+{
+	return swiotlb_phys_to_bus(hwdev, virt_to_abs((unsigned long)addr));
+}
+#endif
+
+dma_addr_t swiotlb_phys_to_bus(struct device *hwdev, phys_addr_t paddr)
+{
+	return (paddr + get_dma_direct_offset(hwdev));
+}
+
+phys_addr_t swiotlb_bus_to_phys(struct device *hwdev, dma_addr_t baddr)
+
+{
+	return (baddr - get_dma_direct_offset(hwdev));
+}
+
+/*
+ * Eventually, we should really be looking at some per-device
+ * quantity stored in archdata, and not a global value, since this is
+ * only needed for devices like PCI that use part of their 32 bit address
+ * space for something other than mapping memory.
+ *
+ * For now, require swiotlb mapping above MAX_32B_DIRECT_DMA_ADDR for devs
+ * that are not 64-bit capable.  This should be determined using the PCI mem
+ * allocations in the devtree. -beckyb
+ */
+#define MAX_32B_DIRECT_DMA_ADDR 0x80000000
+
+/* determine if an address needs bounce buffering via swiotlb. */
+int
+swiotlb_arch_address_needs_mapping(struct device *hwdev, dma_addr_t addr,
+				   size_t size)
+{
+	dma_addr_t mask = DMA_BIT_MASK(32);
+
+	/* Max dma_address we can access without bounce buffering */
+	dma_addr_t max = swiotlb_phys_to_bus(hwdev, MAX_32B_DIRECT_DMA_ADDR);
+
+	/* BGILL - can we get rid of this? */
+	if (hwdev && hwdev->dma_mask)
+		mask = *hwdev->dma_mask;
+
+	if ((mask <= DMA_BIT_MASK(36)) && (addr + size > max))
+		return 1;
+
+	return !is_buffer_dma_capable(mask, addr, size);
+}
+
+/*
+ * At the moment, all platforms that use this code only require
+ * swiotlb to be used if we're operating on HIGHMEM.  Since
+ * we don't ever call anything other than map_sg, unmap_sg,
+ * map_page, and unmap_page on highmem, use normal dma_ops
+ * for everything else.
+ */
+struct dma_mapping_ops swiotlb_dma_ops = {
+#if 0
+	.alloc_coherent = dma_direct_alloc_coherent,
+	.free_coherent = dma_direct_free_coherent,
+#else
+	.alloc_coherent = swiotlb_alloc_coherent,
+	.free_coherent = swiotlb_free_coherent,
+#endif
+	.map_sg = swiotlb_map_sg_attrs,
+	.unmap_sg = swiotlb_unmap_sg_attrs,
+	.dma_supported = swiotlb_dma_supported,
+	.map_page = swiotlb_map_page,
+	.unmap_page = swiotlb_unmap_page,
+	.sync_single_range_for_cpu = swiotlb_sync_single_range_for_cpu,
+	.sync_single_range_for_device = swiotlb_sync_single_range_for_device,
+	.sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
+	.sync_sg_for_device = swiotlb_sync_sg_for_device
+};
+
+static int ppc_swiotlb_bus_notify(struct notifier_block *nb,
+				  unsigned long action, void *data)
+{
+	struct device *dev = data;
+
+	/* We are only intereted in device addition */
+	if (action != BUS_NOTIFY_ADD_DEVICE)
+		return 0;
+
+#if 0 /* BGILL - should look like this; use other form for debug */
+	if(dma_get_mask(dev) < DMA_BIT_MASK(36))
+		set_dma_ops(dev, &swiotlb_dma_ops);
+#else
+	if(!dev->dma_mask) {
+		printk(KERN_EMERG "ERROR: no dma_mask set for dev\n");
+		set_dma_ops(dev, &swiotlb_dma_ops);
+	}
+	else if(*dev->dma_mask < DMA_BIT_MASK(36)) {
+		set_dma_ops(dev, &swiotlb_dma_ops);
+	}
+	else
+		printk(KERN_EMERG "ERROR; fall thru with direct_ops\n");
+#endif
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block ppc_swiotlb_plat_bus_notifier = {
+	.notifier_call = ppc_swiotlb_bus_notify,
+	.priority = 0,
+};
+
+static struct notifier_block ppc_swiotlb_of_bus_notifier = {
+	.notifier_call = ppc_swiotlb_bus_notify,
+	.priority = 0,
+};
+
+static int __init setup_bus_notifier(void)
+{
+	bus_register_notifier(&platform_bus_type, &ppc_swiotlb_plat_bus_notifier);
+	bus_register_notifier(&of_platform_bus_type, &ppc_swiotlb_of_bus_notifier);
+
+	return 0;
+}
+
+machine_arch_initcall(mpc86xx_hpcn, setup_bus_notifier);
+
diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
index 53c7788..62d80c4 100644
--- a/arch/powerpc/kernel/dma.c
+++ b/arch/powerpc/kernel/dma.c
@@ -19,7 +19,7 @@
  * default the offset is PCI_DRAM_OFFSET.
  */

-static unsigned long get_dma_direct_offset(struct device *dev)
+unsigned long get_dma_direct_offset(struct device *dev)
 {
 	if (dev)
 		return (unsigned long)dev->archdata.dma_data;
diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index 9e1ca74..f038b13 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -332,6 +332,10 @@ void __init setup_arch(char **cmdline_p)
 		ppc_md.setup_arch();
 	if ( ppc_md.progress ) ppc_md.progress("arch: exit", 0x3eab);

+	/* Allow iotlb to do it's setup */
+#ifdef CONFIG_SWIOTLB
+	swiotlb_init();
+#endif
 	paging_init();

 	/* Initialize the MMU context management stuff */

Re: [PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[Christoph Hellwig]

On Thu, Apr 09, 2009 at 06:15:07AM +0900, FUJITA Tomonori wrote:
> ia64 and x86_64 use swiotlb but neither need this function. And
> neither need any above __weak. They were added for dom0 support.
> Yeah, swiotlb is much cleaner and better if we don't add dom0 support.

Agreed.  All these dom0 support patches look like a cat has been barfing
over the source code.

[tip:core/iommu] swiotlb: map_page fix for highmem systems

[Becky Bruce]

Commit-ID:  dd6b02fe427f30520d0adc94aa52352367227873
Gitweb:     http://git.kernel.org/tip/dd6b02fe427f30520d0adc94aa52352367227873
Author:     Becky Bruce <beckyb@kernel.crashing.org>
AuthorDate: Wed, 8 Apr 2009 09:09:17 -0500
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 8 Apr 2009 16:18:36 +0200

swiotlb: map_page fix for highmem systems

The current code calls virt_to_phys() on address that might
be in highmem, which is bad.  This wasn't needed, anyway, because
we already have the physical address we need.

Get rid of the now-unused virtual address as well.

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
Acked-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Cc: jeremy@goop.org
Cc: ian.campbell@citrix.com
LKML-Reference: <1239199761-22886-4-git-send-email-galak@kernel.crashing.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 lib/swiotlb.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 4fd6a76..e8a47c8 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -640,7 +640,6 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 			    struct dma_attrs *attrs)
 {
 	phys_addr_t phys = page_to_phys(page) + offset;
-	void *ptr = page_address(page) + offset;
 	dma_addr_t dev_addr = swiotlb_phys_to_bus(dev, phys);
 	void *map;
 
@@ -651,7 +650,7 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 	 * buffering it.
 	 */
 	if (!address_needs_mapping(dev, dev_addr, size) &&
-	    !range_needs_mapping(virt_to_phys(ptr), size))
+	    !range_needs_mapping(phys, size))
 		return dev_addr;
 
 	/*

[tip:core/iommu] swiotlb: fix compile warning

[Becky Bruce]

Commit-ID:  67131ad0514d7105b55003a0506209cf1bba3f00
Gitweb:     http://git.kernel.org/tip/67131ad0514d7105b55003a0506209cf1bba3f00
Author:     Becky Bruce <beckyb@kernel.crashing.org>
AuthorDate: Wed, 8 Apr 2009 09:09:16 -0500
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 8 Apr 2009 16:18:35 +0200

swiotlb: fix compile warning

Squash a build warning seen on 32-bit powerpc caused by
calling min() with 2 different types. Use min_t() instead.

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
Acked-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Cc: jeremy@goop.org
Cc: ian.campbell@citrix.com
LKML-Reference: <1239199761-22886-3-git-send-email-galak@kernel.crashing.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 lib/swiotlb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 170cf56..4fd6a76 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -341,7 +341,7 @@ static void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
 		unsigned long flags;
 
 		while (size) {
-			sz = min(PAGE_SIZE - offset, size);
+			sz = min_t(size_t, PAGE_SIZE - offset, size);
 
 			local_irq_save(flags);
 			buffer = kmap_atomic(pfn_to_page(pfn),

[tip:core/iommu] swiotlb: comment corrections

[Becky Bruce]

Commit-ID:  ceb5ac3264686e75e6951de6a18d4baa9bdecb92
Gitweb:     http://git.kernel.org/tip/ceb5ac3264686e75e6951de6a18d4baa9bdecb92
Author:     Becky Bruce <beckyb@kernel.crashing.org>
AuthorDate: Wed, 8 Apr 2009 09:09:15 -0500
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 8 Apr 2009 16:18:35 +0200

swiotlb: comment corrections

Impact: cleanup

swiotlb_map/unmap_single are now swiotlb_map/unmap_page;
trivially change all the comments to reference new names.

Also, there were some comments that should have been
referring to just plain old map_single, not swiotlb_map_single;
fix those as well.

Also change a use of the word "pointer", when what is
referred to is actually a dma/physical address.

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
Acked-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Cc: jeremy@goop.org
Cc: ian.campbell@citrix.com
LKML-Reference: <1239199761-22886-2-git-send-email-galak@kernel.crashing.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 lib/swiotlb.c |   24 +++++++++++-------------
 1 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 2b0b5a7..170cf56 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -60,8 +60,8 @@ enum dma_sync_target {
 int swiotlb_force;
 
 /*
- * Used to do a quick range check in swiotlb_unmap_single and
- * swiotlb_sync_single_*, to see if the memory was in fact allocated by this
+ * Used to do a quick range check in unmap_single and
+ * sync_single_*, to see if the memory was in fact allocated by this
  * API.
  */
 static char *io_tlb_start, *io_tlb_end;
@@ -560,7 +560,6 @@ swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 				   size)) {
 		/*
 		 * The allocated memory isn't reachable by the device.
-		 * Fall back on swiotlb_map_single().
 		 */
 		free_pages((unsigned long) ret, order);
 		ret = NULL;
@@ -568,9 +567,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
-		 * swiotlb_map_single(), which will grab memory from
-		 * the lowest available address range.
+		 * 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);
 		if (!ret)
@@ -634,7 +632,7 @@ swiotlb_full(struct device *dev, size_t size, int dir, int do_panic)
  * physical address to use is returned.
  *
  * Once the device is given the dma address, the device owns this memory until
- * either swiotlb_unmap_single or swiotlb_dma_sync_single is performed.
+ * either swiotlb_unmap_page or swiotlb_dma_sync_single is performed.
  */
 dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 			    unsigned long offset, size_t size,
@@ -648,7 +646,7 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 
 	BUG_ON(dir == DMA_NONE);
 	/*
-	 * If the pointer passed in happens to be in the device's DMA window,
+	 * If the address happens to be in the device's DMA window,
 	 * we can safely return the device addr and not worry about bounce
 	 * buffering it.
 	 */
@@ -679,7 +677,7 @@ EXPORT_SYMBOL_GPL(swiotlb_map_page);
 
 /*
  * Unmap a single streaming mode DMA translation.  The dma_addr and size must
- * match what was provided for in a previous swiotlb_map_single call.  All
+ * match what was provided for in a previous swiotlb_map_page call.  All
  * other usages are undefined.
  *
  * After this call, reads by the cpu to the buffer are guaranteed to see
@@ -703,7 +701,7 @@ EXPORT_SYMBOL_GPL(swiotlb_unmap_page);
  * Make physical memory consistent for a single streaming mode DMA translation
  * after a transfer.
  *
- * If you perform a swiotlb_map_single() but wish to interrogate the buffer
+ * If you perform a swiotlb_map_page() but wish to interrogate the buffer
  * using the cpu, yet do not wish to teardown the dma mapping, you must
  * call this function before doing so.  At the next point you give the dma
  * address back to the card, you must first perform a
@@ -777,7 +775,7 @@ EXPORT_SYMBOL_GPL(swiotlb_sync_single_range_for_device);
 
 /*
  * Map a set of buffers described by scatterlist in streaming mode for DMA.
- * This is the scatter-gather version of the above swiotlb_map_single
+ * This is the scatter-gather version of the above swiotlb_map_page
  * interface.  Here the scatter gather list elements are each tagged with the
  * appropriate dma address and length.  They are obtained via
  * sg_dma_{address,length}(SG).
@@ -788,7 +786,7 @@ EXPORT_SYMBOL_GPL(swiotlb_sync_single_range_for_device);
  *       The routine returns the number of addr/length pairs actually
  *       used, at most nents.
  *
- * Device ownership issues as mentioned above for swiotlb_map_single are the
+ * Device ownership issues as mentioned above for swiotlb_map_page are the
  * same here.
  */
 int
@@ -836,7 +834,7 @@ EXPORT_SYMBOL(swiotlb_map_sg);
 
 /*
  * Unmap a set of streaming mode DMA translations.  Again, cpu read rules
- * concerning calls here are the same as for swiotlb_unmap_single() above.
+ * concerning calls here are the same as for swiotlb_unmap_page() above.
  */
 void
 swiotlb_unmap_sg_attrs(struct device *hwdev, struct scatterlist *sgl,

Re: [PATCH V3 0/7] swiotlb: changes for powerpc/highmem

[Ingo Molnar]

* Kumar Gala <galak@kernel.crashing.org> wrote:

> This is a repost of Becky's patches from the v2 version.  The only 
> change is to incorporate feedback on "Create swiotlb_unmap_single" 
> an to add Ack by FUJITA Tomonori for patches that have been 
> reviewed.

thanks - picked them up.

This commit:

  dd6b02f: swiotlb: map_page fix for highmem systems

should perhaps go into an urgent track for .30 too? Jeremy, does Xen 
already make use of the swiotlb? (it does not right now AFAICS but 
.30 mergs of Xen might, so it would be good for them to have this 
fix.)

	Ingo

[PATCH V2 0/7] swiotlb: changes for powerpc/highmem

[Becky Bruce]

This is v2 of a series of fairly minor patches that get swiotlb
working on 32-bit powerpc systems with HIGHMEM, plus some cleanup
of the outdated comments in the code. I've made a couple of things
weak that ppc needs to override, and have changed the prototypes
for a couple of functions to include the hwdev pointer, which
we need to ppc to convert bus addresses to and from phys/virt
addresses. I've also fixed a build warning I've been seeing on
ppc.

In response to commentary on the previous series, I've also
refactored the code a bit, altough I did this slightly
differently than was suggested because I noticed we could use the
new helper function in 2 places instead of one. I've reformatted
a bit of code based on commentary as well.

I have not tested this in any way on any non-ppc platforms,
so commentary/testing from x86/ia64 folks is, once again,
greatly appreciated.

I'm going to be offline for the next week, but will respond to
commentary as soon as I return.

Cheers,
Becky

diffstat:
arch/x86/kernel/pci-swiotlb.c |    2 +-
include/linux/swiotlb.h       |    3 +-
lib/swiotlb.c                 |  115 ++++++++++++++++++++++-------------------
3 files changed, 64 insertions(+), 56 deletions(-)

[PATCH 1/7] swiotlb: comment corrections (no code changes)

[Becky Bruce]

swiotlb_map/unmap_single are now swiotlb_map/unmap_page;
trivially change all the comments to reference new names.
Also, there were some comments that should have been
referring to just plain old map_single, not swiotlb_map_single;
fix those as well. Also change a use of the word "pointer", when
what is referred to is actually a dma/physical address.

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
---
 lib/swiotlb.c |   24 +++++++++++-------------
 1 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 32e2bd3..f59cf30 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -60,8 +60,8 @@ enum dma_sync_target {
 int swiotlb_force;
 
 /*
- * Used to do a quick range check in swiotlb_unmap_single and
- * swiotlb_sync_single_*, to see if the memory was in fact allocated by this
+ * Used to do a quick range check in unmap_single and
+ * sync_single_*, to see if the memory was in fact allocated by this
  * API.
  */
 static char *io_tlb_start, *io_tlb_end;
@@ -560,7 +560,6 @@ swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 				   size)) {
 		/*
 		 * The allocated memory isn't reachable by the device.
-		 * Fall back on swiotlb_map_single().
 		 */
 		free_pages((unsigned long) ret, order);
 		ret = NULL;
@@ -568,9 +567,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
-		 * swiotlb_map_single(), which will grab memory from
-		 * the lowest available address range.
+		 * 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);
 		if (!ret)
@@ -634,7 +632,7 @@ swiotlb_full(struct device *dev, size_t size, int dir, int do_panic)
  * physical address to use is returned.
  *
  * Once the device is given the dma address, the device owns this memory until
- * either swiotlb_unmap_single or swiotlb_dma_sync_single is performed.
+ * either swiotlb_unmap_page or swiotlb_dma_sync_single is performed.
  */
 dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 			    unsigned long offset, size_t size,
@@ -648,7 +646,7 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 
 	BUG_ON(dir == DMA_NONE);
 	/*
-	 * If the pointer passed in happens to be in the device's DMA window,
+	 * If the address happens to be in the device's DMA window,
 	 * we can safely return the device addr and not worry about bounce
 	 * buffering it.
 	 */
@@ -679,7 +677,7 @@ EXPORT_SYMBOL_GPL(swiotlb_map_page);
 
 /*
  * Unmap a single streaming mode DMA translation.  The dma_addr and size must
- * match what was provided for in a previous swiotlb_map_single call.  All
+ * match what was provided for in a previous swiotlb_map_page call.  All
  * other usages are undefined.
  *
  * After this call, reads by the cpu to the buffer are guaranteed to see
@@ -703,7 +701,7 @@ EXPORT_SYMBOL_GPL(swiotlb_unmap_page);
  * Make physical memory consistent for a single streaming mode DMA translation
  * after a transfer.
  *
- * If you perform a swiotlb_map_single() but wish to interrogate the buffer
+ * If you perform a swiotlb_map_page() but wish to interrogate the buffer
  * using the cpu, yet do not wish to teardown the dma mapping, you must
  * call this function before doing so.  At the next point you give the dma
  * address back to the card, you must first perform a
@@ -777,7 +775,7 @@ EXPORT_SYMBOL_GPL(swiotlb_sync_single_range_for_device);
 
 /*
  * Map a set of buffers described by scatterlist in streaming mode for DMA.
- * This is the scatter-gather version of the above swiotlb_map_single
+ * This is the scatter-gather version of the above swiotlb_map_page
  * interface.  Here the scatter gather list elements are each tagged with the
  * appropriate dma address and length.  They are obtained via
  * sg_dma_{address,length}(SG).
@@ -788,7 +786,7 @@ EXPORT_SYMBOL_GPL(swiotlb_sync_single_range_for_device);
  *       The routine returns the number of addr/length pairs actually
  *       used, at most nents.
  *
- * Device ownership issues as mentioned above for swiotlb_map_single are the
+ * Device ownership issues as mentioned above for swiotlb_map_page are the
  * same here.
  */
 int
@@ -836,7 +834,7 @@ EXPORT_SYMBOL(swiotlb_map_sg);
 
 /*
  * Unmap a set of streaming mode DMA translations.  Again, cpu read rules
- * concerning calls here are the same as for swiotlb_unmap_single() above.
+ * concerning calls here are the same as for swiotlb_unmap_page() above.
  */
 void
 swiotlb_unmap_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
-- 
1.5.6.6

[PATCH 2/7] swiotlb: fix compile warning

[Becky Bruce]

Squash a build warning seen on 32-bit powerpc caused by calling min()
with 2 different types. Use min_t() instead

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
---
 lib/swiotlb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index f59cf30..fa62498 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -341,7 +341,7 @@ static void swiotlb_bounce(phys_addr_t phys, char *dma_addr, size_t size,
 		unsigned long flags;
 
 		while (size) {
-			sz = min(PAGE_SIZE - offset, size);
+			sz = min_t(size_t, PAGE_SIZE - offset, size);
 
 			local_irq_save(flags);
 			buffer = kmap_atomic(pfn_to_page(pfn),
-- 
1.5.6.6

[PATCH 3/7] swiotlb: map_page fix for highmem systems

[Becky Bruce]

The current code calls virt_to_phys() on address that might
be in highmem, which is bad.  This wasn't needed, anyway, because
we already have the physical address we need. Get rid of the
now-unused virtual address as well.

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
---
 lib/swiotlb.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index fa62498..eb6dbbd 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -640,7 +640,6 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 			    struct dma_attrs *attrs)
 {
 	phys_addr_t phys = page_to_phys(page) + offset;
-	void *ptr = page_address(page) + offset;
 	dma_addr_t dev_addr = swiotlb_phys_to_bus(dev, phys);
 	void *map;
 
@@ -651,7 +650,7 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 	 * buffering it.
 	 */
 	if (!address_needs_mapping(dev, dev_addr, size) &&
-	    !range_needs_mapping(virt_to_phys(ptr), size))
+	    !range_needs_mapping(phys, size))
 		return dev_addr;
 
 	/*
-- 
1.5.6.6

[PATCH 4/7] swiotlb: Allow arch override of address_needs_mapping

[Becky Bruce]

Some architectures require additional checking to determine
if a device can dma to an address and need to provide their
own address_needs_mapping..

Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
---
 lib/swiotlb.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index eb6dbbd..af2ec25 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -145,6 +145,12 @@ static void *swiotlb_bus_to_virt(dma_addr_t address)
 	return phys_to_virt(swiotlb_bus_to_phys(address));
 }
 
+int __weak swiotlb_arch_address_needs_mapping(struct device *hwdev,
+					       dma_addr_t addr, size_t size)
+{
+	return !is_buffer_dma_capable(dma_get_mask(hwdev), addr, size);
+}
+
 int __weak swiotlb_arch_range_needs_mapping(phys_addr_t paddr, size_t size)
 {
 	return 0;
@@ -309,10 +315,10 @@ cleanup1:
 	return -ENOMEM;
 }
 
-static int
+static inline int
 address_needs_mapping(struct device *hwdev, dma_addr_t addr, size_t size)
 {
-	return !is_buffer_dma_capable(dma_get_mask(hwdev), addr, size);
+	return swiotlb_arch_address_needs_mapping(hwdev, addr, size);
 }
 
 static inline int range_needs_mapping(phys_addr_t paddr, size_t size)
-- 
1.5.6.6