* [PATCH] crypto: aegis128/simd - build 32-bit ARM for v8 architecture explicitly @ 2019-10-02 7:54 Ard Biesheuvel 2019-10-02 14:47 ` Nathan Chancellor ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Ard Biesheuvel @ 2019-10-02 7:54 UTC (permalink / raw) To: linux-crypto; +Cc: herbert, arnd, natechancellor, ndesaulniers, Ard Biesheuvel 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> --- 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] crypto: aegis128/simd - build 32-bit ARM for v8 architecture explicitly 2019-10-02 7:54 [PATCH] crypto: aegis128/simd - build 32-bit ARM for v8 architecture explicitly Ard Biesheuvel @ 2019-10-02 14:47 ` Nathan Chancellor 2019-10-02 16:47 ` Nick Desaulniers 2019-10-10 12:55 ` Herbert Xu 2 siblings, 0 replies; 7+ messages in thread From: Nathan Chancellor @ 2019-10-02 14:47 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-crypto, herbert, arnd, ndesaulniers 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] crypto: aegis128/simd - build 32-bit ARM for v8 architecture explicitly 2019-10-02 7:54 [PATCH] crypto: aegis128/simd - build 32-bit ARM for v8 architecture explicitly Ard Biesheuvel 2019-10-02 14:47 ` Nathan Chancellor @ 2019-10-02 16:47 ` Nick Desaulniers 2019-10-02 17:23 ` Catalin Marinas 2019-10-10 12:55 ` Herbert Xu 2 siblings, 1 reply; 7+ messages in thread From: Nick Desaulniers @ 2019-10-02 16:47 UTC (permalink / raw) To: Ard Biesheuvel Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Herbert Xu, Arnd Bergmann, Nathan Chancellor, Will Deacon, Catalin Marinas 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] crypto: aegis128/simd - build 32-bit ARM for v8 architecture explicitly 2019-10-02 16:47 ` Nick Desaulniers @ 2019-10-02 17:23 ` Catalin Marinas 2019-10-02 18:09 ` Ard Biesheuvel 0 siblings, 1 reply; 7+ messages in thread From: Catalin Marinas @ 2019-10-02 17:23 UTC (permalink / raw) To: Nick Desaulniers Cc: Ard Biesheuvel, open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Herbert Xu, Arnd Bergmann, Nathan Chancellor, Will Deacon On Wed, Oct 02, 2019 at 09:47:41AM -0700, Nick Desaulniers wrote: > 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 think they are optional (or at least most of them). > 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. This page shows the possible combinations: https://sourceware.org/binutils/docs/as/AArch64-Extensions.html#AArch64-Extensions Basically if it's an optional feature in ARMv8.0, you pass armv8-a+... For optional features only in higher versions, it would be armv8.5-a+memtag. The table above also states whether it's enabled by default (i.e. mandatory) in an architecture version. SB for example is supported from 8.0 but only required in 8.5. -- Catalin ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] crypto: aegis128/simd - build 32-bit ARM for v8 architecture explicitly 2019-10-02 17:23 ` Catalin Marinas @ 2019-10-02 18:09 ` Ard Biesheuvel 2019-10-02 21:32 ` Catalin Marinas 0 siblings, 1 reply; 7+ messages in thread From: Ard Biesheuvel @ 2019-10-02 18:09 UTC (permalink / raw) To: Catalin Marinas Cc: Nick Desaulniers, open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Herbert Xu, Arnd Bergmann, Nathan Chancellor, Will Deacon On Wed, 2 Oct 2019 at 19:23, Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Wed, Oct 02, 2019 at 09:47:41AM -0700, Nick Desaulniers wrote: > > 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 think they are optional (or at least most of them). > This is 32-bit ARM so I don't think any of the extensions are mandatory. And the crypto ones are export controlled, so they are definitely not mandatory, except for certain combinations (PMULL requires AES, and SHA512 requires SHA256). I don't think 32-bit ARM cores with crypto extensions are widely available in the field, but since the intrinsics can be compiled to either ISA, it was rather trivial to retain support for it (and 32-bit VMs running on a 64-bit hosts may benefit as well) > > 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. > > This page shows the possible combinations: > > https://sourceware.org/binutils/docs/as/AArch64-Extensions.html#AArch64-Extensions > > Basically if it's an optional feature in ARMv8.0, you pass armv8-a+... > For optional features only in higher versions, it would be > armv8.5-a+memtag. The table above also states whether it's enabled by > default (i.e. mandatory) in an architecture version. SB for example is > supported from 8.0 but only required in 8.5. > I am not convinced (but I haven't checked) that this is used in the same way on 32-bit. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] crypto: aegis128/simd - build 32-bit ARM for v8 architecture explicitly 2019-10-02 18:09 ` Ard Biesheuvel @ 2019-10-02 21:32 ` Catalin Marinas 0 siblings, 0 replies; 7+ messages in thread From: Catalin Marinas @ 2019-10-02 21:32 UTC (permalink / raw) To: Ard Biesheuvel Cc: Nick Desaulniers, open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Herbert Xu, Arnd Bergmann, Nathan Chancellor, Will Deacon On Wed, Oct 02, 2019 at 08:09:18PM +0200, Ard Biesheuvel wrote: > On Wed, 2 Oct 2019 at 19:23, Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Wed, Oct 02, 2019 at 09:47:41AM -0700, Nick Desaulniers wrote: > > > 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. > > > > This page shows the possible combinations: > > > > https://sourceware.org/binutils/docs/as/AArch64-Extensions.html#AArch64-Extensions > > > > Basically if it's an optional feature in ARMv8.0, you pass armv8-a+... > > For optional features only in higher versions, it would be > > armv8.5-a+memtag. The table above also states whether it's enabled by > > default (i.e. mandatory) in an architecture version. SB for example is > > supported from 8.0 but only required in 8.5. > > I am not convinced (but I haven't checked) that this is used in the > same way on 32-bit. Ah, I didn't realise this was about 32-bit. I don't think the above applies in this case. -- Catalin ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] crypto: aegis128/simd - build 32-bit ARM for v8 architecture explicitly 2019-10-02 7:54 [PATCH] crypto: aegis128/simd - build 32-bit ARM for v8 architecture explicitly Ard Biesheuvel 2019-10-02 14:47 ` Nathan Chancellor 2019-10-02 16:47 ` Nick Desaulniers @ 2019-10-10 12:55 ` Herbert Xu 2 siblings, 0 replies; 7+ messages in thread From: Herbert Xu @ 2019-10-10 12:55 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-crypto, arnd, natechancellor, ndesaulniers 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> > --- > crypto/Kconfig | 1 + > crypto/Makefile | 2 +- > 2 files changed, 2 insertions(+), 1 deletion(-) Patch applied. Thanks. -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-10-10 12:55 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-02 7:54 [PATCH] crypto: aegis128/simd - build 32-bit ARM for v8 architecture explicitly Ard Biesheuvel 2019-10-02 14:47 ` Nathan Chancellor 2019-10-02 16:47 ` Nick Desaulniers 2019-10-02 17:23 ` Catalin Marinas 2019-10-02 18:09 ` Ard Biesheuvel 2019-10-02 21:32 ` Catalin Marinas 2019-10-10 12:55 ` Herbert Xu
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).