Date: Wed, 29 Sep 2010 07:47:20 -0400 From: John Baldwin <jhb@freebsd.org> To: Neel Natu <neelnatu@gmail.com> Cc: freebsd-hackers@freebsd.org Subject: Re: PATCH: fix bogus error message "bus_dmamem_alloc failed to align memory properly" Message-ID: <201009290747.20754.jhb@freebsd.org> In-Reply-To: <AANLkTi=YGhsDi=NiKMe-U8ibh%2B5Zm7fSavYKHyYeWxTr@mail.gmail.com> References: <AANLkTik3gtndjQWh22fzq8vr_QTRAUwPRoca-wkVEYY=@mail.gmail.com> <201009280936.40203.jhb@freebsd.org> <AANLkTi=YGhsDi=NiKMe-U8ibh%2B5Zm7fSavYKHyYeWxTr@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tuesday, September 28, 2010 4:02:08 pm Neel Natu wrote:
> Hi John,
>
> On Tue, Sep 28, 2010 at 6:36 AM, John Baldwin <jhb@freebsd.org> wrote:
> > 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@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.
> >
>
> You are right.
>
> I assume that you are ok with the patch and the discussion above was
> an FYI, right?
I think the patch is ok, but my point is that super-page alignment isn't
really part of the design of the current bus_dma and only works for
bus_dmammem_alloc() by accident.
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201009290747.20754.jhb>