diff mbox series

[rdma-core,2/4] mlx5: Implement ibv_query_qp_data_in_order() verb

Message ID 20210609155932.218005-3-yishaih@nvidia.com (mailing list archive)
State New
Headers show
Series verbs: Introduce ibv_query_qp_data_in_order() verb | expand

Commit Message

Yishai Hadas June 9, 2021, 3:59 p.m. UTC
From: Patrisious Haddad <phaddad@nvidia.com>

Implement the ibv_query_qp_data_in_order() verb by using DEVX to read
from firmware the 'in_order_data' capability.

Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
Reviewed-by: Maor Gottlieb <maorg@nvidia.com>
Signed-off-by: Yishai Hadas <yishaih@nvidia.com>
---
 providers/mlx5/mlx5.c     |  1 +
 providers/mlx5/mlx5.h     |  3 +++
 providers/mlx5/mlx5_ifc.h | 39 +++++++++++++++++++++++++++++++--
 providers/mlx5/verbs.c    | 55 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 96 insertions(+), 2 deletions(-)

Comments

Leon Romanovsky June 10, 2021, 5:50 a.m. UTC | #1
On Wed, Jun 09, 2021 at 06:59:30PM +0300, Yishai Hadas wrote:
> From: Patrisious Haddad <phaddad@nvidia.com>
> 
> Implement the ibv_query_qp_data_in_order() verb by using DEVX to read
> from firmware the 'in_order_data' capability.
> 
> Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
> Reviewed-by: Maor Gottlieb <maorg@nvidia.com>
> Signed-off-by: Yishai Hadas <yishaih@nvidia.com>
> ---
>  providers/mlx5/mlx5.c     |  1 +
>  providers/mlx5/mlx5.h     |  3 +++
>  providers/mlx5/mlx5_ifc.h | 39 +++++++++++++++++++++++++++++++--
>  providers/mlx5/verbs.c    | 55 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 96 insertions(+), 2 deletions(-)

<...>

> +int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
> +				uint32_t flags)
> +{
> +	uint32_t in_qp[DEVX_ST_SZ_DW(query_qp_in)] = {};
> +	uint32_t out_qp[DEVX_ST_SZ_DW(query_qp_out)] = {};
> +	struct mlx5_context *mctx = to_mctx(qp->context);
> +	struct mlx5_qp *mqp = to_mqp(qp);
> +	int ret;
> +
> +/* Currently this API is only supported for x86 architectures since most
> + * non-x86 platforms are known to be OOO and need to do a per-platform study.
> + */
> +#if !defined(__i386__) && !defined(__x86_64__)
> +	return 0;
> +#endif

Does it compile without warnings/errors on such platforms?
You have "return 0;" in the middle of function, so the right thing to do
it is to write with "#if ..." over function or inside like below, as
long as "#else" exists.

int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
				uint32_t flags)
{
#if !defined(__i386__) && !defined(__x86_64__)
	/* Currently this API is only supported for x86 architectures since most
	 * non-x86 platforms are known to be OOO and need to do a per-platform study.
	 */
	 return 0;
#else
.....
#endif

> +
> +	if (flags || !mctx->qp_data_in_order_cap)
> +		return 0;
> +
> +	if (mqp->dc_type == MLX5DV_DCTYPE_DCT)
> +		return query_dct_in_order(qp);
> +
> +	if (qp->state != IBV_QPS_RTS)
> +		return 0;
> +
> +	DEVX_SET(query_qp_in, in_qp, opcode, MLX5_CMD_OP_QUERY_QP);
> +	DEVX_SET(query_qp_in, in_qp, qpn, qp->qp_num);
> +	ret = mlx5dv_devx_qp_query(qp, in_qp, sizeof(in_qp), out_qp,
> +				   sizeof(out_qp));
> +	if (ret)
> +		return 0;
> +
> +	return DEVX_GET(query_qp_out, out_qp, qpc.data_in_order);
> +}
> +

Thanks
Jason Gunthorpe June 10, 2021, 11:42 a.m. UTC | #2
On Thu, Jun 10, 2021 at 08:50:59AM +0300, Leon Romanovsky wrote:
> On Wed, Jun 09, 2021 at 06:59:30PM +0300, Yishai Hadas wrote:
> > From: Patrisious Haddad <phaddad@nvidia.com>
> > 
> > Implement the ibv_query_qp_data_in_order() verb by using DEVX to read
> > from firmware the 'in_order_data' capability.
> > 
> > Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
> > Reviewed-by: Maor Gottlieb <maorg@nvidia.com>
> > Signed-off-by: Yishai Hadas <yishaih@nvidia.com>
> >  providers/mlx5/mlx5.c     |  1 +
> >  providers/mlx5/mlx5.h     |  3 +++
> >  providers/mlx5/mlx5_ifc.h | 39 +++++++++++++++++++++++++++++++--
> >  providers/mlx5/verbs.c    | 55 +++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 96 insertions(+), 2 deletions(-)
> 
> <...>
> 
> > +int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
> > +				uint32_t flags)
> > +{
> > +	uint32_t in_qp[DEVX_ST_SZ_DW(query_qp_in)] = {};
> > +	uint32_t out_qp[DEVX_ST_SZ_DW(query_qp_out)] = {};
> > +	struct mlx5_context *mctx = to_mctx(qp->context);
> > +	struct mlx5_qp *mqp = to_mqp(qp);
> > +	int ret;
> > +
> > +/* Currently this API is only supported for x86 architectures since most
> > + * non-x86 platforms are known to be OOO and need to do a per-platform study.
> > + */
> > +#if !defined(__i386__) && !defined(__x86_64__)
> > +	return 0;
> > +#endif
> 
> Does it compile without warnings/errors on such platforms?
> You have "return 0;" in the middle of function, so the right thing to do
> it is to write with "#if ..." over function or inside like below, as
> long as "#else" exists.
> 
> int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
> 				uint32_t flags)
> {
> #if !defined(__i386__) && !defined(__x86_64__)
> 	/* Currently this API is only supported for x86 architectures since most
> 	 * non-x86 platforms are known to be OOO and need to do a per-platform study.
> 	 */
> 	 return 0;
> #else
> .....
> #endif

We should probably put the above in the core code anyhow

Jason
Leon Romanovsky June 10, 2021, 11:50 a.m. UTC | #3
On Thu, Jun 10, 2021 at 08:42:24AM -0300, Jason Gunthorpe wrote:
> On Thu, Jun 10, 2021 at 08:50:59AM +0300, Leon Romanovsky wrote:
> > On Wed, Jun 09, 2021 at 06:59:30PM +0300, Yishai Hadas wrote:
> > > From: Patrisious Haddad <phaddad@nvidia.com>
> > > 
> > > Implement the ibv_query_qp_data_in_order() verb by using DEVX to read
> > > from firmware the 'in_order_data' capability.
> > > 
> > > Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
> > > Reviewed-by: Maor Gottlieb <maorg@nvidia.com>
> > > Signed-off-by: Yishai Hadas <yishaih@nvidia.com>
> > >  providers/mlx5/mlx5.c     |  1 +
> > >  providers/mlx5/mlx5.h     |  3 +++
> > >  providers/mlx5/mlx5_ifc.h | 39 +++++++++++++++++++++++++++++++--
> > >  providers/mlx5/verbs.c    | 55 +++++++++++++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 96 insertions(+), 2 deletions(-)
> > 
> > <...>
> > 
> > > +int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
> > > +				uint32_t flags)
> > > +{
> > > +	uint32_t in_qp[DEVX_ST_SZ_DW(query_qp_in)] = {};
> > > +	uint32_t out_qp[DEVX_ST_SZ_DW(query_qp_out)] = {};
> > > +	struct mlx5_context *mctx = to_mctx(qp->context);
> > > +	struct mlx5_qp *mqp = to_mqp(qp);
> > > +	int ret;
> > > +
> > > +/* Currently this API is only supported for x86 architectures since most
> > > + * non-x86 platforms are known to be OOO and need to do a per-platform study.
> > > + */
> > > +#if !defined(__i386__) && !defined(__x86_64__)
> > > +	return 0;
> > > +#endif
> > 
> > Does it compile without warnings/errors on such platforms?
> > You have "return 0;" in the middle of function, so the right thing to do
> > it is to write with "#if ..." over function or inside like below, as
> > long as "#else" exists.
> > 
> > int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
> > 				uint32_t flags)
> > {
> > #if !defined(__i386__) && !defined(__x86_64__)
> > 	/* Currently this API is only supported for x86 architectures since most
> > 	 * non-x86 platforms are known to be OOO and need to do a per-platform study.
> > 	 */
> > 	 return 0;
> > #else
> > .....
> > #endif
> 
> We should probably put the above in the core code anyhow

Agree, it makes sense.

Thanks

> 
> Jason
Yishai Hadas June 10, 2021, 12:51 p.m. UTC | #4
> From: Leon Romanovsky <leon@kernel.org>

<>

> > We should probably put the above in the core code anyhow
> 
> Agree, it makes sense.
> 

Thanks, the PR was updated accordingly.
https://github.com/linux-rdma/rdma-core/pull/1009

Yishai
diff mbox series

Patch

diff --git a/providers/mlx5/mlx5.c b/providers/mlx5/mlx5.c
index 8ef305e..75ffde9 100644
--- a/providers/mlx5/mlx5.c
+++ b/providers/mlx5/mlx5.c
@@ -167,6 +167,7 @@  static const struct verbs_context_ops mlx5_ctx_common_ops = {
 	.unimport_dm = mlx5_unimport_dm,
 	.unimport_mr = mlx5_unimport_mr,
 	.unimport_pd = mlx5_unimport_pd,
+	.query_qp_data_in_order = mlx5_query_qp_data_in_order,
 };
 
 static const struct verbs_context_ops mlx5_ctx_cqev1_ops = {
diff --git a/providers/mlx5/mlx5.h b/providers/mlx5/mlx5.h
index f6c74df..bfa66ab 100644
--- a/providers/mlx5/mlx5.h
+++ b/providers/mlx5/mlx5.h
@@ -398,6 +398,7 @@  struct mlx5_context {
 	struct mlx5_bf			*nc_uar;
 	void				*cq_uar_reg;
 	struct mlx5_reserved_qpns	reserved_qpns;
+	uint8_t				qp_data_in_order_cap:1;
 };
 
 struct mlx5_bitmap {
@@ -1088,6 +1089,8 @@  struct ibv_qp *mlx5_create_qp(struct ibv_pd *pd, struct ibv_qp_init_attr *attr);
 int mlx5_query_qp(struct ibv_qp *qp, struct ibv_qp_attr *attr,
 		  int attr_mask,
 		  struct ibv_qp_init_attr *init_attr);
+int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
+				uint32_t flags);
 int mlx5_modify_qp(struct ibv_qp *qp, struct ibv_qp_attr *attr,
 		   int attr_mask);
 int mlx5_modify_qp_rate_limit(struct ibv_qp *qp,
diff --git a/providers/mlx5/mlx5_ifc.h b/providers/mlx5/mlx5_ifc.h
index 35be5ba..302788f 100644
--- a/providers/mlx5/mlx5_ifc.h
+++ b/providers/mlx5/mlx5_ifc.h
@@ -51,6 +51,7 @@  enum {
 	MLX5_CMD_OP_INIT2INIT_QP = 0x50e,
 	MLX5_CMD_OP_CREATE_PSV = 0x600,
 	MLX5_CMD_OP_DESTROY_PSV = 0x601,
+	MLX5_CMD_OP_QUERY_DCT = 0x713,
 	MLX5_CMD_OP_QUERY_ESW_VPORT_CONTEXT = 0x752,
 	MLX5_CMD_OP_QUERY_NIC_VPORT_CONTEXT = 0x754,
 	MLX5_CMD_OP_QUERY_ROCE_ADDRESS = 0x760,
@@ -641,7 +642,11 @@  struct mlx5_ifc_cmd_hca_cap_bits {
 	u8         reserved_at_21[0xf];
 	u8         vhca_id[0x10];
 
-	u8         reserved_at_40[0x40];
+	u8         reserved_at_40[0x20];
+
+	u8         reserved_at_60[0x2];
+	u8         qp_data_in_order[0x1];
+	u8         reserved_at_63[0x1d];
 
 	u8         log_max_srq_sz[0x8];
 	u8         log_max_qp_sz[0x8];
@@ -2654,6 +2659,12 @@  enum {
 	MLX5_ACTION_IN_FIELD_OUT_GTPU_TEID     = 0x6E,
 };
 
+struct mlx5_ifc_dctc_bits {
+	u8         reserved_at_0[0x1d];
+	u8         data_in_order[0x1];
+	u8         reserved_at_1e[0x362];
+};
+
 struct mlx5_ifc_packet_reformat_context_in_bits {
 	u8	reserved_at_0[0x5];
 	u8	reformat_type[0x3];
@@ -3040,7 +3051,7 @@  struct mlx5_ifc_qpc_bits {
 	u8         log_sq_size[0x4];
 	u8         reserved_at_55[0x3];
 	u8         ts_format[0x2];
-	u8         reserved_at_5a[0x1];
+	u8         data_in_order[0x1];
 	u8         rlky[0x1];
 	u8         ulp_stateless_offload_mode[0x4];
 
@@ -3409,6 +3420,30 @@  struct mlx5_ifc_query_qp_in_bits {
 	u8         reserved_at_60[0x20];
 };
 
+struct mlx5_ifc_query_dct_out_bits {
+	u8         status[0x8];
+	u8         reserved_at_8[0x18];
+
+	u8         syndrome[0x20];
+
+	u8         reserved_at_40[0x40];
+
+	struct mlx5_ifc_dctc_bits dctc;
+};
+
+struct mlx5_ifc_query_dct_in_bits {
+	u8         opcode[0x10];
+	u8         reserved_at_10[0x10];
+
+	u8         reserved_at_20[0x10];
+	u8         op_mod[0x10];
+
+	u8         reserved_at_40[0x8];
+	u8         dctn[0x18];
+
+	u8         reserved_at_60[0x20];
+};
+
 struct mlx5_ifc_tisc_bits {
 	u8         strict_lag_tx_port_affinity[0x1];
 	u8         tls_en[0x1];
diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c
index 8fcef62..88f4c3c 100644
--- a/providers/mlx5/verbs.c
+++ b/providers/mlx5/verbs.c
@@ -2674,6 +2674,57 @@  free:
 	return 0;
 }
 
+static int query_dct_in_order(struct ibv_qp *qp)
+{
+	uint32_t in_dct[DEVX_ST_SZ_DW(query_dct_in)] = {};
+	uint32_t out_dct[DEVX_ST_SZ_DW(query_dct_out)] = {};
+	int ret;
+
+	DEVX_SET(query_dct_in, in_dct, opcode, MLX5_CMD_OP_QUERY_DCT);
+	DEVX_SET(query_dct_in, in_dct, dctn, qp->qp_num);
+	ret = mlx5dv_devx_qp_query(qp, in_dct, sizeof(in_dct), out_dct,
+				   sizeof(out_dct));
+	if (ret)
+		return 0;
+
+	return DEVX_GET(query_dct_out, out_dct, dctc.data_in_order);
+}
+
+int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
+				uint32_t flags)
+{
+	uint32_t in_qp[DEVX_ST_SZ_DW(query_qp_in)] = {};
+	uint32_t out_qp[DEVX_ST_SZ_DW(query_qp_out)] = {};
+	struct mlx5_context *mctx = to_mctx(qp->context);
+	struct mlx5_qp *mqp = to_mqp(qp);
+	int ret;
+
+/* Currently this API is only supported for x86 architectures since most
+ * non-x86 platforms are known to be OOO and need to do a per-platform study.
+ */
+#if !defined(__i386__) && !defined(__x86_64__)
+	return 0;
+#endif
+
+	if (flags || !mctx->qp_data_in_order_cap)
+		return 0;
+
+	if (mqp->dc_type == MLX5DV_DCTYPE_DCT)
+		return query_dct_in_order(qp);
+
+	if (qp->state != IBV_QPS_RTS)
+		return 0;
+
+	DEVX_SET(query_qp_in, in_qp, opcode, MLX5_CMD_OP_QUERY_QP);
+	DEVX_SET(query_qp_in, in_qp, qpn, qp->qp_num);
+	ret = mlx5dv_devx_qp_query(qp, in_qp, sizeof(in_qp), out_qp,
+				   sizeof(out_qp));
+	if (ret)
+		return 0;
+
+	return DEVX_GET(query_qp_out, out_qp, qpc.data_in_order);
+}
+
 int mlx5_query_qp(struct ibv_qp *ibqp, struct ibv_qp_attr *attr,
 		  int attr_mask, struct ibv_qp_init_attr *init_attr)
 {
@@ -3584,6 +3635,10 @@  static void get_hca_general_caps(struct mlx5_context *mctx)
 	if (ret)
 		return;
 
+	mctx->qp_data_in_order_cap =
+		DEVX_GET(query_hca_cap_out, out,
+			 capability.cmd_hca_cap.qp_data_in_order);
+
 	mctx->entropy_caps.num_lag_ports =
 		DEVX_GET(query_hca_cap_out, out,
 			 capability.cmd_hca_cap.num_lag_ports);