diff mbox series

[v3,4/7] selftests/nolibc: add XARCH and ARCH mapping support

Message ID 45cc24c1cf8794782be2ae631ca01bcd136da6d9.1690468707.git.falcon@tinylab.org (mailing list archive)
State New
Headers show
Series tools/nolibc: add 32/64-bit powerpc support | expand

Commit Message

Zhangjin Wu July 27, 2023, 3:03 p.m. UTC
Most of the CPU architectures have different variants, but kernel
usually only accepts parts of them via the ARCH variable, the others
should be customized via kernel config files.

To simplify testing, the external ARCH variable is extended to accept
more CPU variants from user's input, a new internal XARCH variable is
added to save users' ARCH input and used to customize variant specific
variables, at last XARCH is converted to the internal ARCH variable
acceptable by kernel:

  e.g. make run ARCH=<one of the supported variants>
               /
              v
  external ARCH from cmdline -> internal XARCH -> internal ARCH for kernel
                                          |
                                          `--> variant specific variables

XARCH and ARCH are carefully mapped to allow users to pass architecture
variants via external ARCH (or XARCH) from cmdline:

- From developers' perspective

  - XARCH records the architecture variant from user's ARCH input, after
    'override ARCH', ARCH is overridden and mapped from XARCH to the one
    supported by kernel

  - Map from XARCH to the kernel supported ARCH: 'ARCH_<XARCH> = <ARCH>'

  - Configure a default variant for kernel supported ARCH: 'XARCH_<ARCH> = <XARCH>'

- From users' perspective

  - ARCH (or XARCH) are architecture variants of a target architecture

  - the variants are XARCH names from the "ARCH_<XARCH> = <ARCH>" mapping list

PowerPC is the first user and also a very good reference architecture of
this mapping, it has variants with different combinations of
32-bit/64-bit and bit endian/little endian.

To use this mapping, the other architectures can refer to PowerPC, If
the target architecture only has one variant, XARCH is simply an alias
of ARCH, no additional mapping required.

Suggested-by: Willy Tarreau <w@1wt.eu>
Link: https://lore.kernel.org/lkml/20230702171715.GD16233@1wt.eu/
Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/Makefile | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

Comments

Zhangjin Wu July 30, 2023, 6:38 a.m. UTC | #1
Hi, Willy, Thomas

Beside the extra config stuff, here is the left one need your last confirm.

Let's summarize it, without 'override', we are able to use such styles (I added
a $(error ARCH=$(ARCH) XARCH=$(XARCH)) line for tmp test):

    $ make ARCH=powerpc
    Makefile:29: *** ARCH=powerpc, XARCH=ppc.  Stop.
    
    $ make ARCH=powerpc XARCH=ppc
    Makefile:29: *** ARCH=powerpc, XARCH=ppc.  Stop.
    $ make ARCH=powerpc XARCH=ppc64
    Makefile:29: *** ARCH=powerpc, XARCH=ppc64.  Stop.
    $ make ARCH=powerpc XARCH=ppc66le
    Makefile:29: *** ARCH=powerpc, XARCH=ppc66le.  Stop.

    $ make XARCH=ppc
    Makefile:29: *** ARCH=powerpc, XARCH=ppc.  Stop.
    $ make XARCH=ppc64
    Makefile:29: *** ARCH=powerpc, XARCH=ppc64.  Stop.
    $ make XARCH=ppc64le
    Makefile:29: *** ARCH=powerpc, XARCH=ppc64le.  Stop.

with 'override', we are further able to use:

    $ make ARCH=powerpc
    Makefile:29: *** ARCH=powerpc, XARCH=ppc.  Stop.
    $ make ARCH=ppc
    Makefile:29: *** ARCH=powerpc, XARCH=ppc.  Stop.
    $ make ARCH=ppc64
    Makefile:29: *** ARCH=powerpc, XARCH=ppc64.  Stop.
    $ make ARCH=ppc64le
    Makefile:29: *** ARCH=powerpc, XARCH=ppc64le.  Stop.

So, with 'override', users will be able to directly use the famous ARCH, it is
able to accept powerpc, ppc, ppc64, ppc64le and users can simply ignore XARCH
and we are able to only use XARCH as an internal variable to temply save input
ARCH and then convert it to an internal ARCH.

Without 'override', we must carefully document its usage, it may be:

    # XARCH and ARCH mapping
    #
    # Usage:
    #      $ make ARCH=<kernel-supported-ARCH> XARCH=<nolibc-test-supported-variants> ...
    #
    #      e.g. make ARCH=powerpc XARCH=[ppc|ppc64|ppc64le]

    # XARCH is used to save user-input ARCH variant
    # configure default variants for target kernel supported architectures

For the help page, if we only use '\$$XARCH or \$$ARCH', it may mislead users:

	@echo "  run-user               runs the executable under QEMU (uses \$$ARCH or \\$XARCH, \$$TEST)"

That's why I at last add the 'override' keyword to make sure even if users
wrongly and only use ARCH as the argument, it must not fail, or we forcely ask
user pass ARCH and XARCH together.

	@echo "  run-user               runs the executable under QEMU (uses \$$ARCH and \\$XARCH, \$$TEST)"

Or we simply only and always ask users to use XARCH (as the first version does)
for nolibc-test and let ARCH as the default one from a previous build kernel:

	@echo "  run-user               runs the executable under QEMU (uses \$$XARCH, \$$TEST)"

That means, the ugly 'override' does help us to save lots of teach work ;-)

I'm ok with 'override' or not, welcome your confirm, which direction do you
prefer?

Thanks,
Zhangjin

> Most of the CPU architectures have different variants, but kernel
> usually only accepts parts of them via the ARCH variable, the others
> should be customized via kernel config files.
> 
> To simplify testing, the external ARCH variable is extended to accept
> more CPU variants from user's input, a new internal XARCH variable is
> added to save users' ARCH input and used to customize variant specific
> variables, at last XARCH is converted to the internal ARCH variable
> acceptable by kernel:
> 
>   e.g. make run ARCH=<one of the supported variants>
>                /
>               v
>   external ARCH from cmdline -> internal XARCH -> internal ARCH for kernel
>                                           |
>                                           `--> variant specific variables
> 
> XARCH and ARCH are carefully mapped to allow users to pass architecture
> variants via external ARCH (or XARCH) from cmdline:
> 
> - From developers' perspective
> 
>   - XARCH records the architecture variant from user's ARCH input, after
>     'override ARCH', ARCH is overridden and mapped from XARCH to the one
>     supported by kernel
> 
>   - Map from XARCH to the kernel supported ARCH: 'ARCH_<XARCH> = <ARCH>'
> 
>   - Configure a default variant for kernel supported ARCH: 'XARCH_<ARCH> = <XARCH>'
> 
> - From users' perspective
> 
>   - ARCH (or XARCH) are architecture variants of a target architecture
> 
>   - the variants are XARCH names from the "ARCH_<XARCH> = <ARCH>" mapping list
> 
> PowerPC is the first user and also a very good reference architecture of
> this mapping, it has variants with different combinations of
> 32-bit/64-bit and bit endian/little endian.
> 
> To use this mapping, the other architectures can refer to PowerPC, If
> the target architecture only has one variant, XARCH is simply an alias
> of ARCH, no additional mapping required.
> 
> Suggested-by: Willy Tarreau <w@1wt.eu>
> Link: https://lore.kernel.org/lkml/20230702171715.GD16233@1wt.eu/
> Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>
> Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> ---
>  tools/testing/selftests/nolibc/Makefile | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> index 9576f1a0a98d..5afb3e7d7723 100644
> --- a/tools/testing/selftests/nolibc/Makefile
> +++ b/tools/testing/selftests/nolibc/Makefile
> @@ -14,6 +14,14 @@ include $(srctree)/scripts/subarch.include
>  ARCH = $(SUBARCH)
>  endif
>  
> +# XARCH is used to save user-input ARCH variant
> +# configure default variants for target kernel supported architectures
> +XARCH           := $(or $(XARCH_$(ARCH)),$(ARCH))
> +
> +# ARCH is supported by kernel
> +# map from user input variants to their kernel supported architectures
> +override ARCH   := $(or $(ARCH_$(XARCH)),$(XARCH))
> +
>  # kernel image names by architecture
>  IMAGE_i386       = arch/x86/boot/bzImage
>  IMAGE_x86_64     = arch/x86/boot/bzImage
> @@ -24,7 +32,7 @@ IMAGE_mips       = vmlinuz
>  IMAGE_riscv      = arch/riscv/boot/Image
>  IMAGE_s390       = arch/s390/boot/bzImage
>  IMAGE_loongarch  = arch/loongarch/boot/vmlinuz.efi
> -IMAGE            = $(IMAGE_$(ARCH))
> +IMAGE            = $(IMAGE_$(XARCH))
>  IMAGE_NAME       = $(notdir $(IMAGE))
>  
>  # default kernel configurations that appear to be usable
> @@ -37,10 +45,10 @@ DEFCONFIG_mips       = malta_defconfig
>  DEFCONFIG_riscv      = defconfig
>  DEFCONFIG_s390       = defconfig
>  DEFCONFIG_loongarch  = defconfig
> -DEFCONFIG            = $(DEFCONFIG_$(ARCH))
> +DEFCONFIG            = $(DEFCONFIG_$(XARCH))
>  
>  # extra kernel config files under configs/, include common + architecture specific
> -EXTCONFIG            = common.config $(ARCH).config
> +EXTCONFIG            = common.config $(XARCH).config
>  
>  # optional tests to run (default = all)
>  TEST =
> @@ -55,7 +63,7 @@ QEMU_ARCH_mips       = mipsel  # works with malta_defconfig
>  QEMU_ARCH_riscv      = riscv64
>  QEMU_ARCH_s390       = s390x
>  QEMU_ARCH_loongarch  = loongarch64
> -QEMU_ARCH            = $(QEMU_ARCH_$(ARCH))
> +QEMU_ARCH            = $(QEMU_ARCH_$(XARCH))
>  
>  # QEMU_ARGS : some arch-specific args to pass to qemu
>  QEMU_ARGS_i386       = -M pc -append "console=ttyS0,9600 i8042.noaux panic=-1 $(TEST:%=NOLIBC_TEST=%)"
> @@ -67,7 +75,7 @@ QEMU_ARGS_mips       = -M malta -append "panic=-1 $(TEST:%=NOLIBC_TEST=%)"
>  QEMU_ARGS_riscv      = -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
>  QEMU_ARGS_s390       = -M s390-ccw-virtio -m 1G -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
>  QEMU_ARGS_loongarch  = -M virt -append "console=ttyS0,115200 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
> -QEMU_ARGS            = $(QEMU_ARGS_$(ARCH)) $(QEMU_ARGS_EXTRA)
> +QEMU_ARGS            = $(QEMU_ARGS_$(XARCH)) $(QEMU_ARGS_EXTRA)
>  
>  # OUTPUT is only set when run from the main makefile, otherwise
>  # it defaults to this nolibc directory.
> @@ -84,7 +92,7 @@ CFLAGS_mips = -EL
>  CFLAGS_STACKPROTECTOR ?= $(call cc-option,-mstack-protector-guard=global $(call cc-option,-fstack-protector-all))
>  CFLAGS  ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 \
>  		$(call cc-option,-fno-stack-protector) \
> -		$(CFLAGS_$(ARCH)) $(CFLAGS_STACKPROTECTOR)
> +		$(CFLAGS_$(XARCH)) $(CFLAGS_STACKPROTECTOR)
>  LDFLAGS := -s
>  
>  REPORT  ?= awk '/\[OK\][\r]*$$/{p++} /\[FAIL\][\r]*$$/{if (!f) printf("\n"); f++; print;} /\[SKIPPED\][\r]*$$/{s++} \
> @@ -111,12 +119,13 @@ help:
>  	@echo ""
>  	@echo "Currently using the following variables:"
>  	@echo "  ARCH          = $(ARCH)"
> +	@echo "  XARCH         = $(XARCH)"
>  	@echo "  CROSS_COMPILE = $(CROSS_COMPILE)"
>  	@echo "  CC            = $(CC)"
>  	@echo "  OUTPUT        = $(OUTPUT)"
>  	@echo "  TEST          = $(TEST)"
> -	@echo "  QEMU_ARCH     = $(if $(QEMU_ARCH),$(QEMU_ARCH),UNKNOWN_ARCH) [determined from \$$ARCH]"
> -	@echo "  IMAGE_NAME    = $(if $(IMAGE_NAME),$(IMAGE_NAME),UNKNOWN_ARCH) [determined from \$$ARCH]"
> +	@echo "  QEMU_ARCH     = $(if $(QEMU_ARCH),$(QEMU_ARCH),UNKNOWN_ARCH) [determined from \$$XARCH]"
> +	@echo "  IMAGE_NAME    = $(if $(IMAGE_NAME),$(IMAGE_NAME),UNKNOWN_ARCH) [determined from \$$XARCH]"
>  	@echo ""
>  
>  all: run
> -- 
> 2.25.1
Willy Tarreau July 30, 2023, 7:03 a.m. UTC | #2
On Sun, Jul 30, 2023 at 02:38:18PM +0800, Zhangjin Wu wrote:
> with 'override', we are further able to use:
> 
>     $ make ARCH=powerpc
>     Makefile:29: *** ARCH=powerpc, XARCH=ppc.  Stop.
>     $ make ARCH=ppc
>     Makefile:29: *** ARCH=powerpc, XARCH=ppc.  Stop.
>     $ make ARCH=ppc64
>     Makefile:29: *** ARCH=powerpc, XARCH=ppc64.  Stop.
>     $ make ARCH=ppc64le
>     Makefile:29: *** ARCH=powerpc, XARCH=ppc64le.  Stop.
> 
> So, with 'override', users will be able to directly use the famous ARCH, it is
> able to accept powerpc, ppc, ppc64, ppc64le and users can simply ignore XARCH
> and we are able to only use XARCH as an internal variable to temply save input
> ARCH and then convert it to an internal ARCH.

