Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-447-ge2460e13b3-fm-20230525.001-ge2460e13
Mime-Version: 1.0
Message-Id: <63133024-e986-44e2-86f5-efc1c42207ac@app.fastmail.com>
In-Reply-To: <20230606111216.524384-1-falcon@tinylab.org>
References: <d1c83340-af4c-4780-a101-b9d22b47379c@app.fastmail.com>
 <20230606111216.524384-1-falcon@tinylab.org>
Date:   Tue, 06 Jun 2023 13:21:26 +0200
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Zhangjin Wu" <falcon@tinylab.org>,
        =?UTF-8?Q?Thomas_Wei=C3=9Fschuh?= <thomas@t-8ch.de>,
        "Willy Tarreau" <w@1wt.eu>
Cc:     linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        linux-riscv@lists.infradead.org
Subject: Re: [PATCH v3 3/3] selftests/nolibc: riscv: customize makefile for rv32
Content-Type: text/plain
Precedence: bulk

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