* [PATCH v2 0/2] crypto: engine - support for parallel and batch requests @ 2020-02-04 12:34 Iuliana Prodan 2020-02-04 12:34 ` [PATCH v2 1/2] crypto: engine - support for parallel requests Iuliana Prodan 2020-02-04 12:34 ` [PATCH v2 2/2] crypto: engine - support for batch requests Iuliana Prodan 0 siblings, 2 replies; 6+ messages in thread From: Iuliana Prodan @ 2020-02-04 12:34 UTC (permalink / raw) To: Herbert Xu, Baolin Wang, Ard Biesheuvel, Corentin Labbe, Horia Geanta, Maxime Coquelin, Alexandre Torgue, Maxime Ripard Cc: Aymen Sghaier, David S. Miller, Silvano Di Ninno, Franck Lenormand, linux-crypto, linux-kernel, linux-imx, Iuliana Prodan Added support for executing multiple, independent or not, requests for crypto engine. This is based on a callback, can_enqueue_more, which asks the driver if the hardware has free space, to enqueue a new request. If hardware supports batch requests, crypto-engine can handle this use-case through do_batch_requests callback. Since, these new features, cannot be supported by all hardware, the crypto-engine framework is backward compatible: - by using the crypto_engine_alloc_init function, to initialize crypto-engine, the new callbacks are NULL and the engine will work as before these changes; - to support only multiple requests, in parallel, the can_enqueue_more callback must be implemented in driver. On crypto_pump_requests, if can_enqueue_more callback returns true, a new request is send to hardware, until there is no space and the callback returns false. - to support batch requests, do_batch_requests callback must be implemented in driver, to execute a batch of requests. The link between the requests, is expected to be done in driver, in do_one_request(). Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com> --- Changes since V1: - changed the name of can_enqueue_hardware callback to can_enqueue_more, and the argument of this callback to crypto_engine structure (for cases when more than ore crypto-engine is used). - added a new patch with support for batch requests. Changes since V0 (RFC): - removed max_no_req and no_req, as the number of request that can be processed in parallel; - added a new callback, can_enqueue_more, to check whether the hardware can process a new request. Iuliana Prodan (2): crypto: engine - support for parallel requests crypto: engine - support for batch requests crypto/crypto_engine.c | 121 ++++++++++++++++++++++++++++++++---------------- include/crypto/engine.h | 13 ++++-- 2 files changed, 90 insertions(+), 44 deletions(-) -- 2.1.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] crypto: engine - support for parallel requests 2020-02-04 12:34 [PATCH v2 0/2] crypto: engine - support for parallel and batch requests Iuliana Prodan @ 2020-02-04 12:34 ` Iuliana Prodan 2020-02-05 19:11 ` Corentin Labbe 2020-02-04 12:34 ` [PATCH v2 2/2] crypto: engine - support for batch requests Iuliana Prodan 1 sibling, 1 reply; 6+ messages in thread From: Iuliana Prodan @ 2020-02-04 12:34 UTC (permalink / raw) To: Herbert Xu, Baolin Wang, Ard Biesheuvel, Corentin Labbe, Horia Geanta, Maxime Coquelin, Alexandre Torgue, Maxime Ripard Cc: Aymen Sghaier, David S. Miller, Silvano Di Ninno, Franck Lenormand, linux-crypto, linux-kernel, linux-imx, Iuliana Prodan Added support for executing multiple requests, in parallel, for crypto engine. A new callback is added, can_enqueue_more, which asks the driver if the hardware has free space, to enqueue a new request. The new crypto_engine_alloc_init_and_set function, initialize crypto-engine, sets the maximum size for crypto-engine software queue (not hardcoded anymore) and the can_enqueue_more callback. On crypto_pump_requests, if can_enqueue_more callback returns true, a new request is send to hardware, until there is no space and the callback returns false. Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com> --- crypto/crypto_engine.c | 106 ++++++++++++++++++++++++++++++------------------ include/crypto/engine.h | 10 +++-- 2 files changed, 72 insertions(+), 44 deletions(-) diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c index eb029ff..aba934f 100644 --- a/crypto/crypto_engine.c +++ b/crypto/crypto_engine.c @@ -22,32 +22,18 @@ * @err: error number */ static void crypto_finalize_request(struct crypto_engine *engine, - struct crypto_async_request *req, int err) + 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 (engine->cur_req_prepared && - 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; - engine->cur_req_prepared = false; - 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); kthread_queue_work(engine->kworker, &engine->pump_requests); @@ -73,10 +59,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); @@ -108,13 +90,18 @@ static void crypto_pump_requests(struct crypto_engine *engine, goto out; } +start_request: + /* If hw is busy, do not send any request */ + if (engine->can_enqueue_more && + !engine->can_enqueue_more(engine)) + goto out; + /* Get the fist request from the engine queue to handle */ backlog = crypto_get_backlog(&engine->queue); async_req = crypto_dequeue_request(&engine->queue); if (!async_req) goto out; - engine->cur_req = async_req; if (backlog) backlog->complete(backlog, -EINPROGRESS); @@ -130,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_err_2; } } @@ -141,26 +128,38 @@ 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_err_2; } - engine->cur_req_prepared = true; } if (!enginectx->op.do_one_request) { dev_err(engine->dev, "failed to do request\n"); ret = -EINVAL; - goto req_err; + goto req_err_1; } + ret = enginectx->op.do_one_request(engine, async_req); if (ret) { dev_err(engine->dev, "Failed to do one request from queue: %d\n", ret); - goto req_err; + goto req_err_1; } - return; -req_err: - crypto_finalize_request(engine, async_req, ret); - return; + goto retry; + +req_err_1: + 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_err_2: + async_req->complete(async_req, ret); +retry: + if (engine->can_enqueue_more) { + spin_lock_irqsave(&engine->queue_lock, flags); + goto start_request; + } + return; out: spin_unlock_irqrestore(&engine->queue_lock, flags); } @@ -386,15 +385,26 @@ int crypto_engine_stop(struct crypto_engine *engine) EXPORT_SYMBOL_GPL(crypto_engine_stop); /** - * crypto_engine_alloc_init - allocate crypto hardware engine structure and - * initialize it. + * crypto_engine_alloc_init_and_set - allocate crypto hardware engine structure + * and initialize it by setting the maximum number of entries in the software + * crypto-engine queue. * @dev: the device attached with one hardware engine + * @cbk_can_enq: pointer to a callback function to be invoked when pumping + * requests to check whether the hardware can process a new + * request. + * This has the form: + * callback(struct crypto_engine *engine) + * where: + * @engine: the crypto engine structure. * @rt: whether this queue is set to run as a realtime task + * @qlen: maximum size of the crypto-engine 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_init_and_set(struct device *dev, + bool (*cbk_can_enq)(struct crypto_engine *engine), + bool rt, int qlen) { struct sched_param param = { .sched_priority = MAX_RT_PRIO / 2 }; struct crypto_engine *engine; @@ -411,12 +421,12 @@ struct crypto_engine *crypto_engine_alloc_init(struct device *dev, bool rt) engine->running = false; engine->busy = false; engine->idling = false; - engine->cur_req_prepared = false; engine->priv_data = dev; + engine->can_enqueue_more = cbk_can_enq; 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); @@ -433,6 +443,22 @@ struct crypto_engine *crypto_engine_alloc_init(struct device *dev, bool rt) return engine; } +EXPORT_SYMBOL_GPL(crypto_engine_alloc_init_and_set); + +/** + * 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_init_and_set(dev, NULL, 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 e29cd67..07c3f80 100644 --- a/include/crypto/engine.h +++ b/include/crypto/engine.h @@ -24,7 +24,6 @@ * @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 * @list: link with the global crypto engine list * @queue_lock: spinlock to syncronise access to request queue * @queue: the crypto queue of the engine @@ -35,17 +34,17 @@ * @unprepare_crypt_hardware: there are currently no more requests on the * queue so the subsystem notifies the driver that it may relax the * hardware by issuing this call + * @can_enqueue_more: callback to check whether the hardware can process + * a new request * @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]; bool idling; bool busy; bool running; - bool cur_req_prepared; struct list_head list; spinlock_t queue_lock; @@ -56,12 +55,12 @@ struct crypto_engine { int (*prepare_crypt_hardware)(struct crypto_engine *engine); int (*unprepare_crypt_hardware)(struct crypto_engine *engine); + bool (*can_enqueue_more)(struct crypto_engine *engine); struct kthread_worker *kworker; struct kthread_work pump_requests; void *priv_data; - struct crypto_async_request *cur_req; }; /* @@ -102,6 +101,9 @@ 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_init_and_set(struct device *dev, + bool (*cbk_can_enq)(struct crypto_engine *engine), + bool rt, int qlen); int crypto_engine_exit(struct crypto_engine *engine); #endif /* _CRYPTO_ENGINE_H */ -- 2.1.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] crypto: engine - support for parallel requests 2020-02-04 12:34 ` [PATCH v2 1/2] crypto: engine - support for parallel requests Iuliana Prodan @ 2020-02-05 19:11 ` Corentin Labbe 2020-02-07 11:26 ` Iuliana Prodan 0 siblings, 1 reply; 6+ messages in thread From: Corentin Labbe @ 2020-02-05 19:11 UTC (permalink / raw) To: Iuliana Prodan Cc: Herbert Xu, Baolin Wang, Ard Biesheuvel, Horia Geanta, Maxime Coquelin, Alexandre Torgue, Maxime Ripard, Aymen Sghaier, David S. Miller, Silvano Di Ninno, Franck Lenormand, linux-crypto, linux-kernel, linux-imx On Tue, Feb 04, 2020 at 02:34:19PM +0200, Iuliana Prodan wrote: > Added support for executing multiple requests, in parallel, > for crypto engine. > A new callback is added, can_enqueue_more, which asks the > driver if the hardware has free space, to enqueue a new request. > The new crypto_engine_alloc_init_and_set function, initialize > crypto-engine, sets the maximum size for crypto-engine software > queue (not hardcoded anymore) and the can_enqueue_more callback. > On crypto_pump_requests, if can_enqueue_more callback returns true, > a new request is send to hardware, until there is no space and the > callback returns false. > > Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com> > --- > crypto/crypto_engine.c | 106 ++++++++++++++++++++++++++++++------------------ > include/crypto/engine.h | 10 +++-- > 2 files changed, 72 insertions(+), 44 deletions(-) > > diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c > index eb029ff..aba934f 100644 > --- a/crypto/crypto_engine.c > +++ b/crypto/crypto_engine.c > @@ -22,32 +22,18 @@ > * @err: error number > */ > static void crypto_finalize_request(struct crypto_engine *engine, > - struct crypto_async_request *req, int err) > + 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 (engine->cur_req_prepared && > - 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; > - engine->cur_req_prepared = false; > - 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); > > kthread_queue_work(engine->kworker, &engine->pump_requests); > @@ -73,10 +59,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; > - Hello Your patch has the same problem than mine reported by Horia. If a queue has more than one request, a first crypto_pump_requests() will send a request and for drivers which do not block on do_one_request() crypto_pump_requests() will end. Then another crypto_pump_requests() will fire sending a second request while the driver does not support that. So we need to replace engine->cur_req by another locking mechanism. Perhaps the cleaner is to add a "request count" (increased when do_one_request, decreased in crypto_finalize_request) I know that the early version have that and it was removed, but I do not see any better way. Regards ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] crypto: engine - support for parallel requests 2020-02-05 19:11 ` Corentin Labbe @ 2020-02-07 11:26 ` Iuliana Prodan 2020-02-07 12:17 ` Corentin Labbe 0 siblings, 1 reply; 6+ messages in thread From: Iuliana Prodan @ 2020-02-07 11:26 UTC (permalink / raw) To: Corentin Labbe Cc: Herbert Xu, Baolin Wang, Ard Biesheuvel, Horia Geanta, Maxime Coquelin, Alexandre Torgue, Maxime Ripard, Aymen Sghaier, David S. Miller, Silvano Di Ninno, Franck Lenormand, linux-crypto, linux-kernel, dl-linux-imx On 2/5/2020 9:11 PM, Corentin Labbe wrote: > On Tue, Feb 04, 2020 at 02:34:19PM +0200, Iuliana Prodan wrote: >> Added support for executing multiple requests, in parallel, >> for crypto engine. >> A new callback is added, can_enqueue_more, which asks the >> driver if the hardware has free space, to enqueue a new request. >> The new crypto_engine_alloc_init_and_set function, initialize >> crypto-engine, sets the maximum size for crypto-engine software >> queue (not hardcoded anymore) and the can_enqueue_more callback. >> On crypto_pump_requests, if can_enqueue_more callback returns true, >> a new request is send to hardware, until there is no space and the >> callback returns false. >> >> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com> >> --- >> crypto/crypto_engine.c | 106 ++++++++++++++++++++++++++++++------------------ >> include/crypto/engine.h | 10 +++-- >> 2 files changed, 72 insertions(+), 44 deletions(-) >> >> diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c >> index eb029ff..aba934f 100644 >> --- a/crypto/crypto_engine.c >> +++ b/crypto/crypto_engine.c >> @@ -22,32 +22,18 @@ >> * @err: error number >> */ >> static void crypto_finalize_request(struct crypto_engine *engine, >> - struct crypto_async_request *req, int err) >> + 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 (engine->cur_req_prepared && >> - 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; >> - engine->cur_req_prepared = false; >> - 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); >> >> kthread_queue_work(engine->kworker, &engine->pump_requests); >> @@ -73,10 +59,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; >> - > > Hello > > Your patch has the same problem than mine reported by Horia. > If a queue has more than one request, a first crypto_pump_requests() will send a request and for drivers which do not block on do_one_request() crypto_pump_requests() will end. > Then another crypto_pump_requests() will fire sending a second request while the driver does not support that. > So we need to replace engine->cur_req by another locking mechanism. > Perhaps the cleaner is to add a "request count" (increased when do_one_request, decreased in crypto_finalize_request) > I know that the early version have that and it was removed, but I do not see any better way. > The "request count" I've change it to can_enqueue_more, so the hw can "answer" if it can enqueue or not. I'll (re)add the cur_req in crypto-engine. If the new callback, can_enqueue_more, is not implemented the crypto- engine will work as before - will send requests to hardware, one-by-one, on crypto_pump_requests, and complete it, on crypto_finalize_request, and so on. Thanks, Iulia ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] crypto: engine - support for parallel requests 2020-02-07 11:26 ` Iuliana Prodan @ 2020-02-07 12:17 ` Corentin Labbe 0 siblings, 0 replies; 6+ messages in thread From: Corentin Labbe @ 2020-02-07 12:17 UTC (permalink / raw) To: Iuliana Prodan Cc: Herbert Xu, Baolin Wang, Ard Biesheuvel, Horia Geanta, Maxime Coquelin, Alexandre Torgue, Maxime Ripard, Aymen Sghaier, David S. Miller, Silvano Di Ninno, Franck Lenormand, linux-crypto, linux-kernel, dl-linux-imx On Fri, Feb 07, 2020 at 11:26:38AM +0000, Iuliana Prodan wrote: > On 2/5/2020 9:11 PM, Corentin Labbe wrote: > > On Tue, Feb 04, 2020 at 02:34:19PM +0200, Iuliana Prodan wrote: > >> Added support for executing multiple requests, in parallel, > >> for crypto engine. > >> A new callback is added, can_enqueue_more, which asks the > >> driver if the hardware has free space, to enqueue a new request. > >> The new crypto_engine_alloc_init_and_set function, initialize > >> crypto-engine, sets the maximum size for crypto-engine software > >> queue (not hardcoded anymore) and the can_enqueue_more callback. > >> On crypto_pump_requests, if can_enqueue_more callback returns true, > >> a new request is send to hardware, until there is no space and the > >> callback returns false. > >> > >> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com> > >> --- > >> crypto/crypto_engine.c | 106 ++++++++++++++++++++++++++++++------------------ > >> include/crypto/engine.h | 10 +++-- > >> 2 files changed, 72 insertions(+), 44 deletions(-) > >> > >> diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c > >> index eb029ff..aba934f 100644 > >> --- a/crypto/crypto_engine.c > >> +++ b/crypto/crypto_engine.c > >> @@ -22,32 +22,18 @@ > >> * @err: error number > >> */ > >> static void crypto_finalize_request(struct crypto_engine *engine, > >> - struct crypto_async_request *req, int err) > >> + 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 (engine->cur_req_prepared && > >> - 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; > >> - engine->cur_req_prepared = false; > >> - 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); > >> > >> kthread_queue_work(engine->kworker, &engine->pump_requests); > >> @@ -73,10 +59,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; > >> - > > > > Hello > > > > Your patch has the same problem than mine reported by Horia. > > If a queue has more than one request, a first crypto_pump_requests() will send a request and for drivers which do not block on do_one_request() crypto_pump_requests() will end. > > Then another crypto_pump_requests() will fire sending a second request while the driver does not support that. > > > So we need to replace engine->cur_req by another locking mechanism. > > Perhaps the cleaner is to add a "request count" (increased when do_one_request, decreased in crypto_finalize_request) > > I know that the early version have that and it was removed, but I do not see any better way. > > > > The "request count" I've change it to can_enqueue_more, so the hw can > "answer" if it can enqueue or not. > > I'll (re)add the cur_req in crypto-engine. > If the new callback, can_enqueue_more, is not implemented the crypto- > engine will work as before - will send requests to hardware, one-by-one, > on crypto_pump_requests, and complete it, on crypto_finalize_request, > and so on. > But if the crypto_engine use can_enqueue_more, cur_req is a lie, so the name should be changed (or this fact need to be heavy documented on each of its occurence). ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] crypto: engine - support for batch requests 2020-02-04 12:34 [PATCH v2 0/2] crypto: engine - support for parallel and batch requests Iuliana Prodan 2020-02-04 12:34 ` [PATCH v2 1/2] crypto: engine - support for parallel requests Iuliana Prodan @ 2020-02-04 12:34 ` Iuliana Prodan 1 sibling, 0 replies; 6+ messages in thread From: Iuliana Prodan @ 2020-02-04 12:34 UTC (permalink / raw) To: Herbert Xu, Baolin Wang, Ard Biesheuvel, Corentin Labbe, Horia Geanta, Maxime Coquelin, Alexandre Torgue, Maxime Ripard Cc: Aymen Sghaier, David S. Miller, Silvano Di Ninno, Franck Lenormand, linux-crypto, linux-kernel, linux-imx, Iuliana Prodan Added support for batch requests, per crypto engine. A new callback is added, do_batch_requests, which executes a batch of requests. This has the crypto_engine structure as argument (for cases when more than one crypto-engine is used). The crypto_engine_alloc_init_and_set function, initializes crypto-engine, and also, sets the do_batch_requests callback. On crypto_pump_requests, if do_batch_requests callback is implemented in a driver, this will be executed. The link between the requests will be done in driver, in do_one_request(). Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com> --- crypto/crypto_engine.c | 17 ++++++++++++++++- include/crypto/engine.h | 3 +++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c index aba934f..378772e 100644 --- a/crypto/crypto_engine.c +++ b/crypto/crypto_engine.c @@ -162,6 +162,12 @@ static void crypto_pump_requests(struct crypto_engine *engine, return; out: spin_unlock_irqrestore(&engine->queue_lock, flags); + if (engine->do_batch_requests) { + ret = engine->do_batch_requests(engine); + if (ret) + dev_err(engine->dev, "failed to do batch requests: %d\n", + ret); + } } static void crypto_pump_work(struct kthread_work *work) @@ -396,6 +402,12 @@ EXPORT_SYMBOL_GPL(crypto_engine_stop); * callback(struct crypto_engine *engine) * where: * @engine: the crypto engine structure. + * @cbk_do_batch: pointer to a callback function to be invoked when executing a + * a batch of requests. + * This has the form: + * callback(struct crypto_engine *engine) + * where: + * @engine: the crypto engine structure. * @rt: whether this queue is set to run as a realtime task * @qlen: maximum size of the crypto-engine queue * @@ -404,6 +416,7 @@ EXPORT_SYMBOL_GPL(crypto_engine_stop); */ struct crypto_engine *crypto_engine_alloc_init_and_set(struct device *dev, bool (*cbk_can_enq)(struct crypto_engine *engine), + int (*cbk_do_batch)(struct crypto_engine *engine), bool rt, int qlen) { struct sched_param param = { .sched_priority = MAX_RT_PRIO / 2 }; @@ -423,6 +436,8 @@ struct crypto_engine *crypto_engine_alloc_init_and_set(struct device *dev, engine->idling = false; engine->priv_data = dev; engine->can_enqueue_more = cbk_can_enq; + engine->do_batch_requests = cbk_do_batch; + snprintf(engine->name, sizeof(engine->name), "%s-engine", dev_name(dev)); @@ -456,7 +471,7 @@ EXPORT_SYMBOL_GPL(crypto_engine_alloc_init_and_set); */ struct crypto_engine *crypto_engine_alloc_init(struct device *dev, bool rt) { - return crypto_engine_alloc_init_and_set(dev, NULL, rt, + return crypto_engine_alloc_init_and_set(dev, NULL, NULL, 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 07c3f80..27cddc4 100644 --- a/include/crypto/engine.h +++ b/include/crypto/engine.h @@ -34,6 +34,7 @@ * @unprepare_crypt_hardware: there are currently no more requests on the * queue so the subsystem notifies the driver that it may relax the * hardware by issuing this call + * @do_batch_requests: execute a batch of requests * @can_enqueue_more: callback to check whether the hardware can process * a new request * @kworker: kthread worker struct for request pump @@ -55,6 +56,7 @@ struct crypto_engine { int (*prepare_crypt_hardware)(struct crypto_engine *engine); int (*unprepare_crypt_hardware)(struct crypto_engine *engine); + int (*do_batch_requests)(struct crypto_engine *engine); bool (*can_enqueue_more)(struct crypto_engine *engine); struct kthread_worker *kworker; @@ -103,6 +105,7 @@ 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_init_and_set(struct device *dev, bool (*cbk_can_enq)(struct crypto_engine *engine), + int (*cbk_do_batch)(struct crypto_engine *engine), bool rt, int qlen); int crypto_engine_exit(struct crypto_engine *engine); -- 2.1.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-07 12:17 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-04 12:34 [PATCH v2 0/2] crypto: engine - support for parallel and batch requests Iuliana Prodan 2020-02-04 12:34 ` [PATCH v2 1/2] crypto: engine - support for parallel requests Iuliana Prodan 2020-02-05 19:11 ` Corentin Labbe 2020-02-07 11:26 ` Iuliana Prodan 2020-02-07 12:17 ` Corentin Labbe 2020-02-04 12:34 ` [PATCH v2 2/2] crypto: engine - support for batch requests Iuliana Prodan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.