[RESEND PATCH v2 0/6] vfio-pci: Add support for mmapping MSI-X table

Current vfio-pci implementation disallows to mmap the page
containing MSI-X table in case that users can write directly
to MSI-X table and generate an incorrect MSIs.

However, this will cause some performance issue when there
are some critical device registers in the same page as the 
MSI-X table. We have to handle the mmio access to these
registers in QEMU emulation rather than in guest.

To solve this issue, this series allows to expose MSI-X table
to userspace when hardware enables the capability of interrupt
remapping which can ensure that a given PCI device can only
shoot the MSIs assigned for it. And we introduce a new bus_flags
PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
for different archs.

The patch 3 are based on the proposed patchset[1].

Changelog v2: 
- Make the commit log more clear
- Replace pci_bus_check_msi_remapping() with pci_bus_msi_isolated()
  so that we could clearly know what the function does
- Set PCI_BUS_FLAGS_MSI_REMAP in pci_create_root_bus() instead
  of iommu_bus_notifier()
- Reserve VFIO_REGION_INFO_FLAG_CAPS when we allow to mmap MSI-X
  table so that we can know whether we allow to mmap MSI-X table
  in QEMU

[1] https://www.mail-archive.com/linux-kernel%40vger.kernel.org/msg1138820.html

Yongji Xie (6):
  PCI: Add a new PCI_BUS_FLAGS_MSI_REMAP flag
  PCI: Set PCI_BUS_FLAGS_MSI_REMAP if MSI controller enables IRQ remapping
  PCI: Set PCI_BUS_FLAGS_MSI_REMAP if IOMMU have capability of IRQ remapping
  iommu: Set PCI_BUS_FLAGS_MSI_REMAP on iommu driver initialization
  pci-ioda: Set PCI_BUS_FLAGS_MSI_REMAP for IODA host bridge
  vfio-pci: Allow to expose MSI-X table to userspace if interrupt remapping is enabled

 arch/powerpc/platforms/powernv/pci-ioda.c |    8 ++++++++
 drivers/iommu/iommu.c                     |    8 ++++++++
 drivers/pci/msi.c                         |   15 +++++++++++++++
 drivers/pci/probe.c                       |    7 +++++++
 drivers/vfio/pci/vfio_pci.c               |   17 ++++++++++++++---
 drivers/vfio/pci/vfio_pci_rdwr.c          |    3 ++-
 include/linux/msi.h                       |    5 ++++-
 include/linux/pci.h                       |    1 +
 8 files changed, 59 insertions(+), 5 deletions(-)

-- 
1.7.9.5

The capability of IRQ remapping is abstracted on IOMMU side on
some archs. There is a existing flag IOMMU_CAP_INTR_REMAP for this.

To have a universal flag to test this capability for different
archs on PCI side, we set PCI_BUS_FLAGS_MSI_REMAP for PCI buses
when IOMMU_CAP_INTR_REMAP is set.

Signed-off-by: Yongji Xie <redacted>
---
 drivers/pci/probe.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 2b9e3ba..15a33e2 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c

@@ -16,6 +16,7 @@
 #include <linux/aer.h>
 #include <linux/acpi.h>
 #include <linux/irqdomain.h>
+#include <linux/iommu.h>
 #include "pci.h"
 
 #define CARDBUS_LATENCY_TIMER	176	/* secondary latency timer */

@@ -2160,6 +2161,9 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 	pci_set_bus_of_node(b);
 	pci_set_bus_msi_domain(b);
 
+	if (iommu_capable(&pci_bus_type, IOMMU_CAP_INTR_REMAP))
+		b->bus_flags |= PCI_BUS_FLAGS_MSI_REMAP;
+
 	if (!parent)
 		set_dev_node(b->bridge, pcibus_to_node(b));
 

-- 
1.7.9.5

On ARM HW the capability of IRQ remapping is abstracted on
MSI controller side. MSI_FLAG_IRQ_REMAPPING is used to advertise
this [1].

To have a universal flag to test this capability for different
archs on PCI side, we set PCI_BUS_FLAGS_MSI_REMAP for PCI buses
when MSI_FLAG_IRQ_REMAPPING is set.

