[PATCH 0/4] bugfix and feature for Hisilicon DMA PMD

[PATCH 0/4] bugfix and feature for Hisilicon DMA PMD

[Chengwen Feng]

This patch contains four patches for Hisilicon DMA PMD. 

Chengwen Feng (4):
  dma/hisilicon: fix return last-idx when no DMA completed
  app/test: support test last-idx when no DMA completed
  dma/hisilicon: enhance robustness of scan CQ
  dma/hisilicon: support vchan-status ops

 app/test/test_dmadev.c              | 15 +++++++++
 drivers/dma/hisilicon/hisi_dmadev.c | 50 ++++++++++++++++++++++++++---
 drivers/dma/hisilicon/hisi_dmadev.h |  7 +++-
 3 files changed, 66 insertions(+), 6 deletions(-)

-- 
2.33.0

[PATCH 1/4] dma/hisilicon: fix return last-idx when no DMA completed

[Chengwen Feng]

If no DMA request is completed, the ring_idx of the last completed
operation need returned by last_idx parameter. This patch fixes it.

Fixes: 2db4f0b82360 ("dma/hisilicon: add data path")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 drivers/dma/hisilicon/hisi_dmadev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/hisilicon/hisi_dmadev.c b/drivers/dma/hisilicon/hisi_dmadev.c
index 9cef2cbfbe..f5c3cd914d 100644
--- a/drivers/dma/hisilicon/hisi_dmadev.c
+++ b/drivers/dma/hisilicon/hisi_dmadev.c
@@ -702,12 +702,12 @@ hisi_dma_completed(void *dev_private,
 		}
 		sq_head = (sq_head + 1) & hw->sq_depth_mask;
 	}
+	*last_idx = hw->cridx + i - 1;
 	if (i > 0) {
 		hw->cridx += i;
-		*last_idx = hw->cridx - 1;
 		hw->sq_head = sq_head;
+		hw->completed += i;
 	}
-	hw->completed += i;
 
 	return i;
 }
@@ -761,12 +761,12 @@ hisi_dma_completed_status(void *dev_private,
 		hw->status[sq_head] = HISI_DMA_STATUS_SUCCESS;
 		sq_head = (sq_head + 1) & hw->sq_depth_mask;
 	}
+	*last_idx = hw->cridx + cpl_num - 1;
 	if (likely(cpl_num > 0)) {
 		hw->cridx += cpl_num;
-		*last_idx = hw->cridx - 1;
 		hw->sq_head = sq_head;
+		hw->completed += cpl_num;
 	}
-	hw->completed += cpl_num;
 
 	return cpl_num;
 }
-- 
2.33.0

[PATCH 2/4] app/test: support test last-idx when no DMA completed

[Chengwen Feng]

If no DMA request is completed, the ring_idx of the last completed
operation need returned by last_idx parameter. This patch adds
testcase for it.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 app/test/test_dmadev.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
index 2b097e0f47..a7651a486f 100644
--- a/app/test/test_dmadev.c
+++ b/app/test/test_dmadev.c
@@ -177,6 +177,7 @@ do_multi_copies(int16_t dev_id, uint16_t vchan,
 static int
 test_enqueue_copies(int16_t dev_id, uint16_t vchan)
 {
+	enum rte_dma_status_code status;
 	unsigned int i;
 	uint16_t id;
 
@@ -215,6 +216,20 @@ test_enqueue_copies(int16_t dev_id, uint16_t vchan)
 			ERR_RETURN("Error:incorrect job id received, %u [expected %u]\n",
 					id, id_count);
 
+		/* check for completed and id when no job done */
+		if (rte_dma_completed(dev_id, vchan, 1, &id, NULL) != 0)
+			ERR_RETURN("Error with rte_dma_completed when no job done\n");
+		if (id != id_count)
+			ERR_RETURN("Error:incorrect job id received when no job done, %u [expected %u]\n",
+					id, id_count);
+
+		/* check for completed_status and id when no job done */
+		if (rte_dma_completed_status(dev_id, vchan, 1, &id, &status) != 0)
+			ERR_RETURN("Error with rte_dma_completed_status when no job done\n");
+		if (id != id_count)
+			ERR_RETURN("Error:incorrect job id received when no job done, %u [expected %u]\n",
+					id, id_count);
+
 		rte_pktmbuf_free(src);
 		rte_pktmbuf_free(dst);
 
-- 
2.33.0

Re: [PATCH 2/4] app/test: support test last-idx when no DMA completed

[Kevin Laatz]

Tested with IOAT and IDXD, both pass unit tests.

Tested-by: Kevin Laatz <kevin.laatz@intel.com>

On 27/05/2022 04:40, Chengwen Feng wrote:
> If no DMA request is completed, the ring_idx of the last completed
> operation need returned by last_idx parameter. This patch adds
> testcase for it.
>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>   app/test/test_dmadev.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
> index 2b097e0f47..a7651a486f 100644
> --- a/app/test/test_dmadev.c
> +++ b/app/test/test_dmadev.c
> @@ -177,6 +177,7 @@ do_multi_copies(int16_t dev_id, uint16_t vchan,
>   static int
>   test_enqueue_copies(int16_t dev_id, uint16_t vchan)
>   {
> +	enum rte_dma_status_code status;
>   	unsigned int i;
>   	uint16_t id;
>   
> @@ -215,6 +216,20 @@ test_enqueue_copies(int16_t dev_id, uint16_t vchan)
>   			ERR_RETURN("Error:incorrect job id received, %u [expected %u]\n",
>   					id, id_count);
>   
> +		/* check for completed and id when no job done */
> +		if (rte_dma_completed(dev_id, vchan, 1, &id, NULL) != 0)
> +			ERR_RETURN("Error with rte_dma_completed when no job done\n");
> +		if (id != id_count)
> +			ERR_RETURN("Error:incorrect job id received when no job done, %u [expected %u]\n",
> +					id, id_count);
> +
> +		/* check for completed_status and id when no job done */
> +		if (rte_dma_completed_status(dev_id, vchan, 1, &id, &status) != 0)
> +			ERR_RETURN("Error with rte_dma_completed_status when no job done\n");
> +		if (id != id_count)
> +			ERR_RETURN("Error:incorrect job id received when no job done, %u [expected %u]\n",
> +					id, id_count);
> +
>   		rte_pktmbuf_free(src);
>   		rte_pktmbuf_free(dst);
>   

[PATCH 3/4] dma/hisilicon: enhance robustness of scan CQ

[Chengwen Feng]

The CQ (completion queue) descriptors were updated by hardware, and then
scanned by driver to retrieve hardware completion status.

This patch enhances robustness by following:
1. replace while (true) with a finite loop to avoid potential dead loop.
2. check the csq_head field in CQ descriptor to avoid status array
overflows.

Fixes: 2db4f0b82360 ("dma/hisilicon: add data path")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 drivers/dma/hisilicon/hisi_dmadev.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/hisilicon/hisi_dmadev.c b/drivers/dma/hisilicon/hisi_dmadev.c
index f5c3cd914d..fbe09284ed 100644
--- a/drivers/dma/hisilicon/hisi_dmadev.c
+++ b/drivers/dma/hisilicon/hisi_dmadev.c
@@ -634,7 +634,7 @@ hisi_dma_scan_cq(struct hisi_dma_dev *hw)
 	uint16_t count = 0;
 	uint64_t misc;
 
-	while (true) {
+	while (count < hw->cq_depth) {
 		cqe = &hw->cqe[cq_head];
 		misc = cqe->misc;
 		misc = rte_le_to_cpu_64(misc);
@@ -642,6 +642,16 @@ hisi_dma_scan_cq(struct hisi_dma_dev *hw)
 			break;
 
 		csq_head = FIELD_GET(CQE_SQ_HEAD_MASK, misc);
+		if (unlikely(csq_head > hw->sq_depth_mask)) {
+			/**
+			 * Defensive programming to prevent overflow of the
+			 * status array indexed by csq_head. Only error logs
+			 * are used for prompting.
+			 */
+			HISI_DMA_ERR(hw, "invalid csq_head:%u!\n", csq_head);
+			count = 0;
+			break;
+		}
 		if (unlikely(misc & CQE_STATUS_MASK))
 			hw->status[csq_head] = FIELD_GET(CQE_STATUS_MASK,
 							 misc);
-- 
2.33.0

[PATCH 4/4] dma/hisilicon: support vchan-status ops

[Chengwen Feng]

This patch adds support for vchan-status ops.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 drivers/dma/hisilicon/hisi_dmadev.c | 30 +++++++++++++++++++++++++++++
 drivers/dma/hisilicon/hisi_dmadev.h |  7 ++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/hisilicon/hisi_dmadev.c b/drivers/dma/hisilicon/hisi_dmadev.c
index fbe09284ed..9494b60779 100644
--- a/drivers/dma/hisilicon/hisi_dmadev.c
+++ b/drivers/dma/hisilicon/hisi_dmadev.c
@@ -461,6 +461,27 @@ hisi_dma_stats_reset(struct rte_dma_dev *dev, uint16_t vchan)
 	return 0;
 }
 
+static int
+hisi_dma_vchan_status(const struct rte_dma_dev *dev, uint16_t vchan,
+		      enum rte_dma_vchan_status *status)
+{
+	struct hisi_dma_dev *hw = dev->data->dev_private;
+	uint32_t val;
+
+	RTE_SET_USED(vchan);
+
+	val = hisi_dma_read_queue(hw, HISI_DMA_QUEUE_FSM_REG);
+	val = FIELD_GET(HISI_DMA_QUEUE_FSM_STS_M, val);
+	if (val == HISI_DMA_STATE_RUN)
+		*status = RTE_DMA_VCHAN_ACTIVE;
+	else if (val == HISI_DMA_STATE_CPL)
+		*status = RTE_DMA_VCHAN_IDLE;
+	else
+		*status = RTE_DMA_VCHAN_HALTED_ERROR;
+
+	return 0;
+}
+
 static void
 hisi_dma_dump_range(struct hisi_dma_dev *hw, FILE *f, uint32_t start,
 		    uint32_t end)
@@ -816,6 +837,14 @@ hisi_dma_gen_dev_name(const struct rte_pci_device *pci_dev,
  *                           dev_stop|          |
  *                                   |          v
  *                                ------------------
+ *                                |      CPL       |
+ *                                ------------------
+ *                                   ^          |
+ *                      hardware     |          |
+ *                      completed all|          |dev_submit
+ *                      descriptors  |          |
+ *                                   |          |
+ *                                ------------------
  *                                |      RUN       |
  *                                ------------------
  *
@@ -829,6 +858,7 @@ static const struct rte_dma_dev_ops hisi_dmadev_ops = {
 	.vchan_setup      = hisi_dma_vchan_setup,
 	.stats_get        = hisi_dma_stats_get,
 	.stats_reset      = hisi_dma_stats_reset,
+	.vchan_status     = hisi_dma_vchan_status,
 	.dev_dump         = hisi_dma_dump,
 };
 
diff --git a/drivers/dma/hisilicon/hisi_dmadev.h b/drivers/dma/hisilicon/hisi_dmadev.h
index 90b85322ca..deb1357eea 100644
--- a/drivers/dma/hisilicon/hisi_dmadev.h
+++ b/drivers/dma/hisilicon/hisi_dmadev.h
@@ -132,11 +132,16 @@ enum {
 
 /**
  * In fact, there are multiple states, but it need to pay attention to
- * the following two states for the driver:
+ * the following three states for the driver:
  */
 enum {
 	HISI_DMA_STATE_IDLE = 0,
 	HISI_DMA_STATE_RUN,
+	/**
+	 * All of the submitted descriptor are finished, and the queue
+	 * is waiting for new descriptors.
+	 */
+	HISI_DMA_STATE_CPL,
 };
 
 /**
-- 
2.33.0

Re: [PATCH 0/4] bugfix and feature for Hisilicon DMA PMD

[Thomas Monjalon]

27/05/2022 05:40, Chengwen Feng:
> This patch contains four patches for Hisilicon DMA PMD. 
> 
> Chengwen Feng (4):
>   dma/hisilicon: fix return last-idx when no DMA completed
>   app/test: support test last-idx when no DMA completed
>   dma/hisilicon: enhance robustness of scan CQ
>   dma/hisilicon: support vchan-status ops

Series applied, thanks.