Re: [RFC PATCH v2 3/3] vfio-pci: Allow to mmap MSI-X table if EEH is supported

[Yongji Xie]

On 2016/1/5 5:42, Benjamin Herrenschmidt wrote:
> On Mon, 2016-01-04 at 14:07 -0700, Alex Williamson wrote:
> > On Thu, 2015-12-31 at 16:50 +0800, Yongji Xie wrote:
> > > Current vfio-pci implementation disallows to mmap MSI-X
> > > table in case that user get to touch this directly.
> > >
> > > However, EEH mechanism can ensure that a given pci device
> > > can only shoot the MSIs assigned for its PE. So we think
> > > it's safe to expose the MSI-X table to userspace because
> > > the exposed MSI-X table can't be used to do harm to other
> > > memory space.
> > >
> > > And with MSI-X table mmapped, some performance issues which
> > > are caused when PCI adapters have critical registers in the
> > > same page as the MSI-X table also can be resolved.
> > >
> > > So this patch adds a Kconfig option, VFIO_PCI_MMAP_MSIX,
> > > to support for mmapping MSI-X table.
> > >
> > > Signed-off-by: Yongji Xie <xyjxie@xxxxxxxxxxxxxxxxxx>
> > > ---
> > >   drivers/vfio/pci/Kconfig    |    4 ++++
> > >   drivers/vfio/pci/vfio_pci.c |    6 ++++--
> > >   2 files changed, 8 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
> > > index 02912f1..67b0a2c 100644
> > > --- a/drivers/vfio/pci/Kconfig
> > > +++ b/drivers/vfio/pci/Kconfig
> > > @@ -23,6 +23,10 @@ config VFIO_PCI_MMAP
> > >   	depends on VFIO_PCI
> > >   	def_bool y if !S390
> > >   
> > > +config VFIO_PCI_MMAP_MSIX
> > > +	depends on VFIO_PCI_MMAP
> > > +	def_bool y if EEH
> > Does CONFIG_EEH necessarily mean the EEH is enabled?  Could the
> > system
> > not support EEH or could EEH be disabled via kernel commandline
> > options?
> EEH is definitely the wrong thing to test here anyway. What needs to be
> tested is that the PCI Host bridge supports filtering of MSIs, so
> ideally this should be some kind of host bridge attribute set by the
> architecture backend.

So do you mean this attribute can be added in pci_host_bridge like this:

--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -412,6 +412,7 @@ struct pci_host_bridge {
 	void (*release_fn)(struct pci_host_bridge *);
 	void *release_data;
 	unsigned int ignore_reset_delay:1;	/* for entire hierarchy */
+	unsigned int msix_filtered:1;	/* support filtering of MSIs */
 	/* Resource alignment requirements */
 	resource_size_t (*align_resource)(struct pci_dev *dev,
 			const struct resource *res,

I can surely do it if there is no objection from PCI folks. Thanks.

Regards,
Yongji Xie

> This can happen with or without CONFIG_EEH and you are right,
> CONFIG_EEH can be enabled and the machine not support it.
>
> Any IODA bridge will support this.
>
> Cheers,
> Ben.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/