Re: [PATCH 2/3] riscv: traps: make insn fetch common in unknown instruction

[Ben Dooks <ben.dooks@codethink.co.uk>]

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.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv