> > 
> >> From: Zhou Wang <[email protected]>
> > 
> >> 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.

Zhou, Thanks for clarification. I'm ok with this patch as it is definitely 
going to the right direction and removing some race conditions. 
Maybe we need a follow-up patch to address the parallel problem later.

> Best,
> Zhou

Barry

> > 
> >> Signed-off-by: Zhou Wang <[email protected]>
> >> Signed-off-by: Jonathan Cameron <[email protected]>
> >> Signed-off-by: Shukun Tan <[email protected]>
> >> ---
> >>  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;
> >>  

Reply via email to