Re: [PATCH kernel] vfio-pci: Allow mapping MSIX BAR

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2681 bytes --]

On Fri, Dec 15, 2017 at 08:56:51AM -0700, Alex Williamson wrote:
> On Fri, 15 Dec 2017 15:13:22 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > 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 mmap.
> > > 
> > > 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.
> > > 
> > > This allows mmap of the entire BAR containing MSIX vector table.
> > > 
> > > This removes the sparse capability for PCI devices as it becomes useless.
> > > 
> > > 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 userspace
> > > that the entire BAR can be mmapped.
> > > 
> > > 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.
> > > 
> > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>  
> > 
> > This seems reasonable, except for the interface issue that Alex
> > pointed out already.
> > 
> > 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.
> 
> 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.

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]