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