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
From: Thomas Petazzoni <[email protected]>
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 <[email protected]>
---
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
From: Thomas Petazzoni <[email protected]>
The dma_iter parameter of mv_cesa_ahash_dma_add_cache() is never used,
so get rid of it.
Signed-off-by: Thomas Petazzoni <[email protected]>
---
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
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 <[email protected]>
---
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
From: Thomas Petazzoni <[email protected]>
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 <[email protected]>
---
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
From: Thomas Petazzoni <[email protected]>
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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
Romain Perier <[email protected]> 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 <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt