[PATCH v1 0/3] Unmarking Mellanox devices with broken INTx masking

[PATCH v1 0/3] Unmarking Mellanox devices with broken INTx masking

[Noa Osherovich]

The following 3 patches unmark some Mellanox devices as having broken
INTx masking.

The first patch moves all broken INTx masking to FINAL instead of
HEADER so that the fixup can execute on all archs.
The second fixup changes the Mellanox marking to be according to a
list of devices rather than a general fixup by vendor.
The third patch excludes some Mellanox devices from the INTx broken
marking according to device ID / firmware version.

v0 -> v1
Fixes according to Gavin Shan's comments:
- Patch numbering was wrong
- Fixed logic of mellanox_check_broken_intx_masking

Noa Osherovich (3):
  PCI: Use FINAL fixup to mark broken INTx masking
  PCI: Convert Mellanox quirks to be for listed devices only
  PCI: Add checks to mellanox_check_broken_intx_masking

 drivers/pci/quirks.c | 175 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 139 insertions(+), 36 deletions(-)

-- 
1.8.3.1

[PATCH v1 1/3] PCI: Use FINAL fixup to mark broken INTx masking

[Noa Osherovich]

Convert all quirk_broken_intx_masking quirks from HEADER to FINAL.
None of those calls need to be in HEADER but some might need to be
called as FINAL. Moving them all to FINAL will save time during
pci_do_fixups.

Signed-off-by: Noa Osherovich <noaos@mellanox.com>
Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 drivers/pci/quirks.c | 72 ++++++++++++++++++++++++++--------------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index c232729f5b1b..85048fdf2474 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3146,53 +3146,53 @@ static void quirk_broken_intx_masking(struct pci_dev *dev)
 {
 	dev->broken_intx_masking = 1;
 }
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CHELSIO, 0x0030,
-			 quirk_broken_intx_masking);
-DECLARE_PCI_FIXUP_HEADER(0x1814, 0x0601, /* Ralink RT2800 802.11n PCI */
-			 quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x0030,
+			quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(0x1814, 0x0601, /* Ralink RT2800 802.11n PCI */
+			quirk_broken_intx_masking);
 /*
  * Realtek RTL8169 PCI Gigabit Ethernet Controller (rev 10)
  * Subsystem: Realtek RTL8169/8110 Family PCI Gigabit Ethernet NIC
  *
  * RTL8110SC - Fails under PCI device assignment using DisINTx masking.
  */
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_REALTEK, 0x8169,
-			 quirk_broken_intx_masking);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MELLANOX, PCI_ANY_ID,
-			 quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REALTEK, 0x8169,
+			quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MELLANOX, PCI_ANY_ID,
+			quirk_broken_intx_masking);
 
 /*
  * Intel i40e (XL710/X710) 10/20/40GbE NICs all have broken INTx masking,
  * DisINTx can be set but the interrupt status bit is non-functional.
  */
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1572,
-			 quirk_broken_intx_masking);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1574,
-			 quirk_broken_intx_masking);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1580,
-			 quirk_broken_intx_masking);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1581,
-			 quirk_broken_intx_masking);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1583,
-			 quirk_broken_intx_masking);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1584,
-			 quirk_broken_intx_masking);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1585,
-			 quirk_broken_intx_masking);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1586,
-			 quirk_broken_intx_masking);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1587,
-			 quirk_broken_intx_masking);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1588,
-			 quirk_broken_intx_masking);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1589,
-			 quirk_broken_intx_masking);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x37d0,
-			 quirk_broken_intx_masking);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x37d1,
-			 quirk_broken_intx_masking);
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x37d2,
-			 quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1572,
+			quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1574,
+			quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1580,
+			quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1581,
+			quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1583,
+			quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1584,
+			quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1585,
+			quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1586,
+			quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1587,
+			quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1588,
+			quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1589,
+			quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x37d0,
+			quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x37d1,
+			quirk_broken_intx_masking);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x37d2,
+			quirk_broken_intx_masking);
 
 static void quirk_no_bus_reset(struct pci_dev *dev)
 {
-- 
1.8.3.1

[PATCH v1 2/3] PCI: Convert Mellanox quirks to be for listed devices only

[Noa Osherovich]

Change Mellanox's broken_intx_masking quirk from an "all Mellanox
devices" to a quirk for listed devices only.

Signed-off-by: Noa Osherovich <noaos@mellanox.com>
Reviewed-by: Or Gerlitz <ogerlitz@mellanox.com>
Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com> 
---
 drivers/pci/quirks.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 85048fdf2474..228a4e8a8aeb 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3158,8 +3158,59 @@ static void quirk_broken_intx_masking(struct pci_dev *dev)
  */
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REALTEK, 0x8169,
 			quirk_broken_intx_masking);
+
+#define PCI_DEVICE_ID_MELLANOX_HERMON_SDR 0x6340
+#define PCI_DEVICE_ID_MELLANOX_HERMON_DDR 0x634a
+#define PCI_DEVICE_ID_MELLANOX_HERMON_QDR 0x6354
+#define PCI_DEVICE_ID_MELLANOX_HERMON_DDR_GEN2 0x6732
+#define PCI_DEVICE_ID_MELLANOX_HERMON_QDR_GEN2 0x673c
+#define PCI_DEVICE_ID_MELLANOX_HERMON_EN 0x6368
+#define PCI_DEVICE_ID_MELLANOX_HERMON_EN_GEN2 0x6750
+#define PCI_DEVICE_ID_MELLANOX_CONNECTX_EN 0x6372
+#define PCI_DEVICE_ID_MELLANOX_CONNECTX_EN_T_GEN2 0x675a
+#define PCI_DEVICE_ID_MELLANOX_CONNECTX_EN_GEN2 0x6764
+#define PCI_DEVICE_ID_MELLANOX_CONNECTX_EN_5_GEN2 0x6746
+#define PCI_DEVICE_ID_MELLANOX_CONNECTX2 0x676e
+#define PCI_DEVICE_ID_MELLANOX_CONNECTX3 0x1003
+#define PCI_DEVICE_ID_MELLANOX_CONNECTX3_PRO 0x1007
+#define PCI_DEVICE_ID_MELLANOX_CONNECTIB 0x1011
+#define PCI_DEVICE_ID_MELLANOX_CONNECTX4 0x1013
+#define PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX 0x1015
+
+static u16 mellanox_broken_intx_devs[] = {
+	PCI_DEVICE_ID_MELLANOX_HERMON_SDR,
+	PCI_DEVICE_ID_MELLANOX_HERMON_DDR,
+	PCI_DEVICE_ID_MELLANOX_HERMON_QDR,
+	PCI_DEVICE_ID_MELLANOX_HERMON_DDR_GEN2,
+	PCI_DEVICE_ID_MELLANOX_HERMON_QDR_GEN2,
+	PCI_DEVICE_ID_MELLANOX_HERMON_EN,
+	PCI_DEVICE_ID_MELLANOX_HERMON_EN_GEN2,
+	PCI_DEVICE_ID_MELLANOX_CONNECTX_EN,
+	PCI_DEVICE_ID_MELLANOX_CONNECTX_EN_T_GEN2,
+	PCI_DEVICE_ID_MELLANOX_CONNECTX_EN_GEN2,
+	PCI_DEVICE_ID_MELLANOX_CONNECTX_EN_5_GEN2,
+	PCI_DEVICE_ID_MELLANOX_CONNECTX2,
+	PCI_DEVICE_ID_MELLANOX_CONNECTX3,
+	PCI_DEVICE_ID_MELLANOX_CONNECTX3_PRO,
+	PCI_DEVICE_ID_MELLANOX_CONNECTIB,
+	PCI_DEVICE_ID_MELLANOX_CONNECTX4,
+	PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX
+};
+
+static void mellanox_check_broken_intx_masking(struct pci_dev *pdev)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(mellanox_broken_intx_devs); i++) {
+		if (pdev->device == mellanox_broken_intx_devs[i]) {
+			pdev->broken_intx_masking = 1;
+			return;
+		}
+	}
+}
+
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MELLANOX, PCI_ANY_ID,
-			quirk_broken_intx_masking);
+			mellanox_check_broken_intx_masking);
 
 /*
  * Intel i40e (XL710/X710) 10/20/40GbE NICs all have broken INTx masking,
-- 
1.8.3.1

[PATCH v1 3/3] PCI: Add checks to mellanox_check_broken_intx_masking

[Noa Osherovich]

Mellanox devices were marked as having INTx masking ability broken.
As a result, the VFIO driver fails to start when more than one device
function is passed-through to a VM if both have the same INTx pin.

Prior to Connect-IB, Mellanox devices exposed to the operating system
one PCI function per all ports.
Starting from Connect-IB, the devices are function-per-port. When
passing the second function to a VM, VFIO will fail to start.

Exclude ConnectX-4, ConnectX4-Lx and Connect-IB from the list of
Mellanox devices marked as having broken INTx masking:

- ConnectX-4 and ConnectX4-LX firmware version is checked. If INTx
  masking is supported, we unmark the broken INTx masking.
- Connect-IB does not support INTx currently so will not cause any
  problem.

Fixes: 11e42532ada31 ('PCI: Assume all Mellanox devices have ...')
Signed-off-by: Noa Osherovich <noaos@mellanox.com>
Reviewed-by: Or Gerlitz <ogerlitz@mellanox.com>
Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 drivers/pci/quirks.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 228a4e8a8aeb..112a5fcad2f5 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3192,13 +3192,25 @@ static void quirk_broken_intx_masking(struct pci_dev *dev)
 	PCI_DEVICE_ID_MELLANOX_CONNECTX2,
 	PCI_DEVICE_ID_MELLANOX_CONNECTX3,
 	PCI_DEVICE_ID_MELLANOX_CONNECTX3_PRO,
-	PCI_DEVICE_ID_MELLANOX_CONNECTIB,
-	PCI_DEVICE_ID_MELLANOX_CONNECTX4,
-	PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX
 };
 
+#define CONNECTX_4_CURR_MAX_MINOR 99
+#define CONNECTX_4_INTX_SUPPORT_MINOR 14
+
+/*
+ * Checking ConnectX-4/LX FW version to see if it supports legacy interrupts.
+ * If so, don't mark it as broken.
+ * FW minor > 99 means older FW version format and no INTx masking support.
+ * FW minor < 14 means new FW version format and no INTx masking support.
+ */
 static void mellanox_check_broken_intx_masking(struct pci_dev *pdev)
 {
+	__be32 __iomem *fw_ver;
+	u16 fw_major;
+	u16 fw_minor;
+	u16 fw_subminor;
+	u32 fw_maj_min;
+	u32 fw_sub_min;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(mellanox_broken_intx_devs); i++) {
@@ -3207,6 +3219,46 @@ static void mellanox_check_broken_intx_masking(struct pci_dev *pdev)
 			return;
 		}
 	}
