PATCH: fix bogus error message "bus_dmamem_alloc failed to align memory properly"

[John Baldwin]

On Monday, September 27, 2010 5:13:03 pm Neel Natu wrote:
> Hi John,
> 
> Thanks for reviewing this.
> 
> On Mon, Sep 27, 2010 at 8:04 AM, John Baldwin <jhb at freebsd.org> wrote:
> > On Friday, September 24, 2010 9:00:44 pm Neel Natu wrote:
> >> Hi,
> >>
> >> This patch fixes the bogus error message from bus_dmamem_alloc() about
> >> the buffer not being aligned properly.
> >>
> >> The problem is that the check is against a virtual address as opposed
> >> to the physical address. contigmalloc() makes guarantees about
> >> the alignment of physical addresses but not the virtual address
> >> mapping it.
> >>
> >> Any objections if I commit this patch?
> >
> > Hmmm, I guess you are doing super-page alignment rather than sub-page
> > alignment?  In general I thought the busdma code only handled sub-page
> > alignment and doesn't fully handle requests for super-page alignment.
> >
> 
> Yes, this is for allocations with sizes greater than PAGE_SIZE and
> alignment requirements also greater than a PAGE_SIZE.
> 
> > For example, since it insists on walking individual pages at a time, if you
> > had an alignment setting of 4 pages and passed in a single, aligned 4-page
> > buffer, bus_dma would actually bounce the last 3 pages so that each individual
> > page is 4-page aligned.  At least, I think that is what would happen.
> >
> 
> I think you are referring to bus_dmamap_load() operation that would
> follow the bus_dmamem_alloc(), right? The memory allocated by
> bus_dmamem_alloc() does not need to be bounced. In fact, the dmamap
> pointer returned by bus_dmamem_alloc() is NULL.
> 
> At least for the amd64 implementation there is code in
> _bus_dmamap_load_buffer() which will coalesce individual dma segments
> if they satisfy 'boundary' and 'segsize' constraints.

So the problem is earlier in the routine where it does this:

                /*
                 * Get the physical address for this segment.
                 */
                if (pmap)
                        curaddr = pmap_extract(pmap, vaddr);
                else
                        curaddr = pmap_kextract(vaddr);

                /*
                 * Compute the segment size, and adjust counts.
                 */
                max_sgsize = MIN(buflen, dmat->maxsegsz);
                sgsize = PAGE_SIZE - ((vm_offset_t)curaddr & PAGE_MASK);
                if (map->pagesneeded != 0 && run_filter(dmat, curaddr)) {
                        sgsize = roundup2(sgsize, dmat->alignment);
                        sgsize = MIN(sgsize, max_sgsize);
                        curaddr = add_bounce_page(dmat, map, vaddr, sgsize);
                } else {
                        sgsize = MIN(sgsize, max_sgsize);
                }

If you have a map that does need bouncing, then it will split up the pages.
It happens to work for bus_dmamem_alloc() because that returns a NULL map
which doesn't bounce.  But if you had a PCI device which supported only
32-bit addresses on a 64-bit machine with an aligned, 4 page buffer above
4GB and did a bus_dma_map_load() on that buffer, it would get split up into
4 separate 4 page-aligned pages.

-- 
John Baldwin