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

[Paolo Bonzini <pbonzini@redhat.com>]

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

Il 29/04/2013 13:00, David Gibson ha scritto:
> On Mon, Apr 29, 2013 at 10:11:06AM +0200, Paolo Bonzini wrote:
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>> 
>> Il 28/04/2013 03:58, David Gibson ha scritto:
>>>>> Ok, knowing about changes that happen in the IOMMU mapping
>>>>> is indeed out of scope of MemoryListeners.  What about
>>>>> adding a NotifierList? Then VFIO can register a notifier
>>>>> and use it to learn about "events" in the IOMMU mapping.
>>>>> The notifier data can be a MemoryRegionSection or
>>>>> IOMMUTableEntry, whatever you find more convenient.
>>> For the generic case a Notifier could work in principle.
>>> Neither of those structures is suitable as the data though:
>>> constructing a MemoryRegionSection for every page we map into
>>> the IOTLB is far too heavyweight, and the IOMMUTLBEntry doesn't
>>> contain the IOVA.
>> 
>> It did in Avi's patch set.  I removed it because it was unused.
>> I can add it back if you need it.
> 
> Ok, I think I will need it.

Let me know. :)

>>> Thinking over, I think what that mostly amounts to, is that if
>>> the VFIO aspects of the address space are already wired up by
>>> the host bridge, then the individual vfio-pci devices need a
>>> way of going from their qemu iommu address space (which they
>>> get from pci_dev->iommu) to the vfio specific information about
>>> that address space.
>> 
>> That can be done with just a hash table, no?
> 
> Well, yes it can.  But I think that having a whole parallel lookup 
> structure within vfio is a worse ugliness than having a single
> opaque vfio pointer in the MemoryRegion strcuture.

Why should VFIO be any special in this?  It is reassuring to me that
the VFIO maintainer thinks the same. :)

> 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
bridge cannot disappear before the VFIO devices on it are torn down.
So the lifetime should not be a problem.

Paolo

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

iQIcBAEBAgAGBQJRflueAAoJEBvWZb6bTYbybdkP/A5jrip69S4OB6XyDen7GkFy
YdX1SSs23zrVJ/PjrSqGCw4caXUecqp4b+49NCMFOF9Sy7ypvPWZvR84qpSRnFD9
oXYBpKZ3HPI0j0i/Ga84Lhz0ylyhfjTBel2jpAlJJq+odjdaHBttrQbWJIOCikMD
/mPStcU3FoQRp8KysIv0rro56IR9L0sAz7MuuJT0to7NfZHC1HrwrwJJYanB3hGN
OTTYUwRf+l9jZv5JgqanK2YcyDo0j43bvri03rdiKpJ0bj8BCQqxVOhg5TjtK3P6
EK2ABspt/q2LKWkfiC6xeudQoqcdjq2CfjqM5zqFuYExbh80MlTo5/I9hGF3zebw
naCD4qxrV/KwcrbCEqU+ieAvY1rj0ALcdSizBDPp+zemF2QzS87uXS1wjYPIvFar
oMuKnPAuAqwu7+g6uIqOFc24AIPiLwoJbnSAkiZ89wSqySKIbQj7vYJYoodxPidC
rZUr0VmspAKTfaW6lsb08KiepFaj4jySpB6bgS++hZZtPPgZ7ApUc8Yr2oZlrw8r
N+IDwPDigvFOMNSCej2YyIfZfi1CvkG/sJacCF3z+uHPIjwAu5Fy3s/gM7f27WXW
k0Dfak5l/i8QPWD6kZly6meoc+vTwiOVfKZPhLowrFG+C6G36LRuSiaDvHzS5vvM
6rar2khtc7sB+20wWDSD
=bzBg
-----END PGP SIGNATURE-----