| Message ID | 20230518161949.11203-12-andy.chiu@sifive.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | riscv: Add vector ISA support | expand |
On Thu, May 18, 2023 at 04:19:34PM +0000, Andy Chiu wrote: > Vector unit is disabled by default for all user processes. Thus, a > process will take a trap (illegal instruction) into kernel at the first > time when it uses Vector. Only after then, the kernel allocates V > context and starts take care of the context for that user process. > > Suggested-by: Richard Henderson <richard.henderson@linaro.org> > Link: https://lore.kernel.org/r/3923eeee-e4dc-0911-40bf-84c34aee962d@linaro.org > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > --- > Hey Heiko and Conor, I am dropping you guys' A-b, T-b, and R-b because I > added a check in riscv_v_first_use_handler(). > +bool riscv_v_first_use_handler(struct pt_regs *regs) > +{ > + u32 __user *epc = (u32 __user *)regs->epc; > + u32 insn = (u32)regs->badaddr; > + > + /* Do not handle if V is not supported, or disabled */ > + if (!has_vector() || !(elf_hwcap & COMPAT_HWCAP_ISA_V)) > + return false; Remind me please, in what situation is this actually even possible? The COMPAT_HWCAP_ISA_V flag only gets set if CONFIG_RISCV_ISA_V is enabled & v is in the DT. has_vector() is backed by different things whether alternatives are enabled or not. With alternatives, it depends on the bit being set in the riscv_isa bitmap & the Kconfig option. Without alternatives it is backed by __riscv_isa_extension_available() which only depends in the riscv_isa bitmap. Since the bit in the bitmap does not get cleared if CONFIG_RISCV_ISA_V is not set, unlike the elf_hwcap bit which does, it seems like this might be the condition you are trying to prevent? If so, Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Otherwise, please let me know where I have gone wrong! Thanks, Conor.
Hi Conor, On Fri, May 19, 2023 at 1:47 AM Conor Dooley <conor@kernel.org> wrote: > > On Thu, May 18, 2023 at 04:19:34PM +0000, Andy Chiu wrote: > > Vector unit is disabled by default for all user processes. Thus, a > > process will take a trap (illegal instruction) into kernel at the first > > time when it uses Vector. Only after then, the kernel allocates V > > context and starts take care of the context for that user process. > > > > Suggested-by: Richard Henderson <richard.henderson@linaro.org> > > Link: https://lore.kernel.org/r/3923eeee-e4dc-0911-40bf-84c34aee962d@linaro.org > > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > > --- > > Hey Heiko and Conor, I am dropping you guys' A-b, T-b, and R-b because I > > added a check in riscv_v_first_use_handler(). > > > +bool riscv_v_first_use_handler(struct pt_regs *regs) > > +{ > > + u32 __user *epc = (u32 __user *)regs->epc; > > + u32 insn = (u32)regs->badaddr; > > + > > + /* Do not handle if V is not supported, or disabled */ > > + if (!has_vector() || !(elf_hwcap & COMPAT_HWCAP_ISA_V)) > > + return false; > > Remind me please, in what situation is this actually even possible? > The COMPAT_HWCAP_ISA_V flag only gets set if CONFIG_RISCV_ISA_V is > enabled & v is in the DT. > has_vector() is backed by different things whether alternatives are > enabled or not. With alternatives, it depends on the bit being set in > the riscv_isa bitmap & the Kconfig option. > Without alternatives it is backed by __riscv_isa_extension_available() > which only depends in the riscv_isa bitmap. > Since the bit in the bitmap does not get cleared if CONFIG_RISCV_ISA_V > is not set, unlike the elf_hwcap bit which does, it seems like this > might be the condition you are trying to prevent? > In fact the case you mentioned is prevented by Kconfig itself. To be more specific, riscv_v_first_use_handler() always returns false if CONFIG_RISCV_ISA_V is not set. In such config, the function is defined as an inline that returns false in include/asm/vector.h, and kernl/vector.c is not compiled. The case that I intended to protect is another scenario. e.g. If a multicore system has different VLENs across cores, with CONFIG_RISCV_ISA_V set. Since this series assumes an SMP system, it turns off V in ELF_HWCAP if it detects uneven VLENs during smp boot. In this case we must not handle the first-use trap if the user still executes V instruction anyway. > If so, > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > > Otherwise, please let me know where I have gone wrong! > > Thanks, > Conor. Thanks, Andy
On Thu, 18 May 2023 09:19:34 PDT (-0700), andy.chiu@sifive.com wrote: > Vector unit is disabled by default for all user processes. Thus, a > process will take a trap (illegal instruction) into kernel at the first > time when it uses Vector. Only after then, the kernel allocates V > context and starts take care of the context for that user process. > > Suggested-by: Richard Henderson <richard.henderson@linaro.org> > Link: https://lore.kernel.org/r/3923eeee-e4dc-0911-40bf-84c34aee962d@linaro.org > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > --- > Hey Heiko and Conor, I am dropping you guys' A-b, T-b, and R-b because I > added a check in riscv_v_first_use_handler(). > > Changelog v20: > - move has_vector() into vector.c for better code readibility > - check elf_hwcap in the first-use trap because it might get turned off > if cores have different VLENs. > > Changelog v18: > - Add blank lines (Heiko) > - Return immediately in insn_is_vector() if an insn matches (Heiko) > --- > arch/riscv/include/asm/insn.h | 29 ++++++++++ > arch/riscv/include/asm/vector.h | 2 + > arch/riscv/kernel/traps.c | 26 ++++++++- > arch/riscv/kernel/vector.c | 95 +++++++++++++++++++++++++++++++++ > 4 files changed, 150 insertions(+), 2 deletions(-) > > diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h > index 8d5c84f2d5ef..4e1505cef8aa 100644 > --- a/arch/riscv/include/asm/insn.h > +++ b/arch/riscv/include/asm/insn.h > @@ -137,6 +137,26 @@ > #define RVG_OPCODE_JALR 0x67 > #define RVG_OPCODE_JAL 0x6f > #define RVG_OPCODE_SYSTEM 0x73 > +#define RVG_SYSTEM_CSR_OFF 20 > +#define RVG_SYSTEM_CSR_MASK GENMASK(12, 0) > + > +/* parts of opcode for RVF, RVD and RVQ */ > +#define RVFDQ_FL_FS_WIDTH_OFF 12 > +#define RVFDQ_FL_FS_WIDTH_MASK GENMASK(3, 0) > +#define RVFDQ_FL_FS_WIDTH_W 2 > +#define RVFDQ_FL_FS_WIDTH_D 3 > +#define RVFDQ_LS_FS_WIDTH_Q 4 > +#define RVFDQ_OPCODE_FL 0x07 > +#define RVFDQ_OPCODE_FS 0x27 > + > +/* parts of opcode for RVV */ > +#define RVV_OPCODE_VECTOR 0x57 > +#define RVV_VL_VS_WIDTH_8 0 > +#define RVV_VL_VS_WIDTH_16 5 > +#define RVV_VL_VS_WIDTH_32 6 > +#define RVV_VL_VS_WIDTH_64 7 > +#define RVV_OPCODE_VL RVFDQ_OPCODE_FL > +#define RVV_OPCODE_VS RVFDQ_OPCODE_FS > > /* parts of opcode for RVC*/ > #define RVC_OPCODE_C0 0x0 > @@ -304,6 +324,15 @@ static __always_inline bool riscv_insn_is_branch(u32 code) > (RVC_X(x_, RVC_B_IMM_7_6_OPOFF, RVC_B_IMM_7_6_MASK) << RVC_B_IMM_7_6_OFF) | \ > (RVC_IMM_SIGN(x_) << RVC_B_IMM_SIGN_OFF); }) > > +#define RVG_EXTRACT_SYSTEM_CSR(x) \ > + ({typeof(x) x_ = (x); RV_X(x_, RVG_SYSTEM_CSR_OFF, RVG_SYSTEM_CSR_MASK); }) > + > +#define RVFDQ_EXTRACT_FL_FS_WIDTH(x) \ > + ({typeof(x) x_ = (x); RV_X(x_, RVFDQ_FL_FS_WIDTH_OFF, \ > + RVFDQ_FL_FS_WIDTH_MASK); }) > + > +#define RVV_EXRACT_VL_VS_WIDTH(x) RVFDQ_EXTRACT_FL_FS_WIDTH(x) > + > /* > * Get the immediate from a J-type instruction. > * > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h > index ce6a75e9cf62..8e56da67b5cf 100644 > --- a/arch/riscv/include/asm/vector.h > +++ b/arch/riscv/include/asm/vector.h > @@ -21,6 +21,7 @@ > > extern unsigned long riscv_v_vsize; > int riscv_v_setup_vsize(void); > +bool riscv_v_first_use_handler(struct pt_regs *regs); > > static __always_inline bool has_vector(void) > { > @@ -165,6 +166,7 @@ struct pt_regs; > > static inline int riscv_v_setup_vsize(void) { return -EOPNOTSUPP; } > static __always_inline bool has_vector(void) { return false; } > +static inline bool riscv_v_first_use_handler(struct pt_regs *regs) { return false; } > static inline bool riscv_v_vstate_query(struct pt_regs *regs) { return false; } > #define riscv_v_vsize (0) > #define riscv_v_vstate_save(task, regs) do {} while (0) > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > index 8c258b78c925..05ffdcd1424e 100644 > --- a/arch/riscv/kernel/traps.c > +++ b/arch/riscv/kernel/traps.c > @@ -26,6 +26,7 @@ > #include <asm/ptrace.h> > #include <asm/syscall.h> > #include <asm/thread_info.h> > +#include <asm/vector.h> > > int show_unhandled_signals = 1; > > @@ -145,8 +146,29 @@ 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 (user_mode(regs)) { > + irqentry_enter_from_user_mode(regs); > + > + local_irq_enable(); > + > + if (!riscv_v_first_use_handler(regs)) > + do_trap_error(regs, SIGILL, ILL_ILLOPC, regs->epc, > + "Oops - illegal instruction"); > + > + irqentry_exit_to_user_mode(regs); > + } else { > + irqentry_state_t state = irqentry_nmi_enter(regs); > + > + do_trap_error(regs, SIGILL, ILL_ILLOPC, regs->epc, > + "Oops - illegal instruction"); > + > + irqentry_nmi_exit(regs, state); > + } > +} > + > DO_ERROR_INFO(do_trap_load_fault, > SIGSEGV, SEGV_ACCERR, "load access fault"); > #ifndef CONFIG_RISCV_M_MODE > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c > index 120f1ce9abf9..0080798e8d2e 100644 > --- a/arch/riscv/kernel/vector.c > +++ b/arch/riscv/kernel/vector.c > @@ -4,10 +4,19 @@ > * Author: Andy Chiu <andy.chiu@sifive.com> > */ > #include <linux/export.h> > +#include <linux/sched/signal.h> > +#include <linux/types.h> > +#include <linux/slab.h> > +#include <linux/sched.h> > +#include <linux/uaccess.h> > > +#include <asm/thread_info.h> > +#include <asm/processor.h> > +#include <asm/insn.h> > #include <asm/vector.h> > #include <asm/csr.h> > #include <asm/elf.h> > +#include <asm/ptrace.h> > #include <asm/bug.h> > > unsigned long riscv_v_vsize __read_mostly; > @@ -34,3 +43,89 @@ int riscv_v_setup_vsize(void) > > return 0; > } > + > +static bool insn_is_vector(u32 insn_buf) > +{ > + u32 opcode = insn_buf & __INSN_OPCODE_MASK; > + u32 width, csr; > + > + /* > + * All V-related instructions, including CSR operations are 4-Byte. So, > + * do not handle if the instruction length is not 4-Byte. > + */ > + if (unlikely(GET_INSN_LENGTH(insn_buf) != 4)) > + return false; > + > + switch (opcode) { > + case RVV_OPCODE_VECTOR: > + return true; > + case RVV_OPCODE_VL: > + case RVV_OPCODE_VS: > + width = RVV_EXRACT_VL_VS_WIDTH(insn_buf); > + if (width == RVV_VL_VS_WIDTH_8 || width == RVV_VL_VS_WIDTH_16 || > + width == RVV_VL_VS_WIDTH_32 || width == RVV_VL_VS_WIDTH_64) > + return true; > + > + break; > + case RVG_OPCODE_SYSTEM: > + csr = RVG_EXTRACT_SYSTEM_CSR(insn_buf); > + if ((csr >= CSR_VSTART && csr <= CSR_VCSR) || > + (csr >= CSR_VL && csr <= CSR_VLENB)) > + return true; > + } > + > + return false; > +} > + > +static int riscv_v_thread_zalloc(void) > +{ > + void *datap; > + > + datap = kzalloc(riscv_v_vsize, GFP_KERNEL); > + if (!datap) > + return -ENOMEM; > + > + current->thread.vstate.datap = datap; > + memset(¤t->thread.vstate, 0, offsetof(struct __riscv_v_ext_state, > + datap)); > + return 0; > +} > + > +bool riscv_v_first_use_handler(struct pt_regs *regs) > +{ > + u32 __user *epc = (u32 __user *)regs->epc; > + u32 insn = (u32)regs->badaddr; > + > + /* Do not handle if V is not supported, or disabled */ > + if (!has_vector() || !(elf_hwcap & COMPAT_HWCAP_ISA_V)) > + return false; > + > + /* If V has been enabled then it is not the first-use trap */ > + if (riscv_v_vstate_query(regs)) > + return false; > + > + /* Get the instruction */ > + if (!insn) { > + if (__get_user(insn, epc)) > + return false; > + } > + > + /* Filter out non-V instructions */ > + if (!insn_is_vector(insn)) > + return false; > + > + /* Sanity check. datap should be null by the time of the first-use trap */ > + WARN_ON(current->thread.vstate.datap); > + > + /* > + * Now we sure that this is a V instruction. And it executes in the > + * context where VS has been off. So, try to allocate the user's V > + * context and resume execution. > + */ > + if (riscv_v_thread_zalloc()) { > + force_sig(SIGKILL); Is SIGKILL too strong? Maybe we just SIGILL here? Maybe killing the process is the right way to go, though: if we're that out of memory something's getting killed, it might as well be whatever's about to get confused by vectors disappearing. > + return true; > + } > + riscv_v_vstate_on(regs); > + return true; > +} Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
On Tue, May 23, 2023 at 05:49:04PM -0700, Palmer Dabbelt wrote: > On Thu, 18 May 2023 09:19:34 PDT (-0700), andy.chiu@sifive.com wrote: > > Vector unit is disabled by default for all user processes. Thus, a > > process will take a trap (illegal instruction) into kernel at the first > > time when it uses Vector. Only after then, the kernel allocates V > > context and starts take care of the context for that user process. > > > > Suggested-by: Richard Henderson <richard.henderson@linaro.org> > > Link: https://lore.kernel.org/r/3923eeee-e4dc-0911-40bf-84c34aee962d@linaro.org > > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > > --- > > Hey Heiko and Conor, I am dropping you guys' A-b, T-b, and R-b because I > > added a check in riscv_v_first_use_handler(). > > > > Changelog v20: > > - move has_vector() into vector.c for better code readibility > > - check elf_hwcap in the first-use trap because it might get turned off > > if cores have different VLENs. > > > > Changelog v18: > > - Add blank lines (Heiko) > > - Return immediately in insn_is_vector() if an insn matches (Heiko) > > --- > > arch/riscv/include/asm/insn.h | 29 ++++++++++ > > arch/riscv/include/asm/vector.h | 2 + > > arch/riscv/kernel/traps.c | 26 ++++++++- > > arch/riscv/kernel/vector.c | 95 +++++++++++++++++++++++++++++++++ > > 4 files changed, 150 insertions(+), 2 deletions(-) > > > > diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h > > index 8d5c84f2d5ef..4e1505cef8aa 100644 > > --- a/arch/riscv/include/asm/insn.h > > +++ b/arch/riscv/include/asm/insn.h > > @@ -137,6 +137,26 @@ > > #define RVG_OPCODE_JALR 0x67 > > #define RVG_OPCODE_JAL 0x6f > > #define RVG_OPCODE_SYSTEM 0x73 > > +#define RVG_SYSTEM_CSR_OFF 20 > > +#define RVG_SYSTEM_CSR_MASK GENMASK(12, 0) > > + > > +/* parts of opcode for RVF, RVD and RVQ */ > > +#define RVFDQ_FL_FS_WIDTH_OFF 12 > > +#define RVFDQ_FL_FS_WIDTH_MASK GENMASK(3, 0) > > +#define RVFDQ_FL_FS_WIDTH_W 2 > > +#define RVFDQ_FL_FS_WIDTH_D 3 > > +#define RVFDQ_LS_FS_WIDTH_Q 4 > > +#define RVFDQ_OPCODE_FL 0x07 > > +#define RVFDQ_OPCODE_FS 0x27 > > + > > +/* parts of opcode for RVV */ > > +#define RVV_OPCODE_VECTOR 0x57 > > +#define RVV_VL_VS_WIDTH_8 0 > > +#define RVV_VL_VS_WIDTH_16 5 > > +#define RVV_VL_VS_WIDTH_32 6 > > +#define RVV_VL_VS_WIDTH_64 7 > > +#define RVV_OPCODE_VL RVFDQ_OPCODE_FL > > +#define RVV_OPCODE_VS RVFDQ_OPCODE_FS > > > > /* parts of opcode for RVC*/ > > #define RVC_OPCODE_C0 0x0 > > @@ -304,6 +324,15 @@ static __always_inline bool riscv_insn_is_branch(u32 code) > > (RVC_X(x_, RVC_B_IMM_7_6_OPOFF, RVC_B_IMM_7_6_MASK) << RVC_B_IMM_7_6_OFF) | \ > > (RVC_IMM_SIGN(x_) << RVC_B_IMM_SIGN_OFF); }) > > > > +#define RVG_EXTRACT_SYSTEM_CSR(x) \ > > + ({typeof(x) x_ = (x); RV_X(x_, RVG_SYSTEM_CSR_OFF, RVG_SYSTEM_CSR_MASK); }) > > + > > +#define RVFDQ_EXTRACT_FL_FS_WIDTH(x) \ > > + ({typeof(x) x_ = (x); RV_X(x_, RVFDQ_FL_FS_WIDTH_OFF, \ > > + RVFDQ_FL_FS_WIDTH_MASK); }) > > + > > +#define RVV_EXRACT_VL_VS_WIDTH(x) RVFDQ_EXTRACT_FL_FS_WIDTH(x) > > + > > /* > > * Get the immediate from a J-type instruction. > > * > > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h > > index ce6a75e9cf62..8e56da67b5cf 100644 > > --- a/arch/riscv/include/asm/vector.h > > +++ b/arch/riscv/include/asm/vector.h > > @@ -21,6 +21,7 @@ > > > > extern unsigned long riscv_v_vsize; > > int riscv_v_setup_vsize(void); > > +bool riscv_v_first_use_handler(struct pt_regs *regs); > > > > static __always_inline bool has_vector(void) > > { > > @@ -165,6 +166,7 @@ struct pt_regs; > > > > static inline int riscv_v_setup_vsize(void) { return -EOPNOTSUPP; } > > static __always_inline bool has_vector(void) { return false; } > > +static inline bool riscv_v_first_use_handler(struct pt_regs *regs) { return false; } > > static inline bool riscv_v_vstate_query(struct pt_regs *regs) { return false; } > > #define riscv_v_vsize (0) > > #define riscv_v_vstate_save(task, regs) do {} while (0) > > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > > index 8c258b78c925..05ffdcd1424e 100644 > > --- a/arch/riscv/kernel/traps.c > > +++ b/arch/riscv/kernel/traps.c > > @@ -26,6 +26,7 @@ > > #include <asm/ptrace.h> > > #include <asm/syscall.h> > > #include <asm/thread_info.h> > > +#include <asm/vector.h> > > > > int show_unhandled_signals = 1; > > > > @@ -145,8 +146,29 @@ 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 (user_mode(regs)) { > > + irqentry_enter_from_user_mode(regs); > > + > > + local_irq_enable(); > > + > > + if (!riscv_v_first_use_handler(regs)) > > + do_trap_error(regs, SIGILL, ILL_ILLOPC, regs->epc, > > + "Oops - illegal instruction"); > > + > > + irqentry_exit_to_user_mode(regs); > > + } else { > > + irqentry_state_t state = irqentry_nmi_enter(regs); > > + > > + do_trap_error(regs, SIGILL, ILL_ILLOPC, regs->epc, > > + "Oops - illegal instruction"); > > + > > + irqentry_nmi_exit(regs, state); > > + } > > +} > > + > > DO_ERROR_INFO(do_trap_load_fault, > > SIGSEGV, SEGV_ACCERR, "load access fault"); > > #ifndef CONFIG_RISCV_M_MODE > > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c > > index 120f1ce9abf9..0080798e8d2e 100644 > > --- a/arch/riscv/kernel/vector.c > > +++ b/arch/riscv/kernel/vector.c > > @@ -4,10 +4,19 @@ > > * Author: Andy Chiu <andy.chiu@sifive.com> > > */ > > #include <linux/export.h> > > +#include <linux/sched/signal.h> > > +#include <linux/types.h> > > +#include <linux/slab.h> > > +#include <linux/sched.h> > > +#include <linux/uaccess.h> > > > > +#include <asm/thread_info.h> > > +#include <asm/processor.h> > > +#include <asm/insn.h> > > #include <asm/vector.h> > > #include <asm/csr.h> > > #include <asm/elf.h> > > +#include <asm/ptrace.h> > > #include <asm/bug.h> > > > > unsigned long riscv_v_vsize __read_mostly; > > @@ -34,3 +43,89 @@ int riscv_v_setup_vsize(void) > > > > return 0; > > } > > + > > +static bool insn_is_vector(u32 insn_buf) > > +{ > > + u32 opcode = insn_buf & __INSN_OPCODE_MASK; > > + u32 width, csr; > > + > > + /* > > + * All V-related instructions, including CSR operations are 4-Byte. So, > > + * do not handle if the instruction length is not 4-Byte. > > + */ > > + if (unlikely(GET_INSN_LENGTH(insn_buf) != 4)) > > + return false; > > + > > + switch (opcode) { > > + case RVV_OPCODE_VECTOR: > > + return true; > > + case RVV_OPCODE_VL: > > + case RVV_OPCODE_VS: > > + width = RVV_EXRACT_VL_VS_WIDTH(insn_buf); > > + if (width == RVV_VL_VS_WIDTH_8 || width == RVV_VL_VS_WIDTH_16 || > > + width == RVV_VL_VS_WIDTH_32 || width == RVV_VL_VS_WIDTH_64) > > + return true; > > + > > + break; > > + case RVG_OPCODE_SYSTEM: > > + csr = RVG_EXTRACT_SYSTEM_CSR(insn_buf); > > + if ((csr >= CSR_VSTART && csr <= CSR_VCSR) || > > + (csr >= CSR_VL && csr <= CSR_VLENB)) > > + return true; > > + } > > + > > + return false; > > +} > > + > > +static int riscv_v_thread_zalloc(void) > > +{ > > + void *datap; > > + > > + datap = kzalloc(riscv_v_vsize, GFP_KERNEL); > > + if (!datap) > > + return -ENOMEM; > > + > > + current->thread.vstate.datap = datap; > > + memset(¤t->thread.vstate, 0, offsetof(struct __riscv_v_ext_state, > > + datap)); > > + return 0; > > +} > > + > > +bool riscv_v_first_use_handler(struct pt_regs *regs) > > +{ > > + u32 __user *epc = (u32 __user *)regs->epc; > > + u32 insn = (u32)regs->badaddr; > > + > > + /* Do not handle if V is not supported, or disabled */ > > + if (!has_vector() || !(elf_hwcap & COMPAT_HWCAP_ISA_V)) > > + return false; > > + > > + /* If V has been enabled then it is not the first-use trap */ > > + if (riscv_v_vstate_query(regs)) > > + return false; > > + > > + /* Get the instruction */ > > + if (!insn) { > > + if (__get_user(insn, epc)) > > + return false; > > + } > > + > > + /* Filter out non-V instructions */ > > + if (!insn_is_vector(insn)) > > + return false; > > + > > + /* Sanity check. datap should be null by the time of the first-use trap */ > > + WARN_ON(current->thread.vstate.datap); > > + > > + /* > > + * Now we sure that this is a V instruction. And it executes in the > > + * context where VS has been off. So, try to allocate the user's V > > + * context and resume execution. > > + */ > > + if (riscv_v_thread_zalloc()) { > > + force_sig(SIGKILL); > > Is SIGKILL too strong? Maybe we just SIGILL here? Maybe killing the > process is the right way to go, though: if we're that out of memory > something's getting killed, it might as well be whatever's about to get > confused by vectors disappearing. > SIGILL seems misleading; it's not the instruction that is at fault. Maybe SIGSEGV (or SIGBUS), since that's generally what you get if you assume an allocation succeeds when it doesn't, as that is effectively what this patchset does by not providing an adequate way to return allocation failures to the application. > > + return true; > > + } > > + riscv_v_vstate_on(regs); > > + return true; > > +} > > Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linu
On Fri, May 19, 2023 at 12:21 AM Andy Chiu <andy.chiu@sifive.com> wrote: > > Vector unit is disabled by default for all user processes. Thus, a > process will take a trap (illegal instruction) into kernel at the first > time when it uses Vector. Only after then, the kernel allocates V > context and starts take care of the context for that user process. > > Suggested-by: Richard Henderson <richard.henderson@linaro.org> > Link: https://lore.kernel.org/r/3923eeee-e4dc-0911-40bf-84c34aee962d@linaro.org > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > --- > Hey Heiko and Conor, I am dropping you guys' A-b, T-b, and R-b because I > added a check in riscv_v_first_use_handler(). > > Changelog v20: > - move has_vector() into vector.c for better code readibility > - check elf_hwcap in the first-use trap because it might get turned off > if cores have different VLENs. > > Changelog v18: > - Add blank lines (Heiko) > - Return immediately in insn_is_vector() if an insn matches (Heiko) > --- > arch/riscv/include/asm/insn.h | 29 ++++++++++ > arch/riscv/include/asm/vector.h | 2 + > arch/riscv/kernel/traps.c | 26 ++++++++- > arch/riscv/kernel/vector.c | 95 +++++++++++++++++++++++++++++++++ > 4 files changed, 150 insertions(+), 2 deletions(-) > > diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h > index 8d5c84f2d5ef..4e1505cef8aa 100644 > --- a/arch/riscv/include/asm/insn.h > +++ b/arch/riscv/include/asm/insn.h > @@ -137,6 +137,26 @@ > #define RVG_OPCODE_JALR 0x67 > #define RVG_OPCODE_JAL 0x6f > #define RVG_OPCODE_SYSTEM 0x73 > +#define RVG_SYSTEM_CSR_OFF 20 > +#define RVG_SYSTEM_CSR_MASK GENMASK(12, 0) > + > +/* parts of opcode for RVF, RVD and RVQ */ > +#define RVFDQ_FL_FS_WIDTH_OFF 12 > +#define RVFDQ_FL_FS_WIDTH_MASK GENMASK(3, 0) > +#define RVFDQ_FL_FS_WIDTH_W 2 > +#define RVFDQ_FL_FS_WIDTH_D 3 > +#define RVFDQ_LS_FS_WIDTH_Q 4 > +#define RVFDQ_OPCODE_FL 0x07 > +#define RVFDQ_OPCODE_FS 0x27 > + > +/* parts of opcode for RVV */ > +#define RVV_OPCODE_VECTOR 0x57 > +#define RVV_VL_VS_WIDTH_8 0 > +#define RVV_VL_VS_WIDTH_16 5 > +#define RVV_VL_VS_WIDTH_32 6 > +#define RVV_VL_VS_WIDTH_64 7 > +#define RVV_OPCODE_VL RVFDQ_OPCODE_FL > +#define RVV_OPCODE_VS RVFDQ_OPCODE_FS > > /* parts of opcode for RVC*/ > #define RVC_OPCODE_C0 0x0 > @@ -304,6 +324,15 @@ static __always_inline bool riscv_insn_is_branch(u32 code) > (RVC_X(x_, RVC_B_IMM_7_6_OPOFF, RVC_B_IMM_7_6_MASK) << RVC_B_IMM_7_6_OFF) | \ > (RVC_IMM_SIGN(x_) << RVC_B_IMM_SIGN_OFF); }) > > +#define RVG_EXTRACT_SYSTEM_CSR(x) \ > + ({typeof(x) x_ = (x); RV_X(x_, RVG_SYSTEM_CSR_OFF, RVG_SYSTEM_CSR_MASK); }) > + > +#define RVFDQ_EXTRACT_FL_FS_WIDTH(x) \ > + ({typeof(x) x_ = (x); RV_X(x_, RVFDQ_FL_FS_WIDTH_OFF, \ > + RVFDQ_FL_FS_WIDTH_MASK); }) > + > +#define RVV_EXRACT_VL_VS_WIDTH(x) RVFDQ_EXTRACT_FL_FS_WIDTH(x) > + > /* > * Get the immediate from a J-type instruction. > * > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h > index ce6a75e9cf62..8e56da67b5cf 100644 > --- a/arch/riscv/include/asm/vector.h > +++ b/arch/riscv/include/asm/vector.h > @@ -21,6 +21,7 @@ > > extern unsigned long riscv_v_vsize; > int riscv_v_setup_vsize(void); > +bool riscv_v_first_use_handler(struct pt_regs *regs); > > static __always_inline bool has_vector(void) > { > @@ -165,6 +166,7 @@ struct pt_regs; > > static inline int riscv_v_setup_vsize(void) { return -EOPNOTSUPP; } > static __always_inline bool has_vector(void) { return false; } > +static inline bool riscv_v_first_use_handler(struct pt_regs *regs) { return false; } > static inline bool riscv_v_vstate_query(struct pt_regs *regs) { return false; } > #define riscv_v_vsize (0) > #define riscv_v_vstate_save(task, regs) do {} while (0) > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > index 8c258b78c925..05ffdcd1424e 100644 > --- a/arch/riscv/kernel/traps.c > +++ b/arch/riscv/kernel/traps.c > @@ -26,6 +26,7 @@ > #include <asm/ptrace.h> > #include <asm/syscall.h> > #include <asm/thread_info.h> > +#include <asm/vector.h> > > int show_unhandled_signals = 1; > > @@ -145,8 +146,29 @@ 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 (user_mode(regs)) { > + irqentry_enter_from_user_mode(regs); > + > + local_irq_enable(); > + > + if (!riscv_v_first_use_handler(regs)) > + do_trap_error(regs, SIGILL, ILL_ILLOPC, regs->epc, > + "Oops - illegal instruction"); > + > + irqentry_exit_to_user_mode(regs); > + } else { > + irqentry_state_t state = irqentry_nmi_enter(regs); > + > + do_trap_error(regs, SIGILL, ILL_ILLOPC, regs->epc, > + "Oops - illegal instruction"); > + > + irqentry_nmi_exit(regs, state); > + } > +} > + > DO_ERROR_INFO(do_trap_load_fault, > SIGSEGV, SEGV_ACCERR, "load access fault"); > #ifndef CONFIG_RISCV_M_MODE > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c > index 120f1ce9abf9..0080798e8d2e 100644 > --- a/arch/riscv/kernel/vector.c > +++ b/arch/riscv/kernel/vector.c > @@ -4,10 +4,19 @@ > * Author: Andy Chiu <andy.chiu@sifive.com> > */ > #include <linux/export.h> > +#include <linux/sched/signal.h> > +#include <linux/types.h> > +#include <linux/slab.h> > +#include <linux/sched.h> > +#include <linux/uaccess.h> > > +#include <asm/thread_info.h> > +#include <asm/processor.h> > +#include <asm/insn.h> > #include <asm/vector.h> > #include <asm/csr.h> > #include <asm/elf.h> > +#include <asm/ptrace.h> > #include <asm/bug.h> > > unsigned long riscv_v_vsize __read_mostly; > @@ -34,3 +43,89 @@ int riscv_v_setup_vsize(void) > > return 0; > } > + > +static bool insn_is_vector(u32 insn_buf) > +{ > + u32 opcode = insn_buf & __INSN_OPCODE_MASK; > + u32 width, csr; > + > + /* > + * All V-related instructions, including CSR operations are 4-Byte. So, > + * do not handle if the instruction length is not 4-Byte. > + */ > + if (unlikely(GET_INSN_LENGTH(insn_buf) != 4)) > + return false; > + > + switch (opcode) { > + case RVV_OPCODE_VECTOR: > + return true; > + case RVV_OPCODE_VL: > + case RVV_OPCODE_VS: > + width = RVV_EXRACT_VL_VS_WIDTH(insn_buf); > + if (width == RVV_VL_VS_WIDTH_8 || width == RVV_VL_VS_WIDTH_16 || > + width == RVV_VL_VS_WIDTH_32 || width == RVV_VL_VS_WIDTH_64) > + return true; > + > + break; > + case RVG_OPCODE_SYSTEM: > + csr = RVG_EXTRACT_SYSTEM_CSR(insn_buf); > + if ((csr >= CSR_VSTART && csr <= CSR_VCSR) || > + (csr >= CSR_VL && csr <= CSR_VLENB)) > + return true; > + } > + > + return false; > +} > + > +static int riscv_v_thread_zalloc(void) > +{ > + void *datap; > + > + datap = kzalloc(riscv_v_vsize, GFP_KERNEL); > + if (!datap) > + return -ENOMEM; > + > + current->thread.vstate.datap = datap; > + memset(¤t->thread.vstate, 0, offsetof(struct __riscv_v_ext_state, > + datap)); > + return 0; > +} > + > +bool riscv_v_first_use_handler(struct pt_regs *regs) > +{ > + u32 __user *epc = (u32 __user *)regs->epc; > + u32 insn = (u32)regs->badaddr; > + > + /* Do not handle if V is not supported, or disabled */ > + if (!has_vector() || !(elf_hwcap & COMPAT_HWCAP_ISA_V)) > + return false; > + > + /* If V has been enabled then it is not the first-use trap */ > + if (riscv_v_vstate_query(regs)) > + return false; > + > + /* Get the instruction */ > + if (!insn) { > + if (__get_user(insn, epc)) > + return false; > + } As spec has said: 4.1.11 Supervisor Trap Value (stval) Register ... On an illegal instruction trap, stval may be written with the rst XLEN or ILEN bits of the faulting instruction as described below. So u32 insn = (u32)regs->badaddr; is enough. Do you need an ALTERNATIVE fixup here? > + > + /* Filter out non-V instructions */ > + if (!insn_is_vector(insn)) > + return false; > + > + /* Sanity check. datap should be null by the time of the first-use trap */ > + WARN_ON(current->thread.vstate.datap); > + > + /* > + * Now we sure that this is a V instruction. And it executes in the > + * context where VS has been off. So, try to allocate the user's V > + * context and resume execution. > + */ > + if (riscv_v_thread_zalloc()) { > + force_sig(SIGKILL); > + return true; > + } > + riscv_v_vstate_on(regs); > + return true; > +} > -- > 2.17.1 >
diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h index 8d5c84f2d5ef..4e1505cef8aa 100644 --- a/arch/riscv/include/asm/insn.h +++ b/arch/riscv/include/asm/insn.h @@ -137,6 +137,26 @@ #define RVG_OPCODE_JALR 0x67 #define RVG_OPCODE_JAL 0x6f #define RVG_OPCODE_SYSTEM 0x73 +#define RVG_SYSTEM_CSR_OFF 20 +#define RVG_SYSTEM_CSR_MASK GENMASK(12, 0) + +/* parts of opcode for RVF, RVD and RVQ */ +#define RVFDQ_FL_FS_WIDTH_OFF 12 +#define RVFDQ_FL_FS_WIDTH_MASK GENMASK(3, 0) +#define RVFDQ_FL_FS_WIDTH_W 2 +#define RVFDQ_FL_FS_WIDTH_D 3 +#define RVFDQ_LS_FS_WIDTH_Q 4 +#define RVFDQ_OPCODE_FL 0x07 +#define RVFDQ_OPCODE_FS 0x27 + +/* parts of opcode for RVV */ +#define RVV_OPCODE_VECTOR 0x57 +#define RVV_VL_VS_WIDTH_8 0 +#define RVV_VL_VS_WIDTH_16 5 +#define RVV_VL_VS_WIDTH_32 6 +#define RVV_VL_VS_WIDTH_64 7 +#define RVV_OPCODE_VL RVFDQ_OPCODE_FL +#define RVV_OPCODE_VS RVFDQ_OPCODE_FS /* parts of opcode for RVC*/ #define RVC_OPCODE_C0 0x0 @@ -304,6 +324,15 @@ static __always_inline bool riscv_insn_is_branch(u32 code) (RVC_X(x_, RVC_B_IMM_7_6_OPOFF, RVC_B_IMM_7_6_MASK) << RVC_B_IMM_7_6_OFF) | \ (RVC_IMM_SIGN(x_) << RVC_B_IMM_SIGN_OFF); }) +#define RVG_EXTRACT_SYSTEM_CSR(x) \ + ({typeof(x) x_ = (x); RV_X(x_, RVG_SYSTEM_CSR_OFF, RVG_SYSTEM_CSR_MASK); }) + +#define RVFDQ_EXTRACT_FL_FS_WIDTH(x) \ + ({typeof(x) x_ = (x); RV_X(x_, RVFDQ_FL_FS_WIDTH_OFF, \ + RVFDQ_FL_FS_WIDTH_MASK); }) + +#define RVV_EXRACT_VL_VS_WIDTH(x) RVFDQ_EXTRACT_FL_FS_WIDTH(x) + /* * Get the immediate from a J-type instruction. * diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h index ce6a75e9cf62..8e56da67b5cf 100644 --- a/arch/riscv/include/asm/vector.h +++ b/arch/riscv/include/asm/vector.h @@ -21,6 +21,7 @@ extern unsigned long riscv_v_vsize; int riscv_v_setup_vsize(void); +bool riscv_v_first_use_handler(struct pt_regs *regs); static __always_inline bool has_vector(void) { @@ -165,6 +166,7 @@ struct pt_regs; static inline int riscv_v_setup_vsize(void) { return -EOPNOTSUPP; } static __always_inline bool has_vector(void) { return false; } +static inline bool riscv_v_first_use_handler(struct pt_regs *regs) { return false; } static inline bool riscv_v_vstate_query(struct pt_regs *regs) { return false; } #define riscv_v_vsize (0) #define riscv_v_vstate_save(task, regs) do {} while (0) diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index 8c258b78c925..05ffdcd1424e 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -26,6 +26,7 @@ #include <asm/ptrace.h> #include <asm/syscall.h> #include <asm/thread_info.h> +#include <asm/vector.h> int show_unhandled_signals = 1; @@ -145,8 +146,29 @@ 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 (user_mode(regs)) { + irqentry_enter_from_user_mode(regs); + + local_irq_enable(); + + if (!riscv_v_first_use_handler(regs)) + do_trap_error(regs, SIGILL, ILL_ILLOPC, regs->epc, + "Oops - illegal instruction"); + + irqentry_exit_to_user_mode(regs); + } else { + irqentry_state_t state = irqentry_nmi_enter(regs); + + do_trap_error(regs, SIGILL, ILL_ILLOPC, regs->epc, + "Oops - illegal instruction"); + + irqentry_nmi_exit(regs, state); + } +} + DO_ERROR_INFO(do_trap_load_fault, SIGSEGV, SEGV_ACCERR, "load access fault"); #ifndef CONFIG_RISCV_M_MODE diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c index 120f1ce9abf9..0080798e8d2e 100644 --- a/arch/riscv/kernel/vector.c +++ b/arch/riscv/kernel/vector.c @@ -4,10 +4,19 @@ * Author: Andy Chiu <andy.chiu@sifive.com> */ #include <linux/export.h> +#include <linux/sched/signal.h> +#include <linux/types.h> +#include <linux/slab.h> +#include <linux/sched.h> +#include <linux/uaccess.h> +#include <asm/thread_info.h> +#include <asm/processor.h> +#include <asm/insn.h> #include <asm/vector.h> #include <asm/csr.h> #include <asm/elf.h> +#include <asm/ptrace.h> #include <asm/bug.h> unsigned long riscv_v_vsize __read_mostly; @@ -34,3 +43,89 @@ int riscv_v_setup_vsize(void) return 0; } + +static bool insn_is_vector(u32 insn_buf) +{ + u32 opcode = insn_buf & __INSN_OPCODE_MASK; + u32 width, csr; + + /* + * All V-related instructions, including CSR operations are 4-Byte. So, + * do not handle if the instruction length is not 4-Byte. + */ + if (unlikely(GET_INSN_LENGTH(insn_buf) != 4)) + return false; + + switch (opcode) { + case RVV_OPCODE_VECTOR: + return true; + case RVV_OPCODE_VL: + case RVV_OPCODE_VS: + width = RVV_EXRACT_VL_VS_WIDTH(insn_buf); + if (width == RVV_VL_VS_WIDTH_8 || width == RVV_VL_VS_WIDTH_16 || + width == RVV_VL_VS_WIDTH_32 || width == RVV_VL_VS_WIDTH_64) + return true; + + break; + case RVG_OPCODE_SYSTEM: + csr = RVG_EXTRACT_SYSTEM_CSR(insn_buf); + if ((csr >= CSR_VSTART && csr <= CSR_VCSR) || + (csr >= CSR_VL && csr <= CSR_VLENB)) + return true; + } + + return false; +} + +static int riscv_v_thread_zalloc(void) +{ + void *datap; + + datap = kzalloc(riscv_v_vsize, GFP_KERNEL); + if (!datap) + return -ENOMEM; + + current->thread.vstate.datap = datap; + memset(¤t->thread.vstate, 0, offsetof(struct __riscv_v_ext_state, + datap)); + return 0; +} + +bool riscv_v_first_use_handler(struct pt_regs *regs) +{ + u32 __user *epc = (u32 __user *)regs->epc; + u32 insn = (u32)regs->badaddr; + + /* Do not handle if V is not supported, or disabled */ + if (!has_vector() || !(elf_hwcap & COMPAT_HWCAP_ISA_V)) + return false; + + /* If V has been enabled then it is not the first-use trap */ + if (riscv_v_vstate_query(regs)) + return false; + + /* Get the instruction */ + if (!insn) { + if (__get_user(insn, epc)) + return false; + } + + /* Filter out non-V instructions */ + if (!insn_is_vector(insn)) + return false; + + /* Sanity check. datap should be null by the time of the first-use trap */ + WARN_ON(current->thread.vstate.datap); + + /* + * Now we sure that this is a V instruction. And it executes in the + * context where VS has been off. So, try to allocate the user's V + * context and resume execution. + */ + if (riscv_v_thread_zalloc()) { + force_sig(SIGKILL); + return true; + } + riscv_v_vstate_on(regs); + return true; +}
Vector unit is disabled by default for all user processes. Thus, a process will take a trap (illegal instruction) into kernel at the first time when it uses Vector. Only after then, the kernel allocates V context and starts take care of the context for that user process. Suggested-by: Richard Henderson <richard.henderson@linaro.org> Link: https://lore.kernel.org/r/3923eeee-e4dc-0911-40bf-84c34aee962d@linaro.org Signed-off-by: Andy Chiu <andy.chiu@sifive.com> --- Hey Heiko and Conor, I am dropping you guys' A-b, T-b, and R-b because I added a check in riscv_v_first_use_handler(). Changelog v20: - move has_vector() into vector.c for better code readibility - check elf_hwcap in the first-use trap because it might get turned off if cores have different VLENs. Changelog v18: - Add blank lines (Heiko) - Return immediately in insn_is_vector() if an insn matches (Heiko) --- arch/riscv/include/asm/insn.h | 29 ++++++++++ arch/riscv/include/asm/vector.h | 2 + arch/riscv/kernel/traps.c | 26 ++++++++- arch/riscv/kernel/vector.c | 95 +++++++++++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 2 deletions(-)