Re: Adding support for WC (write-combining) memory to bus_dma

On Thu, 2012-07-12 at 10:40 -0400, John Baldwin wrote:
> I have a need to allocate static DMA memory via bus_dmamem_alloc() that is 
> also WC (for a PCI-e device so it can use "nosnoop" transactions).  This is 
> similar to what the nvidia driver needs, but in my case it is much cleaner to 
> allocate the memory via bus dma since the existing code I am extending all 
> uses busdma.
> 
> I have a patch to implement this on 8.x for amd64 that I can port to HEAD if 
> folks don't object.  What I would really like to do is add a new paramter to 
> bus_dmamem_alloc() to specify the memory attribute to use, but I am hesitant 
> to break that API.  Instead, I added a new flag similar to the existing 
> BUS_DMA_NOCACHE used to allocate UC memory.
> 
> While doing this, I ran into an old bug, which is that if you were to call 
> bus_dmamem_alloc() with BUS_DMA_NOCACHE but a tag that otherwise fell through 
> to using malloc() instead of contigmalloc(), bus_dmamem_alloc() would actually
> change the state of the entire page.  This seems wrong.  Instead, I think that 
> any request for a non-default memory attribute should always use 
> contigmalloc().  

The problem I have with this (already, even before your proposed
changes) is that contigmalloc() is only able to allocate pages.  In the
ARM world we have a need to allocate BUS_DMA_COHERENT memory (same
effect as BUS_DMA_NOCACHE; we should consolidate these names) that is
aligned to a 32-byte boundary (cacheline-aligned) but usually the buffer
is far smaller than a page, often smaller than 1k, and sometimes we need
lots of them (allocating 128 pages for ethernet buffers, with only half
of each page used, is unreasonably expensive on a platform with only
64mb to begin with).

I keep thinking what's needed is a busdma allocation helper routine,
something MI that can be used by the various MD busdma implementations,
that can manage a pool of pages that are flagged as uncachable and can
subdivide those pages to provide small blocks of memory that fit various
alignment and boundary restrictions.

To be clear, I'm not objecting to your proposed changes, I'm more just
musing that similar problems exist in non-x86 architectures and maybe an
MI solution is possible (or at least the groundwork could be laid)?

> In fact, even better is to call kmem_alloc_contig() directly
> rather than using contigmalloc().  However, if you change this, then 
> bus_dmamem_free() won't always DTRT as it doesn't have enough state to know if
> a small allocation should be free'd via free() or contigfree() (the latter 
> would be required if it used a non-default memory attribute).  The fix I used 
> for this was to create a new dummy dmamap that is returned by bus_dmamem_alloc 
> if it uses contigmalloc().  bus_dmamem_free() then checks the passed in map 
> pointer to decide which type of free to perform.  Once this is fixed, the 
> actual WC support is rather trivial as it merely consists of passing a 
> different argument to kmem_alloc_contig().
> 
> Oh, and using kmem_alloc_contig() instead of the pmap_change_attr() hack is
> required if you want to be able to export the same pages to userland via
> mmap (e.g. using an OBJT_SG object). :)
> 
> Peter, this is somewhat orthognal (but related) to your bus_dma patch which is
> what prompted me to post this.
> 
> Patch for 8 is below.  Porting it to HEAD should be fairly trivial and direct.
> [patch removed]
> 

-- Ian