Hi Ard, On Mon, Nov 30, 2020 at 10:45 AM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > 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: > > > 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 The code is not unreachable. Both crypto_aegis128_encrypt_simd() and crypto_aegis128_decrypt_simd() call crypto_aegis128_process_ad(..., true); > > unreachable. Which GCC version are you using? I'm using 9.3.0, Kisskb is using 8.1.0. > 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; > } Thanks, that fixes the build for me. Tested-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
