On Tue, Apr 30, 2013 at 12:05:39PM +1000, David Gibson wrote: > On Mon, Apr 29, 2013 at 03:44:43PM +0200, Paolo Bonzini wrote: > > -----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". > > Well.. no. The same problem may well exist for AddressSpace objects, > but in this case it's for the VFIO private per-address-space object. > > > 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. > > Well, sure that's probably also a good idea. But the whole point here > is you're insisting that the MemoryRegion code doesn't know about the > vfio private data, even as an opaque handle, and so there's no > possible assert we can put there to check it has been destroyed.
Oh, yes, forgot to ask. I'm still unclear on what the conceptual
difference is supposed to between a MemoryRegion and an AddressSpace.
AFAICT AddressSpace seems to be roughly just a wrapper on a
MemoryRegion that gives it some more features.
--
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
