From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:54392) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QRlzi-0000D5-4A for qemu-devel@nongnu.org; Wed, 01 Jun 2011 10:01:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QRlzY-0005IF-Vg for qemu-devel@nongnu.org; Wed, 01 Jun 2011 10:01:57 -0400 Received: from mail-pw0-f45.google.com ([209.85.160.45]:40390) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QRlzY-0005Hs-IQ for qemu-devel@nongnu.org; Wed, 01 Jun 2011 10:01:48 -0400 Received: by pwi6 with SMTP id 6so40745pwi.4 for ; Wed, 01 Jun 2011 07:01:47 -0700 (PDT) Sender: Richard Henderson Message-ID: <4DE64646.3010505@twiddle.net> Date: Wed, 01 Jun 2011 07:01:42 -0700 From: Richard Henderson MIME-Version: 1.0 References: <1306892315-7306-1-git-send-email-eduard.munteanu@linux360.ro> <1306892315-7306-2-git-send-email-eduard.munteanu@linux360.ro> In-Reply-To: <1306892315-7306-2-git-send-email-eduard.munteanu@linux360.ro> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH 01/13] Generic DMA memory access interface List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduard - Gabriel Munteanu Cc: aliguori@us.ibm.com, kvm@vger.kernel.org, mst@redhat.com, aik@ozlabs.ru, joro@8bytes.org, seabios@seabios.org, qemu-devel@nongnu.org, agraf@suse.de, blauwirbel@gmail.com, yamahata@valinux.co.jp, paul@codesourcery.com, kevin@koconnor.net, avi@redhat.com, dwg@au1.ibm.com, david@gibson.dropbear.id.au On 05/31/2011 06:38 PM, Eduard - Gabriel Munteanu wrote: > +static inline void dma_memory_rw(DMADevice *dev, > + dma_addr_t addr, > + void *buf, > + dma_addr_t len, > + int is_write) I don't think this needs to be inline... > +{ > + /* > + * Fast-path non-iommu. > + * More importantly, makes it obvious what this function does. > + */ > + if (!dev || !dev->mmu) { > + cpu_physical_memory_rw(addr, buf, len, is_write); > + return; > + } ... because you'll never be able to eliminate the if or the calls. You might as well make the overall code smaller by taking the entire function out of line. > +#define DEFINE_DMA_LD(prefix, suffix, devtype, dmafield, size) \ > +static inline uint##size##_t \ > +dma_ld##suffix(DMADevice *dev, dma_addr_t addr) \ > +{ \ > + int err; \ > + dma_addr_t paddr, plen; \ > + \ > + if (!dev || !dev->mmu) { \ > + return ld##suffix##_phys(addr); \ > + } \ Similarly for all the ld/st functions. > +#define DEFINE_DMA_MEMORY_RW(prefix, devtype, dmafield) > +#define DEFINE_DMA_MEMORY_READ(prefix, devtype, dmafield) > +#define DEFINE_DMA_MEMORY_WRITE(prefix, devtype, dmafield) > + > +#define DEFINE_DMA_OPS(prefix, devtype, dmafield) \ I think this is a bit over the top, really. > + err = dev->mmu->translate(dev, addr, &paddr, &plen, is_write); I see you didn't take my suggestion for using an opaque callback pointer. Really and truly, I won't be able to use this as-is for Alpha. r~