Re: [PATCH v2 09/13] powerpc/ftrace: Implement CONFIG_DYNAMIC_FTRACE_WITH_ARGS

From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>,
	Jiri Kosina <jikos@kernel.org>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	"live-patching@vger.kernel.org" <live-patching@vger.kernel.org>,
	Miroslav Benes <mbenes@suse.cz>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Ingo Molnar <mingo@redhat.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Petr Mladek <pmladek@suse.com>
Subject: Re: [PATCH v2 09/13] powerpc/ftrace: Implement CONFIG_DYNAMIC_FTRACE_WITH_ARGS
Date: Tue, 15 Feb 2022 21:56:59 +0530	[thread overview]
Message-ID: <1644942378.byz0qymic3.naveen@linux.ibm.com> (raw)
In-Reply-To: <20220215093849.556d5444@gandalf.local.home>

Steven Rostedt wrote:
> On Tue, 15 Feb 2022 19:06:48 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> 
>> As I understand it, the reason ftrace_get_regs() was introduced was to 
>> be able to only return the pt_regs, if _all_ registers were saved into 
>> it, which we don't do when coming in through ftrace_caller(). See the 
>> x86 implementation (commit 02a474ca266a47 ("ftrace/x86: Allow for 
>> arguments to be passed in to ftrace_regs by default"), which returns 
>> pt_regs conditionally.
> 
> I can give you the history of ftrace_caller and ftrace_regs_caller.
> 
> ftrace_caller saved just enough as was denoted for gcc mcount trampolines.
> The new fentry which happens at the start of the function, whereas mcount
> happens after the stack frame is set up, may change the rules on some
> architectures.
> 
> As for ftrace_regs_caller, that was created for kprobes. As the majority of
> kprobes were added at the start of the function, it made sense to hook into
> ftrace as the ftrace trampoline call is much faster than taking a
> breakpoint interrupt. But to keep compatibility with breakpoint
> interrupts, we needed to fill in all the registers, and make it act just
> like a breakpoint interrupt.
> 
> I've been wanting to record function parameters, and because the ftrace
> trampoline must at a minimum save the function parameters before calling
> the ftrace callbacks, all the information for those parameters were being
> saved but were never exposed to the ftrace callbacks. I created the the
> DYNAMIC_FTRACE_WITH_ARGS to expose them. I first just used pt_regs with
> just the parameters filled in, but that was criticized as it could be
> confusing where the non filled in pt_regs might be used and thinking they
> are legitimate. So I created ftrace_regs that would give you just the
> function arguments (if DYNAMIC_FTRACE_WITH_ARGS is defined), or it will
> give you a full pt_regs, if the caller came from the ftrace_regs_caller. If
> not, it will give you a NULL pointer.
> 
> The first user to use the args was live kernel patching, as they only need
> that and the return pointer.

Thanks, that helps.

- Naveen