Received-SPF: pass (google.com: domain of linux-kernel+bounces-178759-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Tue, 14 May 2024 15:37:02 +0200
From: Andrew Jones <ajones@ventanamicro.com>
To: "Wang, Xiao W" <xiao.w.wang@intel.com>
Cc: "paul.walmsley@sifive.com" <paul.walmsley@sifive.com>, 
	"palmer@dabbelt.com" <palmer@dabbelt.com>, "aou@eecs.berkeley.edu" <aou@eecs.berkeley.edu>, 
	"luke.r.nels@gmail.com" <luke.r.nels@gmail.com>, "xi.wang@gmail.com" <xi.wang@gmail.com>, 
	"bjorn@kernel.org" <bjorn@kernel.org>, "ast@kernel.org" <ast@kernel.org>, 
	"daniel@iogearbox.net" <daniel@iogearbox.net>, "andrii@kernel.org" <andrii@kernel.org>, 
	"martin.lau@linux.dev" <martin.lau@linux.dev>, "eddyz87@gmail.com" <eddyz87@gmail.com>, 
	"song@kernel.org" <song@kernel.org>, "yonghong.song@linux.dev" <yonghong.song@linux.dev>, 
	"john.fastabend@gmail.com" <john.fastabend@gmail.com>, "kpsingh@kernel.org" <kpsingh@kernel.org>, 
	"sdf@google.com" <sdf@google.com>, "haoluo@google.com" <haoluo@google.com>, 
	"jolsa@kernel.org" <jolsa@kernel.org>, 
	"linux-riscv@lists.infradead.org" <linux-riscv@lists.infradead.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, 
	"bpf@vger.kernel.org" <bpf@vger.kernel.org>, "pulehui@huawei.com" <pulehui@huawei.com>, 
	"Li, Haicheng" <haicheng.li@intel.com>, "conor@kernel.org" <conor@kernel.org>, 
	Ben Dooks <ben.dooks@codethink.co.uk>
Subject: Re: [PATCH v2] riscv, bpf: Optimize zextw insn with Zba extension
Message-ID: <20240514-944dec90b2c531d8b6c783f7@orel>
References: <20240511023436.3282285-1-xiao.w.wang@intel.com>
 <20240513-5c6f04fb4a29963c63d09aa2@orel>
 <DM8PR11MB575179A3EB8D056B3EEECA74B8E32@DM8PR11MB5751.namprd11.prod.outlook.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <DM8PR11MB575179A3EB8D056B3EEECA74B8E32@DM8PR11MB5751.namprd11.prod.outlook.com>

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