All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] Various fixes for the cesa driver
@ 2016-08-09  9:03 ` Romain Perier
  0 siblings, 0 replies; 20+ messages in thread
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Gregory Clement, Thomas Petazzoni, David S. Miller, Russell King,
	linux-crypto, linux-arm-kernel

This patches series contains various fixes for ahash requests, dma
operations and an important fixe in the core of the driver (cesa.c). It
also includes some code cleanups.

Romain Perier (3):
  crypto: marvell - Update transformation context for each dequeued req
  crypto: marvell - Don't overwrite default creq->state during
    initialization
  crypto: marvell - Don't hardcode block size in mv_cesa_ahash_cache_req

Thomas Petazzoni (4):
  crypto: marvell: be explicit about destination in mv_cesa_dma_add_op()
  crypto: marvell: remove unused parameter in
    mv_cesa_ahash_dma_add_cache()
  crypto: marvell: turn mv_cesa_ahash_init() into a function returning
    void
  crypto: marvell: make mv_cesa_ahash_cache_req() return bool

 drivers/crypto/marvell/cesa.c |  1 +
 drivers/crypto/marvell/hash.c | 44 +++++++++++++++++++++----------------------
 drivers/crypto/marvell/tdma.c |  1 +
 3 files changed, 23 insertions(+), 23 deletions(-)

-- 
2.8.1

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

* [PATCH 0/7] Various fixes for the cesa driver
@ 2016-08-09  9:03 ` Romain Perier
  0 siblings, 0 replies; 20+ messages in thread
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: linux-arm-kernel

This patches series contains various fixes for ahash requests, dma
operations and an important fixe in the core of the driver (cesa.c). It
also includes some code cleanups.

Romain Perier (3):
  crypto: marvell - Update transformation context for each dequeued req
  crypto: marvell - Don't overwrite default creq->state during
    initialization
  crypto: marvell - Don't hardcode block size in mv_cesa_ahash_cache_req

Thomas Petazzoni (4):
  crypto: marvell: be explicit about destination in mv_cesa_dma_add_op()
  crypto: marvell: remove unused parameter in
    mv_cesa_ahash_dma_add_cache()
  crypto: marvell: turn mv_cesa_ahash_init() into a function returning
    void
  crypto: marvell: make mv_cesa_ahash_cache_req() return bool

 drivers/crypto/marvell/cesa.c |  1 +
 drivers/crypto/marvell/hash.c | 44 +++++++++++++++++++++----------------------
 drivers/crypto/marvell/tdma.c |  1 +
 3 files changed, 23 insertions(+), 23 deletions(-)

-- 
2.8.1

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

* [PATCH 1/7] crypto: marvell: be explicit about destination in mv_cesa_dma_add_op()
  2016-08-09  9:03 ` Romain Perier
@ 2016-08-09  9:03   ` Romain Perier
  -1 siblings, 0 replies; 20+ messages in thread
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Gregory Clement, Thomas Petazzoni, David S. Miller, Russell King,
	linux-crypto, linux-arm-kernel

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

The mv_cesa_dma_add_op() function builds a mv_cesa_tdma_desc structure
to copy the operation description to the SRAM, but doesn't explicitly
initialize the destination of the copy. It works fine because the
operatin description must be copied at the beginning of the SRAM, and
the mv_cesa_tdma_desc structure is initialized to zero when
allocated. However, it is somewhat confusing to not have a destination
defined.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/crypto/marvell/tdma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/marvell/tdma.c b/drivers/crypto/marvell/tdma.c
index 86a065b..9fd7a5f 100644
--- a/drivers/crypto/marvell/tdma.c
+++ b/drivers/crypto/marvell/tdma.c
@@ -261,6 +261,7 @@ struct mv_cesa_op_ctx *mv_cesa_dma_add_op(struct mv_cesa_tdma_chain *chain,
 	tdma->op = op;
 	tdma->byte_cnt = cpu_to_le32(size | BIT(31));
 	tdma->src = cpu_to_le32(dma_handle);
+	tdma->dst = CESA_SA_CFG_SRAM_OFFSET;
 	tdma->flags = CESA_TDMA_DST_IN_SRAM | CESA_TDMA_OP;
 
 	return op;
-- 
2.8.1

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

* [PATCH 1/7] crypto: marvell: be explicit about destination in mv_cesa_dma_add_op()
@ 2016-08-09  9:03   ` Romain Perier
  0 siblings, 0 replies; 20+ messages in thread
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: linux-arm-kernel

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

The mv_cesa_dma_add_op() function builds a mv_cesa_tdma_desc structure
to copy the operation description to the SRAM, but doesn't explicitly
initialize the destination of the copy. It works fine because the
operatin description must be copied at the beginning of the SRAM, and
the mv_cesa_tdma_desc structure is initialized to zero when
allocated. However, it is somewhat confusing to not have a destination
defined.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/crypto/marvell/tdma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/marvell/tdma.c b/drivers/crypto/marvell/tdma.c
index 86a065b..9fd7a5f 100644
--- a/drivers/crypto/marvell/tdma.c
+++ b/drivers/crypto/marvell/tdma.c
@@ -261,6 +261,7 @@ struct mv_cesa_op_ctx *mv_cesa_dma_add_op(struct mv_cesa_tdma_chain *chain,
 	tdma->op = op;
 	tdma->byte_cnt = cpu_to_le32(size | BIT(31));
 	tdma->src = cpu_to_le32(dma_handle);
+	tdma->dst = CESA_SA_CFG_SRAM_OFFSET;
 	tdma->flags = CESA_TDMA_DST_IN_SRAM | CESA_TDMA_OP;
 
 	return op;
-- 
2.8.1

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

* [PATCH 2/7] crypto: marvell: remove unused parameter in mv_cesa_ahash_dma_add_cache()
  2016-08-09  9:03 ` Romain Perier
@ 2016-08-09  9:03   ` Romain Perier
  -1 siblings, 0 replies; 20+ messages in thread
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Gregory Clement, Thomas Petazzoni, David S. Miller, Russell King,
	linux-crypto, linux-arm-kernel

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

The dma_iter parameter of mv_cesa_ahash_dma_add_cache() is never used,
so get rid of it.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/crypto/marvell/hash.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 82e0f4e6..0d7f5f9 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -455,7 +455,6 @@ mv_cesa_dma_add_frag(struct mv_cesa_tdma_chain *chain,
 
 static int
 mv_cesa_ahash_dma_add_cache(struct mv_cesa_tdma_chain *chain,
-			    struct mv_cesa_ahash_dma_iter *dma_iter,
 			    struct mv_cesa_ahash_req *creq,
 			    gfp_t flags)
 {
@@ -586,7 +585,7 @@ static int mv_cesa_ahash_dma_req_init(struct ahash_request *req)
 	 * Add the cache (left-over data from a previous block) first.
 	 * This will never overflow the SRAM size.
 	 */
-	ret = mv_cesa_ahash_dma_add_cache(&basereq->chain, &iter, creq, flags);
+	ret = mv_cesa_ahash_dma_add_cache(&basereq->chain, creq, flags);
 	if (ret)
 		goto err_free_tdma;
 
-- 
2.8.1

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

* [PATCH 2/7] crypto: marvell: remove unused parameter in mv_cesa_ahash_dma_add_cache()
@ 2016-08-09  9:03   ` Romain Perier
  0 siblings, 0 replies; 20+ messages in thread
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: linux-arm-kernel

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

The dma_iter parameter of mv_cesa_ahash_dma_add_cache() is never used,
so get rid of it.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/crypto/marvell/hash.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 82e0f4e6..0d7f5f9 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -455,7 +455,6 @@ mv_cesa_dma_add_frag(struct mv_cesa_tdma_chain *chain,
 
 static int
 mv_cesa_ahash_dma_add_cache(struct mv_cesa_tdma_chain *chain,
-			    struct mv_cesa_ahash_dma_iter *dma_iter,
 			    struct mv_cesa_ahash_req *creq,
 			    gfp_t flags)
 {
@@ -586,7 +585,7 @@ static int mv_cesa_ahash_dma_req_init(struct ahash_request *req)
 	 * Add the cache (left-over data from a previous block) first.
 	 * This will never overflow the SRAM size.
 	 */
-	ret = mv_cesa_ahash_dma_add_cache(&basereq->chain, &iter, creq, flags);
+	ret = mv_cesa_ahash_dma_add_cache(&basereq->chain, creq, flags);
 	if (ret)
 		goto err_free_tdma;
 
-- 
2.8.1

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

* [PATCH 3/7] crypto: marvell: turn mv_cesa_ahash_init() into a function returning void
  2016-08-09  9:03 ` Romain Perier
@ 2016-08-09  9:03   ` Romain Perier
  -1 siblings, 0 replies; 20+ messages in thread
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Gregory Clement, Thomas Petazzoni, David S. Miller, Russell King,
	linux-crypto, linux-arm-kernel

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

The mv_cesa_ahash_init() function always returns 0, and the return
value is anyway never checked. Turn it into a function returning void.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/crypto/marvell/hash.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 0d7f5f9..7291664 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -374,7 +374,7 @@ static const struct mv_cesa_req_ops mv_cesa_ahash_req_ops = {
 	.complete = mv_cesa_ahash_complete,
 };
 
-static int mv_cesa_ahash_init(struct ahash_request *req,
+static void mv_cesa_ahash_init(struct ahash_request *req,
 			      struct mv_cesa_op_ctx *tmpl, bool algo_le)
 {
 	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
@@ -390,8 +390,6 @@ static int mv_cesa_ahash_init(struct ahash_request *req,
 	creq->op_tmpl = *tmpl;
 	creq->len = 0;
 	creq->algo_le = algo_le;
-
-	return 0;
 }
 
 static inline int mv_cesa_ahash_cra_init(struct crypto_tfm *tfm)
-- 
2.8.1

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

* [PATCH 3/7] crypto: marvell: turn mv_cesa_ahash_init() into a function returning void
@ 2016-08-09  9:03   ` Romain Perier
  0 siblings, 0 replies; 20+ messages in thread
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: linux-arm-kernel

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

The mv_cesa_ahash_init() function always returns 0, and the return
value is anyway never checked. Turn it into a function returning void.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/crypto/marvell/hash.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 0d7f5f9..7291664 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -374,7 +374,7 @@ static const struct mv_cesa_req_ops mv_cesa_ahash_req_ops = {
 	.complete = mv_cesa_ahash_complete,
 };
 
-static int mv_cesa_ahash_init(struct ahash_request *req,
+static void mv_cesa_ahash_init(struct ahash_request *req,
 			      struct mv_cesa_op_ctx *tmpl, bool algo_le)
 {
 	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
@@ -390,8 +390,6 @@ static int mv_cesa_ahash_init(struct ahash_request *req,
 	creq->op_tmpl = *tmpl;
 	creq->len = 0;
 	creq->algo_le = algo_le;
-
-	return 0;
 }
 
 static inline int mv_cesa_ahash_cra_init(struct crypto_tfm *tfm)
-- 
2.8.1

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

* [PATCH 4/7] crypto: marvell: make mv_cesa_ahash_cache_req() return bool
  2016-08-09  9:03 ` Romain Perier
@ 2016-08-09  9:03   ` Romain Perier
  -1 siblings, 0 replies; 20+ messages in thread
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Gregory Clement, Thomas Petazzoni, David S. Miller, Russell King,
	linux-crypto, linux-arm-kernel

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

The mv_cesa_ahash_cache_req() function always returns 0, which makes
its return value pretty much useless. However, in addition to
returning a useless value, it also returns a boolean in a variable
passed by reference to indicate if the request was already cached.

So, this commit changes mv_cesa_ahash_cache_req() to return this
boolean. It consequently simplifies the only call site of
mv_cesa_ahash_cache_req(), where the "ret" variable is no longer
needed.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/crypto/marvell/hash.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 7291664..cf8063d 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -403,15 +403,16 @@ static inline int mv_cesa_ahash_cra_init(struct crypto_tfm *tfm)
 	return 0;
 }
 
-static int mv_cesa_ahash_cache_req(struct ahash_request *req, bool *cached)
+static bool mv_cesa_ahash_cache_req(struct ahash_request *req)
 {
 	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
+	bool cached = false;
 
 	if (creq->cache_ptr + req->nbytes < 64 && !creq->last_req) {
-		*cached = true;
+		cached = true;
 
 		if (!req->nbytes)
-			return 0;
+			return cached;
 
 		sg_pcopy_to_buffer(req->src, creq->src_nents,
 				   creq->cache + creq->cache_ptr,
@@ -420,7 +421,7 @@ static int mv_cesa_ahash_cache_req(struct ahash_request *req, bool *cached)
 		creq->cache_ptr += req->nbytes;
 	}
 
-	return 0;
+	return cached;
 }
 
 static struct mv_cesa_op_ctx *
@@ -665,7 +666,6 @@ err:
 static int mv_cesa_ahash_req_init(struct ahash_request *req, bool *cached)
 {
 	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
-	int ret;
 
 	creq->src_nents = sg_nents_for_len(req->src, req->nbytes);
 	if (creq->src_nents < 0) {
@@ -673,17 +673,15 @@ static int mv_cesa_ahash_req_init(struct ahash_request *req, bool *cached)
 		return creq->src_nents;
 	}
 
-	ret = mv_cesa_ahash_cache_req(req, cached);
-	if (ret)
-		return ret;
+	*cached = mv_cesa_ahash_cache_req(req);
 
 	if (*cached)
 		return 0;
 
 	if (cesa_dev->caps->has_tdma)
-		ret = mv_cesa_ahash_dma_req_init(req);
-
-	return ret;
+		return mv_cesa_ahash_dma_req_init(req);
+	else
+		return 0;
 }
 
 static int mv_cesa_ahash_queue_req(struct ahash_request *req)
-- 
2.8.1

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

* [PATCH 4/7] crypto: marvell: make mv_cesa_ahash_cache_req() return bool
@ 2016-08-09  9:03   ` Romain Perier
  0 siblings, 0 replies; 20+ messages in thread
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: linux-arm-kernel

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

The mv_cesa_ahash_cache_req() function always returns 0, which makes
its return value pretty much useless. However, in addition to
returning a useless value, it also returns a boolean in a variable
passed by reference to indicate if the request was already cached.

So, this commit changes mv_cesa_ahash_cache_req() to return this
boolean. It consequently simplifies the only call site of
mv_cesa_ahash_cache_req(), where the "ret" variable is no longer
needed.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/crypto/marvell/hash.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 7291664..cf8063d 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -403,15 +403,16 @@ static inline int mv_cesa_ahash_cra_init(struct crypto_tfm *tfm)
 	return 0;
 }
 
-static int mv_cesa_ahash_cache_req(struct ahash_request *req, bool *cached)
+static bool mv_cesa_ahash_cache_req(struct ahash_request *req)
 {
 	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
+	bool cached = false;
 
 	if (creq->cache_ptr + req->nbytes < 64 && !creq->last_req) {
-		*cached = true;
+		cached = true;
 
 		if (!req->nbytes)
-			return 0;
+			return cached;
 
 		sg_pcopy_to_buffer(req->src, creq->src_nents,
 				   creq->cache + creq->cache_ptr,
@@ -420,7 +421,7 @@ static int mv_cesa_ahash_cache_req(struct ahash_request *req, bool *cached)
 		creq->cache_ptr += req->nbytes;
 	}
 
-	return 0;
+	return cached;
 }
 
 static struct mv_cesa_op_ctx *
@@ -665,7 +666,6 @@ err:
 static int mv_cesa_ahash_req_init(struct ahash_request *req, bool *cached)
 {
 	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
-	int ret;
 
 	creq->src_nents = sg_nents_for_len(req->src, req->nbytes);
 	if (creq->src_nents < 0) {
@@ -673,17 +673,15 @@ static int mv_cesa_ahash_req_init(struct ahash_request *req, bool *cached)
 		return creq->src_nents;
 	}
 
-	ret = mv_cesa_ahash_cache_req(req, cached);
-	if (ret)
-		return ret;
+	*cached = mv_cesa_ahash_cache_req(req);
 
 	if (*cached)
 		return 0;
 
 	if (cesa_dev->caps->has_tdma)
-		ret = mv_cesa_ahash_dma_req_init(req);
-
-	return ret;
+		return mv_cesa_ahash_dma_req_init(req);
+	else
+		return 0;
 }
 
 static int mv_cesa_ahash_queue_req(struct ahash_request *req)
-- 
2.8.1

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

* [PATCH 5/7] crypto: marvell - Update transformation context for each dequeued req
  2016-08-09  9:03 ` Romain Perier
@ 2016-08-09  9:03   ` Romain Perier
  -1 siblings, 0 replies; 20+ messages in thread
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Gregory Clement, Thomas Petazzoni, David S. Miller, Russell King,
	linux-crypto, linux-arm-kernel

So far, sub part of mv_cesa_int was responsible of dequeuing complete
requests, then call the 'cleanup' operation on these reqs and call the
crypto api callback 'complete'. The problem is that the transformation
context 'ctx' is retrieved only once before the while loop. Which means
that the wrong 'cleanup' operation might be called on the wrong type of
cesa requests, it can lead to memory corruptions with this message:

marvell-cesa f1090000.crypto: dma_pool_free cesa_padding, 5a5a5a5a/5a5a5a5a (bad dma)

This commit fixes the issue, by updating the transformation context for
each dequeued cesa request.

Fixes: commit 85030c5168f1 ("crypto: marvell - Add support for chai...")
Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/crypto/marvell/cesa.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/marvell/cesa.c b/drivers/crypto/marvell/cesa.c
index d64af86..37dadb2 100644
--- a/drivers/crypto/marvell/cesa.c
+++ b/drivers/crypto/marvell/cesa.c
@@ -166,6 +166,7 @@ static irqreturn_t mv_cesa_int(int irq, void *priv)
 			if (!req)
 				break;
 
+			ctx = crypto_tfm_ctx(req->tfm);
 			mv_cesa_complete_req(ctx, req, 0);
 		}
 	}
-- 
2.8.1

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

* [PATCH 5/7] crypto: marvell - Update transformation context for each dequeued req
@ 2016-08-09  9:03   ` Romain Perier
  0 siblings, 0 replies; 20+ messages in thread
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: linux-arm-kernel

So far, sub part of mv_cesa_int was responsible of dequeuing complete
requests, then call the 'cleanup' operation on these reqs and call the
crypto api callback 'complete'. The problem is that the transformation
context 'ctx' is retrieved only once before the while loop. Which means
that the wrong 'cleanup' operation might be called on the wrong type of
cesa requests, it can lead to memory corruptions with this message:

marvell-cesa f1090000.crypto: dma_pool_free cesa_padding, 5a5a5a5a/5a5a5a5a (bad dma)

This commit fixes the issue, by updating the transformation context for
each dequeued cesa request.

Fixes: commit 85030c5168f1 ("crypto: marvell - Add support for chai...")
Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/crypto/marvell/cesa.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/marvell/cesa.c b/drivers/crypto/marvell/cesa.c
index d64af86..37dadb2 100644
--- a/drivers/crypto/marvell/cesa.c
+++ b/drivers/crypto/marvell/cesa.c
@@ -166,6 +166,7 @@ static irqreturn_t mv_cesa_int(int irq, void *priv)
 			if (!req)
 				break;
 
+			ctx = crypto_tfm_ctx(req->tfm);
 			mv_cesa_complete_req(ctx, req, 0);
 		}
 	}
-- 
2.8.1

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

* [PATCH 6/7] crypto: marvell - Don't overwrite default creq->state during initialization
  2016-08-09  9:03 ` Romain Perier
@ 2016-08-09  9:03   ` Romain Perier
  -1 siblings, 0 replies; 20+ messages in thread
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Gregory Clement, Thomas Petazzoni, David S. Miller, Russell King,
	linux-crypto, linux-arm-kernel

Currently, in mv_cesa_{md5,sha1,sha256}_init creq->state is initialized
before the call to mv_cesa_ahash_init. This is wrong because this
function fills creq with zero by using memset, so its 'state' that
contains the default DIGEST is overwritten. This commit fixes the issue
by initializing creq->state just after the call to mv_cesa_ahash_init.

Fixes: commit b0ef51067cb4 ("crypto: marvell/cesa - initialize hash...")
Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/crypto/marvell/hash.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index cf8063d..44a8abe 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -800,13 +800,14 @@ static int mv_cesa_md5_init(struct ahash_request *req)
 	struct mv_cesa_op_ctx tmpl = { };
 
 	mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_MD5);
+
+	mv_cesa_ahash_init(req, &tmpl, true);
+
 	creq->state[0] = MD5_H0;
 	creq->state[1] = MD5_H1;
 	creq->state[2] = MD5_H2;
 	creq->state[3] = MD5_H3;
 
-	mv_cesa_ahash_init(req, &tmpl, true);
-
 	return 0;
 }
 
@@ -868,14 +869,15 @@ static int mv_cesa_sha1_init(struct ahash_request *req)
 	struct mv_cesa_op_ctx tmpl = { };
 
 	mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_SHA1);
+
+	mv_cesa_ahash_init(req, &tmpl, false);
+
 	creq->state[0] = SHA1_H0;
 	creq->state[1] = SHA1_H1;
 	creq->state[2] = SHA1_H2;
 	creq->state[3] = SHA1_H3;
 	creq->state[4] = SHA1_H4;
 
-	mv_cesa_ahash_init(req, &tmpl, false);
-
 	return 0;
 }
 
@@ -937,6 +939,9 @@ static int mv_cesa_sha256_init(struct ahash_request *req)
 	struct mv_cesa_op_ctx tmpl = { };
 
 	mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_SHA256);
+
+	mv_cesa_ahash_init(req, &tmpl, false);
+
 	creq->state[0] = SHA256_H0;
 	creq->state[1] = SHA256_H1;
 	creq->state[2] = SHA256_H2;
@@ -946,8 +951,6 @@ static int mv_cesa_sha256_init(struct ahash_request *req)
 	creq->state[6] = SHA256_H6;
 	creq->state[7] = SHA256_H7;
 
-	mv_cesa_ahash_init(req, &tmpl, false);
-
 	return 0;
 }
 
-- 
2.8.1

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

* [PATCH 6/7] crypto: marvell - Don't overwrite default creq->state during initialization
@ 2016-08-09  9:03   ` Romain Perier
  0 siblings, 0 replies; 20+ messages in thread
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: linux-arm-kernel

Currently, in mv_cesa_{md5,sha1,sha256}_init creq->state is initialized
before the call to mv_cesa_ahash_init. This is wrong because this
function fills creq with zero by using memset, so its 'state' that
contains the default DIGEST is overwritten. This commit fixes the issue
by initializing creq->state just after the call to mv_cesa_ahash_init.

Fixes: commit b0ef51067cb4 ("crypto: marvell/cesa - initialize hash...")
Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/crypto/marvell/hash.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index cf8063d..44a8abe 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -800,13 +800,14 @@ static int mv_cesa_md5_init(struct ahash_request *req)
 	struct mv_cesa_op_ctx tmpl = { };
 
 	mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_MD5);
+
+	mv_cesa_ahash_init(req, &tmpl, true);
+
 	creq->state[0] = MD5_H0;
 	creq->state[1] = MD5_H1;
 	creq->state[2] = MD5_H2;
 	creq->state[3] = MD5_H3;
 
-	mv_cesa_ahash_init(req, &tmpl, true);
-
 	return 0;
 }
 
@@ -868,14 +869,15 @@ static int mv_cesa_sha1_init(struct ahash_request *req)
 	struct mv_cesa_op_ctx tmpl = { };
 
 	mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_SHA1);
+
+	mv_cesa_ahash_init(req, &tmpl, false);
+
 	creq->state[0] = SHA1_H0;
 	creq->state[1] = SHA1_H1;
 	creq->state[2] = SHA1_H2;
 	creq->state[3] = SHA1_H3;
 	creq->state[4] = SHA1_H4;
 
-	mv_cesa_ahash_init(req, &tmpl, false);
-
 	return 0;
 }
 
@@ -937,6 +939,9 @@ static int mv_cesa_sha256_init(struct ahash_request *req)
 	struct mv_cesa_op_ctx tmpl = { };
 
 	mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_SHA256);
+
+	mv_cesa_ahash_init(req, &tmpl, false);
+
 	creq->state[0] = SHA256_H0;
 	creq->state[1] = SHA256_H1;
 	creq->state[2] = SHA256_H2;
@@ -946,8 +951,6 @@ static int mv_cesa_sha256_init(struct ahash_request *req)
 	creq->state[6] = SHA256_H6;
 	creq->state[7] = SHA256_H7;
 
-	mv_cesa_ahash_init(req, &tmpl, false);
-
 	return 0;
 }
 
-- 
2.8.1

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

* [PATCH 7/7] crypto: marvell - Don't hardcode block size in mv_cesa_ahash_cache_req
  2016-08-09  9:03 ` Romain Perier
@ 2016-08-09  9:03   ` Romain Perier
  -1 siblings, 0 replies; 20+ messages in thread
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Gregory Clement, Thomas Petazzoni, David S. Miller, Russell King,
	linux-crypto, linux-arm-kernel

