[PATCH v2] riscv, bpf: Optimize zextw insn with Zba extension

[PATCH v2] riscv, bpf: Optimize zextw insn with Zba extension

[Xiao Wang]

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@intel.com>
---
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
+	depends on RISCV_ALTERNATIVE
+	default y
+	help
+	   Add support for enabling optimisations in the kernel when the Zba
+	   extension is detected at boot.
+
+	   The Zba extension provides instructions to accelerate the generation
+	   of addresses that index into arrays of basic data types.
+
+	   If you don't know what to do here, say Y.
+
 config RISCV_ISA_ZBB
 	bool "Zbb extension support for bit manipulation instructions"
 	depends on TOOLCHAIN_HAS_ZBB
diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
index f4b6b3b9edda..18a7885ba95e 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 rvzba_enabled(void)
+{
+	return IS_ENABLED(CONFIG_RISCV_ISA_ZBA) && riscv_has_extension_likely(RISCV_ISA_EXT_ZBA);
+}
+
 static inline bool rvzbb_enabled(void)
 {
 	return IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && riscv_has_extension_likely(RISCV_ISA_EXT_ZBB);
@@ -937,6 +942,14 @@ static inline u16 rvc_sdsp(u32 imm9, u8 rs2)
 	return rv_css_insn(0x7, imm, rs2, 0x2);
 }
 
+/* RV64-only ZBA instructions. */
+
+static inline u32 rvzba_zextw(u8 rd, u8 rs1)
+{
+	/* add.uw rd, rs1, ZERO */
+	return rv_r_insn(0x04, RV_REG_ZERO, rs1, 0, rd, 0x3b);
+}
+
 #endif /* __riscv_xlen == 64 */
 
 /* Helper functions that emit RVC instructions when possible. */
@@ -1159,6 +1172,11 @@ static inline void emit_zexth(u8 rd, u8 rs, struct rv_jit_context *ctx)
 
 static inline void emit_zextw(u8 rd, u8 rs, struct rv_jit_context *ctx)
 {
+	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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] riscv, bpf: Optimize zextw insn with Zba extension

[Andrew Jones]

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@intel.com>
> ---
> 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?

> +	depends on RISCV_ALTERNATIVE

Also, while riscv_has_extension_likely() will be accelerated with
RISCV_ALTERNATIVE, it's not required.

> +	default y
> +	help
> +	   Add support for enabling optimisations in the kernel when the Zba
> +	   extension is detected at boot.
> +
> +	   The Zba extension provides instructions to accelerate the generation
> +	   of addresses that index into arrays of basic data types.
> +
> +	   If you don't know what to do here, say Y.
> +
>  config RISCV_ISA_ZBB
>  	bool "Zbb extension support for bit manipulation instructions"
>  	depends on TOOLCHAIN_HAS_ZBB
> diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
> index f4b6b3b9edda..18a7885ba95e 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 rvzba_enabled(void)
> +{
> +	return IS_ENABLED(CONFIG_RISCV_ISA_ZBA) && riscv_has_extension_likely(RISCV_ISA_EXT_ZBA);
> +}
> +
>  static inline bool rvzbb_enabled(void)
>  {
>  	return IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && riscv_has_extension_likely(RISCV_ISA_EXT_ZBB);
> @@ -937,6 +942,14 @@ static inline u16 rvc_sdsp(u32 imm9, u8 rs2)
>  	return rv_css_insn(0x7, imm, rs2, 0x2);
>  }
>  
> +/* RV64-only ZBA instructions. */
> +
> +static inline u32 rvzba_zextw(u8 rd, u8 rs1)
> +{
> +	/* add.uw rd, rs1, ZERO */
> +	return rv_r_insn(0x04, RV_REG_ZERO, rs1, 0, rd, 0x3b);
> +}
> +
>  #endif /* __riscv_xlen == 64 */
>  
>  /* Helper functions that emit RVC instructions when possible. */
> @@ -1159,6 +1172,11 @@ static inline void emit_zexth(u8 rd, u8 rs, struct rv_jit_context *ctx)
>  
>  static inline void emit_zextw(u8 rd, u8 rs, struct rv_jit_context *ctx)
>  {
> +	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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

RE: [PATCH v2] riscv, bpf: Optimize zextw insn with Zba extension

[Wang, Xiao W]

> -----Original Message-----
> From: Andrew Jones <ajones@ventanamicro.com>
> Sent: Tuesday, May 14, 2024 12:53 AM
> To: Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: paul.walmsley@sifive.com; palmer@dabbelt.com;
> aou@eecs.berkeley.edu; luke.r.nels@gmail.com; xi.wang@gmail.com;
> bjorn@kernel.org; ast@kernel.org; daniel@iogearbox.net; andrii@kernel.org;
> martin.lau@linux.dev; eddyz87@gmail.com; song@kernel.org;
> yonghong.song@linux.dev; john.fastabend@gmail.com; kpsingh@kernel.org;
> sdf@google.com; haoluo@google.com; jolsa@kernel.org; linux-
> riscv@lists.infradead.org; linux-kernel@vger.kernel.org; bpf@vger.kernel.org;
> pulehui@huawei.com; Li, Haicheng <haicheng.li@intel.com>;
> conor@kernel.org
> 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@intel.com>
> > ---
> > 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.

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] riscv, bpf: Optimize zextw insn with Zba extension

[Andrew Jones]

On Tue, May 14, 2024 at 07:36:04AM GMT, Wang, Xiao W wrote:
> 
> 
> > -----Original Message-----
> > From: Andrew Jones <ajones@ventanamicro.com>
> > Sent: Tuesday, May 14, 2024 12:53 AM
> > To: Wang, Xiao W <xiao.w.wang@intel.com>
> > Cc: paul.walmsley@sifive.com; palmer@dabbelt.com;
> > aou@eecs.berkeley.edu; luke.r.nels@gmail.com; xi.wang@gmail.com;
> > bjorn@kernel.org; ast@kernel.org; daniel@iogearbox.net; andrii@kernel.org;
> > martin.lau@linux.dev; eddyz87@gmail.com; song@kernel.org;
> > yonghong.song@linux.dev; john.fastabend@gmail.com; kpsingh@kernel.org;
> > sdf@google.com; haoluo@google.com; jolsa@kernel.org; linux-
> > riscv@lists.infradead.org; linux-kernel@vger.kernel.org; bpf@vger.kernel.org;
> > pulehui@huawei.com; Li, Haicheng <haicheng.li@intel.com>;
> > conor@kernel.org
> > 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@intel.com>
> > > ---
> > > 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.

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

RE: [PATCH v2] riscv, bpf: Optimize zextw insn with Zba extension

[Wang, Xiao W]

> -----Original Message-----
> From: Andrew Jones <ajones@ventanamicro.com>
> Sent: Tuesday, May 14, 2024 9:37 PM
> To: Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: paul.walmsley@sifive.com; palmer@dabbelt.com;
> aou@eecs.berkeley.edu; luke.r.nels@gmail.com; xi.wang@gmail.com;
> bjorn@kernel.org; ast@kernel.org; daniel@iogearbox.net; andrii@kernel.org;
> martin.lau@linux.dev; eddyz87@gmail.com; song@kernel.org;
> yonghong.song@linux.dev; john.fastabend@gmail.com; kpsingh@kernel.org;
> sdf@google.com; haoluo@google.com; jolsa@kernel.org; linux-
> riscv@lists.infradead.org; linux-kernel@vger.kernel.org; bpf@vger.kernel.org;
> pulehui@huawei.com; Li, Haicheng <haicheng.li@intel.com>;
> conor@kernel.org; Ben Dooks <ben.dooks@codethink.co.uk>
> 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@ventanamicro.com>
> > > Sent: Tuesday, May 14, 2024 12:53 AM
> > > To: Wang, Xiao W <xiao.w.wang@intel.com>
> > > Cc: paul.walmsley@sifive.com; palmer@dabbelt.com;
> > > aou@eecs.berkeley.edu; luke.r.nels@gmail.com; xi.wang@gmail.com;
> > > bjorn@kernel.org; ast@kernel.org; daniel@iogearbox.net;
> andrii@kernel.org;
> > > martin.lau@linux.dev; eddyz87@gmail.com; song@kernel.org;
> > > yonghong.song@linux.dev; john.fastabend@gmail.com;
> kpsingh@kernel.org;
> > > sdf@google.com; haoluo@google.com; jolsa@kernel.org; linux-
> > > riscv@lists.infradead.org; linux-kernel@vger.kernel.org;
> bpf@vger.kernel.org;
> > > pulehui@huawei.com; Li, Haicheng <haicheng.li@intel.com>;
> > > conor@kernel.org
> > > 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@intel.com>
> > > > ---
> > > > 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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] riscv, bpf: Optimize zextw insn with Zba extension

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2136 bytes --]

On Tue, May 14, 2024 at 03:37:02PM +0200, Andrew Jones wrote:
> On Tue, May 14, 2024 at 07:36:04AM GMT, Wang, Xiao W wrote:
> > > From: Andrew Jones <ajones@ventanamicro.com>
>> > > > +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.

While I think it is reasonable to allow the "RISCV_ISA_ZBB" option to
control whether or not bpf is allowed to use it for optimisations, only
allowing bpf to do that if there's toolchain support feels odd to me..
Maybe we need to sorta steal from Charlie's patchset and introduce
some hidden options that have the toolchain dep that are used by the
alternative macros etc?

I'll have a poke at how bad that looks I think.

> I don't mind depending on RISCV_ALTERNATIVE,
> since it's almost required for riscv at this point anyway.

As you say, using on riscv_has_extension_likely() doesn't mean you
depend on alternatives so effectively all this does is rule out use
with XIP, since alternatives are selected when !XIP. Does BPF even work
for XIP?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] riscv, bpf: Optimize zextw insn with Zba extension

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2359 bytes --]

On Wed, May 15, 2024 at 09:19:46AM +0100, Conor Dooley wrote:
> On Tue, May 14, 2024 at 03:37:02PM +0200, Andrew Jones wrote:
> > On Tue, May 14, 2024 at 07:36:04AM GMT, Wang, Xiao W wrote:
> > > > From: Andrew Jones <ajones@ventanamicro.com>
> >> > > > +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.
> 
> While I think it is reasonable to allow the "RISCV_ISA_ZBB" option to
> control whether or not bpf is allowed to use it for optimisations, only
> allowing bpf to do that if there's toolchain support feels odd to me..
> Maybe we need to sorta steal from Charlie's patchset and introduce
> some hidden options that have the toolchain dep that are used by the
> alternative macros etc?
> 
> I'll have a poke at how bad that looks I think.

I don't love this, in particular my option naming, but it would allow
the Zbb optimisations in the kernel to not depend on toolchain support
while not muddying the Kconfig waters for users:
https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=riscv-zbb_split
A similar model could be followed if there were to be some
optimisations for Zba in the future that do require toolchain support:

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

RE: [PATCH v2] riscv, bpf: Optimize zextw insn with Zba extension

[Wang, Xiao W]

