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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
[2]
https://lore.kernel.org/lkml/20200122144134.axqpwx65j7xysyy3@xxxxxxxxxxxxxxxxxxx/
