On Wed, 14 Dec 2016 14:17:37 +0100
Romain Perier <romain.per...@free-electrons.com> wrote:
> Currently, inner IV/DIGEST data are only copied once into the hash
> engines and not set explicitly before launching a request that is not a
> first frag. This is an issue especially when multiple ahash reqs are
> computed in parallel or chained with cipher request, as the state of the
> request being computed is not updated into the hash engine. It leads to
> non-deterministic corrupted digest results.
>
> Fixes: commit 2786cee8e50b ("crypto: marvell - Move SRAM I/O operat...")
> Signed-off-by: Romain Perier <romain.per...@free-electrons.com>
> Cc: <sta...@vger.kernel.org>
Just a minor comment (see below), otherwise
Acked-by: Boris Brezillon <boris.brezil...@free-electrons.com>
> ---
> drivers/crypto/marvell/cesa.h | 3 ++-
> drivers/crypto/marvell/hash.c | 29 ++++++++++++++++++++++++++++-
> drivers/crypto/marvell/tdma.c | 9 ++++++++-
> 3 files changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/crypto/marvell/cesa.h b/drivers/crypto/marvell/cesa.h
> index a768da7..b7872f6 100644
> --- a/drivers/crypto/marvell/cesa.h
> +++ b/drivers/crypto/marvell/cesa.h
> @@ -273,7 +273,8 @@ struct mv_cesa_op_ctx {
> #define CESA_TDMA_SRC_IN_SRAM BIT(30)
> #define CESA_TDMA_END_OF_REQ BIT(29)
> #define CESA_TDMA_BREAK_CHAIN BIT(28)
> -#define CESA_TDMA_TYPE_MSK GENMASK(27, 0)
> +#define CESA_TDMA_SET_STATE BIT(27)
> +#define CESA_TDMA_TYPE_MSK GENMASK(26, 0)
> #define CESA_TDMA_DUMMY 0
> #define CESA_TDMA_DATA 1
> #define CESA_TDMA_OP 2
> diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
> index 317cf02..4eb4564 100644
> --- a/drivers/crypto/marvell/hash.c
> +++ b/drivers/crypto/marvell/hash.c
> @@ -285,8 +285,21 @@ static void mv_cesa_ahash_step(struct
> crypto_async_request *req)
> struct ahash_request *ahashreq = ahash_request_cast(req);
> struct mv_cesa_ahash_req *creq = ahash_request_ctx(ahashreq);
>
> - if (mv_cesa_req_get_type(&creq->base) == CESA_DMA_REQ)
> + if (mv_cesa_req_get_type(&creq->base) == CESA_DMA_REQ) {
> +
> + /* We must explicitely set the digest state. */
> + if (creq->base.chain.first->flags & CESA_TDMA_SET_STATE) {
> + struct mv_cesa_engine *engine = creq->base.engine;
> + int i;
> +
> + /* Set the hash state in the IVDIG regs. */
> + for (i = 0; i < ARRAY_SIZE(creq->state); i++)
> + writel_relaxed(creq->state[i], engine->regs +
> + CESA_IVDIG(i));
> + }
> +
> mv_cesa_dma_step(&creq->base);
Nit: can you move the above code in a function called
mv_cesa_ahash_dma_step()?
> + }
> else
> mv_cesa_ahash_std_step(ahashreq);
I'm pretty sure checkpatch complains here ;-).
> }
> @@ -586,10 +599,14 @@ static int mv_cesa_ahash_dma_req_init(struct
> ahash_request *req)
> unsigned int frag_len;
> int ret;
> u32 type;
> + bool set_state = false;
>
> basereq->chain.first = NULL;
> basereq->chain.last = NULL;
>
> + if (!mv_cesa_mac_op_is_first_frag(&creq->op_tmpl))
> + set_state = true;
> +
> if (creq->src_nents) {
> ret = dma_map_sg(cesa_dev->dev, req->src, creq->src_nents,
> DMA_TO_DEVICE);
> @@ -683,6 +700,16 @@ static int mv_cesa_ahash_dma_req_init(struct
> ahash_request *req)
> if (type != CESA_TDMA_RESULT)
> basereq->chain.last->flags |= CESA_TDMA_BREAK_CHAIN;
>
> + if (set_state) {
> +
> + /*
> + * Put the CESA_TDMA_SET_STATE flag on the first tdma desc to
> + * let the step logic know that the IVDIG registers should be
> + * explicitly set before launching a TDMA chain.
> + */
> + basereq->chain.first->flags |= CESA_TDMA_SET_STATE;
> + }
> +
> return 0;
>
> err_free_tdma:
> diff --git a/drivers/crypto/marvell/tdma.c b/drivers/crypto/marvell/tdma.c
> index 4416b88..c76375f 100644
> --- a/drivers/crypto/marvell/tdma.c
> +++ b/drivers/crypto/marvell/tdma.c
> @@ -109,7 +109,14 @@ void mv_cesa_tdma_chain(struct mv_cesa_engine *engine,
> last->next = dreq->chain.first;
> engine->chain.last = dreq->chain.last;
>
> - if (!(last->flags & CESA_TDMA_BREAK_CHAIN))
> + /*
> + * Break the DMA chain if the CESA_TDMA_BREAK_CHAIN is set on
> + * the last element of the current chain, or if the request
> + * being queued needs the IV regs to be set before lauching
> + * the request.
> + */
> + if (!(last->flags & CESA_TDMA_BREAK_CHAIN) &&
> + !(dreq->chain.first->flags & CESA_TDMA_SET_STATE))
> last->next_dma = dreq->chain.first->cur_dma;
> }
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
