On 2020/5/9 11:42, Song Bao Hua wrote: >> -----Original Message----- >> From: linux-crypto-owner@xxxxxxxxxxxxxxx [mailto:linux-crypto-owner@xxxxxxxxxxxxxxx] On Behalf Of Shukun Tan >> Sent: Friday, May 8, 2020 6:58 PM >> To: herbert@xxxxxxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx >> Cc: linux-crypto@xxxxxxxxxxxxxxx; Xu Zaibo <xuzaibo@xxxxxxxxxx>; Wangzhou (B) <wangzhou1@xxxxxxxxxxxxx> >> Subject: [PATCH 12/13] crypto: hisilicon/zip - Use temporary sqe when doing work > >> From: Zhou Wang <wangzhou1@xxxxxxxxxxxxx> > >> 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 <wangzhou1@xxxxxxxxxxxxx> >> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> >> Signed-off-by: Shukun Tan <tanshukun1@xxxxxxxxxx> >> --- >> 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 > > . >
