From owner-freebsd-hackers@FreeBSD.ORG Tue Sep 28 15:25:58 2010 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B891E1065673 for ; Tue, 28 Sep 2010 15:25:58 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 73BC58FC1E for ; Tue, 28 Sep 2010 15:25:58 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 0C22046B8E; Tue, 28 Sep 2010 11:25:58 -0400 (EDT) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id F31468A04F; Tue, 28 Sep 2010 11:25:56 -0400 (EDT) From: John Baldwin To: Neel Natu Date: Tue, 28 Sep 2010 09:36:39 -0400 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20100819; KDE/4.4.5; amd64; ; ) References: <201009271104.17478.jhb@freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201009280936.40203.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Tue, 28 Sep 2010 11:25:57 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.6 required=4.2 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: freebsd-hackers@freebsd.org Subject: Re: PATCH: fix bogus error message "bus_dmamem_alloc failed to align memory properly" X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Sep 2010 15:25:58 -0000 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 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. -- John Baldwin