But it's extremely confusing as you can see above: the user passes
one value and another one is found instead inside the makefile.
Initially I said that I didn't want that we'd put incorrect values
in ARCH so that it could be properly propagated through the various
makefile layers and include files, and that led to XARCH. 

> Without 'override', we must carefully document its usage, it may be:
> 
>     # XARCH and ARCH mapping
>     #
>     # Usage:
>     #      $ make ARCH=<kernel-supported-ARCH> XARCH=<nolibc-test-supported-variants> ...
>     #
>     #      e.g. make ARCH=powerpc XARCH=[ppc|ppc64|ppc64le]

Please let's do much simpler:

      # XARCH extends the kernel's ARCH with a few variants of the same
      # architecture that only differ by the configuration, the toolchain
      # and the Qemu program used. It is copied as-is into ARCH except for
      # a few specific values which are mapped like this:
      #  XARCH        ARCH      config
      #   ppc        powerpc    32 bits
      #   ppc64      powerpc    64 bits big endian
      #   ppc64le    powerpc    64 bits little endian
      #
      # It is recommended to only use XARCH, though it does not harm if
      # ARCH is already set. For simplicity, ARCH is sufficient for all
      # architectures where both are equal.

This way we'll even have the luxury of adding armv5, armv7 and thumb2
if we want.

>     # XARCH is used to save user-input ARCH variant
>     # configure default variants for target kernel supported architectures
> 
> For the help page, if we only use '\$$XARCH or \$$ARCH', it may mislead users:
> 
> 	@echo "  run-user               runs the executable under QEMU (uses \$$ARCH or \\$XARCH, \$$TEST)"
> 
> That's why I at last add the 'override' keyword to make sure even if users
> wrongly and only use ARCH as the argument, it must not fail, or we forcely ask
> user pass ARCH and XARCH together.
> 
> 	@echo "  run-user               runs the executable under QEMU (uses \$$ARCH and \\$XARCH, \$$TEST)"
> 
> Or we simply only and always ask users to use XARCH (as the first version does)
> for nolibc-test and let ARCH as the default one from a previous build kernel:
> 
> 	@echo "  run-user               runs the executable under QEMU (uses \$$XARCH, \$$TEST)"

No, no, no, we don't use some defaults from a previous build. That makes
problems much harder to debug and reproduce. However I'm fine with only
indicating that QEMU uses XARCH if you want.

> That means, the ugly 'override' does help us to save lots of teach work ;-)

Precisely not. In my opinion you focus a lot on first use but not enough
on troubleshooting. If someone wastes 20 minutes because they didn't want
to take 20 seconds to read a help message, it's their problem. But if
someones wastes one hour trying to debug a horribly inconsistent makefile
that modifies their most critical variables along the execution, and they
have to figure how to insert their stuff there to be accepted by the code,
it's not respectful of their time and it becomes our problem.

