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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 1880 bytes --]

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.

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.
-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]