* [PATCH 0/2] CESA: Fixes for STD ahash requests
@ 2016-12-02 16:05 Romain Perier
2016-12-02 16:05 ` [PATCH 1/2] crypto: marvell - Don't copy hash operation twice into the SRAM Romain Perier
2016-12-02 16:05 ` [PATCH 2/2] crypto: marvell - Don't corrupt state of an STD req for re-stepped ahash Romain Perier
0 siblings, 2 replies; 6+ messages in thread
From: Romain Perier @ 2016-12-02 16:05 UTC (permalink / raw)
To: Boris Brezillon, Arnaud Ebalard
Cc: linux-crypto, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, Thomas Petazzoni, stable
This set of patches fixes two issues for STD ahash requests.
The first one is that the operation template is copied twice to the SRAM
from the step function, it is not needed. The second one is also
contained in the step function which copies creq->state to the engine for
all type of requests , even if this one is re-launched. This might corrupt
the context of the request in some cases.
Romain Perier (2):
crypto: marvell - Don't copy hash operation twice into the SRAM
crypto: marvell - Don't corrupt state of an STD req for re-stepped
ahash
drivers/crypto/marvell/hash.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
--
2.9.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] crypto: marvell - Don't copy hash operation twice into the SRAM
2016-12-02 16:05 [PATCH 0/2] CESA: Fixes for STD ahash requests Romain Perier
@ 2016-12-02 16:05 ` Romain Perier
2016-12-02 16:21 ` Boris Brezillon
2016-12-02 16:30 ` Boris Brezillon
2016-12-02 16:05 ` [PATCH 2/2] crypto: marvell - Don't corrupt state of an STD req for re-stepped ahash Romain Perier
1 sibling, 2 replies; 6+ messages in thread
From: Romain Perier @ 2016-12-02 16:05 UTC (permalink / raw)
To: Boris Brezillon, Arnaud Ebalard
Cc: linux-crypto, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, Thomas Petazzoni, stable
No need to copy the template of an hash operation twice into the SRAM
from the step function.
Fixes: commit 85030c5168f1 ("crypto: marvell - Add support for chai...")
Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
drivers/crypto/marvell/hash.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 2a92605..fbbcbf8 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -172,9 +172,6 @@ static void mv_cesa_ahash_std_step(struct ahash_request *req)
for (i = 0; i < digsize / 4; i++)
writel_relaxed(creq->state[i], engine->regs + CESA_IVDIG(i));
- mv_cesa_adjust_op(engine, &creq->op_tmpl);
- memcpy_toio(engine->sram, &creq->op_tmpl, sizeof(creq->op_tmpl));
-
if (creq->cache_ptr)
memcpy_toio(engine->sram + CESA_SA_DATA_SRAM_OFFSET,
creq->cache, creq->cache_ptr);
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] crypto: marvell - Don't corrupt state of an STD req for re-stepped ahash
2016-12-02 16:05 [PATCH 0/2] CESA: Fixes for STD ahash requests Romain Perier
2016-12-02 16:05 ` [PATCH 1/2] crypto: marvell - Don't copy hash operation twice into the SRAM Romain Perier
@ 2016-12-02 16:05 ` Romain Perier
2016-12-02 16:21 ` Boris Brezillon
1 sibling, 1 reply; 6+ messages in thread
From: Romain Perier @ 2016-12-02 16:05 UTC (permalink / raw)
To: Boris Brezillon, Arnaud Ebalard
Cc: linux-crypto, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, Thomas Petazzoni, stable
mv_cesa_hash_std_step always copies creq->state into the SRAM. If an IRQ
is triggered while the current STD request is not finished, this request
will be stepped again and the initial state will be filled into the
engine.
This commit changes the function in order to copy the state only when
the request is launched for the first time.
Fixes: commit 2786cee8e50b ("crypto: marvell - Move SRAM I/O op...")
Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
drivers/crypto/marvell/hash.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index fbbcbf8..047e05f 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -168,9 +168,11 @@ static void mv_cesa_ahash_std_step(struct ahash_request *req)
mv_cesa_adjust_op(engine, &creq->op_tmpl);
memcpy_toio(engine->sram, &creq->op_tmpl, sizeof(creq->op_tmpl));
- digsize = crypto_ahash_digestsize(crypto_ahash_reqtfm(req));
- for (i = 0; i < digsize / 4; i++)
- writel_relaxed(creq->state[i], engine->regs + CESA_IVDIG(i));
+ if (sreq->offset == 0) {
+ digsize = crypto_ahash_digestsize(crypto_ahash_reqtfm(req));
+ for (i = 0; i < digsize / 4; i++)
+ writel_relaxed(creq->state[i], engine->regs + CESA_IVDIG(i));
+ }
if (creq->cache_ptr)
memcpy_toio(engine->sram + CESA_SA_DATA_SRAM_OFFSET,
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] crypto: marvell - Don't corrupt state of an STD req for re-stepped ahash
2016-12-02 16:05 ` [PATCH 2/2] crypto: marvell - Don't corrupt state of an STD req for re-stepped ahash Romain Perier
@ 2016-12-02 16:21 ` Boris Brezillon
0 siblings, 0 replies; 6+ messages in thread
From: Boris Brezillon @ 2016-12-02 16:21 UTC (permalink / raw)
To: Romain Perier
Cc: Arnaud Ebalard, linux-crypto, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Gregory Clement, Thomas Petazzoni, stable
On Fri, 2 Dec 2016 17:05:51 +0100
Romain Perier <romain.perier@free-electrons.com> wrote:
> mv_cesa_hash_std_step always copies creq->state into the SRAM. If an IRQ
> is triggered while the current STD request is not finished, this request
> will be stepped again and the initial state will be filled into the
> engine.
Hm, it's not really related to the IRQ itself. You get several
interrupts because your request has been split into several chunks to
fit in the SRAM.
The fact that you receive several IRQs for the same crypto request is
just a side-effect of the 'split in several chunk' approach.
Maybe you should rephrase it like:
"
mv_cesa_hash_std_step() copies the creq->state into the SRAM at each
step, but this is only required on the first one. By doing that, we
overwrite the engine state, and get erroneous results when the crypto
request is split in several chunks to fit in the internal SRAM.
"
>
> This commit changes the function in order to copy the state only when
> the request is launched for the first time.
"
This commit changes the function to copy the state only on the first
step.
"
>
> Fixes: commit 2786cee8e50b ("crypto: marvell - Move SRAM I/O op...")
> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
> ---
> drivers/crypto/marvell/hash.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
> index fbbcbf8..047e05f 100644
> --- a/drivers/crypto/marvell/hash.c
> +++ b/drivers/crypto/marvell/hash.c
> @@ -168,9 +168,11 @@ static void mv_cesa_ahash_std_step(struct ahash_request *req)
> mv_cesa_adjust_op(engine, &creq->op_tmpl);
> memcpy_toio(engine->sram, &creq->op_tmpl, sizeof(creq->op_tmpl));
>
> - digsize = crypto_ahash_digestsize(crypto_ahash_reqtfm(req));
> - for (i = 0; i < digsize / 4; i++)
> - writel_relaxed(creq->state[i], engine->regs + CESA_IVDIG(i));
> + if (sreq->offset == 0) {
Just a nit, but I'd prefer
if (!sreq->offset) {
> + digsize = crypto_ahash_digestsize(crypto_ahash_reqtfm(req));
> + for (i = 0; i < digsize / 4; i++)
> + writel_relaxed(creq->state[i], engine->regs + CESA_IVDIG(i));
> + }
>
> if (creq->cache_ptr)
> memcpy_toio(engine->sram + CESA_SA_DATA_SRAM_OFFSET,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] crypto: marvell - Don't copy hash operation twice into the SRAM
2016-12-02 16:05 ` [PATCH 1/2] crypto: marvell - Don't copy hash operation twice into the SRAM Romain Perier
@ 2016-12-02 16:21 ` Boris Brezillon
2016-12-02 16:30 ` Boris Brezillon
1 sibling, 0 replies; 6+ messages in thread
From: Boris Brezillon @ 2016-12-02 16:21 UTC (permalink / raw)
To: Romain Perier
Cc: Arnaud Ebalard, linux-crypto, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Gregory Clement, Thomas Petazzoni, stable
On Fri, 2 Dec 2016 17:05:50 +0100
Romain Perier <romain.perier@free-electrons.com> wrote:
> No need to copy the template of an hash operation twice into the SRAM
> from the step function.
>
> Fixes: commit 85030c5168f1 ("crypto: marvell - Add support for chai...")
> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
Acked-by: Boris Brezillon <boris.brezillon@free-electron.com>
> ---
> drivers/crypto/marvell/hash.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
> index 2a92605..fbbcbf8 100644
> --- a/drivers/crypto/marvell/hash.c
> +++ b/drivers/crypto/marvell/hash.c
> @@ -172,9 +172,6 @@ static void mv_cesa_ahash_std_step(struct ahash_request *req)
> for (i = 0; i < digsize / 4; i++)
> writel_relaxed(creq->state[i], engine->regs + CESA_IVDIG(i));
>
> - mv_cesa_adjust_op(engine, &creq->op_tmpl);
> - memcpy_toio(engine->sram, &creq->op_tmpl, sizeof(creq->op_tmpl));
> -
> if (creq->cache_ptr)
> memcpy_toio(engine->sram + CESA_SA_DATA_SRAM_OFFSET,
> creq->cache, creq->cache_ptr);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] crypto: marvell - Don't copy hash operation twice into the SRAM
2016-12-02 16:05 ` [PATCH 1/2] crypto: marvell - Don't copy hash operation twice into the SRAM Romain Perier
2016-12-02 16:21 ` Boris Brezillon
@ 2016-12-02 16:30 ` Boris Brezillon
1 sibling, 0 replies; 6+ messages in thread
From: Boris Brezillon @ 2016-12-02 16:30 UTC (permalink / raw)
To: Romain Perier
Cc: Arnaud Ebalard, linux-crypto, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Gregory Clement, Thomas Petazzoni, stable
On Fri, 2 Dec 2016 17:05:50 +0100
Romain Perier <romain.perier@free-electrons.com> wrote:
> No need to copy the template of an hash operation twice into the SRAM
> from the step function.
>
> Fixes: commit 85030c5168f1 ("crypto: marvell - Add support for chai...")
> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
Oh, and please add
Cc: <stable@vger.kernel.org>
to both patches.
> ---
> drivers/crypto/marvell/hash.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
> index 2a92605..fbbcbf8 100644
> --- a/drivers/crypto/marvell/hash.c
> +++ b/drivers/crypto/marvell/hash.c
> @@ -172,9 +172,6 @@ static void mv_cesa_ahash_std_step(struct ahash_request *req)
> for (i = 0; i < digsize / 4; i++)
> writel_relaxed(creq->state[i], engine->regs + CESA_IVDIG(i));
>
> - mv_cesa_adjust_op(engine, &creq->op_tmpl);
> - memcpy_toio(engine->sram, &creq->op_tmpl, sizeof(creq->op_tmpl));
> -
> if (creq->cache_ptr)
> memcpy_toio(engine->sram + CESA_SA_DATA_SRAM_OFFSET,
> creq->cache, creq->cache_ptr);
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-02 16:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-02 16:05 [PATCH 0/2] CESA: Fixes for STD ahash requests Romain Perier
2016-12-02 16:05 ` [PATCH 1/2] crypto: marvell - Don't copy hash operation twice into the SRAM Romain Perier
2016-12-02 16:21 ` Boris Brezillon
2016-12-02 16:30 ` Boris Brezillon
2016-12-02 16:05 ` [PATCH 2/2] crypto: marvell - Don't corrupt state of an STD req for re-stepped ahash Romain Perier
2016-12-02 16:21 ` Boris Brezillon
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.