[patch 2.6.13] x86_64: implement dma_sync_single_range_for_{cpu,device}

[patch 2.6.13] x86_64: implement dma_sync_single_range_for_{cpu,device}

[John W. Linville]

Implement dma_sync_single_range_for_{cpu,device}, based on curent
implementations of dma_sync_single_for_{cpu,device}.

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
It is hard to use this API if common platforms do not implement it. :-)
Hopefully I did not miss something obvious?

This is a naive implementation, so flame away...

 include/asm-x86_64/dma-mapping.h |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+)

diff --git a/include/asm-x86_64/dma-mapping.h b/include/asm-x86_64/dma-mapping.h
--- a/include/asm-x86_64/dma-mapping.h
+++ b/include/asm-x86_64/dma-mapping.h
@@ -85,6 +85,34 @@ static inline void dma_sync_single_for_d
 	flush_write_buffers();
 }
 
+static inline void dma_sync_single_range_for_cpu(struct device *hwdev,
+						 dma_addr_t dma_handle,
+						 unsigned long offset,
+						 size_t size, int direction)
+{
+	if (direction == DMA_NONE)
+		out_of_line_bug();
+
+	if (swiotlb)
+		return swiotlb_sync_single_for_cpu(hwdev,dma_handle+offset,size,direction);
+
+	flush_write_buffers();
+}
+
+static inline void dma_sync_single_range_for_device(struct device *hwdev,
+						    dma_addr_t dma_handle,
+						    unsigned long offset,
+						    size_t size, int direction)
+{
+        if (direction == DMA_NONE)
+		out_of_line_bug();
+
+	if (swiotlb)
+		return swiotlb_sync_single_for_device(hwdev,dma_handle+offset,size,direction);
+
+	flush_write_buffers();
+}
+
 static inline void dma_sync_sg_for_cpu(struct device *hwdev,
 				       struct scatterlist *sg,
 				       int nelems, int direction)
-- 
John W. Linville
linville@tuxdriver.com

Re: [patch 2.6.13] x86_64: implement dma_sync_single_range_for_{cpu,device}

[Andi Kleen]

On Monday 29 August 2005 22:09, John W. Linville wrote:
> Implement dma_sync_single_range_for_{cpu,device}, based on curent
> implementations of dma_sync_single_for_{cpu,device}.

Hmm, who or what needs that? It doesn't seem to be documented
in Documentation/DMA* and I also don't remember seeing any 
discussion of it.

If it's commonly used it might better to add new swiotlb_* 
functions that only copy the requested range.

-Andi

Re: [patch 2.6.13] x86_64: implement dma_sync_single_range_for_{cpu,device}

[John W. Linville]

On Mon, Aug 29, 2005 at 10:54:53PM +0200, Andi Kleen wrote:
> On Monday 29 August 2005 22:09, John W. Linville wrote:
> > Implement dma_sync_single_range_for_{cpu,device}, based on curent
> > implementations of dma_sync_single_for_{cpu,device}.
> 
> Hmm, who or what needs that? It doesn't seem to be documented
> in Documentation/DMA* and I also don't remember seeing any 
> discussion of it.

In Documentation/DMA-API.txt it is still referred to as
dma_sync_single_range.  I imagine the *_for_{cpu,device} stuff got added
at about the same time as it did for dma_sync_single, dma_sync_sg,
and the like.

These calls are implemented for basically all the other arches.
And, except for the noted *_for_{cpu,device} discrepancies, these are
documented in Documentation/DMA-API.txt.  It definitely seems to be
an unfortunate omission from include/asm-x86_64/dma-mapping.h.

As for who needs it, well, I suppose I do.  I want to use that API
in a patch I'm working-on.  No one will want to merge my patch if it
will not compile on x86_64... :-(

> If it's commonly used it might better to add new swiotlb_* 
> functions that only copy the requested range.

Perhaps...but I think that sounds more like a discussion of _how_ to
implement the API, rather than _whether_ it should be implemented.
Using some new variant of the swiotlb_* API might be appropriate
for the x86_64 implementation.  But, since this is a portable API,
I don't think calling the (apparently Intel-specific) swiotlb_*
functions would be an appropriate replacement.

I'd be happy to have do the implementation differently (or to have
someone else do so).  Do you have specific suggestions for how to
do so?

Thanks,

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [discuss] Re: [patch 2.6.13] x86_64: implement dma_sync_single_range_for_{cpu,device}

[Andi Kleen]

On Monday 29 August 2005 23:48, John W. Linville wrote:

> Perhaps...but I think that sounds more like a discussion of _how_ to
> implement the API, rather than _whether_ it should be implemented.
> Using some new variant of the swiotlb_* API might be appropriate
> for the x86_64 implementation.  But, since this is a portable API,
> I don't think calling the (apparently Intel-specific) swiotlb_*
> functions would be an appropriate replacement.

What I meant is that instead of the dumb implementation you did
it would be better to implement it in swiotlb_* too and copy 
only the requested byte range there and then call these new
functions from the x86-64 wrapper.

-Andi

Re: [discuss] Re: [patch 2.6.13] x86_64: implement dma_sync_single_range_for_{cpu,device}

[John W. Linville]

On Tue, Aug 30, 2005 at 03:14:34AM +0200, Andi Kleen wrote:
> On Monday 29 August 2005 23:48, John W. Linville wrote:

> > I don't think calling the (apparently Intel-specific) swiotlb_*
> > functions would be an appropriate replacement.
> 
> What I meant is that instead of the dumb implementation you did
> it would be better to implement it in swiotlb_* too and copy 
> only the requested byte range there and then call these new
> functions from the x86-64 wrapper.

Thanks.  That is more helpful than the previous message.

I was leery of disturbing the swiotlb_* API needlessly, especially
since that involves ia64 as well.  But if you think that would be
better, then I'll work in that direction.

Patches to follow...

John

P.S.  BTW, "dumb" is a term that is both subjective and perjorative.
IMHO, it is "dumb" to use the word "dumb" in public discourse...
-- 
John W. Linville
linville@tuxdriver.com

[patch 2.6.13] swiotlb: add swiotlb_sync_single_range_for_{cpu,device}

[John W. Linville]

Add swiotlb_sync_single_range_for_{cpu,device} implementations.  This is
used to support implementation of dma_sync_single_range_for_{cpu,device}
on x86_64.

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---

 arch/ia64/lib/swiotlb.c      |   33 +++++++++++++++++++++++++++++++++
 include/asm-x86_64/swiotlb.h |    8 ++++++++
 2 files changed, 41 insertions(+)

