crypto: x86 - stop using the SIMD helper
Subsystems (2)
v2 cover crypto: x86 - stop using the SIMD helper
Patches 2-9 are almost identical to
https://lore.kernel.org/r/[email protected]/
but now split into multiple patches. Patch 1 is just a resend of
https://lore.kernel.org/r/[email protected]/
which is needed for the series to apply cleanly but is otherwise unrelated. Description of patches 2-9 follows:
Stop wrapping skcipher and aead algorithms with the crypto SIMD helper (crypto/simd.c). The only purpose of doing so was to work around x86 not always supporting kernel-mode FPU in softirqs. Specifically, if a hardirq interrupted a task context kernel-mode FPU section and then a softirqs were run at the end of that hardirq, those softirqs could not use kernel-mode FPU. This has now been fixed. In combination with the fact that the skcipher and aead APIs only support task and softirq contexts, these can now just use kernel-mode FPU unconditionally on x86.
This simplifies the code and improves performance.
En/decryption gets at least somewhat faster for everyone, since the crypto API functions such as crypto_skcipher_encrypt() now go directly to the underlying algorithm rather than taking a detour through crypto/simd.c which involved an extra indirect call. For example, on a Ryzen 9 9950X desktop processor, AES-256-XTS is now 23% faster for 512-byte messages and 7% faster for 4096-byte messages (when accessed through crypto_skcipher_encrypt() or crypto_skcipher_decrypt()).
There's also a much larger performance improvement for crypto API users that only support synchronous algorithms. These users will now actually use the x86 SIMD (e.g. AES-NI or VAES) optimized en/decryption modes, which they couldn't before because they were marked as asynchronous.
Eric Biggers (9):
crypto: x86/aes - drop the avx10_256 AES-XTS and AES-CTR code
crypto: x86/aegis - stop using the SIMD helper
crypto: x86/aes - stop using the SIMD helper
crypto: x86/aria - stop using the SIMD helper
crypto: x86/camellia - stop using the SIMD helper
crypto: x86/cast - stop using the SIMD helper
crypto: x86/serpent - stop using the SIMD helper
crypto: x86/sm4 - stop using the SIMD helper
crypto: x86/twofish - stop using the SIMD helper
arch/x86/crypto/Kconfig | 14 -- arch/x86/crypto/aegis128-aesni-glue.c | 13 +- arch/x86/crypto/aes-ctr-avx-x86_64.S | 47 ++---- arch/x86/crypto/aes-xts-avx-x86_64.S | 118 ++++++-------- arch/x86/crypto/aesni-intel_glue.c | 174 ++++++++------------- arch/x86/crypto/aria_aesni_avx2_glue.c | 22 +-- arch/x86/crypto/aria_aesni_avx_glue.c | 20 +-- arch/x86/crypto/aria_gfni_avx512_glue.c | 22 +-- arch/x86/crypto/camellia_aesni_avx2_glue.c | 21 +-- arch/x86/crypto/camellia_aesni_avx_glue.c | 21 +-- arch/x86/crypto/cast5_avx_glue.c | 21 +-- arch/x86/crypto/cast6_avx_glue.c | 20 +-- arch/x86/crypto/serpent_avx2_glue.c | 21 +-- arch/x86/crypto/serpent_avx_glue.c | 21 +-- arch/x86/crypto/serpent_sse2_glue.c | 21 +-- arch/x86/crypto/sm4_aesni_avx2_glue.c | 31 ++-- arch/x86/crypto/sm4_aesni_avx_glue.c | 31 ++-- arch/x86/crypto/twofish_avx_glue.c | 21 +-- 18 files changed, 227 insertions(+), 432 deletions(-)
base-commit: 91e5bfe317d8f8471fbaa3e70cf66cae1314a516
Signature · 1 line
2.49.0
PATCH v2 1/9 crypto: x86/aes - drop the avx10_256 AES-XTS and AES-CTR code
From: Eric Biggers <[email protected]>
Intel made a late change to the AVX10 specification that removes support for a 256-bit maximum vector length and enumeration of the maximum vector length. AVX10 will imply a maximum vector length of 512 bits. I.e. there won't be any such thing as AVX10/256 or AVX10/512; there will just be AVX10, and it will essentially just consolidate AVX512 features.
As a result of this new development, my strategy of providing both *_avx10_256 and *_avx10_512 functions didn't turn out to be that useful. The only remaining motivation for the 256-bit AVX512 / AVX10 functions is to avoid downclocking on older Intel CPUs. But in the case of AES-XTS and AES-CTR, I already wrote *_avx2 code too (primarily to support CPUs without AVX512), which performs almost as well as *_avx10_256. So we should just use that.
Therefore, remove the *_avx10_256 AES-XTS and AES-CTR functions and algorithms, and rename the *_avx10_512 AES-XTS and AES-CTR functions and algorithms to *_avx512. Make Ice Lake and Tiger Lake use *_avx2 instead of *_avx10_256 which they previously used.
I've left AES-GCM unchanged for now. There is no VAES+AVX2 optimized AES-GCM in the kernel yet, so the path forward for that is not as clear. However, I did write a VAES+AVX2 optimized AES-GCM for BoringSSL. So one option is to port that to the kernel and then do the same cleanup.
--- arch/x86/crypto/aes-ctr-avx-x86_64.S | 47 ++++------- arch/x86/crypto/aes-xts-avx-x86_64.S | 118 ++++++++++++--------------- arch/x86/crypto/aesni-intel_glue.c | 30 +++---- 3 files changed, 74 insertions(+), 121 deletions(-)
arch/x86/crypto/aes-ctr-avx-x86_64.S +14 -33
@@ -46,12 +46,11 @@ // // This file contains x86_64 assembly implementations of AES-CTR and AES-XCTR // using the following sets of CPU features: // - AES-NI && AVX // - VAES && AVX2 -// - VAES && (AVX10/256 || (AVX512BW && AVX512VL)) && BMI2 -// - VAES && (AVX10/512 || (AVX512BW && AVX512VL)) && BMI2 +// - VAES && AVX512BW && AVX512VL && BMI2 // // See the function definitions at the bottom of the file for more information. #include <linux/linkage.h> #include <linux/cfi_types.h>
@@ -74,31 +73,29 @@ .quad 4, 0 .text // Move a vector between memory and a register. -// The register operand must be in the first 16 vector registers. .macro _vmovdqu src, dst .if VL < 64 vmovdqu \src, \dst .else vmovdqu8 \src, \dst .endif .endm // Move a vector between registers. -// The registers must be in the first 16 vector registers. .macro _vmovdqa src, dst .if VL < 64 vmovdqa \src, \dst .else vmovdqa64 \src, \dst .endif .endm // Broadcast a 128-bit value from memory to all 128-bit lanes of a vector -// register. The register operand must be in the first 16 vector registers. +// register. .macro _vbroadcast128 src, dst .if VL == 16 vmovdqu \src, \dst .elseif VL == 32 vbroadcasti128 \src, \dst
@@ -106,11 +103,10 @@ vbroadcasti32x4 \src, \dst .endif .endm // XOR two vectors together. -// Any register operands must be in the first 16 vector registers. .macro _vpxor src1, src2, dst .if VL < 64 vpxor \src1, \src2, \dst .else vpxord \src1, \src2, \dst
@@ -197,20 +193,20 @@ // Prepare the next two vectors of AES inputs in AESDATA\i0 and AESDATA\i1, and // XOR each with the zero-th round key. Also update LE_CTR if !\final. .macro _prepare_2_ctr_vecs is_xctr, i0, i1, final=0 .if \is_xctr - .if USE_AVX10 - _vmovdqa LE_CTR, AESDATA\i0 + .if USE_AVX512 + vmovdqa64 LE_CTR, AESDATA\i0 vpternlogd $0x96, XCTR_IV, RNDKEY0, AESDATA\i0 .else vpxor XCTR_IV, LE_CTR, AESDATA\i0 vpxor RNDKEY0, AESDATA\i0, AESDATA\i0 .endif vpaddq LE_CTR_INC1, LE_CTR, AESDATA\i1 - .if USE_AVX10 + .if USE_AVX512 vpternlogd $0x96, XCTR_IV, RNDKEY0, AESDATA\i1 .else vpxor XCTR_IV, AESDATA\i1, AESDATA\i1 vpxor RNDKEY0, AESDATA\i1, AESDATA\i1 .endif
@@ -479,22 +475,16 @@ _vmovdqa AESDATA3, AESDATA0 .Lxor_tail_partial_vec_0\@: // XOR the remaining 1 <= LEN < VL bytes. It's easy if masked // loads/stores are available; otherwise it's a bit harder... -.if USE_AVX10 - .if VL <= 32 - mov $-1, %eax - bzhi LEN, %eax, %eax - kmovd %eax, %k1 - .else +.if USE_AVX512 mov $-1, %rax bzhi LEN64, %rax, %rax kmovq %rax, %k1 - .endif vmovdqu8 (SRC), AESDATA1{%k1}{z} - _vpxor AESDATA1, AESDATA0, AESDATA0 + vpxord AESDATA1, AESDATA0, AESDATA0 vmovdqu8 AESDATA0, (DST){%k1} .else .if VL == 32 cmp $16, LEN jl 1f
@@ -552,41 +542,32 @@ // with HCTR2" (https://eprint.iacr.org/2021/1441.pdf). XCTR is an // easier-to-implement variant of CTR that uses little endian byte order and // eliminates carries. |ctr| is the per-message block counter starting at 1. .set VL, 16 -.set USE_AVX10, 0 +.set USE_AVX512, 0 SYM_TYPED_FUNC_START(aes_ctr64_crypt_aesni_avx) _aes_ctr_crypt 0 SYM_FUNC_END(aes_ctr64_crypt_aesni_avx) SYM_TYPED_FUNC_START(aes_xctr_crypt_aesni_avx) _aes_ctr_crypt 1 SYM_FUNC_END(aes_xctr_crypt_aesni_avx) #if defined(CONFIG_AS_VAES) && defined(CONFIG_AS_VPCLMULQDQ) .set VL, 32 -.set USE_AVX10, 0 +.set USE_AVX512, 0 SYM_TYPED_FUNC_START(aes_ctr64_crypt_vaes_avx2) _aes_ctr_crypt 0 SYM_FUNC_END(aes_ctr64_crypt_vaes_avx2) SYM_TYPED_FUNC_START(aes_xctr_crypt_vaes_avx2) _aes_ctr_crypt 1 SYM_FUNC_END(aes_xctr_crypt_vaes_avx2) -.set VL, 32 -.set USE_AVX10, 1 -SYM_TYPED_FUNC_START(aes_ctr64_crypt_vaes_avx10_256) - _aes_ctr_crypt 0 -SYM_FUNC_END(aes_ctr64_crypt_vaes_avx10_256) -SYM_TYPED_FUNC_START(aes_xctr_crypt_vaes_avx10_256) - _aes_ctr_crypt 1 -SYM_FUNC_END(aes_xctr_crypt_vaes_avx10_256) - .set VL, 64 -.set USE_AVX10, 1 -SYM_TYPED_FUNC_START(aes_ctr64_crypt_vaes_avx10_512) +.set USE_AVX512, 1 +SYM_TYPED_FUNC_START(aes_ctr64_crypt_vaes_avx512) _aes_ctr_crypt 0 -SYM_FUNC_END(aes_ctr64_crypt_vaes_avx10_512) -SYM_TYPED_FUNC_START(aes_xctr_crypt_vaes_avx10_512) +SYM_FUNC_END(aes_ctr64_crypt_vaes_avx512) +SYM_TYPED_FUNC_START(aes_xctr_crypt_vaes_avx512) _aes_ctr_crypt 1 -SYM_FUNC_END(aes_xctr_crypt_vaes_avx10_512) +SYM_FUNC_END(aes_xctr_crypt_vaes_avx512) #endif // CONFIG_AS_VAES && CONFIG_AS_VPCLMULQDQ
arch/x86/crypto/aes-xts-avx-x86_64.S +50 -68
336 lines not on this page · raw message
arch/x86/crypto/aesni-intel_glue.c +10 -20
@@ -842,12 +842,11 @@ static struct simd_skcipher_alg * \ simd_skcipher_algs_##suffix[ARRAY_SIZE(skcipher_algs_##suffix)]DEFINE_AVX_SKCIPHER_ALGS (aesni_avx,"aesni-avx" ,500 );#if defined(CONFIG_AS_VAES ) && defined(CONFIG_AS_VPCLMULQDQ )DEFINE_AVX_SKCIPHER_ALGS (vaes_avx2,"vaes-avx2" ,600 ); -DEFINE_AVX_SKCIPHER_ALGS (vaes_avx10_256,"vaes-avx10_256" ,700 ); -DEFINE_AVX_SKCIPHER_ALGS (vaes_avx10_512,"vaes-avx10_512" ,800 ); +DEFINE_AVX_SKCIPHER_ALGS (vaes_avx512,"vaes-avx512" ,800 );#endif /* The common part of the x86_64 AES-GCM key struct */ struct aes_gcm_key {/* Expanded AES key and the AES key length in bytes */
@@ -1590,33 +1589,28 @@ static int __init register_avx_algs(void) !boot_cpu_has (X86_FEATURE_BMI2 ) || !cpu_has_xfeatures (XFEATURE_MASK_SSE |XFEATURE_MASK_YMM | XFEATURE_MASK_AVX512, NULL)) return 0; - err =simd_register_skciphers_compat (skcipher_algs_vaes_avx10_256, -ARRAY_SIZE (skcipher_algs_vaes_avx10_256), - simd_skcipher_algs_vaes_avx10_256); -if (err) -return err; err =simd_register_aeads_compat (aes_gcm_algs_vaes_avx10_256,ARRAY_SIZE (aes_gcm_algs_vaes_avx10_256), aes_gcm_simdalgs_vaes_avx10_256);if (err)return err;if (boot_cpu_has (X86_FEATURE_PREFER_YMM )) {int i; -for (i =0 ; i <ARRAY_SIZE (skcipher_algs_vaes_avx10_512); i++) - skcipher_algs_vaes_avx10_512[i].base.cra_priority =1 ; +for (i =0 ; i <ARRAY_SIZE (skcipher_algs_vaes_avx512); i++) + skcipher_algs_vaes_avx512[i].base.cra_priority =1 ;for (i =0 ; i <ARRAY_SIZE (aes_gcm_algs_vaes_avx10_512); i++) aes_gcm_algs_vaes_avx10_512[i].base.cra_priority =1 ; } - err =simd_register_skciphers_compat (skcipher_algs_vaes_avx10_512, -ARRAY_SIZE (skcipher_algs_vaes_avx10_512), - simd_skcipher_algs_vaes_avx10_512); + err =simd_register_skciphers_compat (skcipher_algs_vaes_avx512, +ARRAY_SIZE (skcipher_algs_vaes_avx512), + simd_skcipher_algs_vaes_avx512);if (err)return err; err =simd_register_aeads_compat (aes_gcm_algs_vaes_avx10_512,ARRAY_SIZE (aes_gcm_algs_vaes_avx10_512), aes_gcm_simdalgs_vaes_avx10_512);
@@ -1639,22 +1633,18 @@ static void unregister_avx_algs(void)#if defined(CONFIG_AS_VAES ) && defined(CONFIG_AS_VPCLMULQDQ )if (simd_skcipher_algs_vaes_avx2[0 ])simd_unregister_skciphers (skcipher_algs_vaes_avx2,ARRAY_SIZE (skcipher_algs_vaes_avx2), simd_skcipher_algs_vaes_avx2); -if (simd_skcipher_algs_vaes_avx10_256[0 ]) -simd_unregister_skciphers (skcipher_algs_vaes_avx10_256, -ARRAY_SIZE (skcipher_algs_vaes_avx10_256), - simd_skcipher_algs_vaes_avx10_256);if (aes_gcm_simdalgs_vaes_avx10_256[0 ])simd_unregister_aeads (aes_gcm_algs_vaes_avx10_256,ARRAY_SIZE (aes_gcm_algs_vaes_avx10_256), aes_gcm_simdalgs_vaes_avx10_256); -if (simd_skcipher_algs_vaes_avx10_512[0 ]) -simd_unregister_skciphers (skcipher_algs_vaes_avx10_512, -ARRAY_SIZE (skcipher_algs_vaes_avx10_512), - simd_skcipher_algs_vaes_avx10_512); +if (simd_skcipher_algs_vaes_avx512[0 ]) +simd_unregister_skciphers (skcipher_algs_vaes_avx512, +ARRAY_SIZE (skcipher_algs_vaes_avx512), + simd_skcipher_algs_vaes_avx512);if (aes_gcm_simdalgs_vaes_avx10_512[0 ])simd_unregister_aeads (aes_gcm_algs_vaes_avx10_512,ARRAY_SIZE (aes_gcm_algs_vaes_avx10_512), aes_gcm_simdalgs_vaes_avx10_512);#endif
Signature · 1 line
2.49.0
PATCH v2 2/9 crypto: x86/aegis - stop using the SIMD helper
In reply to Eric BiggersFrom: Eric Biggers <[email protected]>
Stop wrapping skcipher and aead algorithms with the crypto SIMD helper (crypto/simd.c). The only purpose of doing so was to work around x86 not always supporting kernel-mode FPU in softirqs. Specifically, if a hardirq interrupted a task context kernel-mode FPU section and then a softirqs were run at the end of that hardirq, those softirqs could not use kernel-mode FPU. This has now been fixed. In combination with the fact that the skcipher and aead APIs only support task and softirq contexts, these can now just use kernel-mode FPU unconditionally on x86.
This simplifies the code and improves performance.
--- arch/x86/crypto/Kconfig | 1 - arch/x86/crypto/aegis128-aesni-glue.c | 13 ++++--------- 2 files changed, 4 insertions(+), 10 deletions(-)
arch/x86/crypto/Kconfig +0 -1
@@ -367,11 +367,10 @@ config CRYPTO_CHACHA20_X86_64 config CRYPTO_AEGIS128_AESNI_SSE2 tristate "AEAD ciphers: AEGIS-128 (AES-NI/SSE4.1)" depends on X86 && 64BIT select CRYPTO_AEAD - select CRYPTO_SIMD help AEGIS-128 AEAD algorithm Architecture: x86_64 using: - AES-NI (AES New Instructions)
arch/x86/crypto/aegis128-aesni-glue.c +4 -9
@@ -6,11 +6,10 @@* Copyright (c) 2017-2018 Ondrej Mosnacek <[email protected]> * Copyright (C) 2017-2018 Red Hat, Inc. All rights reserved. */ #include <crypto/internal/aead.h> -#include <crypto/internal/simd.h> #include <crypto/internal/skcipher.h> #include <crypto/scatterwalk.h> #include <linux/module.h> #include <asm/fpu/api.h> #include <asm/cpu_device_id.h>
@@ -231,39 +230,35 @@ static struct aead_alg crypto_aegis128_aesni_alg = { .ivsize = AEGIS128_NONCE_SIZE, .maxauthsize = AEGIS128_MAX_AUTH_SIZE, .chunksize = AEGIS128_BLOCK_SIZE, .base = { - .cra_flags = CRYPTO_ALG_INTERNAL, .cra_blocksize = 1, .cra_ctxsize =sizeof (struct aegis_ctx ) + __alignof__(struct aegis_ctx), .cra_priority = 400, - .cra_name = "__aegis128", - .cra_driver_name = "__aegis128-aesni", + .cra_name = "aegis128", + .cra_driver_name = "aegis128-aesni", .cra_module = THIS_MODULE, } }; -static struct simd_aead_alg *simd_alg; -static int __init crypto_aegis128_aesni_module_init (void ) {if (!boot_cpu_has (X86_FEATURE_XMM4_1 ) || !boot_cpu_has (X86_FEATURE_AES ) || !cpu_has_xfeatures (XFEATURE_MASK_SSE ,NULL ))return -ENODEV ; -return simd_register_aeads_compat (&crypto_aegis128_aesni_alg,1 , - &simd_alg); +return crypto_register_aead (&crypto_aegis128_aesni_alg); } static void __exit crypto_aegis128_aesni_module_exit(void) { -simd_unregister_aeads (&crypto_aegis128_aesni_alg,1 , &simd_alg); +crypto_unregister_aead (&crypto_aegis128_aesni_alg); }module_init (crypto_aegis128_aesni_module_init);module_exit (crypto_aegis128_aesni_module_exit);
Signature · 1 line
2.49.0
PATCH v2 3/9 crypto: x86/aes - stop using the SIMD helper
In reply to Eric BiggersFrom: Eric Biggers <[email protected]>
Stop wrapping skcipher and aead algorithms with the crypto SIMD helper (crypto/simd.c). The only purpose of doing so was to work around x86 not always supporting kernel-mode FPU in softirqs. Specifically, if a hardirq interrupted a task context kernel-mode FPU section and then a softirqs were run at the end of that hardirq, those softirqs could not use kernel-mode FPU. This has now been fixed. In combination with the fact that the skcipher and aead APIs only support task and softirq contexts, these can now just use kernel-mode FPU unconditionally on x86.
This simplifies the code and improves performance.
--- arch/x86/crypto/Kconfig | 1 - arch/x86/crypto/aesni-intel_glue.c | 158 +++++++++++------------------ 2 files changed, 59 insertions(+), 100 deletions(-)
arch/x86/crypto/Kconfig +0 -1
@@ -21,11 +21,10 @@ config CRYPTO_AES_NI_INTEL select CRYPTO_AEAD select CRYPTO_LIB_AES select CRYPTO_LIB_GF128MUL select CRYPTO_ALGAPI select CRYPTO_SKCIPHER - select CRYPTO_SIMD help Block cipher: AES cipher algorithms AEAD cipher: AES with GCM Length-preserving ciphers: AES with ECB, CBC, CTS, CTR, XCTR, XTS
arch/x86/crypto/aesni-intel_glue.c +59 -99
380 lines not on this page · raw message
Signature · 1 line
2.49.0
PATCH v2 4/9 crypto: x86/aria - stop using the SIMD helper
In reply to Eric BiggersFrom: Eric Biggers <[email protected]>
Stop wrapping skcipher and aead algorithms with the crypto SIMD helper (crypto/simd.c). The only purpose of doing so was to work around x86 not always supporting kernel-mode FPU in softirqs. Specifically, if a hardirq interrupted a task context kernel-mode FPU section and then a softirqs were run at the end of that hardirq, those softirqs could not use kernel-mode FPU. This has now been fixed. In combination with the fact that the skcipher and aead APIs only support task and softirq contexts, these can now just use kernel-mode FPU unconditionally on x86.
This simplifies the code and improves performance.
--- arch/x86/crypto/Kconfig | 3 --- arch/x86/crypto/aria_aesni_avx2_glue.c | 22 +++++++--------------- arch/x86/crypto/aria_aesni_avx_glue.c | 20 ++++++-------------- arch/x86/crypto/aria_gfni_avx512_glue.c | 22 +++++++--------------- 4 files changed, 20 insertions(+), 47 deletions(-)
arch/x86/crypto/Kconfig +0 -3
@@ -294,11 +294,10 @@ config CRYPTO_TWOFISH_AVX_X86_64 config CRYPTO_ARIA_AESNI_AVX_X86_64 tristate "Ciphers: ARIA with modes: ECB, CTR (AES-NI/AVX/GFNI)" depends on X86 && 64BIT select CRYPTO_SKCIPHER - select CRYPTO_SIMD select CRYPTO_ALGAPI select CRYPTO_ARIA help Length-preserving cipher: ARIA cipher algorithms (RFC 5794) with ECB and CTR modes
@@ -312,11 +311,10 @@ config CRYPTO_ARIA_AESNI_AVX_X86_64 config CRYPTO_ARIA_AESNI_AVX2_X86_64 tristate "Ciphers: ARIA with modes: ECB, CTR (AES-NI/AVX2/GFNI)" depends on X86 && 64BIT select CRYPTO_SKCIPHER - select CRYPTO_SIMD select CRYPTO_ALGAPI select CRYPTO_ARIA select CRYPTO_ARIA_AESNI_AVX_X86_64 help Length-preserving cipher: ARIA cipher algorithms
@@ -331,11 +329,10 @@ config CRYPTO_ARIA_AESNI_AVX2_X86_64 config CRYPTO_ARIA_GFNI_AVX512_X86_64 tristate "Ciphers: ARIA with modes: ECB, CTR (AVX512/GFNI)" depends on X86 && 64BIT && AS_AVX512 && AS_GFNI select CRYPTO_SKCIPHER - select CRYPTO_SIMD select CRYPTO_ALGAPI select CRYPTO_ARIA select CRYPTO_ARIA_AESNI_AVX_X86_64 select CRYPTO_ARIA_AESNI_AVX2_X86_64 help
arch/x86/crypto/aria_aesni_avx2_glue.c +7 -15
@@ -4,11 +4,10 @@* * Copyright (c) 2022 Taehee Yoo <[email protected]> */ #include <crypto/algapi.h> -#include <crypto/internal/simd.h> #include <crypto/aria.h> #include <linux/crypto.h> #include <linux/err.h> #include <linux/module.h> #include <linux/types.h>
@@ -163,28 +162,26 @@ static int aria_avx2_init_tfm(struct crypto_skcipher *tfm)return 0 ; }static struct skcipher_alg aria_algs[] = { { - .base.cra_name ="__ecb(aria)" , - .base.cra_driver_name ="__ecb-aria-avx2" , + .base.cra_name ="ecb(aria)" , + .base.cra_driver_name ="ecb-aria-avx2" , .base.cra_priority =500 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =ARIA_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct aria_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =ARIA_MIN_KEY_SIZE , .max_keysize =ARIA_MAX_KEY_SIZE , .setkey = aria_avx2_set_key, .encrypt = aria_avx2_ecb_encrypt, .decrypt = aria_avx2_ecb_decrypt, }, { - .base.cra_name ="__ctr(aria)" , - .base.cra_driver_name ="__ctr-aria-avx2" , + .base.cra_name ="ctr(aria)" , + .base.cra_driver_name ="ctr-aria-avx2" , .base.cra_priority =500 , - .base.cra_flags =CRYPTO_ALG_INTERNAL | -CRYPTO_ALG_SKCIPHER_REQSIZE_LARGE , + .base.cra_flags =CRYPTO_ALG_SKCIPHER_REQSIZE_LARGE , .base.cra_blocksize =1 , .base.cra_ctxsize =sizeof (struct aria_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =ARIA_MIN_KEY_SIZE , .max_keysize =ARIA_MAX_KEY_SIZE ,
@@ -195,12 +192,10 @@ static struct skcipher_alg aria_algs[] = { .decrypt = aria_avx2_ctr_encrypt, .init = aria_avx2_init_tfm, } }; -static struct simd_skcipher_alg *aria_simd_algs[ARRAY_SIZE (aria_algs)]; -static int __init aria_avx2_init (void ) {const char *feature_name; if (!boot_cpu_has(X86_FEATURE_AVX) ||
@@ -231,19 +226,16 @@ static int __init aria_avx2_init(void) aria_ops.aria_encrypt_32way = aria_aesni_avx2_encrypt_32way; aria_ops.aria_decrypt_32way = aria_aesni_avx2_decrypt_32way; aria_ops.aria_ctr_crypt_32way = aria_aesni_avx2_ctr_crypt_32way; } -return simd_register_skciphers_compat (aria_algs, -ARRAY_SIZE (aria_algs), - aria_simd_algs); +return crypto_register_skciphers (aria_algs,ARRAY_SIZE (aria_algs)); } static void __exit aria_avx2_exit(void) { -simd_unregister_skciphers (aria_algs,ARRAY_SIZE (aria_algs), - aria_simd_algs); +crypto_unregister_skciphers (aria_algs,ARRAY_SIZE (aria_algs)); }module_init (aria_avx2_init);module_exit (aria_avx2_exit);
arch/x86/crypto/aria_aesni_avx_glue.c +6 -14
@@ -4,11 +4,10 @@* * Copyright (c) 2022 Taehee Yoo <[email protected]> */ #include <crypto/algapi.h> -#include <crypto/internal/simd.h> #include <crypto/aria.h> #include <linux/crypto.h> #include <linux/err.h> #include <linux/module.h> #include <linux/types.h>
@@ -150,27 +149,25 @@ static int aria_avx_init_tfm(struct crypto_skcipher *tfm)return 0 ; }static struct skcipher_alg aria_algs[] = { { - .base.cra_name ="__ecb(aria)" , - .base.cra_driver_name ="__ecb-aria-avx" , + .base.cra_name ="ecb(aria)" , + .base.cra_driver_name ="ecb-aria-avx" , .base.cra_priority =400 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =ARIA_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct aria_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =ARIA_MIN_KEY_SIZE , .max_keysize =ARIA_MAX_KEY_SIZE , .setkey = aria_avx_set_key, .encrypt = aria_avx_ecb_encrypt, .decrypt = aria_avx_ecb_decrypt, }, { - .base.cra_name ="__ctr(aria)" , - .base.cra_driver_name ="__ctr-aria-avx" , + .base.cra_name ="ctr(aria)" , + .base.cra_driver_name ="ctr-aria-avx" , .base.cra_priority =400 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =1 , .base.cra_ctxsize =sizeof (struct aria_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =ARIA_MIN_KEY_SIZE , .max_keysize =ARIA_MAX_KEY_SIZE ,
@@ -182,12 +179,10 @@ static struct skcipher_alg aria_algs[] = { .decrypt = aria_avx_ctr_encrypt, .init = aria_avx_init_tfm, } }; -static struct simd_skcipher_alg *aria_simd_algs[ARRAY_SIZE (aria_algs)]; -static int __init aria_avx_init (void ) {const char *feature_name; if (!boot_cpu_has(X86_FEATURE_AVX) ||
@@ -211,19 +206,16 @@ static int __init aria_avx_init(void) aria_ops.aria_encrypt_16way = aria_aesni_avx_encrypt_16way; aria_ops.aria_decrypt_16way = aria_aesni_avx_decrypt_16way; aria_ops.aria_ctr_crypt_16way = aria_aesni_avx_ctr_crypt_16way; } -return simd_register_skciphers_compat (aria_algs, -ARRAY_SIZE (aria_algs), - aria_simd_algs); +return crypto_register_skciphers (aria_algs,ARRAY_SIZE (aria_algs)); } static void __exit aria_avx_exit(void) { -simd_unregister_skciphers (aria_algs,ARRAY_SIZE (aria_algs), - aria_simd_algs); +crypto_unregister_skciphers (aria_algs,ARRAY_SIZE (aria_algs)); }module_init (aria_avx_init);module_exit (aria_avx_exit);
arch/x86/crypto/aria_gfni_avx512_glue.c +7 -15
@@ -4,11 +4,10 @@* * Copyright (c) 2022 Taehee Yoo <[email protected]> */ #include <crypto/algapi.h> -#include <crypto/internal/simd.h> #include <crypto/aria.h> #include <linux/crypto.h> #include <linux/err.h> #include <linux/module.h> #include <linux/types.h>
@@ -163,28 +162,26 @@ static int aria_avx512_init_tfm(struct crypto_skcipher *tfm)return 0 ; }static struct skcipher_alg aria_algs[] = { { - .base.cra_name ="__ecb(aria)" , - .base.cra_driver_name ="__ecb-aria-avx512" , + .base.cra_name ="ecb(aria)" , + .base.cra_driver_name ="ecb-aria-avx512" , .base.cra_priority =600 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =ARIA_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct aria_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =ARIA_MIN_KEY_SIZE , .max_keysize =ARIA_MAX_KEY_SIZE , .setkey = aria_avx512_set_key, .encrypt = aria_avx512_ecb_encrypt, .decrypt = aria_avx512_ecb_decrypt, }, { - .base.cra_name ="__ctr(aria)" , - .base.cra_driver_name ="__ctr-aria-avx512" , + .base.cra_name ="ctr(aria)" , + .base.cra_driver_name ="ctr-aria-avx512" , .base.cra_priority =600 , - .base.cra_flags =CRYPTO_ALG_INTERNAL | -CRYPTO_ALG_SKCIPHER_REQSIZE_LARGE , + .base.cra_flags =CRYPTO_ALG_SKCIPHER_REQSIZE_LARGE , .base.cra_blocksize =1 , .base.cra_ctxsize =sizeof (struct aria_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =ARIA_MIN_KEY_SIZE , .max_keysize =ARIA_MAX_KEY_SIZE ,
@@ -195,12 +192,10 @@ static struct skcipher_alg aria_algs[] = { .decrypt = aria_avx512_ctr_encrypt, .init = aria_avx512_init_tfm, } }; -static struct simd_skcipher_alg *aria_simd_algs[ARRAY_SIZE (aria_algs)]; -static int __init aria_avx512_init (void ) {const char *feature_name; if (!boot_cpu_has(X86_FEATURE_AVX) ||
@@ -227,19 +222,16 @@ static int __init aria_avx512_init(void) aria_ops.aria_ctr_crypt_32way = aria_aesni_avx2_gfni_ctr_crypt_32way; aria_ops.aria_encrypt_64way = aria_gfni_avx512_encrypt_64way; aria_ops.aria_decrypt_64way = aria_gfni_avx512_decrypt_64way; aria_ops.aria_ctr_crypt_64way = aria_gfni_avx512_ctr_crypt_64way; -return simd_register_skciphers_compat (aria_algs, -ARRAY_SIZE (aria_algs), - aria_simd_algs); +return crypto_register_skciphers (aria_algs,ARRAY_SIZE (aria_algs)); } static void __exit aria_avx512_exit(void) { -simd_unregister_skciphers (aria_algs,ARRAY_SIZE (aria_algs), - aria_simd_algs); +crypto_unregister_skciphers (aria_algs,ARRAY_SIZE (aria_algs)); }module_init (aria_avx512_init);module_exit (aria_avx512_exit);
Signature · 1 line
2.49.0
PATCH v2 5/9 crypto: x86/camellia - stop using the SIMD helper
In reply to Eric BiggersFrom: Eric Biggers <[email protected]>
Stop wrapping skcipher and aead algorithms with the crypto SIMD helper (crypto/simd.c). The only purpose of doing so was to work around x86 not always supporting kernel-mode FPU in softirqs. Specifically, if a hardirq interrupted a task context kernel-mode FPU section and then a softirqs were run at the end of that hardirq, those softirqs could not use kernel-mode FPU. This has now been fixed. In combination with the fact that the skcipher and aead APIs only support task and softirq contexts, these can now just use kernel-mode FPU unconditionally on x86.
This simplifies the code and improves performance.
--- arch/x86/crypto/Kconfig | 1 - arch/x86/crypto/camellia_aesni_avx2_glue.c | 21 +++++++-------------- arch/x86/crypto/camellia_aesni_avx_glue.c | 21 +++++++-------------- 3 files changed, 14 insertions(+), 29 deletions(-)
arch/x86/crypto/Kconfig +0 -1
@@ -61,11 +61,10 @@ config CRYPTO_CAMELLIA_X86_64 config CRYPTO_CAMELLIA_AESNI_AVX_X86_64 tristate "Ciphers: Camellia with modes: ECB, CBC (AES-NI/AVX)" depends on X86 && 64BIT select CRYPTO_SKCIPHER select CRYPTO_CAMELLIA_X86_64 - select CRYPTO_SIMD imply CRYPTO_XTS help Length-preserving ciphers: Camellia with ECB and CBC modes Architecture: x86_64 using:
arch/x86/crypto/camellia_aesni_avx2_glue.c +7 -14
@@ -4,11 +4,10 @@* * Copyright © 2013 Jussi Kivilinna <[email protected]> */ #include <crypto/algapi.h> -#include <crypto/internal/simd.h> #include <linux/crypto.h> #include <linux/err.h> #include <linux/module.h> #include <linux/types.h>
@@ -67,27 +66,25 @@ static int cbc_decrypt(struct skcipher_request *req)CBC_WALK_END (); }static struct skcipher_alg camellia_algs[] = { { - .base.cra_name ="__ecb(camellia)" , - .base.cra_driver_name ="__ecb-camellia-aesni-avx2" , + .base.cra_name ="ecb(camellia)" , + .base.cra_driver_name ="ecb-camellia-aesni-avx2" , .base.cra_priority =500 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =CAMELLIA_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct camellia_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =CAMELLIA_MIN_KEY_SIZE , .max_keysize =CAMELLIA_MAX_KEY_SIZE , .setkey = camellia_setkey, .encrypt = ecb_encrypt, .decrypt = ecb_decrypt, }, { - .base.cra_name ="__cbc(camellia)" , - .base.cra_driver_name ="__cbc-camellia-aesni-avx2" , + .base.cra_name ="cbc(camellia)" , + .base.cra_driver_name ="cbc-camellia-aesni-avx2" , .base.cra_priority =500 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =CAMELLIA_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct camellia_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =CAMELLIA_MIN_KEY_SIZE , .max_keysize =CAMELLIA_MAX_KEY_SIZE ,
@@ -96,12 +93,10 @@ static struct skcipher_alg camellia_algs[] = { .encrypt = cbc_encrypt, .decrypt = cbc_decrypt, }, }; -static struct simd_skcipher_alg *camellia_simd_algs[ARRAY_SIZE (camellia_algs)]; -static int __init camellia_aesni_init (void ) {const char *feature_name; if (!boot_cpu_has(X86_FEATURE_AVX) ||
@@ -116,19 +111,17 @@ static int __init camellia_aesni_init(void) &feature_name)) {pr_info ("CPU feature '%s' is not supported.\n" , feature_name);return -ENODEV ; } -return simd_register_skciphers_compat (camellia_algs, -ARRAY_SIZE (camellia_algs), - camellia_simd_algs); +return crypto_register_skciphers (camellia_algs, +ARRAY_SIZE (camellia_algs)); } static void __exit camellia_aesni_fini(void) { -simd_unregister_skciphers (camellia_algs,ARRAY_SIZE (camellia_algs), - camellia_simd_algs); +crypto_unregister_skciphers (camellia_algs,ARRAY_SIZE (camellia_algs)); }module_init (camellia_aesni_init);module_exit (camellia_aesni_fini);
arch/x86/crypto/camellia_aesni_avx_glue.c +7 -14
@@ -4,11 +4,10 @@* * Copyright © 2012-2013 Jussi Kivilinna <[email protected]> */ #include <crypto/algapi.h> -#include <crypto/internal/simd.h> #include <linux/crypto.h> #include <linux/err.h> #include <linux/module.h> #include <linux/types.h>
@@ -67,27 +66,25 @@ static int cbc_decrypt(struct skcipher_request *req)CBC_WALK_END (); }static struct skcipher_alg camellia_algs[] = { { - .base.cra_name ="__ecb(camellia)" , - .base.cra_driver_name ="__ecb-camellia-aesni" , + .base.cra_name ="ecb(camellia)" , + .base.cra_driver_name ="ecb-camellia-aesni" , .base.cra_priority =400 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =CAMELLIA_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct camellia_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =CAMELLIA_MIN_KEY_SIZE , .max_keysize =CAMELLIA_MAX_KEY_SIZE , .setkey = camellia_setkey, .encrypt = ecb_encrypt, .decrypt = ecb_decrypt, }, { - .base.cra_name ="__cbc(camellia)" , - .base.cra_driver_name ="__cbc-camellia-aesni" , + .base.cra_name ="cbc(camellia)" , + .base.cra_driver_name ="cbc-camellia-aesni" , .base.cra_priority =400 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =CAMELLIA_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct camellia_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =CAMELLIA_MIN_KEY_SIZE , .max_keysize =CAMELLIA_MAX_KEY_SIZE ,
@@ -96,12 +93,10 @@ static struct skcipher_alg camellia_algs[] = { .encrypt = cbc_encrypt, .decrypt = cbc_decrypt, } }; -static struct simd_skcipher_alg *camellia_simd_algs[ARRAY_SIZE (camellia_algs)]; -static int __init camellia_aesni_init (void ) {const char *feature_name; if (!boot_cpu_has(X86_FEATURE_AVX) ||
@@ -115,19 +110,17 @@ static int __init camellia_aesni_init(void) &feature_name)) {pr_info ("CPU feature '%s' is not supported.\n" , feature_name);return -ENODEV ; } -return simd_register_skciphers_compat (camellia_algs, -ARRAY_SIZE (camellia_algs), - camellia_simd_algs); +return crypto_register_skciphers (camellia_algs, +ARRAY_SIZE (camellia_algs)); } static void __exit camellia_aesni_fini(void) { -simd_unregister_skciphers (camellia_algs,ARRAY_SIZE (camellia_algs), - camellia_simd_algs); +crypto_unregister_skciphers (camellia_algs,ARRAY_SIZE (camellia_algs)); }module_init (camellia_aesni_init);module_exit (camellia_aesni_fini);
Signature · 1 line
2.49.0
PATCH v2 6/9 crypto: x86/cast - stop using the SIMD helper
In reply to Eric BiggersFrom: Eric Biggers <[email protected]>
Stop wrapping skcipher and aead algorithms with the crypto SIMD helper (crypto/simd.c). The only purpose of doing so was to work around x86 not always supporting kernel-mode FPU in softirqs. Specifically, if a hardirq interrupted a task context kernel-mode FPU section and then a softirqs were run at the end of that hardirq, those softirqs could not use kernel-mode FPU. This has now been fixed. In combination with the fact that the skcipher and aead APIs only support task and softirq contexts, these can now just use kernel-mode FPU unconditionally on x86.
This simplifies the code and improves performance.
--- arch/x86/crypto/Kconfig | 2 -- arch/x86/crypto/cast5_avx_glue.c | 21 +++++++-------------- arch/x86/crypto/cast6_avx_glue.c | 20 ++++++-------------- 3 files changed, 13 insertions(+), 30 deletions(-)
arch/x86/crypto/Kconfig +0 -2
@@ -86,11 +86,10 @@ config CRYPTO_CAST5_AVX_X86_64 tristate "Ciphers: CAST5 with modes: ECB, CBC (AVX)" depends on X86 && 64BIT select CRYPTO_SKCIPHER select CRYPTO_CAST5 select CRYPTO_CAST_COMMON - select CRYPTO_SIMD imply CRYPTO_CTR help Length-preserving ciphers: CAST5 (CAST-128) cipher algorithm (RFC2144) with ECB and CBC modes
@@ -103,11 +102,10 @@ config CRYPTO_CAST6_AVX_X86_64 tristate "Ciphers: CAST6 with modes: ECB, CBC (AVX)" depends on X86 && 64BIT select CRYPTO_SKCIPHER select CRYPTO_CAST6 select CRYPTO_CAST_COMMON - select CRYPTO_SIMD imply CRYPTO_XTS imply CRYPTO_CTR help Length-preserving ciphers: CAST6 (CAST-256) cipher algorithm (RFC2612) with ECB and CBC modes
arch/x86/crypto/cast5_avx_glue.c +7 -14
@@ -6,11 +6,10 @@* <[email protected]> */ #include <crypto/algapi.h> #include <crypto/cast5.h> -#include <crypto/internal/simd.h> #include <linux/crypto.h> #include <linux/err.h> #include <linux/module.h> #include <linux/types.h>
@@ -62,27 +61,25 @@ static int cbc_decrypt(struct skcipher_request *req)CBC_WALK_END (); }static struct skcipher_alg cast5_algs[] = { { - .base.cra_name ="__ecb(cast5)" , - .base.cra_driver_name ="__ecb-cast5-avx" , + .base.cra_name ="ecb(cast5)" , + .base.cra_driver_name ="ecb-cast5-avx" , .base.cra_priority =200 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =CAST5_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct cast5_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =CAST5_MIN_KEY_SIZE , .max_keysize =CAST5_MAX_KEY_SIZE , .setkey = cast5_setkey_skcipher, .encrypt = ecb_encrypt, .decrypt = ecb_decrypt, }, { - .base.cra_name ="__cbc(cast5)" , - .base.cra_driver_name ="__cbc-cast5-avx" , + .base.cra_name ="cbc(cast5)" , + .base.cra_driver_name ="cbc-cast5-avx" , .base.cra_priority =200 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =CAST5_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct cast5_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =CAST5_MIN_KEY_SIZE , .max_keysize =CAST5_MAX_KEY_SIZE ,
@@ -91,31 +88,27 @@ static struct skcipher_alg cast5_algs[] = { .encrypt = cbc_encrypt, .decrypt = cbc_decrypt, } }; -static struct simd_skcipher_alg *cast5_simd_algs[ARRAY_SIZE (cast5_algs)]; -static int __init cast5_init (void ) {const char *feature_name;if (!cpu_has_xfeatures (XFEATURE_MASK_SSE |XFEATURE_MASK_YMM , &feature_name)) {pr_info ("CPU feature '%s' is not supported.\n" , feature_name);return -ENODEV ; } -return simd_register_skciphers_compat (cast5_algs, -ARRAY_SIZE (cast5_algs), - cast5_simd_algs); +return crypto_register_skciphers (cast5_algs, +ARRAY_SIZE (cast5_algs)); } static void __exit cast5_exit(void) { -simd_unregister_skciphers (cast5_algs,ARRAY_SIZE (cast5_algs), - cast5_simd_algs); +crypto_unregister_skciphers (cast5_algs,ARRAY_SIZE (cast5_algs)); }module_init (cast5_init);module_exit (cast5_exit);
arch/x86/crypto/cast6_avx_glue.c +6 -14
@@ -12,11 +12,10 @@#include <linux/types.h> #include <linux/crypto.h> #include <linux/err.h> #include <crypto/algapi.h> #include <crypto/cast6.h> -#include <crypto/internal/simd.h> #include "ecb_cbc_helpers.h" #define CAST6_PARALLEL_BLOCKS 8
@@ -62,27 +61,25 @@ static int cbc_decrypt(struct skcipher_request *req)CBC_WALK_END (); }static struct skcipher_alg cast6_algs[] = { { - .base.cra_name ="__ecb(cast6)" , - .base.cra_driver_name ="__ecb-cast6-avx" , + .base.cra_name ="ecb(cast6)" , + .base.cra_driver_name ="ecb-cast6-avx" , .base.cra_priority =200 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =CAST6_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct cast6_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =CAST6_MIN_KEY_SIZE , .max_keysize =CAST6_MAX_KEY_SIZE , .setkey = cast6_setkey_skcipher, .encrypt = ecb_encrypt, .decrypt = ecb_decrypt, }, { - .base.cra_name ="__cbc(cast6)" , - .base.cra_driver_name ="__cbc-cast6-avx" , + .base.cra_name ="cbc(cast6)" , + .base.cra_driver_name ="cbc-cast6-avx" , .base.cra_priority =200 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =CAST6_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct cast6_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =CAST6_MIN_KEY_SIZE , .max_keysize =CAST6_MAX_KEY_SIZE ,
@@ -91,31 +88,26 @@ static struct skcipher_alg cast6_algs[] = { .encrypt = cbc_encrypt, .decrypt = cbc_decrypt, }, }; -static struct simd_skcipher_alg *cast6_simd_algs[ARRAY_SIZE (cast6_algs)]; -static int __init cast6_init (void ) {const char *feature_name;if (!cpu_has_xfeatures (XFEATURE_MASK_SSE |XFEATURE_MASK_YMM , &feature_name)) {pr_info ("CPU feature '%s' is not supported.\n" , feature_name);return -ENODEV ; } -return simd_register_skciphers_compat (cast6_algs, -ARRAY_SIZE (cast6_algs), - cast6_simd_algs); +return crypto_register_skciphers (cast6_algs,ARRAY_SIZE (cast6_algs)); } static void __exit cast6_exit(void) { -simd_unregister_skciphers (cast6_algs,ARRAY_SIZE (cast6_algs), - cast6_simd_algs); +crypto_unregister_skciphers (cast6_algs,ARRAY_SIZE (cast6_algs)); }module_init (cast6_init);module_exit (cast6_exit);
Signature · 1 line
2.49.0
PATCH v2 7/9 crypto: x86/serpent - stop using the SIMD helper
In reply to Eric BiggersFrom: Eric Biggers <[email protected]>
Stop wrapping skcipher and aead algorithms with the crypto SIMD helper (crypto/simd.c). The only purpose of doing so was to work around x86 not always supporting kernel-mode FPU in softirqs. Specifically, if a hardirq interrupted a task context kernel-mode FPU section and then a softirqs were run at the end of that hardirq, those softirqs could not use kernel-mode FPU. This has now been fixed. In combination with the fact that the skcipher and aead APIs only support task and softirq contexts, these can now just use kernel-mode FPU unconditionally on x86.
This simplifies the code and improves performance.
--- arch/x86/crypto/Kconfig | 3 --- arch/x86/crypto/serpent_avx2_glue.c | 21 +++++++-------------- arch/x86/crypto/serpent_avx_glue.c | 21 +++++++-------------- arch/x86/crypto/serpent_sse2_glue.c | 21 +++++++-------------- 4 files changed, 21 insertions(+), 45 deletions(-)
arch/x86/crypto/Kconfig +0 -3
@@ -132,11 +132,10 @@ config CRYPTO_DES3_EDE_X86_64 config CRYPTO_SERPENT_SSE2_X86_64 tristate "Ciphers: Serpent with modes: ECB, CBC (SSE2)" depends on X86 && 64BIT select CRYPTO_SKCIPHER select CRYPTO_SERPENT - select CRYPTO_SIMD imply CRYPTO_CTR help Length-preserving ciphers: Serpent cipher algorithm with ECB and CBC modes
@@ -148,11 +147,10 @@ config CRYPTO_SERPENT_SSE2_X86_64 config CRYPTO_SERPENT_SSE2_586 tristate "Ciphers: Serpent with modes: ECB, CBC (32-bit with SSE2)" depends on X86 && !64BIT select CRYPTO_SKCIPHER select CRYPTO_SERPENT - select CRYPTO_SIMD imply CRYPTO_CTR help Length-preserving ciphers: Serpent cipher algorithm with ECB and CBC modes
@@ -164,11 +162,10 @@ config CRYPTO_SERPENT_SSE2_586 config CRYPTO_SERPENT_AVX_X86_64 tristate "Ciphers: Serpent with modes: ECB, CBC (AVX)" depends on X86 && 64BIT select CRYPTO_SKCIPHER select CRYPTO_SERPENT - select CRYPTO_SIMD imply CRYPTO_XTS imply CRYPTO_CTR help Length-preserving ciphers: Serpent cipher algorithm with ECB and CBC modes
arch/x86/crypto/serpent_avx2_glue.c +7 -14
@@ -8,11 +8,10 @@#include <linux/module.h> #include <linux/types.h> #include <linux/crypto.h> #include <linux/err.h> #include <crypto/algapi.h> -#include <crypto/internal/simd.h> #include <crypto/serpent.h> #include "serpent-avx.h" #include "ecb_cbc_helpers.h"
@@ -63,27 +62,25 @@ static int cbc_decrypt(struct skcipher_request *req)CBC_WALK_END (); }static struct skcipher_alg serpent_algs[] = { { - .base.cra_name ="__ecb(serpent)" , - .base.cra_driver_name ="__ecb-serpent-avx2" , + .base.cra_name ="ecb(serpent)" , + .base.cra_driver_name ="ecb-serpent-avx2" , .base.cra_priority =600 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =SERPENT_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct serpent_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =SERPENT_MIN_KEY_SIZE , .max_keysize =SERPENT_MAX_KEY_SIZE , .setkey = serpent_setkey_skcipher, .encrypt = ecb_encrypt, .decrypt = ecb_decrypt, }, { - .base.cra_name ="__cbc(serpent)" , - .base.cra_driver_name ="__cbc-serpent-avx2" , + .base.cra_name ="cbc(serpent)" , + .base.cra_driver_name ="cbc-serpent-avx2" , .base.cra_priority =600 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =SERPENT_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct serpent_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =SERPENT_MIN_KEY_SIZE , .max_keysize =SERPENT_MAX_KEY_SIZE ,
@@ -92,12 +89,10 @@ static struct skcipher_alg serpent_algs[] = { .encrypt = cbc_encrypt, .decrypt = cbc_decrypt, }, }; -static struct simd_skcipher_alg *serpent_simd_algs[ARRAY_SIZE (serpent_algs)]; -static int __init serpent_avx2_init (void ) {const char *feature_name;if (!boot_cpu_has (X86_FEATURE_AVX2 ) || !boot_cpu_has (X86_FEATURE_OSXSAVE )) {
@@ -108,19 +103,17 @@ static int __init serpent_avx2_init(void) &feature_name)) {pr_info ("CPU feature '%s' is not supported.\n" , feature_name);return -ENODEV ; } -return simd_register_skciphers_compat (serpent_algs, -ARRAY_SIZE (serpent_algs), - serpent_simd_algs); +return crypto_register_skciphers (serpent_algs, +ARRAY_SIZE (serpent_algs)); } static void __exit serpent_avx2_fini(void) { -simd_unregister_skciphers (serpent_algs,ARRAY_SIZE (serpent_algs), - serpent_simd_algs); +crypto_unregister_skciphers (serpent_algs,ARRAY_SIZE (serpent_algs)); }module_init (serpent_avx2_init);module_exit (serpent_avx2_fini);
arch/x86/crypto/serpent_avx_glue.c +7 -14
@@ -11,11 +11,10 @@#include <linux/module.h> #include <linux/types.h> #include <linux/crypto.h> #include <linux/err.h> #include <crypto/algapi.h> -#include <crypto/internal/simd.h> #include <crypto/serpent.h> #include "serpent-avx.h" #include "ecb_cbc_helpers.h"
@@ -69,27 +68,25 @@ static int cbc_decrypt(struct skcipher_request *req)CBC_WALK_END (); }static struct skcipher_alg serpent_algs[] = { { - .base.cra_name ="__ecb(serpent)" , - .base.cra_driver_name ="__ecb-serpent-avx" , + .base.cra_name ="ecb(serpent)" , + .base.cra_driver_name ="ecb-serpent-avx" , .base.cra_priority =500 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =SERPENT_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct serpent_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =SERPENT_MIN_KEY_SIZE , .max_keysize =SERPENT_MAX_KEY_SIZE , .setkey = serpent_setkey_skcipher, .encrypt = ecb_encrypt, .decrypt = ecb_decrypt, }, { - .base.cra_name ="__cbc(serpent)" , - .base.cra_driver_name ="__cbc-serpent-avx" , + .base.cra_name ="cbc(serpent)" , + .base.cra_driver_name ="cbc-serpent-avx" , .base.cra_priority =500 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =SERPENT_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct serpent_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =SERPENT_MIN_KEY_SIZE , .max_keysize =SERPENT_MAX_KEY_SIZE ,
@@ -98,31 +95,27 @@ static struct skcipher_alg serpent_algs[] = { .encrypt = cbc_encrypt, .decrypt = cbc_decrypt, }, }; -static struct simd_skcipher_alg *serpent_simd_algs[ARRAY_SIZE (serpent_algs)]; -static int __init serpent_init (void ) {const char *feature_name;if (!cpu_has_xfeatures (XFEATURE_MASK_SSE |XFEATURE_MASK_YMM , &feature_name)) {pr_info ("CPU feature '%s' is not supported.\n" , feature_name);return -ENODEV ; } -return simd_register_skciphers_compat (serpent_algs, -ARRAY_SIZE (serpent_algs), - serpent_simd_algs); +return crypto_register_skciphers (serpent_algs, +ARRAY_SIZE (serpent_algs)); } static void __exit serpent_exit(void) { -simd_unregister_skciphers (serpent_algs,ARRAY_SIZE (serpent_algs), - serpent_simd_algs); +crypto_unregister_skciphers (serpent_algs,ARRAY_SIZE (serpent_algs)); }module_init (serpent_init);module_exit (serpent_exit);
arch/x86/crypto/serpent_sse2_glue.c +7 -14
@@ -16,11 +16,10 @@#include <linux/types.h> #include <linux/crypto.h> #include <linux/err.h> #include <crypto/algapi.h> #include <crypto/b128ops.h> -#include <crypto/internal/simd.h> #include <crypto/serpent.h> #include "serpent-sse2.h" #include "ecb_cbc_helpers.h"
@@ -72,27 +71,25 @@ static int cbc_decrypt(struct skcipher_request *req)CBC_WALK_END (); }static struct skcipher_alg serpent_algs[] = { { - .base.cra_name ="__ecb(serpent)" , - .base.cra_driver_name ="__ecb-serpent-sse2" , + .base.cra_name ="ecb(serpent)" , + .base.cra_driver_name ="ecb-serpent-sse2" , .base.cra_priority =400 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =SERPENT_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct serpent_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =SERPENT_MIN_KEY_SIZE , .max_keysize =SERPENT_MAX_KEY_SIZE , .setkey = serpent_setkey_skcipher, .encrypt = ecb_encrypt, .decrypt = ecb_decrypt, }, { - .base.cra_name ="__cbc(serpent)" , - .base.cra_driver_name ="__cbc-serpent-sse2" , + .base.cra_name ="cbc(serpent)" , + .base.cra_driver_name ="cbc-serpent-sse2" , .base.cra_priority =400 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =SERPENT_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct serpent_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =SERPENT_MIN_KEY_SIZE , .max_keysize =SERPENT_MAX_KEY_SIZE ,
@@ -101,28 +98,24 @@ static struct skcipher_alg serpent_algs[] = { .encrypt = cbc_encrypt, .decrypt = cbc_decrypt, }, }; -static struct simd_skcipher_alg *serpent_simd_algs[ARRAY_SIZE (serpent_algs)]; -static int __init serpent_sse2_init (void ) {if (!boot_cpu_has (X86_FEATURE_XMM2 )) {printk (KERN_INFO "SSE2 instructions are not detected.\n" );return -ENODEV ; } -return simd_register_skciphers_compat (serpent_algs, -ARRAY_SIZE (serpent_algs), - serpent_simd_algs); +return crypto_register_skciphers (serpent_algs, +ARRAY_SIZE (serpent_algs)); } static void __exit serpent_sse2_exit(void) { -simd_unregister_skciphers (serpent_algs,ARRAY_SIZE (serpent_algs), - serpent_simd_algs); +crypto_unregister_skciphers (serpent_algs,ARRAY_SIZE (serpent_algs)); }module_init (serpent_sse2_init);module_exit (serpent_sse2_exit);
Signature · 1 line
2.49.0
PATCH v2 8/9 crypto: x86/sm4 - stop using the SIMD helper
In reply to Eric BiggersFrom: Eric Biggers <[email protected]>
Stop wrapping skcipher and aead algorithms with the crypto SIMD helper (crypto/simd.c). The only purpose of doing so was to work around x86 not always supporting kernel-mode FPU in softirqs. Specifically, if a hardirq interrupted a task context kernel-mode FPU section and then a softirqs were run at the end of that hardirq, those softirqs could not use kernel-mode FPU. This has now been fixed. In combination with the fact that the skcipher and aead APIs only support task and softirq contexts, these can now just use kernel-mode FPU unconditionally on x86.
This simplifies the code and improves performance.
--- arch/x86/crypto/Kconfig | 2 -- arch/x86/crypto/sm4_aesni_avx2_glue.c | 31 ++++++++++----------------- arch/x86/crypto/sm4_aesni_avx_glue.c | 31 ++++++++++----------------- 3 files changed, 22 insertions(+), 42 deletions(-)
arch/x86/crypto/Kconfig +0 -2
@@ -190,11 +190,10 @@ config CRYPTO_SERPENT_AVX2_X86_64 config CRYPTO_SM4_AESNI_AVX_X86_64 tristate "Ciphers: SM4 with modes: ECB, CBC, CTR (AES-NI/AVX)" depends on X86 && 64BIT select CRYPTO_SKCIPHER - select CRYPTO_SIMD select CRYPTO_ALGAPI select CRYPTO_SM4 help Length-preserving ciphers: SM4 cipher algorithms (OSCCA GB/T 32907-2016) with ECB, CBC, and CTR modes
@@ -211,11 +210,10 @@ config CRYPTO_SM4_AESNI_AVX_X86_64 config CRYPTO_SM4_AESNI_AVX2_X86_64 tristate "Ciphers: SM4 with modes: ECB, CBC, CTR (AES-NI/AVX2)" depends on X86 && 64BIT select CRYPTO_SKCIPHER - select CRYPTO_SIMD select CRYPTO_ALGAPI select CRYPTO_SM4 select CRYPTO_SM4_AESNI_AVX_X86_64 help Length-preserving ciphers: SM4 cipher algorithms
arch/x86/crypto/sm4_aesni_avx2_glue.c +11 -20
@@ -6,15 +6,14 @@* * Copyright (c) 2021, Alibaba Group. * Copyright (c) 2021 Tianjia Zhang <[email protected]> */ +#include <asm/fpu/api.h> #include <linux/module.h> #include <linux/crypto.h> #include <linux/kernel.h> -#include <asm/simd.h> -#include <crypto/internal/simd.h> #include <crypto/internal/skcipher.h> #include <crypto/sm4.h> #include "sm4-avx.h" #define SM4_CRYPT16_BLOCK_SIZE (SM4_BLOCK_SIZE * 16)
@@ -46,14 +45,13 @@ static int ctr_crypt(struct skcipher_request *req) }static struct skcipher_alg sm4_aesni_avx2_skciphers[] = { { .base = { - .cra_name ="__ecb(sm4)" , - .cra_driver_name ="__ecb-sm4-aesni-avx2" , + .cra_name ="ecb(sm4)" , + .cra_driver_name ="ecb-sm4-aesni-avx2" , .cra_priority =500 , - .cra_flags =CRYPTO_ALG_INTERNAL , .cra_blocksize =SM4_BLOCK_SIZE , .cra_ctxsize =sizeof (struct sm4_ctx ), .cra_module =THIS_MODULE , }, .min_keysize =SM4_KEY_SIZE ,
@@ -62,14 +60,13 @@ static struct skcipher_alg sm4_aesni_avx2_skciphers[] = { .setkey = sm4_skcipher_setkey, .encrypt = sm4_avx_ecb_encrypt, .decrypt = sm4_avx_ecb_decrypt, }, { .base = { - .cra_name = "__cbc(sm4)", - .cra_driver_name = "__cbc-sm4-aesni-avx2", + .cra_name = "cbc(sm4)", + .cra_driver_name = "cbc-sm4-aesni-avx2", .cra_priority = 500, - .cra_flags = CRYPTO_ALG_INTERNAL, .cra_blocksize = SM4_BLOCK_SIZE, .cra_ctxsize = sizeof(struct sm4_ctx), .cra_module = THIS_MODULE, }, .min_keysize = SM4_KEY_SIZE,
@@ -79,14 +76,13 @@ static struct skcipher_alg sm4_aesni_avx2_skciphers[] = { .setkey = sm4_skcipher_setkey, .encrypt = sm4_cbc_encrypt, .decrypt = cbc_decrypt, }, { .base = { - .cra_name = "__ctr(sm4)", - .cra_driver_name = "__ctr-sm4-aesni-avx2", + .cra_name = "ctr(sm4)", + .cra_driver_name = "ctr-sm4-aesni-avx2", .cra_priority = 500, - .cra_flags = CRYPTO_ALG_INTERNAL, .cra_blocksize = 1, .cra_ctxsize = sizeof(struct sm4_ctx), .cra_module = THIS_MODULE, }, .min_keysize = SM4_KEY_SIZE,
@@ -98,13 +94,10 @@ static struct skcipher_alg sm4_aesni_avx2_skciphers[] = { .encrypt = ctr_crypt, .decrypt = ctr_crypt, } }; -static struct simd_skcipher_alg * -simd_sm4_aesni_avx2_skciphers[ARRAY_SIZE (sm4_aesni_avx2_skciphers)]; -static int __init sm4_init (void ) {const char *feature_name; if (!boot_cpu_has(X86_FEATURE_AVX) ||
@@ -119,20 +112,18 @@ static int __init sm4_init(void) &feature_name)) {pr_info ("CPU feature '%s' is not supported.\n" , feature_name);return -ENODEV ; } -return simd_register_skciphers_compat (sm4_aesni_avx2_skciphers, -ARRAY_SIZE (sm4_aesni_avx2_skciphers), - simd_sm4_aesni_avx2_skciphers); +return crypto_register_skciphers (sm4_aesni_avx2_skciphers, +ARRAY_SIZE (sm4_aesni_avx2_skciphers)); } static void __exit sm4_exit(void) { -simd_unregister_skciphers (sm4_aesni_avx2_skciphers, -ARRAY_SIZE (sm4_aesni_avx2_skciphers), - simd_sm4_aesni_avx2_skciphers); +crypto_unregister_skciphers (sm4_aesni_avx2_skciphers, +ARRAY_SIZE (sm4_aesni_avx2_skciphers)); }module_init (sm4_init);module_exit (sm4_exit);
arch/x86/crypto/sm4_aesni_avx_glue.c +11 -20
@@ -6,15 +6,14 @@* * Copyright (c) 2021, Alibaba Group. * Copyright (c) 2021 Tianjia Zhang <[email protected]> */ +#include <asm/fpu/api.h> #include <linux/module.h> #include <linux/crypto.h> #include <linux/kernel.h> -#include <asm/simd.h> -#include <crypto/internal/simd.h> #include <crypto/internal/skcipher.h> #include <crypto/sm4.h> #include "sm4-avx.h" #define SM4_CRYPT8_BLOCK_SIZE (SM4_BLOCK_SIZE * 8)
@@ -261,14 +260,13 @@ static int ctr_crypt(struct skcipher_request *req) }static struct skcipher_alg sm4_aesni_avx_skciphers[] = { { .base = { - .cra_name ="__ecb(sm4)" , - .cra_driver_name ="__ecb-sm4-aesni-avx" , + .cra_name ="ecb(sm4)" , + .cra_driver_name ="ecb-sm4-aesni-avx" , .cra_priority =400 , - .cra_flags =CRYPTO_ALG_INTERNAL , .cra_blocksize =SM4_BLOCK_SIZE , .cra_ctxsize =sizeof (struct sm4_ctx ), .cra_module =THIS_MODULE , }, .min_keysize =SM4_KEY_SIZE ,
@@ -277,14 +275,13 @@ static struct skcipher_alg sm4_aesni_avx_skciphers[] = { .setkey = sm4_skcipher_setkey, .encrypt = sm4_avx_ecb_encrypt, .decrypt = sm4_avx_ecb_decrypt, }, { .base = { - .cra_name = "__cbc(sm4)", - .cra_driver_name = "__cbc-sm4-aesni-avx", + .cra_name = "cbc(sm4)", + .cra_driver_name = "cbc-sm4-aesni-avx", .cra_priority = 400, - .cra_flags = CRYPTO_ALG_INTERNAL, .cra_blocksize = SM4_BLOCK_SIZE, .cra_ctxsize = sizeof(struct sm4_ctx), .cra_module = THIS_MODULE, }, .min_keysize = SM4_KEY_SIZE,
@@ -294,14 +291,13 @@ static struct skcipher_alg sm4_aesni_avx_skciphers[] = { .setkey = sm4_skcipher_setkey, .encrypt = sm4_cbc_encrypt, .decrypt = cbc_decrypt, }, { .base = { - .cra_name = "__ctr(sm4)", - .cra_driver_name = "__ctr-sm4-aesni-avx", + .cra_name = "ctr(sm4)", + .cra_driver_name = "ctr-sm4-aesni-avx", .cra_priority = 400, - .cra_flags = CRYPTO_ALG_INTERNAL, .cra_blocksize = 1, .cra_ctxsize = sizeof(struct sm4_ctx), .cra_module = THIS_MODULE, }, .min_keysize = SM4_KEY_SIZE,
@@ -313,13 +309,10 @@ static struct skcipher_alg sm4_aesni_avx_skciphers[] = { .encrypt = ctr_crypt, .decrypt = ctr_crypt, } }; -static struct simd_skcipher_alg * -simd_sm4_aesni_avx_skciphers[ARRAY_SIZE (sm4_aesni_avx_skciphers)]; -static int __init sm4_init (void ) {const char *feature_name; if (!boot_cpu_has(X86_FEATURE_AVX) ||
@@ -333,20 +326,18 @@ static int __init sm4_init(void) &feature_name)) {pr_info ("CPU feature '%s' is not supported.\n" , feature_name);return -ENODEV ; } -return simd_register_skciphers_compat (sm4_aesni_avx_skciphers, -ARRAY_SIZE (sm4_aesni_avx_skciphers), - simd_sm4_aesni_avx_skciphers); +return crypto_register_skciphers (sm4_aesni_avx_skciphers, +ARRAY_SIZE (sm4_aesni_avx_skciphers)); } static void __exit sm4_exit(void) { -simd_unregister_skciphers (sm4_aesni_avx_skciphers, -ARRAY_SIZE (sm4_aesni_avx_skciphers), - simd_sm4_aesni_avx_skciphers); +crypto_unregister_skciphers (sm4_aesni_avx_skciphers, +ARRAY_SIZE (sm4_aesni_avx_skciphers)); }module_init (sm4_init);module_exit (sm4_exit);
Signature · 1 line
2.49.0
PATCH v2 9/9 crypto: x86/twofish - stop using the SIMD helper
In reply to Eric BiggersFrom: Eric Biggers <[email protected]>
Stop wrapping skcipher and aead algorithms with the crypto SIMD helper (crypto/simd.c). The only purpose of doing so was to work around x86 not always supporting kernel-mode FPU in softirqs. Specifically, if a hardirq interrupted a task context kernel-mode FPU section and then a softirqs were run at the end of that hardirq, those softirqs could not use kernel-mode FPU. This has now been fixed. In combination with the fact that the skcipher and aead APIs only support task and softirq contexts, these can now just use kernel-mode FPU unconditionally on x86.
This simplifies the code and improves performance.
--- arch/x86/crypto/Kconfig | 1 - arch/x86/crypto/twofish_avx_glue.c | 21 +++++++-------------- 2 files changed, 7 insertions(+), 15 deletions(-)
arch/x86/crypto/Kconfig +0 -1
@@ -268,11 +268,10 @@ config CRYPTO_TWOFISH_X86_64_3WAY config CRYPTO_TWOFISH_AVX_X86_64 tristate "Ciphers: Twofish with modes: ECB, CBC (AVX)" depends on X86 && 64BIT select CRYPTO_SKCIPHER - select CRYPTO_SIMD select CRYPTO_TWOFISH_COMMON select CRYPTO_TWOFISH_X86_64 select CRYPTO_TWOFISH_X86_64_3WAY imply CRYPTO_XTS help
arch/x86/crypto/twofish_avx_glue.c +7 -14
@@ -11,11 +11,10 @@#include <linux/module.h> #include <linux/types.h> #include <linux/crypto.h> #include <linux/err.h> #include <crypto/algapi.h> -#include <crypto/internal/simd.h> #include <crypto/twofish.h> #include "twofish.h" #include "ecb_cbc_helpers.h"
@@ -72,27 +71,25 @@ static int cbc_decrypt(struct skcipher_request *req)CBC_WALK_END (); }static struct skcipher_alg twofish_algs[] = { { - .base.cra_name ="__ecb(twofish)" , - .base.cra_driver_name ="__ecb-twofish-avx" , + .base.cra_name ="ecb(twofish)" , + .base.cra_driver_name ="ecb-twofish-avx" , .base.cra_priority =400 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =TF_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct twofish_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =TF_MIN_KEY_SIZE , .max_keysize =TF_MAX_KEY_SIZE , .setkey = twofish_setkey_skcipher, .encrypt = ecb_encrypt, .decrypt = ecb_decrypt, }, { - .base.cra_name ="__cbc(twofish)" , - .base.cra_driver_name ="__cbc-twofish-avx" , + .base.cra_name ="cbc(twofish)" , + .base.cra_driver_name ="cbc-twofish-avx" , .base.cra_priority =400 , - .base.cra_flags =CRYPTO_ALG_INTERNAL , .base.cra_blocksize =TF_BLOCK_SIZE , .base.cra_ctxsize =sizeof (struct twofish_ctx ), .base.cra_module =THIS_MODULE , .min_keysize =TF_MIN_KEY_SIZE , .max_keysize =TF_MAX_KEY_SIZE ,
@@ -101,30 +98,26 @@ static struct skcipher_alg twofish_algs[] = { .encrypt = cbc_encrypt, .decrypt = cbc_decrypt, }, }; -static struct simd_skcipher_alg *twofish_simd_algs[ARRAY_SIZE (twofish_algs)]; -static int __init twofish_init (void ) {const char *feature_name;if (!cpu_has_xfeatures (XFEATURE_MASK_SSE |XFEATURE_MASK_YMM , &feature_name)) {pr_info ("CPU feature '%s' is not supported.\n" , feature_name);return -ENODEV ; } -return simd_register_skciphers_compat (twofish_algs, -ARRAY_SIZE (twofish_algs), - twofish_simd_algs); +return crypto_register_skciphers (twofish_algs, +ARRAY_SIZE (twofish_algs)); } static void __exit twofish_exit(void) { -simd_unregister_skciphers (twofish_algs,ARRAY_SIZE (twofish_algs), - twofish_simd_algs); +crypto_unregister_skciphers (twofish_algs,ARRAY_SIZE (twofish_algs)); }module_init (twofish_init);module_exit (twofish_exit);
Signature · 1 line
2.49.0
Herbert Xu
maintainerQuoted 9 lines · Eric Biggers
Stop wrapping skcipher and aead algorithms with the crypto simd helper (
crypto/simd.c). The only purpose of doing so was to work around x86 not always supporting kernel-mode FPU in softirqs. Specifically, if a hardirq interrupted a task context kernel-mode FPU section and then a softirqs were run at the end of that hardirq, those softirqs could not use kernel-mode FPU. This has now been fixed. In combination with the fact that the skcipher and aead APIs only support task and softirq contexts, these can now just use kernel-mode FPU unconditionally on x86.
Nice work!
So which platform still needs the simd wrapper? I believe arm/arm64 have both been fixed but we haven't finished removing the legacy simd code yet? Ard, would you be able to spare some cycles and finish the removal of simd on arm?
Darn, it looks like powerpc has just started using the simd wrapper so we need to fix it first before we can completely eliminate simd.
Michael/Danny, any chance you guys could implement something similar to what's been done on arm/x86 and make simd usable in softirqs?
Thanks,
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Quoted 12 lines · Herbert Xu
Quoted 9 lines · Eric Biggers
Stop wrapping skcipher and aead algorithms with the crypto simd helper (
crypto/simd.c). The only purpose of doing so was to work around x86 not always supporting kernel-mode FPU in softirqs. Specifically, if a hardirq interrupted a task context kernel-mode FPU section and then a softirqs were run at the end of that hardirq, those softirqs could not use kernel-mode FPU. This has now been fixed. In combination with the fact that the skcipher and aead APIs only support task and softirq contexts, these can now just use kernel-mode FPU unconditionally on x86.
Quoted 2 lines
Nice work!
Yeah good riddance.
Quoted 5 lines
So which platform still needs the simd wrapper? I believe arm/arm64 have both been fixed but we haven't finished removing the legacy simd code yet? Ard, would you be able to spare some cycles and finish the removal of simd on arm?
Removal of what, exactly?
Quoted 19 lines · Ard Biesheuvel
Quoted 12 lines · Herbert Xu
Quoted 9 lines · Eric Biggers
Stop wrapping skcipher and aead algorithms with the crypto simd helper (
crypto/simd.c). The only purpose of doing so was to work around x86 not always supporting kernel-mode FPU in softirqs. Specifically, if a hardirq interrupted a task context kernel-mode FPU section and then a softirqs were run at the end of that hardirq, those softirqs could not use kernel-mode FPU. This has now been fixed. In combination with the fact that the skcipher and aead APIs only support task and softirq contexts, these can now just use kernel-mode FPU unconditionally on x86.Quoted 2 lines
Nice work!
Yeah good riddance.
Quoted 7 lines
Quoted 5 lines
So which platform still needs the simd wrapper? I believe arm/arm64 have both been fixed but we haven't finished removing the legacy simd code yet? Ard, would you be able to spare some cycles and finish the removal of simd on arm?
Removal of what, exactly?
Ah, never mind - I see some calls on 32-bit ARM to simd_skcipher_create_compat(), which have become redundant now that SIMD is guaranteed to be available in softirq context.
Herbert Xu
maintainerQuoted 4 lines · Ard Biesheuvel
Ah, never mind - I see some calls on 32-bit ARM to
simd_skcipher_create_compat(), which have become redundant now that SIMD is guaranteed to be available in softirq context.
Thanks!
We could also remove all the calls to crypto_simd_usable in the Crypto API hashing code, e.g., arch/arm64/crypto/sha1-ce-glue.c.
For the lib/crypto code I think we should make it a rule to not allow any hardirq usage just like the Crypto API. Does anyone know of any uses of lib/crypto in a hardirq?
I thought /dev/random might do that but it looks like Jason has fixed it so that crypto code is no longer used in hardirqs:
commit e3e33fc2ea7fcefd0d761db9d6219f83b4248f5c
Author: Jason A. Donenfeld <[email protected]> Date: Fri May 6 18:30:51 2022 +0200
random: do not use input pool from hard IRQs
Cheers,
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Quoted 6 lines · Herbert Xu
Quoted 4 lines · Ard Biesheuvel
Ah, never mind - I see some calls on 32-bit ARM to
simd_skcipher_create_compat(), which have become redundant now that SIMD is guaranteed to be available in softirq context.
Quoted 8 lines
Thanks!
We could also remove all the calls to
crypto_simd_usablein the Crypto API hashing code, e.g.,arch/arm64/crypto/sha1-ce-glue.c.For the lib/crypto code I think we should make it a rule to not allow any hardirq usage just like the Crypto API. Does anyone know of any uses of lib/crypto in a hardirq?
This seems premature. crypto_shash is documented to be usable in any context. See the "Context:" comments in include/crypto/hash.h. Similarly, developers expect lib/ functions to be available in any context unless otherwise documented.
For skcipher and aead, there are more reasons why it makes sense to limit the contexts:
- skcipher_walk_first() already explicitly errors out if in_hardirq(), which already prevents them from working in hardirq context in most cases
- Even if it was allowed, the skcipher and aead APIs are already difficult to use correctly in a hardirq
- Because of how the crypto API is designed, it's not straightforward to fall back to generic skcipher and aead code in no-SIMD contexts
I could see the limitation being brought into crypto_shash too, though the crypto_shash documentation will need to be updated. The crypto API also really needs to be explicitly checking all its requirements. (It's probably finally time to add a kconfig option like CONFIG_DEBUG_CRYPTO to lib/Kconfig.debug, and put the extra assertions under there. Then they could be added without impacting performance for normal users.)
IMO, doing it for lib/ too would be going too far though. The lib/ functions should be easy to use and not have random requirements on the calling context. And since they're just functions, it's easy for them to fall back to the generic functions when needed. Also note that for very short inputs it can actually be faster to use no-SIMD code, as that avoids the overhead of a kernel-mode SIMD section. So the fallback sometimes exists anyway for that.
- Eric
Herbert Xu
maintainerQuoted 5 lines · Eric Biggers
This seems premature.
crypto_shashis documented to be usable in any context. See the "Context:" comments ininclude/crypto/hash.h. Similarly, developers expectlib/functions to be available in any context unless otherwise documented.
Doing slow computations in a hard IRQ is a bad idea. The whole point of a hard IRQ handler is to set a flag and defer everything to a different context.
Please show me one good reason why we should allow crypto in a hard IRQ.
Quoted 6 lines
IMO, doing it for
lib/too would be going too far though. Thelib/functions should be easy to use and not have random requirements on the calling context. And since they're just functions, it's easy for them to fall back to the generic functions when needed. Also note that for very short inputs it can actually be faster to use no-SIMD code, as that avoids the overhead of a kernel-mode SIMD section. So the fallback sometimes exists anyway for that.
We already disallow SIMD in hard IRQs anyway (may_use_simd is always false in that context). The only thing you could use is the generic implementation.
So making this change in lib/crypto does not take any functionality away. You could still invoke the generic lib/crypto code directly.
It does mean that we take away a completely useless check for people who are actually doing crypto because crypto work should never be done in a hard IRQ.
Cheers,
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Quoted 25 lines · Herbert Xu
Quoted 5 lines · Eric Biggers
This seems premature.
crypto_shashis documented to be usable in any context. See the "Context:" comments ininclude/crypto/hash.h. Similarly, developers expectlib/functions to be available in any context unless otherwise documented.Doing slow computations in a hard IRQ is a bad idea. The whole point of a hard IRQ handler is to set a flag and defer everything to a different context.
Please show me one good reason why we should allow crypto in a hard IRQ.
Quoted 6 lines
IMO, doing it for
lib/too would be going too far though. Thelib/functions should be easy to use and not have random requirements on the calling context. And since they're just functions, it's easy for them to fall back to the generic functions when needed. Also note that for very short inputs it can actually be faster to use no-SIMD code, as that avoids the overhead of a kernel-mode SIMD section. So the fallback sometimes exists anyway for that.We already disallow SIMD in hard IRQs anyway (
may_use_simdis always false in that context). The only thing you could use is the generic implementation.
Quoted 6 lines
So making this change in lib/crypto does not take any functionality away. You could still invoke the generic lib/crypto code directly.
It does mean that we take away a completely useless check for people who are actually doing crypto because crypto work should never be done in a hard IRQ.
It's not the 90s anymore. Crypto is fast now, and used ubiquitously.
And "crypto" doesn't necessarily mean a large operation. It can be hashing just a few bytes of data, for example.
Also as you know, the crypto API includes some non-cryptographic algorithms too.
BTW, x86 does allow SIMD in hardirq context in some cases.
Certainly agreed that crypto in hardirqs is something to be avoided in general, though.
So maybe your proposal is okay, if it's done properly.
The thing I actually have more of a problem with is that you tend to start making random API changes without any of the necessary prerequisites like updating documentation, or adding debug assertions to catch violations of new requirements. You've already started removing the fallbacks from shash (commit 3846c01d42526bc31), but neither of those things have been done. So we're currently in a weird state where the shash API is explicitly documented to work in all contexts, but you've broken that.
- Eric
Herbert Xu
maintainerPATCH 1/1 crypto: hash - Do not use shash in hard IRQs
Quoted 8 lines · Eric Biggers
The thing I actually have more of a problem with is that you tend to start making random API changes without any of the necessary prerequisites like updating documentation, or adding debug assertions to catch violations of new requirements. You've already started removing the fallbacks from shash (commit
3846c01d42526bc31), but neither of those things have been done. So we're currently in a weird state where the shash API is explicitly documented to work in all contexts, but you've broken that.
The documentation is easy enough to fix.
---8<---
Update the documentation to be consistent with the fact that shash may not be used in hard IRQs.
include/crypto/hash.h +9 -9
@@ -847,7 +847,7 @@ static inline void *shash_desc_ctx(struct shash_desc *desc)* cipher handle must point to a keyed message digest cipher in order for this * function to succeed. * -* Context: Any context. +* Context: Softirq or process context. * Return: 0 if the setting of the key was successful; < 0 if an error occurred */ int crypto_shash_setkey(struct crypto_shash *tfm, const u8 *key,
@@ -864,7 +864,7 @@ int crypto_shash_setkey(struct crypto_shash *tfm, const u8 *key,* crypto_shash_update and crypto_shash_final. The parameters have the same * meaning as discussed for those separate three functions. * -* Context: Any context. +* Context: Softirq or process context. * Return: 0 if the message digest creation was successful; < 0 if an error * occurred */
@@ -884,7 +884,7 @@ int crypto_shash_digest(struct shash_desc *desc, const u8 *data,* directly, and it allocates a hash descriptor on the stack internally. * Note that this stack allocation may be fairly large. * -* Context: Any context. +* Context: Softirq or process context. * Return: 0 on success; < 0 if an error occurred. */ int crypto_shash_tfm_digest(struct crypto_shash *tfm, const u8 *data,
@@ -902,7 +902,7 @@ int crypto_hash_digest(struct crypto_ahash *tfm, const u8 *data,* caller-allocated output buffer out which must have sufficient size (e.g. by * calling crypto_shash_descsize). * -* Context: Any context. +* Context: Softirq or process context. * Return: 0 if the export creation was successful; < 0 if an error occurred */ int crypto_shash_export (struct shash_desc *desc,void *out);
@@ -916,7 +916,7 @@ int crypto_shash_export(struct shash_desc *desc, void *out);* the input buffer. That buffer should have been generated with the * crypto_ahash_export function. * -* Context: Any context. +* Context: Softirq or process context. * Return: 0 if the import was successful; < 0 if an error occurred */ int crypto_shash_import (struct shash_desc *desc,const void *in);
@@ -929,7 +929,7 @@ int crypto_shash_import(struct shash_desc *desc, const void *in);* operational state handle. Any potentially existing state created by * previous operations is discarded. * -* Context: Any context. +* Context: Softirq or process context. * Return: 0 if the message digest initialization was successful; < 0 if an * error occurred */
@@ -951,7 +951,7 @@ static inline int crypto_shash_init(struct shash_desc *desc)* * Updates the message digest state of the operational state handle. * -* Context: Any context. +* Context: Softirq or process context. * Return: 0 if the message digest update was successful; < 0 if an error * occurred */
@@ -968,7 +968,7 @@ int crypto_shash_update(struct shash_desc *desc, const u8 *data,* into the output buffer. The caller must ensure that the output buffer is * large enough by using crypto_shash_digestsize. * -* Context: Any context. +* Context: Softirq or process context. * Return: 0 if the message digest creation was successful; < 0 if an error * occurred */
@@ -985,7 +985,7 @@ int crypto_shash_final(struct shash_desc *desc, u8 *out);* crypto_shash_update and crypto_shash_final. The parameters have the same * meaning as discussed for those separate functions. * -* Context: Any context. +* Context: Softirq or process context. * Return: 0 if the message digest creation was successful; < 0 if an error * occurred */
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Herbert Xu
maintainerQuoted 2 lines · Eric Biggers
It's not the 90s anymore. Crypto is fast now, and used ubiquitously.
I have to say that you've done a great job in improving crypto performance on x86 and I'm very pleased with being able to encrypt 256 bytes in just over 100 CPU cycles and doing a whole page takes less than 1000 cycles.
But this is only possible with SIMD instructions which we do not support in hard IRQ context.
Cheers,
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Quoted 4 lines · Herbert Xu
Quoted 2 lines · Eric Biggers
It's not the 90s anymore. Crypto is fast now, and used ubiquitously.
Quoted 8 lines
I have to say that you've done a great job in improving crypto performance on x86 and I'm very pleased with being able to encrypt 256 bytes in just over 100 CPU cycles and doing a whole page takes less than 1000 cycles.
But this is only possible with SIMD instructions which we do not support in hard IRQ context.
What? Take a look at siphash_1u32(), for example. That is crypto, and it is fast. It doesn't use, or need to use, SIMD instructions.
Also, riscv has scalar AES instructions. (They aren't used by the kernel yet, but they could be. The CRC code already uses scalar carryless multiplication.)
Obviously, it's also very common to really need the SIMD unit. That's the way it is. But those are not all cases.
Also, as I said already, x86 does support SIMD instructions in hardirq context in some cases. Whether anyone actually uses that, I don't know, but it is explicitly supported. Check out irq_fpu_usable().
- Eric
Herbert Xu
maintainerBanning crypto in hardirq context (was: [PATCH v2 0/9] crypto: x86 - stop using the SIMD helper)
Quoted 3 lines · Eric Biggers
Also, riscv has scalar AES instructions. (They aren't used by the kernel yet, but they could be. The CRC code already uses scalar carryless multiplication.)
It still doesn't mean that it's a good idea to use AES in a hard IRQ handler, especially if the code is meant to be portable.
Quoted 3 lines
Also, as I said already, x86 does support SIMD instructions in hardirq context in some cases. Whether anyone actually uses that, I don't know, but it is explicitly supported. Check out
irq_fpu_usable().
This is more of an accident than some deliberate strategy of supporting FPU usage in hard IRQs. This test was initially added for aesni:
commit 54b6a1bd5364aca95cd6ffae00f2b64c6511122c
Author: Ying Huang <[email protected]> Date: Sun Jan 18 16:28:34 2009 +1100
crypto: aes-ni - Add support to Intel AES-NI instructions for x86_64 platform
It was then improved by:
Author: Linus Torvalds <[email protected]> Date: Mon Feb 13 13:56:14 2012 -0800
i387: make irq_fpu_usable() tests more robust
Some code - especially the crypto layer - wants to use the x86 FP/MMX/AVX register set in what may be interrupt (typically softirq) context.
At no point was there any intention of using this in a hardirq context.
Until such a time when you have a valid application for using lib/crypto code in a hardirq context, I don't think we should be supporting that at the expense of real users who are in process/softirq context only.
Cheers,
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: Banning crypto in hardirq context (was: [PATCH v2 0/9] crypto: x86 - stop using the SIMD helper)
Quoted 33 lines · Herbert Xu
Quoted 3 lines · Eric Biggers
Also, riscv has scalar AES instructions. (They aren't used by the kernel yet, but they could be. The CRC code already uses scalar carryless multiplication.)
It still doesn't mean that it's a good idea to use AES in a hard IRQ handler, especially if the code is meant to be portable.
Quoted 3 lines
Also, as I said already, x86 does support SIMD instructions in hardirq context in some cases. Whether anyone actually uses that, I don't know, but it is explicitly supported. Check out
irq_fpu_usable().This is more of an accident than some deliberate strategy of supporting FPU usage in hard IRQs. This test was initially added for aesni:
commit
54b6a1bd5364aca95cd6ffae00f2b64c6511122cAuthor: Ying Huang <[email protected]> Date: Sun Jan 18 16:28:34 2009 +1100crypto: aes-ni - Add support to Intel AES-NI instructions for
x86_64platformIt was then improved by:
Author: Linus Torvalds <[email protected]> Date: Mon Feb 13 13:56:14 2012 -0800i387: make
irq_fpu_usable()tests more robustSome code - especially the crypto layer - wants to use the x86 FP/MMX/AVX register set in what may be interrupt (typically softirq) context.
Quoted 7 lines
At no point was there any intention of using this in a hardirq context.
Until such a time when you have a valid application for using lib/crypto code in a hardirq context, I don't think we should be supporting that at the expense of real users who are in process/softirq context only.
Whatever. We agree that "crypto in hardirq" is not a good idea in general. I'm just pointing out that there are certain cases, like SipHash used in a hash table, where it easily could happen and would be fine. And all the shash and crypto library functions currently work in any context, unlike e.g. skcipher and aead which do not. You seem to be trying to claim that it was never supported, but that is incorrect. Making it unsupported would be a change that needs to be properly documented (the functions would no longer be simply "Any context") *and* have proper debug assertions added to enforce it and prevent usage errors. But in a lot of cases there is also no reason to even add that restriction. I'm not sure why you're so eager to make the library functions harder to use.
- Eric
Herbert Xu
maintainerPATCH 1/1 crypto: x86/chacha - Remove SIMD fallback path
Quoted 3 lines · Eric Biggers
But in a lot of cases there is also no reason to even add that restriction. I'm not sure why you're so eager to make the library functions harder to use.
I have no intention of making any changes to siphash. It doesn't even use SIMD.
All I want to do is get rid of the crypto_simd_usable() fallback paths that we currently have in arch/x86/crypto. This code is never used in hardirq context (and should never be).
For example:
---8<---
Get rid of the fallback path as SIMD is now always usable in softirq context.
arch/x86/crypto/chacha_glue.c +11 -35
@@ -6,9 +6,7 @@* Copyright (C) 2015 Martin Willi */ -#include <crypto/algapi.h> #include <crypto/internal/chacha.h> -#include <crypto/internal/simd.h> #include <crypto/internal/skcipher.h> #include <linux/kernel.h> #include <linux/module.h>
@@ -35,7 +33,6 @@ asmlinkage void chacha_4block_xor_avx512vl(u32 *state, u8 *dst, const u8 *src, asmlinkage void chacha_8block_xor_avx512vl(u32 *state, u8 *dst, const u8 *src,unsigned int len,int nrounds); -static __ro_after_init DEFINE_STATIC_KEY_FALSE (chacha_use_simd);static __ro_after_init DEFINE_STATIC_KEY_FALSE (chacha_use_avx2);static __ro_after_init DEFINE_STATIC_KEY_FALSE (chacha_use_avx512vl);
@@ -123,23 +120,15 @@ static void chacha_dosimd(u32 *state, u8 *dst, const u8 *src,void hchacha_block_arch (const u32 *state,u32 *stream,int nrounds) { -if (!static_branch_likely (&chacha_use_simd) || !crypto_simd_usable ()) { -hchacha_block_generic (state, stream, nrounds); - }else { -kernel_fpu_begin (); -hchacha_block_ssse3 (state, stream, nrounds); -kernel_fpu_end (); - } +kernel_fpu_begin (); +hchacha_block_ssse3 (state, stream, nrounds); +kernel_fpu_end (); }EXPORT_SYMBOL (hchacha_block_arch);void chacha_crypt_arch (u32 *state,u8 *dst,const u8 *src,unsigned int bytes,int nrounds) { -if (!static_branch_likely (&chacha_use_simd) || !crypto_simd_usable () || - bytes <=CHACHA_BLOCK_SIZE ) -return chacha_crypt_generic (state, dst, src, bytes, nrounds); - do { unsigned int todo = min_t(unsigned int, bytes, SZ_4K);
@@ -171,18 +160,11 @@ static int chacha_simd_stream_xor(struct skcipher_request *req,if (nbytes < walk.total) nbytes =round_down (nbytes, walk.stride); -if (!static_branch_likely (&chacha_use_simd) || - !crypto_simd_usable ()) { -chacha_crypt_generic (state, walk.dst.virt.addr, - walk.src.virt.addr, nbytes, - ctx->nrounds); - }else { -kernel_fpu_begin (); -chacha_dosimd (state, walk.dst.virt.addr, - walk.src.virt.addr, nbytes, - ctx->nrounds); -kernel_fpu_end (); - } +kernel_fpu_begin (); +chacha_dosimd (state, walk.dst.virt.addr, + walk.src.virt.addr, nbytes, + ctx->nrounds); +kernel_fpu_end (); err =skcipher_walk_done (&walk, walk.nbytes - nbytes); }
@@ -207,13 +189,9 @@ static int xchacha_simd(struct skcipher_request *req)chacha_init (state, ctx->key, req->iv); -if (req->cryptlen >CHACHA_BLOCK_SIZE &&crypto_simd_usable ()) { -kernel_fpu_begin (); -hchacha_block_ssse3 (state, subctx.key, ctx->nrounds); -kernel_fpu_end (); - }else { -hchacha_block_generic (state, subctx.key, ctx->nrounds); - } +kernel_fpu_begin (); +hchacha_block_ssse3 (state, subctx.key, ctx->nrounds); +kernel_fpu_end (); subctx.nrounds = ctx->nrounds;memcpy (&real_iv[0 ], req->iv +24 ,8 );
@@ -275,8 +253,6 @@ static int __init chacha_simd_mod_init(void)if (!boot_cpu_has (X86_FEATURE_SSSE3 ))return 0 ; -static_branch_enable (&chacha_use_simd); -if (boot_cpu_has (X86_FEATURE_AVX ) &&boot_cpu_has (X86_FEATURE_AVX2 ) &&cpu_has_xfeatures (XFEATURE_MASK_SSE |XFEATURE_MASK_YMM ,NULL )) {
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: x86/chacha - Remove SIMD fallback path
Quoted 12 lines · Herbert Xu
Quoted 3 lines · Eric Biggers
But in a lot of cases there is also no reason to even add that restriction. I'm not sure why you're so eager to make the library functions harder to use.
I have no intention of making any changes to siphash. It doesn't even use SIMD.
All I want to do is get rid of the
crypto_simd_usable()fallback paths that we currently have inarch/x86/crypto. This code is never used in hardirq context (and should never be).
Quoted 8 lines
For example:
---8<---
Get rid of the fallback path as SIMD is now always usable in softirq context.Signed-off-by Herbert Xu <[email protected]>
It looks like this broken patch already got applied for some reason.
First, there doesn't seem to be agreement yet that the library functions should have requirements on the calling context.
Second, your patch made unrelated changes that deleted the checks for SSSE3 support. Thus dropping support for CPUs that don't support SSSE3.
- Eric
Herbert Xu
maintainerPATCH 1/1 crypto: x86/chacha - Restore SSSE3 fallback path
Quoted 3 lines · Eric Biggers
First, there doesn't seem to be agreement yet that the library functions should have requirements on the calling context.
Do you have a real example of hard IRQ usage for chacha? Not some imaginary post-crash scenario that ends up calling into generic code.
And if you really wanted to do that, it's much better to fix up kernel_fpu_begin to support hard IRQs rather than adding useless may_use_simd() checks all over the place.
Quoted 2 lines
Second, your patch made unrelated changes that deleted the checks for SSSE3 support. Thus dropping support for CPUs that don't support SSSE3.
Sorry. That was an oversight.
---8<---
The chacha_use_simd static branch is required for x86 machines that lack SSSE3 support. Restore it and the generic fallback code.
arch/x86/crypto/chacha_glue.c +17 -5
@@ -5,11 +5,12 @@* Copyright (C) 2015 Martin Willi */ +#include <asm/simd.h> #include <crypto/chacha.h> +#include <linux/jump_label.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/sizes.h> -#include <asm/simd.h> asmlinkage void chacha_block_xor_ssse3(u32 *state, u8 *dst, const u8 *src,unsigned int len,int nrounds);
@@ -31,6 +32,7 @@ asmlinkage void chacha_4block_xor_avx512vl(u32 *state, u8 *dst, const u8 *src, asmlinkage void chacha_8block_xor_avx512vl(u32 *state, u8 *dst, const u8 *src,unsigned int len,int nrounds); +static __ro_after_init DEFINE_STATIC_KEY_FALSE (chacha_use_simd);static __ro_after_init DEFINE_STATIC_KEY_FALSE (chacha_use_avx2);static __ro_after_init DEFINE_STATIC_KEY_FALSE (chacha_use_avx512vl);
@@ -117,15 +119,23 @@ static void chacha_dosimd(u32 *state, u8 *dst, const u8 *src,void hchacha_block_arch (const u32 *state,u32 *stream,int nrounds) { -kernel_fpu_begin (); -hchacha_block_ssse3 (state, stream, nrounds); -kernel_fpu_end (); +if (!static_branch_likely (&chacha_use_simd)) { +hchacha_block_generic (state, stream, nrounds); + }else { +kernel_fpu_begin (); +hchacha_block_ssse3 (state, stream, nrounds); +kernel_fpu_end (); + } }EXPORT_SYMBOL (hchacha_block_arch);void chacha_crypt_arch (u32 *state,u8 *dst,const u8 *src,unsigned int bytes,int nrounds) { +if (!static_branch_likely (&chacha_use_simd) || + bytes <=CHACHA_BLOCK_SIZE ) +return chacha_crypt_generic (state, dst, src, bytes, nrounds); + do { unsigned int todo = min_t(unsigned int, bytes, SZ_4K);
@@ -142,7 +152,7 @@ EXPORT_SYMBOL(chacha_crypt_arch);bool chacha_is_arch_optimized (void ) { -return true; +return static_key_enabled (&chacha_use_simd); }EXPORT_SYMBOL (chacha_is_arch_optimized);
@@ -151,6 +161,8 @@ static int __init chacha_simd_mod_init(void)if (!boot_cpu_has (X86_FEATURE_SSSE3 ))return 0 ; +static_branch_enable (&chacha_use_simd); +if (boot_cpu_has (X86_FEATURE_AVX ) &&boot_cpu_has (X86_FEATURE_AVX2 ) &&cpu_has_xfeatures (XFEATURE_MASK_SSE |XFEATURE_MASK_YMM ,NULL )) {
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: Banning crypto in hardirq context (was: [PATCH v2 0/9] crypto: x86 - stop using the SIMD helper)
In reply to Eric BiggersQuoted 43 lines · Eric Biggers
Quoted 33 lines · Herbert Xu
Quoted 3 lines · Eric Biggers
Also, riscv has scalar AES instructions. (They aren't used by the kernel yet, but they could be. The CRC code already uses scalar carryless multiplication.)
It still doesn't mean that it's a good idea to use AES in a hard IRQ handler, especially if the code is meant to be portable.
Quoted 3 lines
Also, as I said already, x86 does support SIMD instructions in hardirq context in some cases. Whether anyone actually uses that, I don't know, but it is explicitly supported. Check out
irq_fpu_usable().This is more of an accident than some deliberate strategy of supporting FPU usage in hard IRQs. This test was initially added for aesni:
commit
54b6a1bd5364aca95cd6ffae00f2b64c6511122cAuthor: Ying Huang <[email protected]> Date: Sun Jan 18 16:28:34 2009 +1100crypto: aes-ni - Add support to Intel AES-NI instructions for
x86_64platformIt was then improved by:
Author: Linus Torvalds <[email protected]> Date: Mon Feb 13 13:56:14 2012 -0800i387: make
irq_fpu_usable()tests more robustSome code - especially the crypto layer - wants to use the x86 FP/MMX/AVX register set in what may be interrupt (typically softirq) context.
Quoted 7 lines
At no point was there any intention of using this in a hardirq context.
Until such a time when you have a valid application for using lib/crypto code in a hardirq context, I don't think we should be supporting that at the expense of real users who are in process/softirq context only.
Quoted 11 lines
Whatever. We agree that "crypto in hardirq" is not a good idea in general. I'm just pointing out that there are certain cases, like SipHash used in a hash table, where it easily could happen and would be fine. And all the shash and crypto library functions currently work in any context, unlike e.g. skcipher and aead which do not. You seem to be trying to claim that it was never supported, but that is incorrect. Making it unsupported would be a change that needs to be properly documented (the functions would no longer be simply "Any context") *and* have proper debug assertions added to enforce it and prevent usage errors. But in a lot of cases there is also no reason to even add that restriction. I'm not sure why you're so eager to make the library functions harder to use.
Agree with Eric.
There may be cases where some error condition (machine check etc) is hit while running in hard IRQ context or with IRQs disabled, and the code that produces the diagnostic, writes to pstore, generates the QR code for etc etc may actually be where the library calls to crc32 etc originate from. So pedantically disallowing that rather than falling back to a non-SIMD code path make things worse, because now, the original diagnostic may get lost while the only information left to debug the issue is an OOPS complaining about a library call in hard IRQ context.
So while I agree that knowingly invoking library interfaces with IRQs disabled should be avoided, that is just a variation on the general adage that IRQs should only be disabled when absolutely necessary. But that necessity may derive from a condition that exists one or several layers up.
Herbert Xu
maintainerQuoted 63 lines · Eric Biggers
Patches 2-9 are almost identical to
https://lore.kernel.org/r/[email protected]/
but now split into multiple patches. Patch 1 is just a resend of
https://lore.kernel.org/r/[email protected]/
which is needed for the series to apply cleanly but is otherwise unrelated. Description of patches 2-9 follows:Stop wrapping skcipher and aead algorithms with the crypto SIMD helper (
crypto/simd.c). The only purpose of doing so was to work around x86 not always supporting kernel-mode FPU in softirqs. Specifically, if a hardirq interrupted a task context kernel-mode FPU section and then a softirqs were run at the end of that hardirq, those softirqs could not use kernel-mode FPU. This has now been fixed. In combination with the fact that the skcipher and aead APIs only support task and softirq contexts, these can now just use kernel-mode FPU unconditionally on x86.This simplifies the code and improves performance.
En/decryption gets at least somewhat faster for everyone, since the crypto API functions such as
crypto_skcipher_encrypt()now go directly to the underlying algorithm rather than taking a detour throughcrypto/simd.cwhich involved an extra indirect call. For example, on a Ryzen 9 9950X desktop processor, AES-256-XTS is now 23% faster for 512-byte messages and 7% faster for 4096-byte messages (when accessed throughcrypto_skcipher_encrypt()orcrypto_skcipher_decrypt()).There's also a much larger performance improvement for crypto API users that only support synchronous algorithms. These users will now actually use the x86 SIMD (e.g. AES-NI or VAES) optimized en/decryption modes, which they couldn't before because they were marked as asynchronous.
Eric Biggers (9):
crypto: x86/aes - drop theavx10_256AES-XTS and AES-CTR code
crypto: x86/aegis - stop using the SIMD helper
crypto: x86/aes - stop using the SIMD helper
crypto: x86/aria - stop using the SIMD helper
crypto: x86/camellia - stop using the SIMD helper
crypto: x86/cast - stop using the SIMD helper
crypto: x86/serpent - stop using the SIMD helper
crypto: x86/sm4 - stop using the SIMD helper
crypto: x86/twofish - stop using the SIMD helperarch/x86/crypto/Kconfig | 14 -- arch/x86/crypto/aegis128-aesni-glue.c | 13 +- arch/x86/crypto/aes-ctr-avx-x86_64.S | 47 ++---- arch/x86/crypto/aes-xts-avx-x86_64.S | 118 ++++++-------- arch/x86/crypto/aesni-intel_glue.c | 174 ++++++++------------- arch/x86/crypto/aria_aesni_avx2_glue.c | 22 +-- arch/x86/crypto/aria_aesni_avx_glue.c | 20 +-- arch/x86/crypto/aria_gfni_avx512_glue.c | 22 +-- arch/x86/crypto/camellia_aesni_avx2_glue.c | 21 +-- arch/x86/crypto/camellia_aesni_avx_glue.c | 21 +-- arch/x86/crypto/cast5_avx_glue.c | 21 +-- arch/x86/crypto/cast6_avx_glue.c | 20 +-- arch/x86/crypto/serpent_avx2_glue.c | 21 +-- arch/x86/crypto/serpent_avx_glue.c | 21 +-- arch/x86/crypto/serpent_sse2_glue.c | 21 +-- arch/x86/crypto/sm4_aesni_avx2_glue.c | 31 ++-- arch/x86/crypto/sm4_aesni_avx_glue.c | 31 ++-- arch/x86/crypto/twofish_avx_glue.c | 21 +-- 18 files changed, 227 insertions(+), 432 deletions(-)
Quoted 1 line
base-commit:
91e5bfe317d8f8471fbaa3e70cf66cae1314a516
All applied. Thanks.
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt