Re: [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS

From: Steven Rostedt <rostedt@goodmis.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: linux-kernel@vger.kernel.org, catalin.marinas@arm.com,
	linux-arm-kernel@lists.infradead.org, mhiramat@kernel.org,
	revest@chromium.org, will@kernel.org
Subject: Re: [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS
Date: Tue, 15 Nov 2022 10:01:48 -0500	[thread overview]
Message-ID: <20221115100148.08475da0@gandalf.local.home> (raw)
In-Reply-To: <20221103170520.931305-1-mark.rutland@arm.com>

On Thu,  3 Nov 2022 17:05:16 +0000
Mark Rutland <mark.rutland@arm.com> wrote:

> This series replaces arm64's support for FTRACE_WITH_REGS with support
> for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> removes some latent issues with inconsistent presentation of struct
> pt_regs (which can only be reliably saved/restored at exception
> boundaries).
> 
> The existing FTRACE_WITH_REGS support was added for two major reasons:
> 
> (1) To make it possible to use the ftrace graph tracer with pointer
>     authentication, where it's necessary to snapshot/manipulate the LR
>     before it is signed by the instrumented function.
> 
> (2) To make it possible to implement LIVEPATCH in future, where we need
>     to hook function entry before an instrumented function manipulates
>     the stack or argument registers. Practically speaking, we need to
>     preserve the argument/return registers, PC, LR, and SP.
> 
> Neither of these requires the full set of pt_regs, and only requires us
> to save/restore a subset of registers used for passing
> arguments/return-values and context/return information (which is the
> minimum set we always need to save/restore today).
> 
> As there is no longer a need to save different sets of registers for
> different features, we no longer need distinct `ftrace_caller` and
> `ftrace_regs_caller` trampolines. This allows the trampoline assembly to
> be simpler, and simplifies code which previously had to handle the two
> trampolines.
> 
> I've tested this with the ftrace selftests, where there are no
> unexpected failures.

Were there any "expected" failures?

> 
> I plan to build atop this with subsequent patches to add per-callsite
> ftrace_ops, and I'm sending these patches on their own as I think they
> make sense regardless.
> 
> Since v1 [1]:
> * Change ifdeferry per Steve's request
> * Add ftrace_regs_query_register_offset() per Masami's request
> * Fix a bunch of typos
> 
> [1] https://lore.kernel.org/lkml/20221024140846.3555435-1-mark.rutland@arm.com
> 
> This series can be found in my 'arm64/ftrace/minimal-regs' branch on
> kernel.org:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/
>   git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
> 
> This version is tagged as:
> 
>   arm64-ftrace-minimal-regs-20221103

So I ran this on top of my code through all my ftrace tests (for x86) and
it didn't cause any regressions.

Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>

-- Steve