Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1301093 > unrolled thread

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

Started byAlex Williamson <alex.williamson@redhat.com>
First post2016-01-04 22:10 +0100
Last post2016-01-06 11:00 +0100
Articles 3 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [RFC PATCH v2 3/3] vfio-pci: Allow to mmap MSI-X table if EEH  is supported Alex Williamson <alex.williamson@redhat.com> - 2016-01-04 22:10 +0100
    Re: [RFC PATCH v2 3/3] vfio-pci: Allow to mmap MSI-X table if EEH  is supported Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2016-01-04 23:30 +0100
      Re: [RFC PATCH v2 3/3] vfio-pci: Allow to mmap MSI-X table if EEH is  supported Yongji Xie <xyjxie@linux.vnet.ibm.com> - 2016-01-06 11:00 +0100

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

FromAlex Williamson <alex.williamson@redhat.com>
Date2016-01-04 22:10 +0100
SubjectRe: [RFC PATCH v2 3/3] vfio-pci: Allow to mmap MSI-X table if EEH is supported
Message-ID<qNky6-oU-13@gated-at.bofh.it>
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@linux.vnet.ibm.com>
> ---
>  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?

> +
>  config VFIO_PCI_INTX
>  	depends on VFIO_PCI
>  	def_bool y if !S390
> diff --git a/drivers/vfio/pci/vfio_pci.c
> b/drivers/vfio/pci/vfio_pci.c
> index 09b3805..d536985 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -555,7 +555,8 @@ static long vfio_pci_ioctl(void *device_data,
>  			    IORESOURCE_MEM && (info.size >=
> PAGE_SIZE ||
>  			    pci_resource_page_aligned)) {
>  				info.flags |=
> VFIO_REGION_INFO_FLAG_MMAP;
> -				if (info.index == vdev->msix_bar) {
> +				if
> (!IS_ENABLED(CONFIG_VFIO_PCI_MMAP_MSIX) &&
> +				    info.index == vdev->msix_bar) {
>  					ret =
> msix_sparse_mmap_cap(vdev, &caps);
>  					if (ret)
>  						return ret;
> @@ -967,7 +968,8 @@ static int vfio_pci_mmap(void *device_data,
> struct vm_area_struct *vma)
>  	if (phys_len < PAGE_SIZE || req_start + req_len > phys_len)
>  		return -EINVAL;
>  
> -	if (index == vdev->msix_bar) {
> +	if (!IS_ENABLED(CONFIG_VFIO_PCI_MMAP_MSIX) &&
> +	    index == vdev->msix_bar) {
>  		/*
>  		 * Disallow mmaps overlapping the MSI-X table; users
> don't
>  		 * get to touch this directly.  We could find
> somewhere

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

[toc] | [next] | [standalone]


#1301160

FromBenjamin Herrenschmidt <benh@kernel.crashing.org>
Date2016-01-04 23:30 +0100
Message-ID<qNlNx-189-29@gated-at.bofh.it>
In reply to#1301093
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@linux.vnet.ibm.com>
> > ---
> >  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.

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.

> > +
> >  config VFIO_PCI_INTX
> >  	depends on VFIO_PCI
> >  	def_bool y if !S390
> > diff --git a/drivers/vfio/pci/vfio_pci.c
> > b/drivers/vfio/pci/vfio_pci.c
> > index 09b3805..d536985 100644
> > --- a/drivers/vfio/pci/vfio_pci.c
> > +++ b/drivers/vfio/pci/vfio_pci.c
> > @@ -555,7 +555,8 @@ static long vfio_pci_ioctl(void *device_data,
> >  			    IORESOURCE_MEM && (info.size >=
> > PAGE_SIZE ||
> >  			    pci_resource_page_aligned)) {
> >  				info.flags |=
> > VFIO_REGION_INFO_FLAG_MMAP;
> > -				if (info.index == vdev->msix_bar)
> > {
> > +				if
> > (!IS_ENABLED(CONFIG_VFIO_PCI_MMAP_MSIX) &&
> > +				    info.index == vdev->msix_bar)
> > {
> >  					ret =
> > msix_sparse_mmap_cap(vdev, &caps);
> >  					if (ret)
> >  						return ret;
> > @@ -967,7 +968,8 @@ static int vfio_pci_mmap(void *device_data,
> > struct vm_area_struct *vma)
> >  	if (phys_len < PAGE_SIZE || req_start + req_len >
> > phys_len)
> >  		return -EINVAL;
> >  
> > -	if (index == vdev->msix_bar) {
> > +	if (!IS_ENABLED(CONFIG_VFIO_PCI_MMAP_MSIX) &&
> > +	    index == vdev->msix_bar) {
> >  		/*
> >  		 * Disallow mmaps overlapping the MSI-X table;
> > users
> > don't
> >  		 * get to touch this directly.  We could find
> > somewhere
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


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

FromYongji Xie <xyjxie@linux.vnet.ibm.com>
Date2016-01-06 11:00 +0100
SubjectRe: [RFC PATCH v2 3/3] vfio-pci: Allow to mmap MSI-X table if EEH is supported
Message-ID<qNT2P-7Oz-23@gated-at.bofh.it>
In reply to#1301160
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@linux.vnet.ibm.com>
>>> ---
>>>   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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web