Re: [Qemu-devel] [PATCH 07/17] memory: add address_space_valid

[Paolo Bonzini <pbonzini@redhat.com>]

Il 01/05/2013 06:02, David Gibson ha scritto:
> Hi Paolo,
> 
> I've been looking through your iommu branch, and spotted a few
> things, so sending some comments about them.  Since I haven't see
> where they've been posted before (if anywhere), I've kind of
> reconstructed a mail to reply to from the patch in git.  I hope the
> result isn't too cryptic.

It's understandable, and very much appreciated! :)

Paolo

> On Wed, May 01, 2013 at 01:18:30PM +1000, David Gibson wrote:
>> From: Paolo Bonzini <pbonzini@redhat.com>
>> 
>> Checking whether an address space is possible in the old-style 
>> IOMMU implementation, but there is no equivalent in the memory
>> API. Implement it with a lookup of the dispatch tree.
>> 
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> [snip]
>> --- dma-helpers.c         |    5 +++++ exec.c                |
>> 24 ++++++++++++++++++++++++ include/exec/memory.h |   12
>> ++++++++++++ include/sysemu/dma.h  |    3 ++- 4 files changed, 43
>> insertions(+), 1 deletion(-)
>> 
>> diff --git a/dma-helpers.c b/dma-helpers.c index 272632f..2962b69
>> 100644 --- a/dma-helpers.c +++ b/dma-helpers.c @@ -298,6 +298,11
>> @@ bool iommu_dma_memory_valid(DMAContext *dma, dma_addr_t addr,
>> dma_addr_t len, plen = len; }
>> 
>> +        if (!address_space_valid(dma->as, paddr, len, +
>> dir == DMA_DIRECTION_FROM_DEVICE)) { +            return false; +
>> }
> 
> I think the argument to address_space_valid() should be plen, not 
> len.  Otherwise we're checking that a range the size of the
> original area is valid at the target address of just the first
> page.  Of course, since the target address space will usually be
> falt and much bigger than a page, it's unlikely to trigger.
>