All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC 00/13] Further iMX CAAM updates
@ 2015-12-07 19:11 Russell King - ARM Linux
  2015-12-07 19:12 ` [PATCH RFC 01/11] crypto: caam: fix DMA API mapping leak Russell King
                   ` (12 more replies)
  0 siblings, 13 replies; 20+ messages in thread
From: Russell King - ARM Linux @ 2015-12-07 19:11 UTC (permalink / raw)
  To: Fabio Estevam, Herbert Xu; +Cc: David S. Miller, linux-crypto

Here are further imx-caam updates that I've had since before the
previous merge window.  Please review and (I guess) if Freescale
folk can provide acks etc that would be nice.  Thanks.

 drivers/crypto/caam/caamhash.c | 415 +++++++++++++++++++++++------------------
 drivers/crypto/caam/intern.h   |   1 -
 drivers/crypto/caam/jr.c       |  25 +--
 3 files changed, 246 insertions(+), 195 deletions(-)


-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH RFC 01/11] crypto: caam: fix DMA API mapping leak
  2015-12-07 19:11 [PATCH RFC 00/13] Further iMX CAAM updates Russell King - ARM Linux
@ 2015-12-07 19:12 ` Russell King
  2015-12-07 19:12 ` [PATCH RFC 02/11] crypto: caam: ensure descriptor buffers are cacheline aligned Russell King
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Russell King @ 2015-12-07 19:12 UTC (permalink / raw)
  To: Fabio Estevam, Herbert Xu; +Cc: David S. Miller, linux-crypto

caamhash contains this weird code:

	src_nents = sg_count(req->src, req->nbytes);
	dma_map_sg(jrdev, req->src, src_nents ? : 1, DMA_TO_DEVICE);
	...
	edesc->src_nents = src_nents;

sg_count() returns zero when sg_nents_for_len() returns zero or one.
This means we don't need to use a hardware scatterlist.  However,
setting src_nents to zero causes problems when we unmap:

	if (edesc->src_nents)
		dma_unmap_sg_chained(dev, req->src, edesc->src_nents,
				     DMA_TO_DEVICE, edesc->chained);

as zero here means that we have no entries to unmap.  This causes us
to leak DMA mappings, where we map one scatterlist entry and then
fail to unmap it.

This can be fixed in two ways: either by writing the number of entries
that were requested of dma_map_sg(), or by reworking the "no SG
required" case.

We adopt the re-work solution here - we replace sg_count() with
sg_nents_for_len(), so src_nents now contains the real number of
scatterlist entries, and we then change the test for using the
hardware scatterlist to src_nents > 1 rather than just non-zero.

This change passes my sshd, openssl tests hashing /bin and tcrypt
tests.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/crypto/caam/caamhash.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 49106ea42887..eccde7207f92 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -1085,9 +1085,12 @@ static int ahash_digest(struct ahash_request *req)
 	u32 options;
 	int sh_len;
 
-	src_nents = sg_count(req->src, req->nbytes);
-	dma_map_sg(jrdev, req->src, src_nents ? : 1, DMA_TO_DEVICE);
-	sec4_sg_bytes = src_nents * sizeof(struct sec4_sg_entry);
+	src_nents = sg_nents_for_len(req->src, req->nbytes);
+	dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
+	if (src_nents > 1)
+		sec4_sg_bytes = src_nents * sizeof(struct sec4_sg_entry);
+	else
+		sec4_sg_bytes = 0;
 
 	/* allocate space for base edesc and hw desc commands, link tables */
 	edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes + DESC_JOB_IO_LEN,
@@ -1105,7 +1108,7 @@ static int ahash_digest(struct ahash_request *req)
 	desc = edesc->hw_desc;
 	init_job_desc_shared(desc, ptr, sh_len, HDR_SHARE_DEFER | HDR_REVERSE);
 
