From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60000) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WQVW4-0003S5-Fu for qemu-devel@nongnu.org; Thu, 20 Mar 2014 01:27:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WQVW0-0001tW-Ci for qemu-devel@nongnu.org; Thu, 20 Mar 2014 01:27:44 -0400 Date: Thu, 20 Mar 2014 16:25:18 +1100 From: David Gibson Message-ID: <20140320052518.GC12982@voom.fritz.box> References: <1394603550-11556-1-git-send-email-aik@ozlabs.ru> <1394603550-11556-8-git-send-email-aik@ozlabs.ru> <1395259061.8201.242.camel@ul30vt.home> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="oTHb8nViIGeoXxdp" Content-Disposition: inline In-Reply-To: <1395259061.8201.242.camel@ul30vt.home> Subject: Re: [Qemu-devel] [PATCH v5 07/11] vfio: Add guest side IOMMU support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: Alexey Kardashevskiy , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Alexander Graf --oTHb8nViIGeoXxdp Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Mar 19, 2014 at 01:57:41PM -0600, Alex Williamson wrote: > On Wed, 2014-03-12 at 16:52 +1100, Alexey Kardashevskiy wrote: > > From: David Gibson [snip] > > + if (!memory_region_is_ram(mr)) { > > + DPRINTF("iommu map to non memory area %"HWADDR_PRIx"\n", > > + xlat); > > + return; > > + } > > + if (len & iotlb->addr_mask) { > > + DPRINTF("iommu has granularity incompatible with target AS\n"); >=20 > Is this possible? Assuming len is initially a power-of-2, would the > translate function change it? Maybe worth a comment to explain. translate can absolutely change the length. It will generally truncate it to the IOMMU page size, in fact. [snip] > > + DPRINTF("region_add [iommu] %"HWADDR_PRIx" - %"HWADDR_PRIx"\n", > > + iova, int128_get64(int128_sub(llend, int128_one()))); > > + /* > > + * FIXME: We should do some checking to see if the > > + * capabilities of the host VFIO IOMMU are adequate to model > > + * the guest IOMMU > > + * > > + * FIXME: This assumes that the guest IOMMU is empty of > > + * mappings at this point - we should either enforce this, or > > + * loop through existing mappings to map them into VFIO. > > + * > > + * FIXME: For VFIO iommu types which have KVM acceleration to > > + * avoid bouncing all map/unmaps through qemu this way, this > > + * would be the right place to wire that up (tell the KVM > > + * device emulation the VFIO iommu handles to use). > > + */ >=20 > That's a lot of FIXMEs... The second one in particular looks like it > needs to expand a bit on why this is likely a valid assumption. The > last one is more of a TODO than a FIXME. I think #2 isn't a valid assumption in general. It was true for the situation I was testing at the time, due to the order of pseries initialization, so I left it to get a proof of concept reasonably quickly. But I think that one's a FIXME that actually needs to be fixed. [snip] > > + /* > > + * FIXME: We assume the one big unmap below is adequate to > > + * remove any individual page mappings in the IOMMU which > > + * might have been copied into VFIO. That may not be true for > > + * all IOMMU types > > + */ >=20 > We assume this because the IOVA that gets unmapped is the same > regardless of whether a guest IOMMU is present? Uh.. no. This assumption works for a page table based IOMMU where a big unmap just flattens a large range of IO-PTEs. It might not work for some kind of extent or TLB based IOMMU, where operations are expected to exactly match the addresses of map operations. I don't know if IOMMUs that have trouble with this are a realistic prospect, but they're at least a theoretical possibility, hence the comment. >=20 > > + } > > + > > iova =3D TARGET_PAGE_ALIGN(section->offset_within_address_space); > > end =3D (section->offset_within_address_space + int128_get64(secti= on->size)) & > > TARGET_PAGE_MASK; >=20 >=20 >=20 --=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 --oTHb8nViIGeoXxdp Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJTKnu+AAoJEGw4ysog2bOSu3gQANL2zcdUA6vRJeha8jbOq13L EXDLEpbs2EDtJO5SqxtLt1T0q4zrQa4aSGmqEeAAG5xrWJT0I5X/5vGxUWRYTB8E 2I6OlC+ccq5HoWBn5Oz04IANuwk0PfaLtbX8t2eO5TCuv+chwoBpugYVUh3mFrIH b9qrlvHMTcZMSdidCuDQh9xDyy01AJuacbtyo6SayDS+3yDsebYBg1TP2zYDfTXo SBZOFsNjjqGHa4UDTSg89QYM0UDQZ6T4lgN92+F2BvvxMRCOQuK+jmjZDS51AGt8 ikctORGLv80XEyhFKyoEm1xJpl0H5V7MVaxg3MHRsfYWT3V/IofpPJYvO480kfxo zfGDcNm4fhkY+Jqa89wE34DepIbRwXypYIb8EjFnl9VXlwF94gX06dTIdOO6imYp ZnAMV4vK/rhskHlczg+wkp4CoSmskB/KU7CfDhf179iCF4Y+7ARciR2ull2gd2ck GJQ/R1fxanWuG62kv7psrg52qyl77VJuQnYHfZHI+fcRKuK3eXXmZvgj1qexoOfr D/Y3CK6y6Z4Su98eKVLEySXQ3rGIx5AU0+tOlLdrkyioYFTgo73XN1737L5uvsoM 2b/NOpopFBR0stmDqUgxLZTzxONNhE+372+Hn06vydCWJxPZJDLkteBH/xlBbLTC Dk5agRezcHiFmaz2yuNG =ejfH -----END PGP SIGNATURE----- --oTHb8nViIGeoXxdp--