[1] https://www.mail-archive.com/linux-kernel%40vger.kernel.org/msg1138820.html

Signed-off-by: Yongji Xie <redacted>
---
 drivers/pci/msi.c   |   15 +++++++++++++++
 drivers/pci/probe.c |    3 +++
 include/linux/msi.h |    5 ++++-
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index a080f44..dbdb5c3 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c

@@ -1134,6 +1134,21 @@ void *msi_desc_to_pci_sysdata(struct msi_desc *desc)
 }
 EXPORT_SYMBOL_GPL(msi_desc_to_pci_sysdata);
 
+int pci_bus_msi_isolated(struct pci_bus *bus, struct irq_domain *domain)
+{
+#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
+	struct msi_domain_info *info;
+
+	if (!domain)
+		return 0;
+
+	info = msi_get_domain_info(domain);
+	if (info->flags & MSI_FLAG_IRQ_REMAPPING)
+		return 1;
+#endif
+	return 0;
+}
+
 #ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
 /**
  * pci_msi_domain_write_msg - Helper to write MSI message to PCI config space

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8004f67..2b9e3ba 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c

@@ -713,6 +713,9 @@ static void pci_set_bus_msi_domain(struct pci_bus *bus)
 	if (!d)
 		d = pci_host_bridge_msi_domain(b);
 
+	if (b == bus && pci_bus_msi_isolated(bus, d))
+		bus->bus_flags |= PCI_BUS_FLAGS_MSI_REMAP;
+
 	dev_set_msi_domain(&bus->dev, d);
 }
 

diff --git a/include/linux/msi.h b/include/linux/msi.h
index 08441b1..04cc749 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h

@@ -15,6 +15,8 @@ extern int pci_msi_ignore_mask;
 struct irq_data;
 struct msi_desc;
 struct pci_dev;
+struct pci_bus;
+struct irq_domain;
 struct platform_msi_priv_data;
 void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
 void get_cached_msi_msg(unsigned int irq, struct msi_msg *msg);

@@ -164,6 +166,8 @@ void arch_restore_msi_irqs(struct pci_dev *dev);
 void default_teardown_msi_irqs(struct pci_dev *dev);
 void default_restore_msi_irqs(struct pci_dev *dev);
 
+int pci_bus_msi_isolated(struct pci_bus *bus, struct irq_domain *domain);
+
 struct msi_controller {
 	struct module *owner;
 	struct device *dev;

@@ -182,7 +186,6 @@ struct msi_controller {
 #include <linux/irqhandler.h>
 #include <asm/msi.h>
 
-struct irq_domain;
 struct irq_domain_ops;
 struct irq_chip;
 struct device_node;

-- 
1.7.9.5

Any IODA host bridge have the capability of IRQ remapping.
So we set PCI_BUS_FLAGS_MSI_REMAP when this kind of host birdge
is detected.

Signed-off-by: Yongji Xie <redacted>
Reviewed-by: Alexey Kardashevskiy <redacted>
---
 arch/powerpc/platforms/powernv/pci-ioda.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index c5baaf3..9011268 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c

@@ -3094,6 +3094,12 @@ static void pnv_pci_ioda_fixup(void)
 	pnv_npu_ioda_fixup();
 }
 
+int pnv_pci_ioda_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	bridge->bus->bus_flags |= PCI_BUS_FLAGS_MSI_REMAP;
+	return 0;
+}
+
 /*
  * Returns the alignment for I/O or memory windows for P2P
  * bridges. That actually depends on how PEs are segmented.

@@ -3395,6 +3401,8 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,
 	 */
 	ppc_md.pcibios_fixup = pnv_pci_ioda_fixup;
 
+	ppc_md.pcibios_root_bridge_prepare = pnv_pci_ioda_root_bridge_prepare;
+
 	if (phb->type == PNV_PHB_NPU)
 		hose->controller_ops = pnv_npu_ioda_controller_ops;
 	else

-- 
1.7.9.5

Some iommu drivers would be initialized after PCI device
enumeration. So PCI_BUS_FLAGS_MSI_REMAP would not be set
when probing PCI devices although IOMMU enables capability
of IRQ remapping. This patch tests this capability and
set the flag when iommu driver is initialized.

Signed-off-by: Yongji Xie <redacted>
---
 drivers/iommu/iommu.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b9df141..e6159ab 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c

@@ -872,6 +872,14 @@ static int add_iommu_group(struct device *dev, void *data)
 	const struct iommu_ops *ops = cb->ops;
 	int ret;
 
+	/*
+	 * Set PCI_BUS_FLAGS_MSI_REMAP for all PCI buses when IOMMU
+	 * have capability of IRQ remapping.
+	 */
+	if (dev_is_pci(dev) && ops->capable &&
+			ops->capable(IOMMU_CAP_INTR_REMAP))
+		to_pci_dev(dev)->bus->bus_flags |= PCI_BUS_FLAGS_MSI_REMAP;
+
 	if (!ops->add_device)
 		return 0;
 

-- 
1.7.9.5

This patch tries to expose MSI-X tables to userspace if hardware
enables interrupt remapping. This capability can ensure that a
given PCI device can only shoot the MSIs assigned for it. That
means a userspace driver could never hurt other devices or system
by writing to the exposed MSI-X table directly.

Signed-off-by: Yongji Xie <redacted>
---
 drivers/vfio/pci/vfio_pci.c      |   17 ++++++++++++++---
 drivers/vfio/pci/vfio_pci_rdwr.c |    3 ++-
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 188b1ff..6bae388 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c

