* [PATCH] linux-next 20190731 - aegis128-core.c fails to build
@ 2019-08-01 4:51 Valdis Klētnieks
2019-08-01 5:01 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Valdis Klētnieks @ 2019-08-01 4:51 UTC (permalink / raw)
To: Ard Biesheuvel, Herbert Xu; +Cc: David S. Miller, linux-crypto, linux-kernel
The recent NEON SIMD patches break the build if CONFIG_CRYPTO_AEGIS128_SIMD isn't set:
MODPOST 558 modules
ERROR: "crypto_aegis128_decrypt_chunk_simd" [crypto/aegis128.ko] undefined!
ERROR: "crypto_aegis128_update_simd" [crypto/aegis128.ko] undefined!
ERROR: "crypto_aegis128_encrypt_chunk_simd" [crypto/aegis128.ko] undefined!
make[1]: *** [scripts/Makefile.modpost:105: modules-modpost] Error 1
make: *** [Makefile:1299: modules] Error 2
Add proper definitions and stubs to aegis.h so it builds both ways. This
necessitated moving other stuff from aegis128-core.c to aegis.h so things were
defined in the proper order.
Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
---
diff --git a/crypto/aegis.h b/crypto/aegis.h
index 4d56a85aea49..50a7496ca4ae 100644
--- a/crypto/aegis.h
+++ b/crypto/aegis.h
@@ -13,6 +13,11 @@
#include <linux/bitops.h>
#include <linux/types.h>
+#define AEGIS128_NONCE_SIZE 16
+#define AEGIS128_STATE_BLOCKS 5
+#define AEGIS128_KEY_SIZE 16
+#define AEGIS128_MIN_AUTH_SIZE 8
+#define AEGIS128_MAX_AUTH_SIZE 16
#define AEGIS_BLOCK_SIZE 16
union aegis_block {
@@ -21,6 +26,39 @@ union aegis_block {
u8 bytes[AEGIS_BLOCK_SIZE];
};
+struct aegis_state {
+ union aegis_block blocks[AEGIS128_STATE_BLOCKS];
+};
+
+struct aegis_ctx {
+ union aegis_block key;
+};
+
+struct aegis128_ops {
+ int (*skcipher_walk_init)(struct skcipher_walk *walk,
+ struct aead_request *req, bool atomic);
+
+ void (*crypt_chunk)(struct aegis_state *state, u8 *dst,
+ const u8 *src, unsigned int size);
+};
+
+
+#ifdef CONFIG_CRYPTO_AEGIS128_SIMD
+bool crypto_aegis128_have_simd(void);
+void crypto_aegis128_update_simd(struct aegis_state *state, const void *msg);
+void crypto_aegis128_encrypt_chunk_simd(struct aegis_state *state, u8 *dst,
+ const u8 *src, unsigned int size);
+void crypto_aegis128_decrypt_chunk_simd(struct aegis_state *state, u8 *dst,
+ const u8 *src, unsigned int size);
+#else
+static inline bool crypto_aegis128_have_simd(void) { return false; }
+static inline void crypto_aegis128_update_simd(struct aegis_state *state, const void *msg) { }
+static inline void crypto_aegis128_encrypt_chunk_simd(struct aegis_state *state, u8 *dst,
+ const u8 *src, unsigned int size) { }
+static inline void crypto_aegis128_decrypt_chunk_simd(struct aegis_state *state, u8 *dst,
+ const u8 *src, unsigned int size) { }
+#endif
+
#define AEGIS_BLOCK_ALIGN (__alignof__(union aegis_block))
#define AEGIS_ALIGNED(p) IS_ALIGNED((uintptr_t)p, AEGIS_BLOCK_ALIGN)
diff --git a/crypto/aegis128-core.c b/crypto/aegis128-core.c
index f815b4685156..8b738128a921 100644
--- a/crypto/aegis128-core.c
+++ b/crypto/aegis128-core.c
@@ -20,37 +20,8 @@
#include "aegis.h"
-#define AEGIS128_NONCE_SIZE 16
-#define AEGIS128_STATE_BLOCKS 5
-#define AEGIS128_KEY_SIZE 16
-#define AEGIS128_MIN_AUTH_SIZE 8
-#define AEGIS128_MAX_AUTH_SIZE 16
-
-struct aegis_state {
- union aegis_block blocks[AEGIS128_STATE_BLOCKS];
-};
-
-struct aegis_ctx {
- union aegis_block key;
-};
-
-struct aegis128_ops {
- int (*skcipher_walk_init)(struct skcipher_walk *walk,
- struct aead_request *req, bool atomic);
-
- void (*crypt_chunk)(struct aegis_state *state, u8 *dst,
- const u8 *src, unsigned int size);
-};
-
static bool have_simd;
-bool crypto_aegis128_have_simd(void);
-void crypto_aegis128_update_simd(struct aegis_state *state, const void *msg);
-void crypto_aegis128_encrypt_chunk_simd(struct aegis_state *state, u8 *dst,
- const u8 *src, unsigned int size);
-void crypto_aegis128_decrypt_chunk_simd(struct aegis_state *state, u8 *dst,
- const u8 *src, unsigned int size);
-
static void crypto_aegis128_update(struct aegis_state *state)
{
union aegis_block tmp;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] linux-next 20190731 - aegis128-core.c fails to build
2019-08-01 4:51 [PATCH] linux-next 20190731 - aegis128-core.c fails to build Valdis Klētnieks
@ 2019-08-01 5:01 ` Ard Biesheuvel
2019-08-01 5:46 ` Valdis Klētnieks
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2019-08-01 5:01 UTC (permalink / raw)
To: Valdis Klētnieks, Arnd Bergmann
Cc: Herbert Xu, David S. Miller,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
Linux Kernel Mailing List
(+ Arnd)
On Thu, 1 Aug 2019 at 07:52, Valdis Klētnieks <valdis.kletnieks@vt.edu> wrote:
>
> The recent NEON SIMD patches break the build if CONFIG_CRYPTO_AEGIS128_SIMD isn't set:
>
> MODPOST 558 modules
> ERROR: "crypto_aegis128_decrypt_chunk_simd" [crypto/aegis128.ko] undefined!
> ERROR: "crypto_aegis128_update_simd" [crypto/aegis128.ko] undefined!
> ERROR: "crypto_aegis128_encrypt_chunk_simd" [crypto/aegis128.ko] undefined!
> make[1]: *** [scripts/Makefile.modpost:105: modules-modpost] Error 1
> make: *** [Makefile:1299: modules] Error 2
>
> Add proper definitions and stubs to aegis.h so it builds both ways. This
> necessitated moving other stuff from aegis128-core.c to aegis.h so things were
> defined in the proper order.
>
> Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
Which compiler version are you using? All references to the
crypt_aegis128_xx_simd() routines should disappear if
CONFIG_CRYPTO_AEGIS128_SIMD is not set (in which case have_simd will
always be false and so the compiler should optimize away those calls).
> ---
> diff --git a/crypto/aegis.h b/crypto/aegis.h
> index 4d56a85aea49..50a7496ca4ae 100644
> --- a/crypto/aegis.h
> +++ b/crypto/aegis.h
> @@ -13,6 +13,11 @@
> #include <linux/bitops.h>
> #include <linux/types.h>
>
> +#define AEGIS128_NONCE_SIZE 16
> +#define AEGIS128_STATE_BLOCKS 5
> +#define AEGIS128_KEY_SIZE 16
> +#define AEGIS128_MIN_AUTH_SIZE 8
> +#define AEGIS128_MAX_AUTH_SIZE 16
> #define AEGIS_BLOCK_SIZE 16
>
> union aegis_block {
> @@ -21,6 +26,39 @@ union aegis_block {
> u8 bytes[AEGIS_BLOCK_SIZE];
> };
>
> +struct aegis_state {
> + union aegis_block blocks[AEGIS128_STATE_BLOCKS];
> +};
> +
> +struct aegis_ctx {
> + union aegis_block key;
> +};
> +
> +struct aegis128_ops {
> + int (*skcipher_walk_init)(struct skcipher_walk *walk,
> + struct aead_request *req, bool atomic);
> +
> + void (*crypt_chunk)(struct aegis_state *state, u8 *dst,
> + const u8 *src, unsigned int size);
> +};
> +
> +
> +#ifdef CONFIG_CRYPTO_AEGIS128_SIMD
> +bool crypto_aegis128_have_simd(void);
> +void crypto_aegis128_update_simd(struct aegis_state *state, const void *msg);
> +void crypto_aegis128_encrypt_chunk_simd(struct aegis_state *state, u8 *dst,
> + const u8 *src, unsigned int size);
> +void crypto_aegis128_decrypt_chunk_simd(struct aegis_state *state, u8 *dst,
> + const u8 *src, unsigned int size);
> +#else
> +static inline bool crypto_aegis128_have_simd(void) { return false; }
> +static inline void crypto_aegis128_update_simd(struct aegis_state *state, const void *msg) { }
> +static inline void crypto_aegis128_encrypt_chunk_simd(struct aegis_state *state, u8 *dst,
> + const u8 *src, unsigned int size) { }
> +static inline void crypto_aegis128_decrypt_chunk_simd(struct aegis_state *state, u8 *dst,
> + const u8 *src, unsigned int size) { }
> +#endif
> +
> #define AEGIS_BLOCK_ALIGN (__alignof__(union aegis_block))
> #define AEGIS_ALIGNED(p) IS_ALIGNED((uintptr_t)p, AEGIS_BLOCK_ALIGN)
>
> diff --git a/crypto/aegis128-core.c b/crypto/aegis128-core.c
> index f815b4685156..8b738128a921 100644
> --- a/crypto/aegis128-core.c
> +++ b/crypto/aegis128-core.c
> @@ -20,37 +20,8 @@
>
> #include "aegis.h"
>
> -#define AEGIS128_NONCE_SIZE 16
> -#define AEGIS128_STATE_BLOCKS 5
> -#define AEGIS128_KEY_SIZE 16
> -#define AEGIS128_MIN_AUTH_SIZE 8
> -#define AEGIS128_MAX_AUTH_SIZE 16
> -
> -struct aegis_state {
> - union aegis_block blocks[AEGIS128_STATE_BLOCKS];
> -};
> -
> -struct aegis_ctx {
> - union aegis_block key;
> -};
> -
> -struct aegis128_ops {
> - int (*skcipher_walk_init)(struct skcipher_walk *walk,
> - struct aead_request *req, bool atomic);
> -
> - void (*crypt_chunk)(struct aegis_state *state, u8 *dst,
> - const u8 *src, unsigned int size);
> -};
> -
> static bool have_simd;
>
> -bool crypto_aegis128_have_simd(void);
> -void crypto_aegis128_update_simd(struct aegis_state *state, const void *msg);
> -void crypto_aegis128_encrypt_chunk_simd(struct aegis_state *state, u8 *dst,
> - const u8 *src, unsigned int size);
> -void crypto_aegis128_decrypt_chunk_simd(struct aegis_state *state, u8 *dst,
> - const u8 *src, unsigned int size);
> -
> static void crypto_aegis128_update(struct aegis_state *state)
> {
> union aegis_block tmp;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] linux-next 20190731 - aegis128-core.c fails to build
2019-08-01 5:01 ` Ard Biesheuvel
@ 2019-08-01 5:46 ` Valdis Klētnieks
2019-08-01 6:04 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Valdis Klētnieks @ 2019-08-01 5:46 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Arnd Bergmann, Herbert Xu, David S. Miller,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 1374 bytes --]
On Thu, 01 Aug 2019 08:01:54 +0300, Ard Biesheuvel said:
> > ERROR: "crypto_aegis128_decrypt_chunk_simd" [crypto/aegis128.ko] undefined!
> > ERROR: "crypto_aegis128_update_simd" [crypto/aegis128.ko] undefined!
> > ERROR: "crypto_aegis128_encrypt_chunk_simd" [crypto/aegis128.ko] undefined!
> > make[1]: *** [scripts/Makefile.modpost:105: modules-modpost] Error 1
> > make: *** [Makefile:1299: modules] Error 2
> Which compiler version are you using? All references to the
> crypt_aegis128_xx_simd() routines should disappear if
> CONFIG_CRYPTO_AEGIS128_SIMD is not set (in which case have_simd will
> always be false and so the compiler should optimize away those calls).
gcc 9.1.1 obviously doesn't think it can be optimized away. Apparently, it's
not smart enough to realize that nothing sets have_simd in any of the functions
and therefor it's guaranteed to be zero, and it can do dead code optimization
based on that.
Now, if we had something like:
#ifdef CONFIG_CRYPTO_AEGIS_128_SIMD
static bool have_simd;
#else
#define have_simd (0)
#endif
then that should be enough to tell the compiler it can optimize it away, except
that then runs into problems here:
if (IS_ENABLED(CONFIG_CRYPTO_AEGIS128_SIMD))
have_simd = crypto_aegis128_have_simd();
because it will whine about the lack of an lvalue before it optimizes the assignment away...
[-- Attachment #2: Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] linux-next 20190731 - aegis128-core.c fails to build
2019-08-01 5:46 ` Valdis Klētnieks
@ 2019-08-01 6:04 ` Ard Biesheuvel
2019-08-01 6:08 ` Valdis Klētnieks
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2019-08-01 6:04 UTC (permalink / raw)
To: Valdis Klētnieks
Cc: Arnd Bergmann, Herbert Xu, David S. Miller,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
Linux Kernel Mailing List
On Thu, 1 Aug 2019 at 08:47, Valdis Klētnieks <valdis.kletnieks@vt.edu> wrote:
>
> On Thu, 01 Aug 2019 08:01:54 +0300, Ard Biesheuvel said:
>
> > > ERROR: "crypto_aegis128_decrypt_chunk_simd" [crypto/aegis128.ko] undefined!
> > > ERROR: "crypto_aegis128_update_simd" [crypto/aegis128.ko] undefined!
> > > ERROR: "crypto_aegis128_encrypt_chunk_simd" [crypto/aegis128.ko] undefined!
> > > make[1]: *** [scripts/Makefile.modpost:105: modules-modpost] Error 1
> > > make: *** [Makefile:1299: modules] Error 2
>
> > Which compiler version are you using? All references to the
> > crypt_aegis128_xx_simd() routines should disappear if
> > CONFIG_CRYPTO_AEGIS128_SIMD is not set (in which case have_simd will
> > always be false and so the compiler should optimize away those calls).
>
> gcc 9.1.1 obviously doesn't think it can be optimized away. Apparently, it's
> not smart enough to realize that nothing sets have_simd in any of the functions
> and therefor it's guaranteed to be zero, and it can do dead code optimization
> based on that.
>
> Now, if we had something like:
>
> #ifdef CONFIG_CRYPTO_AEGIS_128_SIMD
> static bool have_simd;
> #else
> #define have_simd (0)
> #endif
>
> then that should be enough to tell the compiler it can optimize it away, except
> that then runs into problems here:
>
> if (IS_ENABLED(CONFIG_CRYPTO_AEGIS128_SIMD))
> have_simd = crypto_aegis128_have_simd();
>
> because it will whine about the lack of an lvalue before it optimizes the assignment away...
The fact that crypto_aegis128_have_simd() does get optimized away, but
crypto_aegis128_update_simd() doesn't (which is only called directly
and not via a function pointer like the other two routines) makes me
suspicious that this is some pathology in the compiler. Is this a
distro build of gcc? Also, which architecture are you compiling for?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] linux-next 20190731 - aegis128-core.c fails to build
2019-08-01 6:04 ` Ard Biesheuvel
@ 2019-08-01 6:08 ` Valdis Klētnieks
2019-08-01 20:17 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Valdis Klētnieks @ 2019-08-01 6:08 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Arnd Bergmann, Herbert Xu, David S. Miller,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 738 bytes --]
On Thu, 01 Aug 2019 09:04:11 +0300, Ard Biesheuvel said:
> The fact that crypto_aegis128_have_simd() does get optimized away, but
> crypto_aegis128_update_simd() doesn't (which is only called directly
> and not via a function pointer like the other two routines) makes me
> suspicious that this is some pathology in the compiler. Is this a
> distro build of gcc? Also, which architecture are you compiling for?
It's the Fedora Rawhide build on x86_64.
[/usr/src/linux-next] gcc --version
gcc (GCC) 9.1.1 20190605 (Red Hat 9.1.1-2)
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
[-- Attachment #2: Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] linux-next 20190731 - aegis128-core.c fails to build
2019-08-01 6:08 ` Valdis Klētnieks
@ 2019-08-01 20:17 ` Ard Biesheuvel
0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2019-08-01 20:17 UTC (permalink / raw)
To: Valdis Klētnieks
Cc: Arnd Bergmann, Herbert Xu, David S. Miller,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
Linux Kernel Mailing List
On Thu, 1 Aug 2019 at 09:08, Valdis Klētnieks <valdis.kletnieks@vt.edu> wrote:
>
> On Thu, 01 Aug 2019 09:04:11 +0300, Ard Biesheuvel said:
>
> > The fact that crypto_aegis128_have_simd() does get optimized away, but
> > crypto_aegis128_update_simd() doesn't (which is only called directly
> > and not via a function pointer like the other two routines) makes me
> > suspicious that this is some pathology in the compiler. Is this a
> > distro build of gcc? Also, which architecture are you compiling for?
>
> It's the Fedora Rawhide build on x86_64.
>
> [/usr/src/linux-next] gcc --version
> gcc (GCC) 9.1.1 20190605 (Red Hat 9.1.1-2)
> Copyright (C) 2019 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
Strange.
Does the following patch make any difference?
https://lore.kernel.org/linux-crypto/20190729074434.21064-1-ard.biesheuvel@linaro.org/
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-08-01 20:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-01 4:51 [PATCH] linux-next 20190731 - aegis128-core.c fails to build Valdis Klētnieks
2019-08-01 5:01 ` Ard Biesheuvel
2019-08-01 5:46 ` Valdis Klētnieks
2019-08-01 6:04 ` Ard Biesheuvel
2019-08-01 6:08 ` Valdis Klētnieks
2019-08-01 20:17 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).