Skip to contentSkip to conversation

x86_64: implement dma_sync_single_range_for_{cpu,device}

PATCH In review John W. Linville 2 participants 7 messages Aug 29, 2005 – Aug 30, 2005

lore
Share & tools
lore

John W. Linville

author
#0 lore raw

PATCH 1/1 x86_64: implement dma_sync_single_range_for_{cpu,device}

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 <[email protected]>

---
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(+)
include/asm-x86_64/dma-mapping.h +28 -0
@@ -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)
Signature · 2 lines
John W. Linville
[email protected]
#1 lore raw
Quoted 2 lines · John W. Linville

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

author
#2 lore raw
Quoted 7 lines · Andi Kleen
Quoted 2 lines · John W. Linville

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... :-(

Quoted 2 lines

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

Signature · 2 lines
John W. Linville
[email protected]
#3 lore raw
Quoted 6 lines · John W. Linville

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

author
#4 lore raw
Quoted 9 lines · Andi Kleen
Quoted 2 lines · John W. Linville

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...

Signature · 2 lines
John W. Linville
[email protected]
author
#5 lore raw

PATCH 1/1 swiotlb: add swiotlb_sync_single_range_for_{cpu,device}

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 <[email protected]>

---

 arch/ia64/lib/swiotlb.c      |   33 +++++++++++++++++++++++++++++++++
 include/asm-x86_64/swiotlb.h |    8 ++++++++
 2 files changed, 41 insertions(+)
arch/ia64/lib/swiotlb.c +33 -0
@@ -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);
include/asm-x86_64/swiotlb.h +8 -0
@@ -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);
Signature · 2 lines
John W. Linville
[email protected]
author
#6 lore raw

PATCH 1/1 x86_64: implement dma_sync_single_range_for_{cpu,device}

Implement dma_sync_single_range_for_{cpu,device} on x86_64.

Signed-off-by John W. Linville <[email protected]>

---
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(+)
include/asm-x86_64/dma-mapping.h +28 -0
@@ -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)
Signature · 2 lines
John W. Linville
[email protected]