> I'm ok with 'override' or not, welcome your confirm, which direction do you
> prefer?

The one with least complications and which doesn't override ARCH. Also
please remember the example I provided where the test can be fired from
the top dir where ARCH has a well-defined set of values. You found yourself
inconvenient to have to change it between commands and that's why you were
trying to add menuconfig here to work around this problem.

Thanks,
Willy
Zhangjin Wu July 30, 2023, 11:36 a.m. UTC | #3
Hi, Willy

> On Sun, Jul 30, 2023 at 02:38:18PM +0800, Zhangjin Wu wrote:
> > with 'override', we are further able to use:
> > 
> >     $ make ARCH=powerpc
> >     Makefile:29: *** ARCH=powerpc, XARCH=ppc.  Stop.
> >     $ make ARCH=ppc
> >     Makefile:29: *** ARCH=powerpc, XARCH=ppc.  Stop.
> >     $ make ARCH=ppc64
> >     Makefile:29: *** ARCH=powerpc, XARCH=ppc64.  Stop.
> >     $ make ARCH=ppc64le
> >     Makefile:29: *** ARCH=powerpc, XARCH=ppc64le.  Stop.
> > 
> > So, with 'override', users will be able to directly use the famous ARCH, it is
> > able to accept powerpc, ppc, ppc64, ppc64le and users can simply ignore XARCH
> > and we are able to only use XARCH as an internal variable to temply save input
> > ARCH and then convert it to an internal ARCH.
> 
> But it's extremely confusing as you can see above: the user passes
> one value and another one is found instead inside the makefile.

