Re: Backlog support for CAAM?

From: Richard Weinberger
Date: Thu Jul 25 2019 - 14:04:13 EST


----- UrsprÃngliche Mail -----
> Von: "horia geanta" <horia.geanta@xxxxxxx>
> An: "richard" <richard@xxxxxx>, "Linux Crypto Mailing List" <linux-crypto@xxxxxxxxxxxxxxx>, "linux-kernel"
> <linux-kernel@xxxxxxxxxxxxxxx>
> CC: "aymen sghaier" <aymen.sghaier@xxxxxxx>, "david" <david@xxxxxxxxxxxxx>, "Baolin Wang" <baolin.wang@xxxxxxxxxx>
> 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