Re: [PATCH RESEND bpf-next v3 4/6] riscv, bpf: Add necessary Zbb instructions
From: Björn Töpel <bjorn@kernel.org>
Date: 2024-01-30 17:34:04
Also in:
bpf, linux-riscv
Pu Lehui [off-list ref] writes:
On 2024/1/30 14:18, Björn Töpel wrote:quoted
Daniel Borkmann [off-list ref] writes:quoted
On 1/29/24 10:13 AM, Pu Lehui wrote:quoted
On 2024/1/28 1:16, Björn Töpel wrote:quoted
Pu Lehui [off-list ref] writes:quoted
From: Pu Lehui <pulehui@huawei.com> Add necessary Zbb instructions introduced by [0] to reduce code size and improve performance of RV64 JIT. Meanwhile, a runtime deteted helper is added to check whether the CPU supports Zbb instructions. Link: https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf [0] Signed-off-by: Pu Lehui <pulehui@huawei.com> --- arch/riscv/net/bpf_jit.h | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h index e30501b46f8f..51f6d214086f 100644 --- a/arch/riscv/net/bpf_jit.h +++ b/arch/riscv/net/bpf_jit.h@@ -18,6 +18,11 @@ static inline bool rvc_enabled(void) return IS_ENABLED(CONFIG_RISCV_ISA_C); } +static inline bool rvzbb_enabled(void) +{ + return IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && riscv_has_extension_likely(RISCV_ISA_EXT_ZBB);Hmm, I'm thinking about the IS_ENABLED(CONFIG_RISCV_ISA_ZBB) semantics for a kernel JIT compiler. IS_ENABLED(CONFIG_RISCV_ISA_ZBB) affects the kernel compiler flags. Should it be enough to just have the run-time check? Should a kernel built w/o Zbb be able to emit Zbb from the JIT?Not enough, because riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) is a platform capability check, and the other one is a kernel image capability check. We can pass the check riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) when CONFIG_RISCV_ISA_ZBB=n. And my local test prove it.What I'm trying to say (and drew as well in the other reply) is that "riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) when CONFIG_RISCV_ISA_ZBB=n" should also make the JIT emit Zbb insns. The platform check should be sufficient.Ooh, this is really beyond my expectation. The test_progs can pass when with only platform check and it can recognize the zbb instructions. Now I know it. Sorry for misleading.🙁 Curious if CONFIG_RISCV_ISA_ZBB is still necessary?
You don't need IS_ENABLED(CONFIG_RISCV_ISA_ZBB) for the JIT, but the kernel needs it. Feel free to follow up with a patch to remove it. Cheers, Björn