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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <DM8PR11MB5751626DA9EFDD7F2DB8D7B1B8EC2@DM8PR11MB5751.namprd11.prod.outlook.com>
Date: Wed, 15 May 2024 07:38:17 +0000
From: "Wang, Xiao W" <xiao.w.wang@...el.com>
To: Andrew Jones <ajones@...tanamicro.com>
CC: "paul.walmsley@...ive.com" <paul.walmsley@...ive.com>,
	"palmer@...belt.com" <palmer@...belt.com>, "aou@...s.berkeley.edu"
	<aou@...s.berkeley.edu>, "luke.r.nels@...il.com" <luke.r.nels@...il.com>,
	"xi.wang@...il.com" <xi.wang@...il.com>, "bjorn@...nel.org"
	<bjorn@...nel.org>, "ast@...nel.org" <ast@...nel.org>, "daniel@...earbox.net"
	<daniel@...earbox.net>, "andrii@...nel.org" <andrii@...nel.org>,
	"martin.lau@...ux.dev" <martin.lau@...ux.dev>, "eddyz87@...il.com"
	<eddyz87@...il.com>, "song@...nel.org" <song@...nel.org>,
	"yonghong.song@...ux.dev" <yonghong.song@...ux.dev>,
	"john.fastabend@...il.com" <john.fastabend@...il.com>, "kpsingh@...nel.org"
	<kpsingh@...nel.org>, "sdf@...gle.com" <sdf@...gle.com>, "haoluo@...gle.com"
	<haoluo@...gle.com>, "jolsa@...nel.org" <jolsa@...nel.org>,
	"linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"bpf@...r.kernel.org" <bpf@...r.kernel.org>, "pulehui@...wei.com"
	<pulehui@...wei.com>, "Li, Haicheng" <haicheng.li@...el.com>,
	"conor@...nel.org" <conor@...nel.org>, Ben Dooks <ben.dooks@...ethink.co.uk>
Subject: RE: [PATCH v2] riscv, bpf: Optimize zextw insn with Zba extension



> -----Original Message-----
> From: Andrew Jones <ajones@...tanamicro.com>
> Sent: Tuesday, May 14, 2024 9:37 PM
> To: Wang, Xiao W <xiao.w.wang@...el.com>
> Cc: paul.walmsley@...ive.com; palmer@...belt.com;
> aou@...s.berkeley.edu; luke.r.nels@...il.com; xi.wang@...il.com;
> bjorn@...nel.org; ast@...nel.org; daniel@...earbox.net; andrii@...nel.org;
> martin.lau@...ux.dev; eddyz87@...il.com; song@...nel.org;
> yonghong.song@...ux.dev; john.fastabend@...il.com; kpsingh@...nel.org;
> sdf@...gle.com; haoluo@...gle.com; jolsa@...nel.org; linux-
> riscv@...ts.infradead.org; linux-kernel@...r.kernel.org; bpf@...r.kernel.org;
> pulehui@...wei.com; Li, Haicheng <haicheng.li@...el.com>;
> conor@...nel.org; Ben Dooks <ben.dooks@...ethink.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@...tanamicro.com>
> > > Sent: Tuesday, May 14, 2024 12:53 AM
> > > To: Wang, Xiao W <xiao.w.wang@...el.com>
> > > Cc: paul.walmsley@...ive.com; palmer@...belt.com;
> > > aou@...s.berkeley.edu; luke.r.nels@...il.com; xi.wang@...il.com;
> > > bjorn@...nel.org; ast@...nel.org; daniel@...earbox.net;
> andrii@...nel.org;
> > > martin.lau@...ux.dev; eddyz87@...il.com; song@...nel.org;
> > > yonghong.song@...ux.dev; john.fastabend@...il.com;
> kpsingh@...nel.org;
> > > sdf@...gle.com; haoluo@...gle.com; jolsa@...nel.org; linux-
> > > riscv@...ts.infradead.org; linux-kernel@...r.kernel.org;
> bpf@...r.kernel.org;
> > > pulehui@...wei.com; Li, Haicheng <haicheng.li@...el.com>;
> > > conor@...nel.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@...el.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