Re: [RFC PATCH V3] Crypto: rockchip/crypto - add hash support for crypto engine in rk3288

[Zain <zain.wang@rock-chips.com>]

Hi,

在 2015年12月11日 16:54, LABBE Corentin 写道:
> Hello
>
> I have some minor comments below.
>
> On Fri, Dec 11, 2015 at 09:58:23AM +0800, Zain Wang wrote:
>> Add md5 sha1 sha256 support for crypto engine in rk3288.
>> This patch can't support multiple updatings because of limited of IC,
>> as result, it can't support import and export too.
>>
>> Signed-off-by: Zain Wang <zain.wang@rock-chips.com>
>> ---
>>
>> Changes in V3:
>> - add switch instead of multiple if.
>>
>> Changes in V2:
>> - add some comments to code.
>> - fix some issues about zero message process.
>>
>>  drivers/crypto/rockchip/Makefile                   |   1 +
>>  drivers/crypto/rockchip/rk3288_crypto.c            |  33 +-
>>  drivers/crypto/rockchip/rk3288_crypto.h            |  50 ++-
>>  drivers/crypto/rockchip/rk3288_crypto_ablkcipher.c |  20 +-
>>  drivers/crypto/rockchip/rk3288_crypto_ahash.c      | 394 +++++++++++++++++++++
>>  5 files changed, 480 insertions(+), 18 deletions(-)
>>  create mode 100644 drivers/crypto/rockchip/rk3288_crypto_ahash.c
>>
>> diff --git a/drivers/crypto/rockchip/rk3288_crypto.h b/drivers/crypto/rockchip/rk3288_crypto.h
>> index 604ffe7..01d8299 100644
>> --- a/drivers/crypto/rockchip/rk3288_crypto.h
>> +++ b/drivers/crypto/rockchip/rk3288_crypto.h
>> @@ -6,6 +6,10 @@
>>  #include <crypto/algapi.h>
>>  #include <linux/interrupt.h>
>>  #include <linux/delay.h>
>> +#include <crypto/internal/hash.h>
>> +
>> +#include "crypto/md5.h"
>> +#include "crypto/sha.h"
> It is proper to include with <> not ""
Ok, done!
>> diff --git a/drivers/crypto/rockchip/rk3288_crypto_ahash.c b/drivers/crypto/rockchip/rk3288_crypto_ahash.c
>> new file mode 100644
>> index 0000000..a5795f6
>> --- /dev/null
>> +++ b/drivers/crypto/rockchip/rk3288_crypto_ahash.c
>> @@ -0,0 +1,394 @@
>> +/*
>> + * Crypto acceleration support for Rockchip RK3288
>> + *
>> + * Copyright (c) 2015, Fuzhou Rockchip Electronics Co., Ltd
>> + *
>> + * Author: Zain Wang <zain.wang@rock-chips.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * Some ideas are from marvell/cesa.c and s5p-sss.c driver.
>> + */
>> +#include "rk3288_crypto.h"
>> +
>> +/*
>> + * IC can not process zero message hash,
>> + * so we put the fixed hash out when met zero message.
>> + */
>> +const static u8 *sha1_zero_message_hash = {
>> +	"\xda\x39\xa3\xee\x5e\x6b\x4b\x0d\x32\x55"
>> +	"\xbf\xef\x95\x60\x18\x90\xaf\xd8\x07\x09"
>> +};
>> +
>> +const static u8 *sha256_zero_message_hash = {
>> +	"\xe3\xb0\xc4\x42\x98\xfc\x1c\x14"
>> +	"\x9a\xfb\xf4\xc8\x99\x6f\xb9\x24"
>> +	"\x27\xae\x41\xe4\x64\x9b\x93\x4c"
>> +	"\xa4\x95\x99\x1b\x78\x52\xb8\x55"
>> +};
>> +
>> +const static u8 *md5_zero_message_hash = {
>> +	"\xd4\x1d\x8c\xd9\x8f\x00\xb2\x04"
>> +	"\xe9\x80\x09\x98\xec\xf8\x42\x7e"
>> +};
>> +
>> +
>> +static int zero_message_process(struct ahash_request *req)
>> +{
>> +	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
>> +
>> +	switch (crypto_ahash_digestsize(tfm)) {
>> +	case SHA1_DIGEST_SIZE:
>> +		memcpy(req->result, sha1_zero_message_hash, SHA1_DIGEST_SIZE);
>> +		break;
>> +	case SHA256_DIGEST_SIZE:
>> +		memcpy(req->result, sha256_zero_message_hash,
>> +		       SHA256_DIGEST_SIZE);
>> +		break;
>> +	case MD5_DIGEST_SIZE:
>> +		memcpy(req->result, md5_zero_message_hash, MD5_DIGEST_SIZE);
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +		break;
>> +	}
> You could store crypto_ahash_digestsize result and use it in memcpy.
> You could also store the name of xxx_zero_message_hash in crypto_alg like n2 do.
> Perhaps a combination of the two could produce more generic code.
You are right, it will fix it as your first way.
>> +struct rk_crypto_tmp rk_ahash_sha256 = {
>> +	.type = ALG_TYPE_HASH,
>> +	.alg.hash = {
>> +	.init = rk_ahash_init,
>> +	.update = rk_ahash_update,
>> +	.final = rk_ahash_final,
>> +	.finup = rk_ahash_finup,
>> +	.digest = rk_ahash_digest,
>> +		.halg = {
>> +			 .digestsize = SHA256_DIGEST_SIZE,
>> +			 .statesize = sizeof(struct sha256_state),
>> +			 .base = {
>> +				  .cra_name = "sha256",
>> +				  .cra_driver_name = "rk-sha256",
>> +				  .cra_priority = 300,
>> +				  .cra_flags = CRYPTO_ALG_ASYNC |
>> +					       CRYPTO_ALG_NEED_FALLBACK,
>> +				  .cra_blocksize = SHA256_BLOCK_SIZE,
>> +				  .cra_ctxsize = sizeof(struct rk_ahash_ctx),
>> +				  .cra_alignmask = 0,
>> +				  .cra_init = rk_cra_hash_init,
>> +				  .cra_exit = rk_cra_hash_exit,
>> +				  .cra_module = THIS_MODULE,
>> +				  }
>> +			 }
>> +	}
>> +};
>> +
>> +struct rk_crypto_tmp rk_ahash_md5 = {
>> +	.type = ALG_TYPE_HASH,
>> +	.alg.hash = {
>> +		.init = rk_ahash_init,
>> +		.update = rk_ahash_update,
>> +		.final = rk_ahash_final,
>> +		.finup = rk_ahash_finup,
>> +		.digest = rk_ahash_digest,
>> +		.halg = {
>> +			 .digestsize = MD5_DIGEST_SIZE,
>> +			 .statesize = sizeof(struct md5_state),
>> +			 .base = {
>> +				  .cra_name = "md5",
>> +				  .cra_driver_name = "rk-md5",
>> +				  .cra_priority = 300,
>> +				  .cra_flags = CRYPTO_ALG_ASYNC |
>> +					       CRYPTO_ALG_NEED_FALLBACK,
> You said that the driver need a fallback, but I do not see any use of it.
I will remove it.
The idea of this part is from mv_ceas.c, and I forgot to remove it that
I did not use a fallback actually.:'(
>
> Regards
>
>
>
>
Thanks,
Zain