[PATCH for-next 0/4] RDMA/bnxt_re: Debug enhancements for bnxt_re driver

[PATCH for-next 0/4] RDMA/bnxt_re: Debug enhancements for bnxt_re driver

[Selvin Xavier]

This series is the first set that enables few debug
options in bnxt_re driver. Implements the basic driver
data collection using the rdma tool. Also, implements
raw data query option to get some of the Hardware specific
information for analysis. Added a debugfs folder for bnxt_re
corresponding to each PCI device to display some of the
driver specific information. This will be enhanced in future
series.

Please review and apply.

Thanks,
Selvin Xavier


Kalesh AP (1):
  RDMA/bnxt_re: Add debugfs hook in the driver

Kashyap Desai (3):
  RDMA/bnxt_re: Support driver specific data collection using rdma tool
  RDMA/bnxt_re: Add support for querying HW contexts
  RDMA/bnxt_re: Support raw data query for each resources

 drivers/infiniband/hw/bnxt_re/Makefile     |   3 +-
 drivers/infiniband/hw/bnxt_re/bnxt_re.h    |  21 +++
 drivers/infiniband/hw/bnxt_re/debugfs.c    | 141 +++++++++++++++
 drivers/infiniband/hw/bnxt_re/debugfs.h    |  21 +++
 drivers/infiniband/hw/bnxt_re/ib_verbs.c   |   4 +
 drivers/infiniband/hw/bnxt_re/ib_verbs.h   |   1 +
 drivers/infiniband/hw/bnxt_re/main.c       | 279 ++++++++++++++++++++++++++++-
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |   2 +
 drivers/infiniband/hw/bnxt_re/qplib_sp.c   |  35 ++++
 drivers/infiniband/hw/bnxt_re/qplib_sp.h   |   2 +
 drivers/infiniband/hw/bnxt_re/roce_hsi.h   |  40 +++++
 11 files changed, 547 insertions(+), 2 deletions(-)
 create mode 100644 drivers/infiniband/hw/bnxt_re/debugfs.c
 create mode 100644 drivers/infiniband/hw/bnxt_re/debugfs.h

-- 
2.5.5

[PATCH for-next 1/4] RDMA/bnxt_re: Support driver specific data collection using rdma tool

[Selvin Xavier]

From: Kashyap Desai <kashyap.desai@broadcom.com>

Allow users to dump driver specific resource details when
queried through rdma tool. This supports the driver data
for QP, CQ, MR and SRQ.

Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/main.c | 148 +++++++++++++++++++++++++++++++++++
 1 file changed, 148 insertions(+)

diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 6715c96..5bed9af 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -882,6 +882,146 @@ static const struct attribute_group bnxt_re_dev_attr_group = {
 	.attrs = bnxt_re_attributes,
 };
 