Yeah, there really is some deviation and confusion.

> Initially I said that I didn't want that we'd put incorrect values
> in ARCH so that it could be properly propagated through the various
> makefile layers and include files, and that led to XARCH. 
>

I remember the good trick to set a default variant for ARCH.

> > Without 'override', we must carefully document its usage, it may be:
> > 
> >     # XARCH and ARCH mapping
> >     #
> >     # Usage:
> >     #      $ make ARCH=<kernel-supported-ARCH> XARCH=<nolibc-test-supported-variants> ...
> >     #
> >     #      e.g. make ARCH=powerpc XARCH=[ppc|ppc64|ppc64le]
> 
> Please let's do much simpler:
> 
>       # XARCH extends the kernel's ARCH with a few variants of the same
>       # architecture that only differ by the configuration, the toolchain
>       # and the Qemu program used. It is copied as-is into ARCH except for
>       # a few specific values which are mapped like this:
>       #  XARCH        ARCH      config
>       #   ppc        powerpc    32 bits
>       #   ppc64      powerpc    64 bits big endian
>       #   ppc64le    powerpc    64 bits little endian
>       #
>       # It is recommended to only use XARCH, though it does not harm if
>       # ARCH is already set. For simplicity, ARCH is sufficient for all
>       # architectures where both are equal.
>