+
+	/* Getting here means Connect-IB cards and up. Connect-IB has no INTx
+	 * support so shouldn't be checked further
+	 */
+	if (pdev->device == PCI_DEVICE_ID_MELLANOX_CONNECTIB)
+		return;
+
+	if (pdev->device != PCI_DEVICE_ID_MELLANOX_CONNECTX4 &&
+	    pdev->device != PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX)
+		return;
+
+	/* For ConnectX-4 and ConnectX-4LX, need to check FW support */
+	if (pci_enable_device_mem(pdev)) {
+		dev_warn(&pdev->dev, "Can't enable device memory\n");
+		return;
+	}
+
+	/* Convert from PCI bus to resource space. */
+	fw_ver = ioremap(pci_resource_start(pdev, 0), 4);
+	if (!fw_ver) {
+		dev_warn(&pdev->dev, "Can't map ConnectX-4 initialization segment\n");
+		return;
+	}
+
+	/* Reading from resource space should be 32b aligned */
+	fw_maj_min = ioread32be(fw_ver);
+	fw_sub_min = ioread32be(fw_ver + 1);
+	fw_major = fw_maj_min & 0xffff;
+	fw_minor = fw_maj_min >> 16;
+	fw_subminor = fw_sub_min & 0xffff;
+	if (fw_minor > CONNECTX_4_CURR_MAX_MINOR ||
+	    fw_minor < CONNECTX_4_INTX_SUPPORT_MINOR) {
+		dev_warn(&pdev->dev, "ConnectX-4: FW %u.%u.%u doesn't support INTx masking, disabling. Please upgrade FW to %d.14.1100 and up for INTx support\n",
+			 fw_major, fw_minor, fw_subminor, pdev->device ==
+			 PCI_DEVICE_ID_MELLANOX_CONNECTX4 ? 12 : 14);
+		pdev->broken_intx_masking = 1;
+		pci_disable_device(pdev);
+	}
+
+	iounmap(fw_ver);
 }
 
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MELLANOX, PCI_ANY_ID,
-- 
1.8.3.1

