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

Groups > linux.kernel > #1321119 > unrolled thread

Re: [RFC PATCH v3 3/5] PCI: Add host bridge attribute to indicate filtering of MSIs is supported

Started byAlex Williamson <alex.williamson@redhat.com>
First post2016-01-28 23:50 +0100
Last post2016-02-01 10:20 +0100
Articles 4 — 2 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 v3 3/5] PCI: Add host bridge attribute to indicate  filtering of MSIs is supported Alex Williamson <alex.williamson@redhat.com> - 2016-01-28 23:50 +0100
    Re: [RFC PATCH v3 3/5] PCI: Add host bridge attribute to indicate  filtering of MSIs is supported Yongji Xie <xyjxie@linux.vnet.ibm.com> - 2016-01-29 11:50 +0100
      Re: [RFC PATCH v3 3/5] PCI: Add host bridge attribute to indicate  filtering of MSIs is supported Alex Williamson <alex.williamson@redhat.com> - 2016-01-29 20:10 +0100
        Re: [RFC PATCH v3 3/5] PCI: Add host bridge attribute to indicate  filtering of MSIs is supported Yongji Xie <xyjxie@linux.vnet.ibm.com> - 2016-02-01 10:20 +0100

#1321119 — Re: [RFC PATCH v3 3/5] PCI: Add host bridge attribute to indicate filtering of MSIs is supported

FromAlex Williamson <alex.williamson@redhat.com>
Date2016-01-28 23:50 +0100
SubjectRe: [RFC PATCH v3 3/5] PCI: Add host bridge attribute to indicate filtering of MSIs is supported
Message-ID<qW3y1-1JH-7@gated-at.bofh.it>
On Fri, 2016-01-15 at 15:06 +0800, Yongji Xie wrote:
> MSI-X tables are not allowed to be mmapped in vfio-pci
> driver in case that user get to touch this directly.
> This will cause some performance issues when when PCI
> adapters have critical registers in the same page as
> the MSI-X table.
> 
> However, some kind of PCI host bridge such as IODA bridge
> on Power support filtering of MSIs, which can ensure that a
> given pci device can only shoot the MSIs assigned for it.
> So we think it's safe to expose the MSI-X table to userspace
> if filtering of MSIs is supported because the exposed MSI-X
> table can't be used to do harm to other memory space.
> 
> To support this case, this patch adds a pci_host_bridge
> attribute to indicate if this PCI host bridge supports
> filtering of MSIs.
> 
> Signed-off-by: Yongji Xie <xyjxie@linux.vnet.ibm.com>
> ---
>  drivers/pci/host-bridge.c |    6 ++++++
>  include/linux/pci.h       |    3 +++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
> index 5f4a2e0..c029267 100644
> --- a/drivers/pci/host-bridge.c
> +++ b/drivers/pci/host-bridge.c
> @@ -96,3 +96,9 @@ void pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
>  	res->end = region->end + offset;
>  }
>  EXPORT_SYMBOL(pcibios_bus_to_resource);
> +
> +bool pci_host_bridge_msi_filtered_enabled(struct pci_dev *pdev)
> +{
> +	return pci_find_host_bridge(pdev->bus)->msi_filtered;
> +}
> +EXPORT_SYMBOL_GPL(pci_host_bridge_msi_filtered_enabled);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index b640d65..b952b78 100644
> --- 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 msi_filtered:1;	/* support filtering of MSIs */
>  	/* Resource alignment requirements */
>  	resource_size_t (*align_resource)(struct pci_dev *dev,
>  			const struct resource *res,
> @@ -430,6 +431,8 @@ void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
>  
>  int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge);
>  
> +bool pci_host_bridge_msi_filtered_enabled(struct pci_dev *pdev);
> +
>  /*
>   * The first PCI_BRIDGE_RESOURCE_NUM PCI bus resources (those that correspond
>   * to P2P or CardBus bridge windows) go in a table.  Additional ones (for

Don't we already have a flag for this in the IOMMU space?

enum iommu_cap {
        IOMMU_CAP_CACHE_COHERENCY,      /* IOMMU can enforce cache coherent DMA
                                           transactions */
--->    IOMMU_CAP_INTR_REMAP,           /* IOMMU supports interrupt isolation */
        IOMMU_CAP_NOEXEC,               /* IOMMU_NOEXEC flag */
};

[toc] | [next] | [standalone]

#1321650

FromYongji Xie <xyjxie@linux.vnet.ibm.com>
Date2016-01-29 11:50 +0100
Message-ID<qWeMO-1un-13@gated-at.bofh.it>
In reply to#1321119
On 2016/1/29 6:46, Alex Williamson wrote:
> On Fri, 2016-01-15 at 15:06 +0800, Yongji Xie wrote:
>> MSI-X tables are not allowed to be mmapped in vfio-pci
>> driver in case that user get to touch this directly.
>> This will cause some performance issues when when PCI
>> adapters have critical registers in the same page as
>> the MSI-X table.
>>   
>> However, some kind of PCI host bridge such as IODA bridge
>> on Power support filtering of MSIs, which can ensure that a
>> given pci device can only shoot the MSIs assigned for it.
>> So we think it's safe to expose the MSI-X table to userspace
>> if filtering of MSIs is supported because the exposed MSI-X
>> table can't be used to do harm to other memory space.
>>   
>> To support this case, this patch adds a pci_host_bridge
>> attribute to indicate if this PCI host bridge supports
>> filtering of MSIs.
>>   
>> Signed-off-by: Yongji Xie <xyjxie@linux.vnet.ibm.com>
>> ---
>>   drivers/pci/host-bridge.c |    6 ++++++
>>   include/linux/pci.h       |    3 +++
>>   2 files changed, 9 insertions(+)
>>   
>> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
>> index 5f4a2e0..c029267 100644
>> --- a/drivers/pci/host-bridge.c
>> +++ b/drivers/pci/host-bridge.c
>> @@ -96,3 +96,9 @@ void pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
>>   	res->end = region->end + offset;
>>   }
>>   EXPORT_SYMBOL(pcibios_bus_to_resource);
>> +
>> +bool pci_host_bridge_msi_filtered_enabled(struct pci_dev *pdev)
>> +{
>> +	return pci_find_host_bridge(pdev->bus)->msi_filtered;
>> +}
>> +EXPORT_SYMBOL_GPL(pci_host_bridge_msi_filtered_enabled);
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index b640d65..b952b78 100644
>> --- 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 msi_filtered:1;	/* support filtering of MSIs */
>>   	/* Resource alignment requirements */
>>   	resource_size_t (*align_resource)(struct pci_dev *dev,
>>   			const struct resource *res,
>> @@ -430,6 +431,8 @@ void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
>>   
>>   int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge);
>>   
>> +bool pci_host_bridge_msi_filtered_enabled(struct pci_dev *pdev);
>> +
>>   /*
>>    * The first PCI_BRIDGE_RESOURCE_NUM PCI bus resources (those that correspond
>>    * to P2P or CardBus bridge windows) go in a table.  Additional ones (for
> Don't we already have a flag for this in the IOMMU space?
>
> enum iommu_cap {
>          IOMMU_CAP_CACHE_COHERENCY,      /* IOMMU can enforce cache coherent DMA
>                                             transactions */
> --->    IOMMU_CAP_INTR_REMAP,           /* IOMMU supports interrupt isolation */
>          IOMMU_CAP_NOEXEC,               /* IOMMU_NOEXEC flag */
> };
>

I saw this flag had been enabled in x86 and ARM arch.

I'm not sure whether we can mmap MSI-X table in those archs. I just 
verify it on PPC64 arch.

Regards.
Yongji Xie

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

#1322003

FromAlex Williamson <alex.williamson@redhat.com>
Date2016-01-29 20:10 +0100
Message-ID<qWmAG-7Lp-17@gated-at.bofh.it>
In reply to#1321650

----- Original Message -----
> On 2016/1/29 6:46, Alex Williamson wrote:
> > On Fri, 2016-01-15 at 15:06 +0800, Yongji Xie wrote:
> >> MSI-X tables are not allowed to be mmapped in vfio-pci
> >> driver in case that user get to touch this directly.
> >> This will cause some performance issues when when PCI
> >> adapters have critical registers in the same page as
> >> the MSI-X table.
> >>   
> >> However, some kind of PCI host bridge such as IODA bridge
> >> on Power support filtering of MSIs, which can ensure that a
> >> given pci device can only shoot the MSIs assigned for it.
> >> So we think it's safe to expose the MSI-X table to userspace
> >> if filtering of MSIs is supported because the exposed MSI-X
> >> table can't be used to do harm to other memory space.
> >>   
> >> To support this case, this patch adds a pci_host_bridge
> >> attribute to indicate if this PCI host bridge supports
> >> filtering of MSIs.
> >>   
> >> Signed-off-by: Yongji Xie <xyjxie@linux.vnet.ibm.com>
> >> ---
> >>   drivers/pci/host-bridge.c |    6 ++++++
> >>   include/linux/pci.h       |    3 +++
> >>   2 files changed, 9 insertions(+)
> >>   
> >> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
> >> index 5f4a2e0..c029267 100644
> >> --- a/drivers/pci/host-bridge.c
> >> +++ b/drivers/pci/host-bridge.c
> >> @@ -96,3 +96,9 @@ void pcibios_bus_to_resource(struct pci_bus *bus, struct
> >> resource *res,
> >>   	res->end = region->end + offset;
> >>   }
> >>   EXPORT_SYMBOL(pcibios_bus_to_resource);
> >> +
> >> +bool pci_host_bridge_msi_filtered_enabled(struct pci_dev *pdev)
> >> +{
> >> +	return pci_find_host_bridge(pdev->bus)->msi_filtered;
> >> +}
> >> +EXPORT_SYMBOL_GPL(pci_host_bridge_msi_filtered_enabled);
> >> diff --git a/include/linux/pci.h b/include/linux/pci.h
> >> index b640d65..b952b78 100644
> >> --- 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 msi_filtered:1;	/* support filtering of MSIs */
> >>   	/* Resource alignment requirements */
> >>   	resource_size_t (*align_resource)(struct pci_dev *dev,
> >>   			const struct resource *res,
> >> @@ -430,6 +431,8 @@ void pci_set_host_bridge_release(struct
> >> pci_host_bridge *bridge,
> >>   
> >>   int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge);
> >>   
> >> +bool pci_host_bridge_msi_filtered_enabled(struct pci_dev *pdev);
> >> +
> >>   /*
> >>    * The first PCI_BRIDGE_RESOURCE_NUM PCI bus resources (those that
> >>    correspond
> >>    * to P2P or CardBus bridge windows) go in a table.  Additional ones
> >>    (for
> > Don't we already have a flag for this in the IOMMU space?
> >
> > enum iommu_cap {
> >          IOMMU_CAP_CACHE_COHERENCY,      /* IOMMU can enforce cache
> >          coherent DMA
> >                                             transactions */
> > --->    IOMMU_CAP_INTR_REMAP,           /* IOMMU supports interrupt
> > isolation */
> >          IOMMU_CAP_NOEXEC,               /* IOMMU_NOEXEC flag */
> > };
> >
> 
> I saw this flag had been enabled in x86 and ARM arch.
> 
> I'm not sure whether we can mmap MSI-X table in those archs. I just
> verify it on PPC64 arch.

Unfortunately that's not a very good excuse for creating an alternate implementation.  When x86 implements interrupt remapping, we get fine grained isolation of MSI vectors and we've always taken this flag to mean that the system is isolated from devices that may perform DoS attacks with MSI writes.  I'm not entirely sure whether ARM really provides that degree of isolation, but they would be incorrect is exposing the capability if they do not.  Thanks,

Alex

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

#1322930

