[PATCH v4 0/3] riscv: Fix build against binutils 2.38

[PATCH v4 0/3] riscv: Fix build against binutils 2.38

[Heinrich Schuchardt]

Binutils 2.37 and 2.38 are based on different revisions of the
"RISC-V Unprivileged Specification" and there fore require
different -march parameters for GCC.

We support both 32bit and 64bit RISC-V boards. Buildman uses a 64bit
toolchain for both which leads to build failures with
-march=rv32imac_zicsr_zifencei.

The first patch install the riscv32 toolchain in the Docker image.
Only after rebuilding and uploading the Docker image we can proceed
with the other patches.

The next patch let's buildman choose the correct tool chain.

The final patch which has already been posted previously corrects
the -march parameter for binutils 2.38+.

v4:
	remove riscv toolchain-alias in Docker image

Alexandre Ghiti (1):
  riscv: Fix build against binutils 2.38

Heinrich Schuchardt (2):
  docker: install riscv32 toolchain
  buildman: differentiate between riscv32, riscv64

 arch/riscv/Makefile      | 11 ++++++++++-
 tools/buildman/boards.py | 11 +++++++++++
 tools/docker/Dockerfile  |  2 +-
 3 files changed, 22 insertions(+), 2 deletions(-)

-- 
2.37.2

[PATCH v4 1/3] docker: install riscv32 toolchain

[Heinrich Schuchardt]

For building riscv32 targets we should use the riscv32 toolchain.
Add it to the Docker image.

Drop the riscv toolchain-alias as we do not need it in future.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v4:
	drop riscv toolchain-alias
v3:
	new patch
---
 tools/docker/Dockerfile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index d3292e752a..cbd837ad4e 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -24,6 +24,7 @@ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_
 RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/x86_64-gcc-11.1.0-nolibc-nios2-linux.tar.xz | tar -C /opt -xJ
 RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/x86_64-gcc-11.1.0-nolibc-powerpc-linux.tar.xz | tar -C /opt -xJ
 RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/x86_64-gcc-11.1.0-nolibc-riscv64-linux.tar.xz | tar -C /opt -xJ
+RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/x86_64-gcc-11.1.0-nolibc-riscv32-linux.tar.xz | tar -C /opt -xJ
 RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/x86_64-gcc-11.1.0-nolibc-sh2-linux.tar.xz | tar -C /opt -xJ
 
 # Manually install other toolchains
@@ -217,6 +218,5 @@ RUN /bin/echo -e "kernelorg = /opt/gcc-11.1.0-nolibc/*" >> ~/.buildman
 RUN /bin/echo -e "arc = /opt/arc_gnu_2021.03_prebuilt_uclibc_le_archs_linux_install" >> ~/.buildman
 RUN /bin/echo -e "\n[toolchain-prefix]\nxtensa = /opt/2020.07/xtensa-dc233c-elf/bin/xtensa-dc233c-elf-" >> ~/.buildman;
 RUN /bin/echo -e "\n[toolchain-alias]\nsh = sh2" >> ~/.buildman
-RUN /bin/echo -e "\nriscv = riscv64" >> ~/.buildman
 RUN /bin/echo -e "\nsandbox = x86_64" >> ~/.buildman
 RUN /bin/echo -e "\nx86 = i386" >> ~/.buildman;
-- 
2.37.2

Re: [PATCH v4 1/3] docker: install riscv32 toolchain

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 378 bytes --]

On Mon, Oct 03, 2022 at 06:07:52PM +0200, Heinrich Schuchardt wrote:

> For building riscv32 targets we should use the riscv32 toolchain.
> Add it to the Docker image.
> 
> Drop the riscv toolchain-alias as we do not need it in future.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v4 1/3] docker: install riscv32 toolchain

[Rick Chen]

> From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Sent: Tuesday, October 04, 2022 12:08 AM
> To: Tom Rini <trini@konsulko.com>
> Cc: Rick Jian-Zhi Chen(陳建志) <rick@andestech.com>; Leo Yu-Chi Liang(梁育齊) <ycliang@andestech.com>; Simon Glass <sjg@chromium.org>; Alexandre Ghiti <alexandre.ghiti@canonical.com>; Aurelien Jarno <aurelien@aurel32.net>; u-boot@lists.denx.de; Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Subject: [PATCH v4 1/3] docker: install riscv32 toolchain
>
> For building riscv32 targets we should use the riscv32 toolchain.
> Add it to the Docker image.
>
> Drop the riscv toolchain-alias as we do not need it in future.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v4:
>         drop riscv toolchain-alias
> v3:
>         new patch
> ---
>  tools/docker/Dockerfile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Rick Chen <rick@andestech.com>

Re: [PATCH v4 1/3] docker: install riscv32 toolchain

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 462 bytes --]

On Mon, Oct 03, 2022 at 06:07:52PM +0200, Heinrich Schuchardt wrote:

> For building riscv32 targets we should use the riscv32 toolchain.
> Add it to the Docker image.
> 
> Drop the riscv toolchain-alias as we do not need it in future.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Reviewed-by: Rick Chen <rick@andestech.com>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v4 2/3] buildman: differentiate between riscv32, riscv64

[Heinrich Schuchardt]

riscv32 needs a different toolchain than riscv64

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v4:
	no change
v3:
	new patch
---
 tools/buildman/boards.py | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/buildman/boards.py b/tools/buildman/boards.py
index 8a0971aa40..cdc4d9ffd2 100644
--- a/tools/buildman/boards.py
+++ b/tools/buildman/boards.py
@@ -263,6 +263,17 @@ class KconfigScanner:
         if params['arch'] == 'arm' and params['cpu'] == 'armv8':
             params['arch'] = 'aarch64'
 
+        # fix-up for riscv
+        if params['arch'] == 'riscv':
+            try:
+                value = self._conf.syms.get('ARCH_RV32I').str_value
+            except:
+                value = ''
+            if value == 'y':
+                params['arch'] = 'riscv32'
+            else:
+                params['arch'] = 'riscv64'
+
         return params
 
 
-- 
2.37.2

Re: [PATCH v4 2/3] buildman: differentiate between riscv32, riscv64

[Simon Glass]

Hi Heinrich,

On Mon, 3 Oct 2022 at 10:08, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> riscv32 needs a different toolchain than riscv64
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v4:
>         no change
> v3:
>         new patch
> ---
>  tools/buildman/boards.py | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/tools/buildman/boards.py b/tools/buildman/boards.py
> index 8a0971aa40..cdc4d9ffd2 100644
> --- a/tools/buildman/boards.py
> +++ b/tools/buildman/boards.py
> @@ -263,6 +263,17 @@ class KconfigScanner:
>          if params['arch'] == 'arm' and params['cpu'] == 'armv8':
>              params['arch'] = 'aarch64'
>
> +        # fix-up for riscv
> +        if params['arch'] == 'riscv':
> +            try:
> +                value = self._conf.syms.get('ARCH_RV32I').str_value
> +            except:
> +                value = ''
> +            if value == 'y':
> +                params['arch'] = 'riscv32'
> +            else:
> +                params['arch'] = 'riscv64'
> +
>          return params

Reviewed-by: Simon Glass <sjg@chromium.org>

I think I understand this better. Can we detect a boards.cfg file
which needs updating? Or perhaps the existing output_is_new() function
is good enough?

Regards.

Simon

Re: [PATCH v4 2/3] buildman: differentiate between riscv32, riscv64

[Heinrich Schuchardt]

On 10/3/22 18:44, Simon Glass wrote:
> Hi Heinrich,
> 
> On Mon, 3 Oct 2022 at 10:08, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> riscv32 needs a different toolchain than riscv64
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>> v4:
>>          no change
>> v3:
>>          new patch
>> ---
>>   tools/buildman/boards.py | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/tools/buildman/boards.py b/tools/buildman/boards.py
>> index 8a0971aa40..cdc4d9ffd2 100644
>> --- a/tools/buildman/boards.py
>> +++ b/tools/buildman/boards.py
>> @@ -263,6 +263,17 @@ class KconfigScanner:
>>           if params['arch'] == 'arm' and params['cpu'] == 'armv8':
>>               params['arch'] = 'aarch64'
>>
>> +        # fix-up for riscv
>> +        if params['arch'] == 'riscv':
>> +            try:
>> +                value = self._conf.syms.get('ARCH_RV32I').str_value
>> +            except:
>> +                value = ''
>> +            if value == 'y':
>> +                params['arch'] = 'riscv32'
>> +            else:
>> +                params['arch'] = 'riscv64'
>> +
>>           return params
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> I think I understand this better. Can we detect a boards.cfg file
> which needs updating? Or perhaps the existing output_is_new() function
> is good enough?

output_is_new() only checks the change date of files called *_defconfig, 
Kconfig*, MAINTAINERS. You could check the date of file boards.py too.

Best regards

Heinrich

Re: [PATCH v4 2/3] buildman: differentiate between riscv32, riscv64

[Rick Chen]

> From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Sent: Tuesday, October 04, 2022 12:08 AM
> To: Tom Rini <trini@konsulko.com>
> Cc: Rick Jian-Zhi Chen(陳建志) <rick@andestech.com>; Leo Yu-Chi Liang(梁育齊) <ycliang@andestech.com>; Simon Glass <sjg@chromium.org>; Alexandre Ghiti <alexandre.ghiti@canonical.com>; Aurelien Jarno <aurelien@aurel32.net>; u-boot@lists.denx.de; Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Subject: [PATCH v4 2/3] buildman: differentiate between riscv32, riscv64
>
> riscv32 needs a different toolchain than riscv64
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v4:
>         no change
> v3:
>         new patch
> ---
>  tools/buildman/boards.py | 11 +++++++++++
>  1 file changed, 11 insertions(+)

