Hey Conor! Conor Dooley <conor@xxxxxxxxxx> writes: > On Tue, Apr 02, 2024 at 04:25:24PM +0200, Björn Töpel wrote: >> Pu Lehui <pulehui@xxxxxxxxxxxxxxx> writes: >> >> > On 2024/3/29 6:07, Conor Dooley wrote: >> >> On Thu, Mar 28, 2024 at 03:34:31PM -0400, Stefan O'Rear wrote: >> >>> On Thu, Mar 28, 2024, at 8:49 AM, Pu Lehui wrote: >> >>>> From: Pu Lehui <pulehui@xxxxxxxxxx> >> >>>> >> >>>> This patch relaxes the restrictions on the Zbb instructions. The hardware >> >>>> is capable of recognizing the Zbb instructions independently, eliminating >> >>>> the need for reliance on kernel compile configurations. >> >>> >> >>> This doesn't make sense to me. >> >> >> >> It doesn't make sense to me either. Of course the hardware's capability >> >> to understand an instruction is independent of whether or not a >> >> toolchain is capable of actually emitting the instruction. >> >> >> >>> RISCV_ISA_ZBB is defined as: >> >>> >> >>> Adds support to dynamically detect the presence of the ZBB >> >>> extension (basic bit manipulation) and enable its usage. >> >>> >> >>> In other words, RISCV_ISA_ZBB=n should disable everything that attempts >> >>> to detect Zbb at runtime. It is mostly relevant for code size reduction, >> >>> which is relevant for BPF since if RISCV_ISA_ZBB=n all rvzbb_enabled() >> >>> checks can be constant-folded. >> > >> > Thanks for review. My initial thought was the same as yours, but after >> > discussions [0] and test verifications, the hardware can indeed >> > recognize the zbb instruction even if the kernel has not enabled >> > CONFIG_RISCV_ISA_ZBB. As Conor mentioned, we are just acting as a JIT to >> > emit zbb instruction here. Maybe is_hw_zbb_capable() will be better? >> >> I still think Lehui's patch is correct; Building a kernel that can boot >> on multiple platforms (w/ or w/o Zbb support) and not having Zbb insn in >> the kernel proper, and iff Zbb is available at run-time the BPF JIT will >> emit Zbb. > > This sentence is -ENOPARSE to me, did you accidentally omit some words? > Additionally he config option has nothing to do with building kernels that > boot on multiple platforms, it only controls whether optimisations for Zbb > are built so that if Zbb is detected they can be used. Ugh, sorry about that! I'm probably confused myself. >> For these kind of optimizations, (IMO) it's better to let the BPF JIT >> decide at run-time. > > Why is bpf a different case to any other user in this regard? > I think that the commit message is misleading and needs to be changed, > because the point "the hardware is capable of recognising the Zbb > instructions independently..." is completely unrelated to the purpose > of the config option. Of course the hardware understanding the option > has nothing to do with kernel configuration. The commit message needs to > explain why bpf is a special case and is exempt from an > > I totally understand any point about bpf being different in terms of > needing toolchain support, but IIRC it was I who pointed out up-thread. > The part of the conversation that you're replying to here is about the > semantics of the Kconfig option and the original patch never mentioned > trying to avoid a dependency on toolchains at all, just kernel > configurations. The toolchain requirements I don't think are even super > hard to fulfill either - the last 3 versions of ld and lld all meet the > criteria. Thanks for making it more clear, and I agree that the toolchain requirements are not hard to fulfull. My view has been that "BPF is like userland", but I realize now that's odd. Let's make BPF similar to the rest of the RV kernel. If ZBB=n, then the BPF JIT doesn't know about emitting Zbb. Björn
