[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 09/17] memory: iommu support
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [PATCH 09/17] memory: iommu support |
Date: |
Thu, 2 May 2013 13:05:08 +1000 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, May 01, 2013 at 06:10:47PM +0200, Paolo Bonzini wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Il 01/05/2013 06:35, David Gibson ha scritto:
> >> From: Avi Kivity <address@hidden>
> >>
> >> Add a new memory region type that translates addresses it is
> >> given, then forwards them to a target address space. This is
> >> similar to an alias, except that the mapping is more flexible
> >> than a linear translation and trucation, and also less efficient
> >> since the translation happens at runtime.
> >>
> >> The implementation uses an AddressSpace mapping the target region
> >> to avoid hierarchical dispatch all the way to the resolved
> >> region; only iommu regions are looked up dynamically.
> >>
> >> Signed-off-by: Avi Kivity <address@hidden> [Modified to put
> >> translation in address_space_translate - Paolo] Signed-off-by:
> >> Paolo Bonzini <address@hidden> --- exec.c |
> >> 35 +++++++++++++++++++++++++++++------ include/exec/memory.h |
> >> 44 ++++++++++++++++++++++++++++++++++++++++++++ memory.c
> >> | 28 ++++++++++++++++++++++++++++ 3 files changed, 101
> >> insertions(+), 6 deletions(-)
> >
> > [snip]
> >> +void memory_region_init_iommu(MemoryRegion *mr, +
> >> MemoryRegionIOMMUOps *ops, +
> >> MemoryRegion *target, + const char
> >> *name, + uint64_t size) +{ +
> >> memory_region_init(mr, name, size); + mr->ops = NULL; +
> >> mr->iommu_ops = ops, + mr->opaque = mr; + mr->terminates =
> >> true; /* then re-forwards */ + mr->destructor =
> >> memory_region_destructor_iommu; + mr->iommu_target_as =
> >> g_new(AddressSpace, 1); +
> >> address_space_init(mr->iommu_target_as, target);
> >
> > Since IOMMUs are very likely to share a target AS (in fact, it
> > will nearly always be system memory), it seems odd to me to
> > construct new AddressSpace objects for each one, rather than just
> > giving the AddressSpace as the parameter to
> > memory_region_init_iommu.
> >
>
> I think the problem is that we do not have reference counting, and
> this makes it simpler to manage the lifetime. It can be changed later.
I don't really follow this logic. In the existing case, the iommu
target is always system memory, and there's already
address_space_memory which always exists.
--
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