[PATCH 2/2] Set of fixes for DMA when dma_addr_t != physical address

[PATCH 2/2] Set of fixes for DMA when dma_addr_t != physical address

[David VomLehn]

Fixes for using DMA on systems where the DMA address and the physical address
differ.

Signed-off-by: Dezhong Diao <dediao@cisco.com>
Signed-off-by: David VomLehn <dvomlehn@cisco.com>
---
 arch/mips/mm/dma-default.c |   22 ++++++++++++++--------
 1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/arch/mips/mm/dma-default.c b/arch/mips/mm/dma-default.c
index 414d7ff..eaa7fb4 100644
--- a/arch/mips/mm/dma-default.c
+++ b/arch/mips/mm/dma-default.c
@@ -24,8 +24,11 @@ static inline unsigned long dma_addr_to_virt(struct device *dev,
 	dma_addr_t dma_addr)
 {
 	unsigned long addr = plat_dma_addr_to_phys(dev, dma_addr);
+	unsigned int offset = (dma_addr & ~PAGE_MASK);
+	struct page *pg;
 
-	return (unsigned long)phys_to_virt(addr);
+	pg = pfn_to_page(addr >> PAGE_SHIFT);
+	return (unsigned long)(page_address(pg) + offset);
 }
 
 /*
@@ -136,7 +139,6 @@ EXPORT_SYMBOL(dma_free_coherent);
 static inline void __dma_sync(unsigned long addr, size_t size,
 	enum dma_data_direction direction)
 {
-
 	BUG_ON(addr < KSEG0);
 
 	switch (direction) {
@@ -197,8 +199,8 @@ int dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 		addr = (unsigned long) sg_virt(sg);
 		if (!plat_device_is_coherent(dev) && (addr >= KSEG0))
 			__dma_sync(addr, sg->length, direction);
-
-		sg->dma_address = sg_phys(sg);
+		sg->dma_address = plat_map_dma_mem_page(dev, sg_page(sg)) +
+			sg->offset;
 	}
 
 	return nents;
@@ -253,7 +255,8 @@ void dma_sync_single_for_cpu(struct device *dev, dma_addr_t dma_handle,
 		unsigned long addr;
 
 		addr = dma_addr_to_virt(dev, dma_handle);
-		__dma_sync(addr, size, direction);
+		if (addr >= KSEG0)
+			__dma_sync(addr, size, direction);
 	}
 }
 
@@ -269,7 +272,8 @@ void dma_sync_single_for_device(struct device *dev, dma_addr_t dma_handle,
 		unsigned long addr;
 
 		addr = dma_addr_to_virt(dev, dma_handle);
-		__dma_sync(addr, size, direction);
+		if (addr >= KSEG0)
+			__dma_sync(addr, size, direction);
 	}
 }
 
@@ -284,7 +288,8 @@ void dma_sync_single_range_for_cpu(struct device *dev, dma_addr_t dma_handle,
 		unsigned long addr;
 
 		addr = dma_addr_to_virt(dev, dma_handle);
-		__dma_sync(addr + offset, size, direction);
+		if (addr >= KSEG0)
+			__dma_sync(addr + offset, size, direction);
 	}
 }
 
@@ -300,7 +305,8 @@ void dma_sync_single_range_for_device(struct device *dev, dma_addr_t dma_handle,
 		unsigned long addr;
 
 		addr = dma_addr_to_virt(dev, dma_handle);
-		__dma_sync(addr + offset, size, direction);
+		if (addr >= KSEG0)
+			__dma_sync(addr + offset, size, direction);
 	}
 }
 

Re: [PATCH 2/2] Set of fixes for DMA when dma_addr_t != physical address

[Thomas Bogendoerfer]

On Wed, Nov 25, 2009 at 03:00:28PM -0500, David VomLehn wrote:
> Fixes for using DMA on systems where the DMA address and the physical address
> differ.

what's the problem ? Even the old Olivetti M700 has an iommu, so dma
address and physical address are always different... and it works without
changes. You just need to tweak the plat_dma_XXX() macros/functions.

And I don't see a reason for the KSEG0 checks in your other patch,
but maybe I'm missing something...

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessary a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH 2/2] Set of fixes for DMA when dma_addr_t != physical address

[Ralf Baechle]

On Wed, Nov 25, 2009 at 03:00:28PM -0500, David VomLehn wrote:

> Fixes for using DMA on systems where the DMA address and the physical address
> differ.
> 
> Signed-off-by: Dezhong Diao <dediao@cisco.com>
> Signed-off-by: David VomLehn <dvomlehn@cisco.com>
> ---
>  arch/mips/mm/dma-default.c |   22 ++++++++++++++--------
>  1 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/mips/mm/dma-default.c b/arch/mips/mm/dma-default.c
> index 414d7ff..eaa7fb4 100644
> --- a/arch/mips/mm/dma-default.c
> +++ b/arch/mips/mm/dma-default.c
> @@ -24,8 +24,11 @@ static inline unsigned long dma_addr_to_virt(struct device *dev,
>  	dma_addr_t dma_addr)
>  {
>  	unsigned long addr = plat_dma_addr_to_phys(dev, dma_addr);
> +	unsigned int offset = (dma_addr & ~PAGE_MASK);
> +	struct page *pg;
>  
> -	return (unsigned long)phys_to_virt(addr);
> +	pg = pfn_to_page(addr >> PAGE_SHIFT);
> +	return (unsigned long)(page_address(pg) + offset);

So this is the core of the two patches.

It'll make I/O to kmapped pages work - but you're not supposed to do that.
The burden to perform I/O on highmem pages is on the subsystem that does
the I/O, not the DMA layer!

>  }
>  
>  /*
> @@ -136,7 +139,6 @@ EXPORT_SYMBOL(dma_free_coherent);
>  static inline void __dma_sync(unsigned long addr, size_t size,
>  	enum dma_data_direction direction)
>  {
> -

This is the blank line the previous patch shouldn't have added.

>  	BUG_ON(addr < KSEG0);
>  
>  	switch (direction) {
> @@ -197,8 +199,8 @@ int dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
>  		addr = (unsigned long) sg_virt(sg);
>  		if (!plat_device_is_coherent(dev) && (addr >= KSEG0))
>  			__dma_sync(addr, sg->length, direction);
> -
> -		sg->dma_address = sg_phys(sg);
> +		sg->dma_address = plat_map_dma_mem_page(dev, sg_page(sg)) +
> +			sg->offset;

Ah, this segment undoes the damage done by the previous patch.  If I apply
both and diff, the change looks like:

-               sg->dma_address = plat_map_dma_mem(dev,
-                                                  (void *)addr, sg->length);
+               sg->dma_address = plat_map_dma_mem_page(dev, sg_page(sg)) +
+                       sg->offset;

sg_page returns a struct page *.  Adding sg->offset yields nonense.  I think
you mean something like ((unsigned long) sg_page(sg)) + sg->offset.

>  	}
>  
>  	return nents;
> @@ -253,7 +255,8 @@ void dma_sync_single_for_cpu(struct device *dev, dma_addr_t dma_handle,
>  		unsigned long addr;
>  
>  		addr = dma_addr_to_virt(dev, dma_handle);
> -		__dma_sync(addr, size, direction);
> +		if (addr >= KSEG0)
> +			__dma_sync(addr, size, direction);

Again this KSEG0 comparison will not work as intended on 64-bit.  And on 32-bit
I don't see why it would be required.  No userspace address should ever be
passed into this function.

>  	}
>  }
>  
> @@ -269,7 +272,8 @@ void dma_sync_single_for_device(struct device *dev, dma_addr_t dma_handle,
>  		unsigned long addr;
>  
>  		addr = dma_addr_to_virt(dev, dma_handle);
> -		__dma_sync(addr, size, direction);
> +		if (addr >= KSEG0)
> +			__dma_sync(addr, size, direction);

Ditto.

>  	}
>  }
>  
> @@ -284,7 +288,8 @@ void dma_sync_single_range_for_cpu(struct device *dev, dma_addr_t dma_handle,
>  		unsigned long addr;
>  
>  		addr = dma_addr_to_virt(dev, dma_handle);
> -		__dma_sync(addr + offset, size, direction);
> +		if (addr >= KSEG0)
> +			__dma_sync(addr + offset, size, direction);

Ditto.

>  	}
>  }
>  
> @@ -300,7 +305,8 @@ void dma_sync_single_range_for_device(struct device *dev, dma_addr_t dma_handle,
>  		unsigned long addr;
>  
>  		addr = dma_addr_to_virt(dev, dma_handle);
> -		__dma_sync(addr + offset, size, direction);
> +		if (addr >= KSEG0)
> +			__dma_sync(addr + offset, size, direction);

Ditto.

>  	}
>  }
>  

  Ralf