linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/14] crypto: inside-secure - self-test fixes
@ 2019-05-27 14:50 Antoine Tenart
  2019-05-27 14:50 ` [PATCH 01/14] crypto: inside-secure - remove empty line Antoine Tenart
                   ` (14 more replies)
  0 siblings, 15 replies; 16+ messages in thread
From: Antoine Tenart @ 2019-05-27 14:50 UTC (permalink / raw)
  To: herbert, davem
  Cc: Antoine Tenart, linux-crypto, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh, igall

Hello,

The crypto runtime self-tests were improved and thanks to this we
spotted new issues within the Inside Secure SafeXcel cryptographic
engine driver:
- Intermediate IV were not retrieved from the hardware nor copied to the
  quest IV buffer for cbc(aes/des).
- HMAC updates wasn't supported.
- We spotted issues with the use of the SG lists.
- There was an issue with the use of result buffers.

The series fixes all those issues, and includes other small changes
found while doing this work.

Thanks!
Antoine

Antoine Tenart (14):
  crypto: inside-secure - remove empty line
  crypto: inside-secure - move comment
  crypto: inside-secure - fix coding style for a condition
  crypto: inside-secure - remove useless check
  crypto: inside-secure - improve the result error format when displayed
  crypto: inside-secure - change returned error when a descriptor
    reports an error
  crypto: inside-secure - enable context reuse
  crypto: inside-secure - unify cache reset
  crypto: inside-secure - fix zeroing of the request in ahash_exit_inv
  crypto: inside-secure - fix queued len computation
  crypto: inside-secure - implement IV retrieval
  crypto: inside-secure - add support for HMAC updates
  crypto: inside-secure - fix use of the SG list
  crypto: inside-secure - do not rely on the hardware last bit for
    result descriptors

 drivers/crypto/inside-secure/safexcel.c       |  13 +-
 drivers/crypto/inside-secure/safexcel.h       |  17 ++-
 .../crypto/inside-secure/safexcel_cipher.c    | 116 +++++++++++-------
 drivers/crypto/inside-secure/safexcel_hash.c  |  92 ++++++++------
 drivers/crypto/inside-secure/safexcel_ring.c  |   3 +
 5 files changed, 157 insertions(+), 84 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 01/14] crypto: inside-secure - remove empty line
  2019-05-27 14:50 [PATCH 00/14] crypto: inside-secure - self-test fixes Antoine Tenart
@ 2019-05-27 14:50 ` Antoine Tenart
  2019-05-27 14:50 ` [PATCH 02/14] crypto: inside-secure - move comment Antoine Tenart
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Antoine Tenart @ 2019-05-27 14:50 UTC (permalink / raw)
  To: herbert, davem
  Cc: Antoine Tenart, linux-crypto, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh, igall

Cosmetic patch removing an empty line in the skcipher token creation
routine.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/crypto/inside-secure/safexcel_cipher.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
index de4be10b172f..aca1cdf33362 100644
--- a/drivers/crypto/inside-secure/safexcel_cipher.c
+++ b/drivers/crypto/inside-secure/safexcel_cipher.c
@@ -73,7 +73,6 @@ static void safexcel_skcipher_token(struct safexcel_cipher_ctx *ctx, u8 *iv,
 			memcpy(cdesc->control_data.token, iv, DES3_EDE_BLOCK_SIZE);
 			cdesc->control_data.options |= EIP197_OPTION_2_TOKEN_IV_CMD;
 			break;
-
 		case SAFEXCEL_AES:
 			offset = AES_BLOCK_SIZE / sizeof(u32);
 			memcpy(cdesc->control_data.token, iv, AES_BLOCK_SIZE);
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 02/14] crypto: inside-secure - move comment
  2019-05-27 14:50 [PATCH 00/14] crypto: inside-secure - self-test fixes Antoine Tenart
  2019-05-27 14:50 ` [PATCH 01/14] crypto: inside-secure - remove empty line Antoine Tenart
@ 2019-05-27 14:50 ` Antoine Tenart
  2019-05-27 14:50 ` [PATCH 03/14] crypto: inside-secure - fix coding style for a condition Antoine Tenart
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Antoine Tenart @ 2019-05-27 14:50 UTC (permalink / raw)
  To: herbert, davem
  Cc: Antoine Tenart, linux-crypto, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh, igall

This cosmetic patch moves a comment before the condition it is related
to. The patch does not change the driver behaviour in any way.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/crypto/inside-secure/safexcel_hash.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel_hash.c b/drivers/crypto/inside-secure/safexcel_hash.c
index ac9282c1a5ec..a7cee9ed3789 100644
--- a/drivers/crypto/inside-secure/safexcel_hash.c
+++ b/drivers/crypto/inside-secure/safexcel_hash.c
@@ -223,10 +223,11 @@ static int safexcel_ahash_send_req(struct crypto_async_request *async, int ring,
 		 * fit into full blocks, cache it for the next send() call.
 		 */
 		extra = queued & (crypto_ahash_blocksize(ahash) - 1);
+
+		/* If this is not the last request and the queued data
+		 * is a multiple of a block, cache the last one for now.
+		 */
 		if (!extra)
-			/* If this is not the last request and the queued data
-			 * is a multiple of a block, cache the last one for now.
-			 */
 			extra = crypto_ahash_blocksize(ahash);
 
 		if (extra) {
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 03/14] crypto: inside-secure - fix coding style for a condition
  2019-05-27 14:50 [PATCH 00/14] crypto: inside-secure - self-test fixes Antoine Tenart
  2019-05-27 14:50 ` [PATCH 01/14] crypto: inside-secure - remove empty line Antoine Tenart
  2019-05-27 14:50 ` [PATCH 02/14] crypto: inside-secure - move comment Antoine Tenart
@ 2019-05-27 14:50 ` Antoine Tenart
  2019-05-27 14:50 ` [PATCH 04/14] crypto: inside-secure - remove useless check Antoine Tenart
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Antoine Tenart @ 2019-05-27 14:50 UTC (permalink / raw)
  To: herbert, davem
  Cc: Antoine Tenart, linux-crypto, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh, igall

This cosmetic patch fixes a cosmetic issue with if brackets.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/crypto/inside-secure/safexcel.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index 86c699c14f84..263bd4ce73c5 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -720,11 +720,10 @@ static inline void safexcel_handle_result_descriptor(struct safexcel_crypto_priv
 	}
 
 acknowledge:
-	if (i) {
+	if (i)
 		writel(EIP197_xDR_PROC_xD_PKT(i) |
 		       EIP197_xDR_PROC_xD_COUNT(tot_descs * priv->config.rd_offset),
 		       EIP197_HIA_RDR(priv, ring) + EIP197_HIA_xDR_PROC_COUNT);
-	}
 
 	/* If the number of requests overflowed the counter, try to proceed more
 	 * requests.
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 04/14] crypto: inside-secure - remove useless check
  2019-05-27 14:50 [PATCH 00/14] crypto: inside-secure - self-test fixes Antoine Tenart
                   ` (2 preceding siblings ...)
  2019-05-27 14:50 ` [PATCH 03/14] crypto: inside-secure - fix coding style for a condition Antoine Tenart
@ 2019-05-27 14:50 ` Antoine Tenart
  2019-05-27 14:50 ` [PATCH 05/14] crypto: inside-secure - improve the result error format when displayed Antoine Tenart
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Antoine Tenart @ 2019-05-27 14:50 UTC (permalink / raw)
  To: herbert, davem
  Cc: Antoine Tenart, linux-crypto, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh, igall

When sending an ahash request, the code checks for the extra variable
not to be 0. This check is useless as the extra variable can't be 0 at
this point (it is checked on the line just before).

This patch does not modify the driver behaviour in any way.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/crypto/inside-secure/safexcel_hash.c | 24 +++++++++-----------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel_hash.c b/drivers/crypto/inside-secure/safexcel_hash.c
index a7cee9ed3789..58ce480690eb 100644
--- a/drivers/crypto/inside-secure/safexcel_hash.c
+++ b/drivers/crypto/inside-secure/safexcel_hash.c
@@ -230,19 +230,17 @@ static int safexcel_ahash_send_req(struct crypto_async_request *async, int ring,
 		if (!extra)
 			extra = crypto_ahash_blocksize(ahash);
 
-		if (extra) {
-			sg_pcopy_to_buffer(areq->src, sg_nents(areq->src),
-					   req->cache_next, extra,
-					   areq->nbytes - extra);
-
-			queued -= extra;
-			len -= extra;
-
-			if (!queued) {
-				*commands = 0;
-				*results = 0;
-				return 0;
-			}
+		sg_pcopy_to_buffer(areq->src, sg_nents(areq->src),
+				   req->cache_next, extra,
+				   areq->nbytes - extra);
+
+		queued -= extra;
+		len -= extra;
+
+		if (!queued) {
+			*commands = 0;
+			*results = 0;
+			return 0;
 		}
 	}
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 05/14] crypto: inside-secure - improve the result error format when displayed
  2019-05-27 14:50 [PATCH 00/14] crypto: inside-secure - self-test fixes Antoine Tenart
                   ` (3 preceding siblings ...)
  2019-05-27 14:50 ` [PATCH 04/14] crypto: inside-secure - remove useless check Antoine Tenart
@ 2019-05-27 14:50 ` Antoine Tenart
  2019-05-27 14:50 ` [PATCH 06/14] crypto: inside-secure - change returned error when a descriptor reports an error Antoine Tenart
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Antoine Tenart @ 2019-05-27 14:50 UTC (permalink / raw)
  To: herbert, davem
  Cc: Antoine Tenart, linux-crypto, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh, igall

The result descriptors contain errors, which are represented as a
bitmap. This patch updates the error message to not treat the error as a
decimal value, but as an hexadecimal one. This helps in knowing the
value does not have a direct meaning (the set bits themselves have).

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/crypto/inside-secure/safexcel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index 263bd4ce73c5..d5392893973c 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -589,7 +589,7 @@ inline int safexcel_rdesc_check_errors(struct safexcel_crypto_priv *priv,
 	if (rdesc->result_data.error_code & 0x407f) {
 		/* Fatal error (bits 0-7, 14) */
 		dev_err(priv->dev,
-			"cipher: result: result descriptor error (%d)\n",
+			"cipher: result: result descriptor error (0x%x)\n",
 			rdesc->result_data.error_code);
 		return -EIO;
 	} else if (rdesc->result_data.error_code == BIT(9)) {
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 06/14] crypto: inside-secure - change returned error when a descriptor reports an error
  2019-05-27 14:50 [PATCH 00/14] crypto: inside-secure - self-test fixes Antoine Tenart
                   ` (4 preceding siblings ...)
  2019-05-27 14:50 ` [PATCH 05/14] crypto: inside-secure - improve the result error format when displayed Antoine Tenart
@ 2019-05-27 14:50 ` Antoine Tenart
  2019-05-27 14:50 ` [PATCH 07/14] crypto: inside-secure - enable context reuse Antoine Tenart
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Antoine Tenart @ 2019-05-27 14:50 UTC (permalink / raw)
  To: herbert, davem
  Cc: Antoine Tenart, linux-crypto, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh, igall

This patch changes the error reported by the Inside Secure SafeXcel
driver when a result descriptor reports an error, from -EIO to -EINVAL,
as this is what the crypto framework expects. This was found while
running the crypto extra tests.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/crypto/inside-secure/safexcel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index d5392893973c..316e5e4c1c74 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -591,7 +591,7 @@ inline int safexcel_rdesc_check_errors(struct safexcel_crypto_priv *priv,
 		dev_err(priv->dev,
 			"cipher: result: result descriptor error (0x%x)\n",
 			rdesc->result_data.error_code);
-		return -EIO;
+		return -EINVAL;
 	} else if (rdesc->result_data.error_code == BIT(9)) {
 		/* Authentication failed */
 		return -EBADMSG;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 07/14] crypto: inside-secure - enable context reuse
  2019-05-27 14:50 [PATCH 00/14] crypto: inside-secure - self-test fixes Antoine Tenart
                   ` (5 preceding siblings ...)
  2019-05-27 14:50 ` [PATCH 06/14] crypto: inside-secure - change returned error when a descriptor reports an error Antoine Tenart
@ 2019-05-27 14:50 ` Antoine Tenart
  2019-05-27 14:51 ` [PATCH 08/14] crypto: inside-secure - unify cache reset Antoine Tenart
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Antoine Tenart @ 2019-05-27 14:50 UTC (permalink / raw)
  To: herbert, davem
  Cc: Antoine Tenart, linux-crypto, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh, igall

The context given to the crypto engine can be reused over time. While
the driver was designed to allow this, the feature wasn't enabled in the
hardware engine. This patch enables it.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/crypto/inside-secure/safexcel.c      | 6 ++++++
 drivers/crypto/inside-secure/safexcel.h      | 7 +++++++
 drivers/crypto/inside-secure/safexcel_ring.c | 3 +++
 3 files changed, 16 insertions(+)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index 316e5e4c1c74..df43a2c6933b 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -398,6 +398,12 @@ static int safexcel_hw_init(struct safexcel_crypto_priv *priv)
 
 		/* Processing Engine configuration */
 
+		/* Token & context configuration */
+		val = EIP197_PE_EIP96_TOKEN_CTRL_CTX_UPDATES |
+		      EIP197_PE_EIP96_TOKEN_CTRL_REUSE_CTX |
+		      EIP197_PE_EIP96_TOKEN_CTRL_POST_REUSE_CTX;
+		writel(val, EIP197_PE(priv) + EIP197_PE_EIP96_TOKEN_CTRL(pe));
+
 		/* H/W capabilities selection */
 		val = EIP197_FUNCTION_RSVD;
 		val |= EIP197_PROTOCOL_ENCRYPT_ONLY | EIP197_PROTOCOL_HASH_ONLY;
diff --git a/drivers/crypto/inside-secure/safexcel.h b/drivers/crypto/inside-secure/safexcel.h
index 65624a81f0fd..0bc8859f9d06 100644
--- a/drivers/crypto/inside-secure/safexcel.h
+++ b/drivers/crypto/inside-secure/safexcel.h
@@ -118,6 +118,7 @@
 #define EIP197_PE_ICE_SCRATCH_CTRL(n)		(0x0d04 + (0x2000 * (n)))
 #define EIP197_PE_ICE_FPP_CTRL(n)		(0x0d80 + (0x2000 * (n)))
 #define EIP197_PE_ICE_RAM_CTRL(n)		(0x0ff0 + (0x2000 * (n)))
+#define EIP197_PE_EIP96_TOKEN_CTRL(n)		(0x1000 + (0x2000 * (n)))
 #define EIP197_PE_EIP96_FUNCTION_EN(n)		(0x1004 + (0x2000 * (n)))
 #define EIP197_PE_EIP96_CONTEXT_CTRL(n)		(0x1008 + (0x2000 * (n)))
 #define EIP197_PE_EIP96_CONTEXT_STAT(n)		(0x100c + (0x2000 * (n)))
@@ -249,6 +250,11 @@
 #define EIP197_PE_ICE_RAM_CTRL_PUE_PROG_EN	BIT(0)
 #define EIP197_PE_ICE_RAM_CTRL_FPP_PROG_EN	BIT(1)
 
+/* EIP197_PE_EIP96_TOKEN_CTRL */
+#define EIP197_PE_EIP96_TOKEN_CTRL_CTX_UPDATES		BIT(16)
+#define EIP197_PE_EIP96_TOKEN_CTRL_REUSE_CTX		BIT(19)
+#define EIP197_PE_EIP96_TOKEN_CTRL_POST_REUSE_CTX	BIT(20)
+
 /* EIP197_PE_EIP96_FUNCTION_EN */
 #define EIP197_FUNCTION_RSVD			(BIT(6) | BIT(15) | BIT(20) | BIT(23))
 #define EIP197_PROTOCOL_HASH_ONLY		BIT(0)
@@ -468,6 +474,7 @@ struct safexcel_control_data_desc {
 
 #define EIP197_OPTION_MAGIC_VALUE	BIT(0)
 #define EIP197_OPTION_64BIT_CTX		BIT(1)
+#define EIP197_OPTION_RC_AUTO		(0x2 << 3)
 #define EIP197_OPTION_CTX_CTRL_IN_CMD	BIT(8)
 #define EIP197_OPTION_2_TOKEN_IV_CMD	GENMASK(11, 10)
 #define EIP197_OPTION_4_TOKEN_IV_CMD	GENMASK(11, 9)
diff --git a/drivers/crypto/inside-secure/safexcel_ring.c b/drivers/crypto/inside-secure/safexcel_ring.c
index eb75fa684876..142bc3f5c45c 100644
--- a/drivers/crypto/inside-secure/safexcel_ring.c
+++ b/drivers/crypto/inside-secure/safexcel_ring.c
@@ -145,6 +145,9 @@ struct safexcel_command_desc *safexcel_add_cdesc(struct safexcel_crypto_priv *pr
 			(lower_32_bits(context) & GENMASK(31, 2)) >> 2;
 		cdesc->control_data.context_hi = upper_32_bits(context);
 
+		if (priv->version == EIP197B || priv->version == EIP197D)
+			cdesc->control_data.options |= EIP197_OPTION_RC_AUTO;
+
 		/* TODO: large xform HMAC with SHA-384/512 uses refresh = 3 */
 		cdesc->control_data.refresh = 2;
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 08/14] crypto: inside-secure - unify cache reset
  2019-05-27 14:50 [PATCH 00/14] crypto: inside-secure - self-test fixes Antoine Tenart
                   ` (6 preceding siblings ...)
  2019-05-27 14:50 ` [PATCH 07/14] crypto: inside-secure - enable context reuse Antoine Tenart
@ 2019-05-27 14:51 ` Antoine Tenart
  2019-05-27 14:51 ` [PATCH 09/14] crypto: inside-secure - fix zeroing of the request in ahash_exit_inv Antoine Tenart
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Antoine Tenart @ 2019-05-27 14:51 UTC (permalink / raw)
  To: herbert, davem
  Cc: Antoine Tenart, linux-crypto, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh, igall

This patch unify the way the cache related data is zeroed when the cache
buffer is DMA unmapped. It should not change the driver behaviour, but
improves the code safety and readability.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/crypto/inside-secure/safexcel_hash.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/crypto/inside-secure/safexcel_hash.c b/drivers/crypto/inside-secure/safexcel_hash.c
index 58ce480690eb..a79a73bb3969 100644
--- a/drivers/crypto/inside-secure/safexcel_hash.c
+++ b/drivers/crypto/inside-secure/safexcel_hash.c
@@ -183,6 +183,7 @@ static int safexcel_handle_req_result(struct safexcel_crypto_priv *priv, int rin
 		dma_unmap_single(priv->dev, sreq->cache_dma, sreq->cache_sz,
 				 DMA_TO_DEVICE);
 		sreq->cache_dma = 0;
+		sreq->cache_sz = 0;
 	}
 
 	if (sreq->finish)
@@ -344,6 +345,7 @@ static int safexcel_ahash_send_req(struct crypto_async_request *async, int ring,
 	if (req->cache_dma) {
 		dma_unmap_single(priv->dev, req->cache_dma, req->cache_sz,
 				 DMA_TO_DEVICE);
+		req->cache_dma = 0;
 		req->cache_sz = 0;
 	}
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 09/14] crypto: inside-secure - fix zeroing of the request in ahash_exit_inv
  2019-05-27 14:50 [PATCH 00/14] crypto: inside-secure - self-test fixes Antoine Tenart
                   ` (7 preceding siblings ...)
  2019-05-27 14:51 ` [PATCH 08/14] crypto: inside-secure - unify cache reset Antoine Tenart
@ 2019-05-27 14:51 ` Antoine Tenart
  2019-05-27 14:51 ` [PATCH 10/14] crypto: inside-secure - fix queued len computation Antoine Tenart
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Antoine Tenart @ 2019-05-27 14:51 UTC (permalink / raw)
  To: herbert, davem
  Cc: Antoine Tenart, linux-crypto, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh, igall

A request is zeroed in safexcel_ahash_exit_inv(). This request total
size is EIP197_AHASH_REQ_SIZE while the memset zeroing it uses
sizeof(struct ahash_request), which happens to be less than
EIP197_AHASH_REQ_SIZE. This patch fixes it.

Fixes: f6beaea30487 ("crypto: inside-secure - authenc(hmac(sha256), cbc(aes)) support")
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/crypto/inside-secure/safexcel_hash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/inside-secure/safexcel_hash.c b/drivers/crypto/inside-secure/safexcel_hash.c
index a79a73bb3969..ba0732fd4ed4 100644
--- a/drivers/crypto/inside-secure/safexcel_hash.c
+++ b/drivers/crypto/inside-secure/safexcel_hash.c
@@ -487,7 +487,7 @@ static int safexcel_ahash_exit_inv(struct crypto_tfm *tfm)
 	struct safexcel_inv_result result = {};
 	int ring = ctx->base.ring;
 
-	memset(req, 0, sizeof(struct ahash_request));
+	memset(req, 0, EIP197_AHASH_REQ_SIZE);
 
 	/* create invalidation request */
 	init_completion(&result.completion);
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 10/14] crypto: inside-secure - fix queued len computation
  2019-05-27 14:50 [PATCH 00/14] crypto: inside-secure - self-test fixes Antoine Tenart
                   ` (8 preceding siblings ...)
  2019-05-27 14:51 ` [PATCH 09/14] crypto: inside-secure - fix zeroing of the request in ahash_exit_inv Antoine Tenart
@ 2019-05-27 14:51 ` Antoine Tenart
  2019-05-27 14:51 ` [PATCH 11/14] crypto: inside-secure - implement IV retrieval Antoine Tenart
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Antoine Tenart @ 2019-05-27 14:51 UTC (permalink / raw)
  To: herbert, davem
  Cc: Antoine Tenart, linux-crypto, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh, igall

This patch fixes the queued len computation, which could theoretically
be wrong if req->len[1] - req->processed[1] > 1. Be future-proof here,
and fix it.

Fixes: b460edb6230a ("crypto: inside-secure - sha512 support")
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/crypto/inside-secure/safexcel_hash.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel_hash.c b/drivers/crypto/inside-secure/safexcel_hash.c
index ba0732fd4ed4..a9197d2c5a48 100644
--- a/drivers/crypto/inside-secure/safexcel_hash.c
+++ b/drivers/crypto/inside-secure/safexcel_hash.c
@@ -50,10 +50,12 @@ struct safexcel_ahash_req {
 
 static inline u64 safexcel_queued_len(struct safexcel_ahash_req *req)
 {
-	if (req->len[1] > req->processed[1])
-		return 0xffffffff - (req->len[0] - req->processed[0]);
+	u64 len, processed;
 
-	return req->len[0] - req->processed[0];
+	len = (0xffffffff * req->len[1]) + req->len[0];
+	processed = (0xffffffff * req->processed[1]) + req->processed[0];
+
+	return len - processed;
 }
 
 static void safexcel_hash_token(struct safexcel_command_desc *cdesc,
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 11/14] crypto: inside-secure - implement IV retrieval
  2019-05-27 14:50 [PATCH 00/14] crypto: inside-secure - self-test fixes Antoine Tenart
                   ` (9 preceding siblings ...)
  2019-05-27 14:51 ` [PATCH 10/14] crypto: inside-secure - fix queued len computation Antoine Tenart
@ 2019-05-27 14:51 ` Antoine Tenart
  2019-05-27 14:51 ` [PATCH 12/14] crypto: inside-secure - add support for HMAC updates Antoine Tenart
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Antoine Tenart @ 2019-05-27 14:51 UTC (permalink / raw)
  To: herbert, davem
  Cc: Antoine Tenart, linux-crypto, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh, igall

This patch adds support for retrieving intermediate IV from the crypto
engine when using the CBC block mode with AES and (3)DES. The retrieved
IV is copied to the request IV buffer, as requested by the kernel crypto
API.

This fix boot tests added by
commit 8efd972ef96a ("crypto: testmgr - support checking skcipher output IV").

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/crypto/inside-secure/safexcel.h       |  8 +++
 .../crypto/inside-secure/safexcel_cipher.c    | 52 ++++++++++++++++---
 2 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel.h b/drivers/crypto/inside-secure/safexcel.h
index 0bc8859f9d06..ca6ece5607cd 100644
--- a/drivers/crypto/inside-secure/safexcel.h
+++ b/drivers/crypto/inside-secure/safexcel.h
@@ -339,6 +339,7 @@ struct safexcel_context_record {
 #define CONTEXT_CONTROL_IV3			BIT(8)
 #define CONTEXT_CONTROL_DIGEST_CNT		BIT(9)
 #define CONTEXT_CONTROL_COUNTER_MODE		BIT(10)
+#define CONTEXT_CONTROL_CRYPTO_STORE		BIT(12)
 #define CONTEXT_CONTROL_HASH_STORE		BIT(19)
 
 /* The hash counter given to the engine in the context has a granularity of
@@ -431,6 +432,10 @@ struct safexcel_token {
 
 #define EIP197_TOKEN_HASH_RESULT_VERIFY		BIT(16)
 
+#define EIP197_TOKEN_CTX_OFFSET(x)		(x)
+#define EIP197_TOKEN_DIRECTION_EXTERNAL		BIT(11)
+#define EIP197_TOKEN_EXEC_IF_SUCCESSFUL		(0x1 << 12)
+
 #define EIP197_TOKEN_STAT_LAST_HASH		BIT(0)
 #define EIP197_TOKEN_STAT_LAST_PACKET		BIT(1)
 #define EIP197_TOKEN_OPCODE_DIRECTION		0x0
@@ -438,6 +443,7 @@ struct safexcel_token {
 #define EIP197_TOKEN_OPCODE_NOOP		EIP197_TOKEN_OPCODE_INSERT
 #define EIP197_TOKEN_OPCODE_RETRIEVE		0x4
 #define EIP197_TOKEN_OPCODE_VERIFY		0xd
+#define EIP197_TOKEN_OPCODE_CTX_ACCESS		0xe
 #define EIP197_TOKEN_OPCODE_BYPASS		GENMASK(3, 0)
 
 static inline void eip197_noop_token(struct safexcel_token *token)
@@ -448,6 +454,8 @@ static inline void eip197_noop_token(struct safexcel_token *token)
 
 /* Instructions */
 #define EIP197_TOKEN_INS_INSERT_HASH_DIGEST	0x1c
+#define EIP197_TOKEN_INS_ORIGIN_IV0		0x14
+#define EIP197_TOKEN_INS_ORIGIN_LEN(x)		((x) << 5)
 #define EIP197_TOKEN_INS_TYPE_OUTPUT		BIT(5)
 #define EIP197_TOKEN_INS_TYPE_HASH		BIT(6)
 #define EIP197_TOKEN_INS_TYPE_CRYTO		BIT(7)
diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
index aca1cdf33362..cedfb121c278 100644
--- a/drivers/crypto/inside-secure/safexcel_cipher.c
+++ b/drivers/crypto/inside-secure/safexcel_cipher.c
@@ -59,26 +59,26 @@ static void safexcel_skcipher_token(struct safexcel_cipher_ctx *ctx, u8 *iv,
 				    u32 length)
 {
 	struct safexcel_token *token;
-	unsigned offset = 0;
+	u32 offset = 0, block_sz = 0;
 
 	if (ctx->mode == CONTEXT_CONTROL_CRYPTO_MODE_CBC) {
 		switch (ctx->alg) {
 		case SAFEXCEL_DES:
-			offset = DES_BLOCK_SIZE / sizeof(u32);
-			memcpy(cdesc->control_data.token, iv, DES_BLOCK_SIZE);
+			block_sz = DES_BLOCK_SIZE;
 			cdesc->control_data.options |= EIP197_OPTION_2_TOKEN_IV_CMD;
 			break;
 		case SAFEXCEL_3DES:
-			offset = DES3_EDE_BLOCK_SIZE / sizeof(u32);
-			memcpy(cdesc->control_data.token, iv, DES3_EDE_BLOCK_SIZE);
+			block_sz = DES3_EDE_BLOCK_SIZE;
 			cdesc->control_data.options |= EIP197_OPTION_2_TOKEN_IV_CMD;
 			break;
 		case SAFEXCEL_AES:
-			offset = AES_BLOCK_SIZE / sizeof(u32);
-			memcpy(cdesc->control_data.token, iv, AES_BLOCK_SIZE);
+			block_sz = AES_BLOCK_SIZE;
 			cdesc->control_data.options |= EIP197_OPTION_4_TOKEN_IV_CMD;
 			break;
 		}
+
+		offset = block_sz / sizeof(u32);
+		memcpy(cdesc->control_data.token, iv, block_sz);
 	}
 
 	token = (struct safexcel_token *)(cdesc->control_data.token + offset);
@@ -90,6 +90,25 @@ static void safexcel_skcipher_token(struct safexcel_cipher_ctx *ctx, u8 *iv,
 	token[0].instructions = EIP197_TOKEN_INS_LAST |
 				EIP197_TOKEN_INS_TYPE_CRYTO |
 				EIP197_TOKEN_INS_TYPE_OUTPUT;
+
+	if (ctx->mode == CONTEXT_CONTROL_CRYPTO_MODE_CBC) {
+		u32 last = (EIP197_MAX_TOKENS - 1) - offset;
+
+		token[last].opcode = EIP197_TOKEN_OPCODE_CTX_ACCESS;
+		token[last].packet_length = EIP197_TOKEN_DIRECTION_EXTERNAL |
+					    EIP197_TOKEN_EXEC_IF_SUCCESSFUL|
+					    EIP197_TOKEN_CTX_OFFSET(0x2);
+		token[last].stat = EIP197_TOKEN_STAT_LAST_HASH |
+			EIP197_TOKEN_STAT_LAST_PACKET;
+		token[last].instructions =
+			EIP197_TOKEN_INS_ORIGIN_LEN(block_sz / sizeof(u32)) |
+			EIP197_TOKEN_INS_ORIGIN_IV0;
+
+		/* Store the updated IV values back in the internal context
+		 * registers.
+		 */
+		cdesc->control_data.control1 |= CONTEXT_CONTROL_CRYPTO_STORE;
+	}
 }
 
 static void safexcel_aead_token(struct safexcel_cipher_ctx *ctx, u8 *iv,
@@ -559,6 +578,7 @@ static int safexcel_skcipher_handle_result(struct safexcel_crypto_priv *priv,
 {
 	struct skcipher_request *req = skcipher_request_cast(async);
 	struct safexcel_cipher_req *sreq = skcipher_request_ctx(req);
+	struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(async->tfm);
 	int err;
 
 	if (sreq->needs_inv) {
@@ -569,6 +589,24 @@ static int safexcel_skcipher_handle_result(struct safexcel_crypto_priv *priv,
 		err = safexcel_handle_req_result(priv, ring, async, req->src,
 						 req->dst, req->cryptlen, sreq,
 						 should_complete, ret);
+
+		if (ctx->mode == CONTEXT_CONTROL_CRYPTO_MODE_CBC) {
+			u32 block_sz = 0;
+
+			switch (ctx->alg) {
+			case SAFEXCEL_DES:
+				block_sz = DES_BLOCK_SIZE;
+				break;
+			case SAFEXCEL_3DES:
+				block_sz = DES3_EDE_BLOCK_SIZE;
+				break;
+			case SAFEXCEL_AES:
+				block_sz = AES_BLOCK_SIZE;
+				break;
+			}
+
+			memcpy(req->iv, ctx->base.ctxr->data, block_sz);
+		}
 	}
 
 	return err;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 12/14] crypto: inside-secure - add support for HMAC updates
  2019-05-27 14:50 [PATCH 00/14] crypto: inside-secure - self-test fixes Antoine Tenart
                   ` (10 preceding siblings ...)
  2019-05-27 14:51 ` [PATCH 11/14] crypto: inside-secure - implement IV retrieval Antoine Tenart
@ 2019-05-27 14:51 ` Antoine Tenart
  2019-05-27 14:51 ` [PATCH 13/14] crypto: inside-secure - fix use of the SG list Antoine Tenart
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Antoine Tenart @ 2019-05-27 14:51 UTC (permalink / raw)
  To: herbert, davem
  Cc: Antoine Tenart, linux-crypto, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh, igall

This patch adds support for HMAC updates in the Inside Secure SafeXcel
crypto engine driver. Updates were supported for hash algorithms, but
were never enabled for HMAC ones. This fixes boot time test issues.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/crypto/inside-secure/safexcel.h      |  2 +-
 drivers/crypto/inside-secure/safexcel_hash.c | 58 +++++++++++++-------
 2 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel.h b/drivers/crypto/inside-secure/safexcel.h
index ca6ece5607cd..e0c202f33674 100644
--- a/drivers/crypto/inside-secure/safexcel.h
+++ b/drivers/crypto/inside-secure/safexcel.h
@@ -644,7 +644,7 @@ struct safexcel_ahash_export_state {
 	u32 digest;
 
 	u32 state[SHA512_DIGEST_SIZE / sizeof(u32)];
-	u8 cache[SHA512_BLOCK_SIZE];
+	u8 cache[SHA512_BLOCK_SIZE << 1];
 };
 
 /*
diff --git a/drivers/crypto/inside-secure/safexcel_hash.c b/drivers/crypto/inside-secure/safexcel_hash.c
index a9197d2c5a48..20950744ea4e 100644
--- a/drivers/crypto/inside-secure/safexcel_hash.c
+++ b/drivers/crypto/inside-secure/safexcel_hash.c
@@ -41,11 +41,11 @@ struct safexcel_ahash_req {
 	u64 len[2];
 	u64 processed[2];
 
-	u8 cache[SHA512_BLOCK_SIZE] __aligned(sizeof(u32));
+	u8 cache[SHA512_BLOCK_SIZE << 1] __aligned(sizeof(u32));
 	dma_addr_t cache_dma;
 	unsigned int cache_sz;
 
-	u8 cache_next[SHA512_BLOCK_SIZE] __aligned(sizeof(u32));
+	u8 cache_next[SHA512_BLOCK_SIZE << 1] __aligned(sizeof(u32));
 };
 
 static inline u64 safexcel_queued_len(struct safexcel_ahash_req *req)
@@ -89,6 +89,9 @@ static void safexcel_context_control(struct safexcel_ahash_ctx *ctx,
 	cdesc->control_data.control0 |= ctx->alg;
 	cdesc->control_data.control0 |= req->digest;
 
+	if (!req->finish)
+		cdesc->control_data.control0 |= CONTEXT_CONTROL_NO_FINISH_HASH;
+
 	if (req->digest == CONTEXT_CONTROL_DIGEST_PRECOMPUTED) {
 		if (req->processed[0] || req->processed[1]) {
 			if (ctx->alg == CONTEXT_CONTROL_CRYPTO_ALG_MD5)
@@ -107,9 +110,6 @@ static void safexcel_context_control(struct safexcel_ahash_ctx *ctx,
 			cdesc->control_data.control0 |= CONTEXT_CONTROL_RESTART_HASH;
 		}
 
-		if (!req->finish)
-			cdesc->control_data.control0 |= CONTEXT_CONTROL_NO_FINISH_HASH;
-
 		/*
 		 * Copy the input digest if needed, and setup the context
 		 * fields. Do this now as we need it to setup the first command
@@ -212,11 +212,15 @@ static int safexcel_ahash_send_req(struct crypto_async_request *async, int ring,
 	struct safexcel_command_desc *cdesc, *first_cdesc = NULL;
 	struct safexcel_result_desc *rdesc;
 	struct scatterlist *sg;
-	int i, extra, n_cdesc = 0, ret = 0;
-	u64 queued, len, cache_len;
+	int i, extra = 0, n_cdesc = 0, ret = 0;
+	u64 queued, len, cache_len, cache_max;
+
+	cache_max = crypto_ahash_blocksize(ahash);
+	if (req->digest == CONTEXT_CONTROL_DIGEST_HMAC)
+		cache_max <<= 1;
 
 	queued = len = safexcel_queued_len(req);
-	if (queued <= crypto_ahash_blocksize(ahash))
+	if (queued <= cache_max)
 		cache_len = queued;
 	else
 		cache_len = queued - areq->nbytes;
@@ -227,6 +231,10 @@ static int safexcel_ahash_send_req(struct crypto_async_request *async, int ring,
 		 */
 		extra = queued & (crypto_ahash_blocksize(ahash) - 1);
 
+		if (req->digest == CONTEXT_CONTROL_DIGEST_HMAC &&
+		    extra < crypto_ahash_blocksize(ahash))
+			extra += crypto_ahash_blocksize(ahash);
+
 		/* If this is not the last request and the queued data
 		 * is a multiple of a block, cache the last one for now.
 		 */
@@ -239,12 +247,6 @@ static int safexcel_ahash_send_req(struct crypto_async_request *async, int ring,
 
 		queued -= extra;
 		len -= extra;
-
-		if (!queued) {
-			*commands = 0;
-			*results = 0;
-			return 0;
-		}
 	}
 
 	/* Add a command descriptor for the cached data, if any */
@@ -522,10 +524,9 @@ static int safexcel_ahash_exit_inv(struct crypto_tfm *tfm)
 /* safexcel_ahash_cache: cache data until at least one request can be sent to
  * the engine, aka. when there is at least 1 block size in the pipe.
  */
-static int safexcel_ahash_cache(struct ahash_request *areq)
+static int safexcel_ahash_cache(struct ahash_request *areq, u32 cache_max)
 {
 	struct safexcel_ahash_req *req = ahash_request_ctx(areq);
-	struct crypto_ahash *ahash = crypto_ahash_reqtfm(areq);
 	u64 queued, cache_len;
 
 	/* queued: everything accepted by the driver which will be handled by
@@ -542,7 +543,7 @@ static int safexcel_ahash_cache(struct ahash_request *areq)
 	 * In case there isn't enough bytes to proceed (less than a
 	 * block size), cache the data until we have enough.
 	 */
-	if (cache_len + areq->nbytes <= crypto_ahash_blocksize(ahash)) {
+	if (cache_len + areq->nbytes <= cache_max) {
 		sg_pcopy_to_buffer(areq->src, sg_nents(areq->src),
 				   req->cache + cache_len,
 				   areq->nbytes, 0);
@@ -602,6 +603,7 @@ static int safexcel_ahash_update(struct ahash_request *areq)
 {
 	struct safexcel_ahash_req *req = ahash_request_ctx(areq);
 	struct crypto_ahash *ahash = crypto_ahash_reqtfm(areq);
+	u32 cache_max;
 
 	/* If the request is 0 length, do nothing */
 	if (!areq->nbytes)
@@ -611,7 +613,11 @@ static int safexcel_ahash_update(struct ahash_request *areq)
 	if (req->len[0] < areq->nbytes)
 		req->len[1]++;
 
-	safexcel_ahash_cache(areq);
+	cache_max = crypto_ahash_blocksize(ahash);
+	if (req->digest == CONTEXT_CONTROL_DIGEST_HMAC)
+		cache_max <<= 1;
+
+	safexcel_ahash_cache(areq, cache_max);
 
 	/*
 	 * We're not doing partial updates when performing an hmac request.
@@ -624,7 +630,7 @@ static int safexcel_ahash_update(struct ahash_request *areq)
 		return safexcel_ahash_enqueue(areq);
 
 	if (!req->last_req &&
-	    safexcel_queued_len(req) > crypto_ahash_blocksize(ahash))
+	    safexcel_queued_len(req) > cache_max)
 		return safexcel_ahash_enqueue(areq);
 
 	return 0;
@@ -681,6 +687,11 @@ static int safexcel_ahash_export(struct ahash_request *areq, void *out)
 	struct crypto_ahash *ahash = crypto_ahash_reqtfm(areq);
 	struct safexcel_ahash_req *req = ahash_request_ctx(areq);
 	struct safexcel_ahash_export_state *export = out;
+	u32 cache_sz;
+
+	cache_sz = crypto_ahash_blocksize(ahash);
+	if (req->digest == CONTEXT_CONTROL_DIGEST_HMAC)
+		cache_sz <<= 1;
 
 	export->len[0] = req->len[0];
 	export->len[1] = req->len[1];
@@ -690,7 +701,7 @@ static int safexcel_ahash_export(struct ahash_request *areq, void *out)
 	export->digest = req->digest;
 
 	memcpy(export->state, req->state, req->state_sz);
-	memcpy(export->cache, req->cache, crypto_ahash_blocksize(ahash));
+	memcpy(export->cache, req->cache, cache_sz);
 
 	return 0;
 }
@@ -700,12 +711,17 @@ static int safexcel_ahash_import(struct ahash_request *areq, const void *in)
 	struct crypto_ahash *ahash = crypto_ahash_reqtfm(areq);
 	struct safexcel_ahash_req *req = ahash_request_ctx(areq);
 	const struct safexcel_ahash_export_state *export = in;
+	u32 cache_sz;
 	int ret;
 
 	ret = crypto_ahash_init(areq);
 	if (ret)
 		return ret;
 
+	cache_sz = crypto_ahash_blocksize(ahash);
+	if (req->digest == CONTEXT_CONTROL_DIGEST_HMAC)
+		cache_sz <<= 1;
+
 	req->len[0] = export->len[0];
 	req->len[1] = export->len[1];
 	req->processed[0] = export->processed[0];
@@ -713,7 +729,7 @@ static int safexcel_ahash_import(struct ahash_request *areq, const void *in)
 
 	req->digest = export->digest;
 
-	memcpy(req->cache, export->cache, crypto_ahash_blocksize(ahash));
+	memcpy(req->cache, export->cache, cache_sz);
 	memcpy(req->state, export->state, req->state_sz);
 
 	return 0;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 13/14] crypto: inside-secure - fix use of the SG list
  2019-05-27 14:50 [PATCH 00/14] crypto: inside-secure - self-test fixes Antoine Tenart
                   ` (11 preceding siblings ...)
  2019-05-27 14:51 ` [PATCH 12/14] crypto: inside-secure - add support for HMAC updates Antoine Tenart
@ 2019-05-27 14:51 ` Antoine Tenart
  2019-05-27 14:51 ` [PATCH 14/14] crypto: inside-secure - do not rely on the hardware last bit for result descriptors Antoine Tenart
  2019-06-06  6:50 ` [PATCH 00/14] crypto: inside-secure - self-test fixes Herbert Xu
  14 siblings, 0 replies; 16+ messages in thread
