From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48231) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UX0EF-0006ms-16 for qemu-devel@nongnu.org; Mon, 29 Apr 2013 22:23:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UX0EA-0003Ur-MN for qemu-devel@nongnu.org; Mon, 29 Apr 2013 22:23:38 -0400 Received: from ozlabs.org ([2402:b800:7003:1:1::1]:58026) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UX0EA-0003UX-1x for qemu-devel@nongnu.org; Mon, 29 Apr 2013 22:23:34 -0400 Date: Tue, 30 Apr 2013 12:23:29 +1000 From: David Gibson Message-ID: <20130430022329.GV20202@truffula.fritz.box> References: <517A83CE.2070505@redhat.com> <20130427094927.GC20202@truffula.fritz.box> <517BC1F2.70405@redhat.com> <20130428015822.GE20202@truffula.fritz.box> <517E2B1A.8000209@redhat.com> <20130429110047.GL20202@truffula.fritz.box> <517E5B9E.1020006@redhat.com> <20130429115653.GQ20202@truffula.fritz.box> <517E794B.1060307@redhat.com> <20130430020539.GT20202@truffula.fritz.box> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="ed2sj0fx90rgoibw" Content-Disposition: inline In-Reply-To: <20130430020539.GT20202@truffula.fritz.box> Subject: Re: [Qemu-devel] [PATCH 3/4] vfio: Move container list to iommu MemoryRegion List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: aik@ozlabs.ru, alex.williamson@redhat.com, qemu-devel@nongnu.org --ed2sj0fx90rgoibw Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 > >=20 > > 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. :) > > >=20 > > > Because device passthrough is a sufficiently special case, IMO. > > > It introduces requirements that are fundamentally different from > > > any emulated device. > > >=20 > > >>> 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. > > >>=20 > > >> There is only one MemoryRegion per PCI host bridge, and the PCI > > >> host > > >=20 > > > That's true for existing examples, but it need not be true. For=20 > > > 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. > > >=20 > > >> bridge cannot disappear before the VFIO devices on it are torn > > >> down. So the lifetime should not be a problem. > > >=20 > > > 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. > >=20 > > 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". >=20 > 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. >=20 > > We should look at generic fixes before dropping hooks in the code. At > > the very least an "assert(mr->parent =3D=3D NULL);" is missing in > > memory_region_destroy. >=20 > 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. --=20 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 --ed2sj0fx90rgoibw Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iEYEARECAAYFAlF/KyEACgkQaILKxv3ab8aHawCff0Il2Xlig3pIH9o08XWq5MBN Z0sAnjwZcBoxZWmseN7Zl7eQWddnZXfZ =gY+S -----END PGP SIGNATURE----- --ed2sj0fx90rgoibw--