On 2020/5/9 11:42, Song Bao Hua wrote:
>> -----Original Message-----
>> From: linux-crypto-ow...@vger.kernel.org
>> [mailto:linux-crypto-ow...@vger.kernel.org] On Behalf Of Shukun Tan
>> Sent: Friday, May 8, 2020 6:58 PM
>> To: herb...@gondor.apana.org.au; da...@davemloft.net
>> Cc: linux-crypto@vger.kernel.org; Xu Zaibo <xuza...@huawei.com>; Wangzhou
>> (B) <wangzh...@hisilicon.com>
>> Subject: [PATCH 12/13] crypto: hisilicon/zip - Use temporary sqe when doing
>> work
>
>> From: Zhou Wang <wangzh...@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.
Yes.
>
> 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?
Currently we do not support multi-threads using one crypto_acomp in this driver.
If you tested like this, it may go wrong, as we do not protect queue which is
in zip ctx.
Best,
Zhou
>
>> Signed-off-by: Zhou Wang <wangzh...@hisilicon.com>
>> Signed-off-by: Jonathan Cameron <jonathan.came...@huawei.com>
>> Signed-off-by: Shukun Tan <tanshuk...@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
>
> .
>
