* Re: [Qemu-devel] [PATCH 09/17] memory: iommu support
@ 2013-05-01 4:35 David Gibson
2013-05-01 16:10 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: David Gibson @ 2013-05-01 4:35 UTC (permalink / raw)
To: pbonzini; +Cc: aik, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2087 bytes --]
> From: Avi Kivity <avi.kivity@gmail.com>
>
> 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 <avi.kivity@gmail.com>
> [Modified to put translation in address_space_translate - Paolo]
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 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.
--
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 --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH 09/17] memory: iommu support
2013-05-01 4:35 [Qemu-devel] [PATCH 09/17] memory: iommu support David Gibson
@ 2013-05-01 16:10 ` Paolo Bonzini
2013-05-02 3:05 ` David Gibson
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2013-05-01 16:10 UTC (permalink / raw)
To: David Gibson; +Cc: aik, qemu-devel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 01/05/2013 06:35, David Gibson ha scritto:
>> From: Avi Kivity <avi.kivity@gmail.com>
>>
>> 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 <avi.kivity@gmail.com> [Modified to put
>> translation in address_space_translate - Paolo] Signed-off-by:
>> Paolo Bonzini <pbonzini@redhat.com> --- 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.
Paolo
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQIcBAEBAgAGBQJRgT6GAAoJEBvWZb6bTYbyk/cP/RCHjFrtXYas+TLti8vcbCS1
zCUGnURgNqKxI4529E9VGF4PlO6/uxDP/mpNgDx7FCkMyA6bNL6fxw7RhvWyKFjU
xvZt6l7HYVOxbuNVAraZ4JmQ+fnvgyxJn1djMKNdGKNuRdpXj6b2NC+xyw8rSSc2
bIR6FT0piAlFP+kXiNFyf3cIZzT84qVX1tqphCRov7jvWyYG8Zv5VHpEEjaY7fJS
LEJFQ4ZfjM94RAekhZ7GkGmmlh9vfTplKz0G+0/EOYcaWBNwd7bKjGf6hWNtLb3z
3chtMKyODe8JRR/FuWuLx0GDZzvKZOE9yRGHerhL0aDCMviNKILUTnES3twe82j9
F8ywYO4CnkdKRIzKXXDTd8hZkgzF60fWzMvtvjOg6oXHJFyNoXRLhD23sZsTFaMi
q+mk7JiPfXrvjrZcUlQ4YwN7gv9gkZ+2JO1di/R9Xq62aqYBLke9Fwke7fdX+qbR
RB7coXgfExheRJdyWB56Lqr9m2KAtnW7T6ISR1G/nV0yr+ye1N9LifUkCAg4MHa8
ANsMMA6NbwmwtqJ9DDqzH+RkK7ZzCzq/IkYGeEial/sUnxXLggu6LanyahTMun49
ZmJDbEUWVAbrA4bkbQ1XHM6B2EFLg//wen7uSdIR+UZKgjkhxpB+PKiLppvPyXW1
5LuT1XZmghaeE3nD/i3A
=GcNo
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH 09/17] memory: iommu support
2013-05-01 16:10 ` Paolo Bonzini
@ 2013-05-02 3:05 ` David Gibson
2013-05-02 5:24 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: David Gibson @ 2013-05-02 3:05 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: aik, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2594 bytes --]
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 <avi.kivity@gmail.com>
> >>
> >> 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 <avi.kivity@gmail.com> [Modified to put
> >> translation in address_space_translate - Paolo] Signed-off-by:
> >> Paolo Bonzini <pbonzini@redhat.com> --- 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
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH 09/17] memory: iommu support
2013-05-02 3:05 ` David Gibson
@ 2013-05-02 5:24 ` Paolo Bonzini
2013-05-02 6:28 ` David Gibson
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2013-05-02 5:24 UTC (permalink / raw)
To: David Gibson; +Cc: aik, qemu-devel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 02/05/2013 05:05, David Gibson ha scritto:
>>> 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.
But does the tce->iommu address space always exist? Can you have
multiple domains and so on?
I can surely change it if you prefer, after all you're the only user
right now (address_space_memory always exists). But I'm not 100% sure
it will create more problems than it solves.
Paolo
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQIcBAEBAgAGBQJRgfimAAoJEBvWZb6bTYby4LgP/0B+I7Hql/TYBputor6VwH8S
xC2EK8uMpmmTr5RknsaQRQQTPmaXzb1YEXNRN7GKbsEoP5ibSfoFFhMTkUurILWS
KdEk+qE3bKnkVqKbyhS3t+zhae7x4fEVO2Cvob3DS/eiaSPuFex3BmHInGyoH7IK
ZrTmJwfhWZV0xOKQ96Wyz0tGDTSEhbgn4kIaZANQXbf6Em1dmKsqAyeegotu+oje
R6rwJ4lDKlKNAhFOs0lJYHOIsh+EH09QNZiAVWbp8Bx5B0m02bpFBwFZUJWxt3gh
1XspLS+TYHqOqiq//WVIGEJ6U6hepyoU9vFUVfeaW02PFU4+D9VuC6Yd5r4sRYzJ
BmwebxeNSrNnCsrDz/R/w07+VDgCgNmhSLDfOx3JEpt/uuw3oxKODpb5+bMMZodO
rz8Qv6RilozMK9b9j9O4+n6KFRLMoaqmyS4qWRiSmeF3myD0m8NaKynsutB8efQ9
Zm0/vinWhyFrl3zR/daRY3RY25IEtV2EtGcVCGVrtIhvgaI02enSMoo41wO2bdWQ
W+JmX1Y6+2NaCzhx6K7Fc2CpAIynGd3jC0d/XG4OL7Ik6SF0eA/3fRDTF/2/d1HZ
7EeaiG7OF6YznA68qAGcizyYCUNY8uRjv8rSFL842xGetvHQW5bZ4LNGJbLfPVpn
qzGnzdS7tqVa1w48qrNv
=RGlp
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 09/17] memory: iommu support
2013-05-02 5:24 ` Paolo Bonzini
@ 2013-05-02 6:28 ` David Gibson
2013-05-02 7:36 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: David Gibson @ 2013-05-02 6:28 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1794 bytes --]
On Thu, May 02, 2013 at 07:24:54AM +0200, Paolo Bonzini wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Il 02/05/2013 05:05, David Gibson ha scritto:
> >>> 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.
>
> But does the tce->iommu address space always exist? Can you have
> multiple domains and so on?
I'm not sure exactly what you mean here. We have multiple
"partitionable endpoints" that are somewhat equivalent to iommu
domains on intel - each corresponds to a different DMA address space.
By convention, though, these are configured by firmware and never
changed during runtime. For simplicity in the qemu guest case, we
only ever have one partitionable endpoint per (guest) PCI host
bridge. That's not really a limitation, because pseries can happily
support multiple host bridges (even dozens or hundreds).
> I can surely change it if you prefer, after all you're the only user
> right now (address_space_memory always exists). But I'm not 100% sure
> it will create more problems than it solves.
Well, we're talking here about the *target* address space of the
iommu. So that's address_space_memory for us too. In fact I don't
know of any realistic cases where the target AS won't be
address_space_memory, although it's certainly theoretically possible.
--
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 --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 09/17] memory: iommu support
2013-05-02 6:28 ` David Gibson
@ 2013-05-02 7:36 ` Paolo Bonzini
0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2013-05-02 7:36 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-devel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 02/05/2013 08:28, David Gibson ha scritto:
> Well, we're talking here about the *target* address space of the
> iommu. So that's address_space_memory for us too. In fact I
> don't know of any realistic cases where the target AS won't be
> address_space_memory, although it's certainly theoretically
> possible.
Ok, that pretty much answers it. Thanks.
Paolo
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQIcBAEBAgAGBQJRgheZAAoJEBvWZb6bTYbyIf0QAJUzHat6Zlstfwga/tRqrlPX
MS6UqXb6vxjNqvhrqJ3lrjvNG4LSkhsYUn+BJCLKMDf0Eg7uA8fqwywbSH0xkjJJ
nJo9rOE+47tKN6uR6aDqtMYu77vmw3Jg1EcHbJcp1zfOcWy5zGP/q24lPzfc3tdr
o6CGqEUJ80LwkgluKQy1Id7hCpObIAXg2Bw8B0/BeCD13xktC0ePQZppSOOOMvA7
emcDqUkJMH7xGaol8GM/gSRaPXnhj7w2PYa22tkVEpINx4oU6Wyt8b2reoG8rP1y
VX7ea3lMs7XS/c1L+hGqXwTVnSZQYBt8ZDg6IF7RTbh77uAgG+kYqPjU3vw72H/d
KQaJm1bOsoO9R6GdUL2IlHmGR1Dom/ljldC/ICfbUrQLNLI6/ohiVebAoqp0VDd7
+4JmWnSINy8nnXWBHmlA/Jm0f2EdXzXL+Gg0ey8XFLLqahEq8kQbQXGHlv9BTSZq
DRWcQfqoT8PqL4YckB8LS56CkZFNoxbO0uK/QZvCYzyf4JOjuLv0egSYfG6qAKTx
ZZvsRCJYABFQ0F38qq0yfwW9mriVwDaImk+zaxzfFYbvNnq/2nUmpkd1XdN40UZb
7pDonAuBzP63kjI0gfN8Pkka3I9+I4v5KqAlgyeUgGtC2/li7SQmPVnNea2zhQ1w
bisa4w8YyhHMuauOja6v
=u23p
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-02 7:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-01 4:35 [Qemu-devel] [PATCH 09/17] memory: iommu support David Gibson
2013-05-01 16:10 ` Paolo Bonzini
2013-05-02 3:05 ` David Gibson
2013-05-02 5:24 ` Paolo Bonzini
2013-05-02 6:28 ` David Gibson
2013-05-02 7:36 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).