From owner-svn-src-head@freebsd.org Tue Nov 10 12:27:51 2015 Return-Path: Delivered-To: svn-src-head@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 56AC4A2AC75; Tue, 10 Nov 2015 12:27:51 +0000 (UTC) (envelope-from royger@gmail.com) Received: from mail-wm0-x22d.google.com (mail-wm0-x22d.google.com [IPv6:2a00:1450:400c:c09::22d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id DF6E41F8E; Tue, 10 Nov 2015 12:27:50 +0000 (UTC) (envelope-from royger@gmail.com) Received: by wmww144 with SMTP id w144so115364736wmw.1; Tue, 10 Nov 2015 04:27:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:subject:to:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-type:content-transfer-encoding; bh=v7V+++GO9+RPz6RYF9YB1Ww0+ES8BnhzS+5cVX2kSAo=; b=RAGXMeOebPeWm24FgQxxe+qIE8SiTaRJotGiAUE8OXeYzRITgauGUKkf5X2wJT+h5b qc0vNr1L3YHM+Tge0aMGjUxu5U9zWXZxgt8ZCQJC87Mo0L0TxjMA4Uvz8Do9P3cB0wtI I4Ka4LbRKGLU4u5T2onfZWZzM5Ocb6K3Y0ov04/mXAPDb8+xVrogtHtnVfE8QPDKnWvJ dHQGitqidPmmQlkRmBOulBPMScyNguqRphNNI/bqA+cdtdi14HItgp6/bRm+4Z7tVN3F EVVa7EGuZkpgef84koQawkh84qHkccA0gw4otjjLVShmuec3DOZ8yszUmnDW7gyXgdFG NohQ== X-Received: by 10.28.6.70 with SMTP id 67mr4809021wmg.54.1447158469329; Tue, 10 Nov 2015 04:27:49 -0800 (PST) Received: from [172.16.1.30] (15.Red-83-50-122.dynamicIP.rima-tde.net. [83.50.122.15]) by smtp.gmail.com with ESMTPSA id u17sm3872670wmd.8.2015.11.10.04.27.48 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 10 Nov 2015 04:27:48 -0800 (PST) Sender: =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= Subject: Re: svn commit: r290610 - head/sys/x86/x86 To: Ian Lepore , Hans Petter Selasky , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201511091219.tA9CJwe7067036@repo.freebsd.org> <564091FF.8090605@selasky.org> <5640BEBE.3070805@FreeBSD.org> <1447096753.91534.514.camel@freebsd.org> From: =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= X-Enigmail-Draft-Status: N1110 Message-ID: <5641E2C2.8020009@FreeBSD.org> Date: Tue, 10 Nov 2015 13:27:46 +0100 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <1447096753.91534.514.camel@freebsd.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Nov 2015 12:27:51 -0000 El 09/11/15 a les 20.19, Ian Lepore ha escrit: > 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. Maybe rename the flag to BUS_DMA_CONTIGUOUS_PG_OFFSET? >>> 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). Yes, that's what I've now tried to do in x86: https://reviews.freebsd.org/D4119 Change the offset in the first segment and then making sure everything is contiguous in terms of offsets. >>> #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). I see, I guess you are right that other arches have very different requirements in terms of bouncing, and that unifying them wouldn't work that well. I got that conclusion after looking at some of the busdma implementations for other arches which right now look quite similar, didn't know there was a major rework planned. Roger.