Subject: [PATCH v3 1/2] crypto: marvell - Use an unique pool to copy results of requests

From: [email-masked] (Boris Brezillon)
Date: Tue, 4 Oct 2016 15:17:03 +0200

On Tue,  4 Oct 2016 14:57:19 +0200
Romain Perier <[email-masked]> wrote:

> So far, we used a dedicated dma pool to copy the result of outer IV for
> cipher requests. Instead of using a dma pool per outer data, we prefer
> use the op dma pool that contains all part of the request from the SRAM.
> Then, the outer data that is likely to be used by the 'complete'
> operation, is copied later. In this way, any type of result can be
> retrieved by DMA for cipher or ahash requests.
> 
> Signed-off-by: Romain Perier <[email-masked]>
> ---
> 
> Changes in v3:
>   - Don't allocate a new op ctx for the last tdma descriptor. Instead
>     we point to the last op ctx in the tdma chain, and copy the context
>     of the current request to this location.
> 
> Changes in v2:
>   - Use the dma pool "op" to retrieve outer data intead of introducing
>     a new one.
> 
>  drivers/crypto/marvell/cesa.c   |  4 ----
>  drivers/crypto/marvell/cesa.h   |  5 ++---
>  drivers/crypto/marvell/cipher.c |  8 +++++---
>  drivers/crypto/marvell/tdma.c   | 28 ++++++++++++++--------------
>  4 files changed, 21 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/crypto/marvell/cesa.c b/drivers/crypto/marvell/cesa.c
> index 37dadb2..6e7a5c7 100644
> --- a/drivers/crypto/marvell/cesa.c
> +++ b/drivers/crypto/marvell/cesa.c
> @@ -375,10 +375,6 @@ static int mv_cesa_dev_dma_init(struct mv_cesa_dev *cesa)
>  	if (!dma->padding_pool)
>  		return -ENOMEM;
>  
> -	dma->iv_pool = dmam_pool_create("cesa_iv", dev, 16, 1, 0);
> -	if (!dma->iv_pool)
> -		return -ENOMEM;
> -
>  	cesa->dma = dma;
>  
>  	return 0;
> diff --git a/drivers/crypto/marvell/cesa.h b/drivers/crypto/marvell/cesa.h
> index e423d33..a768da7 100644
> --- a/drivers/crypto/marvell/cesa.h
> +++ b/drivers/crypto/marvell/cesa.h
> @@ -277,7 +277,7 @@ struct mv_cesa_op_ctx {
>  #define CESA_TDMA_DUMMY				0
>  #define CESA_TDMA_DATA				1
>  #define CESA_TDMA_OP				2
> -#define CESA_TDMA_IV				3
> +#define CESA_TDMA_RESULT			3
>  
>  /**
>   * struct mv_cesa_tdma_desc - TDMA descriptor
> @@ -393,7 +393,6 @@ struct mv_cesa_dev_dma {
>  	struct dma_pool *op_pool;
>  	struct dma_pool *cache_pool;
>  	struct dma_pool *padding_pool;
> -	struct dma_pool *iv_pool;
>  };
>  
>  /**
> @@ -839,7 +838,7 @@ mv_cesa_tdma_desc_iter_init(struct mv_cesa_tdma_chain *chain)
>  	memset(chain, 0, sizeof(*chain));
>  }
>  
> -int mv_cesa_dma_add_iv_op(struct mv_cesa_tdma_chain *chain, dma_addr_t src,
> +int mv_cesa_dma_add_result_op(struct mv_cesa_tdma_chain *chain, dma_addr_t src,
>  			  u32 size, u32 flags, gfp_t gfp_flags);
>  
>  struct mv_cesa_op_ctx *mv_cesa_dma_add_op(struct mv_cesa_tdma_chain *chain,
> diff --git a/drivers/crypto/marvell/cipher.c b/drivers/crypto/marvell/cipher.c
> index d19dc96..098871a 100644
> --- a/drivers/crypto/marvell/cipher.c
> +++ b/drivers/crypto/marvell/cipher.c
> @@ -212,7 +212,8 @@ mv_cesa_ablkcipher_complete(struct crypto_async_request *req)
>  		struct mv_cesa_req *basereq;
>  
>  		basereq = &creq->base;
> -		memcpy(ablkreq->info, basereq->chain.last->data, ivsize);
> +		memcpy(ablkreq->info, basereq->chain.last->op->ctx.blkcipher.iv,
> +		       ivsize);
>  	} else {
>  		memcpy_fromio(ablkreq->info,
>  			      engine->sram + CESA_SA_CRYPT_IV_SRAM_OFFSET,
> @@ -373,8 +374,9 @@ static int mv_cesa_ablkcipher_dma_req_init(struct ablkcipher_request *req,
>  
>  	/* Add output data for IV */
>  	ivsize = crypto_ablkcipher_ivsize(crypto_ablkcipher_reqtfm(req));
> -	ret = mv_cesa_dma_add_iv_op(&basereq->chain, CESA_SA_CRYPT_IV_SRAM_OFFSET,
> -				    ivsize, CESA_TDMA_SRC_IN_SRAM, flags);
> +	ret = mv_cesa_dma_add_result_op(&basereq->chain, CESA_SA_CFG_SRAM_OFFSET,
> +				    CESA_SA_DATA_SRAM_OFFSET,
> +				    CESA_TDMA_SRC_IN_SRAM, flags);
>  
>  	if (ret)
>  		goto err_free_tdma;
> diff --git a/drivers/crypto/marvell/tdma.c b/drivers/crypto/marvell/tdma.c
> index 9fd7a5f..991dc3f 100644
> --- a/drivers/crypto/marvell/tdma.c
> +++ b/drivers/crypto/marvell/tdma.c
> @@ -69,9 +69,6 @@ void mv_cesa_dma_cleanup(struct mv_cesa_req *dreq)
>  		if (type == CESA_TDMA_OP)
>  			dma_pool_free(cesa_dev->dma->op_pool, tdma->op,
>  				      le32_to_cpu(tdma->src));
> -		else if (type == CESA_TDMA_IV)
> -			dma_pool_free(cesa_dev->dma->iv_pool, tdma->data,
> -				      le32_to_cpu(tdma->dst));
>  
>  		tdma = tdma->next;
>  		dma_pool_free(cesa_dev->dma->tdma_desc_pool, old_tdma,
> @@ -209,29 +206,32 @@ mv_cesa_dma_add_desc(struct mv_cesa_tdma_chain *chain, gfp_t flags)
>  	return new_tdma;
>  }
>  
> -int mv_cesa_dma_add_iv_op(struct mv_cesa_tdma_chain *chain, dma_addr_t src,
> +int mv_cesa_dma_add_result_op(struct mv_cesa_tdma_chain *chain, dma_addr_t src,
>  			  u32 size, u32 flags, gfp_t gfp_flags)
>  {
> -
> -	struct mv_cesa_tdma_desc *tdma;
> -	u8 *iv;
> -	dma_addr_t dma_handle;
> +	struct mv_cesa_tdma_desc *tdma, *op_desc;
>  
>  	tdma = mv_cesa_dma_add_desc(chain, gfp_flags);
>  	if (IS_ERR(tdma))
>  		return PTR_ERR(tdma);
>  
> -	iv = dma_pool_alloc(cesa_dev->dma->iv_pool, gfp_flags, &dma_handle);
> -	if (!iv)
> -		return -ENOMEM;

Can you add a comment explaining what you're doing here?

	/* We re-use an existing op_desc object to retrieve the context
	 * and result instead of allocating a new one.
	 * There is at least one object of this type in a CESA crypto
	 * req, just pick the first one in the chain.
	 */

Once this is addressed, you can add my

Acked-by: Boris Brezillon <[email-masked]>

> +	for (op_desc = chain->first; op_desc; op_desc = op_desc->next) {
> +		u32 type = op_desc->flags & CESA_TDMA_TYPE_MSK;
> +
> +		if (type == CESA_TDMA_OP)
> +			break;
> +	}
> +
> +	if (!op_desc)
> +		return -EIO;
>  
>  	tdma->byte_cnt = cpu_to_le32(size | BIT(31));
>  	tdma->src = src;
> -	tdma->dst = cpu_to_le32(dma_handle);
> -	tdma->data = iv;
> +	tdma->dst = op_desc->src;
> +	tdma->op = op_desc->op;
>  
>  	flags &= (CESA_TDMA_DST_IN_SRAM | CESA_TDMA_SRC_IN_SRAM);
> -	tdma->flags = flags | CESA_TDMA_IV;
> +	tdma->flags = flags | CESA_TDMA_RESULT;
>  	return 0;
>  }
>