From: Antoine Tenart @ 2019-05-27 14:51 UTC (permalink / raw)
  To: herbert, davem
  Cc: Antoine Tenart, linux-crypto, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh, igall

Replace sg_nents_for_len by sg_nents when DMA mapping/unmapping buffers
and when looping over the SG entries. This fix cases where the SG
entries aren't used fully, which would in such cases led to using fewer
SG entries than needed (and thus the engine wouldn't have access to the
full input data and the result would be wrong).

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 .../crypto/inside-secure/safexcel_cipher.c    | 39 ++++++-------------
 drivers/crypto/inside-secure/safexcel_hash.c  |  3 +-
 2 files changed, 12 insertions(+), 30 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
index cedfb121c278..6e193baccec7 100644
--- a/drivers/crypto/inside-secure/safexcel_cipher.c
+++ b/drivers/crypto/inside-secure/safexcel_cipher.c
@@ -369,16 +369,10 @@ static int safexcel_handle_req_result(struct safexcel_crypto_priv *priv, int rin
 	safexcel_complete(priv, ring);
 
 	if (src == dst) {
-		dma_unmap_sg(priv->dev, src,
-			     sg_nents_for_len(src, cryptlen),
-			     DMA_BIDIRECTIONAL);
+		dma_unmap_sg(priv->dev, src, sg_nents(src), DMA_BIDIRECTIONAL);
 	} else {
-		dma_unmap_sg(priv->dev, src,
-			     sg_nents_for_len(src, cryptlen),
-			     DMA_TO_DEVICE);
-		dma_unmap_sg(priv->dev, dst,
-			     sg_nents_for_len(dst, cryptlen),
-			     DMA_FROM_DEVICE);
+		dma_unmap_sg(priv->dev, src, sg_nents(src), DMA_TO_DEVICE);
+		dma_unmap_sg(priv->dev, dst, sg_nents(dst), DMA_FROM_DEVICE);
 	}
 
 	*should_complete = true;
@@ -403,26 +397,21 @@ static int safexcel_send_req(struct crypto_async_request *base, int ring,
 	int i, ret = 0;
 
 	if (src == dst) {
-		nr_src = dma_map_sg(priv->dev, src,
-				    sg_nents_for_len(src, totlen),
+		nr_src = dma_map_sg(priv->dev, src, sg_nents(src),
 				    DMA_BIDIRECTIONAL);
 		nr_dst = nr_src;
 		if (!nr_src)
 			return -EINVAL;
 	} else {
-		nr_src = dma_map_sg(priv->dev, src,
-				    sg_nents_for_len(src, totlen),
+		nr_src = dma_map_sg(priv->dev, src, sg_nents(src),
 				    DMA_TO_DEVICE);
 		if (!nr_src)
 			return -EINVAL;
 
-		nr_dst = dma_map_sg(priv->dev, dst,
-				    sg_nents_for_len(dst, totlen),
+		nr_dst = dma_map_sg(priv->dev, dst, sg_nents(dst),
 				    DMA_FROM_DEVICE);
 		if (!nr_dst) {
-			dma_unmap_sg(priv->dev, src,
-				     sg_nents_for_len(src, totlen),
-				     DMA_TO_DEVICE);
+			dma_unmap_sg(priv->dev, src, nr_src, DMA_TO_DEVICE);
 			return -EINVAL;
 		}
 	}
@@ -472,7 +461,7 @@ static int safexcel_send_req(struct crypto_async_request *base, int ring,
 
 	/* result descriptors */
 	for_each_sg(dst, sg, nr_dst, i) {
-		bool first = !i, last = (i == nr_dst - 1);
+		bool first = !i, last = sg_is_last(sg);
 		u32 len = sg_dma_len(sg);
 
 		rdesc = safexcel_add_rdesc(priv, ring, first, last,
@@ -501,16 +490,10 @@ static int safexcel_send_req(struct crypto_async_request *base, int ring,
 		safexcel_ring_rollback_wptr(priv, &priv->ring[ring].cdr);
 
 	if (src == dst) {
-		dma_unmap_sg(priv->dev, src,
-			     sg_nents_for_len(src, totlen),
-			     DMA_BIDIRECTIONAL);
+		dma_unmap_sg(priv->dev, src, nr_src, DMA_BIDIRECTIONAL);
 	} else {
-		dma_unmap_sg(priv->dev, src,
-			     sg_nents_for_len(src, totlen),
-			     DMA_TO_DEVICE);
-		dma_unmap_sg(priv->dev, dst,
-			     sg_nents_for_len(dst, totlen),
-			     DMA_FROM_DEVICE);
+		dma_unmap_sg(priv->dev, src, nr_src, DMA_TO_DEVICE);
+		dma_unmap_sg(priv->dev, dst, nr_dst, DMA_FROM_DEVICE);
 	}
 
 	return ret;
diff --git a/drivers/crypto/inside-secure/safexcel_hash.c b/drivers/crypto/inside-secure/safexcel_hash.c
index 20950744ea4e..a80a5e757b1f 100644
--- a/drivers/crypto/inside-secure/safexcel_hash.c
+++ b/drivers/crypto/inside-secure/safexcel_hash.c
@@ -273,8 +273,7 @@ static int safexcel_ahash_send_req(struct crypto_async_request *async, int ring,
 	}
 
 	/* Now handle the current ahash request buffer(s) */
-	req->nents = dma_map_sg(priv->dev, areq->src,
-				sg_nents_for_len(areq->src, areq->nbytes),
+	req->nents = dma_map_sg(priv->dev, areq->src, sg_nents(areq->src),
 				DMA_TO_DEVICE);
 	if (!req->nents) {
 		ret = -ENOMEM;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 14/14] crypto: inside-secure - do not rely on the hardware last bit for result descriptors
  2019-05-27 14:50 [PATCH 00/14] crypto: inside-secure - self-test fixes Antoine Tenart
                   ` (12 preceding siblings ...)
  2019-05-27 14:51 ` [PATCH 13/14] crypto: inside-secure - fix use of the SG list Antoine Tenart
@ 2019-05-27 14:51 ` Antoine Tenart
  2019-06-06  6:50 ` [PATCH 00/14] crypto: inside-secure - self-test fixes Herbert Xu
  14 siblings, 0 replies; 16+ messages in thread
From: Antoine Tenart @ 2019-05-27 14:51 UTC (permalink / raw)
  To: herbert, davem
  Cc: Antoine Tenart, linux-crypto, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh, igall

When performing a transformation the hardware is given result
descriptors to save the result data. Those result descriptors are
batched using a 'first' and a 'last' bit. There are cases were more
descriptors than needed are given to the engine, leading to the engine
only using some of them, and not setting the last bit on the last
descriptor we gave. This causes issues were the driver and the hardware
aren't in sync anymore about the number of result descriptors given (as
the driver do not give a pool of descriptor to use for any
transformation, but a pool of descriptors to use *per* transformation).

This patch fixes it by attaching the number of given result descriptors
to the requests, and by using this number instead of the 'last' bit
found on the descriptors to process them.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 .../crypto/inside-secure/safexcel_cipher.c    | 24 ++++++++++++++-----
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
index 6e193baccec7..8cdbdbe35681 100644
--- a/drivers/crypto/inside-secure/safexcel_cipher.c
+++ b/drivers/crypto/inside-secure/safexcel_cipher.c
@@ -51,6 +51,8 @@ struct safexcel_cipher_ctx {
 
 struct safexcel_cipher_req {
 	enum safexcel_cipher_direction direction;
+	/* Number of result descriptors associated to the request */
+	unsigned int rdescs;
 	bool needs_inv;
 };
 
@@ -351,7 +353,10 @@ static int safexcel_handle_req_result(struct safexcel_crypto_priv *priv, int rin
 
 	*ret = 0;
 
-	do {
+	if (unlikely(!sreq->rdescs))
+		return 0;
+
+	while (sreq->rdescs--) {
 		rdesc = safexcel_ring_next_rptr(priv, &priv->ring[ring].rdr);
 		if (IS_ERR(rdesc)) {
 			dev_err(priv->dev,
@@ -364,7 +369,7 @@ static int safexcel_handle_req_result(struct safexcel_crypto_priv *priv, int rin
 			*ret = safexcel_rdesc_check_errors(priv, rdesc);
 
 		ndesc++;
-	} while (!rdesc->last_seg);
+	}
 
 	safexcel_complete(priv, ring);
 
@@ -502,6 +507,7 @@ static int safexcel_send_req(struct crypto_async_request *base, int ring,
 static int safexcel_handle_inv_result(struct safexcel_crypto_priv *priv,
 				      int ring,
 				      struct crypto_async_request *base,
+				      struct safexcel_cipher_req *sreq,
 				      bool *should_complete, int *ret)
 {
 	struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(base->tfm);
@@ -510,7 +516,10 @@ static int safexcel_handle_inv_result(struct safexcel_crypto_priv *priv,
 
 	*ret = 0;
 
-	do {
+	if (unlikely(!sreq->rdescs))
+		return 0;
+
+	while (sreq->rdescs--) {
 		rdesc = safexcel_ring_next_rptr(priv, &priv->ring[ring].rdr);
 		if (IS_ERR(rdesc)) {
 			dev_err(priv->dev,
@@ -523,7 +532,7 @@ static int safexcel_handle_inv_result(struct safexcel_crypto_priv *priv,
 			*ret = safexcel_rdesc_check_errors(priv, rdesc);
 
 		ndesc++;
-	} while (!rdesc->last_seg);
+	}
 
 	safexcel_complete(priv, ring);
 
@@ -566,7 +575,7 @@ static int safexcel_skcipher_handle_result(struct safexcel_crypto_priv *priv,
 
 	if (sreq->needs_inv) {
 		sreq->needs_inv = false;
-		err = safexcel_handle_inv_result(priv, ring, async,
+		err = safexcel_handle_inv_result(priv, ring, async, sreq,
 						 should_complete, ret);
 	} else {
 		err = safexcel_handle_req_result(priv, ring, async, req->src,
@@ -607,7 +616,7 @@ static int safexcel_aead_handle_result(struct safexcel_crypto_priv *priv,
 
 	if (sreq->needs_inv) {
 		sreq->needs_inv = false;
-		err = safexcel_handle_inv_result(priv, ring, async,
+		err = safexcel_handle_inv_result(priv, ring, async, sreq,
 						 should_complete, ret);
 	} else {
 		err = safexcel_handle_req_result(priv, ring, async, req->src,
@@ -653,6 +662,8 @@ static int safexcel_skcipher_send(struct crypto_async_request *async, int ring,
 		ret = safexcel_send_req(async, ring, sreq, req->src,
 					req->dst, req->cryptlen, 0, 0, req->iv,
 					commands, results);
+
+	sreq->rdescs = *results;
 	return ret;
 }
 
@@ -675,6 +686,7 @@ static int safexcel_aead_send(struct crypto_async_request *async, int ring,
 					req->cryptlen, req->assoclen,
 					crypto_aead_authsize(tfm), req->iv,
 					commands, results);
+	sreq->rdescs = *results;
 	return ret;
 }
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 00/14] crypto: inside-secure - self-test fixes
  2019-05-27 14:50 [PATCH 00/14] crypto: inside-secure - self-test fixes Antoine Tenart
                   ` (13 preceding siblings ...)
  2019-05-27 14:51 ` [PATCH 14/14] crypto: inside-secure - do not rely on the hardware last bit for result descriptors Antoine Tenart
@ 2019-06-06  6:50 ` Herbert Xu
  14 siblings, 0 replies; 16+ messages in thread
From: Herbert Xu @ 2019-06-06  6:50 UTC (permalink / raw)
  To: Antoine Tenart
  Cc: davem, linux-crypto, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh, igall

On Mon, May 27, 2019 at 04:50:52PM +0200, Antoine Tenart wrote:
> Hello,
> 
> The crypto runtime self-tests were improved and thanks to this we
> spotted new issues within the Inside Secure SafeXcel cryptographic
> engine driver:
> - Intermediate IV were not retrieved from the hardware nor copied to the
>   quest IV buffer for cbc(aes/des).
> - HMAC updates wasn't supported.
> - We spotted issues with the use of the SG lists.
> - There was an issue with the use of result buffers.
> 
> The series fixes all those issues, and includes other small changes
> found while doing this work.
> 
> Thanks!
> Antoine
> 
> Antoine Tenart (14):
>   crypto: inside-secure - remove empty line
>   crypto: inside-secure - move comment
>   crypto: inside-secure - fix coding style for a condition
>   crypto: inside-secure - remove useless check
>   crypto: inside-secure - improve the result error format when displayed
>   crypto: inside-secure - change returned error when a descriptor
>     reports an error
>   crypto: inside-secure - enable context reuse
>   crypto: inside-secure - unify cache reset
>   crypto: inside-secure - fix zeroing of the request in ahash_exit_inv
>   crypto: inside-secure - fix queued len computation
>   crypto: inside-secure - implement IV retrieval
>   crypto: inside-secure - add support for HMAC updates
>   crypto: inside-secure - fix use of the SG list
>   crypto: inside-secure - do not rely on the hardware last bit for
>     result descriptors
> 
>  drivers/crypto/inside-secure/safexcel.c       |  13 +-
>  drivers/crypto/inside-secure/safexcel.h       |  17 ++-
>  .../crypto/inside-secure/safexcel_cipher.c    | 116 +++++++++++-------
>  drivers/crypto/inside-secure/safexcel_hash.c  |  92 ++++++++------
>  drivers/crypto/inside-secure/safexcel_ring.c  |   3 +
>  5 files changed, 157 insertions(+), 84 deletions(-)

All applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-06-06  6:50 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-27 14:50 [PATCH 00/14] crypto: inside-secure - self-test fixes Antoine Tenart
2019-05-27 14:50 ` [PATCH 01/14] crypto: inside-secure - remove empty line Antoine Tenart
2019-05-27 14:50 ` [PATCH 02/14] crypto: inside-secure - move comment Antoine Tenart
2019-05-27 14:50 ` [PATCH 03/14] crypto: inside-secure - fix coding style for a condition Antoine Tenart
2019-05-27 14:50 ` [PATCH 04/14] crypto: inside-secure - remove useless check Antoine Tenart
2019-05-27 14:50 ` [PATCH 05/14] crypto: inside-secure - improve the result error format when displayed Antoine Tenart
2019-05-27 14:50 ` [PATCH 06/14] crypto: inside-secure - change returned error when a descriptor reports an error Antoine Tenart
2019-05-27 14:50 ` [PATCH 07/14] crypto: inside-secure - enable context reuse Antoine Tenart
2019-05-27 14:51 ` [PATCH 08/14] crypto: inside-secure - unify cache reset Antoine Tenart
2019-05-27 14:51 ` [PATCH 09/14] crypto: inside-secure - fix zeroing of the request in ahash_exit_inv Antoine Tenart
2019-05-27 14:51 ` [PATCH 10/14] crypto: inside-secure - fix queued len computation Antoine Tenart
2019-05-27 14:51 ` [PATCH 11/14] crypto: inside-secure - implement IV retrieval Antoine Tenart
2019-05-27 14:51 ` [PATCH 12/14] crypto: inside-secure - add support for HMAC updates Antoine Tenart
2019-05-27 14:51 ` [PATCH 13/14] crypto: inside-secure - fix use of the SG list Antoine Tenart
2019-05-27 14:51 ` [PATCH 14/14] crypto: inside-secure - do not rely on the hardware last bit for result descriptors Antoine Tenart
2019-06-06  6:50 ` [PATCH 00/14] crypto: inside-secure - self-test fixes Herbert Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).