It is clearer enough, applied.

    # XARCH extends the kernel's ARCH with a few variants of the same
    # architecture that only differ by the configuration, the toolchain
    # and the Qemu program used. It is copied as-is into ARCH except for
    # a few specific values which are mapped like this:
    #
    #  XARCH        | ARCH      | config
    #  -------------|-----------|-------------------------
    #  ppc          | powerpc   | 32 bits
    #  ppc64        | powerpc   | 64 bits big endian
    #  ppc64le      | powerpc   | 64 bits little endian
    #
    # It is recommended to only use XARCH, though it does not harm if
    # ARCH is already set. For simplicity, ARCH is sufficient for all
    # architectures where both are equal.
    
    # configure default variants for target kernel supported architectures
    XARCH_powerpc    = ppc
    XARCH            = $(or $(XARCH_$(ARCH)),$(ARCH))

    # map from user input variants to their kernel supported architectures
    ARCH_ppc         = powerpc
    ARCH_ppc64       = powerpc
    ARCH_ppc64le     = powerpc
    ARCH            := $(or $(ARCH_$(XARCH)),$(XARCH))

Any more discovery?

Note, ':=' above is required to fix up the 'recusive' warning when no
ARCH passed for the default x86.

> This way we'll even have the luxury of adding armv5, armv7 and thumb2
> if we want.
> 
> >     # XARCH is used to save user-input ARCH variant
> >     # configure default variants for target kernel supported architectures
> > 
> > For the help page, if we only use '\$$XARCH or \$$ARCH', it may mislead users:
> > 
> > 	@echo "  run-user               runs the executable under QEMU (uses \$$ARCH or \\$XARCH, \$$TEST)"
> > 
> > That's why I at last add the 'override' keyword to make sure even if users
> > wrongly and only use ARCH as the argument, it must not fail, or we forcely ask
> > user pass ARCH and XARCH together.
> > 
> > 	@echo "  run-user               runs the executable under QEMU (uses \$$ARCH and \\$XARCH, \$$TEST)"
> > 
> > Or we simply only and always ask users to use XARCH (as the first version does)
> > for nolibc-test and let ARCH as the default one from a previous build kernel:
> > 
> > 	@echo "  run-user               runs the executable under QEMU (uses \$$XARCH, \$$TEST)"
> 
> No, no, no, we don't use some defaults from a previous build. That makes
> problems much harder to debug and reproduce. However I'm fine with only
> indicating that QEMU uses XARCH if you want.
>

Ok, hope I have not misunderstood again ;-) so, here is the latest version I prepared:

    help:
    	@echo "Supported targets under selftests/nolibc:"
    	@echo "  all          call the \"run\" target below"
    	@echo "  help         this help"
    	@echo "  sysroot      create the nolibc sysroot here (uses \$$ARCH)"
    	@echo "  nolibc-test  build the executable (uses \$$CC and \$$CROSS_COMPILE)"
    	@echo "  libc-test    build an executable using the compiler's default libc instead"
    	@echo "  run-user     runs the executable under QEMU (uses \$$XARCH, \$$TEST)"
    	@echo "  initramfs    prepare the initramfs with nolibc-test"
    	@echo "  defconfig    create a fresh new default config (uses \$$XARCH)"
    	@echo "  kernel       (re)build the kernel with the initramfs (uses \$$XARCH)"
    	@echo "  run          runs the kernel in QEMU after building it (uses \$$XARCH, \$$TEST)"
    	@echo "  rerun        runs a previously prebuilt kernel in QEMU (uses \$$XARCH, \$$TEST)"
    	@echo "  clean        clean the sysroot, initramfs, build and output files"
    	@echo ""
    	@echo "The output file is \"run.out\". Test ranges may be passed using \$$TEST."
    	@echo ""
    	@echo "Currently using the following variables:"
    	@echo "  ARCH          = $(ARCH)"
    	@echo "  XARCH         = $(XARCH)"
    	@echo "  CROSS_COMPILE = $(CROSS_COMPILE)"
    	@echo "  CC            = $(CC)"
    	@echo "  OUTPUT        = $(OUTPUT)"
    	@echo "  TEST          = $(TEST)"
    	@echo "  QEMU_ARCH     = $(if $(QEMU_ARCH),$(QEMU_ARCH),UNKNOWN_ARCH) [determined from \$$XARCH]"
    	@echo "  IMAGE_NAME    = $(if $(IMAGE_NAME),$(IMAGE_NAME),UNKNOWN_ARCH) [determined from \$$XARCH]"
    	@echo ""

> > That means, the ugly 'override' does help us to save lots of teach work ;-)
> 
> Precisely not. In my opinion you focus a lot on first use but not enough
> on troubleshooting. If someone wastes 20 minutes because they didn't want
> to take 20 seconds to read a help message, it's their problem. But if
> someones wastes one hour trying to debug a horribly inconsistent makefile
> that modifies their most critical variables along the execution, and they
> have to figure how to insert their stuff there to be accepted by the code,
> it's not respectful of their time and it becomes our problem.
>

It is reasonable, we did discuss this before, the critical area is small
but is there, so, it may really introduce risks in the future, let's
give up 'override' completely.

> > I'm ok with 'override' or not, welcome your confirm, which direction do you
> > prefer?
> 
> The one with least complications and which doesn't override ARCH. Also
> please remember the example I provided where the test can be fired from
> the top dir where ARCH has a well-defined set of values. You found yourself
> inconvenient to have to change it between commands and that's why you were
> trying to add menuconfig here to work around this problem.

Best regards,
Zhangjin

> 
> Thanks,
> Willy
diff mbox series

Patch

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 9576f1a0a98d..5afb3e7d7723 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -14,6 +14,14 @@  include $(srctree)/scripts/subarch.include
 ARCH = $(SUBARCH)
 endif
 
+# XARCH is used to save user-input ARCH variant
+# configure default variants for target kernel supported architectures
+XARCH           := $(or $(XARCH_$(ARCH)),$(ARCH))
+
+# ARCH is supported by kernel
+# map from user input variants to their kernel supported architectures
+override ARCH   := $(or $(ARCH_$(XARCH)),$(XARCH))
+
 # kernel image names by architecture
 IMAGE_i386       = arch/x86/boot/bzImage
 IMAGE_x86_64     = arch/x86/boot/bzImage
@@ -24,7 +32,7 @@  IMAGE_mips       = vmlinuz
 IMAGE_riscv      = arch/riscv/boot/Image
 IMAGE_s390       = arch/s390/boot/bzImage
 IMAGE_loongarch  = arch/loongarch/boot/vmlinuz.efi
-IMAGE            = $(IMAGE_$(ARCH))
+IMAGE            = $(IMAGE_$(XARCH))
 IMAGE_NAME       = $(notdir $(IMAGE))
 
 # default kernel configurations that appear to be usable
@@ -37,10 +45,10 @@  DEFCONFIG_mips       = malta_defconfig
 DEFCONFIG_riscv      = defconfig
 DEFCONFIG_s390       = defconfig
 DEFCONFIG_loongarch  = defconfig
