Re: [RFC][PATCH 1/2 v2] ftrace/x86: Allow for arguments to be passed in to REGS by default

[Masami Hiramatsu]

On Wed, 28 Oct 2020 11:29:16 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> Hi Masami,
> 
> Talking with Peter and Thomas on IRC, where they really don't like
> passing a partial pt_regs around, got me thinking of redoing the REGS
> parameter of ftrace. Kprobes is the only user that requires the full
> registers being saved, and that's only because some kprobe user might
> want them.

Yes, kprobes can be used for debugging use case. I think we can skip
coller-saved registers (because ftrace is embedded by compiler and it
may save such registers correctly), but we still need a pt_regs on memory
to access it.

> 
> On Wed, 28 Oct 2020 10:25:02 -0400
> Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> >
> >  typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip,
> >  			      struct ftrace_ops *op, struct pt_regs *regs);
> >  
> 
> 
> Most registers of pt_regs at a start of a function is rather useless.
> What if we got rid of FL_SAVE_REGS all together and had a "ftrace_regs"
> structure passed in that would have only access to all the argument
> registers, the stack pointer and the instruction pointer?

Yeah, that's OK to me. If someone wants to investigate suspicious compiler
bug, they should not put a kprobe on the fentry call site. (in most cases
symbol+5 will put a breakpoint right after fentry nop)

> Then kprobes could just create its own pt_regs, fill in all the data
> from ftrace_regs and then fill the rest with zeros or possibly whatever
> the values currently are (does it really matter what those registers
> are?), including flags.

That sounds good to me.

> 
> Not only would this simplify the code, it would probably allow moving
> more of the kprobe code from the arch specific to the generic code, and
> remove a lot of duplication.

Ah, right.

> 
> This would also help speed up the processing of live kernel patching.
> 
> And best of all, it would give everything access to the arguments of a
> function and a stack pointer with out (ab)using pt_regs.
> 
> Do you think this would be feasible?

Yes, I agreed.

Thank you,

> 
> -- Steve


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>