* [PATCH 1/9] crypto: engine: workqueue can only be processed one by one
2020-01-22 10:45 [PATCH 0/9] crypto: engine: permit to handle multiple requests Corentin Labbe
@ 2020-01-22 10:45 ` Corentin Labbe
[not found] ` <VI1PR0402MB3485B787EA6BCDD5A5600BAA980A0@VI1PR0402MB3485.eurprd04.prod.outlook.com>
2020-01-22 10:45 ` [PATCH 2/9] crypto: engine: get rid of cur_req_prepared Corentin Labbe
` (7 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Corentin Labbe @ 2020-01-22 10:45 UTC (permalink / raw)
To: davem, herbert, mripard, wens, iuliana.prodan
Cc: linux-sunxi, Corentin Labbe, linux-crypto, linux-arm-kernel,
linux-kernel
Some bykeshedding are unnecessary since a workqueue can only be executed
one by one.
This behaviour is documented in:
- kernel/kthread.c: comment of kthread_worker_fn()
- Documentation/core-api/workqueue.rst: the functions associated with the work items one after the other
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
crypto/crypto_engine.c | 13 -------------
include/crypto/engine.h | 2 --
2 files changed, 15 deletions(-)
diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
index eb029ff1e05a..feb0d82dd454 100644
--- a/crypto/crypto_engine.c
+++ b/crypto/crypto_engine.c
@@ -73,16 +73,6 @@ static void crypto_pump_requests(struct crypto_engine *engine,
spin_lock_irqsave(&engine->queue_lock, flags);
- /* Make sure we are not already running a request */
- if (engine->cur_req)
- goto out;
-
- /* If another context is idling then defer */
- if (engine->idling) {
- kthread_queue_work(engine->kworker, &engine->pump_requests);
- goto out;
- }
-
/* Check if the engine queue is idle */
if (!crypto_queue_len(&engine->queue) || !engine->running) {
if (!engine->busy)
@@ -96,7 +86,6 @@ static void crypto_pump_requests(struct crypto_engine *engine,
}
engine->busy = false;
- engine->idling = true;
spin_unlock_irqrestore(&engine->queue_lock, flags);
if (engine->unprepare_crypt_hardware &&
@@ -104,7 +93,6 @@ static void crypto_pump_requests(struct crypto_engine *engine,
dev_err(engine->dev, "failed to unprepare crypt hardware\n");
spin_lock_irqsave(&engine->queue_lock, flags);
- engine->idling = false;
goto out;
}
@@ -410,7 +398,6 @@ struct crypto_engine *crypto_engine_alloc_init(struct device *dev, bool rt)
engine->rt = rt;
engine->running = false;
engine->busy = false;
- engine->idling = false;
engine->cur_req_prepared = false;
engine->priv_data = dev;
snprintf(engine->name, sizeof(engine->name),
diff --git a/include/crypto/engine.h b/include/crypto/engine.h
index e29cd67f93c7..7e7cbd9ca3b5 100644
--- a/include/crypto/engine.h
+++ b/include/crypto/engine.h
@@ -21,7 +21,6 @@
/*
* struct crypto_engine - crypto hardware engine
* @name: the engine name
- * @idling: the engine is entering idle state
* @busy: request pump is busy
* @running: the engine is on working
* @cur_req_prepared: current request is prepared
@@ -42,7 +41,6 @@
*/
struct crypto_engine {
char name[ENGINE_NAME_LEN];
- bool idling;
bool busy;
bool running;
bool cur_req_prepared;
--
2.24.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/9] crypto: engine: get rid of cur_req_prepared
2020-01-22 10:45 [PATCH 0/9] crypto: engine: permit to handle multiple requests Corentin Labbe
2020-01-22 10:45 ` [PATCH 1/9] crypto: engine: workqueue can only be processed one by one Corentin Labbe
@ 2020-01-22 10:45 ` Corentin Labbe
2020-01-22 10:45 ` [PATCH 3/9] crypto: engine: get rid of cur_req Corentin Labbe
` (6 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Corentin Labbe @ 2020-01-22 10:45 UTC (permalink / raw)
To: davem, herbert, mripard, wens, iuliana.prodan
Cc: linux-sunxi, Corentin Labbe, linux-crypto, linux-arm-kernel,
linux-kernel
Since we want cryptoengine to support handling more than one request, we
need to remove cur_req_prepared.
It is used only to choose if we need to unprepare() in
crypto_finalize_request().
This choice is needed only in case of error, so let's handle error
without crypto_finalize_request.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
crypto/crypto_engine.c | 17 ++++++++++-------
include/crypto/engine.h | 2 --
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
index feb0d82dd454..dfcb00e92e09 100644
--- a/crypto/crypto_engine.c
+++ b/crypto/crypto_engine.c
@@ -36,7 +36,7 @@ static void crypto_finalize_request(struct crypto_engine *engine,
if (finalize_cur_req) {
enginectx = crypto_tfm_ctx(req->tfm);
- if (engine->cur_req_prepared &&
+ if (enginectx->op.prepare_request &&
enginectx->op.unprepare_request) {
ret = enginectx->op.unprepare_request(engine, req);
if (ret)
@@ -44,7 +44,6 @@ static void crypto_finalize_request(struct crypto_engine *engine,
}
spin_lock_irqsave(&engine->queue_lock, flags);
engine->cur_req = NULL;
- engine->cur_req_prepared = false;
spin_unlock_irqrestore(&engine->queue_lock, flags);
}
@@ -118,7 +117,7 @@ static void crypto_pump_requests(struct crypto_engine *engine,
ret = engine->prepare_crypt_hardware(engine);
if (ret) {
dev_err(engine->dev, "failed to prepare crypt hardware\n");
- goto req_err;
+ goto req_err2;
}
}
@@ -129,9 +128,8 @@ static void crypto_pump_requests(struct crypto_engine *engine,
if (ret) {
dev_err(engine->dev, "failed to prepare request: %d\n",
ret);
- goto req_err;
+ goto req_err2;
}
- engine->cur_req_prepared = true;
}
if (!enginectx->op.do_one_request) {
dev_err(engine->dev, "failed to do request\n");
@@ -146,7 +144,13 @@ static void crypto_pump_requests(struct crypto_engine *engine,
return;
req_err:
- crypto_finalize_request(engine, async_req, ret);
+ if (enginectx->op.unprepare_request) {
+ ret = enginectx->op.unprepare_request(engine, async_req);
+ if (ret)
+ dev_err(engine->dev, "failed to unprepare request\n");
+ }
+req_err2:
+ async_req->complete(async_req, ret);
return;
out:
@@ -398,7 +402,6 @@ struct crypto_engine *crypto_engine_alloc_init(struct device *dev, bool rt)
engine->rt = rt;
engine->running = false;
engine->busy = false;
- engine->cur_req_prepared = false;
engine->priv_data = dev;
snprintf(engine->name, sizeof(engine->name),
"%s-engine", dev_name(dev));
diff --git a/include/crypto/engine.h b/include/crypto/engine.h
index 7e7cbd9ca3b5..4d8a2602c9ed 100644
--- a/include/crypto/engine.h
+++ b/include/crypto/engine.h
@@ -23,7 +23,6 @@
* @name: the engine name
* @busy: request pump is busy
* @running: the engine is on working
- * @cur_req_prepared: current request is prepared
* @list: link with the global crypto engine list
* @queue_lock: spinlock to syncronise access to request queue
* @queue: the crypto queue of the engine
@@ -43,7 +42,6 @@ struct crypto_engine {
char name[ENGINE_NAME_LEN];
bool busy;
bool running;
- bool cur_req_prepared;
struct list_head list;
spinlock_t queue_lock;
--
2.24.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 3/9] crypto: engine: get rid of cur_req
2020-01-22 10:45 [PATCH 0/9] crypto: engine: permit to handle multiple requests Corentin Labbe
2020-01-22 10:45 ` [PATCH 1/9] crypto: engine: workqueue can only be processed one by one Corentin Labbe
2020-01-22 10:45 ` [PATCH 2/9] crypto: engine: get rid of cur_req_prepared Corentin Labbe
@ 2020-01-22 10:45 ` Corentin Labbe
2020-01-22 10:45 ` [PATCH 4/9] crypto: engine: permit to choose queue length Corentin Labbe
` (5 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Corentin Labbe @ 2020-01-22 10:45 UTC (permalink / raw)
To: davem, herbert, mripard, wens, iuliana.prodan
Cc: linux-sunxi, Corentin Labbe, linux-crypto, linux-arm-kernel,
linux-kernel
cur_req was used as a sign of usage of the crypto_engine, now this
behaviour has gone, its usage remains for detecting if we finalize the
cur_req.
But testing if we finalize the cur_req prevent to do more request at a
time and is unnecessary.
It is unnecessary since crypto_finalize_request() is only used for
cryptoengine and so the request finalized will be always the current
request.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
crypto/crypto_engine.c | 25 ++++++-------------------
include/crypto/engine.h | 2 --
2 files changed, 6 insertions(+), 21 deletions(-)
diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
index dfcb00e92e09..c21867106aa4 100644
--- a/crypto/crypto_engine.c
+++ b/crypto/crypto_engine.c
@@ -24,27 +24,15 @@
static void crypto_finalize_request(struct crypto_engine *engine,
struct crypto_async_request *req, int err)
{
- unsigned long flags;
- bool finalize_cur_req = false;
int ret;
struct crypto_engine_ctx *enginectx;
- spin_lock_irqsave(&engine->queue_lock, flags);
- if (engine->cur_req == req)
- finalize_cur_req = true;
- spin_unlock_irqrestore(&engine->queue_lock, flags);
-
- if (finalize_cur_req) {
- enginectx = crypto_tfm_ctx(req->tfm);
- if (enginectx->op.prepare_request &&
- enginectx->op.unprepare_request) {
- ret = enginectx->op.unprepare_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;
- spin_unlock_irqrestore(&engine->queue_lock, flags);
+ enginectx = crypto_tfm_ctx(req->tfm);
+ if (enginectx->op.prepare_request &&
+ enginectx->op.unprepare_request) {
+ ret = enginectx->op.unprepare_request(engine, req);
+ if (ret)
+ dev_err(engine->dev, "failed to unprepare request\n");
}
req->complete(req, err);
@@ -101,7 +89,6 @@ static void crypto_pump_requests(struct crypto_engine *engine,
if (!async_req)
goto out;
- engine->cur_req = async_req;
if (backlog)
backlog->complete(backlog, -EINPROGRESS);
diff --git a/include/crypto/engine.h b/include/crypto/engine.h
index 4d8a2602c9ed..d77080227beb 100644
--- a/include/crypto/engine.h
+++ b/include/crypto/engine.h
@@ -36,7 +36,6 @@
* @kworker: kthread worker struct for request pump
* @pump_requests: work struct for scheduling work to the request pump
* @priv_data: the engine private data
- * @cur_req: the current request which is on processing
*/
struct crypto_engine {
char name[ENGINE_NAME_LEN];
@@ -57,7 +56,6 @@ struct crypto_engine {
struct kthread_work pump_requests;
void *priv_data;
- struct crypto_async_request *cur_req;
};
/*
--
2.24.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 4/9] crypto: engine: permit to choose queue length
2020-01-22 10:45 [PATCH 0/9] crypto: engine: permit to handle multiple requests Corentin Labbe
` (2 preceding siblings ...)
2020-01-22 10:45 ` [PATCH 3/9] crypto: engine: get rid of cur_req Corentin Labbe
@ 2020-01-22 10:45 ` Corentin Labbe
2020-01-22 10:45 ` [PATCH 5/9] crypto: engine: add enqueue_request/can_do_more Corentin Labbe
` (4 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Corentin Labbe @ 2020-01-22 10:45 UTC (permalink / raw)
To: davem, herbert, mripard, wens, iuliana.prodan
Cc: linux-sunxi, Corentin Labbe, linux-crypto, linux-arm-kernel,
linux-kernel
This patch adds a new function which permit to choose the crypto engine
queue length.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
crypto/crypto_engine.c | 23 ++++++++++++++++++++---
include/crypto/engine.h | 2 ++
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
index c21867106aa4..5bcb1e740fd9 100644
--- a/crypto/crypto_engine.c
+++ b/crypto/crypto_engine.c
@@ -365,15 +365,17 @@ int crypto_engine_stop(struct crypto_engine *engine)
EXPORT_SYMBOL_GPL(crypto_engine_stop);
/**
- * crypto_engine_alloc_init - allocate crypto hardware engine structure and
+ * crypto_engine_alloc_init2 - allocate crypto hardware engine structure and
* initialize it.
* @dev: the device attached with one hardware engine
* @rt: whether this queue is set to run as a realtime task
+ * @qlen: The size of the crypto queue
*
* This must be called from context that can sleep.
* Return: the crypto engine structure on success, else NULL.
*/
-struct crypto_engine *crypto_engine_alloc_init(struct device *dev, bool rt)
+struct crypto_engine *crypto_engine_alloc_init2(struct device *dev, bool rt,
+ int qlen)
{
struct sched_param param = { .sched_priority = MAX_RT_PRIO / 2 };
struct crypto_engine *engine;
@@ -393,7 +395,7 @@ struct crypto_engine *crypto_engine_alloc_init(struct device *dev, bool rt)
snprintf(engine->name, sizeof(engine->name),
"%s-engine", dev_name(dev));
- crypto_init_queue(&engine->queue, CRYPTO_ENGINE_MAX_QLEN);
+ crypto_init_queue(&engine->queue, qlen);
spin_lock_init(&engine->queue_lock);
engine->kworker = kthread_create_worker(0, "%s", engine->name);
@@ -410,6 +412,21 @@ struct crypto_engine *crypto_engine_alloc_init(struct device *dev, bool rt)
return engine;
}
+EXPORT_SYMBOL_GPL(crypto_engine_alloc_init2);
+
+/**
+ * crypto_engine_alloc_init - allocate crypto hardware engine structure and
+ * initialize it.
+ * @dev: the device attached with one hardware engine
+ * @rt: whether this queue is set to run as a realtime task
+ *
+ * This must be called from context that can sleep.
+ * Return: the crypto engine structure on success, else NULL.
+ */
+struct crypto_engine *crypto_engine_alloc_init(struct device *dev, bool rt)
+{
+ return crypto_engine_alloc_init2(dev, rt, CRYPTO_ENGINE_MAX_QLEN);
+}
EXPORT_SYMBOL_GPL(crypto_engine_alloc_init);
/**
diff --git a/include/crypto/engine.h b/include/crypto/engine.h
index d77080227beb..03d9f9ec1cea 100644
--- a/include/crypto/engine.h
+++ b/include/crypto/engine.h
@@ -96,6 +96,8 @@ void crypto_finalize_skcipher_request(struct crypto_engine *engine,
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);
+struct crypto_engine *crypto_engine_alloc_init2(struct device *dev, bool rt,
+ int qlen);
int crypto_engine_exit(struct crypto_engine *engine);
#endif /* _CRYPTO_ENGINE_H */
--
2.24.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 5/9] crypto: engine: add enqueue_request/can_do_more
2020-01-22 10:45 [PATCH 0/9] crypto: engine: permit to handle multiple requests Corentin Labbe
` (3 preceding siblings ...)
2020-01-22 10:45 ` [PATCH 4/9] crypto: engine: permit to choose queue length Corentin Labbe
@ 2020-01-22 10:45 ` Corentin Labbe
2020-01-27 22:58 ` Iuliana Prodan
2020-01-22 10:45 ` [PATCH 6/9] crypto: sun8i-ce: move iv data to request context Corentin Labbe
` (3 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Corentin Labbe @ 2020-01-22 10:45 UTC (permalink / raw)
To: davem, herbert, mripard, wens, iuliana.prodan
Cc: linux-sunxi, Corentin Labbe, linux-crypto, linux-arm-kernel,
linux-kernel
This patchs adds two new function wrapper in crypto_engine.
- enqueue_request() for drivers enqueuing request to hardware.
- can_queue_more() for letting drivers to tell if they can
enqueue/prepare more.
Since some drivers (like caam) only enqueue request without "doing"
them, do_one_request() is now optional.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
crypto/crypto_engine.c | 25 ++++++++++++++++++++++---
include/crypto/engine.h | 14 ++++++++------
2 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
index 5bcb1e740fd9..4a28548c49aa 100644
--- a/crypto/crypto_engine.c
+++ b/crypto/crypto_engine.c
@@ -83,6 +83,7 @@ static void crypto_pump_requests(struct crypto_engine *engine,
goto out;
}
+retry:
/* Get the fist request from the engine queue to handle */
backlog = crypto_get_backlog(&engine->queue);
async_req = crypto_dequeue_request(&engine->queue);
@@ -118,10 +119,28 @@ static void crypto_pump_requests(struct crypto_engine *engine,
goto req_err2;
}
}
+
+ if (enginectx->op.enqueue_request) {
+ ret = enginectx->op.enqueue_request(engine, async_req);
+ if (ret) {
+ dev_err(engine->dev, "failed to enqueue request: %d\n",
+ ret);
+ goto req_err;
+ }
+ }
+ if (enginectx->op.can_queue_more && engine->queue.qlen > 0) {
+ ret = enginectx->op.can_queue_more(engine, async_req);
+ if (ret > 0) {
+ spin_lock_irqsave(&engine->queue_lock, flags);
+ goto retry;
+ }
+ if (ret < 0) {
+ dev_err(engine->dev, "failed to call can_queue_more\n");
+ /* TODO */
+ }
+ }
if (!enginectx->op.do_one_request) {
- dev_err(engine->dev, "failed to do request\n");
- ret = -EINVAL;
- goto req_err;
+ return;
}
ret = enginectx->op.do_one_request(engine, async_req);
if (ret) {
diff --git a/include/crypto/engine.h b/include/crypto/engine.h
index 03d9f9ec1cea..8ab9d26e30fe 100644
--- a/include/crypto/engine.h
+++ b/include/crypto/engine.h
@@ -63,14 +63,16 @@ struct crypto_engine {
* @prepare__request: do some prepare if need before handle the current request
* @unprepare_request: undo any work done by prepare_request()
* @do_one_request: do encryption for current request
+ * @enqueue_request: Enqueue the request in the hardware
+ * @can_queue_more: if this function return > 0, it will tell the crypto
+ * engine that more space are availlable for prepare/enqueue request
*/
struct crypto_engine_op {
- int (*prepare_request)(struct crypto_engine *engine,
- void *areq);
- int (*unprepare_request)(struct crypto_engine *engine,
- void *areq);
- int (*do_one_request)(struct crypto_engine *engine,
- void *areq);
+ int (*prepare_request)(struct crypto_engine *engine, void *areq);
+ int (*unprepare_request)(struct crypto_engine *engine, void *areq);
+ int (*do_one_request)(struct crypto_engine *engine, void *areq);
+ int (*enqueue_request)(struct crypto_engine *engine, void *areq);
+ int (*can_queue_more)(struct crypto_engine *engine, void *areq);
};
struct crypto_engine_ctx {
--
2.24.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 5/9] crypto: engine: add enqueue_request/can_do_more
2020-01-22 10:45 ` [PATCH 5/9] crypto: engine: add enqueue_request/can_do_more Corentin Labbe
@ 2020-01-27 22:58 ` Iuliana Prodan
2020-01-28 8:40 ` Corentin Labbe
0 siblings, 1 reply; 15+ messages in thread
From: Iuliana Prodan @ 2020-01-27 22:58 UTC (permalink / raw)
To: Corentin Labbe, davem@davemloft.net, herbert@gondor.apana.org.au,
mripard@kernel.org, wens@csie.org
Cc: linux-sunxi@googlegroups.com, linux-crypto@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
On 1/22/2020 12:45 PM, Corentin Labbe wrote:
> This patchs adds two new function wrapper in crypto_engine.
> - enqueue_request() for drivers enqueuing request to hardware.
> - can_queue_more() for letting drivers to tell if they can
> enqueue/prepare more.
>
> Since some drivers (like caam) only enqueue request without "doing"
> them, do_one_request() is now optional.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
> crypto/crypto_engine.c | 25 ++++++++++++++++++++++---
> include/crypto/engine.h | 14 ++++++++------
> 2 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
> index 5bcb1e740fd9..4a28548c49aa 100644
> --- a/crypto/crypto_engine.c
> +++ b/crypto/crypto_engine.c
> @@ -83,6 +83,7 @@ static void crypto_pump_requests(struct crypto_engine *engine,
> goto out;
> }
>
> +retry:
> /* Get the fist request from the engine queue to handle */
> backlog = crypto_get_backlog(&engine->queue);
> async_req = crypto_dequeue_request(&engine->queue);
> @@ -118,10 +119,28 @@ static void crypto_pump_requests(struct crypto_engine *engine,
> goto req_err2;
> }
> }
> +
> + if (enginectx->op.enqueue_request) {
> + ret = enginectx->op.enqueue_request(engine, async_req);
> + if (ret) {
> + dev_err(engine->dev, "failed to enqueue request: %d\n",
> + ret);
> + goto req_err;
> + }
> + }
> + if (enginectx->op.can_queue_more && engine->queue.qlen > 0) {
> + ret = enginectx->op.can_queue_more(engine, async_req);
> + if (ret > 0) {
> + spin_lock_irqsave(&engine->queue_lock, flags);
> + goto retry;
> + }
> + if (ret < 0) {
> + dev_err(engine->dev, "failed to call can_queue_more\n");
> + /* TODO */
> + }
> + }
> if (!enginectx->op.do_one_request) {
> - dev_err(engine->dev, "failed to do request\n");
> - ret = -EINVAL;
> - goto req_err;
> + return;
> }
> ret = enginectx->op.do_one_request(engine, async_req);
> if (ret) {
> diff --git a/include/crypto/engine.h b/include/crypto/engine.h
> index 03d9f9ec1cea..8ab9d26e30fe 100644
> --- a/include/crypto/engine.h
> +++ b/include/crypto/engine.h
> @@ -63,14 +63,16 @@ struct crypto_engine {
> * @prepare__request: do some prepare if need before handle the current request
> * @unprepare_request: undo any work done by prepare_request()
> * @do_one_request: do encryption for current request
> + * @enqueue_request: Enqueue the request in the hardware
> + * @can_queue_more: if this function return > 0, it will tell the crypto
> + * engine that more space are availlable for prepare/enqueue request
> */
> struct crypto_engine_op {
> - int (*prepare_request)(struct crypto_engine *engine,
> - void *areq);
> - int (*unprepare_request)(struct crypto_engine *engine,
> - void *areq);
> - int (*do_one_request)(struct crypto_engine *engine,
> - void *areq);
> + int (*prepare_request)(struct crypto_engine *engine, void *areq);
> + int (*unprepare_request)(struct crypto_engine *engine, void *areq);
> + int (*do_one_request)(struct crypto_engine *engine, void *areq);
> + int (*enqueue_request)(struct crypto_engine *engine, void *areq);
> + int (*can_queue_more)(struct crypto_engine *engine, void *areq);
> };
As I mentioned in another thread [1], these crypto-engine patches (#1 -
#5) imply modifications in all the drivers that use crypto-engine.
It's not backwards compatible.
Your changes imply that do_one_request executes the request & waits for
completion and enqueue_request sends it to hardware. That means that all
the other drivers need to be modify, to implement enqueue_request,
instead of do_one_request. They need to be compliant with the new
changes, new API. Otherwise, they are not using crypto-engine right,
don't you think?
Also, do_one_request it shouldn’t be blocking. We got this confirmation
from Herbert [2].
[1]
https://lore.kernel.org/lkml/VI1PR04MB44455343230CBA7400D21C998C0C0@VI1PR04MB4445.eurprd04.prod.outlook.com/
[2]
https://lore.kernel.org/lkml/20200122144134.axqpwx65j7xysyy3@gondor.apana.org.au/
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 5/9] crypto: engine: add enqueue_request/can_do_more
2020-01-27 22:58 ` Iuliana Prodan
@ 2020-01-28 8:40 ` Corentin Labbe
2020-01-28 11:00 ` Iuliana Prodan
0 siblings, 1 reply; 15+ messages in thread
From: Corentin Labbe @ 2020-01-28 8:40 UTC (permalink / raw)
To: Iuliana Prodan
Cc: herbert@gondor.apana.org.au, linux-sunxi@googlegroups.com,
linux-kernel@vger.kernel.org, mripard@kernel.org, wens@csie.org,
linux-crypto@vger.kernel.org, davem@davemloft.net,
linux-arm-kernel@lists.infradead.org
On Mon, Jan 27, 2020 at 10:58:36PM +0000, Iuliana Prodan wrote:
> On 1/22/2020 12:45 PM, Corentin Labbe wrote:
> > This patchs adds two new function wrapper in crypto_engine.
> > - enqueue_request() for drivers enqueuing request to hardware.
> > - can_queue_more() for letting drivers to tell if they can
> > enqueue/prepare more.
> >
> > Since some drivers (like caam) only enqueue request without "doing"
> > them, do_one_request() is now optional.
> >
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > ---
> > crypto/crypto_engine.c | 25 ++++++++++++++++++++++---
> > include/crypto/engine.h | 14 ++++++++------
> > 2 files changed, 30 insertions(+), 9 deletions(-)
> >
> > diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
> > index 5bcb1e740fd9..4a28548c49aa 100644
> > --- a/crypto/crypto_engine.c
> > +++ b/crypto/crypto_engine.c
> > @@ -83,6 +83,7 @@ static void crypto_pump_requests(struct crypto_engine *engine,
> > goto out;
> > }
> >
> > +retry:
> > /* Get the fist request from the engine queue to handle */
> > backlog = crypto_get_backlog(&engine->queue);
> > async_req = crypto_dequeue_request(&engine->queue);
> > @@ -118,10 +119,28 @@ static void crypto_pump_requests(struct crypto_engine *engine,
> > goto req_err2;
> > }
> > }
> > +
> > + if (enginectx->op.enqueue_request) {
> > + ret = enginectx->op.enqueue_request(engine, async_req);
> > + if (ret) {
> > + dev_err(engine->dev, "failed to enqueue request: %d\n",
> > + ret);
> > + goto req_err;
> > + }
> > + }
> > + if (enginectx->op.can_queue_more && engine->queue.qlen > 0) {
> > + ret = enginectx->op.can_queue_more(engine, async_req);
> > + if (ret > 0) {
> > + spin_lock_irqsave(&engine->queue_lock, flags);
> > + goto retry;
> > + }
> > + if (ret < 0) {
> > + dev_err(engine->dev, "failed to call can_queue_more\n");
> > + /* TODO */
> > + }
> > + }
> > if (!enginectx->op.do_one_request) {
> > - dev_err(engine->dev, "failed to do request\n");
> > - ret = -EINVAL;
> > - goto req_err;
> > + return;
> > }
> > ret = enginectx->op.do_one_request(engine, async_req);
> > if (ret) {
> > diff --git a/include/crypto/engine.h b/include/crypto/engine.h
> > index 03d9f9ec1cea..8ab9d26e30fe 100644
> > --- a/include/crypto/engine.h
> > +++ b/include/crypto/engine.h
> > @@ -63,14 +63,16 @@ struct crypto_engine {
> > * @prepare__request: do some prepare if need before handle the current request
> > * @unprepare_request: undo any work done by prepare_request()
> > * @do_one_request: do encryption for current request
> > + * @enqueue_request: Enqueue the request in the hardware
> > + * @can_queue_more: if this function return > 0, it will tell the crypto
> > + * engine that more space are availlable for prepare/enqueue request
> > */
> > struct crypto_engine_op {
> > - int (*prepare_request)(struct crypto_engine *engine,
> > - void *areq);
> > - int (*unprepare_request)(struct crypto_engine *engine,
> > - void *areq);
> > - int (*do_one_request)(struct crypto_engine *engine,
> > - void *areq);
> > + int (*prepare_request)(struct crypto_engine *engine, void *areq);
> > + int (*unprepare_request)(struct crypto_engine *engine, void *areq);
> > + int (*do_one_request)(struct crypto_engine *engine, void *areq);
> > + int (*enqueue_request)(struct crypto_engine *engine, void *areq);
> > + int (*can_queue_more)(struct crypto_engine *engine, void *areq);
> > };
>
> As I mentioned in another thread [1], these crypto-engine patches (#1 -
> #5) imply modifications in all the drivers that use crypto-engine.
> It's not backwards compatible.
This is wrong. This is false.
AS I HAVE ALREADY SAID, I have tested and didnt see any behavour change in the current user of crypto engine.
I have tested my serie with omap, virtio, amlogic, sun8i-ss, sun8i-ce and didnt see any change in behavour WITHOUT CHANGING them.
I resaid, I didnt touch omap, virtio, etc...
Only stm32 is not tested because simply there are not board with this driver enabled.
I have also tested your serie which adds support for crypto engine to caam, and the crash is the same with/without my serie.
So no behavour change.
> Your changes imply that do_one_request executes the request & waits for
> completion and enqueue_request sends it to hardware. That means that all
> the other drivers need to be modify, to implement enqueue_request,
> instead of do_one_request. They need to be compliant with the new
> changes, new API. Otherwise, they are not using crypto-engine right,
> don't you think?
>
My change imply nothing, current user work the same.
But if they want, they COULD switch to enqueue_request().
> Also, do_one_request it shouldn’t be blocking. We got this confirmation
> from Herbert [2].
Re-read what Herbert said, "It certainly shouldn't be blocking in the general case." But that means it could.
But this wont change my patch since both behavour are supported.
>
> [1]
> https://lore.kernel.org/lkml/VI1PR04MB44455343230CBA7400D21C998C0C0@VI1PR04MB4445.eurprd04.prod.outlook.com/
> [2]
> https://lore.kernel.org/lkml/20200122144134.axqpwx65j7xysyy3@gondor.apana.org.au/
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 5/9] crypto: engine: add enqueue_request/can_do_more
2020-01-28 8:40 ` Corentin Labbe
@ 2020-01-28 11:00 ` Iuliana Prodan
0 siblings, 0 replies; 15+ messages in thread
From: Iuliana Prodan @ 2020-01-28 11:00 UTC (permalink / raw)
To: Corentin Labbe
Cc: herbert@gondor.apana.org.au, linux-kernel@vger.kernel.org,
mripard@kernel.org, Franck Lenormand, wens@csie.org,
linux-crypto@vger.kernel.org, Silvano Di Ninno,
davem@davemloft.net, linux-arm-kernel@lists.infradead.org
On 1/28/2020 10:40 AM, Corentin Labbe wrote:
> On Mon, Jan 27, 2020 at 10:58:36PM +0000, Iuliana Prodan wrote:
>> On 1/22/2020 12:45 PM, Corentin Labbe wrote:
>>> This patchs adds two new function wrapper in crypto_engine.
>>> - enqueue_request() for drivers enqueuing request to hardware.
>>> - can_queue_more() for letting drivers to tell if they can
>>> enqueue/prepare more.
>>>
>>> Since some drivers (like caam) only enqueue request without "doing"
>>> them, do_one_request() is now optional.
>>>
>>> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
>>> ---
>>> crypto/crypto_engine.c | 25 ++++++++++++++++++++++---
>>> include/crypto/engine.h | 14 ++++++++------
>>> 2 files changed, 30 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
>>> index 5bcb1e740fd9..4a28548c49aa 100644
>>> --- a/crypto/crypto_engine.c
>>> +++ b/crypto/crypto_engine.c
>>> @@ -83,6 +83,7 @@ static void crypto_pump_requests(struct crypto_engine *engine,
>>> goto out;
>>> }
>>>
>>> +retry:
>>> /* Get the fist request from the engine queue to handle */
>>> backlog = crypto_get_backlog(&engine->queue);
>>> async_req = crypto_dequeue_request(&engine->queue);
>>> @@ -118,10 +119,28 @@ static void crypto_pump_requests(struct crypto_engine *engine,
>>> goto req_err2;
>>> }
>>> }
>>> +
>>> + if (enginectx->op.enqueue_request) {
>>> + ret = enginectx->op.enqueue_request(engine, async_req);
>>> + if (ret) {
>>> + dev_err(engine->dev, "failed to enqueue request: %d\n",
>>> + ret);
>>> + goto req_err;
>>> + }
>>> + }
>>> + if (enginectx->op.can_queue_more && engine->queue.qlen > 0) {
>>> + ret = enginectx->op.can_queue_more(engine, async_req);
>>> + if (ret > 0) {
>>> + spin_lock_irqsave(&engine->queue_lock, flags);
>>> + goto retry;
>>> + }
>>> + if (ret < 0) {
>>> + dev_err(engine->dev, "failed to call can_queue_more\n");
>>> + /* TODO */
>>> + }
>>> + }
>>> if (!enginectx->op.do_one_request) {
>>> - dev_err(engine->dev, "failed to do request\n");
>>> - ret = -EINVAL;
>>> - goto req_err;
>>> + return;
>>> }
>>> ret = enginectx->op.do_one_request(engine, async_req);
>>> if (ret) {
>>> diff --git a/include/crypto/engine.h b/include/crypto/engine.h
>>> index 03d9f9ec1cea..8ab9d26e30fe 100644
>>> --- a/include/crypto/engine.h
>>> +++ b/include/crypto/engine.h
>>> @@ -63,14 +63,16 @@ struct crypto_engine {
>>> * @prepare__request: do some prepare if need before handle the current request
>>> * @unprepare_request: undo any work done by prepare_request()
>>> * @do_one_request: do encryption for current request
>>> + * @enqueue_request: Enqueue the request in the hardware
>>> + * @can_queue_more: if this function return > 0, it will tell the crypto
>>> + * engine that more space are availlable for prepare/enqueue request
>>> */
>>> struct crypto_engine_op {
>>> - int (*prepare_request)(struct crypto_engine *engine,
>>> - void *areq);
>>> - int (*unprepare_request)(struct crypto_engine *engine,
>>> - void *areq);
>>> - int (*do_one_request)(struct crypto_engine *engine,
>>> - void *areq);
>>> + int (*prepare_request)(struct crypto_engine *engine, void *areq);
>>> + int (*unprepare_request)(struct crypto_engine *engine, void *areq);
>>> + int (*do_one_request)(struct crypto_engine *engine, void *areq);
>>> + int (*enqueue_request)(struct crypto_engine *engine, void *areq);
>>> + int (*can_queue_more)(struct crypto_engine *engine, void *areq);
>>> };
>>
>> As I mentioned in another thread [1], these crypto-engine patches (#1 -
>> #5) imply modifications in all the drivers that use crypto-engine.
>> It's not backwards compatible.
>
> This is wrong. This is false.
> AS I HAVE ALREADY SAID, I have tested and didnt see any behavour change in the current user of crypto engine.
> I have tested my serie with omap, virtio, amlogic, sun8i-ss, sun8i-ce and didnt see any change in behavour WITHOUT CHANGING them.
> I resaid, I didnt touch omap, virtio, etc...
> Only stm32 is not tested because simply there are not board with this driver enabled.
>
I'm not saying that doesn't compile or anything, is just that you change
the API and those drivers will not comply on this new API.
I believe that "it works" is not sufficient, should work properly!
> I have also tested your serie which adds support for crypto engine to caam, and the crash is the same with/without my serie.
> So no behavour change.
>
Thanks for testing this. I'll look into it!
>> Your changes imply that do_one_request executes the request & waits for
>> completion and enqueue_request sends it to hardware. That means that all
>> the other drivers need to be modify, to implement enqueue_request,
>> instead of do_one_request. They need to be compliant with the new
>> changes, new API. Otherwise, they are not using crypto-engine right,
>> don't you think?
>>
>
> My change imply nothing, current user work the same.
> But if they want, they COULD switch to enqueue_request().
>
>> Also, do_one_request it shouldn’t be blocking. We got this confirmation
>> from Herbert [2].
>
> Re-read what Herbert said, "It certainly shouldn't be blocking in the general case." But that means it could.
> But this wont change my patch since both behavour are supported.
>
Since your driver is the one being different (implements do_one_request
as blocking), it's not fair to change the other drivers just for you
special case when we update the crypto-engine. It should be the other
way around.
Add a special case for you and let the other drivers unchanged.
The updated crypto-engine API should be consistent (same semantics) with
the old one. Your proposal doesn't extend the current API, but
reshuffles the callbacks changing their meaning.
We should agree on how we should update crypto-engine to accommodate all
the scenarios, but maintaining backwards compatibility.
Thanks,
Iulia
>>
>> [1]
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2FVI1PR04MB44455343230CBA7400D21C998C0C0%40VI1PR04MB4445.eurprd04.prod.outlook.com%2F&data=02%7C01%7Ciuliana.prodan%40nxp.com%7C238e3e9a8e5f4d934cf308d7a3cdc3da%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637157976462463995&sdata=rgzYhX0g9hrzlYcHs7aUWVNFYs6mj86gDu7YIowy0Nk%3D&reserved=0
>> [2]
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20200122144134.axqpwx65j7xysyy3%40gondor.apana.org.au%2F&data=02%7C01%7Ciuliana.prodan%40nxp.com%7C238e3e9a8e5f4d934cf308d7a3cdc3da%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637157976462463995&sdata=Jdp0Q8xPnn5uXtcv6hrk3sFbeC5PgzfwRys2itmL09w%3D&reserved=0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 6/9] crypto: sun8i-ce: move iv data to request context
2020-01-22 10:45 [PATCH 0/9] crypto: engine: permit to handle multiple requests Corentin Labbe
` (4 preceding siblings ...)
2020-01-22 10:45 ` [PATCH 5/9] crypto: engine: add enqueue_request/can_do_more Corentin Labbe
@ 2020-01-22 10:45 ` Corentin Labbe
2020-01-22 10:45 ` [PATCH 7/9] crypto: sun8i-ce: increase task list size Corentin Labbe
` (2 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Corentin Labbe @ 2020-01-22 10:45 UTC (permalink / raw)
To: davem, herbert, mripard, wens, iuliana.prodan
Cc: linux-sunxi, Corentin Labbe, linux-crypto, linux-arm-kernel,
linux-kernel
Instead of storing IV data in the channel context, store them in the
request context.
Storing them in the channel structure was conceptualy wrong since they
are per request related.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
.../allwinner/sun8i-ce/sun8i-ce-cipher.c | 27 +++++++++----------
drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h | 10 ++++---
2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
index a5fd8975f3d3..7fd19667bceb 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
@@ -91,7 +91,6 @@ static int sun8i_ce_cipher(struct skcipher_request *areq)
struct scatterlist *sg;
unsigned int todo, len, offset, ivsize;
dma_addr_t addr_iv = 0, addr_key = 0;
- void *backup_iv = NULL;
u32 common, sym;
int flow, i;
int nr_sgs = 0;
@@ -154,24 +153,24 @@ static int sun8i_ce_cipher(struct skcipher_request *areq)
ivsize = crypto_skcipher_ivsize(tfm);
if (areq->iv && crypto_skcipher_ivsize(tfm) > 0) {
- chan->ivlen = ivsize;
- chan->bounce_iv = kzalloc(ivsize, GFP_KERNEL | GFP_DMA);
- if (!chan->bounce_iv) {
+ rctx->ivlen = ivsize;
+ rctx->bounce_iv = kzalloc(ivsize, GFP_KERNEL | GFP_DMA);
+ if (!rctx->bounce_iv) {
err = -ENOMEM;
goto theend_key;
}
if (rctx->op_dir & CE_DECRYPTION) {
- backup_iv = kzalloc(ivsize, GFP_KERNEL);
- if (!backup_iv) {
+ rctx->backup_iv = kzalloc(ivsize, GFP_KERNEL);
+ if (!rctx->backup_iv) {
err = -ENOMEM;
goto theend_key;
}
offset = areq->cryptlen - ivsize;
- scatterwalk_map_and_copy(backup_iv, areq->src, offset,
- ivsize, 0);
+ scatterwalk_map_and_copy(rctx->backup_iv, areq->src,
+ offset, ivsize, 0);
}
- memcpy(chan->bounce_iv, areq->iv, ivsize);
- addr_iv = dma_map_single(ce->dev, chan->bounce_iv, chan->ivlen,
+ memcpy(rctx->bounce_iv, areq->iv, ivsize);
+ addr_iv = dma_map_single(ce->dev, rctx->bounce_iv, rctx->ivlen,
DMA_TO_DEVICE);
cet->t_iv = cpu_to_le32(addr_iv);
if (dma_mapping_error(ce->dev, addr_iv)) {
@@ -252,17 +251,17 @@ static int sun8i_ce_cipher(struct skcipher_request *areq)
theend_iv:
if (areq->iv && ivsize > 0) {
if (addr_iv)
- dma_unmap_single(ce->dev, addr_iv, chan->ivlen,
+ dma_unmap_single(ce->dev, addr_iv, rctx->ivlen,
DMA_TO_DEVICE);
offset = areq->cryptlen - ivsize;
if (rctx->op_dir & CE_DECRYPTION) {
- memcpy(areq->iv, backup_iv, ivsize);
- kzfree(backup_iv);
+ memcpy(areq->iv, rctx->backup_iv, ivsize);
+ kzfree(rctx->backup_iv);
} else {
scatterwalk_map_and_copy(areq->iv, areq->dst, offset,
ivsize, 0);
}
- kfree(chan->bounce_iv);
+ kfree(rctx->bounce_iv);
}
theend_key:
diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
index 8f8404c84a4d..49507ef2ec63 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
@@ -129,8 +129,6 @@ struct ce_task {
/*
* struct sun8i_ce_flow - Information used by each flow
* @engine: ptr to the crypto_engine for this flow
- * @bounce_iv: buffer which contain the IV
- * @ivlen: size of bounce_iv
* @complete: completion for the current task on this flow
* @status: set to 1 by interrupt if task is done
* @t_phy: Physical address of task
@@ -139,8 +137,6 @@ struct ce_task {
*/
struct sun8i_ce_flow {
struct crypto_engine *engine;
- void *bounce_iv;
- unsigned int ivlen;
struct completion complete;
int status;
dma_addr_t t_phy;
@@ -183,10 +179,16 @@ struct sun8i_ce_dev {
* struct sun8i_cipher_req_ctx - context for a skcipher request
* @op_dir: direction (encrypt vs decrypt) for this request
* @flow: the flow to use for this request
+ * @backup_iv: buffer which contain the next IV to store
+ * @bounce_iv: buffer which contain a copy of IV
+ * @ivlen: size of bounce_iv
*/
struct sun8i_cipher_req_ctx {
u32 op_dir;
int flow;
+ void *backup_iv;
+ void *bounce_iv;
+ unsigned int ivlen;
};
/*
--
2.24.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 7/9] crypto: sun8i-ce: increase task list size
2020-01-22 10:45 [PATCH 0/9] crypto: engine: permit to handle multiple requests Corentin Labbe
` (5 preceding siblings ...)
2020-01-22 10:45 ` [PATCH 6/9] crypto: sun8i-ce: move iv data to request context Corentin Labbe
@ 2020-01-22 10:45 ` Corentin Labbe
2020-01-22 10:45 ` [PATCH 8/9] crypto: sun8i-ce: split into prepare/run/unprepare Corentin Labbe
2020-01-22 10:45 ` [PATCH 9/9] crypto: sun8i-ce: permit to batch requests Corentin Labbe
8 siblings, 0 replies; 15+ messages in thread
From: Corentin Labbe @ 2020-01-22 10:45 UTC (permalink / raw)
To: davem, herbert, mripard, wens, iuliana.prodan
Cc: linux-sunxi, Corentin Labbe, linux-crypto, linux-arm-kernel,
linux-kernel
The CE can handle more than 1 task at once, so this patch increase the size of
the task list up to 8.
For the moment I did not see more gain beyong this value.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c | 4 ++--
drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
index f72346a44e69..e8bf7bf31061 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
@@ -321,7 +321,7 @@ static void sun8i_ce_free_chanlist(struct sun8i_ce_dev *ce, int i)
while (i >= 0) {
crypto_engine_exit(ce->chanlist[i].engine);
if (ce->chanlist[i].tl)
- dma_free_coherent(ce->dev, sizeof(struct ce_task),
+ dma_free_coherent(ce->dev, sizeof(struct ce_task) * MAXTASK,
ce->chanlist[i].tl,
ce->chanlist[i].t_phy);
i--;
@@ -356,7 +356,7 @@ static int sun8i_ce_allocate_chanlist(struct sun8i_ce_dev *ce)
goto error_engine;
}
ce->chanlist[i].tl = dma_alloc_coherent(ce->dev,
- sizeof(struct ce_task),
+ sizeof(struct ce_task) * MAXTASK,
&ce->chanlist[i].t_phy,
GFP_KERNEL);
if (!ce->chanlist[i].tl) {
diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
index 49507ef2ec63..049b3175d755 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
@@ -73,6 +73,7 @@
#define CE_MAX_CLOCKS 3
#define MAXFLOW 4
+#define MAXTASK 8
/*
* struct ce_clock - Describe clocks used by sun8i-ce
--
2.24.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 8/9] crypto: sun8i-ce: split into prepare/run/unprepare
2020-01-22 10:45 [PATCH 0/9] crypto: engine: permit to handle multiple requests Corentin Labbe
` (6 preceding siblings ...)
2020-01-22 10:45 ` [PATCH 7/9] crypto: sun8i-ce: increase task list size Corentin Labbe
@ 2020-01-22 10:45 ` Corentin Labbe
2020-01-22 10:45 ` [PATCH 9/9] crypto: sun8i-ce: permit to batch requests Corentin Labbe
8 siblings, 0 replies; 15+ messages in thread
From: Corentin Labbe @ 2020-01-22 10:45 UTC (permalink / raw)
To: davem, herbert, mripard, wens, iuliana.prodan
Cc: linux-sunxi, Corentin Labbe, linux-crypto, linux-arm-kernel,
linux-kernel
This patch split the do_one_request into three.
Prepare will handle all DMA mapping and initialisation of the task
structure.
Unprepare will clean all DMA mapping.
And the do_one_request will be limited to just excuting the task.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
.../allwinner/sun8i-ce/sun8i-ce-cipher.c | 70 ++++++++++++++++---
drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h | 4 ++
2 files changed, 66 insertions(+), 8 deletions(-)
diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
index 7fd19667bceb..fc0a2299c701 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
@@ -78,8 +78,9 @@ static int sun8i_ce_cipher_fallback(struct skcipher_request *areq)
return err;
}
-static int sun8i_ce_cipher(struct skcipher_request *areq)
+static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req)
{
+ struct skcipher_request *areq = container_of(async_req, struct skcipher_request, base);
struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(areq);
struct sun8i_cipher_tfm_ctx *op = crypto_skcipher_ctx(tfm);
struct sun8i_ce_dev *ce = op->ce;
@@ -237,7 +238,9 @@ static int sun8i_ce_cipher(struct skcipher_request *areq)
}
chan->timeout = areq->cryptlen;
- err = sun8i_ce_run_task(ce, flow, crypto_tfm_alg_name(areq->base.tfm));
+ rctx->nr_sgs = nr_sgs;
+ rctx->nr_sgd = nr_sgd;
+ return 0;
theend_sgs:
if (areq->src == areq->dst) {
@@ -271,13 +274,64 @@ static int sun8i_ce_cipher(struct skcipher_request *areq)
return err;
}
-static int sun8i_ce_handle_cipher_request(struct crypto_engine *engine, void *areq)
+int sun8i_ce_cipher_run(struct crypto_engine *engine, void *areq)
{
- int err;
struct skcipher_request *breq = container_of(areq, struct skcipher_request, base);
+ struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(breq);
+ struct sun8i_cipher_tfm_ctx *op = crypto_skcipher_ctx(tfm);
+ struct sun8i_ce_dev *ce = op->ce;
+ struct sun8i_cipher_req_ctx *rctx = skcipher_request_ctx(breq);
+ int flow, err;
- err = sun8i_ce_cipher(breq);
+ flow = rctx->flow;
+ err = sun8i_ce_run_task(ce, flow, crypto_tfm_alg_name(breq->base.tfm));
crypto_finalize_skcipher_request(engine, breq, err);
+ return 0;
+}
+
+static int sun8i_ce_cipher_unprepare(struct crypto_engine *engine, void *async_req)
+{
+ struct skcipher_request *areq = container_of(async_req, struct skcipher_request, base);
+ struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(areq);
+ struct sun8i_cipher_tfm_ctx *op = crypto_skcipher_ctx(tfm);
+ struct sun8i_ce_dev *ce = op->ce;
+ struct sun8i_cipher_req_ctx *rctx = skcipher_request_ctx(areq);
+ struct sun8i_ce_flow *chan;
+ struct ce_task *cet;
+ unsigned int ivsize, offset;
+ int nr_sgs = rctx->nr_sgs;
+ int nr_sgd = rctx->nr_sgd;
+ int flow;
+
+ flow = rctx->flow;
+ chan = &ce->chanlist[flow];
+ cet = chan->tl;
+ ivsize = crypto_skcipher_ivsize(tfm);
+
+ if (areq->src == areq->dst) {
+ dma_unmap_sg(ce->dev, areq->src, nr_sgs, DMA_BIDIRECTIONAL);
+ } else {
+ if (nr_sgs > 0)
+ dma_unmap_sg(ce->dev, areq->src, nr_sgs, DMA_TO_DEVICE);
+ dma_unmap_sg(ce->dev, areq->dst, nr_sgd, DMA_FROM_DEVICE);
+ }
+
+ if (areq->iv && ivsize > 0) {
+ if (cet->t_iv)
+ dma_unmap_single(ce->dev, cet->t_iv, rctx->ivlen,
+ DMA_TO_DEVICE);
+ offset = areq->cryptlen - ivsize;
+ if (rctx->op_dir & CE_DECRYPTION) {
+ memcpy(areq->iv, rctx->backup_iv, ivsize);
+ kzfree(rctx->backup_iv);
+ } else {
+ scatterwalk_map_and_copy(areq->iv, areq->dst, offset,
+ ivsize, 0);
+ }
+ kfree(rctx->bounce_iv);
+ }
+
+ dma_unmap_single(ce->dev, cet->t_key, op->keylen, DMA_TO_DEVICE);
return 0;
}
@@ -347,9 +401,9 @@ int sun8i_ce_cipher_init(struct crypto_tfm *tfm)
crypto_tfm_alg_driver_name(&sktfm->base),
crypto_tfm_alg_driver_name(crypto_skcipher_tfm(&op->fallback_tfm->base)));
- op->enginectx.op.do_one_request = sun8i_ce_handle_cipher_request;
- op->enginectx.op.prepare_request = NULL;
- op->enginectx.op.unprepare_request = NULL;
+ op->enginectx.op.do_one_request = sun8i_ce_cipher_run;
+ op->enginectx.op.prepare_request = sun8i_ce_cipher_prepare;
+ op->enginectx.op.unprepare_request = sun8i_ce_cipher_unprepare;
err = pm_runtime_get_sync(op->ce->dev);
if (err < 0)
diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
index 049b3175d755..2d3325a13bf1 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
@@ -183,6 +183,8 @@ struct sun8i_ce_dev {
* @backup_iv: buffer which contain the next IV to store
* @bounce_iv: buffer which contain a copy of IV
* @ivlen: size of bounce_iv
+ * @nr_sgs: The number of source SG (as given by dma_map_sg())
+ * @nr_sgd: The number of destination SG (as given by dma_map_sg())
*/
struct sun8i_cipher_req_ctx {
u32 op_dir;
@@ -190,6 +192,8 @@ struct sun8i_cipher_req_ctx {
void *backup_iv;
void *bounce_iv;
unsigned int ivlen;
+ int nr_sgs;
+ int nr_sgd;
};
/*
--
2.24.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 9/9] crypto: sun8i-ce: permit to batch requests
2020-01-22 10:45 [PATCH 0/9] crypto: engine: permit to handle multiple requests Corentin Labbe
` (7 preceding siblings ...)
2020-01-22 10:45 ` [PATCH 8/9] crypto: sun8i-ce: split into prepare/run/unprepare Corentin Labbe
@ 2020-01-22 10:45 ` Corentin Labbe
8 siblings, 0 replies; 15+ messages in thread
From: Corentin Labbe @ 2020-01-22 10:45 UTC (permalink / raw)
To: davem, herbert, mripard, wens, iuliana.prodan
Cc: linux-sunxi, Corentin Labbe, linux-crypto, linux-arm-kernel,
linux-kernel
This patch permit to batch request.
This imply:
- linking two task via next
- set interrupt flag just before running the batch in the last task.
- storing all requests for finalizing them later
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
.../allwinner/sun8i-ce/sun8i-ce-cipher.c | 60 +++++++++++++++----
.../crypto/allwinner/sun8i-ce/sun8i-ce-core.c | 15 +++--
drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h | 6 ++
3 files changed, 66 insertions(+), 15 deletions(-)
diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
index fc0a2299c701..832fb4a51da9 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
@@ -96,31 +96,38 @@ static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req
int flow, i;
int nr_sgs = 0;
int nr_sgd = 0;
+ int slot = 0;
int err = 0;
algt = container_of(alg, struct sun8i_ce_alg_template, alg.skcipher);
- dev_dbg(ce->dev, "%s %s %u %x IV(%p %u) key=%u\n", __func__,
+ dev_dbg(ce->dev, "%s %s %u %x IV(%p %u) key=%u slot=%d\n", __func__,
crypto_tfm_alg_name(areq->base.tfm),
areq->cryptlen,
rctx->op_dir, areq->iv, crypto_skcipher_ivsize(tfm),
- op->keylen);
-
-#ifdef CONFIG_CRYPTO_DEV_SUN8I_CE_DEBUG
- algt->stat_req++;
-#endif
+ op->keylen, slot);
flow = rctx->flow;
chan = &ce->chanlist[flow];
+ slot = chan->ct;
+
+#ifdef CONFIG_CRYPTO_DEV_SUN8I_CE_DEBUG
+ algt->stat_req++;
+ if (chan->ct + 1 > chan->tmax)
+ chan->tmax = chan->ct + 1;
+#endif
- cet = chan->tl;
+ cet = &chan->tl[slot];
memset(cet, 0, sizeof(struct ce_task));
cet->t_id = cpu_to_le32(flow);
common = ce->variant->alg_cipher[algt->ce_algo_id];
- common |= rctx->op_dir | CE_COMM_INT;
+ common |= rctx->op_dir;
cet->t_common_ctl = cpu_to_le32(common);
+ if (slot > 0)
+ chan->tl[slot - 1].next = cpu_to_le32(chan->t_phy + 176 * slot);
+
/* CTS and recent CE (H6) need length in bytes, in word otherwise */
if (ce->variant->has_t_dlen_in_bytes)
cet->t_dlen = cpu_to_le32(areq->cryptlen);
@@ -240,6 +247,9 @@ static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req
chan->timeout = areq->cryptlen;
rctx->nr_sgs = nr_sgs;
rctx->nr_sgd = nr_sgd;
+ rctx->slot = slot;
+ chan->lreq[chan->ct] = &areq->base;
+ chan->ct++;
return 0;
theend_sgs:
@@ -281,14 +291,41 @@ int sun8i_ce_cipher_run(struct crypto_engine *engine, void *areq)
struct sun8i_cipher_tfm_ctx *op = crypto_skcipher_ctx(tfm);
struct sun8i_ce_dev *ce = op->ce;
struct sun8i_cipher_req_ctx *rctx = skcipher_request_ctx(breq);
+ struct sun8i_ce_flow *chan;
int flow, err;
+ int i;
flow = rctx->flow;
+ chan = &ce->chanlist[flow];
err = sun8i_ce_run_task(ce, flow, crypto_tfm_alg_name(breq->base.tfm));
- crypto_finalize_skcipher_request(engine, breq, err);
+ for (i = 0; i < chan->ct; i++) {
+ if (!chan->lreq[i]) {
+ dev_err(ce->dev, "Missing request at slot %d\n", i);
+ continue;
+ }
+ breq = container_of(chan->lreq[i], struct skcipher_request, base);
+ crypto_finalize_skcipher_request(engine, breq, err);
+ chan->lreq[i] = NULL;
+ }
+ chan->ct = 0;
return 0;
}
+static int sun8i_ce_qmore(struct crypto_engine *engine, void *async_req)
+{
+ struct skcipher_request *areq = container_of(async_req, struct skcipher_request, base);
+ struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(areq);
+ struct sun8i_cipher_tfm_ctx *op = crypto_skcipher_ctx(tfm);
+ struct sun8i_ce_dev *ce = op->ce;
+ struct sun8i_cipher_req_ctx *rctx = skcipher_request_ctx(areq);
+ struct sun8i_ce_flow *chan;
+ int flow;
+
+ flow = rctx->flow;
+ chan = &ce->chanlist[flow];
+ return MAXTASK - chan->ct;
+}
+
static int sun8i_ce_cipher_unprepare(struct crypto_engine *engine, void *async_req)
{
struct skcipher_request *areq = container_of(async_req, struct skcipher_request, base);
@@ -301,11 +338,13 @@ static int sun8i_ce_cipher_unprepare(struct crypto_engine *engine, void *async_r
unsigned int ivsize, offset;
int nr_sgs = rctx->nr_sgs;
int nr_sgd = rctx->nr_sgd;
+ int slot = rctx->slot;
int flow;
flow = rctx->flow;
chan = &ce->chanlist[flow];
- cet = chan->tl;
+
+ cet = &chan->tl[slot];
ivsize = crypto_skcipher_ivsize(tfm);
if (areq->src == areq->dst) {
@@ -404,6 +443,7 @@ int sun8i_ce_cipher_init(struct crypto_tfm *tfm)
op->enginectx.op.do_one_request = sun8i_ce_cipher_run;
op->enginectx.op.prepare_request = sun8i_ce_cipher_prepare;
op->enginectx.op.unprepare_request = sun8i_ce_cipher_unprepare;
+ op->enginectx.op.can_queue_more = sun8i_ce_qmore;
err = pm_runtime_get_sync(op->ce->dev);
if (err < 0)
diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
index e8bf7bf31061..348d3927344b 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
@@ -104,8 +104,10 @@ int sun8i_ce_run_task(struct sun8i_ce_dev *ce, int flow, const char *name)
int err = 0;
#ifdef CONFIG_CRYPTO_DEV_SUN8I_CE_DEBUG
- ce->chanlist[flow].stat_req++;
+ ce->chanlist[flow].stat_req += ce->chanlist[flow].ct;
#endif
+ /* mark last one */
+ ce->chanlist[flow].tl[ce->chanlist[flow].ct - 1].t_common_ctl |= cpu_to_le32(CE_COMM_INT);
mutex_lock(&ce->mlock);
@@ -120,7 +122,7 @@ int sun8i_ce_run_task(struct sun8i_ce_dev *ce, int flow, const char *name)
/* Be sure all data is written before enabling the task */
wmb();
- v = 1 | (ce->chanlist[flow].tl->t_common_ctl & 0x7F) << 8;
+ v = 1 | (ce->chanlist[flow].tl[0].t_common_ctl & 0x7F) << 8;
writel(v, ce->base + CE_TLR);
mutex_unlock(&ce->mlock);
@@ -128,7 +130,7 @@ int sun8i_ce_run_task(struct sun8i_ce_dev *ce, int flow, const char *name)
msecs_to_jiffies(ce->chanlist[flow].timeout));
if (ce->chanlist[flow].status == 0) {
- dev_err(ce->dev, "DMA timeout for %s\n", name);
+ dev_err(ce->dev, "DMA timeout for %s on flow %d (batch=%d)\n", name, flow, ce->chanlist[flow].ct);
err = -EFAULT;
}
/* No need to lock for this read, the channel is locked so
@@ -285,7 +287,10 @@ static int sun8i_ce_dbgfs_read(struct seq_file *seq, void *v)
int i;
for (i = 0; i < MAXFLOW; i++)
- seq_printf(seq, "Channel %d: nreq %lu\n", i, ce->chanlist[i].stat_req);
+ seq_printf(seq, "Channel %d: nreq %lu tmax %d eqlen=%d/%d\n", i,
+ ce->chanlist[i].stat_req, ce->chanlist[i].tmax,
+ ce->chanlist[i].engine->queue.qlen,
+ ce->chanlist[i].engine->queue.max_qlen);
for (i = 0; i < ARRAY_SIZE(ce_algs); i++) {
if (!ce_algs[i].ce)
@@ -343,7 +348,7 @@ static int sun8i_ce_allocate_chanlist(struct sun8i_ce_dev *ce)
for (i = 0; i < MAXFLOW; i++) {
init_completion(&ce->chanlist[i].complete);
- ce->chanlist[i].engine = crypto_engine_alloc_init(ce->dev, true);
+ ce->chanlist[i].engine = crypto_engine_alloc_init2(ce->dev, true, MAXTASK * 2);
if (!ce->chanlist[i].engine) {
dev_err(ce->dev, "Cannot allocate engine\n");
i--;
diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
index 2d3325a13bf1..59e9985fc6c8 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
@@ -135,6 +135,7 @@ struct ce_task {
* @t_phy: Physical address of task
* @tl: pointer to the current ce_task for this flow
* @stat_req: number of request done by this flow
+ * @tmax: The maximum number of tasks done in one batch
*/
struct sun8i_ce_flow {
struct crypto_engine *engine;
@@ -143,8 +144,11 @@ struct sun8i_ce_flow {
dma_addr_t t_phy;
int timeout;
struct ce_task *tl;
+ struct crypto_async_request *lreq[MAXTASK];
+ int ct;
#ifdef CONFIG_CRYPTO_DEV_SUN8I_CE_DEBUG
unsigned long stat_req;
+ int tmax;
#endif
};
@@ -185,6 +189,7 @@ struct sun8i_ce_dev {
* @ivlen: size of bounce_iv
* @nr_sgs: The number of source SG (as given by dma_map_sg())
* @nr_sgd: The number of destination SG (as given by dma_map_sg())
+ * @slot: The slot in the tasklist used for this requests
*/
struct sun8i_cipher_req_ctx {
u32 op_dir;
@@ -194,6 +199,7 @@ struct sun8i_cipher_req_ctx {
unsigned int ivlen;
int nr_sgs;
int nr_sgd;
+ int slot;
};
/*
--
2.24.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread