[PATCH v3 0/3] riscv: Fix build against binutils 2.38
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+.
The series pass Gitlab CI: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/13658
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 | 1 + 3 files changed, 22 insertions(+), 1 deletion(-)
For building riscv32 targets we should use the riscv32 toolchain. Add it to the Docker image.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- v3: new patch --- tools/docker/Dockerfile | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index d3292e752a..261bddca0e 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/... | tar -C /opt -xJ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/... | tar -C /opt -xJ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/... | tar -C /opt -xJ +RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/... | tar -C /opt -xJ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/... | tar -C /opt -xJ
# Manually install other toolchains
On Sun, Oct 02, 2022 at 04:21:22AM +0200, Heinrich Schuchardt wrote:
For building riscv32 targets we should use the riscv32 toolchain. Add it to the Docker image.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
v3: new patch
tools/docker/Dockerfile | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index d3292e752a..261bddca0e 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/... | tar -C /opt -xJ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/... | tar -C /opt -xJ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/... | tar -C /opt -xJ +RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/... | tar -C /opt -xJ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/... | tar -C /opt -xJ
# Manually install other toolchains
Since we then do not use the "riscv" alias, please drop that here and have this commit reflect that we're switching to individual 32 and 64bit toolchains now.
riscv32 needs a different toolchain than riscv64
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- 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
Hi Heinrich,
On Sat, 1 Oct 2022 at 20:21, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
riscv32 needs a different toolchain than riscv64
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
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 riscvif params['arch'] == 'riscv':try:value = self._conf.syms.get('ARCH_RV32I').str_valueexcept:value = ''if value == 'y':params['arch'] = 'riscv32'else:params['arch'] = 'riscv64'return params-- 2.37.2
Should we instead do what ARM does?
Regards, Simon
On 10/3/22 03:10, Simon Glass wrote:
Hi Heinrich,
On Sat, 1 Oct 2022 at 20:21, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
riscv32 needs a different toolchain than riscv64
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
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 riscvif params['arch'] == 'riscv':try:value = self._conf.syms.get('ARCH_RV32I').str_valueexcept:value = ''if value == 'y':params['arch'] = 'riscv32'else:params['arch'] = 'riscv64'return params-- 2.37.2
Should we instead do what ARM does?
My patch does exactly the same for RISC-V that was done previously for ARM: It sets the correct value of arch in dependence of the bitness of the architecture.
Currently in our Docker image we have an alias entry for 'riscv' in file .buildman. Don't force users to create such an alias value when running buildman locally.
Best regards
Heinrich
Hi Heinrich,
On Mon, 3 Oct 2022 at 03:56, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 10/3/22 03:10, Simon Glass wrote:
Hi Heinrich,
On Sat, 1 Oct 2022 at 20:21, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
riscv32 needs a different toolchain than riscv64
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
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 riscvif params['arch'] == 'riscv':try:value = self._conf.syms.get('ARCH_RV32I').str_valueexcept:value = ''if value == 'y':params['arch'] = 'riscv32'else:params['arch'] = 'riscv64'return params-- 2.37.2
Should we instead do what ARM does?
My patch does exactly the same for RISC-V that was done previously for ARM: It sets the correct value of arch in dependence of the bitness of the architecture.
Sort of. Can we use the 'cpu' for this, insteading of reading a config symbol?
Otherwise, how will this work when the boards.cfg file is already there?
Currently in our Docker image we have an alias entry for 'riscv' in file .buildman. Don't force users to create such an alias value when running buildman locally.
I didn't know I needed to...
Regards, Simon
On Mon, Oct 03, 2022 at 08:57:41AM -0600, Simon Glass wrote:
Hi Heinrich,
On Mon, 3 Oct 2022 at 03:56, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 10/3/22 03:10, Simon Glass wrote:
Hi Heinrich,
On Sat, 1 Oct 2022 at 20:21, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
riscv32 needs a different toolchain than riscv64
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
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 riscvif params['arch'] == 'riscv':try:value = self._conf.syms.get('ARCH_RV32I').str_valueexcept:value = ''if value == 'y':params['arch'] = 'riscv32'else:params['arch'] = 'riscv64'return params-- 2.37.2
Should we instead do what ARM does?
My patch does exactly the same for RISC-V that was done previously for ARM: It sets the correct value of arch in dependence of the bitness of the architecture.
Sort of. Can we use the 'cpu' for this, insteading of reading a config symbol?
The problem is that "cpu" for riscv is not that determines this. None of the existing symbols that buildman puts in a handy table are.
Currently in our Docker image we have an alias entry for 'riscv' in file .buildman. Don't force users to create such an alias value when running buildman locally.
This is because up until recently, a single toolchain was fine for both 32 and 64bit riscv so the alias is just to make things easier.
Hi,
On Mon, 3 Oct 2022 at 09:05, Tom Rini trini@konsulko.com wrote:
On Mon, Oct 03, 2022 at 08:57:41AM -0600, Simon Glass wrote:
Hi Heinrich,
On Mon, 3 Oct 2022 at 03:56, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 10/3/22 03:10, Simon Glass wrote:
Hi Heinrich,
On Sat, 1 Oct 2022 at 20:21, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
riscv32 needs a different toolchain than riscv64
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
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 riscvif params['arch'] == 'riscv':try:value = self._conf.syms.get('ARCH_RV32I').str_valueexcept:value = ''if value == 'y':params['arch'] = 'riscv32'else:params['arch'] = 'riscv64'return params-- 2.37.2
Should we instead do what ARM does?
My patch does exactly the same for RISC-V that was done previously for ARM: It sets the correct value of arch in dependence of the bitness of the architecture.
Sort of. Can we use the 'cpu' for this, insteading of reading a config symbol?
The problem is that "cpu" for riscv is not that determines this. None of the existing symbols that buildman puts in a handy table are.
OK, so can we fix/change that? Or do we need to add yet another field to boards.cfg, just for riscv?
Currently in our Docker image we have an alias entry for 'riscv' in file .buildman. Don't force users to create such an alias value when running buildman locally.
This is because up until recently, a single toolchain was fine for both 32 and 64bit riscv so the alias is just to make things easier.
OK.
Regards, Simon
On Mon, Oct 03, 2022 at 09:23:01AM -0600, Simon Glass wrote:
Hi,
On Mon, 3 Oct 2022 at 09:05, Tom Rini trini@konsulko.com wrote:
On Mon, Oct 03, 2022 at 08:57:41AM -0600, Simon Glass wrote:
Hi Heinrich,
On Mon, 3 Oct 2022 at 03:56, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 10/3/22 03:10, Simon Glass wrote:
Hi Heinrich,
On Sat, 1 Oct 2022 at 20:21, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
riscv32 needs a different toolchain than riscv64
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
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 riscvif params['arch'] == 'riscv':try:value = self._conf.syms.get('ARCH_RV32I').str_valueexcept:value = ''if value == 'y':params['arch'] = 'riscv32'else:params['arch'] = 'riscv64'return params-- 2.37.2
Should we instead do what ARM does?
My patch does exactly the same for RISC-V that was done previously for ARM: It sets the correct value of arch in dependence of the bitness of the architecture.
Sort of. Can we use the 'cpu' for this, insteading of reading a config symbol?
The problem is that "cpu" for riscv is not that determines this. None of the existing symbols that buildman puts in a handy table are.
OK, so can we fix/change that? Or do we need to add yet another field to boards.cfg, just for riscv?
Well, it's a side effect of what we do / don't do with Kconfig. ARCH_RV32I and ARCH_RV64I set 32BIT or 64BIT which are also more common symbols. It's also not that cpu should be what matters here as we have configa/ae350_rv{32,64}_defconfig using the ax25 CPU.
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 --- 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)
participants (3)
-
Heinrich Schuchardt -
Simon Glass -
Tom Rini