Re: [PATCH 1/3] powerpc: dma code cleanup

[Remi Machet <rmachet@slac.stanford.edu>]

On Wed, 2008-10-08 at 08:22 +1100, Benjamin Herrenschmidt wrote:
> On Tue, 2008-10-07 at 14:05 -0700, Remi Machet wrote:
> > +       /*
> > +        * Mark the pages as Reserved: this memory is used by a DMA
> > engine,
> > +        * it cannot be swapped.
> > +        * Also mark each page as un-cached. We ass ume here that a
> > PTE
> > +        * already exist (valid assumption for the DMA Zone)
> > +        */
> > +       for (addr = kaddr, p = page; addr < kaddr+size;
> > +                       addr += PAGE_SIZE, p++) {
> > +               pte_t *pte;
> > +               spinlock_t *ptl;
> > +
> > +               SetPageReserved(p);
> > +               pte = get_locked_pte(&init_mm, addr, &ptl);
> > +               set_pte_at(&init_mm, addr,
> > +                       pte, mk_pte(p,
> > pgprot_noncached(PAGE_KERNEL)));
> > +               pte_unmap_unlock(pte, ptl);
> > +       }
> > +       flush_tlb_kernel_range(kaddr, kaddr+size-1);
> > +
> 
> There are a few problems with the above approach:
> 
>  - Avoid SetPageReserved. It's not useful. Kernel memory doesn't
> get swapped, and if you happen to map it into userspace, it's up
> to that mapping to have the right attributes to prevent that.
> 
>  - Setting the PTE to non-cached will not work for a whole bunch of
> things. The kernel linear mapping is often mapped using large bolted TLB
> entries. For example, on 44x, if you page is in the bottom 768M, it will
> be mapped using a bolted 256M cacheable TLB entry and you may not even
> have a PTE for it. kmap will not help
Do you know of any powerpc architecture where the DMA engine accessible
address space is limited?

The old dma-noncoherent code had some convoluted code to decide whether
or not it should add GFP_DMA to the list of flags in dma_alloc_coherent:

	u64 mask = 0x00ffffff, limit; /* ISA default */
	...
	limit = (mask + 1) & ~mask;
	if ((limit && size >= limit) || size >= (CONSISTENT_END - CONSISTENT_BASE)) {
		printk(KERN_WARNING "coherent allocation too big (requested %#x mask %#Lx)\n",
		       size, mask);
		return NULL;
	}
	...
	if (mask != 0xffffffff)
		gfp |= GFP_DMA;


Since mask is forced to 0x00ffffff, GFP_DMA is always added ... should I
just add it too or leave that to the caller of dma_alloc_coherent? (I
would personally rather leave that to the caller but it may break some
driver).

Remi