subject:"\[PATCH v2 2\/3\] crypto\: inside\-secure\: add SafeXcel EIP197 crypto engine driver"

Re: [PATCH v2 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver

2017-04-21 Thread Antoine Tenart

On Fri, Apr 21, 2017 at 01:36:45PM +0200, Corentin Labbe wrote:
> > > > +   memset(ipad + keylen, 0, blocksize - keylen);
> > > > +   memcpy(opad, ipad, blocksize);
> > > > +
> > > > +   for (i = 0; i < blocksize; i++) {
> > > > +   ipad[i] ^= 0x36;
> > > > +   opad[i] ^= 0x5c;
> > > 
> > > What are these constant ?
> > 
> > They are defined in the HMAC RFC, as ipad and opad values. See
> > https://www.ietf.org/rfc/rfc2104.txt.
> > 
> 
> Since many driver use them, I think defining them in include/ should be done 
> (HMAC_IPAD/HMAC_OPAD)
> I will send a patch for it.

OK, I'll send a following up patch on this driver when your series is
merged.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH v2 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver

2017-04-21 Thread Corentin Labbe

> > > + memset(ipad + keylen, 0, blocksize - keylen);
> > > + memcpy(opad, ipad, blocksize);
> > > +
> > > + for (i = 0; i < blocksize; i++) {
> > > + ipad[i] ^= 0x36;
> > > + opad[i] ^= 0x5c;
> > 
> > What are these constant ?
> 
> They are defined in the HMAC RFC, as ipad and opad values. See
> https://www.ietf.org/rfc/rfc2104.txt.
> 

Since many driver use them, I think defining them in include/ should be done 
(HMAC_IPAD/HMAC_OPAD)
I will send a patch for it.

> > [...]
> > > +struct safexcel_alg_template safexcel_alg_sha256 = {
> > > + .type = SAFEXCEL_ALG_TYPE_AHASH,
> > > + .alg.ahash = {
> > > + .init = safexcel_sha256_init,
> > > + .update = safexcel_ahash_update,
> > > + .final = safexcel_ahash_final,
> > > + .finup = safexcel_ahash_finup,
> > > + .digest = safexcel_sha256_digest,
> > > + .export = safexcel_ahash_export,
> > > + .import = safexcel_ahash_import,
> > > + .halg = {
> > > + .digestsize = SHA256_DIGEST_SIZE,
> > > + .statesize = sizeof(struct safexcel_ahash_export_state),
> > > + .base = {
> > > + .cra_name = "sha256",
> > > + .cra_driver_name = "safexcel-sha256",
> > > + .cra_priority = 300,
> > > + .cra_flags = CRYPTO_ALG_ASYNC |
> > > +  CRYPTO_ALG_KERN_DRIVER_ONLY,
> > 
> > Why do use CRYPTO_ALG_KERN_DRIVER_ONLY ?
> 
> See http://lxr.free-electrons.com/source/include/linux/crypto.h#L97.
> 

Sorry, I had understood that flag as "do not let userspace use me".
Anyway, this flag is totally ignored by the cryptoAPI.

Re: [PATCH v2 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver

2017-04-21 Thread Antoine Tenart

Hi Corentin,

On Fri, Apr 21, 2017 at 09:30:56AM +0200, Corentin Labbe wrote:
> 
> I have some minor comment below

[…]

> > +   /*
> > +* Result Descriptor Ring prepare
> > +*/
> 
> This is not preferred comment format for one line

Sure.

> 
> [...]
> 
> > +static int safexcel_probe(struct platform_device *pdev)
> > +{
> > +   struct device *dev = >dev;
> > +   struct resource *res;
> > +   struct safexcel_crypto_priv *priv;
> > +   int i, ret;
> > +
> > +   priv = devm_kzalloc(dev, sizeof(struct safexcel_crypto_priv),
> > +   GFP_KERNEL);
> 
> sizeof(priv) is preferred as asked by checkpatch
> 
> [...]
> > +   ring_irq = devm_kzalloc(dev, sizeof(struct 
> > safexcel_ring_irq_data),
> > +   GFP_KERNEL);
> 
> same comment here

Sure.

> [...]
> > +#define EIP197_ALG_ARC4BIT(7)
> > +#define EIP197_ALG_AES_ECB BIT(8)
> > +#define EIP197_ALG_AES_CBC BIT(9)
> > +#define EIP197_ALG_AES_CTR_ICM BIT(10)
> > +#define EIP197_ALG_AES_OFB BIT(11)
> > +#define EIP197_ALG_AES_CFB BIT(12)
> > +#define EIP197_ALG_DES_ECB BIT(13)
> > +#define EIP197_ALG_DES_CBC BIT(14)
> > +#define EIP197_ALG_DES_OFB BIT(16)
> > +#define EIP197_ALG_DES_CFB BIT(17)
> > +#define EIP197_ALG_3DES_ECBBIT(18)
> > +#define EIP197_ALG_3DES_CBCBIT(19)
> > +#define EIP197_ALG_3DES_OFBBIT(21)
> > +#define EIP197_ALG_3DES_CFBBIT(22)
> > +#define EIP197_ALG_MD5 BIT(24)
> > +#define EIP197_ALG_HMAC_MD5BIT(25)
> 
> Does MD5, DES and 3DES will be added later ?

They might be added yes. And as these bits describe a register used to
configure the engine it's nice to have a proper definition of what all
combinations do.

> [...]
> > +static const u8 sha1_zero_digest[SHA1_DIGEST_SIZE] = {
> > +   0xda, 0x39, 0xa3, 0xee, 0x5e, 0x6b, 0x4b, 0x0d, 0x32, 0x55,
> > +   0xbf, 0xef, 0x95, 0x60, 0x18, 0x90, 0xaf, 0xd8, 0x07, 0x09,
> > +};
> > +
> > +static const u8 sha224_zero_digest[SHA224_DIGEST_SIZE] = {
> > +   0xd1, 0x4a, 0x02, 0x8c, 0x2a, 0x3a, 0x2b, 0xc9, 0x47, 0x61,
> > +   0x02, 0xbb, 0x28, 0x82, 0x34, 0xc4, 0x15, 0xa2, 0xb0, 0x1f,
> > +   0x82, 0x8e, 0xa6, 0x2a, 0xc5, 0xb3, 0xe4, 0x2f
> > +};
> > +
> > +static const u8 sha256_zero_digest[SHA256_DIGEST_SIZE] = {
> > +   0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb,
> > +   0xf4, 0xc8, 0x99, 0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4,
> > +   0x64, 0x9b, 0x93, 0x4c, 0xa4, 0x95, 0x99, 0x1b, 0x78, 0x52,
> > +   0xb8, 0x55
> > +};
> 
> Thoses structures are already defined in crypto (sha1_zero_message_hash, 
> etc...)
> You can use it since you select SHAxxx in Kconfig

That's right, I'll use these definitions instead.

> [...]
> > +static int safexcel_hmac_init_pad(struct ahash_request *areq,
> > + unsigned int blocksize, const u8 *key,
> > + unsigned int keylen, u8 *ipad, u8 *opad)
> > +{
> > +   struct safexcel_ahash_result result;
> > +   struct scatterlist sg;
> > +   int ret, i;
> > +   u8 *keydup;
> > +
> > +   if (keylen <= blocksize) {
> > +   memcpy(ipad, key, keylen);
> > +   } else {
> > +   keydup = kmemdup(key, keylen, GFP_KERNEL);
> > +   if (!keydup)
> > +   return -ENOMEM;
> > +
> > +   ahash_request_set_callback(areq, CRYPTO_TFM_REQ_MAY_BACKLOG,
> > +  safexcel_ahash_complete, );
> > +   sg_init_one(, keydup, keylen);
> > +   ahash_request_set_crypt(areq, , ipad, keylen);
> > +   init_completion();
> > +
> > +   ret = crypto_ahash_digest(areq);
> > +   if (ret == -EINPROGRESS) {
> > +   wait_for_completion_interruptible();
> > +   ret = result.error;
> > +   }
> > +
> > +   /* Avoid leaking */
> > +   memset(keydup, 0, keylen);
> 
> It is safer to use memzero_explicit

Good to know, I'll update.

> > +   kfree(keydup);
> > +
> > +   if (ret)
> > +   return ret;
> > +
> > +   keylen = crypto_ahash_digestsize(crypto_ahash_reqtfm(areq));
> > +   }
> > +
> > +   memset(ipad + keylen, 0, blocksize - keylen);
> > +   memcpy(opad, ipad, blocksize);
> > +
> > +   for (i = 0; i < blocksize; i++) {
> > +   ipad[i] ^= 0x36;
> > +   opad[i] ^= 0x5c;
> 
> What are these constant ?

They are defined in the HMAC RFC, as ipad and opad values. See
https://www.ietf.org/rfc/rfc2104.txt.

> [...]
> > +static int safexcel_hmac_sha1_setkey(struct crypto_ahash *tfm, const u8 
> > *key,
> > +unsigned int keylen)
> > +{
> > +   struct safexcel_ahash_ctx *ctx = crypto_tfm_ctx(crypto_ahash_tfm(tfm));
> >

Re: [PATCH v2 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver

2017-04-21 Thread Corentin Labbe

Hello

I have some minor comment below

On Wed, Apr 19, 2017 at 09:14:17AM +0200, Antoine Tenart wrote:
> Add support for Inside Secure SafeXcel EIP197 cryptographic engine,
> which can be found on Marvell Armada 7k and 8k boards. This driver
> currently implements: ecb(aes), cbc(aes), sha1, sha224, sha256 and
> hmac(sah1) algorithms.
> 
> Two firmwares are needed for this engine to work. Their are mostly used
> for more advanced operations than the ones supported (as of now), but we
> still need them to pass the data to the internal cryptographic engine.
> 
> Signed-off-by: Antoine Tenart 
> ---
>  drivers/crypto/Kconfig |   17 +
>  drivers/crypto/Makefile|1 +
>  drivers/crypto/inside-secure/Makefile  |2 +
>  drivers/crypto/inside-secure/safexcel.c|  940 +
>  drivers/crypto/inside-secure/safexcel.h|  579 +
>  drivers/crypto/inside-secure/safexcel_cipher.c |  555 +
>  drivers/crypto/inside-secure/safexcel_hash.c   | 1060 
> 
>  drivers/crypto/inside-secure/safexcel_ring.c   |  157 
>  8 files changed, 3311 insertions(+)
>  create mode 100644 drivers/crypto/inside-secure/Makefile
>  create mode 100644 drivers/crypto/inside-secure/safexcel.c
>  create mode 100644 drivers/crypto/inside-secure/safexcel.h
>  create mode 100644 drivers/crypto/inside-secure/safexcel_cipher.c
>  create mode 100644 drivers/crypto/inside-secure/safexcel_hash.c
>  create mode 100644 drivers/crypto/inside-secure/safexcel_ring.c
> 
> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> index 473d31288ad8..d12a40450858 100644
> --- a/drivers/crypto/Kconfig
> +++ b/drivers/crypto/Kconfig
> @@ -619,4 +619,21 @@ config CRYPTO_DEV_BCM_SPU
> Secure Processing Unit (SPU). The SPU driver registers ablkcipher,
> ahash, and aead algorithms with the kernel cryptographic API.
>  

[...]

> + /*
> +  * Result Descriptor Ring prepare
> +  */

This is not preferred comment format for one line

[...]

> +static int safexcel_probe(struct platform_device *pdev)
> +{
> + struct device *dev = >dev;
> + struct resource *res;
> + struct safexcel_crypto_priv *priv;
> + int i, ret;
> +
> + priv = devm_kzalloc(dev, sizeof(struct safexcel_crypto_priv),
> + GFP_KERNEL);

sizeof(priv) is preferred as asked by checkpatch

[...]
> + ring_irq = devm_kzalloc(dev, sizeof(struct 
> safexcel_ring_irq_data),
> + GFP_KERNEL);

same comment here

[...]
> +#define EIP197_ALG_ARC4  BIT(7)
> +#define EIP197_ALG_AES_ECB   BIT(8)
> +#define EIP197_ALG_AES_CBC   BIT(9)
> +#define EIP197_ALG_AES_CTR_ICM   BIT(10)
> +#define EIP197_ALG_AES_OFB   BIT(11)
> +#define EIP197_ALG_AES_CFB   BIT(12)
> +#define EIP197_ALG_DES_ECB   BIT(13)
> +#define EIP197_ALG_DES_CBC   BIT(14)
> +#define EIP197_ALG_DES_OFB   BIT(16)
> +#define EIP197_ALG_DES_CFB   BIT(17)
> +#define EIP197_ALG_3DES_ECB  BIT(18)
> +#define EIP197_ALG_3DES_CBC  BIT(19)
> +#define EIP197_ALG_3DES_OFB  BIT(21)
> +#define EIP197_ALG_3DES_CFB  BIT(22)
> +#define EIP197_ALG_MD5   BIT(24)
> +#define EIP197_ALG_HMAC_MD5  BIT(25)

Does MD5, DES and 3DES will be added later ?

[...]
> +static const u8 sha1_zero_digest[SHA1_DIGEST_SIZE] = {
> + 0xda, 0x39, 0xa3, 0xee, 0x5e, 0x6b, 0x4b, 0x0d, 0x32, 0x55,
> + 0xbf, 0xef, 0x95, 0x60, 0x18, 0x90, 0xaf, 0xd8, 0x07, 0x09,
> +};
> +
> +static const u8 sha224_zero_digest[SHA224_DIGEST_SIZE] = {
> + 0xd1, 0x4a, 0x02, 0x8c, 0x2a, 0x3a, 0x2b, 0xc9, 0x47, 0x61,
> + 0x02, 0xbb, 0x28, 0x82, 0x34, 0xc4, 0x15, 0xa2, 0xb0, 0x1f,
> + 0x82, 0x8e, 0xa6, 0x2a, 0xc5, 0xb3, 0xe4, 0x2f
> +};
> +
> +static const u8 sha256_zero_digest[SHA256_DIGEST_SIZE] = {
> + 0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb,
> + 0xf4, 0xc8, 0x99, 0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4,
> + 0x64, 0x9b, 0x93, 0x4c, 0xa4, 0x95, 0x99, 0x1b, 0x78, 0x52,
> + 0xb8, 0x55
> +};

Thoses structures are already defined in crypto (sha1_zero_message_hash, etc...)
You can use it since you select SHAxxx in Kconfig

[...]
> +static int safexcel_hmac_init_pad(struct ahash_request *areq,
> +   unsigned int blocksize, const u8 *key,
> +   unsigned int keylen, u8 *ipad, u8 *opad)
> +{
> + struct safexcel_ahash_result result;
> + struct scatterlist sg;
> + int ret, i;
> + u8 *keydup;
> +
> + if (keylen <= blocksize) {
> + memcpy(ipad, key, keylen);
> + } else {
> + keydup = kmemdup(key, keylen,

[PATCH v2 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver

2017-04-19 Thread Antoine Tenart

Add support for Inside Secure SafeXcel EIP197 cryptographic engine,
which can be found on Marvell Armada 7k and 8k boards. This driver
currently implements: ecb(aes), cbc(aes), sha1, sha224, sha256 and
hmac(sah1) algorithms.

Two firmwares are needed for this engine to work. Their are mostly used
for more advanced operations than the ones supported (as of now), but we
still need them to pass the data to the internal cryptographic engine.

Signed-off-by: Antoine Tenart 
---
 drivers/crypto/Kconfig |   17 +
 drivers/crypto/Makefile|1 +
 drivers/crypto/inside-secure/Makefile  |2 +
 drivers/crypto/inside-secure/safexcel.c|  940 +
 drivers/crypto/inside-secure/safexcel.h|  579 +
 drivers/crypto/inside-secure/safexcel_cipher.c |  555 +
 drivers/crypto/inside-secure/safexcel_hash.c   | 1060 
 drivers/crypto/inside-secure/safexcel_ring.c   |  157 
 8 files changed, 3311 insertions(+)
 create mode 100644 drivers/crypto/inside-secure/Makefile
 create mode 100644 drivers/crypto/inside-secure/safexcel.c
 create mode 100644 drivers/crypto/inside-secure/safexcel.h
 create mode 100644 drivers/crypto/inside-secure/safexcel_cipher.c
 create mode 100644 drivers/crypto/inside-secure/safexcel_hash.c
 create mode 100644 drivers/crypto/inside-secure/safexcel_ring.c

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 473d31288ad8..d12a40450858 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -619,4 +619,21 @@ config CRYPTO_DEV_BCM_SPU
  Secure Processing Unit (SPU). The SPU driver registers ablkcipher,
  ahash, and aead algorithms with the kernel cryptographic API.
 
+config CRYPTO_DEV_SAFEXCEL
+   tristate "Inside Secure's SafeXcel cryptographic engine driver"
+   depends on HAS_DMA && OF
+   depends on (ARM64 && ARCH_MVEBU) || (COMPILE_TEST && 64BIT)
+   select CRYPTO_AES
+   select CRYPTO_BLKCIPHER
+   select CRYPTO_HASH
+   select CRYPTO_HMAC
+   select CRYPTO_SHA1
+   select CRYPTO_SHA256
+   select CRYPTO_SHA512
+   help
+ This driver interfaces with the SafeXcel EIP-197 cryptographic engine
+ designed by Inside Secure. Select this if you want to use CBC/ECB
+ chain mode, AES cipher mode and SHA1/SHA224/SHA256/SHA512 hash
+ algorithms.
+
 endif # CRYPTO_HW
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index 739609471169..7ed3e7940f76 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -36,3 +36,4 @@ obj-$(CONFIG_CRYPTO_DEV_UX500) += ux500/
 obj-$(CONFIG_CRYPTO_DEV_VIRTIO) += virtio/
 obj-$(CONFIG_CRYPTO_DEV_VMX) += vmx/
 obj-$(CONFIG_CRYPTO_DEV_BCM_SPU) += bcm/
+obj-$(CONFIG_CRYPTO_DEV_SAFEXCEL) += inside-secure/
diff --git a/drivers/crypto/inside-secure/Makefile 
b/drivers/crypto/inside-secure/Makefile
new file mode 100644
index ..302f07dde98c
--- /dev/null
+++ b/drivers/crypto/inside-secure/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_CRYPTO_DEV_SAFEXCEL) += crypto_safexcel.o
+crypto_safexcel-objs := safexcel.o safexcel_ring.o safexcel_cipher.o 
safexcel_hash.o
diff --git a/drivers/crypto/inside-secure/safexcel.c 
b/drivers/crypto/inside-secure/safexcel.c
new file mode 100644
index ..050518be194c
--- /dev/null
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -0,0 +1,940 @@
+/*
+ * Copyright (C) 2017 Marvell
+ *
+ * Antoine Tenart 
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "safexcel.h"
+
+static u32 max_rings = EIP197_MAX_RINGS;
+module_param(max_rings, uint, 0644);
+MODULE_PARM_DESC(max_rings, "Maximum number of rings to use.");
+
+static void eip197_trc_cache_init(struct safexcel_crypto_priv *priv)
+{
+   u32 val, htable_offset;
+   int i;
+
+   /* Enable the record cache memory access */
+   val = readl(priv->base + EIP197_CS_RAM_CTRL);
+   val &= ~EIP197_TRC_ENABLE_MASK;
+   val |= EIP197_TRC_ENABLE_0;
+   writel(val, priv->base + EIP197_CS_RAM_CTRL);
+
+   /* Clear all ECC errors */
+   writel(0, priv->base + EIP197_TRC_ECCCTRL);
+
+   /*
+* Make sure the cache memory is accessible by taking record cache into
+* reset.
+*/
+   val = readl(priv->base + EIP197_TRC_PARAMS);
+   val |= EIP197_TRC_PARAMS_SW_RESET;
+   val &= ~EIP197_TRC_PARAMS_DATA_ACCESS;
+   writel(val, priv->base + EIP197_TRC_PARAMS);
+
+   /* Clear all records */
+   for (i = 0; i < EIP197_CS_RC_MAX; i++) {
+   u32 val, offset = EIP197_CLASSIFICATION_RAMS + i *

Re: [PATCH v2 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver

Re: [PATCH v2 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver

Re: [PATCH v2 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver

Re: [PATCH v2 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver

[PATCH v2 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver

5 matches

Site Navigation

Mail list logo

Footer information