Re: [PATCH] crypto: aegis128/simd - build 32-bit ARM for v8 architecture explicitly

[Nathan Chancellor <natechancellor@gmail.com>]

Thank you for fixing this, it is much appreciated!

On Wed, Oct 02, 2019 at 09:54:48AM +0200, Ard Biesheuvel wrote:
> Now that the Clang compiler has taken it upon itself to police the
> compiler command line, and reject combinations for arguments it views
> as incompatible, the AEGIS128 no longer builds correctly, and errors
> out like this:
> 
>   clang-10: warning: ignoring extension 'crypto' because the 'armv7-a'
>   architecture does not support it [-Winvalid-command-line-argument]
> 
> So let's switch to armv8-a instead, which matches the crypto-neon-fp-armv8
> FPU profile we specify. Since neither were actually supported by GCC
> versions before 4.8, let's tighten the Kconfig dependencies as well so
> we won't run into errors when building with an ancient compiler.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Minor nit below but regardless.

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

> ---
>  crypto/Kconfig  | 1 +
>  crypto/Makefile | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index e928f88b6206..b138b68329dc 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -331,6 +331,7 @@ config CRYPTO_AEGIS128
>  config CRYPTO_AEGIS128_SIMD
>  	bool "Support SIMD acceleration for AEGIS-128"
>  	depends on CRYPTO_AEGIS128 && ((ARM || ARM64) && KERNEL_MODE_NEON)
> +	depends on !ARM || CC_IS_CLANG || GCC_VERSION >= 40800

This !ARM is a bit misleading to me given the (ARM || ARM64) requirement
right above it and the fact that crypto/Makefile gates the addition of
the NEON files to aegis128-y to $(ARCH) = arm or arm64.

>  	default y
>  
>  config CRYPTO_AEGIS128_AESNI_SSE2
> diff --git a/crypto/Makefile b/crypto/Makefile
> index fcb1ee679782..aa740c8492b9 100644
> --- a/crypto/Makefile
> +++ b/crypto/Makefile
> @@ -93,7 +93,7 @@ obj-$(CONFIG_CRYPTO_AEGIS128) += aegis128.o
>  aegis128-y := aegis128-core.o
>  
>  ifeq ($(ARCH),arm)
> -CFLAGS_aegis128-neon-inner.o += -ffreestanding -march=armv7-a -mfloat-abi=softfp
> +CFLAGS_aegis128-neon-inner.o += -ffreestanding -march=armv8-a -mfloat-abi=softfp
>  CFLAGS_aegis128-neon-inner.o += -mfpu=crypto-neon-fp-armv8
>  aegis128-$(CONFIG_CRYPTO_AEGIS128_SIMD) += aegis128-neon.o aegis128-neon-inner.o
>  endif
> -- 
> 2.20.1
>