[PATCH 0/3] crypto: hisilicon - fix resource contention and abnormal branch processing issues

[PATCH 0/3] crypto: hisilicon - fix resource contention and abnormal branch processing issues

[Chenghai Huang]

1. Fix the queue resource competition issue in hpre and zip.
2. Fix the abnormal branch processing problem in sgl.
3. Adjust the header file and printing format problems in trng.

Chenghai Huang (2):
  crypto: hisilicon - add a lock for the qp send operation
  crypto: hisilicon/trng - modifying the order of header files

Yang Shen (1):
  crypto: hisilicon - fix missed error branch

 drivers/crypto/hisilicon/hpre/hpre_crypto.c |  2 ++
 drivers/crypto/hisilicon/sgl.c              | 14 ++++++++++----
 drivers/crypto/hisilicon/trng/trng.c        |  4 ++--
 drivers/crypto/hisilicon/zip/zip_crypto.c   |  3 +++
 4 files changed, 17 insertions(+), 6 deletions(-)

-- 
2.33.0

[PATCH 1/3] crypto: hisilicon - fix missed error branch

[Chenghai Huang]

From: Yang Shen <shenyang39@huawei.com>

If an error occurs in the process after the SGL is mapped
successfully, it need to unmap the SGL.

Otherwise, memory problems may occur.

Signed-off-by: Yang Shen <shenyang39@huawei.com>
---
 drivers/crypto/hisilicon/sgl.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/hisilicon/sgl.c b/drivers/crypto/hisilicon/sgl.c
index 568acd0aee3f..c974f95cd126 100644
--- a/drivers/crypto/hisilicon/sgl.c
+++ b/drivers/crypto/hisilicon/sgl.c
@@ -225,7 +225,7 @@ hisi_acc_sg_buf_map_to_hw_sgl(struct device *dev,
 	dma_addr_t curr_sgl_dma = 0;
 	struct acc_hw_sge *curr_hw_sge;
 	struct scatterlist *sg;
-	int sg_n;
+	int sg_n, ret;
 
 	if (!dev || !sgl || !pool || !hw_sgl_dma || index >= pool->count)
 		return ERR_PTR(-EINVAL);
@@ -240,14 +240,15 @@ hisi_acc_sg_buf_map_to_hw_sgl(struct device *dev,
 
 	if (sg_n_mapped > pool->sge_nr) {
 		dev_err(dev, "the number of entries in input scatterlist is bigger than SGL pool setting.\n");
-		return ERR_PTR(-EINVAL);
+		ret = -EINVAL;
+		goto err_unmap;
 	}
 
 	curr_hw_sgl = acc_get_sgl(pool, index, &curr_sgl_dma);
 	if (IS_ERR(curr_hw_sgl)) {
 		dev_err(dev, "Get SGL error!\n");
-		dma_unmap_sg(dev, sgl, sg_n, DMA_BIDIRECTIONAL);
-		return ERR_PTR(-ENOMEM);
+		ret = -ENOMEM;
+		goto err_unmap;
 	}
 	curr_hw_sgl->entry_length_in_sgl = cpu_to_le16(pool->sge_nr);
 	curr_hw_sge = curr_hw_sgl->sge_entries;
@@ -262,6 +263,11 @@ hisi_acc_sg_buf_map_to_hw_sgl(struct device *dev,
 	*hw_sgl_dma = curr_sgl_dma;
 
 	return curr_hw_sgl;
+
+err_unmap:
+	dma_unmap_sg(dev, sgl, sg_n, DMA_BIDIRECTIONAL);
+
+	return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(hisi_acc_sg_buf_map_to_hw_sgl);
 
-- 
2.33.0

[PATCH 2/3] crypto: hisilicon - add a lock for the qp send operation

[Chenghai Huang]

Apply for a lock before the qp send operation to ensure no
resource competition in multi-concurrency situations.

This modification has almost no impact on performance.

Signed-off-by: Chenghai Huang <huangchenghai2@huawei.com>
---
 drivers/crypto/hisilicon/hpre/hpre_crypto.c | 2 ++
 drivers/crypto/hisilicon/zip/zip_crypto.c   | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/crypto/hisilicon/hpre/hpre_crypto.c b/drivers/crypto/hisilicon/hpre/hpre_crypto.c
index 764532a6ca82..c167dbd6c7d6 100644
--- a/drivers/crypto/hisilicon/hpre/hpre_crypto.c
+++ b/drivers/crypto/hisilicon/hpre/hpre_crypto.c
@@ -575,7 +575,9 @@ static int hpre_send(struct hpre_ctx *ctx, struct hpre_sqe *msg)
 
 	do {
 		atomic64_inc(&dfx[HPRE_SEND_CNT].value);
+		spin_lock_bh(&ctx->req_lock);
 		ret = hisi_qp_send(ctx->qp, msg);
+		spin_unlock_bh(&ctx->req_lock);
 		if (ret != -EBUSY)
 			break;
 		atomic64_inc(&dfx[HPRE_SEND_BUSY_CNT].value);
diff --git a/drivers/crypto/hisilicon/zip/zip_crypto.c b/drivers/crypto/hisilicon/zip/zip_crypto.c
index 94e2d66b04b6..e3a31e3416be 100644
--- a/drivers/crypto/hisilicon/zip/zip_crypto.c
+++ b/drivers/crypto/hisilicon/zip/zip_crypto.c
@@ -213,6 +213,7 @@ static int hisi_zip_do_work(struct hisi_zip_qp_ctx *qp_ctx,
 {
 	struct hisi_acc_sgl_pool *pool = qp_ctx->sgl_pool;
 	struct hisi_zip_dfx *dfx = &qp_ctx->zip_dev->dfx;
+	struct hisi_zip_req_q *req_q = &qp_ctx->req_q;
 	struct acomp_req *a_req = req->req;
 	struct hisi_qp *qp = qp_ctx->qp;
 	struct device *dev = &qp->qm->pdev->dev;
@@ -244,7 +245,9 @@ static int hisi_zip_do_work(struct hisi_zip_qp_ctx *qp_ctx,
 
 	/* send command to start a task */
 	atomic64_inc(&dfx->send_cnt);
+	write_lock(&req_q->req_lock);
 	ret = hisi_qp_send(qp, &zip_sqe);
+	write_unlock(&req_q->req_lock);
 	if (unlikely(ret < 0)) {
 		atomic64_inc(&dfx->send_busy_cnt);
 		ret = -EAGAIN;
-- 
2.33.0

Re: [PATCH 2/3] crypto: hisilicon - add a lock for the qp send operation

[Kuan-Wei Chiu]

On Fri, Aug 30, 2024 at 06:17:17PM +0800, Chenghai Huang wrote:
> Apply for a lock before the qp send operation to ensure no
> resource competition in multi-concurrency situations.
> 
> This modification has almost no impact on performance.
> 
> Signed-off-by: Chenghai Huang <huangchenghai2@huawei.com>
> ---
>  drivers/crypto/hisilicon/hpre/hpre_crypto.c | 2 ++
>  drivers/crypto/hisilicon/zip/zip_crypto.c   | 3 +++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/crypto/hisilicon/hpre/hpre_crypto.c b/drivers/crypto/hisilicon/hpre/hpre_crypto.c
> index 764532a6ca82..c167dbd6c7d6 100644
> --- a/drivers/crypto/hisilicon/hpre/hpre_crypto.c
> +++ b/drivers/crypto/hisilicon/hpre/hpre_crypto.c
> @@ -575,7 +575,9 @@ static int hpre_send(struct hpre_ctx *ctx, struct hpre_sqe *msg)
>  
>  	do {
>  		atomic64_inc(&dfx[HPRE_SEND_CNT].value);
> +		spin_lock_bh(&ctx->req_lock);
>  		ret = hisi_qp_send(ctx->qp, msg);
> +		spin_unlock_bh(&ctx->req_lock);
>  		if (ret != -EBUSY)
>  			break;
>  		atomic64_inc(&dfx[HPRE_SEND_BUSY_CNT].value);
> diff --git a/drivers/crypto/hisilicon/zip/zip_crypto.c b/drivers/crypto/hisilicon/zip/zip_crypto.c
> index 94e2d66b04b6..e3a31e3416be 100644
> --- a/drivers/crypto/hisilicon/zip/zip_crypto.c
> +++ b/drivers/crypto/hisilicon/zip/zip_crypto.c
> @@ -213,6 +213,7 @@ static int hisi_zip_do_work(struct hisi_zip_qp_ctx *qp_ctx,
>  {
>  	struct hisi_acc_sgl_pool *pool = qp_ctx->sgl_pool;
>  	struct hisi_zip_dfx *dfx = &qp_ctx->zip_dev->dfx;
> +	struct hisi_zip_req_q *req_q = &qp_ctx->req_q;
>  	struct acomp_req *a_req = req->req;
>  	struct hisi_qp *qp = qp_ctx->qp;
>  	struct device *dev = &qp->qm->pdev->dev;
> @@ -244,7 +245,9 @@ static int hisi_zip_do_work(struct hisi_zip_qp_ctx *qp_ctx,
>  
>  	/* send command to start a task */
>  	atomic64_inc(&dfx->send_cnt);
> +	write_lock(&req_q->req_lock);
>  	ret = hisi_qp_send(qp, &zip_sqe);
> +	write_unlock(&req_q->req_lock);

Hi Chenghai,

Thanks for your patch. Since Herbert has already applied a patch [1]
changing rw_lock to spinlock in the hisilicon zip controller driver,
applying your patch might cause a conflict. Could you rebase on
Herbert's crypto tree and update write_lock() and write_unlock() to
spin_lock() and spin_unlock(), respectively?

[1]: https://lore.kernel.org/lkml/20240823183856.561166-1-visitorckw@gmail.com/

Regards,
Kuan-Wei

>  	if (unlikely(ret < 0)) {
>  		atomic64_inc(&dfx->send_busy_cnt);
>  		ret = -EAGAIN;
> -- 
> 2.33.0
> 
> 

[PATCH 3/3] crypto: hisilicon/trng - modifying the order of header files

[Chenghai Huang]

Header files is included Order-ref: standard library headers,
OS library headers, and project-specific headers. This patch
modifies the order of header files according to suggestions.

In addition, use %u to print unsigned int variables to prevent
overflow.

Signed-off-by: Chenghai Huang <huangchenghai2@huawei.com>
---
 drivers/crypto/hisilicon/trng/trng.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/hisilicon/trng/trng.c b/drivers/crypto/hisilicon/trng/trng.c
index 451b167bcc73..66c551ecdee8 100644
--- a/drivers/crypto/hisilicon/trng/trng.c
+++ b/drivers/crypto/hisilicon/trng/trng.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2019 HiSilicon Limited. */
 
+#include <crypto/internal/rng.h>
 #include <linux/acpi.h>
 #include <linux/crypto.h>
 #include <linux/err.h>
@@ -13,7 +14,6 @@
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
 #include <linux/random.h>
-#include <crypto/internal/rng.h>
 
 #define HISI_TRNG_REG		0x00F0
 #define HISI_TRNG_BYTES		4
@@ -121,7 +121,7 @@ static int hisi_trng_generate(struct crypto_rng *tfm, const u8 *src,
 	u32 i;
 
 	if (dlen > SW_DRBG_BLOCKS_NUM * SW_DRBG_BYTES || dlen == 0) {
-		pr_err("dlen(%d) exceeds limit(%d)!\n", dlen,
+		pr_err("dlen(%u) exceeds limit(%d)!\n", dlen,
 			SW_DRBG_BLOCKS_NUM * SW_DRBG_BYTES);
 		return -EINVAL;
 	}
-- 
2.33.0