Date: Tue, 28 Sep 2010 13:02:08 -0700 From: Neel Natu <neelnatu@gmail.com> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-hackers@freebsd.org Subject: Re: PATCH: fix bogus error message "bus_dmamem_alloc failed to align memory properly" Message-ID: <AANLkTi=YGhsDi=NiKMe-U8ibh%2B5Zm7fSavYKHyYeWxTr@mail.gmail.com> In-Reply-To: <201009280936.40203.jhb@freebsd.org> References: <AANLkTik3gtndjQWh22fzq8vr_QTRAUwPRoca-wkVEYY=@mail.gmail.com> <201009271104.17478.jhb@freebsd.org> <AANLkTinx5u3NaPBO%2BkPzcm1F3Gx4Pan%2Bo01xQ_TzGo7Z@mail.gmail.com> <201009280936.40203.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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() abou=
t
>> >> 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? =A0In general I thought the busdma code only handled sub-pa=
ge
>> > 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, i=
f 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 in=
dividual
>> > page is 4-page aligned. =A0At 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:
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/*
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * Get the physical address for this segme=
nt.
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (pmap)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0curaddr =3D pmap_extract(p=
map, vaddr);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0curaddr =3D pmap_kextract(=
vaddr);
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/*
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * Compute the segment size, and adjust co=
unts.
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0max_sgsize =3D MIN(buflen, dmat->maxsegsz)=
;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sgsize =3D PAGE_SIZE - ((vm_offset_t)curad=
dr & PAGE_MASK);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (map->pagesneeded !=3D 0 && run_filter(=
dmat, curaddr)) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sgsize =3D roundup2(sgsize=
, dmat->alignment);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sgsize =3D MIN(sgsize, max=
_sgsize);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0curaddr =3D add_bounce_pag=
e(dmat, map, vaddr, sgsize);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sgsize =3D MIN(sgsize, max=
_sgsize);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
>
> If you have a map that does need bouncing, then it will split up the page=
s.
> It happens to work for bus_dmamem_alloc() because that returns a NULL map
> which doesn't bounce. =A0But 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 in=
to
> 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?
best
Neel
> --
> John Baldwin
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTi=YGhsDi=NiKMe-U8ibh%2B5Zm7fSavYKHyYeWxTr>