Re: [PATCH v2] RISC-V: remove I-extension ISA spec version dance

[Aurelien Jarno <aurelien@aurel32.net>]

On 2023-03-10 23:07, Bin Meng wrote:
> > From: Conor Dooley <conor.dooley@microchip.com>
> > 
> > The spec folk, in their infinite wisdom, moved both control and status
> > registers & the FENCE.I instructions out of the I extension into their
> > own extensions (Zicsr, Zifencei) in the 20190608 version of the ISA
> > spec [0].
> > The GCC/binutils crew decided [1] to move their default version of the
> > ISA spec to the 20191213 version of the ISA spec, which came into being
> > for version 2.38 of binutils and GCC 12. Building with this toolchain
> > configuration would result in assembler issues:
> >   CC      arch/riscv/kernel/vdso/vgettimeofday.o
> >   <<BUILDDIR>>/arch/riscv/include/asm/vdso/gettimeofday.h: Assembler messages:
> >   <<BUILDDIR>>/arch/riscv/include/asm/vdso/gettimeofday.h:71: Error: unrecognized opcode `csrr a5,0xc01'
> >   <<BUILDDIR>>/arch/riscv/include/asm/vdso/gettimeofday.h:71: Error: unrecognized opcode `csrr a5,0xc01'
> >   <<BUILDDIR>>/arch/riscv/include/asm/vdso/gettimeofday.h:71: Error: unrecognized opcode `csrr a5,0xc01'
> >   <<BUILDDIR>>/arch/riscv/include/asm/vdso/gettimeofday.h:71: Error: unrecognized opcode `csrr a5,0xc01'
> > This was fixed in commit 6df2a016c0c8 ("riscv: fix build with binutils
> > 2.38") by Aurelien Jarno, but has proven fragile.
> > 
> > Before LLVM 17, LLVM did not support these extensions and, as such, the
> > cc-option check added by Aurelien worked. Since commit 22e199e6afb1
> > ("[RISCV] Accept zicsr and zifencei command line options") however, LLVM
> > *does* support them and the cc-option check passes.
> > 
> > This surfaced as a problem while building the 5.10 stable kernel using
> > the default Tuxmake Debian image [2], as 5.10 did not yet support ld.lld,
> > and uses the Debian provided binutils 2.35.
> > Versions of ld prior to 2.38 will refuse to link if they encounter
> > unknown ISA extensions, and unfortunately Zifencei is not supported by
> > bintuils 2.35.
> > 
> > Instead of dancing around with adding these extensions to march, as we
> > currently do, Palmer suggested locking GCC builds to the same version of
> > the ISA spec that is used by LLVM. As far as I can tell, that is 2.2,
> > with, apparently [3], a lack of interest in implementing a flag like
> > GCC's -misa-spec at present.
> > 
> > Add {cc,as}-option checks to add -misa-spec to KBUILD_{A,C}FLAGS when
> > GCC is used & remove the march dance.
> > 
> > As clang does not accept this argument, I had expected to encounter
> > issues with the assembler, as neither zicsr nor zifencei are present in
> > the ISA string and the spec version *should* be defaulting to a version
> > that requires them to be present. The build passed however and the
> > resulting kernel worked perfectly fine for me on a PolarFire SoC...
> > 
> > Link: https://riscv.org/wp-content/uploads/2019/06/riscv-spec.pdf [0]
> > Link: https://groups.google.com/a/groups.riscv.org/g/sw-dev/c/aE1ZeHHCYf4 [1]
> > Link: https://lore.kernel.org/all/CA+G9fYt9T=ELCLaB9byxaLW2Qf4pZcDO=huCA0D8ug2V2+irJQ@mail.gmail.com/ [2]
> > Link: https://discourse.llvm.org/t/specifying-unpriviledge-spec-version-misa-spec-gcc-flag-equivalent/66935 [3]
> > CC: stable@vger.kernel.org
> > Suggested-by: Palmer Dabbelt <palmer@rivosinc.com>
> > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> > I think Aurelien's original commit message might actually not be quite
> > correct? I found, in my limited testing, that it is not the default
> > behaviour of gas that matters, but rather the toolchain itself?
> > My binutils versions (both those built using the clang-built-linux
> > tc-build scripts which do not set an ISA spec version, and one built
> > using the riscv-gnu-toolchain infra w/ an explicit 20191213 spec version
> > set) do not encounter these issues.
> 
> I am unable to reproduce the build failure as reported by commit 6df2a016c0c8
> ("riscv: fix build with binutils 2.38") by Aurelien Jarno using kernel.org
> pre-built GCC 11.3.0 [1] which includes binutils 2.38.
> 
> [1] https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.3.0/x86_64-gcc-11.3.0-nolibc-x86_64-linux.tar.xz

I guess you mean https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.3.0/x86_64-gcc-11.3.0-nolibc-riscv64-linux.tar.gz

> The defconfig of v5.16 kernel (commit 6df2a016c0c8 lands in v5.17) builds fine
> for me. Anything I am missing?

I can't find the corresponding source and unfortunately binutils doesn't
record the configure options in the binaries, but my guess is that it
has been configured with --with-isa-spec=2.2. This is not the
recommended way to go, but given the mess many distribution went that
road.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net