Hi Pascal,
On Fri, Jul 05, 2019 at 08:49:23AM +0200, Pascal van Leeuwen wrote:
> Signed-off-by: Pascal van Leeuwen <pvanleeuwen@xxxxxxxxxxxxxx>
Could you add a commit message?
> - /* H/W capabilities selection */
> - val = EIP197_FUNCTION_RSVD;
> - val |= EIP197_PROTOCOL_ENCRYPT_ONLY | EIP197_PROTOCOL_HASH_ONLY;
> - val |= EIP197_PROTOCOL_ENCRYPT_HASH | EIP197_PROTOCOL_HASH_DECRYPT;
> - val |= EIP197_ALG_DES_ECB | EIP197_ALG_DES_CBC;
> - val |= EIP197_ALG_3DES_ECB | EIP197_ALG_3DES_CBC;
> - val |= EIP197_ALG_AES_ECB | EIP197_ALG_AES_CBC;
> - val |= EIP197_ALG_MD5 | EIP197_ALG_HMAC_MD5;
> - val |= EIP197_ALG_SHA1 | EIP197_ALG_HMAC_SHA1;
> - val |= EIP197_ALG_SHA2 | EIP197_ALG_HMAC_SHA2;
> - writel(val, EIP197_PE(priv) + EIP197_PE_EIP96_FUNCTION_EN(pe));
> + /* H/W capabilities selection: just enable everything */
> + writel(EIP197_FUNCTION_ALL,
> + EIP197_PE(priv) + EIP197_PE_EIP96_FUNCTION_EN(pe));
This should be in a separate patch. I'm also not sure about it, as
controlling exactly what algs are enabled in the h/w could prevent
misconfiguration issues in the control descriptors.
> @@ -62,9 +63,9 @@ static void safexcel_skcipher_token(struct safexcel_cipher_ctx *ctx, u8 *iv,
> u32 length)
> - if (ctx->mode == CONTEXT_CONTROL_CRYPTO_MODE_CBC) {
> + if (ctx->mode != CONTEXT_CONTROL_CRYPTO_MODE_ECB) {
I think it's better for maintenance and readability to have something
like:
if (ctx->mode == CONTEXT_CONTROL_CRYPTO_MODE_CBC ||
ctx->mode == CONTEXT_CONTROL_CRYPTO_MODE_CTR_LOAD)
> +struct safexcel_alg_template safexcel_alg_ctr_aes = {
> + .type = SAFEXCEL_ALG_TYPE_SKCIPHER,
Same comment as in patch 1 about the .engines member of the struct.
> + .alg.skcipher = {
> + .setkey = safexcel_skcipher_aesctr_setkey,
> + .encrypt = safexcel_ctr_aes_encrypt,
> + .decrypt = safexcel_ctr_aes_decrypt,
> + /* Add 4 to include the 4 byte nonce! */
> + .min_keysize = AES_MIN_KEY_SIZE + 4,
> + .max_keysize = AES_MAX_KEY_SIZE + 4,
You could use CTR_RFC3686_NONCE_SIZE here (maybe in other places in the
patch as well).
> + .ivsize = 8,
And CTR_RFC3686_IV_SIZE here.
Thanks!
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
