From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:53466) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RDpEv-00054S-M5 for qemu-devel@nongnu.org; Tue, 11 Oct 2011 23:12:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RDpEp-0004wn-Mn for qemu-devel@nongnu.org; Tue, 11 Oct 2011 23:12:17 -0400 Received: from e23smtp02.au.ibm.com ([202.81.31.144]:57800) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RDpEp-0004uL-6P for qemu-devel@nongnu.org; Tue, 11 Oct 2011 23:12:11 -0400 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [202.81.31.247]) by e23smtp02.au.ibm.com (8.14.4/8.13.1) with ESMTP id p9C35CTi026056 for ; Wed, 12 Oct 2011 14:05:12 +1100 Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p9C39RTf1384490 for ; Wed, 12 Oct 2011 14:09:27 +1100 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p9C3BnvV032555 for ; Wed, 12 Oct 2011 14:11:50 +1100 Date: Wed, 12 Oct 2011 14:07:46 +1100 From: David Gibson Message-ID: <20111012030746.GO4849@truffala.fritz.box> References: <1315197304-22469-1-git-send-email-david@gibson.dropbear.id.au> <1315197304-22469-2-git-send-email-david@gibson.dropbear.id.au> <20111002102547.GC30747@redhat.com> <4E883CF4.6060606@redhat.com> <20111002105238.GE30747@redhat.com> <4E8843DB.1020404@redhat.com> <20111002111700.GF30747@redhat.com> <4E885286.30905@redhat.com> <20111002121426.GK30747@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20111002121426.GK30747@redhat.com> Subject: Re: [Qemu-devel] [PATCH 1/9] Add stub functions for PCI device models to do PCI DMA List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: aliguori@us.ibm.com, kraxel@redhat.com, joerg.roedel@amd.com, agraf@suse.de, qemu-devel@nongnu.org, Avi Kivity , eduard.munteanu@linux360.ro, rth@twiddle.net On Sun, Oct 02, 2011 at 02:14:28PM +0200, Michael S. Tsirkin wrote: > On Sun, Oct 02, 2011 at 02:01:10PM +0200, Avi Kivity wrote: > > On 10/02/2011 01:17 PM, Michael S. Tsirkin wrote: > > >On Sun, Oct 02, 2011 at 12:58:35PM +0200, Avi Kivity wrote: > > >> On 10/02/2011 12:52 PM, Michael S. Tsirkin wrote: > > >> >On Sun, Oct 02, 2011 at 12:29:08PM +0200, Avi Kivity wrote: > > >> >> On 10/02/2011 12:25 PM, Michael S. Tsirkin wrote: > > >> >> >On Mon, Sep 05, 2011 at 02:34:56PM +1000, David Gibson wrote: > > >> >> >> This patch adds functions to pci.[ch] to perform PCI DMA operations. At > > >> >> >> present, these are just stubs which perform directly cpu physical memory > > >> >> >> accesses. > > >> >> >> > > >> >> >> Using these stubs, however, distinguishes PCI device DMA transactions from > > >> >> >> other accesses to physical memory, which will allow PCI IOMMU support to > > >> >> >> be added in one place, rather than updating every PCI driver at that time. > > >> >> >> > > >> >> >> That is, it allows us to update individual PCI drivers to support an IOMMU > > >> >> >> without having yet determined the details of how the IOMMU emulation will > > >> >> >> operate. This will let us remove the most bitrot-sensitive part of an > > >> >> >> IOMMU patch in advance. > > >> >> >> > > >> >> >> Signed-off-by: David Gibson > > >> >> > > > >> >> >So something I just thought about: > > >> >> > > > >> >> >all wrappers now go through cpu_physical_memory_rw. > > >> >> >This is a problem as e.g. virtio assumes that > > >> >> >accesses such as stw are atomic. cpu_physical_memory_rw > > >> >> >is a memcpy which makes no such guarantees. > > >> >> > > > >> >> > > >> >> Let's change cpu_physical_memory_rw() to provide that guarantee for > > >> >> aligned two and four byte accesses. Having separate paths just for > > >> >> that is not maintainable. > > >> > > > >> >Well, we also have stX_phys convert to target native endian-ness > > >> >(nop for KVM but not necessarily for qemu). > > >> > > > >> >So if we do what you suggest, this patch will become more correct, but > > >> >it would still need to duplicate the endian-ness work. > > >> > > > >> >For that reason, I think calling stX_phys and friends from pci > > >> >makes more sense - we get more simple inline wrappers > > >> >but that code duplication worries me much less than tricky > > >> >endian-ness hidden within a macro. > > >> > > > >> > > >> Good point. Though this is really a virtio specific issue since > > >> other devices have explicit endianness (not guest dependent). > > > > > >Hmm, not entirely virtio specific, some devices use stX macros to do the > > >conversion. E.g. stw_be_phys and stl_le_phys are used in several > > >places. > > > > These are fine - explicit endianness. > > Right. So changing these to e.g. stl_dma and assuming > LE is default seems like a step backwards. Um.. why? PCI is defined by the spec to be LE, so I don't see that we need explicit endianness versions for PCI helpers. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson