[openib-general] [PATCH] mthca updates (2.6.8 dependent)

Roland Dreier roland at topspin.com
Sat Aug 14 09:36:53 PDT 2004


Now that Linus has released 2.6.8 (and 2.6.8.1), I've just committed a
few updates to mthca.  The major change is to use the new MSI/MSI-X
API in 2.6.8 (which means that mthca will not build against older
kernels).  Since mthca now depends on 2.6.8 anyway, I got rid of the
mthca_pci.h file (which just has defines that are all in 2.6.8).
Finally, I changed the logic to unconditionally reset the HCA during
initialization, since I've found that the previous initialization
logic may not work when handed an HCA in an unknown state.

To try MSI or MSI-X, you need a platform that supports MSI (right now,
that means an Intel system -- I've only tested i386, but x86-64 with
an Intel CPU and IA64 should theoretically work too).  Then build your
kernel with CONFIG_PCI_MSI=y and add msi=1 and/or msi_x=1 to your
mthca module options (if both are set, msi_x will be tried first).

I believe MSI-X requires HCA firmware that Mellanox has not yet
released.  MSI mode seems to work with current firmware (although I
have seen some unexplained machine checks when using MSI).

MSI-X seems to improve performance somewhat, because it allows the
driver to register three separate interrupt vectors and avoid having
to do a slow MMIO read of the event cause register in the interrupt
handler.  I don't know if there's much point to plain MSI, because
Linux only allows a driver to have a single interrupt even in MSI mode.

As usual all comments and test reports are gratefully accepted.

 - Roland

Index: hw/mthca/mthca_reset.c
===================================================================
--- hw/mthca/mthca_reset.c	(revision 649)
+++ hw/mthca/mthca_reset.c	(working copy)
@@ -25,6 +25,7 @@
 #include <linux/init.h>
 #include <linux/errno.h>
 #include <linux/pci.h>
+#include <linux/delay.h>
 
 #include "mthca_dev.h"
 #include "mthca_cmd.h"
@@ -33,7 +34,6 @@
 {
 	int i;
 	int err = 0;
-	u8 status;
 	u32 *hca_header    = NULL;
 	u32 *bridge_header = NULL;
 	struct pci_dev *bridge = NULL;
@@ -41,16 +41,9 @@
 #define MTHCA_RESET_OFFSET 0xf0010
 #define MTHCA_RESET_VALUE  cpu_to_be32(1)
 
-	mthca_info(mdev, "HCA already enabled -- restarting.\n");
-	/* Shut down the HCA cleanly first; assume it has two ports */
-	mthca_CLOSE_IB(mdev, 1, &status);
-	mthca_CLOSE_IB(mdev, 2, &status);
-	mthca_CLOSE_HCA(mdev, 0, &status);
-	mthca_SYS_DIS(mdev, &status);
-
 	/*
-	 * Now reset the chip.  This is somewhat ugly because we have
-	 * to save off the PCI header before reset and then restore it
+	 * Reset the chip.  This is somewhat ugly because we have to
+	 * save off the PCI header before reset and then restore it
 	 * after the chip reboots.  We skip config space offsets 22
 	 * and 23 since those have a special meaning.
 	 *
@@ -74,7 +67,12 @@
 		}
 
 		if (!bridge) {
-			mthca_err(mdev, "No bridge found for %s (%s), aborting\n",
+			/*
+			 * Didn't find a bridge for a Tavor device --
+			 * assume we're in no-bridge mode and hope for
+			 * the best.
+			 */
+			mthca_warn(mdev, "No bridge found for %s (%s)\n",
 				  pci_pretty_name(mdev->pdev), pci_name(mdev->pdev));
 			return -ENODEV;
 		}
@@ -139,8 +137,7 @@
 	}
 
 	/* Docs say to wait one second before accessing device */
-        set_current_state(TASK_UNINTERRUPTIBLE);
-	schedule_timeout(HZ);
+	msleep(1000);
 
 	/* Now wait for PCI device to start responding again */
 	{
@@ -156,20 +153,18 @@
 			}
 
 			if (v != 0xffffffff)
-				break;
-		}
+				goto good;
 
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		schedule_timeout(HZ / 10);
-
-		if (c == 100) {
-			err = -ENODEV;
-			mthca_err(mdev, "PCI device did not come back after reset, "
-				  "aborting.\n");
-			goto out;
+			msleep(100);
 		}
+
+		err = -ENODEV;
+		mthca_err(mdev, "PCI device did not come back after reset, "
+			  "aborting.\n");
+		goto out;
 	}
 
