Re: Backlog support for CAAM?

[Richard Weinberger <richard@nod.at>]

----- Ursprüngliche Mail -----
> Von: "horia geanta" <horia.geanta@nxp.com>
> An: "richard" <richard@nod.at>, "Linux Crypto Mailing List" <linux-crypto@vger.kernel.org>, "linux-kernel"
> <linux-kernel@vger.kernel.org>
> CC: "aymen sghaier" <aymen.sghaier@nxp.com>, "david" <david@sigma-star.at>, "Baolin Wang" <baolin.wang@linaro.org>
> Gesendet: Donnerstag, 25. Juli 2019 07:57:28
> Betreff: Re: Backlog support for CAAM?

> On 7/25/2019 12:22 AM, Richard Weinberger wrote:
>> Hi!
>> 
>> Recently I had the pleasure to debug a lockup on a imx6 based platform.
>> It turned out that the lockup was caused by the CAAM driver because it
>> just returns -EBUSY upon a full job ring.
>> 
>> Then I found commits:
>> 0618764cb25f ("dm crypt: fix deadlock when async crypto algorithm returns
>> -EBUSY")
>> c0403ec0bb5a ("Revert "dm crypt: fix deadlock when async crypto algorithm
>> returns -EBUSY"")
>> 
> Truly sorry for the inconvenience.

No need to worry. Nobody got hurt. :-)

> Indeed this is a caam driver issue, and not a dm-crypt one.
> 
>> Is there a reason why the driver has still no proper backlog support?
>> 
> We've been rejected a few times or the implementation had performance issues:
> v1: https://patchwork.kernel.org/patch/7144701
> v2: https://patchwork.kernel.org/patch/7199241
> v3: https://patchwork.kernel.org/patch/7221941
> v4: https://patchwork.kernel.org/patch/7230241
> v5: https://patchwork.kernel.org/patch/9033121
> 
> and we haven't been persistent enough.
> 
>> If it is just a matter of -ENOPATCH, I have some cycles left an can help.
>> But before working on this topic I'd like to figure what the current state
>> or plans are. :-)
>> 
> Right now we're evaluating two options:
> -reworking v5 above
> -using crypto engine (crypto/crypto_engine.c)

I'll look into that to get a better understanding.

> Ideally crypto engine should be the way to go.
> However we need to make sure performance degradation is negligible,
> which unfortunately is not case.
> 
> Currently it seems that crypto engine has an issue with sending
> multiple crypto requests from (SW) engine queue -> (HW) caam queue.
> 
> More exactly, crypto_pump_requests() performs this check:
>        /* Make sure we are not already running a request */
>        if (engine->cur_req)
>                goto out;
> 
> thus it's not possible to add more crypto requests to the caam queue
> until HW finishes the work on the current crypto request and
> calls crypto_finalize_request():
>        if (finalize_cur_req) {
>		[...]
>                engine->cur_req = NULL;

Let me also dig into this.
Thanks for all the pointers!

Thanks,
//richard