linux-crypto.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] crypto: engine: remove request batching support
@ 2025-07-11 18:29 Ovidiu Panait
  2025-07-11 18:29 ` [PATCH 2/2] crypto: engine: remove {prepare,unprepare}_crypt_hardware callbacks Ovidiu Panait
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Ovidiu Panait @ 2025-07-11 18:29 UTC (permalink / raw)
  To: herbert, davem, linux-crypto, linux-kernel
  Cc: Ovidiu Panait, freude, dengler, linux-s390, horia.geanta,
	pankaj.gupta, gaurav.jain, arei.gonglei, virtualization

Remove request batching support from crypto_engine, as there are no
drivers using this feature and it doesn't really work that well.

Instead of doing batching based on backlog, a more optimal approach
would be for the user to handle the batching (similar to how IPsec
can hook into GSO to get 64K of data each time or how block encryption
can use unit sizes much greater than 4K).

Suggested-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Ovidiu Panait <ovidiu.panait.oss@gmail.com>
---
Cc: freude@linux.ibm.com
Cc: dengler@linux.ibm.com
Cc: linux-s390@vger.kernel.org
Cc: horia.geanta@nxp.com
Cc: pankaj.gupta@nxp.com
Cc: gaurav.jain@nxp.com
Cc: arei.gonglei@huawei.com
Cc: virtualization@lists.linux.dev

 arch/s390/crypto/paes_s390.c               |  2 +-
 arch/s390/crypto/phmac_s390.c              |  2 +-
 crypto/crypto_engine.c                     | 25 +---------------------
 drivers/crypto/caam/jr.c                   |  3 +--
 drivers/crypto/virtio/virtio_crypto_core.c |  2 +-
 include/crypto/engine.h                    |  1 -
 include/crypto/internal/engine.h           |  4 ----
 7 files changed, 5 insertions(+), 34 deletions(-)

diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
index 8a340c16acb4..a624a43a2b54 100644
--- a/arch/s390/crypto/paes_s390.c
+++ b/arch/s390/crypto/paes_s390.c
@@ -1633,7 +1633,7 @@ static int __init paes_s390_init(void)
 	/* with this pseudo devie alloc and start a crypto engine */
 	paes_crypto_engine =
 		crypto_engine_alloc_init_and_set(paes_dev.this_device,
-						 true, NULL, false, MAX_QLEN);
+						 true, false, MAX_QLEN);
 	if (!paes_crypto_engine) {
 		rc = -ENOMEM;
 		goto out_err;
diff --git a/arch/s390/crypto/phmac_s390.c b/arch/s390/crypto/phmac_s390.c
index 90602f72108f..7ecfdc4fba2d 100644
--- a/arch/s390/crypto/phmac_s390.c
+++ b/arch/s390/crypto/phmac_s390.c
@@ -1006,7 +1006,7 @@ static int __init s390_phmac_init(void)
 	/* with this pseudo device alloc and start a crypto engine */
 	phmac_crypto_engine =
 		crypto_engine_alloc_init_and_set(phmac_dev.this_device,
-						 true, NULL, false, MAX_QLEN);
+						 true, false, MAX_QLEN);
 	if (!phmac_crypto_engine) {
 		rc = -ENOMEM;
 		goto out_err;
diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
index 445d3c113ee1..8a2400f240d4 100644
--- a/crypto/crypto_engine.c
+++ b/crypto/crypto_engine.c
@@ -195,17 +195,6 @@ static void crypto_pump_requests(struct crypto_engine *engine,
 out:
 	spin_unlock_irqrestore(&engine->queue_lock, flags);
 
-	/*
-	 * Batch requests is possible only if
-	 * hardware can enqueue multiple requests
-	 */
-	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);
-	}
-
 	return;
 }
 
@@ -462,12 +451,6 @@ EXPORT_SYMBOL_GPL(crypto_engine_stop);
  * crypto-engine queue.
  * @dev: the device attached with one hardware engine
  * @retry_support: whether hardware has support for retry mechanism
- * @cbk_do_batch: pointer to a callback function to be invoked when executing
- *                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
  *
@@ -476,7 +459,6 @@ EXPORT_SYMBOL_GPL(crypto_engine_stop);
  */
 struct crypto_engine *crypto_engine_alloc_init_and_set(struct device *dev,
 						       bool retry_support,
-						       int (*cbk_do_batch)(struct crypto_engine *engine),
 						       bool rt, int qlen)
 {
 	struct crypto_engine *engine;
@@ -495,11 +477,6 @@ struct crypto_engine *crypto_engine_alloc_init_and_set(struct device *dev,
 	engine->idling = false;
 	engine->retry_support = retry_support;
 	engine->priv_data = dev;
-	/*
-	 * Batch requests is possible only if
-	 * hardware has support for retry mechanism.
-	 */
-	engine->do_batch_requests = retry_support ? cbk_do_batch : NULL;
 
 	snprintf(engine->name, sizeof(engine->name),
 		 "%s-engine", dev_name(dev));
@@ -534,7 +511,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, false, NULL, rt,
+	return crypto_engine_alloc_init_and_set(dev, false, rt,
 						CRYPTO_ENGINE_MAX_QLEN);
 }
 EXPORT_SYMBOL_GPL(crypto_engine_alloc_init);
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index 9fcdb64084ac..0ef00df9730e 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -629,8 +629,7 @@ static int caam_jr_probe(struct platform_device *pdev)
 	}
 
 	/* Initialize crypto engine */
-	jrpriv->engine = crypto_engine_alloc_init_and_set(jrdev, true, NULL,
-							  false,
+	jrpriv->engine = crypto_engine_alloc_init_and_set(jrdev, true, false,
 							  CRYPTO_ENGINE_MAX_QLEN);
 	if (!jrpriv->engine) {
 		dev_err(jrdev, "Could not init crypto-engine\n");
diff --git a/drivers/crypto/virtio/virtio_crypto_core.c b/drivers/crypto/virtio/virtio_crypto_core.c
index 0d522049f595..3d241446099c 100644
--- a/drivers/crypto/virtio/virtio_crypto_core.c
+++ b/drivers/crypto/virtio/virtio_crypto_core.c
@@ -139,7 +139,7 @@ static int virtcrypto_find_vqs(struct virtio_crypto *vi)
 		spin_lock_init(&vi->data_vq[i].lock);
 		vi->data_vq[i].vq = vqs[i];
 		/* Initialize crypto engine */
-		vi->data_vq[i].engine = crypto_engine_alloc_init_and_set(dev, true, NULL, true,
+		vi->data_vq[i].engine = crypto_engine_alloc_init_and_set(dev, true, true,
 						virtqueue_get_vring_size(vqs[i]));
 		if (!vi->data_vq[i].engine) {
 			ret = -ENOMEM;
diff --git a/include/crypto/engine.h b/include/crypto/engine.h
index 545dbefe3e13..2e60344437da 100644
--- a/include/crypto/engine.h
+++ b/include/crypto/engine.h
@@ -76,7 +76,6 @@ 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 retry_support,
-						       int (*cbk_do_batch)(struct crypto_engine *engine),
 						       bool rt, int qlen);
 void crypto_engine_exit(struct crypto_engine *engine);
 
diff --git a/include/crypto/internal/engine.h b/include/crypto/internal/engine.h
index b6a4ea2240fc..8da1a13619c9 100644
--- a/include/crypto/internal/engine.h
+++ b/include/crypto/internal/engine.h
@@ -37,8 +37,6 @@ struct device;
  * @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. Depends on multiple
- * requests support.
  * @kworker: kthread worker struct for request pump
  * @pump_requests: work struct for scheduling work to the request pump
  * @priv_data: the engine private data
@@ -60,8 +58,6 @@ 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);
-
 
 	struct kthread_worker           *kworker;
 	struct kthread_work             pump_requests;
-- 
2.50.0


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

* [PATCH 2/2] crypto: engine: remove {prepare,unprepare}_crypt_hardware callbacks
  2025-07-11 18:29 [PATCH 1/2] crypto: engine: remove request batching support Ovidiu Panait
@ 2025-07-11 18:29 ` Ovidiu Panait
  2025-07-15 21:38 ` [PATCH 1/2] crypto: engine: remove request batching support Horia Geanta
  2025-07-18 11:13 ` Herbert Xu
  2 siblings, 0 replies; 4+ messages in thread
From: Ovidiu Panait @ 2025-07-11 18:29 UTC (permalink / raw)
  To: herbert, davem, linux-crypto, linux-kernel; +Cc: Ovidiu Panait

The {prepare,unprepare}_crypt_hardware callbacks were added back in 2016
by commit 735d37b5424b ("crypto: engine - Introduce the block request
crypto engine framework"), but they were never implemented by any driver.
Remove them as they are unused.

Since the 'engine->idling' and 'was_busy' flags are no longer needed,
remove them as well.

Signed-off-by: Ovidiu Panait <ovidiu.panait.oss@gmail.com>
---
 Documentation/crypto/crypto_engine.rst |  6 ------
 crypto/crypto_engine.c                 | 30 +-------------------------
 include/crypto/internal/engine.h       | 11 ----------
 3 files changed, 1 insertion(+), 46 deletions(-)

diff --git a/Documentation/crypto/crypto_engine.rst b/Documentation/crypto/crypto_engine.rst
index d562ea17d994..7ef850e28016 100644
--- a/Documentation/crypto/crypto_engine.rst
+++ b/Documentation/crypto/crypto_engine.rst
@@ -36,12 +36,6 @@ engine using ``crypto_engine_stop()`` and destroy the engine with
 Before transferring any request, you have to fill the context enginectx by
 providing functions for the following:
 
-* ``prepare_crypt_hardware``: Called once before any prepare functions are
-  called.
-
-* ``unprepare_crypt_hardware``: Called once after all unprepare functions have
-  been called.
-
 * ``prepare_cipher_request``/``prepare_hash_request``: Called before each
   corresponding request is performed. If some processing or other preparatory
   work is required, do it here.
diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
index 8a2400f240d4..18e1689efe12 100644
--- a/crypto/crypto_engine.c
+++ b/crypto/crypto_engine.c
@@ -74,7 +74,6 @@ static void crypto_pump_requests(struct crypto_engine *engine,
 	struct crypto_engine_alg *alg;
 	struct crypto_engine_op *op;
 	unsigned long flags;
-	bool was_busy = false;
 	int ret;
 
 	spin_lock_irqsave(&engine->queue_lock, flags);
@@ -83,12 +82,6 @@ static void crypto_pump_requests(struct crypto_engine *engine,
 	if (!engine->retry_support && 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)
@@ -102,15 +95,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 &&
-		    engine->unprepare_crypt_hardware(engine))
-			dev_err(engine->dev, "failed to unprepare crypt hardware\n");
-
-		spin_lock_irqsave(&engine->queue_lock, flags);
-		engine->idling = false;
 		goto out;
 	}
 
@@ -129,22 +113,11 @@ static void crypto_pump_requests(struct crypto_engine *engine,
 	if (!engine->retry_support)
 		engine->cur_req = async_req;
 
-	if (engine->busy)
-		was_busy = true;
-	else
+	if (!engine->busy)
 		engine->busy = true;
 
 	spin_unlock_irqrestore(&engine->queue_lock, flags);
 
-	/* Until here we get the request need to be encrypted successfully */
-	if (!was_busy && engine->prepare_crypt_hardware) {
-		ret = engine->prepare_crypt_hardware(engine);
-		if (ret) {
-			dev_err(engine->dev, "failed to prepare crypt hardware\n");
-			goto req_err_1;
-		}
-	}
-
 	alg = container_of(async_req->tfm->__crt_alg,
 			   struct crypto_engine_alg, base);
 	op = &alg->op;
@@ -474,7 +447,6 @@ struct crypto_engine *crypto_engine_alloc_init_and_set(struct device *dev,
 	engine->rt = rt;
 	engine->running = false;
 	engine->busy = false;
-	engine->idling = false;
 	engine->retry_support = retry_support;
 	engine->priv_data = dev;
 
diff --git a/include/crypto/internal/engine.h b/include/crypto/internal/engine.h
index 8da1a13619c9..f19ef376833f 100644
--- a/include/crypto/internal/engine.h
+++ b/include/crypto/internal/engine.h
@@ -21,7 +21,6 @@ struct device;
 /*
  * 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
  * @retry_support: indication that the hardware allows re-execution
@@ -31,12 +30,6 @@ struct device;
  * @list: link with the global crypto engine list
  * @queue_lock: spinlock to synchronise access to request queue
  * @queue: the crypto queue of the engine
- * @prepare_crypt_hardware: a request will soon arrive from the queue
- * so the subsystem requests the driver to prepare the hardware
- * by issuing this call
- * @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
  * @kworker: kthread worker struct for request pump
  * @pump_requests: work struct for scheduling work to the request pump
  * @priv_data: the engine private data
@@ -44,7 +37,6 @@ struct device;
  */
 struct crypto_engine {
 	char			name[ENGINE_NAME_LEN];
-	bool			idling;
 	bool			busy;
 	bool			running;
 
@@ -56,9 +48,6 @@ struct crypto_engine {
 	struct crypto_queue	queue;
 	struct device		*dev;
 
-	int (*prepare_crypt_hardware)(struct crypto_engine *engine);
-	int (*unprepare_crypt_hardware)(struct crypto_engine *engine);
-
 	struct kthread_worker           *kworker;
 	struct kthread_work             pump_requests;
 
-- 
2.50.0


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

* Re: [PATCH 1/2] crypto: engine: remove request batching support
  2025-07-11 18:29 [PATCH 1/2] crypto: engine: remove request batching support Ovidiu Panait
  2025-07-11 18:29 ` [PATCH 2/2] crypto: engine: remove {prepare,unprepare}_crypt_hardware callbacks Ovidiu Panait
@ 2025-07-15 21:38 ` Horia Geanta
  2025-07-18 11:13 ` Herbert Xu
  2 siblings, 0 replies; 4+ messages in thread
From: Horia Geanta @ 2025-07-15 21:38 UTC (permalink / raw)
  To: Ovidiu Panait, herbert@gondor.apana.org.au, davem@davemloft.net,
	linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: freude@linux.ibm.com, dengler@linux.ibm.com,
	linux-s390@vger.kernel.org, Pankaj Gupta, Gaurav Jain,
	arei.gonglei@huawei.com, virtualization@lists.linux.dev

On 7/11/2025 9:30 PM, Ovidiu Panait wrote:
> Remove request batching support from crypto_engine, as there are no
> drivers using this feature and it doesn't really work that well.
> 
> Instead of doing batching based on backlog, a more optimal approach
> would be for the user to handle the batching (similar to how IPsec
> can hook into GSO to get 64K of data each time or how block encryption
> can use unit sizes much greater than 4K).
> 
> Suggested-by: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: Ovidiu Panait <ovidiu.panait.oss@gmail.com>
Reviewed-by: Horia Geantă <horia.geanta@nxp.com>

Thanks,
Horia


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

* Re: [PATCH 1/2] crypto: engine: remove request batching support
  2025-07-11 18:29 [PATCH 1/2] crypto: engine: remove request batching support Ovidiu Panait
  2025-07-11 18:29 ` [PATCH 2/2] crypto: engine: remove {prepare,unprepare}_crypt_hardware callbacks Ovidiu Panait
  2025-07-15 21:38 ` [PATCH 1/2] crypto: engine: remove request batching support Horia Geanta
@ 2025-07-18 11:13 ` Herbert Xu
  2 siblings, 0 replies; 4+ messages in thread
From: Herbert Xu @ 2025-07-18 11:13 UTC (permalink / raw)
  To: Ovidiu Panait
  Cc: davem, linux-crypto, linux-kernel, freude, dengler, linux-s390,
	horia.geanta, pankaj.gupta, gaurav.jain, arei.gonglei,
	virtualization

On Fri, Jul 11, 2025 at 09:29:31PM +0300, Ovidiu Panait wrote:
> Remove request batching support from crypto_engine, as there are no
> drivers using this feature and it doesn't really work that well.
> 
> Instead of doing batching based on backlog, a more optimal approach
> would be for the user to handle the batching (similar to how IPsec
> can hook into GSO to get 64K of data each time or how block encryption
> can use unit sizes much greater than 4K).
> 
> Suggested-by: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: Ovidiu Panait <ovidiu.panait.oss@gmail.com>
> ---
> Cc: freude@linux.ibm.com
> Cc: dengler@linux.ibm.com
> Cc: linux-s390@vger.kernel.org
> Cc: horia.geanta@nxp.com
> Cc: pankaj.gupta@nxp.com
> Cc: gaurav.jain@nxp.com
> Cc: arei.gonglei@huawei.com
> Cc: virtualization@lists.linux.dev
> 
>  arch/s390/crypto/paes_s390.c               |  2 +-
>  arch/s390/crypto/phmac_s390.c              |  2 +-
>  crypto/crypto_engine.c                     | 25 +---------------------
>  drivers/crypto/caam/jr.c                   |  3 +--
>  drivers/crypto/virtio/virtio_crypto_core.c |  2 +-
>  include/crypto/engine.h                    |  1 -
>  include/crypto/internal/engine.h           |  4 ----
>  7 files changed, 5 insertions(+), 34 deletions(-)

All applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

end of thread, other threads:[~2025-07-18 11:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-11 18:29 [PATCH 1/2] crypto: engine: remove request batching support Ovidiu Panait
2025-07-11 18:29 ` [PATCH 2/2] crypto: engine: remove {prepare,unprepare}_crypt_hardware callbacks Ovidiu Panait
2025-07-15 21:38 ` [PATCH 1/2] crypto: engine: remove request batching support Horia Geanta
2025-07-18 11:13 ` Herbert Xu

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