From: Conor Dooley <conor@kernel.org>
To: Jessica Clarke <jrtc27@jrtc27.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>,
Conor Dooley <conor.dooley@microchip.com>,
linux-riscv <linux-riscv@lists.infradead.org>,
Nathan Chancellor <nathan@kernel.org>,
naresh.kamboju@linaro.org, llvm@lists.linux.dev,
stable@vger.kernel.org, aurelien@aurel32.net,
ndesaulniers@google.com, Palmer Dabbelt <palmer@rivosinc.com>
Subject: Re: [PATCH v2] RISC-V: remove I-extension ISA spec version dance
Date: Thu, 09 Mar 2023 06:19:00 +0000 [thread overview]
Message-ID: <DCDDEB71-B9CF-4F35-BFA6-CAD4F5B0ED74@kernel.org> (raw)
In-Reply-To: <3476D6EF-F7DD-4437-A71C-47EF05210AF3@jrtc27.com>
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
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2023-03-09 6:19 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DCDDEB71-B9CF-4F35-BFA6-CAD4F5B0ED74@kernel.org \
--to=conor@kernel.org \
--cc=aurelien@aurel32.net \
--cc=conor.dooley@microchip.com \
--cc=jrtc27@jrtc27.com \
--cc=linux-riscv@lists.infradead.org \
--cc=llvm@lists.linux.dev \
--cc=naresh.kamboju@linaro.org \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=palmer@dabbelt.com \
--cc=palmer@rivosinc.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox