[ofa-general][PATCH 2/2] mlx4: ConnectX multi functional device support

[ofa-general][PATCH 2/2] mlx4: ConnectX multi functional device support

[Yevgeny Petrilin]

MT26468 (0x6764) device can open multiple physical functions.
The current driver can only work with one (primary) pf.
For all other functions, QUERY_FW command would fail with
CMD_STAT_MULTI_FUNC_REQ error code. We should not work on those
devices, but they should remain in the driver's ownership.

Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
---
 drivers/net/mlx4/cmd.c      |    5 ++++-
 drivers/net/mlx4/main.c     |   20 ++++++++++++++++----
 include/linux/mlx4/device.h |    1 +
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/net/mlx4/cmd.c b/drivers/net/mlx4/cmd.c
index 2845a05..92d649c 100644
--- a/drivers/net/mlx4/cmd.c
+++ b/drivers/net/mlx4/cmd.c
@@ -80,7 +80,9 @@ enum {
 	/* Bad management packet (silently discarded): */
 	CMD_STAT_BAD_PKT	= 0x30,
 	/* More outstanding CQEs in CQ than new CQ size: */
-	CMD_STAT_BAD_SIZE	= 0x40
+	CMD_STAT_BAD_SIZE	= 0x40,
+	/* Multi Function device support required: */
+	CMD_STAT_MULTI_FUNC_REQ	= 0x50
 };
 
 enum {
@@ -128,6 +130,7 @@ static int mlx4_status_to_errno(u8 status)
 		[CMD_STAT_LAM_NOT_PRE]	  = -EAGAIN,
 		[CMD_STAT_BAD_PKT]	  = -EINVAL,
 		[CMD_STAT_BAD_SIZE]	  = -ENOMEM,
+		[CMD_STAT_MULTI_FUNC_REQ] = -EACCES,
 	};
 
 	if (status >= ARRAY_SIZE(trans_table) ||
diff --git a/drivers/net/mlx4/main.c b/drivers/net/mlx4/main.c
index 57326f9..541243e 100644
--- a/drivers/net/mlx4/main.c
+++ b/drivers/net/mlx4/main.c
@@ -729,7 +729,10 @@ static int mlx4_init_hca(struct mlx4_dev *dev)
 
 	err = mlx4_QUERY_FW(dev);
 	if (err) {
-		mlx4_err(dev, "QUERY_FW command failed, aborting.\n");
+		if (err == -EACCES)
+			mlx4_dbg(dev, "Function disabled.\n");
+		else
+			mlx4_err(dev, "QUERY_FW command failed, aborting.\n");
 		return err;
 	}
 
@@ -1137,8 +1140,14 @@ static int __mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	}
 
 	err = mlx4_init_hca(dev);
-	if (err)
-		goto err_cmd;
+	if (err) {
+		if (err == -EACCES) {
+			dev->flags |= MLX4_FLAG_NOT_PRIME;
+			pci_set_drvdata(pdev, dev);
+			return 0;
+		} else
+			goto err_cmd;
+	}
 
 	err = mlx4_alloc_eq_table(dev);
 	if (err)
@@ -1234,6 +1243,8 @@ static void mlx4_remove_one(struct pci_dev *pdev)
 	int p;
 
 	if (dev) {
+		if (dev->flags & MLX4_FLAG_NOT_PRIME)
+			goto cmd_cleanup;
 		mlx4_stop_sense(dev);
 		mlx4_unregister_device(dev);
 
@@ -1256,11 +1267,12 @@ static void mlx4_remove_one(struct pci_dev *pdev)
 		mlx4_cleanup_uar_table(dev);
 		mlx4_free_eq_table(dev);
 		mlx4_close_hca(dev);
-		mlx4_cmd_cleanup(dev);
 
 		if (dev->flags & MLX4_FLAG_MSI_X)
 			pci_disable_msix(pdev);
 
+cmd_cleanup:
+		mlx4_cmd_cleanup(dev);
 		kfree(priv);
 		pci_release_region(pdev, 2);
 		pci_release_region(pdev, 0);
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index ce7cc6c..637e72c 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -42,6 +42,7 @@
 enum {
 	MLX4_FLAG_MSI_X		= 1 << 0,
 	MLX4_FLAG_OLD_PORT_CMDS	= 1 << 1,
+	MLX4_FLAG_NOT_PRIME	= 1 << 2,
 };
 
 enum {
-- 
1.6.0

Re: [ofa-general][PATCH 2/2] mlx4: ConnectX multi functional device support

[Roland Dreier]

 > MT26468 (0x6764) device can open multiple physical functions.
 > The current driver can only work with one (primary) pf.
 > For all other functions, QUERY_FW command would fail with
 > CMD_STAT_MULTI_FUNC_REQ error code. We should not work on those
 > devices, but they should remain in the driver's ownership.

Seems this patch should really be 1/2, since we want the driver to be
able to handle multi-func devices before we add the PCI ID for such
devices.  Also, it didn't occur to me before, but why does the driver
need to keep ownership of the non-primary functions?  It seems we could
avoid having the NOT_PRIME flag and all of that if we just gave up on a
device when QUERY_FW told us it wasn't the primary function.

Also from my naive point of view at least, it seems your hardware
interface could be simpler for software to handle if you just used a
different PCI ID for the non-primary physical function.  Not sure if
it's too late to change that (and maybe there's a reason I'm missing to
use the same PCI ID for functions that behave differently and require
different driver behavior to handle them??).

 - R.

Re: [ofa-general][PATCH 2/2] mlx4: ConnectX multi functional device support

[Yevgeny Petrilin]

Roland,
Following our discussion on these patches,
Is there any update, will they make it to 2.6.31?

Thanks,
Yevgeny

Re: [ofa-general][PATCH 2/2] mlx4: ConnectX multi functional device support

[Roland Dreier]

 > Is there any update, will they make it to 2.6.31?

Not sure... need to evaluate the risk of regression

Re: [ofa-general][PATCH 2/2] mlx4: ConnectX multi functional device support

[Tziporet Koren]

Roland Dreier wrote:
>  > Is there any update, will they make it to 2.6.31?
>
> Not sure... need to evaluate the risk of regression
>   
The risk is very low, however without it our new device coming in July 
will not work on the new kernel :-(
And if not on 2.6.31 can you at list queue it on your future work tree?

Tziporet

Re: [ofa-general][PATCH 2/2] mlx4: ConnectX multi functional device support

[Roland Dreier]

 > The risk is very low, however without it our new device coming in July
 > will not work on the new kernel :-(
 > And if not on 2.6.31 can you at list queue it on your future work tree?

Well, I need to think over the patch a little more.  The risk is
certainly not zero, since we are changing device initialization for all
mlx4 devices.  And I'm not sure if missing 2.6.31 support is that big a
deal for you, is it?  How many users do you have building their own
upstream kernels?  You're just going to tell everyone to use OFED
anyway, right?

 - R.

Re: [ofa-general][PATCH 2/2] mlx4: ConnectX multi functional device support

[Tziporet Koren]

Roland Dreier wrote:
>  
> Well, I need to think over the patch a little more.  The risk is
> certainly not zero, since we are changing device initialization for all
> mlx4 devices.  And I'm not sure if missing 2.6.31 support is that big a
> deal for you, is it?  How many users do you have building their own
> upstream kernels?  You're just going to tell everyone to use OFED
> anyway, right?
>   
This is NOT related to OFED at all. This is our 10G NIC driver. See the 
description:

/* MT26468 ConnectX EN 10GigE, PCIe, 2.0 5Gt/s */

We have several customer that take our 10G driver from kernel.org, and 
once the HW is out we will have a problem.
Also to include this code in next Redhat & Novell updates we need to 
have it in the kernel too.

I know the risk is not zero, but you must admit its not high either, and 
we already tested it here thoroughly here with all device IDs
Since we sent the patches on time for inclusion not clear why they are 
being declined now.
Please see what can be done.

Thanks,
Tziporet