[PATCH][PPC32] enhancement to virt_to_bus/bus_to_virt (resent with spell-checked subject line)

[PATCH][PPC32] enhancement to virt_to_bus/bus_to_virt (resent with spell-checked subject line)

[Linh Dang]

In 2.6.9 on non-APUS ppc32 platforms, virt_to_bus() will just subtract
KERNELBASE  from the the virtual address. bus_to_virt() will perform
the reverse operation.

This patch will make virt_to_bus():

     - perform the current operation if the virtual address is between
       KERNELBASE and ioremap_bot.

     - use iopa() (as on APUS platform) otherwise.

The patch will make bus_to_virt():

     - perform the current operation if the bus address is between
       PCI_DRAM_OFFSET and (ioremap_bot - KERNELBASE + PCI_DRAM_OFFSET).

     - use mm_ptov() (as on APUS platform) otherwise.


The patch also changes virt_to_phys()/phys_to_virt() in a similar way.

-- 
Linh Dang

--- include/asm-ppc/io.h~2.6.9	2004-11-12 14:24:46.000000000 -0500
+++ include/asm-ppc/io.h	2004-12-02 09:42:32.000000000 -0500
@@ -6,10 +6,11 @@
 #include <linux/types.h>
 
 #include <asm/page.h>
 #include <asm/byteorder.h>
 #include <asm/mmu.h>
+#include <asm/pgtable.h>
 
 #define SIO_CONFIG_RA	0x398
 #define SIO_CONFIG_RD	0x399
 
 #define SLOW_DOWN_IO
@@ -222,49 +223,60 @@ extern void io_block_mapping(unsigned lo
  * have to be modified [mapped] appropriately.
  */
 extern inline unsigned long virt_to_bus(volatile void * address)
 {
 #ifndef CONFIG_APUS
-        if (address == (void *)0)
+	if (unlikely(address == (void *)0))
 		return 0;
+	if (likely((address >= (void*) KERNELBASE) &&
+		   (address < ((void*) ioremap_bot))))
         return (unsigned long)address - KERNELBASE + PCI_DRAM_OFFSET;
-#else
-	return iopa ((unsigned long) address);
+	else
 #endif
+		return iopa ((unsigned long) address);
 }
 
 extern inline void * bus_to_virt(unsigned long address)
 {
 #ifndef CONFIG_APUS
-        if (address == 0)
+	if (unlikely (address == 0))
 		return NULL;
+	if (likely((address >= PCI_DRAM_OFFSET) &&
+		   (address < (ioremap_bot - KERNELBASE + PCI_DRAM_OFFSET))))
         return (void *)(address - PCI_DRAM_OFFSET + KERNELBASE);
-#else
-	return (void*) mm_ptov (address);
+	else
 #endif
+		return (void*) mm_ptov (address);
 }
 
 /*
  * Change virtual addresses to physical addresses and vv, for
  * addresses in the area where the kernel has the RAM mapped.
  */
 extern inline unsigned long virt_to_phys(volatile void * address)
 {
 #ifndef CONFIG_APUS
+	if (unlikely(address == (void *)0))
+		return 0;
+	if (likely((address >= (void*) KERNELBASE) &&
+		   (address < ((void*) ioremap_bot))))
 	return (unsigned long) address - KERNELBASE;
-#else
-	return iopa ((unsigned long) address);
+	else
 #endif
+		return iopa ((unsigned long) address);
 }
 
 extern inline void * phys_to_virt(unsigned long address)
 {
 #ifndef CONFIG_APUS
+	if (unlikely (address == 0))
+		return NULL;
+	if (likely(address < (ioremap_bot - KERNELBASE)))
 	return (void *) (address + KERNELBASE);
-#else
-	return (void*) mm_ptov (address);
+	else
 #endif
+		return (void*) mm_ptov (address);
 }
 
 /*
  * Change "struct page" to physical address.
  */

Re: [PATCH][PPC32[NEWBIE] enhancement to virt_to_bus/bus_to_virt (try 2)

[Linh Dang]

In 2.6.9 on non-APUS ppc32 platforms, virt_to_bus() will just subtract
KERNELBASE  from the the virtual address. bus_to_virt() will perform
the reverse operation.

This patch will make virt_to_bus():

     - perform the current operation if the virtual address is between
       KERNELBASE and ioremap_bot.

     - use iopa() (as on APUS platform) otherwise.

The patch will make bus_to_virt():

     - perform the current operation if the bus address is between
       PCI_DRAM_OFFSET and (ioremap_bot - KERNELBASE + PCI_DRAM_OFFSET).

     - use mm_ptov() (as on APUS platform) otherwise.


The patch also changes virt_to_phys()/phys_to_virt() in a similar way.

--  Linh Dang




--- include/asm-ppc/io.h~2.6.9	2004-11-12 14:24:46.000000000 -0500
+++ include/asm-ppc/io.h	2004-12-02 11:20:21.000000000 -0500
@@ -6,10 +6,11 @@
 #include <linux/types.h>
 
 #include <asm/page.h>
 #include <asm/byteorder.h>
 #include <asm/mmu.h>
+#include <asm/pgtable.h>
 
 #define SIO_CONFIG_RA	0x398
 #define SIO_CONFIG_RD	0x399
 
 #define SLOW_DOWN_IO
@@ -222,49 +223,56 @@ extern void io_block_mapping(unsigned lo
  * have to be modified [mapped] appropriately.
  */
 extern inline unsigned long virt_to_bus(volatile void * address)
 {
 #ifndef CONFIG_APUS
-        if (address == (void *)0)
+	if (unlikely(address == (void *)0))
 		return 0;
+	if (likely((address >= (void*) KERNELBASE) &&
+		   (address < ((void*) ioremap_bot))))
         return (unsigned long)address - KERNELBASE + PCI_DRAM_OFFSET;
-#else
-	return iopa ((unsigned long) address);
+	else
 #endif
+		return iopa ((unsigned long) address);
 }
 
 extern inline void * bus_to_virt(unsigned long address)
 {
 #ifndef CONFIG_APUS
-        if (address == 0)
+	if (unlikely (address == 0))
 		return NULL;
+	if (likely((address >= PCI_DRAM_OFFSET) &&
+		   (address < (ioremap_bot - KERNELBASE + PCI_DRAM_OFFSET))))
         return (void *)(address - PCI_DRAM_OFFSET + KERNELBASE);
-#else
-	return (void*) mm_ptov (address);
+	else
 #endif
+		return (void*) mm_ptov (address);
 }
 
 /*
  * Change virtual addresses to physical addresses and vv, for
  * addresses in the area where the kernel has the RAM mapped.
  */
 extern inline unsigned long virt_to_phys(volatile void * address)
 {
 #ifndef CONFIG_APUS
+	if (likely((address >= (void*) KERNELBASE) &&
+		   (address < ((void*) ioremap_bot))))
 	return (unsigned long) address - KERNELBASE;
-#else
-	return iopa ((unsigned long) address);
+	else
 #endif
+                return iopa ((unsigned long) address);
 }
 
 extern inline void * phys_to_virt(unsigned long address)
 {
 #ifndef CONFIG_APUS
+	if (likely(address < (ioremap_bot - KERNELBASE)))
 	return (void *) (address + KERNELBASE);
-#else
-	return (void*) mm_ptov (address);
+	else
 #endif
+                return (void*) mm_ptov (address);
 }
 
 /*
  * Change "struct page" to physical address.
  */

Re: [PATCH][PPC32[NEWBIE] enhancement to virt_to_bus/bus_to_virt (try 2)

[Paul Mackerras]

Linh Dang writes:

> In 2.6.9 on non-APUS ppc32 platforms, virt_to_bus() will just subtract
> KERNELBASE  from the the virtual address. bus_to_virt() will perform
> the reverse operation.
> 
> This patch will make virt_to_bus():
> 
>      - perform the current operation if the virtual address is between
>        KERNELBASE and ioremap_bot.

Why do you want to do this?  The only code that should be using
virt_to_bus or bus_to_virt is the DMA API code, and it's happy with
them the way they are.

> The patch also changes virt_to_phys()/phys_to_virt() in a similar way.

What do you want to use them for?  They are only for use in low-level
memory management code.

Paul.

Re: [PATCH][PPC32[NEWBIE] enhancement to virt_to_bus/bus_to_virt (try 2)

[Linh Dang]

On Fri, 3 Dec 2004 07:31:14 +1100, Paul Mackerras <paulus@samba.org> wrote:
> Linh Dang writes:
> 
> > In 2.6.9 on non-APUS ppc32 platforms, virt_to_bus() will just subtract
> > KERNELBASE  from the the virtual address. bus_to_virt() will perform
> > the reverse operation.
> >
> > This patch will make virt_to_bus():
> >
> >      - perform the current operation if the virtual address is between
> >        KERNELBASE and ioremap_bot.
> 
> Why do you want to do this?  The only code that should be using
> virt_to_bus or bus_to_virt is the DMA API code, and it's happy with
> them the way they are.

I wrote a DMA engine (to used by other drivers) that (would like to) accept
all kind of buffers as input (vmalloced, dual-access shared RAM mapped
by BATs, etc). The DMA engine has to decode the virtual address of the
input buffer to (possibly multiple) physical  address(es). virt_to_phys()
has the right name for the job except it only works for the kernel virtual
addresses initially mapped at KERNELBASE

> 
> > The patch also changes virt_to_phys()/phys_to_virt() in a similar way.
> 
> What do you want to use them for?  They are only for use in low-level
> memory management code.

Any driver for a DMA-capable device would use them and the way
virt_to_phys/phys_to_virt is currently written, you can't used them
with vmalloced buffers.

> 
> Paul.
> 
Thanx for the feedback
-- 
Linh Dang

Re: [PATCH][PPC32[NEWBIE] enhancement to virt_to_bus/bus_to_virt (try 2)

[Eugene Surovegin]

On Fri, Dec 03, 2004 at 09:46:00AM -0500, Linh Dang wrote:
> On Fri, 3 Dec 2004 07:31:14 +1100, Paul Mackerras <paulus@samba.org> wrote:
> > Linh Dang writes:
> > 
> > > In 2.6.9 on non-APUS ppc32 platforms, virt_to_bus() will just subtract
> > > KERNELBASE  from the the virtual address. bus_to_virt() will perform
> > > the reverse operation.
> > >
> > > This patch will make virt_to_bus():
> > >
> > >      - perform the current operation if the virtual address is between
> > >        KERNELBASE and ioremap_bot.
> > 
> > Why do you want to do this?  The only code that should be using
> > virt_to_bus or bus_to_virt is the DMA API code, and it's happy with
> > them the way they are.
> 
> I wrote a DMA engine (to used by other drivers) that (would like to) accept
> all kind of buffers as input (vmalloced, dual-access shared RAM mapped
> by BATs, etc). The DMA engine has to decode the virtual address of the
> input buffer to (possibly multiple) physical  address(es). virt_to_phys()
> has the right name for the job except it only works for the kernel virtual
> addresses initially mapped at KERNELBASE

Unfortunately your patch doesn't achieve your goals. vmalloc space on 
PPC32 will be between KERNEL_BASE and ioremap_bot anyway, so 
additional check is mostly useless anyway (it will only call iopa() 
for ioremaps done early during the boot).

Also, even assuming you got the range right and called iopa() for 
vmalloced space, tell me how are you gonna deal with non-physically 
continuous vmalloced buffers in your DMA library?

--
Eugene

Re: [PATCH][PPC32[NEWBIE] enhancement to virt_to_bus/bus_to_virt (try 2)

[Linh Dang]

On Sun, 5 Dec 2004 02:11:10 -0800, Eugene Surovegin <ebs@ebshome.net> wrote:
[...]
> 
> Unfortunately your patch doesn't achieve your goals. vmalloc space on
> PPC32 will be between KERNEL_BASE and ioremap_bot anyway, so
> additional check is mostly useless anyway (it will only call iopa()
> for ioremaps done early during the boot).

Duh! how about using total_lowmem? i.e. use the current way for memory
between KERNEL_BASE and (KERNEL_BASE+total_lowmem),
and iopa for other addresses?

> 
> Also, even assuming you got the range right and called iopa() for
> vmalloced space, tell me how are you gonna deal with non-physically
> continuous vmalloced buffers in your DMA library?

>From a single virtual buffer, the DMA library will build a chained list of
physically contiguous buffers (it  can be one or more physical buffers).
All the DMA engines I'm familiar with (mpc8260, mpc8580, marvell, etc.)
accept a list of physical buffers.

The decoding algorithm (from a single virtual buffer to a chained list of
physical buffers) is dead simple.

Thanx for the reply
-- 
Linh Dang

Re: [PATCH][PPC32[NEWBIE] enhancement to virt_to_bus/bus_to_virt (try 2)

[Eugene Surovegin]

On Sun, Dec 05, 2004 at 02:18:45PM -0500, Linh Dang wrote:
> From a single virtual buffer, the DMA library will build a chained list of
> physically contiguous buffers (it  can be one or more physical buffers).
> All the DMA engines I'm familiar with (mpc8260, mpc8580, marvell, etc.)
> accept a list of physical buffers.
> 
> The decoding algorithm (from a single virtual buffer to a chained list of
> physical buffers) is dead simple.

So you gonna call virt_to_bus several times (for each page) and see 
whether you get new phys page or not? This could work, but for the 
common case of phys-continuous buffer it'll be suboptimal, i.e. you 
waste time calling virt_to_bus when it's not needed. TO make it better 
you have to move that range check from virt_to_bus and friends to your 
DMA library, in this case we end up in the same situation we are 
already :) - no need to modify virt_to_bus....

--
Eugene

Re: [PATCH][PPC32[NEWBIE] enhancement to virt_to_bus/bus_to_virt (try 2)

[Paul Mackerras]

Linh Dang writes:

> I wrote a DMA engine (to used by other drivers) that (would like to) accept
> all kind of buffers as input (vmalloced, dual-access shared RAM mapped
> by BATs, etc). The DMA engine has to decode the virtual address of the
> input buffer to (possibly multiple) physical  address(es). virt_to_phys()
> has the right name for the job except it only works for the kernel virtual
> addresses initially mapped at KERNELBASE

Have you read Documentation/DMA-API.txt?  It explains the official
kernel API for DMA, and drivers should use it in order to be portable
to more than just one architecture.

If you want to create a competing DMA API, you'll have to show us at
least one driver that really needs your new API.

Also, please don't change the existing virt_to_*/*_to_virt functions.
Instead define your own functions (with different names) in the same
source file as your other new code.

Paul.

Re: [PATCH][PPC32[NEWBIE] enhancement to virt_to_bus/bus_to_virt (try 2)

[Linh Dang]

On Mon, 6 Dec 2004 08:12:58 +1100, Paul Mackerras <paulus@samba.org> wrote:
> Linh Dang writes:
> 
> > I wrote a DMA engine (to used by other drivers) that (would like to) accept
> > all kind of buffers as input (vmalloced, dual-access shared RAM mapped
> > by BATs, etc). The DMA engine has to decode the virtual address of the
> > input buffer to (possibly multiple) physical  address(es). virt_to_phys()
> > has the right name for the job except it only works for the kernel virtual
> > addresses initially mapped at KERNELBASE
> 
> Have you read Documentation/DMA-API.txt?  It explains the official
> kernel API for DMA, and drivers should use it in order to be portable
> to more than just one architecture.

Thanx for the pointer, I'll read that carefully

> 
> If you want to create a competing DMA API, you'll have to show us at
> least one driver that really needs your new API.
> 

I think I'll implement the official DMA API for my card (the card uses the
Marvell 64460 as bridge).

> Also, please don't change the existing virt_to_*/*_to_virt functions.
> Instead define your own functions (with different names) in the same
> source file as your other new code.

Thanx for the answers.

-- 
Linh Dang

Re: [PATCH][PPC32[NEWBIE] enhancement to virt_to_bus/bus_to_virt (try 2)

[Linh Dang]

On Mon, 6 Dec 2004 08:12:58 +1100, Paul Mackerras <paulus@samba.org> wrote:
> Linh Dang writes:
> 
> > I wrote a DMA engine (to used by other drivers) that (would like to) accept
> > all kind of buffers as input (vmalloced, dual-access shared RAM mapped
> > by BATs, etc). The DMA engine has to decode the virtual address of the
> > input buffer to (possibly multiple) physical  address(es). virt_to_phys()
> > has the right name for the job except it only works for the kernel virtual
> > addresses initially mapped at KERNELBASE
> 
> Have you read Documentation/DMA-API.txt?  It explains the official
> kernel API for DMA, and drivers should use it in order to be portable
> to more than just one architecture.

>From further reading of that text, I don't think it's what I'm looking for.
The DMA-API.txt file describes the official API for mapping an kernel
virtual address to something usable by the DMA of the bus-master
capable device.

On a many embedded ppc32 platforms, the bridge is capable of
performing DMA  (usually called IDMA) between RAM ram and
NON-busmaster-devices. Examples of such platforms are the
PowerQuiccII, PowerQuiccIII, the Marvell Discovery I,II,II, etc.

Such DMA operations are very useful because CPU cycles are
precious on many embedded platforms.

What I'm looking for is the Linux way or the Linux equivalent of:

  http://fxr.watson.org/fxr/source/dev/marvell/gtidma.c?v=NETBSD


Oh, a side question, in Linux/Unix world, is it acceptable to DMA
data directly from/to the userspace buffer? Something like:

         int fd = open("/dev/my_asic0", O_RDWR);
         int buffer = calloc(40, 4096);
         int n = pread64(fd, buffer, 40 * 4096, SOME_ADDR);

and the IDMA engine would transfer 40 pages from my_asic0's
shared-ram directly into `buffer'.

Thanx
-- 
Linh Dang