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: <DM8PR11MB5751A2BB91C431DAE14F48C1B8EC2@DM8PR11MB5751.namprd11.prod.outlook.com>
Date: Wed, 15 May 2024 11:31:43 +0000
From: "Wang, Xiao W" <xiao.w.wang@...el.com>
To: Conor Dooley <conor.dooley@...rochip.com>, 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: Conor Dooley <conor.dooley@...rochip.com>
> Sent: Wednesday, May 15, 2024 5:33 PM
> To: Andrew Jones <ajones@...tanamicro.com>
> Cc: Wang, Xiao W <xiao.w.wang@...el.com>; 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 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@...tanamicro.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