Date: Mon, 27 Sep 2010 14:13:03 -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: <AANLkTinx5u3NaPBO%2BkPzcm1F3Gx4Pan%2Bo01xQ_TzGo7Z@mail.gmail.com> In-Reply-To: <201009271104.17478.jhb@freebsd.org> References: <AANLkTik3gtndjQWh22fzq8vr_QTRAUwPRoca-wkVEYY=@mail.gmail.com> <201009271104.17478.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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? =A0In 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 y=
ou
> had an alignment setting of 4 pages and passed in a single, aligned 4-pag=
e
> buffer, bus_dma would actually bounce the last 3 pages so that each indiv=
idual
> 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.
683 /*
684 * Insert chunk into a segment, coalescing with
685 * previous segment if possible.
686 */
687 if (first) {
688 segs[seg].ds_addr =3D curaddr;
689 segs[seg].ds_len =3D sgsize;
690 first =3D 0;
691 } else {
692 if (curaddr =3D=3D lastaddr &&
693 (segs[seg].ds_len + sgsize) <=3D
dmat->maxsegsz &&
694 (dmat->boundary =3D=3D 0 ||
695 (segs[seg].ds_addr & bmask) =3D=3D
(curaddr & bmask)))
696 segs[seg].ds_len +=3D sgsize;
697 else {
698 if (++seg >=3D dmat->nsegments)
699 break;
700 segs[seg].ds_addr =3D curaddr;
701 segs[seg].ds_len =3D sgsize;
702 }
703 }
I do get the expected result after I call dma_dmamap_load() i.e. a
single dma segment with the correct alignment and boundary settings.
> For sub-page alignment, the virtual and physical address alignments shoul=
d be
> the same.
>
Yes.
best
Neel
>> best
>> Neel
>>
>> Index: sys/powerpc/powerpc/busdma_machdep.c
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- sys/powerpc/powerpc/busdma_machdep.c =A0 =A0 =A0(revision 213113)
>> +++ sys/powerpc/powerpc/busdma_machdep.c =A0 =A0 =A0(working copy)
>> @@ -529,7 +529,7 @@
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x =
error %d",
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 __func__, dmat, dmat->flags, ENOMEM)=
;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (ENOMEM);
>> - =A0 =A0 } else if ((uintptr_t)*vaddr & (dmat->alignment - 1)) {
>> + =A0 =A0 } else if (vtophys(*vaddr) & (dmat->alignment - 1)) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf("bus_dmamem_alloc failed to align mem=
ory properly.\n");
>> =A0 =A0 =A0 }
>> =A0#ifdef NOTYET
>> Index: sys/sparc64/sparc64/bus_machdep.c
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- sys/sparc64/sparc64/bus_machdep.c (revision 213113)
>> +++ sys/sparc64/sparc64/bus_machdep.c (working copy)
>> @@ -652,7 +652,7 @@
>> =A0 =A0 =A0 }
>> =A0 =A0 =A0 if (*vaddr =3D=3D NULL)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (ENOMEM);
>> - =A0 =A0 if ((uintptr_t)*vaddr % dmat->dt_alignment)
>> + =A0 =A0 if (vtophys(*vaddr) % dmat->dt_alignment)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf("%s: failed to align memory properly.=
\n", __func__);
>> =A0 =A0 =A0 return (0);
>> =A0}
>> Index: sys/ia64/ia64/busdma_machdep.c
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- sys/ia64/ia64/busdma_machdep.c =A0 =A0(revision 213113)
>> +++ sys/ia64/ia64/busdma_machdep.c =A0 =A0(working copy)
>> @@ -455,7 +455,7 @@
>> =A0 =A0 =A0 }
>> =A0 =A0 =A0 if (*vaddr =3D=3D NULL)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (ENOMEM);
>> - =A0 =A0 else if ((uintptr_t)*vaddr & (dmat->alignment - 1))
>> + =A0 =A0 else if (vtophys(*vaddr) & (dmat->alignment - 1))
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf("bus_dmamem_alloc failed to align mem=
ory properly.\n");
>> =A0 =A0 =A0 return (0);
>> =A0}
>> Index: sys/i386/i386/busdma_machdep.c
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- sys/i386/i386/busdma_machdep.c =A0 =A0(revision 213113)
>> +++ sys/i386/i386/busdma_machdep.c =A0 =A0(working copy)
>> @@ -540,7 +540,7 @@
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x =
error %d",
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 __func__, dmat, dmat->flags, ENOMEM)=
;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (ENOMEM);
>> - =A0 =A0 } else if ((uintptr_t)*vaddr & (dmat->alignment - 1)) {
>> + =A0 =A0 } else if (vtophys(*vaddr) & (dmat->alignment - 1)) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf("bus_dmamem_alloc failed to align mem=
ory properly.\n");
>> =A0 =A0 =A0 }
>> =A0 =A0 =A0 if (flags & BUS_DMA_NOCACHE)
>> Index: sys/amd64/amd64/busdma_machdep.c
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- sys/amd64/amd64/busdma_machdep.c =A0(revision 213113)
>> +++ sys/amd64/amd64/busdma_machdep.c =A0(working copy)
>> @@ -526,7 +526,7 @@
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x =
error %d",
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 __func__, dmat, dmat->flags, ENOMEM)=
;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (ENOMEM);
>> - =A0 =A0 } else if ((uintptr_t)*vaddr & (dmat->alignment - 1)) {
>> + =A0 =A0 } else if (vtophys(*vaddr) & (dmat->alignment - 1)) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf("bus_dmamem_alloc failed to align mem=
ory properly.\n");
>> =A0 =A0 =A0 }
>> =A0 =A0 =A0 if (flags & BUS_DMA_NOCACHE)
>> _______________________________________________
>> freebsd-hackers@freebsd.org mailing list
>> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
>> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.or=
g"
>>
>
> --
> John Baldwin
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTinx5u3NaPBO%2BkPzcm1F3Gx4Pan%2Bo01xQ_TzGo7Z>