-	if (src_nents) {
+	if (src_nents > 1) {
 		sg_to_sec4_sg_last(req->src, src_nents, edesc->sec4_sg, 0);
 		edesc->sec4_sg_dma = dma_map_single(jrdev, edesc->sec4_sg,
 					    sec4_sg_bytes, DMA_TO_DEVICE);
@@ -1232,8 +1235,8 @@ static int ahash_update_no_ctx(struct ahash_request *req)
 	to_hash = in_len - *next_buflen;
 
 	if (to_hash) {
-		src_nents = sg_nents_for_len(req->src,
-					     req->nbytes - (*next_buflen));
+		src_nents = sg_nents_for_len(req->src, req->nbytes -
+						       *next_buflen);
 		sec4_sg_bytes = (1 + src_nents) *
 				sizeof(struct sec4_sg_entry);
 
@@ -1429,9 +1432,14 @@ static int ahash_update_first(struct ahash_request *req)
 	to_hash = req->nbytes - *next_buflen;
 
 	if (to_hash) {
-		src_nents = sg_count(req->src, req->nbytes - (*next_buflen));
-		dma_map_sg(jrdev, req->src, src_nents ? : 1, DMA_TO_DEVICE);
-		sec4_sg_bytes = src_nents * sizeof(struct sec4_sg_entry);
+		src_nents = sg_nents_for_len(req->src, req->nbytes -
+						       *next_buflen);
+		dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
+		if (src_nents > 1)
+			sec4_sg_bytes = src_nents *
+					sizeof(struct sec4_sg_entry);
+		else
+			sec4_sg_bytes = 0;
 
 		/*
 		 * allocate space for base edesc and hw desc commands,
@@ -1451,7 +1459,7 @@ static int ahash_update_first(struct ahash_request *req)
 				 DESC_JOB_IO_LEN;
 		edesc->dst_dma = 0;
 
-		if (src_nents) {
+		if (src_nents > 1) {
 			sg_to_sec4_sg_last(req->src, src_nents,
 					   edesc->sec4_sg, 0);
 			edesc->sec4_sg_dma = dma_map_single(jrdev,
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH RFC 02/11] crypto: caam: ensure descriptor buffers are cacheline aligned
  2015-12-07 19:11 [PATCH RFC 00/13] Further iMX CAAM updates Russell King - ARM Linux
  2015-12-07 19:12 ` [PATCH RFC 01/11] crypto: caam: fix DMA API mapping leak Russell King
@ 2015-12-07 19:12 ` Russell King
  2015-12-07 19:12 ` [PATCH RFC 03/11] crypto: caam: incorporate job descriptor into struct ahash_edesc Russell King
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Russell King @ 2015-12-07 19:12 UTC (permalink / raw)
  To: Fabio Estevam, Herbert Xu; +Cc: David S. Miller, linux-crypto

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/crypto/caam/caamhash.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index eccde7207f92..6a6d74f38300 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -99,17 +99,17 @@ static struct list_head hash_list;
 
 /* ahash per-session context */
 struct caam_hash_ctx {
-	struct device *jrdev;
-	u32 sh_desc_update[DESC_HASH_MAX_USED_LEN];
-	u32 sh_desc_update_first[DESC_HASH_MAX_USED_LEN];
-	u32 sh_desc_fin[DESC_HASH_MAX_USED_LEN];
-	u32 sh_desc_digest[DESC_HASH_MAX_USED_LEN];
-	u32 sh_desc_finup[DESC_HASH_MAX_USED_LEN];
-	dma_addr_t sh_desc_update_dma;
+	u32 sh_desc_update[DESC_HASH_MAX_USED_LEN] ____cacheline_aligned;
+	u32 sh_desc_update_first[DESC_HASH_MAX_USED_LEN] ____cacheline_aligned;
+	u32 sh_desc_fin[DESC_HASH_MAX_USED_LEN] ____cacheline_aligned;
+	u32 sh_desc_digest[DESC_HASH_MAX_USED_LEN] ____cacheline_aligned;
+	u32 sh_desc_finup[DESC_HASH_MAX_USED_LEN] ____cacheline_aligned;
+	dma_addr_t sh_desc_update_dma ____cacheline_aligned;
 	dma_addr_t sh_desc_update_first_dma;
 	dma_addr_t sh_desc_fin_dma;
 	dma_addr_t sh_desc_digest_dma;
 	dma_addr_t sh_desc_finup_dma;
+	struct device *jrdev;
 	u32 alg_type;
 	u32 alg_op;
 	u8 key[CAAM_MAX_HASH_KEY_SIZE];
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH RFC 03/11] crypto: caam: incorporate job descriptor into struct ahash_edesc
  2015-12-07 19:11 [PATCH RFC 00/13] Further iMX CAAM updates Russell King - ARM Linux
  2015-12-07 19:12 ` [PATCH RFC 01/11] crypto: caam: fix DMA API mapping leak Russell King
  2015-12-07 19:12 ` [PATCH RFC 02/11] crypto: caam: ensure descriptor buffers are cacheline aligned Russell King
@ 2015-12-07 19:12 ` Russell King
  2015-12-07 19:12 ` [PATCH RFC 04/11] crypto: caam: mark the hardware descriptor as cache line aligned Russell King
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Russell King @ 2015-12-07 19:12 UTC (permalink / raw)
  To: Fabio Estevam, Herbert Xu; +Cc: David S. Miller, linux-crypto

Rather than giving the descriptor as hw_desc[0], give it's real size.
All places where we allocate an ahash_edesc incorporate DESC_JOB_IO_LEN
bytes of job descriptor.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/crypto/caam/caamhash.c | 49 ++++++++++++++++--------------------------
 1 file changed, 19 insertions(+), 30 deletions(-)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 6a6d74f38300..cf557e1291b3 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -604,7 +604,7 @@ struct ahash_edesc {
 	int src_nents;
 	int sec4_sg_bytes;
 	struct sec4_sg_entry *sec4_sg;
-	u32 hw_desc[0];
+	u32 hw_desc[DESC_JOB_IO_LEN / sizeof(u32)];
 };
 
 static inline void ahash_unmap(struct device *dev,
@@ -811,8 +811,8 @@ static int ahash_update_ctx(struct ahash_request *req)
 		 * allocate space for base edesc and hw desc commands,
 		 * link tables
 		 */
-		edesc = kzalloc(sizeof(*edesc) + DESC_JOB_IO_LEN +
-				sec4_sg_bytes, GFP_DMA | flags);
+		edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes,
+				GFP_DMA | flags);
 		if (!edesc) {
 			dev_err(jrdev,
 				"could not allocate extended descriptor\n");
@@ -821,8 +821,7 @@ static int ahash_update_ctx(struct ahash_request *req)
 
 		edesc->src_nents = src_nents;
 		edesc->sec4_sg_bytes = sec4_sg_bytes;
-		edesc->sec4_sg = (void *)edesc + sizeof(struct ahash_edesc) +
-				 DESC_JOB_IO_LEN;
+		edesc->sec4_sg = (void *)(edesc + 1);
 
 		ret = ctx_map_to_sec4_sg(desc, jrdev, state, ctx->ctx_len,
 					 edesc->sec4_sg, DMA_BIDIRECTIONAL);
@@ -921,8 +920,7 @@ static int ahash_final_ctx(struct ahash_request *req)
 	sec4_sg_bytes = sec4_sg_src_index * sizeof(struct sec4_sg_entry);
 
 	/* allocate space for base edesc and hw desc commands, link tables */
-	edesc = kzalloc(sizeof(*edesc) + DESC_JOB_IO_LEN + sec4_sg_bytes,
-			GFP_DMA | flags);
+	edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes, GFP_DMA | flags);
 	if (!edesc) {
 		dev_err(jrdev, "could not allocate extended descriptor\n");
 		return -ENOMEM;
@@ -933,8 +931,7 @@ static int ahash_final_ctx(struct ahash_request *req)
 	init_job_desc_shared(desc, ptr, sh_len, HDR_SHARE_DEFER | HDR_REVERSE);
 
 	edesc->sec4_sg_bytes = sec4_sg_bytes;
-	edesc->sec4_sg = (void *)edesc + sizeof(struct ahash_edesc) +
-			 DESC_JOB_IO_LEN;
+	edesc->sec4_sg = (void *)(edesc + 1);
 	edesc->src_nents = 0;
 
 	ret = ctx_map_to_sec4_sg(desc, jrdev, state, ctx->ctx_len,
@@ -1007,8 +1004,7 @@ static int ahash_finup_ctx(struct ahash_request *req)
 			 sizeof(struct sec4_sg_entry);
 
 	/* allocate space for base edesc and hw desc commands, link tables */
-	edesc = kzalloc(sizeof(*edesc) + DESC_JOB_IO_LEN + sec4_sg_bytes,
-			GFP_DMA | flags);
+	edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes, GFP_DMA | flags);
 	if (!edesc) {
 		dev_err(jrdev, "could not allocate extended descriptor\n");
 		return -ENOMEM;
@@ -1020,8 +1016,7 @@ static int ahash_finup_ctx(struct ahash_request *req)
 
 	edesc->src_nents = src_nents;
 	edesc->sec4_sg_bytes = sec4_sg_bytes;
-	edesc->sec4_sg = (void *)edesc + sizeof(struct ahash_edesc) +
-			 DESC_JOB_IO_LEN;
+	edesc->sec4_sg = (void *)(edesc + 1);
 
 	ret = ctx_map_to_sec4_sg(desc, jrdev, state, ctx->ctx_len,
 				 edesc->sec4_sg, DMA_TO_DEVICE);
@@ -1093,14 +1088,12 @@ static int ahash_digest(struct ahash_request *req)
 		sec4_sg_bytes = 0;
 
 	/* allocate space for base edesc and hw desc commands, link tables */
-	edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes + DESC_JOB_IO_LEN,
-			GFP_DMA | flags);
+	edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes, GFP_DMA | flags);
 	if (!edesc) {
 		dev_err(jrdev, "could not allocate extended descriptor\n");
 		return -ENOMEM;
 	}
-	edesc->sec4_sg = (void *)edesc + sizeof(struct ahash_edesc) +
-			  DESC_JOB_IO_LEN;
+	edesc->sec4_sg = (void *)(edesc + 1);
 	edesc->sec4_sg_bytes = sec4_sg_bytes;
 	edesc->src_nents = src_nents;
 
@@ -1166,7 +1159,7 @@ static int ahash_final_no_ctx(struct ahash_request *req)
 	int sh_len;
 
 	/* allocate space for base edesc and hw desc commands, link tables */
-	edesc = kzalloc(sizeof(*edesc) + DESC_JOB_IO_LEN, GFP_DMA | flags);
+	edesc = kzalloc(sizeof(*edesc), GFP_DMA | flags);
 	if (!edesc) {
 		dev_err(jrdev, "could not allocate extended descriptor\n");
 		return -ENOMEM;
@@ -1244,8 +1237,8 @@ static int ahash_update_no_ctx(struct ahash_request *req)
 		 * allocate space for base edesc and hw desc commands,
 		 * link tables
 		 */
-		edesc = kzalloc(sizeof(*edesc) + DESC_JOB_IO_LEN +
-				sec4_sg_bytes, GFP_DMA | flags);
+		edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes,
+				GFP_DMA | flags);
 		if (!edesc) {
 			dev_err(jrdev,
 				"could not allocate extended descriptor\n");
@@ -1254,8 +1247,7 @@ static int ahash_update_no_ctx(struct ahash_request *req)
 
 		edesc->src_nents = src_nents;
 		edesc->sec4_sg_bytes = sec4_sg_bytes;
-		edesc->sec4_sg = (void *)edesc + sizeof(struct ahash_edesc) +
-				 DESC_JOB_IO_LEN;
+		edesc->sec4_sg = (void *)(edesc + 1);
 		edesc->dst_dma = 0;
 
 		state->buf_dma = buf_map_to_sec4_sg(jrdev, edesc->sec4_sg,
@@ -1350,8 +1342,7 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
 			 sizeof(struct sec4_sg_entry);
 
 	/* allocate space for base edesc and hw desc commands, link tables */
-	edesc = kzalloc(sizeof(*edesc) + DESC_JOB_IO_LEN + sec4_sg_bytes,
-			GFP_DMA | flags);
+	edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes, GFP_DMA | flags);
 	if (!edesc) {
 		dev_err(jrdev, "could not allocate extended descriptor\n");
 		return -ENOMEM;
@@ -1363,8 +1354,7 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
 
 	edesc->src_nents = src_nents;
 	edesc->sec4_sg_bytes = sec4_sg_bytes;
-	edesc->sec4_sg = (void *)edesc + sizeof(struct ahash_edesc) +
-			 DESC_JOB_IO_LEN;
+	edesc->sec4_sg = (void *)(edesc + 1);
 
 	state->buf_dma = try_buf_map_to_sec4_sg(jrdev, edesc->sec4_sg, buf,
 						state->buf_dma, buflen,
@@ -1445,8 +1435,8 @@ static int ahash_update_first(struct ahash_request *req)
 		 * allocate space for base edesc and hw desc commands,
 		 * link tables
 		 */
-		edesc = kzalloc(sizeof(*edesc) + DESC_JOB_IO_LEN +
-				sec4_sg_bytes, GFP_DMA | flags);
+		edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes,
+				GFP_DMA | flags);
 		if (!edesc) {
 			dev_err(jrdev,
 				"could not allocate extended descriptor\n");
@@ -1455,8 +1445,7 @@ static int ahash_update_first(struct ahash_request *req)
 
 		edesc->src_nents = src_nents;
 		edesc->sec4_sg_bytes = sec4_sg_bytes;
-		edesc->sec4_sg = (void *)edesc + sizeof(struct ahash_edesc) +
-				 DESC_JOB_IO_LEN;
+		edesc->sec4_sg = (void *)(edesc + 1);
 		edesc->dst_dma = 0;
 
 		if (src_nents > 1) {
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH RFC 04/11] crypto: caam: mark the hardware descriptor as cache line aligned
  2015-12-07 19:11 [PATCH RFC 00/13] Further iMX CAAM updates Russell King - ARM Linux
                   ` (2 preceding siblings ...)
  2015-12-07 19:12 ` [PATCH RFC 03/11] crypto: caam: incorporate job descriptor into struct ahash_edesc Russell King
@ 2015-12-07 19:12 ` Russell King
  2015-12-07 19:12 ` [PATCH RFC 05/11] crypto: caam: replace sec4_sg pointer with array Russell King
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Russell King @ 2015-12-07 19:12 UTC (permalink / raw)
  To: Fabio Estevam, Herbert Xu; +Cc: David S. Miller, linux-crypto

Mark the hardware descriptor as being cache line aligned; on DMA
incoherent architectures, the hardware descriptor should sit in a
separate cache line from the CPU accessed data to avoid polluting
the caches.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/crypto/caam/caamhash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index cf557e1291b3..0a9665140d26 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -604,7 +604,7 @@ struct ahash_edesc {
 	int src_nents;
 	int sec4_sg_bytes;
 	struct sec4_sg_entry *sec4_sg;
-	u32 hw_desc[DESC_JOB_IO_LEN / sizeof(u32)];
+	u32 hw_desc[DESC_JOB_IO_LEN / sizeof(u32)] ____cacheline_aligned;
 };
 
 static inline void ahash_unmap(struct device *dev,
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH RFC 05/11] crypto: caam: replace sec4_sg pointer with array
  2015-12-07 19:11 [PATCH RFC 00/13] Further iMX CAAM updates Russell King - ARM Linux
                   ` (3 preceding siblings ...)
  2015-12-07 19:12 ` [PATCH RFC 04/11] crypto: caam: mark the hardware descriptor as cache line aligned Russell King
@ 2015-12-07 19:12 ` Russell King
  2015-12-07 19:12 ` [PATCH RFC 06/11] crypto: caam: ensure that we clean up after an error Russell King
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Russell King @ 2015-12-07 19:12 UTC (permalink / raw)
  To: Fabio Estevam, Herbert Xu; +Cc: David S. Miller, linux-crypto

Since the extended descriptor includes the hardware descriptor, and the
sec4 scatterlist immediately follows this, we can declare it as a array
at the very end of the extended descriptor.  This allows us to get rid
of an initialiser for every site where we allocate an extended
descriptor.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/crypto/caam/caamhash.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 0a9665140d26..d48974d1897d 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -595,16 +595,16 @@ static int ahash_setkey(struct crypto_ahash *ahash,
  * @sec4_sg_dma: physical mapped address of h/w link table
  * @src_nents: number of segments in input scatterlist
  * @sec4_sg_bytes: length of dma mapped sec4_sg space
- * @sec4_sg: pointer to h/w link table
  * @hw_desc: the h/w job descriptor followed by any referenced link tables
+ * @sec4_sg: h/w link table
  */
 struct ahash_edesc {
 	dma_addr_t dst_dma;
 	dma_addr_t sec4_sg_dma;
 	int src_nents;
 	int sec4_sg_bytes;
-	struct sec4_sg_entry *sec4_sg;
 	u32 hw_desc[DESC_JOB_IO_LEN / sizeof(u32)] ____cacheline_aligned;
+	struct sec4_sg_entry sec4_sg[0];
 };
 
 static inline void ahash_unmap(struct device *dev,
@@ -821,7 +821,6 @@ static int ahash_update_ctx(struct ahash_request *req)
 
 		edesc->src_nents = src_nents;
 		edesc->sec4_sg_bytes = sec4_sg_bytes;
-		edesc->sec4_sg = (void *)(edesc + 1);
 
 		ret = ctx_map_to_sec4_sg(desc, jrdev, state, ctx->ctx_len,
 					 edesc->sec4_sg, DMA_BIDIRECTIONAL);
@@ -931,7 +930,6 @@ static int ahash_final_ctx(struct ahash_request *req)
 	init_job_desc_shared(desc, ptr, sh_len, HDR_SHARE_DEFER | HDR_REVERSE);
 
 	edesc->sec4_sg_bytes = sec4_sg_bytes;
-	edesc->sec4_sg = (void *)(edesc + 1);
 	edesc->src_nents = 0;
 
 	ret = ctx_map_to_sec4_sg(desc, jrdev, state, ctx->ctx_len,
@@ -1016,7 +1014,6 @@ static int ahash_finup_ctx(struct ahash_request *req)
 
 	edesc->src_nents = src_nents;
 	edesc->sec4_sg_bytes = sec4_sg_bytes;
-	edesc->sec4_sg = (void *)(edesc + 1);
 
 	ret = ctx_map_to_sec4_sg(desc, jrdev, state, ctx->ctx_len,
 				 edesc->sec4_sg, DMA_TO_DEVICE);
@@ -1093,7 +1090,7 @@ static int ahash_digest(struct ahash_request *req)
 		dev_err(jrdev, "could not allocate extended descriptor\n");
 		return -ENOMEM;
 	}
-	edesc->sec4_sg = (void *)(edesc + 1);
+
 	edesc->sec4_sg_bytes = sec4_sg_bytes;
 	edesc->src_nents = src_nents;
 
@@ -1247,7 +1244,6 @@ static int ahash_update_no_ctx(struct ahash_request *req)
 
 		edesc->src_nents = src_nents;
 		edesc->sec4_sg_bytes = sec4_sg_bytes;
-		edesc->sec4_sg = (void *)(edesc + 1);
 		edesc->dst_dma = 0;
 
 		state->buf_dma = buf_map_to_sec4_sg(jrdev, edesc->sec4_sg,
@@ -1354,7 +1350,6 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
 
 	edesc->src_nents = src_nents;
 	edesc->sec4_sg_bytes = sec4_sg_bytes;
-	edesc->sec4_sg = (void *)(edesc + 1);
 
 	state->buf_dma = try_buf_map_to_sec4_sg(jrdev, edesc->sec4_sg, buf,
 						state->buf_dma, buflen,
@@ -1445,7 +1440,6 @@ static int ahash_update_first(struct ahash_request *req)
 
 		edesc->src_nents = src_nents;
 		edesc->sec4_sg_bytes = sec4_sg_bytes;
-		edesc->sec4_sg = (void *)(edesc + 1);
 		edesc->dst_dma = 0;
 
 		if (src_nents > 1) {
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH RFC 06/11] crypto: caam: ensure that we clean up after an error
  2015-12-07 19:11 [PATCH RFC 00/13] Further iMX CAAM updates Russell King - ARM Linux
                   ` (4 preceding siblings ...)
  2015-12-07 19:12 ` [PATCH RFC 05/11] crypto: caam: replace sec4_sg pointer with array Russell King
@ 2015-12-07 19:12 ` Russell King
  2015-12-09 15:08   ` Horia Geantă
  2015-12-07 19:12 ` [PATCH RFC 07/11] crypto: caam: check and use dma_map_sg() return code Russell King
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 20+ messages in thread
From: Russell King @ 2015-12-07 19:12 UTC (permalink / raw)
  To: Fabio Estevam, Herbert Xu; +Cc: David S. Miller, linux-crypto

Ensure that we clean up allocations and DMA mappings after encountering
an error rather than just giving up and leaking memory and resources.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/crypto/caam/caamhash.c | 57 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index d48974d1897d..076cfddf32bb 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -824,8 +824,12 @@ static int ahash_update_ctx(struct ahash_request *req)
 
 		ret = ctx_map_to_sec4_sg(desc, jrdev, state, ctx->ctx_len,
 					 edesc->sec4_sg, DMA_BIDIRECTIONAL);
-		if (ret)
+		if (ret) {
+			ahash_unmap_ctx(jrdev, edesc, req, ctx->ctx_len,
+					DMA_BIDIRECTIONAL);
+			kfree(edesc);
 			return ret;
+		}
 
 		state->buf_dma = try_buf_map_to_sec4_sg(jrdev,
 							edesc->sec4_sg + 1,
@@ -856,6 +860,9 @@ static int ahash_update_ctx(struct ahash_request *req)
 						     DMA_TO_DEVICE);
 		if (dma_mapping_error(jrdev, edesc->sec4_sg_dma)) {
 			dev_err(jrdev, "unable to map S/G table\n");
+			ahash_unmap_ctx(jrdev, edesc, req, ctx->ctx_len,
+					DMA_BIDIRECTIONAL);
+			kfree(edesc);
 			return -ENOMEM;
 		}
 
@@ -934,8 +941,11 @@ static int ahash_final_ctx(struct ahash_request *req)
 
 	ret = ctx_map_to_sec4_sg(desc, jrdev, state, ctx->ctx_len,
 				 edesc->sec4_sg, DMA_TO_DEVICE);
-	if (ret)
+	if (ret) {
+		ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_FROM_DEVICE);
+		kfree(edesc);
 		return ret;
+	}
 
 	state->buf_dma = try_buf_map_to_sec4_sg(jrdev, edesc->sec4_sg + 1,
 						buf, state->buf_dma, buflen,
@@ -946,6 +956,8 @@ static int ahash_final_ctx(struct ahash_request *req)
 					    sec4_sg_bytes, DMA_TO_DEVICE);
 	if (dma_mapping_error(jrdev, edesc->sec4_sg_dma)) {
 		dev_err(jrdev, "unable to map S/G table\n");
+		ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_FROM_DEVICE);
+		kfree(edesc);
 		return -ENOMEM;
 	}
 
@@ -956,6 +968,8 @@ static int ahash_final_ctx(struct ahash_request *req)
 						digestsize);
 	if (dma_mapping_error(jrdev, edesc->dst_dma)) {
 		dev_err(jrdev, "unable to map dst\n");
+		ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_FROM_DEVICE);
+		kfree(edesc);
 		return -ENOMEM;
 	}
 
@@ -1017,8 +1031,11 @@ static int ahash_finup_ctx(struct ahash_request *req)
 
 	ret = ctx_map_to_sec4_sg(desc, jrdev, state, ctx->ctx_len,
 				 edesc->sec4_sg, DMA_TO_DEVICE);
-	if (ret)
+	if (ret) {
+		ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_FROM_DEVICE);
+		kfree(edesc);
 		return ret;
+	}
 
 	state->buf_dma = try_buf_map_to_sec4_sg(jrdev, edesc->sec4_sg + 1,
 						buf, state->buf_dma, buflen,
@@ -1031,6 +1048,8 @@ static int ahash_finup_ctx(struct ahash_request *req)
 					    sec4_sg_bytes, DMA_TO_DEVICE);
 	if (dma_mapping_error(jrdev, edesc->sec4_sg_dma)) {
 		dev_err(jrdev, "unable to map S/G table\n");
+		ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_FROM_DEVICE);
+		kfree(edesc);
 		return -ENOMEM;
 	}
 
@@ -1041,6 +1060,8 @@ static int ahash_finup_ctx(struct ahash_request *req)
 						digestsize);
 	if (dma_mapping_error(jrdev, edesc->dst_dma)) {
 		dev_err(jrdev, "unable to map dst\n");
+		ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_FROM_DEVICE);
+		kfree(edesc);
 		return -ENOMEM;
 	}
 
@@ -1104,6 +1125,8 @@ static int ahash_digest(struct ahash_request *req)
 					    sec4_sg_bytes, DMA_TO_DEVICE);
 		if (dma_mapping_error(jrdev, edesc->sec4_sg_dma)) {
 			dev_err(jrdev, "unable to map S/G table\n");
+			ahash_unmap(jrdev, edesc, req, digestsize);
+			kfree(edesc);
 			return -ENOMEM;
 		}
 		src_dma = edesc->sec4_sg_dma;
@@ -1118,6 +1141,8 @@ static int ahash_digest(struct ahash_request *req)
 						digestsize);
 	if (dma_mapping_error(jrdev, edesc->dst_dma)) {
 		dev_err(jrdev, "unable to map dst\n");
+		ahash_unmap(jrdev, edesc, req, digestsize);
+		kfree(edesc);
 		return -ENOMEM;
 	}
 
@@ -1170,6 +1195,8 @@ static int ahash_final_no_ctx(struct ahash_request *req)
 	state->buf_dma = dma_map_single(jrdev, buf, buflen, DMA_TO_DEVICE);
 	if (dma_mapping_error(jrdev, state->buf_dma)) {
 		dev_err(jrdev, "unable to map src\n");
+		ahash_unmap(jrdev, edesc, req, digestsize);
+		kfree(edesc);
 		return -ENOMEM;
 	}
 
@@ -1179,6 +1206,8 @@ static int ahash_final_no_ctx(struct ahash_request *req)
 						digestsize);
 	if (dma_mapping_error(jrdev, edesc->dst_dma)) {
 		dev_err(jrdev, "unable to map dst\n");
+		ahash_unmap(jrdev, edesc, req, digestsize);
+		kfree(edesc);
 		return -ENOMEM;
 	}
 	edesc->src_nents = 0;
@@ -1268,14 +1297,21 @@ static int ahash_update_no_ctx(struct ahash_request *req)
 						    DMA_TO_DEVICE);
 		if (dma_mapping_error(jrdev, edesc->sec4_sg_dma)) {
 			dev_err(jrdev, "unable to map S/G table\n");
+			ahash_unmap_ctx(jrdev, edesc, req, ctx->ctx_len,
+					DMA_TO_DEVICE);
+			kfree(edesc);
 			return -ENOMEM;
 		}
 
 		append_seq_in_ptr(desc, edesc->sec4_sg_dma, to_hash, LDST_SGF);
 
 		ret = map_seq_out_ptr_ctx(desc, jrdev, state, ctx->ctx_len);
-		if (ret)
+		if (ret) {
+			ahash_unmap_ctx(jrdev, edesc, req, ctx->ctx_len,
+					DMA_TO_DEVICE);
+			kfree(edesc);
 			return ret;
+		}
 
 #ifdef DEBUG
 		print_hex_dump(KERN_ERR, "jobdesc@"__stringify(__LINE__)": ",
@@ -1361,6 +1397,8 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
 					    sec4_sg_bytes, DMA_TO_DEVICE);
 	if (dma_mapping_error(jrdev, edesc->sec4_sg_dma)) {
 		dev_err(jrdev, "unable to map S/G table\n");
+		ahash_unmap(jrdev, edesc, req, digestsize);
+		kfree(edesc);
 		return -ENOMEM;
 	}
 
@@ -1371,6 +1409,8 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
 						digestsize);
 	if (dma_mapping_error(jrdev, edesc->dst_dma)) {
 		dev_err(jrdev, "unable to map dst\n");
+		ahash_unmap(jrdev, edesc, req, digestsize);
+		kfree(edesc);
 		return -ENOMEM;
 	}
 
@@ -1451,6 +1491,9 @@ static int ahash_update_first(struct ahash_request *req)
 							    DMA_TO_DEVICE);
 			if (dma_mapping_error(jrdev, edesc->sec4_sg_dma)) {
 				dev_err(jrdev, "unable to map S/G table\n");
+				ahash_unmap_ctx(jrdev, edesc, req, ctx->ctx_len,
+						DMA_TO_DEVICE);
+				kfree(edesc);
 				return -ENOMEM;
 			}
 			src_dma = edesc->sec4_sg_dma;
@@ -1472,8 +1515,12 @@ static int ahash_update_first(struct ahash_request *req)
 		append_seq_in_ptr(desc, src_dma, to_hash, options);
 
 		ret = map_seq_out_ptr_ctx(desc, jrdev, state, ctx->ctx_len);
-		if (ret)
+		if (ret) {
+			ahash_unmap_ctx(jrdev, edesc, req, ctx->ctx_len,
+					DMA_TO_DEVICE);
+			kfree(edesc);
 			return ret;
+		}
 
 #ifdef DEBUG
 		print_hex_dump(KERN_ERR, "jobdesc@"__stringify(__LINE__)": ",
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH RFC 07/11] crypto: caam: check and use dma_map_sg() return code
  2015-12-07 19:11 [PATCH RFC 00/13] Further iMX CAAM updates Russell King - ARM Linux
                   ` (5 preceding siblings ...)
  2015-12-07 19:12 ` [PATCH RFC 06/11] crypto: caam: ensure that we clean up after an error Russell King
@ 2015-12-07 19:12 ` Russell King
  2015-12-09 15:20   ` Horia Geantă
  2015-12-07 19:12 ` [PATCH RFC 08/11] crypto: caam: add ahash_edesc_alloc() for descriptor allocation Russell King
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 20+ messages in thread
From: Russell King @ 2015-12-07 19:12 UTC (permalink / raw)
  To: Fabio Estevam, Herbert Xu; +Cc: David S. Miller, linux-crypto

Strictly, dma_map_sg() may coalesce SG entries, but in practise on iMX
hardware, this will never happen.  However, dma_map_sg() can fail, and
we completely fail to check its return value.  So, fix this properly.

Arrange the code to map the scatterlist early, so we know how many
scatter table entries to allocate, and then fill them in.  This allows
us to keep relatively simple error cleanup paths.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/crypto/caam/caamhash.c | 109 ++++++++++++++++++++++++++++-------------
 1 file changed, 74 insertions(+), 35 deletions(-)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 076cfddf32bb..9638e9f4f001 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -187,15 +187,6 @@ static inline dma_addr_t buf_map_to_sec4_sg(struct device *jrdev,
 	return buf_dma;
 }
 
-/* Map req->src and put it in link table */
-static inline void src_map_to_sec4_sg(struct device *jrdev,
-				      struct scatterlist *src, int src_nents,
-				      struct sec4_sg_entry *sec4_sg)
-{
-	dma_map_sg(jrdev, src, src_nents, DMA_TO_DEVICE);
-	sg_to_sec4_sg_last(src, src_nents, sec4_sg, 0);
-}
-
 /*
  * Only put buffer in link table if it contains data, which is possible,
  * since a buffer has previously been used, and needs to be unmapped,
@@ -791,7 +782,7 @@ static int ahash_update_ctx(struct ahash_request *req)
 	int in_len = *buflen + req->nbytes, to_hash;
 	u32 *sh_desc = ctx->sh_desc_update, *desc;
 	dma_addr_t ptr = ctx->sh_desc_update_dma;
-	int src_nents, sec4_sg_bytes, sec4_sg_src_index;
+	int src_nents, mapped_nents, sec4_sg_bytes, sec4_sg_src_index;
 	struct ahash_edesc *edesc;
 	int ret = 0;
 	int sh_len;
@@ -803,8 +794,19 @@ static int ahash_update_ctx(struct ahash_request *req)
 	if (to_hash) {
 		src_nents = sg_nents_for_len(req->src,
 					     req->nbytes - (*next_buflen));
+		if (src_nents) {
+			mapped_nents = dma_map_sg(jrdev, req->src, src_nents,
+						  DMA_TO_DEVICE);
+			if (!mapped_nents) {
+				dev_err(jrdev, "unable to DMA map source\n");
+				return -ENOMEM;
+			}
+		} else {
+			mapped_nents = 0;
+		}
+
 		sec4_sg_src_index = 1 + (*buflen ? 1 : 0);
-		sec4_sg_bytes = (sec4_sg_src_index + src_nents) *
+		sec4_sg_bytes = (sec4_sg_src_index + mapped_nents) *
 				 sizeof(struct sec4_sg_entry);
 
 		/*
@@ -816,6 +818,7 @@ static int ahash_update_ctx(struct ahash_request *req)
 		if (!edesc) {
 			dev_err(jrdev,
 				"could not allocate extended descriptor\n");
+			dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 			return -ENOMEM;
 		}
 
@@ -836,9 +839,10 @@ static int ahash_update_ctx(struct ahash_request *req)
 							buf, state->buf_dma,
 							*buflen, last_buflen);
 
-		if (src_nents) {
-			src_map_to_sec4_sg(jrdev, req->src, src_nents,
-					   edesc->sec4_sg + sec4_sg_src_index);
+		if (mapped_nents) {
+			sg_to_sec4_sg_last(req->src, mapped_nents,
+					   edesc->sec4_sg + sec4_sg_src_index,
+					   0);
 			if (*next_buflen)
 				scatterwalk_map_and_copy(next_buf, req->src,
 							 to_hash - *buflen,
@@ -1004,21 +1008,28 @@ static int ahash_finup_ctx(struct ahash_request *req)
 	u32 *sh_desc = ctx->sh_desc_finup, *desc;
 	dma_addr_t ptr = ctx->sh_desc_finup_dma;
 	int sec4_sg_bytes, sec4_sg_src_index;
-	int src_nents;
+	int src_nents, mapped_nents;
 	int digestsize = crypto_ahash_digestsize(ahash);
 	struct ahash_edesc *edesc;
 	int ret = 0;
 	int sh_len;
 
 	src_nents = sg_nents_for_len(req->src, req->nbytes);
+	mapped_nents = dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
+	if (!mapped_nents) {
+		dev_err(jrdev, "unable to DMA map source\n");
+		return -ENOMEM;
+	}
+
 	sec4_sg_src_index = 1 + (buflen ? 1 : 0);
-	sec4_sg_bytes = (sec4_sg_src_index + src_nents) *
+	sec4_sg_bytes = (sec4_sg_src_index + mapped_nents) *
 			 sizeof(struct sec4_sg_entry);
 
 	/* allocate space for base edesc and hw desc commands, link tables */
 	edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes, GFP_DMA | flags);
 	if (!edesc) {
 		dev_err(jrdev, "could not allocate extended descriptor\n");
+		dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 		return -ENOMEM;
 	}
 
@@ -1041,8 +1052,8 @@ static int ahash_finup_ctx(struct ahash_request *req)
 						buf, state->buf_dma, buflen,
 						last_buflen);
 
-	src_map_to_sec4_sg(jrdev, req->src, src_nents, edesc->sec4_sg +
-			   sec4_sg_src_index);
+	sg_to_sec4_sg_last(req->src, mapped_nents,
+			   edesc->sec4_sg + sec4_sg_src_index, 0);
 
 	edesc->sec4_sg_dma = dma_map_single(jrdev, edesc->sec4_sg,
 					    sec4_sg_bytes, DMA_TO_DEVICE);
@@ -1091,7 +1102,7 @@ static int ahash_digest(struct ahash_request *req)
 	u32 *sh_desc = ctx->sh_desc_digest, *desc;
 	dma_addr_t ptr = ctx->sh_desc_digest_dma;
 	int digestsize = crypto_ahash_digestsize(ahash);
-	int src_nents, sec4_sg_bytes;
+	int src_nents, mapped_nents, sec4_sg_bytes;
 	dma_addr_t src_dma;
 	struct ahash_edesc *edesc;
 	int ret = 0;
@@ -1099,9 +1110,14 @@ static int ahash_digest(struct ahash_request *req)
 	int sh_len;
 
 	src_nents = sg_nents_for_len(req->src, req->nbytes);
-	dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
-	if (src_nents > 1)
-		sec4_sg_bytes = src_nents * sizeof(struct sec4_sg_entry);
+	mapped_nents = dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
+	if (mapped_nents == 0) {
+		dev_err(jrdev, "unable to map source for DMA\n");
+		return -ENOMEM;
+	}
+
+	if (mapped_nents > 1)
+		sec4_sg_bytes = mapped_nents * sizeof(struct sec4_sg_entry);
 	else
 		sec4_sg_bytes = 0;
 
@@ -1109,6 +1125,7 @@ static int ahash_digest(struct ahash_request *req)
 	edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes, GFP_DMA | flags);
 	if (!edesc) {
 		dev_err(jrdev, "could not allocate extended descriptor\n");
+		dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 		return -ENOMEM;
 	}
 
@@ -1120,7 +1137,7 @@ static int ahash_digest(struct ahash_request *req)
 	init_job_desc_shared(desc, ptr, sh_len, HDR_SHARE_DEFER | HDR_REVERSE);
 
 	if (src_nents > 1) {
-		sg_to_sec4_sg_last(req->src, src_nents, edesc->sec4_sg, 0);
+		sg_to_sec4_sg_last(req->src, mapped_nents, edesc->sec4_sg, 0);
 		edesc->sec4_sg_dma = dma_map_single(jrdev, edesc->sec4_sg,
 					    sec4_sg_bytes, DMA_TO_DEVICE);
 		if (dma_mapping_error(jrdev, edesc->sec4_sg_dma)) {
@@ -1243,7 +1260,7 @@ static int ahash_update_no_ctx(struct ahash_request *req)
 	int *next_buflen = state->current_buf ? &state->buflen_0 :
 			   &state->buflen_1;
 	int in_len = *buflen + req->nbytes, to_hash;
-	int sec4_sg_bytes, src_nents;
+	int sec4_sg_bytes, src_nents, mapped_nents;
 	struct ahash_edesc *edesc;
 	u32 *desc, *sh_desc = ctx->sh_desc_update_first;
 	dma_addr_t ptr = ctx->sh_desc_update_first_dma;
@@ -1256,7 +1273,14 @@ static int ahash_update_no_ctx(struct ahash_request *req)
 	if (to_hash) {
 		src_nents = sg_nents_for_len(req->src, req->nbytes -
 						       *next_buflen);
-		sec4_sg_bytes = (1 + src_nents) *
+		mapped_nents = dma_map_sg(jrdev, req->src, src_nents,
+					  DMA_TO_DEVICE);
+		if (!mapped_nents) {
+			dev_err(jrdev, "unable to DMA map source\n");
+			return -ENOMEM;
+		}
+
+		sec4_sg_bytes = (1 + mapped_nents) *
 				sizeof(struct sec4_sg_entry);
 
 		/*
@@ -1268,6 +1292,7 @@ static int ahash_update_no_ctx(struct ahash_request *req)
 		if (!edesc) {
 			dev_err(jrdev,
 				"could not allocate extended descriptor\n");
+			dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 			return -ENOMEM;
 		}
 
@@ -1277,8 +1302,9 @@ static int ahash_update_no_ctx(struct ahash_request *req)
 
 		state->buf_dma = buf_map_to_sec4_sg(jrdev, edesc->sec4_sg,
 						    buf, *buflen);
-		src_map_to_sec4_sg(jrdev, req->src, src_nents,
-				   edesc->sec4_sg + 1);
+		sg_to_sec4_sg_last(req->src, mapped_nents,
+				   edesc->sec4_sg + 1, 0);
+
 		if (*next_buflen) {
 			scatterwalk_map_and_copy(next_buf, req->src,
 						 to_hash - *buflen,
@@ -1362,21 +1388,28 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
 			  state->buflen_1;
 	u32 *sh_desc = ctx->sh_desc_digest, *desc;
 	dma_addr_t ptr = ctx->sh_desc_digest_dma;
-	int sec4_sg_bytes, sec4_sg_src_index, src_nents;
+	int sec4_sg_bytes, sec4_sg_src_index, src_nents, mapped_nents;
 	int digestsize = crypto_ahash_digestsize(ahash);
 	struct ahash_edesc *edesc;
 	int sh_len;
 	int ret = 0;
 
 	src_nents = sg_nents_for_len(req->src, req->nbytes);
+	mapped_nents = dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
+	if (!mapped_nents) {
+		dev_err(jrdev, "unable to DMA map source\n");
+		return -ENOMEM;
+	}
+
 	sec4_sg_src_index = 2;
-	sec4_sg_bytes = (sec4_sg_src_index + src_nents) *
+	sec4_sg_bytes = (sec4_sg_src_index + mapped_nents) *
 			 sizeof(struct sec4_sg_entry);
 
 	/* allocate space for base edesc and hw desc commands, link tables */
 	edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes, GFP_DMA | flags);
 	if (!edesc) {
 		dev_err(jrdev, "could not allocate extended descriptor\n");
+		dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 		return -ENOMEM;
 	}
 
@@ -1391,7 +1424,7 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
 						state->buf_dma, buflen,
 						last_buflen);
 
-	src_map_to_sec4_sg(jrdev, req->src, src_nents, edesc->sec4_sg + 1);
+	sg_to_sec4_sg_last(req->src, mapped_nents, edesc->sec4_sg + 1, 0);
 
 	edesc->sec4_sg_dma = dma_map_single(jrdev, edesc->sec4_sg,
 					    sec4_sg_bytes, DMA_TO_DEVICE);
@@ -1445,7 +1478,7 @@ static int ahash_update_first(struct ahash_request *req)
 	int to_hash;
 	u32 *sh_desc = ctx->sh_desc_update_first, *desc;
 	dma_addr_t ptr = ctx->sh_desc_update_first_dma;
-	int sec4_sg_bytes, src_nents;
+	int sec4_sg_bytes, src_nents, mapped_nents;
 	dma_addr_t src_dma;
 	u32 options;
 	struct ahash_edesc *edesc;
@@ -1459,9 +1492,14 @@ static int ahash_update_first(struct ahash_request *req)
 	if (to_hash) {
 		src_nents = sg_nents_for_len(req->src, req->nbytes -
 						       *next_buflen);
-		dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
-		if (src_nents > 1)
-			sec4_sg_bytes = src_nents *
+		mapped_nents = dma_map_sg(jrdev, req->src, src_nents,
+					  DMA_TO_DEVICE);
+		if (mapped_nents == 0) {
+			dev_err(jrdev, "unable to map source for DMA\n");
+			return -ENOMEM;
+		}
+		if (mapped_nents > 1)
+			sec4_sg_bytes = mapped_nents *
 					sizeof(struct sec4_sg_entry);
 		else
 			sec4_sg_bytes = 0;
@@ -1475,6 +1513,7 @@ static int ahash_update_first(struct ahash_request *req)
 		if (!edesc) {
 			dev_err(jrdev,
 				"could not allocate extended descriptor\n");
+			dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 			return -ENOMEM;
 		}
 
@@ -1483,7 +1522,7 @@ static int ahash_update_first(struct ahash_request *req)
 		edesc->dst_dma = 0;
 
 		if (src_nents > 1) {
-			sg_to_sec4_sg_last(req->src, src_nents,
+			sg_to_sec4_sg_last(req->src, mapped_nents,
 					   edesc->sec4_sg, 0);
 			edesc->sec4_sg_dma = dma_map_single(jrdev,
 							    edesc->sec4_sg,
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH RFC 08/11] crypto: caam: add ahash_edesc_alloc() for descriptor allocation
  2015-12-07 19:11 [PATCH RFC 00/13] Further iMX CAAM updates Russell King - ARM Linux
                   ` (6 preceding siblings ...)
  2015-12-07 19:12 ` [PATCH RFC 07/11] crypto: caam: check and use dma_map_sg() return code Russell King
@ 2015-12-07 19:12 ` Russell King
  2015-12-07 19:12 ` [PATCH RFC 09/11] crypto: caam: move job descriptor initialisation to ahash_edesc_alloc() Russell King
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Russell King @ 2015-12-07 19:12 UTC (permalink / raw)
  To: Fabio Estevam, Herbert Xu; +Cc: David S. Miller, linux-crypto

Add a helper function to perform the descriptor allocation.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/crypto/caam/caamhash.c | 60 +++++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 27 deletions(-)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 9638e9f4f001..f35f4cfc27a7 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -765,6 +765,25 @@ static void ahash_done_ctx_dst(struct device *jrdev, u32 *desc, u32 err,
 	req->base.complete(&req->base, err);
 }
 
+/*
+ * Allocate an enhanced descriptor, which contains the hardware descriptor
+ * and space for hardware scatter table containing sg_num entries.
+ */
+static struct ahash_edesc *ahash_edesc_alloc(struct caam_hash_ctx *ctx,
+					     int sg_num, gfp_t flags)
+{
+	struct ahash_edesc *edesc;
+	unsigned int sg_size = sg_num * sizeof(struct sec4_sg_entry);
+
+	edesc = kzalloc(sizeof(*edesc) + sg_size, GFP_DMA | flags);
+	if (!edesc) {
+		dev_err(ctx->jrdev, "could not allocate extended descriptor\n");
+		return NULL;
+	}
+
+	return edesc;
+}
+
 /* submit update job descriptor */
 static int ahash_update_ctx(struct ahash_request *req)
 {
@@ -813,11 +832,9 @@ static int ahash_update_ctx(struct ahash_request *req)
 		 * allocate space for base edesc and hw desc commands,
 		 * link tables
 		 */
-		edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes,
-				GFP_DMA | flags);
+		edesc = ahash_edesc_alloc(ctx, sec4_sg_src_index + mapped_nents,
+					  flags);
 		if (!edesc) {
-			dev_err(jrdev,
-				"could not allocate extended descriptor\n");
 			dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 			return -ENOMEM;
 		}
@@ -930,11 +947,9 @@ static int ahash_final_ctx(struct ahash_request *req)
 	sec4_sg_bytes = sec4_sg_src_index * sizeof(struct sec4_sg_entry);
 
 	/* allocate space for base edesc and hw desc commands, link tables */
-	edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes, GFP_DMA | flags);
-	if (!edesc) {
-		dev_err(jrdev, "could not allocate extended descriptor\n");
+	edesc = ahash_edesc_alloc(ctx, sec4_sg_src_index, flags);
+	if (!edesc)
 		return -ENOMEM;
-	}
 
 	sh_len = desc_len(sh_desc);
 	desc = edesc->hw_desc;
@@ -1026,9 +1041,9 @@ static int ahash_finup_ctx(struct ahash_request *req)
 			 sizeof(struct sec4_sg_entry);
 
 	/* allocate space for base edesc and hw desc commands, link tables */
-	edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes, GFP_DMA | flags);
+	edesc = ahash_edesc_alloc(ctx, sec4_sg_src_index + mapped_nents,
+				  flags);
 	if (!edesc) {
-		dev_err(jrdev, "could not allocate extended descriptor\n");
 		dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 		return -ENOMEM;
 	}
@@ -1122,9 +1137,9 @@ static int ahash_digest(struct ahash_request *req)
 		sec4_sg_bytes = 0;
 
 	/* allocate space for base edesc and hw desc commands, link tables */
-	edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes, GFP_DMA | flags);
+	edesc = ahash_edesc_alloc(ctx, mapped_nents > 1 ? mapped_nents : 0,
+				  flags);
 	if (!edesc) {
-		dev_err(jrdev, "could not allocate extended descriptor\n");
 		dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 		return -ENOMEM;
 	}
@@ -1198,13 +1213,10 @@ static int ahash_final_no_ctx(struct ahash_request *req)
 	int sh_len;
 
 	/* allocate space for base edesc and hw desc commands, link tables */
-	edesc = kzalloc(sizeof(*edesc), GFP_DMA | flags);
-	if (!edesc) {
-		dev_err(jrdev, "could not allocate extended descriptor\n");
+	edesc = ahash_edesc_alloc(ctx, 0, flags);
+	if (!edesc)
 		return -ENOMEM;
-	}
 
-	edesc->sec4_sg_bytes = 0;
 	sh_len = desc_len(sh_desc);
 	desc = edesc->hw_desc;
 	init_job_desc_shared(desc, ptr, sh_len, HDR_SHARE_DEFER | HDR_REVERSE);
@@ -1287,11 +1299,8 @@ static int ahash_update_no_ctx(struct ahash_request *req)
 		 * allocate space for base edesc and hw desc commands,
 		 * link tables
 		 */
-		edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes,
-				GFP_DMA | flags);
+		edesc = ahash_edesc_alloc(ctx, 1 + mapped_nents, flags);
 		if (!edesc) {
-			dev_err(jrdev,
-				"could not allocate extended descriptor\n");
 			dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 			return -ENOMEM;
 		}
@@ -1406,9 +1415,8 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
 			 sizeof(struct sec4_sg_entry);
 
 	/* allocate space for base edesc and hw desc commands, link tables */
-	edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes, GFP_DMA | flags);
+	edesc = ahash_edesc_alloc(ctx, sec4_sg_src_index + mapped_nents, flags);
 	if (!edesc) {
-		dev_err(jrdev, "could not allocate extended descriptor\n");
 		dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 		return -ENOMEM;
 	}
@@ -1508,11 +1516,9 @@ static int ahash_update_first(struct ahash_request *req)
 		 * allocate space for base edesc and hw desc commands,
 		 * link tables
 		 */
-		edesc = kzalloc(sizeof(*edesc) + sec4_sg_bytes,
-				GFP_DMA | flags);
+		edesc = ahash_edesc_alloc(ctx, mapped_nents > 1 ?
+					  mapped_nents : 0, flags);
 		if (!edesc) {
-			dev_err(jrdev,
-				"could not allocate extended descriptor\n");
 			dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 			return -ENOMEM;
 		}
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH RFC 09/11] crypto: caam: move job descriptor initialisation to ahash_edesc_alloc()
  2015-12-07 19:11 [PATCH RFC 00/13] Further iMX CAAM updates Russell King - ARM Linux
                   ` (7 preceding siblings ...)
  2015-12-07 19:12 ` [PATCH RFC 08/11] crypto: caam: add ahash_edesc_alloc() for descriptor allocation Russell King
@ 2015-12-07 19:12 ` Russell King
  2015-12-07 19:12 ` [PATCH RFC 10/11] crypto: caam: add ahash_edesc_add_src() Russell King
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Russell King @ 2015-12-07 19:12 UTC (permalink / raw)
  To: Fabio Estevam, Herbert Xu; +Cc: David S. Miller, linux-crypto

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/crypto/caam/caamhash.c | 84 +++++++++++++++++-------------------------
 1 file changed, 34 insertions(+), 50 deletions(-)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index f35f4cfc27a7..241268d108ec 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -770,7 +770,9 @@ static void ahash_done_ctx_dst(struct device *jrdev, u32 *desc, u32 err,
  * and space for hardware scatter table containing sg_num entries.
  */
 static struct ahash_edesc *ahash_edesc_alloc(struct caam_hash_ctx *ctx,
-					     int sg_num, gfp_t flags)
+					     int sg_num, u32 *sh_desc,
+					     dma_addr_t sh_desc_dma,
+					     gfp_t flags)
 {
 	struct ahash_edesc *edesc;
 	unsigned int sg_size = sg_num * sizeof(struct sec4_sg_entry);
@@ -781,6 +783,9 @@ static struct ahash_edesc *ahash_edesc_alloc(struct caam_hash_ctx *ctx,
 		return NULL;
 	}
 
+	init_job_desc_shared(edesc->hw_desc, sh_desc_dma, desc_len(sh_desc),
+			     HDR_SHARE_DEFER | HDR_REVERSE);
+
 	return edesc;
 }
 
@@ -799,12 +804,10 @@ static int ahash_update_ctx(struct ahash_request *req)
 	int *next_buflen = state->current_buf ? &state->buflen_0 :
 			   &state->buflen_1, last_buflen;
 	int in_len = *buflen + req->nbytes, to_hash;
-	u32 *sh_desc = ctx->sh_desc_update, *desc;
-	dma_addr_t ptr = ctx->sh_desc_update_dma;
+	u32 *desc;
 	int src_nents, mapped_nents, sec4_sg_bytes, sec4_sg_src_index;
 	struct ahash_edesc *edesc;
 	int ret = 0;
-	int sh_len;
 
 	last_buflen = *next_buflen;
 	*next_buflen = in_len & (crypto_tfm_alg_blocksize(&ahash->base) - 1);
@@ -833,7 +836,8 @@ static int ahash_update_ctx(struct ahash_request *req)
 		 * link tables
 		 */
 		edesc = ahash_edesc_alloc(ctx, sec4_sg_src_index + mapped_nents,
-					  flags);
+					  ctx->sh_desc_update,
+					  ctx->sh_desc_update_dma, flags);
 		if (!edesc) {
 			dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 			return -ENOMEM;
@@ -871,10 +875,7 @@ static int ahash_update_ctx(struct ahash_request *req)
 
 		state->current_buf = !state->current_buf;
 
-		sh_len = desc_len(sh_desc);
 		desc = edesc->hw_desc;
-		init_job_desc_shared(desc, ptr, sh_len, HDR_SHARE_DEFER |
-				     HDR_REVERSE);
 
 		edesc->sec4_sg_dma = dma_map_single(jrdev, edesc->sec4_sg,
 						     sec4_sg_bytes,
@@ -935,25 +936,23 @@ static int ahash_final_ctx(struct ahash_request *req)
 	int buflen = state->current_buf ? state->buflen_1 : state->buflen_0;
 	int last_buflen = state->current_buf ? state->buflen_0 :
 			  state->buflen_1;
-	u32 *sh_desc = ctx->sh_desc_fin, *desc;
-	dma_addr_t ptr = ctx->sh_desc_fin_dma;
+	u32 *desc;
 	int sec4_sg_bytes, sec4_sg_src_index;
 	int digestsize = crypto_ahash_digestsize(ahash);
 	struct ahash_edesc *edesc;
 	int ret = 0;
-	int sh_len;
 
 	sec4_sg_src_index = 1 + (buflen ? 1 : 0);
 	sec4_sg_bytes = sec4_sg_src_index * sizeof(struct sec4_sg_entry);
 
 	/* allocate space for base edesc and hw desc commands, link tables */
-	edesc = ahash_edesc_alloc(ctx, sec4_sg_src_index, flags);
+	edesc = ahash_edesc_alloc(ctx, sec4_sg_src_index,
+				  ctx->sh_desc_fin, ctx->sh_desc_fin_dma,
+				  flags);
 	if (!edesc)
 		return -ENOMEM;
 
-	sh_len = desc_len(sh_desc);
 	desc = edesc->hw_desc;
-	init_job_desc_shared(desc, ptr, sh_len, HDR_SHARE_DEFER | HDR_REVERSE);
 
 	edesc->sec4_sg_bytes = sec4_sg_bytes;
 	edesc->src_nents = 0;
@@ -1020,14 +1019,12 @@ static int ahash_finup_ctx(struct ahash_request *req)
 	int buflen = state->current_buf ? state->buflen_1 : state->buflen_0;
 	int last_buflen = state->current_buf ? state->buflen_0 :
 			  state->buflen_1;
-	u32 *sh_desc = ctx->sh_desc_finup, *desc;
-	dma_addr_t ptr = ctx->sh_desc_finup_dma;
+	u32 *desc;
 	int sec4_sg_bytes, sec4_sg_src_index;
 	int src_nents, mapped_nents;
 	int digestsize = crypto_ahash_digestsize(ahash);
 	struct ahash_edesc *edesc;
 	int ret = 0;
-	int sh_len;
 
 	src_nents = sg_nents_for_len(req->src, req->nbytes);
 	mapped_nents = dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
@@ -1042,15 +1039,14 @@ static int ahash_finup_ctx(struct ahash_request *req)
 
 	/* allocate space for base edesc and hw desc commands, link tables */
 	edesc = ahash_edesc_alloc(ctx, sec4_sg_src_index + mapped_nents,
+				  ctx->sh_desc_finup, ctx->sh_desc_finup_dma,
 				  flags);
 	if (!edesc) {
 		dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 		return -ENOMEM;
 	}
 
-	sh_len = desc_len(sh_desc);
 	desc = edesc->hw_desc;
-	init_job_desc_shared(desc, ptr, sh_len, HDR_SHARE_DEFER | HDR_REVERSE);
 
 	edesc->src_nents = src_nents;
 	edesc->sec4_sg_bytes = sec4_sg_bytes;
@@ -1114,15 +1110,13 @@ static int ahash_digest(struct ahash_request *req)
 	struct device *jrdev = ctx->jrdev;
 	gfp_t flags = (req->base.flags & (CRYPTO_TFM_REQ_MAY_BACKLOG |
 		       CRYPTO_TFM_REQ_MAY_SLEEP)) ? GFP_KERNEL : GFP_ATOMIC;
-	u32 *sh_desc = ctx->sh_desc_digest, *desc;
-	dma_addr_t ptr = ctx->sh_desc_digest_dma;
+	u32 *desc;
 	int digestsize = crypto_ahash_digestsize(ahash);
 	int src_nents, mapped_nents, sec4_sg_bytes;
 	dma_addr_t src_dma;
 	struct ahash_edesc *edesc;
 	int ret = 0;
 	u32 options;
-	int sh_len;
 
 	src_nents = sg_nents_for_len(req->src, req->nbytes);
 	mapped_nents = dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
@@ -1138,6 +1132,7 @@ static int ahash_digest(struct ahash_request *req)
 
 	/* allocate space for base edesc and hw desc commands, link tables */
 	edesc = ahash_edesc_alloc(ctx, mapped_nents > 1 ? mapped_nents : 0,
+				  ctx->sh_desc_digest, ctx->sh_desc_digest_dma,
 				  flags);
 	if (!edesc) {
 		dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
@@ -1147,9 +1142,7 @@ static int ahash_digest(struct ahash_request *req)
 	edesc->sec4_sg_bytes = sec4_sg_bytes;
 	edesc->src_nents = src_nents;
 
-	sh_len = desc_len(sh_desc);
 	desc = edesc->hw_desc;
-	init_job_desc_shared(desc, ptr, sh_len, HDR_SHARE_DEFER | HDR_REVERSE);
 
 	if (src_nents > 1) {
 		sg_to_sec4_sg_last(req->src, mapped_nents, edesc->sec4_sg, 0);
@@ -1205,21 +1198,18 @@ static int ahash_final_no_ctx(struct ahash_request *req)
 		       CRYPTO_TFM_REQ_MAY_SLEEP)) ? GFP_KERNEL : GFP_ATOMIC;
 	u8 *buf = state->current_buf ? state->buf_1 : state->buf_0;
 	int buflen = state->current_buf ? state->buflen_1 : state->buflen_0;
-	u32 *sh_desc = ctx->sh_desc_digest, *desc;
-	dma_addr_t ptr = ctx->sh_desc_digest_dma;
+	u32 *desc;
 	int digestsize = crypto_ahash_digestsize(ahash);
 	struct ahash_edesc *edesc;
 	int ret = 0;
-	int sh_len;
 
 	/* allocate space for base edesc and hw desc commands, link tables */
-	edesc = ahash_edesc_alloc(ctx, 0, flags);
+	edesc = ahash_edesc_alloc(ctx, 0, ctx->sh_desc_digest,
+				  ctx->sh_desc_digest_dma, flags);
 	if (!edesc)
 		return -ENOMEM;
 
-	sh_len = desc_len(sh_desc);
 	desc = edesc->hw_desc;
-	init_job_desc_shared(desc, ptr, sh_len, HDR_SHARE_DEFER | HDR_REVERSE);
 
 	state->buf_dma = dma_map_single(jrdev, buf, buflen, DMA_TO_DEVICE);
 	if (dma_mapping_error(jrdev, state->buf_dma)) {
@@ -1274,10 +1264,8 @@ static int ahash_update_no_ctx(struct ahash_request *req)
 	int in_len = *buflen + req->nbytes, to_hash;
 	int sec4_sg_bytes, src_nents, mapped_nents;
 	struct ahash_edesc *edesc;
-	u32 *desc, *sh_desc = ctx->sh_desc_update_first;
-	dma_addr_t ptr = ctx->sh_desc_update_first_dma;
+	u32 *desc;
 	int ret = 0;
-	int sh_len;
 
 	*next_buflen = in_len & (crypto_tfm_alg_blocksize(&ahash->base) - 1);
 	to_hash = in_len - *next_buflen;
@@ -1299,7 +1287,10 @@ static int ahash_update_no_ctx(struct ahash_request *req)
 		 * allocate space for base edesc and hw desc commands,
 		 * link tables
 		 */
-		edesc = ahash_edesc_alloc(ctx, 1 + mapped_nents, flags);
+		edesc = ahash_edesc_alloc(ctx, 1 + mapped_nents,
+					  ctx->sh_desc_update_first,
+					  ctx->sh_desc_update_first_dma,
+					  flags);
 		if (!edesc) {
 			dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 			return -ENOMEM;
@@ -1322,10 +1313,7 @@ static int ahash_update_no_ctx(struct ahash_request *req)
 
 		state->current_buf = !state->current_buf;
 
-		sh_len = desc_len(sh_desc);
 		desc = edesc->hw_desc;
-		init_job_desc_shared(desc, ptr, sh_len, HDR_SHARE_DEFER |
-				     HDR_REVERSE);
 
 		edesc->sec4_sg_dma = dma_map_single(jrdev, edesc->sec4_sg,
 						    sec4_sg_bytes,
@@ -1395,12 +1383,10 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
 	int buflen = state->current_buf ? state->buflen_1 : state->buflen_0;
 	int last_buflen = state->current_buf ? state->buflen_0 :
 			  state->buflen_1;
-	u32 *sh_desc = ctx->sh_desc_digest, *desc;
-	dma_addr_t ptr = ctx->sh_desc_digest_dma;
+	u32 *desc;
 	int sec4_sg_bytes, sec4_sg_src_index, src_nents, mapped_nents;
 	int digestsize = crypto_ahash_digestsize(ahash);
 	struct ahash_edesc *edesc;
-	int sh_len;
 	int ret = 0;
 
 	src_nents = sg_nents_for_len(req->src, req->nbytes);
@@ -1415,15 +1401,15 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
 			 sizeof(struct sec4_sg_entry);
 
 	/* allocate space for base edesc and hw desc commands, link tables */
-	edesc = ahash_edesc_alloc(ctx, sec4_sg_src_index + mapped_nents, flags);
+	edesc = ahash_edesc_alloc(ctx, sec4_sg_src_index + mapped_nents,
+				  ctx->sh_desc_digest, ctx->sh_desc_digest_dma,
+				  flags);
 	if (!edesc) {
 		dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 		return -ENOMEM;
 	}
 
-	sh_len = desc_len(sh_desc);
 	desc = edesc->hw_desc;
-	init_job_desc_shared(desc, ptr, sh_len, HDR_SHARE_DEFER | HDR_REVERSE);
 
 	edesc->src_nents = src_nents;
 	edesc->sec4_sg_bytes = sec4_sg_bytes;
@@ -1484,14 +1470,12 @@ static int ahash_update_first(struct ahash_request *req)
 	int *next_buflen = state->current_buf ?
 		&state->buflen_1 : &state->buflen_0;
 	int to_hash;
-	u32 *sh_desc = ctx->sh_desc_update_first, *desc;
-	dma_addr_t ptr = ctx->sh_desc_update_first_dma;
+	u32 *desc;
 	int sec4_sg_bytes, src_nents, mapped_nents;
 	dma_addr_t src_dma;
 	u32 options;
 	struct ahash_edesc *edesc;
 	int ret = 0;
-	int sh_len;
 
 	*next_buflen = req->nbytes & (crypto_tfm_alg_blocksize(&ahash->base) -
 				      1);
@@ -1517,7 +1501,10 @@ static int ahash_update_first(struct ahash_request *req)
 		 * link tables
 		 */
 		edesc = ahash_edesc_alloc(ctx, mapped_nents > 1 ?
-					  mapped_nents : 0, flags);
+					  mapped_nents : 0,
+					  ctx->sh_desc_update_first,
+					  ctx->sh_desc_update_first_dma,
+					  flags);
 		if (!edesc) {
 			dma_unmap_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
 			return -ENOMEM;
@@ -1552,10 +1539,7 @@ static int ahash_update_first(struct ahash_request *req)
 			scatterwalk_map_and_copy(next_buf, req->src, to_hash,
 						 *next_buflen, 0);
 
-		sh_len = desc_len(sh_desc);
 		desc = edesc->hw_desc;
-		init_job_desc_shared(desc, ptr, sh_len, HDR_SHARE_DEFER |
-				     HDR_REVERSE);
 
 		append_seq_in_ptr(desc, src_dma, to_hash, options);
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH RFC 10/11] crypto: caam: add ahash_edesc_add_src()
  2015-12-07 19:11 [PATCH RFC 00/13] Further iMX CAAM updates Russell King - ARM Linux
                   ` (8 preceding siblings ...)
  2015-12-07 19:12 ` [PATCH RFC 09/11] crypto: caam: move job descriptor initialisation to ahash_edesc_alloc() Russell King
@ 2015-12-07 19:12 ` Russell King
  2015-12-09 16:21   ` Horia Geantă
  2015-12-07 19:12 ` [PATCH RFC 11/11] crypto: caam: get rid of tasklet Russell King
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 20+ messages in thread
From: Russell King @ 2015-12-07 19:12 UTC (permalink / raw)
  To: Fabio Estevam, Herbert Xu; +Cc: David S. Miller, linux-crypto

Add a helper to map the source scatterlist into the descriptor.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/crypto/caam/caamhash.c | 106 +++++++++++++++++++----------------------
 1 file changed, 49 insertions(+), 57 deletions(-)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 241268d108ec..4e73d3218481 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -789,6 +789,40 @@ static struct ahash_edesc *ahash_edesc_alloc(struct caam_hash_ctx *ctx,
 	return edesc;
 }
 
+static int ahash_edesc_add_src(struct caam_hash_ctx *ctx,
+			       struct ahash_edesc *edesc,
+			       struct ahash_request *req, int nents,
+			       unsigned int first_sg, unsigned int first_bytes)
+{
+	dma_addr_t src_dma;
+	u32 options;
+
+	if (nents > 1 || first_sg) {
+		struct sec4_sg_entry *sg = edesc->sec4_sg;
+		unsigned int sgsize = sizeof(*sg) * (first_sg + nents);
+
+		sg_to_sec4_sg_last(req->src, nents, sg + first_sg, 0);
+
+		src_dma = dma_map_single(ctx->jrdev, sg, sgsize, DMA_TO_DEVICE);
+		if (dma_mapping_error(ctx->jrdev, src_dma)) {
+			dev_err(ctx->jrdev, "unable to map S/G table\n");
+			return -ENOMEM;
+		}
+
+		edesc->sec4_sg_bytes = sgsize;
+		edesc->sec4_sg_dma = src_dma;
+		options = LDST_SGF;
+	} else {
+		src_dma = sg_dma_address(req->src);
+		options = 0;
+	}
+
+	append_seq_in_ptr(edesc->hw_desc, src_dma, first_bytes + req->nbytes,
+			  options);
+
+	return 0;
+}
+
 /* submit update job descriptor */
 static int ahash_update_ctx(struct ahash_request *req)
 {
@@ -1112,11 +1146,9 @@ static int ahash_digest(struct ahash_request *req)
 		       CRYPTO_TFM_REQ_MAY_SLEEP)) ? GFP_KERNEL : GFP_ATOMIC;
 	u32 *desc;
 	int digestsize = crypto_ahash_digestsize(ahash);
-	int src_nents, mapped_nents, sec4_sg_bytes;
-	dma_addr_t src_dma;
+	int src_nents, mapped_nents;
 	struct ahash_edesc *edesc;
 	int ret = 0;
-	u32 options;
 
 	src_nents = sg_nents_for_len(req->src, req->nbytes);
 	mapped_nents = dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
@@ -1125,11 +1157,6 @@ static int ahash_digest(struct ahash_request *req)
 		return -ENOMEM;
 	}
 
-	if (mapped_nents > 1)
-		sec4_sg_bytes = mapped_nents * sizeof(struct sec4_sg_entry);
-	else
-		sec4_sg_bytes = 0;
-
 	/* allocate space for base edesc and hw desc commands, link tables */
 	edesc = ahash_edesc_alloc(ctx, mapped_nents > 1 ? mapped_nents : 0,
 				  ctx->sh_desc_digest, ctx->sh_desc_digest_dma,
@@ -1139,28 +1166,16 @@ static int ahash_digest(struct ahash_request *req)
 		return -ENOMEM;
 	}
 
-	edesc->sec4_sg_bytes = sec4_sg_bytes;
 	edesc->src_nents = src_nents;
 
-	desc = edesc->hw_desc;
-
-	if (src_nents > 1) {
-		sg_to_sec4_sg_last(req->src, mapped_nents, edesc->sec4_sg, 0);
-		edesc->sec4_sg_dma = dma_map_single(jrdev, edesc->sec4_sg,
-					    sec4_sg_bytes, DMA_TO_DEVICE);
-		if (dma_mapping_error(jrdev, edesc->sec4_sg_dma)) {
-			dev_err(jrdev, "unable to map S/G table\n");
-			ahash_unmap(jrdev, edesc, req, digestsize);
-			kfree(edesc);
-			return -ENOMEM;
-		}
-		src_dma = edesc->sec4_sg_dma;
-		options = LDST_SGF;
-	} else {
-		src_dma = sg_dma_address(req->src);
-		options = 0;
+	ret = ahash_edesc_add_src(ctx, edesc, req, mapped_nents, 0, 0);
+	if (ret) {
+		ahash_unmap(jrdev, edesc, req, digestsize);
+		kfree(edesc);
+		return ret;
 	}
-	append_seq_in_ptr(desc, src_dma, req->nbytes, options);
+
+	desc = edesc->hw_desc;
 
 	edesc->dst_dma = map_seq_out_ptr_result(desc, jrdev, req->result,
 						digestsize);
@@ -1471,9 +1486,7 @@ static int ahash_update_first(struct ahash_request *req)
 		&state->buflen_1 : &state->buflen_0;
 	int to_hash;
 	u32 *desc;
-	int sec4_sg_bytes, src_nents, mapped_nents;
-	dma_addr_t src_dma;
-	u32 options;
+	int src_nents, mapped_nents;
 	struct ahash_edesc *edesc;
 	int ret = 0;
 
@@ -1490,11 +1503,6 @@ static int ahash_update_first(struct ahash_request *req)
 			dev_err(jrdev, "unable to map source for DMA\n");
 			return -ENOMEM;
 		}
-		if (mapped_nents > 1)
-			sec4_sg_bytes = mapped_nents *
-					sizeof(struct sec4_sg_entry);
-		else
-			sec4_sg_bytes = 0;
 
 		/*
 		 * allocate space for base edesc and hw desc commands,
@@ -1511,28 +1519,14 @@ static int ahash_update_first(struct ahash_request *req)
 		}
 
 		edesc->src_nents = src_nents;
-		edesc->sec4_sg_bytes = sec4_sg_bytes;
 		edesc->dst_dma = 0;
 
-		if (src_nents > 1) {
-			sg_to_sec4_sg_last(req->src, mapped_nents,
-					   edesc->sec4_sg, 0);
-			edesc->sec4_sg_dma = dma_map_single(jrdev,
-							    edesc->sec4_sg,
-							    sec4_sg_bytes,
-							    DMA_TO_DEVICE);
-			if (dma_mapping_error(jrdev, edesc->sec4_sg_dma)) {
-				dev_err(jrdev, "unable to map S/G table\n");
-				ahash_unmap_ctx(jrdev, edesc, req, ctx->ctx_len,
-						DMA_TO_DEVICE);
-				kfree(edesc);
-				return -ENOMEM;
-			}
-			src_dma = edesc->sec4_sg_dma;
-			options = LDST_SGF;
-		} else {
-			src_dma = sg_dma_address(req->src);
-			options = 0;
+		ret = ahash_edesc_add_src(ctx, edesc, req, mapped_nents, 0, 0);
+		if (ret) {
+			ahash_unmap_ctx(jrdev, edesc, req, ctx->ctx_len,
+					DMA_TO_DEVICE);
+			kfree(edesc);
+			return ret;
 		}
 
 		if (*next_buflen)
@@ -1541,8 +1535,6 @@ static int ahash_update_first(struct ahash_request *req)
 
 		desc = edesc->hw_desc;
 
-		append_seq_in_ptr(desc, src_dma, to_hash, options);
-
 		ret = map_seq_out_ptr_ctx(desc, jrdev, state, ctx->ctx_len);
 		if (ret) {
 			ahash_unmap_ctx(jrdev, edesc, req, ctx->ctx_len,
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH RFC 11/11] crypto: caam: get rid of tasklet
  2015-12-07 19:11 [PATCH RFC 00/13] Further iMX CAAM updates Russell King - ARM Linux
                   ` (9 preceding siblings ...)
  2015-12-07 19:12 ` [PATCH RFC 10/11] crypto: caam: add ahash_edesc_add_src() Russell King
@ 2015-12-07 19:12 ` Russell King
  2015-12-07 20:59 ` [PATCH RFC 00/13] Further iMX CAAM updates Fabio Estevam
  2015-12-09 15:06 ` Horia Geantă
  12 siblings, 0 replies; 20+ messages in thread
From: Russell King @ 2015-12-07 19:12 UTC (permalink / raw)
  To: Fabio Estevam, Herbert Xu; +Cc: David S. Miller, linux-crypto

Threaded interrupts can perform the function of the tasklet, and much
more safely too - without races when trying to take the tasklet and
interrupt down on device removal.

With the old code, there is a window where we call tasklet_kill().  If
the interrupt handler happens to be running on a different CPU, and
subsequently calls tasklet_schedule(), the tasklet will be re-scheduled
for execution.

Switching to a hardirq/threadirq combination implementation avoids this,
and it also means generic code deals with the teardown sequencing of the
threaded and non-threaded parts.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/crypto/caam/intern.h |  1 -
 drivers/crypto/caam/jr.c     | 25 +++++++++----------------
 2 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index e2bcacc1a921..5d4c05074a5c 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -41,7 +41,6 @@ struct caam_drv_private_jr {
 	struct device		*dev;
 	int ridx;
 	struct caam_job_ring __iomem *rregs;	/* JobR's register space */
-	struct tasklet_struct irqtask;
 	int irq;			/* One per queue */
 
 	/* Number of scatterlist crypt transforms active on the JobR */
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index f7e0d8d4c3da..b77ee2b88f37 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -73,8 +73,6 @@ int caam_jr_shutdown(struct device *dev)
 
 	ret = caam_reset_hw_jr(dev);
 
-	tasklet_kill(&jrp->irqtask);
-
 	/* Release interrupt */
 	free_irq(jrp->irq, dev);
 
@@ -130,7 +128,7 @@ static irqreturn_t caam_jr_interrupt(int irq, void *st_dev)
 
 	/*
 	 * Check the output ring for ready responses, kick
-	 * tasklet if jobs done.
+	 * the threaded irq if jobs done.
 	 */
 	irqstate = rd_reg32(&jrp->rregs->jrintstatus);
 	if (!irqstate)
@@ -152,18 +150,13 @@ static irqreturn_t caam_jr_interrupt(int irq, void *st_dev)
 	/* Have valid interrupt at this point, just ACK and trigger */
 	wr_reg32(&jrp->rregs->jrintstatus, irqstate);
 
-	preempt_disable();
-	tasklet_schedule(&jrp->irqtask);
-	preempt_enable();
-
-	return IRQ_HANDLED;
+	return IRQ_WAKE_THREAD;
 }
 
-/* Deferred service handler, run as interrupt-fired tasklet */
-static void caam_jr_dequeue(unsigned long devarg)
+static irqreturn_t caam_jr_threadirq(int irq, void *st_dev)
 {
 	int hw_idx, sw_idx, i, head, tail;
-	struct device *dev = (struct device *)devarg;
+	struct device *dev = st_dev;
 	struct caam_drv_private_jr *jrp = dev_get_drvdata(dev);
 	void (*usercall)(struct device *dev, u32 *desc, u32 status, void *arg);
 	u32 *userdesc, userstatus;
@@ -237,6 +230,8 @@ static void caam_jr_dequeue(unsigned long devarg)
 
 	/* reenable / unmask IRQs */
 	clrbits32(&jrp->rregs->rconfig_lo, JRCFG_IMSK);
+
+	return IRQ_HANDLED;
 }
 
 /**
@@ -394,11 +389,10 @@ static int caam_jr_init(struct device *dev)
 
 	jrp = dev_get_drvdata(dev);
 
-	tasklet_init(&jrp->irqtask, caam_jr_dequeue, (unsigned long)dev);
-
 	/* Connect job ring interrupt handler. */
-	error = request_irq(jrp->irq, caam_jr_interrupt, IRQF_SHARED,
-			    dev_name(dev), dev);
+	error = request_threaded_irq(jrp->irq, caam_jr_interrupt,
+				     caam_jr_threadirq, IRQF_SHARED,
+				     dev_name(dev), dev);
 	if (error) {
 		dev_err(dev, "can't connect JobR %d interrupt (%d)\n",
 			jrp->ridx, jrp->irq);
@@ -460,7 +454,6 @@ static int caam_jr_init(struct device *dev)
 out_free_irq:
 	free_irq(jrp->irq, dev);
 out_kill_deq:
-	tasklet_kill(&jrp->irqtask);
 	return error;
 }
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 00/13] Further iMX CAAM updates
  2015-12-07 19:11 [PATCH RFC 00/13] Further iMX CAAM updates Russell King - ARM Linux
                   ` (10 preceding siblings ...)
  2015-12-07 19:12 ` [PATCH RFC 11/11] crypto: caam: get rid of tasklet Russell King
@ 2015-12-07 20:59 ` Fabio Estevam
  2015-12-09 15:06 ` Horia Geantă
  12 siblings, 0 replies; 20+ messages in thread
From: Fabio Estevam @ 2015-12-07 20:59 UTC (permalink / raw)
  To: Russell King - ARM Linux, Horia Geantă, vicki.milhoan
  Cc: Fabio Estevam, Herbert Xu, David S. Miller, linux-crypto

Horia and Victoria,

On Mon, Dec 7, 2015 at 5:11 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> Here are further imx-caam updates that I've had since before the
> previous merge window.  Please review and (I guess) if Freescale
> folk can provide acks etc that would be nice.  Thanks.

Could you please help reviewing? Thanks

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 00/13] Further iMX CAAM updates
  2015-12-07 19:11 [PATCH RFC 00/13] Further iMX CAAM updates Russell King - ARM Linux
                   ` (11 preceding siblings ...)
  2015-12-07 20:59 ` [PATCH RFC 00/13] Further iMX CAAM updates Fabio Estevam
@ 2015-12-09 15:06 ` Horia Geantă
  2015-12-09 18:21   ` Russell King - ARM Linux
  12 siblings, 1 reply; 20+ messages in thread
From: Horia Geantă @ 2015-12-09 15:06 UTC (permalink / raw)
  To: Russell King - ARM Linux, Fabio Estevam, Herbert Xu
  Cc: David S. Miller, linux-crypto

On 12/7/2015 9:11 PM, Russell King - ARM Linux wrote:
> Here are further imx-caam updates that I've had since before the
> previous merge window.  Please review and (I guess) if Freescale
> folk can provide acks etc that would be nice.  Thanks.

Thanks Russell.

Note that the patch set does not apply cleanly, please rebase on latest
cryptodev-2.6 tree.

I'll add specific comments where needed.

Horia

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 06/11] crypto: caam: ensure that we clean up after an error
  2015-12-07 19:12 ` [PATCH RFC 06/11] crypto: caam: ensure that we clean up after an error Russell King
@ 2015-12-09 15:08   ` Horia Geantă
  2015-12-09 18:30     ` Russell King - ARM Linux
  0 siblings, 1 reply; 20+ messages in thread
From: Horia Geantă @ 2015-12-09 15:08 UTC (permalink / raw)
  To: Russell King, Fabio Estevam, Herbert Xu; +Cc: David S. Miller, linux-crypto

On 12/7/2015 9:12 PM, Russell King wrote:
> Ensure that we clean up allocations and DMA mappings after encountering
> an error rather than just giving up and leaking memory and resources.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

I guess the error cleanup code should be grouped under an "err" label,
instead of duplicating it.

Horia

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 07/11] crypto: caam: check and use dma_map_sg() return code
  2015-12-07 19:12 ` [PATCH RFC 07/11] crypto: caam: check and use dma_map_sg() return code Russell King
@ 2015-12-09 15:20   ` Horia Geantă
  2015-12-09 18:31     ` Russell King - ARM Linux
  0 siblings, 1 reply; 20+ messages in thread
From: Horia Geantă @ 2015-12-09 15:20 UTC (permalink / raw)
  To: Russell King, Fabio Estevam, Herbert Xu; +Cc: David S. Miller, linux-crypto

On 12/7/2015 9:12 PM, Russell King wrote:
> Strictly, dma_map_sg() may coalesce SG entries, but in practise on iMX
> hardware, this will never happen.  However, dma_map_sg() can fail, and
> we completely fail to check its return value.  So, fix this properly.
> 
> Arrange the code to map the scatterlist early, so we know how many
> scatter table entries to allocate, and then fill them in.  This allows
> us to keep relatively simple error cleanup paths.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Some tcrypt tests fail - looks like those with zero plaintext:
caam_jr ffe301000.jr: unable to map source for DMA
alg: hash: digest failed on test 1 for sha1-caam: ret=12
[...]

Need to be careful, dma_map_sg() returning zero is an error only if ptxt
is not null (alternatively src_nents returned by sg_nents_for_len()
could be checked).

> @@ -1091,7 +1102,7 @@ static int ahash_digest(struct ahash_request *req)
>  	u32 *sh_desc = ctx->sh_desc_digest, *desc;
>  	dma_addr_t ptr = ctx->sh_desc_digest_dma;
>  	int digestsize = crypto_ahash_digestsize(ahash);
> -	int src_nents, sec4_sg_bytes;
> +	int src_nents, mapped_nents, sec4_sg_bytes;
>  	dma_addr_t src_dma;
>  	struct ahash_edesc *edesc;
>  	int ret = 0;
> @@ -1099,9 +1110,14 @@ static int ahash_digest(struct ahash_request *req)
>  	int sh_len;
>  
>  	src_nents = sg_nents_for_len(req->src, req->nbytes);
> -	dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
> -	if (src_nents > 1)
> -		sec4_sg_bytes = src_nents * sizeof(struct sec4_sg_entry);
> +	mapped_nents = dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
> +	if (mapped_nents == 0) {
> +		dev_err(jrdev, "unable to map source for DMA\n");
> +		return -ENOMEM;
> +	}

This is at least one of the places where the error condition must change
to smth. like (src_nents != 0 && mapped_nents == 0).

Horia

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 10/11] crypto: caam: add ahash_edesc_add_src()
  2015-12-07 19:12 ` [PATCH RFC 10/11] crypto: caam: add ahash_edesc_add_src() Russell King
@ 2015-12-09 16:21   ` Horia Geantă
  0 siblings, 0 replies; 20+ messages in thread
From: Horia Geantă @ 2015-12-09 16:21 UTC (permalink / raw)
  To: Russell King, Fabio Estevam, Herbert Xu; +Cc: David S. Miller, linux-crypto

On 12/7/2015 9:12 PM, Russell King wrote:
> Add a helper to map the source scatterlist into the descriptor.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

After appending 07/11 ("crypto: caam: check and use dma_map_sg() return code")
as follows:

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 241268d108ec..00b35e763f58 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -1120,7 +1120,7 @@ static int ahash_digest(struct ahash_request *req)

src_nents = sg_nents_for_len(req->src, req->nbytes);
mapped_nents = dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
- if (mapped_nents == 0) {
+ if (src_nents && mapped_nents == 0) {
dev_err(jrdev, "unable to map source for DMA\n");
return -ENOMEM;
}

I still see tcrypts test failures:
caam_jr ffe301000.jr: 4000131c: DECO: desc idx 19: DECO Watchdog timer timeout error
alt: hash: update failed on test 6 for hmac-sha1-caam: ret=-1073746716
alg: hash: Test 4 failed for sha1-caam
00000000: 75 e1 d9 26 df 3b 5c 31 d7 a3 02 ca 79 26 55 0e
00000010: 31 96 8d 9f
[...]

git bisect points to this commit.

> ---
>  drivers/crypto/caam/caamhash.c | 106 +++++++++++++++++++----------------------
>  1 file changed, 49 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
> index 241268d108ec..4e73d3218481 100644
> --- a/drivers/crypto/caam/caamhash.c
> +++ b/drivers/crypto/caam/caamhash.c
> @@ -789,6 +789,40 @@ static struct ahash_edesc *ahash_edesc_alloc(struct caam_hash_ctx *ctx,
>  	return edesc;
>  }
>  
> +static int ahash_edesc_add_src(struct caam_hash_ctx *ctx,
> +			       struct ahash_edesc *edesc,
> +			       struct ahash_request *req, int nents,
> +			       unsigned int first_sg, unsigned int first_bytes)
> +{
> +	dma_addr_t src_dma;
> +	u32 options;
> +
> +	if (nents > 1 || first_sg) {
> +		struct sec4_sg_entry *sg = edesc->sec4_sg;
> +		unsigned int sgsize = sizeof(*sg) * (first_sg + nents);
> +
> +		sg_to_sec4_sg_last(req->src, nents, sg + first_sg, 0);
> +
> +		src_dma = dma_map_single(ctx->jrdev, sg, sgsize, DMA_TO_DEVICE);
> +		if (dma_mapping_error(ctx->jrdev, src_dma)) {
> +			dev_err(ctx->jrdev, "unable to map S/G table\n");
> +			return -ENOMEM;
> +		}
> +
> +		edesc->sec4_sg_bytes = sgsize;
> +		edesc->sec4_sg_dma = src_dma;
> +		options = LDST_SGF;
> +	} else {
> +		src_dma = sg_dma_address(req->src);
> +		options = 0;
> +	}
> +
> +	append_seq_in_ptr(edesc->hw_desc, src_dma, first_bytes + req->nbytes,
> +			  options);
> +
> +	return 0;
> +}
[snip]
> @@ -1471,9 +1486,7 @@ static int ahash_update_first(struct ahash_request *req)
>  		&state->buflen_1 : &state->buflen_0;
>  	int to_hash;
>  	u32 *desc;
> -	int sec4_sg_bytes, src_nents, mapped_nents;
> -	dma_addr_t src_dma;
> -	u32 options;
> +	int src_nents, mapped_nents;
>  	struct ahash_edesc *edesc;
>  	int ret = 0;
>  
> @@ -1490,11 +1503,6 @@ static int ahash_update_first(struct ahash_request *req)
>  			dev_err(jrdev, "unable to map source for DMA\n");
>  			return -ENOMEM;
>  		}
> -		if (mapped_nents > 1)
> -			sec4_sg_bytes = mapped_nents *
> -					sizeof(struct sec4_sg_entry);
> -		else
> -			sec4_sg_bytes = 0;
>  
>  		/*
>  		 * allocate space for base edesc and hw desc commands,
> @@ -1511,28 +1519,14 @@ static int ahash_update_first(struct ahash_request *req)
>  		}
>  
>  		edesc->src_nents = src_nents;
> -		edesc->sec4_sg_bytes = sec4_sg_bytes;
>  		edesc->dst_dma = 0;
>  
> -		if (src_nents > 1) {
> -			sg_to_sec4_sg_last(req->src, mapped_nents,
> -					   edesc->sec4_sg, 0);
> -			edesc->sec4_sg_dma = dma_map_single(jrdev,
> -							    edesc->sec4_sg,
> -							    sec4_sg_bytes,
> -							    DMA_TO_DEVICE);
> -			if (dma_mapping_error(jrdev, edesc->sec4_sg_dma)) {
> -				dev_err(jrdev, "unable to map S/G table\n");
> -				ahash_unmap_ctx(jrdev, edesc, req, ctx->ctx_len,
> -						DMA_TO_DEVICE);
> -				kfree(edesc);
> -				return -ENOMEM;
> -			}
> -			src_dma = edesc->sec4_sg_dma;
> -			options = LDST_SGF;
> -		} else {
> -			src_dma = sg_dma_address(req->src);
> -			options = 0;
> +		ret = ahash_edesc_add_src(ctx, edesc, req, mapped_nents, 0, 0);
> +		if (ret) {
> +			ahash_unmap_ctx(jrdev, edesc, req, ctx->ctx_len,
> +					DMA_TO_DEVICE);
> +			kfree(edesc);
> +			return ret;
>  		}
>  
>  		if (*next_buflen)
> @@ -1541,8 +1535,6 @@ static int ahash_update_first(struct ahash_request *req)
>  
>  		desc = edesc->hw_desc;
>  
> -		append_seq_in_ptr(desc, src_dma, to_hash, options);
> -

The refactoring changes the logic here: instead of hashing over "to_hash" input
bytes, it will do over req->nbytes.

Current patch could be fixed as follows, but I guess it's abusing the
"first_bytes" param:

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 6d903398fb0d..7d40765e94c5 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -792,7 +792,7 @@ static struct ahash_edesc *ahash_edesc_alloc(struct
caam_hash_ctx *ctx,
 static int ahash_edesc_add_src(struct caam_hash_ctx *ctx,
                               struct ahash_edesc *edesc,
                               struct ahash_request *req, int nents,
-                              unsigned int first_sg, unsigned int first_bytes)
+                              unsigned int first_sg, int first_bytes)
 {
        dma_addr_t src_dma;
        u32 options;
@@ -817,7 +817,7 @@ static int ahash_edesc_add_src(struct caam_hash_ctx *ctx,
                options = 0;
        }

-       append_seq_in_ptr(edesc->hw_desc, src_dma, first_bytes + req->nbytes,
+       append_seq_in_ptr(edesc->hw_desc, src_dma, req->nbytes + first_bytes,
                          options);

        return 0;
@@ -1521,7 +1521,8 @@ static int ahash_update_first(struct ahash_request *req)
                edesc->src_nents = src_nents;
                edesc->dst_dma = 0;

-               ret = ahash_edesc_add_src(ctx, edesc, req, mapped_nents, 0, 0);
+               ret = ahash_edesc_add_src(ctx, edesc, req, mapped_nents, 0,
+                                         -*next_buflen);
                if (ret) {
                        ahash_unmap_ctx(jrdev, edesc, req, ctx->ctx_len,
                                        DMA_TO_DEVICE);

Horia

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 00/13] Further iMX CAAM updates
  2015-12-09 15:06 ` Horia Geantă
@ 2015-12-09 18:21   ` Russell King - ARM Linux
  0 siblings, 0 replies; 20+ messages in thread
From: Russell King - ARM Linux @ 2015-12-09 18:21 UTC (permalink / raw)
  To: Horia Geantă
  Cc: Fabio Estevam, Herbert Xu, David S. Miller, linux-crypto

On Wed, Dec 09, 2015 at 05:06:03PM +0200, Horia Geantă wrote:
> On 12/7/2015 9:11 PM, Russell King - ARM Linux wrote:
> > Here are further imx-caam updates that I've had since before the
> > previous merge window.  Please review and (I guess) if Freescale
> > folk can provide acks etc that would be nice.  Thanks.
> 
> Thanks Russell.
> 
> Note that the patch set does not apply cleanly, please rebase on latest
> cryptodev-2.6 tree.

I'm afraid I'm not a fan of digging out random git trees, rebasing
patches on top of them, and then not being able to test them locally
without dragging the entire tree into my own tree.  I'd rather wait
another kernel cycle instead of polluting my own tree with commits
from other trees.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 06/11] crypto: caam: ensure that we clean up after an error
  2015-12-09 15:08   ` Horia Geantă
@ 2015-12-09 18:30     ` Russell King - ARM Linux
  0 siblings, 0 replies; 20+ messages in thread
From: Russell King - ARM Linux @ 2015-12-09 18:30 UTC (permalink / raw)
  To: Horia Geantă
  Cc: Fabio Estevam, Herbert Xu, David S. Miller, linux-crypto

On Wed, Dec 09, 2015 at 05:08:41PM +0200, Horia Geantă wrote:
> On 12/7/2015 9:12 PM, Russell King wrote:
> > Ensure that we clean up allocations and DMA mappings after encountering
> > an error rather than just giving up and leaking memory and resources.
> > 
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> I guess the error cleanup code should be grouped under an "err" label,
> instead of duplicating it.

It'd be useful if you could quote the code you were commenting on please.
I now need to dig out the patch from my git tree to work out the relevance
of your comment is.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 07/11] crypto: caam: check and use dma_map_sg() return code
  2015-12-09 15:20   ` Horia Geantă
@ 2015-12-09 18:31     ` Russell King - ARM Linux
  0 siblings, 0 replies; 20+ messages in thread
From: Russell King - ARM Linux @ 2015-12-09 18:31 UTC (permalink / raw)
  To: Horia Geantă
  Cc: Fabio Estevam, Herbert Xu, David S. Miller, linux-crypto

On Wed, Dec 09, 2015 at 05:20:45PM +0200, Horia Geantă wrote:
> On 12/7/2015 9:12 PM, Russell King wrote:
> > Strictly, dma_map_sg() may coalesce SG entries, but in practise on iMX
> > hardware, this will never happen.  However, dma_map_sg() can fail, and
> > we completely fail to check its return value.  So, fix this properly.
> > 
> > Arrange the code to map the scatterlist early, so we know how many
> > scatter table entries to allocate, and then fill them in.  This allows
> > us to keep relatively simple error cleanup paths.
> > 
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> Some tcrypt tests fail - looks like those with zero plaintext:
> caam_jr ffe301000.jr: unable to map source for DMA
> alg: hash: digest failed on test 1 for sha1-caam: ret=12
> [...]
> 
> Need to be careful, dma_map_sg() returning zero is an error only if ptxt
> is not null (alternatively src_nents returned by sg_nents_for_len()
> could be checked).
> 
> > @@ -1091,7 +1102,7 @@ static int ahash_digest(struct ahash_request *req)
> >  	u32 *sh_desc = ctx->sh_desc_digest, *desc;
> >  	dma_addr_t ptr = ctx->sh_desc_digest_dma;
> >  	int digestsize = crypto_ahash_digestsize(ahash);
> > -	int src_nents, sec4_sg_bytes;
> > +	int src_nents, mapped_nents, sec4_sg_bytes;
> >  	dma_addr_t src_dma;
> >  	struct ahash_edesc *edesc;
> >  	int ret = 0;
> > @@ -1099,9 +1110,14 @@ static int ahash_digest(struct ahash_request *req)
> >  	int sh_len;
> >  
> >  	src_nents = sg_nents_for_len(req->src, req->nbytes);
> > -	dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
> > -	if (src_nents > 1)
> > -		sec4_sg_bytes = src_nents * sizeof(struct sec4_sg_entry);
> > +	mapped_nents = dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
> > +	if (mapped_nents == 0) {
> > +		dev_err(jrdev, "unable to map source for DMA\n");
> > +		return -ENOMEM;
> > +	}
> 
> This is at least one of the places where the error condition must change
> to smth. like (src_nents != 0 && mapped_nents == 0).

I guess we should avoid calling dma_map_sg() and dma_unmap_sg() when
src_nents is zero.  Thanks, I'll work that into the next patch revision.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2015-12-09 18:31 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-07 19:11 [PATCH RFC 00/13] Further iMX CAAM updates Russell King - ARM Linux
2015-12-07 19:12 ` [PATCH RFC 01/11] crypto: caam: fix DMA API mapping leak Russell King
2015-12-07 19:12 ` [PATCH RFC 02/11] crypto: caam: ensure descriptor buffers are cacheline aligned Russell King
2015-12-07 19:12 ` [PATCH RFC 03/11] crypto: caam: incorporate job descriptor into struct ahash_edesc Russell King
2015-12-07 19:12 ` [PATCH RFC 04/11] crypto: caam: mark the hardware descriptor as cache line aligned Russell King
2015-12-07 19:12 ` [PATCH RFC 05/11] crypto: caam: replace sec4_sg pointer with array Russell King
2015-12-07 19:12 ` [PATCH RFC 06/11] crypto: caam: ensure that we clean up after an error Russell King
2015-12-09 15:08   ` Horia Geantă
2015-12-09 18:30     ` Russell King - ARM Linux
2015-12-07 19:12 ` [PATCH RFC 07/11] crypto: caam: check and use dma_map_sg() return code Russell King
2015-12-09 15:20   ` Horia Geantă
2015-12-09 18:31     ` Russell King - ARM Linux
2015-12-07 19:12 ` [PATCH RFC 08/11] crypto: caam: add ahash_edesc_alloc() for descriptor allocation Russell King
2015-12-07 19:12 ` [PATCH RFC 09/11] crypto: caam: move job descriptor initialisation to ahash_edesc_alloc() Russell King
2015-12-07 19:12 ` [PATCH RFC 10/11] crypto: caam: add ahash_edesc_add_src() Russell King
2015-12-09 16:21   ` Horia Geantă
2015-12-07 19:12 ` [PATCH RFC 11/11] crypto: caam: get rid of tasklet Russell King
2015-12-07 20:59 ` [PATCH RFC 00/13] Further iMX CAAM updates Fabio Estevam
2015-12-09 15:06 ` Horia Geantă
2015-12-09 18:21   ` Russell King - ARM Linux

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.