| Message ID | 20230125142056.18356-10-andy.chiu@sifive.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Palmer Dabbelt |
| Headers | show |
| Series | riscv: Add vector ISA support | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Series has a cover letter |
| conchuod/tree_selection | success | Guessed tree name to be for-next |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 13 and now 13 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | fail | Failed to build the tree with this patch. |
| conchuod/alphanumeric_selects | success | Out of order selects before the patch: 57 and now 57 |
| conchuod/build_rv32_defconfig | fail | Build failed |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 2 this patch: 2 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 164 lines checked |
| conchuod/source_inline | success | Was 0 now: 0 |
| conchuod/build_rv64_nommu_k210_defconfig | fail | Build failed |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | fail | Build failed |
On Wed, Jan 25, 2023 at 02:20:46PM +0000, Andy Chiu wrote: > From: Greentime Hu <greentime.hu@sifive.com> > > This patch adds task switch support for vector. It also supports all > lengths of vlen. > > [guoren@linux.alibaba.com: First available porting to support vector > context switching] > [nick.knight@sifive.com: Rewrite vector.S to support dynamic vlen, xlen and > code refine] > [vincent.chen@sifive.com: Fix the might_sleep issue in vstate_save, > vstate_restore] > [andrew@sifive.com: Optimize task switch codes of vector] > [ruinland.tsai@sifive.com: Fix the arch_release_task_struct free wrong > datap issue] > [vineetg: Fixed lkp warning with W=1 build] > [andy.chiu: Use inline asm for task switches] > > Suggested-by: Andrew Waterman <andrew@sifive.com> > Co-developed-by: Nick Knight <nick.knight@sifive.com> > Signed-off-by: Nick Knight <nick.knight@sifive.com> > Co-developed-by: Guo Ren <guoren@linux.alibaba.com> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > Co-developed-by: Vincent Chen <vincent.chen@sifive.com> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com> > Co-developed-by: Ruinland Tsai <ruinland.tsai@sifive.com> > Signed-off-by: Ruinland Tsai <ruinland.tsai@sifive.com> > Signed-off-by: Greentime Hu <greentime.hu@sifive.com> > Signed-off-by: Vineet Gupta <vineetg@rivosinc.com> > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> More comments about what people did than patch description, lol! Anyways, this patch breaks the build for every config we have, so please fix that when you are re-submitting: https://patchwork.kernel.org/project/linux-riscv/patch/20230125142056.18356-10-andy.chiu@sifive.com/ Any of allmodconfig, rv32_defconfig, nommu_{k210,virt}_defconfig should reproduce with gcc 12.2 - but I have no idea if it's the same same failures for all 4. > --- > arch/riscv/include/asm/processor.h | 1 + > arch/riscv/include/asm/switch_to.h | 18 ++++++++++++++++++ > arch/riscv/include/asm/thread_info.h | 3 +++ > arch/riscv/include/asm/vector.h | 26 ++++++++++++++++++++++++++ > arch/riscv/kernel/process.c | 18 ++++++++++++++++++ > arch/riscv/kernel/traps.c | 14 ++++++++++++-- > 6 files changed, 78 insertions(+), 2 deletions(-) > > diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h > index 94a0590c6971..44d2eb381ca6 100644 > --- a/arch/riscv/include/asm/processor.h > +++ b/arch/riscv/include/asm/processor.h > @@ -39,6 +39,7 @@ struct thread_struct { > unsigned long s[12]; /* s[0]: frame pointer */ > struct __riscv_d_ext_state fstate; > unsigned long bad_cause; > + struct __riscv_v_state vstate; __riscv_d_ext_state __riscv_v_state :thinking: These should ideally match, probably no harm in adding the _ext to the v one, no? > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > index 549bde5c970a..1a48ff89b2b5 100644 > --- a/arch/riscv/kernel/traps.c > +++ b/arch/riscv/kernel/traps.c > @@ -24,6 +24,7 @@ > #include <asm/processor.h> > #include <asm/ptrace.h> > #include <asm/thread_info.h> > +#include <asm/vector.h> > > int show_unhandled_signals = 1; > > @@ -111,8 +112,17 @@ DO_ERROR_INFO(do_trap_insn_misaligned, > SIGBUS, BUS_ADRALN, "instruction address misaligned"); > DO_ERROR_INFO(do_trap_insn_fault, > SIGSEGV, SEGV_ACCERR, "instruction access fault"); > -DO_ERROR_INFO(do_trap_insn_illegal, > - SIGILL, ILL_ILLOPC, "illegal instruction"); > + > +asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *regs) > +{ > + if (has_vector() && user_mode(regs)) { > + if (rvv_first_use_handler(regs)) And there's your build error, as this function is only added in the next patch. Thanks, Conor. > + return; > + } > + do_trap_error(regs, SIGILL, ILL_ILLOPC, regs->epc, > + "Oops - illegal instruction"); > +}
Hi Andy, For some reason I was looking closely at this patch today. On 1/25/23 06:20, Andy Chiu wrote: > /* Whitelist the fstate from the task_struct for hardened usercopy */ > diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h > index df1aa589b7fd..69e24140195d 100644 > --- a/arch/riscv/include/asm/switch_to.h > +++ b/arch/riscv/include/asm/switch_to.h > @@ -8,6 +8,7 @@ > > #include <linux/jump_label.h> > #include <linux/sched/task_stack.h> > +#include <asm/vector.h> > #include <asm/hwcap.h> > #include <asm/processor.h> > #include <asm/ptrace.h> > @@ -68,6 +69,21 @@ static __always_inline bool has_fpu(void) { return false; } > #define __switch_to_fpu(__prev, __next) do { } while (0) > #endif > > +#ifdef CONFIG_RISCV_ISA_V > +static inline void __switch_to_vector(struct task_struct *prev, > + struct task_struct *next) > +{ > + struct pt_regs *regs; > + > + regs = task_pt_regs(prev); > + if (unlikely(regs->status & SR_SD)) Do we really need to check SR_SD, isn't checking for SR_VS_DIRTY enough. If yes, we can remove the check here and keep the existing one on vstate_save() > + vstate_save(prev, regs); > + vstate_restore(next, task_pt_regs(next)); > +} > +#else /* ! CONFIG_RISCV_ISA_V */ > +#define __switch_to_vector(__prev, __next) do { } while (0) > +#endif /* CONFIG_RISCV_ISA_V */ > + Can we de-lutter switch_to.h some more and move both the definitions of __switch_to_vector into vector.h ? > extern struct task_struct *__switch_to(struct task_struct *, > struct task_struct *); > > @@ -77,6 +93,8 @@ do { \ > struct task_struct *__next = (next); \ > if (has_fpu()) \ > __switch_to_fpu(__prev, __next); \ > + if (has_vector()) \ > + __switch_to_vector(__prev, __next); \ > ((last) = __switch_to(__prev, __next)); \ > } while (0) >
diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h index 94a0590c6971..44d2eb381ca6 100644 --- a/arch/riscv/include/asm/processor.h +++ b/arch/riscv/include/asm/processor.h @@ -39,6 +39,7 @@ struct thread_struct { unsigned long s[12]; /* s[0]: frame pointer */ struct __riscv_d_ext_state fstate; unsigned long bad_cause; + struct __riscv_v_state vstate; }; /* Whitelist the fstate from the task_struct for hardened usercopy */ diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h index df1aa589b7fd..69e24140195d 100644 --- a/arch/riscv/include/asm/switch_to.h +++ b/arch/riscv/include/asm/switch_to.h @@ -8,6 +8,7 @@ #include <linux/jump_label.h> #include <linux/sched/task_stack.h> +#include <asm/vector.h> #include <asm/hwcap.h> #include <asm/processor.h> #include <asm/ptrace.h> @@ -68,6 +69,21 @@ static __always_inline bool has_fpu(void) { return false; } #define __switch_to_fpu(__prev, __next) do { } while (0) #endif +#ifdef CONFIG_RISCV_ISA_V +static inline void __switch_to_vector(struct task_struct *prev, + struct task_struct *next) +{ + struct pt_regs *regs; + + regs = task_pt_regs(prev); + if (unlikely(regs->status & SR_SD)) + vstate_save(prev, regs); + vstate_restore(next, task_pt_regs(next)); +} +#else /* ! CONFIG_RISCV_ISA_V */ +#define __switch_to_vector(__prev, __next) do { } while (0) +#endif /* CONFIG_RISCV_ISA_V */ + extern struct task_struct *__switch_to(struct task_struct *, struct task_struct *); @@ -77,6 +93,8 @@ do { \ struct task_struct *__next = (next); \ if (has_fpu()) \ __switch_to_fpu(__prev, __next); \ + if (has_vector()) \ + __switch_to_vector(__prev, __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 67322f878e0d..2f0f0d7d0fc0 100644 --- a/arch/riscv/include/asm/thread_info.h +++ b/arch/riscv/include/asm/thread_info.h @@ -79,6 +79,9 @@ struct thread_info { .preempt_count = INIT_PREEMPT_COUNT, \ } +void arch_release_task_struct(struct task_struct *tsk); +int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src); + #endif /* !__ASSEMBLY__ */ /* diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h index 842a859609b5..f8a9e37c4374 100644 --- a/arch/riscv/include/asm/vector.h +++ b/arch/riscv/include/asm/vector.h @@ -10,6 +10,8 @@ #ifdef CONFIG_RISCV_ISA_V +#include <linux/sched.h> +#include <asm/ptrace.h> #include <asm/hwcap.h> #include <asm/csr.h> #include <asm/asm.h> @@ -109,6 +111,28 @@ static inline void __vstate_restore(struct __riscv_v_state *restore_from, rvv_disable(); } +static inline void vstate_save(struct task_struct *task, + struct pt_regs *regs) +{ + if ((regs->status & SR_VS) == SR_VS_DIRTY) { + struct __riscv_v_state *vstate = &task->thread.vstate; + + __vstate_save(vstate, vstate->datap); + __vstate_clean(regs); + } +} + +static inline void vstate_restore(struct task_struct *task, + struct pt_regs *regs) +{ + if ((regs->status & SR_VS) != SR_VS_OFF) { + struct __riscv_v_state *vstate = &task->thread.vstate; + + __vstate_restore(vstate, vstate->datap); + __vstate_clean(regs); + } +} + #else /* ! CONFIG_RISCV_ISA_V */ struct pt_regs; @@ -116,6 +140,8 @@ struct pt_regs; static __always_inline bool has_vector(void) { return false; } static inline bool vstate_query(struct pt_regs *regs) { return false; } #define riscv_vsize (0) +#define vstate_save(task, regs) do {} while (0) +#define vstate_restore(task, regs) do {} while (0) #define vstate_off(regs) do {} while (0) #define vstate_on(regs) do {} while (0) diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c index 8955f2432c2d..d4860c6c5197 100644 --- a/arch/riscv/kernel/process.c +++ b/arch/riscv/kernel/process.c @@ -24,6 +24,7 @@ #include <asm/switch_to.h> #include <asm/thread_info.h> #include <asm/cpuidle.h> +#include <asm/vector.h> register unsigned long gp_in_global __asm__("gp"); @@ -148,12 +149,28 @@ void flush_thread(void) fstate_off(current, task_pt_regs(current)); memset(¤t->thread.fstate, 0, sizeof(current->thread.fstate)); #endif +#ifdef CONFIG_RISCV_ISA_V + /* Reset vector state */ + vstate_off(task_pt_regs(current)); + kfree(current->thread.vstate.datap); + memset(¤t->thread.vstate, 0, sizeof(struct __riscv_v_state)); +#endif +} + +void arch_release_task_struct(struct task_struct *tsk) +{ + /* Free the vector context of datap. */ + if (has_vector() && tsk->thread.vstate.datap) + kfree(tsk->thread.vstate.datap); } int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src) { fstate_save(src, task_pt_regs(src)); *dst = *src; + /* clear entire V context, including datap for a new task */ + memset(&dst->thread.vstate, 0, sizeof(struct __riscv_v_state)); + return 0; } @@ -186,6 +203,7 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args) childregs->a0 = 0; /* Return value of fork() */ p->thread.ra = (unsigned long)ret_from_fork; } + vstate_off(childregs); p->thread.sp = (unsigned long)childregs; /* kernel sp */ return 0; } diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index 549bde5c970a..1a48ff89b2b5 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -24,6 +24,7 @@ #include <asm/processor.h> #include <asm/ptrace.h> #include <asm/thread_info.h> +#include <asm/vector.h> int show_unhandled_signals = 1; @@ -111,8 +112,17 @@ DO_ERROR_INFO(do_trap_insn_misaligned, SIGBUS, BUS_ADRALN, "instruction address misaligned"); DO_ERROR_INFO(do_trap_insn_fault, SIGSEGV, SEGV_ACCERR, "instruction access fault"); -DO_ERROR_INFO(do_trap_insn_illegal, - SIGILL, ILL_ILLOPC, "illegal instruction"); + +asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *regs) +{ + if (has_vector() && user_mode(regs)) { + if (rvv_first_use_handler(regs)) + return; + } + do_trap_error(regs, SIGILL, ILL_ILLOPC, regs->epc, + "Oops - illegal instruction"); +} + DO_ERROR_INFO(do_trap_load_fault, SIGSEGV, SEGV_ACCERR, "load access fault"); #ifndef CONFIG_RISCV_M_MODE