crypto: caam/qi2 - algorithm priority configurable

dpaa2_caam registers its skcipher, aead and ahash algorithms at a fixed priority above the ARMv8 Crypto Extensions, so on DPAA2 SoCs every in-kernel consumers of AES, SHA or GCM use the SEC.

Patch 1 adds dpaa2_caam.priority module parameter Patch 2 doc fsl,qi2-crypto-priority property on the SEC node Patch 3 reads it at probe, the module parameter takes precedence

---
Vincent Jardin (3):
crypto: caam/qi2 - algorithm priority be a parameter
dt-bindings: crypto: fsl,sec-v4.0: add fsl,qi2-crypto-priority
crypto: caam/qi2 - priority from the DTS

 .../devicetree/bindings/crypto/fsl,sec-v4.0.yaml   |  9 ++++++
 drivers/crypto/caam/caamalg_qi2.c                  | 35 ++++++++++++++++++++--
 2 files changed, 41 insertions(+), 3 deletions(-)

---
base-commit: 67aefeccc4101a93627f36abbd049f191b54f903
change-id: 20260928-for-upstream-caam-qi2-priority-76301b8eb3a6

Best regards,

Vincent Jardin <[email protected]>

dpaa2_caam registers every skcipher, aead and ahash at a fixed priority of 2000.

It is above the ARMv8 Crypto Extensions (200 to 300), so on any DPAA2 SoC, every in-kernel user of AES, SHA or GCM moves from the CPU to the SEC, whatever the request size.

Whether the SEC should be the default or only be reached by driver name depends on the system, and there is no way to choose/tune it.

Add a priority parameter
dpaa2_caam.priority=
on the command line

---
 drivers/crypto/caam/caamalg_qi2.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

index 6b47bcc16a50..dbb84a6996a2 100644
@@ -28,6 +28,13 @@
 
 #define CAAM_CRA_PRIORITY	2000
 
+static unsigned int priority;
+module_param(priority, uint, 0444);
+MODULE_PARM_DESC(priority,
+		 "Crypto API priority of the algorithms (0: default, 2000)");
+
+static int cra_priority = CAAM_CRA_PRIORITY;
+
 /* max key is sum of AES_MAX_KEY_SIZE, max split key size */
 #define CAAM_MAX_KEY_SIZE	(AES_MAX_KEY_SIZE + CTR_RFC3686_NONCE_SIZE + \
 				 SHA512_DIGEST_SIZE * 2)

@@ -3014,7 +3021,7 @@ static void caam_skcipher_alg_init(struct caam_skcipher_alg *t_alg)
 	struct skcipher_alg *alg = &t_alg->skcipher;
 
 	alg->base.cra_module = THIS_MODULE;
-	alg->base.cra_priority = CAAM_CRA_PRIORITY;
+	alg->base.cra_priority = cra_priority;
 	alg->base.cra_ctxsize = sizeof(struct caam_ctx) + crypto_dma_padding();
 	alg->base.cra_flags |= (CRYPTO_ALG_ASYNC | CRYPTO_ALG_ALLOCATES_MEMORY |
 			      CRYPTO_ALG_KERN_DRIVER_ONLY);

@@ -3028,7 +3035,7 @@ static void caam_aead_alg_init(struct caam_aead_alg *t_alg)
 	struct aead_alg *alg = &t_alg->aead;
 
 	alg->base.cra_module = THIS_MODULE;
-	alg->base.cra_priority = CAAM_CRA_PRIORITY;
+	alg->base.cra_priority = cra_priority;
 	alg->base.cra_ctxsize = sizeof(struct caam_ctx) + crypto_dma_padding();
 	alg->base.cra_flags = CRYPTO_ALG_ASYNC | CRYPTO_ALG_ALLOCATES_MEMORY |
 			      CRYPTO_ALG_KERN_DRIVER_ONLY;

@@ -4659,7 +4666,7 @@ static struct caam_hash_alg *caam_hash_alloc(struct device *dev,
 	alg->cra_init = caam_hash_cra_init;
 	alg->cra_exit = caam_hash_cra_exit;
 	alg->cra_ctxsize = sizeof(struct caam_hash_ctx) + crypto_dma_padding();
-	alg->cra_priority = CAAM_CRA_PRIORITY;
+	alg->cra_priority = cra_priority;
 	alg->cra_blocksize = template->blocksize;
 	alg->cra_alignmask = 0;
 	alg->cra_flags = CRYPTO_ALG_ASYNC | CRYPTO_ALG_ALLOCATES_MEMORY;

@@ -5208,6 +5215,9 @@ static int dpaa2_caam_probe(struct fsl_mc_device *dpseci_dev)
 
 	caam_imx = false;
 
+	if (priority)
+		cra_priority = priority;
+
 	dev = &dpseci_dev->dev;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);

2.43.0

You can easily change the priority from user-space through the crypto_user interface.

So why not lower the default priority and let the user adjust it if necessary?

Cheers,

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Hi,

Yes, that's definitively the way to go. I'll move toward this option.

agree, users should drive it, it'll be more flexible.

Thanks,
Vincent

Let a board set the crypto API priority of the algorithms the DPAA2 front end (dpaa2_caam) registers.

Benefits: Its DTS decides whether the SEC or the CPU is the default for in-kernel crypto.

---
 Documentation/devicetree/bindings/crypto/fsl,sec-v4.0.yaml | 9 +++++++++
 1 file changed, 9 insertions(+)

index dcc755d2709a..0231fe3853b0 100644
@@ -86,6 +86,15 @@ properties:
     description: Defines the 'ERA' of the SEC device.
     $ref: /schemas/types.yaml#/definitions/uint32
 
+  fsl,qi2-crypto-priority:
+    description:
+      Crypto API priority of the algorithms the DPAA2 (QI2) front end
+      registers. Below the CPU's own implementations, the SEC is used only
+      when asked for by driver name. The dpaa2_caam.priority module
+      parameter overrides it.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 2000
+
 patternProperties:
   '^jr@[0-9a-f]+$':
     type: object

2.43.0

Same as previous tries for crypto drivers - not a hardware property.

You described the desired Linux feature or behavior, not the actual hardware. The bindings are about the latter, so instead you need to rephrase the property and its description to match actual hardware capabilities/features/configuration etc.

Best regards,
Krzysztof

Hi,

agree, I'll avoid it.

Vincent

Read fsl,qi2-crypto-priority from the SEC node when dpaa2_caam probes, so the board's DTS sets the priority its algorithms register with.

The dpaa2_caam.priority parameter has precedence when it is set. When they are not set, we keep the default behaviour.

---
 drivers/crypto/caam/caamalg_qi2.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

index dbb84a6996a2..7d6a5e4ee86e 100644
@@ -19,6 +19,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/fsl/mc.h>
 #include <linux/kernel.h>
+#include <linux/of.h>
 #include <linux/string.h>
 #include <linux/string_choices.h>
 #include <soc/fsl/dpaa2-io.h>

@@ -35,6 +36,25 @@ MODULE_PARM_DESC(priority,
 
 static int cra_priority = CAAM_CRA_PRIORITY;
 
+static int dpaa2_caam_cra_priority(void)
+{
+	struct device_node *np;
+	u32 prio;
+	int err;
+
+	if (priority)
+		return priority;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,sec-v4.0");
+	if (!np)
+		return CAAM_CRA_PRIORITY;
+
+	err = of_property_read_u32(np, "fsl,qi2-crypto-priority", &prio);
+	of_node_put(np);
+
+	return err ? CAAM_CRA_PRIORITY : prio;
+}
+
 /* max key is sum of AES_MAX_KEY_SIZE, max split key size */
 #define CAAM_MAX_KEY_SIZE	(AES_MAX_KEY_SIZE + CTR_RFC3686_NONCE_SIZE + \
 				 SHA512_DIGEST_SIZE * 2)

@@ -5215,8 +5235,7 @@ static int dpaa2_caam_probe(struct fsl_mc_device *dpseci_dev)
 
 	caam_imx = false;
 
-	if (priority)
-		cra_priority = priority;
+	cra_priority = dpaa2_caam_cra_priority();
 
 	dev = &dpseci_dev->dev;
 

2.43.0

Is there *any* real-world use case in which these caamalg_qi2.c algorithms are worth using? This looks like another one of those problematic drivers pushed by the hardware vendor as a checkbox feature. Just doing the crypto on the CPU is almost always much faster and more reliable.

As shown by your other patch
(https://lore.kernel.org/linux-crypto/20260928-for-upstream-caam-qi-plain-keylen-v1-1-6edb56649cf9@free.fr/)
it also seems that this driver has been critically broken for the last year, with it being unable to set keys. Evidently, no one has tested or used it in the last year until now.

It also has the usual anti-patterns like supporting MD5 and DES.

I really don't see the point. Why do people put themselves through these issues at all? It seems this functionality should just be disabled everywhere, without putting policy in the device tree which as has been noted many times isn't the right place for it.

- Eric

Hi Eric,

It is not black and white, and I am working on some optimizations. Today, with one key, one A72 core with the Crypto Extensions is about 2x better than the SEC, at every buffer size. However, that core is then at its limit: 6 to 9 Gbps of AES-128-GCM, fully busy.

With many keys and many buffers in flight, it changes. With 16 KiB buffers, 16 keys and some WIP fixes (MC firmware configuration mosty, maybe few kernel fixes), one A72 core feeding the SEC reaches 47.1 Gbps, while the same core does 9.3 Gbps with the Crypto Extensions. At 4 KiB it is 14.4 against 8.5 Gbps. At low network packet size (WIP about 1400 octet) the A72 wins.

For single flows, the SEC should not be used: with the current kernel code, one key cannot go past about 4 Gbps.

So, even if it is tempting to drop caamalg_qi2.c, I believe some users still have a use for it. Note: I only focused on AES-128-GCM.

Agreed. CONFIG_CRYPTO_SELFTESTS caught it on the first boot.

I cannot argue with that, but I would rather not be the one who drops them from caamalg_qi2.c.

Definitely, the device tree is not the right place, I get the point. v2 will move away from it.

My first goal is to get these features working again on the LX2160A, and to be able to select the crypto backend.

Best regards,
Vincent