> -----Original Message-----
> From: Conor Dooley <conor.dooley@microchip.com>
> Sent: Wednesday, May 15, 2024 5:33 PM
> To: Andrew Jones <ajones@ventanamicro.com>
> Cc: Wang, Xiao W <xiao.w.wang@intel.com>; paul.walmsley@sifive.com;
> palmer@dabbelt.com; aou@eecs.berkeley.edu; luke.r.nels@gmail.com;
> xi.wang@gmail.com; bjorn@kernel.org; ast@kernel.org;
> daniel@iogearbox.net; andrii@kernel.org; martin.lau@linux.dev;
> eddyz87@gmail.com; song@kernel.org; yonghong.song@linux.dev;
> john.fastabend@gmail.com; kpsingh@kernel.org; sdf@google.com;
> haoluo@google.com; jolsa@kernel.org; linux-riscv@lists.infradead.org; linux-
> kernel@vger.kernel.org; bpf@vger.kernel.org; pulehui@huawei.com; Li,
> Haicheng <haicheng.li@intel.com>; conor@kernel.org; Ben Dooks
> <ben.dooks@codethink.co.uk>
> Subject: Re: [PATCH v2] riscv, bpf: Optimize zextw insn with Zba extension
> 
> On Wed, May 15, 2024 at 09:19:46AM +0100, Conor Dooley wrote:
> > On Tue, May 14, 2024 at 03:37:02PM +0200, Andrew Jones wrote:
> > > On Tue, May 14, 2024 at 07:36:04AM GMT, Wang, Xiao W wrote:
> > > > > From: Andrew Jones <ajones@ventanamicro.com>
> > >> > > > +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.
> >
> > While I think it is reasonable to allow the "RISCV_ISA_ZBB" option to
> > control whether or not bpf is allowed to use it for optimisations, only
> > allowing bpf to do that if there's toolchain support feels odd to me..
> > Maybe we need to sorta steal from Charlie's patchset and introduce
> > some hidden options that have the toolchain dep that are used by the
> > alternative macros etc?
> >
> > I'll have a poke at how bad that looks I think.
> 
> I don't love this, in particular my option naming, but it would allow
> the Zbb optimisations in the kernel to not depend on toolchain support
> while not muddying the Kconfig waters for users:
> https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=ri
> scv-zbb_split

In that patch, I think the bpt jit part should check IS_ENABLED(CONFIG_RISCV_ISA_ZBB)
rather than IS_ENABLED(CONFIG_RISCV_ISA_ZBB_ALT).

> A similar model could be followed if there were to be some
> optimisations for Zba in the future that do require toolchain support:

Though this model introduces extra hidden Kconfig option, it does provide finer 
config granularity. This should be a separate patch in the future, we can discuss about
the option naming there.

BRs,
Xiao


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] riscv, bpf: Optimize zextw insn with Zba extension

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1778 bytes --]

On Wed, May 15, 2024 at 11:31:43AM +0000, Wang, Xiao W wrote:
> > From: Conor Dooley <conor.dooley@microchip.com>
> > > > 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.
> > >
> > > While I think it is reasonable to allow the "RISCV_ISA_ZBB" option to
> > > control whether or not bpf is allowed to use it for optimisations, only
> > > allowing bpf to do that if there's toolchain support feels odd to me..
> > > Maybe we need to sorta steal from Charlie's patchset and introduce
> > > some hidden options that have the toolchain dep that are used by the
> > > alternative macros etc?
> > >
> > > I'll have a poke at how bad that looks I think.
> > 
> > I don't love this, in particular my option naming, but it would allow
> > the Zbb optimisations in the kernel to not depend on toolchain support
> > while not muddying the Kconfig waters for users:
> > https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=ri
> > scv-zbb_split
> 
> In that patch, I think the bpt jit part should check IS_ENABLED(CONFIG_RISCV_ISA_ZBB)
> rather than IS_ENABLED(CONFIG_RISCV_ISA_ZBB_ALT).

D'oh, you're right. The bpf code being different was meant to be the whole
point of the change...

> > A similar model could be followed if there were to be some
> > optimisations for Zba in the future that do require toolchain support:
> 
> Though this model introduces extra hidden Kconfig option, it does provide finer 
> config granularity. This should be a separate patch in the future, we can discuss about
> the option naming there.

Yeah, not expecting you to do this as part of this patch.

Thanks,
Conor.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv