From owner-svn-src-all@freebsd.org Mon Nov 9 19:19:22 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 20567A2A617 for ; Mon, 9 Nov 2015 19:19:22 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from pmta2.delivery6.ore.mailhop.org (pmta2.delivery6.ore.mailhop.org [54.200.129.228]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id E8B43153D for ; Mon, 9 Nov 2015 19:19:21 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from ilsoft.org (unknown [73.34.117.227]) by outbound2.ore.mailhop.org (Halon Mail Gateway) with ESMTPSA; Mon, 9 Nov 2015 19:19:22 +0000 (UTC) Received: from rev (rev [172.22.42.240]) by ilsoft.org (8.14.9/8.14.9) with ESMTP id tA9JJDs1001472; Mon, 9 Nov 2015 12:19:13 -0700 (MST) (envelope-from ian@freebsd.org) Message-ID: <1447096753.91534.514.camel@freebsd.org> Subject: Re: svn commit: r290610 - head/sys/x86/x86 From: Ian Lepore To: Roger Pau =?ISO-8859-1?Q?Monn=E9?= , Hans Petter Selasky , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Date: Mon, 09 Nov 2015 12:19:13 -0700 In-Reply-To: <5640BEBE.3070805@FreeBSD.org> References: <201511091219.tA9CJwe7067036@repo.freebsd.org> <564091FF.8090605@selasky.org> <5640BEBE.3070805@FreeBSD.org> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.16.5 FreeBSD GNOME Team Port Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Nov 2015 19:19:22 -0000 On Mon, 2015-11-09 at 16:41 +0100, Roger Pau Monné wrote: > Hello, > > El 09/11/15 a les 13.30, Hans Petter Selasky ha escrit: > > On 11/09/15 13:19, Roger Pau Monné wrote: > > > + if (dmat->common.flags & BUS_DMA_KEEP_PG_OFFSET) { > > > + /* > > > + * If we have to keep the offset of each page this > > > function > > > + * is not suitable, switch back to > > > bus_dmamap_load_ma_triv > > > + * which is going to do the right thing in this case. > > > + */ > > > + error = bus_dmamap_load_ma_triv(dmat, map, ma, buflen, > > > ma_offs, > > > + flags, segs, segp); > > > + return (error); > > > + } > > > > Hi, > > > > There has been an update made to the USB stack, which is currently > > the > > only client of "BUS_DMA_KEEP_PG_OFFSET", which means this check can > > The only in-tree client. We don't know if there are other clients out > of > the tree. > The BUS_DMA_KEEP_PG_OFFSET flag is not documented anywhere except in a short coment block near its #define which is less than 100% rigorous in specifying exactly what the flag even means. For example it is documented as being a "hint" but also includes the word "must". It doesn't say whether the flag is to be used to create a tag, to create a map, or to load a buffer. It says the offset must be kept the same in the first segment but doesn't really fully explain what the USB requirements are (and the flag was added specifically for USB). To me, all of that adds up to freedom to clarify the meaning of the flag in both code and documentation without a lot of worrying about out of tree code. And the mips and arm busdma implementations are soon going to leverage that freedom into much better implementations based on the new understanding of what that flag really requires. > > probably be skipped or relaxed a bit. The condition which must be > > fullfilled is: > > So you basically want a contiguous bounce buffer. I don't think we > can > just change BUS_DMA_KEEP_PG_OFFSET to mean "use a contiguous bounce > buffer". Maybe a new flag could be introduced to describe this new > requirement and the old one deprecated. > It's not "I want a contiguous buffer". It is "I want contiguous offsets when transitioning between (potentially non-continguous) pages." That grants you the freedom to bounce a couple different ways depending on what's most efficient for the platform and the situation. For example, you can maintain the offset-within-page in the first segment and allow the offset in all subsequent pages (bounced or not) to be zero. That's what all current implementations are doing, but it can require two full bounce pages to handle a 2-byte transfer that happens to be split across two pages (and yes that happens; it's not even rare in USB, as lots of DMA is done specifying buffers and variables on kernel stacks or in softc member variables). It also allows for the possibility of changing the offset in the first segment if doing so avoids any page crossings at all (which handles everything from the 2-byte worst case to a 4096-byte buffer). > > #ifdef USB_DEBUG > > if (nseg > 1 && > > ((segs->ds_addr + segs->ds_len) & (USB_PAGE_SIZE - 1)) > > != > > ((segs + 1)->ds_addr & (USB_PAGE_SIZE - 1))) { > > /* > > * This check verifies there is no page offset hole > > * between the first and second segment. See the > > * BUS_DMA_KEEP_PG_OFFSET flag. > > */ > > DPRINTFN(0, "Page offset was not preserved\n"); > > error = 1; > > goto done; > > } > > #endif > > AFAICT with the current bounce implementation on x86 you would have > to > specify an alignment of PAGE_SIZE in order to have this guarantee > without specifying BUS_DMA_KEEP_PG_OFFSET. > > IMHO, we should change all the current bounce buffer code and switch > to > use memdescs for everything (ie: bios and mbufs should use a memdesc > internally). Then each arch should provide functions to copy from the > different kinds of memdescs (either memdescs containing physical or > virtual memory), so the bounce code could be unified between all > arches. > Of course that's easier said than done... > I completely disagree with this, mostly because I'm halfway through a set of changes that will make arm and mips bounce handling look completely different than x86 or other platforms. That reflects the differences in underlying hardware that leads to completely different reasons for bouncing in the first place. x86 code bounces DMA when there are exclusion regions in the physical address space where DMA cannot occur. On ARM and MIPS that situation just basically never occurs (except maybe on a few ancient arm xscale systems), and virtually all bouncing is due to DMA buffers not being on cache line boundaries. Because bio and mbuf DMA typically is cache -aligned, that means that almost all bouncing is a handful of bytes at a time, and the page-replacement bounce scheme that was copied from x86 just isn't efficient (in fact it wastes megabytes of memory in preallocated bounce buffers that small-memory arm and mips systems can't really afford to waste). -- Ian