+static int bnxt_re_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ib_mr)
+{
+	struct bnxt_qplib_hwq *mr_hwq;
+	struct nlattr *table_attr;
+	struct bnxt_re_mr *mr;
+
+	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_DRIVER);
+	if (!table_attr)
+		return -EMSGSIZE;
+
+	mr = container_of(ib_mr, struct bnxt_re_mr, ib_mr);
+	mr_hwq = &mr->qplib_mr.hwq;
+
+	if (rdma_nl_put_driver_string(msg, "owner",
+				      mr_hwq->is_user ?  "user" : "kernel"))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "page_size",
+				   mr_hwq->qe_ppg * mr_hwq->element_size))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "max_elements", mr_hwq->max_elements))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "element_size", mr_hwq->element_size))
+		goto err;
+	if (rdma_nl_put_driver_u64_hex(msg, "hwq", (unsigned long)mr_hwq))
+		goto err;
+	if (rdma_nl_put_driver_u64_hex(msg, "va", mr->qplib_mr.va))
+		goto err;
+
+	nla_nest_end(msg, table_attr);
+	return 0;
+
+err:
+	nla_nest_cancel(msg, table_attr);
+	return -EMSGSIZE;
+}
+
+static int bnxt_re_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ib_cq)
+{
+	struct bnxt_qplib_hwq *cq_hwq;
+	struct nlattr *table_attr;
+	struct bnxt_re_cq *cq;
+
+	cq = container_of(ib_cq, struct bnxt_re_cq, ib_cq);
+	cq_hwq = &cq->qplib_cq.hwq;
+
+	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_DRIVER);
+	if (!table_attr)
+		return -EMSGSIZE;
+
+	if (rdma_nl_put_driver_u32(msg, "cq_depth", cq_hwq->depth))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "max_elements", cq_hwq->max_elements))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "element_size", cq_hwq->element_size))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "max_wqe", cq->qplib_cq.max_wqe))
+		goto err;
+
+	nla_nest_end(msg, table_attr);
+	return 0;
+
+err:
+	nla_nest_cancel(msg, table_attr);
+	return -EMSGSIZE;
+}
+
+static int bnxt_re_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ib_qp)
+{
+	struct bnxt_qplib_qp *qplib_qp;
+	struct nlattr *table_attr;
+	struct bnxt_re_qp *qp;
+
+	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_DRIVER);
+	if (!table_attr)
+		return -EMSGSIZE;
+
+	qp = container_of(ib_qp, struct bnxt_re_qp, ib_qp);
+	qplib_qp = &qp->qplib_qp;
+
+	if (rdma_nl_put_driver_string(msg, "owner",
+				      ib_qp->uobject ?  "user" : "kernel"))
+		goto err;
+
+	if (rdma_nl_put_driver_u32(msg, "sq_max_wqe", qplib_qp->sq.max_wqe))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "sq_max_sge", qplib_qp->sq.max_sge))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "sq_wqe_size", qplib_qp->sq.wqe_size))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "sq_swq_start", qplib_qp->sq.swq_start))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "sq_swq_last", qplib_qp->sq.swq_last))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "rq_max_wqe", qplib_qp->rq.max_wqe))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "rq_max_sge", qplib_qp->rq.max_sge))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "rq_wqe_size", qplib_qp->rq.wqe_size))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "rq_swq_start", qplib_qp->rq.swq_start))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "rq_swq_last", qplib_qp->rq.swq_last))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "timeout", qplib_qp->timeout))
+		goto err;
+
+	nla_nest_end(msg, table_attr);
+	return 0;
+
+err:
+	nla_nest_cancel(msg, table_attr);
+	return -EMSGSIZE;
+}
+
+static int bnxt_re_fill_res_srq_entry(struct sk_buff *msg, struct ib_srq *ib_srq)
+{
+	struct nlattr *table_attr;
+	struct bnxt_re_srq *srq;
+
+	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_DRIVER);
+	if (!table_attr)
+		return -EMSGSIZE;
+
+	srq = container_of(ib_srq, struct bnxt_re_srq, ib_srq);
+
+	if (rdma_nl_put_driver_u32_hex(msg, "wqe_size", srq->qplib_srq.wqe_size))
+		goto err;
+	if (rdma_nl_put_driver_u32_hex(msg, "max_wqe", srq->qplib_srq.max_wqe))
+		goto err;
+	if (rdma_nl_put_driver_u32_hex(msg, "max_sge", srq->qplib_srq.max_sge))
+		goto err;
+
+	nla_nest_end(msg, table_attr);
+	return 0;
+
+err:
+	nla_nest_cancel(msg, table_attr);
+	return -EMSGSIZE;
+}
+
 static const struct ib_device_ops bnxt_re_dev_ops = {
 	.owner = THIS_MODULE,
 	.driver_id = RDMA_DRIVER_BNXT_RE,
@@ -939,6 +1079,13 @@ static const struct ib_device_ops bnxt_re_dev_ops = {
 	INIT_RDMA_OBJ_SIZE(ib_ucontext, bnxt_re_ucontext, ib_uctx),
 };
 
+static const struct ib_device_ops restrack_ops = {
+	.fill_res_cq_entry = bnxt_re_fill_res_cq_entry,
+	.fill_res_qp_entry = bnxt_re_fill_res_qp_entry,
+	.fill_res_mr_entry = bnxt_re_fill_res_mr_entry,
+	.fill_res_srq_entry = bnxt_re_fill_res_srq_entry,
+};
+
 static int bnxt_re_register_ib(struct bnxt_re_dev *rdev)
 {
 	struct ib_device *ibdev = &rdev->ibdev;
@@ -960,6 +1107,7 @@ static int bnxt_re_register_ib(struct bnxt_re_dev *rdev)
 		ibdev->driver_def = bnxt_re_uapi_defs;
 
 	ib_set_device_ops(ibdev, &bnxt_re_dev_ops);
+	ib_set_device_ops(ibdev, &restrack_ops);
 	ret = ib_device_set_netdev(&rdev->ibdev, rdev->netdev, 1);
 	if (ret)
 		return ret;
-- 
2.5.5

[PATCH for-next 2/4] RDMA/bnxt_re: Add support for querying HW contexts

[Selvin Xavier]

From: Kashyap Desai <kashyap.desai@broadcom.com>

Implements support for querying the hardware resource
contexts. This raw data can be used for the debugging
of the field issues.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/bnxt_re.h    | 19 ++++++++++++++
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  2 ++
 drivers/infiniband/hw/bnxt_re/qplib_sp.c   | 35 ++++++++++++++++++++++++++
 drivers/infiniband/hw/bnxt_re/qplib_sp.h   |  2 ++
 drivers/infiniband/hw/bnxt_re/roce_hsi.h   | 40 ++++++++++++++++++++++++++++++
 5 files changed, 98 insertions(+)

diff --git a/drivers/infiniband/hw/bnxt_re/bnxt_re.h b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
index e94518b..311bb7c 100644
--- a/drivers/infiniband/hw/bnxt_re/bnxt_re.h
+++ b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
@@ -239,4 +239,23 @@ static inline void bnxt_re_set_pacing_dev_state(struct bnxt_re_dev *rdev)
 	rdev->qplib_res.pacing_data->dev_err_state =
 		test_bit(BNXT_RE_FLAG_ERR_DEVICE_DETACHED, &rdev->flags);
 }
+
+static inline int bnxt_re_read_context_allowed(struct bnxt_re_dev *rdev)
+{
+	if (!bnxt_qplib_is_chip_gen_p5_p7(rdev->chip_ctx) ||
+	    rdev->rcfw.res->cctx->hwrm_intf_ver < HWRM_VERSION_READ_CTX)
+		return -EOPNOTSUPP;
+	return 0;
+}
+
+#define BNXT_RE_CONTEXT_TYPE_QPC_SIZE_P5	1088
+#define BNXT_RE_CONTEXT_TYPE_CQ_SIZE_P5		128
+#define BNXT_RE_CONTEXT_TYPE_MRW_SIZE_P5	128
+#define BNXT_RE_CONTEXT_TYPE_SRQ_SIZE_P5	192
+
+#define BNXT_RE_CONTEXT_TYPE_QPC_SIZE_P7	1088
+#define BNXT_RE_CONTEXT_TYPE_CQ_SIZE_P7		192
+#define BNXT_RE_CONTEXT_TYPE_MRW_SIZE_P7	192
+#define BNXT_RE_CONTEXT_TYPE_SRQ_SIZE_P7	192
+
 #endif
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
index 45996e6..3e723d7 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
@@ -131,6 +131,8 @@ static inline u32 bnxt_qplib_set_cmd_slots(struct cmdq_base *req)
 #define RCFW_CMD_IS_BLOCKING		0x8000
 
 #define HWRM_VERSION_DEV_ATTR_MAX_DPI  0x1000A0000000DULL
+/* HWRM version 1.10.3.18 */
+#define HWRM_VERSION_READ_CTX           0x1000A00030012
 
 /* Crsq buf is 1024-Byte */
 struct bnxt_qplib_crsbe {
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_sp.c b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
index e29fbbd..7e20ae3 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_sp.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
@@ -981,3 +981,38 @@ int bnxt_qplib_modify_cc(struct bnxt_qplib_res *res,
 	rc = bnxt_qplib_rcfw_send_message(res->rcfw, &msg);
 	return rc;
 }
+
+int bnxt_qplib_read_context(struct bnxt_qplib_rcfw *rcfw, u8 res_type,
+			    u32 xid, u32 resp_size, void *resp_va)
+{
+	struct creq_read_context resp = {};
+	struct bnxt_qplib_cmdqmsg msg = {};
+	struct cmdq_read_context req = {};
+	struct bnxt_qplib_rcfw_sbuf sbuf;
+	int rc;
+
+	sbuf.size = resp_size;
+	sbuf.sb = dma_alloc_coherent(&rcfw->pdev->dev, sbuf.size,
+				     &sbuf.dma_addr, GFP_KERNEL);
+	if (!sbuf.sb)
+		return -ENOMEM;
+
+	bnxt_qplib_rcfw_cmd_prep((struct cmdq_base *)&req,
+				 CMDQ_BASE_OPCODE_READ_CONTEXT, sizeof(req));
+	req.resp_addr = cpu_to_le64(sbuf.dma_addr);
+	req.resp_size = resp_size / BNXT_QPLIB_CMDQE_UNITS;
+
+	req.xid = cpu_to_le32(xid);
+	req.type = res_type;
+
+	bnxt_qplib_fill_cmdqmsg(&msg, &req, &resp, &sbuf, sizeof(req),
+				sizeof(resp), 0);
+	rc = bnxt_qplib_rcfw_send_message(rcfw, &msg);
+	if (rc)
+		goto free_mem;
+
+	memcpy(resp_va, sbuf.sb, resp_size);
+free_mem:
+	dma_free_coherent(&rcfw->pdev->dev, sbuf.size, sbuf.sb, sbuf.dma_addr);
+	return rc;
+}
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_sp.h b/drivers/infiniband/hw/bnxt_re/qplib_sp.h
index ecf3f45..e6beeb5 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_sp.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_sp.h
@@ -353,6 +353,8 @@ int bnxt_qplib_qext_stat(struct bnxt_qplib_rcfw *rcfw, u32 fid,
 			 struct bnxt_qplib_ext_stat *estat);
 int bnxt_qplib_modify_cc(struct bnxt_qplib_res *res,
 			 struct bnxt_qplib_cc_param *cc_param);
+int bnxt_qplib_read_context(struct bnxt_qplib_rcfw *rcfw, u8 type, u32 xid,
+			    u32 resp_size, void *resp_va);
 
 #define BNXT_VAR_MAX_WQE       4352
 #define BNXT_VAR_MAX_SLOT_ALIGN 256
diff --git a/drivers/infiniband/hw/bnxt_re/roce_hsi.h b/drivers/infiniband/hw/bnxt_re/roce_hsi.h
index 3ec8952..57f19f6 100644
--- a/drivers/infiniband/hw/bnxt_re/roce_hsi.h
+++ b/drivers/infiniband/hw/bnxt_re/roce_hsi.h
@@ -2251,6 +2251,46 @@ struct creq_set_func_resources_resp {
 	u8	reserved48[6];
 };
 
+/* cmdq_read_context (size:192b/24B) */
+struct cmdq_read_context {
+	u8	opcode;
+	#define CMDQ_READ_CONTEXT_OPCODE_READ_CONTEXT 0x85UL
+	#define CMDQ_READ_CONTEXT_OPCODE_LAST        CMDQ_READ_CONTEXT_OPCODE_READ_CONTEXT
+	u8	cmd_size;
+	__le16	flags;
+	__le16	cookie;
+	u8	resp_size;
+	u8	reserved8;
+	__le64	resp_addr;
+	__le32	xid;
+	u8	type;
+	#define CMDQ_READ_CONTEXT_TYPE_QPC 0x0UL
+	#define CMDQ_READ_CONTEXT_TYPE_CQ  0x1UL
+	#define CMDQ_READ_CONTEXT_TYPE_MRW 0x2UL
+	#define CMDQ_READ_CONTEXT_TYPE_SRQ 0x3UL
+	#define CMDQ_READ_CONTEXT_TYPE_LAST CMDQ_READ_CONTEXT_TYPE_SRQ
+	u8	unused_0[3];
+};
+
+/* creq_read_context (size:128b/16B) */
+struct creq_read_context {
+	u8	type;
+	#define CREQ_READ_CONTEXT_TYPE_MASK    0x3fUL
+	#define CREQ_READ_CONTEXT_TYPE_SFT     0
+	#define CREQ_READ_CONTEXT_TYPE_QP_EVENT  0x38UL
+	#define CREQ_READ_CONTEXT_TYPE_LAST     CREQ_READ_CONTEXT_TYPE_QP_EVENT
+	u8	status;
+	__le16	cookie;
+	__le32	reserved32;
+	u8	v;
+	#define CREQ_READ_CONTEXT_V     0x1UL
+	u8	event;
+	#define CREQ_READ_CONTEXT_EVENT_READ_CONTEXT 0x85UL
+	#define CREQ_READ_CONTEXT_EVENT_LAST        CREQ_READ_CONTEXT_EVENT_READ_CONTEXT
+	__le16	reserved16;
+	__le32	reserved_32;
+};
+
 /* cmdq_map_tc_to_cos (size:192b/24B) */
 struct cmdq_map_tc_to_cos {
 	u8	opcode;
-- 
2.5.5

[PATCH for-next 3/4] RDMA/bnxt_re: Support raw data query for each resources

[Selvin Xavier]

From: Kashyap Desai <kashyap.desai@broadcom.com>

Support interfaces to get the raw data for each of
the resources. Use this interface to get some of the
HW structures from active resources.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/main.c | 118 +++++++++++++++++++++++++++++++++++
 1 file changed, 118 insertions(+)

diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 5bed9af..0a18d24 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -918,6 +918,35 @@ static int bnxt_re_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ib_mr)
 	return -EMSGSIZE;
 }
 
+static int bnxt_re_fill_res_mr_entry_raw(struct sk_buff *msg, struct ib_mr *ib_mr)
+{
+	struct bnxt_re_dev *rdev;
+	struct bnxt_re_mr *mr;
+	int err, len;
+	void *data;
+
+	mr = container_of(ib_mr, struct bnxt_re_mr, ib_mr);
+	rdev = mr->rdev;
+
+	err = bnxt_re_read_context_allowed(rdev);
+	if (err)
+		return err;
+
+	len = bnxt_qplib_is_chip_gen_p7(rdev->chip_ctx) ? BNXT_RE_CONTEXT_TYPE_MRW_SIZE_P7 :
+							  BNXT_RE_CONTEXT_TYPE_MRW_SIZE_P5;
+	data = kzalloc(len, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	err = bnxt_qplib_read_context(&rdev->rcfw, CMDQ_READ_CONTEXT_TYPE_MRW,
+				      mr->qplib_mr.lkey, len, data);
+	if (!err)
+		err = nla_put(msg, RDMA_NLDEV_ATTR_RES_RAW, len, data);
+
+	kfree(data);
+	return err;
+}
+
 static int bnxt_re_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ib_cq)
 {
 	struct bnxt_qplib_hwq *cq_hwq;
@@ -948,6 +977,36 @@ static int bnxt_re_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ib_cq)
 	return -EMSGSIZE;
 }
 
+static int bnxt_re_fill_res_cq_entry_raw(struct sk_buff *msg, struct ib_cq *ib_cq)
+{
+	struct bnxt_re_dev *rdev;
+	struct bnxt_re_cq *cq;
+	int err, len;
+	void *data;
+
+	cq = container_of(ib_cq, struct bnxt_re_cq, ib_cq);
+	rdev = cq->rdev;
+
+	err = bnxt_re_read_context_allowed(rdev);
+	if (err)
+		return err;
+
+	len = bnxt_qplib_is_chip_gen_p7(rdev->chip_ctx) ? BNXT_RE_CONTEXT_TYPE_CQ_SIZE_P7 :
+					BNXT_RE_CONTEXT_TYPE_CQ_SIZE_P5;
+	data = kzalloc(len, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	err = bnxt_qplib_read_context(&rdev->rcfw,
+				      CMDQ_READ_CONTEXT_TYPE_CQ,
+				      cq->qplib_cq.id, len, data);
+	if (!err)
+		err = nla_put(msg, RDMA_NLDEV_ATTR_RES_RAW, len, data);
+
+	kfree(data);
+	return err;
+}
+
 static int bnxt_re_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ib_qp)
 {
 	struct bnxt_qplib_qp *qplib_qp;
@@ -996,6 +1055,31 @@ static int bnxt_re_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ib_qp)
 	return -EMSGSIZE;
 }
 
