Here's my attempt at fixing both the use of an FPU on XIP kernels and
the issue that Jason ran into where CONFIG_FPU, which needs the
alternatives frame work for has_fpu() checks, could be enabled without
the alternatives actually being present.
For the former, a "slow" fallback that does not use alternatives is
added to riscv_has_extension_[un]likely() that can be used with XIP.
Obviously, we want to make use of Jisheng's alternatives based approach
where possible, so any users of riscv_has_extension_[un]likely() will
want to make sure that they select RISCV_ALTERNATIVE.
If they don't however, they'll hit the fallback path which (should,
sparing a silly mistake from me!) behave in the same way, thus
succeeding silently. Sounds like a
To prevent "depends on !XIP_KERNEL; select RISCV_ALTERNATIVE" spreading
like the plague through the various places that want to check for the
presence of extensions, and sidestep the potential silent "success"
mentioned above, all users RISCV_ALTERNATIVE are converted from selects
to dependencies, with the option being selected for all !XIP_KERNEL
builds.
I know that the VDSO was a key place that Jisheng wanted to use the new
helper rather than static branches, and I think the fallback path
should not cause issues there.
See the thread at [1] for the prior discussion.
Cheers,
Conor.
1 - https://lore.kernel.org/linux-riscv/[email protected]/T/#m21390d570997145d31dd8bb95002fd61f99c6573
CC: Paul Walmsley <[email protected]>
CC: Palmer Dabbelt <[email protected]>
CC: Conor Dooley <[email protected]>
CC: Heiko Stuebner <[email protected]>
CC: Andrew Jones <[email protected]>
CC: Anup Patel <[email protected]>
CC: Jisheng Zhang <[email protected]>
CC: Andrew Jones <[email protected]>
CC: Jason A. Donenfeld <[email protected]>
CC: [email protected]
CC: [email protected]
Conor Dooley (2):
RISC-V: add non-alternative fallback for
riscv_has_extension_[un]likely()
RISC-V: always select RISCV_ALTERNATIVE for non-xip kernels
arch/riscv/Kconfig | 12 ++++----
arch/riscv/Kconfig.erratas | 6 ++--
arch/riscv/include/asm/hwcap.h | 50 ++++++++++++++++++++--------------
3 files changed, 38 insertions(+), 30 deletions(-)
--
2.39.2
The has_fpu() check, which in turn calls riscv_has_extension_likely(),
relies on alternatives to figure out whether the system has an FPU.
As a result, it will malfunction on XIP kernels, as they do not support
the alternatives mechanism.
When alternatives support is not present, fall back to using
__riscv_isa_extension_available() in riscv_has_extension_[un]likely()
instead stead, which handily takes the same argument, so that kernels
that do not support alternatives can accurately report the presence of
FPU support.
Fixes: 702e64550b12 ("riscv: fpu: switch has_fpu() to riscv_has_extension_likely()")
Link: https://lore.kernel.org/all/ad445951-3d13-4644-94d9-e0989cda39c3@spud/
Signed-off-by: Conor Dooley <[email protected]>
---
arch/riscv/include/asm/hwcap.h | 50 ++++++++++++++++++++--------------
1 file changed, 30 insertions(+), 20 deletions(-)
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index e3021b2590de..6263a0de1c6a 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -57,18 +57,31 @@ struct riscv_isa_ext_data {
unsigned int isa_ext_id;
};
+unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
+
+#define riscv_isa_extension_mask(ext) BIT_MASK(RISCV_ISA_EXT_##ext)
+
+bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
+#define riscv_isa_extension_available(isa_bitmap, ext) \
+ __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
+
static __always_inline bool
riscv_has_extension_likely(const unsigned long ext)
{
compiletime_assert(ext < RISCV_ISA_EXT_MAX,
"ext must be < RISCV_ISA_EXT_MAX");
- asm_volatile_goto(
- ALTERNATIVE("j %l[l_no]", "nop", 0, %[ext], 1)
- :
- : [ext] "i" (ext)
- :
- : l_no);
+ if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) {
+ asm_volatile_goto(
+ ALTERNATIVE("j %l[l_no]", "nop", 0, %[ext], 1)
+ :
+ : [ext] "i" (ext)
+ :
+ : l_no);
+ } else {
+ if (!__riscv_isa_extension_available(NULL, ext))
+ goto l_no;
+ }
return true;
l_no:
@@ -81,26 +94,23 @@ riscv_has_extension_unlikely(const unsigned long ext)
compiletime_assert(ext < RISCV_ISA_EXT_MAX,
"ext must be < RISCV_ISA_EXT_MAX");
- asm_volatile_goto(
- ALTERNATIVE("nop", "j %l[l_yes]", 0, %[ext], 1)
- :
- : [ext] "i" (ext)
- :
- : l_yes);
+ if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) {
+ asm_volatile_goto(
+ ALTERNATIVE("nop", "j %l[l_yes]", 0, %[ext], 1)
+ :
+ : [ext] "i" (ext)
+ :
+ : l_yes);
+ } else {
+ if (__riscv_isa_extension_available(NULL, ext))
+ goto l_yes;
+ }
return false;
l_yes:
return true;
}
-unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
-
-#define riscv_isa_extension_mask(ext) BIT_MASK(RISCV_ISA_EXT_##ext)
-
-bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
-#define riscv_isa_extension_available(isa_bitmap, ext) \
- __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
-
#endif
#endif /* _ASM_RISCV_HWCAP_H */
--
2.39.2
When moving switch_to's has_fpu() over to using
riscv_has_extension_likely() rather than static branches, the FPU code
gained a dependency on the alternatives framework.
That dependency has now been removed, as riscv_has_extension_ikely() now
contains a fallback path, using __riscv_isa_extension_available(), but
if CONFIG_RISCV_ALTERNATIVE isn't selected when CONFIG_FPU is, has_fpu()
checks will not benefit from the "fast path" that the alternatives
framework provides.
We want to ensure that alternatives are available whenever
riscv_has_extension_[un]likely() is used, rather than silently falling
back to the slow path, but rather than rely on selecting
RISCV_ALTERNATIVE in the myriad of locations that may use
riscv_has_extension_[un]likely(), select it (almost) always instead by
adding it to the main RISCV config entry.
xip kernels cannot make use of the alternatives framework, so it is not
enabled for those configurations, although this is the status quo.
All current sites that select RISCV_ALTERNATIVE are converted to
dependencies on the option instead. The explicit dependencies on
!XIP_KERNEL can be dropped, as RISCV_ALTERNATIVE is not user selectable.
Fixes: 702e64550b12 ("riscv: fpu: switch has_fpu() to riscv_has_extension_likely()")
Link: https://lore.kernel.org/all/[email protected]/
Reported-by: Jason A. Donenfeld <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
arch/riscv/Kconfig | 12 ++++++------
arch/riscv/Kconfig.erratas | 6 ++----
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index c5e42cc37604..2f6976418d0a 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -126,6 +126,7 @@ config RISCV
select OF_IRQ
select PCI_DOMAINS_GENERIC if PCI
select PCI_MSI if PCI
+ select RISCV_ALTERNATIVE if !XIP_KERNEL
select RISCV_INTC
select RISCV_TIMER if RISCV_SBI
select SIFIVE_PLIC
@@ -401,9 +402,8 @@ config RISCV_ISA_C
config RISCV_ISA_SVPBMT
bool "SVPBMT extension support"
depends on 64BIT && MMU
- depends on !XIP_KERNEL
+ depends on RISCV_ALTERNATIVE
default y
- select RISCV_ALTERNATIVE
help
Adds support to dynamically detect the presence of the SVPBMT
ISA-extension (Supervisor-mode: page-based memory types) and
@@ -428,8 +428,8 @@ config TOOLCHAIN_HAS_ZBB
config RISCV_ISA_ZBB
bool "Zbb extension support for bit manipulation instructions"
depends on TOOLCHAIN_HAS_ZBB
- depends on !XIP_KERNEL && MMU
- select RISCV_ALTERNATIVE
+ depends on MMU
+ depends on RISCV_ALTERNATIVE
default y
help
Adds support to dynamically detect the presence of the ZBB
@@ -443,9 +443,9 @@ config RISCV_ISA_ZBB
config RISCV_ISA_ZICBOM
bool "Zicbom extension support for non-coherent DMA operation"
- depends on !XIP_KERNEL && MMU
+ depends on MMU
+ depends on RISCV_ALTERNATIVE
default y
- select RISCV_ALTERNATIVE
select RISCV_DMA_NONCOHERENT
help
Adds support to dynamically detect the presence of the ZICBOM
diff --git a/arch/riscv/Kconfig.erratas b/arch/riscv/Kconfig.erratas
index 69621ae6d647..0c8f4652cd82 100644
--- a/arch/riscv/Kconfig.erratas
+++ b/arch/riscv/Kconfig.erratas
@@ -2,8 +2,7 @@ menu "CPU errata selection"
config ERRATA_SIFIVE
bool "SiFive errata"
- depends on !XIP_KERNEL
- select RISCV_ALTERNATIVE
+ depends on RISCV_ALTERNATIVE
help
All SiFive errata Kconfig depend on this Kconfig. Disabling
this Kconfig will disable all SiFive errata. Please say "Y"
@@ -35,8 +34,7 @@ config ERRATA_SIFIVE_CIP_1200
config ERRATA_THEAD
bool "T-HEAD errata"
- depends on !XIP_KERNEL
- select RISCV_ALTERNATIVE
+ depends on RISCV_ALTERNATIVE
help
All T-HEAD errata Kconfig depend on this Kconfig. Disabling
this Kconfig will disable all T-HEAD errata. Please say "Y"
--
2.39.2
On Fri, Mar 24, 2023 at 10:05:37AM +0000, Conor Dooley wrote:
> Here's my attempt at fixing both the use of an FPU on XIP kernels and
> the issue that Jason ran into where CONFIG_FPU, which needs the
> alternatives frame work for has_fpu() checks, could be enabled without
> the alternatives actually being present.
>
> For the former, a "slow" fallback that does not use alternatives is
> added to riscv_has_extension_[un]likely() that can be used with XIP.
> Obviously, we want to make use of Jisheng's alternatives based approach
> where possible, so any users of riscv_has_extension_[un]likely() will
> want to make sure that they select RISCV_ALTERNATIVE.
> If they don't however, they'll hit the fallback path which (should,
> sparing a silly mistake from me!) behave in the same way, thus
> succeeding silently. Sounds like a
>
> To prevent "depends on !XIP_KERNEL; select RISCV_ALTERNATIVE" spreading
> like the plague through the various places that want to check for the
> presence of extensions, and sidestep the potential silent "success"
> mentioned above, all users RISCV_ALTERNATIVE are converted from selects
> to dependencies, with the option being selected for all !XIP_KERNEL
> builds.
>
> I know that the VDSO was a key place that Jisheng wanted to use the new
> helper rather than static branches, and I think the fallback path
> should not cause issues there.
>
> See the thread at [1] for the prior discussion.
>
> Cheers,
> Conor.
>
> 1 - https://lore.kernel.org/linux-riscv/[email protected]/T/#m21390d570997145d31dd8bb95002fd61f99c6573
>
> CC: Paul Walmsley <[email protected]>
> CC: Palmer Dabbelt <[email protected]>
> CC: Conor Dooley <[email protected]>
> CC: Heiko Stuebner <[email protected]>
> CC: Andrew Jones <[email protected]>
> CC: Anup Patel <[email protected]>
> CC: Jisheng Zhang <[email protected]>
> CC: Andrew Jones <[email protected]>
> CC: Jason A. Donenfeld <[email protected]>
> CC: [email protected]
> CC: [email protected]
>
> Conor Dooley (2):
> RISC-V: add non-alternative fallback for
> riscv_has_extension_[un]likely()
> RISC-V: always select RISCV_ALTERNATIVE for non-xip kernels
>
> arch/riscv/Kconfig | 12 ++++----
> arch/riscv/Kconfig.erratas | 6 ++--
> arch/riscv/include/asm/hwcap.h | 50 ++++++++++++++++++++--------------
> 3 files changed, 38 insertions(+), 30 deletions(-)
>
> --
> 2.39.2
>
LGTM, but if it was based on for-next then it could also immediately be
applied to zicboz. For the series,
Reviewed-by: Andrew Jones <[email protected]>
Thanks,
drew
On Fri, Mar 24, 2023 at 12:31:07PM +0100, Andrew Jones wrote:
> On Fri, Mar 24, 2023 at 10:05:37AM +0000, Conor Dooley wrote:
> > Here's my attempt at fixing both the use of an FPU on XIP kernels and
> > the issue that Jason ran into where CONFIG_FPU, which needs the
> > alternatives frame work for has_fpu() checks, could be enabled without
> > the alternatives actually being present.
> >
> > For the former, a "slow" fallback that does not use alternatives is
> > added to riscv_has_extension_[un]likely() that can be used with XIP.
> > Obviously, we want to make use of Jisheng's alternatives based approach
> > where possible, so any users of riscv_has_extension_[un]likely() will
> > want to make sure that they select RISCV_ALTERNATIVE.
> > If they don't however, they'll hit the fallback path which (should,
> > sparing a silly mistake from me!) behave in the same way, thus
> > succeeding silently. Sounds like a
> >
> > To prevent "depends on !XIP_KERNEL; select RISCV_ALTERNATIVE" spreading
> > like the plague through the various places that want to check for the
> > presence of extensions, and sidestep the potential silent "success"
> > mentioned above, all users RISCV_ALTERNATIVE are converted from selects
> > to dependencies, with the option being selected for all !XIP_KERNEL
> > builds.
> >
> > I know that the VDSO was a key place that Jisheng wanted to use the new
> > helper rather than static branches, and I think the fallback path
> > should not cause issues there.
> >
> > See the thread at [1] for the prior discussion.
> >
> > Cheers,
> > Conor.
> >
> > 1 - https://lore.kernel.org/linux-riscv/[email protected]/T/#m21390d570997145d31dd8bb95002fd61f99c6573
> >
> > CC: Paul Walmsley <[email protected]>
> > CC: Palmer Dabbelt <[email protected]>
> > CC: Conor Dooley <[email protected]>
> > CC: Heiko Stuebner <[email protected]>
> > CC: Andrew Jones <[email protected]>
> > CC: Anup Patel <[email protected]>
> > CC: Jisheng Zhang <[email protected]>
> > CC: Andrew Jones <[email protected]>
> > CC: Jason A. Donenfeld <[email protected]>
> > CC: [email protected]
> > CC: [email protected]
> >
> > Conor Dooley (2):
> > RISC-V: add non-alternative fallback for
> > riscv_has_extension_[un]likely()
> > RISC-V: always select RISCV_ALTERNATIVE for non-xip kernels
> >
> > arch/riscv/Kconfig | 12 ++++----
> > arch/riscv/Kconfig.erratas | 6 ++--
> > arch/riscv/include/asm/hwcap.h | 50 ++++++++++++++++++++--------------
> > 3 files changed, 38 insertions(+), 30 deletions(-)
> >
> > --
> > 2.39.2
> >
>
> LGTM, but if it was based on for-next then it could also immediately be
> applied to zicboz. For the series,
Hmm, I did it on top of fixes since this needs to go into v6.3.
Perhaps I can create a standalone patch for Zicboz and Palmer could merge
these two into both fixes & for-next, with the standalone applied on
top?
On Fri, Mar 24, 2023 at 11:37:05AM +0000, Conor Dooley wrote:
> On Fri, Mar 24, 2023 at 12:31:07PM +0100, Andrew Jones wrote:
> > On Fri, Mar 24, 2023 at 10:05:37AM +0000, Conor Dooley wrote:
> > > Here's my attempt at fixing both the use of an FPU on XIP kernels and
> > > the issue that Jason ran into where CONFIG_FPU, which needs the
> > > alternatives frame work for has_fpu() checks, could be enabled without
> > > the alternatives actually being present.
> > >
> > > For the former, a "slow" fallback that does not use alternatives is
> > > added to riscv_has_extension_[un]likely() that can be used with XIP.
> > > Obviously, we want to make use of Jisheng's alternatives based approach
> > > where possible, so any users of riscv_has_extension_[un]likely() will
> > > want to make sure that they select RISCV_ALTERNATIVE.
> > > If they don't however, they'll hit the fallback path which (should,
> > > sparing a silly mistake from me!) behave in the same way, thus
> > > succeeding silently. Sounds like a
> > >
> > > To prevent "depends on !XIP_KERNEL; select RISCV_ALTERNATIVE" spreading
> > > like the plague through the various places that want to check for the
> > > presence of extensions, and sidestep the potential silent "success"
> > > mentioned above, all users RISCV_ALTERNATIVE are converted from selects
> > > to dependencies, with the option being selected for all !XIP_KERNEL
> > > builds.
> > >
> > > I know that the VDSO was a key place that Jisheng wanted to use the new
> > > helper rather than static branches, and I think the fallback path
> > > should not cause issues there.
> > >
> > > See the thread at [1] for the prior discussion.
> > >
> > > Cheers,
> > > Conor.
> > >
> > > 1 - https://lore.kernel.org/linux-riscv/[email protected]/T/#m21390d570997145d31dd8bb95002fd61f99c6573
> > >
> > > CC: Paul Walmsley <[email protected]>
> > > CC: Palmer Dabbelt <[email protected]>
> > > CC: Conor Dooley <[email protected]>
> > > CC: Heiko Stuebner <[email protected]>
> > > CC: Andrew Jones <[email protected]>
> > > CC: Anup Patel <[email protected]>
> > > CC: Jisheng Zhang <[email protected]>
> > > CC: Andrew Jones <[email protected]>
> > > CC: Jason A. Donenfeld <[email protected]>
> > > CC: [email protected]
> > > CC: [email protected]
> > >
> > > Conor Dooley (2):
> > > RISC-V: add non-alternative fallback for
> > > riscv_has_extension_[un]likely()
> > > RISC-V: always select RISCV_ALTERNATIVE for non-xip kernels
> > >
> > > arch/riscv/Kconfig | 12 ++++----
> > > arch/riscv/Kconfig.erratas | 6 ++--
> > > arch/riscv/include/asm/hwcap.h | 50 ++++++++++++++++++++--------------
> > > 3 files changed, 38 insertions(+), 30 deletions(-)
> > >
> > > --
> > > 2.39.2
> > >
> >
> > LGTM, but if it was based on for-next then it could also immediately be
> > applied to zicboz. For the series,
>
> Hmm, I did it on top of fixes since this needs to go into v6.3.
Ah, sure.
> Perhaps I can create a standalone patch for Zicboz and Palmer could merge
> these two into both fixes & for-next, with the standalone applied on
> top?
Sounds good.
Thanks,
drew
Seems like a good approach to me. I'm not a RISC-V maintainer or
reviewer, but in case it helps,
Reviewed-by: Jason A. Donenfeld <[email protected]>
On Fri, 24 Mar 2023 10:05:37 +0000, Conor Dooley wrote:
> Here's my attempt at fixing both the use of an FPU on XIP kernels and
> the issue that Jason ran into where CONFIG_FPU, which needs the
> alternatives frame work for has_fpu() checks, could be enabled without
> the alternatives actually being present.
>
> For the former, a "slow" fallback that does not use alternatives is
> added to riscv_has_extension_[un]likely() that can be used with XIP.
> Obviously, we want to make use of Jisheng's alternatives based approach
> where possible, so any users of riscv_has_extension_[un]likely() will
> want to make sure that they select RISCV_ALTERNATIVE.
> If they don't however, they'll hit the fallback path which (should,
> sparing a silly mistake from me!) behave in the same way, thus
> succeeding silently. Sounds like a
>
> [...]
Applied, thanks!
[1/2] RISC-V: add non-alternative fallback for riscv_has_extension_[un]likely()
https://git.kernel.org/palmer/c/1aa866931b80
[2/2] RISC-V: always select RISCV_ALTERNATIVE for non-xip kernels
https://git.kernel.org/palmer/c/1ee7fc3f4d0a
Best regards,
--
Palmer Dabbelt <[email protected]>
Hello:
This series was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <[email protected]>:
On Fri, 24 Mar 2023 10:05:37 +0000 you wrote:
> Here's my attempt at fixing both the use of an FPU on XIP kernels and
> the issue that Jason ran into where CONFIG_FPU, which needs the
> alternatives frame work for has_fpu() checks, could be enabled without
> the alternatives actually being present.
>
> For the former, a "slow" fallback that does not use alternatives is
> added to riscv_has_extension_[un]likely() that can be used with XIP.
> Obviously, we want to make use of Jisheng's alternatives based approach
> where possible, so any users of riscv_has_extension_[un]likely() will
> want to make sure that they select RISCV_ALTERNATIVE.
> If they don't however, they'll hit the fallback path which (should,
> sparing a silly mistake from me!) behave in the same way, thus
> succeeding silently. Sounds like a
>
> [...]
Here is the summary with links:
- [v1,1/2] RISC-V: add non-alternative fallback for riscv_has_extension_[un]likely()
https://git.kernel.org/riscv/c/1aa866931b80
- [v1,2/2] RISC-V: always select RISCV_ALTERNATIVE for non-xip kernels
https://git.kernel.org/riscv/c/1ee7fc3f4d0a
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html