diff mbox series

[6/9] providers: Remove normal query_device() from providers that have _ex

Message ID 6-v1-34e141ddf17e+89-query_device_ex_jgg@nvidia.com (mailing list archive)
State Not Applicable
Headers show
Series Simplify query_device() in libibverbs | expand

Commit Message

Jason Gunthorpe Nov. 16, 2020, 8:23 p.m. UTC 
The ex callback can implement both versions, no reason for duplicate
code in two paths. Have the core code call the _ex version instead.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 libibverbs/dummy_ops.c |  9 ++++-
 providers/efa/efa.c    |  1 -
 providers/efa/verbs.c  | 36 +++++---------------
 providers/efa/verbs.h  |  1 -
 providers/mlx4/mlx4.c  |  1 -
 providers/mlx4/mlx4.h  |  2 --
 providers/mlx4/verbs.c | 45 +++++++++----------------
 providers/mlx5/mlx5.c  |  1 -
 providers/mlx5/mlx5.h  |  2 --
 providers/mlx5/verbs.c | 75 +++++++++++++++++-------------------------
 10 files changed, 61 insertions(+), 112 deletions(-)

Comments

Gal Pressman Nov. 18, 2020, 12:47 p.m. UTC | #1 
On 16/11/2020 22:23, Jason Gunthorpe wrote:
> The ex callback can implement both versions, no reason for duplicate
> code in two paths. Have the core code call the _ex version instead.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

Reviewed-by: Gal Pressman <galpress@amazon.com>
diff mbox series

Patch

diff --git a/libibverbs/dummy_ops.c b/libibverbs/dummy_ops.c
index e5af9e4eac8e34..711dfafb5caed5 100644
--- a/libibverbs/dummy_ops.c
+++ b/libibverbs/dummy_ops.c
@@ -380,7 +380,14 @@  static int post_srq_recv(struct ibv_srq *srq, struct ibv_recv_wr *recv_wr,
 static int query_device(struct ibv_context *context,
 			struct ibv_device_attr *device_attr)
 {
-	return EOPNOTSUPP;
+	const struct verbs_context_ops *ops = get_ops(context);
+
+	if (!ops->query_device_ex)
+		return EOPNOTSUPP;
+	return ops->query_device_ex(
+		context, NULL,
+		container_of(device_attr, struct ibv_device_attr_ex, orig_attr),
+		sizeof(*device_attr));
 }
 
 /* Provide a generic implementation for all providers that don't implement
diff --git a/providers/efa/efa.c b/providers/efa/efa.c
index b24c14f7fa1fe1..f6d314dad51e58 100644
--- a/providers/efa/efa.c
+++ b/providers/efa/efa.c
@@ -40,7 +40,6 @@  static const struct verbs_context_ops efa_ctx_ops = {
 	.poll_cq = efa_poll_cq,
 	.post_recv = efa_post_recv,
 	.post_send = efa_post_send,
-	.query_device = efa_query_device,
 	.query_device_ex = efa_query_device_ex,
 	.query_port = efa_query_port,
 	.query_qp = efa_query_qp,
diff --git a/providers/efa/verbs.c b/providers/efa/verbs.c
index 52d6285f1f409c..d50206c13d4295 100644
--- a/providers/efa/verbs.c
+++ b/providers/efa/verbs.c
@@ -56,27 +56,6 @@  struct efa_wq_init_attr {
 	uint16_t sub_cq_idx;
 };
 
-int efa_query_device(struct ibv_context *ibvctx,
-		     struct ibv_device_attr *dev_attr)
-{
-	struct efa_context *ctx = to_efa_context(ibvctx);
-	struct ibv_query_device cmd;
-	uint8_t fw_ver[8];
-	int err;
-
-	err = ibv_cmd_query_device(ibvctx, dev_attr, (uint64_t *)&fw_ver,
-				   &cmd, sizeof(cmd));
-	if (err)
-		return err;
-
-	dev_attr->max_qp_wr = min_t(int, dev_attr->max_qp_wr,
-				    ctx->max_llq_size / sizeof(struct efa_io_tx_wqe));
-	snprintf(dev_attr->fw_ver, sizeof(dev_attr->fw_ver), "%u.%u.%u.%u",
-		 fw_ver[0], fw_ver[1], fw_ver[2], fw_ver[3]);
-
-	return 0;
-}
-
 int efa_query_port(struct ibv_context *ibvctx, uint8_t port,
 		   struct ibv_port_attr *port_attr)
 {
@@ -91,23 +70,24 @@  int efa_query_device_ex(struct ibv_context *context,
 			size_t attr_size)
 {
 	struct efa_context *ctx = to_efa_context(context);
-	int cmd_supp_uhw = ctx->cmds_supp_udata_mask &
-			   EFA_USER_CMDS_SUPP_UDATA_QUERY_DEVICE;
 	struct ibv_device_attr *a = &attr->orig_attr;
 	struct efa_query_device_ex_resp resp = {};
-	struct ibv_query_device_ex cmd = {};
+	size_t resp_size = (ctx->cmds_supp_udata_mask &
+			    EFA_USER_CMDS_SUPP_UDATA_QUERY_DEVICE) ?
+				   sizeof(resp) :
+				   sizeof(resp.ibv_resp);
 	uint8_t fw_ver[8];
 	int err;
 
-	err = ibv_cmd_query_device_ex(
-		context, input, attr, attr_size, (uint64_t *)&fw_ver, &cmd,
-		sizeof(cmd), &resp.ibv_resp,
-		cmd_supp_uhw ? sizeof(resp) : sizeof(resp.ibv_resp));
+	err = ibv_cmd_query_device_any(context, input, attr, attr_size,
+				       &resp.ibv_resp, &resp_size);
 	if (err)
 		return err;
 
 	a->max_qp_wr = min_t(int, a->max_qp_wr,
 			     ctx->max_llq_size / sizeof(struct efa_io_tx_wqe));
+	memcpy(fw_ver, &resp.ibv_resp.base.fw_ver,
+	       sizeof(resp.ibv_resp.base.fw_ver));
 	snprintf(a->fw_ver, sizeof(a->fw_ver), "%u.%u.%u.%u",
 		 fw_ver[0], fw_ver[1], fw_ver[2], fw_ver[3]);
 
diff --git a/providers/efa/verbs.h b/providers/efa/verbs.h
index 3b0e4e0d498761..b7ae3f0a15c00c 100644
--- a/providers/efa/verbs.h
+++ b/providers/efa/verbs.h
@@ -10,7 +10,6 @@ 
 #include <infiniband/verbs.h>
 
 int efa_query_device_ctx(struct efa_context *ctx);
-int efa_query_device(struct ibv_context *uctx, struct ibv_device_attr *attr);
 int efa_query_port(struct ibv_context *uctx, uint8_t port,
 		   struct ibv_port_attr *attr);
 int efa_query_device_ex(struct ibv_context *context,
diff --git a/providers/mlx4/mlx4.c b/providers/mlx4/mlx4.c
index 619b841d788cb2..1e71cde4a1f9dc 100644
--- a/providers/mlx4/mlx4.c
+++ b/providers/mlx4/mlx4.c
@@ -84,7 +84,6 @@  static const struct verbs_match_ent hca_table[] = {
 };
 
 static const struct verbs_context_ops mlx4_ctx_ops = {
-	.query_device  = mlx4_query_device,
 	.query_port    = mlx4_query_port,
 	.alloc_pd      = mlx4_alloc_pd,
 	.dealloc_pd    = mlx4_free_pd,
diff --git a/providers/mlx4/mlx4.h b/providers/mlx4/mlx4.h
index 3c0787144e7e51..6c6ffc77657463 100644
--- a/providers/mlx4/mlx4.h
+++ b/providers/mlx4/mlx4.h
@@ -305,8 +305,6 @@  void mlx4_free_db(struct mlx4_context *context, enum mlx4_db_type type,
 		  __be32 *db);
 
 void mlx4_query_device_ctx(struct mlx4_device *mdev, struct mlx4_context *mctx);
-int mlx4_query_device(struct ibv_context *context,
-		       struct ibv_device_attr *attr);
 int mlx4_query_device_ex(struct ibv_context *context,
 			 const struct ibv_query_device_ex_input *input,
 			 struct ibv_device_attr_ex *attr,
diff --git a/providers/mlx4/verbs.c b/providers/mlx4/verbs.c
index 4fe5c1d87d6d91..ea8e882bb363ba 100644
--- a/providers/mlx4/verbs.c
+++ b/providers/mlx4/verbs.c
@@ -45,51 +45,36 @@ 
 #include "mlx4.h"
 #include "mlx4-abi.h"
 
-int mlx4_query_device(struct ibv_context *context, struct ibv_device_attr *attr)
-{
-	struct ibv_query_device cmd;
-	uint64_t raw_fw_ver;
-	unsigned major, minor, sub_minor;
-	int ret;
-
-	ret = ibv_cmd_query_device(context, attr, &raw_fw_ver, &cmd, sizeof cmd);
-	if (ret)
-		return ret;
-
-	major     = (raw_fw_ver >> 32) & 0xffff;
-	minor     = (raw_fw_ver >> 16) & 0xffff;
-	sub_minor = raw_fw_ver & 0xffff;
-
-	snprintf(attr->fw_ver, sizeof attr->fw_ver,
-		 "%d.%d.%03d", major, minor, sub_minor);
-
-	return 0;
-}
-
 int mlx4_query_device_ex(struct ibv_context *context,
 			 const struct ibv_query_device_ex_input *input,
 			 struct ibv_device_attr_ex *attr,
 			 size_t attr_size)
 {
-	struct mlx4_query_device_ex_resp resp = {};
-	struct mlx4_query_device_ex cmd = {};
+	struct mlx4_query_device_ex_resp resp;
+	size_t resp_size = sizeof(resp);
 	uint64_t raw_fw_ver;
 	unsigned sub_minor;
 	unsigned major;
 	unsigned minor;
 	int err;
 
-	err = ibv_cmd_query_device_ex(context, input, attr, attr_size,
-				      &raw_fw_ver, &cmd.ibv_cmd, sizeof(cmd),
-				      &resp.ibv_resp, sizeof(resp));
+	err = ibv_cmd_query_device_any(context, input, attr, attr_size,
+				       &resp.ibv_resp, &resp_size);
 	if (err)
 		return err;
 
-	attr->rss_caps.rx_hash_fields_mask = resp.rss_caps.rx_hash_fields_mask;
-	attr->rss_caps.rx_hash_function = resp.rss_caps.rx_hash_function;
-	attr->tso_caps.max_tso = resp.tso_caps.max_tso;
-	attr->tso_caps.supported_qpts = resp.tso_caps.supported_qpts;
+	if (attr_size >= offsetofend(struct ibv_device_attr_ex, rss_caps)) {
+		attr->rss_caps.rx_hash_fields_mask =
+			resp.rss_caps.rx_hash_fields_mask;
+		attr->rss_caps.rx_hash_function =
+			resp.rss_caps.rx_hash_function;
+	}
+	if (attr_size >= offsetofend(struct ibv_device_attr_ex, tso_caps)) {
+		attr->tso_caps.max_tso = resp.tso_caps.max_tso;
+		attr->tso_caps.supported_qpts = resp.tso_caps.supported_qpts;
+	}
 
+	raw_fw_ver = resp.ibv_resp.base.fw_ver;
 	major     = (raw_fw_ver >> 32) & 0xffff;
 	minor     = (raw_fw_ver >> 16) & 0xffff;
 	sub_minor = raw_fw_ver & 0xffff;
diff --git a/providers/mlx5/mlx5.c b/providers/mlx5/mlx5.c
index 06b9a52ebb3019..cf0a62928705bc 100644
--- a/providers/mlx5/mlx5.c
+++ b/providers/mlx5/mlx5.c
@@ -90,7 +90,6 @@  uint32_t mlx5_debug_mask = 0;
 int mlx5_freeze_on_error_cqe;
 
 static const struct verbs_context_ops mlx5_ctx_common_ops = {
-	.query_device  = mlx5_query_device,
 	.query_port    = mlx5_query_port,
 	.alloc_pd      = mlx5_alloc_pd,
 	.async_event   = mlx5_async_event,
diff --git a/providers/mlx5/mlx5.h b/providers/mlx5/mlx5.h
index 72e710b7b5e4aa..8821015c6d503e 100644
--- a/providers/mlx5/mlx5.h
+++ b/providers/mlx5/mlx5.h
@@ -879,8 +879,6 @@  void mlx5_free_db(struct mlx5_context *context, __be32 *db, struct ibv_pd *pd,
 		  bool custom_alloc);
 
 void mlx5_query_device_ctx(struct mlx5_context *mctx);
-int mlx5_query_device(struct ibv_context *context,
-		       struct ibv_device_attr *attr);
 int mlx5_query_device_ex(struct ibv_context *context,
 			 const struct ibv_query_device_ex_input *input,
 			 struct ibv_device_attr_ex *attr,
diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c
index 42c984033d8eaa..5882e209b06b54 100644
--- a/providers/mlx5/verbs.c
+++ b/providers/mlx5/verbs.c
@@ -65,27 +65,6 @@  static inline int is_xrc_tgt(int type)
 	return type == IBV_QPT_XRC_RECV;
 }
 
-int mlx5_query_device(struct ibv_context *context, struct ibv_device_attr *attr)
-{
-	struct ibv_query_device cmd;
-	uint64_t raw_fw_ver;
-	unsigned major, minor, sub_minor;
-	int ret;
-
-	ret = ibv_cmd_query_device(context, attr, &raw_fw_ver, &cmd, sizeof cmd);
-	if (ret)
-		return ret;
-
-	major     = (raw_fw_ver >> 32) & 0xffff;
-	minor     = (raw_fw_ver >> 16) & 0xffff;
-	sub_minor = raw_fw_ver & 0xffff;
-
-	snprintf(attr->fw_ver, sizeof attr->fw_ver,
-		 "%d.%d.%04d", major, minor, sub_minor);
-
-	return 0;
-}
-
 static int mlx5_read_clock(struct ibv_context *context, uint64_t *cycles)
 {
 	unsigned int clockhi, clocklo, clockhi1;
@@ -3481,37 +3460,47 @@  int mlx5_query_device_ex(struct ibv_context *context,
 			 size_t attr_size)
 {
 	struct mlx5_context *mctx = to_mctx(context);
-	struct mlx5_query_device_ex_resp resp;
-	struct mlx5_query_device_ex cmd;
+	struct mlx5_query_device_ex_resp resp = {};
+	size_t resp_size =
+		(mctx->cmds_supp_uhw & MLX5_USER_CMDS_SUPP_UHW_QUERY_DEVICE) ?
+			sizeof(resp) :
+			sizeof(resp.ibv_resp);
 	struct ibv_device_attr *a;
 	uint64_t raw_fw_ver;
 	unsigned sub_minor;
 	unsigned major;
 	unsigned minor;
 	int err;
-	int cmd_supp_uhw = mctx->cmds_supp_uhw &
-		MLX5_USER_CMDS_SUPP_UHW_QUERY_DEVICE;
 
-	memset(&cmd, 0, sizeof(cmd));
-	memset(&resp, 0, sizeof(resp));
-	err = ibv_cmd_query_device_ex(
-		context, input, attr, attr_size, &raw_fw_ver, &cmd.ibv_cmd,
-		sizeof(cmd), &resp.ibv_resp,
-		cmd_supp_uhw ? sizeof(resp) : sizeof(resp.ibv_resp));
+	err = ibv_cmd_query_device_any(context, input, attr, attr_size,
+				       &resp.ibv_resp, &resp_size);
 	if (err)
 		return err;
 
-	attr->tso_caps.max_tso = resp.tso_caps.max_tso;
-	attr->tso_caps.supported_qpts = resp.tso_caps.supported_qpts;
-	attr->rss_caps.rx_hash_fields_mask = resp.rss_caps.rx_hash_fields_mask;
-	attr->rss_caps.rx_hash_function = resp.rss_caps.rx_hash_function;
-	attr->packet_pacing_caps.qp_rate_limit_min =
-		resp.packet_pacing_caps.qp_rate_limit_min;
-	attr->packet_pacing_caps.qp_rate_limit_max =
-		resp.packet_pacing_caps.qp_rate_limit_max;
-	attr->packet_pacing_caps.supported_qpts =
-		resp.packet_pacing_caps.supported_qpts;
+	if (attr_size >= offsetofend(struct ibv_device_attr_ex, tso_caps)) {
+		attr->tso_caps.max_tso = resp.tso_caps.max_tso;
+		attr->tso_caps.supported_qpts = resp.tso_caps.supported_qpts;
+	}
+	if (attr_size >= offsetofend(struct ibv_device_attr_ex, rss_caps)) {
+		attr->rss_caps.rx_hash_fields_mask =
+			resp.rss_caps.rx_hash_fields_mask;
+		attr->rss_caps.rx_hash_function =
+			resp.rss_caps.rx_hash_function;
+	}
+	if (attr_size >=
+	    offsetofend(struct ibv_device_attr_ex, packet_pacing_caps)) {
+		attr->packet_pacing_caps.qp_rate_limit_min =
+			resp.packet_pacing_caps.qp_rate_limit_min;
+		attr->packet_pacing_caps.qp_rate_limit_max =
+			resp.packet_pacing_caps.qp_rate_limit_max;
+		attr->packet_pacing_caps.supported_qpts =
+			resp.packet_pacing_caps.supported_qpts;
+	}
+
+	if (attr_size >= offsetofend(struct ibv_device_attr_ex, pci_atomic_caps))
+		get_pci_atomic_caps(context, attr);
 
+	raw_fw_ver = resp.ibv_resp.base.fw_ver;
 	major     = (raw_fw_ver >> 32) & 0xffff;
 	minor     = (raw_fw_ver >> 16) & 0xffff;
 	sub_minor = raw_fw_ver & 0xffff;
@@ -3519,10 +3508,6 @@  int mlx5_query_device_ex(struct ibv_context *context,
 	snprintf(a->fw_ver, sizeof(a->fw_ver), "%d.%d.%04d",
 		 major, minor, sub_minor);
 
-	if (attr_size >= offsetof(struct ibv_device_attr_ex, pci_atomic_caps) +
-			sizeof(attr->pci_atomic_caps))
-		get_pci_atomic_caps(context, attr);
-
 	return 0;
 }