Re: [PATCH 4/6] crypto: x86/aria - add missing vzeroupper in AVX2 code

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing `vzeroupper` in the AVX-512 optimized ARIA code (`aria-gfni-avx512-asm_64.S`), causing a significant performance regression due to AVX-SSE transition penalties. This problem wasn't introduced by this patch, but the patch fixes the exact same issue in the AVX2 implementation while missing the AVX-512 code.
--

--- Patch [4]: [PATCH 4/6] crypto: x86/aria - add missing vzeroupper in AVX2 code ---
commit 92a2e79950d33a63291c62ee57a893ce808d68d2
Author: Eric Biggers <[email protected]>

crypto: x86/aria - add missing vzeroupper in AVX2 code

This commit adds missing vzeroupper instructions to the AVX2 ARIA assembly
code. This prevents performance degradation for any later SSE code that might
be executed by avoiding transition penalties from the YMM registers being left
in a dirty state.

> diff --git a/arch/x86/crypto/aria-aesni-avx2-asm_64.S b/arch/x86/crypto/aria-aesni-avx2-asm_64.S
> index ed53d4f46bd7c..fda8cb8a99a8a 100644
> --- a/arch/x86/crypto/aria-aesni-avx2-asm_64.S
> +++ b/arch/x86/crypto/aria-aesni-avx2-asm_64.S
[ ... ]
> @@ -1428,6 +1433,7 @@ SYM_TYPED_FUNC_START(aria_aesni_avx2_gfni_ctr_crypt_32way)
>  		     %ymm8, %ymm9, %ymm10, %ymm11, %ymm12, %ymm13, %ymm14,
>  		     %ymm15, %r10);
>  
> +	vzeroupper;
>  	FRAME_END
>  	RET;
>  SYM_FUNC_END(aria_aesni_avx2_gfni_ctr_crypt_32way)

[Severity: High]
This is a pre-existing issue, but does the AVX-512 ARIA code also need the
same fix?

Looking at arch/x86/crypto/aria-gfni-avx512-asm_64.S, the AVX-512 optimized
functions like aria_gfni_avx512_encrypt_64way(),
aria_gfni_avx512_decrypt_64way(), and aria_gfni_avx512_ctr_crypt_64way()
return without executing vzeroupper:

arch/x86/crypto/aria-gfni-avx512-asm_64.S:aria_gfni_avx512_encrypt_64way() {
    ...
	FRAME_END
	RET;
SYM_FUNC_END(aria_gfni_avx512_encrypt_64way)
}

Since zmm registers are used, wouldn't returning without clearing the EVEX
dirty state cause the same performance transition penalties for subsequent
legacy SSE code?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4