Re: [PATCH v2 03/10] crypto: sun8i-ce - move bounce_iv and backup_iv to request context

[Corentin Labbe <clabbe.montjoie@gmail.com>]

Le Thu, Jun 26, 2025 at 12:58:06PM +0300, Ovidiu Panait a écrit :
> Currently, the iv buffers are allocated once per flow during driver probe.
> Having a single iv buffer for all requests works with the current setup
> where requests are processed one by one, but it wouldn't work if multiple
> requests are chained together and processed in one go.
> 
> In preparation for introducing request batching, allocate iv buffers per
> request, rather than per flow.
> 
> Signed-off-by: Ovidiu Panait <ovidiu.panait.oss@gmail.com>
> ---
>  .../allwinner/sun8i-ce/sun8i-ce-cipher.c       | 18 +++++++++---------
>  .../crypto/allwinner/sun8i-ce/sun8i-ce-core.c  | 12 ------------
>  drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h   |  8 ++++----
>  3 files changed, 13 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
> index 113a1100f2ae..9963e5962551 100644
> --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
> +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
> @@ -209,11 +209,11 @@ static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req
>  	if (areq->iv && ivsize > 0) {
>  		if (rctx->op_dir & CE_DECRYPTION) {
>  			offset = areq->cryptlen - ivsize;
> -			scatterwalk_map_and_copy(chan->backup_iv, areq->src,
> +			scatterwalk_map_and_copy(rctx->backup_iv, areq->src,
>  						 offset, ivsize, 0);
>  		}
> -		memcpy(chan->bounce_iv, areq->iv, ivsize);
> -		rctx->addr_iv = dma_map_single(ce->dev, chan->bounce_iv, ivsize,
> +		memcpy(rctx->bounce_iv, areq->iv, ivsize);
> +		rctx->addr_iv = dma_map_single(ce->dev, rctx->bounce_iv, ivsize,
>  					       DMA_TO_DEVICE);
>  		if (dma_mapping_error(ce->dev, rctx->addr_iv)) {
>  			dev_err(ce->dev, "Cannot DMA MAP IV\n");
> @@ -299,13 +299,13 @@ static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req
>  
>  		offset = areq->cryptlen - ivsize;
>  		if (rctx->op_dir & CE_DECRYPTION) {
> -			memcpy(areq->iv, chan->backup_iv, ivsize);
> -			memzero_explicit(chan->backup_iv, ivsize);
> +			memcpy(areq->iv, rctx->backup_iv, ivsize);
> +			memzero_explicit(rctx->backup_iv, ivsize);
>  		} else {
>  			scatterwalk_map_and_copy(areq->iv, areq->dst, offset,
>  						 ivsize, 0);
>  		}
> -		memzero_explicit(chan->bounce_iv, ivsize);
> +		memzero_explicit(rctx->bounce_iv, ivsize);
>  	}
>  
>  	dma_unmap_single(ce->dev, rctx->addr_key, op->keylen, DMA_TO_DEVICE);
> @@ -348,13 +348,13 @@ static void sun8i_ce_cipher_unprepare(struct crypto_engine *engine,
>  					 DMA_TO_DEVICE);
>  		offset = areq->cryptlen - ivsize;
>  		if (rctx->op_dir & CE_DECRYPTION) {
> -			memcpy(areq->iv, chan->backup_iv, ivsize);
> -			memzero_explicit(chan->backup_iv, ivsize);
> +			memcpy(areq->iv, rctx->backup_iv, ivsize);
> +			memzero_explicit(rctx->backup_iv, ivsize);
>  		} else {
>  			scatterwalk_map_and_copy(areq->iv, areq->dst, offset,
>  						 ivsize, 0);
>  		}
> -		memzero_explicit(chan->bounce_iv, ivsize);
> +		memzero_explicit(rctx->bounce_iv, ivsize);
>  	}
>  
>  	dma_unmap_single(ce->dev, rctx->addr_key, op->keylen, DMA_TO_DEVICE);
> diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
> index 79ec172e5c99..930a6579d853 100644
> --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
> +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
> @@ -757,18 +757,6 @@ static int sun8i_ce_allocate_chanlist(struct sun8i_ce_dev *ce)
>  			err = -ENOMEM;
>  			goto error_engine;
>  		}
> -		ce->chanlist[i].bounce_iv = devm_kmalloc(ce->dev, AES_BLOCK_SIZE,
> -							 GFP_KERNEL | GFP_DMA);
> -		if (!ce->chanlist[i].bounce_iv) {
> -			err = -ENOMEM;
> -			goto error_engine;
> -		}
> -		ce->chanlist[i].backup_iv = devm_kmalloc(ce->dev, AES_BLOCK_SIZE,
> -							 GFP_KERNEL);
> -		if (!ce->chanlist[i].backup_iv) {
> -			err = -ENOMEM;
> -			goto error_engine;
> -		}
>  	}
>  	return 0;
>  error_engine:
> diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
> index f12c32d1843f..0d46531c475c 100644
> --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
> +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
> @@ -188,8 +188,6 @@ struct ce_task {
>   * @status:	set to 1 by interrupt if task is done
>   * @t_phy:	Physical address of task
>   * @tl:		pointer to the current ce_task for this flow
> - * @backup_iv:		buffer which contain the next IV to store
> - * @bounce_iv:		buffer which contain the IV
>   * @stat_req:	number of request done by this flow
>   */
>  struct sun8i_ce_flow {
> @@ -198,8 +196,6 @@ struct sun8i_ce_flow {
>  	int status;
>  	dma_addr_t t_phy;
>  	struct ce_task *tl;
> -	void *backup_iv;
> -	void *bounce_iv;
>  #ifdef CONFIG_CRYPTO_DEV_SUN8I_CE_DEBUG
>  	unsigned long stat_req;
>  #endif
> @@ -264,6 +260,8 @@ static inline __le32 desc_addr_val_le32(struct sun8i_ce_dev *dev,
>   * @nr_sgd:		The number of destination SG (as given by dma_map_sg())
>   * @addr_iv:		The IV addr returned by dma_map_single, need to unmap later
>   * @addr_key:		The key addr returned by dma_map_single, need to unmap later
> + * @bounce_iv:		Current IV buffer
> + * @backup_iv:		Next IV buffer
>   * @fallback_req:	request struct for invoking the fallback skcipher TFM
>   */
>  struct sun8i_cipher_req_ctx {
> @@ -273,6 +271,8 @@ struct sun8i_cipher_req_ctx {
>  	int nr_sgd;
>  	dma_addr_t addr_iv;
>  	dma_addr_t addr_key;
> +	u8 bounce_iv[AES_BLOCK_SIZE] ____cacheline_aligned;
> +	u8 backup_iv[AES_BLOCK_SIZE] ____cacheline_aligned;
>  	struct skcipher_request fallback_req;   // keep at the end

Hello

Are you sure you could do DMA on sun8i_cipher_req_ctx ?

Regards