-DEFCONFIG            = $(DEFCONFIG_$(ARCH))
+DEFCONFIG            = $(DEFCONFIG_$(XARCH))
 
 # extra kernel config files under configs/, include common + architecture specific
-EXTCONFIG            = common.config $(ARCH).config
+EXTCONFIG            = common.config $(XARCH).config
 
 # optional tests to run (default = all)
 TEST =
@@ -55,7 +63,7 @@  QEMU_ARCH_mips       = mipsel  # works with malta_defconfig
 QEMU_ARCH_riscv      = riscv64
 QEMU_ARCH_s390       = s390x
 QEMU_ARCH_loongarch  = loongarch64
-QEMU_ARCH            = $(QEMU_ARCH_$(ARCH))
+QEMU_ARCH            = $(QEMU_ARCH_$(XARCH))
 
 # QEMU_ARGS : some arch-specific args to pass to qemu
 QEMU_ARGS_i386       = -M pc -append "console=ttyS0,9600 i8042.noaux panic=-1 $(TEST:%=NOLIBC_TEST=%)"
@@ -67,7 +75,7 @@  QEMU_ARGS_mips       = -M malta -append "panic=-1 $(TEST:%=NOLIBC_TEST=%)"
 QEMU_ARGS_riscv      = -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
 QEMU_ARGS_s390       = -M s390-ccw-virtio -m 1G -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
 QEMU_ARGS_loongarch  = -M virt -append "console=ttyS0,115200 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
-QEMU_ARGS            = $(QEMU_ARGS_$(ARCH)) $(QEMU_ARGS_EXTRA)
+QEMU_ARGS            = $(QEMU_ARGS_$(XARCH)) $(QEMU_ARGS_EXTRA)
 
 # OUTPUT is only set when run from the main makefile, otherwise
 # it defaults to this nolibc directory.
@@ -84,7 +92,7 @@  CFLAGS_mips = -EL
 CFLAGS_STACKPROTECTOR ?= $(call cc-option,-mstack-protector-guard=global $(call cc-option,-fstack-protector-all))
 CFLAGS  ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 \
 		$(call cc-option,-fno-stack-protector) \
-		$(CFLAGS_$(ARCH)) $(CFLAGS_STACKPROTECTOR)
+		$(CFLAGS_$(XARCH)) $(CFLAGS_STACKPROTECTOR)
 LDFLAGS := -s
 
 REPORT  ?= awk '/\[OK\][\r]*$$/{p++} /\[FAIL\][\r]*$$/{if (!f) printf("\n"); f++; print;} /\[SKIPPED\][\r]*$$/{s++} \
@@ -111,12 +119,13 @@  help:
 	@echo ""
 	@echo "Currently using the following variables:"
 	@echo "  ARCH          = $(ARCH)"
+	@echo "  XARCH         = $(XARCH)"
 	@echo "  CROSS_COMPILE = $(CROSS_COMPILE)"
 	@echo "  CC            = $(CC)"
 	@echo "  OUTPUT        = $(OUTPUT)"
 	@echo "  TEST          = $(TEST)"
-	@echo "  QEMU_ARCH     = $(if $(QEMU_ARCH),$(QEMU_ARCH),UNKNOWN_ARCH) [determined from \$$ARCH]"
-	@echo "  IMAGE_NAME    = $(if $(IMAGE_NAME),$(IMAGE_NAME),UNKNOWN_ARCH) [determined from \$$ARCH]"
+	@echo "  QEMU_ARCH     = $(if $(QEMU_ARCH),$(QEMU_ARCH),UNKNOWN_ARCH) [determined from \$$XARCH]"
+	@echo "  IMAGE_NAME    = $(if $(IMAGE_NAME),$(IMAGE_NAME),UNKNOWN_ARCH) [determined from \$$XARCH]"
 	@echo ""
 
 all: run