Re: [PATCH v2 09/10] crypto: caam - add crypto_engine support for RSA algorithms

[Horia Geanta]

On 1/13/2020 11:48 AM, Iuliana Prodan wrote:
> On 1/10/2020 10:46 AM, Horia Geanta wrote:
>> On 1/3/2020 3:03 AM, Iuliana Prodan wrote:
>>> +static int akcipher_enqueue_req(struct device *jrdev, u32 *desc,
>>> +				void (*cbk)(struct device *jrdev, u32 *desc,
>>> +					    u32 err, void *context),
>>> +				struct akcipher_request *req,
>>> +				struct rsa_edesc *edesc)
>>> +{
>>> +	struct caam_drv_private_jr *jrpriv = dev_get_drvdata(jrdev);
>>> +	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
>>> +	struct caam_rsa_ctx *ctx = akcipher_tfm_ctx(tfm);
>>> +	struct caam_rsa_key *key = &ctx->key;
>>> +	int ret;
>>> +
>>> +	if (req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)
>>> +		return crypto_transfer_akcipher_request_to_engine(jrpriv->engine,
>>> +								  req);
>> Resource leak in case transfer fails.
>>
>>> +	else
>>> +		ret = caam_jr_enqueue(jrdev, desc, cbk, &edesc->jrentry);
>> What's the problem with transferring all requests to crypto engine?
>>
> I'll address all your comments in v3.
> 
> Regarding the transfer request to crypto-engine: if sending all requests 
> to crypto-engine, multibuffer tests, for non-backlogging requests fail 
> after only 10 requests, since crypto-engine queue has 10 entries.
> Here's an example:
> root@imx6qpdlsolox:~# insmod tcrypt.ko mode=422 num_mb=1024
> insmod: ERROR: could not insert module tcrypt.ko: Resource temporarily 
> unavailable
> root@imx6qpdlsolox:~#
> root@imx6qpdlsolox:~# dmesg
> ...
> testing speed of multibuffer sha1 (sha1-caam)
> tcrypt: test  0 (   16 byte blocks,   16 bytes per update,   1 updates):
> tcrypt: concurrent request 11 error -28
> tcrypt: concurrent request 13 error -28
> tcrypt: concurrent request 14 error -28
> tcrypt: concurrent request 16 error -28
> tcrypt: concurrent request 18 error -28
> tcrypt: concurrent request 20 error -28
> tcrypt: concurrent request 22 error -28
> tcrypt: concurrent request 24 error -28
> tcrypt: concurrent request 26 error -28
> tcrypt: concurrent request 28 error -28
> tcrypt: concurrent request 30 error -28
> tcrypt: concurrent request 32 error -28
> tcrypt: concurrent request 34 error -28
> 
> tcrypt: concurrent request 1020 error -28
> tcrypt: concurrent request 1022 error -28
> tcrypt: At least one hashing failed ret=-28
> root@imx6qpdlsolox:~#
> 
> If sending just the backlog request to crypto-engine, and non-blocking 
> directly to CAAM, these tests have a better chance to pass since JR has 
> 1024 entries.
> 
> Will need to work/update crypto-engine: set queue length when initialize 
> crypto-engine, and remove serialization of requests in crypto-engine. 
> But, until then, I would like to have a backlogging solution in CAAM driver.
> 
My point is you need to add details about the current limitations
in the commit message (even in the source code, it wouldn't hurt),
justifying the choice of not using crypto engine for all requests.

Horia