From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58304) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uc4t4-0000by-01 for qemu-devel@nongnu.org; Mon, 13 May 2013 22:22:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uc4sy-00028S-IT for qemu-devel@nongnu.org; Mon, 13 May 2013 22:22:45 -0400 Received: from ozlabs.org ([2402:b800:7003:1:1::1]:53832) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uc4sx-00028B-UC for qemu-devel@nongnu.org; Mon, 13 May 2013 22:22:40 -0400 Date: Tue, 14 May 2013 11:58:30 +1000 From: David Gibson Message-ID: <20130514015830.GG14944@truffula.fritz.box> References: <1368442465-14363-1-git-send-email-david@gibson.dropbear.id.au> <1368442465-14363-9-git-send-email-david@gibson.dropbear.id.au> <1368480786.5520.81.camel@ul30vt.home> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="yZnyZsPjQYjG7xG7" Content-Disposition: inline In-Reply-To: <1368480786.5520.81.camel@ul30vt.home> Subject: Re: [Qemu-devel] [PATCH 8/8] vfio: Create VFIOAddressSpace objects as needed List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: aik@ozlabs.ru, pbonzini@redhat.com, qemu-devel@nongnu.org, agraf@suse.de, mst@redhat.com --yZnyZsPjQYjG7xG7 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, May 13, 2013 at 03:33:06PM -0600, Alex Williamson wrote: > On Mon, 2013-05-13 at 20:54 +1000, David Gibson wrote: > > So far, VFIO has a notion of different logical DMA address spaces, but > > only ever uses one (system memory). This patch extends this, creating > > new VFIOAddressSpace objects as necessary, according to the AddressSpace > > reported by the PCI subsystem for this device's DMAs. > >=20 > > This isn't enough yet to support guest side IOMMUs with VFIO, but it do= es > > mean we could now support VFIO devices on, for example, a guest side PCI > > host bridge which maps system memory at somewhere other than 0 in PCI > > space. > >=20 > > Signed-off-by: David Gibson > > --- > > hw/misc/vfio.c | 36 +++++++++++++++++++++++++++++------- > > 1 file changed, 29 insertions(+), 7 deletions(-) > >=20 > > diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c > > index b1e9220..3850d39 100644 > > --- a/hw/misc/vfio.c > > +++ b/hw/misc/vfio.c > > @@ -116,9 +116,10 @@ enum { > > typedef struct VFIOAddressSpace { > > AddressSpace *as; > > QLIST_HEAD(, VFIOContainer) containers; > > + QLIST_ENTRY(VFIOAddressSpace) list; > > } VFIOAddressSpace; > > =20 > > -static VFIOAddressSpace vfio_address_space_memory; > > +QLIST_HEAD(, VFIOAddressSpace) vfio_address_spaces; > > =20 > > struct VFIOGroup; > > =20 > > @@ -2635,6 +2636,23 @@ static void vfio_address_space_init(VFIOAddressS= pace *vas, AddressSpace *as) > > QLIST_INIT(&vas->containers); > > } > > =20 > > +static VFIOAddressSpace *vfio_address_space_get(AddressSpace *as) >=20 > vfio_get_address_space is a better match for the rest of the code. Ok. > > +{ > > + VFIOAddressSpace *vas; > > + > > + QLIST_FOREACH(vas, &vfio_address_spaces, list) { > > + if (vas->as =3D=3D as) > > + return vas; > > + } > > + > > + /* No suitable VFIOAddressSpace, create a new one */ > > + vas =3D g_malloc0(sizeof(*vas)); > > + vfio_address_space_init(vas, as); > > + QLIST_INSERT_HEAD(&vfio_address_spaces, vas, list); >=20 > Do we still need vfio_address_space_init? Seems like it should be > rolled in here. Ah, true. I had some notions of allowing host bridges to statically allocate a vfio address space as part of their own structure, but this current code assumes they are malloc()ed by get_address_space, so yes, I'll fold that in. > > + > > + return vas; > > +} > > + > > static int vfio_connect(VFIOGroup *group, VFIOAddressSpace *vas) > > { > > VFIOContainer *container; > > @@ -2727,6 +2745,8 @@ static void vfio_disconnect(VFIOGroup *group) > > group->container =3D NULL; > > =20 > > if (QLIST_EMPTY(&container->group_list)) { > > + VFIOAddressSpace *vas =3D container->vas; > > + > > if (container->iommu_data.release) { > > container->iommu_data.release(container); > > } > > @@ -2734,6 +2754,11 @@ static void vfio_disconnect(VFIOGroup *group) > > DPRINTF("vfio_disconnect: close container->fd\n"); > > close(container->fd); > > g_free(container); > > + > > + if (QLIST_EMPTY(&vas->containers)) { > > + QLIST_REMOVE(vas, list); > > + g_free(vas); > > + } >=20 > vfio_put_address_space? Where there's a get... Fair enough, will revise. >=20 > > } > > } > > =20 > > @@ -2984,6 +3009,7 @@ static int vfio_initfn(PCIDevice *pdev) > > { > > VFIODevice *pvdev, *vdev =3D DO_UPCAST(VFIODevice, pdev, pdev); > > VFIOGroup *group; > > + VFIOAddressSpace *vas; > > char path[PATH_MAX], iommu_group_path[PATH_MAX], *group_name; > > ssize_t len; > > struct stat st; > > @@ -3019,12 +3045,9 @@ static int vfio_initfn(PCIDevice *pdev) > > DPRINTF("%s(%04x:%02x:%02x.%x) group %d\n", __func__, vdev->host.d= omain, > > vdev->host.bus, vdev->host.slot, vdev->host.function, grou= pid); > > =20 > > - if (pci_iommu_as(pdev) !=3D &address_space_memory) { > > - error_report("vfio: DMA address space must be system memory"); > > - return -ENXIO; > > - } > > + vas =3D vfio_address_space_get(pci_iommu_as(pdev)); >=20 > I don't think the structure malloc'd here will get cleaned up in all > cases on error. Thanks, Good point, auditing now.. --=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 --yZnyZsPjQYjG7xG7 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iEYEARECAAYFAlGRmkYACgkQaILKxv3ab8ZlUwCfXK+kBdWU+9cgt4u1c50D+dBT XW4AnRhcgYQjUMGO5ohzpJy1hVevngUL =hNJf -----END PGP SIGNATURE----- --yZnyZsPjQYjG7xG7--