[PATCH for-next 0/2] RDMA/erdma: Introduce hardware statistics support

[PATCH for-next 0/2] RDMA/erdma: Introduce hardware statistics support

[Cheng Xu]

This small patchset introduces the support of hardware statistics.
Statistics counters can not be put in CQEs due to limited CQE size. To
address this, we provide an extra dma buffer to hardware when posting
statistics query request, and then hardware writes back the response to
this dma buffer. Based on this, we add the hardware statistics support
of erdma.

- #1 introduces dma pool used for hardware responses of CMDQ requests.
- #2 adds hardware statistics support.

Cheng Xu (2):
  RDMA/erdma: Introduce dma pool for hardware responses of CMDQ requests
  RDMA/erdma: Add hardware statistics support

 drivers/infiniband/hw/erdma/erdma.h       |   2 +
 drivers/infiniband/hw/erdma/erdma_hw.h    |  40 +++++++++
 drivers/infiniband/hw/erdma/erdma_main.c  |  40 ++++++++-
 drivers/infiniband/hw/erdma/erdma_verbs.c | 100 +++++++++++++++++++++-
 drivers/infiniband/hw/erdma/erdma_verbs.h |   4 +
 5 files changed, 180 insertions(+), 6 deletions(-)

-- 
2.31.1

[PATCH for-next 1/2] RDMA/erdma: Introduce dma pool for hardware responses of CMDQ requests

[Cheng Xu]

Hardware response, such as the result of query statistics, may be too
long to be directly accommodated within the CQE structure. To address
this, we introduce a DMA pool to hold the hardware's responses of CMDQ
requests.

Signed-off-by: Cheng Xu <chengyou@linux.alibaba.com>
---
 drivers/infiniband/hw/erdma/erdma.h      |  2 ++
 drivers/infiniband/hw/erdma/erdma_main.c | 38 ++++++++++++++++++++++--
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/erdma/erdma.h b/drivers/infiniband/hw/erdma/erdma.h
index f190111840e9..5df401a30cb9 100644
--- a/drivers/infiniband/hw/erdma/erdma.h
+++ b/drivers/infiniband/hw/erdma/erdma.h
@@ -212,6 +212,8 @@ struct erdma_dev {
 
 	atomic_t num_ctx;
 	struct list_head cep_list;
+
+	struct dma_pool *resp_pool;
 };
 
 static inline void *get_queue_entry(void *qbuf, u32 idx, u32 depth, u32 shift)
diff --git a/drivers/infiniband/hw/erdma/erdma_main.c b/drivers/infiniband/hw/erdma/erdma_main.c
index 0880c79a978c..541e77aea494 100644
--- a/drivers/infiniband/hw/erdma/erdma_main.c
+++ b/drivers/infiniband/hw/erdma/erdma_main.c
@@ -168,18 +168,48 @@ static void erdma_comm_irq_uninit(struct erdma_dev *dev)
 	free_irq(dev->comm_irq.msix_vector, dev);
 }
 
