> -----Original Message----- > From: Andrew Jones <ajones@xxxxxxxxxxxxxxxx> > Sent: Tuesday, May 14, 2024 9:37 PM > To: Wang, Xiao W <xiao.w.wang@xxxxxxxxx> > Cc: paul.walmsley@xxxxxxxxxx; palmer@xxxxxxxxxxx; > aou@xxxxxxxxxxxxxxxxx; luke.r.nels@xxxxxxxxx; xi.wang@xxxxxxxxx; > bjorn@xxxxxxxxxx; ast@xxxxxxxxxx; daniel@xxxxxxxxxxxxx; andrii@xxxxxxxxxx; > martin.lau@xxxxxxxxx; eddyz87@xxxxxxxxx; song@xxxxxxxxxx; > yonghong.song@xxxxxxxxx; john.fastabend@xxxxxxxxx; kpsingh@xxxxxxxxxx; > sdf@xxxxxxxxxx; haoluo@xxxxxxxxxx; jolsa@xxxxxxxxxx; linux- > riscv@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; bpf@xxxxxxxxxxxxxxx; > pulehui@xxxxxxxxxx; Li, Haicheng <haicheng.li@xxxxxxxxx>; > conor@xxxxxxxxxx; Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx> > Subject: Re: [PATCH v2] riscv, bpf: Optimize zextw insn with Zba extension > > On Tue, May 14, 2024 at 07:36:04AM GMT, Wang, Xiao W wrote: > > > > > > > -----Original Message----- > > > From: Andrew Jones <ajones@xxxxxxxxxxxxxxxx> > > > Sent: Tuesday, May 14, 2024 12:53 AM > > > To: Wang, Xiao W <xiao.w.wang@xxxxxxxxx> > > > Cc: paul.walmsley@xxxxxxxxxx; palmer@xxxxxxxxxxx; > > > aou@xxxxxxxxxxxxxxxxx; luke.r.nels@xxxxxxxxx; xi.wang@xxxxxxxxx; > > > bjorn@xxxxxxxxxx; ast@xxxxxxxxxx; daniel@xxxxxxxxxxxxx; > andrii@xxxxxxxxxx; > > > martin.lau@xxxxxxxxx; eddyz87@xxxxxxxxx; song@xxxxxxxxxx; > > > yonghong.song@xxxxxxxxx; john.fastabend@xxxxxxxxx; > kpsingh@xxxxxxxxxx; > > > sdf@xxxxxxxxxx; haoluo@xxxxxxxxxx; jolsa@xxxxxxxxxx; linux- > > > riscv@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > bpf@xxxxxxxxxxxxxxx; > > > pulehui@xxxxxxxxxx; Li, Haicheng <haicheng.li@xxxxxxxxx>; > > > conor@xxxxxxxxxx > > > Subject: Re: [PATCH v2] riscv, bpf: Optimize zextw insn with Zba extension > > > > > > On Sat, May 11, 2024 at 10:34:36AM GMT, Xiao Wang wrote: > > > > The Zba extension provides add.uw insn which can be used to implement > > > > zext.w with rs2 set as ZERO. > > > > > > > > Signed-off-by: Xiao Wang <xiao.w.wang@xxxxxxxxx> > > > > --- > > > > v2: > > > > * Add Zba description in the Kconfig. (Lehui) > > > > * Reword the Kconfig help message to make it clearer. (Conor) > > > > --- > > > > arch/riscv/Kconfig | 22 ++++++++++++++++++++++ > > > > arch/riscv/net/bpf_jit.h | 18 ++++++++++++++++++ > > > > 2 files changed, 40 insertions(+) > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > index 6bec1bce6586..e262a8668b41 100644 > > > > --- a/arch/riscv/Kconfig > > > > +++ b/arch/riscv/Kconfig > > > > @@ -586,6 +586,14 @@ config RISCV_ISA_V_PREEMPTIVE > > > > preemption. Enabling this config will result in higher memory > > > > consumption due to the allocation of per-task's kernel Vector > > > context. > > > > > > > > +config TOOLCHAIN_HAS_ZBA > > > > + bool > > > > + default y > > > > + depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zba) > > > > + depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zba) > > > > + depends on LLD_VERSION >= 150000 || LD_VERSION >= 23900 > > > > + depends on AS_HAS_OPTION_ARCH > > > > + > > > > config TOOLCHAIN_HAS_ZBB > > > > bool > > > > default y > > > > @@ -601,6 +609,20 @@ config TOOLCHAIN_HAS_VECTOR_CRYPTO > > > > def_bool $(as-instr, .option arch$(comma) +v$(comma) +zvkb) > > > > depends on AS_HAS_OPTION_ARCH > > > > > > > > +config RISCV_ISA_ZBA > > > > + bool "Zba extension support for bit manipulation instructions" > > > > + depends on TOOLCHAIN_HAS_ZBA > > > > > > We handcraft the instruction, so why do we need toolchain support? > > > > Good point, we don't need toolchain support for this bpf jit case. > > > > > > > > > + depends on RISCV_ALTERNATIVE > > > > > > Also, while riscv_has_extension_likely() will be accelerated with > > > RISCV_ALTERNATIVE, it's not required. > > > > Agree, it's not required. For this bpf jit case, we should drop these two > dependencies. > > > > BTW, Zbb is used in bpf jit, the usage there also doesn't depend on toolchain > and > > RISCV_ALTERNATIVE, but the Kconfig for RISCV_ISA_ZBB has forced the > dependencies > > due to Zbb assembly programming elsewhere. > > Maybe we could just dynamically check the existence of RISCV_ISA_ZB* > before jit code > > emission? or introduce new config options for bpf jit? I prefer the first > method and > > welcome any comments. > > My preferences is to remove as much of the TOOLCHAIN_HAS_ stuff as > possible. We should audit the extensions which have them to see if > they're really necessary. I don't mind depending on RISCV_ALTERNATIVE, > since it's almost required for riscv at this point anyway. I go through all the existing TOOLCHAIN_HAS_* stuff, all of them are helpful for compiling the corresponding assembly code. So they're really necessary. For this patch, I would drop the two dependencies for RISCV_ISA_ZBA Kconfig, as the jit doesn't depend on them. BRs, Xiao > > Thanks, > drew > > > > > Thanks, > > Xiao > > > > [...] > > > > { > > > > + if (rvzba_enabled()) { > > > > + emit(rvzba_zextw(rd, rs), ctx); > > > > + return; > > > > + } > > > > + > > > > emit_slli(rd, rs, 32, ctx); > > > > emit_srli(rd, rd, 32, ctx); > > > > } > > > > -- > > > > 2.25.1 > > > > > > > > > > Thanks, > > > drew