[PATCH rdma-next v1 0/6] Add DC transport support to mlx5

[PATCH rdma-next v1 0/6] Add DC transport support to mlx5

[Leon Romanovsky]

Changelog:
 v0 -> v1: Updated "net/mlx5: Add DCT command interface" patch to
declare variables in reversed Christmas tree order.

-----
>From Moni:

Following the discussion [1] on the mailing list regarding
"Custom/proprietary QP type support", the following patchset
implements the direct connected QP types functionality over posted RFC.

The dynamically connected (DC) transport service provides a datagram-like
model that allows a DC QP to target multiple remote processes in multiple
remote nodes.

As far as reachability is concerned, the DC model is somewhat similar to
the unreliable datagram (UD) model in the sense that each WR submitted to
the DC SQ carries the information that identifies the remote destination.

The new QP type - IB_QPT_DRIVER is introduced to represent all type of QPs
that are driver specific, like the proposed DC QP. Such minimal change
to the IB/core allows reuse of existing verbs functions to control those
new QPs with already existing functions.

[1] https://www.spinics.net/lists/linux-rdma/msg57598.html

The patches are available in the git repository at:
  git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git tags/rdma-next-2017-12-27

	Thanks
---------------------------------------

Moni Shoua (6):
  net/mlx5: Add DCT command interface
  net/mlx5: Enable DC transport
  IB/core: Introduce driver QP type
  IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP
  IB/mlx5: Add support for DC Initiator QP
  IB/mlx5: Add support for DC target QP

 drivers/infiniband/hw/mlx5/main.c              |  27 +-
 drivers/infiniband/hw/mlx5/mlx5_ib.h           |  11 +
 drivers/infiniband/hw/mlx5/qp.c                | 365 ++++++++++++++++++++++++-
 drivers/net/ethernet/mellanox/mlx5/core/eq.c   |   9 +-
 drivers/net/ethernet/mellanox/mlx5/core/main.c |   3 +
 drivers/net/ethernet/mellanox/mlx5/core/qp.c   | 124 ++++++++-
 include/linux/mlx5/device.h                    |   9 +
 include/linux/mlx5/driver.h                    |   8 +
 include/linux/mlx5/qp.h                        |  12 +
 include/rdma/ib_verbs.h                        |   1 +
 include/uapi/rdma/mlx5-abi.h                   |  12 +-
 11 files changed, 559 insertions(+), 22 deletions(-)

--
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next v1 1/6] net/mlx5: Add DCT command interface

[Leon Romanovsky]

From: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Add a missing command interface to work with a DCT. It includes: creating,
destroying and get events for.

Signed-off-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/eq.c |   9 +-
 drivers/net/ethernet/mellanox/mlx5/core/qp.c | 124 ++++++++++++++++++++++++---
 include/linux/mlx5/device.h                  |   9 ++
 include/linux/mlx5/driver.h                  |   8 ++
 include/linux/mlx5/qp.h                      |  12 +++
 5 files changed, 149 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eq.c b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
index 60771865c99c..7d3d503fa675 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eq.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
@@ -417,7 +417,11 @@ static irqreturn_t mlx5_eq_int(int irq, void *eq_ptr)
 			cqn = be32_to_cpu(eqe->data.comp.cqn) & 0xffffff;
 			mlx5_cq_completion(dev, cqn);
 			break;
-
+		case MLX5_EVENT_TYPE_DCT_DRAINED:
+			rsn = be32_to_cpu(eqe->data.dct.dctn) & 0xffffff;
+			rsn |= (MLX5_RES_DCT << MLX5_USER_INDEX_LEN);
+			mlx5_rsc_event(dev, rsn, eqe->type);
+			break;
 		case MLX5_EVENT_TYPE_PATH_MIG:
 		case MLX5_EVENT_TYPE_COMM_EST:
 		case MLX5_EVENT_TYPE_SQ_DRAINED:
@@ -715,6 +719,9 @@ int mlx5_start_eqs(struct mlx5_core_dev *dev)
 
 	if (MLX5_CAP_GEN(dev, fpga))
 		async_event_mask |= (1ull << MLX5_EVENT_TYPE_FPGA_ERROR);
+	if (MLX5_CAP_GEN_MAX(dev, dct))
+		async_event_mask |= (1ull << MLX5_EVENT_TYPE_DCT_DRAINED);
+
 
 	err = mlx5_create_map_eq(dev, &table->cmd_eq, MLX5_EQ_VEC_CMD,
 				 MLX5_NUM_CMD_EQE, 1ull << MLX5_EVENT_TYPE_CMD,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/qp.c b/drivers/net/ethernet/mellanox/mlx5/core/qp.c
index db9e665ab104..b907fef91ef3 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/qp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/qp.c
@@ -98,6 +98,11 @@ static u64 sq_allowed_event_types(void)
 	return BIT(MLX5_EVENT_TYPE_WQ_CATAS_ERROR);
 }
 