+good:
 	/* Now restore the PCI headers */
 	if (bridge) {
 		/*
@@ -217,7 +212,7 @@
 		goto out;
 	}
 
- out:
+out:
 	if (bridge)
 		pci_dev_put(bridge);
 	kfree(bridge_header);
Index: hw/mthca/Kconfig
===================================================================
--- hw/mthca/Kconfig	(revision 649)
+++ hw/mthca/Kconfig	(working copy)
@@ -14,24 +14,6 @@
 	  messages.  Select this is you are developing the driver or
 	  trying to diagnose a problem.
 
-config INFINIBAND_MTHCA_MSI
-	bool "MSI (Message Signaled Interrupt) support (EXPERIMENTAL)"
-	depends on INFINIBAND_MTHCA && PCI_USE_VECTOR && EXPERIMENTAL
-	---help---
-	  This option will have the mthca driver attempt to use MSI
-	  (message signaled interrupts) instead of old-style INTx PCI
-	  interrupts.
-
-config INFINIBAND_MTHCA_MSI_X
-	bool "MSI-X support (EXPERIMENTAL)"
-	depends on INFINIBAND_MTHCA && PCI_USE_VECTOR && EXPERIMENTAL && BROKEN
-	---help---
-	  This option will have the mthca driver attempt to use MSI-X
-	  (extended message signaled interrupts).  This allows the
-	  driver to use separate interrupts for each event queue,
-	  which may improve performance.  If both MSI and MSI-X are
-	  selected, MSI-X will be tried first.
-
 config INFINIBAND_MTHCA_SSE_DOORBELL
 	bool "SSE doorbell code"
 	depends on INFINIBAND_MTHCA && X86 && !X86_64
Index: hw/mthca/mthca_main.c
===================================================================
--- hw/mthca/mthca_main.c	(revision 649)
+++ hw/mthca/mthca_main.c	(working copy)
@@ -35,7 +35,6 @@
 #endif
 
 #include "mthca_dev.h"
-#include "mthca_pci.h"
 #include "mthca_config_reg.h"
 #include "mthca_cmd.h"
 #include "mthca_profile.h"
@@ -45,6 +44,23 @@
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_VERSION(DRV_VERSION);
 
+#ifdef CONFIG_PCI_MSI
+
+static int msi_x = 0;
+module_param(msi_x, int, 0444);
+MODULE_PARM_DESC(msi_x, "attempt to use MSI-X if nonzero");
+
+static int msi = 0;
+module_param(msi, int, 0444);
+MODULE_PARM_DESC(msi_x, "attempt to use MSI if nonzero");
+
+#else /* CONFIG_PCI_MSI */
+
+#define msi_x (0)
+#define msi   (0)
+
+#endif /* CONFIG_PCI_MSI */
+
 static const char mthca_version[] __devinitdata =
 	"ib_mthca: Mellanox InfiniBand HCA driver v"
 	DRV_VERSION " (" DRV_RELDATE ")\n";
@@ -106,36 +122,9 @@
 		return err;
 	}
 	if (status) {
-		if (status == MTHCA_CMD_STAT_BAD_SYS_STATE ||
-		    status == MTHCA_CMD_STAT_BAD_OP) {
-			/*
-			 * The HCA is already running, probably
-			 * because a boot ROM left it enabled.
-			 * Disable it and try again.
-			 */
-			err = mthca_reset(mdev);
-			if (err) {
-				mthca_err(mdev, "Failed to reset running HCA, "
-					  "aborting.\n");
-				return err;
-			}
-
-			err = mthca_SYS_EN(mdev, &status);
-			if (err) {
-				mthca_err(mdev, "SYS_EN command failed, "
-					  "aborting.\n");
-				return err;
-			}
-			if (status) {
-				mthca_err(mdev, "SYS_EN returned status 0x%02x, "
-					  "aborting.\n", status);
-				return -EINVAL;
-			}
-		} else  {
-			mthca_err(mdev, "SYS_EN returned status 0x%02x, "
-				  "aborting.\n", status);
-			return -EINVAL;
-		}
+		mthca_err(mdev, "SYS_EN returned status 0x%02x, "
+			  "aborting.\n", status);
+		return -EINVAL;
 	}
 
 	err = mthca_QUERY_FW(mdev, &status);
@@ -409,7 +398,6 @@
 		pci_release_region(pdev, 4);
 }
 
-#ifdef CONFIG_INFINIBAND_MTHCA_MSI_X
 static int mthca_enable_msi_x(struct mthca_dev *mdev)
 {
 	struct msix_entry entries[3];
@@ -419,7 +407,7 @@
 	entries[1].entry = 1;
 	entries[2].entry = 2;
 
-	err = pci_enable_msix(mdev->pdev, &entries, ARRAY_SIZE(entries));
+	err = pci_enable_msix(mdev->pdev, entries, ARRAY_SIZE(entries));
 	if (err) {
 		if (err > 0)
 			mthca_info(mdev, "Only %d MSI-X vectors available, "
@@ -433,19 +421,7 @@
 
 	return 0;
 }
-#endif /* CONFIG_INFINIBAND_MTHCA_MSI_X */
 
-static void mthca_free_irqs(struct mthca_dev *dev)
-{
-	int i;
-
-	if (dev->eq_table.have_irq)
-		free_irq(dev->pdev->irq, dev);
-	for (i = 0; i < MTHCA_NUM_EQ; ++i)
-		if (dev->eq_table.eq[i].have_irq)
-			free_irq(dev->eq_table.eq[i].msi_x_vector, dev);
-}
-
 static int __devinit mthca_init_one(struct pci_dev *pdev,
 				    const struct pci_device_id *id)
 {
@@ -534,15 +510,22 @@
 	if (ddr_hidden)
 		mdev->mthca_flags |= MTHCA_FLAG_DDR_HIDDEN;
 
-#ifdef CONFIG_INFINIBAND_MTHCA_MSI_X
-	if (!mthca_enable_msi_x(mdev))
+	/*
+	 * Now reset the HCA before we touch the PCI capabilities or
+	 * attempt a firmware command, since a boot ROM may have left
+	 * the HCA in an undefined state.
+	 */
+	err = mthca_reset(mdev);
+	if (err) {
+		mthca_err(mdev, "Failed to reset HCA, aborting.\n");
+		goto err_out_free_dev;
+	}
+
+	if (msi_x && !mthca_enable_msi_x(mdev))
 		mdev->mthca_flags |= MTHCA_FLAG_MSI_X;
-#endif
-#ifdef CONFIG_INFINIBAND_MTHCA_MSI
-	if (!(mdev->mthca_flags & MTHCA_FLAG_MSI_X) &&
+	if (msi && !(mdev->mthca_flags & MTHCA_FLAG_MSI_X) &&
 	    !pci_enable_msi(pdev))
 		mdev->mthca_flags |= MTHCA_FLAG_MSI;
-#endif
 
 	sema_init(&mdev->cmd.hcr_sem, 1);
 	sema_init(&mdev->cmd.poll_sem, 1);
@@ -619,8 +602,6 @@
 		mthca_SYS_DIS(mdev, &status);
 	}
 
-	mthca_free_irqs(mdev);
-
 err_out_iounmap_kar:
 	iounmap((void *) mdev->kar);
 
@@ -631,6 +612,11 @@
 	iounmap((void *) mdev->hcr);
 
 err_out_free_dev:
+	if (mdev->mthca_flags & MTHCA_FLAG_MSI_X)
+		pci_disable_msix(pdev);
+	if (mdev->mthca_flags & MTHCA_FLAG_MSI)
+		pci_disable_msi(pdev);
+
 	kfree(mdev);
 
 err_out_free_res:
@@ -668,15 +654,15 @@
 
 		mthca_CLOSE_HCA(mdev, 0, &status);
 		mthca_SYS_DIS(mdev, &status);
-		/*
-		 * We don't free our IRQ(s) until after SYS_DIS,
-		 * because it seems that SYS_DIS rewrites the PCI
-		 * config space, and if we are using MSI, we want to
-		 * make sure MSI stays disabled after we unload.
-		 */
-		mthca_free_irqs(mdev);
+
 		iounmap((void *) mdev->hcr);
 		iounmap((void *) mdev->clr_base);
+
+		if (mdev->mthca_flags & MTHCA_FLAG_MSI_X)
+			pci_disable_msix(pdev);
+		if (mdev->mthca_flags & MTHCA_FLAG_MSI)
+			pci_disable_msi(pdev);
+
 		kfree(mdev);
 		mthca_release_regions(pdev, mdev->mthca_flags &
 				      MTHCA_FLAG_DDR_HIDDEN);
Index: hw/mthca/ChangeLog
===================================================================
--- hw/mthca/ChangeLog	(revision 649)
+++ hw/mthca/ChangeLog	(working copy)
@@ -31,3 +31,8 @@
 		   HCA firmware that has not been released yet.  Both MSI
 		   and MSI-X may be less stable than using standard INTx).
 		   Implement more asynchronous events.
+		   Always reset the HCA on initialization since we we
+		   might not be able to tell that the HCA was left running.
+		   Remove mthca_pci.h (since we already depend on kernel
+		   2.6.8).
+		   Rework API to conform with new OpenIB verbs.
Index: hw/mthca/mthca_profile.c
===================================================================
--- hw/mthca/mthca_profile.c	(revision 649)
+++ hw/mthca/mthca_profile.c	(working copy)
@@ -26,7 +26,7 @@
 
 #include "mthca_profile.h"
 
-static int use_profile = 0;
+static int use_profile;
 module_param(use_profile, int, 0444);
 MODULE_PARM_DESC(use_profile,
 		 "load HCA profile through sysfs firmware "
Index: hw/mthca/mthca_eq.c
===================================================================
--- hw/mthca/mthca_eq.c	(revision 649)
+++ hw/mthca/mthca_eq.c	(working copy)
@@ -502,6 +502,18 @@
 	kfree(mailbox);
 }
 
+static void mthca_free_irqs(struct mthca_dev *dev)
+{
+	int i;
+
+	if (dev->eq_table.have_irq)
+		free_irq(dev->pdev->irq, dev);
+	for (i = 0; i < MTHCA_NUM_EQ; ++i)
+		if (dev->eq_table.eq[i].have_irq)
+			free_irq(dev->eq_table.eq[i].msi_x_vector,
+				 dev->eq_table.eq + i);
+}
+
 int __devinit mthca_init_eq_table(struct mthca_dev *dev)
 {
 	int err;
@@ -561,7 +573,7 @@
 		for (i = 0; i < MTHCA_NUM_EQ; ++i) {
 			err = request_irq(dev->eq_table.eq[i].msi_x_vector,
 					  mthca_msi_x_interrupt, 0,
-					  eq_name[i], &dev->eq_table.eq[i]);
+					  eq_name[i], dev->eq_table.eq + i);
 			if (err)
 				goto err_out_cmd;
 			dev->eq_table.eq[i].have_irq = 1;
@@ -593,6 +605,7 @@
 	return 0;
 
 err_out_cmd:
+	mthca_free_irqs(dev);
 	mthca_free_eq(dev, &dev->eq_table.eq[MTHCA_EQ_CMD]);
 
 err_out_async:
@@ -611,6 +624,8 @@
 	u8 status;
 	int i;
 
+	mthca_free_irqs(dev);
+
 	mthca_MAP_EQ(dev, MTHCA_ASYNC_EVENT_MASK,
 		     1, dev->eq_table.eq[MTHCA_EQ_ASYNC].eqn, &status);
 	mthca_MAP_EQ(dev, MTHCA_CMD_EVENT_MASK,
Index: hw/mthca/mthca_pci.h
===================================================================
--- hw/mthca/mthca_pci.h	(revision 649)
+++ hw/mthca/mthca_pci.h	(working copy)
@@ -1,55 +0,0 @@
-/*
- * This software is available to you under a choice of one of two
- * licenses.  You may choose to be licensed under the terms of the GNU
- * General Public License (GPL) Version 2, available at
- * <http://www.fsf.org/copyleft/gpl.html>, or the OpenIB.org BSD
- * license, available in the LICENSE.TXT file accompanying this
- * software.  These details are also available at
- * <http://openib.org/license.html>.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
- * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
- * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
- * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
- * SOFTWARE.
- *
- * Copyright (c) 2004 Topspin Communications.  All rights reserved.
- *
- * $Id$
- */
-
-/*
- * This file will be removed once all IDs are merged into an official
- * kernel release (they are already in Linus's BK tree and 2.6.7-rc2).
- */
-
-#ifndef MTHCA_PCI_H
-#define MTHCA_PCI_H
-
-#if !defined(PCI_VENDOR_ID_MELLANOX)
-#define PCI_VENDOR_ID_MELLANOX 0x15b3
-#endif
-
-#if !defined(PCI_VENDOR_ID_TOPSPIN)
-#define PCI_VENDOR_ID_TOPSPIN 0x1867
-#endif
-
-#if !defined(PCI_DEVICE_ID_MELLANOX_TAVOR)
-#define PCI_DEVICE_ID_MELLANOX_TAVOR 23108
-#endif
-
-#if !defined(PCI_DEVICE_ID_MELLANOX_ARBEL_COMPAT)
-#define PCI_DEVICE_ID_MELLANOX_ARBEL_COMPAT 25208
-#endif
-
-#endif /* MTHCA_PCI_H */
-
-/*
- * Local Variables:
- * c-file-style: "linux"
- * indent-tabs-mode: t
- * End:
- */



More information about the general mailing list