Re: [Buildroot] [PATCH for-next 01/11] arch/Config.in.riscv: add Zicsr and Zifencei standalone extensions

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello Romain,

On Sun, 29 May 2022 15:18:01 +0200
Romain Naour <romain.naour@gmail.com> wrote:

> Since gcc 12, default Riscv ISA spec version was bump to 20191213 [1].
> 
> This introduce a major incompatibility issue is the csr read/write
> (csrr*/csrw*) instructions and fence.i instruction has separated from
> the "I" extension, become two standalone extensions: Zicsr and
> Zifencei; so you might get error messages like that: unrecognized
> opcode "csrr" (or "fence.i").
> 
> Indeed, without Zifencei we can't build opensbi bootloader [3]:
> 
> opensbi-1.0/lib/sbi/sbi_tlb.c: Assembler messages:
> opensbi-1.0/lib/sbi/sbi_tlb.c:190: Error: unrecognized opcode `fence.i', extension `zifencei' required
> 
> As a workaround, opensbi build system has been patched [4] to use
> -march=rv64imafdc_zicsr_zifencei when needed.
> This workaround doesn't work in Buildroot due to the local patch
> 0001-Makefile-Don-t-specify-mabi-or-march.patch removing -march
> from CFLAGS.
> 
> Fix this issue by introducing two additional Kconfig option
> enabling Zicsr and Zifencei standalone extensions for gcc >= 12
> as recommanded by [2].
> 
> Select Zicsr and Zifencei for General purpose (G) architecture variant
> (BR2_riscv_g) since theses extentions were implicitely enabled
> previously.
> 
> [1] https://gcc.gnu.org/gcc-12/changes.html
> [2] https://groups.google.com/a/groups.riscv.org/g/sw-dev/c/aE1ZeHHCYf4
> [3] https://github.com/riscv-software-src/opensbi/blob/v0.9/lib/sbi/sbi_tlb.c#L173
> [4] https://github.com/riscv-software-src/opensbi/commit/5d53b55aa77ffeefd4012445dfa6ad3535e1ff2c
> 
> Signed-off-by: Romain Naour <romain.naour@gmail.com>
> Cc: Mark Corbin <mark@dibsco.co.uk>

Thanks for the patch and explanation, but I'm wondering if this isn't
perhaps too complicated.


> +	select BR2_RISCV_ISA_RVZicsr if BR2_TOOLCHAIN_GCC_AT_LEAST_12
> +	select BR2_RISCV_ISA_RVZifencei if BR2_TOOLCHAIN_GCC_AT_LEAST_12

[...]

>  	help
> -	  General purpose (G) is equivalent to IMAFD.
> +	  General purpose (G) is equivalent to IMAFD
> +	  (with Zicsr and Zifencei since gcc >= 12).
>  
>  config BR2_riscv_custom
>  	bool "Custom architecture"
> @@ -63,6 +72,16 @@ config BR2_RISCV_ISA_CUSTOM_RVD
>  config BR2_RISCV_ISA_CUSTOM_RVC
>  	bool "Compressed Instructions (C)"
>  	select BR2_RISCV_ISA_RVC
> +
> +if BR2_TOOLCHAIN_GCC_AT_LEAST_12

The problem with this is that options in arch/ now depend on the gcc
version that is selected, which is something we have tried to avoid.

Do a git grep GCC_AT_LEAST in arch/, you will see that we only use
"select BR2_ARCH_NEEDS_GCC_AT_LEAST_xyz", i.e the choice of the
architecture/variant/ABI is what drives the available gcc versions.

And here you are doing exactly the opposite: it's the selected gcc
version that drives the architecture/variant/ABI options that are
available.

At this point, I don't really see the need from a Buildroot perspective
to separate icsr and ifencei from the base BR2_RISCV_ISA_RVI
instruction set.

So what I would do is exactly what OpenSBI has done in
https://github.com/riscv-software-src/opensbi/commit/5d53b55aa77ffeefd4012445dfa6ad3535e1ff2c:
when gcc >= 12 is used, we simply use -march with a _zicsr_zifencei
suffix, and that's it.

I.e something like this:

diff --git a/arch/arch.mk.riscv b/arch/arch.mk.riscv
index f3bf2b3467..d77f88dd70 100644
--- a/arch/arch.mk.riscv
+++ b/arch/arch.mk.riscv
@@ -27,4 +27,8 @@ ifeq ($(BR2_RISCV_ISA_RVC),y)
 GCC_TARGET_ARCH := $(GCC_TARGET_ARCH)c
 endif
 
+ifeq ($(BR2_TOOLCHAIN_GCC_AT_LEAST_12),y)
+GCC_TARGET_ARCH := $(GCC_TARGET_ARCH)_zicsr_zifencei
+endif
+
 endif

I don't think we will see RISC-V cores running Linux that support the
base I instruction, but not the csr and fence extensions... as even
OpenSBI assumes the RISC-V core supports them!

What do you think?

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot