Re: [PATCH v3 2/3] riscv: Add support for per-thread envcfg CSR values

[Charlie Jenkins <charlie@rivosinc.com>]

On Wed, Jul 17, 2024 at 05:47:55PM -0700, Samuel Holland wrote:
> Some bits in the [ms]envcfg CSR, such as the CFI state and pointer
> masking mode, need to be controlled on a per-thread basis. Support this
> by keeping a copy of the CSR value in struct thread_struct and writing
> it during context switches. It is safe to discard the old CSR value
> during the context switch because the CSR is modified only by software,
> so the CSR will remain in sync with the copy in thread_struct.
> 
> Use ALTERNATIVE directly instead of riscv_has_extension_unlikely() to
> minimize branchiness in the context switching code.
> 
> Since thread_struct is copied during fork(), setting the value for the
> init task sets the default value for all other threads.
> 
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Deepak Gupta <debug@rivosinc.com>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>

Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>

> ---
> 
> (no changes since v1)
> 
>  arch/riscv/include/asm/processor.h | 1 +
>  arch/riscv/include/asm/switch_to.h | 8 ++++++++
>  arch/riscv/kernel/cpufeature.c     | 2 +-
>  3 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> index 8702b8721a27..586e4ab701c4 100644
> --- a/arch/riscv/include/asm/processor.h
> +++ b/arch/riscv/include/asm/processor.h
> @@ -124,6 +124,7 @@ struct thread_struct {
>  	unsigned long s[12];	/* s[0]: frame pointer */
>  	struct __riscv_d_ext_state fstate;
>  	unsigned long bad_cause;
> +	unsigned long envcfg;
>  	u32 riscv_v_flags;
>  	u32 vstate_ctrl;
>  	struct __riscv_v_ext_state vstate;
> diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h
> index 7594df37cc9f..9685cd85e57c 100644
> --- a/arch/riscv/include/asm/switch_to.h
> +++ b/arch/riscv/include/asm/switch_to.h
> @@ -70,6 +70,13 @@ 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)
> +{
> +	asm volatile (ALTERNATIVE("nop", "csrw " __stringify(CSR_ENVCFG) ", %0",
> +				  0, RISCV_ISA_EXT_XLINUXENVCFG, 1)
> +			:: "r" (next->thread.envcfg) : "memory");
> +}
> +
>  extern struct task_struct *__switch_to(struct task_struct *,
>  				       struct task_struct *);
>  
> @@ -103,6 +110,7 @@ do {							\
>  		__switch_to_vector(__prev, __next);	\
>  	if (switch_to_should_flush_icache(__next))	\
>  		local_flush_icache_all();		\
> +	__switch_to_envcfg(__next);			\
>  	((last) = __switch_to(__prev, __next));		\
>  } while (0)
>  
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 4efaf12b470e..43fdae953379 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -878,7 +878,7 @@ unsigned long riscv_get_elf_hwcap(void)
>  void riscv_user_isa_enable(void)
>  {
>  	if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOZ))
> -		csr_set(CSR_ENVCFG, ENVCFG_CBZE);
> +		current->thread.envcfg |= ENVCFG_CBZE;
>  	else if (any_cpu_has_zicboz)
>  		pr_warn_once("Zicboz disabled as it is unavailable on some harts\n");
>  }
> -- 
> 2.45.1
>