From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52388) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SWP0q-0007Un-1m for qemu-devel@nongnu.org; Mon, 21 May 2012 05:34:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SWP0l-0004jf-A8 for qemu-devel@nongnu.org; Mon, 21 May 2012 05:34:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:63104) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SWP0l-0004jW-1s for qemu-devel@nongnu.org; Mon, 21 May 2012 05:34:43 -0400 Date: Mon, 21 May 2012 12:34:41 +0300 From: "Michael S. Tsirkin" Message-ID: <20120521093440.GK4674@redhat.com> References: <4FB60EFF.6070205@redhat.com> <1337379992.2513.17.camel@pasglop> <4FB74AB0.7090608@redhat.com> <1337549768.2458.0.camel@pasglop> <1337565405.2458.12.camel@pasglop> <4FB9F89A.90702@redhat.com> <20120521083132.GI4674@redhat.com> <1337590688.2779.37.camel@pasglop> <1337591230.2779.42.camel@pasglop> <1337591787.2779.48.camel@pasglop> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1337591787.2779.48.camel@pasglop> Subject: Re: [Qemu-devel] [PATCH] Add a memory barrier to guest memory access functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Benjamin Herrenschmidt Cc: Paolo Bonzini , qemu-devel@nongnu.org, Anthony Liguori , David Gibson On Mon, May 21, 2012 at 07:16:27PM +1000, Benjamin Herrenschmidt wrote: > On Mon, 2012-05-21 at 19:07 +1000, Benjamin Herrenschmidt wrote: > > > One thing that might alleviate some of your concerns would possibly be > > to "remember" in a global (to be replaced with a thread var eventually) > > the last transfer direction and use a simple test to chose the barrier, > > ie, store + store -> wmb, load + load -> rmb, other -> mb. But how do you know guest did a store? > > > > But first I'd be curious if some x86 folks could actually measure the > > impact of the patch as I proposed it. That would give us an idea of how > > bad the performance problem is and how far we need to go to address it. > > Another option.... go back to something more like the original patch, > ie, put the barrier in the new dma_* accessors (and provide a > non-barrier one while at it) rather than the low level cpu_physical_* > accessor. > > That makes it a lot easier for selected driver to be converted to avoid > the barrier in thing like code running in the vcpu context. It also > means that virtio doesn't get any added barriers which is what we want > as well. > > IE. Have something along the lines (based on the accessors added by the > iommu series) (using __ kernel-style, feel free to provide a better > naming) > > static inline int __dma_memory_rw( ... args ... ) > { > if (!dma_has_iommu(dma)) { > /* Fast-path for no IOMMU */ > cpu_physical_memory_rw( ... args ...); > return 0; > } else { > return iommu_dma_memory_rw( ... args ...); > } > } > > static inline int dma_memory_rw( ... args ... ) > { > smp_mb(); /* Or use finer grained as discussied earlier */ > > return __dma_memory_rw( ... args ... ) Heh. But don't we need an mb afterwards too? > } > > And corresponding __dma_memory_read/__dma_memory_write (again, feel > free to suggest a more "qemu'ish" naming if you don't like __, it's > a kernel habit, not sure what you guys do in qemu land). > > Cheers, > Ben. And my preference is to first convert everyone to __ variants and carefully switch devices to the barrier version after a bit of consideration. -- MST