[PATCH 00/11] crypto: ccree - fixes and cleanups

[PATCH 00/11] crypto: ccree - fixes and cleanups

[Gilad Ben-Yossef]

A bunch of fixes and code cleanups for the ccree driver

Gilad Ben-Yossef (8):
  crypto: ccree - fix AEAD decrypt auth fail
  crypto: ccree - turn errors to debug msgs
  crypto: ccree - fix pm wrongful error reporting
  crypto: ccree - cc_do_send_request() is void func
  crypto: ccree - fix PM race condition
  crypto: ccree - split overloaded usage of irq field
  crypto: ccree - make cc_pm_put_suspend() void
  crypto: ccree - erase unneeded inline funcs

Hadar Gat (2):
  crypto: ccree: fix typos in error msgs
  crypto: ccree: fix typo in comment

Ofir Drang (1):
  crypto: ccree - fix FDE descriptor sequence

 drivers/crypto/ccree/cc_aead.c        | 22 +++----
 drivers/crypto/ccree/cc_cipher.c      | 54 ++++++++++++++--
 drivers/crypto/ccree/cc_driver.c      | 16 ++---
 drivers/crypto/ccree/cc_driver.h      |  5 +-
 drivers/crypto/ccree/cc_pm.c          | 37 +++--------
 drivers/crypto/ccree/cc_pm.h          | 17 +----
 drivers/crypto/ccree/cc_request_mgr.c | 90 ++++-----------------------
 drivers/crypto/ccree/cc_request_mgr.h |  8 ---
 8 files changed, 93 insertions(+), 156 deletions(-)

-- 
2.23.0

[PATCH 01/11] crypto: ccree: fix typos in error msgs

[Gilad Ben-Yossef]

From: Hadar Gat <hadar.gat@arm.com>

Fixed typos in ccree error msgs.

Signed-off-by: Hadar Gat <hadar.gat@arm.com>
---
 drivers/crypto/ccree/cc_request_mgr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/ccree/cc_request_mgr.c b/drivers/crypto/ccree/cc_request_mgr.c
index a5606dc04b06..d37b4ab50a25 100644
--- a/drivers/crypto/ccree/cc_request_mgr.c
+++ b/drivers/crypto/ccree/cc_request_mgr.c
@@ -423,7 +423,7 @@ int cc_send_request(struct cc_drvdata *drvdata, struct cc_crypto_req *cc_req,
 
 	rc = cc_pm_get(dev);
 	if (rc) {
-		dev_err(dev, "ssi_power_mgr_runtime_get returned %x\n", rc);
+		dev_err(dev, "cc_pm_get returned %x\n", rc);
 		return rc;
 	}
 
@@ -473,7 +473,7 @@ int cc_send_sync_request(struct cc_drvdata *drvdata,
 
 	rc = cc_pm_get(dev);
 	if (rc) {
-		dev_err(dev, "ssi_power_mgr_runtime_get returned %x\n", rc);
+		dev_err(dev, "cc_pm_get returned %x\n", rc);
 		return rc;
 	}
 
-- 
2.23.0

[PATCH 02/11] crypto: ccree: fix typo in comment

[Gilad Ben-Yossef]

From: Hadar Gat <hadar.gat@arm.com>

Fixed a typo in a commnet.

Signed-off-by: Hadar Gat <hadar.gat@arm.com>
---
 drivers/crypto/ccree/cc_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/ccree/cc_pm.c b/drivers/crypto/ccree/cc_pm.c
index c1066f433a28..4de25c85d127 100644
--- a/drivers/crypto/ccree/cc_pm.c
+++ b/drivers/crypto/ccree/cc_pm.c
@@ -48,7 +48,7 @@ int cc_pm_resume(struct device *dev)
 		dev_err(dev, "failed getting clock back on. We're toast.\n");
 		return rc;
 	}
-	/* wait for Crytpcell reset completion */
+	/* wait for Cryptocell reset completion */
 	if (!cc_wait_for_reset_completion(drvdata)) {
 		dev_err(dev, "Cryptocell reset not completed");
 		return -EBUSY;
-- 
2.23.0

[PATCH 03/11] crypto: ccree - fix AEAD decrypt auth fail

[Gilad Ben-Yossef]

On AEAD decryption authentication failure we are suppose to
zero out the output plaintext buffer. However, we've missed
skipping the optional associated data that may prefix the
ciphertext. This commit fixes this issue.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Fixes: e88b27c8eaa8 ("crypto: ccree - use std api sg_zero_buffer")
Cc: stable@vger.kernel.org
---
 drivers/crypto/ccree/cc_aead.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/ccree/cc_aead.c b/drivers/crypto/ccree/cc_aead.c
index d014c8e063a7..754de302a3b5 100644
--- a/drivers/crypto/ccree/cc_aead.c
+++ b/drivers/crypto/ccree/cc_aead.c
@@ -237,7 +237,7 @@ static void cc_aead_complete(struct device *dev, void *cc_req, int err)
 			 * revealed the decrypted message --> zero its memory.
 			 */
 			sg_zero_buffer(areq->dst, sg_nents(areq->dst),
-				       areq->cryptlen, 0);
+				       areq->cryptlen, areq->assoclen);
 			err = -EBADMSG;
 		}
 	/*ENCRYPT*/
-- 
2.23.0

[PATCH 04/11] crypto: ccree - turn errors to debug msgs

[Gilad Ben-Yossef]

We have several loud error log messages that are already reported
via the normal return code mechanism and produce a lot of noise
when the new testmgr extra test are enabled. Turn these into
debug only messages

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 drivers/crypto/ccree/cc_aead.c   | 20 ++++++++++----------
 drivers/crypto/ccree/cc_cipher.c |  6 +++---
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/crypto/ccree/cc_aead.c b/drivers/crypto/ccree/cc_aead.c
index 754de302a3b5..2fc0e0da790b 100644
--- a/drivers/crypto/ccree/cc_aead.c
+++ b/drivers/crypto/ccree/cc_aead.c
@@ -385,13 +385,13 @@ static int validate_keys_sizes(struct cc_aead_ctx *ctx)
 			return -EINVAL;
 		break;
 	default:
-		dev_err(dev, "Invalid auth_mode=%d\n", ctx->auth_mode);
+		dev_dbg(dev, "Invalid auth_mode=%d\n", ctx->auth_mode);
 		return -EINVAL;
 	}
 	/* Check cipher key size */
 	if (ctx->flow_mode == S_DIN_to_DES) {
 		if (ctx->enc_keylen != DES3_EDE_KEY_SIZE) {
-			dev_err(dev, "Invalid cipher(3DES) key size: %u\n",
+			dev_dbg(dev, "Invalid cipher(3DES) key size: %u\n",
 				ctx->enc_keylen);
 			return -EINVAL;
 		}
@@ -399,7 +399,7 @@ static int validate_keys_sizes(struct cc_aead_ctx *ctx)
 		if (ctx->enc_keylen != AES_KEYSIZE_128 &&
 		    ctx->enc_keylen != AES_KEYSIZE_192 &&
 		    ctx->enc_keylen != AES_KEYSIZE_256) {
-			dev_err(dev, "Invalid cipher(AES) key size: %u\n",
+			dev_dbg(dev, "Invalid cipher(AES) key size: %u\n",
 				ctx->enc_keylen);
 			return -EINVAL;
 		}
@@ -1559,7 +1559,7 @@ static int config_ccm_adata(struct aead_request *req)
 	/* taken from crypto/ccm.c */
 	/* 2 <= L <= 8, so 1 <= L' <= 7. */
 	if (l < 2 || l > 8) {
-		dev_err(dev, "illegal iv value %X\n", req->iv[0]);
+		dev_dbg(dev, "illegal iv value %X\n", req->iv[0]);
 		return -EINVAL;
 	}
 	memcpy(b0, req->iv, AES_BLOCK_SIZE);
@@ -2056,7 +2056,7 @@ static int cc_rfc4309_ccm_encrypt(struct aead_request *req)
 	int rc = -EINVAL;
 
 	if (!valid_assoclen(req)) {
-		dev_err(dev, "invalid Assoclen:%u\n", req->assoclen);
+		dev_dbg(dev, "invalid Assoclen:%u\n", req->assoclen);
 		goto out;
 	}
 
@@ -2106,7 +2106,7 @@ static int cc_rfc4309_ccm_decrypt(struct aead_request *req)
 	int rc = -EINVAL;
 
 	if (!valid_assoclen(req)) {
-		dev_err(dev, "invalid Assoclen:%u\n", req->assoclen);
+		dev_dbg(dev, "invalid Assoclen:%u\n", req->assoclen);
 		goto out;
 	}
 
@@ -2225,7 +2225,7 @@ static int cc_rfc4106_gcm_encrypt(struct aead_request *req)
 	int rc = -EINVAL;
 
 	if (!valid_assoclen(req)) {
-		dev_err(dev, "invalid Assoclen:%u\n", req->assoclen);
+		dev_dbg(dev, "invalid Assoclen:%u\n", req->assoclen);
 		goto out;
 	}
 
@@ -2256,7 +2256,7 @@ static int cc_rfc4543_gcm_encrypt(struct aead_request *req)
 	int rc = -EINVAL;
 
 	if (!valid_assoclen(req)) {
-		dev_err(dev, "invalid Assoclen:%u\n", req->assoclen);
+		dev_dbg(dev, "invalid Assoclen:%u\n", req->assoclen);
 		goto out;
 	}
 
@@ -2290,7 +2290,7 @@ static int cc_rfc4106_gcm_decrypt(struct aead_request *req)
 	int rc = -EINVAL;
 
 	if (!valid_assoclen(req)) {
-		dev_err(dev, "invalid Assoclen:%u\n", req->assoclen);
+		dev_dbg(dev, "invalid Assoclen:%u\n", req->assoclen);
 		goto out;
 	}
 
@@ -2321,7 +2321,7 @@ static int cc_rfc4543_gcm_decrypt(struct aead_request *req)
 	int rc = -EINVAL;
 
 	if (!valid_assoclen(req)) {
-		dev_err(dev, "invalid Assoclen:%u\n", req->assoclen);
+		dev_dbg(dev, "invalid Assoclen:%u\n", req->assoclen);
 		goto out;
 	}
 
diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index 7493a32f12b9..03aa4fb8e6cb 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -302,7 +302,7 @@ static int cc_cipher_sethkey(struct crypto_skcipher *sktfm, const u8 *key,
 	keylen = hki.keylen;
 
 	if (validate_keys_sizes(ctx_p, keylen)) {
-		dev_err(dev, "Unsupported key size %d.\n", keylen);
+		dev_dbg(dev, "Unsupported key size %d.\n", keylen);
 		return -EINVAL;
 	}
 
@@ -392,7 +392,7 @@ static int cc_cipher_setkey(struct crypto_skcipher *sktfm, const u8 *key,
 	/* STAT_PHASE_0: Init and sanity checks */
 
 	if (validate_keys_sizes(ctx_p, keylen)) {
-		dev_err(dev, "Unsupported key size %d.\n", keylen);
+		dev_dbg(dev, "Unsupported key size %d.\n", keylen);
 		return -EINVAL;
 	}
 
@@ -833,7 +833,7 @@ static int cc_cipher_process(struct skcipher_request *req,
 
 	/* TODO: check data length according to mode */
 	if (validate_data_size(ctx_p, nbytes)) {
-		dev_err(dev, "Unsupported data size %d.\n", nbytes);
+		dev_dbg(dev, "Unsupported data size %d.\n", nbytes);
 		rc = -EINVAL;
 		goto exit_process;
 	}
-- 
2.23.0

[PATCH 05/11] crypto: ccree - fix pm wrongful error reporting

[Gilad Ben-Yossef]

pm_runtime_get_sync() can return 1 as a valid (none error) return
code. Treat it as such.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Cc: stable@vger.kernel.org # v4.19+
---
 drivers/crypto/ccree/cc_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/ccree/cc_pm.c b/drivers/crypto/ccree/cc_pm.c
index 4de25c85d127..79c612144310 100644
--- a/drivers/crypto/ccree/cc_pm.c
+++ b/drivers/crypto/ccree/cc_pm.c
@@ -85,7 +85,7 @@ int cc_pm_get(struct device *dev)
 	else
 		pm_runtime_get_noresume(dev);
 
-	return rc;
+	return (rc == 1 ? 0 : rc);
 }
 
 int cc_pm_put_suspend(struct device *dev)
-- 
2.23.0

[PATCH 06/11] crypto: ccree - cc_do_send_request() is void func

[Gilad Ben-Yossef]

cc_do_send_request() cannot fail and always returns
-EINPROGRESS. Turn it into a void function and simplify
code.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 drivers/crypto/ccree/cc_request_mgr.c | 36 ++++++++-------------------
 1 file changed, 11 insertions(+), 25 deletions(-)

diff --git a/drivers/crypto/ccree/cc_request_mgr.c b/drivers/crypto/ccree/cc_request_mgr.c
index d37b4ab50a25..ce09c430c8b9 100644
--- a/drivers/crypto/ccree/cc_request_mgr.c
+++ b/drivers/crypto/ccree/cc_request_mgr.c
@@ -275,12 +275,11 @@ static int cc_queues_status(struct cc_drvdata *drvdata,
  * \param len The crypto sequence length
  * \param add_comp If "true": add an artificial dout DMA to mark completion
  *
- * \return int Returns -EINPROGRESS or error code
  */
-static int cc_do_send_request(struct cc_drvdata *drvdata,
-			      struct cc_crypto_req *cc_req,
-			      struct cc_hw_desc *desc, unsigned int len,
-				bool add_comp)
+static void cc_do_send_request(struct cc_drvdata *drvdata,
+			       struct cc_crypto_req *cc_req,
+			       struct cc_hw_desc *desc, unsigned int len,
+			       bool add_comp)
 {
 	struct cc_req_mgr_handle *req_mgr_h = drvdata->request_mgr_handle;
 	unsigned int used_sw_slots;
@@ -328,9 +327,6 @@ static int cc_do_send_request(struct cc_drvdata *drvdata,
 		/* Update the free slots in HW queue */
 		req_mgr_h->q_free_slots -= total_seq_len;
 	}
-
-	/* Operation still in process */
-	return -EINPROGRESS;
 }
 
 static void cc_enqueue_backlog(struct cc_drvdata *drvdata,
@@ -390,16 +386,10 @@ static void cc_proc_backlog(struct cc_drvdata *drvdata)
 			return;
 		}
 
-		rc = cc_do_send_request(drvdata, &bli->creq, bli->desc,
-					bli->len, false);
-
+		cc_do_send_request(drvdata, &bli->creq, bli->desc, bli->len,
+				   false);
 		spin_unlock(&mgr->hw_lock);
 
-		if (rc != -EINPROGRESS) {
-			cc_pm_put_suspend(dev);
-			creq->user_cb(dev, req, rc);
-		}
-
 		/* Remove ourselves from the backlog list */
 		spin_lock(&mgr->bl_lock);
 		list_del(&bli->list);
@@ -452,8 +442,10 @@ int cc_send_request(struct cc_drvdata *drvdata, struct cc_crypto_req *cc_req,
 		return -EBUSY;
 	}
 
-	if (!rc)
-		rc = cc_do_send_request(drvdata, cc_req, desc, len, false);
+	if (!rc) {
+		cc_do_send_request(drvdata, cc_req, desc, len, false);
+		rc = -EINPROGRESS;
+	}
 
 	spin_unlock_bh(&mgr->hw_lock);
 	return rc;
@@ -493,14 +485,8 @@ int cc_send_sync_request(struct cc_drvdata *drvdata,
 		reinit_completion(&drvdata->hw_queue_avail);
 	}
 
-	rc = cc_do_send_request(drvdata, cc_req, desc, len, true);
+	cc_do_send_request(drvdata, cc_req, desc, len, true);
 	spin_unlock_bh(&mgr->hw_lock);
-
-	if (rc != -EINPROGRESS) {
-		cc_pm_put_suspend(dev);
-		return rc;
-	}
-
 	wait_for_completion(&cc_req->seq_compl);
 	return 0;
 }
-- 
2.23.0

[PATCH 07/11] crypto: ccree - fix FDE descriptor sequence

[Gilad Ben-Yossef]

From: Ofir Drang <ofir.drang@arm.com>

In FDE mode (xts, essiv and bitlocker) the cryptocell hardware requires
that the the XEX key will be loaded after Key1.

Signed-off-by: Ofir Drang <ofir.drang@arm.com>
Cc: stable@vger.kernel.org 
---
 drivers/crypto/ccree/cc_cipher.c | 48 ++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index 03aa4fb8e6cb..7d6252d892d7 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -520,6 +520,7 @@ static void cc_setup_readiv_desc(struct crypto_tfm *tfm,
 	}
 }
 
+
 static void cc_setup_state_desc(struct crypto_tfm *tfm,
 				 struct cipher_req_ctx *req_ctx,
 				 unsigned int ivsize, unsigned int nbytes,
@@ -531,8 +532,6 @@ static void cc_setup_state_desc(struct crypto_tfm *tfm,
 	int cipher_mode = ctx_p->cipher_mode;
 	int flow_mode = ctx_p->flow_mode;
 	int direction = req_ctx->gen_ctx.op_type;
-	dma_addr_t key_dma_addr = ctx_p->user.key_dma_addr;
-	unsigned int key_len = ctx_p->keylen;
 	dma_addr_t iv_dma_addr = req_ctx->gen_ctx.iv_dma_addr;
 	unsigned int du_size = nbytes;
 
@@ -567,6 +566,47 @@ static void cc_setup_state_desc(struct crypto_tfm *tfm,
 		break;
 	case DRV_CIPHER_XTS:
 	case DRV_CIPHER_ESSIV:
+	case DRV_CIPHER_BITLOCKER:
+		break;
+	default:
+		dev_err(dev, "Unsupported cipher mode (%d)\n", cipher_mode);
+	}
+}
+
+
+static void cc_setup_xex_state_desc(struct crypto_tfm *tfm,
+				 struct cipher_req_ctx *req_ctx,
+				 unsigned int ivsize, unsigned int nbytes,
+				 struct cc_hw_desc desc[],
+				 unsigned int *seq_size)
+{
+	struct cc_cipher_ctx *ctx_p = crypto_tfm_ctx(tfm);
+	struct device *dev = drvdata_to_dev(ctx_p->drvdata);
+	int cipher_mode = ctx_p->cipher_mode;
+	int flow_mode = ctx_p->flow_mode;
+	int direction = req_ctx->gen_ctx.op_type;
+	dma_addr_t key_dma_addr = ctx_p->user.key_dma_addr;
+	unsigned int key_len = ctx_p->keylen;
+	dma_addr_t iv_dma_addr = req_ctx->gen_ctx.iv_dma_addr;
+	unsigned int du_size = nbytes;
+
+	struct cc_crypto_alg *cc_alg =
+		container_of(tfm->__crt_alg, struct cc_crypto_alg,
+			     skcipher_alg.base);
+
+	if (cc_alg->data_unit)
+		du_size = cc_alg->data_unit;
+
+	switch (cipher_mode) {
+	case DRV_CIPHER_ECB:
+		break;
+	case DRV_CIPHER_CBC:
+	case DRV_CIPHER_CBC_CTS:
+	case DRV_CIPHER_CTR:
+	case DRV_CIPHER_OFB:
+		break;
+	case DRV_CIPHER_XTS:
+	case DRV_CIPHER_ESSIV:
 	case DRV_CIPHER_BITLOCKER:
 		/* load XEX key */
 		hw_desc_init(&desc[*seq_size]);
@@ -877,12 +917,14 @@ static int cc_cipher_process(struct skcipher_request *req,
 
 	/* STAT_PHASE_2: Create sequence */
 
-	/* Setup IV and XEX key used */
+	/* Setup state (IV)  */
 	cc_setup_state_desc(tfm, req_ctx, ivsize, nbytes, desc, &seq_len);
 	/* Setup MLLI line, if needed */
 	cc_setup_mlli_desc(tfm, req_ctx, dst, src, nbytes, req, desc, &seq_len);
 	/* Setup key */
 	cc_setup_key_desc(tfm, req_ctx, nbytes, desc, &seq_len);
+	/* Setup state (IV and XEX key)  */
+	cc_setup_xex_state_desc(tfm, req_ctx, ivsize, nbytes, desc, &seq_len);
 	/* Data processing */
 	cc_setup_flow_desc(tfm, req_ctx, dst, src, nbytes, desc, &seq_len);
 	/* Read next IV */
-- 
2.23.0

[PATCH 08/11] crypto: ccree - fix PM race condition

[Gilad Ben-Yossef]

The PM code was racy, possibly causing the driver to submit
requests to a powered down device. Fix the race and while
at it simplify the PM code.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Fixes: 1358c13a48c4 ("crypto: ccree - fix resume race condition on init")
Cc: stable@kernel.org # v4.20
---
 drivers/crypto/ccree/cc_driver.h      |  1 +
 drivers/crypto/ccree/cc_pm.c          | 28 ++++-----------
 drivers/crypto/ccree/cc_request_mgr.c | 50 ---------------------------
 drivers/crypto/ccree/cc_request_mgr.h |  8 -----
 4 files changed, 7 insertions(+), 80 deletions(-)

diff --git a/drivers/crypto/ccree/cc_driver.h b/drivers/crypto/ccree/cc_driver.h
index 7b6b5d6f1b33..9d77cfdb10d9 100644
--- a/drivers/crypto/ccree/cc_driver.h
+++ b/drivers/crypto/ccree/cc_driver.h
@@ -160,6 +160,7 @@ struct cc_drvdata {
 	int std_bodies;
 	bool sec_disabled;
 	u32 comp_mask;
+	bool pm_on;
 };
 
 struct cc_crypto_alg {
diff --git a/drivers/crypto/ccree/cc_pm.c b/drivers/crypto/ccree/cc_pm.c
index 79c612144310..ee9e9cba2fbb 100644
--- a/drivers/crypto/ccree/cc_pm.c
+++ b/drivers/crypto/ccree/cc_pm.c
@@ -22,14 +22,8 @@ const struct dev_pm_ops ccree_pm = {
 int cc_pm_suspend(struct device *dev)
 {
 	struct cc_drvdata *drvdata = dev_get_drvdata(dev);
-	int rc;
 
 	dev_dbg(dev, "set HOST_POWER_DOWN_EN\n");
-	rc = cc_suspend_req_queue(drvdata);
-	if (rc) {
-		dev_err(dev, "cc_suspend_req_queue (%x)\n", rc);
-		return rc;
-	}
 	fini_cc_regs(drvdata);
 	cc_iowrite(drvdata, CC_REG(HOST_POWER_DOWN_EN), POWER_DOWN_ENABLE);
 	cc_clk_off(drvdata);
@@ -63,13 +57,6 @@ int cc_pm_resume(struct device *dev)
 	/* check if tee fips error occurred during power down */
 	cc_tee_handle_fips_error(drvdata);
 
-	rc = cc_resume_req_queue(drvdata);
-	if (rc) {
-		dev_err(dev, "cc_resume_req_queue (%x)\n", rc);
-		return rc;
-	}
-
-	/* must be after the queue resuming as it uses the HW queue*/
 	cc_init_hash_sram(drvdata);
 
 	return 0;
@@ -80,10 +67,8 @@ int cc_pm_get(struct device *dev)
 	int rc = 0;
 	struct cc_drvdata *drvdata = dev_get_drvdata(dev);
 
-	if (cc_req_queue_suspended(drvdata))
+	if (drvdata->pm_on)
 		rc = pm_runtime_get_sync(dev);
-	else
-		pm_runtime_get_noresume(dev);
 
 	return (rc == 1 ? 0 : rc);
 }
@@ -93,14 +78,11 @@ int cc_pm_put_suspend(struct device *dev)
 	int rc = 0;
 	struct cc_drvdata *drvdata = dev_get_drvdata(dev);
 
-	if (!cc_req_queue_suspended(drvdata)) {
+	if (drvdata->pm_on) {
 		pm_runtime_mark_last_busy(dev);
 		rc = pm_runtime_put_autosuspend(dev);
-	} else {
-		/* Something wrong happens*/
-		dev_err(dev, "request to suspend already suspended queue");
-		rc = -EBUSY;
 	}
+
 	return rc;
 }
 
@@ -117,7 +99,7 @@ int cc_pm_init(struct cc_drvdata *drvdata)
 	/* must be before the enabling to avoid redundant suspending */
 	pm_runtime_set_autosuspend_delay(dev, CC_SUSPEND_TIMEOUT);
 	pm_runtime_use_autosuspend(dev);
-	/* activate the PM module */
+	/* set us as active - note we won't do PM ops until cc_pm_go()! */
 	return pm_runtime_set_active(dev);
 }
 
@@ -125,9 +107,11 @@ int cc_pm_init(struct cc_drvdata *drvdata)
 void cc_pm_go(struct cc_drvdata *drvdata)
 {
 	pm_runtime_enable(drvdata_to_dev(drvdata));
+	drvdata->pm_on = true;
 }
 
 void cc_pm_fini(struct cc_drvdata *drvdata)
 {
 	pm_runtime_disable(drvdata_to_dev(drvdata));
+	drvdata->pm_on = false;
 }
diff --git a/drivers/crypto/ccree/cc_request_mgr.c b/drivers/crypto/ccree/cc_request_mgr.c
index ce09c430c8b9..9d61e6f12478 100644
--- a/drivers/crypto/ccree/cc_request_mgr.c
+++ b/drivers/crypto/ccree/cc_request_mgr.c
@@ -41,7 +41,6 @@ struct cc_req_mgr_handle {
 #else
 	struct tasklet_struct comptask;
 #endif
-	bool is_runtime_suspended;
 };
 
 struct cc_bl_item {
@@ -664,52 +663,3 @@ static void comp_handler(unsigned long devarg)
 	cc_proc_backlog(drvdata);
 	dev_dbg(dev, "Comp. handler done.\n");
 }
-
-/*
- * resume the queue configuration - no need to take the lock as this happens
- * inside the spin lock protection
- */
-#if defined(CONFIG_PM)
-int cc_resume_req_queue(struct cc_drvdata *drvdata)
-{
-	struct cc_req_mgr_handle *request_mgr_handle =
-		drvdata->request_mgr_handle;
-
-	spin_lock_bh(&request_mgr_handle->hw_lock);
-	request_mgr_handle->is_runtime_suspended = false;
-	spin_unlock_bh(&request_mgr_handle->hw_lock);
-
-	return 0;
-}
-
-/*
- * suspend the queue configuration. Since it is used for the runtime suspend
- * only verify that the queue can be suspended.
- */
-int cc_suspend_req_queue(struct cc_drvdata *drvdata)
-{
-	struct cc_req_mgr_handle *request_mgr_handle =
-						drvdata->request_mgr_handle;
-
-	/* lock the send_request */
-	spin_lock_bh(&request_mgr_handle->hw_lock);
-	if (request_mgr_handle->req_queue_head !=
-	    request_mgr_handle->req_queue_tail) {
-		spin_unlock_bh(&request_mgr_handle->hw_lock);
-		return -EBUSY;
-	}
-	request_mgr_handle->is_runtime_suspended = true;
-	spin_unlock_bh(&request_mgr_handle->hw_lock);
-
-	return 0;
-}
-
-bool cc_req_queue_suspended(struct cc_drvdata *drvdata)
-{
-	struct cc_req_mgr_handle *request_mgr_handle =
-						drvdata->request_mgr_handle;
-
-	return	request_mgr_handle->is_runtime_suspended;
-}
-
-#endif
diff --git a/drivers/crypto/ccree/cc_request_mgr.h b/drivers/crypto/ccree/cc_request_mgr.h
index f46cf766fe4d..ff7746aaaf35 100644
--- a/drivers/crypto/ccree/cc_request_mgr.h
+++ b/drivers/crypto/ccree/cc_request_mgr.h
@@ -40,12 +40,4 @@ void complete_request(struct cc_drvdata *drvdata);
 
 void cc_req_mgr_fini(struct cc_drvdata *drvdata);
 
-#if defined(CONFIG_PM)
-int cc_resume_req_queue(struct cc_drvdata *drvdata);
-
-int cc_suspend_req_queue(struct cc_drvdata *drvdata);
-
-bool cc_req_queue_suspended(struct cc_drvdata *drvdata);
-#endif
-
 #endif /*__REQUEST_MGR_H__*/
-- 
2.23.0

[PATCH 09/11] crypto: ccree - split overloaded usage of irq field

[Gilad Ben-Yossef]

We were using the irq field of the drvdata struct in
an overloaded fahsion - saving the IRQ number during init
and then storing the pending itnerrupt sources during
interrupt in the same field.

This worked because these usage are mutually exclusive but
are confusing. So simplify the code and change the init use
case to use a simple local variable.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 drivers/crypto/ccree/cc_driver.c | 16 ++++++++--------
 drivers/crypto/ccree/cc_driver.h |  4 +---
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/ccree/cc_driver.c b/drivers/crypto/ccree/cc_driver.c
index 1bbe82fce4a5..532bc95a8373 100644
--- a/drivers/crypto/ccree/cc_driver.c
+++ b/drivers/crypto/ccree/cc_driver.c
@@ -271,6 +271,7 @@ static int init_cc_resources(struct platform_device *plat_dev)
 	const struct cc_hw_data *hw_rev;
 	const struct of_device_id *dev_id;
 	struct clk *clk;
+	int irq;
 	int rc = 0;
 
 	new_drvdata = devm_kzalloc(dev, sizeof(*new_drvdata), GFP_KERNEL);
@@ -337,9 +338,9 @@ static int init_cc_resources(struct platform_device *plat_dev)
 		&req_mem_cc_regs->start, new_drvdata->cc_base);
 
 	/* Then IRQ */
-	new_drvdata->irq = platform_get_irq(plat_dev, 0);
-	if (new_drvdata->irq < 0)
-		return new_drvdata->irq;
+	irq = platform_get_irq(plat_dev, 0);
+	if (irq < 0)
+		return irq;
 
 	init_completion(&new_drvdata->hw_queue_avail);
 
@@ -442,14 +443,13 @@ static int init_cc_resources(struct platform_device *plat_dev)
 	dev_info(dev, "ARM CryptoCell %s Driver: HW version 0x%08X/0x%8X, Driver version %s\n",
 		 hw_rev->name, hw_rev_pidr, sig_cidr, DRV_MODULE_VERSION);
 	/* register the driver isr function */
-	rc = devm_request_irq(dev, new_drvdata->irq, cc_isr,
-			      IRQF_SHARED, "ccree", new_drvdata);
+	rc = devm_request_irq(dev, irq, cc_isr, IRQF_SHARED, "ccree",
+			      new_drvdata);
 	if (rc) {
-		dev_err(dev, "Could not register to interrupt %d\n",
-			new_drvdata->irq);
+		dev_err(dev, "Could not register to interrupt %d\n", irq);
 		goto post_clk_err;
 	}
-	dev_dbg(dev, "Registered to IRQ: %d\n", new_drvdata->irq);
+	dev_dbg(dev, "Registered to IRQ: %d\n", irq);
 
 	rc = init_cc_regs(new_drvdata, true);
 	if (rc) {
diff --git a/drivers/crypto/ccree/cc_driver.h b/drivers/crypto/ccree/cc_driver.h
index 9d77cfdb10d9..c227718ba992 100644
--- a/drivers/crypto/ccree/cc_driver.h
+++ b/drivers/crypto/ccree/cc_driver.h
@@ -132,13 +132,11 @@ struct cc_crypto_req {
 /**
  * struct cc_drvdata - driver private data context
  * @cc_base:	virt address of the CC registers
- * @irq:	device IRQ number
- * @irq_mask:	Interrupt mask shadow (1 for masked interrupts)
+ * @irq:	bitmap indicating source of last interrupt
  */
 struct cc_drvdata {
 	void __iomem *cc_base;
 	int irq;
-	u32 irq_mask;
 	struct completion hw_queue_avail; /* wait for HW queue availability */
 	struct platform_device *plat_dev;
 	cc_sram_addr_t mlli_sram_addr;
-- 
2.23.0

[PATCH 10/11] crypto: ccree - make cc_pm_put_suspend() void

[Gilad Ben-Yossef]

cc_pm_put_suspend() return value was never checked and is not
useful. Turn it into a void functions.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 drivers/crypto/ccree/cc_pm.c | 7 ++-----
 drivers/crypto/ccree/cc_pm.h | 7 ++-----
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/crypto/ccree/cc_pm.c b/drivers/crypto/ccree/cc_pm.c
index ee9e9cba2fbb..24c368b866f6 100644
--- a/drivers/crypto/ccree/cc_pm.c
+++ b/drivers/crypto/ccree/cc_pm.c
@@ -73,17 +73,14 @@ int cc_pm_get(struct device *dev)
 	return (rc == 1 ? 0 : rc);
 }
 
-int cc_pm_put_suspend(struct device *dev)
+void cc_pm_put_suspend(struct device *dev)
 {
-	int rc = 0;
 	struct cc_drvdata *drvdata = dev_get_drvdata(dev);
 
 	if (drvdata->pm_on) {
 		pm_runtime_mark_last_busy(dev);
-		rc = pm_runtime_put_autosuspend(dev);
+		pm_runtime_put_autosuspend(dev);
 	}
-
-	return rc;
 }
 
 bool cc_pm_is_dev_suspended(struct device *dev)
diff --git a/drivers/crypto/ccree/cc_pm.h b/drivers/crypto/ccree/cc_pm.h
index a7d98a5da2e1..04289beb6e3e 100644
--- a/drivers/crypto/ccree/cc_pm.h
+++ b/drivers/crypto/ccree/cc_pm.h
@@ -21,7 +21,7 @@ void cc_pm_fini(struct cc_drvdata *drvdata);
 int cc_pm_suspend(struct device *dev);
 int cc_pm_resume(struct device *dev);
 int cc_pm_get(struct device *dev);
-int cc_pm_put_suspend(struct device *dev);
+void cc_pm_put_suspend(struct device *dev);
 bool cc_pm_is_dev_suspended(struct device *dev);
 
 #else
@@ -50,10 +50,7 @@ static inline int cc_pm_get(struct device *dev)
 	return 0;
 }
 
-static inline int cc_pm_put_suspend(struct device *dev)
-{
-	return 0;
-}
+static inline void cc_pm_put_suspend(struct device *dev) {}
 
 static inline bool cc_pm_is_dev_suspended(struct device *dev)
 {
-- 
2.23.0

[PATCH 11/11] crypto: ccree - erase unneeded inline funcs

[Gilad Ben-Yossef]

These inline versions of PM function for the case of CONFIG_PM is
not set are never used. Erase them.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 drivers/crypto/ccree/cc_pm.h | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/crypto/ccree/cc_pm.h b/drivers/crypto/ccree/cc_pm.h
index 04289beb6e3e..80a18e11cae4 100644
--- a/drivers/crypto/ccree/cc_pm.h
+++ b/drivers/crypto/ccree/cc_pm.h
@@ -35,16 +35,6 @@ static inline void cc_pm_go(struct cc_drvdata *drvdata) {}
 
 static inline void cc_pm_fini(struct cc_drvdata *drvdata) {}
 
-static inline int cc_pm_suspend(struct device *dev)
-{
-	return 0;
-}
-
-static inline int cc_pm_resume(struct device *dev)
-{
-	return 0;
-}
-
 static inline int cc_pm_get(struct device *dev)
 {
 	return 0;
-- 
2.23.0

Re: [PATCH 00/11] crypto: ccree - fixes and cleanups

[Herbert Xu]

On Thu, Jan 16, 2020 at 12:14:35PM +0200, Gilad Ben-Yossef wrote:
> A bunch of fixes and code cleanups for the ccree driver
> 
> Gilad Ben-Yossef (8):
>   crypto: ccree - fix AEAD decrypt auth fail
>   crypto: ccree - turn errors to debug msgs
>   crypto: ccree - fix pm wrongful error reporting
>   crypto: ccree - cc_do_send_request() is void func
>   crypto: ccree - fix PM race condition
>   crypto: ccree - split overloaded usage of irq field
>   crypto: ccree - make cc_pm_put_suspend() void
>   crypto: ccree - erase unneeded inline funcs
> 
> Hadar Gat (2):
>   crypto: ccree: fix typos in error msgs
>   crypto: ccree: fix typo in comment
> 
> Ofir Drang (1):
>   crypto: ccree - fix FDE descriptor sequence
> 
>  drivers/crypto/ccree/cc_aead.c        | 22 +++----
>  drivers/crypto/ccree/cc_cipher.c      | 54 ++++++++++++++--
>  drivers/crypto/ccree/cc_driver.c      | 16 ++---
>  drivers/crypto/ccree/cc_driver.h      |  5 +-
>  drivers/crypto/ccree/cc_pm.c          | 37 +++--------
>  drivers/crypto/ccree/cc_pm.h          | 17 +----
>  drivers/crypto/ccree/cc_request_mgr.c | 90 ++++-----------------------
>  drivers/crypto/ccree/cc_request_mgr.h |  8 ---
>  8 files changed, 93 insertions(+), 156 deletions(-)

All applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 00/11] crypto: ccree - fixes and cleanups

[Geert Uytterhoeven]

Hi Gilad,

On Thu, Jan 16, 2020 at 11:25 AM Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> A bunch of fixes and code cleanups for the ccree driver

Thank you!

I wanted to give this a try, but it looks like CCREE is no longer working
on R-Car H3, both with/without this series.

E.g. with renesas-devel[*] and renesas_defconfig +
CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n, I get the following crash:

ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version
0xAF400001/0xDCC63000, Driver version 5.0
alg: No test for authenc(xcbc(aes),cbc(aes)) (authenc-xcbc-aes-cbc-aes-ccree)
alg: No test for authenc(xcbc(aes),rfc3686(ctr(aes)))
(authenc-xcbc-aes-rfc3686-ctr-aes-ccree)
------------[ cut here ]------------
kernel BUG at kernel/dma/swiotlb.c:497!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
CPU: 7 PID: 189 Comm: cryptomgr_test Not tainted 5.5.0-rc7-arm64-renesas #463
Hardware name: Renesas Salvator-X 2nd version board based on r8a77951 (DT)
pstate: 80000005 (Nzcv daif -PAN -UAO)
pc : swiotlb_tbl_map_single+0x30c/0x380
lr : swiotlb_map+0xb0/0x300
sp : ffff800012313430
x29: ffff800012313430 x28: 0000000000000000
x27: 0000000000000000 x26: 0000000738e7e000
x25: ffff0006fa5f8010 x24: 0000000000000000
x23: ffff800011aed000 x22: 0000000000000000
x21: 0000000000000000 x20: 00000000000e8000
x19: ffff80001105e000 x18: ffffffffffffffff
x17: 0000000000000007 x16: 0000000000000001
x15: ffff800010f5f908 x14: ffff800092313cf7
x13: ffff0006ff0b4000 x12: 0000000000000001
x11: 0000000000000003 x10: 0000000000200000
x9 : 0000000000000000 x8 : 0000000000000001
x7 : ffff800011aed9e0 x6 : 0000000000000000
x5 : 0000000000000000 x4 : 0000000000000000
x3 : 0000000000000000 x2 : 0000000000000000
x1 : 0000000074000000 x0 : 0000000000000000
Call trace:
 swiotlb_tbl_map_single+0x30c/0x380
 swiotlb_map+0xb0/0x300
 dma_direct_map_page+0xb8/0x140
 dma_direct_map_sg+0x78/0xe0
 cc_map_sg+0x10c/0x1a8
 cc_map_aead_request+0x160/0x990
 cc_proc_aead+0x140/0xef8
 cc_aead_encrypt+0x48/0x68
 crypto_aead_encrypt+0x20/0x30
 test_aead_vec_cfg+0x20c/0x848
 test_aead+0xb8/0x140
 alg_test_aead+0x94/0x178
 alg_test+0x108/0x3f8
 cryptomgr_test+0x40/0x48
 kthread+0x11c/0x120
 ret_from_fork+0x10/0x18
Code: f9402fbc 17ffffa0 f9000bb3 f9002fbc (d4210000)
---[ end trace 272124cd4e3fd6f0 ]---
note: cryptomgr_test[189] exited with preempt_count 1
------------[ cut here ]------------

FWIW, the same happens on R-Car H3 ES1.0.
I haven't tried investigating when it stopped working.
I stopped running the crypto manager tests when they were broken by
CONFIG_HARDENED_USERCOPY_PAGESPAN=y.

Do you have a clue?
Thanks!

[*] https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-devel.git/

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 00/11] crypto: ccree - fixes and cleanups

[Gilad Ben-Yossef]

Hi,

On Wed, Jan 22, 2020 at 6:51 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Gilad,
>
> On Thu, Jan 16, 2020 at 11:25 AM Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> > A bunch of fixes and code cleanups for the ccree driver
>
> Thank you!
>
> I wanted to give this a try, but it looks like CCREE is no longer working
> on R-Car H3, both with/without this series.
>
> E.g. with renesas-devel[*] and renesas_defconfig +
> CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n, I get the following crash:
>

Thank you for the bug report Geert!

My R-Car board is on loan at the moment to another project. I didn't
see this on our internal test board.
I will track down my R-Car board and reproduce this - hopefully
beginning of next week and will get back to you.

Thanks again,
Gilad

> ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version
> 0xAF400001/0xDCC63000, Driver version 5.0
> alg: No test for authenc(xcbc(aes),cbc(aes)) (authenc-xcbc-aes-cbc-aes-ccree)
> alg: No test for authenc(xcbc(aes),rfc3686(ctr(aes)))
> (authenc-xcbc-aes-rfc3686-ctr-aes-ccree)
> ------------[ cut here ]------------
> kernel BUG at kernel/dma/swiotlb.c:497!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> CPU: 7 PID: 189 Comm: cryptomgr_test Not tainted 5.5.0-rc7-arm64-renesas #463
> Hardware name: Renesas Salvator-X 2nd version board based on r8a77951 (DT)
> pstate: 80000005 (Nzcv daif -PAN -UAO)
> pc : swiotlb_tbl_map_single+0x30c/0x380
> lr : swiotlb_map+0xb0/0x300
> sp : ffff800012313430
> x29: ffff800012313430 x28: 0000000000000000
> x27: 0000000000000000 x26: 0000000738e7e000
> x25: ffff0006fa5f8010 x24: 0000000000000000
> x23: ffff800011aed000 x22: 0000000000000000
> x21: 0000000000000000 x20: 00000000000e8000
> x19: ffff80001105e000 x18: ffffffffffffffff
> x17: 0000000000000007 x16: 0000000000000001
> x15: ffff800010f5f908 x14: ffff800092313cf7
> x13: ffff0006ff0b4000 x12: 0000000000000001
> x11: 0000000000000003 x10: 0000000000200000
> x9 : 0000000000000000 x8 : 0000000000000001
> x7 : ffff800011aed9e0 x6 : 0000000000000000
> x5 : 0000000000000000 x4 : 0000000000000000
> x3 : 0000000000000000 x2 : 0000000000000000
> x1 : 0000000074000000 x0 : 0000000000000000
> Call trace:
>  swiotlb_tbl_map_single+0x30c/0x380
>  swiotlb_map+0xb0/0x300
>  dma_direct_map_page+0xb8/0x140
>  dma_direct_map_sg+0x78/0xe0
>  cc_map_sg+0x10c/0x1a8
>  cc_map_aead_request+0x160/0x990
>  cc_proc_aead+0x140/0xef8
>  cc_aead_encrypt+0x48/0x68
>  crypto_aead_encrypt+0x20/0x30
>  test_aead_vec_cfg+0x20c/0x848
>  test_aead+0xb8/0x140
>  alg_test_aead+0x94/0x178
>  alg_test+0x108/0x3f8
>  cryptomgr_test+0x40/0x48
>  kthread+0x11c/0x120
>  ret_from_fork+0x10/0x18
> Code: f9402fbc 17ffffa0 f9000bb3 f9002fbc (d4210000)
> ---[ end trace 272124cd4e3fd6f0 ]---
> note: cryptomgr_test[189] exited with preempt_count 1
> ------------[ cut here ]------------
>
> FWIW, the same happens on R-Car H3 ES1.0.
> I haven't tried investigating when it stopped working.
> I stopped running the crypto manager tests when they were broken by
> CONFIG_HARDENED_USERCOPY_PAGESPAN=y.
>
> Do you have a clue?
> Thanks!
>
> [*] https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-devel.git/
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

values of β will give rise to dom!

Re: [PATCH 00/11] crypto: ccree - fixes and cleanups

[Geert Uytterhoeven]

Hi Gilad

On Thu, Jan 23, 2020 at 12:44 PM Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Wed, Jan 22, 2020 at 6:51 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Thu, Jan 16, 2020 at 11:25 AM Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> > > A bunch of fixes and code cleanups for the ccree driver
> >
> > Thank you!
> >
> > I wanted to give this a try, but it looks like CCREE is no longer working
> > on R-Car H3, both with/without this series.
> >
> > E.g. with renesas-devel[*] and renesas_defconfig +
> > CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n, I get the following crash:
>
> Thank you for the bug report Geert!
>
> My R-Car board is on loan at the moment to another project. I didn't
> see this on our internal test board.
> I will track down my R-Car board and reproduce this - hopefully
> beginning of next week and will get back to you.

In the mean time, I've bisected this failure to commit cdfee5623290bc89
("driver core: initialize a default DMA mask for platform device").
However, this looks like a red herring, and seems to be only an exposer
of an underlying problem.

What's happening is that cc_map_aead_request() receives a request with
cryptlen = 0.  Due to DRV_CRYPTO_DIRECTION_ENCRYPT, the length to map is
increased by 8.  This seems to works fine if there is sufficient space
in the request's scatterlist.  However, if the scatterlist has only a
single entry of size zero, cc_map_sg() tries to map a zero-length DMA
buffer, and the BUG)() is triggered.

I noticed commits 04e6d25c5bb244c1 ("crypto: caam - fix zero-length
buffer DMA mapping") and 07586d3ddf284dd7 ("crypto: caam/qi2 - fix
zero-length buffer DMA mapping") fixed other issues related to
zero-length DMA buffers.  But this one seems to be different, and a bit
more complex.

Adding "if (!req->cryptlen) return 0;" to the top of cc_proc_aead() fixes
the crash, but makes the tests fail:

    cc_proc_aead:1946: cryptlen is zero!
    alg: aead: ccm-aes-ccree encryption test failed (wrong result) on
test vector 9, cfg="in-place"
    cc_proc_aead:1946: cryptlen is zero!
    alg: aead: gcm-aes-ccree encryption test failed (wrong result) on
test vector 0, cfg="in-place"

Do you have a clue?
Thanks!

> > ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version
> > 0xAF400001/0xDCC63000, Driver version 5.0
> > alg: No test for authenc(xcbc(aes),cbc(aes)) (authenc-xcbc-aes-cbc-aes-ccree)
> > alg: No test for authenc(xcbc(aes),rfc3686(ctr(aes)))
> > (authenc-xcbc-aes-rfc3686-ctr-aes-ccree)
> > ------------[ cut here ]------------
> > kernel BUG at kernel/dma/swiotlb.c:497!
> > Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> > CPU: 7 PID: 189 Comm: cryptomgr_test Not tainted 5.5.0-rc7-arm64-renesas #463
> > Hardware name: Renesas Salvator-X 2nd version board based on r8a77951 (DT)
> > pstate: 80000005 (Nzcv daif -PAN -UAO)
> > pc : swiotlb_tbl_map_single+0x30c/0x380
> > lr : swiotlb_map+0xb0/0x300
> > sp : ffff800012313430
> > x29: ffff800012313430 x28: 0000000000000000
> > x27: 0000000000000000 x26: 0000000738e7e000
> > x25: ffff0006fa5f8010 x24: 0000000000000000
> > x23: ffff800011aed000 x22: 0000000000000000
> > x21: 0000000000000000 x20: 00000000000e8000
> > x19: ffff80001105e000 x18: ffffffffffffffff
> > x17: 0000000000000007 x16: 0000000000000001
> > x15: ffff800010f5f908 x14: ffff800092313cf7
> > x13: ffff0006ff0b4000 x12: 0000000000000001
> > x11: 0000000000000003 x10: 0000000000200000
> > x9 : 0000000000000000 x8 : 0000000000000001
> > x7 : ffff800011aed9e0 x6 : 0000000000000000
> > x5 : 0000000000000000 x4 : 0000000000000000
> > x3 : 0000000000000000 x2 : 0000000000000000
> > x1 : 0000000074000000 x0 : 0000000000000000
> > Call trace:
> >  swiotlb_tbl_map_single+0x30c/0x380
> >  swiotlb_map+0xb0/0x300
> >  dma_direct_map_page+0xb8/0x140
> >  dma_direct_map_sg+0x78/0xe0
> >  cc_map_sg+0x10c/0x1a8
> >  cc_map_aead_request+0x160/0x990
> >  cc_proc_aead+0x140/0xef8
> >  cc_aead_encrypt+0x48/0x68
> >  crypto_aead_encrypt+0x20/0x30
> >  test_aead_vec_cfg+0x20c/0x848
> >  test_aead+0xb8/0x140
> >  alg_test_aead+0x94/0x178
> >  alg_test+0x108/0x3f8
> >  cryptomgr_test+0x40/0x48
> >  kthread+0x11c/0x120
> >  ret_from_fork+0x10/0x18
> > Code: f9402fbc 17ffffa0 f9000bb3 f9002fbc (d4210000)
> > ---[ end trace 272124cd4e3fd6f0 ]---
> > note: cryptomgr_test[189] exited with preempt_count 1
> > ------------[ cut here ]------------
> >
> > FWIW, the same happens on R-Car H3 ES1.0.
> > I haven't tried investigating when it stopped working.
> > I stopped running the crypto manager tests when they were broken by
> > CONFIG_HARDENED_USERCOPY_PAGESPAN=y.
> >
> > Do you have a clue?
> > Thanks!
> >
> > [*] https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-devel.git/

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 00/11] crypto: ccree - fixes and cleanups

[Gilad Ben-Yossef]

On Thu, Jan 23, 2020 at 5:46 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Gilad
>
> On Thu, Jan 23, 2020 at 12:44 PM Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> > On Wed, Jan 22, 2020 at 6:51 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Thu, Jan 16, 2020 at 11:25 AM Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> > > > A bunch of fixes and code cleanups for the ccree driver
> > >
> > > Thank you!
> > >
> > > I wanted to give this a try, but it looks like CCREE is no longer working
> > > on R-Car H3, both with/without this series.
> > >
> > > E.g. with renesas-devel[*] and renesas_defconfig +
> > > CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n, I get the following crash:
> >
> > Thank you for the bug report Geert!
> >
> > My R-Car board is on loan at the moment to another project. I didn't
> > see this on our internal test board.
> > I will track down my R-Car board and reproduce this - hopefully
> > beginning of next week and will get back to you.
>
> In the mean time, I've bisected this failure to commit cdfee5623290bc89
> ("driver core: initialize a default DMA mask for platform device").
> However, this looks like a red herring, and seems to be only an exposer
> of an underlying problem.

Thank you for continue digging into this.

> What's happening is that cc_map_aead_request() receives a request with
> cryptlen = 0.  Due to DRV_CRYPTO_DIRECTION_ENCRYPT, the length to map is
> increased by 8.  This seems to works fine if there is sufficient space
> in the request's scatterlist.  However, if the scatterlist has only a
> single entry of size zero, cc_map_sg() tries to map a zero-length DMA
> buffer, and the BUG)() is triggered.
>

OK, this does rings a bell - can you verify please if
CONFIG_CRYPTO_MANAGER_EXTRA_TESTS is enabled and if it does can you
see if it happens if it is turned off?

There was an issue I've seen happen only with
CONFIG_CRYPTO_MANAGER_EXTRA_TESTS enabled and only after commit
49763fc6b1af4 ("crypto: testmgr - generate inauthentic AEAD test
vectors") which I am chasing. I was half starting to believe it
was an issue in the testmgr commit and not the ccree driver.

However, the stack trace doesn't look exactly the same but the
description of the issue does. It seems you are seeing the same issue
in another code path.

Thank you - this is very helpful!

I now have a better direction to look into...

Gilad

Re: [PATCH 00/11] crypto: ccree - fixes and cleanups

[Geert Uytterhoeven]

Hi Gilad,

On Thu, Jan 23, 2020 at 7:19 PM Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Thu, Jan 23, 2020 at 5:46 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Thu, Jan 23, 2020 at 12:44 PM Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> > > On Wed, Jan 22, 2020 at 6:51 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Thu, Jan 16, 2020 at 11:25 AM Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> > > > > A bunch of fixes and code cleanups for the ccree driver
> > > >
> > > > Thank you!
> > > >
> > > > I wanted to give this a try, but it looks like CCREE is no longer working
> > > > on R-Car H3, both with/without this series.
> > > >
> > > > E.g. with renesas-devel[*] and renesas_defconfig +
> > > > CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n, I get the following crash:
> > >
> > > Thank you for the bug report Geert!
> > >
> > > My R-Car board is on loan at the moment to another project. I didn't
> > > see this on our internal test board.
> > > I will track down my R-Car board and reproduce this - hopefully
> > > beginning of next week and will get back to you.
> >
> > In the mean time, I've bisected this failure to commit cdfee5623290bc89
> > ("driver core: initialize a default DMA mask for platform device").
> > However, this looks like a red herring, and seems to be only an exposer
> > of an underlying problem.
>
> Thank you for continue digging into this.
>
> > What's happening is that cc_map_aead_request() receives a request with
> > cryptlen = 0.  Due to DRV_CRYPTO_DIRECTION_ENCRYPT, the length to map is
> > increased by 8.  This seems to works fine if there is sufficient space
> > in the request's scatterlist.  However, if the scatterlist has only a
> > single entry of size zero, cc_map_sg() tries to map a zero-length DMA
> > buffer, and the BUG)() is triggered.
> >
>
> OK, this does rings a bell - can you verify please if
> CONFIG_CRYPTO_MANAGER_EXTRA_TESTS is enabled and if it does can you
> see if it happens if it is turned off?

No, I didn't have that option enabled.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 00/11] crypto: ccree - fixes and cleanups

[Gilad Ben-Yossef]

On Thu, Jan 23, 2020 at 10:09 PM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Gilad,
>
> On Thu, Jan 23, 2020 at 7:19 PM Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> > On Thu, Jan 23, 2020 at 5:46 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Thu, Jan 23, 2020 at 12:44 PM Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> > > > On Wed, Jan 22, 2020 at 6:51 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > On Thu, Jan 16, 2020 at 11:25 AM Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> > > > > > A bunch of fixes and code cleanups for the ccree driver
> > > > >
> > > > > Thank you!
> > > > >
> > > > > I wanted to give this a try, but it looks like CCREE is no longer working
> > > > > on R-Car H3, both with/without this series.
> > > > >
> > > > > E.g. with renesas-devel[*] and renesas_defconfig +
> > > > > CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n, I get the following crash:
> > > >
> > > > Thank you for the bug report Geert!
> > > >
> > > > My R-Car board is on loan at the moment to another project. I didn't
> > > > see this on our internal test board.
> > > > I will track down my R-Car board and reproduce this - hopefully
> > > > beginning of next week and will get back to you.
> > >
> > > In the mean time, I've bisected this failure to commit cdfee5623290bc89
> > > ("driver core: initialize a default DMA mask for platform device").
> > > However, this looks like a red herring, and seems to be only an exposer
> > > of an underlying problem.
> >
> > Thank you for continue digging into this.
> >
> > > What's happening is that cc_map_aead_request() receives a request with
> > > cryptlen = 0.  Due to DRV_CRYPTO_DIRECTION_ENCRYPT, the length to map is
> > > increased by 8.  This seems to works fine if there is sufficient space
> > > in the request's scatterlist.  However, if the scatterlist has only a
> > > single entry of size zero, cc_map_sg() tries to map a zero-length DMA
> > > buffer, and the BUG)() is triggered.
> > >
> >
> > OK, this does rings a bell - can you verify please if
> > CONFIG_CRYPTO_MANAGER_EXTRA_TESTS is enabled and if it does can you
> > see if it happens if it is turned off?
>
> No, I didn't have that option enabled.

OK, I still did not get reunited with my R-Cat board, but I think I
have a direction.

I'm about to send an RFC patch which while  probably does not address
the root cause
will stop the crash if the issue is what I think it is and so will let
me know what you are
seeing is what I think you are seeing.

I'd be delighted if you can give it a spin...

Thanks,
Gilad