+static u64 dct_allowed_event_types(void)
+{
+	return BIT(MLX5_EVENT_TYPE_DCT_DRAINED);
+}
+
 static bool is_event_type_allowed(int rsc_type, int event_type)
 {
 	switch (rsc_type) {
@@ -107,6 +112,8 @@ static bool is_event_type_allowed(int rsc_type, int event_type)
 		return BIT(event_type) & rq_allowed_event_types();
 	case MLX5_EVENT_QUEUE_TYPE_SQ:
 		return BIT(event_type) & sq_allowed_event_types();
+	case MLX5_EVENT_QUEUE_TYPE_DCT:
+		return BIT(event_type) & dct_allowed_event_types();
 	default:
 		WARN(1, "Event arrived for unknown resource type");
 		return false;
@@ -116,6 +123,7 @@ static bool is_event_type_allowed(int rsc_type, int event_type)
 void mlx5_rsc_event(struct mlx5_core_dev *dev, u32 rsn, int event_type)
 {
 	struct mlx5_core_rsc_common *common = mlx5_get_rsc(dev, rsn);
+	struct mlx5_core_dct *dct;
 	struct mlx5_core_qp *qp;
 
 	if (!common)
@@ -134,7 +142,11 @@ void mlx5_rsc_event(struct mlx5_core_dev *dev, u32 rsn, int event_type)
 		qp = (struct mlx5_core_qp *)common;
 		qp->event(qp, event_type);
 		break;
-
+	case MLX5_RES_DCT:
+		dct = (struct mlx5_core_dct *)common;
+		if (event_type == MLX5_EVENT_TYPE_DCT_DRAINED)
+			complete(&dct->drained);
+		break;
 	default:
 		mlx5_core_warn(dev, "invalid resource type for 0x%x\n", rsn);
 	}
@@ -142,9 +154,9 @@ void mlx5_rsc_event(struct mlx5_core_dev *dev, u32 rsn, int event_type)
 	mlx5_core_put_rsc(common);
 }
 
-static int create_qprqsq_common(struct mlx5_core_dev *dev,
-				struct mlx5_core_qp *qp,
-				int rsc_type)
+static int create_resource_common(struct mlx5_core_dev *dev,
+				  struct mlx5_core_qp *qp,
+				  int rsc_type)
 {
 	struct mlx5_qp_table *table = &dev->priv.qp_table;
 	int err;
@@ -165,8 +177,8 @@ static int create_qprqsq_common(struct mlx5_core_dev *dev,
 	return 0;
 }
 
-static void destroy_qprqsq_common(struct mlx5_core_dev *dev,
-				  struct mlx5_core_qp *qp)
+static void destroy_resource_common(struct mlx5_core_dev *dev,
+				    struct mlx5_core_qp *qp)
 {
 	struct mlx5_qp_table *table = &dev->priv.qp_table;
 	unsigned long flags;
@@ -179,6 +191,39 @@ static void destroy_qprqsq_common(struct mlx5_core_dev *dev,
 	wait_for_completion(&qp->common.free);
 }
 
+int mlx5_core_create_dct(struct mlx5_core_dev *dev,
+			 struct mlx5_core_dct *dct,
+			 u32 *in, int inlen)
+{
+	u32 out[MLX5_ST_SZ_DW(create_dct_out)]   = {0};
+	u32 din[MLX5_ST_SZ_DW(destroy_dct_in)]   = {0};
+	u32 dout[MLX5_ST_SZ_DW(destroy_dct_out)] = {0};
+	struct mlx5_core_qp *qp = &dct->mqp;
+	int err;
+
+	init_completion(&dct->drained);
+	MLX5_SET(create_dct_in, in, opcode, MLX5_CMD_OP_CREATE_DCT);
+
+	err = mlx5_cmd_exec(dev, in, inlen, &out, sizeof(out));
+	if (err) {
+		mlx5_core_warn(dev, "create DCT failed, ret %d\n", err);
+		return err;
+	}
+
+	qp->qpn = MLX5_GET(create_dct_out, out, dctn);
+	err = create_resource_common(dev, qp, MLX5_RES_DCT);
+	if (err)
+		goto err_cmd;
+
+	return 0;
+err_cmd:
+	MLX5_SET(destroy_dct_in, din, opcode, MLX5_CMD_OP_DESTROY_DCT);
+	MLX5_SET(destroy_dct_in, din, dctn, qp->qpn);
+	mlx5_cmd_exec(dev, (void *)&in, sizeof(din), (void *)&out, sizeof(dout));
+	return err;
+}
+EXPORT_SYMBOL_GPL(mlx5_core_create_dct);
+
 int mlx5_core_create_qp(struct mlx5_core_dev *dev,
 			struct mlx5_core_qp *qp,
 			u32 *in, int inlen)
@@ -197,7 +242,7 @@ int mlx5_core_create_qp(struct mlx5_core_dev *dev,
 	qp->qpn = MLX5_GET(create_qp_out, out, qpn);
 	mlx5_core_dbg(dev, "qpn = 0x%x\n", qp->qpn);
 
-	err = create_qprqsq_common(dev, qp, MLX5_RES_QP);
+	err = create_resource_common(dev, qp, MLX5_RES_QP);
 	if (err)
 		goto err_cmd;
 
@@ -220,6 +265,47 @@ int mlx5_core_create_qp(struct mlx5_core_dev *dev,
 }
 EXPORT_SYMBOL_GPL(mlx5_core_create_qp);
 
+static int mlx5_core_drain_dct(struct mlx5_core_dev *dev,
+			       struct mlx5_core_dct *dct)
+{
+	u32 out[MLX5_ST_SZ_DW(drain_dct_out)] = {0};
+	u32 in[MLX5_ST_SZ_DW(drain_dct_in)]   = {0};
+	struct mlx5_core_qp *qp = &dct->mqp;
+
+	MLX5_SET(drain_dct_in, in, opcode, MLX5_CMD_OP_DRAIN_DCT);
+	MLX5_SET(drain_dct_in, in, dctn, qp->qpn);
+	return mlx5_cmd_exec(dev, (void *)&in, sizeof(in),
+			     (void *)&out, sizeof(out));
+}
+
+int mlx5_core_destroy_dct(struct mlx5_core_dev *dev,
+			  struct mlx5_core_dct *dct)
+{
+	u32 out[MLX5_ST_SZ_DW(destroy_dct_out)] = {0};
+	u32 in[MLX5_ST_SZ_DW(destroy_dct_in)]   = {0};
+	struct mlx5_core_qp *qp = &dct->mqp;
+	int err;
+
+	err = mlx5_core_drain_dct(dev, dct);
+	if (err) {
+		if (dev->state == MLX5_DEVICE_STATE_INTERNAL_ERROR) {
+			goto destroy;
+		} else {
+			mlx5_core_warn(dev, "failed drain DCT 0x%x with error 0x%x\n", qp->qpn, err);
+			return err;
+		}
+	}
+	wait_for_completion(&dct->drained);
+destroy:
+	destroy_resource_common(dev, &dct->mqp);
+	MLX5_SET(destroy_dct_in, in, opcode, MLX5_CMD_OP_DESTROY_DCT);
+	MLX5_SET(destroy_dct_in, in, dctn, qp->qpn);
+	err = mlx5_cmd_exec(dev, (void *)&in, sizeof(in),
+			    (void *)&out, sizeof(out));
+	return err;
+}
+EXPORT_SYMBOL_GPL(mlx5_core_destroy_dct);
+
 int mlx5_core_destroy_qp(struct mlx5_core_dev *dev,
 			 struct mlx5_core_qp *qp)
 {
@@ -229,7 +315,7 @@ int mlx5_core_destroy_qp(struct mlx5_core_dev *dev,
 
 	mlx5_debug_qp_remove(dev, qp);
 
-	destroy_qprqsq_common(dev, qp);
+	destroy_resource_common(dev, qp);
 
 	MLX5_SET(destroy_qp_in, in, opcode, MLX5_CMD_OP_DESTROY_QP);
 	MLX5_SET(destroy_qp_in, in, qpn, qp->qpn);
@@ -405,6 +491,20 @@ int mlx5_core_qp_query(struct mlx5_core_dev *dev, struct mlx5_core_qp *qp,
 }
 EXPORT_SYMBOL_GPL(mlx5_core_qp_query);
 
+int mlx5_core_dct_query(struct mlx5_core_dev *dev, struct mlx5_core_dct *dct,
+			u32 *out, int outlen)
+{
+	u32 in[MLX5_ST_SZ_DW(query_dct_in)] = {0};
+	struct mlx5_core_qp *qp = &dct->mqp;
+
+	MLX5_SET(query_dct_in, in, opcode, MLX5_CMD_OP_QUERY_DCT);
+	MLX5_SET(query_dct_in, in, dctn, qp->qpn);
+
+	return mlx5_cmd_exec(dev, (void *)&in, sizeof(in),
+			     (void *)out, outlen);
+}
+EXPORT_SYMBOL_GPL(mlx5_core_dct_query);
+
 int mlx5_core_xrcd_alloc(struct mlx5_core_dev *dev, u32 *xrcdn)
 {
 	u32 out[MLX5_ST_SZ_DW(alloc_xrcd_out)] = {0};
@@ -441,7 +541,7 @@ int mlx5_core_create_rq_tracked(struct mlx5_core_dev *dev, u32 *in, int inlen,
 		return err;
 
 	rq->qpn = rqn;
-	err = create_qprqsq_common(dev, rq, MLX5_RES_RQ);
+	err = create_resource_common(dev, rq, MLX5_RES_RQ);
 	if (err)
 		goto err_destroy_rq;
 
@@ -457,7 +557,7 @@ EXPORT_SYMBOL(mlx5_core_create_rq_tracked);
 void mlx5_core_destroy_rq_tracked(struct mlx5_core_dev *dev,
 				  struct mlx5_core_qp *rq)
 {
-	destroy_qprqsq_common(dev, rq);
+	destroy_resource_common(dev, rq);
 	mlx5_core_destroy_rq(dev, rq->qpn);
 }
 EXPORT_SYMBOL(mlx5_core_destroy_rq_tracked);
@@ -473,7 +573,7 @@ int mlx5_core_create_sq_tracked(struct mlx5_core_dev *dev, u32 *in, int inlen,
 		return err;
 
 	sq->qpn = sqn;
-	err = create_qprqsq_common(dev, sq, MLX5_RES_SQ);
+	err = create_resource_common(dev, sq, MLX5_RES_SQ);
 	if (err)
 		goto err_destroy_sq;
 
@@ -489,7 +589,7 @@ EXPORT_SYMBOL(mlx5_core_create_sq_tracked);
 void mlx5_core_destroy_sq_tracked(struct mlx5_core_dev *dev,
 				  struct mlx5_core_qp *sq)
 {
-	destroy_qprqsq_common(dev, sq);
+	destroy_resource_common(dev, sq);
 	mlx5_core_destroy_sq(dev, sq->qpn);
 }
 EXPORT_SYMBOL(mlx5_core_destroy_sq_tracked);
diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
index 2ff9f48ebec3..e5258ee4e38b 100644
--- a/include/linux/mlx5/device.h
+++ b/include/linux/mlx5/device.h
@@ -291,6 +291,7 @@ enum {
 	MLX5_EVENT_QUEUE_TYPE_QP = 0,
 	MLX5_EVENT_QUEUE_TYPE_RQ = 1,
 	MLX5_EVENT_QUEUE_TYPE_SQ = 2,
+	MLX5_EVENT_QUEUE_TYPE_DCT = 6,
 };
 
 enum mlx5_event {
@@ -326,6 +327,8 @@ enum mlx5_event {
 	MLX5_EVENT_TYPE_PAGE_FAULT	   = 0xc,
 	MLX5_EVENT_TYPE_NIC_VPORT_CHANGE   = 0xd,
 
+	MLX5_EVENT_TYPE_DCT_DRAINED        = 0x1c,
+
 	MLX5_EVENT_TYPE_FPGA_ERROR         = 0x20,
 };
 
@@ -618,6 +621,11 @@ struct mlx5_eqe_pps {
 	u8		rsvd2[12];
 } __packed;
 
+struct mlx5_eqe_dct {
+	__be32  reserved[6];
+	__be32  dctn;
+};
+
 union ev_data {
 	__be32				raw[7];
 	struct mlx5_eqe_cmd		cmd;
@@ -633,6 +641,7 @@ union ev_data {
 	struct mlx5_eqe_vport_change	vport_change;
 	struct mlx5_eqe_port_module	port_module;
 	struct mlx5_eqe_pps		pps;
+	struct mlx5_eqe_dct             dct;
 } __packed;
 
 struct mlx5_eqe {
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index 4d1ed35fdbb4..b5eb5c55dcaf 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -154,6 +154,13 @@ enum mlx5_dcbx_oper_mode {
 	MLX5E_DCBX_PARAM_VER_OPER_AUTO  = 0x3,
 };
 
+enum mlx5_dct_atomic_mode {
+	MLX5_ATOMIC_MODE_DCT_OFF        = 20,
+	MLX5_ATOMIC_MODE_DCT_NONE       = 0 << MLX5_ATOMIC_MODE_DCT_OFF,
+	MLX5_ATOMIC_MODE_DCT_IB_COMP    = 1 << MLX5_ATOMIC_MODE_DCT_OFF,
+	MLX5_ATOMIC_MODE_DCT_CX         = 2 << MLX5_ATOMIC_MODE_DCT_OFF,
+};
+
 enum {
 	MLX5_ATOMIC_OPS_CMP_SWAP	= 1 << 0,
 	MLX5_ATOMIC_OPS_FETCH_ADD	= 1 << 1,
@@ -432,6 +439,7 @@ enum mlx5_res_type {
 	MLX5_RES_SRQ	= 3,
 	MLX5_RES_XSRQ	= 4,
 	MLX5_RES_XRQ	= 5,
+	MLX5_RES_DCT	= MLX5_EVENT_QUEUE_TYPE_DCT,
 };
 
 struct mlx5_core_rsc_common {
diff --git a/include/linux/mlx5/qp.h b/include/linux/mlx5/qp.h
index 62af7512dabb..4778d41085d4 100644
--- a/include/linux/mlx5/qp.h
+++ b/include/linux/mlx5/qp.h
@@ -473,6 +473,11 @@ struct mlx5_core_qp {
 	int			pid;
 };
 
+struct mlx5_core_dct {
+	struct mlx5_core_qp	mqp;
+	struct completion	drained;
+};
+
 struct mlx5_qp_path {
 	u8			fl_free_ar;
 	u8			rsvd3;
@@ -549,6 +554,9 @@ static inline struct mlx5_core_mkey *__mlx5_mr_lookup(struct mlx5_core_dev *dev,
 	return radix_tree_lookup(&dev->priv.mkey_table.tree, key);
 }
 
+int mlx5_core_create_dct(struct mlx5_core_dev *dev,
+			 struct mlx5_core_dct *qp,
+			 u32 *in, int inlen);
 int mlx5_core_create_qp(struct mlx5_core_dev *dev,
 			struct mlx5_core_qp *qp,
 			u32 *in,
@@ -558,8 +566,12 @@ int mlx5_core_qp_modify(struct mlx5_core_dev *dev, u16 opcode,
 			struct mlx5_core_qp *qp);
 int mlx5_core_destroy_qp(struct mlx5_core_dev *dev,
 			 struct mlx5_core_qp *qp);
+int mlx5_core_destroy_dct(struct mlx5_core_dev *dev,
+			  struct mlx5_core_dct *dct);
 int mlx5_core_qp_query(struct mlx5_core_dev *dev, struct mlx5_core_qp *qp,
 		       u32 *out, int outlen);
+int mlx5_core_dct_query(struct mlx5_core_dev *dev, struct mlx5_core_dct *dct,
+			u32 *out, int outlen);
 
 int mlx5_core_set_delay_drop(struct mlx5_core_dev *dev,
 			     u32 timeout_usec);
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next v1 2/6] net/mlx5: Enable DC transport

[Leon Romanovsky]

From: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Enable DC transport in the firmware to enable using its functionality.

Signed-off-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 77c9f7e42a99..75774f0fb579 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -560,6 +560,9 @@ static int handle_hca_cap(struct mlx5_core_dev *dev)
 			 num_vhca_ports,
 			 MLX5_CAP_GEN_MAX(dev, num_vhca_ports));
 
+	if (MLX5_CAP_GEN_MAX(dev, dct))
+		MLX5_SET(cmd_hca_cap, set_hca_cap, dct, 1);
+
 	err = set_caps(dev, set_ctx, set_sz,
 		       MLX5_SET_HCA_CAP_OP_MOD_GENERAL_DEVICE);
 
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next v1 3/6] IB/core: Introduce driver QP type

[Leon Romanovsky]

From: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Vendors can implement type of QPs that are not described in the
InfiniBand specification. To still be able to use the IB/core layer
services (e.g. user object management) without tainting this layer with
driver proprietary logic, a new QP type is added - IB_QPT_DRIVER. This
will be a general QP type that the core layer doesn't know about its true nature.
When a command like create_qp() is passed to a hardware driver the extra
data that is required is taken from the driver channel.
Downstream patches from this series will use that QP type in the mlx5
driver.

Signed-off-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/rdma/ib_verbs.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 64032f4c8d5a..41d4f59cac5e 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -1083,6 +1083,7 @@ enum ib_qp_type {
 	IB_QPT_XRC_INI = 9,
 	IB_QPT_XRC_TGT,
 	IB_QPT_MAX,
+	IB_QPT_DRIVER = 0xFF,
 	/* Reserve a range for qp types internal to the low level driver.
 	 * These qp types will not be visible at the IB core layer, so the
 	 * IB_QPT_MAX usages should not be affected in the core layer
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next v1 4/6] IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP

[Leon Romanovsky]

From: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The QP type IB_QPT_DRIVER doesn't describe the transport or the service that
the QP provides but those are known only to the hardware driver.
The extra data that tells more about the QP is in the driver
channel.
Take the real QP type from the driver channel and modify the QP initial
attributes before continuing with create_qp(). Also, define the layout
of the data structure that comes in the driver channel.
Downstream patches from this series will add support for both DCI and
DCT driver QPs.

Signed-off-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/mlx5_ib.h |  3 ++
 drivers/infiniband/hw/mlx5/qp.c      | 57 +++++++++++++++++++++++++++++++++++-
 include/uapi/rdma/mlx5-abi.h         |  7 ++++-
 3 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 88b89ed15d1c..66e9c2190f2d 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -202,6 +202,8 @@ struct mlx5_ib_flow_db {
  * creates the actual hardware QP.
  */
 #define MLX5_IB_QPT_HW_GSI	IB_QPT_RESERVED2
+#define MLX5_IB_QPT_DCI		IB_QPT_RESERVED3
+#define MLX5_IB_QPT_DCT		IB_QPT_RESERVED4
 #define MLX5_IB_WR_UMR		IB_WR_RESERVED1
 
 #define MLX5_IB_UMR_OCTOWORD	       16
@@ -406,6 +408,7 @@ struct mlx5_ib_qp {
 	u32			rate_limit;
 	u32                     underlay_qpn;
 	bool			tunnel_offload_en;
+	u32			driver_qp_type;
 };
 
 struct mlx5_ib_cq_buf {
diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index 4b0420e67870..f1a2b1ade804 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -2115,20 +2115,62 @@ static const char *ib_qp_type_str(enum ib_qp_type type)
 		return "IB_QPT_RAW_PACKET";
 	case MLX5_IB_QPT_REG_UMR:
 		return "MLX5_IB_QPT_REG_UMR";
+	case IB_QPT_DRIVER:
+		return "IB_QPT_DRIVER";
 	case IB_QPT_MAX:
 	default:
 		return "Invalid QP type";
 	}
 }
 
+static int validate_driver_qp(struct mlx5_ib_dev *dev,
+			      struct ib_qp_init_attr *init_attr,
+			      struct mlx5_ib_create_qp *ucmd,
+			      struct ib_udata *udata)
+{
+	int err;
+	bool dc_supp = MLX5_CAP_GEN(dev->mdev, dct);
+
+	if (!udata)
+		return -EINVAL;
+
+	if (udata->inlen < sizeof(*ucmd)) {
+		mlx5_ib_dbg(dev, "create_qp user command is smaller than expected\n");
+		return -EINVAL;
+	}
+	err = ib_copy_from_udata(ucmd, udata, sizeof(*ucmd));
+	if (err)
+		return err;
+
+	if ((ucmd->flags & MLX5_QP_FLAG_TYPE_DCT) && (ucmd->flags & MLX5_QP_FLAG_TYPE_DCI)) {
+		mlx5_ib_dbg(dev, "Only one DC QP type should be specified\n");
+		return -EINVAL;
+	}
+	if (!((ucmd->flags & MLX5_QP_FLAG_TYPE_DCT) || (ucmd->flags & MLX5_QP_FLAG_TYPE_DCI))) {
+		mlx5_ib_dbg(dev, "DC QP type is not specified\n");
+		return -EINVAL;
+	}
+	if (!dc_supp) {
+		mlx5_ib_dbg(dev, "DC transport is not supported\n");
+		return -EOPNOTSUPP;
+	}
+	if (ucmd->flags & MLX5_QP_FLAG_TYPE_DCI)
+		init_attr->qp_type = MLX5_IB_QPT_DCI;
+	else
+		init_attr->qp_type = MLX5_IB_QPT_DCT;
+	return 0;
+}
+
 struct ib_qp *mlx5_ib_create_qp(struct ib_pd *pd,
-				struct ib_qp_init_attr *init_attr,
+				struct ib_qp_init_attr *verbs_init_attr,
 				struct ib_udata *udata)
 {
 	struct mlx5_ib_dev *dev;
 	struct mlx5_ib_qp *qp;
 	u16 xrcdn = 0;
 	int err;
+	struct ib_qp_init_attr mlx_init_attr;
+	struct ib_qp_init_attr *init_attr = verbs_init_attr;
 
 	if (pd) {
 		dev = to_mdev(pd->device);
@@ -2153,6 +2195,16 @@ struct ib_qp *mlx5_ib_create_qp(struct ib_pd *pd,
 		dev = to_mdev(to_mxrcd(init_attr->xrcd)->ibxrcd.device);
 	}
 
+	if (init_attr->qp_type == IB_QPT_DRIVER) {
+		struct mlx5_ib_create_qp ucmd;
+
+		init_attr = &mlx_init_attr;
+		memcpy(init_attr, verbs_init_attr, sizeof(*verbs_init_attr));
+		err = validate_driver_qp(dev, init_attr, &ucmd, udata);
+		if (err)
+			return ERR_PTR(err);
+	}
+
 	switch (init_attr->qp_type) {
 	case IB_QPT_XRC_TGT:
 	case IB_QPT_XRC_INI:
@@ -2214,6 +2266,9 @@ struct ib_qp *mlx5_ib_create_qp(struct ib_pd *pd,
 		return ERR_PTR(-EINVAL);
 	}
 
+	if (verbs_init_attr->qp_type == IB_QPT_DRIVER)
+		qp->driver_qp_type = init_attr->qp_type;
+
 	return &qp->ibqp;
 }
 
diff --git a/include/uapi/rdma/mlx5-abi.h b/include/uapi/rdma/mlx5-abi.h
index 0f7e45680ce5..83bde975d3f9 100644
--- a/include/uapi/rdma/mlx5-abi.h
+++ b/include/uapi/rdma/mlx5-abi.h
@@ -42,6 +42,8 @@ enum {
 	MLX5_QP_FLAG_SCATTER_CQE	= 1 << 1,
 	MLX5_QP_FLAG_TUNNEL_OFFLOADS	= 1 << 2,
 	MLX5_QP_FLAG_BFREG_INDEX	= 1 << 3,
+	MLX5_QP_FLAG_TYPE_DCT		= 1 << 4,
+	MLX5_QP_FLAG_TYPE_DCI		= 1 << 5,
 };
 
 enum {
@@ -284,7 +286,10 @@ struct mlx5_ib_create_qp {
 	__u32	flags;
 	__u32	uidx;
 	__u32	bfreg_index;
-	__u64	sq_buf_addr;
+	union {
+		__u64	sq_buf_addr;
+		__u64	access_key;
+	};
 };
 
 /* RX Hash function flags */
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next v1 5/6] IB/mlx5: Add support for DC Initiator QP

[Leon Romanovsky]

From: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

DC Initiator (DCI) QP is represented like any other QP in the hardware.
However, like any other transport QP there are attributes and settings
that are special to DCI QP and needs specific attention and care.
Make necessary changes to configure DCI QP.

Signed-off-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/qp.c | 77 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 72 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index f1a2b1ade804..87613c8edac1 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -613,6 +613,7 @@ static int to_mlx5_st(enum ib_qp_type type)
 	case IB_QPT_XRC_TGT:		return MLX5_QP_ST_XRC;
 	case IB_QPT_SMI:		return MLX5_QP_ST_QP0;
 	case MLX5_IB_QPT_HW_GSI:	return MLX5_QP_ST_QP1;
+	case MLX5_IB_QPT_DCI:		return MLX5_QP_ST_DCI;
 	case IB_QPT_RAW_IPV6:		return MLX5_QP_ST_RAW_IPV6;
 	case IB_QPT_RAW_PACKET:
 	case IB_QPT_RAW_ETHERTYPE:	return MLX5_QP_ST_RAW_ETHERTYPE;
@@ -1044,6 +1045,7 @@ static void destroy_qp_kernel(struct mlx5_ib_dev *dev, struct mlx5_ib_qp *qp)
 static u32 get_rx_type(struct mlx5_ib_qp *qp, struct ib_qp_init_attr *attr)
 {
 	if (attr->srq || (attr->qp_type == IB_QPT_XRC_TGT) ||
+	    (attr->qp_type == MLX5_IB_QPT_DCI) ||
 	    (attr->qp_type == IB_QPT_XRC_INI))
 		return MLX5_SRQ_RQ;
 	else if (!qp->has_rq)
@@ -2203,6 +2205,14 @@ struct ib_qp *mlx5_ib_create_qp(struct ib_pd *pd,
 		err = validate_driver_qp(dev, init_attr, &ucmd, udata);
 		if (err)
 			return ERR_PTR(err);
+
+		if (init_attr->qp_type == MLX5_IB_QPT_DCI) {
+			if (init_attr->cap.max_recv_wr ||
+			    init_attr->cap.max_recv_sge) {
+				mlx5_ib_dbg(dev, "DCI QP requires zero size receive queue\n");
+				return ERR_PTR(-EINVAL);
+			}
+		}
 	}
 
 	switch (init_attr->qp_type) {
@@ -2226,6 +2236,7 @@ struct ib_qp *mlx5_ib_create_qp(struct ib_pd *pd,
 	case IB_QPT_SMI:
 	case MLX5_IB_QPT_HW_GSI:
 	case MLX5_IB_QPT_REG_UMR:
+	case MLX5_IB_QPT_DCI:
 		qp = kzalloc(sizeof(*qp), GFP_KERNEL);
 		if (!qp)
 			return ERR_PTR(-ENOMEM);
@@ -2847,7 +2858,8 @@ static int __mlx5_ib_modify_qp(struct ib_qp *ibqp,
 	if (!context)
 		return -ENOMEM;
 
-	err = to_mlx5_st(ibqp->qp_type);
+	err = to_mlx5_st(ibqp->qp_type == IB_QPT_DRIVER ?
+			 qp->driver_qp_type : ibqp->qp_type);
 	if (err < 0) {
 		mlx5_ib_dbg(dev, "unsupported qp type %d\n", ibqp->qp_type);
 		goto out;
@@ -3007,7 +3019,8 @@ static int __mlx5_ib_modify_qp(struct ib_qp *ibqp,
 
 	mlx5_cur = to_mlx5_state(cur_state);
 	mlx5_new = to_mlx5_state(new_state);
-	mlx5_st = to_mlx5_st(ibqp->qp_type);
+	mlx5_st = to_mlx5_st(ibqp->qp_type == IB_QPT_DRIVER ?
+			     qp->driver_qp_type : ibqp->qp_type);
 	if (mlx5_st < 0)
 		goto out;
 
@@ -3079,6 +3092,50 @@ static int __mlx5_ib_modify_qp(struct ib_qp *ibqp,
 	return err;
 }
 
+static inline bool is_valid_mask(int mask, int req, int opt)
+{
+	if ((mask & req) != req)
+		return false;
+
+	if (mask & ~(req | opt))
+		return false;
+
+	return true;
+}
+
+/* check valid transition for driver QP types
+ * for now the only QP type that this function supports is DCI
+ */
+static bool mlx5_ib_modify_qp_is_ok(enum ib_qp_state cur_state, enum ib_qp_state new_state,
+				    enum ib_qp_attr_mask attr_mask)
+{
+	int req = IB_QP_STATE;
+	int opt = 0;
+
+	if (cur_state == IB_QPS_RESET && new_state == IB_QPS_INIT) {
+		req |= IB_QP_PKEY_INDEX | IB_QP_PORT;
+		return is_valid_mask(attr_mask, req, opt);
+	} else if (cur_state == IB_QPS_INIT && new_state == IB_QPS_INIT) {
+		opt = IB_QP_PKEY_INDEX | IB_QP_PORT;
+		return is_valid_mask(attr_mask, req, opt);
+	} else if (cur_state == IB_QPS_INIT && new_state == IB_QPS_RTR) {
+		req |= IB_QP_PATH_MTU;
+		opt = IB_QP_PKEY_INDEX;
+		return is_valid_mask(attr_mask, req, opt);
+	} else if (cur_state == IB_QPS_RTR && new_state == IB_QPS_RTS) {
+		req |= IB_QP_TIMEOUT | IB_QP_RETRY_CNT | IB_QP_RNR_RETRY |
+		       IB_QP_MAX_QP_RD_ATOMIC | IB_QP_SQ_PSN;
+		opt = IB_QP_MIN_RNR_TIMER;
+		return is_valid_mask(attr_mask, req, opt);
+	} else if (cur_state == IB_QPS_RTS && new_state == IB_QPS_RTS) {
+		opt = IB_QP_MIN_RNR_TIMER;
+		return is_valid_mask(attr_mask, req, opt);
+	} else if (cur_state != IB_QPS_RESET && new_state == IB_QPS_ERR) {
+		return is_valid_mask(attr_mask, req, opt);
+	}
+	return false;
+}
+
 int mlx5_ib_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
 		      int attr_mask, struct ib_udata *udata)
 {
@@ -3096,8 +3153,12 @@ int mlx5_ib_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
 	if (unlikely(ibqp->qp_type == IB_QPT_GSI))
 		return mlx5_ib_gsi_modify_qp(ibqp, attr, attr_mask);
 
-	qp_type = (unlikely(ibqp->qp_type == MLX5_IB_QPT_HW_GSI)) ?
-		IB_QPT_GSI : ibqp->qp_type;
+	if (ibqp->qp_type == IB_QPT_DRIVER)
+		qp_type = qp->driver_qp_type;
+	else
+		qp_type = (unlikely(ibqp->qp_type == MLX5_IB_QPT_HW_GSI)) ?
+			IB_QPT_GSI : ibqp->qp_type;
+
 
 	mutex_lock(&qp->mutex);
 
@@ -3116,10 +3177,16 @@ int mlx5_ib_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
 			goto out;
 		}
 	} else if (qp_type != MLX5_IB_QPT_REG_UMR &&
-	    !ib_modify_qp_is_ok(cur_state, new_state, qp_type, attr_mask, ll)) {
+		   qp_type != MLX5_IB_QPT_DCI &&
+		   !ib_modify_qp_is_ok(cur_state, new_state, qp_type, attr_mask, ll)) {
 		mlx5_ib_dbg(dev, "invalid QP state transition from %d to %d, qp_type %d, attr_mask 0x%x\n",
 			    cur_state, new_state, ibqp->qp_type, attr_mask);
 		goto out;
+	} else if (qp_type == MLX5_IB_QPT_DCI &&
+		   !mlx5_ib_modify_qp_is_ok(cur_state, new_state, attr_mask)) {
+		mlx5_ib_dbg(dev, "invalid QP state transition from %d to %d, qp_type %d, attr_mask 0x%x\n",
+			    cur_state, new_state, qp_type, attr_mask);
+		goto out;
 	}
 
 	if ((attr_mask & IB_QP_PORT) &&
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next v1 6/6] IB/mlx5: Add support for DC target QP

[Leon Romanovsky]

From: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

A DC Target (DCT) QP is represented in the hardware as a unique object.
This object is created by CREATE_DCT command and destroyed by
DESTROY_DCT command. However, in the driver we describe it as a QP.
The hardware command that creates a DCT needs parameters that
the verb create_qp() does not provide. Those remaining parameters are
provided with the call to the verb modify_qp(). Therefore we delay the
actual creation of a DCT in the hardware until the stage of
modify_qp() to RTR.
A support for query_qp() was added as well. It uses QUERY_DCT command to
retrieve the applicable fields.

Signed-off-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/main.c    |  27 +++-
 drivers/infiniband/hw/mlx5/mlx5_ib.h |   8 ++
 drivers/infiniband/hw/mlx5/qp.c      | 231 +++++++++++++++++++++++++++++++++++
 include/uapi/rdma/mlx5-abi.h         |   5 +
 4 files changed, 269 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 9c737cc65503..5d596bff798b 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -561,11 +561,11 @@ static int mlx5_get_vport_access_method(struct ib_device *ibdev)
 }
 
 static void get_atomic_caps(struct mlx5_ib_dev *dev,
+			    u8 atomic_size_qp,
 			    struct ib_device_attr *props)
 {
 	u8 tmp;
 	u8 atomic_operations = MLX5_CAP_ATOMIC(dev->mdev, atomic_operations);
-	u8 atomic_size_qp = MLX5_CAP_ATOMIC(dev->mdev, atomic_size_qp);
 	u8 atomic_req_8B_endianness_mode =
 		MLX5_CAP_ATOMIC(dev->mdev, atomic_req_8B_endianness_mode);
 
@@ -582,6 +582,29 @@ static void get_atomic_caps(struct mlx5_ib_dev *dev,
 	}
 }
 
+static void get_atomic_caps_qp(struct mlx5_ib_dev *dev,
+			       struct ib_device_attr *props)
+{
+	u8 atomic_size_qp = MLX5_CAP_ATOMIC(dev->mdev, atomic_size_qp);
+
+	get_atomic_caps(dev, atomic_size_qp, props);
+}
+
+static void get_atomic_caps_dc(struct mlx5_ib_dev *dev,
+			       struct ib_device_attr *props)
+{
+	u8 atomic_size_qp = MLX5_CAP_ATOMIC(dev->mdev, atomic_size_dc);
+
+	get_atomic_caps(dev, atomic_size_qp, props);
+}
+
+bool mlx5_ib_dc_atomic_is_supported(struct mlx5_ib_dev *dev)
+{
+	struct ib_device_attr props = {};
+
+	get_atomic_caps_dc(dev, &props);
+	return (props.atomic_cap == IB_ATOMIC_HCA) ? true : false;
+}
 static int mlx5_query_system_image_guid(struct ib_device *ibdev,
 					__be64 *sys_image_guid)
 {
@@ -873,7 +896,7 @@ static int mlx5_ib_query_device(struct ib_device *ibdev,
 	props->max_srq_sge	   = max_rq_sg - 1;
 	props->max_fast_reg_page_list_len =
 		1 << MLX5_CAP_GEN(mdev, log_max_klm_list_size);
-	get_atomic_caps(dev, props);
+	get_atomic_caps_qp(dev, props);
 	props->masked_atomic_cap   = IB_ATOMIC_NONE;
 	props->max_mcast_grp	   = 1 << MLX5_CAP_GEN(mdev, log_max_mcg);
 	props->max_mcast_qp_attach = MLX5_CAP_GEN(mdev, max_qp_mcg);
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 66e9c2190f2d..81de3c1f5e97 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -364,12 +364,18 @@ struct mlx5_bf {
 	struct mlx5_sq_bfreg   *bfreg;
 };
 
+struct mlx5_ib_dct {
+	struct mlx5_core_dct	mdct;
+	u32			*in;
+};
+
 struct mlx5_ib_qp {
 	struct ib_qp		ibqp;
 	union {
 		struct mlx5_ib_qp_trans trans_qp;
 		struct mlx5_ib_raw_packet_qp raw_packet_qp;
 		struct mlx5_ib_rss_qp rss_qp;
+		struct mlx5_ib_dct dct;
 	};
 	struct mlx5_buf		buf;
 
@@ -1022,6 +1028,8 @@ struct ib_rwq_ind_table *mlx5_ib_create_rwq_ind_table(struct ib_device *device,
 						      struct ib_rwq_ind_table_init_attr *init_attr,
 						      struct ib_udata *udata);
 int mlx5_ib_destroy_rwq_ind_table(struct ib_rwq_ind_table *wq_ind_table);
+bool mlx5_ib_dc_atomic_is_supported(struct mlx5_ib_dev *dev);
+
 
 #ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
 void mlx5_ib_internal_fill_odp_caps(struct mlx5_ib_dev *dev);
diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index 87613c8edac1..e43825ac09e3 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -2125,6 +2125,52 @@ static const char *ib_qp_type_str(enum ib_qp_type type)
 	}
 }
 
+static struct ib_qp *mlx5_ib_create_dct(struct ib_pd *pd,
+					struct ib_qp_init_attr *attr,
+					struct mlx5_ib_create_qp *ucmd)
+{
+	struct mlx5_ib_dev *dev;
+	struct mlx5_ib_qp *qp;
+	int err = 0;
+	u32 uidx = MLX5_IB_DEFAULT_UIDX;
+	void *dctc;
+
+	if (!attr->srq || !attr->recv_cq)
+		return ERR_PTR(-EINVAL);
+
+	dev = to_mdev(pd->device);
+
+	err = get_qp_user_index(to_mucontext(pd->uobject->context),
+				ucmd, sizeof(*ucmd), &uidx);
+	if (err)
+		return ERR_PTR(err);
+
+	qp = kzalloc(sizeof(*qp), GFP_KERNEL);
+	if (!qp)
+		return ERR_PTR(-ENOMEM);
+
+	qp->dct.in = kzalloc(MLX5_ST_SZ_BYTES(create_dct_in), GFP_KERNEL);
+	if (!qp->dct.in) {
+		err = -ENOMEM;
+		goto err_free;
+	}
+
+	dctc = MLX5_ADDR_OF(create_dct_in, qp->dct.in, dct_context_entry);
+	qp->driver_qp_type = MLX5_IB_QPT_DCT;
+	MLX5_SET(dctc, dctc, pd, to_mpd(pd)->pdn);
+	MLX5_SET(dctc, dctc, srqn_xrqn, to_msrq(attr->srq)->msrq.srqn);
+	MLX5_SET(dctc, dctc, cqn, to_mcq(attr->recv_cq)->mcq.cqn);
+	MLX5_SET64(dctc, dctc, dc_access_key, ucmd->access_key);
+	MLX5_SET(dctc, dctc, user_index, uidx);
+
+	qp->state = IB_QPS_RESET;
+
+	return &qp->ibqp;
+err_free:
+	kfree(qp);
+	return ERR_PTR(err);
+}
+
 static int validate_driver_qp(struct mlx5_ib_dev *dev,
 			      struct ib_qp_init_attr *init_attr,
 			      struct mlx5_ib_create_qp *ucmd,
@@ -2212,6 +2258,8 @@ struct ib_qp *mlx5_ib_create_qp(struct ib_pd *pd,
 				mlx5_ib_dbg(dev, "DCI QP requires zero size receive queue\n");
 				return ERR_PTR(-EINVAL);
 			}
+		} else {
+			return mlx5_ib_create_dct(pd, init_attr, &ucmd);
 		}
 	}
 
@@ -2283,6 +2331,25 @@ struct ib_qp *mlx5_ib_create_qp(struct ib_pd *pd,
 	return &qp->ibqp;
 }
 
+static int mlx5_ib_destroy_dct(struct mlx5_ib_qp *mqp)
+{
+	struct mlx5_ib_dev *dev = to_mdev(mqp->ibqp.device);
+
+	if (mqp->state == IB_QPS_RTR) {
+		int err;
+
+		err = mlx5_core_destroy_dct(dev->mdev, &mqp->dct.mdct);
+		if (err) {
+			mlx5_ib_warn(dev, "failed to destroy DCT %d\n", err);
+			return err;
+		}
+	}
+
+	kfree(mqp->dct.in);
+	kfree(mqp);
+	return 0;
+}
+
 int mlx5_ib_destroy_qp(struct ib_qp *qp)
 {
 	struct mlx5_ib_dev *dev = to_mdev(qp->device);
@@ -2291,6 +2358,9 @@ int mlx5_ib_destroy_qp(struct ib_qp *qp)
 	if (unlikely(qp->qp_type == IB_QPT_GSI))
 		return mlx5_ib_gsi_destroy_qp(qp);
 
+	if (mqp->driver_qp_type == MLX5_IB_QPT_DCT)
+		return mlx5_ib_destroy_dct(mqp);
+
 	destroy_qp_common(dev, mqp);
 
 	kfree(mqp);
@@ -3136,6 +3206,95 @@ static bool mlx5_ib_modify_qp_is_ok(enum ib_qp_state cur_state, enum ib_qp_state
 	return false;
 }
 
+/* mlx5_ib_modify_dct: modify a DCT QP
+ * valid transitions are:
+ * RESET to INIT: must set access_flags, pkey_index and port
+ * INIT  to RTR : must set min_rnr_timer, tclass, flow_label,
+ *			   mtu, gid_index and hop_limit
+ * Other transitions and attributes are illegal
+ */
+static int mlx5_ib_modify_dct(struct ib_qp *ibqp, struct ib_qp_attr *attr,
+			      int attr_mask, struct ib_udata *udata)
+{
+	struct mlx5_ib_qp *qp = to_mqp(ibqp);
+	struct mlx5_ib_dev *dev = to_mdev(ibqp->device);
+	enum ib_qp_state cur_state, new_state;
+	int err = 0;
+	int required = IB_QP_STATE;
+	void *dctc;
+
+	if (!(attr_mask & IB_QP_STATE))
+		return -EINVAL;
+
+	cur_state = qp->state;
+	new_state = attr->qp_state;
+
+	dctc = MLX5_ADDR_OF(create_dct_in, qp->dct.in, dct_context_entry);
+	if (cur_state == IB_QPS_RESET && new_state == IB_QPS_INIT) {
+		required |= IB_QP_ACCESS_FLAGS | IB_QP_PKEY_INDEX | IB_QP_PORT;
+		if (!is_valid_mask(attr_mask, required, 0))
+			return -EINVAL;
+
+		if (attr->port_num == 0 ||
+		    attr->port_num > MLX5_CAP_GEN(dev->mdev, num_ports)) {
+			mlx5_ib_dbg(dev, "invalid port number %d. number of ports is %d\n",
+				    attr->port_num, dev->num_ports);
+			return -EINVAL;
+		}
+		if (attr->qp_access_flags & IB_ACCESS_REMOTE_READ)
+			MLX5_SET(dctc, dctc, rre, 1);
+		if (attr->qp_access_flags & IB_ACCESS_REMOTE_WRITE)
+			MLX5_SET(dctc, dctc, rwe, 1);
+		if (attr->qp_access_flags & IB_ACCESS_REMOTE_ATOMIC) {
+			if (!mlx5_ib_dc_atomic_is_supported(dev))
+				return -EOPNOTSUPP;
+			MLX5_SET(dctc, dctc, rae, 1);
+			MLX5_SET(dctc, dctc, atomic_mode, MLX5_ATOMIC_MODE_DCT_CX);
+		}
+		MLX5_SET(dctc, dctc, pkey_index, attr->pkey_index);
+		MLX5_SET(dctc, dctc, port, attr->port_num);
+		MLX5_SET(dctc, dctc, counter_set_id, dev->port[attr->port_num - 1].cnts.set_id);
+
+	} else if (cur_state == IB_QPS_INIT && new_state == IB_QPS_RTR) {
+		struct mlx5_ib_modify_qp_resp resp = {};
+		u32 min_resp_len = offsetof(typeof(resp), dctn) +
+				   sizeof(resp.dctn);
+
+		if (udata->outlen < min_resp_len)
+			return -EINVAL;
+		resp.response_length = min_resp_len;
+
+		required |= IB_QP_MIN_RNR_TIMER | IB_QP_AV | IB_QP_PATH_MTU;
+		if (!is_valid_mask(attr_mask, required, 0))
+			return -EINVAL;
+		MLX5_SET(dctc, dctc, min_rnr_nak, attr->min_rnr_timer);
+		MLX5_SET(dctc, dctc, tclass, attr->ah_attr.grh.traffic_class);
+		MLX5_SET(dctc, dctc, flow_label, attr->ah_attr.grh.flow_label);
+		MLX5_SET(dctc, dctc, mtu, attr->path_mtu);
+		MLX5_SET(dctc, dctc, my_addr_index, attr->ah_attr.grh.sgid_index);
+		MLX5_SET(dctc, dctc, hop_limit, attr->ah_attr.grh.hop_limit);
+
+		err = mlx5_core_create_dct(dev->mdev, &qp->dct.mdct, qp->dct.in,
+					   MLX5_ST_SZ_BYTES(create_dct_in));
+		if (err)
+			return err;
+		resp.dctn = qp->dct.mdct.mqp.qpn;
+		err = ib_copy_to_udata(udata, &resp, resp.response_length);
+		if (err) {
+			mlx5_core_destroy_dct(dev->mdev, &qp->dct.mdct);
+			return err;
+		}
+	} else {
+		mlx5_ib_warn(dev, "Modify DCT: Invalid transition from %d to %d\n", cur_state, new_state);
+		return -EINVAL;
+	}
+	if (err)
+		qp->state = IB_QPS_ERR;
+	else
+		qp->state = new_state;
+	return err;
+}
+
 int mlx5_ib_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
 		      int attr_mask, struct ib_udata *udata)
 {
@@ -3159,6 +3318,8 @@ int mlx5_ib_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
 		qp_type = (unlikely(ibqp->qp_type == MLX5_IB_QPT_HW_GSI)) ?
 			IB_QPT_GSI : ibqp->qp_type;
 
+	if (qp_type == MLX5_IB_QPT_DCT)
+		return mlx5_ib_modify_dct(ibqp, attr, attr_mask, udata);
 
 	mutex_lock(&qp->mutex);
 
@@ -4729,6 +4890,71 @@ static int query_qp_attr(struct mlx5_ib_dev *dev, struct mlx5_ib_qp *qp,
 	return err;
 }
 
+static int mlx5_ib_dct_query_qp(struct mlx5_ib_dev *dev, struct mlx5_ib_qp *mqp,
+				struct ib_qp_attr *qp_attr, int qp_attr_mask,
+				struct ib_qp_init_attr *qp_init_attr)
+{
+	struct mlx5_core_dct	*dct = &mqp->dct.mdct;
+	u32 *out;
+	u32 access_flags = 0;
+	int outlen = MLX5_ST_SZ_BYTES(query_dct_out);
+	void *dctc;
+	int err;
+	int supported_mask = IB_QP_STATE |
+			     IB_QP_ACCESS_FLAGS |
+			     IB_QP_PORT |
+			     IB_QP_MIN_RNR_TIMER |
+			     IB_QP_AV |
+			     IB_QP_PATH_MTU |
+			     IB_QP_PKEY_INDEX;
+
+	if (qp_attr_mask & ~supported_mask)
+		return -EINVAL;
+	if (mqp->state != IB_QPS_RTR)
+		return -EINVAL;
+
+	out = kzalloc(outlen, GFP_KERNEL);
+	if (!out)
+		return -ENOMEM;
+
+	err = mlx5_core_dct_query(dev->mdev, dct, out, outlen);
+	if (err)
+		goto out;
+
+	dctc = MLX5_ADDR_OF(query_dct_out, out, dct_context_entry);
+
+	if (qp_attr_mask & IB_QP_STATE)
+		qp_attr->qp_state = IB_QPS_RTR;
+
+	if (qp_attr_mask & IB_QP_ACCESS_FLAGS) {
+		if (MLX5_GET(dctc, dctc, rre))
+			access_flags |= IB_ACCESS_REMOTE_READ;
+		if (MLX5_GET(dctc, dctc, rwe))
+			access_flags |= IB_ACCESS_REMOTE_WRITE;
+		if (MLX5_GET(dctc, dctc, rae))
+			access_flags |= IB_ACCESS_REMOTE_ATOMIC;
+		qp_attr->qp_access_flags = access_flags;
+	}
+
+	if (qp_attr_mask & IB_QP_PORT)
+		qp_attr->port_num = MLX5_GET(dctc, dctc, port);
+	if (qp_attr_mask & IB_QP_MIN_RNR_TIMER)
+		qp_attr->min_rnr_timer = MLX5_GET(dctc, dctc, min_rnr_nak);
+	if (qp_attr_mask & IB_QP_AV) {
+		qp_attr->ah_attr.grh.traffic_class = MLX5_GET(dctc, dctc, tclass);
+		qp_attr->ah_attr.grh.flow_label = MLX5_GET(dctc, dctc, flow_label);
+		qp_attr->ah_attr.grh.sgid_index = MLX5_GET(dctc, dctc, my_addr_index);
+		qp_attr->ah_attr.grh.hop_limit = MLX5_GET(dctc, dctc, hop_limit);
+	}
+	if (qp_attr_mask & IB_QP_PATH_MTU)
+		qp_attr->path_mtu = MLX5_GET(dctc, dctc, mtu);
+	if (qp_attr_mask & IB_QP_PKEY_INDEX)
+		qp_attr->pkey_index = MLX5_GET(dctc, dctc, pkey_index);
+out:
+	kfree(out);
+	return err;
+}
+
 int mlx5_ib_query_qp(struct ib_qp *ibqp, struct ib_qp_attr *qp_attr,
 		     int qp_attr_mask, struct ib_qp_init_attr *qp_init_attr)
 {
@@ -4748,6 +4974,11 @@ int mlx5_ib_query_qp(struct ib_qp *ibqp, struct ib_qp_attr *qp_attr,
 	memset(qp_init_attr, 0, sizeof(*qp_init_attr));
 	memset(qp_attr, 0, sizeof(*qp_attr));
 
+	if (unlikely(ibqp->qp_type == IB_QPT_DRIVER &&
+		     qp->driver_qp_type == MLX5_IB_QPT_DCT))
+		return mlx5_ib_dct_query_qp(dev, qp, qp_attr,
+					    qp_attr_mask, qp_init_attr);
+
 	mutex_lock(&qp->mutex);
 
 	if (qp->ibqp.qp_type == IB_QPT_RAW_PACKET ||
diff --git a/include/uapi/rdma/mlx5-abi.h b/include/uapi/rdma/mlx5-abi.h
index 83bde975d3f9..f6d319dfc7bf 100644
--- a/include/uapi/rdma/mlx5-abi.h
+++ b/include/uapi/rdma/mlx5-abi.h
@@ -362,6 +362,11 @@ struct mlx5_ib_create_ah_resp {
 	__u8	reserved[6];
 };
 
+struct mlx5_ib_modify_qp_resp {
+	__u32	response_length;
+	__u32	dctn;
+};
+
 struct mlx5_ib_create_wq_resp {
 	__u32	response_length;
 	__u32	reserved;
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next v1 4/6] IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP

[Or Gerlitz]

On Wed, Dec 27, 2017 at 7:15 PM, Leon wrote:
> From: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

> +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> @@ -202,6 +202,8 @@ struct mlx5_ib_flow_db {
>   * creates the actual hardware QP.
>   */
>  #define MLX5_IB_QPT_HW_GSI     IB_QPT_RESERVED2
> +#define MLX5_IB_QPT_DCI                IB_QPT_RESERVED3
> +#define MLX5_IB_QPT_DCT                IB_QPT_RESERVED4

When we added the reserved QPTs we made an explicit statement at the
verbs header file that this is a "range for qp types internal to the
low level driver"

In the down stream patches you are actually assuming user-space
knows that for mlx5 DCI/DCT equals IB_QPT_RESERVED3/4 which
violates this statement.

What you are looking for is more of vendor QPT

Maybe you need

IB_QPT_VENDOR1
IB_QPT_VENDOR2
...
IB_QPT_VENDOR10

and use the per driver ABI header to advertize to user space the
semantics, e.g in
the mlx5 ABI you put

IB_QPT_VENDOR1 = DCI
IB_QPT_VENDOR2 = DCT

you added these two flags in the ABI file, but you never use them in downstream

MLX5_QP_FLAG_TYPE_DCT
MLX5_QP_FLAG_TYPE_DCI

patches, something must be wrong here even according to your design
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next v1 4/6] IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP

[Jason Gunthorpe]

On Wed, Dec 27, 2017 at 11:16:09PM +0200, Or Gerlitz wrote:
> On Wed, Dec 27, 2017 at 7:15 PM, Leon wrote:
> > From: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> > +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> > @@ -202,6 +202,8 @@ struct mlx5_ib_flow_db {
> >   * creates the actual hardware QP.
> >   */
> >  #define MLX5_IB_QPT_HW_GSI     IB_QPT_RESERVED2
> > +#define MLX5_IB_QPT_DCI                IB_QPT_RESERVED3
> > +#define MLX5_IB_QPT_DCT                IB_QPT_RESERVED4
> 
> When we added the reserved QPTs we made an explicit statement at the
> verbs header file that this is a "range for qp types internal to the
> low level driver"
> 
> In the down stream patches you are actually assuming user-space
> knows that for mlx5 DCI/DCT equals IB_QPT_RESERVED3/4 which
> violates this statement.

IB_QPT_RESERVED should NOT leak to the user driver ever, for some
reason a flag is used. I thought we agreed to an enum during the
original RFC?

> Maybe you need
> 
> IB_QPT_VENDOR1
> IB_QPT_VENDOR2
> ...
> IB_QPT_VENDOR10

No, no. The driver specific QP type is carried only within the driver
specific udata and is never part of the common API.

> you added these two flags in the ABI file, but you never use them in downstream
> 
> MLX5_QP_FLAG_TYPE_DCT
> MLX5_QP_FLAG_TYPE_DCI
> 
> patches, something must be wrong here even according to your design

Hm? they are used here:

+       if (ucmd->flags & MLX5_QP_FLAG_TYPE_DCI)
+               init_attr->qp_type = MLX5_IB_QPT_DCI;
+       else
+               init_attr->qp_type = MLX5_IB_QPT_DCT;

Which is a bad ABI design, check flags explicitly never use else.

Not sure how much I like this - why try so hard to keep the udata
unchanged? I would have added a 'u32 mlx_qp_type' and a variable
length struct I think..

Or use the new kabi :|

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next v1 4/6] IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP

[Or Gerlitz]

On Thu, Dec 28, 2017 at 12:02 AM, Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> On Wed, Dec 27, 2017 at 11:16:09PM +0200, Or Gerlitz wrote:

>> you added these two flags in the ABI file, but you never use them in downstream
>> MLX5_QP_FLAG_TYPE_DCT
>> MLX5_QP_FLAG_TYPE_DCI
>> patches, something must be wrong here even according to your design

> Hm? they are used here:
>
> +       if (ucmd->flags & MLX5_QP_FLAG_TYPE_DCI)
> +               init_attr->qp_type = MLX5_IB_QPT_DCI;
> +       else
> +               init_attr->qp_type = MLX5_IB_QPT_DCT;
>
> Which is a bad ABI design, check flags explicitly never use else.

Maybe, but when I said that in downstream it is assumed
user-space knows that for mlx5 DCI/DCT equals
IB_QPT_RESERVED3/4 I was wrong, it is derived from the flags.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next v1 4/6] IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP

[Or Gerlitz]

On Wed, Dec 27, 2017 at 7:15 PM, Leon Romanovsky wrote:
> From: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> The QP type IB_QPT_DRIVER doesn't describe the transport or the service that
> the QP provides but those are known only to the hardware driver.
> The extra data that tells more about the QP is in the driver
> channel.
> Take the real QP type from the driver channel and modify the QP initial
> attributes before continuing with create_qp(). Also, define the layout
> of the data structure that comes in the driver channel.
> Downstream patches from this series will add support for both DCI and
> DCT driver QPs.

> +static int validate_driver_qp(struct mlx5_ib_dev *dev,
> +                             struct ib_qp_init_attr *init_attr,
> +                             struct mlx5_ib_create_qp *ucmd,
> +                             struct ib_udata *udata)
> +{

[...]

> +       if (ucmd->flags & MLX5_QP_FLAG_TYPE_DCI)
> +               init_attr->qp_type = MLX5_IB_QPT_DCI;
> +       else
> +               init_attr->qp_type = MLX5_IB_QPT_DCT;
> +       return 0;
> +}

from maintenance/clarity standpoint, not sure if it is a good idea that
a validate_yyy function does assignment


> +
>  struct ib_qp *mlx5_ib_create_qp(struct ib_pd *pd,
> -                               struct ib_qp_init_attr *init_attr,
> +                               struct ib_qp_init_attr *verbs_init_attr,
>                                 struct ib_udata *udata)
>  {

[...]

> @@ -2153,6 +2195,16 @@ struct ib_qp *mlx5_ib_create_qp(struct ib_pd *pd,
>                 dev = to_mdev(to_mxrcd(init_attr->xrcd)->ibxrcd.device);
>         }
>
> +       if (init_attr->qp_type == IB_QPT_DRIVER) {
> +               struct mlx5_ib_create_qp ucmd;
> +
> +               init_attr = &mlx_init_attr;
> +               memcpy(init_attr, verbs_init_attr, sizeof(*verbs_init_attr));
> +               err = validate_driver_qp(dev, init_attr, &ucmd, udata);
> +               if (err)
> +                       return ERR_PTR(err);
> +       }
> +
>         switch (init_attr->qp_type) {
>         case IB_QPT_XRC_TGT:
>         case IB_QPT_XRC_INI:
> @@ -2214,6 +2266,9 @@ struct ib_qp *mlx5_ib_create_qp(struct ib_pd *pd,
>                 return ERR_PTR(-EINVAL);
>         }
>
> +       if (verbs_init_attr->qp_type == IB_QPT_DRIVER)
> +               qp->driver_qp_type = init_attr->qp_type;
> +
>         return &qp->ibqp;
>  }

so if the values passed by user space are legal and the FW supports
DC, no error is returned to user-space, right? this doesn't seem bisectable,
if only patches 1...4 are applied, create qp will succeed?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next v1 4/6] IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP

[Moni Shoua]

>> +       if (verbs_init_attr->qp_type == IB_QPT_DRIVER)
>> +               qp->driver_qp_type = init_attr->qp_type;
>> +
>>         return &qp->ibqp;
>>  }

> so if the values passed by user space are legal and the FW supports
> DC, no error is returned to user-space, right? this doesn't seem bisectable,
> if only patches 1...4 are applied, create qp will succeed?
Wrong. Create QP will  fail before code reaches this line. Only after
applying the following patches in the series the operations of
create/modify will succeed.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next v1 4/6] IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP

[Yishai Hadas]

On 12/28/2017 12:02 AM, Jason Gunthorpe wrote:
> On Wed, Dec 27, 2017 at 11:16:09PM +0200, Or Gerlitz wrote:
>> On Wed, Dec 27, 2017 at 7:15 PM, Leon wrote:
>>> From: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>
>>> +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
>>> @@ -202,6 +202,8 @@ struct mlx5_ib_flow_db {
>>>    * creates the actual hardware QP.
>>>    */
>>>   #define MLX5_IB_QPT_HW_GSI     IB_QPT_RESERVED2
>>> +#define MLX5_IB_QPT_DCI                IB_QPT_RESERVED3
>>> +#define MLX5_IB_QPT_DCT                IB_QPT_RESERVED4
>>
>> When we added the reserved QPTs we made an explicit statement at the
>> verbs header file that this is a "range for qp types internal to the
>> low level driver"
>>
>> In the down stream patches you are actually assuming user-space
>> knows that for mlx5 DCI/DCT equals IB_QPT_RESERVED3/4 which
>> violates this statement.
> 
> IB_QPT_RESERVED should NOT leak to the user driver ever, for some
> reason a flag is used. I thought we agreed to an enum during the
> original RFC?
> 
>> Maybe you need
>>
>> IB_QPT_VENDOR1
>> IB_QPT_VENDOR2
>> ...
>> IB_QPT_VENDOR10
> 
> No, no. The driver specific QP type is carried only within the driver
> specific udata and is never part of the common API.
> 

Correct, this is really what had been done, so we are fine here.

>> you added these two flags in the ABI file, but you never use them in downstream
>>
>> MLX5_QP_FLAG_TYPE_DCT
>> MLX5_QP_FLAG_TYPE_DCI
>>
>> patches, something must be wrong here even according to your design
> 
> Hm? they are used here:
> 
> +       if (ucmd->flags & MLX5_QP_FLAG_TYPE_DCI)
> +               init_attr->qp_type = MLX5_IB_QPT_DCI;
> +       else
> +               init_attr->qp_type = MLX5_IB_QPT_DCT;
> 
> Which is a bad ABI design, check flags explicitly never use else.

We can drop this 'else' as few lines above there was a check that both 
can't be set together. However, not sure that this change worth V2.

> Not sure how much I like this - why try so hard to keep the udata
> unchanged? 

We prefer to not increase the udata length if it's not really required. 
Doing that might force other changes around to preserve compatibility 
with legacy user space [1].

 From application point of view the API will be very clear to ask for a 
specific QP type. Here we are talking on some 'user driver <-> kernel 
driver' API to pass the request and as of the above we preferred to go 
by that way without increasing the udata.

[1] 
http://elixir.free-electrons.com/linux/latest/source/drivers/infiniband/hw/mlx5/qp.c#L1595.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next v1 4/6] IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP

[Or Gerlitz]

On Thu, Dec 28, 2017 at 12:02 PM, Yishai Hadas
<yishaih-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
> On 12/28/2017 12:02 AM, Jason Gunthorpe wrote:

>> +       if (ucmd->flags & MLX5_QP_FLAG_TYPE_DCI)
>> +               init_attr->qp_type = MLX5_IB_QPT_DCI;
>> +       else
>> +               init_attr->qp_type = MLX5_IB_QPT_DCT;

>> Which is a bad ABI design, check flags explicitly never use else.

> We can drop this 'else' as few lines above there was a check that both can't
> be set together. However, not sure that this change worth V2.

Yishai,

if something should be fixed lets just do that, why not?

validate_driver_qp() deals @ 95% of it's contents with DC but has no
DC in it's name, also it does assignment which goes beyond validation.

There are bunch of things to consider fixing here. The patch spaghettizes
the driver code w.r.t where the qp type is stored, in the ib qp or the mlx5 qp
struct in a new field. Why not do a refactoring patch that uses always the
type from the mlx5 qp or do a conversion at the code that parses the UAPI
and step over the user space provided  DRIVER type with the RES3 (DCI)
or RES4 (DCT)
that you defined, or any other idea which will keep us with less
branching across
the place on where the qp type is stored.

Also you added a "is qp ok" self function which AFAIK uses a different practice
from the IB core copy, did you consider to refactor the core code and allow
a re-use from the driver? did you consider to use a similar practice
for your "is ok" function?

You served as the reviewer here, I will be happy to get your op on
these matters.

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next v1 4/6] IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP

[Yishai Hadas]

On 12/29/2017 5:23 PM, Or Gerlitz wrote:
> validate_driver_qp() deals @ 95% of it's contents with DC but has no
> DC in it's name, also it does assignment which goes beyond validation.
> 

OK, will rename it to be prepare_driver_qp().

The function name follows the input QP type (i.e. IB_QPT_DRIVER) and 
what it does on. Specifically, its purpose is to prepare the input data 
(ucmd, init_attr) to be used later on. Similar name and logic is used as 
part of RQ creation as part of this file. (i.e. prepare_user_rq()).

> There are bunch of things to consider fixing here. The patch spaghettizes
> the driver code w.r.t where the qp type is stored, in the ib qp

It doesn't make sense to overwrite the input QP type (i.e. 
IB_QPT_DRIVER) which was supplied by the core layer to some internal 
sub-type.

> or the mlx5 qp

The sub type is from type enum ib_qp_type (MLX5_IB_QPT_DCI 
<->IB_QPT_RESERVED3) and not an internal mlx5 PRM QP type.

However, to make the code more clearer and accurate, we'll change as 
part of V2 'u32 driver_qp_type' to be 'enum ib_qp_type sub_qp_type' and 
clean some redundant checks when applicable. (e.g. in 
mlx5_ib_query_qp()).

> Also you added a "is qp ok" self function which AFAIK uses a different practice
> from the IB core copy, did you consider to refactor the core code and allow
> a re-use from the driver?

Yes, we considered that, however, as this QP type has some specific 
driver behavior we found it better to stay with that specific logic for 
DCI QP in the driver code, no reason to refactor the core layer as of 
that. As part of V2 will rename 'mlx5_ib_modify_qp_is_ok()' to be 
'modify_dc_qp_is_ok()' to better represent the specific use case around.


> did you consider to use a similar practice
> for your "is ok" function?

Yes, however, we found it more clearer and simple to use current code 
for the specific DCI case instead of introducing some general mapping 
table without any real use.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next v1 4/6] IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP

[Or Gerlitz]

On Sun, Dec 31, 2017 at 1:45 PM, Yishai Hadas
<yishaih-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
> On 12/29/2017 5:23 PM, Or Gerlitz wrote:
>>
>> validate_driver_qp() deals @ 95% of it's contents with DC but has no
>> DC in it's name, also it does assignment which goes beyond validation.
>>
>
> OK, will rename it to be prepare_driver_qp().

sounds better, not it is very DC specific now, so you are happy with
the way it is now w.r.t future extensions?

> The function name follows the input QP type (i.e. IB_QPT_DRIVER) and what it
> does on. Specifically, its purpose is to prepare the input data (ucmd,
> init_attr) to be used later on. Similar name and logic is used as part of RQ
> creation as part of this file. (i.e. prepare_user_rq()).

>> There are bunch of things to consider fixing here. The patch spaghettizes
>> the driver code w.r.t where the qp type is stored, in the ib qp

> It doesn't make sense to overwrite the input QP type (i.e. IB_QPT_DRIVER)
> which was supplied by the core layer to some internal sub-type.

why?

>> or the mlx5 qp

> The sub type is from type enum ib_qp_type (MLX5_IB_QPT_DCI
> <->IB_QPT_RESERVED3) and not an internal mlx5 PRM QP type.

> However, to make the code more clearer and accurate, we'll change as part of
> V2 'u32 driver_qp_type' to be 'enum ib_qp_type sub_qp_type' and clean some
> redundant checks when applicable. (e.g. in mlx5_ib_query_qp()).

just do your best to have the driver checks **one** field for the qp type
for all QPs, otherwise I expect nice pile of future bugs around this corner.


>> Also you added a "is qp ok" self function which AFAIK uses a different
>> practice
>> from the IB core copy, did you consider to refactor the core code and
>> allow
>> a re-use from the driver?
>
>
> Yes, we considered that, however, as this QP type has some specific driver
> behavior we found it better to stay with that specific logic for DCI QP in
> the driver code, no reason to refactor the core layer as of that. As part of
> V2 will rename 'mlx5_ib_modify_qp_is_ok()' to be 'modify_dc_qp_is_ok()' to
> better represent the specific use case around.

So what you are saying is the core logic being good for all
QP types X all drivers X all transports and only mlx5/DC needs this different
treatment? interesting, so what is the source for this misalignment?

>> did you consider to use a similar practice
>> for your "is ok" function?

> Yes, however, we found it more clearer and simple to use current code for
> the specific DCI case instead of introducing some general mapping table
> without any real use.

if you can map DC to the general framework, it means you can actually
refactor the core helper and call it from mlx5, sounds better to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next v1 4/6] IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP

[Yishai Hadas]

On 12/31/2017 2:37 PM, Or Gerlitz wrote:
>> It doesn't make sense to overwrite the input QP type (i.e. IB_QPT_DRIVER)
>> which was supplied by the core layer to some internal sub-type.
> 
> why?

A driver code doesn't expect to change an input IB core property (i.e 
qp_type). Specifically the DC QP type does not expect to be recognized 
out of the mlx5 driver, for that reason a sub qp type will be used as 
part of V2 in the mlx5 driver code.

> So what you are saying is the core logic being good for all
> QP types X all drivers X all transports and only mlx5/DC needs this different
> treatment? interesting, so what is the source for this misalignment?
> 

We believe that it doesn't justify a core refactoring for that specific 
mask checking.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next v1 4/6] IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP

[Jason Gunthorpe]

On Sun, Dec 31, 2017 at 01:45:41PM +0200, Yishai Hadas wrote:
> On 12/29/2017 5:23 PM, Or Gerlitz wrote:
> >validate_driver_qp() deals @ 95% of it's contents with DC but has no
> >DC in it's name, also it does assignment which goes beyond validation.
> >
> 
> OK, will rename it to be prepare_driver_qp().
> 
> The function name follows the input QP type (i.e. IB_QPT_DRIVER) and what it
> does on. Specifically, its purpose is to prepare the input data (ucmd,
> init_attr) to be used later on. Similar name and logic is used as part of RQ
> creation as part of this file. (i.e. prepare_user_rq()).

If we are going to bike shed this to death, here is my
suggestion. Name the function after what it actually does, and tidy
the flags checking to be less dangerous if more QP types are added.

static int set_mlx_qp_type(struct mlx5_ib_dev *dev,
			   struct ib_qp_init_attr *init_attr,
			   struct mlx5_ib_create_qp *ucmd,
			   struct ib_udata *udata)
{
	enum { MLX_QP_FLAGS = MLX5_QP_FLAG_TYPE_DCT | MLX5_QP_FLAG_TYPE_DCI };
	int err;

	if (!udata)
		return -EINVAL;

	if (udata->inlen < sizeof(*ucmd)) {
		mlx5_ib_dbg(dev, "create_qp user command is smaller than expected\n");
		return -EINVAL;
	}
	err = ib_copy_from_udata(ucmd, udata, sizeof(*ucmd));
	if (err)
		return err;

	if ((ucmd->flags & MLX_QP_FLAGS) == MLX5_QP_FLAG_TYPE_DCI)
		init_attr->qp_type = MLX5_IB_QPT_DCI;
	else if ((ucmd->flags & MLX_QP_FLAGS) == MLX5_QP_FLAG_TYPE_DCT)
		init_attr->qp_type = MLX5_IB_QPT_DCT;
 	else {
		mlx5_ib_dbg(dev, "Invalid QP flags\n");
		return -EINVAL;
	}

	if (!MLX5_CAP_GEN(dev->mdev, dct)) {
		mlx5_ib_dbg(dev, "DC transport is not supported\n");
		return -EOPNOTSUPP;
	}

	return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next v1 4/6] IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP

[Moni Shoua]

> static int set_mlx_qp_type(struct mlx5_ib_dev *dev,
>                            struct ib_qp_init_attr *init_attr,
>                            struct mlx5_ib_create_qp *ucmd,
>                            struct ib_udata *udata)
> {
>         enum { MLX_QP_FLAGS = MLX5_QP_FLAG_TYPE_DCT | MLX5_QP_FLAG_TYPE_DCI };
>         int err;
>
>         if (!udata)
>                 return -EINVAL;
>
>         if (udata->inlen < sizeof(*ucmd)) {
>                 mlx5_ib_dbg(dev, "create_qp user command is smaller than expected\n");
>                 return -EINVAL;
>         }
>         err = ib_copy_from_udata(ucmd, udata, sizeof(*ucmd));
>         if (err)
>                 return err;
>
>         if ((ucmd->flags & MLX_QP_FLAGS) == MLX5_QP_FLAG_TYPE_DCI)
>                 init_attr->qp_type = MLX5_IB_QPT_DCI;
>         else if ((ucmd->flags & MLX_QP_FLAGS) == MLX5_QP_FLAG_TYPE_DCT)
>                 init_attr->qp_type = MLX5_IB_QPT_DCT;
>         else {
>                 mlx5_ib_dbg(dev, "Invalid QP flags\n");
>                 return -EINVAL;
>         }
>
>         if (!MLX5_CAP_GEN(dev->mdev, dct)) {
>                 mlx5_ib_dbg(dev, "DC transport is not supported\n");
>                 return -EOPNOTSUPP;
>         }
>
>         return 0;
> }
I like this :)
Will take it in.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next v1 4/6] IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP

[Or Gerlitz]

On Mon, Jan 1, 2018 at 12:14 PM, Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> static int set_mlx_qp_type(struct mlx5_ib_dev *dev,
>>                            struct ib_qp_init_attr *init_attr,
>>                            struct mlx5_ib_create_qp *ucmd,
>>                            struct ib_udata *udata)
>> {
>>         enum { MLX_QP_FLAGS = MLX5_QP_FLAG_TYPE_DCT | MLX5_QP_FLAG_TYPE_DCI };
>>         int err;
>>
>>         if (!udata)
>>                 return -EINVAL;
>>
>>         if (udata->inlen < sizeof(*ucmd)) {
>>                 mlx5_ib_dbg(dev, "create_qp user command is smaller than expected\n");
>>                 return -EINVAL;
>>         }
>>         err = ib_copy_from_udata(ucmd, udata, sizeof(*ucmd));
>>         if (err)
>>                 return err;
>>
>>         if ((ucmd->flags & MLX_QP_FLAGS) == MLX5_QP_FLAG_TYPE_DCI)
>>                 init_attr->qp_type = MLX5_IB_QPT_DCI;
>>         else if ((ucmd->flags & MLX_QP_FLAGS) == MLX5_QP_FLAG_TYPE_DCT)
>>                 init_attr->qp_type = MLX5_IB_QPT_DCT;
>>         else {
>>                 mlx5_ib_dbg(dev, "Invalid QP flags\n");
>>                 return -EINVAL;
>>         }
>>
>>         if (!MLX5_CAP_GEN(dev->mdev, dct)) {
>>                 mlx5_ib_dbg(dev, "DC transport is not supported\n");
>>                 return -EOPNOTSUPP;
>>         }
>>
>>         return 0;
>> }
> I like this :) Will take it in.

LGTM (the new suit and that we are wearing it)
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next v1 4/6] IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP

[Or Gerlitz]

On Sun, Dec 31, 2017 at 3:05 PM, Yishai Hadas
<yishaih-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
> On 12/31/2017 2:37 PM, Or Gerlitz wrote:

>>> It doesn't make sense to overwrite the input QP type (i.e. IB_QPT_DRIVER)
>>> which was supplied by the core layer to some internal sub-type.

>> why?

> A driver code doesn't expect to change an input IB core property (i.e
> qp_type). Specifically the DC QP type does not expect to be recognized out
> of the mlx5 driver, for that reason a sub qp type will be used as part of V2
> in the mlx5 driver code.

sub qp type sound better from qp type stored in two places... I think we should
still strongly aim to have the qp type in one place... I guess it
would be correct
for this to be a driver maintainer decision.

>> So what you are saying is the core logic being good for all
>> QP types X all drivers X all transports and only mlx5/DC needs this
>> different treatment? interesting, so what is the source for this misalignment?

> We believe that it doesn't justify a core refactoring for that specific mask
> checking.

ok, just remember that C&Ps have the tendency to add bugs, so look
carefully on the code..
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next v1 4/6] IB/mlx5: Handle type IB_QPT_DRIVER when creating a QP

[Jason Gunthorpe]

On Thu, Dec 28, 2017 at 12:02:56PM +0200, Yishai Hadas wrote:

> From application point of view the API will be very clear to ask for a
> specific QP type. Here we are talking on some 'user driver <-> kernel
> driver' API to pass the request and as of the above we preferred to go by
> that way without increasing the udata.

Okay, worst that can happen is future extensions will need to use the
new kabi.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html