diff mbox series

[v2,13/13] selftests/nolibc: riscv: customize makefile for rv32

Message ID 6f065441a6be9e63238ffb3d43cf09a6e4ac6773.1685387485.git.falcon@tinylab.org (mailing list archive)
State New
Headers show
Series nolibc: add part2 of support for rv32 | expand

Commit Message

Zhangjin Wu May 29, 2023, 8:03 p.m. UTC 
Both riscv64 and riscv32 have:

* the same ARCH value, it is riscv
* the same arch/riscv source code tree

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, there is no
need to add it as a whole new architecture but just need a flag to
record and reflect the difference.

The 32bit mips and loongarch may be able to use the same method, so,
let's use a meaningful flag: CONFIG_32BIT. If required in the future,
this flag can also be automatically loaded from
include/config/auto.conf.

With this patch, it is able to run nolibc test for rv32 like this:

    $ make run ARCH=riscv32 CROSS_COMPILE=riscv64-linux-gnu- ...

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/Makefile | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Zhangjin Wu June 2, 2023, 4:06 a.m. UTC | #1 
Willy, Arnd and Thomas

Based on your suggestions, in the comming v3, I plan to split the whole rv32
support to something like this:

1. Generic part1

   (The old feedbacks are applied with the new Suggested-by lines, welcome your
    additional feedbacks if there are ;-))

    selftests/nolibc: syscall_args: use generic __NR_statx
    tools/nolibc: add missing nanoseconds support for __NR_statx
    selftests/nolibc: allow specify extra arguments for qemu
    selftests/nolibc: fix up compile warning with glibc on x86_64
    selftests/nolibc: not include limits.h for nolibc
    selftests/nolibc: use INT_MAX instead of __INT_MAX__
    tools/nolibc: arm: add missing my_syscall6
    tools/nolibc: open: fix up compile warning for arm
    selftests/nolibc: support two errnos with EXPECT_SYSER2()
    selftests/nolibc: remove gettimeofday_bad1/2 completely
    selftests/nolibc: add new gettimeofday test cases

2. Add Compile support for rv32

   (Convert all of the unsupported syscalls to a return of -ENOSYS, this
    allows us to fix up the test failures one by one not that urgently later)

    tools/nolibc: fix up #error compile failures with -ENOSYS
    tools/nolibc: fix up undeclared syscall macros with #ifdef and -ENOSYS
    selftests/nolibc: riscv: customize makefile for rv32

   (The first two are new but clear enough, based on the idea of suggestion from Arnd [1])

3. Fix up the left test failures one by one

   (Plan to add everyone as a standalone patchset, which will easier the review
    and merge progress)

   wait4 -> waitid
   lseek -> llseek
   gettimeofday -> clock_gettime/clock_gettime64
   select -> pselect6/pselect6_time64
   ppoll -> ppoll_time64

4. Clean up some old test cases one by one

   Like statx ...

Best regards,
Zhangjin

[1]: https://lore.kernel.org/linux-riscv/5e7d2adf-e96f-41ca-a4c6-5c87a25d4c9c@app.fastmail.com/

