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>