Re: [PATCH 1/5] PCI: Add a new PCI_BUS_FLAGS_MSI_REMAP flag

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Apr 27, 2016 at 08:43:26PM +0800, Yongji Xie wrote:
> We introduce a new pci_bus_flags, PCI_BUS_FLAGS_MSI_REMAP
> which indicates all devices on the bus are protected by the
> hardware which supports IRQ remapping(intel naming).

This changelog is ambiguous.  It's possible that there is hardware
that *supports* IRQ remapping, but does not actually *do* IRQ
remapping.  For example, an IRQ remapping capability may be present
but not enabled.

I think your intent is to set this flag only when MSI remapping is
actually *enabled* for all devices on the bus.

I'd also like to know exactly what protection is implied by
PCI_BUS_FLAGS_MSI_REMAP and IOMMU_CAP_INTR_REMAP.  I guess it means a
device can only generate MSIs to a certain set of CPUs?  I assume the
remapping hardware only checks the target address, not the data being
written?

> This flag will be used to know whether it's safe to expose
> MSI-X tables of PCI BARs to userspace. Because the capability
> of IRQ remapping can guarantee the PCI device cannot trigger
> MSIs that correspond to interrupt IDs of other devices.
> 
> There is a existing flag for this in the IOMMU space:
> 
> enum iommu_cap {
> 	IOMMU_CAP_CACHE_COHERENCY,
> --->	IOMMU_CAP_INTR_REMAP,
> 	IOMMU_CAP_NOEXEC,
> };
> 
> and Eric also posted a patchset [1] to abstract this
> capability on MSI controller side for ARM. But it would
> make sense to have a more common flag like
> PCI_BUS_FLAGS_MSI_REMAP in this patch so that we can use
> a universal flag to test this capability on PCI side for
> different archs.
> 
> [1] http://www.spinics.net/lists/kvm/msg130256.html
> 
> Signed-off-by: Yongji Xie <xyjxie@linux.vnet.ibm.com>
> ---
>  include/linux/pci.h |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 27df4a6..d619228 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -193,6 +193,7 @@ typedef unsigned short __bitwise pci_bus_flags_t;
>  enum pci_bus_flags {
>  	PCI_BUS_FLAGS_NO_MSI   = (__force pci_bus_flags_t) 1,
>  	PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2,
> +	PCI_BUS_FLAGS_MSI_REMAP = (__force pci_bus_flags_t) 4,
>  };
>  
>  /* These values come from the PCI Express Spec */
> -- 
> 1.7.9.5
>