| Message ID | 20241113091703.3133017-3-ben.dooks@codethink.co.uk (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] riscv: ptrace: add regs_set_register() | expand |
On 13/11/2024 09:17, Ben Dooks wrote: > Add the trapped instruction (insn) as the second argument to > riscv_v_first_use_handler() from the trap handler so when we > add more handlers we can do the fetch of the instruction just > once. > > Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> > --- > - fixed wording of patch from rfc > --- > arch/riscv/include/asm/vector.h | 4 ++-- > arch/riscv/kernel/traps.c | 11 ++++++++++- > arch/riscv/kernel/vector.c | 11 +---------- > 3 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h > index be7d309cca8a..c9f0b02cd975 100644 > --- a/arch/riscv/include/asm/vector.h > +++ b/arch/riscv/include/asm/vector.h > @@ -21,7 +21,7 @@ > > extern unsigned long riscv_v_vsize; > int riscv_v_setup_vsize(void); > -bool riscv_v_first_use_handler(struct pt_regs *regs); > +bool riscv_v_first_use_handler(struct pt_regs *regs, u32 insn); > void kernel_vector_begin(void); > void kernel_vector_end(void); > void get_cpu_vector_context(void); > @@ -268,7 +268,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_first_use_handler(struct pt_regs *regs, u32 insn) { return false; } > static inline bool riscv_v_vstate_query(struct pt_regs *regs) { return false; } > static inline bool riscv_v_vstate_ctrl_user_allowed(void) { return false; } > #define riscv_v_vsize (0) > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > index 51ebfd23e007..1c3fab272fd1 100644 > --- a/arch/riscv/kernel/traps.c > +++ b/arch/riscv/kernel/traps.c > @@ -172,11 +172,20 @@ asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *re > bool handled; > > if (user_mode(regs)) { > + u32 __user *epc = (u32 __user *)regs->epc; > + u32 insn = (u32)regs->badaddr; > + > irqentry_enter_from_user_mode(regs); > > local_irq_enable(); > > - handled = riscv_v_first_use_handler(regs); > + if (!insn) { > + if (__get_user(insn, epc)) { > + /* todo */ > + } grr, of course as soon as it is sent I notice the /todo/ here Not sure if we should print something if __get_user() fails for what /should/ be an good instruction addrss.... should probably at-least bail out of do_trap_insn_illegal() if not also print a warning. > + } > + > + handled = riscv_v_first_use_handler(regs, insn); > > local_irq_disable(); > > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c > index 682b3feee451..b852648cb8d5 100644 > --- a/arch/riscv/kernel/vector.c > +++ b/arch/riscv/kernel/vector.c > @@ -168,11 +168,8 @@ bool riscv_v_vstate_ctrl_user_allowed(void) > } > EXPORT_SYMBOL_GPL(riscv_v_vstate_ctrl_user_allowed); > > -bool riscv_v_first_use_handler(struct pt_regs *regs) > +bool riscv_v_first_use_handler(struct pt_regs *regs, u32 insn) > { > - u32 __user *epc = (u32 __user *)regs->epc; > - u32 insn = (u32)regs->badaddr; > - > if (!has_vector()) > return false; > > @@ -184,12 +181,6 @@ bool riscv_v_first_use_handler(struct pt_regs *regs) > 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;
On 13/11/2024 09:21, Ben Dooks wrote: > On 13/11/2024 09:17, Ben Dooks wrote: >> Add the trapped instruction (insn) as the second argument to >> riscv_v_first_use_handler() from the trap handler so when we >> add more handlers we can do the fetch of the instruction just >> once. >> >> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> >> --- >> - fixed wording of patch from rfc >> --- >> arch/riscv/include/asm/vector.h | 4 ++-- >> arch/riscv/kernel/traps.c | 11 ++++++++++- >> arch/riscv/kernel/vector.c | 11 +---------- >> 3 files changed, 13 insertions(+), 13 deletions(-) >> >> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/ >> vector.h >> index be7d309cca8a..c9f0b02cd975 100644 >> --- a/arch/riscv/include/asm/vector.h >> +++ b/arch/riscv/include/asm/vector.h >> @@ -21,7 +21,7 @@ >> extern unsigned long riscv_v_vsize; >> int riscv_v_setup_vsize(void); >> -bool riscv_v_first_use_handler(struct pt_regs *regs); >> +bool riscv_v_first_use_handler(struct pt_regs *regs, u32 insn); >> void kernel_vector_begin(void); >> void kernel_vector_end(void); >> void get_cpu_vector_context(void); >> @@ -268,7 +268,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_first_use_handler(struct pt_regs *regs, >> u32 insn) { return false; } >> static inline bool riscv_v_vstate_query(struct pt_regs *regs) >> { return false; } >> static inline bool riscv_v_vstate_ctrl_user_allowed(void) { return >> false; } >> #define riscv_v_vsize (0) >> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c >> index 51ebfd23e007..1c3fab272fd1 100644 >> --- a/arch/riscv/kernel/traps.c >> +++ b/arch/riscv/kernel/traps.c >> @@ -172,11 +172,20 @@ asmlinkage __visible __trap_section void >> do_trap_insn_illegal(struct pt_regs *re >> bool handled; >> if (user_mode(regs)) { >> + u32 __user *epc = (u32 __user *)regs->epc; >> + u32 insn = (u32)regs->badaddr; >> + >> irqentry_enter_from_user_mode(regs); >> local_irq_enable(); >> - handled = riscv_v_first_use_handler(regs); >> + if (!insn) { >> + if (__get_user(insn, epc)) { >> + /* todo */ >> + } > > grr, of course as soon as it is sent I notice the /todo/ here > > Not sure if we should print something if __get_user() fails for > what /should/ be an good instruction addrss.... should probably > at-least bail out of do_trap_insn_illegal() if not also print a > warning. > i think doing: if (___get_user(insn, epr)) { handled = false; insn = 0; } then just checking for "if (insn)" before hand handlers.
diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h index be7d309cca8a..c9f0b02cd975 100644 --- a/arch/riscv/include/asm/vector.h +++ b/arch/riscv/include/asm/vector.h @@ -21,7 +21,7 @@ extern unsigned long riscv_v_vsize; int riscv_v_setup_vsize(void); -bool riscv_v_first_use_handler(struct pt_regs *regs); +bool riscv_v_first_use_handler(struct pt_regs *regs, u32 insn); void kernel_vector_begin(void); void kernel_vector_end(void); void get_cpu_vector_context(void); @@ -268,7 +268,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_first_use_handler(struct pt_regs *regs, u32 insn) { return false; } static inline bool riscv_v_vstate_query(struct pt_regs *regs) { return false; } static inline bool riscv_v_vstate_ctrl_user_allowed(void) { return false; } #define riscv_v_vsize (0) diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index 51ebfd23e007..1c3fab272fd1 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -172,11 +172,20 @@ asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *re bool handled; if (user_mode(regs)) { + u32 __user *epc = (u32 __user *)regs->epc; + u32 insn = (u32)regs->badaddr; + irqentry_enter_from_user_mode(regs); local_irq_enable(); - handled = riscv_v_first_use_handler(regs); + if (!insn) { + if (__get_user(insn, epc)) { + /* todo */ + } + } + + handled = riscv_v_first_use_handler(regs, insn); local_irq_disable(); diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c index 682b3feee451..b852648cb8d5 100644 --- a/arch/riscv/kernel/vector.c +++ b/arch/riscv/kernel/vector.c @@ -168,11 +168,8 @@ bool riscv_v_vstate_ctrl_user_allowed(void) } EXPORT_SYMBOL_GPL(riscv_v_vstate_ctrl_user_allowed); -bool riscv_v_first_use_handler(struct pt_regs *regs) +bool riscv_v_first_use_handler(struct pt_regs *regs, u32 insn) { - u32 __user *epc = (u32 __user *)regs->epc; - u32 insn = (u32)regs->badaddr; - if (!has_vector()) return false; @@ -184,12 +181,6 @@ bool riscv_v_first_use_handler(struct pt_regs *regs) 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;
Add the trapped instruction (insn) as the second argument to riscv_v_first_use_handler() from the trap handler so when we add more handlers we can do the fetch of the instruction just once. Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> --- - fixed wording of patch from rfc --- arch/riscv/include/asm/vector.h | 4 ++-- arch/riscv/kernel/traps.c | 11 ++++++++++- arch/riscv/kernel/vector.c | 11 +---------- 3 files changed, 13 insertions(+), 13 deletions(-)