linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] crypto: engine: permit to enqueue ashash_request
@ 2016-05-30 13:32 LABBE Corentin
  2016-05-30 13:32 ` [PATCH v2 1/2] " LABBE Corentin
  2016-05-30 13:32 ` [PATCH v2 2/2] crypto: omap: convert to the new cryptoengine API LABBE Corentin
  0 siblings, 2 replies; 10+ messages in thread
From: LABBE Corentin @ 2016-05-30 13:32 UTC (permalink / raw)
  To: herbert, davem, baolin.wang; +Cc: linux-crypto, linux-kernel, LABBE Corentin

Hello

I wanted to use the crypto engine for my Allwinner crypto driver but something
prevented me to use it: it cannot enqueue hash requests.
The first patch convert crypto engine to permit enqueuing of ahash_requests.
The second patch convert the only driver using crypto engine.

The second patch was only compile tested but the crypto engine with
hash support was tested on two different offtree driver (sun4i-ss and sun8i-ce)

Regards

Changes since v1:
- rebased on cryptodev for handling omap-des

LABBE Corentin (2):
  crypto: engine: permit to enqueue ashash_request
  crypto: omap: convert to the new cryptoengine API

 crypto/crypto_engine.c    | 17 +++++++----------
 drivers/crypto/omap-aes.c | 10 ++++++----
 include/crypto/algapi.h   | 14 +++++++-------
 3 files changed, 20 insertions(+), 21 deletions(-)

-- 
2.7.3

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

* [PATCH v2 1/2] crypto: engine: permit to enqueue ashash_request
  2016-05-30 13:32 [PATCH v2 0/2] crypto: engine: permit to enqueue ashash_request LABBE Corentin
@ 2016-05-30 13:32 ` LABBE Corentin
  2016-06-01  2:27   ` Baolin Wang
  2016-06-02  8:32   ` Herbert Xu
  2016-05-30 13:32 ` [PATCH v2 2/2] crypto: omap: convert to the new cryptoengine API LABBE Corentin
  1 sibling, 2 replies; 10+ messages in thread
From: LABBE Corentin @ 2016-05-30 13:32 UTC (permalink / raw)
  To: herbert, davem, baolin.wang; +Cc: linux-crypto, linux-kernel, LABBE Corentin

The current crypto engine allow only ablkcipher_request to be enqueued.
Thus denying any use of it for hardware that also handle hash algo.

This patch convert all ablkcipher_request references to the
more general crypto_async_request.

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
 crypto/crypto_engine.c  | 17 +++++++----------
 include/crypto/algapi.h | 14 +++++++-------
 2 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
index a55c82d..b658cb8 100644
--- a/crypto/crypto_engine.c
+++ b/crypto/crypto_engine.c
@@ -19,7 +19,7 @@
 #define CRYPTO_ENGINE_MAX_QLEN 10
 
 void crypto_finalize_request(struct crypto_engine *engine,
-			     struct ablkcipher_request *req, int err);
+			     struct crypto_async_request *req, int err);
 
 /**
  * crypto_pump_requests - dequeue one request from engine queue to process
@@ -34,7 +34,6 @@ static void crypto_pump_requests(struct crypto_engine *engine,
 				 bool in_kthread)
 {
 	struct crypto_async_request *async_req, *backlog;
-	struct ablkcipher_request *req;
 	unsigned long flags;
 	bool was_busy = false;
 	int ret;
@@ -82,9 +81,7 @@ static void crypto_pump_requests(struct crypto_engine *engine,
 	if (!async_req)
 		goto out;
 
-	req = ablkcipher_request_cast(async_req);
-
-	engine->cur_req = req;
+	engine->cur_req = async_req;
 	if (backlog)
 		backlog->complete(backlog, -EINPROGRESS);
 
@@ -142,7 +139,7 @@ static void crypto_pump_work(struct kthread_work *work)
  * @req: the request need to be listed into the engine queue
  */
 int crypto_transfer_request(struct crypto_engine *engine,
-			    struct ablkcipher_request *req, bool need_pump)
+			    struct crypto_async_request *req, bool need_pump)
 {
 	unsigned long flags;
 	int ret;
@@ -154,7 +151,7 @@ int crypto_transfer_request(struct crypto_engine *engine,
 		return -ESHUTDOWN;
 	}
 
-	ret = ablkcipher_enqueue_request(&engine->queue, req);
+	ret = crypto_enqueue_request(&engine->queue, req);
 
 	if (!engine->busy && need_pump)
 		queue_kthread_work(&engine->kworker, &engine->pump_requests);
@@ -171,7 +168,7 @@ EXPORT_SYMBOL_GPL(crypto_transfer_request);
  * @req: the request need to be listed into the engine queue
  */
 int crypto_transfer_request_to_engine(struct crypto_engine *engine,
-				      struct ablkcipher_request *req)
+				      struct crypto_async_request *req)
 {
 	return crypto_transfer_request(engine, req, true);
 }
