linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: "Wangshaobo (bobo)" <bobo.shaobowang@huawei.com>,
	cj.chengjian@huawei.com, huawei.libin@huawei.com,
	xiexiuqi@huawei.com, liwei391@huawei.com,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com,
	will@kernel.org, zengshun.wu@outlook.com
Subject: Re: [RFC PATCH -next v2 0/4] arm64/ftrace: support dynamic trampoline
Date: Wed, 25 May 2022 13:45:13 +0100	[thread overview]
Message-ID: <Yo4k2Y8oNcKG5ca0@FVFF77S0Q05N> (raw)
In-Reply-To: <20220421083758.37b239a4@gandalf.local.home>

On Thu, Apr 21, 2022 at 08:37:58AM -0400, Steven Rostedt wrote:
> On Thu, 21 Apr 2022 09:13:01 +0800
> "Wangshaobo (bobo)" <bobo.shaobowang@huawei.com> wrote:
> 
> > Not yet, Steve, ftrace_location() looks has no help to find a right 
> > rec->ip in our case,
> > 
> > ftrace_location() can find a right rec->ip when input ip is in the range 
> > between
> > 
> > sym+0 and sym+$end, but our question is how to  identify rec->ip from 
> > __mcount_loc,
> 
> Are you saying that the "ftrace location" is not between sym+0 and sym+$end?

IIUC yes -- this series as-is moves the call to the trampoline *before* sym+0.

Among other things that completely wrecks backtracing, so I'd *really* like to
avoid that (hance my suggested alternative).

> > this changed the patchable entry before bti to after in gcc:
> > 
> >     [1] https://reviews.llvm.org/D73680
> > 
> > gcc tells the place of first nop of the 5 NOPs when using 
> > -fpatchable-function-entry=5,3,
> > 
> > but not tells the first nop after bti, so we don't know how to adjust 
> > our rec->ip for ftrace.
> 
> OK, so I do not understand how the compiler is injecting bti with mcount
> calls, so I'll just walk away for now ;-)

When using BTI, the compiler has to drop a BTI *at* the function entry point
(i.e. sym+0) for any function that can be called indirectly, but can omit this
when the function is only directly called (which is the case for most functions
created via insterprocedural specialization, or for a number of static
functions).

Today, when we pass:

	-fpatchable-function-entry=2

... the compiler places 2 NOPs *after* any BTI, and records the location of the
first NOP. So the two cases we get are:

	__func_without_bti:
		NOP		<--- recorded location
		NOP

	__func_with_bti:
		BTI
		NOP		<--- recorded location
		NOP

... which works just fine, since either sym+0 or sym+4 are reasonable
locations for the patch-site to live.

However, if we were to pass:

	-fpatchable-function-entry=5,3

... the compiler places 3 NOPs *before* any BTI, and 2 NOPs *after* any BTI,
still recording the location of the first NOP. So in the two cases we get:

		NOP		<--- recorded location
		NOP
		NOP
	__func_without_bti:
		NOP
		NOP

		NOP		<--- recorded location
		NOP
		NOP
	__func_with_bti:
		BTI
		NOP
		NOP

... so where we want to patch one of the later nops to banch to a pre-function
NOP, we need to know whether or not the compiler generated a BTI. We can
discover discover that either by:

* Checking whether the recorded location is at sym+0 (no BTI) or sym+4 (BTI).

* Reading the instruction before the recorded location, and seeing if this is a
  BTI.

... and depending on how we handle thigns the two cases *might* need different
trampolines.

Thanks,
Mark.

  reply	other threads:[~2022-05-25 12:45 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-16 10:01 [RFC PATCH -next v2 0/4] arm64/ftrace: support dynamic trampoline Wang ShaoBo
2022-03-16 10:01 ` [RFC PATCH -next v2 1/4] arm64: introduce aarch64_insn_gen_load_literal Wang ShaoBo
2022-03-16 10:01 ` [RFC PATCH -next v2 2/4] arm64/ftrace: introduce ftrace dynamic trampoline entrances Wang ShaoBo
2022-03-16 10:01 ` [RFC PATCH -next v2 3/4] arm64/ftrace: support dynamically allocated trampolines Wang ShaoBo
2022-04-21 13:10   ` Mark Rutland
2022-04-21 14:06     ` Steven Rostedt
2022-04-21 14:08       ` Steven Rostedt
2022-04-21 15:14       ` Mark Rutland
2022-04-21 15:42         ` Steven Rostedt
2022-04-21 16:27           ` Mark Rutland
2022-04-21 17:06             ` Steven Rostedt
2022-04-22 10:12               ` Mark Rutland
2022-04-22 15:45                 ` Steven Rostedt
2022-04-22 17:27                   ` Mark Rutland
2022-04-26  8:47                     ` Masami Hiramatsu
2022-05-04 10:24                       ` Mark Rutland
2022-05-05  3:15                         ` Masami Hiramatsu
2022-05-09 18:22                           ` Steven Rostedt
2022-05-10  9:10                             ` Masami Hiramatsu
2022-05-10 14:44                               ` Steven Rostedt
2022-05-11 14:34                                 ` Masami Hiramatsu
2022-05-11 15:12                                   ` Steven Rostedt
2022-05-12 12:02                                     ` Masami Hiramatsu
2022-05-12 13:50                                       ` Steven Rostedt
2022-05-25 12:17                                       ` Mark Rutland
2022-05-25 13:43                                         ` Steven Rostedt
2022-05-25 17:12                                           ` Mark Rutland
2022-05-30  1:03                                         ` Masami Hiramatsu
2022-05-30 12:38                                           ` Jiri Olsa
2022-05-31  1:00                                             ` Masami Hiramatsu
2022-05-04 12:43               ` Mark Rutland
2022-05-05  2:57             ` Wangshaobo (bobo)
2022-05-25 12:27               ` Mark Rutland
2022-04-27  8:54       ` Wangshaobo (bobo)
2022-03-16 10:01 ` [RFC PATCH -next v2 4/4] arm64/ftrace: implement long jump for dynamic trampolines Wang ShaoBo
2022-04-21 13:47   ` Mark Rutland
2022-03-16 14:29 ` [RFC PATCH -next v2 0/4] arm64/ftrace: support dynamic trampoline Steven Rostedt
2022-04-20 18:11 ` Steven Rostedt
2022-04-21  1:13   ` Wangshaobo (bobo)
2022-04-21 12:37     ` Steven Rostedt
2022-05-25 12:45       ` Mark Rutland [this message]
2022-05-25 13:58         ` Steven Rostedt
2022-05-25 17:26           ` Mark Rutland
2022-04-21 12:53 ` Mark Rutland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Yo4k2Y8oNcKG5ca0@FVFF77S0Q05N \
    --to=mark.rutland@arm.com \
    --cc=bobo.shaobowang@huawei.com \
    --cc=catalin.marinas@arm.com \
    --cc=cj.chengjian@huawei.com \
    --cc=huawei.libin@huawei.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liwei391@huawei.com \
    --cc=rostedt@goodmis.org \
    --cc=will@kernel.org \
    --cc=xiexiuqi@huawei.com \
    --cc=zengshun.wu@outlook.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).