Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote: + } +#endif + +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5 + err = crypto_register_shash(sunxi_md5_alg); Do not use shash for such device. This is clearly and ahash (and async in general) device. The rule of a thumb here is that you use sync algos only for devices which have dedicated instructions for computing the transformation. For devices which are attached to some kind of bus, you use async algos (ahash etc). I'm sorry that I didn't catch this earlier but there is no such rule. Unless you need the async interface you should stick to the sync interfaces for the sake of simplicity. We have a number of existing drivers that are synchronous but using the async interface. They should either be converted over to the sync interface or made interrupt-driven if possible. Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote: On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote: + } +#endif + +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5 + err = crypto_register_shash(sunxi_md5_alg); Do not use shash for such device. This is clearly and ahash (and async in general) device. The rule of a thumb here is that you use sync algos only for devices which have dedicated instructions for computing the transformation. For devices which are attached to some kind of bus, you use async algos (ahash etc). I'm sorry that I didn't catch this earlier but there is no such rule. Unless you need the async interface you should stick to the sync interfaces for the sake of simplicity. We have a number of existing drivers that are synchronous but using the async interface. They should either be converted over to the sync interface or made interrupt-driven if possible. Sure, but this device is interrupt driven and uses DMA to feed the crypto engine, therefore async, right ? Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
On Wed, Jul 23, 2014 at 04:07:20PM +0200, Marek Vasut wrote: On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote: On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote: + } +#endif + +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5 + err = crypto_register_shash(sunxi_md5_alg); Do not use shash for such device. This is clearly and ahash (and async in general) device. The rule of a thumb here is that you use sync algos only for devices which have dedicated instructions for computing the transformation. For devices which are attached to some kind of bus, you use async algos (ahash etc). I'm sorry that I didn't catch this earlier but there is no such rule. Unless you need the async interface you should stick to the sync interfaces for the sake of simplicity. We have a number of existing drivers that are synchronous but using the async interface. They should either be converted over to the sync interface or made interrupt-driven if possible. Sure, but this device is interrupt driven and uses DMA to feed the crypto engine, therefore async, right ? If it's interrupt-driven, then yes it would certainly make sense to be async. But all I see is polling in the latest posting, was the first version different? Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
On Wednesday, July 23, 2014 at 04:13:09 PM, Herbert Xu wrote: On Wed, Jul 23, 2014 at 04:07:20PM +0200, Marek Vasut wrote: On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote: On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote: + } +#endif + +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5 + err = crypto_register_shash(sunxi_md5_alg); Do not use shash for such device. This is clearly and ahash (and async in general) device. The rule of a thumb here is that you use sync algos only for devices which have dedicated instructions for computing the transformation. For devices which are attached to some kind of bus, you use async algos (ahash etc). I'm sorry that I didn't catch this earlier but there is no such rule. Unless you need the async interface you should stick to the sync interfaces for the sake of simplicity. We have a number of existing drivers that are synchronous but using the async interface. They should either be converted over to the sync interface or made interrupt-driven if possible. Sure, but this device is interrupt driven and uses DMA to feed the crypto engine, therefore async, right ? If it's interrupt-driven, then yes it would certainly make sense to be async. But all I see is polling in the latest posting, was the first version different? I stand corrected then, sorry. Is it possible to use DMA to feed the crypto accelerator, Corentin? Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
Le 23/07/2014 17:51, Marek Vasut a écrit : On Wednesday, July 23, 2014 at 04:13:09 PM, Herbert Xu wrote: On Wed, Jul 23, 2014 at 04:07:20PM +0200, Marek Vasut wrote: On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote: On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote: +} +#endif + +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5 +err = crypto_register_shash(sunxi_md5_alg); Do not use shash for such device. This is clearly and ahash (and async in general) device. The rule of a thumb here is that you use sync algos only for devices which have dedicated instructions for computing the transformation. For devices which are attached to some kind of bus, you use async algos (ahash etc). I'm sorry that I didn't catch this earlier but there is no such rule. Unless you need the async interface you should stick to the sync interfaces for the sake of simplicity. We have a number of existing drivers that are synchronous but using the async interface. They should either be converted over to the sync interface or made interrupt-driven if possible. Sure, but this device is interrupt driven and uses DMA to feed the crypto engine, therefore async, right ? If it's interrupt-driven, then yes it would certainly make sense to be async. But all I see is polling in the latest posting, was the first version different? I stand corrected then, sorry. Is it possible to use DMA to feed the crypto accelerator, Corentin? Best regards, Marek Vasut Yes, DMA is possible and will be implemented soon. So if I have well understood, I keep using async interface. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
On Wednesday, July 23, 2014 at 08:52:12 PM, Corentin LABBE wrote: Le 23/07/2014 17:51, Marek Vasut a écrit : On Wednesday, July 23, 2014 at 04:13:09 PM, Herbert Xu wrote: On Wed, Jul 23, 2014 at 04:07:20PM +0200, Marek Vasut wrote: On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote: On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote: + } +#endif + +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5 + err = crypto_register_shash(sunxi_md5_alg); Do not use shash for such device. This is clearly and ahash (and async in general) device. The rule of a thumb here is that you use sync algos only for devices which have dedicated instructions for computing the transformation. For devices which are attached to some kind of bus, you use async algos (ahash etc). I'm sorry that I didn't catch this earlier but there is no such rule. Unless you need the async interface you should stick to the sync interfaces for the sake of simplicity. We have a number of existing drivers that are synchronous but using the async interface. They should either be converted over to the sync interface or made interrupt-driven if possible. Sure, but this device is interrupt driven and uses DMA to feed the crypto engine, therefore async, right ? If it's interrupt-driven, then yes it would certainly make sense to be async. But all I see is polling in the latest posting, was the first version different? I stand corrected then, sorry. Is it possible to use DMA to feed the crypto accelerator, Corentin? Best regards, Marek Vasut Yes, DMA is possible and will be implemented soon. So if I have well understood, I keep using async interface. Yeah, in this case, using DMA and async interface combo is the way to go. Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
On Wed, Jul 23, 2014 at 09:38:35PM +0200, Marek Vasut wrote: On Wednesday, July 23, 2014 at 08:52:12 PM, Corentin LABBE wrote: Le 23/07/2014 17:51, Marek Vasut a écrit : On Wednesday, July 23, 2014 at 04:13:09 PM, Herbert Xu wrote: On Wed, Jul 23, 2014 at 04:07:20PM +0200, Marek Vasut wrote: On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote: On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote: +} +#endif + +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5 +err = crypto_register_shash(sunxi_md5_alg); Do not use shash for such device. This is clearly and ahash (and async in general) device. The rule of a thumb here is that you use sync algos only for devices which have dedicated instructions for computing the transformation. For devices which are attached to some kind of bus, you use async algos (ahash etc). I'm sorry that I didn't catch this earlier but there is no such rule. Unless you need the async interface you should stick to the sync interfaces for the sake of simplicity. We have a number of existing drivers that are synchronous but using the async interface. They should either be converted over to the sync interface or made interrupt-driven if possible. Sure, but this device is interrupt driven and uses DMA to feed the crypto engine, therefore async, right ? If it's interrupt-driven, then yes it would certainly make sense to be async. But all I see is polling in the latest posting, was the first version different? I stand corrected then, sorry. Is it possible to use DMA to feed the crypto accelerator, Corentin? Best regards, Marek Vasut Yes, DMA is possible and will be implemented soon. So if I have well understood, I keep using async interface. Yeah, in this case, using DMA and async interface combo is the way to go. OK fair enough. -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
Le 24/05/2014 14:00, Marek Vasut a écrit :
On Thursday, May 22, 2014 at 05:09:56 PM, LABBE Corentin wrote:
Do I have to repeat myself ? :)
No, sorry for the commit message, I begin to use git
Generally I agree all your remarks with some comments below.
Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com
---
drivers/crypto/Kconfig| 49 ++
drivers/crypto/Makefile |1 +
drivers/crypto/sunxi-ss.c | 1476
+ 3 files changed, 1526
insertions(+)
create mode 100644 drivers/crypto/sunxi-ss.c
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 03ccdb0..5ea0922 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -418,4 +418,53 @@ config CRYPTO_DEV_MXS_DCP
To compile this driver as a module, choose M here: the module
will be called mxs-dcp.
+config CRYPTO_DEV_SUNXI_SS
+tristate Support for Allwinner Security System cryptographic
accelerator +depends on ARCH_SUNXI
+help
+ Some Allwinner processors have a crypto accelerator named
+ Security System. Select this if you want to use it.
+
+ To compile this driver as a module, choose M here: the module
+ will be called sunxi-ss.
+
+if CRYPTO_DEV_SUNXI_SS
+config CRYPTO_DEV_SUNXI_SS_PRNG
+bool Security System PRNG
+select CRYPTO_RNG2
+help
+ If you enable this option, the SS will provide a pseudo random
+ number generator.
+config CRYPTO_DEV_SUNXI_SS_MD5
+bool Security System MD5
+select CRYPTO_MD5
+help
+ If you enable this option, the SS will provide MD5 hardware
+ acceleration.
This is one IP block and it provides 5 algorithms. Put it under one config
option please.
I want to let the user choose what it want to be used. Someone could want only
to accelerate md5 and to not use the PRNG.
Lots of other hw crypto driver do the same.
Also, just shorted this to CONFIG_CRYPTO_SUNXI_SS , the _DEV stuff in the
name
is useless.
I think not, most of cryptographic hardware driver begin with CRYPTO_DEV
(CRYPTO_DEV_PADLOCK, CRYPTO_DEV_GEODE, CRYPTO_DEV_TALITOS etc...), only S390
does not have a _DEV.
,
[...]
diff --git a/drivers/crypto/sunxi-ss.c b/drivers/crypto/sunxi-ss.c
new file mode 100644
index 000..bbf57bc
--- /dev/null
+++ b/drivers/crypto/sunxi-ss.c
@@ -0,0 +1,1476 @@
+/*
+ * sunxi-ss.c - hardware cryptographic accelerator for Allwinner A20 SoC
Why can this not be generic Allwinner-all-chips driver ? Does the IP block
change that dramatically between the chips?
As I said in my introduction email, lots of allwinner chips seems to have the
same crypto device.
But since I do not own any of those hardware, and in most case does not have a
datasheet, I only assume support for A20.
I will add this comment in the header of the driver.
[...]
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
+#include crypto/md5.h
+#define SUNXI_SS_HASH_COMMON
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_SHA1
+#include crypto/sha.h
+#define SUNXI_SS_HASH_COMMON
+#endif
+#ifdef SUNXI_SS_HASH_COMMON
+#include crypto/hash.h
+#include crypto/internal/hash.h
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_AES
+#include crypto/aes.h
+#endif
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_3DES
+#define SUNXI_SS_DES
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_DES
+#define SUNXI_SS_DES
+#endif
+#ifdef SUNXI_SS_DES
+#include crypto/des.h
+#endif
Please discard this mayhem when getting rid of all the configuration option.
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
+#include crypto/internal/rng.h
+
+struct prng_context {
+u8 seed[192/8];
+unsigned int slen;
+};
+#endif
+
+#define SUNXI_SS_CTL0x00
+#define SUNXI_SS_KEY0 0x04
+#define SUNXI_SS_KEY1 0x08
+#define SUNXI_SS_KEY2 0x0C
+#define SUNXI_SS_KEY3 0x10
+#define SUNXI_SS_KEY4 0x14
+#define SUNXI_SS_KEY5 0x18
+#define SUNXI_SS_KEY6 0x1C
+#define SUNXI_SS_KEY7 0x20
+
+#define SUNXI_SS_IV00x24
+#define SUNXI_SS_IV10x28
+#define SUNXI_SS_IV20x2C
+#define SUNXI_SS_IV30x30
+
+#define SUNXI_SS_CNT0 0x34
+#define SUNXI_SS_CNT1 0x38
+#define SUNXI_SS_CNT2 0x3C
+#define SUNXI_SS_CNT3 0x40
+
+#define SUNXI_SS_FCSR 0x44
+#define SUNXI_SS_ICSR 0x48
+
+#define SUNXI_SS_MD00x4C
+#define SUNXI_SS_MD10x50
+#define SUNXI_SS_MD20x54
+#define SUNXI_SS_MD30x58
+#define SUNXI_SS_MD40x5C
+
+#define SS_RXFIFO 0x200
+#define SS_TXFIFO 0x204
You don't have much consistency in the register naming scheme, please fix
this
somehow. I suppose renaming the SS_[RT]XFIFO registers to SUNXI_SS_[RT]XFIFO
is
a good idea.
+/* SUNXI_SS_CTL configuration values */
+
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
On Sunday, May 25, 2014 at 01:58:39 PM, Corentin LABBE wrote: [...] This is one IP block and it provides 5 algorithms. Put it under one config option please. I want to let the user choose what it want to be used. Someone could want only to accelerate md5 and to not use the PRNG. Lots of other hw crypto driver do the same. I don't find this useful, most users will enable all of them anyway. Also, just shorted this to CONFIG_CRYPTO_SUNXI_SS , the _DEV stuff in the name is useless. I think not, most of cryptographic hardware driver begin with CRYPTO_DEV (CRYPTO_DEV_PADLOCK, CRYPTO_DEV_GEODE, CRYPTO_DEV_TALITOS etc...), only S390 does not have a _DEV. OK. I don't mind either way. [...] diff --git a/drivers/crypto/sunxi-ss.c b/drivers/crypto/sunxi-ss.c new file mode 100644 index 000..bbf57bc --- /dev/null +++ b/drivers/crypto/sunxi-ss.c @@ -0,0 +1,1476 @@ +/* + * sunxi-ss.c - hardware cryptographic accelerator for Allwinner A20 SoC Why can this not be generic Allwinner-all-chips driver ? Does the IP block change that dramatically between the chips? As I said in my introduction email, lots of allwinner chips seems to have the same crypto device. But since I do not own any of those hardware, and in most case does not have a datasheet, I only assume support for A20. I will add this comment in the header of the driver. Can you ask others to test with other chips? Surely, you can easily prepare some kind of a test for others to verify on their chips. [...] + dev_dbg(ss_ctx-dev, DEBUG Seed %d %x\n, i, v); + } But this debug instrumentation looks quite useless anyway. As I said in my introduction mail, I cannot get PRNG to work, so it is the reason of lots of dev_dbg() Anyway, I will remove them. Then please don't submit non-functional code for inclusion into kernel. Just discard the PRNG part completely until it's operational. Submit just the portions of code that are working please. [...] -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
On Thursday, May 22, 2014 at 05:09:56 PM, LABBE Corentin wrote:
Do I have to repeat myself ? :)
Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com
---
drivers/crypto/Kconfig| 49 ++
drivers/crypto/Makefile |1 +
drivers/crypto/sunxi-ss.c | 1476
+ 3 files changed, 1526
insertions(+)
create mode 100644 drivers/crypto/sunxi-ss.c
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 03ccdb0..5ea0922 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -418,4 +418,53 @@ config CRYPTO_DEV_MXS_DCP
To compile this driver as a module, choose M here: the module
will be called mxs-dcp.
+config CRYPTO_DEV_SUNXI_SS
+tristate Support for Allwinner Security System cryptographic
accelerator +depends on ARCH_SUNXI
+help
+ Some Allwinner processors have a crypto accelerator named
+ Security System. Select this if you want to use it.
+
+ To compile this driver as a module, choose M here: the module
+ will be called sunxi-ss.
+
+if CRYPTO_DEV_SUNXI_SS
+config CRYPTO_DEV_SUNXI_SS_PRNG
+ bool Security System PRNG
+ select CRYPTO_RNG2
+ help
+ If you enable this option, the SS will provide a pseudo random
+ number generator.
+config CRYPTO_DEV_SUNXI_SS_MD5
+ bool Security System MD5
+ select CRYPTO_MD5
+ help
+ If you enable this option, the SS will provide MD5 hardware
+ acceleration.
This is one IP block and it provides 5 algorithms. Put it under one config
option please.
Also, just shorted this to CONFIG_CRYPTO_SUNXI_SS , the _DEV stuff in the name
is useless.
[...]
diff --git a/drivers/crypto/sunxi-ss.c b/drivers/crypto/sunxi-ss.c
new file mode 100644
index 000..bbf57bc
--- /dev/null
+++ b/drivers/crypto/sunxi-ss.c
@@ -0,0 +1,1476 @@
+/*
+ * sunxi-ss.c - hardware cryptographic accelerator for Allwinner A20 SoC
Why can this not be generic Allwinner-all-chips driver ? Does the IP block
change that dramatically between the chips?
[...]
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
+#include crypto/md5.h
+#define SUNXI_SS_HASH_COMMON
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_SHA1
+#include crypto/sha.h
+#define SUNXI_SS_HASH_COMMON
+#endif
+#ifdef SUNXI_SS_HASH_COMMON
+#include crypto/hash.h
+#include crypto/internal/hash.h
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_AES
+#include crypto/aes.h
+#endif
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_3DES
+#define SUNXI_SS_DES
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_DES
+#define SUNXI_SS_DES
+#endif
+#ifdef SUNXI_SS_DES
+#include crypto/des.h
+#endif
Please discard this mayhem when getting rid of all the configuration option.
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
+#include crypto/internal/rng.h
+
+struct prng_context {
+ u8 seed[192/8];
+ unsigned int slen;
+};
+#endif
+
+#define SUNXI_SS_CTL0x00
+#define SUNXI_SS_KEY0 0x04
+#define SUNXI_SS_KEY1 0x08
+#define SUNXI_SS_KEY2 0x0C
+#define SUNXI_SS_KEY3 0x10
+#define SUNXI_SS_KEY4 0x14
+#define SUNXI_SS_KEY5 0x18
+#define SUNXI_SS_KEY6 0x1C
+#define SUNXI_SS_KEY7 0x20
+
+#define SUNXI_SS_IV00x24
+#define SUNXI_SS_IV10x28
+#define SUNXI_SS_IV20x2C
+#define SUNXI_SS_IV30x30
+
+#define SUNXI_SS_CNT0 0x34
+#define SUNXI_SS_CNT1 0x38
+#define SUNXI_SS_CNT2 0x3C
+#define SUNXI_SS_CNT3 0x40
+
+#define SUNXI_SS_FCSR 0x44
+#define SUNXI_SS_ICSR 0x48
+
+#define SUNXI_SS_MD00x4C
+#define SUNXI_SS_MD10x50
+#define SUNXI_SS_MD20x54
+#define SUNXI_SS_MD30x58
+#define SUNXI_SS_MD40x5C
+
+#define SS_RXFIFO 0x200
+#define SS_TXFIFO 0x204
You don't have much consistency in the register naming scheme, please fix this
somehow. I suppose renaming the SS_[RT]XFIFO registers to SUNXI_SS_[RT]XFIFO is
a good idea.
+/* SUNXI_SS_CTL configuration values */
+
+/* AES/DES/3DES key select - bits 24-27 */
+#define SUNXI_SS_KEYSELECT_KEYN (0 24)
Uh oh , you ORR some values with this flag, which is always zero. This seems
broken.
[...]
+/* SS Method - bits 4-6 */
+#define SUNXI_OP_AES(0 4)
+#define SUNXI_OP_DES(1 4)
+#define SUNXI_OP_3DES (2 4)
+#define SUNXI_OP_SHA1 (3 4)
+#define SUNXI_OP_MD5(4 4)
+#define SUNXI_OP_PRNG (5 4)
+
+/* Data end bit - bit 2 */
+#define SUNXI_SS_DATA_END BIT(2)
Please use the BIT() macros in consistent fashion. Either use it or don't use
it
(I'd much rather see it not used), but don't mix the stuff :-(
[...]
+/* General notes:
+ * I cannot use a key/IV
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
On Friday, May 23, 2014 at 12:46:10 PM, Arnd Bergmann wrote: On Thursday 22 May 2014, Corentin LABBE wrote: Le 22/05/2014 17:28, Arnd Bergmann a écrit : On Thursday 22 May 2014 17:09:56 LABBE Corentin wrote: Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com My feeling is that this should either be one driver that provides all five algorithms unconditionally, or five drivers that are each separate loadable modules and based on top of a common module that only exports functions but has no active logic itself I agree for the split. It was my first intention but I feared to add too many files. So I propose to split in 6, sunxi-ss-hash.c, sunxi-ss-des.c, sunxi-ss-aes.c, sunxi-ss-rng.c, sunxi-ss-common.c and sunxi-ss.h Does can I add a sunxi-ss directory in drivers/crypto ? Yes, I think a subdirectory would be best. Full ACK on this one. Use drivers/crypto/sunxi-ss/ . [...] Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com
---
drivers/crypto/Kconfig| 49 ++
drivers/crypto/Makefile |1 +
drivers/crypto/sunxi-ss.c | 1476 +
3 files changed, 1526 insertions(+)
create mode 100644 drivers/crypto/sunxi-ss.c
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 03ccdb0..5ea0922 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -418,4 +418,53 @@ config CRYPTO_DEV_MXS_DCP
To compile this driver as a module, choose M here: the module
will be called mxs-dcp.
+config CRYPTO_DEV_SUNXI_SS
+tristate Support for Allwinner Security System cryptographic
accelerator
+depends on ARCH_SUNXI
+help
+ Some Allwinner processors have a crypto accelerator named
+ Security System. Select this if you want to use it.
+
+ To compile this driver as a module, choose M here: the module
+ will be called sunxi-ss.
+
+if CRYPTO_DEV_SUNXI_SS
+config CRYPTO_DEV_SUNXI_SS_PRNG
+ bool Security System PRNG
+ select CRYPTO_RNG2
+ help
+ If you enable this option, the SS will provide a pseudo random
+ number generator.
+config CRYPTO_DEV_SUNXI_SS_MD5
+ bool Security System MD5
+ select CRYPTO_MD5
+ help
+ If you enable this option, the SS will provide MD5 hardware
+ acceleration.
+config CRYPTO_DEV_SUNXI_SS_SHA1
+ bool Security System SHA1
+ select CRYPTO_SHA1
+ help
+ If you enable this option, the SS will provide SHA1 hardware
+ acceleration.
+config CRYPTO_DEV_SUNXI_SS_AES
+ bool Security System AES
+ select CRYPTO_AES
+ help
+ If you enable this option, the SS will provide AES hardware
+ acceleration.
+config CRYPTO_DEV_SUNXI_SS_DES
+ bool Security System DES
+ select CRYPTO_DES
+ help
+ If you enable this option, the SS will provide DES hardware
+ acceleration.
+config CRYPTO_DEV_SUNXI_SS_3DES
+ bool Security System 3DES
+ select CRYPTO_DES
+ help
+ If you enable this option, the SS will provide 3DES hardware
+ acceleration.
+endif #CRYPTO_DEV_SUNXI_SS
+
endif # CRYPTO_HW
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index 482f090..490dae5 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -23,3 +23,4 @@ obj-$(CONFIG_CRYPTO_DEV_S5P) += s5p-sss.o
obj-$(CONFIG_CRYPTO_DEV_SAHARA) += sahara.o
obj-$(CONFIG_CRYPTO_DEV_TALITOS) += talitos.o
obj-$(CONFIG_CRYPTO_DEV_UX500) += ux500/
+obj-$(CONFIG_CRYPTO_DEV_SUNXI_SS) += sunxi-ss.o
diff --git a/drivers/crypto/sunxi-ss.c b/drivers/crypto/sunxi-ss.c
new file mode 100644
index 000..bbf57bc
--- /dev/null
+++ b/drivers/crypto/sunxi-ss.c
@@ -0,0 +1,1476 @@
+/*
+ * sunxi-ss.c - hardware cryptographic accelerator for Allwinner A20 SoC
+ *
+ * Copyright (C) 2013-2014 Corentin LABBE clabbe.montj...@gmail.com
+ *
+ * Support AES cipher with 128,192,256 bits keysize.
+ * Support MD5 and SHA1 hash algorithms.
+ * Support DES and 3DES
+ * Support PRNG
+ *
+ * You could find the datasheet at
+ * http://dl.linux-sunxi.org/A20/A20%20User%20Manual%202013-03-22.pdf
+ *
+ *
+ * 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 version 2 of the License
+ */
+
+#include linux/clk.h
+#include linux/crypto.h
+#include linux/io.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include crypto/scatterwalk.h
+#include linux/scatterlist.h
+#include linux/interrupt.h
+#include linux/delay.h
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
+#include crypto/md5.h
+#define SUNXI_SS_HASH_COMMON
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_SHA1
+#include crypto/sha.h
+#define SUNXI_SS_HASH_COMMON
+#endif
+#ifdef SUNXI_SS_HASH_COMMON
+#include crypto/hash.h
+#include crypto/internal/hash.h
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_AES
+#include crypto/aes.h
+#endif
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_3DES
+#define SUNXI_SS_DES
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_DES
+#define SUNXI_SS_DES
+#endif
+#ifdef SUNXI_SS_DES
+#include crypto/des.h
+#endif
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
+#include crypto/internal/rng.h
+
+struct prng_context {
+ u8 seed[192/8];
+ unsigned int slen;
+};
+#endif
+
+#define SUNXI_SS_CTL0x00
+#define SUNXI_SS_KEY0 0x04
+#define SUNXI_SS_KEY1 0x08
+#define SUNXI_SS_KEY2 0x0C
+#define SUNXI_SS_KEY3 0x10
+#define SUNXI_SS_KEY4 0x14
+#define SUNXI_SS_KEY5 0x18
+#define SUNXI_SS_KEY6 0x1C
+#define SUNXI_SS_KEY7 0x20
+
+#define SUNXI_SS_IV00x24
+#define SUNXI_SS_IV10x28
+#define SUNXI_SS_IV20x2C
+#define SUNXI_SS_IV30x30
+
+#define SUNXI_SS_CNT0 0x34
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
On Thursday 22 May 2014 17:09:56 LABBE Corentin wrote:
Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com
---
drivers/crypto/Kconfig| 49 ++
drivers/crypto/Makefile |1 +
drivers/crypto/sunxi-ss.c | 1476
+
3 files changed, 1526 insertions(+)
create mode 100644 drivers/crypto/sunxi-ss.c
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 03ccdb0..5ea0922 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -418,4 +418,53 @@ config CRYPTO_DEV_MXS_DCP
To compile this driver as a module, choose M here: the module
will be called mxs-dcp.
+config CRYPTO_DEV_SUNXI_SS
+tristate Support for Allwinner Security System cryptographic
accelerator
+depends on ARCH_SUNXI
+help
+ Some Allwinner processors have a crypto accelerator named
+ Security System. Select this if you want to use it.
+
+ To compile this driver as a module, choose M here: the module
+ will be called sunxi-ss.
+
+if CRYPTO_DEV_SUNXI_SS
+config CRYPTO_DEV_SUNXI_SS_PRNG
+ bool Security System PRNG
+ select CRYPTO_RNG2
+ help
+ If you enable this option, the SS will provide a pseudo random
+ number generator.
+config CRYPTO_DEV_SUNXI_SS_MD5
+ bool Security System MD5
+ select CRYPTO_MD5
+ help
+ If you enable this option, the SS will provide MD5 hardware
+ acceleration.
My feeling is that this should either be one driver that provides
all five algorithms unconditionally, or five drivers that are each
separate loadable modules and based on top of a common module
that only exports functions but has no active logic itself
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
+#include crypto/md5.h
+#define SUNXI_SS_HASH_COMMON
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_SHA1
+#include crypto/sha.h
+#define SUNXI_SS_HASH_COMMON
+#endif
+#ifdef SUNXI_SS_HASH_COMMON
+#include crypto/hash.h
+#include crypto/internal/hash.h
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_AES
+#include crypto/aes.h
+#endif
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_3DES
+#define SUNXI_SS_DES
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_DES
+#define SUNXI_SS_DES
+#endif
+#ifdef SUNXI_SS_DES
+#include crypto/des.h
+#endif
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
+#include crypto/internal/rng.h
That would cleanup this #ifdef chain either way.
+/* General notes:
+ * I cannot use a key/IV cache because each time one of these change ALL
stuff
+ * need to be re-writed.
+ * And for example, with dm-crypt IV changes on each request.
+ *
+ * After each request the device must be disabled.
+ *
+ * For performance reason, we use writel_relaxed/read_relaxed for all
+ * operations on RX and TX FIFO.
+ * For all other registers, we use writel.
+ * See http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117644
+ * and http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117640
+ * */
+
+static struct sunxi_ss_ctx {
+ void *base;
__iomem ?
+ int irq;
+ struct clk *busclk;
+ struct clk *ssclk;
+ struct device *dev;
+ struct resource *res;
+ void *buf_in; /* pointer to data to be uploaded to the device */
+ size_t buf_in_size; /* size of buf_in */
+ void *buf_out;
+ size_t buf_out_size;
+} _ss_ctx, *ss_ctx = _ss_ctx;
+
+static DEFINE_MUTEX(lock);
+static DEFINE_MUTEX(bufout_lock);
+static DEFINE_MUTEX(bufin_lock);
I guess the mutexes should be part of sunxi_ss_ctx.
Are you sure you need the global _ss_ctx structure?
Normally you should get a pointer from whoever calls you.
+struct sunxi_req_ctx {
+ u8 key[AES_MAX_KEY_SIZE * 8];
+ u32 keylen;
+ u32 mode;
+ u64 byte_count; /* number of bytes uploaded to the device */
+ u32 waitbuf; /* a partial word waiting to be completed and
+ uploaded to the device */
+ /* number of bytes to be uploaded in the waitbuf word */
+ unsigned int nbwait;
+};
+
+#ifdef SUNXI_SS_HASH_COMMON
+/**/
+/**/
Please remove all the lines.
+ /* If we have more than one SG, we cannot use kmap_atomic since
+ * we hold the mapping too long*/
+ src_addr = kmap(sg_page(in_sg)) + in_sg-offset;
+ if (src_addr == NULL) {
+ dev_err(ss_ctx-dev, KMAP error for src SG\n);
+ return -1;
+ }
+ dst_addr = kmap(sg_page(out_sg)) + out_sg-offset;
+ if (dst_addr == NULL) {
+ kunmap(src_addr);
+ dev_err(ss_ctx-dev, KMAP error for dst SG\n);
+ return -1;
+ }
+ src32 = (u32 *)src_addr;
+ dst32 = (u32 *)dst_addr;
+ ileft = nbytes / 4;
+ oleft = nbytes / 4;
+ sgileft = in_sg-length / 4;
+ sgoleft =
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
Le 22/05/2014 17:28, Arnd Bergmann a écrit :
On Thursday 22 May 2014 17:09:56 LABBE Corentin wrote:
Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com
---
drivers/crypto/Kconfig| 49 ++
drivers/crypto/Makefile |1 +
drivers/crypto/sunxi-ss.c | 1476
+
3 files changed, 1526 insertions(+)
create mode 100644 drivers/crypto/sunxi-ss.c
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 03ccdb0..5ea0922 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -418,4 +418,53 @@ config CRYPTO_DEV_MXS_DCP
To compile this driver as a module, choose M here: the module
will be called mxs-dcp.
+config CRYPTO_DEV_SUNXI_SS
+tristate Support for Allwinner Security System cryptographic
accelerator
+depends on ARCH_SUNXI
+help
+ Some Allwinner processors have a crypto accelerator named
+ Security System. Select this if you want to use it.
+
+ To compile this driver as a module, choose M here: the module
+ will be called sunxi-ss.
+
+if CRYPTO_DEV_SUNXI_SS
+config CRYPTO_DEV_SUNXI_SS_PRNG
+bool Security System PRNG
+select CRYPTO_RNG2
+help
+ If you enable this option, the SS will provide a pseudo random
+ number generator.
+config CRYPTO_DEV_SUNXI_SS_MD5
+bool Security System MD5
+select CRYPTO_MD5
+help
+ If you enable this option, the SS will provide MD5 hardware
+ acceleration.
My feeling is that this should either be one driver that provides
all five algorithms unconditionally, or five drivers that are each
separate loadable modules and based on top of a common module
that only exports functions but has no active logic itself
I agree for the split.
It was my first intention but I feared to add too many files.
So I propose to split in 6, sunxi-ss-hash.c, sunxi-ss-des.c, sunxi-ss-aes.c,
sunxi-ss-rng.c, sunxi-ss-common.c and sunxi-ss.h
Does can I add a sunxi-ss directory in drivers/crypto ?
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
+#include crypto/md5.h
+#define SUNXI_SS_HASH_COMMON
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_SHA1
+#include crypto/sha.h
+#define SUNXI_SS_HASH_COMMON
+#endif
+#ifdef SUNXI_SS_HASH_COMMON
+#include crypto/hash.h
+#include crypto/internal/hash.h
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_AES
+#include crypto/aes.h
+#endif
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_3DES
+#define SUNXI_SS_DES
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_DES
+#define SUNXI_SS_DES
+#endif
+#ifdef SUNXI_SS_DES
+#include crypto/des.h
+#endif
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
+#include crypto/internal/rng.h
That would cleanup this #ifdef chain either way.
+/* General notes:
+ * I cannot use a key/IV cache because each time one of these change ALL
stuff
+ * need to be re-writed.
+ * And for example, with dm-crypt IV changes on each request.
+ *
+ * After each request the device must be disabled.
+ *
+ * For performance reason, we use writel_relaxed/read_relaxed for all
+ * operations on RX and TX FIFO.
+ * For all other registers, we use writel.
+ * See http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117644
+ * and http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117640
+ * */
+
+static struct sunxi_ss_ctx {
+void *base;
__iomem ?
ok
+int irq;
+struct clk *busclk;
+struct clk *ssclk;
+struct device *dev;
+struct resource *res;
+void *buf_in; /* pointer to data to be uploaded to the device */
+size_t buf_in_size; /* size of buf_in */
+void *buf_out;
+size_t buf_out_size;
+} _ss_ctx, *ss_ctx = _ss_ctx;
+
+static DEFINE_MUTEX(lock);
+static DEFINE_MUTEX(bufout_lock);
+static DEFINE_MUTEX(bufin_lock);
I guess the mutexes should be part of sunxi_ss_ctx.
Are you sure you need the global _ss_ctx structure?
Normally you should get a pointer from whoever calls you.
I will use platform_get_drvdata/platform_set_drvdata for cleaning all thoses
global structures..
+struct sunxi_req_ctx {
+u8 key[AES_MAX_KEY_SIZE * 8];
+u32 keylen;
+u32 mode;
+u64 byte_count; /* number of bytes uploaded to the device */
+u32 waitbuf; /* a partial word waiting to be completed and
+uploaded to the device */
+/* number of bytes to be uploaded in the waitbuf word */
+unsigned int nbwait;
+};
+
+#ifdef SUNXI_SS_HASH_COMMON
+/**/
+/**/
Please remove all the lines.
ok
+/* If we have more than one SG, we cannot use kmap_atomic since
+ * we hold the mapping too long*/
+src_addr = kmap(sg_page(in_sg)) + in_sg-offset;
+if (src_addr == NULL) {
+dev_err(ss_ctx-dev, KMAP error for src SG\n);
+