@@ -184,7 +181,7 @@ EXPORT_SYMBOL_GPL(crypto_transfer_request_to_engine);
  * @err: error number
  */
 void crypto_finalize_request(struct crypto_engine *engine,
-			     struct ablkcipher_request *req, int err)
+			     struct crypto_async_request *req, int err)
 {
 	unsigned long flags;
 	bool finalize_cur_req = false;
@@ -208,7 +205,7 @@ void crypto_finalize_request(struct crypto_engine *engine,
 		spin_unlock_irqrestore(&engine->queue_lock, flags);
 	}
 
-	req->base.complete(&req->base, err);
+	req->complete(req, err);
 
 	queue_kthread_work(&engine->kworker, &engine->pump_requests);
 }
diff --git a/include/crypto/algapi.h b/include/crypto/algapi.h
index eeafd21..d720a2a 100644
--- a/include/crypto/algapi.h
+++ b/include/crypto/algapi.h
@@ -173,26 +173,26 @@ struct crypto_engine {
 	int (*unprepare_crypt_hardware)(struct crypto_engine *engine);
 
 	int (*prepare_request)(struct crypto_engine *engine,
-			       struct ablkcipher_request *req);
+			       struct crypto_async_request *req);
 	int (*unprepare_request)(struct crypto_engine *engine,
-				 struct ablkcipher_request *req);
+				 struct crypto_async_request *req);
 	int (*crypt_one_request)(struct crypto_engine *engine,
-				 struct ablkcipher_request *req);
+				 struct crypto_async_request *req);
 
 	struct kthread_worker           kworker;
 	struct task_struct              *kworker_task;
 	struct kthread_work             pump_requests;
 
 	void				*priv_data;
-	struct ablkcipher_request	*cur_req;
+	struct crypto_async_request	*cur_req;
 };
 
 int crypto_transfer_request(struct crypto_engine *engine,
-			    struct ablkcipher_request *req, bool need_pump);
+			    struct crypto_async_request *req, bool need_pump);
 int crypto_transfer_request_to_engine(struct crypto_engine *engine,
-				      struct ablkcipher_request *req);
+				      struct crypto_async_request *req);
 void crypto_finalize_request(struct crypto_engine *engine,
-			     struct ablkcipher_request *req, int err);
+			     struct crypto_async_request *req, int err);
 int crypto_engine_start(struct crypto_engine *engine);
 int crypto_engine_stop(struct crypto_engine *engine);
 struct crypto_engine *crypto_engine_alloc_init(struct device *dev, bool rt);
-- 
2.7.3

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

* [PATCH v2 2/2] crypto: omap: convert to the new cryptoengine API
  2016-05-30 13:32 [PATCH v2 0/2] crypto: engine: permit to enqueue ashash_request LABBE Corentin
  2016-05-30 13:32 ` [PATCH v2 1/2] " LABBE Corentin
@ 2016-05-30 13:32 ` LABBE Corentin
  2016-06-01  2:28   ` Baolin Wang
  1 sibling, 1 reply; 10+ messages in thread
From: LABBE Corentin @ 2016-05-30 13:32 UTC (permalink / raw)
  To: herbert, davem, baolin.wang; +Cc: linux-crypto, linux-kernel, LABBE Corentin

Since the crypto engine has been converted to use crypto_async_request
instead of ablkcipher_request, minor changes are needed to use it.

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
 drivers/crypto/omap-aes.c | 10 ++++++----
 drivers/crypto/omap-des.c | 10 ++++++----
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/omap-aes.c b/drivers/crypto/omap-aes.c
index ce174d3..7007f13 100644
--- a/drivers/crypto/omap-aes.c
+++ b/drivers/crypto/omap-aes.c
@@ -519,7 +519,7 @@ static void omap_aes_finish_req(struct omap_aes_dev *dd, int err)
 
 	pr_debug("err: %d\n", err);
 
-	crypto_finalize_request(dd->engine, req, err);
+	crypto_finalize_request(dd->engine, &req->base, err);
 }
 
 static int omap_aes_crypt_dma_stop(struct omap_aes_dev *dd)
@@ -592,14 +592,15 @@ static int omap_aes_handle_queue(struct omap_aes_dev *dd,
 				 struct ablkcipher_request *req)
 {
 	if (req)
-		return crypto_transfer_request_to_engine(dd->engine, req);
+		return crypto_transfer_request_to_engine(dd->engine, &req->base);
 
 	return 0;
 }
 
 static int omap_aes_prepare_req(struct crypto_engine *engine,
-				struct ablkcipher_request *req)
+				struct crypto_async_request *areq)
 {
+	struct ablkcipher_request *req = ablkcipher_request_cast(areq);
 	struct omap_aes_ctx *ctx = crypto_ablkcipher_ctx(
 			crypto_ablkcipher_reqtfm(req));
 	struct omap_aes_dev *dd = omap_aes_find_dev(ctx);
@@ -642,8 +643,9 @@ static int omap_aes_prepare_req(struct crypto_engine *engine,
 }
 
 static int omap_aes_crypt_req(struct crypto_engine *engine,
-			      struct ablkcipher_request *req)
+			      struct crypto_async_request *areq)
 {
+	struct ablkcipher_request *req = ablkcipher_request_cast(areq);
 	struct omap_aes_ctx *ctx = crypto_ablkcipher_ctx(
 			crypto_ablkcipher_reqtfm(req));
 	struct omap_aes_dev *dd = omap_aes_find_dev(ctx);
diff --git a/drivers/crypto/omap-des.c b/drivers/crypto/omap-des.c
index 3eedb03..0da5686 100644
--- a/drivers/crypto/omap-des.c
+++ b/drivers/crypto/omap-des.c
@@ -506,7 +506,7 @@ static void omap_des_finish_req(struct omap_des_dev *dd, int err)
 	pr_debug("err: %d\n", err);
 
 	pm_runtime_put(dd->dev);
-	crypto_finalize_request(dd->engine, req, err);
+	crypto_finalize_request(dd->engine, &req->base, err);
 }
 
 static int omap_des_crypt_dma_stop(struct omap_des_dev *dd)
@@ -572,14 +572,15 @@ static int omap_des_handle_queue(struct omap_des_dev *dd,
 				 struct ablkcipher_request *req)
 {
 	if (req)
-		return crypto_transfer_request_to_engine(dd->engine, req);
+		return crypto_transfer_request_to_engine(dd->engine, &req->base);
 
 	return 0;
 }
 
 static int omap_des_prepare_req(struct crypto_engine *engine,
-				struct ablkcipher_request *req)
+				struct crypto_async_request *areq)
 {
+	struct ablkcipher_request *req = ablkcipher_request_cast(areq);
 	struct omap_des_ctx *ctx = crypto_ablkcipher_ctx(
 			crypto_ablkcipher_reqtfm(req));
 	struct omap_des_dev *dd = omap_des_find_dev(ctx);
@@ -620,8 +621,9 @@ static int omap_des_prepare_req(struct crypto_engine *engine,
 }
 
 static int omap_des_crypt_req(struct crypto_engine *engine,
-			      struct ablkcipher_request *req)
+			      struct crypto_async_request *areq)
 {
+	struct ablkcipher_request *req = ablkcipher_request_cast(areq);
 	struct omap_des_ctx *ctx = crypto_ablkcipher_ctx(
 			crypto_ablkcipher_reqtfm(req));
 	struct omap_des_dev *dd = omap_des_find_dev(ctx);
-- 
2.7.3

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

* Re: [PATCH v2 1/2] crypto: engine: permit to enqueue ashash_request
  2016-05-30 13:32 ` [PATCH v2 1/2] " LABBE Corentin
