| Message ID | 1409048930-21598-4-git-send-email-punit.agrawal@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Punit, On Tue, Aug 26, 2014 at 11:28:47AM +0100, Punit Agrawal wrote: > Add support to register hooks for undefined instructions. The handlers > will be called when the undefined instruction and the processor state > (as contained in pstate) match criteria used at registration. > > Note: The patch only deals with ARM instruction encodings and needs > fixing to handle thumb instructions as well. [...] > +static int call_undef_hook(struct pt_regs *regs) > +{ > + struct undef_hook *hook; > + unsigned long flags; > + u32 instr; > + int (*fn)(struct pt_regs *regs, u32 instr) = NULL; > + void __user *pc = (void __user *)instruction_pointer(regs); > + > + /* > + * Currently, undefined instruction patching is only supported > + * for user mode. Also, as we're not emulating any thumb > + * instructions lets not add thumb instruction decoding until > + * it is needed. > + */ > + if (!compat_user_mode(regs) || compat_thumb_mode(regs)) > + return 1; What do you mean by `undefined instruction patching'? I don't see anything in the mechanism that means this can't be reused for kernel code, then we just register the SWP emulation hook for userspace only using the mode (like we do for kgdb). > + get_user(instr, (u32 __user *)pc); > + instr = le32_to_cpu(instr); > + > + raw_spin_lock_irqsave(&undef_lock, flags); > + list_for_each_entry(hook, &undef_hook, node) > + if ((instr & hook->instr_mask) == hook->instr_val && > + (regs->pstate & hook->pstate_mask) == hook->pstate_val) > + fn = hook->fn; > + > + raw_spin_unlock_irqrestore(&undef_lock, flags); > + > + return fn ? fn(regs, instr) : 1; > +} > + > asmlinkage void __exception do_undefinstr(struct pt_regs *regs) > { > siginfo_t info; > @@ -266,6 +329,9 @@ asmlinkage void __exception do_undefinstr(struct pt_regs *regs) > if (!aarch32_break_handler(regs)) > return; > > + if (call_undef_hook(regs) == 0) > + return; I'd like to reuse this hook for the aarch32 break hooks (you can see the direct call in the context above). That means adding support for thumb after all. Is there a reason you've been avoiding that? Will
Will Deacon <will.deacon@arm.com> writes: > Hi Punit, > > On Tue, Aug 26, 2014 at 11:28:47AM +0100, Punit Agrawal wrote: >> Add support to register hooks for undefined instructions. The handlers >> will be called when the undefined instruction and the processor state >> (as contained in pstate) match criteria used at registration. >> >> Note: The patch only deals with ARM instruction encodings and needs >> fixing to handle thumb instructions as well. > > [...] > >> +static int call_undef_hook(struct pt_regs *regs) >> +{ >> + struct undef_hook *hook; >> + unsigned long flags; >> + u32 instr; >> + int (*fn)(struct pt_regs *regs, u32 instr) = NULL; >> + void __user *pc = (void __user *)instruction_pointer(regs); >> + >> + /* >> + * Currently, undefined instruction patching is only supported >> + * for user mode. Also, as we're not emulating any thumb >> + * instructions lets not add thumb instruction decoding until >> + * it is needed. >> + */ >> + if (!compat_user_mode(regs) || compat_thumb_mode(regs)) >> + return 1; > > What do you mean by `undefined instruction patching'? That's poorly worded. I'll improve upon it. > I don't see anything > in the mechanism that means this can't be reused for kernel code, then we > just register the SWP emulation hook for userspace only using the mode (like > we do for kgdb). There's nothing in the mechanism to prevent it's use for kernel code. I was erring on the side of caution as I hadn't tested it. > >> + get_user(instr, (u32 __user *)pc); >> + instr = le32_to_cpu(instr); >> + >> + raw_spin_lock_irqsave(&undef_lock, flags); >> + list_for_each_entry(hook, &undef_hook, node) >> + if ((instr & hook->instr_mask) == hook->instr_val && >> + (regs->pstate & hook->pstate_mask) == hook->pstate_val) >> + fn = hook->fn; >> + >> + raw_spin_unlock_irqrestore(&undef_lock, flags); >> + >> + return fn ? fn(regs, instr) : 1; >> +} >> + >> asmlinkage void __exception do_undefinstr(struct pt_regs *regs) >> { >> siginfo_t info; >> @@ -266,6 +329,9 @@ asmlinkage void __exception do_undefinstr(struct pt_regs *regs) >> if (!aarch32_break_handler(regs)) >> return; >> >> + if (call_undef_hook(regs) == 0) >> + return; > > I'd like to reuse this hook for the aarch32 break hooks (you can see the > direct call in the context above). That means adding support for thumb > after all. Is there a reason you've been avoiding that? None, other than to not add code before it's needed. I've just had a quick look at break handler and it looks pretty straight forward to fold that change in this set. Are you OK with that? The code started out when it wasn't clear if the code would target upstream kernels. I tried to not enable too many, what was by some considered, hacks. > > Will > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Aug 26, 2014 at 03:56:56PM +0100, Punit Agrawal wrote: > Will Deacon <will.deacon@arm.com> writes: > > On Tue, Aug 26, 2014 at 11:28:47AM +0100, Punit Agrawal wrote: > >> asmlinkage void __exception do_undefinstr(struct pt_regs *regs) > >> { > >> siginfo_t info; > >> @@ -266,6 +329,9 @@ asmlinkage void __exception do_undefinstr(struct pt_regs *regs) > >> if (!aarch32_break_handler(regs)) > >> return; > >> > >> + if (call_undef_hook(regs) == 0) > >> + return; > > > > I'd like to reuse this hook for the aarch32 break hooks (you can see the > > direct call in the context above). That means adding support for thumb > > after all. Is there a reason you've been avoiding that? > > None, other than to not add code before it's needed. I've just had a > quick look at break handler and it looks pretty straight forward to fold > that change in this set. Are you OK with that? Just do it as a separate patch on top of this one. Will
On Tue, Aug 26, 2014 at 03:56:56PM +0100, Punit Agrawal wrote: > Will Deacon <will.deacon@arm.com> writes: > > On Tue, Aug 26, 2014 at 11:28:47AM +0100, Punit Agrawal wrote: > >> Add support to register hooks for undefined instructions. The handlers > >> will be called when the undefined instruction and the processor state > >> (as contained in pstate) match criteria used at registration. > >> > >> Note: The patch only deals with ARM instruction encodings and needs > >> fixing to handle thumb instructions as well. > > > > [...] > > > >> +static int call_undef_hook(struct pt_regs *regs) > >> +{ > >> + struct undef_hook *hook; > >> + unsigned long flags; > >> + u32 instr; > >> + int (*fn)(struct pt_regs *regs, u32 instr) = NULL; > >> + void __user *pc = (void __user *)instruction_pointer(regs); > >> + > >> + /* > >> + * Currently, undefined instruction patching is only supported > >> + * for user mode. Also, as we're not emulating any thumb > >> + * instructions lets not add thumb instruction decoding until > >> + * it is needed. > >> + */ > >> + if (!compat_user_mode(regs) || compat_thumb_mode(regs)) > >> + return 1; [...] > > I don't see anything > > in the mechanism that means this can't be reused for kernel code, then we > > just register the SWP emulation hook for userspace only using the mode (like > > we do for kgdb). > > There's nothing in the mechanism to prevent it's use for kernel code. I > was erring on the side of caution as I hadn't tested it. I don't think we even have a guaranteed undefined instruction for AArch64, so I don't see the point of undef hook for kernel. But we could implement different entry paths rather than just one do_undefinstr: do_aarch64_undef, do_aarch32_undef (or maybe kernel/user split where the kernel one just panics).
diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h index 10ca8ff..4faaf03 100644 --- a/arch/arm64/include/asm/traps.h +++ b/arch/arm64/include/asm/traps.h @@ -18,6 +18,22 @@ #ifndef __ASM_TRAP_H #define __ASM_TRAP_H +#include <linux/list.h> + +struct pt_regs; + +struct undef_hook { + struct list_head node; + u32 instr_mask; + u32 instr_val; + u64 pstate_mask; + u64 pstate_val; + int (*fn)(struct pt_regs *regs, u32 instr); +}; + +int register_undef_hook(struct undef_hook *hook); +void unregister_undef_hook(struct undef_hook *hook); + static inline int in_exception_text(unsigned long ptr) { extern char __exception_text_start[]; diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c index 7ffaddd..6cc8fce 100644 --- a/arch/arm64/kernel/traps.c +++ b/arch/arm64/kernel/traps.c @@ -257,6 +257,69 @@ void arm64_notify_die(const char *str, struct pt_regs *regs, die(str, regs, err); } +static LIST_HEAD(undef_hook); +static DEFINE_RAW_SPINLOCK(undef_lock); + +int register_undef_hook(struct undef_hook *hook) +{ + unsigned long flags; + + /* + * We currently don't support handling undefined Thumb mode + * instructions. + */ + if (hook->pstate_mask & COMPAT_PSR_T_BIT) { + pr_warn("Thumb mode not supported for undefined instruction hooks\n"); + return -EINVAL; + } + + raw_spin_lock_irqsave(&undef_lock, flags); + list_add(&hook->node, &undef_hook); + raw_spin_unlock_irqrestore(&undef_lock, flags); + + return 0; +} + +void unregister_undef_hook(struct undef_hook *hook) +{ + unsigned long flags; + + raw_spin_lock_irqsave(&undef_lock, flags); + list_del(&hook->node); + raw_spin_unlock_irqrestore(&undef_lock, flags); +} + +static int call_undef_hook(struct pt_regs *regs) +{ + struct undef_hook *hook; + unsigned long flags; + u32 instr; + int (*fn)(struct pt_regs *regs, u32 instr) = NULL; + void __user *pc = (void __user *)instruction_pointer(regs); + + /* + * Currently, undefined instruction patching is only supported + * for user mode. Also, as we're not emulating any thumb + * instructions lets not add thumb instruction decoding until + * it is needed. + */ + if (!compat_user_mode(regs) || compat_thumb_mode(regs)) + return 1; + + get_user(instr, (u32 __user *)pc); + instr = le32_to_cpu(instr); + + raw_spin_lock_irqsave(&undef_lock, flags); + list_for_each_entry(hook, &undef_hook, node) + if ((instr & hook->instr_mask) == hook->instr_val && + (regs->pstate & hook->pstate_mask) == hook->pstate_val) + fn = hook->fn; + + raw_spin_unlock_irqrestore(&undef_lock, flags); + + return fn ? fn(regs, instr) : 1; +} + asmlinkage void __exception do_undefinstr(struct pt_regs *regs) { siginfo_t info; @@ -266,6 +329,9 @@ asmlinkage void __exception do_undefinstr(struct pt_regs *regs) if (!aarch32_break_handler(regs)) return; + if (call_undef_hook(regs) == 0) + return; + if (show_unhandled_signals && unhandled_signal(current, SIGILL) && printk_ratelimit()) { pr_info("%s[%d]: undefined instruction: pc=%p\n",
Add support to register hooks for undefined instructions. The handlers will be called when the undefined instruction and the processor state (as contained in pstate) match criteria used at registration. Note: The patch only deals with ARM instruction encodings and needs fixing to handle thumb instructions as well. Signed-off-by: Punit Agrawal <punit.agrawal@arm.com> --- arch/arm64/include/asm/traps.h | 16 ++++++++++ arch/arm64/kernel/traps.c | 66 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+)