Subject: [PATCH RFC v2] kbuild/x86: Use $(KBUILD_AFLAGS) in Kbuild's version of $(as-instr)

From: Dmitry Safonov <[email protected]>

At Arista some products use compatible 32-bit userspace running on x86.
As a part of disto build for ia32 it also compiles the 64-bit kernel.
While the toolchain for the kernel build is yet the same, with 64-bit gcc:
> / @Bru-na-Boinne% file /usr/bin/gcc-11
> /usr/bin/gcc-11: ELF 64-bit LSB pie executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, BuildID[sha1]=6571ad50d8f12eece053f1bac7a95a2c767f32c9, for GNU/Linux 3.2.0, stripped

It seems that gcc is being smart and detects that it's running in
a 32-bit container (personality flag? 32-bit mmap base? something else
inherited post-exec? haven't yet figured it out) and by default tries
to build 32-bit binaries.

That results in a failing toolchain check:
> / @Bru-na-Boinne% printf "%b\n" "wrussq %rax, (%rbx)" | /usr/bin/gcc-11 -Wa,--fatal-warnings -c -x assembler-with-cpp -o /dev/null -
> <stdin>: Assembler messages:
> <stdin>:1: Error: `wrussq' is only supported in 64-bit mode

Which passes when -m64 is directly specify for the build check:
> / @Bru-na-Boinne% printf "%b\n" "wrussq %rax, (%rbx)" | /usr/bin/gcc-11 -m64 -Wa,--fatal-warnings -c -x assembler-with-cpp -o /dev/null -
> / @Bru-na-Boinne% echo $?
> 0

As a result, kbuild produces different value for CONFIG_AS_WRUSS
for native 64-bit containers and ia32 containers with 64-bit gcc,
which produces different kernels with enabled/disabled
CONFIG_X86_USER_SHADOW_STACK.

arch/x86/Makefile already properly defines KBUILD_AFLAGS += -m64,
which is luckly already available at the point of toolchain check
in arch/x86/Kconfig.assembler

By hacking around Kbuild variable the following way:
> --- a/scripts/Kconfig.include
> +++ b/scripts/Kconfig.include
> @@ -13,7 +13,8 @@ left_paren := (
>
> # $(if-success,<command>,<then>,<else>)
> # Return <then> if <command> exits with 0, <else> otherwise.
> -if-success = $(shell,{ $(1); } >/dev/null 2>&1 && echo "$(2)" || echo "$(3)")
> +if-success = $(shell,echo '$(1)' 1>&2;{ $(1); } >/dev/null 2>&1 && echo "$(2)" || echo "$(3)")

I got the following output for the toolchain check, before:
> linux @Bru-na-Boinne% make ARCH=x86_64 oldconfig V=1 2>&1 | grep wrus
> printf "%b\n" "wrussq %rax,(%rbx)" | gcc -c -x assembler-with-cpp -o /dev/null -

and after:
> linux @Bru-na-Boinne% make ARCH=x86_64 oldconfig V=1 2>&1 | grep wrus
> printf "%b\n" "wrussq %rax,(%rbx)" | gcc -D__ASSEMBLY__ -fno-PIE -m64 -c -x assembler-with-cpp -o /dev/null -

Which seems appropriate to me.
This also reflects the existing definition in scripts/Makefile.compiler
for $(as-instr) that already has $(KBUILD_AFLAGS).

In order to eliminate a possible circular dependency of
Kconfig => arch/.../Makefile => Kconfig => ...
which exist i.e. in arm64/Makefile for KASAN_SHADOW_SCALE_SHIFT that
depends on CONFIG_KASAH_SW_TAGS and CONFIG_KASAN_GENERIC kconfigs,
ignore KBUILD_AFLAGS difference in auto.conf.cmd as it is expected that
the variable will differ between fist and later Makefile passes.
Use that in Kconfig toolchain checks.

Signed-off-by: Dmitry Safonov <[email protected]>
---
Changes in v2:
- I made a mistake in version 1 by not testing it on a clean tree.
Which passed on an existing "dirty" tree, but went into build
busy-loop over a fresh clean kernel tree. With a little debug,
I found out that the reason was subdir-asflags-y in scripts/.
They are "empty", but they update KBUILD_AFLAGS with an extra
trailing space. Ugh.
That resulted in include/config/auto.conf.cmd being unhappy and
triggering rebuild after rebuild.
- Then I've added a patch to scripts/kconfig/preprocess.c that removes
any extra head or tail spaces from the environment variables.
- That was a working patch set on x86_64, but that broke arm64 by
creating a circular dependency on KASAN_SHADOW_SCALE_SHIFT.
- In the end, I decided to just ignore KBUILD_AFLAGS in auto.conf.cmd
for RFC v2. Probably, I could just add $(as-instr-config,...)
function to Kconfig that would take -m64 just for AS_WRUSS,
but that seems like fixing one specific place, rather than more
generic fix. Unsure, sending this as RFC, please advise/vote on
the best approach.
- Link to v1: https://lore.kernel.org/r/[email protected]
---
scripts/Kconfig.include | 2 +-
scripts/kconfig/preprocess.c | 14 +++++++++++++-
2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
index 3ee8ecfb8c04..d8574c1faf2d 100644
--- a/scripts/Kconfig.include
+++ b/scripts/Kconfig.include
@@ -33,7 +33,7 @@ ld-option = $(success,$(LD) -v $(1))

# $(as-instr,<instr>)
# Return y if the assembler supports <instr>, n otherwise
-as-instr = $(success,printf "%b\n" "$(1)" | $(CC) $(CLANG_FLAGS) -Wa$(comma)--fatal-warnings -c -x assembler-with-cpp -o /dev/null -)
+as-instr = $(success,printf "%b\n" "$(1)" | $(CC) $(CLANG_FLAGS) $(KBUILD_AFLAGS) -Wa$(comma)--fatal-warnings -c -x assembler-with-cpp -o /dev/null -)

# check if $(CC) and $(LD) exist
$(error-if,$(failure,command -v $(CC)),C compiler '$(CC)' not found)
diff --git a/scripts/kconfig/preprocess.c b/scripts/kconfig/preprocess.c
index f0a4a218c4a5..cf7389f13f00 100644
--- a/scripts/kconfig/preprocess.c
+++ b/scripts/kconfig/preprocess.c
@@ -61,6 +61,17 @@ static void env_del(struct env *e)
free(e);
}

+static bool env_ignore(const char *name)
+{
+ /*
+ * Break the circular dependency Kconfig => arch/.../Makefile => Kconfig
+ * On the first Makefile pass only AFLAGS that do not depend on
+ * CONFIG_* set will be defined. Those are used by Kconfig for
+ * toolchain checks.
+ */
+ return !strcmp(name, "KBUILD_AFLAGS");
+}
+
/* The returned pointer must be freed when done */
static char *env_expand(const char *name)
{
@@ -83,7 +94,8 @@ static char *env_expand(const char *name)
* We need to remember all referenced environment variables.
* They will be written out to include/config/auto.conf.cmd
*/
- env_add(name, value);
+ if (!env_ignore(name))
+ env_add(name, value);

return xstrdup(value);
}

---
base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670
change-id: 20240410-as-instr-opt-wrussq-48ec37e36204

Best regards,
--
Dmitry Safonov <[email protected]>




2024-06-12 05:07:12

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH RFC v2] kbuild/x86: Use $(KBUILD_AFLAGS) in Kbuild's version of $(as-instr)

On Wed, Jun 12, 2024 at 12:29 PM Dmitry Safonov via B4 Relay
<[email protected]> wrote:
>
> From: Dmitry Safonov <[email protected]>
>
> At Arista some products use compatible 32-bit userspace running on x86.
> As a part of disto build for ia32 it also compiles the 64-bit kernel.
> While the toolchain for the kernel build is yet the same, with 64-bit gcc:
> > / @Bru-na-Boinne% file /usr/bin/gcc-11
> > /usr/bin/gcc-11: ELF 64-bit LSB pie executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, BuildID[sha1]=6571ad50d8f12eece053f1bac7a95a2c767f32c9, for GNU/Linux 3.2.0, stripped
>
> It seems that gcc is being smart and detects that it's running in
> a 32-bit container (personality flag? 32-bit mmap base? something else
> inherited post-exec? haven't yet figured it out) and by default tries
> to build 32-bit binaries.
>
> That results in a failing toolchain check:
> > / @Bru-na-Boinne% printf "%b\n" "wrussq %rax, (%rbx)" | /usr/bin/gcc-11 -Wa,--fatal-warnings -c -x assembler-with-cpp -o /dev/null -
> > <stdin>: Assembler messages:
> > <stdin>:1: Error: `wrussq' is only supported in 64-bit mode
>
> Which passes when -m64 is directly specify for the build check:
> > / @Bru-na-Boinne% printf "%b\n" "wrussq %rax, (%rbx)" | /usr/bin/gcc-11 -m64 -Wa,--fatal-warnings -c -x assembler-with-cpp -o /dev/null -
> > / @Bru-na-Boinne% echo $?
> > 0
>
> As a result, kbuild produces different value for CONFIG_AS_WRUSS
> for native 64-bit containers and ia32 containers with 64-bit gcc,
> which produces different kernels with enabled/disabled
> CONFIG_X86_USER_SHADOW_STACK.
>
> arch/x86/Makefile already properly defines KBUILD_AFLAGS += -m64,
> which is luckly already available at the point of toolchain check
> in arch/x86/Kconfig.assembler
>
> By hacking around Kbuild variable the following way:
> > --- a/scripts/Kconfig.include
> > +++ b/scripts/Kconfig.include
> > @@ -13,7 +13,8 @@ left_paren := (
> >
> > # $(if-success,<command>,<then>,<else>)
> > # Return <then> if <command> exits with 0, <else> otherwise.
> > -if-success = $(shell,{ $(1); } >/dev/null 2>&1 && echo "$(2)" || echo "$(3)")
> > +if-success = $(shell,echo '$(1)' 1>&2;{ $(1); } >/dev/null 2>&1 && echo "$(2)" || echo "$(3)")
>
> I got the following output for the toolchain check, before:
> > linux @Bru-na-Boinne% make ARCH=x86_64 oldconfig V=1 2>&1 | grep wrus
> > printf "%b\n" "wrussq %rax,(%rbx)" | gcc -c -x assembler-with-cpp -o /dev/null -
>
> and after:
> > linux @Bru-na-Boinne% make ARCH=x86_64 oldconfig V=1 2>&1 | grep wrus
> > printf "%b\n" "wrussq %rax,(%rbx)" | gcc -D__ASSEMBLY__ -fno-PIE -m64 -c -x assembler-with-cpp -o /dev/null -
>
> Which seems appropriate to me.
> This also reflects the existing definition in scripts/Makefile.compiler
> for $(as-instr) that already has $(KBUILD_AFLAGS).
>
> In order to eliminate a possible circular dependency of
> Kconfig => arch/.../Makefile => Kconfig => ...
> which exist i.e. in arm64/Makefile for KASAN_SHADOW_SCALE_SHIFT that
> depends on CONFIG_KASAH_SW_TAGS and CONFIG_KASAN_GENERIC kconfigs,
> ignore KBUILD_AFLAGS difference in auto.conf.cmd as it is expected that
> the variable will differ between fist and later Makefile passes.
> Use that in Kconfig toolchain checks.
>
> Signed-off-by: Dmitry Safonov <[email protected]>



Perhaps, you could pass CROSS_COMPILE=x86_64-linux-gnu-
when building the 64-bit kernel.

x86_64-linux-gnu-gcc may understand 'wrussq %rax,(%rbx)'
even if the default gcc does not.

I am not sure if x86_64-linux-gnu-gcc is available
on your build machine.


Anyway, I think it should be possible to fix it with less hacky code.


Please test this patch:
https://lore.kernel.org/all/[email protected]/T/#u




--
Best Regards
Masahiro Yamada