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/[email protected]/
[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(-)
--
2.25.1
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 <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Suggested-by: Arnd Bergmann <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Suggested-by: Willy Tarreau <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]/
Signed-off-by: Zhangjin Wu <[email protected]>
---
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))
--
2.25.1
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 <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Reviewed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Zhangjin Wu <[email protected]>
---
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))
--
2.25.1
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 <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Reviewed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Zhangjin Wu <[email protected]>
---
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
}
--
2.25.1
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
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
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.
(...)
> [1]: https://lore.kernel.org/linux-riscv/[email protected]/
> [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
I already queued these two ones to relieve you from them while we
discuss the arch customization.
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