diff --git a/arch/ia64/lib/swiotlb.c b/arch/ia64/lib/swiotlb.c
--- a/arch/ia64/lib/swiotlb.c
+++ b/arch/ia64/lib/swiotlb.c
@@ -522,6 +522,37 @@ swiotlb_sync_single_for_device(struct de
 }
 
 /*
+ * Same as above, but for a sub-range of the mapping.
+ */
+void
+swiotlb_sync_single_range_for_cpu(struct device *hwdev, dma_addr_t dev_addr,
+				  unsigned long offset, size_t size, int dir)
+{
+	char *dma_addr = phys_to_virt(dev_addr) + offset;
+
+	if (dir == DMA_NONE)
+		BUG();
+	if (dma_addr >= io_tlb_start && dma_addr < io_tlb_end)
+		sync_single(hwdev, dma_addr, size, dir);
+	else if (dir == DMA_FROM_DEVICE)
+		mark_clean(dma_addr, size);
+}
+
+void
+swiotlb_sync_single_range_for_device(struct device *hwdev, dma_addr_t dev_addr,
+				     unsigned long offset, size_t size, int dir)
+{
+	char *dma_addr = phys_to_virt(dev_addr) + offset;
+
+	if (dir == DMA_NONE)
+		BUG();
+	if (dma_addr >= io_tlb_start && dma_addr < io_tlb_end)
+		sync_single(hwdev, dma_addr, size, dir);
+	else if (dir == DMA_FROM_DEVICE)
+		mark_clean(dma_addr, size);
+}
+
+/*
  * 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
  * interface.  Here the scatter gather list elements are each tagged with the
@@ -650,6 +681,8 @@ EXPORT_SYMBOL(swiotlb_map_sg);
 EXPORT_SYMBOL(swiotlb_unmap_sg);
 EXPORT_SYMBOL(swiotlb_sync_single_for_cpu);
 EXPORT_SYMBOL(swiotlb_sync_single_for_device);
+EXPORT_SYMBOL_GPL(swiotlb_sync_single_range_for_cpu);
+EXPORT_SYMBOL_GPL(swiotlb_sync_single_range_for_device);
 EXPORT_SYMBOL(swiotlb_sync_sg_for_cpu);
 EXPORT_SYMBOL(swiotlb_sync_sg_for_device);
 EXPORT_SYMBOL(swiotlb_dma_mapping_error);
diff --git a/include/asm-x86_64/swiotlb.h b/include/asm-x86_64/swiotlb.h
--- a/include/asm-x86_64/swiotlb.h
+++ b/include/asm-x86_64/swiotlb.h
@@ -15,6 +15,14 @@ extern void swiotlb_sync_single_for_cpu(
 extern void swiotlb_sync_single_for_device(struct device *hwdev,
 					    dma_addr_t dev_addr,
 					    size_t size, int dir);
+extern void swiotlb_sync_single_range_for_cpu(struct device *hwdev,
+					      dma_addr_t dev_addr,
+					      unsigned long offset,
+					      size_t size, int dir);
+extern void swiotlb_sync_single_range_for_device(struct device *hwdev,
+						 dma_addr_t dev_addr,
+						 unsigned long offset,
+						 size_t size, int dir);
 extern void swiotlb_sync_sg_for_cpu(struct device *hwdev,
 				     struct scatterlist *sg, int nelems,
 				     int dir);
-- 
John W. Linville
linville@tuxdriver.com

[patch 2.6.13] x86_64: implement dma_sync_single_range_for_{cpu,device}

[John W. Linville]

Implement dma_sync_single_range_for_{cpu,device} on x86_64.

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
Makes use of swiotlb_sync_single_range_for_{cpu,device} implemented
in preceding patch.

 include/asm-x86_64/dma-mapping.h |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+)

diff --git a/include/asm-x86_64/dma-mapping.h b/include/asm-x86_64/dma-mapping.h
--- a/include/asm-x86_64/dma-mapping.h
+++ b/include/asm-x86_64/dma-mapping.h
@@ -85,6 +85,34 @@ static inline void dma_sync_single_for_d
 	flush_write_buffers();
 }
 
+static inline void dma_sync_single_range_for_cpu(struct device *hwdev,
+						 dma_addr_t dma_handle,
+						 unsigned long offset,
+						 size_t size, int direction)
+{
+	if (direction == DMA_NONE)
+		out_of_line_bug();
+
+	if (swiotlb)
+		return swiotlb_sync_single_range_for_cpu(hwdev,dma_handle,offset,size,direction);
+
+	flush_write_buffers();
+}
+
+static inline void dma_sync_single_range_for_device(struct device *hwdev,
+						    dma_addr_t dma_handle,
+						    unsigned long offset,
+						    size_t size, int direction)
+{
+        if (direction == DMA_NONE)
+		out_of_line_bug();
+
+	if (swiotlb)
+		return swiotlb_sync_single_range_for_device(hwdev,dma_handle,offset,size,direction);
+
+	flush_write_buffers();
+}
+
 static inline void dma_sync_sg_for_cpu(struct device *hwdev,
 				       struct scatterlist *sg,
 				       int nelems, int direction)
-- 
John W. Linville
linville@tuxdriver.com

RE: [patch 2.6.13] swiotlb: add swiotlb_sync_single_range_for_{cpu,device}

[Luck, Tony]

>+swiotlb_sync_single_range_for_cpu(struct device *hwdev, 
>+swiotlb_sync_single_range_for_device(struct device *hwdev, 

Huh?  These look identical ... same args, same code, just a
different name.

-Tony

Re: [patch 2.6.13] swiotlb: add swiotlb_sync_single_range_for_{cpu,device}

[John W. Linville]

On Tue, Aug 30, 2005 at 11:03:35AM -0700, Luck, Tony wrote:
> 
> >+swiotlb_sync_single_range_for_cpu(struct device *hwdev, 
> >+swiotlb_sync_single_range_for_device(struct device *hwdev, 
> 
> Huh?  These look identical ... same args, same code, just a
> different name.

Have you looked at the implementations for swiotlb_sync_single_for_cpu
and swiotlb_sync_single_for_device?  Those are already identical.

I'm just following the existing style/practice in that file.  I could
do an additional patch to rectify the replication in those functions
if you'd like?

Who is responsible for the swiotlb code?

John
-- 
John W. Linville
linville@tuxdriver.com