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

From: Benjamin Herrenschmidt <[email-masked]>
Date: Wed, 08 Oct 2008 08:22:14 +1100

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

 - You are potentially creating a lot of permanently kmap'ed memory. But
the kmap pool might get exhausted... bad.

I would suggest instead:

 - Keep using a separate virtual mapping

 - But instead of using your own allocator, just use the vmalloc/ioremap
one. You can either continue using a specific virtual address range, or
just use the main "pool" of ioremap / vmalloc, I don't see why you
couldn't do the later in fact.

That doesn't solve the potential issue of having the double mapping of
some memory both cacheable and non cacheable, but that's life. We deal
with it on 440 by having guarded, essentially preventing any prefetch
from getting in the way.

Cheers,
Ben.