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

[Richard Henderson <rth@twiddle.net>]

On 06/02/2011 08:12 AM, David Gibson wrote:
> +    err = iommu->translate(dev, addr, &paddr, &plen, is_write);
> +    if (err) {
> +        return NULL;
> +    }
> +
> +    /*
> +     * If this is true, the virtual region is contiguous,
> +     * but the translated physical region isn't. We just
> +     * clamp *len, much like cpu_physical_memory_map() does.
> +     */
> +    if (plen < *len) {
> +        *len = plen;
> +    }
> +
> +    buf = cpu_physical_memory_map(paddr, &plen, is_write);
> +    *len = plen;
> +
> +    /* We treat maps as remote TLBs to cope with stuff like AIO. */

Oh, that reminds me.  There's a bug here in Eduard's original:

PLEN is set to the maximum length of the transfer by the 
translate function.  What we do *not* want is to pass a
very very large region to cpu_physical_memory_map.

The effects of this are hard to see with the AMD IOMMU, since
it is entirely page based and thus PLEN will be increased by
no more than 4k, but Alpha IOMMUs have direct-mapped translation
windows that can be up to 128GB.

I'm unsure whether I prefer to force the translator function
to never increase PLEN (which is what I implemented in my 
own branch) or whether all callers of the translate function
must be aware that the returned PLEN can increase.


r~