From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Gibson Subject: Re: [PATCH kernel] vfio-pci: Allow mapping MSIX BAR Date: Mon, 18 Dec 2017 13:46:33 +1100 Message-ID: <20171218024633.GN7753@umbus.fritz.box> References: <20171208041829.27520-1-aik@ozlabs.ru> <20171215041322.GF7753@umbus.fritz.box> <20171215085651.22f126e8@w520.home> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="U6leaJ20qZQc29iB" Cc: Alexey Kardashevskiy , kvm@vger.kernel.org, Eric Auger To: Alex Williamson Return-path: Received: from ozlabs.org ([103.22.144.67]:43611 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757621AbdLRD0F (ORCPT ); Sun, 17 Dec 2017 22:26:05 -0500 Content-Disposition: inline In-Reply-To: <20171215085651.22f126e8@w520.home> Sender: kvm-owner@vger.kernel.org List-ID: --U6leaJ20qZQc29iB Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Dec 15, 2017 at 08:56:51AM -0700, Alex Williamson wrote: > On Fri, 15 Dec 2017 15:13:22 +1100 > David Gibson wrote: >=20 > > On Fri, Dec 08, 2017 at 03:18:29PM +1100, Alexey Kardashevskiy wrote: > > > By default VFIO disables mapping of MSIX BAR to the userspace as > > > the userspace may program it in a way allowing spurious interrupts; > > > instead the userspace uses the VFIO_DEVICE_SET_IRQS ioctl. > > > In order to eliminate guessing from the userspace about what is > > > mmapable, VFIO also advertises a sparse list of regions allowed to mm= ap. > > >=20 > > > This works fine as long as the system page size equals to the MSIX > > > alignment requirement which is 4KB. However with a bigger page size > > > the existing code prohibits mapping non-MSIX parts of a page with MSIX > > > structures so these parts have to be emulated via slow reads/writes on > > > a VFIO device fd. If these emulated bits are accessed often, this has > > > serious impact on performance. > > >=20 > > > This allows mmap of the entire BAR containing MSIX vector table. > > >=20 > > > This removes the sparse capability for PCI devices as it becomes usel= ess. > > >=20 > > > As the userspace needs to know for sure whether mmapping of the MSIX > > > vector containing data can succeed, this adds a new capability - > > > VFIO_REGION_INFO_CAP_MSIX_MAPPABLE - which explicitly tells the users= pace > > > that the entire BAR can be mmapped. > > >=20 > > > This does not touch the MSIX mangling in the BAR read/write handlers = as > > > we are doing this just to enable direct access to non MSIX registers. > > >=20 > > > Signed-off-by: Alexey Kardashevskiy =20 > >=20 > > This seems reasonable, except for the interface issue that Alex > > pointed out already. > >=20 > > TBH, I'm not really sold on the need for a new kernel cap, rather than > > just having qemu attempt the mamp() and fall back if it fails. But > > I'm not really fussed either way. >=20 > Userspace could never handle an mmap failure as an error in this case, > they'd always just assume the old interface. If the user sees the > capability and the mmap semantics don't match, something is wrong. > Userspace is of course free to try to mmap and fall back to the old > assumptions regardless of this capability, but having this capability > provides more deterministic behavior. Thanks, Hm, yeah, I guess so. Like I say, not really fussed either way. --=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 --U6leaJ20qZQc29iB Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEdfRlhq5hpmzETofcbDjKyiDZs5IFAlo3LAkACgkQbDjKyiDZ s5J+Sw/7BLneMzMsgrPdFBsKc9XBMVmpzS/0R0YCZkraX+eBWbn+Q1oGD496ocom PK0zkBd/s4L276yn+hLnCrzKkZ4FSSkIk7IKsddgzf5TihDjhFdV++qozfPhLEWE qZDuhctiTzRN2OETsSdKvg728c9g3cBKPri/UTe5dqW/czjBAZeMfRUcGd68BSZo PdF3ZO6dDxIhPojjrM8eYiTXTMWdg8zVkd2uNi5cuXqVQKbdcVURuyIi0Xyisz+p qEGhsIwMzgEHQQQ7ZGSbfYM13IOBRY2jSkYo9hkVv0F7/Kdm5YljEPJ8/Hq1pB+Z m+tzX+smMFvFZx0wAYZcndKRp3pVZ18LCQtB+b2qRDMMK/RBZ0Ye8KREVHIgqfoo 9neRXuklRWG00I174+gi81y0FrIgdOB3GR4HEw6uPvTO8Dd+Berys6thAWxVqJBm TuXyMFQcSrhiPOrsh9aauot36eV35YVL+XT8HfmUFRZnFRX4kdayIqbZiNo2YSH5 hY+sLQHbAWe+orF8Mz53EppfoOFEu/4RpW9KGlY1hAWPH1D5yQHR1qtPU9pMSS4I yWx969/S/wlyiy6a7MV7iEe6xKKTkxCljnNtWYoU3DOESMm2jpP/PEOrBfqIV3YF goHGRBNs7Rq5yBV50dFZ0WYGjKUBsK08z4d6sqSazijamhZqN7s= =WK1s -----END PGP SIGNATURE----- --U6leaJ20qZQc29iB--