Re: [PATCH v3 3/3] selftests/nolibc: riscv: customize makefile for rv32

["Arnd Bergmann" <arnd@arndb.de>]

On Tue, Jun 6, 2023, at 13:12, Zhangjin Wu wrote:
>> On Sat, Jun 3, 2023, at 11:05, Zhangjin Wu wrote:
>> would seem more consistent with how x86 is handled, and would
>> probably be more easily extensible if we want to also make
>> this work with other sub-targets like mipseb, armv5 or ppc32
>> in the future.
>
> As Arnd and Thomas suggested to align with x86, I just tried to find a
> solution to avoid mixing the use of _ARCH and ARCH in this Makefile.
>
> Since both riscv32 and riscv64 share the same SRCARCH=riscv (arch/riscv),
> and the kernel side doesn't accept riscv32 or riscv64 currently, we need to
> manually convert them to _ARCH=riscv and pass them to the kernel makefile
> like this: ARCH=$(_ARCH), it mixes the use of _ARCH and ARCH, this is why I
> used the '$(if' method currently.
>
> The solution is adding something like x86 in the kernel Makefile:
>
>     diff --git a/Makefile b/Makefile
>     index 9d765ebcccf1..a442c893d795 100644
>     --- a/Makefile
>     +++ b/Makefile
>     @@ -415,6 +415,14 @@ ifeq ($(ARCH),parisc64)
>             SRCARCH := parisc
>      endif
>
>     +# Additional ARCH settings for riscv
>     +ifeq ($(ARCH),riscv32)
>     +        SRCARCH := riscv
>     +endif
>     +ifeq ($(ARCH),riscv64)
>     +        SRCARCH := riscv
>     +endif
>     +
>      export cross_compiling :=
>      ifneq ($(SRCARCH),$(SUBARCH))
>      cross_compiling := 1

I've never been a big fan of the top-level $(ARCH) setting
in the kernel, is there a reason this has to be the same
as the variable in tools/include/nolibc? If not, I'd just
leave the Linux Makefile unchanged.

For userspace we have a lot more target names than
arch/*/ directories in the kernel, and I don't think
I'd want to enumerate all the possibilities in the
build system globally.
> b/tools/testing/selftests/nolibc/Makefile
>     index 4a3a105e1fdf..1b2247a6365d 100644
>     --- a/tools/testing/selftests/nolibc/Makefile
>     +++ b/tools/testing/selftests/nolibc/Makefile
>     @@ -21,6 +21,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
>     @@ -34,6 +36,8 @@ DEFCONFIG_x86        = defconfig
>      DEFCONFIG_arm64      = defconfig
>      DEFCONFIG_arm        = multi_v7_defconfig
>      DEFCONFIG_mips       = malta_defconfig
>     +DEFCONFIG_riscv32    = rv32_defconfig
>     +DEFCONFIG_riscv64    = defconfig
...

Right, that part looks good to me.

      Arnd