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 <j...@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 <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. > > > > 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 _______________________________________________ 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"