Reviewed-by: Rick Chen <rick@andestech.com>

Re: [PATCH v4 2/3] buildman: differentiate between riscv32, riscv64

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 345 bytes --]

On Mon, Oct 03, 2022 at 06:07:53PM +0200, Heinrich Schuchardt wrote:

> riscv32 needs a different toolchain than riscv64
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Rick Chen <rick@andestech.com>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v4 3/3] riscv: Fix build against binutils 2.38

[Heinrich Schuchardt]

From: Alexandre Ghiti <alexandre.ghiti@canonical.com>

The following description is copied from the equivalent patch for the
Linux Kernel proposed by Aurelien Jarno:

>From version 2.38, binutils default to ISA spec version 20191213. This
means that the csr read/write (csrr*/csrw*) instructions and fence.i
instruction has separated from the `I` extension, become two standalone
extensions: Zicsr and Zifencei. As the kernel uses those instruction,
this causes the following build failure:

arch/riscv/cpu/mtrap.S: Assembler messages:
arch/riscv/cpu/mtrap.S:65: Error: unrecognized opcode `csrr a0,scause'
arch/riscv/cpu/mtrap.S:66: Error: unrecognized opcode `csrr a1,sepc'
arch/riscv/cpu/mtrap.S:67: Error: unrecognized opcode `csrr a2,stval'
arch/riscv/cpu/mtrap.S:70: Error: unrecognized opcode `csrw sepc,a0'

Signed-off-by: Alexandre Ghiti <alexandre.ghiti@canonical.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Tested-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Christian Stewart <christian@paral.in>
Reviewed-by: Rick Chen <rick@andestech.com>
---
v4:
	no change
v3:
	no change
---
 arch/riscv/Makefile | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 0b80eb8d86..53d1194ffb 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -24,7 +24,16 @@ ifeq ($(CONFIG_CMODEL_MEDANY),y)
 	CMODEL = medany
 endif
 
-ARCH_FLAGS = -march=$(ARCH_BASE)$(ARCH_A)$(ARCH_C) -mabi=$(ABI) \
+RISCV_MARCH = $(ARCH_BASE)$(ARCH_A)$(ARCH_C)
+
+# Newer binutils versions default to ISA spec version 20191213 which moves some
+# instructions from the I extension to the Zicsr and Zifencei extensions.
+toolchain-need-zicsr-zifencei := $(call cc-option-yn, -mabi=$(ABI) -march=$(RISCV_MARCH)_zicsr_zifencei)
+ifeq ($(toolchain-need-zicsr-zifencei),y)
+	RISCV_MARCH := $(RISCV_MARCH)_zicsr_zifencei
+endif
+
+ARCH_FLAGS = -march=$(RISCV_MARCH) -mabi=$(ABI) \
 	     -mcmodel=$(CMODEL)
 
 PLATFORM_CPPFLAGS	+= $(ARCH_FLAGS)
-- 
2.37.2

Re: [PATCH v4 3/3] riscv: Fix build against binutils 2.38

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]

On Mon, Oct 03, 2022 at 06:07:54PM +0200, Heinrich Schuchardt wrote:

> From: Alexandre Ghiti <alexandre.ghiti@canonical.com>
> 
> The following description is copied from the equivalent patch for the
> Linux Kernel proposed by Aurelien Jarno:
> 
> >From version 2.38, binutils default to ISA spec version 20191213. This
> means that the csr read/write (csrr*/csrw*) instructions and fence.i
> instruction has separated from the `I` extension, become two standalone
> extensions: Zicsr and Zifencei. As the kernel uses those instruction,
> this causes the following build failure:
> 
> arch/riscv/cpu/mtrap.S: Assembler messages:
> arch/riscv/cpu/mtrap.S:65: Error: unrecognized opcode `csrr a0,scause'
> arch/riscv/cpu/mtrap.S:66: Error: unrecognized opcode `csrr a1,sepc'
> arch/riscv/cpu/mtrap.S:67: Error: unrecognized opcode `csrr a2,stval'
> arch/riscv/cpu/mtrap.S:70: Error: unrecognized opcode `csrw sepc,a0'
> 
> Signed-off-by: Alexandre Ghiti <alexandre.ghiti@canonical.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Tested-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> Tested-by: Christian Stewart <christian@paral.in>
> Reviewed-by: Rick Chen <rick@andestech.com>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]