Hi John,
Thanks for reviewing this.
On Mon, Sep 27, 2010 at 8:04 AM, John Baldwin <j...@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.
683 /*
684 * Insert chunk into a segment, coalescing with
685 * previous segment if possible.
686 */
687 if (first) {
688 segs[seg].ds_addr = curaddr;
689 segs[seg].ds_len = sgsize;
690 first = 0;
691 } else {
692 if (curaddr == lastaddr &&
693 (segs[seg].ds_len + sgsize) <=
dmat->maxsegsz &&
694 (dmat->boundary == 0 ||
695 (segs[seg].ds_addr & bmask) ==
(curaddr & bmask)))
696 segs[seg].ds_len += sgsize;
697 else {
698 if (++seg >= dmat->nsegments)
699 break;
700 segs[seg].ds_addr = curaddr;
701 segs[seg].ds_len = 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 should be
> the same.
>
Yes.
best
Neel
>> best
>> Neel
>>
>> Index: sys/powerpc/powerpc/busdma_machdep.c
>> ===================================================================
>> --- sys/powerpc/powerpc/busdma_machdep.c (revision 213113)
>> +++ sys/powerpc/powerpc/busdma_machdep.c (working copy)
>> @@ -529,7 +529,7 @@
>> CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x error %d",
>> __func__, dmat, dmat->flags, ENOMEM);
>> return (ENOMEM);
>> - } else if ((uintptr_t)*vaddr & (dmat->alignment - 1)) {
>> + } else if (vtophys(*vaddr) & (dmat->alignment - 1)) {
>> printf("bus_dmamem_alloc failed to align memory properly.\n");
>> }
>> #ifdef NOTYET
>> Index: sys/sparc64/sparc64/bus_machdep.c
>> ===================================================================
>> --- sys/sparc64/sparc64/bus_machdep.c (revision 213113)
>> +++ sys/sparc64/sparc64/bus_machdep.c (working copy)
>> @@ -652,7 +652,7 @@
>> }
>> if (*vaddr == NULL)
>> return (ENOMEM);
>> - if ((uintptr_t)*vaddr % dmat->dt_alignment)
>> + if (vtophys(*vaddr) % dmat->dt_alignment)
>> printf("%s: failed to align memory properly.\n", __func__);
>> return (0);
>> }
>> Index: sys/ia64/ia64/busdma_machdep.c
>> ===================================================================
>> --- sys/ia64/ia64/busdma_machdep.c (revision 213113)
>> +++ sys/ia64/ia64/busdma_machdep.c (working copy)
>> @@ -455,7 +455,7 @@
>> }
>> if (*vaddr == NULL)
>> return (ENOMEM);
>> - else if ((uintptr_t)*vaddr & (dmat->alignment - 1))
>> + else if (vtophys(*vaddr) & (dmat->alignment - 1))
>> printf("bus_dmamem_alloc failed to align memory properly.\n");
>> return (0);
>> }
>> Index: sys/i386/i386/busdma_machdep.c
>> ===================================================================
>> --- sys/i386/i386/busdma_machdep.c (revision 213113)
>> +++ sys/i386/i386/busdma_machdep.c (working copy)
>> @@ -540,7 +540,7 @@
>> CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x error %d",
>> __func__, dmat, dmat->flags, ENOMEM);
>> return (ENOMEM);
>> - } else if ((uintptr_t)*vaddr & (dmat->alignment - 1)) {
>> + } else if (vtophys(*vaddr) & (dmat->alignment - 1)) {
>> printf("bus_dmamem_alloc failed to align memory properly.\n");
>> }
>> if (flags & BUS_DMA_NOCACHE)
>> Index: sys/amd64/amd64/busdma_machdep.c
>> ===================================================================
>> --- sys/amd64/amd64/busdma_machdep.c (revision 213113)
>> +++ sys/amd64/amd64/busdma_machdep.c (working copy)
>> @@ -526,7 +526,7 @@
>> CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x error %d",
>> __func__, dmat, dmat->flags, ENOMEM);
>> return (ENOMEM);
>> - } else if ((uintptr_t)*vaddr & (dmat->alignment - 1)) {
>> + } else if (vtophys(*vaddr) & (dmat->alignment - 1)) {
>> printf("bus_dmamem_alloc failed to align memory properly.\n");
>> }
>> 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-unsubscr...@freebsd.org"
>>
>
> --
> John Baldwin
>
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