Don't use 64 'as is', as max block size in mv_cesa_ahash_cache_req. Use
CESA_MAX_HASH_BLOCK_SIZE instead, this is better for readability.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/crypto/marvell/hash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 44a8abe..9f28468 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -408,7 +408,7 @@ static bool mv_cesa_ahash_cache_req(struct ahash_request *req)
 	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
 	bool cached = false;
 
-	if (creq->cache_ptr + req->nbytes < 64 && !creq->last_req) {
+	if (creq->cache_ptr + req->nbytes < CESA_MAX_HASH_BLOCK_SIZE && !creq->last_req) {
 		cached = true;
 
 		if (!req->nbytes)
-- 
2.8.1

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

* [PATCH 7/7] crypto: marvell - Don't hardcode block size in mv_cesa_ahash_cache_req
@ 2016-08-09  9:03   ` Romain Perier
  0 siblings, 0 replies; 20+ messages in thread
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: linux-arm-kernel

Don't use 64 'as is', as max block size in mv_cesa_ahash_cache_req. Use
CESA_MAX_HASH_BLOCK_SIZE instead, this is better for readability.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/crypto/marvell/hash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 44a8abe..9f28468 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -408,7 +408,7 @@ static bool mv_cesa_ahash_cache_req(struct ahash_request *req)
 	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
 	bool cached = false;
 
-	if (creq->cache_ptr + req->nbytes < 64 && !creq->last_req) {
+	if (creq->cache_ptr + req->nbytes < CESA_MAX_HASH_BLOCK_SIZE && !creq->last_req) {
 		cached = true;
 
 		if (!req->nbytes)
-- 
2.8.1

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

* Re: [PATCH 0/7] Various fixes for the cesa driver
  2016-08-09  9:03 ` Romain Perier
@ 2016-08-09  9:48   ` Thomas Petazzoni
  -1 siblings, 0 replies; 20+ messages in thread
From: Thomas Petazzoni @ 2016-08-09  9:48 UTC (permalink / raw)
  To: Romain Perier
  Cc: Boris Brezillon, Arnaud Ebalard, Russell King, linux-crypto,
	Gregory Clement, David S. Miller, linux-arm-kernel

Hello,

Is it normal that Herbert, as the crypto maintainer, is not Cc'ed on
those patches?

On Tue,  9 Aug 2016 11:03:13 +0200, Romain Perier wrote:
> This patches series contains various fixes for ahash requests, dma
> operations and an important fixe in the core of the driver (cesa.c). It
> also includes some code cleanups.
> 
> Romain Perier (3):
>   crypto: marvell - Update transformation context for each dequeued req
>   crypto: marvell - Don't overwrite default creq->state during
>     initialization

I think those two patches should have come first in the series, to make
it clear that they are the two fixes that are important to merge for
the 4.8 release cycle.

>   crypto: marvell - Don't hardcode block size in mv_cesa_ahash_cache_req
> 
> Thomas Petazzoni (4):
>   crypto: marvell: be explicit about destination in mv_cesa_dma_add_op()
>   crypto: marvell: remove unused parameter in
>     mv_cesa_ahash_dma_add_cache()
>   crypto: marvell: turn mv_cesa_ahash_init() into a function returning
>     void
>   crypto: marvell: make mv_cesa_ahash_cache_req() return bool

Those 5 other patches (the four from me and the last one from you) are
more cleanups/improvements, which can wait the 4.9 release cycle.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

* [PATCH 0/7] Various fixes for the cesa driver
@ 2016-08-09  9:48   ` Thomas Petazzoni
  0 siblings, 0 replies; 20+ messages in thread
From: Thomas Petazzoni @ 2016-08-09  9:48 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

Is it normal that Herbert, as the crypto maintainer, is not Cc'ed on
those patches?

On Tue,  9 Aug 2016 11:03:13 +0200, Romain Perier wrote:
> This patches series contains various fixes for ahash requests, dma
> operations and an important fixe in the core of the driver (cesa.c). It
> also includes some code cleanups.
> 
> Romain Perier (3):
>   crypto: marvell - Update transformation context for each dequeued req
>   crypto: marvell - Don't overwrite default creq->state during
>     initialization

I think those two patches should have come first in the series, to make
it clear that they are the two fixes that are important to merge for
the 4.8 release cycle.

>   crypto: marvell - Don't hardcode block size in mv_cesa_ahash_cache_req
> 
> Thomas Petazzoni (4):
>   crypto: marvell: be explicit about destination in mv_cesa_dma_add_op()
>   crypto: marvell: remove unused parameter in
>     mv_cesa_ahash_dma_add_cache()
>   crypto: marvell: turn mv_cesa_ahash_init() into a function returning
>     void
>   crypto: marvell: make mv_cesa_ahash_cache_req() return bool

Those 5 other patches (the four from me and the last one from you) are
more cleanups/improvements, which can wait the 4.9 release cycle.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

* Re: [PATCH 0/7] Various fixes for the cesa driver
  2016-08-09  9:03 ` Romain Perier
@ 2016-08-09 11:05   ` Herbert Xu
  -1 siblings, 0 replies; 20+ messages in thread
From: Herbert Xu @ 2016-08-09 11:05 UTC (permalink / raw)
  To: Romain Perier
  Cc: thomas.petazzoni, boris.brezillon, linux, arno, linux-crypto,
	gregory.clement, davem, linux-arm-kernel

Romain Perier <romain.perier@free-electrons.com> wrote:
> This patches series contains various fixes for ahash requests, dma
> operations and an important fixe in the core of the driver (cesa.c). It
> also includes some code cleanups.

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] 20+ messages in thread

* [PATCH 0/7] Various fixes for the cesa driver
@ 2016-08-09 11:05   ` Herbert Xu
  0 siblings, 0 replies; 20+ messages in thread
From: Herbert Xu @ 2016-08-09 11:05 UTC (permalink / raw)
  To: linux-arm-kernel

Romain Perier <romain.perier@free-electrons.com> wrote:
> This patches series contains various fixes for ahash requests, dma
> operations and an important fixe in the core of the driver (cesa.c). It
> also includes some code cleanups.

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] 20+ messages in thread

end of thread, other threads:[~2016-08-09 11:05 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-09  9:03 [PATCH 0/7] Various fixes for the cesa driver Romain Perier
2016-08-09  9:03 ` Romain Perier
2016-08-09  9:03 ` [PATCH 1/7] crypto: marvell: be explicit about destination in mv_cesa_dma_add_op() Romain Perier
2016-08-09  9:03   ` Romain Perier
2016-08-09  9:03 ` [PATCH 2/7] crypto: marvell: remove unused parameter in mv_cesa_ahash_dma_add_cache() Romain Perier
2016-08-09  9:03   ` Romain Perier
2016-08-09  9:03 ` [PATCH 3/7] crypto: marvell: turn mv_cesa_ahash_init() into a function returning void Romain Perier
2016-08-09  9:03   ` Romain Perier
2016-08-09  9:03 ` [PATCH 4/7] crypto: marvell: make mv_cesa_ahash_cache_req() return bool Romain Perier
2016-08-09  9:03   ` Romain Perier
2016-08-09  9:03 ` [PATCH 5/7] crypto: marvell - Update transformation context for each dequeued req Romain Perier
2016-08-09  9:03   ` Romain Perier
2016-08-09  9:03 ` [PATCH 6/7] crypto: marvell - Don't overwrite default creq->state during initialization Romain Perier
2016-08-09  9:03   ` Romain Perier
2016-08-09  9:03 ` [PATCH 7/7] crypto: marvell - Don't hardcode block size in mv_cesa_ahash_cache_req Romain Perier
2016-08-09  9:03   ` Romain Perier
2016-08-09  9:48 ` [PATCH 0/7] Various fixes for the cesa driver Thomas Petazzoni
2016-08-09  9:48   ` Thomas Petazzoni
2016-08-09 11:05 ` Herbert Xu
2016-08-09 11:05   ` Herbert Xu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.