FromYongji Xie <xyjxie@linux.vnet.ibm.com>
Date2016-02-01 10:20 +0100
Message-ID<qXiOm-t4-19@gated-at.bofh.it>
In reply to#1322003
On 2016/1/30 3:05, Alex Williamson wrote:
>
> ----- Original Message -----
>> On 2016/1/29 6:46, Alex Williamson wrote:
>>> On Fri, 2016-01-15 at 15:06 +0800, Yongji Xie wrote:
>>>> MSI-X tables are not allowed to be mmapped in vfio-pci
>>>> driver in case that user get to touch this directly.
>>>> This will cause some performance issues when when PCI
>>>> adapters have critical registers in the same page as
>>>> the MSI-X table.
>>>>    
>>>> However, some kind of PCI host bridge such as IODA bridge
>>>> on Power support filtering of MSIs, which can ensure that a
>>>> given pci device can only shoot the MSIs assigned for it.
>>>> So we think it's safe to expose the MSI-X table to userspace
>>>> if filtering of MSIs is supported because the exposed MSI-X
>>>> table can't be used to do harm to other memory space.
>>>>    
>>>> To support this case, this patch adds a pci_host_bridge
>>>> attribute to indicate if this PCI host bridge supports
>>>> filtering of MSIs.
>>>>    
>>>> Signed-off-by: Yongji Xie <xyjxie@linux.vnet.ibm.com>
>>>> ---
>>>>    drivers/pci/host-bridge.c |    6 ++++++
>>>>    include/linux/pci.h       |    3 +++
>>>>    2 files changed, 9 insertions(+)
>>>>    
>>>> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
>>>> index 5f4a2e0..c029267 100644
>>>> --- a/drivers/pci/host-bridge.c
>>>> +++ b/drivers/pci/host-bridge.c
>>>> @@ -96,3 +96,9 @@ void pcibios_bus_to_resource(struct pci_bus *bus, struct
>>>> resource *res,
>>>>    	res->end = region->end + offset;
>>>>    }
>>>>    EXPORT_SYMBOL(pcibios_bus_to_resource);
>>>> +
>>>> +bool pci_host_bridge_msi_filtered_enabled(struct pci_dev *pdev)
>>>> +{
>>>> +	return pci_find_host_bridge(pdev->bus)->msi_filtered;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(pci_host_bridge_msi_filtered_enabled);
>>>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>>>> index b640d65..b952b78 100644
>>>> --- 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 msi_filtered:1;	/* support filtering of MSIs */
>>>>    	/* Resource alignment requirements */
>>>>    	resource_size_t (*align_resource)(struct pci_dev *dev,
>>>>    			const struct resource *res,
>>>> @@ -430,6 +431,8 @@ void pci_set_host_bridge_release(struct
>>>> pci_host_bridge *bridge,
>>>>    
>>>>    int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge);
>>>>    
>>>> +bool pci_host_bridge_msi_filtered_enabled(struct pci_dev *pdev);
>>>> +
>>>>    /*
>>>>     * The first PCI_BRIDGE_RESOURCE_NUM PCI bus resources (those that
>>>>     correspond
>>>>     * to P2P or CardBus bridge windows) go in a table.  Additional ones
>>>>     (for
>>> Don't we already have a flag for this in the IOMMU space?
>>>
>>> enum iommu_cap {
>>>           IOMMU_CAP_CACHE_COHERENCY,      /* IOMMU can enforce cache
>>>           coherent DMA
>>>                                              transactions */
>>> --->    IOMMU_CAP_INTR_REMAP,           /* IOMMU supports interrupt
>>> isolation */
>>>           IOMMU_CAP_NOEXEC,               /* IOMMU_NOEXEC flag */
>>> };
>>>
>> I saw this flag had been enabled in x86 and ARM arch.
>>
>> I'm not sure whether we can mmap MSI-X table in those archs. I just
>> verify it on PPC64 arch.
> Unfortunately that's not a very good excuse for creating an alternate implementation.  When x86 implements interrupt remapping, we get fine grained isolation of MSI vectors and we've always taken this flag to mean that the system is isolated from devices that may perform DoS attacks with MSI writes.  I'm not entirely sure whether ARM really provides that degree of isolation, but they would be incorrect is exposing the capability if they do not.  Thanks,
>
> Alex
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

OK, I will use this flag in next version. Thanks.

Regards,
Yongji Xie

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web