Re: [Qemu-devel] [PATCH qemu] RFC: vfio-pci: Allow mmap of MSIX BAR

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

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

On Fri, Dec 15, 2017 at 09:04:28AM -0700, Alex Williamson wrote:
> On Fri, 15 Dec 2017 15:07:31 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Tue, Dec 12, 2017 at 04:21:31PM +1100, Alexey Kardashevskiy wrote:
> > > This makes use of a new VFIO_REGION_INFO_CAP_MSIX_MAPPABLE capability
> > > which tells that a region with MSIX data can be mapped entirely, i.e.
> > > the VFIO PCI driver won't prevent MSIX vectors area from being mapped.
> > > 
> > > This adds a "msix-no-mmap" property to the vfio-pci device, it is "true"
> > > by default and "false" for pseries-2.12+ machines.
> > > 
> > > This requites kernel's "vfio-pci: Allow mapping MSIX BAR"
> > > https://www.spinics.net/lists/kvm/msg160282.html
> > > 
> > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > > ---
> > > 
> > > This is an RFC as it requires kernel headers update which is not there yet.
> > > 
> > > I'd like to make it "msix-mmap" (without "no") but could not find a way
> > > of enabling a device property for machine versions newer than some value.
> > > 
> > > I changed 2.11 machine just for the demonstration purpose.  
> > 
> > As Alex says, the mmap()ability of the MSI-X BAR isn't really the
> > point.  The point is whether we need to intercept guest MMIOs to the
> > MSI-X region.  Still, the logic's basically right, just rename your
> > property to, say, "intercept_msix_mmio".  It would be true by default,
> > set to false by the pseries machine type.
> > 
> > I don't think you actually need to make it vary depending on the
> > version of the pseries machine type: whether the BAR is mmap()ed or
> > qemu emulated shouldn't be a guest visible change.  No PAPR guest
> > should have been directly poking the MSI-X region (ever), so we
> > shouldn't need to intercept the region even for old versions.
> 
> I have to ask, is the vfio-pci driver really the right point in the VM
> to be understanding whether the platform requires MSI-X MMIO
> emulation?  vfio-pci is only unique here in that enabling that
> emulation harms performance, but AIUI it's unused on any device and
> there may eventually be other devices affected in the same way as
> vfio-pci.  So should there be some post-realize platform code that
> disables MSI-X MemoryRegions or should the MSI-X code call out to some
> platform hook to determine whether to enable emulation?  Seems like a
> case where the impact might be unique to vfio, but the root of the
> problem is not.  Thanks,

That's a good point.  If we can reasonably do it at the level of a
generic PCI device, that would be preferable.

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