Hi, Willy
This is the revision of the v4 part2 of support for rv32 [1], this further split the generic KARCH code out of the old rv32 compile patch and also add kernel specific KARCH and nolibc specific NARCH for tools/include/nolibc/Makefile too.
This is rebased on the dev.2023.06.14a branch of linux-rcu repo [2] with basic run-user and run tests.
Changes from v4 -> v5:
* selftests/nolibc: allow customize kernel specific ARCH variable
The KARCH customize support part splitted out of the old rv32 compile patch and removed the one passed to tools/include/nolibc/Makefile.
* tools/nolibc: add kernel and nolibc specific ARCH variables
Pass original ARCH to tools/include/nolibc/Makefile, add KARCH and NARCH for kernel and nolibc respectively.
* selftests/nolibc: riscv: customize makefile for rv32
Now, it is rv32 specific, no generic code.
Best regards, Zhangjin --- [1]: https://lore.kernel.org/linux-riscv/cover.1686128703.git.falcon@tinylab.org/ [2]: https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/
Zhangjin Wu (5): tools/nolibc: fix up #error compile failures with -ENOSYS tools/nolibc: fix up undeclared syscall macros with #ifdef and -ENOSYS selftests/nolibc: allow customize kernel specific ARCH variable tools/nolibc: add kernel and nolibc specific ARCH variables selftests/nolibc: riscv: customize makefile for rv32
tools/include/nolibc/Makefile | 18 +++++++++--- tools/include/nolibc/sys.h | 38 ++++++++++++++++--------- tools/testing/selftests/nolibc/Makefile | 18 ++++++++++-- 3 files changed, 55 insertions(+), 19 deletions(-)
Compiling nolibc for rv32 got such errors:
In file included from nolibc/sysroot/riscv/include/nolibc.h:99, from nolibc/sysroot/riscv/include/errno.h:26, from nolibc/sysroot/riscv/include/stdio.h:14, from tools/testing/selftests/nolibc/nolibc-test.c:12: nolibc/sysroot/riscv/include/sys.h:946:2: error: #error Neither __NR_ppoll nor __NR_poll defined, cannot implement sys_poll() 946 | #error Neither __NR_ppoll nor __NR_poll defined, cannot implement sys_poll() | ^~~~~ nolibc/sysroot/riscv/include/sys.h:1062:2: error: #error None of __NR_select, __NR_pselect6, nor __NR__newselect defined, cannot implement sys_select() 1062 | #error None of __NR_select, __NR_pselect6, nor __NR__newselect defined, cannot implement sys_select()
If a syscall is not supported by a target platform, 'return -ENOSYS' is better than '#error', which lets the other syscalls work as-is and allows developers to fix up the test failures reported by nolibc-test one by one later.
This converts all of the '#error' to 'return -ENOSYS', so, all of the '#error' failures are fixed.
Suggested-by: Arnd Bergmann arnd@arndb.de Link: https://lore.kernel.org/linux-riscv/5e7d2adf-e96f-41ca-a4c6-5c87a25d4c9c@app... Reviewed-by: Arnd Bergmann arnd@arndb.de Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/include/nolibc/sys.h | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index 856249a11890..78c86f124335 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -124,7 +124,7 @@ int sys_chmod(const char *path, mode_t mode) #elif defined(__NR_chmod) return my_syscall2(__NR_chmod, path, mode); #else -#error Neither __NR_fchmodat nor __NR_chmod defined, cannot implement sys_chmod() + return -ENOSYS; #endif }
@@ -153,7 +153,7 @@ int sys_chown(const char *path, uid_t owner, gid_t group) #elif defined(__NR_chown) return my_syscall3(__NR_chown, path, owner, group); #else -#error Neither __NR_fchownat nor __NR_chown defined, cannot implement sys_chown() + return -ENOSYS; #endif }
@@ -251,7 +251,7 @@ int sys_dup2(int old, int new) #elif defined(__NR_dup2) return my_syscall2(__NR_dup2, old, new); #else -#error Neither __NR_dup3 nor __NR_dup2 defined, cannot implement sys_dup2() + return -ENOSYS; #endif }
@@ -351,7 +351,7 @@ pid_t sys_fork(void) #elif defined(__NR_fork) return my_syscall0(__NR_fork); #else -#error Neither __NR_clone nor __NR_fork defined, cannot implement sys_fork() + return -ENOSYS; #endif } #endif @@ -648,7 +648,7 @@ int sys_link(const char *old, const char *new) #elif defined(__NR_link) return my_syscall2(__NR_link, old, new); #else -#error Neither __NR_linkat nor __NR_link defined, cannot implement sys_link() + return -ENOSYS; #endif }
@@ -700,7 +700,7 @@ int sys_mkdir(const char *path, mode_t mode) #elif defined(__NR_mkdir) return my_syscall2(__NR_mkdir, path, mode); #else -#error Neither __NR_mkdirat nor __NR_mkdir defined, cannot implement sys_mkdir() + return -ENOSYS; #endif }
@@ -729,7 +729,7 @@ long sys_mknod(const char *path, mode_t mode, dev_t dev) #elif defined(__NR_mknod) return my_syscall3(__NR_mknod, path, mode, dev); #else -#error Neither __NR_mknodat nor __NR_mknod defined, cannot implement sys_mknod() + return -ENOSYS; #endif }
@@ -848,7 +848,7 @@ int sys_open(const char *path, int flags, mode_t mode) #elif defined(__NR_open) return my_syscall3(__NR_open, path, flags, mode); #else -#error Neither __NR_openat nor __NR_open defined, cannot implement sys_open() + return -ENOSYS; #endif }
@@ -943,7 +943,7 @@ int sys_poll(struct pollfd *fds, int nfds, int timeout) #elif defined(__NR_poll) return my_syscall3(__NR_poll, fds, nfds, timeout); #else -#error Neither __NR_ppoll nor __NR_poll defined, cannot implement sys_poll() + return -ENOSYS; #endif }
@@ -1059,7 +1059,7 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva #endif return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout); #else -#error None of __NR_select, __NR_pselect6, nor __NR__newselect defined, cannot implement sys_select() + return -ENOSYS; #endif }
@@ -1196,7 +1196,7 @@ int sys_stat(const char *path, struct stat *buf) #elif defined(__NR_stat) ret = my_syscall2(__NR_stat, path, &stat); #else -#error Neither __NR_newfstatat nor __NR_stat defined, cannot implement sys_stat() + return -ENOSYS; #endif buf->st_dev = stat.st_dev; buf->st_ino = stat.st_ino; @@ -1243,7 +1243,7 @@ int sys_symlink(const char *old, const char *new) #elif defined(__NR_symlink) return my_syscall2(__NR_symlink, old, new); #else -#error Neither __NR_symlinkat nor __NR_symlink defined, cannot implement sys_symlink() + return -ENOSYS; #endif }
@@ -1312,7 +1312,7 @@ int sys_unlink(const char *path) #elif defined(__NR_unlink) return my_syscall1(__NR_unlink, path); #else -#error Neither __NR_unlinkat nor __NR_unlink defined, cannot implement sys_unlink() + return -ENOSYS; #endif }
Hi Zhangjin,
On Mon, Jun 19, 2023 at 08:24:15PM +0800, Zhangjin Wu wrote:
Compiling nolibc for rv32 got such errors:
In file included from nolibc/sysroot/riscv/include/nolibc.h:99, from nolibc/sysroot/riscv/include/errno.h:26, from nolibc/sysroot/riscv/include/stdio.h:14, from tools/testing/selftests/nolibc/nolibc-test.c:12: nolibc/sysroot/riscv/include/sys.h:946:2: error: #error Neither __NR_ppoll nor __NR_poll defined, cannot implement sys_poll() 946 | #error Neither __NR_ppoll nor __NR_poll defined, cannot implement sys_poll() | ^~~~~ nolibc/sysroot/riscv/include/sys.h:1062:2: error: #error None of __NR_select, __NR_pselect6, nor __NR__newselect defined, cannot implement sys_select() 1062 | #error None of __NR_select, __NR_pselect6, nor __NR__newselect defined, cannot implement sys_select()
If a syscall is not supported by a target platform, 'return -ENOSYS' is better than '#error', which lets the other syscalls work as-is and allows developers to fix up the test failures reported by nolibc-test one by one later.
This converts all of the '#error' to 'return -ENOSYS', so, all of the '#error' failures are fixed.
Yeah, I like this! Initially we wanted the ifdef to spot include file issues (which were the major concern at the beginning), and the selftest was very limited. But now the coverage has significantly improved, the major concern is cross-platform support and we know we won't get all of it at once, so -ENOSYS is far better.
Thanks for this! Willy
Compiling nolibc for rv32 got such errors:
nolibc/sysroot/riscv/include/sys.h: In function ‘sys_gettimeofday’: nolibc/sysroot/riscv/include/sys.h:557:21: error: ‘__NR_gettimeofday’ undeclared (first use in this function); did you mean ‘sys_gettimeofday’? 557 | return my_syscall2(__NR_gettimeofday, tv, tz); | ^~~~~~~~~~~~~~~~~ nolibc/sysroot/riscv/include/sys.h: In function ‘sys_lseek’: nolibc/sysroot/riscv/include/sys.h:675:21: error: ‘__NR_lseek’ undeclared (first use in this function) 675 | return my_syscall3(__NR_lseek, fd, offset, whence); | ^~~~~~~~~~ nolibc/sysroot/riscv/include/sys.h: In function ‘sys_wait4’: nolibc/sysroot/riscv/include/sys.h:1341:21: error: ‘__NR_wait4’ undeclared (first use in this function) 1341 | return my_syscall4(__NR_wait4, pid, status, options, rusage);
If a syscall macro is not supported by a target platform, wrap it with '#ifdef' and 'return -ENOSYS' for the '#else' branch, which lets the other syscalls work as-is and allows developers to fix up the test failures reported by nolibc-test one by one later.
This wraps all of the failed syscall macros with '#ifdef' and 'return -ENOSYS' for the '#else' branch, so, all of the undeclared failures are fixed.
Suggested-by: Arnd Bergmann arnd@arndb.de Link: https://lore.kernel.org/linux-riscv/5e7d2adf-e96f-41ca-a4c6-5c87a25d4c9c@app... Reviewed-by: Arnd Bergmann arnd@arndb.de Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/include/nolibc/sys.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index 78c86f124335..5464f93e863e 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -554,7 +554,11 @@ long getpagesize(void) static __attribute__((unused)) int sys_gettimeofday(struct timeval *tv, struct timezone *tz) { +#ifdef __NR_gettimeofday return my_syscall2(__NR_gettimeofday, tv, tz); +#else + return -ENOSYS; +#endif }
static __attribute__((unused)) @@ -672,7 +676,11 @@ int link(const char *old, const char *new) static __attribute__((unused)) off_t sys_lseek(int fd, off_t offset, int whence) { +#ifdef __NR_lseek return my_syscall3(__NR_lseek, fd, offset, whence); +#else + return -ENOSYS; +#endif }
static __attribute__((unused)) @@ -1338,7 +1346,11 @@ int unlink(const char *path) static __attribute__((unused)) pid_t sys_wait4(pid_t pid, int *status, int options, struct rusage *rusage) { +#ifdef __NR_wait4 return my_syscall4(__NR_wait4, pid, status, options, rusage); +#else + return -ENOSYS; +#endif }
static __attribute__((unused))
Some architectures (e.g. riscv, mips) share the same ARCH variable (the one supported by top-level kernel Makefile) between 32bit and 64bit architecture variants and even more, some of them have little endian and big endian variants.
Let's add KARCH to allow architectures customize their own kernel specific ARCH variable, so, ARCH=$(KARCH) is required by all of the top-level kernel Makefile targets.
Suggested-by: Willy Tarreau w@1wt.eu Link: https://lore.kernel.org/lkml/ZIAywHvr6UB1J4of@1wt.eu/ Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/testing/selftests/nolibc/Makefile | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile index 000621f21adc..ebecb8cfd947 100644 --- a/tools/testing/selftests/nolibc/Makefile +++ b/tools/testing/selftests/nolibc/Makefile @@ -14,6 +14,9 @@ include $(srctree)/scripts/subarch.include ARCH = $(SUBARCH) endif
+# kernel supported ARCH names by architecture +KARCH = $(or $(KARCH_$(ARCH)),$(ARCH)) + # kernel image names by architecture IMAGE_i386 = arch/x86/boot/bzImage IMAGE_x86_64 = arch/x86/boot/bzImage @@ -143,10 +146,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=$(KARCH) 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=$(KARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) $(IMAGE_NAME) CONFIG_INITRAMFS_SOURCE=$(CURDIR)/initramfs
# run the tests after building the kernel run: kernel
Like the KARCH added for tools/testing/selftests/nolibc/Makefile, adds KARCH for tools/include/nolibc/Makefile too, at the same time, adds NARCH for the ARCH supported by nolibc (arch-<NARCH>.h).
It allows users to customize both kernel and nolibc specific ARCH variables for different architectures and their variants easily.
Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/include/nolibc/Makefile | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/tools/include/nolibc/Makefile b/tools/include/nolibc/Makefile index 64d67b080744..14a6416fa57f 100644 --- a/tools/include/nolibc/Makefile +++ b/tools/include/nolibc/Makefile @@ -23,8 +23,14 @@ else Q=@ endif
-nolibc_arch := $(patsubst arm64,aarch64,$(ARCH)) -arch_file := arch-$(nolibc_arch).h +# kernel supported ARCH names by architecture +KARCH = $(or $(KARCH_$(ARCH)),$(ARCH)) + +# nolibc supported ARCH names by architecture +NARCH_arm64 = aarch64 +NARCH = $(or $(NARCH_$(ARCH)),$(ARCH)) + +arch_file := arch-$(NARCH).h all_files := \ compiler.h \ ctype.h \ @@ -83,8 +89,8 @@ headers: fi > $(OUTPUT)sysroot/include/arch.h
headers_standalone: headers - $(Q)$(MAKE) -C $(srctree) headers - $(Q)$(MAKE) -C $(srctree) headers_install INSTALL_HDR_PATH=$(OUTPUT)sysroot + $(Q)$(MAKE) -C $(srctree) ARCH=$(KARCH) headers + $(Q)$(MAKE) -C $(srctree) ARCH=$(KARCH) headers_install INSTALL_HDR_PATH=$(OUTPUT)sysroot
clean: $(call QUIET_CLEAN, nolibc) rm -rf "$(OUTPUT)sysroot"
Both riscv64 and riscv32 have:
* the same arch/riscv source code tree * the same tools/include/nolibc/arch-riscv.h * the same ARCH=riscv value passed to top-level kernel Makefile
The only differences are:
* riscv64 uses defconfig, riscv32 uses rv32_defconfig * riscv64 uses qemu-system-riscv64, riscv32 uses qemu-system-riscv32 * riscv32 has different compiler options (-march= and -mabi=)
So, riscv32 can share most of the settings with riscv64, add riscv32 support like the original ARCH=riscv support.
To align with x86, the default riscv is reserved for riscv64 and a new riscv64 is also added to allow users pass ARCH=riscv64 directly.
Since top-level kernel Makefile only accept ARCH=riscv, to make kernel happy, let's set kernel specific KARCH as riscv for both riscv32 and riscv64.
And since they share the same arch-riscv.h, let's set nolibc specific NARCH as riscv too.
Usage:
$ make defconfig ARCH=riscv32 CROSS_COMPILE=riscv64-linux-gnu- ... $ make run ARCH=riscv32 CROSS_COMPILE=riscv64-linux-gnu- ...
Suggested-by: Thomas Weißschuh linux@weissschuh.net Link: https://lore.kernel.org/linux-riscv/4a3b1cdf-91d5-4668-925e-21f8f5c64a92@t-8... Suggested-by: Arnd Bergmann arnd@arndb.de Link: https://lore.kernel.org/linux-riscv/d1c83340-af4c-4780-a101-b9d22b47379c@app... Suggested-by: Willy Tarreau w@1wt.eu Link: https://lore.kernel.org/lkml/ZIAywHvr6UB1J4of@1wt.eu/ Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/include/nolibc/Makefile | 4 ++++ tools/testing/selftests/nolibc/Makefile | 11 +++++++++++ 2 files changed, 15 insertions(+)
diff --git a/tools/include/nolibc/Makefile b/tools/include/nolibc/Makefile index 14a6416fa57f..875e13e3c851 100644 --- a/tools/include/nolibc/Makefile +++ b/tools/include/nolibc/Makefile @@ -24,9 +24,13 @@ Q=@ endif
# kernel supported ARCH names by architecture +KARCH_riscv32 = riscv +KARCH_riscv64 = riscv KARCH = $(or $(KARCH_$(ARCH)),$(ARCH))
# nolibc supported ARCH names by architecture +NARCH_riscv32 = riscv +NARCH_riscv64 = riscv NARCH_arm64 = aarch64 NARCH = $(or $(NARCH_$(ARCH)),$(ARCH))
diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile index ebecb8cfd947..848884204a84 100644 --- a/tools/testing/selftests/nolibc/Makefile +++ b/tools/testing/selftests/nolibc/Makefile @@ -15,6 +15,8 @@ ARCH = $(SUBARCH) endif
# kernel supported ARCH names by architecture +KARCH_riscv32 = riscv +KARCH_riscv64 = riscv KARCH = $(or $(KARCH_$(ARCH)),$(ARCH))
# kernel image names by architecture @@ -24,6 +26,8 @@ IMAGE_x86 = arch/x86/boot/bzImage IMAGE_arm64 = arch/arm64/boot/Image IMAGE_arm = arch/arm/boot/zImage IMAGE_mips = vmlinuz +IMAGE_riscv32 = arch/riscv/boot/Image +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 @@ -37,6 +41,8 @@ DEFCONFIG_x86 = defconfig DEFCONFIG_arm64 = defconfig DEFCONFIG_arm = multi_v7_defconfig DEFCONFIG_mips = malta_defconfig +DEFCONFIG_riscv32 = rv32_defconfig +DEFCONFIG_riscv64 = defconfig DEFCONFIG_riscv = defconfig DEFCONFIG_s390 = defconfig DEFCONFIG_loongarch = defconfig @@ -52,6 +58,8 @@ QEMU_ARCH_x86 = x86_64 QEMU_ARCH_arm64 = aarch64 QEMU_ARCH_arm = arm QEMU_ARCH_mips = mipsel # works with malta_defconfig +QEMU_ARCH_riscv32 = riscv32 +QEMU_ARCH_riscv64 = riscv64 QEMU_ARCH_riscv = riscv64 QEMU_ARCH_s390 = s390x QEMU_ARCH_loongarch = loongarch64 @@ -64,6 +72,8 @@ QEMU_ARGS_x86 = -M pc -append "console=ttyS0,9600 i8042.noaux panic=-1 $( QEMU_ARGS_arm64 = -M virt -cpu cortex-a53 -append "panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_arm = -M virt -append "panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_mips = -M malta -append "panic=-1 $(TEST:%=NOLIBC_TEST=%)" +QEMU_ARGS_riscv32 = -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" +QEMU_ARGS_riscv64 = -M virt -append "console=ttyS0 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=%)" @@ -79,6 +89,7 @@ else Q=@ endif
+CFLAGS_riscv32 = -march=rv32im -mabi=ilp32 CFLAGS_s390 = -m64 CFLAGS_mips = -EL CFLAGS_STACKPROTECTOR ?= $(call cc-option,-mstack-protector-guard=global $(call cc-option,-fstack-protector-all))
On Mon, Jun 19, 2023 at 08:29:38PM +0800, Zhangjin Wu wrote:
Both riscv64 and riscv32 have:
- the same arch/riscv source code tree
- the same tools/include/nolibc/arch-riscv.h
- the same ARCH=riscv value passed to top-level kernel Makefile
The only differences are:
- riscv64 uses defconfig, riscv32 uses rv32_defconfig
- riscv64 uses qemu-system-riscv64, riscv32 uses qemu-system-riscv32
- riscv32 has different compiler options (-march= and -mabi=)
So, riscv32 can share most of the settings with riscv64, add riscv32 support like the original ARCH=riscv support.
To align with x86, the default riscv is reserved for riscv64 and a new riscv64 is also added to allow users pass ARCH=riscv64 directly.
Since top-level kernel Makefile only accept ARCH=riscv, to make kernel happy, let's set kernel specific KARCH as riscv for both riscv32 and riscv64.
And since they share the same arch-riscv.h, let's set nolibc specific NARCH as riscv too.
Usage:
$ make defconfig ARCH=riscv32 CROSS_COMPILE=riscv64-linux-gnu- ... $ make run ARCH=riscv32 CROSS_COMPILE=riscv64-linux-gnu- ...
I'm hesitating on this one. Till now the ARCH variable taken on input was *exactly* the one used by the kernel. We include some scripts very early and we don't control the possible usage of ARCH. There's also this at the top of the makefile:
# when run as make -C tools/ nolibc_<foo> the arch is not set ifeq ($(ARCH),) include $(srctree)/scripts/subarch.include ARCH = $(SUBARCH) endif
So as you can see $(ARCH) is still very intimate with the kernel's. For x86 it's no big deal because the i386 and x86_64 names are real valid archs. The difficulty we're having with riscv is that 32 and 64 are two distinct archs for all tools but not for the kernel, and it looks like the only difference is in the config itself.
Given that we call all tools explicitly and that the kernel does a lot of implicit things with $(ARCH), I'm wondering if it wouldn't be more robust for the long term to instead add a "VARIANT" variable for the test only that would enforce "riscv32" or "riscv64" where needed (note that I'm not sold on this variable's name, it's to illustrate). Because if you look closely, you'll note that the nolibc source does not use this difference since its arch is always equal to the kernel's, and only the test requires it. I wouldn't be shocked by having more test options than we have architectures, and I noticed in another series that you were also proposing to extend config options, so I think it goes in the same direction. Then we could have in the test's Makefile a check for this VARIANT being set, which would preset ARCH when defined, and being used to configure Qemu. Maybe it could more or less look like this (for the selftest Makefile I mean) :
# maps variants to nominal archs ARCH_VARIANT_riscv32 = riscv ARCH_VARIANT_riscv64 = riscv
# default variants for some archs DEF_VARIANT_riscv = riscv64
VARIANT := ARCH ?= $(or $(ARCH_VARIANT_$(VARIANT)),$(VARIANT)) VARIANT ?= $(or $(DEF_VARIANT_$(ARCH)),$(ARCH))
Modulo the possible typos above, you probably get the idea. If ARCH is set, it will be used and automatically set the variant to the default one for the arch. And if VARIANT is set, it will set the correct default ARCH. It's possible to force the two in conflicting ways that will not work but we don't care, it's like for the rest of the variables. But at least we're never passing invalid values into ARCH anymore and I find this much safer.
What do you think ?
Thanks, Willy
On Mon, Jun 19, 2023 at 08:29:38PM +0800, Zhangjin Wu wrote:
Both riscv64 and riscv32 have:
- the same arch/riscv source code tree
- the same tools/include/nolibc/arch-riscv.h
- the same ARCH=riscv value passed to top-level kernel Makefile
The only differences are:
- riscv64 uses defconfig, riscv32 uses rv32_defconfig
- riscv64 uses qemu-system-riscv64, riscv32 uses qemu-system-riscv32
- riscv32 has different compiler options (-march= and -mabi=)
So, riscv32 can share most of the settings with riscv64, add riscv32 support like the original ARCH=riscv support.
To align with x86, the default riscv is reserved for riscv64 and a new riscv64 is also added to allow users pass ARCH=riscv64 directly.
Since top-level kernel Makefile only accept ARCH=riscv, to make kernel happy, let's set kernel specific KARCH as riscv for both riscv32 and riscv64.
And since they share the same arch-riscv.h, let's set nolibc specific NARCH as riscv too.
Usage:
$ make defconfig ARCH=riscv32 CROSS_COMPILE=riscv64-linux-gnu- ... $ make run ARCH=riscv32 CROSS_COMPILE=riscv64-linux-gnu- ...
I'm hesitating on this one. Till now the ARCH variable taken on input was *exactly* the one used by the kernel. We include some scripts very early and we don't control the possible usage of ARCH. There's also this at the top of the makefile:
# when run as make -C tools/ nolibc_<foo> the arch is not set ifeq ($(ARCH),) include $(srctree)/scripts/subarch.include ARCH = $(SUBARCH) endif
So as you can see $(ARCH) is still very intimate with the kernel's. For x86 it's no big deal because the i386 and x86_64 names are real valid archs. The difficulty we're having with riscv is that 32 and 64 are two distinct archs for all tools but not for the kernel, and it looks like the only difference is in the config itself.
Given that we call all tools explicitly and that the kernel does a lot of implicit things with $(ARCH), I'm wondering if it wouldn't be more robust for the long term to instead add a "VARIANT" variable for the test only that would enforce "riscv32" or "riscv64" where needed (note that I'm not sold on this variable's name, it's to illustrate). Because if you look closely, you'll note that the nolibc source does not use this difference since its arch is always equal to the kernel's, and only the test requires it. I wouldn't be shocked by having more test options than we have architectures, and I noticed in another series that you were also proposing to extend config options, so I think it goes in the same direction. Then we could have in the test's Makefile a check for this VARIANT being set, which would preset ARCH when defined, and being used to configure Qemu. Maybe it could more or less look like this (for the selftest Makefile I mean) :
# maps variants to nominal archs ARCH_VARIANT_riscv32 = riscv ARCH_VARIANT_riscv64 = riscv
# default variants for some archs DEF_VARIANT_riscv = riscv64
VARIANT := ARCH ?= $(or $(ARCH_VARIANT_$(VARIANT)),$(VARIANT)) VARIANT ?= $(or $(DEF_VARIANT_$(ARCH)),$(ARCH))
Modulo the possible typos above, you probably get the idea. If ARCH is set, it will be used and automatically set the variant to the default one for the arch. And if VARIANT is set, it will set the correct default ARCH. It's possible to force the two in conflicting ways that will not work but we don't care, it's like for the rest of the variables. But at least we're never passing invalid values into ARCH anymore and I find this much safer.
What do you think ?
agree, to distinguish the user input one and the kernel accepted one. If want to reserve ARCH as the default Kernel ARCH, what about use something like UARCH (User input ARCH) or XARCH (eXtended ARCH), it is shorter than VARIANT.
# UARCH and ARCH mapping, ARCH is supported by kernel, UARCH is input from user UARCH_riscv = riscv64 UARCH ?= $(or $(UARCH_$(ARCH)),$(ARCH))
ARCH_riscv32 = riscv ARCH_riscv64 = riscv ARCH ?= $(or $(ARCH_$(UARCH)),$(UARCH)
With this, we can simply customize the variables with UARCH or XARCH and left the others as before.
We can delay this after the minimal config patchset, I'm still preparing the v2 of the tinyconfig patchset, it is almost ready. these two have some conflicts.
With the v2 tinyconfig patchset, we are able to run nolibc-test for different architectures independently.
Best regards, Zhangjin
Thanks, Willy
Hi Zhangjin,
On Mon, Jun 19, 2023 at 08:22:43PM +0800, Zhangjin Wu wrote:
Hi, Willy
This is the revision of the v4 part2 of support for rv32 [1], this further split the generic KARCH code out of the old rv32 compile patch and also add kernel specific KARCH and nolibc specific NARCH for tools/include/nolibc/Makefile too.
(...)
Zhangjin Wu (5): tools/nolibc: fix up #error compile failures with -ENOSYS tools/nolibc: fix up undeclared syscall macros with #ifdef and -ENOSYS
I already queued these two ones to relieve you from them while we discuss the arch customization.
Thanks, Willy
linux-kselftest-mirror@lists.linaro.org