Received-SPF: pass (google.com: domain of linux-kernel+bounces-193282-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: "Wang, Xiao W" <xiao.w.wang@intel.com>
To: Conor Dooley <conor@kernel.org>, "linux-riscv@lists.infradead.org"
	<linux-riscv@lists.infradead.org>
CC: Conor Dooley <conor.dooley@microchip.com>, Andrew Jones
	<ajones@ventanamicro.com>, "pulehui@huawei.com" <pulehui@huawei.com>,
	"Charlie Jenkins" <charlie@rivosinc.com>, Paul Walmsley
	<paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Samuel Holland
	<samuel.holland@sifive.com>, Pu Lehui <pulehui@huaweicloud.com>,
	=?Windows-1252?Q?Bj=F6rn_T=F6pel?= <bjorn@kernel.org>
Subject: RE: [PATCH v3 2/2] RISC-V: separate Zbb optimisations requiring and
 not requiring toolchain support
Thread-Topic: [PATCH v3 2/2] RISC-V: separate Zbb optimisations requiring and
 not requiring toolchain support
Thread-Index: AQHasO/Q81BwKHiJLU+eTWZMI9PyE7GtY8Bg
Date: Wed, 29 May 2024 01:01:24 +0000
Message-ID: <DM8PR11MB5751DBB6A068008269E7D399B8F22@DM8PR11MB5751.namprd11.prod.outlook.com>
References: <20240528-applaud-violin-facef8d9d846@spud>
 <20240528-opossum-flavorful-3411811b87e2@spud>
In-Reply-To: <20240528-opossum-flavorful-3411811b87e2@spud>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?Windows-1252?Q?CDAa7VvhdV0KZhpM+wBjhst5USD+/JyJI6IQB/66BAFLSg7pscNKah+1?=
 =?Windows-1252?Q?B7aB73I9k8zifKANQOAFE4p/HYkU/fnz7lXTTIczXQATtum0hSxo/Xdb?=
 =?Windows-1252?Q?1I+pDY+whg43KNDX9sEo7PP24nzY4TqbxeOMjeYckpUc5VbvAcizoPA/?=
 =?Windows-1252?Q?N6ZjRC7loVyBwQYq2xgaZsaIGaj82IJNcigLlDaXUjgDQdvWqW/XdwUp?=
 =?Windows-1252?Q?X1CyNuHfTVfka+RwKga3+B0UB41Cik+m+Dx0b9n8F2De7oW2lXhF1HQo?=
 =?Windows-1252?Q?xBw00+l08N7WoUdRqrPLS32m4lNiUt5MBH6TcHuv0M7xxKE4zAzINzu5?=
 =?Windows-1252?Q?61CFqJd98em3i0XeJv0m/LxGSZeQBkWZXUNg9DgFL3JKdd1gnmrYXm7+?=
 =?Windows-1252?Q?nYTYUwSDEd3jUrs16EjkLtZ3DwL6aA9yVxg6MicKRh10mpIb5nF95CWE?=
 =?Windows-1252?Q?i2MVZD+CkHl7A0lxWIrfpw7Ofn5PblI0nK8QR9Dmze/fCezoUiB/sJmv?=
 =?Windows-1252?Q?FH9rvPakxcdm5cD5wWpncC9qVPzmPDxswOP37lsvTL8Nnjv83qKcJcLA?=
 =?Windows-1252?Q?ninK5qPJzgUNPmgyzQeLBpiIQCDHXL1mAtYF+DYxR89F/GjZTO8Tu3iq?=
 =?Windows-1252?Q?SrmRJmRb9+GawLCK0k+R61QTDNvXoBPEadgqSla1pbPQsL4/KHFid+GZ?=
 =?Windows-1252?Q?MaJxtjLAa9SGmwLMj45iY8jbWPVs/QEoeDPru96WMLDDQLn3MaSGJzTV?=
 =?Windows-1252?Q?ZYTQEUmypf9CW4JFn8ko03voU03LWIHy6DPl6FtMzp/HbZBbJrq5Xet5?=
 =?Windows-1252?Q?F1gdXrPqLFvjJq45ESmDsaAnCS1BubwicpcoLOPkDXYsEhRMiCZ94WMV?=
 =?Windows-1252?Q?ChWk6rI6p38fybfnU+l3C3jhCFAIzGTw66uT4oGl4PT/eg3P4AAjwVnn?=
 =?Windows-1252?Q?TSSL1WR2ifozzUYiYknwUQenU1PdcxSgkEs946TsWv8MER7gk1gJMOya?=
 =?Windows-1252?Q?nUyTWhdCduzAFT7JdNcLI9kXAYy05+opJjZHS2vf1x9+isLRz7IV4P5Q?=
 =?Windows-1252?Q?TBcO86Vy1DaGZ2pxjv6xkwGIzugVOrskqMGYR0BzPss4deG0tZn5T7yo?=
 =?Windows-1252?Q?BBY34csyyuYhYAA13o71XSz/aIcrSo/NTPhEHFY54lo2Y7Fj/jFV4HZi?=
 =?Windows-1252?Q?urlbr/RotRS5JiB5Qbz7ueun9iFeZuDwKt2kbNcGnMdBCe3LAgxHLqDl?=
 =?Windows-1252?Q?+ax6oyvK2pPjYu7MR6kR9TOuFcNI2kyD1lwYoaeSv9L1Y8yk2EW8XPdh?=
 =?Windows-1252?Q?q5+ARlPmmHg+pN2PxTiOriHG3mCQm2CrGqg6zgCSLCw+Dq6ZyNW3UoXD?=
 =?Windows-1252?Q?BmqZ/mIIYqWWfaRcP7hu5vgmhEaYAzvTQay42PaoEec1OI4c1DEl7hFk?=
 =?Windows-1252?Q?YpJ8QEHWM+PJJO9l/KSHOm3EWoxX9/aimyVNj4gNptA1gMJ1pJvUQvUv?=
 =?Windows-1252?Q?4QA2BOuwxtZFuYzt2KT24UOiULgcWYQzfL0FH9LRT2kt7u/MaHziHZ0K?=
 =?Windows-1252?Q?33nEpX7xHswNYEOvcLseX9LJnrjQAfVwm3sSiH43BO+yZxfyJ+tC99+o?=
 =?Windows-1252?Q?yiZHBCIfTJBZqLr0PiJnYEFKfSPxUWMxGzFjBdV3hGWJlLaqsLU3thrO?=
 =?Windows-1252?Q?nU0C1+tY3yHnQDRqRH0oujGY7IiS5oDX?=
Content-Type: text/plain; charset="Windows-1252"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM8PR11MB5751.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 44809241-5633-43a6-7234-08dc7f7adcaa
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 May 2024 01:01:24.4242
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: hsrtcvzV5poAcEQ2BMifw5W31k1VBW0Az3nSxVYvWKEMvCwSJ5VBcVYThmTg5GS7kFjYStCkXGa6k3EhVtfXBg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6955
X-OriginatorOrg: intel.com


> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: Tuesday, May 28, 2024 7:11 PM
> To: linux-riscv@lists.infradead.org
> Cc: conor@kernel.org; Conor Dooley <conor.dooley@microchip.com>; Wang,
> Xiao W <xiao.w.wang@intel.com>; Andrew Jones
> <ajones@ventanamicro.com>; pulehui@huawei.com; Charlie Jenkins
> <charlie@rivosinc.com>; Paul Walmsley <paul.walmsley@sifive.com>; Palmer
> Dabbelt <palmer@dabbelt.com>; linux-kernel@vger.kernel.org; Samuel
> Holland <samuel.holland@sifive.com>; Pu Lehui
> <pulehui@huaweicloud.com>; Bj=F6rn T=F6pel <bjorn@kernel.org>
> Subject: [PATCH v3 2/2] RISC-V: separate Zbb optimisations requiring and =
not
> requiring toolchain support
>=20
> From: Conor Dooley <conor.dooley@microchip.com>
>=20
> It seems a bit ridiculous to require toolchain support for BPF to
> assemble Zbb instructions, so move the dependency on toolchain support
> for Zbb optimisations out of the Kconfig option and to the callsites.
>=20
> Zbb support has always depended on alternatives, so while adjusting the
> config options guarding optimisations, remove any checks for
> whether or not alternatives are enabled.
>=20
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> v2/v3:
> - Per Drew's suggestion, drop the stub Kconfig option and instead push
>   out the toolchain dependency to the relevant callsites.
> - Delete a bunch of comments about only attempting Zbb if alternatives
>   are available, since they always are.
> ---
>  arch/riscv/Kconfig                    |  4 ++--
>  arch/riscv/include/asm/arch_hweight.h |  6 +++---
>  arch/riscv/include/asm/bitops.h       |  4 ++--
>  arch/riscv/include/asm/checksum.h     |  3 +--
>  arch/riscv/lib/csum.c                 | 21 +++------------------
>  arch/riscv/lib/strcmp.S               |  5 +++--
>  arch/riscv/lib/strlen.S               |  5 +++--
>  arch/riscv/lib/strncmp.S              |  5 +++--
>  8 files changed, 20 insertions(+), 33 deletions(-)
>=20
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 3b702e6cc051..a91c53b096e8 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -613,12 +613,12 @@ config TOOLCHAIN_HAS_VECTOR_CRYPTO
>=20
>  config RISCV_ISA_ZBB
>  	bool "Zbb extension support for bit manipulation instructions"
> -	depends on TOOLCHAIN_HAS_ZBB
>  	depends on RISCV_ALTERNATIVE
>  	default y
>  	help
>  	   Add support for enabling optimisations in the kernel when the
> -	   Zbb extension is detected at boot.
> +	   Zbb extension is detected at boot. Some optimisations may
> +	   additionally depend on toolchain support for Zbb.
>=20
>  	   The Zbb extension provides instructions to accelerate a number
>  	   of bit-specific operations (count bit population, sign extending,
> diff --git a/arch/riscv/include/asm/arch_hweight.h
> b/arch/riscv/include/asm/arch_hweight.h
> index 85b2c443823e..b94db541901a 100644
> --- a/arch/riscv/include/asm/arch_hweight.h
> +++ b/arch/riscv/include/asm/arch_hweight.h
> @@ -19,7 +19,7 @@
>=20
>  static __always_inline unsigned int __arch_hweight32(unsigned int w)
>  {
> -#ifdef CONFIG_RISCV_ISA_ZBB
> +#if defined(CONFIG_RISCV_ISA_ZBB) &&
> defined(CONFIG_TOOLCHAIN_HAS_ZBB)
>  	asm goto(ALTERNATIVE("j %l[legacy]", "nop", 0,
>  				      RISCV_ISA_EXT_ZBB, 1)
>  			  : : : : legacy);
> @@ -50,7 +50,7 @@ static inline unsigned int __arch_hweight8(unsigned int
> w)
>  #if BITS_PER_LONG =3D=3D 64
>  static __always_inline unsigned long __arch_hweight64(__u64 w)
>  {
> -# ifdef CONFIG_RISCV_ISA_ZBB
> +#if defined(CONFIG_RISCV_ISA_ZBB) &&
> defined(CONFIG_TOOLCHAIN_HAS_ZBB)
>  	asm goto(ALTERNATIVE("j %l[legacy]", "nop", 0,
>  				      RISCV_ISA_EXT_ZBB, 1)
>  			  : : : : legacy);
> @@ -64,7 +64,7 @@ static __always_inline unsigned long
> __arch_hweight64(__u64 w)
>  	return w;
>=20
>  legacy:
> -# endif
> +#endif
>  	return __sw_hweight64(w);
>  }
>  #else /* BITS_PER_LONG =3D=3D 64 */
> diff --git a/arch/riscv/include/asm/bitops.h
> b/arch/riscv/include/asm/bitops.h
> index 880606b0469a..6966d00c3a8a 100644
> --- a/arch/riscv/include/asm/bitops.h
> +++ b/arch/riscv/include/asm/bitops.h
> @@ -15,7 +15,7 @@
>  #include <asm/barrier.h>
>  #include <asm/bitsperlong.h>
>=20
> -#if !defined(CONFIG_RISCV_ISA_ZBB) || defined(NO_ALTERNATIVE)
> +#if !(defined(CONFIG_RISCV_ISA_ZBB) &&
> defined(CONFIG_TOOLCHAIN_HAS_ZBB)) || defined(NO_ALTERNATIVE)
>  #include <asm-generic/bitops/__ffs.h>
>  #include <asm-generic/bitops/__fls.h>
>  #include <asm-generic/bitops/ffs.h>
> @@ -175,7 +175,7 @@ static __always_inline int variable_fls(unsigned int =
x)
>  	 variable_fls(x_);					\
>  })
>=20
> -#endif /* !defined(CONFIG_RISCV_ISA_ZBB) || defined(NO_ALTERNATIVE)
> */
> +#endif /* !(defined(CONFIG_RISCV_ISA_ZBB) &&
> defined(CONFIG_TOOLCHAIN_HAS_ZBB)) || defined(NO_ALTERNATIVE) */
>=20
>  #include <asm-generic/bitops/ffz.h>
>  #include <asm-generic/bitops/fls64.h>
> diff --git a/arch/riscv/include/asm/checksum.h
> b/arch/riscv/include/asm/checksum.h
> index 88e6f1499e88..da378856f1d5 100644
> --- a/arch/riscv/include/asm/checksum.h
> +++ b/arch/riscv/include/asm/checksum.h
> @@ -49,8 +49,7 @@ static inline __sum16 ip_fast_csum(const void *iph,
> unsigned int ihl)
>  	 * ZBB only saves three instructions on 32-bit and five on 64-bit so no=
t
>  	 * worth checking if supported without Alternatives.
>  	 */
> -	if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) &&
> -	    IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) {
> +	if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) &&
> IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB)) {
>  		unsigned long fold_temp;
>=20
>  		asm goto(ALTERNATIVE("j %l[no_zbb]", "nop", 0,
> diff --git a/arch/riscv/lib/csum.c b/arch/riscv/lib/csum.c
> index 7fb12c59e571..9408f50ca59a 100644
> --- a/arch/riscv/lib/csum.c
> +++ b/arch/riscv/lib/csum.c
> @@ -40,12 +40,7 @@ __sum16 csum_ipv6_magic(const struct in6_addr
> *saddr,
>  	uproto =3D (__force unsigned int)htonl(proto);
>  	sum +=3D uproto;
>=20
> -	/*
> -	 * Zbb support saves 4 instructions, so not worth checking without
> -	 * alternatives if supported
> -	 */
> -	if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) &&
> -	    IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) {
> +	if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) &&
> IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB)) {
>  		unsigned long fold_temp;
>=20
>  		/*
> @@ -157,12 +152,7 @@ do_csum_with_alignment(const unsigned char
> *buff, int len)
>  	csum =3D do_csum_common(ptr, end, data);
>=20
>  #ifdef CC_HAS_ASM_GOTO_TIED_OUTPUT
> -	/*
> -	 * Zbb support saves 6 instructions, so not worth checking without
> -	 * alternatives if supported
> -	 */
> -	if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) &&
> -	    IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) {
> +	if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) &&
> IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB)) {
>  		unsigned long fold_temp;
>=20
>  		/*
> @@ -244,12 +234,7 @@ do_csum_no_alignment(const unsigned char *buff,
> int len)
>  	end =3D (const unsigned long *)(buff + len);
>  	csum =3D do_csum_common(ptr, end, data);
>=20
> -	/*
> -	 * Zbb support saves 6 instructions, so not worth checking without
> -	 * alternatives if supported
> -	 */
> -	if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) &&
> -	    IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) {
> +	if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) &&
> IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB)) {
>  		unsigned long fold_temp;
>=20
>  		/*
> diff --git a/arch/riscv/lib/strcmp.S b/arch/riscv/lib/strcmp.S
> index 687b2bea5c43..204fb1c184f3 100644
> --- a/arch/riscv/lib/strcmp.S
> +++ b/arch/riscv/lib/strcmp.S
> @@ -8,7 +8,8 @@
>  /* int strcmp(const char *cs, const char *ct) */
>  SYM_FUNC_START(strcmp)
>=20
> -	ALTERNATIVE("nop", "j strcmp_zbb", 0, RISCV_ISA_EXT_ZBB,
> CONFIG_RISCV_ISA_ZBB)
> +	__ALTERNATIVE_CFG("nop", "j strcmp_zbb", 0, RISCV_ISA_EXT_ZBB,
> +		IS_ENABLED(CONFIG_RISCV_ISA_ZBB_ALT) &&
> IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB))

