Re: [PATCH 2/2] crypto: engine - Permit to enqueue skcipher request

Linux cryptographic layer development
 help / color / mirror / Atom feed

* Re: [PATCH 2/2] crypto: engine - Permit to enqueue skcipher request
@ 2017-05-24  7:32 Julia Lawall
  0 siblings, 0 replies; 2+ messages in thread
From: Julia Lawall @ 2017-05-24  7:32 UTC (permalink / raw)
  To: Corentin Labbe
  Cc: herbert, davem, linux-crypto, linux-kernel, Corentin Labbe,
	kbuild-all

Please check whether an unlock is needed before line 454.

julia

---------- Forwarded message ----------
Date: Wed, 24 May 2017 12:16:29 +0800
From: kbuild test robot <fengguang.wu@intel.com>
To: kbuild@01.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Subject: Re: [PATCH 2/2] crypto: engine - Permit to enqueue skcipher request

CC: kbuild-all@01.org
In-Reply-To: <20170523120903.31637-2-clabbe.montjoie@gmail.com>
TO: Corentin Labbe <clabbe.montjoie@gmail.com>
CC: herbert@gondor.apana.org.au, davem@davemloft.net
CC: linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org, Corentin Labbe <clabbe.montjoie@gmail.com>

Hi Corentin,

[auto build test WARNING on cryptodev/master]
[also build test WARNING on v4.12-rc2 next-20170523]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Corentin-Labbe/crypto-engine-replace-pr_xxx-by-dev_xxx/20170524-061949
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
:::::: branch date: 6 hours ago
:::::: commit date: 6 hours ago

>> crypto/crypto_engine.c:454:2-8: preceding lock on line 444

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 1d8d483820540c10cd7056015fa0309e27c0b0e7
vim +454 crypto/crypto_engine.c

735d37b5 Baolin Wang    2016-01-26  438   * Return 0 on success, else on fail.
735d37b5 Baolin Wang    2016-01-26  439   */
735d37b5 Baolin Wang    2016-01-26  440  int crypto_engine_start(struct crypto_engine *engine)
735d37b5 Baolin Wang    2016-01-26  441  {
735d37b5 Baolin Wang    2016-01-26  442  	unsigned long flags;
735d37b5 Baolin Wang    2016-01-26  443
735d37b5 Baolin Wang    2016-01-26 @444  	spin_lock_irqsave(&engine->queue_lock, flags);
735d37b5 Baolin Wang    2016-01-26  445
735d37b5 Baolin Wang    2016-01-26  446  	if (engine->running || engine->busy) {
735d37b5 Baolin Wang    2016-01-26  447  		spin_unlock_irqrestore(&engine->queue_lock, flags);
735d37b5 Baolin Wang    2016-01-26  448  		return -EBUSY;
735d37b5 Baolin Wang    2016-01-26  449  	}
735d37b5 Baolin Wang    2016-01-26  450
1d8d4838 Corentin Labbe 2017-05-23  451  	if (!engine->skcipher_one_request && !engine->cipher_one_request &&
1d8d4838 Corentin Labbe 2017-05-23  452  	    !engine->hash_one_request) {
1d8d4838 Corentin Labbe 2017-05-23  453  		dev_err(engine->dev, "need at least one request type\n");
1d8d4838 Corentin Labbe 2017-05-23 @454  		return -EINVAL;
1d8d4838 Corentin Labbe 2017-05-23  455  	}
1d8d4838 Corentin Labbe 2017-05-23  456
1d8d4838 Corentin Labbe 2017-05-23  457  	if (engine->skcipher_one_request && engine->cipher_one_request) {

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

* [PATCH 1/2] crypto: engine - replace pr_xxx by dev_xxx
@ 2017-05-23 12:09 Corentin Labbe
  2017-05-23 12:09 ` [PATCH 2/2] crypto: engine - Permit to enqueue skcipher request Corentin Labbe
  0 siblings, 1 reply; 2+ messages in thread
From: Corentin Labbe @ 2017-05-23 12:09 UTC (permalink / raw)
  To: herbert, davem; +Cc: linux-crypto, linux-kernel, Corentin Labbe

By adding a struct device *dev to struct engine, we could store the
device used at register time and so use all dev_xxx functions instead of
pr_xxx.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 crypto/crypto_engine.c  | 23 +++++++++++++----------
 include/crypto/engine.h |  1 +
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
index 727bd5c3569e..61e7c4e02fd2 100644
--- a/crypto/crypto_engine.c
+++ b/crypto/crypto_engine.c
@@ -70,7 +70,7 @@ static void crypto_pump_requests(struct crypto_engine *engine,
 
 		if (engine->unprepare_crypt_hardware &&
 		    engine->unprepare_crypt_hardware(engine))
-			pr_err("failed to unprepare crypt hardware\n");
+			dev_err(engine->dev, "failed to unprepare crypt hardware\n");
 
 		spin_lock_irqsave(&engine->queue_lock, flags);
 		engine->idling = false;
@@ -99,7 +99,7 @@ static void crypto_pump_requests(struct crypto_engine *engine,
 	if (!was_busy && engine->prepare_crypt_hardware) {
 		ret = engine->prepare_crypt_hardware(engine);
 		if (ret) {
-			pr_err("failed to prepare crypt hardware\n");
+			dev_err(engine->dev, "failed to prepare crypt hardware\n");
 			goto req_err;
 		}
 	}
@@ -110,14 +110,15 @@ static void crypto_pump_requests(struct crypto_engine *engine,
 		if (engine->prepare_hash_request) {
 			ret = engine->prepare_hash_request(engine, hreq);
 			if (ret) {
-				pr_err("failed to prepare request: %d\n", ret);
+				dev_err(engine->dev, "failed to prepare request: %d\n",
+					ret);
 				goto req_err;
 			}
 			engine->cur_req_prepared = true;
 		}
 		ret = engine->hash_one_request(engine, hreq);
 		if (ret) {
-			pr_err("failed to hash one request from queue\n");
+			dev_err(engine->dev, "failed to hash one request from queue\n");
 			goto req_err;
 		}
 		return;
@@ -126,19 +127,20 @@ static void crypto_pump_requests(struct crypto_engine *engine,
 		if (engine->prepare_cipher_request) {
 			ret = engine->prepare_cipher_request(engine, breq);
 			if (ret) {
-				pr_err("failed to prepare request: %d\n", ret);
+				dev_err(engine->dev, "failed to prepare request: %d\n",
+					ret);
 				goto req_err;
 			}
 			engine->cur_req_prepared = true;
 		}
 		ret = engine->cipher_one_request(engine, breq);
 		if (ret) {
-			pr_err("failed to cipher one request from queue\n");
+			dev_err(engine->dev, "failed to cipher one request from queue\n");
 			goto req_err;
 		}
 		return;
 	default:
-		pr_err("failed to prepare request of unknown type\n");
+		dev_err(engine->dev, "failed to prepare request of unknown type\n");
 		return;
 	}
 
@@ -275,7 +277,7 @@ void crypto_finalize_cipher_request(struct crypto_engine *engine,
 		    engine->unprepare_cipher_request) {
 			ret = engine->unprepare_cipher_request(engine, req);
 			if (ret)
-				pr_err("failed to unprepare request\n");
+				dev_err(engine->dev, "failed to unprepare request\n");
 		}
 		spin_lock_irqsave(&engine->queue_lock, flags);
 		engine->cur_req = NULL;
@@ -312,7 +314,7 @@ void crypto_finalize_hash_request(struct crypto_engine *engine,
 		    engine->unprepare_hash_request) {
 			ret = engine->unprepare_hash_request(engine, req);
 			if (ret)
-				pr_err("failed to unprepare request\n");
+				dev_err(engine->dev, "failed to unprepare request\n");
 		}
 		spin_lock_irqsave(&engine->queue_lock, flags);
 		engine->cur_req = NULL;
@@ -384,7 +386,7 @@ int crypto_engine_stop(struct crypto_engine *engine)
 	spin_unlock_irqrestore(&engine->queue_lock, flags);
 
 	if (ret)
-		pr_warn("could not stop engine\n");
+		dev_warn(engine->dev, "could not stop engine\n");
 
 	return ret;
 }
@@ -411,6 +413,7 @@ struct crypto_engine *crypto_engine_alloc_init(struct device *dev, bool rt)
 	if (!engine)
 		return NULL;
 
+	engine->dev = dev;
 	engine->rt = rt;
 	engine->running = false;
 	engine->busy = false;
diff --git a/include/crypto/engine.h b/include/crypto/engine.h
index 1bf600fc99f7..9e968a1033d0 100644
--- a/include/crypto/engine.h
+++ b/include/crypto/engine.h
@@ -58,6 +58,7 @@ struct crypto_engine {
 	struct list_head	list;
 	spinlock_t		queue_lock;
 	struct crypto_queue	queue;
+	struct device *dev;
 
 	bool			rt;
 
-- 
2.13.0

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

* [PATCH 2/2] crypto: engine - Permit to enqueue skcipher request
  2017-05-23 12:09 [PATCH 1/2] crypto: engine - replace pr_xxx by dev_xxx Corentin Labbe
@ 2017-05-23 12:09 ` Corentin Labbe
  0 siblings, 0 replies; 2+ messages in thread
From: Corentin Labbe @ 2017-05-23 12:09 UTC (permalink / raw)
  To: herbert, davem; +Cc: linux-crypto, linux-kernel, Corentin Labbe

The crypto engine could actually only enqueue hash and ablkcipher request.
This patch permit it to enqueue skcipher requets by adding all necessary
functions.
The only problem is that ablkcipher and skcipher id are the same, so
only one cipher type is usable on the same crypto engine.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 crypto/crypto_engine.c  | 136 ++++++++++++++++++++++++++++++++++++++++++++----
 include/crypto/engine.h |  14 +++++
 2 files changed, 139 insertions(+), 11 deletions(-)

diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
index 61e7c4e02fd2..9f690e04006a 100644
--- a/crypto/crypto_engine.c
+++ b/crypto/crypto_engine.c
@@ -36,6 +36,7 @@ static void crypto_pump_requests(struct crypto_engine *engine,
 	struct crypto_async_request *async_req, *backlog;
 	struct ahash_request *hreq;
 	struct ablkcipher_request *breq;
+	struct skcipher_request *skreq;
 	unsigned long flags;
 	bool was_busy = false;
 	int ret, rtype;
@@ -123,17 +124,34 @@ static void crypto_pump_requests(struct crypto_engine *engine,
 		}
 		return;
 	case CRYPTO_ALG_TYPE_ABLKCIPHER:
-		breq = ablkcipher_request_cast(engine->cur_req);
-		if (engine->prepare_cipher_request) {
-			ret = engine->prepare_cipher_request(engine, breq);
-			if (ret) {
-				dev_err(engine->dev, "failed to prepare request: %d\n",
-					ret);
-				goto req_err;
+	/* since CRYPTO_ALG_TYPE_ABLKCIPHER == CRYPTO_ALG_TYPE_SKCIPHER
+	 * differenciate both with the presence of cipher_one_request
+	 */
+		if (engine->cipher_one_request) {
+			breq = ablkcipher_request_cast(engine->cur_req);
+			if (engine->prepare_cipher_request) {
+				ret = engine->prepare_cipher_request(engine, breq);
+				if (ret) {
+					dev_err(engine->dev, "failed to prepare request: %d\n",
+						ret);
+					goto req_err;
+				}
+				engine->cur_req_prepared = true;
 			}
-			engine->cur_req_prepared = true;
+			ret = engine->cipher_one_request(engine, breq);
+		} else {
+			skreq = skcipher_request_cast(engine->cur_req);
+			if (engine->prepare_skcipher_request) {
+				ret = engine->prepare_skcipher_request(engine, skreq);
+				if (ret) {
+					dev_err(engine->dev, "failed to prepare request: %d\n",
+						ret);
+					goto req_err;
+				}
+				engine->cur_req_prepared = true;
+		}
+		ret = engine->skcipher_one_request(engine, skreq);
 		}
-		ret = engine->cipher_one_request(engine, breq);
 		if (ret) {
 			dev_err(engine->dev, "failed to cipher one request from queue\n");
 			goto req_err;
@@ -151,8 +169,13 @@ static void crypto_pump_requests(struct crypto_engine *engine,
 		crypto_finalize_hash_request(engine, hreq, ret);
 		break;
 	case CRYPTO_ALG_TYPE_ABLKCIPHER:
-		breq = ablkcipher_request_cast(engine->cur_req);
-		crypto_finalize_cipher_request(engine, breq, ret);
+		if (engine->cipher_one_request) {
+			breq = ablkcipher_request_cast(engine->cur_req);
+			crypto_finalize_cipher_request(engine, breq, ret);
+		} else {
+			skreq = skcipher_request_cast(engine->cur_req);
+			crypto_finalize_skcipher_request(engine, skreq, ret);
+		}
 		break;
 	}
 	return;
@@ -213,6 +236,49 @@ int crypto_transfer_cipher_request_to_engine(struct crypto_engine *engine,
 EXPORT_SYMBOL_GPL(crypto_transfer_cipher_request_to_engine);
 
 /**
+ * crypto_transfer_skcipher_request - transfer the new request into the
+ * enginequeue
+ * @engine: the hardware engine
+ * @req: the request need to be listed into the engine queue
+ */
+int crypto_transfer_skcipher_request(struct crypto_engine *engine,
+				     struct skcipher_request *req,
+				     bool need_pump)
+{
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&engine->queue_lock, flags);
+
+	if (!engine->running) {
+		spin_unlock_irqrestore(&engine->queue_lock, flags);
+		return -ESHUTDOWN;
+	}
+
+	ret = crypto_enqueue_request(&engine->queue, &req->base);
+
+	if (!engine->busy && need_pump)
+		kthread_queue_work(engine->kworker, &engine->pump_requests);
+
+	spin_unlock_irqrestore(&engine->queue_lock, flags);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(crypto_transfer_skcipher_request);
+
+/**
+ * crypto_transfer_skcipher_request_to_engine - transfer one request to list
+ * into the engine queue
+ * @engine: the hardware engine
+ * @req: the request need to be listed into the engine queue
+ */
+int crypto_transfer_skcipher_request_to_engine(struct crypto_engine *engine,
+					       struct skcipher_request *req)
+{
+	return crypto_transfer_skcipher_request(engine, req, true);
+}
+EXPORT_SYMBOL_GPL(crypto_transfer_skcipher_request_to_engine);
+
+/**
  * crypto_transfer_hash_request - transfer the new request into the
  * enginequeue
  * @engine: the hardware engine
@@ -292,6 +358,43 @@ void crypto_finalize_cipher_request(struct crypto_engine *engine,
 EXPORT_SYMBOL_GPL(crypto_finalize_cipher_request);
 
 /**
+ * crypto_finalize_skcipher_request - finalize one request if the request is done
+ * @engine: the hardware engine
+ * @req: the request need to be finalized
+ * @err: error number
+ */
+void crypto_finalize_skcipher_request(struct crypto_engine *engine,
+				      struct skcipher_request *req, int err)
+{
+	unsigned long flags;
+	bool finalize_cur_req = false;
+	int ret;
+
+	spin_lock_irqsave(&engine->queue_lock, flags);
+	if (engine->cur_req == &req->base)
+		finalize_cur_req = true;
+	spin_unlock_irqrestore(&engine->queue_lock, flags);
+
+	if (finalize_cur_req) {
+		if (engine->cur_req_prepared &&
+		    engine->unprepare_skcipher_request) {
+			ret = engine->unprepare_skcipher_request(engine, req);
+			if (ret)
+				dev_err(engine->dev, "failed to unprepare request\n");
+		}
+		spin_lock_irqsave(&engine->queue_lock, flags);
+		engine->cur_req = NULL;
+		engine->cur_req_prepared = false;
+		spin_unlock_irqrestore(&engine->queue_lock, flags);
+	}
+
+	req->base.complete(&req->base, err);
+
+	kthread_queue_work(engine->kworker, &engine->pump_requests);
+}
+EXPORT_SYMBOL_GPL(crypto_finalize_skcipher_request);
+
+/**
  * crypto_finalize_hash_request - finalize one request if the request is done
  * @engine: the hardware engine
  * @req: the request need to be finalized
@@ -345,6 +448,17 @@ int crypto_engine_start(struct crypto_engine *engine)
 		return -EBUSY;
 	}
 
+	if (!engine->skcipher_one_request && !engine->cipher_one_request &&
+	    !engine->hash_one_request) {
+		dev_err(engine->dev, "need at least one request type\n");
+		return -EINVAL;
+	}
+
+	if (engine->skcipher_one_request && engine->cipher_one_request) {
+		dev_err(engine->dev, "Cannot use both skcipher and ablkcipher\n");
+		return -EINVAL;
+	}
+
 	engine->running = true;
 	spin_unlock_irqrestore(&engine->queue_lock, flags);
 
diff --git a/include/crypto/engine.h b/include/crypto/engine.h
index 9e968a1033d0..a83139311e59 100644
--- a/include/crypto/engine.h
+++ b/include/crypto/engine.h
@@ -18,6 +18,7 @@
 #include <linux/kthread.h>
 #include <crypto/algapi.h>
 #include <crypto/hash.h>
+#include <crypto/skcipher.h>
 
 #define ENGINE_NAME_LEN	30
 /*
@@ -69,12 +70,18 @@ struct crypto_engine {
 				      struct ablkcipher_request *req);
 	int (*unprepare_cipher_request)(struct crypto_engine *engine,
 					struct ablkcipher_request *req);
+	int (*prepare_skcipher_request)(struct crypto_engine *engine,
+					struct skcipher_request *req);
+	int (*unprepare_skcipher_request)(struct crypto_engine *engine,
+					  struct skcipher_request *req);
 	int (*prepare_hash_request)(struct crypto_engine *engine,
 				    struct ahash_request *req);
 	int (*unprepare_hash_request)(struct crypto_engine *engine,
 				      struct ahash_request *req);
 	int (*cipher_one_request)(struct crypto_engine *engine,
 				  struct ablkcipher_request *req);
+	int (*skcipher_one_request)(struct crypto_engine *engine,
+				    struct skcipher_request *req);
 	int (*hash_one_request)(struct crypto_engine *engine,
 				struct ahash_request *req);
 
@@ -90,12 +97,19 @@ int crypto_transfer_cipher_request(struct crypto_engine *engine,
 				   bool need_pump);
 int crypto_transfer_cipher_request_to_engine(struct crypto_engine *engine,
 					     struct ablkcipher_request *req);
+int crypto_transfer_skcipher_request(struct crypto_engine *engine,
+				     struct skcipher_request *req,
+				     bool need_pump);
+int crypto_transfer_skcipher_request_to_engine(struct crypto_engine *engine,
+					       struct skcipher_request *req);
 int crypto_transfer_hash_request(struct crypto_engine *engine,
 				 struct ahash_request *req, bool need_pump);
 int crypto_transfer_hash_request_to_engine(struct crypto_engine *engine,
 					   struct ahash_request *req);
 void crypto_finalize_cipher_request(struct crypto_engine *engine,
 				    struct ablkcipher_request *req, int err);
+void crypto_finalize_skcipher_request(struct crypto_engine *engine,
+				      struct skcipher_request *req, int err);
 void crypto_finalize_hash_request(struct crypto_engine *engine,
 				  struct ahash_request *req, int err);
 int crypto_engine_start(struct crypto_engine *engine);
-- 
2.13.0

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

end of thread, other threads:[~2017-05-24  7:32 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-24  7:32 [PATCH 2/2] crypto: engine - Permit to enqueue skcipher request Julia Lawall
  -- strict thread matches above, loose matches on Subject: below --
2017-05-23 12:09 [PATCH 1/2] crypto: engine - replace pr_xxx by dev_xxx Corentin Labbe
2017-05-23 12:09 ` [PATCH 2/2] crypto: engine - Permit to enqueue skcipher request Corentin Labbe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox