Chain crypto requests together at the DMA level

Adding a macro constant to be used for the size of the crypto queue, instead of using a numeric value directly. It will be easier to maintain in case we add more than one crypto queue of the same size.

Signed-off-by Romain Perier <[email protected]>

---
 drivers/crypto/marvell/cesa.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

drivers/crypto/marvell/cesa.c +4 -1

index 056a754..fb403e1 100644
@@ -31,6 +31,9 @@
 
 #include "cesa.h"
 
+/* Limit of the crypto queue before reaching the backlog */
+#define CESA_CRYPTO_DEFAULT_MAX_QLEN 50
+
 static int allhwsupport = !IS_ENABLED(CONFIG_CRYPTO_DEV_MV_CESA);
 module_param_named(allhwsupport, allhwsupport, int, 0444);
 MODULE_PARM_DESC(allhwsupport, "Enable support for all hardware (even it if overlaps with the mv_cesa driver)");

@@ -416,7 +419,7 @@ static int mv_cesa_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	spin_lock_init(&cesa->lock);
-	crypto_init_queue(&cesa->queue, 50);
+	crypto_init_queue(&cesa->queue, CESA_CRYPTO_DEFAULT_MAX_QLEN);
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
 	cesa->regs = devm_ioremap_resource(dev, res);
 	if (IS_ERR(cesa->regs))

Adding BUG_ON() macro to be sure that the step operation is not about to activate a request on the engine if the corresponding engine is already processing a crypto request. This is helpful when the support for chaining crypto requests will be added. Instead of hanging the system when the engine is in an incoherent state, we add this macro

which throws an understandable error.

Signed-off-by Romain Perier <[email protected]>

---
 drivers/crypto/marvell/cipher.c | 2 ++
 drivers/crypto/marvell/hash.c   | 2 ++
 drivers/crypto/marvell/tdma.c   | 2 ++
 3 files changed, 6 insertions(+)

drivers/crypto/marvell/cipher.c +0 -0

index dcf1fce..8d0fabb 100644

drivers/crypto/marvell/tdma.c +0 -0

 	writel(CESA_SA_CMD_EN_CESA_SA_ACCL0, engine->regs + CESA_SA_CMD);
 }
 

"
Add a BUG_ON() call when the driver tries to launch a crypto request while the engine is still processing the previous one. This replaces a silent system hang by a verbose kernel panic with the associated backtrace to let the user know that something went wrong in the CESA driver.
"

Quoted 13 lines

---
  drivers/crypto/marvell/cipher.c | 2 ++
  drivers/crypto/marvell/hash.c   | 2 ++
  drivers/crypto/marvell/tdma.c   | 2 ++
  3 files changed, 6 insertions(+)

drivers/crypto/marvell/cipher.c +0 -0

index dcf1fce..8d0fabb 100644
@@ -106,6 +106,8 @@ static void mv_cesa_ablkcipher_std_step(struct ablkcipher_request *req)
 

drivers/crypto/marvell/cipher.c static void mv_cesa_ablkcipher_std_step(struct ablkcipher_request *req) · line 110

Quoted 4 lines

drivers/crypto/marvell/cipher.c +2 -0

  	mv_cesa_set_int_mask(engine, CESA_SA_INT_ACCEL0_DONE);
  	writel_relaxed(CESA_SA_CFG_PARA_DIS, engine->regs + CESA_SA_CFG);
+	BUG_ON(readl(engine->regs + CESA_SA_CMD)
+				  & CESA_SA_CMD_EN_CESA_SA_ACCL0);

Nit: please put the '&' operator at the end of the first line and align CESA_SA_CMD_EN_CESA_SA_ACCL0 on the open parenthesis.

	BUG_ON(readl(engine->regs + CESA_SA_CMD) &
	       CESA_SA_CMD_EN_CESA_SA_ACCL0);

Quoted 9 lines

drivers/crypto/marvell/cipher.c +0 -0

  	writel(CESA_SA_CMD_EN_CESA_SA_ACCL0, engine->regs + CESA_SA_CMD);
  }
 

drivers/crypto/marvell/hash.c +0 -0

index 7ca2e0f..0fae351 100644
@@ -237,6 +237,8 @@ static void mv_cesa_ahash_std_step(struct ahash_request *req)
 

drivers/crypto/marvell/hash.c static void mv_cesa_ahash_std_step(struct ahash_request *req) · line 241

Quoted 4 lines

drivers/crypto/marvell/hash.c +2 -0

  	mv_cesa_set_int_mask(engine, CESA_SA_INT_ACCEL0_DONE);
  	writel_relaxed(CESA_SA_CFG_PARA_DIS, engine->regs + CESA_SA_CFG);
+	BUG_ON(readl(engine->regs + CESA_SA_CMD)
+				  & CESA_SA_CMD_EN_CESA_SA_ACCL0);

Ditto.

Quoted 4 lines

drivers/crypto/marvell/hash.c +0 -0

  	writel(CESA_SA_CMD_EN_CESA_SA_ACCL0, engine->regs + CESA_SA_CMD);
  }
 

drivers/crypto/marvell/tdma.c void mv_cesa_dma_step(struct mv_cesa_tdma_req *dreq) · line 57

Quoted 10 lines

drivers/crypto/marvell/tdma.c +2 -0

index 7642798..d493714 100644
@@ -53,6 +53,8 @@ void mv_cesa_dma_step(struct mv_cesa_tdma_req *dreq)
  		       engine->regs + CESA_SA_CFG);
  	writel_relaxed(dreq->chain.first->cur_dma,
  		       engine->regs + CESA_TDMA_NEXT_ADDR);
+	BUG_ON(readl(engine->regs + CESA_SA_CMD)
+				  & CESA_SA_CMD_EN_CESA_SA_ACCL0);

Ditto.

Adding a TDMA descriptor at the end of the request for copying the output IV vector via a DMA transfer. This is required for processing cipher requests asynchroniously in chained mode, otherwise the content

of the IV vector will be overwriten for each new finished request.

Signed-off-by Romain Perier <[email protected]>

---
 drivers/crypto/marvell/cesa.c   |  4 ++++
 drivers/crypto/marvell/cesa.h   |  5 +++++
 drivers/crypto/marvell/cipher.c | 40 +++++++++++++++++++++++++++-------------
 drivers/crypto/marvell/tdma.c   | 29 +++++++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 13 deletions(-)

drivers/crypto/marvell/cesa.c +4 -0

index fb403e1..93700cd 100644
@@ -312,6 +312,10 @@ 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;

drivers/crypto/marvell/cesa.h +0 -0

index 74071e4..74b84bd 100644

drivers/crypto/marvell/cesa.h +4 -0

 /**
  * struct mv_cesa_tdma_desc - TDMA descriptor

@@ -390,6 +391,7 @@ 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;
 };
 
 /**

@@ -790,6 +792,9 @@ 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,
+			  u32 size, u32 flags, gfp_t gfp_flags);
+
 struct mv_cesa_op_ctx *mv_cesa_dma_add_op(struct mv_cesa_tdma_chain *chain,
 					const struct mv_cesa_op_ctx *op_templ,
 					bool skip_ctx,

drivers/crypto/marvell/cipher.c +10 -4

index 8d0fabb..f42620e 100644
@@ -118,6 +118,7 @@ static int mv_cesa_ablkcipher_std_process(struct ablkcipher_request *req,
 	struct mv_cesa_ablkcipher_std_req *sreq = &creq->req.std;
 	struct mv_cesa_engine *engine = sreq->base.engine;
 	size_t len;
+	unsigned int ivsize;
 
 	len = sg_pcopy_from_buffer(req->dst, creq->dst_nents,
 				   engine->sram + CESA_SA_DATA_SRAM_OFFSET,

@@ -127,6 +128,10 @@ static int mv_cesa_ablkcipher_std_process(struct ablkcipher_request *req,
 	if (sreq->offset < req->nbytes)
 		return -EINPROGRESS;
 
+	ivsize = crypto_ablkcipher_ivsize(crypto_ablkcipher_reqtfm(req));
+	memcpy_fromio(req->info,
+		      engine->sram + CESA_SA_CRYPT_IV_SRAM_OFFSET, ivsize);
+
 	return 0;
 }
 

@@ -135,23 +140,23 @@ static int mv_cesa_ablkcipher_process(struct crypto_async_request *req,
 {
 	struct ablkcipher_request *ablkreq = ablkcipher_request_cast(req);
 	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(ablkreq);
-	struct mv_cesa_ablkcipher_std_req *sreq = &creq->req.std;
-	struct mv_cesa_engine *engine = sreq->base.engine;
-	int ret;
 
-	if (creq->req.base.type == CESA_DMA_REQ)
+	if (creq->req.base.type == CESA_DMA_REQ) {
+		int ret;
+		struct mv_cesa_tdma_req *dreq;
+		unsigned int ivsize;
+

drivers/crypto/marvell/cipher.c +9 -0

 static void mv_cesa_ablkcipher_step(struct crypto_async_request *req)

@@ -302,6 +307,7 @@ static int mv_cesa_ablkcipher_dma_req_init(struct ablkcipher_request *req,
 	struct mv_cesa_tdma_chain chain;
 	bool skip_ctx = false;
 	int ret;
+	unsigned int ivsize;
 
 	dreq->base.type = CESA_DMA_REQ;
 	dreq->chain.first = NULL;

@@ -360,6 +366,14 @@ static int mv_cesa_ablkcipher_dma_req_init(struct ablkcipher_request *req,
 
 	} while (mv_cesa_ablkcipher_req_iter_next_op(&iter));
 
+	/* Add output data for IV */
+	ivsize = crypto_ablkcipher_ivsize(crypto_ablkcipher_reqtfm(req));
+	ret = mv_cesa_dma_add_iv_op(&chain, CESA_SA_CRYPT_IV_SRAM_OFFSET,
+				    ivsize, CESA_TDMA_SRC_IN_SRAM, flags);
+
+	if (ret)
+		goto err_free_tdma;
+
 	dreq->chain = chain;
 
 	return 0;

drivers/crypto/marvell/tdma.c +0 -0

index d493714..88c87be 100644

drivers/crypto/marvell/tdma.c +2 -0

+			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,

drivers/crypto/marvell/tdma.c +7 -0

+	dma_addr_t dma_handle;
+
+	tdma = mv_cesa_dma_add_desc(chain, gfp_flags);
+	if (IS_ERR(tdma))
+		return PTR_ERR(tdma);
+
+	cache = dma_pool_alloc(cesa_dev->dma->iv_pool, flags, &dma_handle);

drivers/crypto/marvell/tdma.c +3 -0