s/CONFIG_RISCV_ISA_ZBB_ALT/ CONFIG_RISCV_ISA_ZBB
Same issue for below changes in strn*.S

BRs,
Xiao

>=20
>  	/*
>  	 * Returns
> @@ -43,7 +44,7 @@ SYM_FUNC_START(strcmp)
>   * The code was published as part of the bitmanip manual
>   * in Appendix A.
>   */
> -#ifdef CONFIG_RISCV_ISA_ZBB
> +#if defined(CONFIG_RISCV_ISA_ZBB) &&
> defined(CONFIG_TOOLCHAIN_HAS_ZBB)
>  strcmp_zbb:
>=20
>  .option push
> diff --git a/arch/riscv/lib/strlen.S b/arch/riscv/lib/strlen.S
> index 8ae3064e45ff..84909807d988 100644
> --- a/arch/riscv/lib/strlen.S
> +++ b/arch/riscv/lib/strlen.S
> @@ -8,7 +8,8 @@
>  /* int strlen(const char *s) */
>  SYM_FUNC_START(strlen)
>=20
> -	ALTERNATIVE("nop", "j strlen_zbb", 0, RISCV_ISA_EXT_ZBB,
> CONFIG_RISCV_ISA_ZBB)
> +	__ALTERNATIVE_CFG("nop", "j strlen_zbb", 0, RISCV_ISA_EXT_ZBB,
> +		IS_ENABLED(CONFIG_RISCV_ISA_ZBB_ALT) &&
> IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB))
>=20
>  	/*
>  	 * Returns
> @@ -33,7 +34,7 @@ SYM_FUNC_START(strlen)
>  /*
>   * Variant of strlen using the ZBB extension if available
>   */
> -#ifdef CONFIG_RISCV_ISA_ZBB
> +#if defined(CONFIG_RISCV_ISA_ZBB) &&
> defined(CONFIG_TOOLCHAIN_HAS_ZBB)
>  strlen_zbb:
>=20
>  #ifdef CONFIG_CPU_BIG_ENDIAN
> diff --git a/arch/riscv/lib/strncmp.S b/arch/riscv/lib/strncmp.S
> index aba5b3148621..87e7c83c1672 100644
> --- a/arch/riscv/lib/strncmp.S
> +++ b/arch/riscv/lib/strncmp.S
> @@ -8,7 +8,8 @@
>  /* int strncmp(const char *cs, const char *ct, size_t count) */
>  SYM_FUNC_START(strncmp)
>=20
> -	ALTERNATIVE("nop", "j strncmp_zbb", 0, RISCV_ISA_EXT_ZBB,
> CONFIG_RISCV_ISA_ZBB)
> +	__ALTERNATIVE_CFG("nop", "j strncmp_zbb", 0, RISCV_ISA_EXT_ZBB,
> +		IS_ENABLED(CONFIG_RISCV_ISA_ZBB_ALT) &&
> IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB))
>=20
>  	/*
>  	 * Returns
> @@ -46,7 +47,7 @@ SYM_FUNC_START(strncmp)
>  /*
>   * Variant of strncmp using the ZBB extension if available
>   */
> -#ifdef CONFIG_RISCV_ISA_ZBB
> +#if defined(CONFIG_RISCV_ISA_ZBB) &&
> defined(CONFIG_TOOLCHAIN_HAS_ZBB)
>  strncmp_zbb:
>=20
>  .option push
> --
> 2.43.0