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 <pbonz...@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 <pbonz...@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
signature.asc
Description: Digital signature
