[RFC,v2,01/31] tracing: Add a comment about ftrace_regs definition

  From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

To clarify what will be expected on ftrace_regs, add a comment to the
architecture independent definition of the ftrace_regs.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 Changes in v2:
  - newly added.
---
 include/linux/ftrace.h |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
  

On Wed,  8 Nov 2023 23:24:32 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> To clarify what will be expected on ftrace_regs, add a comment to the
> architecture independent definition of the ftrace_regs.
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  Changes in v2:
>   - newly added.
> ---
>  include/linux/ftrace.h |   25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index e8921871ef9a..b174af91d8be 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -118,6 +118,31 @@ extern int ftrace_enabled;
>  
>  #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
>  
> +/**
> + * ftrace_regs - ftrace partial/optimal register set
> + *
> + * ftrace_regs represents a group of registers which is used at the
> + * function entry and exit. There are three types of registers.
> + *
> + * - Registers for passing the parameters to callee, including the stack
> + *   pointer. (e.g. rcx, rdx, rdi, rsi, r8, r9 and rsp on x86_64)
> + * - Registers for passing the return values to caller.
> + *   (e.g. rax and rdx on x86_64)
> + * - Registers for hooking the function return including the frame pointer
> + *   (the frame pointer is architecture/config dependent)
> + *   (e.g. rbp and rsp for x86_64)

Oops, I found the program counter/instruction pointer must be saved too.
This is used for live patching. One question is that if the IP is modified
at the return handler, what should we do? Return to the specified address?

Thanks,

> + *
> + * Also, architecture dependent fields can be used for internal process.
> + * (e.g. orig_ax on x86_64)
> + *
> + * On the function entry, those registers will be restored except for
> + * the stack pointer, so that user can change the function parameters.
> + * On the function exit, onlu registers which is used for return values
> + * are restored.
> + *
> + * NOTE: user *must not* access regs directly, only do it via APIs, because
> + * the member can be changed according to the architecture.
> + */
>  struct ftrace_regs {
>  	struct pt_regs		regs;
>  };
>
  

On Thu, Nov 09, 2023 at 08:14:52AM +0900, Masami Hiramatsu wrote:
> On Wed,  8 Nov 2023 23:24:32 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > To clarify what will be expected on ftrace_regs, add a comment to the
> > architecture independent definition of the ftrace_regs.
> > 
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > ---
> >  Changes in v2:
> >   - newly added.
> > ---
> >  include/linux/ftrace.h |   25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> > index e8921871ef9a..b174af91d8be 100644
> > --- a/include/linux/ftrace.h
> > +++ b/include/linux/ftrace.h
> > @@ -118,6 +118,31 @@ extern int ftrace_enabled;
> >  
> >  #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
> >  
> > +/**
> > + * ftrace_regs - ftrace partial/optimal register set
> > + *
> > + * ftrace_regs represents a group of registers which is used at the
> > + * function entry and exit. There are three types of registers.
> > + *
> > + * - Registers for passing the parameters to callee, including the stack
> > + *   pointer. (e.g. rcx, rdx, rdi, rsi, r8, r9 and rsp on x86_64)
> > + * - Registers for passing the return values to caller.
> > + *   (e.g. rax and rdx on x86_64)
> > + * - Registers for hooking the function return including the frame pointer
> > + *   (the frame pointer is architecture/config dependent)
> > + *   (e.g. rbp and rsp for x86_64)
> 
> Oops, I found the program counter/instruction pointer must be saved too.
> This is used for live patching. One question is that if the IP is modified
> at the return handler, what should we do? Return to the specified address?

I'm a bit confused here; currently we use fgraph_ret_regs for function returns,
are we going to replace that with ftrace_regs?

I think it makes sense for the PC/IP to be the address the return handler will
eventually return to (and hence allowing it to be overridden), but that does
mean we'll need to go recover the return address *before* we invoke any return
handlers.

Thanks,
Mark.
  

On Fri, 10 Nov 2023 11:11:31 +0000
Mark Rutland <mark.rutland@arm.com> wrote:

> On Thu, Nov 09, 2023 at 08:14:52AM +0900, Masami Hiramatsu wrote:
> > On Wed,  8 Nov 2023 23:24:32 +0900
> > "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> > 
> > > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > > 
> > > To clarify what will be expected on ftrace_regs, add a comment to the
> > > architecture independent definition of the ftrace_regs.
> > > 
> > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > > ---
> > >  Changes in v2:
> > >   - newly added.
> > > ---
> > >  include/linux/ftrace.h |   25 +++++++++++++++++++++++++
> > >  1 file changed, 25 insertions(+)
> > > 
> > > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> > > index e8921871ef9a..b174af91d8be 100644
> > > --- a/include/linux/ftrace.h
> > > +++ b/include/linux/ftrace.h
> > > @@ -118,6 +118,31 @@ extern int ftrace_enabled;
> > >  
> > >  #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
> > >  
> > > +/**
> > > + * ftrace_regs - ftrace partial/optimal register set
> > > + *
> > > + * ftrace_regs represents a group of registers which is used at the
> > > + * function entry and exit. There are three types of registers.
> > > + *
> > > + * - Registers for passing the parameters to callee, including the stack
> > > + *   pointer. (e.g. rcx, rdx, rdi, rsi, r8, r9 and rsp on x86_64)
> > > + * - Registers for passing the return values to caller.
> > > + *   (e.g. rax and rdx on x86_64)
> > > + * - Registers for hooking the function return including the frame pointer
> > > + *   (the frame pointer is architecture/config dependent)
> > > + *   (e.g. rbp and rsp for x86_64)
> > 
> > Oops, I found the program counter/instruction pointer must be saved too.
> > This is used for live patching. One question is that if the IP is modified
> > at the return handler, what should we do? Return to the specified address?
> 
> I'm a bit confused here; currently we use fgraph_ret_regs for function returns,
> are we going to replace that with ftrace_regs?

Yes. It is limited and does not have APIs compatibility.

> 
> I think it makes sense for the PC/IP to be the address the return handler will
> eventually return to (and hence allowing it to be overridden), but that does
> mean we'll need to go recover the return address *before* we invoke any return
> handlers.

The actual return address has been recovered from shadow stack at first,
and callback the handlers. See __ftrace_return_to_handler() and
ftrace_pop_return_trace().
So it is easy to set it to the ftrace_regs :)

Thank you!

> 
> Thanks,
> Mark.