From: Nick Desaulniers <ndesaulniers@google.com> To: Nathan Chancellor <nathan@kernel.org> Cc: Russell King <linux@armlinux.org.uk>, Arnd Bergmann <arnd@arndb.de>, Ard Biesheuvel <ardb@kernel.org>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org, "kernelci.org bot" <bot@kernelci.org> Subject: Re: [PATCH] ARM: Reduce __thumb2__ definition to crypto files that require it Date: Thu, 22 Dec 2022 11:52:27 -0800 [thread overview] Message-ID: <CAKwvOd=WQ5cAL74z+gbGgxG9WrOcDJtrGXJWxEEcWnmyoypu0w@mail.gmail.com> (raw) In-Reply-To: <20221222193039.2267074-1-nathan@kernel.org> On Thu, Dec 22, 2022 at 11:30 AM Nathan Chancellor <nathan@kernel.org> wrote: > > Commit 1d2e9b67b001 ("ARM: 9265/1: pass -march= only to compiler") added > a __thumb2__ define to ASFLAGS to avoid build errors in the crypto code, > which relies on __thumb2__ for preprocessing. Commit 59e2cf8d21e0 ("ARM: > 9275/1: Drop '-mthumb' from AFLAGS_ISA") followed up on this by removing > -mthumb from AFLAGS so that __thumb2__ would not be defined when the > default target was ARMv7 or newer. > > Unfortunately, the second commit's fix assumes that the toolchain > defaults to -mno-thumb / -marm, which is not the case for Debian's > arm-linux-gnueabihf target, which defaults to -mthumb: > > $ echo | arm-linux-gnueabihf-gcc -dM -E - | grep __thumb > #define __thumb2__ 1 > #define __thumb__ 1 Interesting, that was hard to foresee in review of 1d2e9b67b001 and 59e2cf8d21e0. FWIW, their non-hf target does not. $ echo | arm-linux-gnueabi-gcc -dM -E - | grep __thumb $ > > This target is used by several CI systems, which will still see > redefined macro warnings, despite '-mthumb' not being present in the > flags: > > <command-line>: warning: "__thumb2__" redefined > <built-in>: note: this is the location of the previous definition > > Remove the global AFLAGS __thumb2__ define and move it to the crypto > folder where it is required by the imported OpenSSL algorithms; the rest > of the kernel should use the internal CONFIG_THUMB2_KERNEL symbol to > know whether or not Thumb2 is being used or not. Be sure that __thumb2__ > is undefined first so that there are no macro redefinition warnings. > > Link: https://github.com/ClangBuiltLinux/linux/issues/1772 > Reported-by: "kernelci.org bot" <bot@kernelci.org> > Suggested-by: Ard Biesheuvel <ardb@kernel.org> > Signed-off-by: Nathan Chancellor <nathan@kernel.org> Thanks Nathan and Ard. I ran the following tests: 1. build w/ llvm stock config CONFIG_THUMB2_KERNEL=y kernel. 2. build w/ llvm+debian's OOT patch https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/raw/622dbcbd40b316ed3905a2d25d9623544a06e6b1/debian/patches/930008-arm.diff CONFIG_THUMB2_KERNEL=y kernel. 3. check that -U<foo> removes <foo> from being set by preprocessor for both debian's arm-linux-gnueabihf-gcc and clang -mthumb $ echo | arm-linux-gnueabihf-gcc -dM -E - | grep __thumb #define __thumb2__ 1 #define __thumb__ 1 $ echo | arm-linux-gnueabihf-gcc -dM -E - -U__thumb2__ | grep __thumb #define __thumb__ 1 $ echo | clang --target=arm-linux-gnueabihf -mthumb -dM -E - -mcpu=cortex-a7 | grep __thumb #define __thumb2__ 1 #define __thumb__ 1 $ echo | clang --target=arm-linux-gnueabihf -mthumb -dM -E - -mcpu=cortex-a7 -U__thumb2__ | grep __thumb #define __thumb__ 1 Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> Tested-by: Nick Desaulniers <ndesaulniers@google.com> > --- > arch/arm/Makefile | 2 +- > arch/arm/crypto/Makefile | 7 ++++++- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/Makefile b/arch/arm/Makefile > index 4067f5169144..955b0362cdfb 100644 > --- a/arch/arm/Makefile > +++ b/arch/arm/Makefile > @@ -132,7 +132,7 @@ AFLAGS_NOWARN :=$(call as-option,-Wa$(comma)-mno-warn-deprecated,-Wa$(comma)-W) > > ifeq ($(CONFIG_THUMB2_KERNEL),y) > CFLAGS_ISA :=-Wa,-mimplicit-it=always $(AFLAGS_NOWARN) > -AFLAGS_ISA :=$(CFLAGS_ISA) -Wa$(comma)-mthumb -D__thumb2__=2 > +AFLAGS_ISA :=$(CFLAGS_ISA) -Wa$(comma)-mthumb > CFLAGS_ISA +=-mthumb > else > CFLAGS_ISA :=$(call cc-option,-marm,) $(AFLAGS_NOWARN) > diff --git a/arch/arm/crypto/Makefile b/arch/arm/crypto/Makefile > index 971e74546fb1..13e62c7c25dc 100644 > --- a/arch/arm/crypto/Makefile > +++ b/arch/arm/crypto/Makefile > @@ -53,7 +53,12 @@ $(obj)/%-core.S: $(src)/%-armv4.pl > > clean-files += poly1305-core.S sha256-core.S sha512-core.S > > +aflags-thumb2-$(CONFIG_THUMB2_KERNEL) := -U__thumb2__ -D__thumb2__=1 > + > +AFLAGS_sha256-core.o += $(aflags-thumb2-y) > +AFLAGS_sha512-core.o += $(aflags-thumb2-y) > + > # massage the perlasm code a bit so we only get the NEON routine if we need it > poly1305-aflags-$(CONFIG_CPU_V7) := -U__LINUX_ARM_ARCH__ -D__LINUX_ARM_ARCH__=5 > poly1305-aflags-$(CONFIG_KERNEL_MODE_NEON) := -U__LINUX_ARM_ARCH__ -D__LINUX_ARM_ARCH__=7 > -AFLAGS_poly1305-core.o += $(poly1305-aflags-y) > +AFLAGS_poly1305-core.o += $(poly1305-aflags-y) $(aflags-thumb2-y) > > base-commit: 71a7507afbc3f27c346898f13ab9bfd918613c34 > -- > 2.39.0 > -- Thanks, ~Nick Desaulniers
WARNING: multiple messages have this Message-ID (diff)
From: Nick Desaulniers <ndesaulniers@google.com> To: Nathan Chancellor <nathan@kernel.org> Cc: Russell King <linux@armlinux.org.uk>, Arnd Bergmann <arnd@arndb.de>, Ard Biesheuvel <ardb@kernel.org>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org, "kernelci.org bot" <bot@kernelci.org> Subject: Re: [PATCH] ARM: Reduce __thumb2__ definition to crypto files that require it Date: Thu, 22 Dec 2022 11:52:27 -0800 [thread overview] Message-ID: <CAKwvOd=WQ5cAL74z+gbGgxG9WrOcDJtrGXJWxEEcWnmyoypu0w@mail.gmail.com> (raw) In-Reply-To: <20221222193039.2267074-1-nathan@kernel.org> On Thu, Dec 22, 2022 at 11:30 AM Nathan Chancellor <nathan@kernel.org> wrote: > > Commit 1d2e9b67b001 ("ARM: 9265/1: pass -march= only to compiler") added > a __thumb2__ define to ASFLAGS to avoid build errors in the crypto code, > which relies on __thumb2__ for preprocessing. Commit 59e2cf8d21e0 ("ARM: > 9275/1: Drop '-mthumb' from AFLAGS_ISA") followed up on this by removing > -mthumb from AFLAGS so that __thumb2__ would not be defined when the > default target was ARMv7 or newer. > > Unfortunately, the second commit's fix assumes that the toolchain > defaults to -mno-thumb / -marm, which is not the case for Debian's > arm-linux-gnueabihf target, which defaults to -mthumb: > > $ echo | arm-linux-gnueabihf-gcc -dM -E - | grep __thumb > #define __thumb2__ 1 > #define __thumb__ 1 Interesting, that was hard to foresee in review of 1d2e9b67b001 and 59e2cf8d21e0. FWIW, their non-hf target does not. $ echo | arm-linux-gnueabi-gcc -dM -E - | grep __thumb $ > > This target is used by several CI systems, which will still see > redefined macro warnings, despite '-mthumb' not being present in the > flags: > > <command-line>: warning: "__thumb2__" redefined > <built-in>: note: this is the location of the previous definition > > Remove the global AFLAGS __thumb2__ define and move it to the crypto > folder where it is required by the imported OpenSSL algorithms; the rest > of the kernel should use the internal CONFIG_THUMB2_KERNEL symbol to > know whether or not Thumb2 is being used or not. Be sure that __thumb2__ > is undefined first so that there are no macro redefinition warnings. > > Link: https://github.com/ClangBuiltLinux/linux/issues/1772 > Reported-by: "kernelci.org bot" <bot@kernelci.org> > Suggested-by: Ard Biesheuvel <ardb@kernel.org> > Signed-off-by: Nathan Chancellor <nathan@kernel.org> Thanks Nathan and Ard. I ran the following tests: 1. build w/ llvm stock config CONFIG_THUMB2_KERNEL=y kernel. 2. build w/ llvm+debian's OOT patch https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/raw/622dbcbd40b316ed3905a2d25d9623544a06e6b1/debian/patches/930008-arm.diff CONFIG_THUMB2_KERNEL=y kernel. 3. check that -U<foo> removes <foo> from being set by preprocessor for both debian's arm-linux-gnueabihf-gcc and clang -mthumb $ echo | arm-linux-gnueabihf-gcc -dM -E - | grep __thumb #define __thumb2__ 1 #define __thumb__ 1 $ echo | arm-linux-gnueabihf-gcc -dM -E - -U__thumb2__ | grep __thumb #define __thumb__ 1 $ echo | clang --target=arm-linux-gnueabihf -mthumb -dM -E - -mcpu=cortex-a7 | grep __thumb #define __thumb2__ 1 #define __thumb__ 1 $ echo | clang --target=arm-linux-gnueabihf -mthumb -dM -E - -mcpu=cortex-a7 -U__thumb2__ | grep __thumb #define __thumb__ 1 Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> Tested-by: Nick Desaulniers <ndesaulniers@google.com> > --- > arch/arm/Makefile | 2 +- > arch/arm/crypto/Makefile | 7 ++++++- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/Makefile b/arch/arm/Makefile > index 4067f5169144..955b0362cdfb 100644 > --- a/arch/arm/Makefile > +++ b/arch/arm/Makefile > @@ -132,7 +132,7 @@ AFLAGS_NOWARN :=$(call as-option,-Wa$(comma)-mno-warn-deprecated,-Wa$(comma)-W) > > ifeq ($(CONFIG_THUMB2_KERNEL),y) > CFLAGS_ISA :=-Wa,-mimplicit-it=always $(AFLAGS_NOWARN) > -AFLAGS_ISA :=$(CFLAGS_ISA) -Wa$(comma)-mthumb -D__thumb2__=2 > +AFLAGS_ISA :=$(CFLAGS_ISA) -Wa$(comma)-mthumb > CFLAGS_ISA +=-mthumb > else > CFLAGS_ISA :=$(call cc-option,-marm,) $(AFLAGS_NOWARN) > diff --git a/arch/arm/crypto/Makefile b/arch/arm/crypto/Makefile > index 971e74546fb1..13e62c7c25dc 100644 > --- a/arch/arm/crypto/Makefile > +++ b/arch/arm/crypto/Makefile > @@ -53,7 +53,12 @@ $(obj)/%-core.S: $(src)/%-armv4.pl > > clean-files += poly1305-core.S sha256-core.S sha512-core.S > > +aflags-thumb2-$(CONFIG_THUMB2_KERNEL) := -U__thumb2__ -D__thumb2__=1 > + > +AFLAGS_sha256-core.o += $(aflags-thumb2-y) > +AFLAGS_sha512-core.o += $(aflags-thumb2-y) > + > # massage the perlasm code a bit so we only get the NEON routine if we need it > poly1305-aflags-$(CONFIG_CPU_V7) := -U__LINUX_ARM_ARCH__ -D__LINUX_ARM_ARCH__=5 > poly1305-aflags-$(CONFIG_KERNEL_MODE_NEON) := -U__LINUX_ARM_ARCH__ -D__LINUX_ARM_ARCH__=7 > -AFLAGS_poly1305-core.o += $(poly1305-aflags-y) > +AFLAGS_poly1305-core.o += $(poly1305-aflags-y) $(aflags-thumb2-y) > > base-commit: 71a7507afbc3f27c346898f13ab9bfd918613c34 > -- > 2.39.0 > -- Thanks, ~Nick Desaulniers _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-12-22 19:52 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-12-22 19:30 [PATCH] ARM: Reduce __thumb2__ definition to crypto files that require it Nathan Chancellor 2022-12-22 19:30 ` Nathan Chancellor 2022-12-22 19:52 ` Nick Desaulniers [this message] 2022-12-22 19:52 ` Nick Desaulniers 2023-01-23 10:48 ` Russell King (Oracle) 2023-01-23 10:48 ` Russell King (Oracle) 2023-01-23 14:12 ` Nathan Chancellor 2023-01-23 14:12 ` Nathan Chancellor
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAKwvOd=WQ5cAL74z+gbGgxG9WrOcDJtrGXJWxEEcWnmyoypu0w@mail.gmail.com' \ --to=ndesaulniers@google.com \ --cc=ardb@kernel.org \ --cc=arnd@arndb.de \ --cc=bot@kernelci.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-crypto@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=nathan@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.