Re: [PATCH 08/12] crypto: caam - support crypto_engine framework for SKCIPHER algorithms

[Iuliana Prodan]

Hi,

On 12/10/2019 5:27 PM, Bastian Krause wrote:
> 
> Hi Iulia,
> 
> On 11/17/19 11:30 PM, Iuliana Prodan wrote:
>> Integrate crypto_engine into CAAM, to make use of the engine queue.
>> Add support for SKCIPHER algorithms.
>>
>> This is intended to be used for CAAM backlogging support.
>> The requests, with backlog flag (e.g. from dm-crypt) will be listed
>> into crypto-engine queue and processed by CAAM when free.
>> This changes the return codes for caam_jr_enqueue:
>> -EINPROGRESS if OK, -EBUSY if request is backlogged,
>> -ENOSPC if the queue is full, -EIO if it cannot map the caller's
>> descriptor, -EINVAL if crypto_tfm not supported by crypto_engine.
>>
>> Signed-off-by: Iuliana Prodan <iuliana.prodan@xxxxxxx>
>> Signed-off-by: Franck LENORMAND <franck.lenormand@xxxxxxx>
>> Reviewed-by: Horia Geantă <horia.geanta@xxxxxxx>
>> ---
>>   drivers/crypto/caam/Kconfig   |  1 +
>>   drivers/crypto/caam/caamalg.c | 84 +++++++++++++++++++++++++++++++++++--------
>>   drivers/crypto/caam/intern.h  |  2 ++
>>   drivers/crypto/caam/jr.c      | 51 ++++++++++++++++++++++++--
>>   4 files changed, 122 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig
>> index 87053e4..1930e19 100644
>> --- a/drivers/crypto/caam/Kconfig
>> +++ b/drivers/crypto/caam/Kconfig
>> @@ -33,6 +33,7 @@ config CRYPTO_DEV_FSL_CAAM_DEBUG
>>   
...
>>   
>> +static int skcipher_do_one_req(struct crypto_engine *engine, void *areq)
>> +{
>> +	struct skcipher_request *req = skcipher_request_cast(areq);
>> +	struct caam_ctx *ctx = crypto_skcipher_ctx(crypto_skcipher_reqtfm(req));
>> +	struct caam_skcipher_req_ctx *rctx = skcipher_request_ctx(req);
>> +	struct caam_jr_request_entry *jrentry;
>> +	u32 *desc = rctx->edesc->hw_desc;
>> +	int ret;
>> +
>> +	jrentry = &rctx->edesc->jrentry;
>> +	jrentry->bklog = true;
>> +
>> +	ret = caam_jr_enqueue_no_bklog(ctx->jrdev, desc,
>> +				       rctx->skcipher_op_done, jrentry);
>> +
>> +	if (ret != -EINPROGRESS) {
>> +		skcipher_unmap(ctx->jrdev, rctx->edesc, req);
>> +		kfree(rctx->edesc);
>> +	} else {
>> +		ret = 0;
>> +	}
>> +
>> +	return ret;
> 
> While testing this on a i.MX6 DualLite I see -ENOSPC being returned here
> after a couple of GiB of data being encrypted (via dm-crypt with LUKS
> extension). This results in these messages from crypto_engine:
> 
>    caam_jr 2101000.jr0: Failed to do one request from queue: -28
> 
> And later..
> 
>    Buffer I/O error on device dm-0, logical block 59392
>    JBD2: Detected IO errors while flushing file data on dm-0-8
> 
> Reproducible with something like this:
> 
>    echo "testkey" | cryptsetup luksFormat \
>      --cipher=aes-cbc-essiv:sha256 \
>      --key-file=- \
>      --key-size=256 \
>      /dev/mmcblk1p8
>    echo "testkey" | cryptsetup open \
>      --type luks \
>      --key-file=- \
>      /dev/mmcblk1p8 data
> 
>    mkfs.ext4 /dev/mapper/data
>    mount /dev/mapper/data /mnt
> 
>    set -x
>    while [ true ]; do
>      dd if=/dev/zero of=/mnt/big_file bs=1M count=1024
>      sync
>    done
> 
> Any ideas?
> 

Thanks for testing this!
I reproduced this issue on imx6dl, _but_ only with the bypass sw queue 
patch. It only reproduces on some targets, e.g. on imx7d I don't get the 
-ENOSPC error. So, I believe there is a timing issue between 
crypto-engine and CAAM driver, both sending requests to CAAM hw.
I'm debugging this and I'll let you know my findings.

Best regards,
Iulia