On Wed, 24 Apr 2024 15:19:24 +0200 Florent Revest <revest@xxxxxxxxxxxx> wrote: > On Wed, Apr 24, 2024 at 2:23 PM Florent Revest <revest@xxxxxxxxxxxx> wrote: > > > > On Mon, Apr 15, 2024 at 2:49 PM Masami Hiramatsu (Google) > > <mhiramat@xxxxxxxxxx> wrote: > > > > > > From: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> > > > > > > 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@xxxxxxxxxx> > > > Acked-by: Mark Rutland <mark.rutland@xxxxxxx> > > > --- > > > Changes in v8: > > > - Update that the saved registers depends on the context. > > > Changes in v3: > > > - Add instruction pointer > > > Changes in v2: > > > - newly added. > > > --- > > > include/linux/ftrace.h | 26 ++++++++++++++++++++++++++ > > > 1 file changed, 26 insertions(+) > > > > > > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > > > index 54d53f345d14..b81f1afa82a1 100644 > > > --- a/include/linux/ftrace.h > > > +++ b/include/linux/ftrace.h > > > @@ -118,6 +118,32 @@ 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) > > > > Ooc, have we ever considered skipping argument registers that are not > > return value registers in the exit code paths ? For example, why would > > we want to save rdi in a return handler ? > > > > But if we want to avoid the situation of having "sparse ftrace_regs" > > all over again, we'd have to split ftrace_regs into a ftrace_args_regs > > and a ftrace_ret_regs which would make this refactoring even more > > painful, just to skip a few instructions. :| > > > > I don't necessarily think it's worth it, I just wanted to make sure > > this was considered. > > Ah, well, I just reached patch 22 and noticed that there you add add: > > + * Basically, ftrace_regs stores the registers related to the context. > + * On function entry, registers for function parameters and hooking the > + * function call are stored, and on function exit, registers for function > + * return value and frame pointers are stored. > > So ftrace_regs can be a a sparse structure then. That's fair enough with me! ;) Yes, and in this patch, I explained that too :) > + * On the function entry, those registers will be restored except for > + * the stack pointer, so that user can change the function parameters > + * and instruction pointer (e.g. live patching.) > + * On the function exit, only registers which is used for return values > > + * are restored. So the function exit, ftrace_regs will be sparse. Thank you, -- Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
