From: Conor Dooley <[email protected]>
Since one depends on the other, albeit trivially, here's a v2 of the Zbb
toolchain dep removal alongside the rewording of Kconfig options I'd
sent out before the merge window. I think I like this implementation
better than v1, but I couldn't think of a good name for a "public"
version of __ALTERNATIVE(), so I used it here directly.
Unfortunately "ALTERNATIVE_2_CFG" already exists and I couldn't think of
a good way to name an alternative macro that allows for several config
options that didn't make the distinction sufficiently clear.. Yell
if you have better suggestions than I did.
I am a wee bit "worried" that this makes the Kconfig option confusing as
it isn't immediately obvious if someone is or is not going to get the
toolchain based optimisations.
Cheers,
Conor.
CC: [email protected]
CC: Andrew Jones <[email protected]>
CC: [email protected]
CC: Charlie Jenkins <[email protected]>
CC: Paul Walmsley <[email protected]>
CC: Palmer Dabbelt <[email protected]>
CC: Conor Dooley <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Samuel Holland <[email protected]>
CC: Pu Lehui <[email protected]>
CC: Björn Töpel <[email protected]>
CC: Andrew Jones <[email protected]>
CC: Paul Walmsley <[email protected]>
CC: Palmer Dabbelt <[email protected]>
CC: [email protected]
Conor Dooley (2):
RISC-V: clarify what some RISCV_ISA* config options do
RISC-V: separate Zbb optimisations requiring and not requiring
toolchain support
arch/riscv/Kconfig | 38 ++++++++++++++-------------
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, 38 insertions(+), 49 deletions(-)
--
2.43.0
From: Conor Dooley <[email protected]>
During some discussion on IRC yesterday and on Pu's bpf patch [1]
I noticed that these RISCV_ISA* Kconfig options are not really clear
about their implications. Many of these options have no impact on what
userspace is allowed to do, for example an application can use Zbb
regardless of whether or not the kernel does. Change the help text to
try and clarify whether or not an option affects just the kernel, or
also userspace. None of these options actually control whether or not an
extension is detected dynamically as that's done regardless of Kconfig
options, so drop any text that implies the option is required for
dynamic detection, rewording them as "do x when y is detected".
Link: https://lore.kernel.org/linux-riscv/20240328-ferocity-repose-c554f75a676c@spud/ [1]
Reviewed-by: Andrew Jones <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
arch/riscv/Kconfig | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index b94176e25be1..3b702e6cc051 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -501,7 +501,8 @@ config RISCV_ISA_C
help
Adds "C" to the ISA subsets that the toolchain is allowed to emit
when building Linux, which results in compressed instructions in the
- Linux binary.
+ Linux binary. This option produces a kernel that will not run on
+ systems that do not support compressed instructions.
If you don't know what to do here, say Y.
@@ -511,8 +512,8 @@ config RISCV_ISA_SVNAPOT
depends on RISCV_ALTERNATIVE
default y
help
- Allow kernel to detect the Svnapot ISA-extension dynamically at boot
- time and enable its usage.
+ Add support for the Svnapot ISA-extension in the kernel when it
+ is detected at boot.
The Svnapot extension is used to mark contiguous PTEs as a range
of contiguous virtual-to-physical translations for a naturally
@@ -530,9 +531,8 @@ config RISCV_ISA_SVPBMT
depends on RISCV_ALTERNATIVE
default y
help
- Adds support to dynamically detect the presence of the Svpbmt
- ISA-extension (Supervisor-mode: page-based memory types) and
- enable its usage.
+ Add support for the Svpbmt ISA-extension (Supervisor-mode:
+ page-based memory types) in the kernel when it is detected at boot.
The memory type for a page contains a combination of attributes
that indicate the cacheability, idempotency, and ordering
@@ -551,14 +551,15 @@ config TOOLCHAIN_HAS_V
depends on AS_HAS_OPTION_ARCH
config RISCV_ISA_V
- bool "VECTOR extension support"
+ bool "Vector extension support"
depends on TOOLCHAIN_HAS_V
depends on FPU
select DYNAMIC_SIGFRAME
default y
help
- Say N here if you want to disable all vector related procedure
- in the kernel.
+ Add support for the Vector extension when it is detected at boot.
+ When this option is disabled, neither the kernel nor userspace may
+ use vector procedures.
If you don't know what to do here, say Y.
@@ -616,8 +617,8 @@ config RISCV_ISA_ZBB
depends on RISCV_ALTERNATIVE
default y
help
- Adds support to dynamically detect the presence of the ZBB
- extension (basic bit manipulation) and enable its usage.
+ Add support for enabling optimisations in the kernel when the
+ Zbb extension is detected at boot.
The Zbb extension provides instructions to accelerate a number
of bit-specific operations (count bit population, sign extending,
@@ -633,9 +634,9 @@ config RISCV_ISA_ZICBOM
select RISCV_DMA_NONCOHERENT
select DMA_DIRECT_REMAP
help
- Adds support to dynamically detect the presence of the ZICBOM
- extension (Cache Block Management Operations) and enable its
- usage.
+ Add support for the Zicbom extension (Cache Block Management
+ Operations) and enable its use in the kernel when it is detected
+ at boot.
The Zicbom extension can be used to handle for example
non-coherent DMA support on devices that need it.
@@ -648,7 +649,7 @@ config RISCV_ISA_ZICBOZ
default y
help
Enable the use of the Zicboz extension (cbo.zero instruction)
- when available.
+ in the kernel when it is detected at boot.
The Zicboz extension is used for faster zeroing of memory.
@@ -693,8 +694,9 @@ config FPU
bool "FPU support"
default y
help
- Say N here if you want to disable all floating-point related procedure
- in the kernel.
+ Add support for floating point operations when an FPU is detected at
+ boot. When this option is disabled, neither the kernel nor userspace
+ may use the floating point unit.
If you don't know what to do here, say Y.
--
2.43.0
From: Conor Dooley <[email protected]>
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.
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.
Signed-off-by: Conor Dooley <[email protected]>
---
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(-)
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
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.
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 @@
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 == 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;
legacy:
-# endif
+#endif
return __sw_hweight64(w);
}
#else /* BITS_PER_LONG == 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>
-#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_); \
})
-#endif /* !defined(CONFIG_RISCV_ISA_ZBB) || defined(NO_ALTERNATIVE) */
+#endif /* !(defined(CONFIG_RISCV_ISA_ZBB) && defined(CONFIG_TOOLCHAIN_HAS_ZBB)) || defined(NO_ALTERNATIVE) */
#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 not
* 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;
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 = (__force unsigned int)htonl(proto);
sum += uproto;
- /*
- * 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;
/*
@@ -157,12 +152,7 @@ do_csum_with_alignment(const unsigned char *buff, int len)
csum = do_csum_common(ptr, end, data);
#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;
/*
@@ -244,12 +234,7 @@ do_csum_no_alignment(const unsigned char *buff, int len)
end = (const unsigned long *)(buff + len);
csum = do_csum_common(ptr, end, data);
- /*
- * 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;
/*
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)
- 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))
/*
* 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:
.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)
- 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))
/*
* 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:
#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)
- 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))
/*
* 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:
.option push
--
2.43.0
On Tue, May 28, 2024 at 12:11:12PM GMT, Conor Dooley wrote:
> From: Conor Dooley <[email protected]>
>
> 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.
>
> 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.
>
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> 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(-)
Reviewed-by: Andrew Jones <[email protected]>
> -----Original Message-----
> From: Conor Dooley <[email protected]>
> Sent: Tuesday, May 28, 2024 7:11 PM
> To: [email protected]
> Cc: [email protected]; Conor Dooley <[email protected]>; Wang,
> Xiao W <[email protected]>; Andrew Jones
> <[email protected]>; [email protected]; Charlie Jenkins
> <[email protected]>; Paul Walmsley <[email protected]>; Palmer
> Dabbelt <[email protected]>; [email protected]; Samuel
> Holland <[email protected]>; Pu Lehui
> <[email protected]>; Bj?rn T?pel <[email protected]>
> Subject: [PATCH v3 2/2] RISC-V: separate Zbb optimisations requiring and not
> requiring toolchain support
>
> From: Conor Dooley <[email protected]>
>
> 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.
>
> 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.
>
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> 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(-)
>
> 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
>
> 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.
>
> 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 @@
>
> 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 == 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;
>
> legacy:
> -# endif
> +#endif
> return __sw_hweight64(w);
> }
> #else /* BITS_PER_LONG == 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>
>
> -#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_); \
> })
>
> -#endif /* !defined(CONFIG_RISCV_ISA_ZBB) || defined(NO_ALTERNATIVE)
> */
> +#endif /* !(defined(CONFIG_RISCV_ISA_ZBB) &&
> defined(CONFIG_TOOLCHAIN_HAS_ZBB)) || defined(NO_ALTERNATIVE) */
>
> #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 not
> * 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;
>
> 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 = (__force unsigned int)htonl(proto);
> sum += uproto;
>
> - /*
> - * 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;
>
> /*
> @@ -157,12 +152,7 @@ do_csum_with_alignment(const unsigned char
> *buff, int len)
> csum = do_csum_common(ptr, end, data);
>
> #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;
>
> /*
> @@ -244,12 +234,7 @@ do_csum_no_alignment(const unsigned char *buff,
> int len)
> end = (const unsigned long *)(buff + len);
> csum = do_csum_common(ptr, end, data);
>
> - /*
> - * 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;
>
> /*
> 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)
>
> - 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
>
> /*
> * 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:
>
> .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)
>
> - 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))
>
> /*
> * 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:
>
> #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)
>
> - 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))
>
> /*
> * 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:
>
> .option push
> --
> 2.43.0
On Wed, May 29, 2024 at 01:01:24AM +0000, Wang, Xiao W wrote:
> > From: Conor Dooley <[email protected]>
> > Sent: Tuesday, May 28, 2024 7:11 PM
> > - 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
I could have sworn I grepped for it and there was nothing, obviously
not... Thanks, I'll fix it up in a new version.
Thanks,
Conor
Hi Conor,
On 28/05/2024 13:11, Conor Dooley wrote:
> From: Conor Dooley <[email protected]>
>
> During some discussion on IRC yesterday and on Pu's bpf patch [1]
> I noticed that these RISCV_ISA* Kconfig options are not really clear
> about their implications. Many of these options have no impact on what
> userspace is allowed to do, for example an application can use Zbb
> regardless of whether or not the kernel does. Change the help text to
> try and clarify whether or not an option affects just the kernel, or
> also userspace. None of these options actually control whether or not an
> extension is detected dynamically as that's done regardless of Kconfig
> options, so drop any text that implies the option is required for
> dynamic detection, rewording them as "do x when y is detected".
>
> Link: https://lore.kernel.org/linux-riscv/20240328-ferocity-repose-c554f75a676c@spud/ [1]
> Reviewed-by: Andrew Jones <[email protected]>
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> arch/riscv/Kconfig | 36 +++++++++++++++++++-----------------
> 1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index b94176e25be1..3b702e6cc051 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -501,7 +501,8 @@ config RISCV_ISA_C
> help
> Adds "C" to the ISA subsets that the toolchain is allowed to emit
> when building Linux, which results in compressed instructions in the
> - Linux binary.
> + Linux binary. This option produces a kernel that will not run on
> + systems that do not support compressed instructions.
>
> If you don't know what to do here, say Y.
>
> @@ -511,8 +512,8 @@ config RISCV_ISA_SVNAPOT
> depends on RISCV_ALTERNATIVE
> default y
> help
> - Allow kernel to detect the Svnapot ISA-extension dynamically at boot
> - time and enable its usage.
> + Add support for the Svnapot ISA-extension in the kernel when it
> + is detected at boot.
To me, the new version makes things even more confusing: svnapot
mappings will indeed be handled by the kernel (since only the kernel
sets up the page tables) but it will only be used (for now) for HugeTLB
mappings in userspace.
>
> The Svnapot extension is used to mark contiguous PTEs as a range
> of contiguous virtual-to-physical translations for a naturally
> @@ -530,9 +531,8 @@ config RISCV_ISA_SVPBMT
> depends on RISCV_ALTERNATIVE
> default y
> help
> - Adds support to dynamically detect the presence of the Svpbmt
> - ISA-extension (Supervisor-mode: page-based memory types) and
> - enable its usage.
> + Add support for the Svpbmt ISA-extension (Supervisor-mode:
> + page-based memory types) in the kernel when it is detected at boot.
>
> The memory type for a page contains a combination of attributes
> that indicate the cacheability, idempotency, and ordering
> @@ -551,14 +551,15 @@ config TOOLCHAIN_HAS_V
> depends on AS_HAS_OPTION_ARCH
>
> config RISCV_ISA_V
> - bool "VECTOR extension support"
> + bool "Vector extension support"
> depends on TOOLCHAIN_HAS_V
> depends on FPU
> select DYNAMIC_SIGFRAME
> default y
> help
> - Say N here if you want to disable all vector related procedure
> - in the kernel.
> + Add support for the Vector extension when it is detected at boot.
> + When this option is disabled, neither the kernel nor userspace may
> + use vector procedures.
>
> If you don't know what to do here, say Y.
>
> @@ -616,8 +617,8 @@ config RISCV_ISA_ZBB
> depends on RISCV_ALTERNATIVE
> default y
> help
> - Adds support to dynamically detect the presence of the ZBB
> - extension (basic bit manipulation) and enable its usage.
> + Add support for enabling optimisations in the kernel when the
> + Zbb extension is detected at boot.
>
> The Zbb extension provides instructions to accelerate a number
> of bit-specific operations (count bit population, sign extending,
> @@ -633,9 +634,9 @@ config RISCV_ISA_ZICBOM
> select RISCV_DMA_NONCOHERENT
> select DMA_DIRECT_REMAP
> help
> - Adds support to dynamically detect the presence of the ZICBOM
> - extension (Cache Block Management Operations) and enable its
> - usage.
> + Add support for the Zicbom extension (Cache Block Management
> + Operations) and enable its use in the kernel when it is detected
> + at boot.
>
> The Zicbom extension can be used to handle for example
> non-coherent DMA support on devices that need it.
> @@ -648,7 +649,7 @@ config RISCV_ISA_ZICBOZ
> default y
> help
> Enable the use of the Zicboz extension (cbo.zero instruction)
> - when available.
> + in the kernel when it is detected at boot.
>
> The Zicboz extension is used for faster zeroing of memory.
>
> @@ -693,8 +694,9 @@ config FPU
> bool "FPU support"
> default y
> help
> - Say N here if you want to disable all floating-point related procedure
> - in the kernel.
> + Add support for floating point operations when an FPU is detected at
> + boot. When this option is disabled, neither the kernel nor userspace
> + may use the floating point unit.
>
> If you don't know what to do here, say Y.
>
On Wed, May 29, 2024 at 10:47:40AM +0200, Alexandre Ghiti wrote:
> Hi Conor,
>
> On 28/05/2024 13:11, Conor Dooley wrote:
> > From: Conor Dooley <[email protected]>
> >
> > During some discussion on IRC yesterday and on Pu's bpf patch [1]
> > I noticed that these RISCV_ISA* Kconfig options are not really clear
> > about their implications. Many of these options have no impact on what
> > userspace is allowed to do, for example an application can use Zbb
> > regardless of whether or not the kernel does. Change the help text to
> > try and clarify whether or not an option affects just the kernel, or
> > also userspace. None of these options actually control whether or not an
> > extension is detected dynamically as that's done regardless of Kconfig
> > options, so drop any text that implies the option is required for
> > dynamic detection, rewording them as "do x when y is detected".
> >
> > Link: https://lore.kernel.org/linux-riscv/20240328-ferocity-repose-c554f75a676c@spud/ [1]
> > Reviewed-by: Andrew Jones <[email protected]>
> > Signed-off-by: Conor Dooley <[email protected]>
> > ---
> > arch/riscv/Kconfig | 36 +++++++++++++++++++-----------------
> > 1 file changed, 19 insertions(+), 17 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index b94176e25be1..3b702e6cc051 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -501,7 +501,8 @@ config RISCV_ISA_C
> > help
> > Adds "C" to the ISA subsets that the toolchain is allowed to emit
> > when building Linux, which results in compressed instructions in the
> > - Linux binary.
> > + Linux binary. This option produces a kernel that will not run on
> > + systems that do not support compressed instructions.
> > If you don't know what to do here, say Y.
> > @@ -511,8 +512,8 @@ config RISCV_ISA_SVNAPOT
> > depends on RISCV_ALTERNATIVE
> > default y
> > help
> > - Allow kernel to detect the Svnapot ISA-extension dynamically at boot
> > - time and enable its usage.
> > + Add support for the Svnapot ISA-extension in the kernel when it
> > + is detected at boot.
>
>
> To me, the new version makes things even more confusing: svnapot mappings
> will indeed be handled by the kernel (since only the kernel sets up the page
> tables) but it will only be used (for now) for HugeTLB mappings in
> userspace.
How would you suggest that I word it? "Enable the use of the Svnapot
ISA-extension when it is detected at boot"? The current text implies that
these options control detection of extensions (which they do not) and
that is what I am looking to remove as it has caused confusion.
On 29/05/2024 10:54, Conor Dooley wrote:
> On Wed, May 29, 2024 at 10:47:40AM +0200, Alexandre Ghiti wrote:
>> Hi Conor,
>>
>> On 28/05/2024 13:11, Conor Dooley wrote:
>>> From: Conor Dooley <[email protected]>
>>>
>>> During some discussion on IRC yesterday and on Pu's bpf patch [1]
>>> I noticed that these RISCV_ISA* Kconfig options are not really clear
>>> about their implications. Many of these options have no impact on what
>>> userspace is allowed to do, for example an application can use Zbb
>>> regardless of whether or not the kernel does. Change the help text to
>>> try and clarify whether or not an option affects just the kernel, or
>>> also userspace. None of these options actually control whether or not an
>>> extension is detected dynamically as that's done regardless of Kconfig
>>> options, so drop any text that implies the option is required for
>>> dynamic detection, rewording them as "do x when y is detected".
>>>
>>> Link: https://lore.kernel.org/linux-riscv/20240328-ferocity-repose-c554f75a676c@spud/ [1]
>>> Reviewed-by: Andrew Jones <[email protected]>
>>> Signed-off-by: Conor Dooley <[email protected]>
>>> ---
>>> arch/riscv/Kconfig | 36 +++++++++++++++++++-----------------
>>> 1 file changed, 19 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>>> index b94176e25be1..3b702e6cc051 100644
>>> --- a/arch/riscv/Kconfig
>>> +++ b/arch/riscv/Kconfig
>>> @@ -501,7 +501,8 @@ config RISCV_ISA_C
>>> help
>>> Adds "C" to the ISA subsets that the toolchain is allowed to emit
>>> when building Linux, which results in compressed instructions in the
>>> - Linux binary.
>>> + Linux binary. This option produces a kernel that will not run on
>>> + systems that do not support compressed instructions.
>>> If you don't know what to do here, say Y.
>>> @@ -511,8 +512,8 @@ config RISCV_ISA_SVNAPOT
>>> depends on RISCV_ALTERNATIVE
>>> default y
>>> help
>>> - Allow kernel to detect the Svnapot ISA-extension dynamically at boot
>>> - time and enable its usage.
>>> + Add support for the Svnapot ISA-extension in the kernel when it
>>> + is detected at boot.
>>
>> To me, the new version makes things even more confusing: svnapot mappings
>> will indeed be handled by the kernel (since only the kernel sets up the page
>> tables) but it will only be used (for now) for HugeTLB mappings in
>> userspace.
> How would you suggest that I word it? "Enable the use of the Svnapot
> ISA-extension when it is detected at boot"? The current text implies that
> these options control detection of extensions (which they do not) and
> that is what I am looking to remove as it has caused confusion.
Ok, I see what you mean. Your above suggestion looks great then :)
Thanks,
Alex
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv