On Mon, 30 Nov 2020 at 10:43, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > On Mon, 30 Nov 2020 at 10:37, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > > > Hi Ard, > > > > On Tue, Nov 17, 2020 at 2:38 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > > This series supersedes [0] '[PATCH] crypto: aegis128/neon - optimize tail > > > block handling', which is included as patch #3 here, but hasn't been > > > modified substantially. > > > > > > Patch #1 should probably go to -stable, even though aegis128 does not appear > > > to be widely used. > > > > > > Patches #2 and #3 improve the SIMD code paths. > > > > > > Patch #4 enables fuzz testing for the SIMD code by registering the generic > > > code as a separate driver if the SIMD code path is enabled. > > > > > > Changes since v2: > > > - add Ondrej's ack to #1 > > > - fix an issue spotted by Ondrej in #4 where the generic code path would still > > > use some of the SIMD helpers > > > > > > Cc: Ondrej Mosnacek <omosnacek@xxxxxxxxx> > > > Cc: Eric Biggers <ebiggers@xxxxxxxxxx> > > > > > > [0] https://lore.kernel.org/linux-crypto/20201107195516.13952-1-ardb@xxxxxxxxxx/ > > > > > > Ard Biesheuvel (4): > > > crypto: aegis128 - wipe plaintext and tag if decryption fails > > > crypto: aegis128/neon - optimize tail block handling > > > crypto: aegis128/neon - move final tag check to SIMD domain > > > > crypto/aegis128-core.c: In function ‘crypto_aegis128_decrypt’: > > crypto/aegis128-core.c:454:40: error: passing argument 2 of > > ‘crypto_aegis128_process_crypt’ from incompatible pointer type > > [-Werror=incompatible-pointer-types] > > 454 | crypto_aegis128_process_crypt(NULL, req, &walk, > > | ^~~ > > | | > > | struct aead_request * > > crypto/aegis128-core.c:335:29: note: expected ‘struct skcipher_walk *’ > > but argument is of type ‘struct aead_request *’ > > 335 | struct skcipher_walk *walk, > > | ~~~~~~~~~~~~~~~~~~~~~~^~~~ > > crypto/aegis128-core.c:454:45: error: passing argument 3 of > > ‘crypto_aegis128_process_crypt’ from incompatible pointer type > > [-Werror=incompatible-pointer-types] > > 454 | crypto_aegis128_process_crypt(NULL, req, &walk, > > | ^~~~~ > > | | > > | struct skcipher_walk * > > crypto/aegis128-core.c:336:14: note: expected ‘void (*)(struct > > aegis_state *, u8 *, const u8 *, unsigned int)’ {aka ‘void (*)(struct > > aegis_state *, unsigned char *, const unsigned char *, unsigned int)’} > > but argument is of type ‘struct skcipher_walk *’ > > 336 | void (*crypt)(struct aegis_state *state, > > | ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > 337 | u8 *dst, const u8 *src, > > | ~~~~~~~~~~~~~~~~~~~~~~~ > > 338 | unsigned int size)) > > | ~~~~~~~~~~~~~~~~~~ > > crypto/aegis128-core.c:454:4: error: too many arguments to function > > ‘crypto_aegis128_process_crypt’ > > 454 | crypto_aegis128_process_crypt(NULL, req, &walk, > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > crypto/aegis128-core.c:334:5: note: declared here > > 334 | int crypto_aegis128_process_crypt(struct aegis_state *state, > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > cc1: some warnings being treated as errors > > make[1]: *** [scripts/Makefile.build:283: crypto/aegis128-core.o] Error 1 > > > > > crypto: aegis128 - expose SIMD code path as separate driver > > > > Fixes the above, but causes > > > > ERROR: modpost: "crypto_aegis128_update_simd" [crypto/aegis128.ko] undefined! > > > > as reported by noreply@xxxxxxxxxxxxxx for m68k/defconfig and > > m68k/sun3_defconfig. > > (neon depends on arm). > > > > Thanks for the report. > > It seems like GCC is not optimizing away calls to routines that are > unreachable. Which GCC version are you using? Also, mind checking whether the below works around this? diff --git a/crypto/aegis128-core.c b/crypto/aegis128-core.c index 2b05f79475d3..89dc1c559689 100644 --- a/crypto/aegis128-core.c +++ b/crypto/aegis128-core.c @@ -89,7 +89,7 @@ static void crypto_aegis128_update_a(struct aegis_state *state, const union aegis_block *msg, bool do_simd) { - if (do_simd) { + if (IS_ENABLED(CONFIG_CRYPTO_AEGIS128_SIMD) && do_simd) { crypto_aegis128_update_simd(state, msg); return; } @@ -101,7 +101,7 @@ static void crypto_aegis128_update_a(struct aegis_state *state, static void crypto_aegis128_update_u(struct aegis_state *state, const void *msg, bool do_simd) { - if (do_simd) { + if (IS_ENABLED(CONFIG_CRYPTO_AEGIS128_SIMD) && do_simd) { crypto_aegis128_update_simd(state, msg); return; }