@@ -487,8 +487,12 @@ static int msix_sparse_mmap_cap(struct vfio_pci_device *vdev,
 
 	end = pci_resource_len(vdev->pdev, vdev->msix_bar);
 
-	/* If MSI-X table is aligned to the start or end, only one area */
-	if (((vdev->msix_offset & PAGE_MASK) == 0) ||
+	/*
+	 * If MSI-X table is allowed to mmap because of the capability
+	 * of IRQ remapping or aligned to the start or end, only one area
+	 */
+	if ((vdev->pdev->bus->bus_flags & PCI_BUS_FLAGS_MSI_REMAP) ||
+	    ((vdev->msix_offset & PAGE_MASK) == 0) ||
 	    (PAGE_ALIGN(vdev->msix_offset + vdev->msix_size) >= end))
 		nr_areas = 1;
 

@@ -503,6 +507,12 @@ static int msix_sparse_mmap_cap(struct vfio_pci_device *vdev,
 			      struct vfio_region_info_cap_sparse_mmap, header);
 	sparse->nr_areas = nr_areas;
 
+	if (vdev->pdev->bus->bus_flags & PCI_BUS_FLAGS_MSI_REMAP) {
+		sparse->areas[i].offset = 0;
+		sparse->areas[i].size = end;
+		return 0;
+	}
+
 	if (vdev->msix_offset & PAGE_MASK) {
 		sparse->areas[i].offset = 0;
 		sparse->areas[i].size = vdev->msix_offset & PAGE_MASK;

@@ -1061,7 +1071,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 (index == vdev->msix_bar &&
+		!(pdev->bus->bus_flags & PCI_BUS_FLAGS_MSI_REMAP)) {
 		/*
 		 * Disallow mmaps overlapping the MSI-X table; users don't
 		 * get to touch this directly.  We could find somewhere

diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
index 5ffd1d9..dbf9cd0 100644
--- a/drivers/vfio/pci/vfio_pci_rdwr.c
+++ b/drivers/vfio/pci/vfio_pci_rdwr.c

@@ -164,7 +164,8 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_device *vdev, char __user *buf,
 	} else
 		io = vdev->barmap[bar];
 
-	if (bar == vdev->msix_bar) {
+	if (bar == vdev->msix_bar &&
+		!(pdev->bus->bus_flags & PCI_BUS_FLAGS_MSI_REMAP)) {
 		x_start = vdev->msix_offset;
 		x_end = vdev->msix_offset + vdev->msix_size;
 	}

-- 
1.7.9.5

We introduce a new pci_bus_flags, PCI_BUS_FLAGS_MSI_REMAP
which indicates interrupts of all devices on the bus are
managed by the hardware enabling IRQ remapping(intel naming).
When the capability is enabled, a given PCI device can only
shoot the MSIs assigned for it. In other words, the hardware
can protect system from invalid MSIs of the device by checking
the target address and data when there is something wrong
with MSI part in device or device driver.

There is a existing flag for this capability 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 it on MSI
controller side for ARM. But it would make sense to have a
more common flag like PCI_BUS_FLAGS_MSI_REMAP so that we can
use a universal flag to test this capability on PCI side for
different archs.

With this flag enabled, we can easily know whether it's safe
to expose MSI-X tables of PCI BARs to userspace. Some usespace
drivers such as VFIO may benefit from this.

[1] https://www.mail-archive.com/linux-kernel%40vger.kernel.org/msg1138820.html

Signed-off-by: Yongji Xie <redacted>
---
 include/linux/pci.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 932ec74..4ec37da 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h

@@ -187,6 +187,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

AFAICT, you posted this *3* days ago, has something changed here or is
this just expedited nagging for review?  This also depends on a
non-upstream series and crosses multiple functional areas, all of which
make it difficult for maintainers to actually do anything with this
series.  Thanks,

Alex

On Thu,  2 Jun 2016 14:09:57 +0800
Yongji Xie [off-list ref] wrote:

Current vfio-pci implementation disallows to mmap the page
containing MSI-X table in case that users can write directly
to MSI-X table and generate an incorrect MSIs.

However, this will cause some performance issue when there
are some critical device registers in the same page as the 
MSI-X table. We have to handle the mmio access to these
registers in QEMU emulation rather than in guest.

To solve this issue, this series allows to expose MSI-X table
to userspace when hardware enables the capability of interrupt
remapping which can ensure that a given PCI device can only
shoot the MSIs assigned for it. And we introduce a new bus_flags
PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
for different archs.

The patch 3 are based on the proposed patchset[1].

Changelog v2: 
- Make the commit log more clear
- Replace pci_bus_check_msi_remapping() with pci_bus_msi_isolated()
  so that we could clearly know what the function does
- Set PCI_BUS_FLAGS_MSI_REMAP in pci_create_root_bus() instead
  of iommu_bus_notifier()
- Reserve VFIO_REGION_INFO_FLAG_CAPS when we allow to mmap MSI-X
  table so that we can know whether we allow to mmap MSI-X table
  in QEMU

[1] https://www.mail-archive.com/linux-kernel%40vger.kernel.org/msg1138820.html

Yongji Xie (6):
  PCI: Add a new PCI_BUS_FLAGS_MSI_REMAP flag
  PCI: Set PCI_BUS_FLAGS_MSI_REMAP if MSI controller enables IRQ remapping
  PCI: Set PCI_BUS_FLAGS_MSI_REMAP if IOMMU have capability of IRQ remapping
  iommu: Set PCI_BUS_FLAGS_MSI_REMAP on iommu driver initialization
  pci-ioda: Set PCI_BUS_FLAGS_MSI_REMAP for IODA host bridge
  vfio-pci: Allow to expose MSI-X table to userspace if interrupt remapping is enabled

 arch/powerpc/platforms/powernv/pci-ioda.c |    8 ++++++++
 drivers/iommu/iommu.c                     |    8 ++++++++
 drivers/pci/msi.c                         |   15 +++++++++++++++
 drivers/pci/probe.c                       |    7 +++++++
 drivers/vfio/pci/vfio_pci.c               |   17 ++++++++++++++---
 drivers/vfio/pci/vfio_pci_rdwr.c          |    3 ++-
 include/linux/msi.h                       |    5 ++++-
 include/linux/pci.h                       |    1 +
 8 files changed, 59 insertions(+), 5 deletions(-)

Hi, Alex

On 2016/6/2 21:11, Alex Williamson wrote:

AFAICT, you posted this *3* days ago, has something changed here or is
this just expedited nagging for review?  This also depends on a
non-upstream series and crosses multiple functional areas, all of which
make it difficult for maintainers to actually do anything with this
series.  Thanks,

Alex

I resend this series because I found the message-id of the previous email
is changed unexpectedly. The cover letter is not in the same thread as other
patches.  It would be a little hard for people to review.

Thanks,
Yongji

On Thu,  2 Jun 2016 14:09:57 +0800
Yongji Xie [off-list ref] wrote:

quoted

Current vfio-pci implementation disallows to mmap the page
containing MSI-X table in case that users can write directly
to MSI-X table and generate an incorrect MSIs.

However, this will cause some performance issue when there
are some critical device registers in the same page as the
MSI-X table. We have to handle the mmio access to these
registers in QEMU emulation rather than in guest.

To solve this issue, this series allows to expose MSI-X table
to userspace when hardware enables the capability of interrupt
remapping which can ensure that a given PCI device can only
shoot the MSIs assigned for it. And we introduce a new bus_flags
PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
for different archs.

The patch 3 are based on the proposed patchset[1].

Changelog v2:
- Make the commit log more clear
- Replace pci_bus_check_msi_remapping() with pci_bus_msi_isolated()
   so that we could clearly know what the function does
- Set PCI_BUS_FLAGS_MSI_REMAP in pci_create_root_bus() instead
   of iommu_bus_notifier()
- Reserve VFIO_REGION_INFO_FLAG_CAPS when we allow to mmap MSI-X
   table so that we can know whether we allow to mmap MSI-X table
   in QEMU

[1] https://www.mail-archive.com/linux-kernel%40vger.kernel.org/msg1138820.html

Yongji Xie (6):
   PCI: Add a new PCI_BUS_FLAGS_MSI_REMAP flag
   PCI: Set PCI_BUS_FLAGS_MSI_REMAP if MSI controller enables IRQ remapping
   PCI: Set PCI_BUS_FLAGS_MSI_REMAP if IOMMU have capability of IRQ remapping
   iommu: Set PCI_BUS_FLAGS_MSI_REMAP on iommu driver initialization
   pci-ioda: Set PCI_BUS_FLAGS_MSI_REMAP for IODA host bridge
   vfio-pci: Allow to expose MSI-X table to userspace if interrupt remapping is enabled

  arch/powerpc/platforms/powernv/pci-ioda.c |    8 ++++++++
  drivers/iommu/iommu.c                     |    8 ++++++++
  drivers/pci/msi.c                         |   15 +++++++++++++++
  drivers/pci/probe.c                       |    7 +++++++
  drivers/vfio/pci/vfio_pci.c               |   17 ++++++++++++++---
  drivers/vfio/pci/vfio_pci_rdwr.c          |    3 ++-
  include/linux/msi.h                       |    5 ++++-
  include/linux/pci.h                       |    1 +
  8 files changed, 59 insertions(+), 5 deletions(-)

Hi,

[auto build test ERROR on vfio/next]
[also build test ERROR on v4.7-rc1 next-20160603]
[cannot apply to pci/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Yongji-Xie/PCI-Add-a-new-PCI_BUS_FLAGS_MSI_REMAP-flag/20160530-215348
base:   https://github.com/awilliam/linux-vfio.git next
config: arm-sa1100 (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `pci_set_bus_msi_domain':

quoted

arm_pmu.c:(.text+0x5c58): undefined reference to `pci_bus_msi_isolated'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Hi,

[auto build test ERROR on vfio/next]
[also build test ERROR on v4.7-rc1 next-20160603]
[cannot apply to pci/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Yongji-Xie/PCI-Add-a-new-PCI_BUS_FLAGS_MSI_REMAP-flag/20160530-215348
base:   https://github.com/awilliam/linux-vfio.git next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/pci/msi.c: In function 'pci_bus_msi_isolated':

quoted

drivers/pci/msi.c:1146:20: error: 'MSI_FLAG_IRQ_REMAPPING' undeclared (first use in this function)

     if (info->flags & MSI_FLAG_IRQ_REMAPPING)
                       ^
   drivers/pci/msi.c:1146:20: note: each undeclared identifier is reported only once for each function it appears in

vim +/MSI_FLAG_IRQ_REMAPPING +1146 drivers/pci/msi.c

  1140		struct msi_domain_info *info;
  1141	
  1142		if (!domain)
  1143			return 0;
  1144	
  1145		info = msi_get_domain_info(domain);

1146		if (info->flags & MSI_FLAG_IRQ_REMAPPING)

  1147			return 1;
  1148	#endif
  1149		return 0;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

On 2016/6/6 4:10, kbuild test robot wrote:

Hi,

[auto build test ERROR on vfio/next]
[also build test ERROR on v4.7-rc1 next-20160603]
[cannot apply to pci/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Yongji-Xie/PCI-Add-a-new-PCI_BUS_FLAGS_MSI_REMAP-flag/20160530-215348
base:   https://github.com/awilliam/linux-vfio.git next
config: arm-sa1100 (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
         chmod +x ~/bin/make.cross
         # save the attached .config to linux build tree
         make.cross ARCH=arm

All errors (new ones prefixed by >>):

    drivers/built-in.o: In function `pci_set_bus_msi_domain':

quoted

quoted

arm_pmu.c:(.text+0x5c58): undefined reference to `pci_bus_msi_isolated'

Sorry for this fault. I will fix it in next version.  The 
pci_bus_msi_isolated() should not
depend on CONFIG_PCI_MSI, I would move this function from 
drivers/pci/msi.c to
drivers/pci/probe.c.

Thanks,
Yongji

On 2016/6/6 5:11, kbuild test robot wrote:

Hi,

[auto build test ERROR on vfio/next]
[also build test ERROR on v4.7-rc1 next-20160603]
[cannot apply to pci/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Yongji-Xie/PCI-Add-a-new-PCI_BUS_FLAGS_MSI_REMAP-flag/20160530-215348
base:   https://github.com/awilliam/linux-vfio.git next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
         chmod +x ~/bin/make.cross
         # save the attached .config to linux build tree
         make.cross ARCH=arm

All errors (new ones prefixed by >>):

    drivers/pci/msi.c: In function 'pci_bus_msi_isolated':

quoted

quoted

drivers/pci/msi.c:1146:20: error: 'MSI_FLAG_IRQ_REMAPPING' undeclared (first use in this function)

      if (info->flags & MSI_FLAG_IRQ_REMAPPING)
                        ^
    drivers/pci/msi.c:1146:20: note: each undeclared identifier is reported only once for each function it appears in

vim +/MSI_FLAG_IRQ_REMAPPING +1146 drivers/pci/msi.c

   1140		struct msi_domain_info *info;
   1141	
   1142		if (!domain)
   1143			return 0;
   1144	
   1145		info = msi_get_domain_info(domain);

quoted

1146		if (info->flags & MSI_FLAG_IRQ_REMAPPING)

   1147			return 1;
   1148	#endif
   1149		return 0;

This fault is expected because this patch is based on patchset [1] which
introduce flag MSI_FLAG_IRQ_REMAPPING.

[1] 
https://www.mail-archive.com/linux-kernel%40vger.kernel.org/msg1138820.html

Thanks,
Yongji

Hi Yongji,

Le 02/06/2016 à 08:09, Yongji Xie a écrit :

Current vfio-pci implementation disallows to mmap the page
containing MSI-X table in case that users can write directly
to MSI-X table and generate an incorrect MSIs.

However, this will cause some performance issue when there
are some critical device registers in the same page as the 
MSI-X table. We have to handle the mmio access to these
registers in QEMU emulation rather than in guest.

To solve this issue, this series allows to expose MSI-X table
to userspace when hardware enables the capability of interrupt
remapping which can ensure that a given PCI device can only
shoot the MSIs assigned for it. And we introduce a new bus_flags
PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
for different archs.

The patch 3 are based on the proposed patchset[1].

You may have noticed I sent a respin of [1] yesterday:
http://www.gossamer-threads.com/lists/linux/kernel/2455187.

Unfortunately you will see I removed the patch defining the new
msi_domain_info MSI_FLAG_IRQ_REMAPPING flag you rely on in this series.
I did so because I was not using it anymore. At the beginning this was
used to detect whether the MSI assignment was safe but this
method was covering cases where the MSI controller was
upstream to the IOMMU. So now I rely on a mechanism where MSI controller
are supposed to register their MSI doorbells and tag whether it is safe.

I don't know yet how this change will be welcomed though. Depending
on reviews/discussions, might happen we revert to the previous flag.

If you need the feature you can embed the used patches in your series and
follow the review process separately. Sorry for the setback.

Best Regards

Eric

Changelog v2: 
- Make the commit log more clear
- Replace pci_bus_check_msi_remapping() with pci_bus_msi_isolated()
  so that we could clearly know what the function does
- Set PCI_BUS_FLAGS_MSI_REMAP in pci_create_root_bus() instead
  of iommu_bus_notifier()
- Reserve VFIO_REGION_INFO_FLAG_CAPS when we allow to mmap MSI-X
  table so that we can know whether we allow to mmap MSI-X table
  in QEMU

[1] https://www.mail-archive.com/linux-kernel%40vger.kernel.org/msg1138820.html

Yongji Xie (6):
  PCI: Add a new PCI_BUS_FLAGS_MSI_REMAP flag
  PCI: Set PCI_BUS_FLAGS_MSI_REMAP if MSI controller enables IRQ remapping
  PCI: Set PCI_BUS_FLAGS_MSI_REMAP if IOMMU have capability of IRQ remapping
  iommu: Set PCI_BUS_FLAGS_MSI_REMAP on iommu driver initialization
  pci-ioda: Set PCI_BUS_FLAGS_MSI_REMAP for IODA host bridge
  vfio-pci: Allow to expose MSI-X table to userspace if interrupt remapping is enabled

 arch/powerpc/platforms/powernv/pci-ioda.c |    8 ++++++++
 drivers/iommu/iommu.c                     |    8 ++++++++
 drivers/pci/msi.c                         |   15 +++++++++++++++
 drivers/pci/probe.c                       |    7 +++++++
 drivers/vfio/pci/vfio_pci.c               |   17 ++++++++++++++---
 drivers/vfio/pci/vfio_pci_rdwr.c          |    3 ++-
 include/linux/msi.h                       |    5 ++++-
 include/linux/pci.h                       |    1 +
 8 files changed, 59 insertions(+), 5 deletions(-)

Hi, Eric

On 2016/6/8 15:41, Auger Eric wrote:

Hi Yongji,

Le 02/06/2016 à 08:09, Yongji Xie a écrit :

quoted

Current vfio-pci implementation disallows to mmap the page
containing MSI-X table in case that users can write directly
to MSI-X table and generate an incorrect MSIs.

However, this will cause some performance issue when there
are some critical device registers in the same page as the
MSI-X table. We have to handle the mmio access to these
registers in QEMU emulation rather than in guest.

To solve this issue, this series allows to expose MSI-X table
to userspace when hardware enables the capability of interrupt
remapping which can ensure that a given PCI device can only
shoot the MSIs assigned for it. And we introduce a new bus_flags
PCI_BUS_FLAGS_MSI_REMAP to test this capability on PCI side
for different archs.

The patch 3 are based on the proposed patchset[1].

You may have noticed I sent a respin of [1] yesterday:
http://www.gossamer-threads.com/lists/linux/kernel/2455187.

Unfortunately you will see I removed the patch defining the new
msi_domain_info MSI_FLAG_IRQ_REMAPPING flag you rely on in this series.
I did so because I was not using it anymore. At the beginning this was
used to detect whether the MSI assignment was safe but this
method was covering cases where the MSI controller was
upstream to the IOMMU. So now I rely on a mechanism where MSI controller
are supposed to register their MSI doorbells and tag whether it is safe.

I don't know yet how this change will be welcomed though. Depending
on reviews/discussions, might happen we revert to the previous flag.

If you need the feature you can embed the used patches in your series and
follow the review process separately. Sorry for the setback.

Thanks for your notification. I'd better wait until your patches get
settled. Then I could exactly know which way we should use to test the
capability of interrupt remapping on ARM in my series.

Thanks,
Yongji