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

[Nick Desaulniers <ndesaulniers@google.com>]

On Wed, Oct 2, 2019 at 12:55 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> 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>

Thank you Ard, this fixes the build error for us.  Do you know if the
"crypto extensions" are mandatory ISA extensions?  I'm running into
some inconsistencies between how clang parses target arch between
command line flag, function __attribute__, assembler directive, and
disassembler.  I see arch's like: armv8-a+crc, armv8-a+sve,
armv8-a+fp16, armv8-a+memtag, armv8-a+lse, but I'm not familiar with
the `+...` part of the target arch.

Either way, thanks for the patch:
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
Reported-by: <ci_notify@linaro.org>
Link: https://github.com/ClangBuiltLinux/linux/issues/730

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


--
Thanks,
~Nick Desaulniers