From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37339) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abR2M-0002nU-Pe for qemu-devel@nongnu.org; Thu, 03 Mar 2016 06:03:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1abR2J-0001st-RR for qemu-devel@nongnu.org; Thu, 03 Mar 2016 06:03:18 -0500 Date: Thu, 3 Mar 2016 17:08:37 +1100 From: David Gibson Message-ID: <20160303060837.GK1620@voom.redhat.com> References: <1456823441-46757-1-git-send-email-aik@ozlabs.ru> <1456823441-46757-11-git-send-email-aik@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="5dNcufZ4prhark0F" Content-Disposition: inline In-Reply-To: <1456823441-46757-11-git-send-email-aik@ozlabs.ru> Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu v13 10/16] vfio: Use different page size for different IOMMU types List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: Alex Williamson , qemu-ppc@nongnu.org, qemu-devel@nongnu.org --5dNcufZ4prhark0F Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Mar 01, 2016 at 08:10:35PM +1100, Alexey Kardashevskiy wrote: > The existing memory listener is called on RAM or PCI address space > which implies potentially different page size. >=20 > This uses new memory_region_iommu_get_page_sizes() for IOMMU regions > or falls back to qemu_real_host_page_size if RAM. >=20 > Signed-off-by: Alexey Kardashevskiy This doesn't seem right to me.. but neither does the original code. As far as I can tell, these checks against page sizes are trying to make sure that the regions are aligned in such a way that we can actually map them in the host IOMMU. But TARGET_PAGE_SIZE is a property of the guest, rather than the host. So, changing TARGET_PAGE_SIZE to qemu_real_host_page_size seems correct to me for RAM regions. But memory_region_iommu_get_page_sizes() is *not* the right choice for IOMMU regions, because that gives you the granularity of the guest IOMMU, whereas you need the granularity of the host IOMMU. Unless I'm mistaking the purpose of these checks, which I hope Alex can clarify us on when he gets back from holiday next week. > --- > Changes: > * uses the smallest page size for mask as IOMMU MR can support multple > page sizes > --- > hw/vfio/common.c | 28 ++++++++++++++++++++-------- > 1 file changed, 20 insertions(+), 8 deletions(-) >=20 > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index 0e67a5a..3aaa6b5 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -318,6 +318,16 @@ static hwaddr vfio_container_granularity(VFIOContain= er *container) > return (hwaddr)1 << ctz64(container->iova_pgsizes); > } > =20 > +static hwaddr vfio_iommu_page_mask(MemoryRegion *mr) > +{ > + if (memory_region_is_iommu(mr)) { > + int smallest =3D ffs(memory_region_iommu_get_page_sizes(mr)) - 1; > + > + return ~((1ULL << smallest) - 1); > + } > + return qemu_real_host_page_mask; > +} > + > static void vfio_listener_region_add(VFIOMemoryListener *vlistener, > MemoryRegionSection *section) > { > @@ -326,6 +336,7 @@ static void vfio_listener_region_add(VFIOMemoryListen= er *vlistener, > Int128 llend; > void *vaddr; > int ret; > + hwaddr page_mask =3D vfio_iommu_page_mask(section->mr); > =20 > if (vfio_listener_skipped_section(section)) { > trace_vfio_listener_region_add_skip( > @@ -335,16 +346,16 @@ static void vfio_listener_region_add(VFIOMemoryList= ener *vlistener, > return; > } > =20 > - if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MA= SK) !=3D > - (section->offset_within_region & ~TARGET_PAGE_MASK))) { > + if (unlikely((section->offset_within_address_space & ~page_mask) !=3D > + (section->offset_within_region & ~page_mask))) { > error_report("%s received unaligned region", __func__); > return; > } > =20 > - iova =3D TARGET_PAGE_ALIGN(section->offset_within_address_space); > + iova =3D ROUND_UP(section->offset_within_address_space, ~page_mask += 1); > llend =3D int128_make64(section->offset_within_address_space); > llend =3D int128_add(llend, section->size); > - llend =3D int128_and(llend, int128_exts64(TARGET_PAGE_MASK)); > + llend =3D int128_and(llend, int128_exts64(page_mask)); > =20 > if (int128_ge(int128_make64(iova), llend)) { > return; > @@ -432,6 +443,7 @@ static void vfio_listener_region_del(VFIOMemoryListen= er *vlistener, > hwaddr iova, end; > int ret; > MemoryRegion *iommu =3D NULL; > + hwaddr page_mask =3D vfio_iommu_page_mask(section->mr); > =20 > if (vfio_listener_skipped_section(section)) { > trace_vfio_listener_region_del_skip( > @@ -441,8 +453,8 @@ static void vfio_listener_region_del(VFIOMemoryListen= er *vlistener, > return; > } > =20 > - if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MA= SK) !=3D > - (section->offset_within_region & ~TARGET_PAGE_MASK))) { > + if (unlikely((section->offset_within_address_space & ~page_mask) !=3D > + (section->offset_within_region & ~page_mask))) { > error_report("%s received unaligned region", __func__); > return; > } > @@ -469,9 +481,9 @@ static void vfio_listener_region_del(VFIOMemoryListen= er *vlistener, > */ > } > =20 > - iova =3D TARGET_PAGE_ALIGN(section->offset_within_address_space); > + iova =3D ROUND_UP(section->offset_within_address_space, ~page_mask += 1); > end =3D (section->offset_within_address_space + int128_get64(section= ->size)) & > - TARGET_PAGE_MASK; > + page_mask; > =20 > if (iova >=3D end) { > return; --=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 --5dNcufZ4prhark0F Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJW19TlAAoJEGw4ysog2bOS5pcQALD6bDs+w9sgH0Nbc+6X/F2P TuN+csS66XQq1Ohu/NXPY3irSPoqaaG4cKUKF3OejC6qaDNwgDU7I8HWNHYQ/aKO rgavF8GqzbSH69+udXu8MNtuecUGiDDFZ/1QuHQLrzbWVqJJSH+BgVGL9UcA15j5 zOYfIJ5+a2HGLi9RwBkNKmIA3zNxdu1DIcDnHVHu2RM7mi07JtLUhlGXHuVTWJ/x kP2Zig2MouugtkFXI9u8F5qEhbtZnlOYhGGLK4WYcOlwZKntlF5HDjhxBtgxoKXN OwEFU39pHjOKRUvbaaFQJDTX4bzb0FBPezaGgN5m6xJCoxTQVOOkJ11UF6jOI/XL 1Wh77bkAO88rctv60saln9XixUWbREoqGkNLrHKhKVWybcg8vQCHqNnt3bpalCUj KxwIF6N25WwawlBTM6aejABlR0Dap2d6v6yVxaRM/r0Fs9y+gfLCRb9XyMmIDn3W FCVxXl2X3Yua/JktmEfKBwCX6dDPf9ZKHBaJr2fE2/dyUNf9/XapOyetoz8AaXgS 0NO8Ee9Ur3WDJ5xRyqYzsvCPew1h1aZ1Xl/BD/97/9PLWvF2+kBdpW2PqGQdxQ+3 RG9gNyrLbyDV2hIZO75cfhd8maOwPND7ECoCCxrGaSH4isj0tE6RI6RsPD+e5sN7 4IRTDcuC/aV45JDE6jdZ =AFwn -----END PGP SIGNATURE----- --5dNcufZ4prhark0F--