From: Deepak Gupta <debug@rivosinc.com> To: samuel.holland@sifive.com Cc: Palmer Dabbelt <palmer@dabbelt.com>, linux-riscv@lists.infradead.org, devicetree@vger.kernel.org, Catalin Marinas <catalin.marinas@arm.com>, linux-kernel@vger.kernel.org, tech-j-ext@lists.risc-v.org, Conor Dooley <conor@kernel.org>, kasan-dev@googlegroups.com, Evgenii Stepanov <eugenis@google.com>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Rob Herring <robh+dt@kernel.org>, Andrew Jones <ajones@ventanamicro.com>, Guo Ren <guoren@kernel.org>, Heiko Stuebner <heiko@sntech.de>, Paul Walmsley <paul.walmsley@sifive.com> Subject: Re: [RISC-V] [tech-j-ext] [RFC PATCH 5/9] riscv: Split per-CPU and per-thread envcfg bits Date: Tue, 19 Mar 2024 16:55:47 -0700 [thread overview] Message-ID: <CAKC1njSg9-hJo6hibcM9a-=FUmMWyR39QUYqQ1uwiWhpBZQb9A@mail.gmail.com> (raw) In-Reply-To: <20240319215915.832127-6-samuel.holland@sifive.com> On Tue, Mar 19, 2024 at 2:59 PM Samuel Holland via lists.riscv.org <samuel.holland=sifive.com@lists.riscv.org> 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@sifive.com> > --- > > 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@lists.riscv.org > Unsubscribe: https://lists.riscv.org/g/tech-j-ext/unsub [debug@rivosinc.com] > -=-=-=-=-=-=-=-=-=-=-=- > >
WARNING: multiple messages have this Message-ID (diff)
From: Deepak Gupta <debug@rivosinc.com> To: samuel.holland@sifive.com Cc: Palmer Dabbelt <palmer@dabbelt.com>, linux-riscv@lists.infradead.org, devicetree@vger.kernel.org, Catalin Marinas <catalin.marinas@arm.com>, linux-kernel@vger.kernel.org, tech-j-ext@lists.risc-v.org, Conor Dooley <conor@kernel.org>, kasan-dev@googlegroups.com, Evgenii Stepanov <eugenis@google.com>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Rob Herring <robh+dt@kernel.org>, Andrew Jones <ajones@ventanamicro.com>, Guo Ren <guoren@kernel.org>, Heiko Stuebner <heiko@sntech.de>, Paul Walmsley <paul.walmsley@sifive.com> Subject: Re: [RISC-V] [tech-j-ext] [RFC PATCH 5/9] riscv: Split per-CPU and per-thread envcfg bits Date: Tue, 19 Mar 2024 16:55:47 -0700 [thread overview] Message-ID: <CAKC1njSg9-hJo6hibcM9a-=FUmMWyR39QUYqQ1uwiWhpBZQb9A@mail.gmail.com> (raw) In-Reply-To: <20240319215915.832127-6-samuel.holland@sifive.com> On Tue, Mar 19, 2024 at 2:59 PM Samuel Holland via lists.riscv.org <samuel.holland=sifive.com@lists.riscv.org> 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@sifive.com> > --- > > 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@lists.riscv.org > Unsubscribe: https://lists.riscv.org/g/tech-j-ext/unsub [debug@rivosinc.com] > -=-=-=-=-=-=-=-=-=-=-=- > > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2024-03-19 23:55 UTC|newest] Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-03-19 21:58 [RFC PATCH 0/9] riscv: Userspace pointer masking and tagged address ABI Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 21:58 ` [RFC PATCH 1/9] dt-bindings: riscv: Add pointer masking ISA extensions Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 21:58 ` [RFC PATCH 2/9] riscv: Add ISA extension parsing for pointer masking Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 21:58 ` [RFC PATCH 3/9] riscv: Add CSR definitions " Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 21:58 ` [RFC PATCH 4/9] riscv: Define is_compat_thread() Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 21:58 ` [RFC PATCH 5/9] riscv: Split per-CPU and per-thread envcfg bits Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 23:55 ` Deepak Gupta [this message] 2024-03-19 23:55 ` [RISC-V] [tech-j-ext] " Deepak Gupta 2024-03-20 2:20 ` Samuel Holland 2024-03-20 2:20 ` Samuel Holland 2024-03-20 4:39 ` Deepak Gupta 2024-03-20 4:39 ` Deepak Gupta 2024-03-22 0:13 ` Samuel Holland 2024-03-22 0:13 ` Samuel Holland 2024-03-22 17:13 ` Deepak Gupta 2024-03-22 17:13 ` Deepak Gupta 2024-03-23 9:35 ` Andrew Jones 2024-03-23 9:35 ` Andrew Jones 2024-03-23 20:37 ` Deepak Gupta 2024-03-23 20:37 ` Deepak Gupta 2024-03-22 8:09 ` Andrew Jones 2024-03-22 8:09 ` Andrew Jones 2024-03-22 16:52 ` Deepak Gupta 2024-03-22 16:52 ` Deepak Gupta 2024-03-20 8:06 ` Conor Dooley 2024-03-20 8:06 ` Conor Dooley [not found] ` <17BE5F38AFE245E5.29196@lists.riscv.org> 2024-03-20 23:27 ` Deepak Gupta 2024-03-20 23:27 ` Deepak Gupta 2024-03-22 3:43 ` Samuel Holland 2024-03-22 3:43 ` Samuel Holland 2024-03-22 7:58 ` Andrew Jones 2024-03-22 7:58 ` Andrew Jones 2024-03-28 1:58 ` Deepak Gupta 2024-03-28 1:58 ` Deepak Gupta [not found] ` <17C0CB122DBB0EAE.6770@lists.riscv.org> 2024-03-28 19:34 ` Deepak Gupta 2024-03-28 19:34 ` Deepak Gupta 2024-03-19 21:58 ` [RFC PATCH 6/9] riscv: Add support for userspace pointer masking Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 21:58 ` [RFC PATCH 7/9] riscv: Add support for the tagged address ABI Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 21:58 ` [RFC PATCH 8/9] riscv: Allow ptrace control of " Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 21:58 ` [RFC PATCH 9/9] selftests: riscv: Add a pointer masking test Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-20 17:21 ` Conor Dooley 2024-03-20 17:21 ` Conor Dooley 2024-03-20 18:04 ` Samuel Holland 2024-03-20 18:04 ` Samuel Holland
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAKC1njSg9-hJo6hibcM9a-=FUmMWyR39QUYqQ1uwiWhpBZQb9A@mail.gmail.com' \ --to=debug@rivosinc.com \ --cc=ajones@ventanamicro.com \ --cc=catalin.marinas@arm.com \ --cc=conor@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=eugenis@google.com \ --cc=guoren@kernel.org \ --cc=heiko@sntech.de \ --cc=kasan-dev@googlegroups.com \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=palmer@dabbelt.com \ --cc=paul.walmsley@sifive.com \ --cc=robh+dt@kernel.org \ --cc=samuel.holland@sifive.com \ --cc=tech-j-ext@lists.risc-v.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.