2024-04-03 23:42:49

by Deepak Gupta

Subject: [PATCH v3 01/29] riscv: envcfg save and restore on task switching

envcfg CSR defines enabling bits for cache management instructions and
soon will control enabling for control flow integrity and pointer
masking features.

Control flow integrity enabling for forward cfi and backward cfi are
controlled via envcfg and thus need to be enabled on per thread basis.

This patch creates a place holder for envcfg CSR in `thread_info` and
adds logic to save and restore on task switching.

Signed-off-by: Deepak Gupta <[email protected]>
---
arch/riscv/include/asm/switch_to.h | 10 ++++++++++
arch/riscv/include/asm/thread_info.h | 1 +
2 files changed, 11 insertions(+)

diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h
index 7efdb0584d47..2d9a00a30394 100644
--- a/arch/riscv/include/asm/switch_to.h
+++ b/arch/riscv/include/asm/switch_to.h
@@ -69,6 +69,15 @@ static __always_inline bool has_fpu(void) { return false; }
#define __switch_to_fpu(__prev, __next) do { } while (0)
#endif

+static inline void __switch_to_envcfg(struct task_struct *next)
+{
+ register unsigned long envcfg = next->thread_info.envcfg;
+
+ asm volatile (ALTERNATIVE("nop", "csrw " __stringify(CSR_ENVCFG) ", %0", 0,
+ RISCV_ISA_EXT_XLINUXENVCFG, 1)
+ :: "r" (envcfg) : "memory");
+}
+
extern struct task_struct *__switch_to(struct task_struct *,
struct task_struct *);

@@ -80,6 +89,7 @@ do { \
__switch_to_fpu(__prev, __next); \
if (has_vector()) \
__switch_to_vector(__prev, __next); \
+ __switch_to_envcfg(__next); \
((last) = __switch_to(__prev, __next)); \
} while (0)

diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
index 5d473343634b..a503bdc2f6dd 100644
--- a/arch/riscv/include/asm/thread_info.h
+++ b/arch/riscv/include/asm/thread_info.h
@@ -56,6 +56,7 @@ struct thread_info {
long user_sp; /* User stack pointer */
int cpu;
unsigned long syscall_work; /* SYSCALL_WORK_ flags */
+ unsigned long envcfg;
#ifdef CONFIG_SHADOW_CALL_STACK
void *scs_base;
void *scs_sp;
--
2.43.2

2024-05-09 00:11:18

by Charlie Jenkins

Subject: Re: [PATCH v3 01/29] riscv: envcfg save and restore on task switching

On Wed, Apr 03, 2024 at 04:34:49PM -0700, Deepak Gupta wrote:
> envcfg CSR defines enabling bits for cache management instructions and
> soon will control enabling for control flow integrity and pointer
> masking features.
>
> Control flow integrity enabling for forward cfi and backward cfi are
> controlled via envcfg and thus need to be enabled on per thread basis.
>
> This patch creates a place holder for envcfg CSR in `thread_info` and
> adds logic to save and restore on task switching.
>
> Signed-off-by: Deepak Gupta <[email protected]>
> ---
> arch/riscv/include/asm/switch_to.h | 10 ++++++++++
> arch/riscv/include/asm/thread_info.h | 1 +
> 2 files changed, 11 insertions(+)
>
> diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h
> index 7efdb0584d47..2d9a00a30394 100644
> --- a/arch/riscv/include/asm/switch_to.h
> +++ b/arch/riscv/include/asm/switch_to.h
> @@ -69,6 +69,15 @@ static __always_inline bool has_fpu(void) { return false; }
> #define __switch_to_fpu(__prev, __next) do { } while (0)
> #endif
>
> +static inline void __switch_to_envcfg(struct task_struct *next)
> +{
> + register unsigned long envcfg = next->thread_info.envcfg;

This doesn't need the register storage class.

> +
> + asm volatile (ALTERNATIVE("nop", "csrw " __stringify(CSR_ENVCFG) ", %0", 0,
> + RISCV_ISA_EXT_XLINUXENVCFG, 1)
> + :: "r" (envcfg) : "memory");
> +}
> +

Something like:

static inline void __switch_to_envcfg(struct task_struct *next)
{
if (riscv_has_extension_unlikely(RISCV_ISA_EXT_XLINUXENVCFG))
csr_write(CSR_ENVCFG, next->thread_info.envcfg);
}

would be easier to read, but the alternative you have written doesn't
have the jump that riscv_has_extension_unlikely has so what you have
will be more performant.

Does envcfg need to be save/restored always or just with
CONFIG_RISCV_USER_CFI?

- Charlie

> extern struct task_struct *__switch_to(struct task_struct *,
> struct task_struct *);
>
> @@ -80,6 +89,7 @@ do { \
> __switch_to_fpu(__prev, __next); \
> if (has_vector()) \
> __switch_to_vector(__prev, __next); \
> + __switch_to_envcfg(__next); \
> ((last) = __switch_to(__prev, __next)); \
> } while (0)
>
> diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
> index 5d473343634b..a503bdc2f6dd 100644
> --- a/arch/riscv/include/asm/thread_info.h
> +++ b/arch/riscv/include/asm/thread_info.h
> @@ -56,6 +56,7 @@ struct thread_info {
> long user_sp; /* User stack pointer */
> int cpu;
> unsigned long syscall_work; /* SYSCALL_WORK_ flags */
> + unsigned long envcfg;
> #ifdef CONFIG_SHADOW_CALL_STACK
> void *scs_base;
> void *scs_sp;
> --
> 2.43.2
>