Re: [Qemu-devel] [PATCH] dma/rc4030: do multiple calls to address_space_rw when doing DMA transfers

[Paolo Bonzini]

On 16/06/2015 19:48, Aurelien Jarno wrote:
> The code assumes that if you don't have an IOMMU, the address range in
> the underlying memory region is linear.

I think this is exactly what Peter Crosthwaite's infamous :) "exec:
Respect as_translate_internal length clamp" patch was trying to fix.
However, address_space_translate_internal uses section->mr->size instead
of section->size.  I'll post a patch once I'm through the email deluge
from 1 week of absence.

If I read correctly the patch that introduced address_space_translate,
the bug has always been there.

> One fix would be to adjust the
> length even without IOMMU. That would have some performance impact
> though, so maybe we want to make this assumption clear and always use an
> IOMMU in that case.

I don't think there would be a performance impact, except in buggy cases
such as the one Hervé is fixing.

Paolo

>>> I therefore wonder if
>>> you therefore shouldn't model this DMA translation tables by using IOMMU
>>> ops instead of subregions.
>>>
>> No, in my opinion, that's an implementation detail. Paolo said that it was 
>> OK:
>> "Both are okay.  The IOMMU makes address space changes faster; your
>> scheme is basically a form of caching, it trades update performance for
>> improved translation performance."
>> http://lists.gnu.org/archive/html/qemu-devel/2015-03/msg05486.html
> 
> It seems wrong with the current code. And if we fix the bug by adjusting
> the length, the above sentence about the performances might becomes
> wrong
> 
> Aurelien.
>