Re: [PATCH] linux-next 20190731 - aegis128-core.c fails to build

[Ard Biesheuvel]

(+ Arnd)

On Thu, 1 Aug 2019 at 07:52, Valdis KlÄtnieks <valdis.kletnieks@xxxxxx> 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@xxxxxx>

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;
>