@ 2016-06-01  2:27   ` Baolin Wang
  2016-06-02  8:32   ` Herbert Xu
  1 sibling, 0 replies; 10+ messages in thread
From: Baolin Wang @ 2016-06-01  2:27 UTC (permalink / raw)
  To: LABBE Corentin; +Cc: Herbert Xu, David Miller, linux-crypto, LKML

On 30 May 2016 at 21:32, LABBE Corentin <clabbe.montjoie@gmail.com> wrote:
> The current crypto engine allow only ablkcipher_request to be enqueued.
> Thus denying any use of it for hardware that also handle hash algo.
>
> This patch convert all ablkcipher_request references to the
> more general crypto_async_request.
>
> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> ---
>  crypto/crypto_engine.c  | 17 +++++++----------
>  include/crypto/algapi.h | 14 +++++++-------
>  2 files changed, 14 insertions(+), 17 deletions(-)
>
> diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
> index a55c82d..b658cb8 100644
> --- a/crypto/crypto_engine.c
> +++ b/crypto/crypto_engine.c
> @@ -19,7 +19,7 @@
>  #define CRYPTO_ENGINE_MAX_QLEN 10
>
>  void crypto_finalize_request(struct crypto_engine *engine,
> -                            struct ablkcipher_request *req, int err);
> +                            struct crypto_async_request *req, int err);
>
>  /**
>   * crypto_pump_requests - dequeue one request from engine queue to process
> @@ -34,7 +34,6 @@ static void crypto_pump_requests(struct crypto_engine *engine,
>                                  bool in_kthread)
>  {
>         struct crypto_async_request *async_req, *backlog;
> -       struct ablkcipher_request *req;
>         unsigned long flags;
>         bool was_busy = false;
>         int ret;
> @@ -82,9 +81,7 @@ static void crypto_pump_requests(struct crypto_engine *engine,
>         if (!async_req)
>                 goto out;
>
> -       req = ablkcipher_request_cast(async_req);
> -
> -       engine->cur_req = req;
> +       engine->cur_req = async_req;
>         if (backlog)
>                 backlog->complete(backlog, -EINPROGRESS);
>
> @@ -142,7 +139,7 @@ static void crypto_pump_work(struct kthread_work *work)
>   * @req: the request need to be listed into the engine queue
>   */
>  int crypto_transfer_request(struct crypto_engine *engine,
> -                           struct ablkcipher_request *req, bool need_pump)
> +                           struct crypto_async_request *req, bool need_pump)
>  {
>         unsigned long flags;
>         int ret;
> @@ -154,7 +151,7 @@ int crypto_transfer_request(struct crypto_engine *engine,
>                 return -ESHUTDOWN;
>         }
>
> -       ret = ablkcipher_enqueue_request(&engine->queue, req);
> +       ret = crypto_enqueue_request(&engine->queue, req);
>
>         if (!engine->busy && need_pump)
>                 queue_kthread_work(&engine->kworker, &engine->pump_requests);
> @@ -171,7 +168,7 @@ EXPORT_SYMBOL_GPL(crypto_transfer_request);
>   * @req: the request need to be listed into the engine queue
>   */
>  int crypto_transfer_request_to_engine(struct crypto_engine *engine,
> -                                     struct ablkcipher_request *req)
> +                                     struct crypto_async_request *req)
>  {
>         return crypto_transfer_request(engine, req, true);
>  }
> @@ -184,7 +181,7 @@ EXPORT_SYMBOL_GPL(crypto_transfer_request_to_engine);
>   * @err: error number
>   */
>  void crypto_finalize_request(struct crypto_engine *engine,
> -                            struct ablkcipher_request *req, int err)
> +                            struct crypto_async_request *req, int err)
>  {
>         unsigned long flags;
>         bool finalize_cur_req = false;
> @@ -208,7 +205,7 @@ void crypto_finalize_request(struct crypto_engine *engine,
>                 spin_unlock_irqrestore(&engine->queue_lock, flags);
>         }
>
> -       req->base.complete(&req->base, err);
> +       req->complete(req, err);
>
>         queue_kthread_work(&engine->kworker, &engine->pump_requests);
>  }
> diff --git a/include/crypto/algapi.h b/include/crypto/algapi.h
> index eeafd21..d720a2a 100644
> --- a/include/crypto/algapi.h
> +++ b/include/crypto/algapi.h
> @@ -173,26 +173,26 @@ struct crypto_engine {
>         int (*unprepare_crypt_hardware)(struct crypto_engine *engine);
>
>         int (*prepare_request)(struct crypto_engine *engine,
> -                              struct ablkcipher_request *req);
> +                              struct crypto_async_request *req);
>         int (*unprepare_request)(struct crypto_engine *engine,
> -                                struct ablkcipher_request *req);
> +                                struct crypto_async_request *req);
>         int (*crypt_one_request)(struct crypto_engine *engine,
> -                                struct ablkcipher_request *req);
> +                                struct crypto_async_request *req);
>
>         struct kthread_worker           kworker;
>         struct task_struct              *kworker_task;
>         struct kthread_work             pump_requests;
>
>         void                            *priv_data;
> -       struct ablkcipher_request       *cur_req;
> +       struct crypto_async_request     *cur_req;
>  };
>
>  int crypto_transfer_request(struct crypto_engine *engine,
> -                           struct ablkcipher_request *req, bool need_pump);
> +                           struct crypto_async_request *req, bool need_pump);
>  int crypto_transfer_request_to_engine(struct crypto_engine *engine,
> -                                     struct ablkcipher_request *req);
> +                                     struct crypto_async_request *req);
>  void crypto_finalize_request(struct crypto_engine *engine,
> -                            struct ablkcipher_request *req, int err);
> +                            struct crypto_async_request *req, int err);
>  int crypto_engine_start(struct crypto_engine *engine);
>  int crypto_engine_stop(struct crypto_engine *engine);
>  struct crypto_engine *crypto_engine_alloc_init(struct device *dev, bool rt);
> --
> 2.7.3
>

