Re: [Qemu-devel] [RFC PATCH 01/13] Generic DMA memory access interface

Richard Henderson
2011-06-01T14:01:47Z

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~