| Message ID | 2ebfb48c66b18a5fd7d0bd6b7c832a5d8ce6486f.1687176996.git.falcon@tinylab.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | nolibc: add part2 of support for rv32 | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/tree_selection | fail | Failed to apply to next/pending-fixes, riscv/for-next or riscv/master |
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
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))
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-8ch.de/ Suggested-by: Arnd Bergmann <arnd@arndb.de> Link: https://lore.kernel.org/linux-riscv/d1c83340-af4c-4780-a101-b9d22b47379c@app.fastmail.com/ 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(+)