* [PATCH v2] RISC-V: remove I-extension ISA spec version dance
@ 2023-03-08 22:08 Conor Dooley
2023-03-09 2:19 ` Jessica Clarke
2023-03-10 15:07 ` Bin Meng
0 siblings, 2 replies; 14+ messages in thread
From: Conor Dooley @ 2023-03-08 22:08 UTC (permalink / raw)
To: palmer
Cc: conor, Conor Dooley, linux-riscv, nathan, naresh.kamboju, llvm,
stable, aurelien, ndesaulniers, Palmer Dabbelt
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.
From *my* testing, I was only able to reproduce the above build failures
because of *GCC* defaulting to a newer ISA spec version, and saw no
issues with CC=clang builds, where -misa-spec is not (AFAICT) passed to
gas.
I'm far from a toolchain person, so I am very very happy to have the
reason for that explained to me, as I've been scratching my head about
it all evening.
I'm also not super confident in my "as-option"ing, but it worked for me,
so it's gotta be perfect, right? riiight??
Changes from v1:
- entirely new approach to the issue
---
arch/riscv/Makefile | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 6203c3378922..2df7a5dc071c 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -57,10 +57,8 @@ riscv-march-$(CONFIG_ARCH_RV64I) := rv64ima
riscv-march-$(CONFIG_FPU) := $(riscv-march-y)fd
riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)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, -march=$(riscv-march-y)_zicsr_zifencei)
-riscv-march-$(toolchain-need-zicsr-zifencei) := $(riscv-march-y)_zicsr_zifencei
+KBUILD_CFLAGS += $(call cc-option,-misa-spec=2.2)
+KBUILD_AFLAGS += $(call as-option,-Wa$(comma)-misa-spec=2.2)
# Check if the toolchain supports Zihintpause extension
riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE) := $(riscv-march-y)_zihintpause
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2] RISC-V: remove I-extension ISA spec version dance 2023-03-08 22:08 [PATCH v2] RISC-V: remove I-extension ISA spec version dance Conor Dooley @ 2023-03-09 2:19 ` Jessica Clarke 2023-03-09 6:19 ` Conor Dooley 2023-03-10 15:07 ` Bin Meng 1 sibling, 1 reply; 14+ messages in thread From: Jessica Clarke @ 2023-03-09 2:19 UTC (permalink / raw) To: Conor Dooley Cc: Palmer Dabbelt, Conor Dooley, linux-riscv, Nathan Chancellor, naresh.kamboju, llvm, stable, aurelien, ndesaulniers, Palmer Dabbelt On 8 Mar 2023, at 22:08, Conor Dooley <conor@kernel.org> 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... For what it’s worth, LLVM is likely to move from only supporting the old ratified spec to only supporting the latest one, with no ugly -misa-spec like the GNU world. You may therefore wish to reconsider this... Jess > 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. > From *my* testing, I was only able to reproduce the above build failures > because of *GCC* defaulting to a newer ISA spec version, and saw no > issues with CC=clang builds, where -misa-spec is not (AFAICT) passed to > gas. > I'm far from a toolchain person, so I am very very happy to have the > reason for that explained to me, as I've been scratching my head about > it all evening. > > I'm also not super confident in my "as-option"ing, but it worked for me, > so it's gotta be perfect, right? riiight?? > > Changes from v1: > - entirely new approach to the issue > --- > arch/riscv/Makefile | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > index 6203c3378922..2df7a5dc071c 100644 > --- a/arch/riscv/Makefile > +++ b/arch/riscv/Makefile > @@ -57,10 +57,8 @@ riscv-march-$(CONFIG_ARCH_RV64I) := rv64ima > riscv-march-$(CONFIG_FPU) := $(riscv-march-y)fd > riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)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, -march=$(riscv-march-y)_zicsr_zifencei) > -riscv-march-$(toolchain-need-zicsr-zifencei) := $(riscv-march-y)_zicsr_zifencei > +KBUILD_CFLAGS += $(call cc-option,-misa-spec=2.2) > +KBUILD_AFLAGS += $(call as-option,-Wa$(comma)-misa-spec=2.2) > > # Check if the toolchain supports Zihintpause extension > riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE) := $(riscv-march-y)_zihintpause > -- > 2.39.2 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] RISC-V: remove I-extension ISA spec version dance 2023-03-09 2:19 ` Jessica Clarke @ 2023-03-09 6:19 ` Conor Dooley 0 siblings, 0 replies; 14+ messages in thread From: Conor Dooley @ 2023-03-09 6:19 UTC (permalink / raw) To: Jessica Clarke Cc: Palmer Dabbelt, Conor Dooley, linux-riscv, Nathan Chancellor, naresh.kamboju, llvm, stable, aurelien, ndesaulniers, Palmer Dabbelt On 9 March 2023 02:19:38 GMT, Jessica Clarke <jrtc27@jrtc27.com> wrote: >On 8 Mar 2023, at 22:08, Conor Dooley <conor@kernel.org> 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... > >For what it’s worth, LLVM is likely to move from only supporting the >old ratified spec to only supporting the latest one, with no ugly >-misa-spec like the GNU world. You may therefore wish to reconsider >this... Oh well, at least we tried. Back to the Kconfig way of doing it I suppose... > >Jess > >> 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. >> From *my* testing, I was only able to reproduce the above build failures >> because of *GCC* defaulting to a newer ISA spec version, and saw no >> issues with CC=clang builds, where -misa-spec is not (AFAICT) passed to >> gas. >> I'm far from a toolchain person, so I am very very happy to have the >> reason for that explained to me, as I've been scratching my head about >> it all evening. >> >> I'm also not super confident in my "as-option"ing, but it worked for me, >> so it's gotta be perfect, right? riiight?? >> >> Changes from v1: >> - entirely new approach to the issue >> --- >> arch/riscv/Makefile | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile >> index 6203c3378922..2df7a5dc071c 100644 >> --- a/arch/riscv/Makefile >> +++ b/arch/riscv/Makefile >> @@ -57,10 +57,8 @@ riscv-march-$(CONFIG_ARCH_RV64I) := rv64ima >> riscv-march-$(CONFIG_FPU) := $(riscv-march-y)fd >> riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)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, -march=$(riscv-march-y)_zicsr_zifencei) >> -riscv-march-$(toolchain-need-zicsr-zifencei) := $(riscv-march-y)_zicsr_zifencei >> +KBUILD_CFLAGS += $(call cc-option,-misa-spec=2.2) >> +KBUILD_AFLAGS += $(call as-option,-Wa$(comma)-misa-spec=2.2) >> >> # Check if the toolchain supports Zihintpause extension >> riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE) := $(riscv-march-y)_zihintpause >> -- >> 2.39.2 >> >> >> _______________________________________________ >> linux-riscv mailing list >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] RISC-V: remove I-extension ISA spec version dance 2023-03-08 22:08 [PATCH v2] RISC-V: remove I-extension ISA spec version dance Conor Dooley 2023-03-09 2:19 ` Jessica Clarke @ 2023-03-10 15:07 ` Bin Meng 2023-03-10 15:35 ` Bin Meng 2023-03-11 8:52 ` Aurelien Jarno 1 sibling, 2 replies; 14+ messages in thread From: Bin Meng @ 2023-03-10 15:07 UTC (permalink / raw) To: conor Cc: aurelien, conor.dooley, linux-riscv, llvm, naresh.kamboju, nathan, ndesaulniers, palmer, palmer, stable > 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 The defconfig of v5.16 kernel (commit 6df2a016c0c8 lands in v5.17) builds fine for me. Anything I am missing? > >From *my* testing, I was only able to reproduce the above build failures > because of *GCC* defaulting to a newer ISA spec version, and saw no > issues with CC=clang builds, where -misa-spec is not (AFAICT) passed to > gas. > I'm far from a toolchain person, so I am very very happy to have the > reason for that explained to me, as I've been scratching my head about > it all evening. > > I'm also not super confident in my "as-option"ing, but it worked for me, > so it's gotta be perfect, right? riiight?? > > Changes from v1: > - entirely new approach to the issue > --- > arch/riscv/Makefile | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > index 6203c3378922..2df7a5dc071c 100644 > --- a/arch/riscv/Makefile > +++ b/arch/riscv/Makefile > @@ -57,10 +57,8 @@ riscv-march-$(CONFIG_ARCH_RV64I) := rv64ima > riscv-march-$(CONFIG_FPU) := $(riscv-march-y)fd > riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)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, -march=$(riscv-march-y)_zicsr_zifencei) > -riscv-march-$(toolchain-need-zicsr-zifencei) := $(riscv-march-y)_zicsr_zifencei > +KBUILD_CFLAGS += $(call cc-option,-misa-spec=2.2) > +KBUILD_AFLAGS += $(call as-option,-Wa$(comma)-misa-spec=2.2) > > # Check if the toolchain supports Zihintpause extension > riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE) := $(riscv-march-y)_zihintpause > -- ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] RISC-V: remove I-extension ISA spec version dance 2023-03-10 15:07 ` Bin Meng @ 2023-03-10 15:35 ` Bin Meng 2023-03-10 16:40 ` Conor Dooley 2023-03-11 8:54 ` Aurelien Jarno 2023-03-11 8:52 ` Aurelien Jarno 1 sibling, 2 replies; 14+ messages in thread From: Bin Meng @ 2023-03-10 15:35 UTC (permalink / raw) To: conor Cc: aurelien, conor.dooley, linux-riscv, llvm, naresh.kamboju, nathan, ndesaulniers, palmer, palmer, stable On Fri, Mar 10, 2023 at 11:07 PM Bin Meng <bmeng.cn@gmail.com> 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 > > The defconfig of v5.16 kernel (commit 6df2a016c0c8 lands in v5.17) builds fine > for me. Anything I am missing? > Some further note: After I switched to kernel.org pre-built GCC 12.2.0 [1] which includes binutils 2.39, I was able to reproduce the exact same build failure of v5.16 kernel as described in the commit 6df2a016c0c8 ("riscv: fix build with binutils 2.38") by Aurelien Jarno. To verify the commit message of 6df2a016c0c8 is accurate or not, I built a GAS from binutils 2.37 and replaced the GAS 2.39 in the kernel.org package, surprisingly kernel v5.16 did not build with the same build failure. So it seems that it's GCC that caused the build failure instead of GAS from binutils?? [1] https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/12.2.0/x86_64-gcc-12.2.0-nolibc-riscv64-linux.tar.xz Regards, Bin ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] RISC-V: remove I-extension ISA spec version dance 2023-03-10 15:35 ` Bin Meng @ 2023-03-10 16:40 ` Conor Dooley 2023-03-11 5:41 ` Bin Meng 2023-03-11 10:11 ` Aurelien Jarno 2023-03-11 8:54 ` Aurelien Jarno 1 sibling, 2 replies; 14+ messages in thread From: Conor Dooley @ 2023-03-10 16:40 UTC (permalink / raw) To: Bin Meng Cc: aurelien, conor.dooley, linux-riscv, llvm, naresh.kamboju, nathan, ndesaulniers, palmer, palmer, stable [-- Attachment #1: Type: text/plain, Size: 6239 bytes --] On Fri, Mar 10, 2023 at 11:35:57PM +0800, Bin Meng wrote: > On Fri, Mar 10, 2023 at 11:07 PM Bin Meng <bmeng.cn@gmail.com> 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 > > > > The defconfig of v5.16 kernel (commit 6df2a016c0c8 lands in v5.17) builds fine > > for me. Anything I am missing? > > > > Some further note: > > After I switched to kernel.org pre-built GCC 12.2.0 [1] which includes > binutils 2.39, I was able to reproduce the exact same build failure of > v5.16 kernel as described in the commit 6df2a016c0c8 ("riscv: fix > build with binutils 2.38") by Aurelien Jarno. > > To verify the commit message of 6df2a016c0c8 is accurate or not, I > built a GAS from binutils 2.37 and replaced the GAS 2.39 in the > kernel.org package, surprisingly kernel v5.16 did not build with the > same build failure. > > So it seems that it's GCC that caused the build failure instead of GAS > from binutils?? Right, that's what I was getting at in the bit below the --- line in my patch. I think Aurelien was misled by the failure message and your email ([1] in my links above) which claimed that binutils would default to the 20191213 spec. It appears (and I'm not a tc person) that GCC must call GAS with the --misa-spec argument, and in GCC 12 the value used is 20191213. Either GCC 11 must pass --misa-spec=2.2 to binutils, or it passes nothing, leading binutils to be permissive about what -march=rv64i means. The permissive option would "seem" to be correct, as building with clang (that to my knowledge doesn't pass --misa-spec to GAS) and with -march=rv64i has no issues assembling. It'd appear to me that binutils is a *player* in this issue, but is not the culprit of the issue Aurelien sought to fix. I dunno what I am talking about though, this is all from playing around with many tc variants and see how it goes! Cheers, Conor. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] RISC-V: remove I-extension ISA spec version dance 2023-03-10 16:40 ` Conor Dooley @ 2023-03-11 5:41 ` Bin Meng 2023-03-11 10:49 ` Aurelien Jarno 2023-03-11 10:11 ` Aurelien Jarno 1 sibling, 1 reply; 14+ messages in thread From: Bin Meng @ 2023-03-11 5:41 UTC (permalink / raw) To: Conor Dooley Cc: aurelien, conor.dooley, linux-riscv, llvm, naresh.kamboju, nathan, ndesaulniers, palmer, palmer, stable On Sat, Mar 11, 2023 at 12:40 AM Conor Dooley <conor@kernel.org> wrote: > > On Fri, Mar 10, 2023 at 11:35:57PM +0800, Bin Meng wrote: > > On Fri, Mar 10, 2023 at 11:07 PM Bin Meng <bmeng.cn@gmail.com> 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 > > > > > > The defconfig of v5.16 kernel (commit 6df2a016c0c8 lands in v5.17) builds fine > > > for me. Anything I am missing? > > > > > > > Some further note: > > > > After I switched to kernel.org pre-built GCC 12.2.0 [1] which includes > > binutils 2.39, I was able to reproduce the exact same build failure of > > v5.16 kernel as described in the commit 6df2a016c0c8 ("riscv: fix > > build with binutils 2.38") by Aurelien Jarno. > > > > To verify the commit message of 6df2a016c0c8 is accurate or not, I > > built a GAS from binutils 2.37 and replaced the GAS 2.39 in the > > kernel.org package, surprisingly kernel v5.16 did not build with the > > same build failure. > > > > So it seems that it's GCC that caused the build failure instead of GAS > > from binutils?? > > Right, that's what I was getting at in the bit below the --- line in my > patch. I think Aurelien was misled by the failure message and your email > ([1] in my links above) which claimed that binutils would default to > the 20191213 spec. > It appears (and I'm not a tc person) that GCC must call GAS with the > --misa-spec argument, and in GCC 12 the value used is 20191213. > Either GCC 11 must pass --misa-spec=2.2 to binutils, or it passes > nothing, leading binutils to be permissive about what -march=rv64i > means. I verified that "-misa-spec" is a new option introduced in GCC 12 and the default value is set to 20191213 which is unfortunately backward incompatible. GCC 11 does not have the "-misa-spec" option, so I assume it produces backward compatible codes. IOW, what commit 6df2a016c0c8 was trying to fix has nothing to do with binutils 2.38+. It's the GCC changes that is the culprit. For this patch, I think it LGTM, so: Reviewed-by: Bin Meng <bmeng@tinylab.org> > > The permissive option would "seem" to be correct, as building with clang > (that to my knowledge doesn't pass --misa-spec to GAS) and with > -march=rv64i has no issues assembling. > > It'd appear to me that binutils is a *player* in this issue, but is not > the culprit of the issue Aurelien sought to fix. > > I dunno what I am talking about though, this is all from playing around > with many tc variants and see how it goes! > Regards, Bin ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] RISC-V: remove I-extension ISA spec version dance 2023-03-11 5:41 ` Bin Meng @ 2023-03-11 10:49 ` Aurelien Jarno 2023-03-11 13:09 ` Bin Meng 0 siblings, 1 reply; 14+ messages in thread From: Aurelien Jarno @ 2023-03-11 10:49 UTC (permalink / raw) To: Bin Meng Cc: Conor Dooley, conor.dooley, linux-riscv, llvm, naresh.kamboju, nathan, ndesaulniers, palmer, palmer, stable On 2023-03-11 13:41, Bin Meng wrote: > On Sat, Mar 11, 2023 at 12:40 AM Conor Dooley <conor@kernel.org> wrote: > > > > On Fri, Mar 10, 2023 at 11:35:57PM +0800, Bin Meng wrote: > > > On Fri, Mar 10, 2023 at 11:07 PM Bin Meng <bmeng.cn@gmail.com> 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 > > > > > > > > The defconfig of v5.16 kernel (commit 6df2a016c0c8 lands in v5.17) builds fine > > > > for me. Anything I am missing? > > > > > > > > > > Some further note: > > > > > > After I switched to kernel.org pre-built GCC 12.2.0 [1] which includes > > > binutils 2.39, I was able to reproduce the exact same build failure of > > > v5.16 kernel as described in the commit 6df2a016c0c8 ("riscv: fix > > > build with binutils 2.38") by Aurelien Jarno. > > > > > > To verify the commit message of 6df2a016c0c8 is accurate or not, I > > > built a GAS from binutils 2.37 and replaced the GAS 2.39 in the > > > kernel.org package, surprisingly kernel v5.16 did not build with the > > > same build failure. > > > > > > So it seems that it's GCC that caused the build failure instead of GAS > > > from binutils?? > > > > Right, that's what I was getting at in the bit below the --- line in my > > patch. I think Aurelien was misled by the failure message and your email > > ([1] in my links above) which claimed that binutils would default to > > the 20191213 spec. > > It appears (and I'm not a tc person) that GCC must call GAS with the > > --misa-spec argument, and in GCC 12 the value used is 20191213. > > Either GCC 11 must pass --misa-spec=2.2 to binutils, or it passes > > nothing, leading binutils to be permissive about what -march=rv64i > > means. > > I verified that "-misa-spec" is a new option introduced in GCC 12 and > the default value is set to 20191213 which is unfortunately backward > incompatible. > > GCC 11 does not have the "-misa-spec" option, so I assume it produces > backward compatible codes. > > IOW, what commit 6df2a016c0c8 was trying to fix has nothing to do with > binutils 2.38+. It's the GCC changes that is the culprit. I disagree with that statement. Your tests seems to have been done using the toolchains from [1], which changes two parameters, i.e. both the GCC and binutils version. The original issue can be demonstrated the following way: - Revert commit 6df2a016c0c8 - Get toolchain from: 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 - Get toolchain from: 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.xz - Replace the as version in the 11.1.0 toolchain (2.36.1) with the one from the 1.3.0 one (2.38), for instance using: ln -sf ../../../../gcc-11.3.0-nolibc/riscv64-linux/riscv64-linux/bin/as gcc-11.1.0-nolibc/riscv64-linux/riscv64-linux/bin/as - Build a defconfig kernel using the 11.1.0 toolchain, so using GCC 11.1 with binutils 2.38 [1] https://mirrors.edge.kernel.org/pub/tools/crosstool/ -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] RISC-V: remove I-extension ISA spec version dance 2023-03-11 10:49 ` Aurelien Jarno @ 2023-03-11 13:09 ` Bin Meng 0 siblings, 0 replies; 14+ messages in thread From: Bin Meng @ 2023-03-11 13:09 UTC (permalink / raw) To: Aurelien Jarno Cc: Conor Dooley, conor.dooley, linux-riscv, llvm, naresh.kamboju, nathan, ndesaulniers, palmer, palmer, stable On Sat, Mar 11, 2023 at 6:49 PM Aurelien Jarno <aurelien@aurel32.net> wrote: > > On 2023-03-11 13:41, Bin Meng wrote: > > On Sat, Mar 11, 2023 at 12:40 AM Conor Dooley <conor@kernel.org> wrote: > > > > > > On Fri, Mar 10, 2023 at 11:35:57PM +0800, Bin Meng wrote: > > > > On Fri, Mar 10, 2023 at 11:07 PM Bin Meng <bmeng.cn@gmail.com> 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 > > > > > > > > > > The defconfig of v5.16 kernel (commit 6df2a016c0c8 lands in v5.17) builds fine > > > > > for me. Anything I am missing? > > > > > > > > > > > > > Some further note: > > > > > > > > After I switched to kernel.org pre-built GCC 12.2.0 [1] which includes > > > > binutils 2.39, I was able to reproduce the exact same build failure of > > > > v5.16 kernel as described in the commit 6df2a016c0c8 ("riscv: fix > > > > build with binutils 2.38") by Aurelien Jarno. > > > > > > > > To verify the commit message of 6df2a016c0c8 is accurate or not, I > > > > built a GAS from binutils 2.37 and replaced the GAS 2.39 in the > > > > kernel.org package, surprisingly kernel v5.16 did not build with the > > > > same build failure. > > > > > > > > So it seems that it's GCC that caused the build failure instead of GAS > > > > from binutils?? > > > > > > Right, that's what I was getting at in the bit below the --- line in my > > > patch. I think Aurelien was misled by the failure message and your email > > > ([1] in my links above) which claimed that binutils would default to > > > the 20191213 spec. > > > It appears (and I'm not a tc person) that GCC must call GAS with the > > > --misa-spec argument, and in GCC 12 the value used is 20191213. > > > Either GCC 11 must pass --misa-spec=2.2 to binutils, or it passes > > > nothing, leading binutils to be permissive about what -march=rv64i > > > means. > > > > I verified that "-misa-spec" is a new option introduced in GCC 12 and > > the default value is set to 20191213 which is unfortunately backward > > incompatible. > > > > GCC 11 does not have the "-misa-spec" option, so I assume it produces > > backward compatible codes. > > > > IOW, what commit 6df2a016c0c8 was trying to fix has nothing to do with > > binutils 2.38+. It's the GCC changes that is the culprit. > > I disagree with that statement. Your tests seems to have been done using > the toolchains from [1], which changes two parameters, i.e. both the GCC > and binutils version. > > The original issue can be demonstrated the following way: > > - Revert commit 6df2a016c0c8 > - Get toolchain from: > 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 > - Get toolchain from: > 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.xz > - Replace the as version in the 11.1.0 toolchain (2.36.1) with the one > from the 1.3.0 one (2.38), for instance using: > ln -sf ../../../../gcc-11.3.0-nolibc/riscv64-linux/riscv64-linux/bin/as gcc-11.1.0-nolibc/riscv64-linux/riscv64-linux/bin/as > - Build a defconfig kernel using the 11.1.0 toolchain, so using GCC 11.1 > with binutils 2.38 > > [1] https://mirrors.edge.kernel.org/pub/tools/crosstool/ > Okay, thanks for sharing the info of binutils's configuration of "-misa-spec". Indeed I checked the "-misa-spec" default value was changed from 2.2 to 20191213 in binutils 2.38 via the following commit: commit aed44286efa8ae8717a77d94b51ac3614e2ca6dc Author: Nelson Chu <nelson.chu@sifive.com> Date: Thu Dec 30 23:23:46 2021 +0800 RISC-V: Updated the default ISA spec to 20191213. Update the default ISA spec from 2.2 to 20191213 will change the default version of i from 2.0 to 2.1. Since zicsr and zifencei are separated from i 2.1, users need to add them in the architecture string if they need fence.i and csr instructions. Besides, we also allow old ISA spec can recognize zicsr and zifencei, but we won't output them since they are already included in the i extension when i's version is less than 2.1. So the original commit message of 6df2a016c0c8 was correct at the time of writing. As you mentioned, now GCC also brings the "-misa-spec" starting from GCC 12. This makes things complicated ... Regards, Bin ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] RISC-V: remove I-extension ISA spec version dance 2023-03-10 16:40 ` Conor Dooley 2023-03-11 5:41 ` Bin Meng @ 2023-03-11 10:11 ` Aurelien Jarno 2023-03-11 10:40 ` Conor Dooley 1 sibling, 1 reply; 14+ messages in thread From: Aurelien Jarno @ 2023-03-11 10:11 UTC (permalink / raw) To: Conor Dooley Cc: Bin Meng, conor.dooley, linux-riscv, llvm, naresh.kamboju, nathan, ndesaulniers, palmer, palmer, stable [-- Attachment #1: Type: text/plain, Size: 7248 bytes --] On 2023-03-10 16:40, Conor Dooley wrote: > On Fri, Mar 10, 2023 at 11:35:57PM +0800, Bin Meng wrote: > > On Fri, Mar 10, 2023 at 11:07 PM Bin Meng <bmeng.cn@gmail.com> 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 > > > > > > The defconfig of v5.16 kernel (commit 6df2a016c0c8 lands in v5.17) builds fine > > > for me. Anything I am missing? > > > > > > > Some further note: > > > > After I switched to kernel.org pre-built GCC 12.2.0 [1] which includes > > binutils 2.39, I was able to reproduce the exact same build failure of > > v5.16 kernel as described in the commit 6df2a016c0c8 ("riscv: fix > > build with binutils 2.38") by Aurelien Jarno. > > > > To verify the commit message of 6df2a016c0c8 is accurate or not, I > > built a GAS from binutils 2.37 and replaced the GAS 2.39 in the > > kernel.org package, surprisingly kernel v5.16 did not build with the > > same build failure. > > > > So it seems that it's GCC that caused the build failure instead of GAS > > from binutils?? > > Right, that's what I was getting at in the bit below the --- line in my > patch. I think Aurelien was misled by the failure message and your email > ([1] in my links above) which claimed that binutils would default to > the 20191213 spec. No I was not misled by that, at that time GCC 11.3 and GCC 12 were not released. binutils definitely defaults to 20191213 if you do not use the --with-isa-spec with a different value when configuring it. > It appears (and I'm not a tc person) that GCC must call GAS with the > --misa-spec argument, and in GCC 12 the value used is 20191213. > Either GCC 11 must pass --misa-spec=2.2 to binutils, or it passes > nothing, leading binutils to be permissive about what -march=rv64i > means. GCC 11.1 and 11.2 pass nothing to binutils, hence the issue I reported and the patch fixing that. Basic support for misa-spec has been backported to GCC 11.3, with a default to --misa-spec=2.2, hence the "permissive" behaviour you observe. > The permissive option would "seem" to be correct, as building with clang > (that to my knowledge doesn't pass --misa-spec to GAS) and with > -march=rv64i has no issues assembling. The "permissive" way only work if we drop support for GCC 11.1 and 11.2, which sounds acceptable to me. > It'd appear to me that binutils is a *player* in this issue, but is not > the culprit of the issue Aurelien sought to fix. I *disagree*. At the time the commit has been merged, binutils was the only culprit. With more recent versions of GCC 11.3 and GCC 12 released in the meantime, the situation is way more complex. -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] RISC-V: remove I-extension ISA spec version dance 2023-03-11 10:11 ` Aurelien Jarno @ 2023-03-11 10:40 ` Conor Dooley 2023-03-11 10:54 ` Aurelien Jarno 0 siblings, 1 reply; 14+ messages in thread From: Conor Dooley @ 2023-03-11 10:40 UTC (permalink / raw) To: Aurelien Jarno Cc: Bin Meng, conor.dooley, linux-riscv, llvm, naresh.kamboju, nathan, ndesaulniers, palmer, palmer, stable [-- Attachment #1: Type: text/plain, Size: 8422 bytes --] On Sat, Mar 11, 2023 at 11:11:57AM +0100, Aurelien Jarno wrote: > On 2023-03-10 16:40, Conor Dooley wrote: > > On Fri, Mar 10, 2023 at 11:35:57PM +0800, Bin Meng wrote: > > > On Fri, Mar 10, 2023 at 11:07 PM Bin Meng <bmeng.cn@gmail.com> 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 > > > > > > > > The defconfig of v5.16 kernel (commit 6df2a016c0c8 lands in v5.17) builds fine > > > > for me. Anything I am missing? > > > > > > > > > > Some further note: > > > > > > After I switched to kernel.org pre-built GCC 12.2.0 [1] which includes > > > binutils 2.39, I was able to reproduce the exact same build failure of > > > v5.16 kernel as described in the commit 6df2a016c0c8 ("riscv: fix > > > build with binutils 2.38") by Aurelien Jarno. > > > > > > To verify the commit message of 6df2a016c0c8 is accurate or not, I > > > built a GAS from binutils 2.37 and replaced the GAS 2.39 in the > > > kernel.org package, surprisingly kernel v5.16 did not build with the > > > same build failure. > > > > > > So it seems that it's GCC that caused the build failure instead of GAS > > > from binutils?? > > > > Right, that's what I was getting at in the bit below the --- line in my > > patch. I think Aurelien was misled by the failure message and your email > > ([1] in my links above) which claimed that binutils would default to > > the 20191213 spec. > > No I was not misled by that, at that time GCC 11.3 and GCC 12 were not > released. > > binutils definitely defaults to 20191213 if you do not use the > --with-isa-spec with a different value when configuring it. I specifically built binutils without that flag, and had no issues building the kernel with clang, which is the source of my confusion here really. I'd have expected that to fail > > It appears (and I'm not a tc person) that GCC must call GAS with the > > --misa-spec argument, and in GCC 12 the value used is 20191213. > > Either GCC 11 must pass --misa-spec=2.2 to binutils, or it passes > > nothing, leading binutils to be permissive about what -march=rv64i > > means. > > GCC 11.1 and 11.2 pass nothing to binutils, hence the issue I reported > and the patch fixing that. Basic support for misa-spec has been > backported to GCC 11.3, with a default to --misa-spec=2.2, hence the > "permissive" behaviour you observe. > > > The permissive option would "seem" to be correct, as building with clang > > (that to my knowledge doesn't pass --misa-spec to GAS) and with > > -march=rv64i has no issues assembling. > > The "permissive" way only work if we drop support for GCC 11.1 and 11.2, > which sounds acceptable to me. I don't think that we can do that. > > It'd appear to me that binutils is a *player* in this issue, but is not > > the culprit of the issue Aurelien sought to fix. > > I *disagree*. At the time the commit has been merged, binutils was the > only culprit. Okay, fair enough! I was only going off my observations with current-day, and clearly you remember well the situation that you encountered this with! > With more recent versions of GCC 11.3 and GCC 12 released > in the meantime, the situation is way more complex. This patch is going to be problematic w/ versions of gcc that do not understand -misa-spec + binutils 2.38 then, isn't it. Based on what Jess has said, pursuing this approach seems ill-advised anyway. I'm inclined to go back to my approach in v1 & only apply your march addition when AS_IS_GNU. That still leaves a problem for a configuration where GAS is 2.38 and LD is 2.35, which I don't think can be handled. I feel the only option there is just erroring the build if someones tries it. Ditto for a future IAS + LD 2.35 combination, if IAS is using a 20190608 or later of the ISA spec. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] RISC-V: remove I-extension ISA spec version dance 2023-03-11 10:40 ` Conor Dooley @ 2023-03-11 10:54 ` Aurelien Jarno 0 siblings, 0 replies; 14+ messages in thread From: Aurelien Jarno @ 2023-03-11 10:54 UTC (permalink / raw) To: Conor Dooley Cc: Bin Meng, conor.dooley, linux-riscv, llvm, naresh.kamboju, nathan, ndesaulniers, palmer, palmer, stable [-- Attachment #1: Type: text/plain, Size: 9078 bytes --] On 2023-03-11 10:40, Conor Dooley wrote: > On Sat, Mar 11, 2023 at 11:11:57AM +0100, Aurelien Jarno wrote: > > On 2023-03-10 16:40, Conor Dooley wrote: > > > On Fri, Mar 10, 2023 at 11:35:57PM +0800, Bin Meng wrote: > > > > On Fri, Mar 10, 2023 at 11:07 PM Bin Meng <bmeng.cn@gmail.com> 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 > > > > > > > > > > The defconfig of v5.16 kernel (commit 6df2a016c0c8 lands in v5.17) builds fine > > > > > for me. Anything I am missing? > > > > > > > > > > > > > Some further note: > > > > > > > > After I switched to kernel.org pre-built GCC 12.2.0 [1] which includes > > > > binutils 2.39, I was able to reproduce the exact same build failure of > > > > v5.16 kernel as described in the commit 6df2a016c0c8 ("riscv: fix > > > > build with binutils 2.38") by Aurelien Jarno. > > > > > > > > To verify the commit message of 6df2a016c0c8 is accurate or not, I > > > > built a GAS from binutils 2.37 and replaced the GAS 2.39 in the > > > > kernel.org package, surprisingly kernel v5.16 did not build with the > > > > same build failure. > > > > > > > > So it seems that it's GCC that caused the build failure instead of GAS > > > > from binutils?? > > > > > > Right, that's what I was getting at in the bit below the --- line in my > > > patch. I think Aurelien was misled by the failure message and your email > > > ([1] in my links above) which claimed that binutils would default to > > > the 20191213 spec. > > > > No I was not misled by that, at that time GCC 11.3 and GCC 12 were not > > released. > > > > binutils definitely defaults to 20191213 if you do not use the > > --with-isa-spec with a different value when configuring it. > > I specifically built binutils without that flag, and had no issues > building the kernel with clang, which is the source of my confusion here > really. > I'd have expected that to fail > > > > It appears (and I'm not a tc person) that GCC must call GAS with the > > > --misa-spec argument, and in GCC 12 the value used is 20191213. > > > Either GCC 11 must pass --misa-spec=2.2 to binutils, or it passes > > > nothing, leading binutils to be permissive about what -march=rv64i > > > means. > > > > GCC 11.1 and 11.2 pass nothing to binutils, hence the issue I reported > > and the patch fixing that. Basic support for misa-spec has been > > backported to GCC 11.3, with a default to --misa-spec=2.2, hence the > > "permissive" behaviour you observe. > > > > > The permissive option would "seem" to be correct, as building with clang > > > (that to my knowledge doesn't pass --misa-spec to GAS) and with > > > -march=rv64i has no issues assembling. > > > > The "permissive" way only work if we drop support for GCC 11.1 and 11.2, > > which sounds acceptable to me. > > I don't think that we can do that. > > > > It'd appear to me that binutils is a *player* in this issue, but is not > > > the culprit of the issue Aurelien sought to fix. > > > > I *disagree*. At the time the commit has been merged, binutils was the > > only culprit. > > Okay, fair enough! I was only going off my observations with > current-day, and clearly you remember well the situation that you > encountered this with! > > > With more recent versions of GCC 11.3 and GCC 12 released > > in the meantime, the situation is way more complex. > > This patch is going to be problematic w/ versions of gcc that do not > understand -misa-spec + binutils 2.38 then, isn't it. > Based on what Jess has said, pursuing this approach seems ill-advised > anyway. It appears that while GCC 11.1 and GCC 11.2 do not pass -misa-spec=2.2 by default to as (contrary to GCC 11.3), they do accept getting called with that option and correctly pass it to as. That way your patch works fine with all GCC 11 and GCC 12 versions. That said I found various discussions on IRC from that time, and forcing an old ISA instead of enabling additional extensions was discouraged. From what I understand, it might create some incompatibilities in the future when new extensions are added (IOW enabling an extension could be incompatible with an old ISA spec). -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] RISC-V: remove I-extension ISA spec version dance 2023-03-10 15:35 ` Bin Meng 2023-03-10 16:40 ` Conor Dooley @ 2023-03-11 8:54 ` Aurelien Jarno 1 sibling, 0 replies; 14+ messages in thread From: Aurelien Jarno @ 2023-03-11 8:54 UTC (permalink / raw) To: Bin Meng Cc: conor, conor.dooley, linux-riscv, llvm, naresh.kamboju, nathan, ndesaulniers, palmer, palmer, stable On 2023-03-10 23:35, Bin Meng wrote: > On Fri, Mar 10, 2023 at 11:07 PM Bin Meng <bmeng.cn@gmail.com> 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 > > > > The defconfig of v5.16 kernel (commit 6df2a016c0c8 lands in v5.17) builds fine > > for me. Anything I am missing? > > > > Some further note: > > After I switched to kernel.org pre-built GCC 12.2.0 [1] which includes > binutils 2.39, I was able to reproduce the exact same build failure of > v5.16 kernel as described in the commit 6df2a016c0c8 ("riscv: fix > build with binutils 2.38") by Aurelien Jarno. > > To verify the commit message of 6df2a016c0c8 is accurate or not, I > built a GAS from binutils 2.37 and replaced the GAS 2.39 in the > kernel.org package, surprisingly kernel v5.16 did not build with the > same build failure. > > So it seems that it's GCC that caused the build failure instead of GAS > from binutils?? No, GCC 12 was not yet released as the time of the commit, and the issue was definitely due to binutils. That said GCC 12 also introduced some support of the new extensions, which made the situation even more complex. -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] RISC-V: remove I-extension ISA spec version dance 2023-03-10 15:07 ` Bin Meng 2023-03-10 15:35 ` Bin Meng @ 2023-03-11 8:52 ` Aurelien Jarno 1 sibling, 0 replies; 14+ messages in thread From: Aurelien Jarno @ 2023-03-11 8:52 UTC (permalink / raw) To: Bin Meng Cc: conor, conor.dooley, linux-riscv, llvm, naresh.kamboju, nathan, ndesaulniers, palmer, palmer, stable 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-03-11 13:09 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-08 22:08 [PATCH v2] RISC-V: remove I-extension ISA spec version dance Conor Dooley 2023-03-09 2:19 ` Jessica Clarke 2023-03-09 6:19 ` Conor Dooley 2023-03-10 15:07 ` Bin Meng 2023-03-10 15:35 ` Bin Meng 2023-03-10 16:40 ` Conor Dooley 2023-03-11 5:41 ` Bin Meng 2023-03-11 10:49 ` Aurelien Jarno 2023-03-11 13:09 ` Bin Meng 2023-03-11 10:11 ` Aurelien Jarno 2023-03-11 10:40 ` Conor Dooley 2023-03-11 10:54 ` Aurelien Jarno 2023-03-11 8:54 ` Aurelien Jarno 2023-03-11 8:52 ` Aurelien Jarno
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox