From mboxrd@z Thu Jan  1 00:00:00 1970
From: Richard Henderson <rth@twiddle.net>
Subject: Re: [Qemu-devel] [RFC PATCH 01/13] Generic DMA memory access interface
Date: Wed, 01 Jun 2011 07:01:42 -0700
Message-ID: <4DE64646.3010505@twiddle.net>
References: <1306892315-7306-1-git-send-email-eduard.munteanu@linux360.ro> <1306892315-7306-2-git-send-email-eduard.munteanu@linux360.ro>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Cc: mst@redhat.com, aliguori@us.ibm.com, david@gibson.dropbear.id.au,
	kvm@vger.kernel.org, aik@ozlabs.ru, joro@8bytes.org,
	seabios@seabios.org, qemu-devel@nongnu.org, agraf@suse.de,
	blauwirbel@gmail.com, yamahata@valinux.co.jp, kevin@koconnor.net,
	avi@redhat.com, dwg@au1.ibm.com, paul@codesourcery.com
To: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-pw0-f46.google.com ([209.85.160.46]:48548 "EHLO
	mail-pw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753247Ab1FAOBr (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 1 Jun 2011 10:01:47 -0400
Received: by pwi15 with SMTP id 15so35575pwi.19
        for <kvm@vger.kernel.org>; Wed, 01 Jun 2011 07:01:47 -0700 (PDT)
In-Reply-To: <1306892315-7306-2-git-send-email-eduard.munteanu@linux360.ro>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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~