----- Original Message -----
> From: John Baldwin <j...@freebsd.org>
> To: curr...@freebsd.org
> Cc: sco...@freebsd.org; Peter Jeremy <pe...@rulingia.com>
> Sent: Thursday, July 12, 2012 7:40 AM
> Subject: Adding support for WC (write-combining) memory to bus_dma
>
> 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.
>
Please don't add new parameters. Now that I'm carefully documenting the
evolution of the APIs, it's becoming glaringly apparent how sloppy we are
with API design and interface compatibility. I'm just as guilty of it as
anyone,
but I'd really like to see less instances of call signature changes in existing
functions; they make driver maintenance tedious and are hard to effectively
document. Some options I can think of:
1. create bus_dmamem_alloc_attr(). I don't really like leafy API growth like
this either, but it's not a horrible solution.
2. There are existing placeholder flags, BUS_DMA_BUS[1234] that could be
aliased and repurposed to hold 4 bits of attribute information for this
function.
The 3 and 4 variants are already in use, but I haven't looked closely to see
their scope.
3. Reserve the top 16 bits of the flags for attribute information.
4. Move the attribute information into the tag and create new setter/getter
accessors for attribute information. This would probably be the cleanest,
though it breaks the existing sloppiness of allowing different pseudo-attributes
for different allocations under the same tag. I've wanted to break down the
existing bus_dma_tag_create() into finer-grained setter/getters for a while in
any case.
5. Move the attribute information into the map and force everyone to start
creating maps for static memory allocations. This would actually add some
missing uniformity to the API and might actually be cleaner that option 4.
> 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(). 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().
Yup, this is a problem, and I like your fix; this kind of state is exactly what
belongs in the map.
>
> 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.
>
> Index: amd64/amd64/busdma_machdep.c
Thanks,
Scott
_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
