Hi,
On Wed, Jun 07, 2023 at 09:20:32AM +0800, Zhangjin Wu wrote:
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 := 1I'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.
Actually it's not exactly used by nolibc, except to pass to the kernel for the install part to extract kernel headers (make headers_install). It's one of the parts that has required to stick to most of the kernel's variables very closely (the other one being for nolibc-test which needs to build a kernel).
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 +
That can be one approach indeed. Another one if we continue to face difficulties for this one would be to use a distinct KARCH variable to assign to ARCH in all kernel-specific operations.
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.
I don't even see why the other ones below are needed, given that as long as they remain assigned as macros, they will be replaced in-place where they are used, so they will reference the last known assignment to 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)
Why did you need to split this one like this instead of proceeding like for the other ones ? Because of the "?=" maybe ? Please double-check that you really need to turn this from a macro to a variable, if as I suspect it it's not needed, it would be even simpler.
Thanks, Willy