> Both riscv64 and riscv32 have:
> 
> * the same ARCH value, it is riscv
> * the same arch/riscv source code tree
> 
> 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, there is no
> need to add it as a whole new architecture but just need a flag to
> record and reflect the difference.
> 
> The 32bit mips and loongarch may be able to use the same method, so,
> let's use a meaningful flag: CONFIG_32BIT. If required in the future,
> this flag can also be automatically loaded from
> include/config/auto.conf.
> 
> With this patch, it is able to run nolibc test for rv32 like this:
> 
>     $ make run ARCH=riscv32 CROSS_COMPILE=riscv64-linux-gnu- ...
> 
> Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> ---
>  tools/testing/selftests/nolibc/Makefile | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> index 44088535682e..ea434a0acdc1 100644
> --- a/tools/testing/selftests/nolibc/Makefile
> +++ b/tools/testing/selftests/nolibc/Makefile
> @@ -14,6 +14,12 @@ include $(srctree)/scripts/subarch.include
>  ARCH = $(SUBARCH)
>  endif
>  
> +# Allow pass ARCH=riscv|riscv32|riscv64, riscv implies riscv64
> +ifneq ($(findstring xriscv,x$(ARCH)),)
> +  CONFIG_32BIT := $(if $(findstring 32x,$(ARCH)x),1)
> +  override ARCH := riscv
> +endif
> +
>  # kernel image names by architecture
>  IMAGE_i386       = arch/x86/boot/bzImage
>  IMAGE_x86_64     = arch/x86/boot/bzImage
> @@ -34,7 +40,7 @@ DEFCONFIG_x86        = defconfig
>  DEFCONFIG_arm64      = defconfig
>  DEFCONFIG_arm        = multi_v7_defconfig
>  DEFCONFIG_mips       = malta_defconfig
> -DEFCONFIG_riscv      = defconfig
> +DEFCONFIG_riscv      = $(if $(CONFIG_32BIT),rv32_defconfig,defconfig)
>  DEFCONFIG_s390       = defconfig
>  DEFCONFIG_loongarch  = defconfig
>  DEFCONFIG            = $(DEFCONFIG_$(ARCH))
> @@ -49,7 +55,7 @@ QEMU_ARCH_x86        = x86_64
>  QEMU_ARCH_arm64      = aarch64
>  QEMU_ARCH_arm        = arm
>  QEMU_ARCH_mips       = mipsel  # works with malta_defconfig
> -QEMU_ARCH_riscv      = riscv64
> +QEMU_ARCH_riscv      = $(if $(CONFIG_32BIT),riscv32,riscv64)
>  QEMU_ARCH_s390       = s390x
>  QEMU_ARCH_loongarch  = loongarch64
>  QEMU_ARCH            = $(QEMU_ARCH_$(ARCH))
> @@ -76,6 +82,7 @@ else
>  Q=@
>  endif
>  
> +CFLAGS_riscv = $(if $(CONFIG_32BIT),-march=rv32i -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 \
> -- 
> 2.25.1
Thomas Weißschuh June 2, 2023, 10:33 a.m. UTC | #2 
On 2023-06-02 12:06:25+0800, Zhangjin Wu wrote:
> Willy, Arnd and Thomas
> 
> Based on your suggestions, in the comming v3, I plan to split the whole rv32
> support to something like this:

Is each of these parts a new patchset?
I would suggest to do so.

> 1. Generic part1
> 
>    (The old feedbacks are applied with the new Suggested-by lines, welcome your
>     additional feedbacks if there are ;-))
> 
>     selftests/nolibc: syscall_args: use generic __NR_statx
>     tools/nolibc: add missing nanoseconds support for __NR_statx
>     selftests/nolibc: allow specify extra arguments for qemu
>     selftests/nolibc: fix up compile warning with glibc on x86_64
>     selftests/nolibc: not include limits.h for nolibc
>     selftests/nolibc: use INT_MAX instead of __INT_MAX__
>     tools/nolibc: arm: add missing my_syscall6
>     tools/nolibc: open: fix up compile warning for arm
>     selftests/nolibc: support two errnos with EXPECT_SYSER2()
>     selftests/nolibc: remove gettimeofday_bad1/2 completely
>     selftests/nolibc: add new gettimeofday test cases

These all look good and non-controversial.

> 2. Add Compile support for rv32
> 
>    (Convert all of the unsupported syscalls to a return of -ENOSYS, this
>     allows us to fix up the test failures one by one not that urgently later)
> 
>     tools/nolibc: fix up #error compile failures with -ENOSYS
>     tools/nolibc: fix up undeclared syscall macros with #ifdef and -ENOSYS

These should be their own series in my opinion.
It will likely generate some discussion.

>     selftests/nolibc: riscv: customize makefile for rv32
> 
>    (The first two are new but clear enough, based on the idea of suggestion from Arnd [1])
> 
> 3. Fix up the left test failures one by one

I'm not a fan of adding an "official" rv32 support with still failing
tests.

>    (Plan to add everyone as a standalone patchset, which will easier the review
>     and merge progress)
> 
>    wait4 -> waitid
>    lseek -> llseek
>    gettimeofday -> clock_gettime/clock_gettime64
>    select -> pselect6/pselect6_time64
>    ppoll -> ppoll_time64

I guess these new codepaths will also be used on non-rv32 architectures
and will therefore validated without rv32.

So you could submit these before the final rv32 patch in a series.

