On Thu, 12 Nov 2020 09:21:44 +0100 Peter Zijlstra <pet...@infradead.org> wrote:
> On Wed, Nov 11, 2020 at 08:15:19PM -0500, Steven Rostedt wrote: > > > diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h > > index e00fe88146e0..235385a38bd9 100644 > > --- a/arch/x86/include/asm/ftrace.h > > +++ b/arch/x86/include/asm/ftrace.h > > @@ -54,6 +54,9 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs) > > return NULL; > > return &fregs->regs; > > } > > + > > +#define ftrace_regs_set_ip(fregs, _ip) \ > > + do { (fregs)->regs.ip = (_ip); } while (0) > > #endif > > > > #ifdef CONFIG_DYNAMIC_FTRACE > > diff --git a/arch/x86/include/asm/livepatch.h > > b/arch/x86/include/asm/livepatch.h > > index 1fde1ab6559e..59a08d5c6f1d 100644 > > --- a/arch/x86/include/asm/livepatch.h > > +++ b/arch/x86/include/asm/livepatch.h > > @@ -12,9 +12,9 @@ > > #include <asm/setup.h> > > #include <linux/ftrace.h> > > > > -static inline void klp_arch_set_pc(struct pt_regs *regs, unsigned long ip) > > +static inline void klp_arch_set_pc(struct ftrace_regs *fregs, unsigned > > long ip) > > { > > - regs->ip = ip; > > + ftrace_regs_set_ip(fregs, ip); > > } > > > > The normal variant is called instruction_pointer_set(), should this be > called ftrace_instruction_pointer_set() ? Sure, I can change that. > > (and yes, I hate the long name too). ftrace_regs_ip_set()? ;-) > > Also, do you want something like: > > unsigned long ftrace_regs_get_register(struct ftrace_regs *regs, unsigned int > offset) > { I haven't gotten this far yet. I'm looking at generic use cases on how to get args across archs. Each arch will have its own method. > switch (offset / sizeof(long)) { > case 4: /* RBP */ > > case 8: /* R9 */ > case 9: /* R8 */ > case 10: /* RAX */ > case 11: /* RCX */ > case 12: /* RDX */ > case 13: /* RSI */ > case 14: /* RDI */ > case 15: /* ORIG_RAX */ > case 16: /* RIP */ > return *(unsigned long *)regs->regs + offset; > > default: > WARN_ON_ONCE(1); Not sure we even want to warn. Perhaps have this as: bool ftrace_regs_get_register(struct ftrace_regs *regs, unsigned int offset, unsigned long *val) { if (regs->cs) { *val = regs_get_register(regs->regs, offset); return true; } switch (offset / sizeof(long)) { case ...: *val = *(unsigned long *)regs->regs + offset; return true; default; return false; > } -- Steve