Re: [PATCH v1 3/3] PCI: Add checks to mellanox_check_broken_intx_masking

[Gavin Shan]

On Tue, Nov 15, 2016 at 10:00:00AM +0200, Noa Osherovich wrote:
>Mellanox devices were marked as having INTx masking ability broken.
>As a result, the VFIO driver fails to start when more than one device
>function is passed-through to a VM if both have the same INTx pin.
>
>Prior to Connect-IB, Mellanox devices exposed to the operating system
>one PCI function per all ports.
>Starting from Connect-IB, the devices are function-per-port. When
>passing the second function to a VM, VFIO will fail to start.
>
>Exclude ConnectX-4, ConnectX4-Lx and Connect-IB from the list of
>Mellanox devices marked as having broken INTx masking:
>
>- ConnectX-4 and ConnectX4-LX firmware version is checked. If INTx
>  masking is supported, we unmark the broken INTx masking.
>- Connect-IB does not support INTx currently so will not cause any
>  problem.
>
>Fixes: 11e42532ada31 ('PCI: Assume all Mellanox devices have ...')
>Signed-off-by: Noa Osherovich <noaos@mellanox.com>
>Reviewed-by: Or Gerlitz <ogerlitz@mellanox.com>
>Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>---
> drivers/pci/quirks.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 55 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>index 228a4e8a8aeb..112a5fcad2f5 100644
>--- a/drivers/pci/quirks.c
>+++ b/drivers/pci/quirks.c
>@@ -3192,13 +3192,25 @@ static void quirk_broken_intx_masking(struct pci_dev *dev)
> 	PCI_DEVICE_ID_MELLANOX_CONNECTX2,
> 	PCI_DEVICE_ID_MELLANOX_CONNECTX3,
> 	PCI_DEVICE_ID_MELLANOX_CONNECTX3_PRO,
>-	PCI_DEVICE_ID_MELLANOX_CONNECTIB,
>-	PCI_DEVICE_ID_MELLANOX_CONNECTX4,
>-	PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX
> };
>
>+#define CONNECTX_4_CURR_MAX_MINOR 99
>+#define CONNECTX_4_INTX_SUPPORT_MINOR 14
>+
>+/*
>+ * Checking ConnectX-4/LX FW version to see if it supports legacy interrupts.
>+ * If so, don't mark it as broken.
>+ * FW minor > 99 means older FW version format and no INTx masking support.
>+ * FW minor < 14 means new FW version format and no INTx masking support.
>+ */
> static void mellanox_check_broken_intx_masking(struct pci_dev *pdev)
> {
>+	__be32 __iomem *fw_ver;
>+	u16 fw_major;
>+	u16 fw_minor;
>+	u16 fw_subminor;
>+	u32 fw_maj_min;
>+	u32 fw_sub_min;
> 	int i;
>
> 	for (i = 0; i < ARRAY_SIZE(mellanox_broken_intx_devs); i++) {
>@@ -3207,6 +3219,46 @@ static void mellanox_check_broken_intx_masking(struct pci_dev *pdev)
> 			return;
> 		}
> 	}
>+
>+	/* Getting here means Connect-IB cards and up. Connect-IB has no INTx
>+	 * support so shouldn't be checked further
>+	 */
>+	if (pdev->device == PCI_DEVICE_ID_MELLANOX_CONNECTIB)
>+		return;
>+
>+	if (pdev->device != PCI_DEVICE_ID_MELLANOX_CONNECTX4 &&
>+	    pdev->device != PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX)
>+		return;
>+
>+	/* For ConnectX-4 and ConnectX-4LX, need to check FW support */
>+	if (pci_enable_device_mem(pdev)) {
>+		dev_warn(&pdev->dev, "Can't enable device memory\n");
>+		return;
>+	}
>+
>+	/* Convert from PCI bus to resource space. */
>+	fw_ver = ioremap(pci_resource_start(pdev, 0), 4);
>+	if (!fw_ver) {
>+		dev_warn(&pdev->dev, "Can't map ConnectX-4 initialization segment\n");

pci_disable_device(pdev) is missed.

>+		return;
>+	}
>+
>+	/* Reading from resource space should be 32b aligned */
>+	fw_maj_min = ioread32be(fw_ver);
>+	fw_sub_min = ioread32be(fw_ver + 1);
>+	fw_major = fw_maj_min & 0xffff;
>+	fw_minor = fw_maj_min >> 16;
>+	fw_subminor = fw_sub_min & 0xffff;
>+	if (fw_minor > CONNECTX_4_CURR_MAX_MINOR ||
>+	    fw_minor < CONNECTX_4_INTX_SUPPORT_MINOR) {
>+		dev_warn(&pdev->dev, "ConnectX-4: FW %u.%u.%u doesn't support INTx masking, disabling. Please upgrade FW to %d.14.1100 and up for INTx support\n",
>+			 fw_major, fw_minor, fw_subminor, pdev->device ==
>+			 PCI_DEVICE_ID_MELLANOX_CONNECTX4 ? 12 : 14);
>+		pdev->broken_intx_masking = 1;
>+		pci_disable_device(pdev);

This pci_disable_device() should be called unconditionally
before iounmap();

>+	}
>+
>+	iounmap(fw_ver);
> }
>
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MELLANOX, PCI_ANY_ID,

