Re: [PATCH 3/3 v5] livepatch: Use the default ftrace_ops instead of REGS when ARGS is available
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2020-11-12 14:59:19
Also in:
lkml
On Thu, 12 Nov 2020 09:21:44 +0100 Peter Zijlstra [off-list ref] wrote:
On Wed, Nov 11, 2020 at 08:15:19PM -0500, Steven Rostedt wrote:quoted
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_FTRACEdiff --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