+static int erdma_dma_pools_init(struct erdma_dev *dev)
+{
+	dev->resp_pool = dma_pool_create("erdma_resp_pool", &dev->pdev->dev,
+					 ERDMA_HW_RESP_SIZE, ERDMA_HW_RESP_SIZE,
+					 0);
+	if (!dev->resp_pool)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static void erdma_dma_pools_destroy(struct erdma_dev *dev)
+{
+	dma_pool_destroy(dev->resp_pool);
+}
+
 static int erdma_device_init(struct erdma_dev *dev, struct pci_dev *pdev)
 {
 	int ret;
 
+	ret = erdma_dma_pools_init(dev);
+	if (ret)
+		return ret;
+
 	ret = dma_set_mask_and_coherent(&pdev->dev,
 					DMA_BIT_MASK(ERDMA_PCI_WIDTH));
 	if (ret)
-		return ret;
+		goto destroy_pool;
 
 	dma_set_max_seg_size(&pdev->dev, UINT_MAX);
 
 	return 0;
+
+destroy_pool:
+	erdma_dma_pools_destroy(dev);
+
+	return ret;
+}
+
+static void erdma_device_uninit(struct erdma_dev *dev)
+{
+	erdma_dma_pools_destroy(dev);
 }
 
 static void erdma_hw_reset(struct erdma_dev *dev)
@@ -273,7 +303,7 @@ static int erdma_probe_dev(struct pci_dev *pdev)
 
 	err = erdma_request_vectors(dev);
 	if (err)
-		goto err_iounmap_func_bar;
+		goto err_uninit_device;
 
 	err = erdma_comm_irq_init(dev);
 	if (err)
@@ -314,6 +344,9 @@ static int erdma_probe_dev(struct pci_dev *pdev)
 err_free_vectors:
 	pci_free_irq_vectors(dev->pdev);
 
+err_uninit_device:
+	erdma_device_uninit(dev);
+
 err_iounmap_func_bar:
 	devm_iounmap(&pdev->dev, dev->func_bar);
 
@@ -339,6 +372,7 @@ static void erdma_remove_dev(struct pci_dev *pdev)
 	erdma_aeq_destroy(dev);
 	erdma_comm_irq_uninit(dev);
 	pci_free_irq_vectors(dev->pdev);
+	erdma_device_uninit(dev);
 
 	devm_iounmap(&pdev->dev, dev->func_bar);
 	pci_release_selected_regions(pdev, ERDMA_BAR_MASK);
-- 
2.31.1

[PATCH for-next 2/2] RDMA/erdma: Add hardware statistics support

[Cheng Xu]

First, we add a new command to query hardware statistics, and then
implement two functions: ib_device_ops.alloc_hw_port_stats and
ib_device_ops.get_hw_stats to allow rdma tool can get the statistics
of erdma device.

Signed-off-by: Cheng Xu <chengyou@linux.alibaba.com>
---
 drivers/infiniband/hw/erdma/erdma_hw.h    |  40 +++++++++
 drivers/infiniband/hw/erdma/erdma_main.c  |   2 +
 drivers/infiniband/hw/erdma/erdma_verbs.c | 100 +++++++++++++++++++++-
 drivers/infiniband/hw/erdma/erdma_verbs.h |   4 +
 4 files changed, 142 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/erdma/erdma_hw.h b/drivers/infiniband/hw/erdma/erdma_hw.h
index 9d316fdc6f9a..2c030f254ff7 100644
--- a/drivers/infiniband/hw/erdma/erdma_hw.h
+++ b/drivers/infiniband/hw/erdma/erdma_hw.h
@@ -146,6 +146,7 @@ enum CMDQ_COMMON_OPCODE {
 	CMDQ_OPCODE_DESTROY_EQ = 1,
 	CMDQ_OPCODE_QUERY_FW_INFO = 2,
 	CMDQ_OPCODE_CONF_MTU = 3,
+	CMDQ_OPCODE_GET_STATS = 4,
 	CMDQ_OPCODE_CONF_DEVICE = 5,
 	CMDQ_OPCODE_ALLOC_DB = 8,
 	CMDQ_OPCODE_FREE_DB = 9,
@@ -357,6 +358,45 @@ struct erdma_cmdq_reflush_req {
 	u32 rq_pi;
 };
 
+/* Response Definitions for Query Command Category */
+#define ERDMA_HW_RESP_SIZE 256
+
+struct erdma_cmdq_query_req {
+	u64 hdr;
+	u32 rsvd;
+	u32 index;
+
+	u64 target_addr;
+	u32 target_length;
+};
+
+#define ERDMA_HW_RESP_MAGIC 0x5566
+
+struct erdma_cmdq_query_resp_hdr {
+	u16 magic;
+	u8 ver;
+	u8 length;
+
+	u32 index;
+	u32 rsvd[2];
+};
+
+struct erdma_cmdq_query_stats_resp {
+	struct erdma_cmdq_query_resp_hdr hdr;
+
+	u64 tx_req_cnt;
+	u64 tx_packets_cnt;
+	u64 tx_bytes_cnt;
+	u64 tx_drop_packets_cnt;
+	u64 tx_bps_meter_drop_packets_cnt;
+	u64 tx_pps_meter_drop_packets_cnt;
+	u64 rx_packets_cnt;
+	u64 rx_bytes_cnt;
+	u64 rx_drop_packets_cnt;
+	u64 rx_bps_meter_drop_packets_cnt;
+	u64 rx_pps_meter_drop_packets_cnt;
+};
+
 /* cap qword 0 definition */
 #define ERDMA_CMD_DEV_CAP_MAX_CQE_MASK GENMASK_ULL(47, 40)
 #define ERDMA_CMD_DEV_CAP_FLAGS_MASK GENMASK_ULL(31, 24)
diff --git a/drivers/infiniband/hw/erdma/erdma_main.c b/drivers/infiniband/hw/erdma/erdma_main.c
index 541e77aea494..0c35f7e464c8 100644
--- a/drivers/infiniband/hw/erdma/erdma_main.c
+++ b/drivers/infiniband/hw/erdma/erdma_main.c
@@ -482,6 +482,7 @@ static const struct ib_device_ops erdma_device_ops = {
 	.driver_id = RDMA_DRIVER_ERDMA,
 	.uverbs_abi_ver = ERDMA_ABI_VERSION,
 
+	.alloc_hw_port_stats = erdma_alloc_hw_port_stats,
 	.alloc_mr = erdma_ib_alloc_mr,
 	.alloc_pd = erdma_alloc_pd,
 	.alloc_ucontext = erdma_alloc_ucontext,
@@ -493,6 +494,7 @@ static const struct ib_device_ops erdma_device_ops = {
 	.destroy_cq = erdma_destroy_cq,
 	.destroy_qp = erdma_destroy_qp,
 	.get_dma_mr = erdma_get_dma_mr,
+	.get_hw_stats = erdma_get_hw_stats,
 	.get_port_immutable = erdma_get_port_immutable,
 	.iw_accept = erdma_accept,
 	.iw_add_ref = erdma_qp_get_ref,
diff --git a/drivers/infiniband/hw/erdma/erdma_verbs.c b/drivers/infiniband/hw/erdma/erdma_verbs.c
index c317947563fb..2c67e7f48336 100644
--- a/drivers/infiniband/hw/erdma/erdma_verbs.c
+++ b/drivers/infiniband/hw/erdma/erdma_verbs.c
@@ -1599,10 +1599,9 @@ static int erdma_init_kernel_cq(struct erdma_cq *cq)
 {
 	struct erdma_dev *dev = to_edev(cq->ibcq.device);
 
-	cq->kern_cq.qbuf =
-		dma_alloc_coherent(&dev->pdev->dev,
-				   WARPPED_BUFSIZE(cq->depth << CQE_SHIFT),
-				   &cq->kern_cq.qbuf_dma_addr, GFP_KERNEL);
+	cq->kern_cq.qbuf = dma_alloc_coherent(
+		&dev->pdev->dev, WARPPED_BUFSIZE(cq->depth << CQE_SHIFT),
+		&cq->kern_cq.qbuf_dma_addr, GFP_KERNEL);
 	if (!cq->kern_cq.qbuf)
 		return -ENOMEM;
 
@@ -1708,3 +1707,96 @@ void erdma_port_event(struct erdma_dev *dev, enum ib_event_type reason)
 
 	ib_dispatch_event(&event);
 }
+
+enum counters {
+	ERDMA_STATS_TX_REQS_CNT,
+	ERDMA_STATS_TX_PACKETS_CNT,
+	ERDMA_STATS_TX_BYTES_CNT,
+	ERDMA_STATS_TX_DISABLE_DROP_CNT,
+	ERDMA_STATS_TX_BPS_METER_DROP_CNT,
+	ERDMA_STATS_TX_PPS_METER_DROP_CNT,
+
+	ERDMA_STATS_RX_PACKETS_CNT,
+	ERDMA_STATS_RX_BYTES_CNT,
+	ERDMA_STATS_RX_DISABLE_DROP_CNT,
+	ERDMA_STATS_RX_BPS_METER_DROP_CNT,
+	ERDMA_STATS_RX_PPS_METER_DROP_CNT,
+
+	ERDMA_STATS_MAX
+};
+
+static const struct rdma_stat_desc erdma_descs[] = {
+	[ERDMA_STATS_TX_REQS_CNT].name = "hw_tx_reqs_cnt",
+	[ERDMA_STATS_TX_PACKETS_CNT].name = "hw_tx_packets_cnt",
+	[ERDMA_STATS_TX_BYTES_CNT].name = "hw_tx_bytes_cnt",
+	[ERDMA_STATS_TX_DISABLE_DROP_CNT].name = "hw_disable_drop_cnt",
+	[ERDMA_STATS_TX_BPS_METER_DROP_CNT].name = "hw_bps_limit_drop_cnt",
+	[ERDMA_STATS_TX_PPS_METER_DROP_CNT].name = "hw_pps_limit_drop_cnt",
+	[ERDMA_STATS_RX_PACKETS_CNT].name = "hw_rx_packets_cnt",
+	[ERDMA_STATS_RX_BYTES_CNT].name = "hw_rx_bytes_cnt",
+	[ERDMA_STATS_RX_DISABLE_DROP_CNT].name = "hw_rx_disable_drop_cnt",
+	[ERDMA_STATS_RX_BPS_METER_DROP_CNT].name = "hw_rx_bps_limit_drop_cnt",
+	[ERDMA_STATS_RX_PPS_METER_DROP_CNT].name = "hw_rx_pps_limit_drop_cnt",
+};
+
+struct rdma_hw_stats *erdma_alloc_hw_port_stats(struct ib_device *device,
+						u32 port_num)
+{
+	return rdma_alloc_hw_stats_struct(erdma_descs, ERDMA_STATS_MAX,
+					  RDMA_HW_STATS_DEFAULT_LIFESPAN);
+}
+
+int erdma_query_hw_stats(struct erdma_dev *dev, struct rdma_hw_stats *stats)
+{
+	struct erdma_cmdq_query_stats_resp *resp;
+	struct erdma_cmdq_query_req req;
+	dma_addr_t dma_addr;
+	int err;
+
+	erdma_cmdq_build_reqhdr(&req.hdr, CMDQ_SUBMOD_COMMON,
+				CMDQ_OPCODE_GET_STATS);
+
+	resp = dma_pool_alloc(dev->resp_pool, GFP_KERNEL | __GFP_ZERO,
+			      &dma_addr);
+	if (!resp)
+		return -ENOMEM;
+
+	req.target_addr = dma_addr;
+	req.target_length = ERDMA_HW_RESP_SIZE;
+
+	err = erdma_post_cmd_wait(&dev->cmdq, &req, sizeof(req), NULL, NULL);
+	if (err)
+		goto out;
+
+	if (resp->hdr.magic != ERDMA_HW_RESP_MAGIC) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	memcpy(&stats->value[0], &resp->tx_req_cnt,
+	       sizeof(u64) * stats->num_counters);
+
+out:
+	dma_pool_free(dev->resp_pool, resp, dma_addr);
+
+	return err;
+}
+
+int erdma_get_hw_stats(struct ib_device *ibdev, struct rdma_hw_stats *stats,
+		       u32 port, int index)
+{
+	struct erdma_dev *dev = to_edev(ibdev);
+	int ret;
+
+	if (port == 0)
+		return 0;
+
+	if (port > 1)
+		return -EINVAL;
+
+	ret = erdma_query_hw_stats(dev, stats);
+	if (ret)
+		return ret;
+
+	return stats->num_counters;
+}
diff --git a/drivers/infiniband/hw/erdma/erdma_verbs.h b/drivers/infiniband/hw/erdma/erdma_verbs.h
index eb9c0f92fb6f..db6018529ccc 100644
--- a/drivers/infiniband/hw/erdma/erdma_verbs.h
+++ b/drivers/infiniband/hw/erdma/erdma_verbs.h
@@ -361,5 +361,9 @@ int erdma_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sg, int sg_nents,
 		    unsigned int *sg_offset);
 void erdma_port_event(struct erdma_dev *dev, enum ib_event_type reason);
 void erdma_set_mtu(struct erdma_dev *dev, u32 mtu);
+struct rdma_hw_stats *erdma_alloc_hw_port_stats(struct ib_device *device,
+						u32 port_num);
+int erdma_get_hw_stats(struct ib_device *ibdev, struct rdma_hw_stats *stats,
+		       u32 port, int index);
 
 #endif
-- 
2.31.1

Re: [PATCH for-next 1/2] RDMA/erdma: Introduce dma pool for hardware responses of CMDQ requests

[Leon Romanovsky]

On Wed, Dec 20, 2023 at 04:54:23PM +0800, Cheng Xu wrote:
> Hardware response, such as the result of query statistics, may be too
> long to be directly accommodated within the CQE structure. To address
> this, we introduce a DMA pool to hold the hardware's responses of CMDQ
> requests.
> 
> Signed-off-by: Cheng Xu <chengyou@linux.alibaba.com>
> ---
>  drivers/infiniband/hw/erdma/erdma.h      |  2 ++
>  drivers/infiniband/hw/erdma/erdma_main.c | 38 ++++++++++++++++++++++--
>  2 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/erdma/erdma.h b/drivers/infiniband/hw/erdma/erdma.h
> index f190111840e9..5df401a30cb9 100644
> --- a/drivers/infiniband/hw/erdma/erdma.h
> +++ b/drivers/infiniband/hw/erdma/erdma.h
> @@ -212,6 +212,8 @@ struct erdma_dev {
>  
>  	atomic_t num_ctx;
>  	struct list_head cep_list;
> +
> +	struct dma_pool *resp_pool;
>  };
>  
>  static inline void *get_queue_entry(void *qbuf, u32 idx, u32 depth, u32 shift)
> diff --git a/drivers/infiniband/hw/erdma/erdma_main.c b/drivers/infiniband/hw/erdma/erdma_main.c
> index 0880c79a978c..541e77aea494 100644
> --- a/drivers/infiniband/hw/erdma/erdma_main.c
> +++ b/drivers/infiniband/hw/erdma/erdma_main.c
> @@ -168,18 +168,48 @@ static void erdma_comm_irq_uninit(struct erdma_dev *dev)
>  	free_irq(dev->comm_irq.msix_vector, dev);
>  }
>  
> +static int erdma_dma_pools_init(struct erdma_dev *dev)
> +{
> +	dev->resp_pool = dma_pool_create("erdma_resp_pool", &dev->pdev->dev,
> +					 ERDMA_HW_RESP_SIZE, ERDMA_HW_RESP_SIZE,
> +					 0);
> +	if (!dev->resp_pool)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
> +static void erdma_dma_pools_destroy(struct erdma_dev *dev)
> +{
> +	dma_pool_destroy(dev->resp_pool);
> +}

Please don't add extra layer of functions which will be called in same
file anyway. Call directly to dma_pool_destroy(), same goes for dma_pool_create().

Thanks

Re: [PATCH for-next 2/2] RDMA/erdma: Add hardware statistics support

[Leon Romanovsky]

On Wed, Dec 20, 2023 at 04:54:24PM +0800, Cheng Xu wrote:
> First, we add a new command to query hardware statistics, and then
> implement two functions: ib_device_ops.alloc_hw_port_stats and
> ib_device_ops.get_hw_stats to allow rdma tool can get the statistics
> of erdma device.
> 
> Signed-off-by: Cheng Xu <chengyou@linux.alibaba.com>
> ---
>  drivers/infiniband/hw/erdma/erdma_hw.h    |  40 +++++++++
>  drivers/infiniband/hw/erdma/erdma_main.c  |   2 +
>  drivers/infiniband/hw/erdma/erdma_verbs.c | 100 +++++++++++++++++++++-
>  drivers/infiniband/hw/erdma/erdma_verbs.h |   4 +
>  4 files changed, 142 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/erdma/erdma_hw.h b/drivers/infiniband/hw/erdma/erdma_hw.h
> index 9d316fdc6f9a..2c030f254ff7 100644
> --- a/drivers/infiniband/hw/erdma/erdma_hw.h
> +++ b/drivers/infiniband/hw/erdma/erdma_hw.h
> @@ -146,6 +146,7 @@ enum CMDQ_COMMON_OPCODE {
>  	CMDQ_OPCODE_DESTROY_EQ = 1,
>  	CMDQ_OPCODE_QUERY_FW_INFO = 2,
>  	CMDQ_OPCODE_CONF_MTU = 3,
> +	CMDQ_OPCODE_GET_STATS = 4,
>  	CMDQ_OPCODE_CONF_DEVICE = 5,
>  	CMDQ_OPCODE_ALLOC_DB = 8,
>  	CMDQ_OPCODE_FREE_DB = 9,
> @@ -357,6 +358,45 @@ struct erdma_cmdq_reflush_req {
>  	u32 rq_pi;
>  };
>  
> +/* Response Definitions for Query Command Category */
> +#define ERDMA_HW_RESP_SIZE 256
> +
> +struct erdma_cmdq_query_req {
> +	u64 hdr;
> +	u32 rsvd;
> +	u32 index;
> +
> +	u64 target_addr;
> +	u32 target_length;
> +};
> +
> +#define ERDMA_HW_RESP_MAGIC 0x5566
> +
> +struct erdma_cmdq_query_resp_hdr {
> +	u16 magic;
> +	u8 ver;
> +	u8 length;
> +
> +	u32 index;
> +	u32 rsvd[2];
> +};
> +
> +struct erdma_cmdq_query_stats_resp {
> +	struct erdma_cmdq_query_resp_hdr hdr;
> +
> +	u64 tx_req_cnt;
> +	u64 tx_packets_cnt;
> +	u64 tx_bytes_cnt;
> +	u64 tx_drop_packets_cnt;
> +	u64 tx_bps_meter_drop_packets_cnt;
> +	u64 tx_pps_meter_drop_packets_cnt;
> +	u64 rx_packets_cnt;
> +	u64 rx_bytes_cnt;
> +	u64 rx_drop_packets_cnt;
> +	u64 rx_bps_meter_drop_packets_cnt;
> +	u64 rx_pps_meter_drop_packets_cnt;
> +};
> +
>  /* cap qword 0 definition */
>  #define ERDMA_CMD_DEV_CAP_MAX_CQE_MASK GENMASK_ULL(47, 40)
>  #define ERDMA_CMD_DEV_CAP_FLAGS_MASK GENMASK_ULL(31, 24)
> diff --git a/drivers/infiniband/hw/erdma/erdma_main.c b/drivers/infiniband/hw/erdma/erdma_main.c
> index 541e77aea494..0c35f7e464c8 100644
> --- a/drivers/infiniband/hw/erdma/erdma_main.c
> +++ b/drivers/infiniband/hw/erdma/erdma_main.c
> @@ -482,6 +482,7 @@ static const struct ib_device_ops erdma_device_ops = {
>  	.driver_id = RDMA_DRIVER_ERDMA,
>  	.uverbs_abi_ver = ERDMA_ABI_VERSION,
>  
> +	.alloc_hw_port_stats = erdma_alloc_hw_port_stats,
>  	.alloc_mr = erdma_ib_alloc_mr,
>  	.alloc_pd = erdma_alloc_pd,
>  	.alloc_ucontext = erdma_alloc_ucontext,
> @@ -493,6 +494,7 @@ static const struct ib_device_ops erdma_device_ops = {
>  	.destroy_cq = erdma_destroy_cq,
>  	.destroy_qp = erdma_destroy_qp,
>  	.get_dma_mr = erdma_get_dma_mr,
> +	.get_hw_stats = erdma_get_hw_stats,
>  	.get_port_immutable = erdma_get_port_immutable,
>  	.iw_accept = erdma_accept,
>  	.iw_add_ref = erdma_qp_get_ref,
> diff --git a/drivers/infiniband/hw/erdma/erdma_verbs.c b/drivers/infiniband/hw/erdma/erdma_verbs.c
> index c317947563fb..2c67e7f48336 100644
> --- a/drivers/infiniband/hw/erdma/erdma_verbs.c
> +++ b/drivers/infiniband/hw/erdma/erdma_verbs.c
> @@ -1599,10 +1599,9 @@ static int erdma_init_kernel_cq(struct erdma_cq *cq)
>  {
>  	struct erdma_dev *dev = to_edev(cq->ibcq.device);
>  
> -	cq->kern_cq.qbuf =
> -		dma_alloc_coherent(&dev->pdev->dev,
> -				   WARPPED_BUFSIZE(cq->depth << CQE_SHIFT),
> -				   &cq->kern_cq.qbuf_dma_addr, GFP_KERNEL);
> +	cq->kern_cq.qbuf = dma_alloc_coherent(
> +		&dev->pdev->dev, WARPPED_BUFSIZE(cq->depth << CQE_SHIFT),
> +		&cq->kern_cq.qbuf_dma_addr, GFP_KERNEL);

It looks like unrelated change.

>  	if (!cq->kern_cq.qbuf)
>  		return -ENOMEM;
>  
> @@ -1708,3 +1707,96 @@ void erdma_port_event(struct erdma_dev *dev, enum ib_event_type reason)
>  
>  	ib_dispatch_event(&event);
>  }
> +
> +enum counters {
> +	ERDMA_STATS_TX_REQS_CNT,
> +	ERDMA_STATS_TX_PACKETS_CNT,
> +	ERDMA_STATS_TX_BYTES_CNT,
> +	ERDMA_STATS_TX_DISABLE_DROP_CNT,
> +	ERDMA_STATS_TX_BPS_METER_DROP_CNT,
> +	ERDMA_STATS_TX_PPS_METER_DROP_CNT,
> +
> +	ERDMA_STATS_RX_PACKETS_CNT,
> +	ERDMA_STATS_RX_BYTES_CNT,
> +	ERDMA_STATS_RX_DISABLE_DROP_CNT,
> +	ERDMA_STATS_RX_BPS_METER_DROP_CNT,
> +	ERDMA_STATS_RX_PPS_METER_DROP_CNT,
> +
> +	ERDMA_STATS_MAX
> +};
> +
> +static const struct rdma_stat_desc erdma_descs[] = {
> +	[ERDMA_STATS_TX_REQS_CNT].name = "hw_tx_reqs_cnt",
> +	[ERDMA_STATS_TX_PACKETS_CNT].name = "hw_tx_packets_cnt",
> +	[ERDMA_STATS_TX_BYTES_CNT].name = "hw_tx_bytes_cnt",
> +	[ERDMA_STATS_TX_DISABLE_DROP_CNT].name = "hw_disable_drop_cnt",
> +	[ERDMA_STATS_TX_BPS_METER_DROP_CNT].name = "hw_bps_limit_drop_cnt",
> +	[ERDMA_STATS_TX_PPS_METER_DROP_CNT].name = "hw_pps_limit_drop_cnt",
> +	[ERDMA_STATS_RX_PACKETS_CNT].name = "hw_rx_packets_cnt",
> +	[ERDMA_STATS_RX_BYTES_CNT].name = "hw_rx_bytes_cnt",
> +	[ERDMA_STATS_RX_DISABLE_DROP_CNT].name = "hw_rx_disable_drop_cnt",
> +	[ERDMA_STATS_RX_BPS_METER_DROP_CNT].name = "hw_rx_bps_limit_drop_cnt",
> +	[ERDMA_STATS_RX_PPS_METER_DROP_CNT].name = "hw_rx_pps_limit_drop_cnt",
> +};

There is no need in "hw_" prefix, the counters will be in hw_counters
folder anyway.

> +
> +struct rdma_hw_stats *erdma_alloc_hw_port_stats(struct ib_device *device,
> +						u32 port_num)
> +{
> +	return rdma_alloc_hw_stats_struct(erdma_descs, ERDMA_STATS_MAX,
> +					  RDMA_HW_STATS_DEFAULT_LIFESPAN);
> +}
> +
> +int erdma_query_hw_stats(struct erdma_dev *dev, struct rdma_hw_stats *stats)
> +{
> +	struct erdma_cmdq_query_stats_resp *resp;
> +	struct erdma_cmdq_query_req req;
> +	dma_addr_t dma_addr;
> +	int err;
> +
> +	erdma_cmdq_build_reqhdr(&req.hdr, CMDQ_SUBMOD_COMMON,
> +				CMDQ_OPCODE_GET_STATS);
> +
> +	resp = dma_pool_alloc(dev->resp_pool, GFP_KERNEL | __GFP_ZERO,
> +			      &dma_addr);

dma_pool_zalloc()

> +	if (!resp)
> +		return -ENOMEM;
> +
> +	req.target_addr = dma_addr;
> +	req.target_length = ERDMA_HW_RESP_SIZE;
> +
> +	err = erdma_post_cmd_wait(&dev->cmdq, &req, sizeof(req), NULL, NULL);
> +	if (err)
> +		goto out;
> +
> +	if (resp->hdr.magic != ERDMA_HW_RESP_MAGIC) {
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	memcpy(&stats->value[0], &resp->tx_req_cnt,
> +	       sizeof(u64) * stats->num_counters);
> +
> +out:
> +	dma_pool_free(dev->resp_pool, resp, dma_addr);
> +
> +	return err;
> +}
> +
> +int erdma_get_hw_stats(struct ib_device *ibdev, struct rdma_hw_stats *stats,
> +		       u32 port, int index)
> +{
> +	struct erdma_dev *dev = to_edev(ibdev);
> +	int ret;
> +
> +	if (port == 0)
> +		return 0;
> +
> +	if (port > 1)

Is it possible?

> +		return -EINVAL;
> +
> +	ret = erdma_query_hw_stats(dev, stats);
> +	if (ret)
> +		return ret;
> +
> +	return stats->num_counters;
> +}
> diff --git a/drivers/infiniband/hw/erdma/erdma_verbs.h b/drivers/infiniband/hw/erdma/erdma_verbs.h
> index eb9c0f92fb6f..db6018529ccc 100644
> --- a/drivers/infiniband/hw/erdma/erdma_verbs.h
> +++ b/drivers/infiniband/hw/erdma/erdma_verbs.h
> @@ -361,5 +361,9 @@ int erdma_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sg, int sg_nents,
>  		    unsigned int *sg_offset);
>  void erdma_port_event(struct erdma_dev *dev, enum ib_event_type reason);
>  void erdma_set_mtu(struct erdma_dev *dev, u32 mtu);
> +struct rdma_hw_stats *erdma_alloc_hw_port_stats(struct ib_device *device,
> +						u32 port_num);
> +int erdma_get_hw_stats(struct ib_device *ibdev, struct rdma_hw_stats *stats,
> +		       u32 port, int index);
>  
>  #endif
> -- 
> 2.31.1
> 
> 

Re: [PATCH for-next 1/2] RDMA/erdma: Introduce dma pool for hardware responses of CMDQ requests

[Cheng Xu]

On 12/20/23 6:12 PM, Leon Romanovsky wrote:
> On Wed, Dec 20, 2023 at 04:54:23PM +0800, Cheng Xu wrote:
>> Hardware response, such as the result of query statistics, may be too
>> long to be directly accommodated within the CQE structure. To address
>> this, we introduce a DMA pool to hold the hardware's responses of CMDQ
>> requests.
>>
>> Signed-off-by: Cheng Xu <chengyou@linux.alibaba.com>
>> ---
>>  drivers/infiniband/hw/erdma/erdma.h      |  2 ++
>>  drivers/infiniband/hw/erdma/erdma_main.c | 38 ++++++++++++++++++++++--
>>  2 files changed, 38 insertions(+), 2 deletions(-)
>>

<...>

>> +static int erdma_dma_pools_init(struct erdma_dev *dev)
>> +{
>> +	dev->resp_pool = dma_pool_create("erdma_resp_pool", &dev->pdev->dev,
>> +					 ERDMA_HW_RESP_SIZE, ERDMA_HW_RESP_SIZE,
>> +					 0);
>> +	if (!dev->resp_pool)
>> +		return -ENOMEM;
>> +
>> +	return 0;
>> +}
>> +
>> +static void erdma_dma_pools_destroy(struct erdma_dev *dev)
>> +{
>> +	dma_pool_destroy(dev->resp_pool);
>> +}
> 
> Please don't add extra layer of functions which will be called in same
> file anyway. Call directly to dma_pool_destroy(), same goes for dma_pool_create().

Get it.

Will fix in v2.

Thanks,
Cheng Xu

 
> Thanks

Re: [PATCH for-next 2/2] RDMA/erdma: Add hardware statistics support

[Cheng Xu]

On 12/20/23 6:16 PM, Leon Romanovsky wrote:
> On Wed, Dec 20, 2023 at 04:54:24PM +0800, Cheng Xu wrote:
>> First, we add a new command to query hardware statistics, and then
>> implement two functions: ib_device_ops.alloc_hw_port_stats and
>> ib_device_ops.get_hw_stats to allow rdma tool can get the statistics
>> of erdma device.
>>

<...>

>> diff --git a/drivers/infiniband/hw/erdma/erdma_verbs.c b/drivers/infiniband/hw/erdma/erdma_verbs.c
>> index c317947563fb..2c67e7f48336 100644
>> --- a/drivers/infiniband/hw/erdma/erdma_verbs.c
>> +++ b/drivers/infiniband/hw/erdma/erdma_verbs.c
>> @@ -1599,10 +1599,9 @@ static int erdma_init_kernel_cq(struct erdma_cq *cq)
>>  {
>>  	struct erdma_dev *dev = to_edev(cq->ibcq.device);
>>  
>> -	cq->kern_cq.qbuf =
>> -		dma_alloc_coherent(&dev->pdev->dev,
>> -				   WARPPED_BUFSIZE(cq->depth << CQE_SHIFT),
>> -				   &cq->kern_cq.qbuf_dma_addr, GFP_KERNEL);
>> +	cq->kern_cq.qbuf = dma_alloc_coherent(
>> +		&dev->pdev->dev, WARPPED_BUFSIZE(cq->depth << CQE_SHIFT),
>> +		&cq->kern_cq.qbuf_dma_addr, GFP_KERNEL);
> 
> It looks like unrelated change.
>

Oh, this is changed by clang-format, and I forgot to remove it.

Will remove it in v2.

>>  	if (!cq->kern_cq.qbuf)
>>  		return -ENOMEM;
>>

  
<...>

>> +
>> +static const struct rdma_stat_desc erdma_descs[] = {
>> +	[ERDMA_STATS_TX_REQS_CNT].name = "hw_tx_reqs_cnt",
>> +	[ERDMA_STATS_TX_PACKETS_CNT].name = "hw_tx_packets_cnt",
>> +	[ERDMA_STATS_TX_BYTES_CNT].name = "hw_tx_bytes_cnt",
>> +	[ERDMA_STATS_TX_DISABLE_DROP_CNT].name = "hw_disable_drop_cnt",
>> +	[ERDMA_STATS_TX_BPS_METER_DROP_CNT].name = "hw_bps_limit_drop_cnt",
>> +	[ERDMA_STATS_TX_PPS_METER_DROP_CNT].name = "hw_pps_limit_drop_cnt",
>> +	[ERDMA_STATS_RX_PACKETS_CNT].name = "hw_rx_packets_cnt",
>> +	[ERDMA_STATS_RX_BYTES_CNT].name = "hw_rx_bytes_cnt",
>> +	[ERDMA_STATS_RX_DISABLE_DROP_CNT].name = "hw_rx_disable_drop_cnt",
>> +	[ERDMA_STATS_RX_BPS_METER_DROP_CNT].name = "hw_rx_bps_limit_drop_cnt",
>> +	[ERDMA_STATS_RX_PPS_METER_DROP_CNT].name = "hw_rx_pps_limit_drop_cnt",
>> +};
> 
> There is no need in "hw_" prefix, the counters will be in hw_counters
> folder anyway.
>

Will fix in v2.

>> +
>> +struct rdma_hw_stats *erdma_alloc_hw_port_stats(struct ib_device *device,
>> +						u32 port_num)
>> +{
>> +	return rdma_alloc_hw_stats_struct(erdma_descs, ERDMA_STATS_MAX,
>> +					  RDMA_HW_STATS_DEFAULT_LIFESPAN);
>> +}
>> +
>> +int erdma_query_hw_stats(struct erdma_dev *dev, struct rdma_hw_stats *stats)
>> +{
>> +	struct erdma_cmdq_query_stats_resp *resp;
>> +	struct erdma_cmdq_query_req req;
>> +	dma_addr_t dma_addr;
>> +	int err;
>> +
>> +	erdma_cmdq_build_reqhdr(&req.hdr, CMDQ_SUBMOD_COMMON,
>> +				CMDQ_OPCODE_GET_STATS);
>> +
>> +	resp = dma_pool_alloc(dev->resp_pool, GFP_KERNEL | __GFP_ZERO,
>> +			      &dma_addr);
> 
> dma_pool_zalloc()
> 

Thanks, it's better, Will fix in v2.

>> +	if (!resp)
>> +		return -ENOMEM;
>> +
>> +	req.target_addr = dma_addr;
>> +	req.target_length = ERDMA_HW_RESP_SIZE;
>> +
>> +	err = erdma_post_cmd_wait(&dev->cmdq, &req, sizeof(req), NULL, NULL);
>> +	if (err)
>> +		goto out;
>> +
>> +	if (resp->hdr.magic != ERDMA_HW_RESP_MAGIC) {
>> +		err = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	memcpy(&stats->value[0], &resp->tx_req_cnt,
>> +	       sizeof(u64) * stats->num_counters);
>> +
>> +out:
>> +	dma_pool_free(dev->resp_pool, resp, dma_addr);
>> +
>> +	return err;
>> +}
>> +
>> +int erdma_get_hw_stats(struct ib_device *ibdev, struct rdma_hw_stats *stats,
>> +		       u32 port, int index)
>> +{
>> +	struct erdma_dev *dev = to_edev(ibdev);
>> +	int ret;
>> +
>> +	if (port == 0)
>> +		return 0;
>> +
>> +	if (port > 1)
> 
> Is it possible?
>
Thanks, I checked the core code, and core code will make sure that the port index
is valid. Will remove this check in v2.


Cheng Xu