[PATCH] allow coherent DMA API to work before main page allocator is set up

[PATCH] allow coherent DMA API to work before main page allocator is set up

[Marcelo Tosatti]

Hi Dan, Matt,

The following patch changes dma_alloc_coherent() to, in case the 
main page allocator is not yet up and running, use the bootmem
allocator instead.

It also adds a new parameter to m8xx_cpm_hostalloc() to send back
the physical address to its caller.

With such modification in place it will be possible for early drivers 
to rely on the coherent DMA allocator. Special casing such as 
drivers/serial/cpm_uart/cpm_uart_cpm1.c's cpm_uart_allocbuf() can be 
removed:

        if (is_con) {
                mem_addr = (u8 *) m8xx_cpm_hostalloc(memsz);
                dma_addr = 0;
        } else
                mem_addr = dma_alloc_coherent(NULL, memsz, &dma_addr,
                                              GFP_KERNEL);

What is your opinion on this approach? 

If virt_to_phys() usage is discouraged any different method to walk 
the pagetables and retrieve the physical address can be used.

TIA

diff -Nur linux-2.6.11.orig/arch/ppc/8xx_io/commproc.c linux-2.6.11/arch/ppc/8xx_io/commproc.c
--- linux-2.6.11.orig/arch/ppc/8xx_io/commproc.c	2005-05-20 13:53:17.000000000 -0300
+++ linux-2.6.11/arch/ppc/8xx_io/commproc.c	2005-05-20 16:02:13.000000000 -0300
@@ -39,7 +39,8 @@
 #include <asm/tlbflush.h>
 #include <asm/rheap.h>
 
-extern int get_pteptr(struct mm_struct *mm, unsigned long addr, pte_t **ptep);
+
+phys_addr_t	physaddr;
 
 static void m8xx_cpm_dpinit(void);
 static	uint	host_buffer;	/* One page of host buffer */
@@ -111,11 +112,10 @@
 extern void flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr);
 
 void
-m8xx_cpm_reset(uint bootpage)
+m8xx_cpm_reset(void)
 {
 	volatile immap_t	 *imp;
 	volatile cpm8xx_t	*commproc;
-	pte_t *pte;
 
 	imp = (immap_t *)IMAP_ADDR;
 	commproc = (cpm8xx_t *)&imp->im_cpm;
@@ -143,17 +143,6 @@
 	/* Reclaim the DP memory for our use. */
 	m8xx_cpm_dpinit();
 
-	/* get the PTE for the bootpage */
-	if (!get_pteptr(&init_mm, bootpage, &pte))
-	       panic("get_pteptr failed\n");
-																							
-	/* and make it uncachable */
-	pte_val(*pte) |= _PAGE_NO_CACHE;
-	_tlbie(bootpage);
-
-	host_buffer = bootpage;
-	host_end = host_buffer + PAGE_SIZE;
-
 	/* Tell everyone where the comm processor resides.
 	*/
 	cpmp = (cpm8xx_t *)commproc;
@@ -165,7 +154,6 @@
 static void
 alloc_host_memory(void)
 {
-	dma_addr_t	physaddr;
 
 	/* Set the host page for allocation.
 	*/
@@ -325,7 +313,7 @@
  * UART "fifos" and the like.
  */
 uint
-m8xx_cpm_hostalloc(uint size)
+m8xx_cpm_hostalloc(uint size, phys_addr_t *handle)
 {
 	uint	retloc;
 
@@ -336,7 +324,9 @@
 		return(0);
 
 	retloc = host_buffer;
+	*handle = physaddr;
 	host_buffer += size;
+	physaddr += size;
 
 	return(retloc);
 }
diff -Nur linux-2.6.11.orig/arch/ppc/kernel/dma-mapping.c linux-2.6.11/arch/ppc/kernel/dma-mapping.c
--- linux-2.6.11.orig/arch/ppc/kernel/dma-mapping.c	2005-05-20 13:53:17.000000000 -0300
+++ linux-2.6.11/arch/ppc/kernel/dma-mapping.c	2005-05-20 14:03:56.000000000 -0300
@@ -55,6 +55,8 @@
 
 int map_page(unsigned long va, phys_addr_t pa, int flags);
 
+extern int mem_init_done;
+
 #include <asm/tlbflush.h>
 
 /*
@@ -121,7 +123,10 @@
 	unsigned long flags;
 	struct vm_region *c, *new;
 
-	new = kmalloc(sizeof(struct vm_region), gfp);
+	if (mem_init_done)
+		new = kmalloc(sizeof(struct vm_region), gfp);
+	else
+		new = alloc_bootmem(sizeof(struct vm_region));
 	if (!new)
 		goto out;
 
@@ -175,10 +180,11 @@
 void *
 __dma_alloc_coherent(size_t size, dma_addr_t *handle, int gfp)
 {
-	struct page *page;
+	struct page *page = NULL;
 	struct vm_region *c;
 	unsigned long order;
 	u64 mask = 0x00ffffff, limit; /* ISA default */
+	void *bootmem_address = NULL;
 
 	if (!consistent_pte) {
 		printk(KERN_ERR "%s: not initialised\n", __func__);
@@ -199,14 +205,21 @@
 	if (mask != 0xffffffff)
 		gfp |= GFP_DMA;
 
-	page = alloc_pages(gfp, order);
-	if (!page)
-		goto no_page;
+	if (mem_init_done) {
+		page = alloc_pages(gfp, order);
+		if (!page)
+			goto no_page;
+	} else {
+		bootmem_address = alloc_bootmem_pages(PAGE_SIZE << order);
+		if (!bootmem_address)
+			goto no_page;
+	}
 
 	/*
 	 * Invalidate any data that might be lurking in the
 	 * kernel direct-mapped region for device DMA.
 	 */
+	if (mem_init_done)
 	{
 		unsigned long kaddr = (unsigned long)page_address(page);
 		memset(page_address(page), 0, size);
@@ -226,27 +239,37 @@
 		/*
 		 * Set the "dma handle"
 		 */
-		*handle = page_to_bus(page);
+		if (mem_init_done)
+			*handle = page_to_bus(page);
+		else
+			*handle = virt_to_phys(bootmem_address);
 
 		do {
-			BUG_ON(!pte_none(*pte));
+			if (mem_init_done) {
+				BUG_ON(!pte_none(*pte));
 
-			set_page_count(page, 1);
-			SetPageReserved(page);
-			set_pte_at(&init_mm, vaddr,
-				   pte, mk_pte(page, pgprot_noncached(PAGE_KERNEL)));
-			page++;
-			pte++;
-			vaddr += PAGE_SIZE;
+				set_page_count(page, 1);
+				SetPageReserved(page);
+				set_pte_at(&init_mm, vaddr,
+					   pte, mk_pte(page, pgprot_noncached(PAGE_KERNEL)));
+				page++;
+				pte++;
+				vaddr += PAGE_SIZE;
+			} else { 
+				set_pte(pte,__pte((pte_t)virt_to_phys(bootmem_address) | pgprot_val(pgprot_noncached(PAGE_KERNEL))));
+				bootmem_address += PAGE_SIZE;
+			}
 		} while (size -= PAGE_SIZE);
 
 		/*
 		 * Free the otherwise unused pages.
 		 */
-		while (page < end) {
-			set_page_count(page, 1);
-			__free_page(page);
-			page++;
+		if (mem_init_done) {
+			while (page < end) {
+				set_page_count(page, 1);
+				__free_page(page);
+				page++;
+			}
 		}
 
 		return (void *)c->vm_start;
diff -Nur linux-2.6.11.orig/arch/ppc/syslib/m8xx_setup.c linux-2.6.11/arch/ppc/syslib/m8xx_setup.c
--- linux-2.6.11.orig/arch/ppc/syslib/m8xx_setup.c	2005-05-20 13:53:17.000000000 -0300
+++ linux-2.6.11/arch/ppc/syslib/m8xx_setup.c	2005-05-20 15:59:24.000000000 -0300
@@ -57,7 +57,7 @@
 extern void m8xx_ide_init(void);
 
 extern unsigned long find_available_memory(void);
-extern void m8xx_cpm_reset(uint cpm_page);
+extern void m8xx_cpm_reset();
 extern void m8xx_wdt_handler_install(bd_t *bp);
 extern void rpxfb_alloc_pages(void);
 extern void cpm_interrupt_init(void);
@@ -70,13 +70,9 @@
 void __init
 m8xx_setup_arch(void)
 {
-	int	cpm_page;
-
-	cpm_page = (int) alloc_bootmem_pages(PAGE_SIZE);
-
 	/* Reset the Communication Processor Module.
 	*/
-	m8xx_cpm_reset(cpm_page);
+	m8xx_cpm_reset();
 
 #ifdef CONFIG_FB_RPX
 	rpxfb_alloc_pages();
diff -Nur linux-2.6.11.orig/include/asm-ppc/commproc.h linux-2.6.11/include/asm-ppc/commproc.h
--- linux-2.6.11.orig/include/asm-ppc/commproc.h	2005-05-20 13:53:40.000000000 -0300
+++ linux-2.6.11/include/asm-ppc/commproc.h	2005-05-20 16:02:37.000000000 -0300
@@ -79,7 +79,7 @@
 extern void *cpm_dpram_addr(uint offset);
 extern void cpm_setbrg(uint brg, uint rate);
 
-extern uint m8xx_cpm_hostalloc(uint size);
+extern uint m8xx_cpm_hostalloc(uint size, phys_addr_t *handle);
 extern int  m8xx_cpm_hostfree(uint start);
 extern void m8xx_cpm_hostdump(void);
 

Re: [PATCH] allow coherent DMA API to work before main page allocator is set up

[Marcelo Tosatti]

On Fri, May 20, 2005 at 05:03:59PM -0400, Dan Malek wrote:
> 
> On May 20, 2005, at 10:12 AM, Marcelo Tosatti wrote:
> 
> >The following patch changes dma_alloc_coherent() to, in case the
> >main page allocator is not yet up and running, use the bootmem
> >allocator instead.
> 
> I'm concerned about this ...  Why did the drivers change in 2.6
> such that they now call dma_alloc_coherent() prior to the VM set up?

Well, as far as I can see, it is expected that console_init() gets called
before the main page allocator.

The startup sequence, on both v2.4 and v2.6, is:

setup_arch()
...
	console_init()
		console_8xx_init()
			register_console()
				release_console_sem() 
...
	mem_init()

register_console calls the driver ->setup method, and release_console_sem() 
is going to ->write pending data from the log buffer to the device.

As you well know, ->setup calls m8xx_cpm_hostalloc(). 

I must be missing something very obvious here, can you explain how 
did you arrange things in v2.4 such that the console's ->setup 
method is called _after_ mem_init() ? 

> >It also adds a new parameter to m8xx_cpm_hostalloc() to send back
> >the physical address to its caller.
> 
> Anyone calling m8xx_cpm_hostalloc() is likely to be immediately
> calling dma_alloc_coherent().  There is some design problem here
> if we can't properly use these interfaces and get the mapping we
> need.  The TLB pinning option won't work if we can't do this.
> 
> 
> > ..... Special casing such as
> >drivers/serial/cpm_uart/cpm_uart_cpm1.c's cpm_uart_allocbuf() can be
> >removed:
> >
> >        if (is_con) {
> >                mem_addr = (u8 *) m8xx_cpm_hostalloc(memsz);
> >                dma_addr = 0;
> >        } else
> >                mem_addr = dma_alloc_coherent(NULL, memsz, &dma_addr,
> >                                              GFP_KERNEL);
> 
> This is bogus and should not be necessary.  The original serial
> driver was able to work without doing this, I spent lots of time so
> stuff like this wasn't necessary, and it's sad to see us take such
> a step backward. 

Well, if you got this right in v2.4 then it must be possible to get 
it right in v2.6 too...

Thanks!

Re: [PATCH] allow coherent DMA API to work before main page allocator is set up

[Dan Malek]

On May 20, 2005, at 10:12 AM, Marcelo Tosatti wrote:

> The following patch changes dma_alloc_coherent() to, in case the
> main page allocator is not yet up and running, use the bootmem
> allocator instead.

I'm concerned about this ...  Why did the drivers change in 2.6
such that they now call dma_alloc_coherent() prior to the VM set up?

> It also adds a new parameter to m8xx_cpm_hostalloc() to send back
> the physical address to its caller.

Anyone calling m8xx_cpm_hostalloc() is likely to be immediately
calling dma_alloc_coherent().  There is some design problem here
if we can't properly use these interfaces and get the mapping we
need.  The TLB pinning option won't work if we can't do this.


>  ..... Special casing such as
> drivers/serial/cpm_uart/cpm_uart_cpm1.c's cpm_uart_allocbuf() can be
> removed:
>
>         if (is_con) {
>                 mem_addr = (u8 *) m8xx_cpm_hostalloc(memsz);
>                 dma_addr = 0;
>         } else
>                 mem_addr = dma_alloc_coherent(NULL, memsz, &dma_addr,
>                                               GFP_KERNEL);

This is bogus and should not be necessary.  The original serial
driver was able to work without doing this, I spent lots of time so
stuff like this wasn't necessary, and it's sad to see us take such
a step backward.

> What is your opinion on this approach?

As you can tell, I don't like it and we shouldn't need it :-)
The first call to m8xx_cpm_hostalloc() should be able to call
dma_consistent_alloc() to get a consistent page.  If this isn't the
case, we have to fix the drivers so they can do this.  The only
change that should be necessary is to return the physical address
from m8xx_cpm_hostalloc(), if for some reason we see some
need to get rid of the immensely useful iopa() function.

Thanks.


	-- Dan

Re: [PATCH] allow coherent DMA API to work before main page allocator is set up

[Dan Malek]

On May 20, 2005, at 1:51 PM, Marcelo Tosatti wrote:

> I must be missing something very obvious here, can you explain how
> did you arrange things in v2.4 such that the console's ->setup
> method is called _after_ mem_init() ?

The driver used to use the serial port configuration as left by the
boot rom until very late into the initialization.  As I have said many
times in the past, the serial driver went through three different
levels of configuration.  There was an early configuration that was
used for kgdb, a later one used by the serial kernel printk, and
finally a full configuration by the serial driver.

It's sad that "moving forward" is done at a cost of throwing away
features that were important.  The hack to allocate from boot memory
is a quick fix, but this further ensures features like pinned TLB
entries won't work.

Thanks.


	-- Dan

[RFT] 8xx cpm_hostalloc patch was: allow coherent DMA API to work before main page allocator is set up

[Marcelo Tosatti]

[-- Attachment #1: Type: text/plain, Size: 2299 bytes --]


Can someone with 8xx using the cpm_uart driver please test 
the attached patch? It should avoid corruption of the pinned
8Mbyte CONFIG_PIN_TLB entry, as noted by Dan. 

On Sat, May 21, 2005 at 05:21:47PM -0400, Dan Malek wrote:
> 
> On May 20, 2005, at 1:51 PM, Marcelo Tosatti wrote:
> 
> >I must be missing something very obvious here, can you explain how
> >did you arrange things in v2.4 such that the console's ->setup
> >method is called _after_ mem_init() ?
> 
> The driver used to use the serial port configuration as left by the
> boot rom until very late into the initialization.  As I have said many
> times in the past, the serial driver went through three different
> levels of configuration.  There was an early configuration that was
> used for kgdb, a later one used by the serial kernel printk, and
> finally a full configuration by the serial driver. 

I have missed your statements about that in the past.

OK, the v2.4 driver is using the dual port RAM: 

        /* Allocate space for two FIFOs.  We can't allocate from host
         * memory yet because vm allocator isn't initialized
         * during this early console init.
         */
        dp_addr = m8xx_cpm_dpalloc(2*EARLY_BUF_SIZE);
        mem_addr = (uint)(&cpmp->cp_dpmem[dp_addr]);

> It's sad that "moving forward" is done at a cost of throwing away
> features that were important. 

Now that I think of it, no v2.6 driver should be calling
dma_alloc_coherent() before mem_init(). I was seeing the problem
because we're still using the old uart driver, which does that, and 
our timer setup routine also.

> is a quick fix, but this further ensures features like pinned TLB
> entries won't work. 

Actually, the quick fix to use bootmem allocator ensures that the 
pinned TLB is _preserved_ (by using a pagetableentry from the consistent 
DMA address space to mark as uncached and invalidate, instead of a pte 
from the kernel direct mapped virtual space which blows away the 8Mbyte
entry - current v2.6 state).

Why do you say that "further ensures that features like pinned TLB 
entries won't work" ? 

If there is really a problem with using the bootmem allocator for 
noncacheable purposes (which I can't see), it should be pretty easy 
to modify cpm_uart to use dpram instead of host bootmem RAM, right?


[-- Attachment #2: 8xx.patch --]
[-- Type: text/plain, Size: 1981 bytes --]

--- linux-2.6.11.orig/arch/ppc/syslib/m8xx_setup.c	2005-05-20 13:53:17.000000000 -0300
+++ linux-2.6.11/arch/ppc/syslib/m8xx_setup.c	2005-05-20 15:59:24.000000000 -0300
@@ -57,7 +57,7 @@
 extern void m8xx_ide_init(void);
 
 extern unsigned long find_available_memory(void);
-extern void m8xx_cpm_reset(uint cpm_page);
+extern void m8xx_cpm_reset();
 extern void m8xx_wdt_handler_install(bd_t *bp);
 extern void rpxfb_alloc_pages(void);
 extern void cpm_interrupt_init(void);
@@ -70,13 +70,9 @@
 void __init
 m8xx_setup_arch(void)
 {
-	int	cpm_page;
-
-	cpm_page = (int) alloc_bootmem_pages(PAGE_SIZE);
-
 	/* Reset the Communication Processor Module.
 	*/
-	m8xx_cpm_reset(cpm_page);
+	m8xx_cpm_reset();
 
 #ifdef CONFIG_FB_RPX
 	rpxfb_alloc_pages();
--- linux-2.6.11.orig/arch/ppc/8xx_io/commproc.c	2005-05-20 13:53:17.000000000 -0300
+++ linux-2.6.11/arch/ppc/8xx_io/commproc.c	2005-05-22 00:29:42.000000000 -0300
@@ -39,8 +39,6 @@
 #include <asm/tlbflush.h>
 #include <asm/rheap.h>
 
-extern int get_pteptr(struct mm_struct *mm, unsigned long addr, pte_t **ptep);
-
 static void m8xx_cpm_dpinit(void);
 static	uint	host_buffer;	/* One page of host buffer */
 static	uint	host_end;	/* end + 1 */
@@ -111,11 +109,10 @@
 extern void flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr);
 
 void
-m8xx_cpm_reset(uint bootpage)
+m8xx_cpm_reset(void)
 {
 	volatile immap_t	 *imp;
 	volatile cpm8xx_t	*commproc;
-	pte_t *pte;
 
 	imp = (immap_t *)IMAP_ADDR;
 	commproc = (cpm8xx_t *)&imp->im_cpm;
@@ -143,17 +140,6 @@
 	/* Reclaim the DP memory for our use. */
 	m8xx_cpm_dpinit();
 
-	/* get the PTE for the bootpage */
-	if (!get_pteptr(&init_mm, bootpage, &pte))
-	       panic("get_pteptr failed\n");
-																							
-	/* and make it uncachable */
-	pte_val(*pte) |= _PAGE_NO_CACHE;
-	_tlbie(bootpage);
-
-	host_buffer = bootpage;
-	host_end = host_buffer + PAGE_SIZE;
-
 	/* Tell everyone where the comm processor resides.
 	*/
 	cpmp = (cpm8xx_t *)commproc;

Re: [RFT] 8xx cpm_hostalloc patch was: allow coherent DMA API to work before main page allocator is set up

[Pantelis Antoniou]

Marcelo Tosatti wrote:
> Can someone with 8xx using the cpm_uart driver please test 
> the attached patch? It should avoid corruption of the pinned
> 8Mbyte CONFIG_PIN_TLB entry, as noted by Dan. 
> 

You shouldn't have these discussions on the weekend :)

I'm the culprit of the code in question.

This is the second take of the code. The hack with the 'if' was
not actually in the first edition, since I had the hostalloc
function working properly before initialization by using the
remainder of the bootpage, as a backup memory area, for when
the mm is not yet ready.

Somewhere along the way, it fell through the cracks.

IMHO that's the proper way to fix it.

There were some objection on that however.

Regards

Pantelis

Re: [RFT] 8xx cpm_hostalloc patch was: allow coherent DMA API to work before main page allocator is set up

[Dan Malek]

On May 23, 2005, at 2:27 AM, Pantelis Antoniou wrote:

> You shouldn't have these discussions on the weekend :)

Sorry.  I try not to, but then people think I'm not interested :-)
I was working on my race car data acquisition telemetry
project, anyway.

> I'm the culprit of the code in question.

I'm not trying to blame anyone.  The new driver is better in
many ways, we just have to fix up this one little thing so
it works in all possible VM configurations.

I also have to look at the new consistent mapping functions,
as those also seem to have changed.  Maybe this will
work fine, or can be made to work with minor changes.

Thanks.


	-- Dan

[PATCH] PSC with devfs on 5200

[Mark Chambers]

[-- Attachment #1: Type: text/plain, Size: 199 bytes --]

The attached patch allows the PSC ports on the MPC5200 to work with the
devfs file system.

I've never posted a patch before, somebody let me know if I should do
anything differently.

Mark Chambers

[-- Attachment #2: psc.patch --]
[-- Type: application/octet-stream, Size: 818 bytes --]

--- arch/ppc/5xxx_io/psc.c	2005-05-27 11:09:36.000000000 -0400
+++ /opt/eldk/ppc_82xx/usr/src/linux-2.4.24/arch/ppc/5xxx_io/psc.c	2003-12-03 14:08:19.000000000 -0500
@@ -806,11 +806,7 @@
 	memset(&rs_driver, 0, sizeof(rs_driver));
 	rs_driver.magic = TTY_DRIVER_MAGIC;
 	rs_driver.driver_name = "serial";
-#ifdef CONFIG_DEVFS_FS
-	rs_driver.name = "tts/%d";
-#else
 	rs_driver.name = "ttyS";
-#endif
 	rs_driver.major = TTY_MAJOR;
 	rs_driver.minor_start = 64;
 	rs_driver.num = MPC5xxx_PSC_NPORTS;
@@ -841,11 +837,7 @@
 	rs_driver.hangup = gs_hangup;
 
 	rs_callout_driver = rs_driver;
-#ifdef CONFIG_DEVFS_FS
-	rs_callout_driver.name = "cua/%d";
-#else
 	rs_callout_driver.name = "cua";
-#endif
 	rs_callout_driver.major = TTYAUX_MAJOR;
 	rs_callout_driver.subtype = SERIAL_TYPE_CALLOUT;