ixgbe: add IntMode module parameter

From: Nick Nunley <[email protected]>

The 82598 has an erratum where some system designs can increase the likelihood of a system hang due to a PCIe resource contention between the 82598 and another device in legacy INTx mode.

This issue can happen in any number of configurations with the 82598 and another device in legacy interrupt mode. While we know of a specific case where this is happening, this can happen with other devices as well if configured in a certain way depending on the system's slot configuration among other things. This is alos why we need to have the flexibility to disable MSI-X interrupts for the 82598 when these configurations are found having this deadlock situation.

The errata being worked - around with this patch - has to do with any type of device that has a pending legacy INTx to the IOH's internal IOAPIC which has priority from the shared resources (round-robin to ensure forward progress / fairness). The internal IOAPIC is a PCI-E endpoint hence transactions to it are viewed as peer-to-peer from a shared resource perspective. To make forward progress on the legacy INTx, the outbound completion FIFOs for the IOU (resource pool1) are checked for available space due to shared resources. The IOH is allowed to have in/out dependency as a root is full and not making forward progress because the 82598 has entered its "erratum window" where the 82598 is throttling the release of posted credits back to the IOH as it has pending MSI-X interrupts to the IOH. The 82598 will only release the posted credits back to IOH after the inbound posted writes are serviced and the IOH releases the posted credits back to the 82598 (i.e., the 82598 in/out dependency erratum). Since the priority is on the other device's legacy INTx, the 82598 inbound posted transaction cannot make forward progress that results in a link/forward progress deadlock condition.

The 82598 Specification Update has the errata list and can be found here, it's erratum 38:
http://www.intel.com/design/network/specupdt/321040.pdf

This patch adds the IntMode module parameter to ixgbe to allow user selection of interrupt mode (MSI-X, MSI, legacy) on driver load. To work around the errata described above, ixgbe needs to be able to disable MSI-X for affected configurations.

---

 drivers/net/ixgbe/Makefile      |    2 -
 drivers/net/ixgbe/ixgbe.h       |    1 
 drivers/net/ixgbe/ixgbe_main.c  |   26 ++++---
 drivers/net/ixgbe/ixgbe_param.c |  153 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 172 insertions(+), 10 deletions(-)
 create mode 100644 drivers/net/ixgbe/ixgbe_param.c

index 8f81efb..820491f 100644
@@ -34,7 +34,7 @@ obj-$(CONFIG_IXGBE) += ixgbe.o
 
 ixgbe-objs := ixgbe_main.o ixgbe_common.o ixgbe_ethtool.o \
               ixgbe_82599.o ixgbe_82598.o ixgbe_phy.o ixgbe_sriov.o \
-              ixgbe_mbx.o
+              ixgbe_mbx.o ixgbe_param.o
 
 ixgbe-$(CONFIG_IXGBE_DCB) +=  ixgbe_dcb.o ixgbe_dcb_82598.o \
                               ixgbe_dcb_82599.o ixgbe_dcb_nl.o

index 79c35ae..106f197 100644
@@ -436,6 +436,7 @@ extern int ixgbe_copy_dcb_cfg(struct ixgbe_dcb_config *src_dcb_cfg,
 extern char ixgbe_driver_name[];
 extern const char ixgbe_driver_version[];
 
+extern void ixgbe_check_options(struct ixgbe_adapter *adapter);
 extern int ixgbe_up(struct ixgbe_adapter *adapter);
 extern void ixgbe_down(struct ixgbe_adapter *adapter);
 extern void ixgbe_reinit_locked(struct ixgbe_adapter *adapter);

index a98ff0e..d04e095 100644
@@ -3949,6 +3949,9 @@ static int ixgbe_set_interrupt_capability(struct ixgbe_adapter *adapter)
 	int err = 0;
 	int vector, v_budget;
 
+	if (!(adapter->flags & IXGBE_FLAG_MSIX_CAPABLE))
+		goto try_msi;
+
 	/*
 	 * It's easy to be greedy for MSI-X vectors, but it really
 	 * doesn't do us much good if we have a lot more vectors

@@ -3981,16 +3984,10 @@ static int ixgbe_set_interrupt_capability(struct ixgbe_adapter *adapter)
 			goto out;
 	}
 
-	adapter->flags &= ~IXGBE_FLAG_DCB_ENABLED;
-	adapter->flags &= ~IXGBE_FLAG_RSS_ENABLED;
-	adapter->flags &= ~IXGBE_FLAG_FDIR_HASH_CAPABLE;
-	adapter->flags &= ~IXGBE_FLAG_FDIR_PERFECT_CAPABLE;
-	adapter->atr_sample_rate = 0;
-	if (adapter->flags & IXGBE_FLAG_SRIOV_ENABLED)
-		ixgbe_disable_sriov(adapter);
-
-	ixgbe_set_num_queues(adapter);
 
+try_msi:
+	if (!(adapter->flags & IXGBE_FLAG_MSI_CAPABLE))
+		goto legacy;
 	err = pci_enable_msi(adapter->pdev);
 	if (!err) {
 		adapter->flags |= IXGBE_FLAG_MSI_ENABLED;

@@ -4000,7 +3997,16 @@ static int ixgbe_set_interrupt_capability(struct ixgbe_adapter *adapter)
 		/* reset err */
 		err = 0;
 	}
+legacy:
+	adapter->flags &= ~IXGBE_FLAG_DCB_ENABLED;
+	adapter->flags &= ~IXGBE_FLAG_RSS_ENABLED;
+	adapter->flags &= ~IXGBE_FLAG_FDIR_HASH_CAPABLE;
+	adapter->flags &= ~IXGBE_FLAG_FDIR_PERFECT_CAPABLE;
+	adapter->atr_sample_rate = 0;
+	if (adapter->flags & IXGBE_FLAG_SRIOV_ENABLED)
+		ixgbe_disable_sriov(adapter);
 
+	ixgbe_set_num_queues(adapter);
 out:
 	return err;
 }

@@ -6199,6 +6205,8 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
 		goto err_sw_init;
 	}
 
+	ixgbe_check_options(adapter);
+
 	ixgbe_probe_vf(adapter, ii);
 
 	netdev->features = NETIF_F_SG |

From: Jeff Kirsher <[email protected]>
Date: Sun, 18 Apr 2010 22:03:59 -0700

This is not how we handle situations like this.

Here is one acceptable way to handle this:

Turn MSI/MSI-X off for every system that uses this PCI-E complex chipset. Add a whitelist that allows enabling.

Here is another:

Turn MSI/MSI-X on by default and have blacklists.

Anything that requires unusual user interactions, such as specifying special module parameters, for correct operations is absolutely unacceptable.

I don't want to hear how difficult it is to determine whether a system will hit this bug or not. If it's hard, just turn MSI off unconditionally with this chipset until you can detect things properly.