Re: [PATCH v3 3/3] selftests/nolibc: riscv: customize makefile for rv32

Arnd, Thomas, Willy

...

...

+# Additional ARCH settings for riscv
+ifeq ($(ARCH),riscv32)
+        SRCARCH := riscv
+endif
+ifeq ($(ARCH),riscv64)
+        SRCARCH := riscv
+endif
+
 export cross_compiling :=
 ifneq ($(SRCARCH),$(SUBARCH))
 cross_compiling := 1

I've never been a big fan of the top-level $(ARCH) setting in the kernel, is there a reason this has to be the same as the variable in tools/include/nolibc? If not, I'd just leave the Linux Makefile unchanged.

For userspace we have a lot more target names than arch/*/ directories in the kernel, and I don't think I'd want to enumerate all the possibilities in the build system globally.

Good news, I did find a better solution without touching the top-level Makefile, that is overriding the ARCH to 'riscv' just before the targets and after we got the necessary settings with the original ARCH=riscv32 or ARCH=riscv64, but it requires to convert the '=' assignments to ':=', which is not that hard to do and it is more acceptable, just verified it and it worked well.

...

LDFLAGS := -s

+# top-level kernel Makefile only accept ARCH=riscv, override ARCH to make kernel happy +ifneq ($(findstring riscv,$(ARCH)),) +override ARCH := riscv +endif + help: @echo "Supported targets under selftests/nolibc:" @echo " all call the "run" target below"

This change is not that big, and the left changes can keep consistent with the other platforms. but I still need to add a standalone patch to convert the '=' to ':=' to avoid the before setting using our new overridded ARCH.

++ b/tools/testing/selftests/nolibc/Makefile @@ -26,7 +26,7 @@ IMAGE_riscv64 = arch/riscv/boot/Image IMAGE_riscv = arch/riscv/boot/Image IMAGE_s390 = arch/s390/boot/bzImage IMAGE_loongarch = arch/loongarch/boot/vmlinuz.efi -IMAGE = $(IMAGE_$(ARCH)) +IMAGE := $(IMAGE_$(ARCH)) IMAGE_NAME = $(notdir $(IMAGE))

# default kernel configurations that appear to be usable @@ -41,7 +41,7 @@ DEFCONFIG_riscv64 = defconfig DEFCONFIG_riscv = defconfig DEFCONFIG_s390 = defconfig DEFCONFIG_loongarch = defconfig -DEFCONFIG = $(DEFCONFIG_$(ARCH)) +DEFCONFIG := $(DEFCONFIG_$(ARCH))

# optional tests to run (default = all) TEST = @@ -58,7 +58,7 @@ QEMU_ARCH_riscv64 = riscv64 QEMU_ARCH_riscv = riscv64 QEMU_ARCH_s390 = s390x QEMU_ARCH_loongarch = loongarch64 -QEMU_ARCH = $(QEMU_ARCH_$(ARCH)) +QEMU_ARCH := $(QEMU_ARCH_$(ARCH))

# 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=%)" @@ -72,7 +72,7 @@ QEMU_ARGS_riscv64 = -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_T 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_$(ARCH)) $(QEMU_ARGS_EXTRA)

# OUTPUT is only set when run from the main makefile, otherwise # it defaults to this nolibc directory. @@ -87,11 +87,18 @@ endif CFLAGS_riscv32 = -march=rv32im -mabi=ilp32 CFLAGS_s390 = -m64 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 \ +CFLAGS_default := -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 \ $(call cc-option,-fno-stack-protector) \ $(CFLAGS_$(ARCH)) $(CFLAGS_STACKPROTECTOR) + +CFLAGS ?= $(CFLAGS_default)

Thanks a lot, will send v4 later.

Best regards, Zhangjin

Ok, agree very much, it is the root cause why we used the old method before, because I don't want to touch the top-level Makefile, here explains the details again just as did for Thomas and Willy [1] ;-)

Without the top-level makefile change, we must add something in selftests/nolibc/Makefile like this, because the kernel makefile doesn't accept something like ARCH=riscv32 and ARCH=riscv64 currently, it only accepts ARCH=riscv (will paste the code later).

ifneq ($(findstring riscv,$(ARCH)),)
  _ARCH = riscv
else
  _ARCH = $(ARCH)
endif

...

sysroot/$(ARCH)/include:

$(Q)rm -rf sysroot/$(ARCH) sysroot/sysroot $(QUIET_MKDIR)mkdir -p sysroot $(Q)$(MAKE) -C ../../../include/nolibc ARCH=$(_ARCH) OUTPUT=$(CURDIR)/sysroot/ headers_standalone $(Q)mv sysroot/sysroot sysroot/$(ARCH)

defconfig:
	$(Q)$(MAKE) -C $(srctree) ARCH=$(_ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) mrproper $(DEFCONFIG) prepare

kernel: initramfs
	$(Q)$(MAKE) -C $(srctree) ARCH=$(_ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) $(IMAGE_NAME) CONFIG_INITRAMFS_SOURCE=$(CURDIR)/initramfs

The above change really works, but it looks not that good, this is the mixing use of _ARCH and ARCH I mentioned in last reply.

Otherwise, we will get such error:

$ make run ARCH=riscv64 CROSS_COMPILE=riscv64-linux-gnu-
  MKDIR   sysroot/riscv64/include
make[1]: Entering directory '/labs/linux-lab/src/linux-stable/tools/include/nolibc'
make[2]: Entering directory '/labs/linux-lab/src/linux-stable'
Makefile:763: arch/riscv64/Makefile: No such file or directory
make[2]: *** No rule to make target 'arch/riscv64/Makefile'.  Stop.
make[2]: Leaving directory '/labs/linux-lab/src/linux-stable'
make[1]: *** [Makefile:87: headers_standalone] Error 2
make[1]: Leaving directory '/labs/linux-lab/src/linux-stable/tools/include/nolibc'
make: *** [Makefile:129: sysroot/riscv64/include] Error 2
$ make run ARCH=riscv32 CROSS_COMPILE=riscv64-linux-gnu-
  MKDIR   sysroot/riscv32/include
make[1]: Entering directory '/labs/linux-lab/src/linux-stable/tools/include/nolibc'
make[2]: Entering directory '/labs/linux-lab/src/linux-stable'
Makefile:763: arch/riscv32/Makefile: No such file or directory
make[2]: *** No rule to make target 'arch/riscv32/Makefile'.  Stop.
make[2]: Leaving directory '/labs/linux-lab/src/linux-stable'
make[1]: *** [Makefile:87: headers_standalone] Error 2
make[1]: Leaving directory '/labs/linux-lab/src/linux-stable/tools/include/nolibc'
make: *** [Makefile:129: sysroot/riscv32/include] Error 2

That's because in top-level Makefile, it doesn't accept ARCH=riscv32 and ARCH=riscv64, but x86 and sparc and even parisc support such variants, this allows the ARCH variants share the same arch/<SRCARCH>/ source code tree, otherwise, they will directly find the arch/<ARCH>/ source code, then fails.

top-level Makefile:

...
ARCH            ?= $(SUBARCH)

# Architecture as present in compile.h
UTS_MACHINE     := $(ARCH)
SRCARCH         := $(ARCH)   ---> SRCARCH is assigned as ARCH by default

# Additional ARCH settings for x86
ifeq ($(ARCH),i386)
        SRCARCH := x86
endif
ifeq ($(ARCH),x86_64)
        SRCARCH := x86
endif

# Additional ARCH settings for sparc
ifeq ($(ARCH),sparc32)
       SRCARCH := sparc
endif
ifeq ($(ARCH),sparc64)
       SRCARCH := sparc
endif

# Additional ARCH settings for parisc
ifeq ($(ARCH),parisc64)
       SRCARCH := parisc
endif

So, to really align with x86, we should let the top-level makefile be able to get the right SRCARCH for riscv32 and riscv64 too ;-)

I even tried to pass SRCARCH=riscv to the top-level Makefile, but it failed:

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 1b2247a6365d..04067776b569 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -14,6 +14,10 @@ include $(srctree)/scripts/subarch.include
 ARCH = $(SUBARCH)
 endif

+ifneq ($(findstring riscv,$(ARCH)),)
+SRCARCH := SRCARCH=riscv
+endif
+
 # kernel image names by architecture
 IMAGE_i386       = arch/x86/boot/bzImage
 IMAGE_x86_64     = arch/x86/boot/bzImage
@@ -126,7 +130,7 @@ sysroot: sysroot/$(ARCH)/include
 sysroot/$(ARCH)/include:
        $(Q)rm -rf sysroot/$(ARCH) sysroot/sysroot
        $(QUIET_MKDIR)mkdir -p sysroot
-       $(Q)$(MAKE) -C ../../../include/nolibc ARCH=$(ARCH) OUTPUT=$(CURDIR)/sysroot/ headers_standalone
+       $(Q)$(MAKE) -C ../../../include/nolibc ARCH=$(ARCH) $(SRCARCH) OUTPUT=$(CURDIR)/sysroot/ headers_standalone
        $(Q)mv sysroot/sysroot sysroot/$(ARCH)

 nolibc-test: nolibc-test.c sysroot/$(ARCH)/include
@@ -150,10 +154,10 @@ initramfs: nolibc-test
        $(Q)cp nolibc-test initramfs/init

 defconfig:
-       $(Q)$(MAKE) -C $(srctree) ARCH=$(ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) mrproper $(DEFCONFIG) prepare
+       $(Q)$(MAKE) -C $(srctree) ARCH=$(ARCH) $(SRCARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) mrproper $(DEFCONFIG) prepare

 kernel: initramfs
-       $(Q)$(MAKE) -C $(srctree) ARCH=$(ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) $(IMAGE_NAME) CONFIG_INITRAMFS_SOURCE=$(CURDIR)/initramfs
+       $(Q)$(MAKE) -C $(srctree) ARCH=$(ARCH) $(SRCARCH )CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) $(IMAGE_NAME) CONFIG_INITRAMFS_SOURCE=$(CURDIR)/initramfs

 # run the tests after building the kernel
 run: kernel

$ make run ARCH=riscv32 CROSS_COMPILE=riscv64-linux-gnu- QEMU_ARGS_EXTRA="-bios /labs/linux-lab/opensbi-riscv32-generic-fw_dynamic.bin"
  MKDIR   sysroot/riscv32/include
make[1]: Entering directory '/labs/linux-lab/src/linux-stable/tools/include/nolibc'
make[2]: Entering directory '/labs/linux-lab/src/linux-stable'
Makefile:397: srcarch: riscv
make[2]: Leaving directory '/labs/linux-lab/src/linux-stable'
make[2]: Entering directory '/labs/linux-lab/src/linux-stable'
Makefile:397: srcarch: riscv
  INSTALL /labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/sysroot/sysroot/include
make[2]: Leaving directory '/labs/linux-lab/src/linux-stable'
make[1]: Leaving directory '/labs/linux-lab/src/linux-stable/tools/include/nolibc'
  CC      nolibc-test
  MKDIR   initramfs
  INSTALL initramfs/init
make[1]: Entering directory '/labs/linux-lab/src/linux-stable'
Makefile:397: srcarch: riscv32
  SYNC    include/config/auto.conf.cmd
Makefile:397: srcarch: riscv32
Makefile:687: arch/riscv32/Makefile: No such file or directory
make[2]: *** No rule to make target 'arch/riscv32/Makefile'.  Stop.
make[1]: *** [Makefile:795: include/config/auto.conf.cmd] Error 2
make[1]: Leaving directory '/labs/linux-lab/src/linux-stable'

So, to keep consistent eventually, perhaps we do need to touch the top-level Makefile.

Best regards, Zhangjin

...

Arnd