RE: [PATCH 12/13] crypto: hisilicon/zip - Use temporary sqe when doing work

[Song Bao Hua <song.bao.hua@hisilicon.com>]

> -----Original Message-----
> From: linux-crypto-owner@vger.kernel.org [mailto:linux-crypto-owner@vger.kernel.org] On Behalf Of Shukun Tan
> Sent: Friday, May 8, 2020 6:58 PM
> To: herbert@gondor.apana.org.au; davem@davemloft.net
> Cc: linux-crypto@vger.kernel.org; Xu Zaibo <xuzaibo@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>
> Subject: [PATCH 12/13] crypto: hisilicon/zip - Use temporary sqe when doing work

> From: Zhou Wang <wangzhou1@hisilicon.com>

> Currently zip sqe is stored in hisi_zip_qp_ctx, which will bring corruption with multiple parallel users of the crypto tfm.

> This patch removes the zip_sqe in hisi_zip_qp_ctx and uses a temporary sqe instead.

This looks like a quite correct fix as in the old code, the 2nd request will overwrite the zip_sqe of the 1st request if we send the 2nd request before the 1st one is completed.
So this will lead to the mistakes of both request1 and request2.

unfortunately, it seems the hang issue can still be reproduced with this patch applied if we ask multi-threads running on multi-cores to send requests in parallel. Maybe something more needs fix?

> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Shukun Tan <tanshukun1@huawei.com>
> ---
>  drivers/crypto/hisilicon/zip/zip_crypto.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)

> diff --git a/drivers/crypto/hisilicon/zip/zip_crypto.c b/drivers/crypto/hisilicon/zip/zip_crypto.c
> index 369ec32..5fb9d4b 100644
> --- a/drivers/crypto/hisilicon/zip/zip_crypto.c
> +++ b/drivers/crypto/hisilicon/zip/zip_crypto.c
> @@ -64,7 +64,6 @@ struct hisi_zip_req_q {
>  
>  struct hisi_zip_qp_ctx {
>  	struct hisi_qp *qp;
> -	struct hisi_zip_sqe zip_sqe;
>  	struct hisi_zip_req_q req_q;
>  	struct hisi_acc_sgl_pool *sgl_pool;
>  	struct hisi_zip *zip_dev;
> @@ -484,11 +483,11 @@ static struct hisi_zip_req *hisi_zip_create_req(struct acomp_req *req,  static int hisi_zip_do_work(struct hisi_zip_req *req,
>  			    struct hisi_zip_qp_ctx *qp_ctx)
>  {
> -	struct hisi_zip_sqe *zip_sqe = &qp_ctx->zip_sqe;
>  	struct acomp_req *a_req = req->req;
>  	struct hisi_qp *qp = qp_ctx->qp;
>  	struct device *dev = &qp->qm->pdev->dev;
>  	struct hisi_acc_sgl_pool *pool = qp_ctx->sgl_pool;
> +	struct hisi_zip_sqe zip_sqe;
>  	dma_addr_t input;
>  	dma_addr_t output;
>  	int ret;
> @@ -511,13 +510,13 @@ static int hisi_zip_do_work(struct hisi_zip_req *req,
>  	}
>  	req->dma_dst = output;
>  
> -	hisi_zip_fill_sqe(zip_sqe, qp->req_type, input, output, a_req->slen,
> +	hisi_zip_fill_sqe(&zip_sqe, qp->req_type, input, output, a_req->slen,
>  			  a_req->dlen, req->sskip, req->dskip);
> -	hisi_zip_config_buf_type(zip_sqe, HZIP_SGL);
> -	hisi_zip_config_tag(zip_sqe, req->req_id);
> +	hisi_zip_config_buf_type(&zip_sqe, HZIP_SGL);
> +	hisi_zip_config_tag(&zip_sqe, req->req_id);
>  
>  	/* send command to start a task */
> -	ret = hisi_qp_send(qp, zip_sqe);
> +	ret = hisi_qp_send(qp, &zip_sqe);
>  	if (ret < 0)
>  		goto err_unmap_output;
>  

Best Regards
Barry