Thanks,
Gavin

Re: [PATCH v1 3/3] PCI: Add checks to mellanox_check_broken_intx_masking

[Bjorn Helgaas]

On Wed, Nov 16, 2016 at 09:42:30AM +1100, Gavin Shan wrote:
> On Tue, Nov 15, 2016 at 10:00:00AM +0200, Noa Osherovich wrote:
> >Mellanox devices were marked as having INTx masking ability broken.
> >As a result, the VFIO driver fails to start when more than one device
> >function is passed-through to a VM if both have the same INTx pin.
> >
> >Prior to Connect-IB, Mellanox devices exposed to the operating system
> >one PCI function per all ports.
> >Starting from Connect-IB, the devices are function-per-port. When
> >passing the second function to a VM, VFIO will fail to start.
> >
> >Exclude ConnectX-4, ConnectX4-Lx and Connect-IB from the list of
> >Mellanox devices marked as having broken INTx masking:
> >
> >- ConnectX-4 and ConnectX4-LX firmware version is checked. If INTx
> >  masking is supported, we unmark the broken INTx masking.
> >- Connect-IB does not support INTx currently so will not cause any
> >  problem.
> >
> >Fixes: 11e42532ada31 ('PCI: Assume all Mellanox devices have ...')
> >Signed-off-by: Noa Osherovich <noaos@mellanox.com>
> >Reviewed-by: Or Gerlitz <ogerlitz@mellanox.com>
> >Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> >---
> > drivers/pci/quirks.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 55 insertions(+), 3 deletions(-)
> >
> >diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> >index 228a4e8a8aeb..112a5fcad2f5 100644
> >--- a/drivers/pci/quirks.c
> >+++ b/drivers/pci/quirks.c
> >@@ -3192,13 +3192,25 @@ static void quirk_broken_intx_masking(struct pci_dev *dev)
> > 	PCI_DEVICE_ID_MELLANOX_CONNECTX2,
> > 	PCI_DEVICE_ID_MELLANOX_CONNECTX3,
> > 	PCI_DEVICE_ID_MELLANOX_CONNECTX3_PRO,
> >-	PCI_DEVICE_ID_MELLANOX_CONNECTIB,
> >-	PCI_DEVICE_ID_MELLANOX_CONNECTX4,
> >-	PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX
> > };
> >
> >+#define CONNECTX_4_CURR_MAX_MINOR 99
> >+#define CONNECTX_4_INTX_SUPPORT_MINOR 14
> >+
> >+/*
> >+ * Checking ConnectX-4/LX FW version to see if it supports legacy interrupts.
> >+ * If so, don't mark it as broken.
> >+ * FW minor > 99 means older FW version format and no INTx masking support.
> >+ * FW minor < 14 means new FW version format and no INTx masking support.
> >+ */
> > static void mellanox_check_broken_intx_masking(struct pci_dev *pdev)
> > {
> >+	__be32 __iomem *fw_ver;
> >+	u16 fw_major;
> >+	u16 fw_minor;
> >+	u16 fw_subminor;
> >+	u32 fw_maj_min;
> >+	u32 fw_sub_min;
> > 	int i;
> >
> > 	for (i = 0; i < ARRAY_SIZE(mellanox_broken_intx_devs); i++) {
> >@@ -3207,6 +3219,46 @@ static void mellanox_check_broken_intx_masking(struct pci_dev *pdev)
> > 			return;
> > 		}
> > 	}
> >+
> >+	/* Getting here means Connect-IB cards and up. Connect-IB has no INTx
> >+	 * support so shouldn't be checked further
> >+	 */
> >+	if (pdev->device == PCI_DEVICE_ID_MELLANOX_CONNECTIB)
> >+		return;
> >+
> >+	if (pdev->device != PCI_DEVICE_ID_MELLANOX_CONNECTX4 &&
> >+	    pdev->device != PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX)
> >+		return;
> >+
> >+	/* For ConnectX-4 and ConnectX-4LX, need to check FW support */
> >+	if (pci_enable_device_mem(pdev)) {
> >+		dev_warn(&pdev->dev, "Can't enable device memory\n");
> >+		return;
> >+	}
> >+
> >+	/* Convert from PCI bus to resource space. */

This comment is wrong (it occurs other places in this file, and it's
wrong there, too).  pci_resource_start() already gives you something
in resource space (a CPU physical address), and ioremap() maps that
into the virtual address space.  The PCI bus address is not involved
here.  I'd just drop the comment altogether.

> >+	fw_ver = ioremap(pci_resource_start(pdev, 0), 4);
> >+	if (!fw_ver) {
> >+		dev_warn(&pdev->dev, "Can't map ConnectX-4 initialization segment\n");
> 
> pci_disable_device(pdev) is missed.
> 
> >+		return;
> >+	}
> >+
> >+	/* Reading from resource space should be 32b aligned */
> >+	fw_maj_min = ioread32be(fw_ver);
> >+	fw_sub_min = ioread32be(fw_ver + 1);
> >+	fw_major = fw_maj_min & 0xffff;
> >+	fw_minor = fw_maj_min >> 16;
> >+	fw_subminor = fw_sub_min & 0xffff;
> >+	if (fw_minor > CONNECTX_4_CURR_MAX_MINOR ||
> >+	    fw_minor < CONNECTX_4_INTX_SUPPORT_MINOR) {
> >+		dev_warn(&pdev->dev, "ConnectX-4: FW %u.%u.%u doesn't support INTx masking, disabling. Please upgrade FW to %d.14.1100 and up for INTx support\n",
> >+			 fw_major, fw_minor, fw_subminor, pdev->device ==
> >+			 PCI_DEVICE_ID_MELLANOX_CONNECTX4 ? 12 : 14);
> >+		pdev->broken_intx_masking = 1;
> >+		pci_disable_device(pdev);
> 
> This pci_disable_device() should be called unconditionally
> before iounmap();

I agree pci_disable_device() should be called unconditionally, but I
think it should be called *after* the iounmap(), e.g.,

  pci_enable_device_mem();
  ioremap();
  iounmap();
  pci_disable_device();

> >+	}
> >+
> >+	iounmap(fw_ver);
> > }
> >
> > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MELLANOX, PCI_ANY_ID,
> 
> Thanks,
> Gavin
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 0/3] Unmarking Mellanox devices with broken INTx masking

[Bjorn Helgaas]

On Tue, Nov 15, 2016 at 09:59:57AM +0200, Noa Osherovich wrote:
> The following 3 patches unmark some Mellanox devices as having broken
> INTx masking.
> 
> The first patch moves all broken INTx masking to FINAL instead of
> HEADER so that the fixup can execute on all archs.
> The second fixup changes the Mellanox marking to be according to a
> list of devices rather than a general fixup by vendor.
> The third patch excludes some Mellanox devices from the INTx broken
> marking according to device ID / firmware version.
> 
> v0 -> v1
> Fixes according to Gavin Shan's comments:
> - Patch numbering was wrong
> - Fixed logic of mellanox_check_broken_intx_masking
> 
> Noa Osherovich (3):
>   PCI: Use FINAL fixup to mark broken INTx masking
>   PCI: Convert Mellanox quirks to be for listed devices only
>   PCI: Add checks to mellanox_check_broken_intx_masking
> 
>  drivers/pci/quirks.c | 175 ++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 139 insertions(+), 36 deletions(-)

I applied these (with the tweaks Gavin & I talked about) to
pci/virtualization for v4.10.  Please take a look and make sure it still
works for you.

I also removed the device ID #defines in the interest of brevity and moved
the Mellanox code out of the middle of the other INTx quirks.

Bjorn

Re: [PATCH v1 0/3] Unmarking Mellanox devices with broken INTx masking

[Noa Osherovich]

On 11/17/2016 12:41 AM, Bjorn Helgaas wrote:

> On Tue, Nov 15, 2016 at 09:59:57AM +0200, Noa Osherovich wrote:
>> The following 3 patches unmark some Mellanox devices as having broken
>> INTx masking.
>>
>> The first patch moves all broken INTx masking to FINAL instead of
>> HEADER so that the fixup can execute on all archs.
>> The second fixup changes the Mellanox marking to be according to a
>> list of devices rather than a general fixup by vendor.
>> The third patch excludes some Mellanox devices from the INTx broken
>> marking according to device ID / firmware version.
>>
>> v0 -> v1
>> Fixes according to Gavin Shan's comments:
>> - Patch numbering was wrong
>> - Fixed logic of mellanox_check_broken_intx_masking
>>
>> Noa Osherovich (3):
>>   PCI: Use FINAL fixup to mark broken INTx masking
>>   PCI: Convert Mellanox quirks to be for listed devices only
>>   PCI: Add checks to mellanox_check_broken_intx_masking
>>
>>  drivers/pci/quirks.c | 175 ++++++++++++++++++++++++++++++++++++++++-----------
>>  1 file changed, 139 insertions(+), 36 deletions(-)
> I applied these (with the tweaks Gavin & I talked about) to
> pci/virtualization for v4.10.  Please take a look and make sure it still
> works for you.
>
> I also removed the device ID #defines in the interest of brevity and moved
> the Mellanox code out of the middle of the other INTx quirks.
>
> Bjorn

Hi Bjorn,

This may have broken the kbuild robot - it sent an error saying those defines were undeclared.
How do you want to proceed?

Thanks,
Noa

Re: [PATCH v1 0/3] Unmarking Mellanox devices with broken INTx masking

[Bjorn Helgaas]

On Thu, Nov 17, 2016 at 08:52:36AM +0200, Noa Osherovich wrote:
> On 11/17/2016 12:41 AM, Bjorn Helgaas wrote:
> 
> > On Tue, Nov 15, 2016 at 09:59:57AM +0200, Noa Osherovich wrote:
> >> The following 3 patches unmark some Mellanox devices as having broken
> >> INTx masking.
> >>
> >> The first patch moves all broken INTx masking to FINAL instead of
> >> HEADER so that the fixup can execute on all archs.
> >> The second fixup changes the Mellanox marking to be according to a
> >> list of devices rather than a general fixup by vendor.
> >> The third patch excludes some Mellanox devices from the INTx broken
> >> marking according to device ID / firmware version.
> >>
> >> v0 -> v1
> >> Fixes according to Gavin Shan's comments:
> >> - Patch numbering was wrong
> >> - Fixed logic of mellanox_check_broken_intx_masking
> >>
> >> Noa Osherovich (3):
> >>   PCI: Use FINAL fixup to mark broken INTx masking
> >>   PCI: Convert Mellanox quirks to be for listed devices only
> >>   PCI: Add checks to mellanox_check_broken_intx_masking
> >>
> >>  drivers/pci/quirks.c | 175 ++++++++++++++++++++++++++++++++++++++++-----------
> >>  1 file changed, 139 insertions(+), 36 deletions(-)
> > I applied these (with the tweaks Gavin & I talked about) to
> > pci/virtualization for v4.10.  Please take a look and make sure it still
> > works for you.
> >
> > I also removed the device ID #defines in the interest of brevity and moved
> > the Mellanox code out of the middle of the other INTx quirks.
> >
> > Bjorn
> 
> Hi Bjorn,
> 
> This may have broken the kbuild robot - it sent an error saying those defines were undeclared.
> How do you want to proceed?

I broke it; I'll fix it :)

Bjorn