Re: Re: [PATCH RESEND bpf-next v3 4/6] riscv, bpf: Add necessary Zbb instructions

[Andrew Jones <ajones@ventanamicro.com>]

On Sat, Jan 27, 2024 at 06:16:41PM +0100, Björn Töpel wrote:
> Pu Lehui <pulehui@huaweicloud.com> writes:
> 
> > 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?
>

My two cents (which might be worth less than two cents due to my lack of
BPF knowledge) is yes, the JIT should be allowed to emit Zbb instructions
even when the kernel is not built with a compiler which has done so. In
fact, we have insn-def.h for situations similar to this.

Thanks,
drew