Linux cryptographic layer development
 help / color / mirror / Atom feed
* [PATCH RFC] crypto: caam - never share the gcm and skcipher descriptors serially
@ 2026-11-08 23:42 Vincent Jardin via B4 Relay
  2026-11-08 23:55 ` sashiko-bot
  2026-11-12 10:14 ` Sahil Malhotra
  0 siblings, 2 replies; 4+ messages in thread
From: Vincent Jardin via B4 Relay @ 2026-11-08 23:42 UTC (permalink / raw)
  To: Horia Geantă, Pankaj Gupta, Sahil Malhotra, Herbert Xu,
	David S. Miller
  Cc: linux-crypto, linux-kernel, Vincent Jardin

From: Vincent Jardin <vjardin@free.fr>

The gcm and skcipher shared descriptors were built with HDR_SHARE_SERIAL,
so the SEC runs the jobs of one key one after the other:
it leads to about 4 Gbps per key on LX2160A, whatever the number of
CPUs or queues. No hardware reason for this code has been found.

From benchmarks, when built with HDR_SHARE_NEVER, the performance
is much better and scales: for instance with one CPU, AES-128-GCM,
the gains are:
  - 325 -> 592 MB/s at 1420 B
  - 498 -> 6283 MB/s at 16 KiB

Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 drivers/crypto/caam/caamalg_desc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/caam/caamalg_desc.c b/drivers/crypto/caam/caamalg_desc.c
index 04c1105eb1f5..32d614a3431d 100644
--- a/drivers/crypto/caam/caamalg_desc.c
+++ b/drivers/crypto/caam/caamalg_desc.c
@@ -639,7 +639,7 @@ void cnstr_shdsc_gcm_encap(u32 * const desc, struct alginfo *cdata,
 	u32 *key_jump_cmd, *zero_payload_jump_cmd, *zero_assoc_jump_cmd1,
 	    *zero_assoc_jump_cmd2;
 
-	init_sh_desc(desc, HDR_SHARE_SERIAL);
+	init_sh_desc(desc, HDR_SHARE_NEVER);
 
 	/* skip key loading if they are loaded due to sharing */
 	key_jump_cmd = append_jump(desc, JUMP_JSL | JUMP_TEST_ALL |
@@ -765,7 +765,7 @@ void cnstr_shdsc_gcm_decap(u32 * const desc, struct alginfo *cdata,
 {
 	u32 *key_jump_cmd, *zero_payload_jump_cmd, *zero_assoc_jump_cmd1;
 
-	init_sh_desc(desc, HDR_SHARE_SERIAL);
+	init_sh_desc(desc, HDR_SHARE_NEVER);
 
 	/* skip key loading if they are loaded due to sharing */
 	key_jump_cmd = append_jump(desc, JUMP_JSL |
@@ -1463,7 +1463,7 @@ void cnstr_shdsc_skcipher_encap(u32 * const desc, struct alginfo *cdata,
 	bool is_chacha20 = ((cdata->algtype & OP_ALG_ALGSEL_MASK) ==
 			    OP_ALG_ALGSEL_CHACHA20);
 
-	init_sh_desc(desc, HDR_SHARE_SERIAL | HDR_SAVECTX);
+	init_sh_desc(desc, HDR_SHARE_NEVER | HDR_SAVECTX);
 	/* Skip if already shared */
 	key_jump_cmd = append_jump(desc, JUMP_JSL | JUMP_TEST_ALL |
 				   JUMP_COND_SHRD);
@@ -1539,7 +1539,7 @@ void cnstr_shdsc_skcipher_decap(u32 * const desc, struct alginfo *cdata,
 	bool is_chacha20 = ((cdata->algtype & OP_ALG_ALGSEL_MASK) ==
 			    OP_ALG_ALGSEL_CHACHA20);
 
-	init_sh_desc(desc, HDR_SHARE_SERIAL | HDR_SAVECTX);
+	init_sh_desc(desc, HDR_SHARE_NEVER | HDR_SAVECTX);
 	/* Skip if already shared */
 	key_jump_cmd = append_jump(desc, JUMP_JSL | JUMP_TEST_ALL |
 				   JUMP_COND_SHRD);

---
base-commit: 82e91fe19fb838f235f7c8829afc1eb389c93bdf
change-id: 20261108-for-upstream-caam-shdsc-share-never-v1-e6f7f5e0ad23

Best regards,
-- 
Vincent Jardin <vjardin@free.fr>



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

* Re: [PATCH RFC] crypto: caam - never share the gcm and skcipher descriptors serially
  2026-11-08 23:42 [PATCH RFC] crypto: caam - never share the gcm and skcipher descriptors serially Vincent Jardin via B4 Relay
@ 2026-11-08 23:55 ` sashiko-bot
  2026-11-09  9:24   ` Vincent Jardin
  2026-11-12 10:14 ` Sahil Malhotra
  1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-11-08 23:55 UTC (permalink / raw)
  To: Vincent Jardin, Horia Geantă, Pankaj Gupta, Sahil Malhotra,
	Herbert Xu, David S. Miller
  Cc: linux-crypto

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] crypto: caam: dead sharing jumps and HDR_SAVECTX with
  HDR_SHARE_NEVER
--

commit 9b09bbe924ba4f39f4ea4b61b1bb908c38315173
Author: Vincent Jardin <vjardin@free.fr>

crypto: caam - never share the gcm and skcipher descriptors serially

This commit changes the GCM and skcipher shared descriptors from
HDR_SHARE_SERIAL to HDR_SHARE_NEVER so jobs using the same key are not
serialized by the SEC hardware.

> diff --git a/drivers/crypto/caam/caamalg_desc.c b/drivers/crypto/caam/caamalg_desc.c
> index 04c1105eb1f58..32d614a3431da 100644
> --- a/drivers/crypto/caam/caamalg_desc.c
> +++ b/drivers/crypto/caam/caamalg_desc.c
> @@ -639,7 +639,7 @@ void cnstr_shdsc_gcm_encap(u32 * const desc, struct alginfo *cdata,
>  	u32 *key_jump_cmd, *zero_payload_jump_cmd, *zero_assoc_jump_cmd1,
>  	    *zero_assoc_jump_cmd2;
>  
> -	init_sh_desc(desc, HDR_SHARE_SERIAL);
> +	init_sh_desc(desc, HDR_SHARE_NEVER);
>  
>  	/* skip key loading if they are loaded due to sharing */
>  	key_jump_cmd = append_jump(desc, JUMP_JSL | JUMP_TEST_ALL |
> @@ -765,7 +765,7 @@ void cnstr_shdsc_gcm_decap(u32 * const desc, struct alginfo *cdata,
>  {
>  	u32 *key_jump_cmd, *zero_payload_jump_cmd, *zero_assoc_jump_cmd1;
>  
> -	init_sh_desc(desc, HDR_SHARE_SERIAL);
> +	init_sh_desc(desc, HDR_SHARE_NEVER);
>  
>  	/* skip key loading if they are loaded due to sharing */
>  	key_jump_cmd = append_jump(desc, JUMP_JSL |
> @@ -1463,7 +1463,7 @@ void cnstr_shdsc_skcipher_encap(u32 * const desc, struct alginfo *cdata,
>  	bool is_chacha20 = ((cdata->algtype & OP_ALG_ALGSEL_MASK) ==
>  			    OP_ALG_ALGSEL_CHACHA20);
>  
> -	init_sh_desc(desc, HDR_SHARE_SERIAL | HDR_SAVECTX);
> +	init_sh_desc(desc, HDR_SHARE_NEVER | HDR_SAVECTX);
>  	/* Skip if already shared */
>  	key_jump_cmd = append_jump(desc, JUMP_JSL | JUMP_TEST_ALL |
>  				   JUMP_COND_SHRD);
> @@ -1539,7 +1539,7 @@ void cnstr_shdsc_skcipher_decap(u32 * const desc, struct alginfo *cdata,
>  	bool is_chacha20 = ((cdata->algtype & OP_ALG_ALGSEL_MASK) ==
>  			    OP_ALG_ALGSEL_CHACHA20);
>  
> -	init_sh_desc(desc, HDR_SHARE_SERIAL | HDR_SAVECTX);
> +	init_sh_desc(desc, HDR_SHARE_NEVER | HDR_SAVECTX);
>  	/* Skip if already shared */
>  	key_jump_cmd = append_jump(desc, JUMP_JSL | JUMP_TEST_ALL |
>  				   JUMP_COND_SHRD);

[Severity: Low]
With HDR_SHARE_NEVER, should HDR_SAVECTX and the JUMP_COND_SHRD jumps also
be dropped from these four descriptors?

All CAAM job backends defer sharing mode to the shared descriptor header,
so HDR_SHARE_NEVER disables descriptor sharing across jobs and JUMP_COND_SHRD
is never true. Keeping key_jump_cmd and the JUMP_COND_SHRD branch in
append_dec_op1() called by cnstr_shdsc_skcipher_decap() leaves unreachable
instructions in the descriptors that waste space and DECO cycles on every
job, and HDR_SAVECTX only applies when a descriptor is self-shared.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-for-upstream-caam-shdsc-share-never-v1-v1-1-65a1dd146db2@free.fr?part=1

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

* Re: [PATCH RFC] crypto: caam - never share the gcm and skcipher descriptors serially
  2026-11-08 23:55 ` sashiko-bot
@ 2026-11-09  9:24   ` Vincent Jardin
  0 siblings, 0 replies; 4+ messages in thread
From: Vincent Jardin @ 2026-11-09  9:24 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: Horia Geantă, Pankaj Gupta, Sahil Malhotra, Herbert Xu,
	David S. Miller, linux-crypto

Hi Mx Bot,

> [Severity: Low]
> With HDR_SHARE_NEVER, should HDR_SAVECTX and the JUMP_COND_SHRD jumps also
> be dropped from these four descriptors?
> 
> All CAAM job backends defer sharing mode to the shared descriptor header,
> so HDR_SHARE_NEVER disables descriptor sharing across jobs and JUMP_COND_SHRD
> is never true. Keeping key_jump_cmd and the JUMP_COND_SHRD branch in
> append_dec_op1() called by cnstr_shdsc_skcipher_decap() leaves unreachable
> instructions in the descriptors that waste space and DECO cycles on every
> job, and HDR_SAVECTX only applies when a descriptor is self-shared.


Right, both are void under HDR_SHARE_NEVER: the SHRD condition never
holds, and Save Context only acts under serial self-sharing (SEC RM, HEADER
command).

They are here on purpose for this RFC: to focus on the change under discussion.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261109-for-upstream-caam-shdsc-share-never-v1-v1-1-65a1dd146db2@free.fr?part=1

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

* Re: [PATCH RFC] crypto: caam - never share the gcm and skcipher descriptors serially
  2026-11-08 23:42 [PATCH RFC] crypto: caam - never share the gcm and skcipher descriptors serially Vincent Jardin via B4 Relay
  2026-11-08 23:55 ` sashiko-bot
@ 2026-11-12 10:14 ` Sahil Malhotra
  1 sibling, 0 replies; 4+ messages in thread
From: Sahil Malhotra @ 2026-11-12 10:14 UTC (permalink / raw)
  To: Vincent Jardin
  Cc: horia.geanta, pankaj.gupta, herbert, davem, linux-crypto,
	linux-kernel

On Mon, Nov 09, 2026 at 12:42:54AM +0100, Vincent Jardin wrote:
> The gcm and skcipher shared descriptors were built with HDR_SHARE_SERIAL,
> so the SEC runs the jobs of one key one after the other:
> it leads to about 4 Gbps per key on LX2160A, whatever the number of
> CPUs or queues. No hardware reason for this code has been found.
>
> From benchmarks, when built with HDR_SHARE_NEVER, the performance
> is much better and scales: for instance with one CPU, AES-128-GCM,
> the gains are:
>   - 325 -> 592 MB/s at 1420 B
>   - 498 -> 6283 MB/s at 16 KiB

Hi Vincent,

Thanks for the patch and the numbers. They are LX2160A only, but this
change affects every CAAM user, and most CAAM instances look very
different from LX2160A.

Some CAAM-based SoCs have a single DECO, and others only 2-3.
On a single-DECO CAAM, SERIAL sharing is the best case: consecutive
same-key jobs self-share in the one DECO and keep the class 1/2 keys
(and context, with SC). NEVER cannot add any parallelism
there. It only adds a shared descriptor refetch, a key reload, and the
AES decrypt-key derivation again. So I would expect a small regression on
those parts, worst for small packets.

So this needs to be tested on other platforms before it can go in.
If you have numbers on other SoCs with fewer DECOs, that would be
helpful. We can also run it on i.MX boards on our side, if you don't
have access to other boards.

If the regression shows up, one option is to pick the share mode at
probe time from CHA_NUM_MS[DECO_NUM]: NEVER only when there is more than
one DECO, SERIAL otherwise.

On the commit message: "No hardware reason for this code has been
found" is not quite right. SERIAL is a deliberate key-reuse trade-off
that suits few-DECO parts. Please reword the log so it describes a
trade-off.

> -	init_sh_desc(desc, HDR_SHARE_SERIAL);
> +	init_sh_desc(desc, HDR_SHARE_NEVER);

Do you have similar numbers for authenc and xts, since they use the
same kind of descriptors?

> -	init_sh_desc(desc, HDR_SHARE_SERIAL | HDR_SAVECTX);
> +	init_sh_desc(desc, HDR_SHARE_NEVER | HDR_SAVECTX);
>  	/* Skip if already shared */
>  	key_jump_cmd = append_jump(desc, JUMP_JSL | JUMP_TEST_ALL |
>  				   JUMP_COND_SHRD);

With NEVER, the JUMP_COND_SHRD key-skip and HDR_SAVECTX become dead
code (as the bot noted).

Thanks,
Sahil

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

end of thread, other threads:[~2026-11-12 10:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-08 23:42 [PATCH RFC] crypto: caam - never share the gcm and skcipher descriptors serially Vincent Jardin via B4 Relay
2026-11-08 23:55 ` sashiko-bot
2026-11-09  9:24   ` Vincent Jardin
2026-11-12 10:14 ` Sahil Malhotra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox