[PATCH net-next 00/10] Mellanox ethernet driver updates Jan-27-2015

[PATCH net-next 00/10] Mellanox ethernet driver updates Jan-27-2015

[Amir Vadai]

Hi,

This patchset introduces some bug fixes, code cleanups and support in a new
firmware event called recoverable error events.

Patches were applied and tested against commit b8665c6 ("net: dsa/mv88e6352:
make mv88e6352_wait generic")

Thanks,
Amir

Jack Morgenstein (7):
  net/mlx4_core: Add bad-cable event support
  net/mlx4_core: Add reserved lkey for VFs to QUERY_FUNC_CAP
  net/mlx4_core: Fix mem leak in SRIOV mlx4_init_one error flow
  net/mlx4_core: Adjust command timeouts to conform to the firmware spec
  net/mlx4_core: Fix HW2SW_EQ to conform to the firmware spec
  net/mlx4_core: Fix struct mlx4_vhcr_cmd to make implicit padding
    explicit
  net/mlx4_core: Remove duplicate code line from procedure mlx4_bf_alloc

Matan Barak (1):
  net/mlx4: Fix memory corruption in mlx4_MAD_IFC_wrapper

Or Gerlitz (1):
  net/mlx4_core: Fix device capabilities dumping

Saeed Mahameed (1):
  net/mlx4_en: Use ethtool cmd->autoneg as a hint for ethtool set
    settings

 drivers/net/ethernet/mellanox/mlx4/cmd.c           |  6 ++-
 drivers/net/ethernet/mellanox/mlx4/en_ethtool.c    | 18 ++++---
 drivers/net/ethernet/mellanox/mlx4/eq.c            | 48 ++++++++++---------
 drivers/net/ethernet/mellanox/mlx4/fw.c            | 55 +++++++++++++++-------
 drivers/net/ethernet/mellanox/mlx4/fw.h            |  1 +
 drivers/net/ethernet/mellanox/mlx4/main.c          |  5 +-
 drivers/net/ethernet/mellanox/mlx4/mlx4.h          |  3 +-
 drivers/net/ethernet/mellanox/mlx4/mr.c            |  4 +-
 drivers/net/ethernet/mellanox/mlx4/pd.c            |  1 -
 .../net/ethernet/mellanox/mlx4/resource_tracker.c  | 16 ++-----
 include/linux/mlx4/cmd.h                           |  6 +--
 include/linux/mlx4/device.h                        | 14 +++++-
 12 files changed, 104 insertions(+), 73 deletions(-)

-- 
1.9.3

[PATCH net-next 01/10] net/mlx4_core: Add bad-cable event support

[Amir Vadai]

From: Jack Morgenstein <jackm@dev.mellanox.co.il>

If the firmware can detect a bad cable, allow it to generate an
event, and print the problem in the log.

Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/eq.c | 22 ++++++++++++++++++++++
 drivers/net/ethernet/mellanox/mlx4/fw.c |  9 ++++++++-
 include/linux/mlx4/device.h             | 14 +++++++++++++-
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/eq.c b/drivers/net/ethernet/mellanox/mlx4/eq.c
index 2f2e606..4df006d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/eq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/eq.c
@@ -88,6 +88,8 @@ static u64 get_async_ev_mask(struct mlx4_dev *dev)
 	u64 async_ev_mask = MLX4_ASYNC_EVENT_MASK;
 	if (dev->caps.flags & MLX4_DEV_CAP_FLAG_PORT_MNG_CHG_EV)
 		async_ev_mask |= (1ull << MLX4_EVENT_TYPE_PORT_MNG_CHG_EVENT);
+	if (dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_RECOVERABLE_ERROR_EVENT)
+		async_ev_mask |= (1ull << MLX4_EVENT_TYPE_RECOVERABLE_ERROR_EVENT);
 
 	return async_ev_mask;
 }
@@ -736,6 +738,26 @@ static int mlx4_eq_int(struct mlx4_dev *dev, struct mlx4_eq *eq)
 					    (unsigned long) eqe);
 			break;
 
+		case MLX4_EVENT_TYPE_RECOVERABLE_ERROR_EVENT:
+			switch (eqe->subtype) {
+			case MLX4_RECOVERABLE_ERROR_EVENT_SUBTYPE_BAD_CABLE:
+				mlx4_warn(dev, "Bad cable detected on port %u\n",
+					  eqe->event.bad_cable.port);
+				break;
+			case MLX4_RECOVERABLE_ERROR_EVENT_SUBTYPE_UNSUPPORTED_CABLE:
+				mlx4_warn(dev, "Unsupported cable detected\n");
+				break;
+			default:
+				mlx4_dbg(dev,
+					 "Unhandled recoverable error event detected: %02x(%02x) on EQ %d at index %u. owner=%x, nent=0x%x, ownership=%s\n",
+					 eqe->type, eqe->subtype, eq->eqn,
+					 eq->cons_index, eqe->owner, eq->nent,
+					 !!(eqe->owner & 0x80) ^
+					 !!(eq->cons_index & eq->nent) ? "HW" : "SW");
+				break;
+			}
+			break;
+
 		case MLX4_EVENT_TYPE_EEC_CATAS_ERROR:
 		case MLX4_EVENT_TYPE_ECC_DETECT:
 		default:
diff --git a/drivers/net/ethernet/mellanox/mlx4/fw.c b/drivers/net/ethernet/mellanox/mlx4/fw.c
index 982861d..2eadc28 100644
--- a/drivers/net/ethernet/mellanox/mlx4/fw.c
+++ b/drivers/net/ethernet/mellanox/mlx4/fw.c
@@ -145,7 +145,8 @@ static void dump_dev_cap_flags2(struct mlx4_dev *dev, u64 flags)
 		[16] = "CONFIG DEV support",
 		[17] = "Asymmetric EQs support",
 		[18] = "More than 80 VFs support",
-		[19] = "Performance optimized for limited rule configuration flow steering support"
+		[19] = "Performance optimized for limited rule configuration flow steering support",
+		[20] = "Recoverable error events support"
 	};
 	int i;
 
@@ -859,6 +860,8 @@ int mlx4_QUERY_DEV_CAP(struct mlx4_dev *dev, struct mlx4_dev_cap *dev_cap)
 	MLX4_GET(field32, outbox, QUERY_DEV_CAP_ETH_BACKPL_OFFSET);
 	if (field32 & (1 << 0))
 		dev_cap->flags2 |= MLX4_DEV_CAP_FLAG2_ETH_BACKPL_AN_REP;
+	if (field32 & (1 << 7))
+		dev_cap->flags2 |= MLX4_DEV_CAP_FLAG2_RECOVERABLE_ERROR_EVENT;
 	MLX4_GET(field, outbox, QUERY_DEV_CAP_FW_REASSIGN_MAC);
 	if (field & 1<<6)
 		dev_cap->flags2 |= MLX4_DEV_CAP_FLAG2_REASSIGN_MAC_EN;
@@ -1562,6 +1565,7 @@ int mlx4_INIT_HCA(struct mlx4_dev *dev, struct mlx4_init_hca_param *param)
 #define INIT_HCA_VXLAN_OFFSET		 0x0c
 #define INIT_HCA_CACHELINE_SZ_OFFSET	 0x0e
 #define INIT_HCA_FLAGS_OFFSET		 0x014
+#define INIT_HCA_RECOVERABLE_ERROR_EVENT_OFFSET 0x018
 #define INIT_HCA_QPC_OFFSET		 0x020
 #define	 INIT_HCA_QPC_BASE_OFFSET	 (INIT_HCA_QPC_OFFSET + 0x10)
 #define	 INIT_HCA_LOG_QP_OFFSET		 (INIT_HCA_QPC_OFFSET + 0x17)
@@ -1668,6 +1672,9 @@ int mlx4_INIT_HCA(struct mlx4_dev *dev, struct mlx4_init_hca_param *param)
 		dev->caps.userspace_caps |= MLX4_USER_DEV_CAP_LARGE_CQE;
 	}
 
+	if (dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_RECOVERABLE_ERROR_EVENT)
+		*(inbox + INIT_HCA_RECOVERABLE_ERROR_EVENT_OFFSET / 4) |= cpu_to_be32(1 << 31);
+
 	/* QPC/EEC/CQC/EQC/RDMARC attributes */
 
 	MLX4_PUT(inbox, param->qpc_base,      INIT_HCA_QPC_BASE_OFFSET);
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index 5ef54e1..c95d659 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -200,7 +200,8 @@ enum {
 	MLX4_DEV_CAP_FLAG2_CONFIG_DEV		= 1LL <<  16,
 	MLX4_DEV_CAP_FLAG2_SYS_EQS		= 1LL <<  17,
 	MLX4_DEV_CAP_FLAG2_80_VFS		= 1LL <<  18,
-	MLX4_DEV_CAP_FLAG2_FS_A0		= 1LL <<  19
+	MLX4_DEV_CAP_FLAG2_FS_A0		= 1LL <<  19,
+	MLX4_DEV_CAP_FLAG2_RECOVERABLE_ERROR_EVENT = 1LL << 20
 };
 
 enum {
@@ -280,6 +281,7 @@ enum mlx4_event {
 	MLX4_EVENT_TYPE_FATAL_WARNING	   = 0x1b,
 	MLX4_EVENT_TYPE_FLR_EVENT	   = 0x1c,
 	MLX4_EVENT_TYPE_PORT_MNG_CHG_EVENT = 0x1d,
+	MLX4_EVENT_TYPE_RECOVERABLE_ERROR_EVENT  = 0x3e,
 	MLX4_EVENT_TYPE_NONE		   = 0xff,
 };
 
@@ -289,6 +291,11 @@ enum {
 };
 
 enum {
+	MLX4_RECOVERABLE_ERROR_EVENT_SUBTYPE_BAD_CABLE		= 1,
+	MLX4_RECOVERABLE_ERROR_EVENT_SUBTYPE_UNSUPPORTED_CABLE	= 2,
+};
+
+enum {
 	MLX4_FATAL_WARNING_SUBTYPE_WARMING = 0,
 };
 
@@ -860,6 +867,11 @@ struct mlx4_eqe {
 				} __packed tbl_change_info;
 			} params;
 		} __packed port_mgmt_change;
+		struct {
+			u8 reserved[3];
+			u8 port;
+			u32 reserved1[5];
+		} __packed bad_cable;
 	}			event;
 	u8			slave_id;
 	u8			reserved3[2];
-- 
1.9.3

[PATCH net-next 02/10] net/mlx4_core: Add reserved lkey for VFs to QUERY_FUNC_CAP

[Amir Vadai]

From: Jack Morgenstein <jackm@dev.mellanox.co.il>

The reserved lKey is different for each VF.
A base lkey value is returned in QUERY_DEV_CAP at offset 0x98.

The reserved L_key value for a VF is:
    VF_lkey = base_lkey + (VF_number << 8).

This VF L_key value should be returned in QUERY_FUNC_CAP
(opcode-modifier = 0) at offset 0x48.

To indicate that the lkey value at offset 0x48 is valid, the Hypervisor
sets a flag bit in dword 0x0, offset 27 in the QUERY_FUNC_CAP wrapper
function.

When the VF calls QUERY_FUNC_CAP, it should check if this flag bit is set.
If it is set, the VF should take the reserved lkey value at offset 0x48.
If the bit is not set, the VF should not use a reserved lkey
(i.e., should set its reserved lkey value to 0).


Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/fw.c   | 19 +++++++++++++++++--
 drivers/net/ethernet/mellanox/mlx4/fw.h   |  1 +
 drivers/net/ethernet/mellanox/mlx4/main.c |  1 +
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/fw.c b/drivers/net/ethernet/mellanox/mlx4/fw.c
index 2eadc28..2aa7c23 100644
--- a/drivers/net/ethernet/mellanox/mlx4/fw.c
+++ b/drivers/net/ethernet/mellanox/mlx4/fw.c
@@ -260,6 +260,7 @@ int mlx4_QUERY_FUNC_CAP_wrapper(struct mlx4_dev *dev, int slave,
 #define QUERY_FUNC_CAP_MCG_QUOTA_OFFSET_DEP	0x28
 #define QUERY_FUNC_CAP_MAX_EQ_OFFSET		0x2c
 #define QUERY_FUNC_CAP_RESERVED_EQ_OFFSET	0x30
+#define QUERY_FUNC_CAP_QP_RESD_LKEY_OFFSET	0x48
 
 #define QUERY_FUNC_CAP_QP_QUOTA_OFFSET		0x50
 #define QUERY_FUNC_CAP_CQ_QUOTA_OFFSET		0x54
@@ -274,6 +275,7 @@ int mlx4_QUERY_FUNC_CAP_wrapper(struct mlx4_dev *dev, int slave,
 #define QUERY_FUNC_CAP_FLAG_RDMA		0x40
 #define QUERY_FUNC_CAP_FLAG_ETH			0x80
 #define QUERY_FUNC_CAP_FLAG_QUOTAS		0x10
+#define QUERY_FUNC_CAP_FLAG_RESD_LKEY		0x08
 #define QUERY_FUNC_CAP_FLAG_VALID_MAILBOX	0x04
 
 #define QUERY_FUNC_CAP_EXTRA_FLAGS_BF_QP_ALLOC_FLAG	(1UL << 31)
@@ -345,9 +347,12 @@ int mlx4_QUERY_FUNC_CAP_wrapper(struct mlx4_dev *dev, int slave,
 	} else if (vhcr->op_modifier == 0) {
 		struct mlx4_active_ports actv_ports =
 			mlx4_get_active_ports(dev, slave);
-		/* enable rdma and ethernet interfaces, and new quota locations */
+		/* enable rdma and ethernet interfaces, new quota locations,
+		 * and reserved lkey
+		 */
 		field = (QUERY_FUNC_CAP_FLAG_ETH | QUERY_FUNC_CAP_FLAG_RDMA |
-			 QUERY_FUNC_CAP_FLAG_QUOTAS | QUERY_FUNC_CAP_FLAG_VALID_MAILBOX);
+			 QUERY_FUNC_CAP_FLAG_QUOTAS | QUERY_FUNC_CAP_FLAG_VALID_MAILBOX |
+			 QUERY_FUNC_CAP_FLAG_RESD_LKEY);
 		MLX4_PUT(outbox->buf, field, QUERY_FUNC_CAP_FLAGS_OFFSET);
 
 		field = min(
@@ -412,6 +417,9 @@ int mlx4_QUERY_FUNC_CAP_wrapper(struct mlx4_dev *dev, int slave,
 		size = QUERY_FUNC_CAP_EXTRA_FLAGS_BF_QP_ALLOC_FLAG |
 			QUERY_FUNC_CAP_EXTRA_FLAGS_A0_QP_ALLOC_FLAG;
 		MLX4_PUT(outbox->buf, size, QUERY_FUNC_CAP_EXTRA_FLAGS_OFFSET);
+
+		size = dev->caps.reserved_lkey + ((slave << 8) & 0xFF00);
+		MLX4_PUT(outbox->buf, size, QUERY_FUNC_CAP_QP_RESD_LKEY_OFFSET);
 	} else
 		err = -EINVAL;
 
@@ -504,6 +512,13 @@ int mlx4_QUERY_FUNC_CAP(struct mlx4_dev *dev, u8 gen_or_port,
 		MLX4_GET(size, outbox, QUERY_FUNC_CAP_RESERVED_EQ_OFFSET);
 		func_cap->reserved_eq = size & 0xFFFFFF;
 
+		if (func_cap->flags & QUERY_FUNC_CAP_FLAG_RESD_LKEY) {
+			MLX4_GET(size, outbox, QUERY_FUNC_CAP_QP_RESD_LKEY_OFFSET);
+			func_cap->reserved_lkey = size;
+		} else {
+			func_cap->reserved_lkey = 0;
+		}
+
 		func_cap->extra_flags = 0;
 
 		/* Mailbox data from 0x6c and onward should only be treated if
diff --git a/drivers/net/ethernet/mellanox/mlx4/fw.h b/drivers/net/ethernet/mellanox/mlx4/fw.h
index 62562b6..f44f7f6 100644
--- a/drivers/net/ethernet/mellanox/mlx4/fw.h
+++ b/drivers/net/ethernet/mellanox/mlx4/fw.h
@@ -147,6 +147,7 @@ struct mlx4_func_cap {
 	u32	qp0_proxy_qpn;
 	u32	qp1_tunnel_qpn;
 	u32	qp1_proxy_qpn;
+	u32	reserved_lkey;
 	u8	physical_port;
 	u8	port_flags;
 	u8	flags1;
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 9c7ef0b..46531c2 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -797,6 +797,7 @@ static int mlx4_slave_cap(struct mlx4_dev *dev)
 	dev->caps.num_mpts		= 1 << hca_param.log_mpt_sz;
 	dev->caps.num_eqs		= func_cap.max_eq;
 	dev->caps.reserved_eqs		= func_cap.reserved_eq;
+	dev->caps.reserved_lkey		= func_cap.reserved_lkey;
 	dev->caps.num_pds               = MLX4_NUM_PDS;
 	dev->caps.num_mgms              = 0;
 	dev->caps.num_amgms             = 0;
-- 
1.9.3

[PATCH net-next 03/10] net/mlx4_core: Fix mem leak in SRIOV mlx4_init_one error flow

[Amir Vadai]

From: Jack Morgenstein <jackm@dev.mellanox.co.il>

Structs allocated for the resource tracker must be freed in
the error flow.


Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 46531c2..ab990f9 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2980,8 +2980,10 @@ err_free_eq:
 	mlx4_free_eq_table(dev);
 
 err_master_mfunc:
-	if (mlx4_is_master(dev))
+	if (mlx4_is_master(dev)) {
+		mlx4_free_resource_tracker(dev, RES_TR_FREE_STRUCTS_ONLY);
 		mlx4_multi_func_cleanup(dev);
+	}
 
 	if (mlx4_is_slave(dev)) {
 		kfree(dev->caps.qp0_qkey);
-- 
1.9.3

[PATCH net-next 04/10] net/mlx4_core: Adjust command timeouts to conform to the firmware spec

[Amir Vadai]

From: Jack Morgenstein <jackm@dev.mellanox.co.il>

The firmware spec states that the timeout for all commands should be 60 seconds.

In the past, the spec indicated that there were several classes of timeout
(short, medium, and long).  The driver has these different timeout classes.
We leave the class differentiation in the driver as-is (to protect against any
future spec changes), but set the timeout for all classes to be 60 seconds.

In addition, we fix a few commands which had hard-coded numeric timeouts specified.

Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/fw.c | 19 ++++++++++---------
 drivers/net/ethernet/mellanox/mlx4/mr.c |  4 ++--
 include/linux/mlx4/cmd.h                |  6 +++---
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/fw.c b/drivers/net/ethernet/mellanox/mlx4/fw.c
index 2aa7c23..0c90d10 100644
--- a/drivers/net/ethernet/mellanox/mlx4/fw.c
+++ b/drivers/net/ethernet/mellanox/mlx4/fw.c
@@ -1774,8 +1774,8 @@ int mlx4_INIT_HCA(struct mlx4_dev *dev, struct mlx4_init_hca_param *param)
 		MLX4_PUT(inbox, parser_params,	INIT_HCA_VXLAN_OFFSET);
 	}
 
-	err = mlx4_cmd(dev, mailbox->dma, 0, 0, MLX4_CMD_INIT_HCA, 10000,
-		       MLX4_CMD_NATIVE);
+	err = mlx4_cmd(dev, mailbox->dma, 0, 0, MLX4_CMD_INIT_HCA,
+		       MLX4_CMD_TIME_CLASS_C, MLX4_CMD_NATIVE);
 
 	if (err)
 		mlx4_err(dev, "INIT_HCA returns %d\n", err);
@@ -2029,7 +2029,7 @@ int mlx4_CLOSE_PORT_wrapper(struct mlx4_dev *dev, int slave,
 	if (dev->caps.port_mask[port] != MLX4_PORT_TYPE_IB) {
 		if (priv->mfunc.master.init_port_ref[port] == 1) {
 			err = mlx4_cmd(dev, 0, port, 0, MLX4_CMD_CLOSE_PORT,
-				       1000, MLX4_CMD_NATIVE);
+				       MLX4_CMD_TIME_CLASS_A, MLX4_CMD_NATIVE);
 			if (err)
 				return err;
 		}
@@ -2040,7 +2040,7 @@ int mlx4_CLOSE_PORT_wrapper(struct mlx4_dev *dev, int slave,
 			if (!priv->mfunc.master.qp0_state[port].qp0_active &&
 			    priv->mfunc.master.qp0_state[port].port_active) {
 				err = mlx4_cmd(dev, 0, port, 0, MLX4_CMD_CLOSE_PORT,
-					       1000, MLX4_CMD_NATIVE);
+					       MLX4_CMD_TIME_CLASS_A, MLX4_CMD_NATIVE);
 				if (err)
 					return err;
 				priv->mfunc.master.slave_state[slave].init_port_mask &= ~(1 << port);
@@ -2055,15 +2055,15 @@ int mlx4_CLOSE_PORT_wrapper(struct mlx4_dev *dev, int slave,
 
 int mlx4_CLOSE_PORT(struct mlx4_dev *dev, int port)
 {
-	return mlx4_cmd(dev, 0, port, 0, MLX4_CMD_CLOSE_PORT, 1000,
-			MLX4_CMD_WRAPPED);
+	return mlx4_cmd(dev, 0, port, 0, MLX4_CMD_CLOSE_PORT,
+			MLX4_CMD_TIME_CLASS_A, MLX4_CMD_WRAPPED);
 }
 EXPORT_SYMBOL_GPL(mlx4_CLOSE_PORT);
 
 int mlx4_CLOSE_HCA(struct mlx4_dev *dev, int panic)
 {
-	return mlx4_cmd(dev, 0, 0, panic, MLX4_CMD_CLOSE_HCA, 1000,
-			MLX4_CMD_NATIVE);
+	return mlx4_cmd(dev, 0, 0, panic, MLX4_CMD_CLOSE_HCA,
+			MLX4_CMD_TIME_CLASS_C, MLX4_CMD_NATIVE);
 }
 
 struct mlx4_config_dev {
@@ -2202,7 +2202,8 @@ int mlx4_SET_ICM_SIZE(struct mlx4_dev *dev, u64 icm_size, u64 *aux_pages)
 int mlx4_NOP(struct mlx4_dev *dev)
 {
 	/* Input modifier of 0x1f means "finish as soon as possible." */
-	return mlx4_cmd(dev, 0, 0x1f, 0, MLX4_CMD_NOP, 100, MLX4_CMD_NATIVE);
+	return mlx4_cmd(dev, 0, 0x1f, 0, MLX4_CMD_NOP, MLX4_CMD_TIME_CLASS_A,
+			MLX4_CMD_NATIVE);
 }
 
 int mlx4_get_phys_port_id(struct mlx4_dev *dev)
diff --git a/drivers/net/ethernet/mellanox/mlx4/mr.c b/drivers/net/ethernet/mellanox/mlx4/mr.c
index 8dbdf1d..d21e884 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mr.c
+++ b/drivers/net/ethernet/mellanox/mlx4/mr.c
@@ -1155,7 +1155,7 @@ EXPORT_SYMBOL_GPL(mlx4_fmr_free);
 
 int mlx4_SYNC_TPT(struct mlx4_dev *dev)
 {
-	return mlx4_cmd(dev, 0, 0, 0, MLX4_CMD_SYNC_TPT, 1000,
-			MLX4_CMD_NATIVE);
+	return mlx4_cmd(dev, 0, 0, 0, MLX4_CMD_SYNC_TPT,
+			MLX4_CMD_TIME_CLASS_A, MLX4_CMD_NATIVE);
 }
 EXPORT_SYMBOL_GPL(mlx4_SYNC_TPT);
diff --git a/include/linux/mlx4/cmd.h b/include/linux/mlx4/cmd.h
index c989442..ae95adc 100644
--- a/include/linux/mlx4/cmd.h
+++ b/include/linux/mlx4/cmd.h
@@ -165,9 +165,9 @@ enum {
 };
 
 enum {
-	MLX4_CMD_TIME_CLASS_A	= 10000,
-	MLX4_CMD_TIME_CLASS_B	= 10000,
-	MLX4_CMD_TIME_CLASS_C	= 10000,
+	MLX4_CMD_TIME_CLASS_A	= 60000,
+	MLX4_CMD_TIME_CLASS_B	= 60000,
+	MLX4_CMD_TIME_CLASS_C	= 60000,
 };
 
 enum {
-- 
1.9.3

[PATCH net-next 05/10] net/mlx4_core: Fix HW2SW_EQ to conform to the firmware spec

[Amir Vadai]

From: Jack Morgenstein <jackm@dev.mellanox.co.il>

The driver incorrectly assigned an out-mailbox to this command,
and used an opcode modifier = 0, which is a reserved value (it
should use opcode modifier = 1).

Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/cmd.c           |  2 +-
 drivers/net/ethernet/mellanox/mlx4/eq.c            | 26 ++++------------------
 .../net/ethernet/mellanox/mlx4/resource_tracker.c  | 16 ++++---------
 3 files changed, 9 insertions(+), 35 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
index 2b48932..928b706 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
@@ -1221,7 +1221,7 @@ static struct mlx4_cmd_info cmd_info[] = {
 	{
 		.opcode = MLX4_CMD_HW2SW_EQ,
 		.has_inbox = false,
-		.has_outbox = true,
+		.has_outbox = false,
 		.out_is_imm = false,
 		.encode_slave_id = true,
 		.verify = NULL,
diff --git a/drivers/net/ethernet/mellanox/mlx4/eq.c b/drivers/net/ethernet/mellanox/mlx4/eq.c
index 4df006d..264bc15 100644
--- a/drivers/net/ethernet/mellanox/mlx4/eq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/eq.c
@@ -868,12 +868,10 @@ static int mlx4_SW2HW_EQ(struct mlx4_dev *dev, struct mlx4_cmd_mailbox *mailbox,
 			MLX4_CMD_WRAPPED);
 }
 
-static int mlx4_HW2SW_EQ(struct mlx4_dev *dev, struct mlx4_cmd_mailbox *mailbox,
-			 int eq_num)
+static int mlx4_HW2SW_EQ(struct mlx4_dev *dev,  int eq_num)
 {
-	return mlx4_cmd_box(dev, 0, mailbox->dma, eq_num,
-			    0, MLX4_CMD_HW2SW_EQ, MLX4_CMD_TIME_CLASS_A,
-			    MLX4_CMD_WRAPPED);
+	return mlx4_cmd(dev, 0, eq_num, 1, MLX4_CMD_HW2SW_EQ,
+			MLX4_CMD_TIME_CLASS_A, MLX4_CMD_WRAPPED);
 }
 
 static int mlx4_num_eq_uar(struct mlx4_dev *dev)
@@ -1046,7 +1044,6 @@ static void mlx4_free_eq(struct mlx4_dev *dev,
 			 struct mlx4_eq *eq)
 {
 	struct mlx4_priv *priv = mlx4_priv(dev);
-	struct mlx4_cmd_mailbox *mailbox;
 	int err;
 	int i;
 	/* CX3 is capable of extending the CQE/EQE from 32 to 64 bytes, with
@@ -1054,24 +1051,10 @@ static void mlx4_free_eq(struct mlx4_dev *dev,
 	 */
 	int npages = PAGE_ALIGN(dev->caps.eqe_size  * eq->nent) / PAGE_SIZE;
 
-	mailbox = mlx4_alloc_cmd_mailbox(dev);
-	if (IS_ERR(mailbox))
-		return;
-
-	err = mlx4_HW2SW_EQ(dev, mailbox, eq->eqn);
+	err = mlx4_HW2SW_EQ(dev, eq->eqn);
 	if (err)
 		mlx4_warn(dev, "HW2SW_EQ failed (%d)\n", err);
 
-	if (0) {
-		mlx4_dbg(dev, "Dumping EQ context %02x:\n", eq->eqn);
-		for (i = 0; i < sizeof (struct mlx4_eq_context) / 4; ++i) {
-			if (i % 4 == 0)
-				pr_cont("[%02x] ", i * 4);
-			pr_cont(" %08x", be32_to_cpup(mailbox->buf + i * 4));
-			if ((i + 1) % 4 == 0)
-				pr_cont("\n");
-		}
-	}
 	synchronize_irq(eq->irq);
 	tasklet_disable(&eq->tasklet_ctx.task);
 
@@ -1083,7 +1066,6 @@ static void mlx4_free_eq(struct mlx4_dev *dev,
 
 	kfree(eq->page_list);
 	mlx4_bitmap_free(&priv->eq_table.bitmap, eq->eqn, MLX4_USE_RR);
-	mlx4_free_cmd_mailbox(dev, mailbox);
 }
 
 static void mlx4_free_irqs(struct mlx4_dev *dev)
diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
index 3e93879..79feeb6 100644
--- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
+++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
@@ -4677,7 +4677,6 @@ static void rem_slave_eqs(struct mlx4_dev *dev, int slave)
 	int state;
 	LIST_HEAD(tlist);
 	int eqn;
-	struct mlx4_cmd_mailbox *mailbox;
 
 	err = move_all_busy(dev, slave, RES_EQ);
 	if (err)
@@ -4703,20 +4702,13 @@ static void rem_slave_eqs(struct mlx4_dev *dev, int slave)
 					break;
 
 				case RES_EQ_HW:
-					mailbox = mlx4_alloc_cmd_mailbox(dev);
-					if (IS_ERR(mailbox)) {
-						cond_resched();
-						continue;
-					}
-					err = mlx4_cmd_box(dev, slave, 0,
-							   eqn & 0xff, 0,
-							   MLX4_CMD_HW2SW_EQ,
-							   MLX4_CMD_TIME_CLASS_A,
-							   MLX4_CMD_NATIVE);
+					err = mlx4_cmd(dev, slave, eqn & 0xff,
+						       1, MLX4_CMD_HW2SW_EQ,
+						       MLX4_CMD_TIME_CLASS_A,
+						       MLX4_CMD_NATIVE);
 					if (err)
 						mlx4_dbg(dev, "rem_slave_eqs: failed to move slave %d eqs %d to SW ownership\n",
 							 slave, eqn);
-					mlx4_free_cmd_mailbox(dev, mailbox);
 					atomic_dec(&eq->mtt->ref_count);
 					state = RES_EQ_RESERVED;
 					break;
-- 
1.9.3

[PATCH net-next 06/10] net/mlx4_core: Fix struct mlx4_vhcr_cmd to make implicit padding explicit

[Amir Vadai]

From: Jack Morgenstein <jackm@dev.mellanox.co.il>

Struct mlx4_vhcr was implicitly padded by the gcc compiler.
This commit makes that padding explicit, to prevent issues with
changing compilers. Note that we added the padding dword (rather
than simply packing the structure) in order to maintain compatibility
with previous kernels.


Reported-by: Alexander Schmidt <alexs@linux.vnet.ibm.com>
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/mlx4.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
index 096a81c..595e18a 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
@@ -196,13 +196,14 @@ struct mlx4_vhcr {
 struct mlx4_vhcr_cmd {
 	__be64 in_param;
 	__be32 in_modifier;
+	u32 reserved1;
 	__be64 out_param;
 	__be16 token;
 	u16 reserved;
 	u8 status;
 	u8 flags;
 	__be16 opcode;
-};
+} __packed;
 
 struct mlx4_cmd_info {
 	u16 opcode;
-- 
1.9.3

[PATCH net-next 07/10] net/mlx4_core: Remove duplicate code line from procedure mlx4_bf_alloc

[Amir Vadai]

From: Jack Morgenstein <jackm@dev.mellanox.co.il>

mlx4_bf_alloc had an unnecessary/duplicate code line. Did no harm,
but not good practice.

Reported by the Mellanox Beijing team.

Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/pd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/pd.c b/drivers/net/ethernet/mellanox/mlx4/pd.c
index a42b4c0..609c59d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/pd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/pd.c
@@ -214,7 +214,6 @@ int mlx4_bf_alloc(struct mlx4_dev *dev, struct mlx4_bf *bf, int node)
 		list_add(&uar->bf_list, &priv->bf_list);
 	}
 
-	bf->uar = uar;
 	idx = ffz(uar->free_bf_bmap);
 	uar->free_bf_bmap |= 1 << idx;
 	bf->uar = uar;
-- 
1.9.3

[PATCH net-next 08/10] net/mlx4_en: Use ethtool cmd->autoneg as a hint for ethtool set settings

[Amir Vadai]

From: Saeed Mahameed <saeedm@mellanox.com>

Use cmd->autoneg as a user hint to decide what to set in ethtool set settings callback.
When cmd->autoneg == AUTONEG_ENABLE set according to ethtool->advertise otherwise,
set according to ethtool->speed.

Usage:
	- ethtool -s eth<x> speed 56000 autoneg off
	- ethtool -s eth<x> advertise 0x800000 autoneg on

While we're here:
	- Move proto_admin masking outcome check to be adjacent to the operation.
	- Move en_warn("port reset..") print to "port reset" block.

Fixes: 312df74 ("net/mlx4_en: mlx4_en_set_settings() always fails when autoneg is set")
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
index 569eda9..a7b58ba 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
@@ -770,22 +770,20 @@ static int mlx4_en_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 		return 0;
 	}
 
-	proto_admin = cpu_to_be32(ptys_adv);
-	if (speed >= 0 && speed != priv->port_state.link_speed)
-		/* If speed was set then speed decides :-) */
-		proto_admin = speed_set_ptys_admin(priv, speed,
-						   ptys_reg.eth_proto_cap);
+	proto_admin = cmd->autoneg == AUTONEG_ENABLE ?
+		cpu_to_be32(ptys_adv) :
+		speed_set_ptys_admin(priv, speed,
+				     ptys_reg.eth_proto_cap);
 
 	proto_admin &= ptys_reg.eth_proto_cap;
-
-	if (proto_admin == ptys_reg.eth_proto_admin)
-		return 0; /* Nothing to change */
-
 	if (!proto_admin) {
 		en_warn(priv, "Not supported link mode(s) requested, check supported link modes.\n");
 		return -EINVAL; /* nothing to change due to bad input */
 	}
 
+	if (proto_admin == ptys_reg.eth_proto_admin)
+		return 0; /* Nothing to change */
+
 	en_dbg(DRV, priv, "mlx4_ACCESS_PTYS_REG SET: ptys_reg.eth_proto_admin = 0x%x\n",
 	       be32_to_cpu(proto_admin));
 
@@ -798,9 +796,9 @@ static int mlx4_en_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 		return ret;
 	}
 
-	en_warn(priv, "Port link mode changed, restarting port...\n");
 	mutex_lock(&priv->mdev->state_lock);
 	if (priv->port_up) {
+		en_warn(priv, "Port link mode changed, restarting port...\n");
 		mlx4_en_stop_port(dev, 1);
 		if (mlx4_en_start_port(dev))
 			en_err(priv, "Failed restarting port %d\n", priv->port);
-- 
1.9.3

[PATCH net-next 09/10] net/mlx4: Fix memory corruption in mlx4_MAD_IFC_wrapper

[Amir Vadai]

From: Matan Barak <matanb@mellanox.com>

Fix a memory corruption at mlx4_MAD_IFC_wrapper.

A table of size dev->caps.pkey_table_len[port]*sizeof(*table)
was allocated, but get_full_pkey_table() assumes that the number
of entries in the table is a multiplication of 32 (which isn't always
correct).

Fixes: 0a9a018 ('mlx4: MAD_IFC paravirtualization')
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Matan Barak <matanb@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/cmd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
index 928b706..154effb 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
@@ -901,7 +901,9 @@ static int mlx4_MAD_IFC_wrapper(struct mlx4_dev *dev, int slave,
 				index = be32_to_cpu(smp->attr_mod);
 				if (port < 1 || port > dev->caps.num_ports)
 					return -EINVAL;
-				table = kcalloc(dev->caps.pkey_table_len[port], sizeof *table, GFP_KERNEL);
+				table = kcalloc((dev->caps.pkey_table_len[port] / 32) + 1,
+						sizeof(*table) * 32, GFP_KERNEL);
+
 				if (!table)
 					return -ENOMEM;
 				/* need to get the full pkey table because the paravirtualized
-- 
1.9.3

[PATCH net-next 10/10] net/mlx4_core: Fix device capabilities dumping

[Amir Vadai]

From: Or Gerlitz <ogerlitz@mellanox.com>

We are dumping device capabilities which are supported both by the
firmware and the driver. Align the array that holds the capability
strings with this practice.

Reported-by: Yuval Shaia <yuval.shaia@oracle.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/fw.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/fw.c b/drivers/net/ethernet/mellanox/mlx4/fw.c
index 0c90d10..0ab81a9 100644
--- a/drivers/net/ethernet/mellanox/mlx4/fw.c
+++ b/drivers/net/ethernet/mellanox/mlx4/fw.c
@@ -84,13 +84,10 @@ static void dump_dev_cap_flags(struct mlx4_dev *dev, u64 flags)
 		[ 1] = "UC transport",
 		[ 2] = "UD transport",
 		[ 3] = "XRC transport",
-		[ 4] = "reliable multicast",
-		[ 5] = "FCoIB support",
 		[ 6] = "SRQ support",
 		[ 7] = "IPoIB checksum offload",
 		[ 8] = "P_Key violation counter",
 		[ 9] = "Q_Key violation counter",
-		[10] = "VMM",
 		[12] = "Dual Port Different Protocol (DPDP) support",
 		[15] = "Big LSO headers",
 		[16] = "MW support",
@@ -99,12 +96,11 @@ static void dump_dev_cap_flags(struct mlx4_dev *dev, u64 flags)
 		[19] = "Raw multicast support",
 		[20] = "Address vector port checking support",
 		[21] = "UD multicast support",
-		[24] = "Demand paging support",
-		[25] = "Router support",
 		[30] = "IBoE support",
 		[32] = "Unicast loopback support",
 		[34] = "FCS header control",
-		[38] = "Wake On LAN support",
+		[37] = "Wake On LAN (port1) support",
+		[38] = "Wake On LAN (port2) support",
 		[40] = "UDP RSS support",
 		[41] = "Unicast VEP steering support",
 		[42] = "Multicast VEP steering support",
-- 
1.9.3

RE: [PATCH net-next 06/10] net/mlx4_core: Fix struct mlx4_vhcr_cmd to make implicit padding explicit

[David Laight]

From: Amir Vadai
> From: Jack Morgenstein <jackm@dev.mellanox.co.il>
> 
> Struct mlx4_vhcr was implicitly padded by the gcc compiler.
> This commit makes that padding explicit, to prevent issues with
> changing compilers. Note that we added the padding dword (rather
> than simply packing the structure) in order to maintain compatibility
> with previous kernels.

It isn't a 'compiler' option, but depends on the architecture.

> Reported-by: Alexander Schmidt <alexs@linux.vnet.ibm.com>
> Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
> Signed-off-by: Amir Vadai <amirv@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlx4/mlx4.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
> index 096a81c..595e18a 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
> +++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
> @@ -196,13 +196,14 @@ struct mlx4_vhcr {
>  struct mlx4_vhcr_cmd {
>  	__be64 in_param;
>  	__be32 in_modifier;
> +	u32 reserved1;

Adding a pad here changes the alignment on most 32bit architectures (eg i386)
where 64bit integers are only aligned to 4 byte boundaries.
So you are not 'maintaining compatibility with previous kernels'.

>  	__be64 out_param;
>  	__be16 token;
>  	u16 reserved;
>  	u8 status;
>  	u8 flags;
>  	__be16 opcode;
> -};
> +} __packed;

Don't add '__packed' unless you expect the structure to be misaligned
in memory.
On systems that fault mis-aligned memory requests you've requested the
compiler generate code to read/write everything using byte sized memory
accesses and a lot of shifting and masking.

	David

Re: [PATCH net-next 06/10] net/mlx4_core: Fix struct mlx4_vhcr_cmd to make implicit padding explicit

[Jack Morgenstein]

On Tue, 27 Jan 2015 09:43:27 +0000
David Laight <David.Laight@ACULAB.COM> wrote:

> From: Amir Vadai
> > From: Jack Morgenstein <jackm@dev.mellanox.co.il>
> > 
> > Struct mlx4_vhcr was implicitly padded by the gcc compiler.
> > This commit makes that padding explicit, to prevent issues with
> > changing compilers. Note that we added the padding dword (rather
> > than simply packing the structure) in order to maintain
> > compatibility with previous kernels.
> 
> It isn't a 'compiler' option, but depends on the architecture.
> 
> > Reported-by: Alexander Schmidt <alexs@linux.vnet.ibm.com>
> > Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
> > Signed-off-by: Amir Vadai <amirv@mellanox.com>
> > ---
> >  drivers/net/ethernet/mellanox/mlx4/mlx4.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
> > b/drivers/net/ethernet/mellanox/mlx4/mlx4.h index 096a81c..595e18a
> > 100644 --- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
> > +++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
> > @@ -196,13 +196,14 @@ struct mlx4_vhcr {
> >  struct mlx4_vhcr_cmd {
> >  	__be64 in_param;
> >  	__be32 in_modifier;
> > +	u32 reserved1;
> 
> Adding a pad here changes the alignment on most 32bit architectures
> (eg i386) where 64bit integers are only aligned to 4 byte boundaries.
> So you are not 'maintaining compatibility with previous kernels'.

You are correct. However, this structure is used ONLY for communication
between a Host and a Guest on that host.  In the current code (before
this fix), it was not possible to run a 32-bit Guest over a 64-bit Host,
because of the implicit padding that occurred on the 64-bit Host
and did not occur on the 32-bit Guest.

With this fix, a 32-bit Guest (with the fix) will be able to run over a
64-bit Host (with or without the fix).

The padding dword thus serves to maintain 64-bit Host (old or
new) compatibility with 64-bit Guests (old or new), while allowing
64-bit Hosts to support new 32-bit Guests.

I'll fix the changelog for the next submission.

> 
> >  	__be64 out_param;
> >  	__be16 token;
> >  	u16 reserved;
> >  	u8 status;
> >  	u8 flags;
> >  	__be16 opcode;
> > -};
> > +} __packed;
> 
> Don't add '__packed' unless you expect the structure to be misaligned
> in memory.
> On systems that fault mis-aligned memory requests you've requested the
> compiler generate code to read/write everything using byte sized
> memory accesses and a lot of shifting and masking.

OK, I'll remove the "packed" in the next submission, since with the
padding dword 64-bit arch and 32-bit arch compilers should produce the
same offsets.

> 
> 	David
> 

Thanks for the review!

-Jack

RE: [PATCH net-next 06/10] net/mlx4_core: Fix struct mlx4_vhcr_cmd to make implicit padding explicit

[David Laight]

From: Jack Morgenstein
> On Tue, 27 Jan 2015 09:43:27 +0000
> David Laight <David.Laight@ACULAB.COM> wrote:
> 
> > From: Amir Vadai
> > > From: Jack Morgenstein <jackm@dev.mellanox.co.il>
> > >
> > > Struct mlx4_vhcr was implicitly padded by the gcc compiler.
> > > This commit makes that padding explicit, to prevent issues with
> > > changing compilers. Note that we added the padding dword (rather
> > > than simply packing the structure) in order to maintain
> > > compatibility with previous kernels.
> >
> > It isn't a 'compiler' option, but depends on the architecture.
> >
> > > Reported-by: Alexander Schmidt <alexs@linux.vnet.ibm.com>
> > > Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
> > > Signed-off-by: Amir Vadai <amirv@mellanox.com>
> > > ---
> > >  drivers/net/ethernet/mellanox/mlx4/mlx4.h | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
> > > b/drivers/net/ethernet/mellanox/mlx4/mlx4.h index 096a81c..595e18a
> > > 100644 --- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
> > > +++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
> > > @@ -196,13 +196,14 @@ struct mlx4_vhcr {
> > >  struct mlx4_vhcr_cmd {
> > >  	__be64 in_param;
> > >  	__be32 in_modifier;
> > > +	u32 reserved1;
> >
> > Adding a pad here changes the alignment on most 32bit architectures
> > (eg i386) where 64bit integers are only aligned to 4 byte boundaries.
> > So you are not 'maintaining compatibility with previous kernels'.
> 
> You are correct. However, this structure is used ONLY for communication
> between a Host and a Guest on that host.  In the current code (before
> this fix), it was not possible to run a 32-bit Guest over a 64-bit Host,
> because of the implicit padding that occurred on the 64-bit Host
> and did not occur on the 32-bit Guest.
> 
> With this fix, a 32-bit Guest (with the fix) will be able to run over a
> 64-bit Host (with or without the fix).

Haven't you broken communication between an old and new
32bit Guests and 32bit hosts (assuming they exist)?

	David

Re: [PATCH net-next 06/10] net/mlx4_core: Fix struct mlx4_vhcr_cmd to make implicit padding explicit

[Jack Morgenstein]

On Tue, 27 Jan 2015 13:38:28 +0000
David Laight <David.Laight@ACULAB.COM> wrote:

> Haven't you broken communication between an old and new
> 32bit Guests and 32bit hosts (assuming they exist)?

Yes, but I don't see any interest on the net in running 32-bit Hosts
with virtualization and SR IOV.  I do see that there is interest in
running 32-bit Guests over 64-bit Hosts (which configuration KVM
supports).

Since this change is *required* for supporting 32-bit Guests on 64-bit
Hosts, I don't see that we have much choice here.

As you say, an old 32-bit Host will not be compatible with a new 32-bit
guest, and vice-versa. However, I would assume that almost all Hosts
would have 64-bit architectures, because of the performance and
memory-space requirements for running SRIOV and virtualization.
Therefore, I don't see this as a stopper.

-Jack

Re: [PATCH net-next 06/10] net/mlx4_core: Fix struct mlx4_vhcr_cmd to make implicit padding explicit

[David Miller]

From: David Laight <David.Laight@ACULAB.COM>
Date: Tue, 27 Jan 2015 09:43:27 +0000

>>  	__be64 out_param;
>>  	__be16 token;
>>  	u16 reserved;
>>  	u8 status;
>>  	u8 flags;
>>  	__be16 opcode;
>> -};
>> +} __packed;
> 
> Don't add '__packed' unless you expect the structure to be misaligned
> in memory.
> On systems that fault mis-aligned memory requests you've requested the
> compiler generate code to read/write everything using byte sized memory
> accesses and a lot of shifting and masking.

Indeed, I'm really sick of seeing these packed structures being
created all over the place.

They are to be used in absolutely extreme cases where no other
solution is possible.

Re: [PATCH net-next 06/10] net/mlx4_core: Fix struct mlx4_vhcr_cmd to make implicit padding explicit

[Jack Morgenstein]

On Tue, 27 Jan 2015 11:13:34 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> Indeed, I'm really sick of seeing these packed structures being
> created all over the place.
> 
> They are to be used in absolutely extreme cases where no other
> solution is possible.

The V2 of this patch does not have "packed".

-Jack

Re: [PATCH net-next 06/10] net/mlx4_core: Fix struct mlx4_vhcr_cmd to make implicit padding explicit

[Jack Morgenstein]

>>On Tue, 27 Jan 2015 11:13:34 -0800 (PST)
>>David Miller <davem@davemloft.net> wrote:

>> Indeed, I'm really sick of seeing these packed structures being
>> created all over the place.
>> 
>> They are to be used in absolutely extreme cases where no other
>> solution is possible.

>The V2 of this patch does not have "packed".

I meant the **V1** version of the patch, does not have "packed".
 
-Jack