Re: [PATCH RFC] crypto: caam - never share the gcm and skcipher descriptors serially

[sashiko-bot@kernel.org]

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