> 4. Clean up some old test cases one by one
> 
>    Like statx ...
> 
> Best regards,
> Zhangjin
> 
> [1]: https://lore.kernel.org/linux-riscv/5e7d2adf-e96f-41ca-a4c6-5c87a25d4c9c@app.fastmail.com/
> 
> > Both riscv64 and riscv32 have:
> > 
> > * the same ARCH value, it is riscv
> > * the same arch/riscv source code tree
> > 
> > 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, there is no
> > need to add it as a whole new architecture but just need a flag to
> > record and reflect the difference.
> > 
> > The 32bit mips and loongarch may be able to use the same method, so,
> > let's use a meaningful flag: CONFIG_32BIT. If required in the future,
> > this flag can also be automatically loaded from
> > include/config/auto.conf.
> > 
> > With this patch, it is able to run nolibc test for rv32 like this:
> > 
> >     $ make run ARCH=riscv32 CROSS_COMPILE=riscv64-linux-gnu- ...
> > 
> > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > ---
> >  tools/testing/selftests/nolibc/Makefile | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > index 44088535682e..ea434a0acdc1 100644
> > --- a/tools/testing/selftests/nolibc/Makefile
> > +++ b/tools/testing/selftests/nolibc/Makefile
> > @@ -14,6 +14,12 @@ include $(srctree)/scripts/subarch.include
> >  ARCH = $(SUBARCH)
> >  endif
> >  
> > +# Allow pass ARCH=riscv|riscv32|riscv64, riscv implies riscv64
> > +ifneq ($(findstring xriscv,x$(ARCH)),)
> > +  CONFIG_32BIT := $(if $(findstring 32x,$(ARCH)x),1)
> > +  override ARCH := riscv
> > +endif
> > +
> >  # kernel image names by architecture
> >  IMAGE_i386       = arch/x86/boot/bzImage
> >  IMAGE_x86_64     = arch/x86/boot/bzImage
> > @@ -34,7 +40,7 @@ DEFCONFIG_x86        = defconfig
> >  DEFCONFIG_arm64      = defconfig
> >  DEFCONFIG_arm        = multi_v7_defconfig
> >  DEFCONFIG_mips       = malta_defconfig
> > -DEFCONFIG_riscv      = defconfig
> > +DEFCONFIG_riscv      = $(if $(CONFIG_32BIT),rv32_defconfig,defconfig)
> >  DEFCONFIG_s390       = defconfig
> >  DEFCONFIG_loongarch  = defconfig
> >  DEFCONFIG            = $(DEFCONFIG_$(ARCH))
> > @@ -49,7 +55,7 @@ QEMU_ARCH_x86        = x86_64
> >  QEMU_ARCH_arm64      = aarch64
> >  QEMU_ARCH_arm        = arm
> >  QEMU_ARCH_mips       = mipsel  # works with malta_defconfig
> > -QEMU_ARCH_riscv      = riscv64
> > +QEMU_ARCH_riscv      = $(if $(CONFIG_32BIT),riscv32,riscv64)
> >  QEMU_ARCH_s390       = s390x
> >  QEMU_ARCH_loongarch  = loongarch64
> >  QEMU_ARCH            = $(QEMU_ARCH_$(ARCH))
> > @@ -76,6 +82,7 @@ else
> >  Q=@
> >  endif
> >  
> > +CFLAGS_riscv = $(if $(CONFIG_32BIT),-march=rv32i -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 \
> > -- 
> > 2.25.1
>
Zhangjin Wu June 2, 2023, 11:56 a.m. UTC | #3 
> On 2023-06-02 12:06:25+0800, Zhangjin Wu wrote:
> > Willy, Arnd and Thomas
> > 
> > Based on your suggestions, in the comming v3, I plan to split the whole rv32
> > support to something like this:
> 
> Is each of these parts a new patchset?

Yeah, It is also my plan, just like the v2 series.

> I would suggest to do so.
> 
> > 1. Generic part1
> > 
> >    (The old feedbacks are applied with the new Suggested-by lines, welcome your
> >     additional feedbacks if there are ;-))
> > 
> >     selftests/nolibc: syscall_args: use generic __NR_statx
> >     tools/nolibc: add missing nanoseconds support for __NR_statx
> >     selftests/nolibc: allow specify extra arguments for qemu
> >     selftests/nolibc: fix up compile warning with glibc on x86_64
> >     selftests/nolibc: not include limits.h for nolibc
> >     selftests/nolibc: use INT_MAX instead of __INT_MAX__
> >     tools/nolibc: arm: add missing my_syscall6
> >     tools/nolibc: open: fix up compile warning for arm
> >     selftests/nolibc: support two errnos with EXPECT_SYSER2()
> >     selftests/nolibc: remove gettimeofday_bad1/2 completely
> >     selftests/nolibc: add new gettimeofday test cases
> 
> These all look good and non-controversial.
> 
> > 2. Add Compile support for rv32
> > 
> >    (Convert all of the unsupported syscalls to a return of -ENOSYS, this
> >     allows us to fix up the test failures one by one not that urgently later)
> > 
> >     tools/nolibc: fix up #error compile failures with -ENOSYS
> >     tools/nolibc: fix up undeclared syscall macros with #ifdef and -ENOSYS
> 
> These should be their own series in my opinion.
> It will likely generate some discussion.

