Re: [Qemu-devel] [PATCH 3/4] vfio: Move container list to iommu MemoryRegion

[Paolo Bonzini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 29/04/2013 13:56, David Gibson ha scritto:
>> Why should VFIO be any special in this?  It is reassuring to me
>> that the VFIO maintainer thinks the same. :)
> 
> Because device passthrough is a sufficiently special case, IMO.
> It introduces requirements that are fundamentally different from
> any emulated device.
> 
>>> It does also require making sure the lifetime handling is
>>> correct. The entry from the hash table must be removed before
>>> the corresponding MemoryRegion is free()ed; otherwise we could
>>> end up using the same pointer for a newly constructed
>>> MemoryRegion, and get a false lookup in the hash.  Whether that
>>> happens essentially never, or almost immediately in practice
>>> depends on the malloc() implementation, of course.
>> 
>> There is only one MemoryRegion per PCI host bridge, and the PCI
>> host
> 
> That's true for existing examples, but it need not be true.  For 
> example the Intel IOMMU supports multiple "iommu domains" which
> are different address spaces on the same host bridge.  When one of
> those domains is reconfigured, we will again need to call into vfio
> by some mechanism to readjust the host side iommu configuration
> accordingly.
> 
>> bridge cannot disappear before the VFIO devices on it are torn
>> down. So the lifetime should not be a problem.
> 
> I think it is very risky to assume that existing constraints
> control the lifetime for us, since we don't know what variety of
> iommus we may have in future.  I really think we should have an
> explicit hook which allows us to call vfio side cleanup code when a
> guest side iommu region is destroyed.  Personally, I still think a
> special-purpose vfio hook is the simplest way to do that, but a
> more general use Notifier list or something in the right place
> could do the job too.

I think this is a different problem.  Basically the question is "what
happens if a MemoryRegion 'disappears' while an AddressSpace is still
referring to it", and the answer right now is "badness".

We should look at generic fixes before dropping hooks in the code.  At
the very least an "assert(mr->parent == NULL);" is missing in
memory_region_destroy.

Paolo

> I do realise why we don't really want vfio specific hooks in the 
> generic code.  But we absolutely do need some kind of hooks which
> vfio can use, and until we have a second potential user of those
> hooks, I think general hooks are overengineering.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRfnlKAAoJEBvWZb6bTYbyC5MP/2mq2mZ9Maov0bq+AZl4zrA+
li7I1/GWXyZewfWrtkQ8onTjt1jYBcEiWlIPEYpx9S39XZtXke80w5dO4L8gA0Ow
tvFbHx8LdZRSNIuMvcq8Gw22olWKfjDisqYGHhPTw0hrSGBV6K19Jfqv4RFqovRw
mzxY3CKF3G5hJVQJmBipd/1dn0kqS3+OFZI/6Q51b3VfAxRLbNmxW0kfIbVsKbGq
5ctcIMoBJwHiQ5uuUfm1FSgSAtU8hIzHZPRYg/FnAM08u6ERQbVCW54h5RA6ECcx
hfQzTmgHsIYZEMGhvsq9h9hsro9tehchN6u2ANzrFinFNAw8U4xaPQO5OQej2Le2
50f9VoqF7Y7hhnYVivHGQVol45SNdGKV8GJws9nVxJfAIHb7Tu6SYrr7FS+5EYPl
gwH5nUqbWZP8Ss4rLdCOt4hatHwARHv5XScxQWOyjAXC1tJO228i18e1QmsOxaBN
eYN4635InosNW7jtIl1f7zv+iqkcYqkecR26Dw5Qrcy3AlivqU1s/m4mtL1GCZvT
+xO+oFCROuwNLaNBN0if4FlQfPRkDnt5uoloZYLBr4BQJ8ZIUyp4lkiAxZM3OsYp
7r+A0gT4DomxolBF0xOHZCMfc3tSOFZEPPF7cwxqRPJf9uaMG5SfYK6IHUE0zyEg
OGG/l3wDDNtGz/ZEVTfW
=biGU
-----END PGP SIGNATURE-----