On Thu, May 15, 2025 at 10:48:35AM +0200, Radim Krčmář wrote:
2025-05-15T09:28:25+02:00, Alexandre Ghiti <a...@ghiti.fr>:
On 06/05/2025 12:10, Radim Krčmář wrote:
2025-05-02T16:30:36-07:00, Deepak Gupta <de...@rivosinc.com>:
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
@@ -91,6 +91,32 @@
+.macro restore_userssp tmp
+ ALTERNATIVE("nops(2)",
+ __stringify( \
+ REG_L \tmp, TASK_TI_USER_SSP(tp); \
+ csrw CSR_SSP, \tmp),
+ 0,
+ RISCV_ISA_EXT_ZICFISS,
+ CONFIG_RISCV_USER_CFI)
+.endm
Do we need to emit the nops when CONFIG_RISCV_USER_CFI isn't selected?
(Why not put #ifdef CONFIG_RISCV_USER_CFI around the ALTERNATIVES?)
The alternatives are used to create a generic kernel that contains the
code for a large number of extensions and only enable it at runtime
depending on the platform capabilities. This way distros can ship a
single kernel that works on all platforms.
Yup, and if a kernel is compiled without CONFIG_RISCV_USER_CFI, the nops
will only enlarge the binary and potentially slow down execution.
In other words, why we don't do something like this
(!CONFIG_RISCV_USER_CFI ? "" :
(RISCV_ISA_EXT_ZICFISS ? __stringify(...) : "nops(x)"))
instead of the current
(CONFIG_RISCV_USER_CFI &&
RISCV_ISA_EXT_ZICFISS ? __stringify(...) : "nops(x)")
It could be a new preprocessor macro in case we wanted to make it nice,
but it's probably not a common case, so an ifdef could work as well.
Do we just generally not care about such minor optimizations?
On its own just for this series, I am not sure if I would call it even a
minor optimization.
But sure, it may (or may not) have noticeable effect if someone were
to go around and muck with ALTERNATIVES macro and emit `old_c` only
if config were selected. That should be a patch set on its own with
data providing benefits from it.
(If we wanted to go an extra mile, we could also keep the nops when both
CONFIG_RISCV_USER_CFI and RISCV_ISA_EXT_ZICFISS are present, but
command line riscv_nousercfi disabled backward cfi.)
Thanks.