Reviewed-by: Baolin Wang <baolin.wang@linaro.org>


-- 
Baolin.wang
Best Regards

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

* Re: [PATCH v2 2/2] crypto: omap: convert to the new cryptoengine API
  2016-05-30 13:32 ` [PATCH v2 2/2] crypto: omap: convert to the new cryptoengine API LABBE Corentin
@ 2016-06-01  2:28   ` Baolin Wang
  0 siblings, 0 replies; 10+ messages in thread
From: Baolin Wang @ 2016-06-01  2:28 UTC (permalink / raw)
  To: LABBE Corentin; +Cc: Herbert Xu, David Miller, linux-crypto, LKML

On 30 May 2016 at 21:32, LABBE Corentin <clabbe.montjoie@gmail.com> wrote:
> Since the crypto engine has been converted to use crypto_async_request
> instead of ablkcipher_request, minor changes are needed to use it.
>
> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> ---
>  drivers/crypto/omap-aes.c | 10 ++++++----
>  drivers/crypto/omap-des.c | 10 ++++++----
>  2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/crypto/omap-aes.c b/drivers/crypto/omap-aes.c
> index ce174d3..7007f13 100644
> --- a/drivers/crypto/omap-aes.c
> +++ b/drivers/crypto/omap-aes.c
> @@ -519,7 +519,7 @@ static void omap_aes_finish_req(struct omap_aes_dev *dd, int err)
>
>         pr_debug("err: %d\n", err);
>
> -       crypto_finalize_request(dd->engine, req, err);
> +       crypto_finalize_request(dd->engine, &req->base, err);
>  }
>
>  static int omap_aes_crypt_dma_stop(struct omap_aes_dev *dd)
> @@ -592,14 +592,15 @@ static int omap_aes_handle_queue(struct omap_aes_dev *dd,
>                                  struct ablkcipher_request *req)
>  {
>         if (req)
> -               return crypto_transfer_request_to_engine(dd->engine, req);
> +               return crypto_transfer_request_to_engine(dd->engine, &req->base);
>
>         return 0;
>  }
>
>  static int omap_aes_prepare_req(struct crypto_engine *engine,
> -                               struct ablkcipher_request *req)
> +                               struct crypto_async_request *areq)
>  {
> +       struct ablkcipher_request *req = ablkcipher_request_cast(areq);
>         struct omap_aes_ctx *ctx = crypto_ablkcipher_ctx(
>                         crypto_ablkcipher_reqtfm(req));
>         struct omap_aes_dev *dd = omap_aes_find_dev(ctx);
> @@ -642,8 +643,9 @@ static int omap_aes_prepare_req(struct crypto_engine *engine,
>  }
>
>  static int omap_aes_crypt_req(struct crypto_engine *engine,
> -                             struct ablkcipher_request *req)
> +                             struct crypto_async_request *areq)
>  {
> +       struct ablkcipher_request *req = ablkcipher_request_cast(areq);
>         struct omap_aes_ctx *ctx = crypto_ablkcipher_ctx(
>                         crypto_ablkcipher_reqtfm(req));
>         struct omap_aes_dev *dd = omap_aes_find_dev(ctx);
> diff --git a/drivers/crypto/omap-des.c b/drivers/crypto/omap-des.c
> index 3eedb03..0da5686 100644
> --- a/drivers/crypto/omap-des.c
> +++ b/drivers/crypto/omap-des.c
> @@ -506,7 +506,7 @@ static void omap_des_finish_req(struct omap_des_dev *dd, int err)
>         pr_debug("err: %d\n", err);
>
>         pm_runtime_put(dd->dev);
> -       crypto_finalize_request(dd->engine, req, err);
> +       crypto_finalize_request(dd->engine, &req->base, err);
>  }
>
>  static int omap_des_crypt_dma_stop(struct omap_des_dev *dd)
> @@ -572,14 +572,15 @@ static int omap_des_handle_queue(struct omap_des_dev *dd,
>                                  struct ablkcipher_request *req)
>  {
>         if (req)
> -               return crypto_transfer_request_to_engine(dd->engine, req);
> +               return crypto_transfer_request_to_engine(dd->engine, &req->base);
>
>         return 0;
>  }
>
>  static int omap_des_prepare_req(struct crypto_engine *engine,
> -                               struct ablkcipher_request *req)
> +                               struct crypto_async_request *areq)
>  {
> +       struct ablkcipher_request *req = ablkcipher_request_cast(areq);
>         struct omap_des_ctx *ctx = crypto_ablkcipher_ctx(
>                         crypto_ablkcipher_reqtfm(req));
>         struct omap_des_dev *dd = omap_des_find_dev(ctx);
> @@ -620,8 +621,9 @@ static int omap_des_prepare_req(struct crypto_engine *engine,
>  }
>
>  static int omap_des_crypt_req(struct crypto_engine *engine,
> -                             struct ablkcipher_request *req)
> +                             struct crypto_async_request *areq)
>  {
> +       struct ablkcipher_request *req = ablkcipher_request_cast(areq);
>         struct omap_des_ctx *ctx = crypto_ablkcipher_ctx(
>                         crypto_ablkcipher_reqtfm(req));
>         struct omap_des_dev *dd = omap_des_find_dev(ctx);
> --
> 2.7.3
>

Reviewed-by: Baolin Wang <baolin.wang@linaro.org>

-- 
Baolin.wang
Best Regards

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

* Re: [PATCH v2 1/2] crypto: engine: permit to enqueue ashash_request
  2016-05-30 13:32 ` [PATCH v2 1/2] " LABBE Corentin
  2016-06-01  2:27   ` Baolin Wang
@ 2016-06-02  8:32   ` Herbert Xu
  2016-06-02  9:12     ` LABBE Corentin
  1 sibling, 1 reply; 10+ messages in thread
From: Herbert Xu @ 2016-06-02  8:32 UTC (permalink / raw)
  To: LABBE Corentin; +Cc: davem, baolin.wang, linux-crypto, linux-kernel

On Mon, May 30, 2016 at 03:32:01PM +0200, LABBE Corentin wrote:
> The current crypto engine allow only ablkcipher_request to be enqueued.
> Thus denying any use of it for hardware that also handle hash algo.
> 
> This patch convert all ablkcipher_request references to the
> more general crypto_async_request.
> 
> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>

First of all your patches break bisection which is unacceptable.

Secondly you should not be casting generic requests to a specific type.

Assuming a single engine only has to deal with one type of requests,
what you could do is to create a separate engine type for each
crypto type that you want to support.

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

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

* Re: [PATCH v2 1/2] crypto: engine: permit to enqueue ashash_request
  2016-06-02  8:32   ` Herbert Xu
@ 2016-06-02  9:12     ` LABBE Corentin
  2016-06-02  9:19       ` Herbert Xu
  0 siblings, 1 reply; 10+ messages in thread
From: LABBE Corentin @ 2016-06-02  9:12 UTC (permalink / raw)
  To: Herbert Xu; +Cc: davem, baolin.wang, linux-crypto, linux-kernel

On Thu, Jun 02, 2016 at 04:32:59PM +0800, Herbert Xu wrote:
> On Mon, May 30, 2016 at 03:32:01PM +0200, LABBE Corentin wrote:
> > The current crypto engine allow only ablkcipher_request to be enqueued.
> > Thus denying any use of it for hardware that also handle hash algo.
> > 
> > This patch convert all ablkcipher_request references to the
> > more general crypto_async_request.
> > 
> > Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> 
> First of all your patches break bisection which is unacceptable.
> 

How do I break bisection ?

> Secondly you should not be casting generic requests to a specific type.
> 
I didnt add any request type check since omap use engine only for ciphers.
My view if usage of crypt_one_request() if hash and ciphers coule be used is to test
crypto_tfm_alg_type(areq->tfm) to check which alg is used (CRYPTO_ALG_TYPE_AHASH vs CRYPTO_ALG_TYPE_ABLKCIPHER)

For example, this is my setted crypt_one_request function:
int handle_request(struct crypto_engine *engine, struct crypto_async_request *areq)
{
        int rtype;
        struct ahash_request *hreq;
        struct ablkcipher_request *breq;
        int err = -EINVAL;
        rtype = crypto_tfm_alg_type(areq->tfm);
        switch (rtype) {
        case CRYPTO_ALG_TYPE_AHASH:
                hreq = ahash_request_cast(areq);
                err = sun4i_ss_hash(hreq);
                break;
        case CRYPTO_ALG_TYPE_ABLKCIPHER:
                breq = ablkcipher_request_cast(areq);
                err = sun4i_ss_cipher(breq);
        }
        crypto_finalize_request(engine, areq, err);
        return 0;
}


> Assuming a single engine only has to deal with one type of requests,
> what you could do is to create a separate engine type for each
> crypto type that you want to support.
> 

So, if my hwcrypto can handle hash and ciphers, I need to have two engine and each crypt_one_request()/hash_one_request()
need to lock the engine.
Having only one engine that handle all types permit to avoid this locking.

Regards

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

* Re: [PATCH v2 1/2] crypto: engine: permit to enqueue ashash_request
  2016-06-02  9:12     ` LABBE Corentin
@ 2016-06-02  9:19       ` Herbert Xu
  2016-06-02  9:38         ` LABBE Corentin
  0 siblings, 1 reply; 10+ messages in thread
From: Herbert Xu @ 2016-06-02  9:19 UTC (permalink / raw)
  To: LABBE Corentin; +Cc: davem, baolin.wang, linux-crypto, linux-kernel

On Thu, Jun 02, 2016 at 11:12:13AM +0200, LABBE Corentin wrote:
> On Thu, Jun 02, 2016 at 04:32:59PM +0800, Herbert Xu wrote:
> > On Mon, May 30, 2016 at 03:32:01PM +0200, LABBE Corentin wrote:
> > > The current crypto engine allow only ablkcipher_request to be enqueued.
> > > Thus denying any use of it for hardware that also handle hash algo.
> > > 
> > > This patch convert all ablkcipher_request references to the
> > > more general crypto_async_request.
> > > 
> > > Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> > 
> > First of all your patches break bisection which is unacceptable.
> > 
> 
> How do I break bisection ?

Because the kernel won't compile after your first patch.

Either do it as one single patch or use the more elaborate "new
interafce" + "switchover" + "delete old interface" ritual.

> So, if my hwcrypto can handle hash and ciphers, I need to have two engine and each crypt_one_request()/hash_one_request()
> need to lock the engine.
> Having only one engine that handle all types permit to avoid this locking.

OK then we should add some type-checking as you suggested.  What
I don't want is just blind casting by the user of crypto_engine.

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

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

* Re: [PATCH v2 1/2] crypto: engine: permit to enqueue ashash_request
  2016-06-02  9:19       ` Herbert Xu
@ 2016-06-02  9:38         ` LABBE Corentin
  2016-06-02  9:42           ` Herbert Xu
  0 siblings, 1 reply; 10+ messages in thread
From: LABBE Corentin @ 2016-06-02  9:38 UTC (permalink / raw)
  To: Herbert Xu; +Cc: davem, baolin.wang, linux-crypto, linux-kernel

On Thu, Jun 02, 2016 at 05:19:40PM +0800, Herbert Xu wrote:
> On Thu, Jun 02, 2016 at 11:12:13AM +0200, LABBE Corentin wrote:
> > On Thu, Jun 02, 2016 at 04:32:59PM +0800, Herbert Xu wrote:
> > > On Mon, May 30, 2016 at 03:32:01PM +0200, LABBE Corentin wrote:
> > > > The current crypto engine allow only ablkcipher_request to be enqueued.
> > > > Thus denying any use of it for hardware that also handle hash algo.
> > > > 
> > > > This patch convert all ablkcipher_request references to the
> > > > more general crypto_async_request.
> > > > 
> > > > Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> > > 
> > > First of all your patches break bisection which is unacceptable.
> > > 
> > 
> > How do I break bisection ?
> 
> Because the kernel won't compile after your first patch.
> 
> Either do it as one single patch or use the more elaborate "new
> interafce" + "switchover" + "delete old interface" ritual.
> 

Since my patch is small and easy (and only one client is modified), do you mind if I choose the first one ?

> > So, if my hwcrypto can handle hash and ciphers, I need to have two engine and each crypt_one_request()/hash_one_request()
> > need to lock the engine.
> > Having only one engine that handle all types permit to avoid this locking.
> 
> OK then we should add some type-checking as you suggested.  What
> I don't want is just blind casting by the user of crypto_engine.

I will add this type checking on my patch against omap-aes/des.

Regards

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

* Re: [PATCH v2 1/2] crypto: engine: permit to enqueue ashash_request
  2016-06-02  9:38         ` LABBE Corentin
@ 2016-06-02  9:42           ` Herbert Xu
  0 siblings, 0 replies; 10+ messages in thread
From: Herbert Xu @ 2016-06-02  9:42 UTC (permalink / raw)
  To: LABBE Corentin; +Cc: davem, baolin.wang, linux-crypto, linux-kernel

On Thu, Jun 02, 2016 at 11:38:35AM +0200, LABBE Corentin wrote:
>
> Since my patch is small and easy (and only one client is modified), do you mind if I choose the first one ?

Sure.

> I will add this type checking on my patch against omap-aes/des.

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

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

end of thread, other threads:[~2016-06-02  9:43 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-30 13:32 [PATCH v2 0/2] crypto: engine: permit to enqueue ashash_request LABBE Corentin
2016-05-30 13:32 ` [PATCH v2 1/2] " LABBE Corentin
2016-06-01  2:27   ` Baolin Wang
2016-06-02  8:32   ` Herbert Xu
2016-06-02  9:12     ` LABBE Corentin
2016-06-02  9:19       ` Herbert Xu
2016-06-02  9:38         ` LABBE Corentin
2016-06-02  9:42           ` Herbert Xu
2016-05-30 13:32 ` [PATCH v2 2/2] crypto: omap: convert to the new cryptoengine API LABBE Corentin
2016-06-01  2:28   ` Baolin Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).