Re: [PATCH v6 4/4] crypto: Add Allwinner Security System crypto accelerator
Le 26/03/2015 19:31, Boris Brezillon a écrit :
Hi Corentin,
Here is a quick review, there surely are a lot of other things I didn't
spot.
On Mon, 16 Mar 2015 20:01:22 +0100
LABBE Corentin clabbe.montj...@gmail.com wrote:
Add support for the Security System included in Allwinner SoC A20.
The Security System is a hardware cryptographic accelerator that support:
- MD5 and SHA1 hash algorithms
- AES block cipher in CBC mode with 128/196/256bits keys.
- DES and 3DES block cipher in CBC mode
Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com
---
[...]
+static int sunxi_ss_cipher(struct ablkcipher_request *areq, u32 mode)
+{
+struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(areq);
+struct sunxi_tfm_ctx *op = crypto_ablkcipher_ctx(tfm);
+const char *cipher_type;
+struct sunxi_ss_ctx *ss = op-ss;
+
+if (areq-nbytes == 0)
+return 0;
+
+if (areq-info == NULL) {
+dev_err(ss-dev, ERROR: Empty IV\n);
+return -EINVAL;
+}
+
+if (areq-src == NULL || areq-dst == NULL) {
+dev_err(ss-dev, ERROR: Some SGs are NULL\n);
+return -EINVAL;
+}
+
+cipher_type = crypto_tfm_alg_name(crypto_ablkcipher_tfm(tfm));
+
+if (strcmp(cbc(aes), cipher_type) == 0) {
+mode |= SS_OP_AES | SS_CBC | SS_ENABLED | op-keymode;
+return sunxi_ss_aes_poll(areq, mode);
+}
+
+if (strcmp(cbc(des), cipher_type) == 0) {
+mode |= SS_OP_DES | SS_CBC | SS_ENABLED | op-keymode;
+return sunxi_ss_des_poll(areq, mode);
+}
+
+if (strcmp(cbc(des3_ede), cipher_type) == 0) {
+mode |= SS_OP_3DES | SS_CBC | SS_ENABLED | op-keymode;
+return sunxi_ss_des_poll(areq, mode);
+}
Hm, I'm not sure doing these string comparisons in the crypto operation
path is a good idea.
Moreover, you're doing 3 string comparisons, even though only one can
be valid at a time (using 'else if' would have been a bit better).
Anyway, IMHO this function should be split into 3 functions, and
referenced by your alg template definitions.
Something like:
int sunxi_ss_xxx_encrypt(struct ablkcipher_request *areq)
{
/* put your cipher specific intialization here */
return sunxi_ss_xxx_poll(areq, SS_ENCRYPTION);
}
int sunxi_ss_xxx_decrypt(struct ablkcipher_request *areq)
{
/* put your cipher specific intialization here */
return sunxi_ss_xxx_poll(areq, SS_DECRYPTION);
}
You are right, I have added more block mode, and that was added too much strcmp.
So splitting in simplier functions is good.
+
+int sunxi_ss_cipher_init(struct crypto_tfm *tfm)
+{
+const char *name = crypto_tfm_alg_name(tfm);
+struct sunxi_tfm_ctx *op = crypto_tfm_ctx(tfm);
+struct crypto_alg *alg = tfm-__crt_alg;
+struct sunxi_ss_alg_template *algt;
+struct sunxi_ss_ctx *ss;
+
+memset(op, 0, sizeof(struct sunxi_tfm_ctx));
+
+algt = container_of(alg, struct sunxi_ss_alg_template, alg.crypto);
+ss = algt-ss;
+op-ss = algt-ss;
+
+/* fallback is needed only for DES/3DES */
+if (strcmp(cbc(des), name) == 0 ||
+strcmp(cbc(des3_ede), name) == 0) {
+op-fallback = crypto_alloc_ablkcipher(name, 0,
+CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
+if (IS_ERR(op-fallback)) {
+dev_err(ss-dev, ERROR: allocating fallback algo %s\n,
+name);
+return PTR_ERR(op-fallback);
+}
+}
Ditto: just create a specific init function for the des case:
int sunxi_ss_cbc_des_init(struct crypto_tfm *tfm)
{
sunxi_ss_cipher_init(tfm);
op-fallback = crypto_alloc_ablkcipher(name, 0,
CRYPTO_ALG_ASYNC |
CRYPTO_ALG_NEED_FALLBACK);
if (IS_ERR(op-fallback)) {
dev_err(ss-dev, ERROR: allocating fallback algo %s\n,
name);
return PTR_ERR(op-fallback);
}
return 0;
}
Ok
[..]
+/*
+ * Optimized function for the case where we have only one SG,
+ * so we can use kmap_atomic
+ */
+static int sunxi_ss_aes_poll_atomic(struct ablkcipher_request *areq)
+{
+u32 spaces;
+struct scatterlist *in_sg = areq-src;
+struct scatterlist *out_sg = areq-dst;
+void *src_addr;
+void *dst_addr;
+unsigned int ileft = areq-nbytes;
+unsigned int oleft = areq-nbytes;
+unsigned int todo;
+u32 *src32;
+u32 *dst32;
+u32 rx_cnt = 32;
+u32 tx_cnt = 0;
+int i;
+struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(areq);
+struct sunxi_tfm_ctx *op = crypto_ablkcipher_ctx(tfm);
+struct sunxi_ss_ctx *ss = op-ss;
+
+src_addr = kmap_atomic(sg_page(in_sg)) + in_sg-offset;
+if (src_addr == NULL) {
+
Re: [PATCH v6 4/4] crypto: Add Allwinner Security System crypto accelerator
Hi Corentin,
Here is a quick review, there surely are a lot of other things I didn't
spot.
On Mon, 16 Mar 2015 20:01:22 +0100
LABBE Corentin clabbe.montj...@gmail.com wrote:
Add support for the Security System included in Allwinner SoC A20.
The Security System is a hardware cryptographic accelerator that support:
- MD5 and SHA1 hash algorithms
- AES block cipher in CBC mode with 128/196/256bits keys.
- DES and 3DES block cipher in CBC mode
Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com
---
[...]
+static int sunxi_ss_cipher(struct ablkcipher_request *areq, u32 mode)
+{
+ struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(areq);
+ struct sunxi_tfm_ctx *op = crypto_ablkcipher_ctx(tfm);
+ const char *cipher_type;
+ struct sunxi_ss_ctx *ss = op-ss;
+
+ if (areq-nbytes == 0)
+ return 0;
+
+ if (areq-info == NULL) {
+ dev_err(ss-dev, ERROR: Empty IV\n);
+ return -EINVAL;
+ }
+
+ if (areq-src == NULL || areq-dst == NULL) {
+ dev_err(ss-dev, ERROR: Some SGs are NULL\n);
+ return -EINVAL;
+ }
+
+ cipher_type = crypto_tfm_alg_name(crypto_ablkcipher_tfm(tfm));
+
+ if (strcmp(cbc(aes), cipher_type) == 0) {
+ mode |= SS_OP_AES | SS_CBC | SS_ENABLED | op-keymode;
+ return sunxi_ss_aes_poll(areq, mode);
+ }
+
+ if (strcmp(cbc(des), cipher_type) == 0) {
+ mode |= SS_OP_DES | SS_CBC | SS_ENABLED | op-keymode;
+ return sunxi_ss_des_poll(areq, mode);
+ }
+
+ if (strcmp(cbc(des3_ede), cipher_type) == 0) {
+ mode |= SS_OP_3DES | SS_CBC | SS_ENABLED | op-keymode;
+ return sunxi_ss_des_poll(areq, mode);
+ }
Hm, I'm not sure doing these string comparisons in the crypto operation
path is a good idea.
Moreover, you're doing 3 string comparisons, even though only one can
be valid at a time (using 'else if' would have been a bit better).
Anyway, IMHO this function should be split into 3 functions, and
referenced by your alg template definitions.
Something like:
int sunxi_ss_xxx_encrypt(struct ablkcipher_request *areq)
{
/* put your cipher specific intialization here */
return sunxi_ss_xxx_poll(areq, SS_ENCRYPTION);
}
int sunxi_ss_xxx_decrypt(struct ablkcipher_request *areq)
{
/* put your cipher specific intialization here */
return sunxi_ss_xxx_poll(areq, SS_DECRYPTION);
}
+
+int sunxi_ss_cipher_init(struct crypto_tfm *tfm)
+{
+ const char *name = crypto_tfm_alg_name(tfm);
+ struct sunxi_tfm_ctx *op = crypto_tfm_ctx(tfm);
+ struct crypto_alg *alg = tfm-__crt_alg;
+ struct sunxi_ss_alg_template *algt;
+ struct sunxi_ss_ctx *ss;
+
+ memset(op, 0, sizeof(struct sunxi_tfm_ctx));
+
+ algt = container_of(alg, struct sunxi_ss_alg_template, alg.crypto);
+ ss = algt-ss;
+ op-ss = algt-ss;
+
+ /* fallback is needed only for DES/3DES */
+ if (strcmp(cbc(des), name) == 0 ||
+ strcmp(cbc(des3_ede), name) == 0) {
+ op-fallback = crypto_alloc_ablkcipher(name, 0,
+ CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
+ if (IS_ERR(op-fallback)) {
+ dev_err(ss-dev, ERROR: allocating fallback algo %s\n,
+ name);
+ return PTR_ERR(op-fallback);
+ }
+ }
Ditto: just create a specific init function for the des case:
int sunxi_ss_cbc_des_init(struct crypto_tfm *tfm)
{
sunxi_ss_cipher_init(tfm);
op-fallback = crypto_alloc_ablkcipher(name, 0,
CRYPTO_ALG_ASYNC |
CRYPTO_ALG_NEED_FALLBACK);
if (IS_ERR(op-fallback)) {
dev_err(ss-dev, ERROR: allocating fallback algo %s\n,
name);
return PTR_ERR(op-fallback);
}
return 0;
}
[..]
+/*
+ * Optimized function for the case where we have only one SG,
+ * so we can use kmap_atomic
+ */
+static int sunxi_ss_aes_poll_atomic(struct ablkcipher_request *areq)
+{
+ u32 spaces;
+ struct scatterlist *in_sg = areq-src;
+ struct scatterlist *out_sg = areq-dst;
+ void *src_addr;
+ void *dst_addr;
+ unsigned int ileft = areq-nbytes;
+ unsigned int oleft = areq-nbytes;
+ unsigned int todo;
+ u32 *src32;
+ u32 *dst32;
+ u32 rx_cnt = 32;
+ u32 tx_cnt = 0;
+ int i;
+ struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(areq);
+ struct sunxi_tfm_ctx *op = crypto_ablkcipher_ctx(tfm);
+ struct sunxi_ss_ctx *ss = op-ss;
+
+ src_addr = kmap_atomic(sg_page(in_sg)) + in_sg-offset;
+ if (src_addr == NULL) {
+ dev_err(ss-dev, kmap_atomic error for src SG\n);
+ return -EINVAL;
+ }
+
+ dst_addr = kmap_atomic(sg_page(out_sg)) +
[PATCH v6 4/4] crypto: Add Allwinner Security System crypto accelerator
Add support for the Security System included in Allwinner SoC A20.
The Security System is a hardware cryptographic accelerator that support:
- MD5 and SHA1 hash algorithms
- AES block cipher in CBC mode with 128/196/256bits keys.
- DES and 3DES block cipher in CBC mode
Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com
---
drivers/crypto/Kconfig| 17 ++
drivers/crypto/Makefile | 1 +
drivers/crypto/sunxi-ss/Makefile | 2 +
drivers/crypto/sunxi-ss/sunxi-ss-cipher.c | 408 +
drivers/crypto/sunxi-ss/sunxi-ss-core.c | 339 +
drivers/crypto/sunxi-ss/sunxi-ss-hash.c | 475 ++
drivers/crypto/sunxi-ss/sunxi-ss.h| 200 +
7 files changed, 1442 insertions(+)
create mode 100644 drivers/crypto/sunxi-ss/Makefile
create mode 100644 drivers/crypto/sunxi-ss/sunxi-ss-cipher.c
create mode 100644 drivers/crypto/sunxi-ss/sunxi-ss-core.c
create mode 100644 drivers/crypto/sunxi-ss/sunxi-ss-hash.c
create mode 100644 drivers/crypto/sunxi-ss/sunxi-ss.h
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 2fb0fdf..9ba9759 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -436,4 +436,21 @@ config CRYPTO_DEV_QCE
hardware. To compile this driver as a module, choose M here. The
module will be called qcrypto.
+config CRYPTO_DEV_SUNXI_SS
+ tristate Support for Allwinner Security System cryptographic
accelerator
+ depends on ARCH_SUNXI
+ select CRYPTO_MD5
+ select CRYPTO_SHA1
+ select CRYPTO_AES
+ select CRYPTO_DES
+ select CRYPTO_BLKCIPHER
+ help
+ Some Allwinner SoC have a crypto accelerator named
+ Security System. Select this if you want to use it.
+ The Security System handle AES/DES/3DES ciphers in CBC mode
+ and SHA1 and MD5 hash algorithms.
+
+ To compile this driver as a module, choose M here: the module
+ will be called sunxi-ss.
+
endif # CRYPTO_HW
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index 3924f93..856545c 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -25,3 +25,4 @@ obj-$(CONFIG_CRYPTO_DEV_TALITOS) += talitos.o
obj-$(CONFIG_CRYPTO_DEV_UX500) += ux500/
obj-$(CONFIG_CRYPTO_DEV_QAT) += qat/
obj-$(CONFIG_CRYPTO_DEV_QCE) += qce/
+obj-$(CONFIG_CRYPTO_DEV_SUNXI_SS) += sunxi-ss/
diff --git a/drivers/crypto/sunxi-ss/Makefile b/drivers/crypto/sunxi-ss/Makefile
new file mode 100644
index 000..8bb287d
--- /dev/null
+++ b/drivers/crypto/sunxi-ss/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_CRYPTO_DEV_SUNXI_SS) += sunxi-ss.o
+sunxi-ss-y += sunxi-ss-core.o sunxi-ss-hash.o sunxi-ss-cipher.o
diff --git a/drivers/crypto/sunxi-ss/sunxi-ss-cipher.c
b/drivers/crypto/sunxi-ss/sunxi-ss-cipher.c
new file mode 100644
index 000..3ed0ad0
--- /dev/null
+++ b/drivers/crypto/sunxi-ss/sunxi-ss-cipher.c
@@ -0,0 +1,408 @@
+/*
+ * sunxi-ss-cipher.c - hardware cryptographic accelerator for Allwinner A20 SoC
+ *
+ * Copyright (C) 2013-2015 Corentin LABBE clabbe.montj...@gmail.com
+ *
+ * This file add support for AES cipher with 128,192,256 bits
+ * keysize in CBC mode.
+ * Add support also for DES and 3DES in CBC mode.
+ *
+ * You could find the datasheet in Documentation/arm/sunxi/README
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include sunxi-ss.h
+
+static int sunxi_ss_cipher(struct ablkcipher_request *areq, u32 mode)
+{
+ struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(areq);
+ struct sunxi_tfm_ctx *op = crypto_ablkcipher_ctx(tfm);
+ const char *cipher_type;
+ struct sunxi_ss_ctx *ss = op-ss;
+
+ if (areq-nbytes == 0)
+ return 0;
+
+ if (areq-info == NULL) {
+ dev_err(ss-dev, ERROR: Empty IV\n);
+ return -EINVAL;
+ }
+
+ if (areq-src == NULL || areq-dst == NULL) {
+ dev_err(ss-dev, ERROR: Some SGs are NULL\n);
+ return -EINVAL;
+ }
+
+ cipher_type = crypto_tfm_alg_name(crypto_ablkcipher_tfm(tfm));
+
+ if (strcmp(cbc(aes), cipher_type) == 0) {
+ mode |= SS_OP_AES | SS_CBC | SS_ENABLED | op-keymode;
+ return sunxi_ss_aes_poll(areq, mode);
+ }
+
+ if (strcmp(cbc(des), cipher_type) == 0) {
+ mode |= SS_OP_DES | SS_CBC | SS_ENABLED | op-keymode;
+ return sunxi_ss_des_poll(areq, mode);
+ }
+
+ if (strcmp(cbc(des3_ede), cipher_type) == 0) {
+ mode |= SS_OP_3DES | SS_CBC | SS_ENABLED | op-keymode;
+ return sunxi_ss_des_poll(areq, mode);
+ }
+
+ dev_err(ss-dev, ERROR: Cipher %s not handled\n, cipher_type);
+ return