The 1st one is not rv32 specific, but the 2nd one requires rv32 compile support
to be validated.

> 
> >     selftests/nolibc: riscv: customize makefile for rv32
> > 
> >    (The first two are new but clear enough, based on the idea of suggestion from Arnd [1])
> > 
> > 3. Fix up the left test failures one by one
> 
> I'm not a fan of adding an "official" rv32 support with still failing
> tests.
>

That is reasonable, but in another side, without the rv32 compile support, It
may be a little hard to test the left patchsets (see below explain).

The other reasons for rv32 compile support is:

* Some people may use nolibc without the left syscalls.
* It is able to detect the new test failures.

But anyway, the compile support is not urgent.

> >    (Plan to add everyone as a standalone patchset, which will easier the review
> >     and merge progress)
> > 
> >    wait4 -> waitid
> >    lseek -> llseek
> >    gettimeofday -> clock_gettime/clock_gettime64
> >    select -> pselect6/pselect6_time64
> >    ppoll -> ppoll_time64
> 
> I guess these new codepaths will also be used on non-rv32 architectures
> and will therefore validated without rv32.
>

Unfortunately, most of them are time32 syscalls related (except the
llseek), rv32 is the first architecture who has no kernel side time32
syscalls support, that's why I plan to add compile support at first ;-)

If the new time64 syscalls will be added as the first 'branch', then, they will
be validated on the other 32bit architecture, but some of them may be not added
as the first 'branch', for example, the waitid() emulated wait4() is bigger
than the original one.

> So you could submit these before the final rv32 patch in a series.
>

Thanks for your suggestion.

I'm working on cleaning up them independently and carefully, will send them out
as standalone patchsets.

Best regards,
Zhangjin
 
> > 4. Clean up some old test cases one by one
> > 
> >    Like statx ...
> > 
> > Best regards,
> > Zhangjin
> > 
> > [1]: https://lore.kernel.org/linux-riscv/5e7d2adf-e96f-41ca-a4c6-5c87a25d4c9c@app.fastmail.com/
diff mbox series

Patch

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 44088535682e..ea434a0acdc1 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -14,6 +14,12 @@  include $(srctree)/scripts/subarch.include
 ARCH = $(SUBARCH)
 endif
 
+# Allow pass ARCH=riscv|riscv32|riscv64, riscv implies riscv64
+ifneq ($(findstring xriscv,x$(ARCH)),)
+  CONFIG_32BIT := $(if $(findstring 32x,$(ARCH)x),1)
+  override ARCH := riscv
+endif
+
 # kernel image names by architecture
 IMAGE_i386       = arch/x86/boot/bzImage
 IMAGE_x86_64     = arch/x86/boot/bzImage
@@ -34,7 +40,7 @@  DEFCONFIG_x86        = defconfig
 DEFCONFIG_arm64      = defconfig
 DEFCONFIG_arm        = multi_v7_defconfig
 DEFCONFIG_mips       = malta_defconfig
-DEFCONFIG_riscv      = defconfig
+DEFCONFIG_riscv      = $(if $(CONFIG_32BIT),rv32_defconfig,defconfig)
 DEFCONFIG_s390       = defconfig
 DEFCONFIG_loongarch  = defconfig
 DEFCONFIG            = $(DEFCONFIG_$(ARCH))
@@ -49,7 +55,7 @@  QEMU_ARCH_x86        = x86_64
 QEMU_ARCH_arm64      = aarch64
 QEMU_ARCH_arm        = arm
 QEMU_ARCH_mips       = mipsel  # works with malta_defconfig
-QEMU_ARCH_riscv      = riscv64
+QEMU_ARCH_riscv      = $(if $(CONFIG_32BIT),riscv32,riscv64)
 QEMU_ARCH_s390       = s390x
 QEMU_ARCH_loongarch  = loongarch64
 QEMU_ARCH            = $(QEMU_ARCH_$(ARCH))
@@ -76,6 +82,7 @@  else
 Q=@
 endif
 
+CFLAGS_riscv = $(if $(CONFIG_32BIT),-march=rv32i -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 \