+	return 0;
+}
+
 struct mv_cesa_op_ctx *mv_cesa_dma_add_op(struct mv_cesa_tdma_chain *chain,
 					const struct mv_cesa_op_ctx *op_templ,
 					bool skip_ctx,

Quoted 3 lines · On Wed, 15 Jun 2016 21:15:30 +0200 Romain Perier…

Adding a TDMA descriptor at the end of the request for copying the output IV vector via a DMA transfer. This is required for processing cipher requests asynchroniously in chained mode, otherwise the content

asynchronously

Quoted 1 line

of the IV vector will be overwriten for each new finished request.

BTW, Not sure the term 'asynchronously' is appropriate here. The standard (AKA non-DMA) processing is also asynchronous. The real reason here is that you want to chain the requests and offload as much processing as possible to the DMA and crypto engine. And as you explained, this is only possible if we retrieve the updated IV using DMA.

drivers/crypto/marvell/cesa.h struct mv_cesa_op_ctx { · line 278

Quoted 9 lines

drivers/crypto/marvell/cesa.h +1 -0

index 74071e4..74b84bd 100644
@@ -275,6 +275,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				4

Should be 3 and not 4: TDMA_TYPE is an enum, not a bit field.

Sometime it's better to offend the < 80 characters rule than doing funky stuff ;).

drivers/crypto/marvell/cesa.h struct mv_cesa_op_ctx {

Quoted 10 lines

drivers/crypto/marvell/cesa.h +2 -6

+		memcpy_fromio(ablkreq->info, dreq->chain.last->data, ivsize);
  		return ret;
-
-	memcpy_fromio(ablkreq->info,
-		      engine->sram + CESA_SA_CRYPT_IV_SRAM_OFFSET,
-		      crypto_ablkcipher_ivsize(crypto_ablkcipher_reqtfm(ablkreq)));
-
-	return 0;
+	}

Missing blank line.

drivers/crypto/marvell/cesa.h

Quoted 2 lines

drivers/crypto/marvell/cesa.h +1 -0

+	return mv_cesa_ablkcipher_std_process(ablkreq, status);

This version is more readable IMHO:

	struct mv_cesa_tdma_req *dreq;
	unsigned int ivsize;
	int ret;

	if (creq->req.base.type == CESA_STD_REQ)
		return mv_cesa_ablkcipher_std_process(ablkreq, status);

	ret = mv_cesa_dma_process(&creq->req.dma, status);
	if (ret)
		return ret;

	dreq = &creq->req.dma;
	ivsize =
	crypto_ablkcipher_ivsize(crypto_ablkcipher_reqtfm(ablkreq));
	memcpy_fromio(ablkreq->info, dreq->chain.last->data, ivsize);

	return 0;

Quoted 30 lines

static void mv_cesa_ablkcipher_step(struct crypto_async_request *req)

drivers/crypto/marvell/cesa.h +9 -0

@@ -302,6 +307,7 @@ static int mv_cesa_ablkcipher_dma_req_init(struct ablkcipher_request *req,
  	struct mv_cesa_tdma_chain chain;
  	bool skip_ctx = false;
  	int ret;
+	unsigned int ivsize;
 
  	dreq->base.type = CESA_DMA_REQ;
  	dreq->chain.first = NULL;

@@ -360,6 +366,14 @@ static int mv_cesa_ablkcipher_dma_req_init(struct ablkcipher_request *req,
 
  	} while (mv_cesa_ablkcipher_req_iter_next_op(&iter));
 
+	/* Add output data for IV */
+	ivsize = crypto_ablkcipher_ivsize(crypto_ablkcipher_reqtfm(req));
+	ret = mv_cesa_dma_add_iv_op(&chain, CESA_SA_CRYPT_IV_SRAM_OFFSET,
+				    ivsize, CESA_TDMA_SRC_IN_SRAM, flags);
+
+	if (ret)
+		goto err_free_tdma;
+
  	dreq->chain = chain;
 
  	return 0;

drivers/crypto/marvell/tdma.c +0 -0

index d493714..88c87be 100644

drivers/crypto/marvell/tdma.c void mv_cesa_dma_cleanup(struct mv_cesa_tdma_req *dreq) · line 68

Quoted 1 line

drivers/crypto/marvell/tdma.c +0 -0

@@ -68,6 +68,9 @@ void mv_cesa_dma_cleanup(struct mv_cesa_tdma_req *dreq)
  		if (tdma->flags & CESA_TDMA_OP)

I realize this test is wrong.

It should be

		type = tdma->flags & CESA_TDMA_TYPE_MSK;
		if (type == CESA_TDMA_OP)

drivers/crypto/marvell/tdma.c void mv_cesa_dma_cleanup(struct mv_cesa_tdma_req *dreq) · line 71

Quoted 3 lines

drivers/crypto/marvell/tdma.c +1 -0

  			dma_pool_free(cesa_dev->dma->op_pool, tdma->op,
  				      le32_to_cpu(tdma->src));
+		else if (tdma->flags & CESA_TDMA_IV)

and here

		else if (type == CESA_TDMA_IV)

Quoted 9 lines

drivers/crypto/marvell/tdma.c +2 -0

+			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,

@@ -120,6 +123,32 @@ mv_cesa_dma_add_desc(struct mv_cesa_tdma_chain *chain, gfp_t flags)
  	return new_tdma;
  }
 

drivers/crypto/marvell/tdma.c mv_cesa_dma_add_desc(struct mv_cesa_tdma_chain *chain, gfp_t flags) · line 131

Quoted 6 lines

drivers/crypto/marvell/tdma.c +6 -0

+int mv_cesa_dma_add_iv_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 *cache;

Why do you name that one cache? iv would be a better name.

Quoted 13 lines

drivers/crypto/marvell/tdma.c +12 -0

+	dma_addr_t dma_handle;
+
+	tdma = mv_cesa_dma_add_desc(chain, gfp_flags);
+	if (IS_ERR(tdma))
+		return PTR_ERR(tdma);
+
+	cache = dma_pool_alloc(cesa_dev->dma->iv_pool, flags, &dma_handle);
+	if (!cache)
+		return -ENOMEM;
+
+	tdma->byte_cnt = cpu_to_le32(size | BIT(31));
+	tdma->src = src;

drivers/crypto/marvell/tdma.c mv_cesa_dma_add_desc(struct mv_cesa_tdma_chain *chain, gfp_t flags) · line 148

Quoted 5 lines

drivers/crypto/marvell/tdma.c +5 -0

+	tdma->dst = cpu_to_le32(dma_handle);
+	tdma->data = cache;
+
+	flags &= (CESA_TDMA_DST_IN_SRAM | CESA_TDMA_SRC_IN_SRAM);
+	tdma->flags = flags | CESA_TDMA_DATA | CESA_TDMA_IV;

You should not mix 2 different types, it's either CESA_TDMA_DATA or CESA_TDMA_IV, and in this case it should be CESA_TDMA_IV.

drivers/crypto/marvell/tdma.c mv_cesa_dma_add_desc(struct mv_cesa_tdma_chain *chain, gfp_t flags) · line 154

Quoted 7 lines

drivers/crypto/marvell/tdma.c +3 -0

+	return 0;
+}
+
  struct mv_cesa_op_ctx *mv_cesa_dma_add_op(struct mv_cesa_tdma_chain *chain,
  					const struct mv_cesa_op_ctx *op_templ,
  					bool skip_ctx,

Quoted 12 lines

drivers/crypto/marvell/cesa.h struct mv_cesa_op_ctx { · line 278

Quoted 10 lines

drivers/crypto/marvell/cesa.h +1 -0

index 74071e4..74b84bd 100644
@@ -275,6 +275,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				4

Should be 3 and not 4: TDMA_TYPE is an enum, not a bit field.

Ok

Quoted 3 lines

Sometime it's better to offend the < 80 characters rule than doing funky stuff ;).

I just wanted to make checkpatch happy :D

drivers/crypto/marvell/cipher.c +5 -4

@@ -135,23 +140,23 @@ static int mv_cesa_ablkcipher_process(struct crypto_async_request *req,
 {
 	struct ablkcipher_request *ablkreq = ablkcipher_request_cast(req);
 	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(ablkreq);
-	struct mv_cesa_ablkcipher_std_req *sreq = &creq->req.std;
-	struct mv_cesa_engine *engine = sreq->base.engine;
-	int ret;
 
-	if (creq->req.base.type == CESA_DMA_REQ)
+	if (creq->req.base.type == CESA_DMA_REQ) {
+		int ret;
+		struct mv_cesa_tdma_req *dreq;
+		unsigned int ivsize;
+
 		ret = mv_cesa_dma_process(&creq->req.dma, status);

Actually the only way to access the tdma chain is to use the 'req' union

from a mv_cesa_{ablkcipher,ahash}. This will soon become a problem if we want to handle the TDMA chaining vs standard/non-DMA processing in a generic way (with generic functions at the cesa.c level detecting whether the request should be queued at the DMA level or not). Hence the decision to move the chain field a the mv_cesa_req level at the expense

of adding 2 void * fields to all request contexts (including non-DMA ones). To limit the overhead, we get rid of the type field, which can now be deduced from the req->chain.first value.

Signed-off-by Romain Perier <[email protected]>

---
 drivers/crypto/marvell/cesa.c   |  3 ++-
 drivers/crypto/marvell/cesa.h   | 31 +++++++++++++------------------
 drivers/crypto/marvell/cipher.c | 40 ++++++++++++++++++++++------------------
 drivers/crypto/marvell/hash.c   | 36 +++++++++++++++---------------------
 drivers/crypto/marvell/tdma.c   |  8 ++++----
 5 files changed, 56 insertions(+), 62 deletions(-)

drivers/crypto/marvell/cesa.c +2 -1

index 93700cd..fe04d1b 100644
@@ -111,7 +111,8 @@ static irqreturn_t mv_cesa_int(int irq, void *priv)
 	return ret;
 }
 
-int mv_cesa_queue_req(struct crypto_async_request *req)
+int mv_cesa_queue_req(struct crypto_async_request *req,
+		      struct mv_cesa_req *creq)
 {
 	int ret;
 	int i;

drivers/crypto/marvell/cesa.h +0 -0

index 74b84bd..158ff82 100644

drivers/crypto/marvell/cesa.h +0 -10

  */
 struct mv_cesa_req {
-	enum mv_cesa_req_type type;
 	struct mv_cesa_engine *engine;
-};
-
-/**
- * struct mv_cesa_tdma_req - CESA TDMA request
- * @base:	base information
- * @chain:	TDMA chain
- */
-struct mv_cesa_tdma_req {
-	struct mv_cesa_req base;
 	struct mv_cesa_tdma_chain chain;
 };
 

drivers/crypto/marvell/cesa.h +12 -6

 	} req;
 	int src_nents;

@@ -587,7 +576,6 @@ struct mv_cesa_ahash_std_req {
  * @cache_dma:		DMA address of the cache buffer
  */
 struct mv_cesa_ahash_dma_req {
-	struct mv_cesa_tdma_req base;
 	u8 *padding;
 	dma_addr_t padding_dma;
 	u8 *cache;

@@ -625,6 +613,12 @@ struct mv_cesa_ahash_req {
 
 extern struct mv_cesa_dev *cesa_dev;
 
+static inline enum mv_cesa_req_type
+mv_cesa_req_get_type(struct mv_cesa_req *req)
+{
+	return req->chain.first ? CESA_DMA_REQ : CESA_STD_REQ;
+}
+
 static inline void mv_cesa_update_op_cfg(struct mv_cesa_op_ctx *op,
 					 u32 cfg, u32 mask)
 {

@@ -697,7 +691,8 @@ static inline bool mv_cesa_mac_op_is_first_frag(const struct mv_cesa_op_ctx *op)
 		CESA_SA_DESC_CFG_FIRST_FRAG;
 }
 
-int mv_cesa_queue_req(struct crypto_async_request *req);
+int mv_cesa_queue_req(struct crypto_async_request *req,
+		      struct mv_cesa_req *creq);
 
 /*
  * Helper function that indicates whether a crypto request needs to be

@@ -767,9 +762,9 @@ static inline bool mv_cesa_req_dma_iter_next_op(struct mv_cesa_dma_iter *iter)
 	return iter->op_len;
 }
 
-void mv_cesa_dma_step(struct mv_cesa_tdma_req *dreq);
+void mv_cesa_dma_step(struct mv_cesa_req *dreq);
 
-static inline int mv_cesa_dma_process(struct mv_cesa_tdma_req *dreq,
+static inline int mv_cesa_dma_process(struct mv_cesa_req *dreq,
 				      u32 status)
 {
 	if (!(status & CESA_SA_INT_ACC0_IDMA_DONE))

@@ -781,10 +776,10 @@ static inline int mv_cesa_dma_process(struct mv_cesa_tdma_req *dreq,
 	return 0;
 }
 
-void mv_cesa_dma_prepare(struct mv_cesa_tdma_req *dreq,
+void mv_cesa_dma_prepare(struct mv_cesa_req *dreq,
 			 struct mv_cesa_engine *engine);
+void mv_cesa_dma_cleanup(struct mv_cesa_req *dreq);
 
-void mv_cesa_dma_cleanup(struct mv_cesa_tdma_req *dreq);
 
 static inline void
 mv_cesa_tdma_desc_iter_init(struct mv_cesa_tdma_chain *chain)

drivers/crypto/marvell/cipher.c +2 -2

index f42620e..15d2c5a 100644
@@ -70,14 +70,14 @@ mv_cesa_ablkcipher_dma_cleanup(struct ablkcipher_request *req)
 		dma_unmap_sg(cesa_dev->dev, req->src, creq->src_nents,
 			     DMA_BIDIRECTIONAL);
 	}
-	mv_cesa_dma_cleanup(&creq->req.dma);
+	mv_cesa_dma_cleanup(&creq->req.base);
 }
 
 static inline void mv_cesa_ablkcipher_cleanup(struct ablkcipher_request *req)
 {
 	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(req);
 
-	if (creq->req.base.type == CESA_DMA_REQ)
+	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)
 		mv_cesa_ablkcipher_dma_cleanup(req);
 }
 

@@ -141,19 +141,19 @@ static int mv_cesa_ablkcipher_process(struct crypto_async_request *req,
 	struct ablkcipher_request *ablkreq = ablkcipher_request_cast(req);
 	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(ablkreq);

drivers/crypto/marvell/cipher.c +4 -4

 		if (ret)
 			return ret;
 
-		dreq = &creq->req.dma;
+		basereq = &creq->req.base;
 		ivsize = crypto_ablkcipher_ivsize(
 					     crypto_ablkcipher_reqtfm(ablkreq));
-		memcpy_fromio(ablkreq->info, dreq->chain.last->data, ivsize);
+		memcpy_fromio(ablkreq->info, basereq->chain.last->data, ivsize);
 		return ret;
 	}
 	return mv_cesa_ablkcipher_std_process(ablkreq, status);

@@ -164,8 +164,8 @@ static void mv_cesa_ablkcipher_step(struct crypto_async_request *req)
 	struct ablkcipher_request *ablkreq = ablkcipher_request_cast(req);
 	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(ablkreq);
 
-	if (creq->req.base.type == CESA_DMA_REQ)
-		mv_cesa_dma_step(&creq->req.dma);
+	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)
+		mv_cesa_dma_step(&creq->req.base);
 	else
 		mv_cesa_ablkcipher_std_step(ablkreq);
 }

drivers/crypto/marvell/cipher.c +2 -2

-	mv_cesa_dma_prepare(dreq, dreq->base.engine);
+	mv_cesa_dma_prepare(dreq, dreq->engine);
 }
 
 static inline void

@@ -199,7 +199,7 @@ static inline void mv_cesa_ablkcipher_prepare(struct crypto_async_request *req,
 	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(ablkreq);
 	creq->req.base.engine = engine;
 
-	if (creq->req.base.type == CESA_DMA_REQ)
+	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)
 		mv_cesa_ablkcipher_dma_prepare(ablkreq);
 	else
 		mv_cesa_ablkcipher_std_prepare(ablkreq);

drivers/crypto/marvell/cipher.c +9 -5

 	struct mv_cesa_ablkcipher_dma_iter iter;
 	struct mv_cesa_tdma_chain chain;
 	bool skip_ctx = false;
 	int ret;
 	unsigned int ivsize;
 
-	dreq->base.type = CESA_DMA_REQ;
 	dreq->chain.first = NULL;
 	dreq->chain.last = NULL;
 

@@ -397,10 +396,12 @@ mv_cesa_ablkcipher_std_req_init(struct ablkcipher_request *req,
 {
 	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(req);
 	struct mv_cesa_ablkcipher_std_req *sreq = &creq->req.std;
+	struct mv_cesa_req *basereq = &creq->req.base;
 
-	sreq->base.type = CESA_STD_REQ;
 	sreq->op = *op_templ;
 	sreq->skip_ctx = false;
+	basereq->chain.first = NULL;
+	basereq->chain.last = NULL;
 
 	return 0;
 }

@@ -442,6 +443,7 @@ static int mv_cesa_ablkcipher_req_init(struct ablkcipher_request *req,
 static int mv_cesa_des_op(struct ablkcipher_request *req,
 			  struct mv_cesa_op_ctx *tmpl)
 {
+	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(req);
 	struct mv_cesa_des_ctx *ctx = crypto_tfm_ctx(req->base.tfm);
 	int ret;
 

@@ -454,7 +456,7 @@ static int mv_cesa_des_op(struct ablkcipher_request *req,
 	if (ret)
 		return ret;
 
-	ret = mv_cesa_queue_req(&req->base);
+	ret = mv_cesa_queue_req(&req->base, &creq->req.base);
 	if (mv_cesa_req_needs_cleanup(&req->base, ret))
 		mv_cesa_ablkcipher_cleanup(req);
 

@@ -562,6 +564,7 @@ struct crypto_alg mv_cesa_cbc_des_alg = {
 static int mv_cesa_des3_op(struct ablkcipher_request *req,
 			   struct mv_cesa_op_ctx *tmpl)
 {
+	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(req);
 	struct mv_cesa_des3_ctx *ctx = crypto_tfm_ctx(req->base.tfm);
 	int ret;
 

@@ -574,7 +577,7 @@ static int mv_cesa_des3_op(struct ablkcipher_request *req,
 	if (ret)
 		return ret;
 
-	ret = mv_cesa_queue_req(&req->base);
+	ret = mv_cesa_queue_req(&req->base, &creq->req.base);
 	if (mv_cesa_req_needs_cleanup(&req->base, ret))
 		mv_cesa_ablkcipher_cleanup(req);
 

@@ -688,6 +691,7 @@ struct crypto_alg mv_cesa_cbc_des3_ede_alg = {
 static int mv_cesa_aes_op(struct ablkcipher_request *req,
 			  struct mv_cesa_op_ctx *tmpl)
 {
+	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(req);
 	struct mv_cesa_aes_ctx *ctx = crypto_tfm_ctx(req->base.tfm);
 	int ret, i;
 	u32 *key;

@@ -716,7 +720,7 @@ static int mv_cesa_aes_op(struct ablkcipher_request *req,
 	if (ret)
 		return ret;
 
-	ret = mv_cesa_queue_req(&req->base);
+	ret = mv_cesa_queue_req(&req->base, &creq->req.base);
 	if (mv_cesa_req_needs_cleanup(&req->base, ret))
 		mv_cesa_ablkcipher_cleanup(req);
 

drivers/crypto/marvell/hash.c +3 -3

index 0fae351..cc7c5b0 100644
@@ -103,14 +103,14 @@ static inline void mv_cesa_ahash_dma_cleanup(struct ahash_request *req)
 
 	dma_unmap_sg(cesa_dev->dev, req->src, creq->src_nents, DMA_TO_DEVICE);
 	mv_cesa_ahash_dma_free_cache(&creq->req.dma);
-	mv_cesa_dma_cleanup(&creq->req.dma.base);
+	mv_cesa_dma_cleanup(&creq->req.base);
 }
 
 static inline void mv_cesa_ahash_cleanup(struct ahash_request *req)
 {
 	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
 
-	if (creq->req.base.type == CESA_DMA_REQ)
+	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)
 		mv_cesa_ahash_dma_cleanup(req);
 }
 

@@ -118,7 +118,7 @@ static void mv_cesa_ahash_last_cleanup(struct ahash_request *req)
 {
 	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
 
-	if (creq->req.base.type == CESA_DMA_REQ)
+	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)
 		mv_cesa_ahash_dma_last_cleanup(req);
 }
 

drivers/crypto/marvell/hash.c +6 -6

-	mv_cesa_dma_prepare(dreq, dreq->base.engine);
+	mv_cesa_dma_prepare(dreq, dreq->engine);
 }
 
 static void mv_cesa_ahash_std_prepare(struct ahash_request *req)

@@ -277,8 +277,8 @@ 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 (creq->req.base.type == CESA_DMA_REQ)
-		mv_cesa_dma_step(&creq->req.dma.base);
+	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)
+		mv_cesa_dma_step(&creq->req.base);
 	else
 		mv_cesa_ahash_std_step(ahashreq);
 }

@@ -291,8 +291,8 @@ static int mv_cesa_ahash_process(struct crypto_async_request *req, u32 status)
 	unsigned int digsize;
 	int ret, i;
 
-	if (creq->req.base.type == CESA_DMA_REQ)
-		ret = mv_cesa_dma_process(&creq->req.dma.base, status);
+	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)
+		ret = mv_cesa_dma_process(&creq->req.base, status);
 	else
 		ret = mv_cesa_ahash_std_process(ahashreq, status);
 

@@ -340,7 +340,7 @@ static void mv_cesa_ahash_prepare(struct crypto_async_request *req,
 
 	creq->req.base.engine = engine;
 
-	if (creq->req.base.type == CESA_DMA_REQ)
+	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)
 		mv_cesa_ahash_dma_prepare(ahashreq);
 	else
 		mv_cesa_ahash_std_prepare(ahashreq);

drivers/crypto/marvell/hash.c +3 -3

 		ret = mv_cesa_ahash_dma_req_init(req);
 
 	return ret;

@@ -700,7 +694,7 @@ static int mv_cesa_ahash_update(struct ahash_request *req)
 	if (cached)
 		return 0;
 
-	ret = mv_cesa_queue_req(&req->base);
+	ret = mv_cesa_queue_req(&req->base, &creq->req.base);
 	if (mv_cesa_req_needs_cleanup(&req->base, ret))
 		mv_cesa_ahash_cleanup(req);
 

@@ -725,7 +719,7 @@ static int mv_cesa_ahash_final(struct ahash_request *req)
 	if (cached)
 		return 0;
 
-	ret = mv_cesa_queue_req(&req->base);
+	ret = mv_cesa_queue_req(&req->base, &creq->req.base);
 	if (mv_cesa_req_needs_cleanup(&req->base, ret))
 		mv_cesa_ahash_cleanup(req);
 

@@ -750,7 +744,7 @@ static int mv_cesa_ahash_finup(struct ahash_request *req)
 	if (cached)
 		return 0;
 
-	ret = mv_cesa_queue_req(&req->base);
+	ret = mv_cesa_queue_req(&req->base, &creq->req.base);
 	if (mv_cesa_req_needs_cleanup(&req->base, ret))
 		mv_cesa_ahash_cleanup(req);
 

drivers/crypto/marvell/tdma.c +4 -4

index 88c87be..9a424f9 100644
@@ -37,9 +37,9 @@ bool mv_cesa_req_dma_iter_next_transfer(struct mv_cesa_dma_iter *iter,
 	return true;
 }
 
-void mv_cesa_dma_step(struct mv_cesa_tdma_req *dreq)
+void mv_cesa_dma_step(struct mv_cesa_req *dreq)
 {
-	struct mv_cesa_engine *engine = dreq->base.engine;
+	struct mv_cesa_engine *engine = dreq->engine;
 
 	writel_relaxed(0, engine->regs + CESA_SA_CFG);
 

@@ -58,7 +58,7 @@ void mv_cesa_dma_step(struct mv_cesa_tdma_req *dreq)
 	writel(CESA_SA_CMD_EN_CESA_SA_ACCL0, engine->regs + CESA_SA_CMD);
 }
 
-void mv_cesa_dma_cleanup(struct mv_cesa_tdma_req *dreq)
+void mv_cesa_dma_cleanup(struct mv_cesa_req *dreq)
 {
 	struct mv_cesa_tdma_desc *tdma;
 

@@ -81,7 +81,7 @@ void mv_cesa_dma_cleanup(struct mv_cesa_tdma_req *dreq)
 	dreq->chain.last = NULL;
 }
 
-void mv_cesa_dma_prepare(struct mv_cesa_tdma_req *dreq,
+void mv_cesa_dma_prepare(struct mv_cesa_req *dreq,
 			 struct mv_cesa_engine *engine)
 {
 	struct mv_cesa_tdma_desc *tdma;

Quoted 1 line · On Wed, 15 Jun 2016 21:15:31 +0200 Romain Perier…

Actually the only way to access the tdma chain is to use the 'req' union

Currently, ...

Now that the dma specific fields are part of the base request there's no reason to keep this union.

You can just put struct mv_cesa_req base; directly under struct mv_cesa_ablkcipher_req, and move mv_cesa_ablkcipher_std_req fields in mv_cesa_ablkcipher_req.

Initialize basereq earlier and pass it as the first argument of mv_cesa_dma_process().

drivers/crypto/marvell/cipher.c static inline void · line 177

Quoted 6 lines

drivers/crypto/marvell/cipher.c +1 -1

@@ -174,9 +174,9 @@ static inline void
  mv_cesa_ablkcipher_dma_prepare(struct ablkcipher_request *req)
  {
  	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(req);
-	struct mv_cesa_tdma_req *dreq = &creq->req.dma;
+	struct mv_cesa_req *dreq = &creq->req.base;

You named it basereq in mv_cesa_ablkcipher_step(). Try to be consistent, no matter the name.

Quoted 11 lines

drivers/crypto/marvell/cipher.c +1 -1

-	mv_cesa_dma_prepare(dreq, dreq->base.engine);
+	mv_cesa_dma_prepare(dreq, dreq->engine);
  }
 
  static inline void

@@ -199,7 +199,7 @@ static inline void mv_cesa_ablkcipher_prepare(struct crypto_async_request *req,
  	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(ablkreq);
  	creq->req.base.engine = engine;
 

drivers/crypto/marvell/cipher.c static int mv_cesa_ablkcipher_dma_req_init(struct ablkcipher_request *req, · line 305

Quoted 11 lines

drivers/crypto/marvell/cipher.c +2 -2

-	if (creq->req.base.type == CESA_DMA_REQ)
+	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)
  		mv_cesa_ablkcipher_dma_prepare(ablkreq);
  	else
  		mv_cesa_ablkcipher_std_prepare(ablkreq);

@@ -302,14 +302,13 @@ static int mv_cesa_ablkcipher_dma_req_init(struct ablkcipher_request *req,
  	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(req);
  	gfp_t flags = (req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
  		      GFP_KERNEL : GFP_ATOMIC;
-	struct mv_cesa_tdma_req *dreq = &creq->req.dma;
+	struct mv_cesa_req *dreq = &creq->req.base;

Ditto.

drivers/crypto/marvell/hash.c static int mv_cesa_ahash_std_process(struct ahash_request *req, u32 status) · line 259

Quoted 6 lines

drivers/crypto/marvell/hash.c +1 -1

@@ -256,9 +256,9 @@ static int mv_cesa_ahash_std_process(struct ahash_request *req, u32 status)
  static inline void mv_cesa_ahash_dma_prepare(struct ahash_request *req)
  {
  	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
-	struct mv_cesa_tdma_req *dreq = &creq->req.dma.base;
+	struct mv_cesa_req *dreq = &creq->req.base;

Ditto.

Quoted 4 lines

drivers/crypto/marvell/hash.c +0 -0

@@ -340,7 +340,7 @@ static void mv_cesa_ahash_prepare(struct crypto_async_request *req,
 
  	creq->req.base.engine = engine;
 

drivers/crypto/marvell/hash.c static int mv_cesa_ahash_dma_req_init(struct ahash_request *req) · line 558

Quoted 12 lines

drivers/crypto/marvell/hash.c +2 -3

-	if (creq->req.base.type == CESA_DMA_REQ)
+	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)
  		mv_cesa_ahash_dma_prepare(ahashreq);
  	else
  		mv_cesa_ahash_std_prepare(ahashreq);

@@ -555,8 +555,7 @@ static int mv_cesa_ahash_dma_req_init(struct ahash_request *req)
  	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
  	gfp_t flags = (req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
  		      GFP_KERNEL : GFP_ATOMIC;
-	struct mv_cesa_ahash_dma_req *ahashdreq = &creq->req.dma;
-	struct mv_cesa_tdma_req *dreq = &ahashdreq->base;
+	struct mv_cesa_req *dreq = &creq->req.base;

Ditto.

Quoted 8 lines

drivers/crypto/marvell/hash.c +0 -0

  	struct mv_cesa_ahash_dma_iter iter;
  	struct mv_cesa_op_ctx *op = NULL;
  	unsigned int frag_len;

@@ -662,11 +661,6 @@ 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;
 

drivers/crypto/marvell/hash.c static int mv_cesa_ahash_req_init(struct ahash_request *req, bool *cached) · line 669

Quoted 5 lines

drivers/crypto/marvell/hash.c +0 -5

-	if (cesa_dev->caps->has_tdma)
-		creq->req.base.type = CESA_DMA_REQ;
-	else
-		creq->req.base.type = CESA_STD_REQ;
-

Hm, where is it decided now? I mean, I don't see this test anywhere else in your patch, which means you're now always using standard mode.

Quoted 8 lines

drivers/crypto/marvell/hash.c +0 -0

  	creq->src_nents = sg_nents_for_len(req->src, req->nbytes);
  	if (creq->src_nents < 0) {
  		dev_err(cesa_dev->dev, "Invalid number of src SG");

@@ -680,7 +674,7 @@ static int mv_cesa_ahash_req_init(struct ahash_request *req, bool *cached)
  	if (*cached)
  		return 0;
 

Review static int mv_cesa_ahash_req_init(struct ahash_request *req, bool *cached) · line 677

Quoted 2 lines

(quoted diff) +1 -1

-	if (creq->req.base.type == CESA_DMA_REQ)
+	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)

Should be

	if (cesa_dev->caps->has_tdma)

Quoted 1 line

drivers/crypto/marvell/hash.c +0 -0

  		ret = mv_cesa_ahash_dma_req_init(req);

Quoted 6 lines

Now that the dma specific fields are part of the base request there's no reason to keep this union.

You can just put struct mv_cesa_req base; directly under struct mv_cesa_ablkcipher_req, and move mv_cesa_ablkcipher_std_req fields in mv_cesa_ablkcipher_req.

Well, I think that I might keep the changes related to mv_cesa_tdma_req in this commit (+ put struct mv_cesa_req base; direct under struct mv_cesa_ablkcipher_req) and move the changes related to mv_cesa_ablkcipher_std_req into another commit. What do you think ?

Quoted 8 lines

Quoted 8 lines

 
  	struct mv_cesa_ahash_dma_iter iter;
  	struct mv_cesa_op_ctx *op = NULL;
  	unsigned int frag_len;

drivers/crypto/marvell/hash.c +0 -0

@@ -662,11 +661,6 @@ 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;
 

Quoted 8 lines

drivers/crypto/marvell/hash.c static int mv_cesa_ahash_req_init(struct ahash_request *req, bool *cached) · line 669

Quoted 5 lines

drivers/crypto/marvell/hash.c +0 -5

-	if (cesa_dev->caps->has_tdma)
-		creq->req.base.type = CESA_DMA_REQ;
-	else
-		creq->req.base.type = CESA_STD_REQ;
-  

Hm, where is it decided now? I mean, I don't see this test anywhere else in your patch, which means you're now always using standard mode.

It has been replaced by mv_cesa_req_get_type() + initializing chain.first to NULL in std_init. So, that's the same thing, no ?

Quoted 11 lines

drivers/crypto/marvell/hash.c static int mv_cesa_ahash_req_init(struct ahash_request *req, bool *cached) · line 677

Quoted 11 lines

drivers/crypto/marvell/hash.c +1 -1

  	creq->src_nents = sg_nents_for_len(req->src, req->nbytes);
  	if (creq->src_nents < 0) {
  		dev_err(cesa_dev->dev, "Invalid number of src SG");

@@ -680,7 +674,7 @@ static int mv_cesa_ahash_req_init(struct ahash_request *req, bool *cached)
  	if (*cached)
  		return 0;
 
-	if (creq->req.base.type == CESA_DMA_REQ)
+	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)  

Quoted 6 lines

Should be

	if (cesa_dev->caps->has_tdma)

Quoted 1 line

drivers/crypto/marvell/hash.c +0 -0

  		ret = mv_cesa_ahash_dma_req_init(req);  

Why ? mv_cesa_req_get_type() tests mv_cesa_req->chain and returns a code depending on its value. This value is initialized according to what is set un "has_tdma"...

Quoted 6 lines

Now that the dma specific fields are part of the base request there's no reason to keep this union.

You can just put struct mv_cesa_req base; directly under struct mv_cesa_ablkcipher_req, and move mv_cesa_ablkcipher_std_req fields in mv_cesa_ablkcipher_req.

Well, I think that I might keep the changes related to mv_cesa_tdma_req in this commit (+ put struct mv_cesa_req base; direct under struct mv_cesa_ablkcipher_req) and move the changes related to mv_cesa_ablkcipher_std_req into another commit. What do you think ?

So far, the 'process' operation was used to check if the current request was correctly handled by the engine, if it was the case it copied information from the SRAM to the main memory. Now, we split this operation. We keep the 'process' operation, which still checks if the request was correctly handled by the engine or not, then we add a new operation for completion. The 'complete' method copies the content of the SRAM to memory. This will soon become useful if we want to call the process and the complete operations from different locations depending on the type of the request (different cleanup logic).

Signed-off-by Romain Perier <[email protected]>

---
 drivers/crypto/marvell/cesa.c   |  1 +
 drivers/crypto/marvell/cesa.h   |  3 +++
 drivers/crypto/marvell/cipher.c | 47 ++++++++++++++++++++++++-----------------
 drivers/crypto/marvell/hash.c   | 22 ++++++++++---------
 4 files changed, 44 insertions(+), 29 deletions(-)

drivers/crypto/marvell/cesa.c +1 -0

index fe04d1b..af96426 100644
@@ -98,6 +98,7 @@ static irqreturn_t mv_cesa_int(int irq, void *priv)
 				engine->req = NULL;
 				mv_cesa_dequeue_req_unlocked(engine);
 				spin_unlock_bh(&engine->lock);
+				ctx->ops->complete(req);
 				ctx->ops->cleanup(req);
 				local_bh_disable();
 				req->complete(req, res);

drivers/crypto/marvell/cesa.h +3 -0

index 158ff82..32de08b 100644
@@ -456,6 +456,8 @@ struct mv_cesa_engine {
  *		code)
  * @step:	launch the crypto operation on the next chunk
  * @cleanup:	cleanup the crypto request (release associated data)
+ * @complete:	complete the request, i.e copy result from sram or contexts
+ * 		when it is needed.
  */
 struct mv_cesa_req_ops {
 	void (*prepare)(struct crypto_async_request *req,

@@ -463,6 +465,7 @@ struct mv_cesa_req_ops {
 	int (*process)(struct crypto_async_request *req, u32 status);
 	void (*step)(struct crypto_async_request *req);
 	void (*cleanup)(struct crypto_async_request *req);
+	void (*complete)(struct crypto_async_request *req);
 };
 
 /**

drivers/crypto/marvell/cipher.c +2 -19

index 15d2c5a..fbaae2f 100644
@@ -118,7 +118,6 @@ static int mv_cesa_ablkcipher_std_process(struct ablkcipher_request *req,
 	struct mv_cesa_ablkcipher_std_req *sreq = &creq->req.std;
 	struct mv_cesa_engine *engine = sreq->base.engine;
 	size_t len;
-	unsigned int ivsize;
 
 	len = sg_pcopy_from_buffer(req->dst, creq->dst_nents,
 				   engine->sram + CESA_SA_DATA_SRAM_OFFSET,

@@ -128,10 +127,6 @@ static int mv_cesa_ablkcipher_std_process(struct ablkcipher_request *req,
 	if (sreq->offset < req->nbytes)
 		return -EINPROGRESS;
 
-	ivsize = crypto_ablkcipher_ivsize(crypto_ablkcipher_reqtfm(req));
-	memcpy_fromio(req->info,
-		      engine->sram + CESA_SA_CRYPT_IV_SRAM_OFFSET, ivsize);
-
 	return 0;
 }
 

@@ -141,21 +136,9 @@ static int mv_cesa_ablkcipher_process(struct crypto_async_request *req,
 	struct ablkcipher_request *ablkreq = ablkcipher_request_cast(req);
 	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(ablkreq);
 
-	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ) {
-		int ret;
-		struct mv_cesa_req *basereq;
-		unsigned int ivsize;
-
-		ret = mv_cesa_dma_process(&creq->req.base, status);
-		if (ret)
-			return ret;
+	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)
+		return mv_cesa_dma_process(&creq->req.base, status);
 
-		basereq = &creq->req.base;
-		ivsize = crypto_ablkcipher_ivsize(
-					     crypto_ablkcipher_reqtfm(ablkreq));
-		memcpy_fromio(ablkreq->info, basereq->chain.last->data, ivsize);
-		return ret;
-	}
 	return mv_cesa_ablkcipher_std_process(ablkreq, status);
 }
 

drivers/crypto/marvell/cipher.c +6 -0

 	creq->req.base.engine = engine;
 
 	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)

@@ -213,11 +197,36 @@ mv_cesa_ablkcipher_req_cleanup(struct crypto_async_request *req)
 	mv_cesa_ablkcipher_cleanup(ablkreq);
 }
 
+static void
+mv_cesa_ablkcipher_complete(struct crypto_async_request *req)
+{
+	struct ablkcipher_request *ablkreq = ablkcipher_request_cast(req);
+	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(ablkreq);
+	struct mv_cesa_engine *engine = creq->req.base.engine;

drivers/crypto/marvell/cipher.c +8 -0

+	} else {
+		memcpy_fromio(ablkreq->info,
+			      engine->sram + CESA_SA_CRYPT_IV_SRAM_OFFSET,
+			      ivsize);
+	}
+}
+
 static const struct mv_cesa_req_ops mv_cesa_ablkcipher_req_ops = {
 	.step = mv_cesa_ablkcipher_step,
 	.process = mv_cesa_ablkcipher_process,
 	.prepare = mv_cesa_ablkcipher_prepare,
 	.cleanup = mv_cesa_ablkcipher_req_cleanup,
+	.complete = mv_cesa_ablkcipher_complete,
 };
 
 static int mv_cesa_ablkcipher_cra_init(struct crypto_tfm *tfm)

drivers/crypto/marvell/hash.c +12 -10

index cc7c5b0..f7f84cc 100644
@@ -287,17 +287,20 @@ static int mv_cesa_ahash_process(struct crypto_async_request *req, u32 status)
 {
 	struct ahash_request *ahashreq = ahash_request_cast(req);
 	struct mv_cesa_ahash_req *creq = ahash_request_ctx(ahashreq);
-	struct mv_cesa_engine *engine = creq->req.base.engine;
-	unsigned int digsize;
-	int ret, i;
 
 	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)
-		ret = mv_cesa_dma_process(&creq->req.base, status);
-	else
-		ret = mv_cesa_ahash_std_process(ahashreq, status);
+		return mv_cesa_dma_process(&creq->req.base, status);
 
-	if (ret == -EINPROGRESS)
-		return ret;
+	return mv_cesa_ahash_std_process(ahashreq, status);
+}
+
+static void mv_cesa_ahash_complete(struct crypto_async_request *req)
+{
+	struct ahash_request *ahashreq = ahash_request_cast(req);
+	struct mv_cesa_ahash_req *creq = ahash_request_ctx(ahashreq);
+	struct mv_cesa_engine *engine = creq->req.base.engine;
+	unsigned int digsize;
+	int i;
 
 	digsize = crypto_ahash_digestsize(crypto_ahash_reqtfm(ahashreq));
 	for (i = 0; i < digsize / 4; i++)

@@ -326,8 +329,6 @@ static int mv_cesa_ahash_process(struct crypto_async_request *req, u32 status)
 				result[i] = cpu_to_be32(creq->state[i]);
 		}
 	}
-
-	return ret;
 }
 
 static void mv_cesa_ahash_prepare(struct crypto_async_request *req,

@@ -366,6 +367,7 @@ static const struct mv_cesa_req_ops mv_cesa_ahash_req_ops = {
 	.process = mv_cesa_ahash_process,
 	.prepare = mv_cesa_ahash_prepare,
 	.cleanup = mv_cesa_ahash_req_cleanup,
+	.complete = mv_cesa_ahash_complete,
 };
 
 static int mv_cesa_ahash_init(struct ahash_request *req,

Quoted 1 line

+

Nit: not sure you should mix this cosmetic change with the other changes.

You already have ivsize initialized.

drivers/crypto/marvell/cipher.c mv_cesa_ablkcipher_req_cleanup(struct crypto_async_request *req) · line 216

Quoted 1 line

drivers/crypto/marvell/cipher.c +1 -0

+		memcpy_fromio(ablkreq->info, basereq->chain.last->data, ivsize);

Use memcpy() here.

This commits adds support for fine grained load balancing on multi-engine IPs. The engine is pre-selected based on its current load and on the weight of the crypto request that is about to be processed. The global crypto queue is also moved to each engine. These changes are

useful for preparing the code to support TDMA chaining between crypto requests, because each tdma chain will be handled per engine.

By using
a crypto queue per engine, we make sure that we keep the state of the tdma chain synchronized with the crypto queue. We also reduce contention on 'cesa_dev->lock' and improve parallelism.

Signed-off-by Romain Perier <[email protected]>

---
 drivers/crypto/marvell/cesa.c   | 30 +++++++++----------
 drivers/crypto/marvell/cesa.h   | 26 +++++++++++++++--
 drivers/crypto/marvell/cipher.c | 59 ++++++++++++++++++-------------------
 drivers/crypto/marvell/hash.c   | 65 +++++++++++++++++++----------------------
 4 files changed, 97 insertions(+), 83 deletions(-)

drivers/crypto/marvell/cipher.c +3 -0

index fbaae2f..02aa38f 100644
@@ -89,6 +89,9 @@ static void mv_cesa_ablkcipher_std_step(struct ablkcipher_request *req)
 	size_t  len = min_t(size_t, req->nbytes - sreq->offset,
 			    CESA_SA_SRAM_PAYLOAD_SIZE);
 
+	mv_cesa_adjust_op(engine, &sreq->op);
+	memcpy_toio(engine->sram, &sreq->op, sizeof(sreq->op));
+
 	len = sg_pcopy_to_buffer(req->src, creq->src_nents,
 				 engine->sram + CESA_SA_DATA_SRAM_OFFSET,
 				 len, sreq->offset);

drivers/crypto/marvell/cipher.c +0 -0

 }

static int mv_cesa_ecb_aes_encrypt(struct ablkcipher_request *req)

drivers/crypto/marvell/hash.c +9 -0

index f7f84cc..5946a69 100644
@@ -162,6 +162,15 @@ static void mv_cesa_ahash_std_step(struct ahash_request *req)
 	unsigned int new_cache_ptr = 0;
 	u32 frag_mode;
 	size_t  len;
+	unsigned int digsize;
+	int i;
+
+	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 (creq->cache_ptr)
 		memcpy_toio(engine->sram + CESA_SA_DATA_SRAM_OFFSET,

drivers/crypto/marvell/hash.c +0 -0

 }

Quoted 4 lines · On Wed, 15 Jun 2016 21:15:33 +0200 Romain Perier…

This commits adds support for fine grained load balancing on multi-engine IPs. The engine is pre-selected based on its current load and on the weight of the crypto request that is about to be processed. The global crypto queue is also moved to each engine. These changes are

to the mv_cesa_engine object.

Quoted 2 lines

useful for preparing the code to support TDMA chaining between crypto requests, because each tdma chain will be handled per engine.

These changes are required to allow chaining crypto requests at the DMA level.

Quoted 20 lines

drivers/crypto/marvell/cipher.c +3 -1

index fbaae2f..02aa38f 100644
@@ -89,6 +89,9 @@ static void mv_cesa_ablkcipher_std_step(struct ablkcipher_request *req)
  	size_t  len = min_t(size_t, req->nbytes - sreq->offset,
  			    CESA_SA_SRAM_PAYLOAD_SIZE);
 
+	mv_cesa_adjust_op(engine, &sreq->op);
+	memcpy_toio(engine->sram, &sreq->op, sizeof(sreq->op));
+
  	len = sg_pcopy_to_buffer(req->src, creq->src_nents,
  				 engine->sram + CESA_SA_DATA_SRAM_OFFSET,
  				 len, sreq->offset);

@@ -167,12 +170,9 @@ mv_cesa_ablkcipher_std_prepare(struct ablkcipher_request *req)
  {
  	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(req);
  	struct mv_cesa_ablkcipher_std_req *sreq = &creq->req.std;
-	struct mv_cesa_engine *engine = sreq->base.engine;
 

drivers/crypto/marvell/cipher.c mv_cesa_ablkcipher_std_prepare(struct ablkcipher_request *req) · line 175

Quoted 4 lines

drivers/crypto/marvell/cipher.c +0 -2

  	sreq->size = 0;
  	sreq->offset = 0;
-	mv_cesa_adjust_op(engine, &sreq->op);
-	memcpy_toio(engine->sram, &sreq->op, sizeof(sreq->op));

Are these changes really related to this load balancing support? AFAICT, it's something that could have been done earlier, and is not dependent on the changes your introducing here, but maybe I'm missing something.

Quoted 2 lines

drivers/crypto/marvell/cipher.c +0 -0

  }

[...]

Quoted 27 lines

static int mv_cesa_ecb_aes_encrypt(struct ablkcipher_request *req)

drivers/crypto/marvell/hash.c +9 -1

index f7f84cc..5946a69 100644
@@ -162,6 +162,15 @@ static void mv_cesa_ahash_std_step(struct ahash_request *req)
  	unsigned int new_cache_ptr = 0;
  	u32 frag_mode;
  	size_t  len;
+	unsigned int digsize;
+	int i;
+
+	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 (creq->cache_ptr)
  		memcpy_toio(engine->sram + CESA_SA_DATA_SRAM_OFFSET,

@@ -265,11 +274,8 @@ static void mv_cesa_ahash_std_prepare(struct ahash_request *req)
  {
  	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
  	struct mv_cesa_ahash_std_req *sreq = &creq->req.std;
-	struct mv_cesa_engine *engine = sreq->base.engine;
 

drivers/crypto/marvell/hash.c static void mv_cesa_ahash_std_prepare(struct ahash_request *req) · line 272

Quoted 3 lines

drivers/crypto/marvell/hash.c +0 -2

  	sreq->offset = 0;
-	mv_cesa_adjust_op(engine, &creq->op_tmpl);
-	memcpy_toio(engine->sram, &creq->op_tmpl, sizeof(creq->op_tmpl));

Same as above: it doesn't seem related to the load balancing stuff.

The Cryptographic Engines and Security Accelerators (CESA) supports the Multi-Packet Chain Mode. With this mode enabled, multiple tdma requests can be chained and processed by the hardware without software interferences.

This mode was already activated, however the crypto
requests were not chained together. By doing so, we reduce significantly

the number of IRQs. Instead of being interrupted at the end of each crypto request, we are interrupted at the end of the last cryptographic request processed by the engine.

This commits re-factorizes the code, changes the code architecture and adds the required data structures to chain cryptographic requests together before sending them to an engine.

Signed-off-by Romain Perier <[email protected]>

---
 drivers/crypto/marvell/cesa.c   | 117 +++++++++++++++++++++++++++++++---------
 drivers/crypto/marvell/cesa.h   |  38 ++++++++++++-
 drivers/crypto/marvell/cipher.c |   3 +-
 drivers/crypto/marvell/hash.c   |   9 +++-
 drivers/crypto/marvell/tdma.c   |  81 ++++++++++++++++++++++++++++
 5 files changed, 218 insertions(+), 30 deletions(-)

drivers/crypto/marvell/cesa.c +1 -1

index f9e6688..33411f6 100644
@@ -32,7 +32,7 @@
 #include "cesa.h"
 
 /* Limit of the crypto queue before reaching the backlog */
-#define CESA_CRYPTO_DEFAULT_MAX_QLEN 50
+#define CESA_CRYPTO_DEFAULT_MAX_QLEN 128
 
 static int allhwsupport = !IS_ENABLED(CONFIG_CRYPTO_DEV_MV_CESA);
 module_param_named(allhwsupport, allhwsupport, int, 0444);

drivers/crypto/marvell/cesa.c +14 -3

+{
+	struct crypto_async_request *req;
+
+	*backlog = crypto_get_backlog(&engine->queue);
+	req = crypto_dequeue_request(&engine->queue);
+
+	if (!req)
+		return NULL;
+
+	return req;
+}
+
+static void mv_cesa_rearm_engine(struct mv_cesa_engine *engine)
 {
 	struct crypto_async_request *req, *backlog;
 	struct mv_cesa_ctx *ctx;
 
-	backlog = crypto_get_backlog(&engine->queue);
-	req = crypto_dequeue_request(&engine->queue);
-	engine->req = req;
 
+	spin_lock_bh(&engine->lock);

drivers/crypto/marvell/cesa.c +11 -0

+out_unlock:
+	spin_unlock_bh(&engine->lock);
+}
+
+static int mv_cesa_std_process(struct mv_cesa_engine *engine, u32 status)
+{
+	struct crypto_async_request *req;
+	struct mv_cesa_ctx *ctx;
+	int res;
+
+	req = engine->req;

drivers/crypto/marvell/cesa.c +26 -16

+{
+	ctx->ops->cleanup(req);
+	local_bh_disable();
+	req->complete(req, res);
+	local_bh_enable();
 }
 
 static irqreturn_t mv_cesa_int(int irq, void *priv)

@@ -83,26 +143,31 @@ static irqreturn_t mv_cesa_int(int irq, void *priv)
 		writel(~status, engine->regs + CESA_SA_FPGA_INT_STATUS);
 		writel(~status, engine->regs + CESA_SA_INT_STATUS);
 
+		/* Process fetched requests */
+		res = mv_cesa_int_process(engine, status & mask);
 		ret = IRQ_HANDLED;
+
 		spin_lock_bh(&engine->lock);
 		req = engine->req;
+		if (res != -EINPROGRESS)
+			engine->req = NULL;
 		spin_unlock_bh(&engine->lock);
-		if (req) {
-			ctx = crypto_tfm_ctx(req->tfm);
-			res = ctx->ops->process(req, status & mask);
-			if (res != -EINPROGRESS) {
-				spin_lock_bh(&engine->lock);
-				engine->req = NULL;
-				mv_cesa_dequeue_req_unlocked(engine);
-				spin_unlock_bh(&engine->lock);
-				ctx->ops->complete(req);
-				ctx->ops->cleanup(req);
-				local_bh_disable();
-				req->complete(req, res);
-				local_bh_enable();
-			} else {
-				ctx->ops->step(req);
-			}
+
+		ctx = crypto_tfm_ctx(req->tfm);
+
+		if (res && res != -EINPROGRESS)
+			mv_cesa_complete_req(ctx, req, res);
+
+		/* Launch the next pending request */
+		mv_cesa_rearm_engine(engine);
+
+		/* Iterate over the complete queue */
+		while (true) {
+			req = mv_cesa_engine_dequeue_complete_request(engine);
+			if (!req)
+				break;
+
+			mv_cesa_complete_req(ctx, req, 0);
 		}
 	}
 

drivers/crypto/marvell/cesa.c +2 -4

 	ret = crypto_enqueue_request(&engine->queue, req);
 	spin_unlock_bh(&engine->lock);
 
 	if (ret != -EINPROGRESS)
 		return ret;
 
-	spin_lock_bh(&engine->lock);
-	if (!engine->req)
-		mv_cesa_dequeue_req_unlocked(engine);
-	spin_unlock_bh(&engine->lock);
+	mv_cesa_rearm_engine(engine);
 
 	return -EINPROGRESS;
 }

@@ -496,6 +560,7 @@ static int mv_cesa_probe(struct platform_device *pdev)
 
 		crypto_init_queue(&engine->queue, CESA_CRYPTO_DEFAULT_MAX_QLEN);
 		atomic_set(&engine->load, 0);
+		INIT_LIST_HEAD(&engine->complete_queue);
 	}
 
 	cesa_dev = cesa;

drivers/crypto/marvell/cesa.h +0 -0

index 5626aa7..e0fee1f 100644