On Tue, Mar 19, 2024 at 2:59 PM Samuel Holland via lists.riscv.org
<samuel.holland=sifive.com@xxxxxxxxxxxxxxx> wrote:
>
> Some envcfg bits need to be controlled on a per-thread basis, such as
> the pointer masking mode. However, the envcfg CSR value cannot simply be
> stored in struct thread_struct, because some hardware may implement a
> different subset of envcfg CSR bits is across CPUs. As a result, we need
> to combine the per-CPU and per-thread bits whenever we switch threads.
>
Why not do something like this
diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index b3400517b0a9..01ba87954da2 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -202,6 +202,8 @@
#define ENVCFG_CBIE_FLUSH _AC(0x1, UL)
#define ENVCFG_CBIE_INV _AC(0x3, UL)
#define ENVCFG_FIOM _AC(0x1, UL)
+/* by default all threads should be able to zero cache */
+#define ENVCFG_BASE ENVCFG_CBZE
/* Smstateen bits */
#define SMSTATEEN0_AIA_IMSIC_SHIFT 58
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index 4f21d970a129..2420123444c4 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -152,6 +152,7 @@ void start_thread(struct pt_regs *regs, unsigned long pc,
else
regs->status |= SR_UXL_64;
#endif
+ current->thread_info.envcfg = ENVCFG_BASE;
}
And instead of context switching in `_switch_to`,
In `entry.S` pick up `envcfg` from `thread_info` and write it into CSR.
This construction avoids
- declaring per cpu riscv_cpu_envcfg
- syncing up
- collection of *envcfg bits.
> Signed-off-by: Samuel Holland <samuel.holland@xxxxxxxxxx>
> ---
>
> arch/riscv/include/asm/cpufeature.h | 2 ++
> arch/riscv/include/asm/processor.h | 1 +
> arch/riscv/include/asm/switch_to.h | 12 ++++++++++++
> arch/riscv/kernel/cpufeature.c | 4 +++-
> 4 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
> index 0bd11862b760..b1ad8d0b4599 100644
> --- a/arch/riscv/include/asm/cpufeature.h
> +++ b/arch/riscv/include/asm/cpufeature.h
> @@ -33,6 +33,8 @@ DECLARE_PER_CPU(long, misaligned_access_speed);
> /* Per-cpu ISA extensions. */
> extern struct riscv_isainfo hart_isa[NR_CPUS];
>
> +DECLARE_PER_CPU(unsigned long, riscv_cpu_envcfg);
> +
> void riscv_user_isa_enable(void);
>
> #ifdef CONFIG_RISCV_MISALIGNED
> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> index a8509cc31ab2..06b87402a4d8 100644
> --- a/arch/riscv/include/asm/processor.h
> +++ b/arch/riscv/include/asm/processor.h
> @@ -118,6 +118,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 7efdb0584d47..256a354a5c4a 100644
> --- a/arch/riscv/include/asm/switch_to.h
> +++ b/arch/riscv/include/asm/switch_to.h
> @@ -69,6 +69,17 @@ static __always_inline bool has_fpu(void) { return false; }
> #define __switch_to_fpu(__prev, __next) do { } while (0)
> #endif
>
> +static inline void sync_envcfg(struct task_struct *task)
> +{
> + csr_write(CSR_ENVCFG, this_cpu_read(riscv_cpu_envcfg) | task->thread.envcfg);
> +}
> +
> +static inline void __switch_to_envcfg(struct task_struct *next)
> +{
> + if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_XLINUXENVCFG))
I've seen `riscv_cpu_has_extension_unlikely` generating branchy code
even if ALTERNATIVES was turned on.
Can you check disasm on your end as well. IMHO, `entry.S` is a better
place to pick up *envcfg.
> + sync_envcfg(next);
> +}
> +
> extern struct task_struct *__switch_to(struct task_struct *,
> struct task_struct *);
>
> @@ -80,6 +91,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/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index d1846aab1f78..32aaaf41f8a8 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -44,6 +44,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
> /* Per-cpu ISA extensions. */
> struct riscv_isainfo hart_isa[NR_CPUS];
>
> +DEFINE_PER_CPU(unsigned long, riscv_cpu_envcfg);
> +
> /* Performance information */
> DEFINE_PER_CPU(long, misaligned_access_speed);
>
> @@ -978,7 +980,7 @@ arch_initcall(check_unaligned_access_all_cpus);
> void riscv_user_isa_enable(void)
> {
> if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ))
> - csr_set(CSR_ENVCFG, ENVCFG_CBZE);
> + this_cpu_or(riscv_cpu_envcfg, ENVCFG_CBZE);
> }
>
> #ifdef CONFIG_RISCV_ALTERNATIVE
> --
> 2.43.1
>
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#659): https://lists.riscv.org/g/tech-j-ext/message/659
> Mute This Topic: https://lists.riscv.org/mt/105033914/7300952
> Group Owner: tech-j-ext+owner@xxxxxxxxxxxxxxx
> Unsubscribe: https://lists.riscv.org/g/tech-j-ext/unsub [debug@xxxxxxxxxxxx]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