+static int bnxt_re_fill_res_qp_entry_raw(struct sk_buff *msg, struct ib_qp *ibqp)
+{
+	struct bnxt_re_dev *rdev = to_bnxt_re_dev(ibqp->device, ibdev);
+	int err, len;
+	void *data;
+
+	err = bnxt_re_read_context_allowed(rdev);
+	if (err)
+		return err;
+
+	len = bnxt_qplib_is_chip_gen_p7(rdev->chip_ctx) ? BNXT_RE_CONTEXT_TYPE_QPC_SIZE_P7 :
+							  BNXT_RE_CONTEXT_TYPE_QPC_SIZE_P5;
+	data = kzalloc(len, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	err = bnxt_qplib_read_context(&rdev->rcfw, CMDQ_READ_CONTEXT_TYPE_QPC,
+				      ibqp->qp_num, len, data);
+	if (!err)
+		err = nla_put(msg, RDMA_NLDEV_ATTR_RES_RAW, len, data);
+
+	kfree(data);
+	return err;
+}
+
 static int bnxt_re_fill_res_srq_entry(struct sk_buff *msg, struct ib_srq *ib_srq)
 {
 	struct nlattr *table_attr;
@@ -1022,6 +1106,36 @@ static int bnxt_re_fill_res_srq_entry(struct sk_buff *msg, struct ib_srq *ib_srq
 	return -EMSGSIZE;
 }
 
+static int bnxt_re_fill_res_srq_entry_raw(struct sk_buff *msg, struct ib_srq *ib_srq)
+{
+	struct bnxt_re_dev *rdev;
+	struct bnxt_re_srq *srq;
+	int err, len;
+	void *data;
+
+	srq = container_of(ib_srq, struct bnxt_re_srq, ib_srq);
+	rdev = srq->rdev;
+
+	err = bnxt_re_read_context_allowed(rdev);
+	if (err)
+		return err;
+
+	len = bnxt_qplib_is_chip_gen_p7(rdev->chip_ctx) ? BNXT_RE_CONTEXT_TYPE_SRQ_SIZE_P7 :
+							  BNXT_RE_CONTEXT_TYPE_SRQ_SIZE_P5;
+
+	data = kzalloc(len, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	err = bnxt_qplib_read_context(&rdev->rcfw, CMDQ_READ_CONTEXT_TYPE_SRQ,
+				      srq->qplib_srq.id, len, data);
+	if (!err)
+		err = nla_put(msg, RDMA_NLDEV_ATTR_RES_RAW, len, data);
+
+	kfree(data);
+	return err;
+}
+
 static const struct ib_device_ops bnxt_re_dev_ops = {
 	.owner = THIS_MODULE,
 	.driver_id = RDMA_DRIVER_BNXT_RE,
@@ -1081,9 +1195,13 @@ static const struct ib_device_ops bnxt_re_dev_ops = {
 
 static const struct ib_device_ops restrack_ops = {
 	.fill_res_cq_entry = bnxt_re_fill_res_cq_entry,
+	.fill_res_cq_entry_raw = bnxt_re_fill_res_cq_entry_raw,
 	.fill_res_qp_entry = bnxt_re_fill_res_qp_entry,
+	.fill_res_qp_entry_raw = bnxt_re_fill_res_qp_entry_raw,
 	.fill_res_mr_entry = bnxt_re_fill_res_mr_entry,
+	.fill_res_mr_entry_raw = bnxt_re_fill_res_mr_entry_raw,
 	.fill_res_srq_entry = bnxt_re_fill_res_srq_entry,
+	.fill_res_srq_entry_raw = bnxt_re_fill_res_srq_entry_raw,
 };
 
 static int bnxt_re_register_ib(struct bnxt_re_dev *rdev)
-- 
2.5.5

[PATCH for-next 4/4] RDMA/bnxt_re: Add debugfs hook in the driver

[Selvin Xavier]

From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

Adding support for a per device debugfs folder for exporting
some of the device specific debug information.
Added support to get QP info for now. The same folder can be
used to export other debug features in future.

Signed-off-by: Saravanan Vajravel <saravanan.vajravel@broadcom.com>
Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/Makefile   |   3 +-
 drivers/infiniband/hw/bnxt_re/bnxt_re.h  |   2 +
 drivers/infiniband/hw/bnxt_re/debugfs.c  | 141 +++++++++++++++++++++++++++++++
 drivers/infiniband/hw/bnxt_re/debugfs.h  |  21 +++++
 drivers/infiniband/hw/bnxt_re/ib_verbs.c |   4 +
 drivers/infiniband/hw/bnxt_re/ib_verbs.h |   1 +
 drivers/infiniband/hw/bnxt_re/main.c     |  13 ++-
 7 files changed, 183 insertions(+), 2 deletions(-)
 create mode 100644 drivers/infiniband/hw/bnxt_re/debugfs.c
 create mode 100644 drivers/infiniband/hw/bnxt_re/debugfs.h

diff --git a/drivers/infiniband/hw/bnxt_re/Makefile b/drivers/infiniband/hw/bnxt_re/Makefile
index ee9bb1b..f63417d 100644
--- a/drivers/infiniband/hw/bnxt_re/Makefile
+++ b/drivers/infiniband/hw/bnxt_re/Makefile
@@ -4,4 +4,5 @@ ccflags-y := -I $(srctree)/drivers/net/ethernet/broadcom/bnxt
 obj-$(CONFIG_INFINIBAND_BNXT_RE) += bnxt_re.o
 bnxt_re-y := main.o ib_verbs.o \
 	     qplib_res.o qplib_rcfw.o	\
-	     qplib_sp.o qplib_fp.o  hw_counters.o
+	     qplib_sp.o qplib_fp.o  hw_counters.o	\
+	     debugfs.o
diff --git a/drivers/infiniband/hw/bnxt_re/bnxt_re.h b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
index 311bb7c..7f931c7 100644
--- a/drivers/infiniband/hw/bnxt_re/bnxt_re.h
+++ b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
@@ -213,6 +213,8 @@ struct bnxt_re_dev {
 	struct delayed_work dbq_pacing_work;
 	DECLARE_HASHTABLE(cq_hash, MAX_CQ_HASH_BITS);
 	DECLARE_HASHTABLE(srq_hash, MAX_SRQ_HASH_BITS);
+	struct dentry			*dbg_root;
+	struct dentry			*qp_debugfs;
 };
 
 #define to_bnxt_re_dev(ptr, member)	\
diff --git a/drivers/infiniband/hw/bnxt_re/debugfs.c b/drivers/infiniband/hw/bnxt_re/debugfs.c
new file mode 100644
index 0000000..dd38dd6c
--- /dev/null
+++ b/drivers/infiniband/hw/bnxt_re/debugfs.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+/*
+ * Copyright (c) 2024, Broadcom. All rights reserved.  The term
+ * Broadcom refers to Broadcom Limited and/or its subsidiaries.
+ *
+ * Description: Debugfs component of the bnxt_re driver
+ */
+
+#include <linux/debugfs.h>
+#include <linux/pci.h>
+#include <rdma/ib_addr.h>
+
+#include "bnxt_ulp.h"
+#include "roce_hsi.h"
+#include "qplib_res.h"
+#include "qplib_sp.h"
+#include "qplib_fp.h"
+#include "qplib_rcfw.h"
+#include "bnxt_re.h"
+#include "ib_verbs.h"
+#include "debugfs.h"
+
+static struct dentry *bnxt_re_debugfs_root;
+
+static inline const char *bnxt_re_qp_state_str(u8 state)
+{
+	switch (state) {
+	case CMDQ_MODIFY_QP_NEW_STATE_RESET:
+		return "RST";
+	case CMDQ_MODIFY_QP_NEW_STATE_INIT:
+		return "INIT";
+	case CMDQ_MODIFY_QP_NEW_STATE_RTR:
+		return "RTR";
+	case CMDQ_MODIFY_QP_NEW_STATE_RTS:
+		return "RTS";
+	case CMDQ_MODIFY_QP_NEW_STATE_SQE:
+		return "SQER";
+	case CMDQ_MODIFY_QP_NEW_STATE_SQD:
+		return "SQD";
+	case CMDQ_MODIFY_QP_NEW_STATE_ERR:
+		return "ERR";
+	default:
+		return "Invalid QP state";
+	}
+}
+
+static inline const char *bnxt_re_qp_type_str(u8 type)
+{
+	switch (type) {
+	case CMDQ_CREATE_QP1_TYPE_GSI: return "QP1";
+	case CMDQ_CREATE_QP_TYPE_GSI: return "QP1";
+	case CMDQ_CREATE_QP_TYPE_RC: return "RC";
+	case CMDQ_CREATE_QP_TYPE_UD: return "UD";
+	case CMDQ_CREATE_QP_TYPE_RAW_ETHERTYPE: return "RAW_ETHERTYPE";
+	default: return "Invalid transport type";
+	}
+}
+
+static ssize_t qp_info_read(struct file *filep,
+			    char __user *buffer,
+			    size_t count, loff_t *ppos)
+{
+	struct bnxt_re_qp *qp = filep->private_data;
+	char *buf;
+	int len;
+
+	if (*ppos)
+		return 0;
+
+	buf = kasprintf(GFP_KERNEL,
+			"QPN\t\t: %d\n"
+			"transport\t: %s\n"
+			"state\t\t: %s\n"
+			"mtu\t\t: %d\n"
+			"timeout\t\t: %d\n"
+			"remote QPN\t: %d\n",
+			qp->qplib_qp.id,
+			bnxt_re_qp_type_str(qp->qplib_qp.type),
+			bnxt_re_qp_state_str(qp->qplib_qp.state),
+			qp->qplib_qp.mtu,
+			qp->qplib_qp.timeout,
+			qp->qplib_qp.dest_qpn);
+	if (!buf)
+		return -ENOMEM;
+	if (count < strlen(buf)) {
+		kfree(buf);
+		return -ENOSPC;
+	}
+	len = simple_read_from_buffer(buffer, count, ppos, buf, strlen(buf));
+	kfree(buf);
+	return len;
+}
+
+static const struct file_operations debugfs_qp_fops = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.read = qp_info_read,
+};
+
+void bnxt_re_debug_add_qpinfo(struct bnxt_re_dev *rdev, struct bnxt_re_qp *qp)
+{
+	char resn[32];
+
+	sprintf(resn, "0x%x", qp->qplib_qp.id);
+	qp->dentry = debugfs_create_file(resn, 0400, rdev->qp_debugfs, qp, &debugfs_qp_fops);
+}
+
+void bnxt_re_debug_rem_qpinfo(struct bnxt_re_dev *rdev, struct bnxt_re_qp *qp)
+{
+	if (!bnxt_re_debugfs_root || !rdev->qp_debugfs)
+		return;
+
+	debugfs_remove(qp->dentry);
+}
+
+void bnxt_re_debugfs_add_pdev(struct bnxt_re_dev *rdev)
+{
+	struct pci_dev *pdev = rdev->en_dev->pdev;
+
+	rdev->dbg_root = debugfs_create_dir(dev_name(&pdev->dev), bnxt_re_debugfs_root);
+
+	rdev->qp_debugfs = debugfs_create_dir("QPs", rdev->dbg_root);
+}
+
+void bnxt_re_debugfs_rem_pdev(struct bnxt_re_dev *rdev)
+{
+	debugfs_remove_recursive(rdev->qp_debugfs);
+
+	debugfs_remove_recursive(rdev->dbg_root);
+	rdev->dbg_root = NULL;
+}
+
+void bnxt_re_register_debugfs(void)
+{
+	bnxt_re_debugfs_root = debugfs_create_dir("bnxt_re", NULL);
+}
+
+void bnxt_re_unregister_debugfs(void)
+{
+	debugfs_remove(bnxt_re_debugfs_root);
+}
diff --git a/drivers/infiniband/hw/bnxt_re/debugfs.h b/drivers/infiniband/hw/bnxt_re/debugfs.h
new file mode 100644
index 0000000..cd3be0a9
--- /dev/null
+++ b/drivers/infiniband/hw/bnxt_re/debugfs.h
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+/*
+ * Copyright (c) 2024, Broadcom. All rights reserved.  The term
+ * Broadcom refers to Broadcom Limited and/or its subsidiaries.
+ *
+ * Description: Debugfs header
+ */
+
+#ifndef __BNXT_RE_DEBUGFS__
+#define __BNXT_RE_DEBUGFS__
+
+void bnxt_re_debug_add_qpinfo(struct bnxt_re_dev *rdev, struct bnxt_re_qp *qp);
+void bnxt_re_debug_rem_qpinfo(struct bnxt_re_dev *rdev, struct bnxt_re_qp *qp);
+
+void bnxt_re_debugfs_add_pdev(struct bnxt_re_dev *rdev);
+void bnxt_re_debugfs_rem_pdev(struct bnxt_re_dev *rdev);
+
+void bnxt_re_register_debugfs(void);
+void bnxt_re_unregister_debugfs(void);
+
+#endif
diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index e66ae9f..10e96f1 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -62,6 +62,7 @@
 
 #include "bnxt_re.h"
 #include "ib_verbs.h"
+#include "debugfs.h"
 
 #include <rdma/uverbs_types.h>
 #include <rdma/uverbs_std_types.h>
@@ -939,6 +940,8 @@ int bnxt_re_destroy_qp(struct ib_qp *ib_qp, struct ib_udata *udata)
 	else if (qp->qplib_qp.type == CMDQ_CREATE_QP_TYPE_UD)
 		atomic_dec(&rdev->stats.res.ud_qp_count);
 
+	bnxt_re_debug_rem_qpinfo(rdev, qp);
+
 	ib_umem_release(qp->rumem);
 	ib_umem_release(qp->sumem);
 
@@ -1622,6 +1625,7 @@ int bnxt_re_create_qp(struct ib_qp *ib_qp, struct ib_qp_init_attr *qp_init_attr,
 		if (active_qps > rdev->stats.res.ud_qp_watermark)
 			rdev->stats.res.ud_qp_watermark = active_qps;
 	}
+	bnxt_re_debug_add_qpinfo(rdev, qp);
 
 	return 0;
 qp_destroy:
diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.h b/drivers/infiniband/hw/bnxt_re/ib_verbs.h
index b789e47..e02a5e7 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.h
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.h
@@ -95,6 +95,7 @@ struct bnxt_re_qp {
 	struct ib_ud_header	qp1_hdr;
 	struct bnxt_re_cq	*scq;
 	struct bnxt_re_cq	*rcq;
+	struct dentry		*dentry;
 };
 
 struct bnxt_re_cq {
diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 0a18d24..04fa81e 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -67,6 +67,7 @@
 #include <rdma/bnxt_re-abi.h>
 #include "bnxt.h"
 #include "hw_counters.h"
+#include "debugfs.h"
 
 static char version[] =
 		BNXT_RE_DESC "\n";
@@ -1870,6 +1871,8 @@ static void bnxt_re_dev_uninit(struct bnxt_re_dev *rdev, u8 op_type)
 	u8 type;
 	int rc;
 
+	bnxt_re_debugfs_rem_pdev(rdev);
+
 	if (test_and_clear_bit(BNXT_RE_FLAG_QOS_WORK_REG, &rdev->flags))
 		cancel_delayed_work_sync(&rdev->worker);
 
@@ -2070,6 +2073,8 @@ static int bnxt_re_dev_init(struct bnxt_re_dev *rdev, u8 op_type)
 	if (rdev->chip_ctx->modes.toggle_bits & BNXT_QPLIB_SRQ_TOGGLE_BIT)
 		hash_init(rdev->srq_hash);
 
+	bnxt_re_debugfs_add_pdev(rdev);
+
 	return 0;
 free_sctx:
 	bnxt_re_net_stats_ctx_free(rdev, rdev->qplib_ctx.stats.fw_id);
@@ -2389,18 +2394,24 @@ static int __init bnxt_re_mod_init(void)
 	int rc;
 
 	pr_info("%s: %s", ROCE_DRV_MODULE_NAME, version);
+	bnxt_re_register_debugfs();
+
 	rc = auxiliary_driver_register(&bnxt_re_driver);
 	if (rc) {
 		pr_err("%s: Failed to register auxiliary driver\n",
 			ROCE_DRV_MODULE_NAME);
-		return rc;
+		goto err_debug;
 	}
 	return 0;
+err_debug:
+	bnxt_re_unregister_debugfs();
+	return rc;
 }
 
 static void __exit bnxt_re_mod_exit(void)
 {
 	auxiliary_driver_unregister(&bnxt_re_driver);
+	bnxt_re_unregister_debugfs();
 }
 
 module_init(bnxt_re_mod_init);
-- 
2.5.5

Re: [PATCH for-next 1/4] RDMA/bnxt_re: Support driver specific data collection using rdma tool

[Leon Romanovsky]

On Tue, Oct 22, 2024 at 03:11:53AM -0700, Selvin Xavier wrote:
> From: Kashyap Desai <kashyap.desai@broadcom.com>
> 
> Allow users to dump driver specific resource details when
> queried through rdma tool. This supports the driver data
> for QP, CQ, MR and SRQ.
> 
> Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> ---
>  drivers/infiniband/hw/bnxt_re/main.c | 148 +++++++++++++++++++++++++++++++++++
>  1 file changed, 148 insertions(+)
> 
> diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
> index 6715c96..5bed9af 100644
> --- a/drivers/infiniband/hw/bnxt_re/main.c
> +++ b/drivers/infiniband/hw/bnxt_re/main.c
> @@ -882,6 +882,146 @@ static const struct attribute_group bnxt_re_dev_attr_group = {
>  	.attrs = bnxt_re_attributes,
>  };
>  
> +static int bnxt_re_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ib_mr)
> +{
> +	struct bnxt_qplib_hwq *mr_hwq;
> +	struct nlattr *table_attr;
> +	struct bnxt_re_mr *mr;
> +
> +	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_DRIVER);
> +	if (!table_attr)
> +		return -EMSGSIZE;
> +
> +	mr = container_of(ib_mr, struct bnxt_re_mr, ib_mr);
> +	mr_hwq = &mr->qplib_mr.hwq;
> +
> +	if (rdma_nl_put_driver_string(msg, "owner",
> +				      mr_hwq->is_user ?  "user" : "kernel"))

Two comments:
1. There is already a helper function to decide if owner is user or kernel - rdma_is_kernel_res().
2. This print duplicates existing information. The difference between
user and kernel can be easily seen by looking on the PID output.

> +		goto err;
> +	if (rdma_nl_put_driver_u32(msg, "page_size",
> +				   mr_hwq->qe_ppg * mr_hwq->element_size))
> +		goto err;
> +	if (rdma_nl_put_driver_u32(msg, "max_elements", mr_hwq->max_elements))
> +		goto err;
> +	if (rdma_nl_put_driver_u32(msg, "element_size", mr_hwq->element_size))
> +		goto err;
> +	if (rdma_nl_put_driver_u64_hex(msg, "hwq", (unsigned long)mr_hwq))
> +		goto err;
> +	if (rdma_nl_put_driver_u64_hex(msg, "va", mr->qplib_mr.va))
> +		goto err;

<...>

> +static int bnxt_re_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ib_qp)
> +{
> +	struct bnxt_qplib_qp *qplib_qp;
> +	struct nlattr *table_attr;
> +	struct bnxt_re_qp *qp;
> +
> +	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_DRIVER);
> +	if (!table_attr)
> +		return -EMSGSIZE;
> +
> +	qp = container_of(ib_qp, struct bnxt_re_qp, ib_qp);
> +	qplib_qp = &qp->qplib_qp;
> +
> +	if (rdma_nl_put_driver_string(msg, "owner",
> +				      ib_qp->uobject ?  "user" : "kernel"))
> +		goto err;
> +
> +	if (rdma_nl_put_driver_u32(msg, "sq_max_wqe", qplib_qp->sq.max_wqe))
> +		goto err;
> +	if (rdma_nl_put_driver_u32(msg, "sq_max_sge", qplib_qp->sq.max_sge))

Doesn't this information already exist in other places? devinfo?

Thanks

Re: [PATCH for-next 1/4] RDMA/bnxt_re: Support driver specific data collection using rdma tool

[Selvin Xavier]

[-- Attachment #1: Type: text/plain, Size: 3865 bytes --]

On Tue, Oct 29, 2024 at 7:33 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Tue, Oct 22, 2024 at 03:11:53AM -0700, Selvin Xavier wrote:
> > From: Kashyap Desai <kashyap.desai@broadcom.com>
> >
> > Allow users to dump driver specific resource details when
> > queried through rdma tool. This supports the driver data
> > for QP, CQ, MR and SRQ.
> >
> > Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
> > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> > ---
> >  drivers/infiniband/hw/bnxt_re/main.c | 148 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 148 insertions(+)
> >
> > diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
> > index 6715c96..5bed9af 100644
> > --- a/drivers/infiniband/hw/bnxt_re/main.c
> > +++ b/drivers/infiniband/hw/bnxt_re/main.c
> > @@ -882,6 +882,146 @@ static const struct attribute_group bnxt_re_dev_attr_group = {
> >       .attrs = bnxt_re_attributes,
> >  };
> >
> > +static int bnxt_re_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ib_mr)
> > +{
> > +     struct bnxt_qplib_hwq *mr_hwq;
> > +     struct nlattr *table_attr;
> > +     struct bnxt_re_mr *mr;
> > +
> > +     table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_DRIVER);
> > +     if (!table_attr)
> > +             return -EMSGSIZE;
> > +
> > +     mr = container_of(ib_mr, struct bnxt_re_mr, ib_mr);
> > +     mr_hwq = &mr->qplib_mr.hwq;
> > +
> > +     if (rdma_nl_put_driver_string(msg, "owner",
> > +                                   mr_hwq->is_user ?  "user" : "kernel"))
>
> Two comments:
> 1. There is already a helper function to decide if owner is user or kernel - rdma_is_kernel_res().
> 2. This print duplicates existing information. The difference between
> user and kernel can be easily seen by looking on the PID output.
Got it. I will remove this in the follow up patch.
>
> > +             goto err;
> > +     if (rdma_nl_put_driver_u32(msg, "page_size",
> > +                                mr_hwq->qe_ppg * mr_hwq->element_size))
> > +             goto err;
> > +     if (rdma_nl_put_driver_u32(msg, "max_elements", mr_hwq->max_elements))
> > +             goto err;
> > +     if (rdma_nl_put_driver_u32(msg, "element_size", mr_hwq->element_size))
> > +             goto err;
> > +     if (rdma_nl_put_driver_u64_hex(msg, "hwq", (unsigned long)mr_hwq))
> > +             goto err;
> > +     if (rdma_nl_put_driver_u64_hex(msg, "va", mr->qplib_mr.va))
> > +             goto err;
>
> <...>
>
> > +static int bnxt_re_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ib_qp)
> > +{
> > +     struct bnxt_qplib_qp *qplib_qp;
> > +     struct nlattr *table_attr;
> > +     struct bnxt_re_qp *qp;
> > +
> > +     table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_DRIVER);
> > +     if (!table_attr)
> > +             return -EMSGSIZE;
> > +
> > +     qp = container_of(ib_qp, struct bnxt_re_qp, ib_qp);
> > +     qplib_qp = &qp->qplib_qp;
> > +
> > +     if (rdma_nl_put_driver_string(msg, "owner",
> > +                                   ib_qp->uobject ?  "user" : "kernel"))
> > +             goto err;
> > +
> > +     if (rdma_nl_put_driver_u32(msg, "sq_max_wqe", qplib_qp->sq.max_wqe))
> > +             goto err;
> > +     if (rdma_nl_put_driver_u32(msg, "sq_max_sge", qplib_qp->sq.max_sge))
>
> Doesn't this information already exist in other places? devinfo?
device level max_sge that can be retrieved from devinfo.
This is a per QP value we are displaying here . It is important for
the latest GenP7 adapters as we support variable size
 Work Queue entries (and variable size SGEs) now.
This information is available in the query_qp response. But it can be
handy to dump this while debugging.

>
> Thanks

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

Re: [PATCH for-next 4/4] RDMA/bnxt_re: Add debugfs hook in the driver

[Junxian Huang]

On 2024/10/22 18:11, Selvin Xavier wrote:
> From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> 
> Adding support for a per device debugfs folder for exporting
> some of the device specific debug information.
> Added support to get QP info for now. The same folder can be
> used to export other debug features in future.
> 
> Signed-off-by: Saravanan Vajravel <saravanan.vajravel@broadcom.com>
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> ---
>  drivers/infiniband/hw/bnxt_re/Makefile   |   3 +-
>  drivers/infiniband/hw/bnxt_re/bnxt_re.h  |   2 +
>  drivers/infiniband/hw/bnxt_re/debugfs.c  | 141 +++++++++++++++++++++++++++++++
>  drivers/infiniband/hw/bnxt_re/debugfs.h  |  21 +++++
>  drivers/infiniband/hw/bnxt_re/ib_verbs.c |   4 +
>  drivers/infiniband/hw/bnxt_re/ib_verbs.h |   1 +
>  drivers/infiniband/hw/bnxt_re/main.c     |  13 ++-
>  7 files changed, 183 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/infiniband/hw/bnxt_re/debugfs.c
>  create mode 100644 drivers/infiniband/hw/bnxt_re/debugfs.h
> 
> diff --git a/drivers/infiniband/hw/bnxt_re/Makefile b/drivers/infiniband/hw/bnxt_re/Makefile
> index ee9bb1b..f63417d 100644
> --- a/drivers/infiniband/hw/bnxt_re/Makefile
> +++ b/drivers/infiniband/hw/bnxt_re/Makefile
> @@ -4,4 +4,5 @@ ccflags-y := -I $(srctree)/drivers/net/ethernet/broadcom/bnxt
>  obj-$(CONFIG_INFINIBAND_BNXT_RE) += bnxt_re.o
>  bnxt_re-y := main.o ib_verbs.o \
>  	     qplib_res.o qplib_rcfw.o	\
> -	     qplib_sp.o qplib_fp.o  hw_counters.o
> +	     qplib_sp.o qplib_fp.o  hw_counters.o	\
> +	     debugfs.o
> diff --git a/drivers/infiniband/hw/bnxt_re/bnxt_re.h b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
> index 311bb7c..7f931c7 100644
> --- a/drivers/infiniband/hw/bnxt_re/bnxt_re.h
> +++ b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
> @@ -213,6 +213,8 @@ struct bnxt_re_dev {
>  	struct delayed_work dbq_pacing_work;
>  	DECLARE_HASHTABLE(cq_hash, MAX_CQ_HASH_BITS);
>  	DECLARE_HASHTABLE(srq_hash, MAX_SRQ_HASH_BITS);
> +	struct dentry			*dbg_root;
> +	struct dentry			*qp_debugfs;
>  };
>  
>  #define to_bnxt_re_dev(ptr, member)	\
> diff --git a/drivers/infiniband/hw/bnxt_re/debugfs.c b/drivers/infiniband/hw/bnxt_re/debugfs.c
> new file mode 100644
> index 0000000..dd38dd6c
> --- /dev/null
> +++ b/drivers/infiniband/hw/bnxt_re/debugfs.c
> @@ -0,0 +1,141 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +/*
> + * Copyright (c) 2024, Broadcom. All rights reserved.  The term
> + * Broadcom refers to Broadcom Limited and/or its subsidiaries.
> + *
> + * Description: Debugfs component of the bnxt_re driver
> + */
> +
> +#include <linux/debugfs.h>
> +#include <linux/pci.h>
> +#include <rdma/ib_addr.h>
> +
> +#include "bnxt_ulp.h"
> +#include "roce_hsi.h"
> +#include "qplib_res.h"
> +#include "qplib_sp.h"
> +#include "qplib_fp.h"
> +#include "qplib_rcfw.h"
> +#include "bnxt_re.h"
> +#include "ib_verbs.h"
> +#include "debugfs.h"
> +
> +static struct dentry *bnxt_re_debugfs_root;
> +
> +static inline const char *bnxt_re_qp_state_str(u8 state)
> +{
> +	switch (state) {
> +	case CMDQ_MODIFY_QP_NEW_STATE_RESET:
> +		return "RST";
> +	case CMDQ_MODIFY_QP_NEW_STATE_INIT:
> +		return "INIT";
> +	case CMDQ_MODIFY_QP_NEW_STATE_RTR:
> +		return "RTR";
> +	case CMDQ_MODIFY_QP_NEW_STATE_RTS:
> +		return "RTS";
> +	case CMDQ_MODIFY_QP_NEW_STATE_SQE:
> +		return "SQER";
> +	case CMDQ_MODIFY_QP_NEW_STATE_SQD:
> +		return "SQD";
> +	case CMDQ_MODIFY_QP_NEW_STATE_ERR:
> +		return "ERR";
> +	default:
> +		return "Invalid QP state";
> +	}
> +}
> +
> +static inline const char *bnxt_re_qp_type_str(u8 type)
> +{
> +	switch (type) {
> +	case CMDQ_CREATE_QP1_TYPE_GSI: return "QP1";
> +	case CMDQ_CREATE_QP_TYPE_GSI: return "QP1";
> +	case CMDQ_CREATE_QP_TYPE_RC: return "RC";
> +	case CMDQ_CREATE_QP_TYPE_UD: return "UD";
> +	case CMDQ_CREATE_QP_TYPE_RAW_ETHERTYPE: return "RAW_ETHERTYPE";
> +	default: return "Invalid transport type";
> +	}
> +}
> +

Would it be better to use table-driven approach for these two functions?

> +static ssize_t qp_info_read(struct file *filep,
> +			    char __user *buffer,
> +			    size_t count, loff_t *ppos)
> +{
> +	struct bnxt_re_qp *qp = filep->private_data;
> +	char *buf;
> +	int len;
> +
> +	if (*ppos)
> +		return 0;
> +
> +	buf = kasprintf(GFP_KERNEL,
> +			"QPN\t\t: %d\n"
> +			"transport\t: %s\n"
> +			"state\t\t: %s\n"
> +			"mtu\t\t: %d\n"
> +			"timeout\t\t: %d\n"
> +			"remote QPN\t: %d\n",
> +			qp->qplib_qp.id,
> +			bnxt_re_qp_type_str(qp->qplib_qp.type),
> +			bnxt_re_qp_state_str(qp->qplib_qp.state),
> +			qp->qplib_qp.mtu,
> +			qp->qplib_qp.timeout,
> +			qp->qplib_qp.dest_qpn);
> +	if (!buf)
> +		return -ENOMEM;
> +	if (count < strlen(buf)) {
> +		kfree(buf);
> +		return -ENOSPC;
> +	}
> +	len = simple_read_from_buffer(buffer, count, ppos, buf, strlen(buf));
> +	kfree(buf);
> +	return len;
> +}
> +
> +static const struct file_operations debugfs_qp_fops = {
> +	.owner = THIS_MODULE,
> +	.open = simple_open,
> +	.read = qp_info_read,
> +};
> +
> +void bnxt_re_debug_add_qpinfo(struct bnxt_re_dev *rdev, struct bnxt_re_qp *qp)
> +{
> +	char resn[32];
> +
> +	sprintf(resn, "0x%x", qp->qplib_qp.id);
> +	qp->dentry = debugfs_create_file(resn, 0400, rdev->qp_debugfs, qp, &debugfs_qp_fops);
> +}
> +
> +void bnxt_re_debug_rem_qpinfo(struct bnxt_re_dev *rdev, struct bnxt_re_qp *qp)
> +{
> +	if (!bnxt_re_debugfs_root || !rdev->qp_debugfs)
> +		return;

debugfs functions can handle cases where the input dentry is an error,
so this checking is unnecessary.

Junxian

> +
> +	debugfs_remove(qp->dentry);> +}
> +
> +void bnxt_re_debugfs_add_pdev(struct bnxt_re_dev *rdev)
> +{
> +	struct pci_dev *pdev = rdev->en_dev->pdev;
> +
> +	rdev->dbg_root = debugfs_create_dir(dev_name(&pdev->dev), bnxt_re_debugfs_root);
> +
> +	rdev->qp_debugfs = debugfs_create_dir("QPs", rdev->dbg_root);
> +}
> +
> +void bnxt_re_debugfs_rem_pdev(struct bnxt_re_dev *rdev)
> +{
> +	debugfs_remove_recursive(rdev->qp_debugfs);
> +
> +	debugfs_remove_recursive(rdev->dbg_root);
> +	rdev->dbg_root = NULL;
> +}
> +
> +void bnxt_re_register_debugfs(void)
> +{
> +	bnxt_re_debugfs_root = debugfs_create_dir("bnxt_re", NULL);
> +}
> +
> +void bnxt_re_unregister_debugfs(void)
> +{
> +	debugfs_remove(bnxt_re_debugfs_root);
> +}
> diff --git a/drivers/infiniband/hw/bnxt_re/debugfs.h b/drivers/infiniband/hw/bnxt_re/debugfs.h
> new file mode 100644
> index 0000000..cd3be0a9
> --- /dev/null
> +++ b/drivers/infiniband/hw/bnxt_re/debugfs.h
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +/*
> + * Copyright (c) 2024, Broadcom. All rights reserved.  The term
> + * Broadcom refers to Broadcom Limited and/or its subsidiaries.
> + *
> + * Description: Debugfs header
> + */
> +
> +#ifndef __BNXT_RE_DEBUGFS__
> +#define __BNXT_RE_DEBUGFS__
> +
> +void bnxt_re_debug_add_qpinfo(struct bnxt_re_dev *rdev, struct bnxt_re_qp *qp);
> +void bnxt_re_debug_rem_qpinfo(struct bnxt_re_dev *rdev, struct bnxt_re_qp *qp);
> +
> +void bnxt_re_debugfs_add_pdev(struct bnxt_re_dev *rdev);
> +void bnxt_re_debugfs_rem_pdev(struct bnxt_re_dev *rdev);
> +
> +void bnxt_re_register_debugfs(void);
> +void bnxt_re_unregister_debugfs(void);
> +
> +#endif
> diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> index e66ae9f..10e96f1 100644
> --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> @@ -62,6 +62,7 @@
>  
>  #include "bnxt_re.h"
>  #include "ib_verbs.h"
> +#include "debugfs.h"
>  
>  #include <rdma/uverbs_types.h>
>  #include <rdma/uverbs_std_types.h>
> @@ -939,6 +940,8 @@ int bnxt_re_destroy_qp(struct ib_qp *ib_qp, struct ib_udata *udata)
>  	else if (qp->qplib_qp.type == CMDQ_CREATE_QP_TYPE_UD)
>  		atomic_dec(&rdev->stats.res.ud_qp_count);
>  
> +	bnxt_re_debug_rem_qpinfo(rdev, qp);
> +
>  	ib_umem_release(qp->rumem);
>  	ib_umem_release(qp->sumem);
>  
> @@ -1622,6 +1625,7 @@ int bnxt_re_create_qp(struct ib_qp *ib_qp, struct ib_qp_init_attr *qp_init_attr,
>  		if (active_qps > rdev->stats.res.ud_qp_watermark)
>  			rdev->stats.res.ud_qp_watermark = active_qps;
>  	}
> +	bnxt_re_debug_add_qpinfo(rdev, qp);
>  
>  	return 0;
>  qp_destroy:
> diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.h b/drivers/infiniband/hw/bnxt_re/ib_verbs.h
> index b789e47..e02a5e7 100644
> --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.h
> +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.h
> @@ -95,6 +95,7 @@ struct bnxt_re_qp {
>  	struct ib_ud_header	qp1_hdr;
>  	struct bnxt_re_cq	*scq;
>  	struct bnxt_re_cq	*rcq;
> +	struct dentry		*dentry;
>  };
>  
>  struct bnxt_re_cq {
> diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
> index 0a18d24..04fa81e 100644
> --- a/drivers/infiniband/hw/bnxt_re/main.c
> +++ b/drivers/infiniband/hw/bnxt_re/main.c
> @@ -67,6 +67,7 @@
>  #include <rdma/bnxt_re-abi.h>
>  #include "bnxt.h"
>  #include "hw_counters.h"
> +#include "debugfs.h"
>  
>  static char version[] =
>  		BNXT_RE_DESC "\n";
> @@ -1870,6 +1871,8 @@ static void bnxt_re_dev_uninit(struct bnxt_re_dev *rdev, u8 op_type)
>  	u8 type;
>  	int rc;
>  
> +	bnxt_re_debugfs_rem_pdev(rdev);
> +
>  	if (test_and_clear_bit(BNXT_RE_FLAG_QOS_WORK_REG, &rdev->flags))
>  		cancel_delayed_work_sync(&rdev->worker);
>  
> @@ -2070,6 +2073,8 @@ static int bnxt_re_dev_init(struct bnxt_re_dev *rdev, u8 op_type)
>  	if (rdev->chip_ctx->modes.toggle_bits & BNXT_QPLIB_SRQ_TOGGLE_BIT)
>  		hash_init(rdev->srq_hash);
>  
> +	bnxt_re_debugfs_add_pdev(rdev);
> +
>  	return 0;
>  free_sctx:
>  	bnxt_re_net_stats_ctx_free(rdev, rdev->qplib_ctx.stats.fw_id);
> @@ -2389,18 +2394,24 @@ static int __init bnxt_re_mod_init(void)
>  	int rc;
>  
>  	pr_info("%s: %s", ROCE_DRV_MODULE_NAME, version);
> +	bnxt_re_register_debugfs();
> +
>  	rc = auxiliary_driver_register(&bnxt_re_driver);
>  	if (rc) {
>  		pr_err("%s: Failed to register auxiliary driver\n",
>  			ROCE_DRV_MODULE_NAME);
> -		return rc;
> +		goto err_debug;
>  	}
>  	return 0;
> +err_debug:
> +	bnxt_re_unregister_debugfs();
> +	return rc;
>  }
>  
>  static void __exit bnxt_re_mod_exit(void)
>  {
>  	auxiliary_driver_unregister(&bnxt_re_driver);
> +	bnxt_re_unregister_debugfs();
>  }
>  
>  module_init(bnxt_re_mod_init);

Re: [PATCH for-next 4/4] RDMA/bnxt_re: Add debugfs hook in the driver

[Leon Romanovsky]

On Wed, Oct 30, 2024 at 06:10:18PM +0800, Junxian Huang wrote:
> 
> 
> On 2024/10/22 18:11, Selvin Xavier wrote:
> > From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > 
> > Adding support for a per device debugfs folder for exporting
> > some of the device specific debug information.
> > Added support to get QP info for now. The same folder can be
> > used to export other debug features in future.
> > 
> > Signed-off-by: Saravanan Vajravel <saravanan.vajravel@broadcom.com>
> > Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> > ---
> >  drivers/infiniband/hw/bnxt_re/Makefile   |   3 +-
> >  drivers/infiniband/hw/bnxt_re/bnxt_re.h  |   2 +
> >  drivers/infiniband/hw/bnxt_re/debugfs.c  | 141 +++++++++++++++++++++++++++++++
> >  drivers/infiniband/hw/bnxt_re/debugfs.h  |  21 +++++
> >  drivers/infiniband/hw/bnxt_re/ib_verbs.c |   4 +
> >  drivers/infiniband/hw/bnxt_re/ib_verbs.h |   1 +
> >  drivers/infiniband/hw/bnxt_re/main.c     |  13 ++-
> >  7 files changed, 183 insertions(+), 2 deletions(-)
> >  create mode 100644 drivers/infiniband/hw/bnxt_re/debugfs.c
> >  create mode 100644 drivers/infiniband/hw/bnxt_re/debugfs.h

<...>

> > +static inline const char *bnxt_re_qp_type_str(u8 type)
> > +{
> > +	switch (type) {
> > +	case CMDQ_CREATE_QP1_TYPE_GSI: return "QP1";
> > +	case CMDQ_CREATE_QP_TYPE_GSI: return "QP1";
> > +	case CMDQ_CREATE_QP_TYPE_RC: return "RC";
> > +	case CMDQ_CREATE_QP_TYPE_UD: return "UD";
> > +	case CMDQ_CREATE_QP_TYPE_RAW_ETHERTYPE: return "RAW_ETHERTYPE";
> > +	default: return "Invalid transport type";
> > +	}
> > +}
> > +
> 
> Would it be better to use table-driven approach for these two functions?

No, proposed variant is better as it can be consumed very easily by tools.
Table are good for humans but bad for tooling.

Thanks

Re: [PATCH for-next 1/4] RDMA/bnxt_re: Support driver specific data collection using rdma tool

[Leon Romanovsky]

On Wed, Oct 30, 2024 at 01:59:06PM +0530, Selvin Xavier wrote:
> On Tue, Oct 29, 2024 at 7:33 PM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > On Tue, Oct 22, 2024 at 03:11:53AM -0700, Selvin Xavier wrote:
> > > From: Kashyap Desai <kashyap.desai@broadcom.com>
> > >
> > > Allow users to dump driver specific resource details when
> > > queried through rdma tool. This supports the driver data
> > > for QP, CQ, MR and SRQ.
> > >
> > > Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > > Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
> > > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> > > ---
> > >  drivers/infiniband/hw/bnxt_re/main.c | 148 +++++++++++++++++++++++++++++++++++
> > >  1 file changed, 148 insertions(+)
> > >
> > > diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
> > > index 6715c96..5bed9af 100644
> > > --- a/drivers/infiniband/hw/bnxt_re/main.c
> > > +++ b/drivers/infiniband/hw/bnxt_re/main.c
> > > @@ -882,6 +882,146 @@ static const struct attribute_group bnxt_re_dev_attr_group = {
> > >       .attrs = bnxt_re_attributes,
> > >  };
> > >
> > > +static int bnxt_re_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ib_mr)
> > > +{
> > > +     struct bnxt_qplib_hwq *mr_hwq;
> > > +     struct nlattr *table_attr;
> > > +     struct bnxt_re_mr *mr;
> > > +
> > > +     table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_DRIVER);
> > > +     if (!table_attr)
> > > +             return -EMSGSIZE;
> > > +
> > > +     mr = container_of(ib_mr, struct bnxt_re_mr, ib_mr);
> > > +     mr_hwq = &mr->qplib_mr.hwq;
> > > +
> > > +     if (rdma_nl_put_driver_string(msg, "owner",
> > > +                                   mr_hwq->is_user ?  "user" : "kernel"))
> >
> > Two comments:
> > 1. There is already a helper function to decide if owner is user or kernel - rdma_is_kernel_res().
> > 2. This print duplicates existing information. The difference between
> > user and kernel can be easily seen by looking on the PID output.
> Got it. I will remove this in the follow up patch.

Thanks