linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Wang ShaoBo <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,
	Masami Hiramatsu <mhiramat@kernel.org>
Subject: Re: [RFC PATCH -next v2 3/4] arm64/ftrace: support dynamically allocated trampolines
Date: Tue, 26 Apr 2022 17:47:49 +0900	[thread overview]
Message-ID: <20220426174749.b5372c5769af7bf901649a05@kernel.org> (raw)
In-Reply-To: <YmLlmaXF00hPkOID@lakrids>

Hi Mark,

On Fri, 22 Apr 2022 18:27:53 +0100
Mark Rutland <mark.rutland@arm.com> wrote:

> On Fri, Apr 22, 2022 at 11:45:41AM -0400, Steven Rostedt wrote:
> > On Fri, 22 Apr 2022 11:12:39 +0100
> > Mark Rutland <mark.rutland@arm.com> wrote:
> > 
> > > As an aside, I'd also love to remove the REGS/!REGs distinction, and always
> > > save a minimum amount of state (like ARGS, but never saving a full pt_regs),
> > > since on arm64 the extra state stored for the REGS case isn't useful (and we
> > > can't reliably capture all of the pt_regs state anyway, so bits of it are made
> > > up or not filled in).
> > 
> > Note, the reason for the addition of REGS was a requirement of kprobes.
> > Because before ftrace, kprobes would be triggered at the start of a
> > function by a breakpoint that would load in all the regs. And for backward
> > compatibility, Masami wanted to make sure that kprobes coming from ftrace
> > had all the regs just like it had when coming from a breakpoint.

Yes. Since this kprobes->ftrace conversion is done by kprobes transparently,
user doesn't know their kprobe handler is called from sw break or ftrace.

> > 
> > IIUC, kprobes is the only reason we have the "regs" variant (all other use
> > cases could get by with the ARGS version).
> 
> I see. FWIW, we don't have KPROBES_ON_FTRACE on arm64.

Right. Since x86 fentry puts the entry on function address, I need such
compatibility.

But on arm64, ftrace leads some preparation instructions, kprobes can put
the sw break on the function address there. And may not need to put the
kprobes on it. So it depends on arch. I would like to keep the kprobes 
available at the function address so that it can trace any registers.
(like debugger usage)

> Also, the same problems apply to KRETPROBES: the synthetic `pstate`
> value is bogus and we don't fill in other bits of the regs (e.g. the PMR
> value), so it's not a "real" pt_regs, and things like
> interrupts_enabled(regs) won't work correctly.

Would you mean the process which kprobes_save/restore_local_irqflag() does?
Is the regs->pstate saved correctly in sw break or ftrace? (sorry, I missed
the context)

> In addition, as
> KRETPROBES only hooks function entry/exit and x9-x17 + x19-x28 are
> meaningless at those times, no-one's going to care what they contain
> anyway.

It depends on what bug they are trying to trace. C source level bug
will not need such information, but assembly level bug (or compiler
level bug) may need such registers. Anyway, this also depends on user.
I just won't like limit the usage.

> The state we can correctly snapshot (and that would be useful)
> is the same as ARGS.
> 
> It'd be nice if KRETPROBES could just use ARGS, but a standard KPROBE
> that traps could provide regs (since it actually gets "real" regs, and
> within a function the other GPRs could be important).

Here, the KRETPROBES means the exit handler, or including entry handler?
Since kretprobes uses a standard kprobe to trap the function entry.

If you talk about fprobes (ftrace probe interface), it will only use the
ftrace. Thus your idea is acceptable for it (because fprobe is different
from kprobes *).

* Of course we have to talk with BPF people so that they will only access
ARGS from BPF program on fprobes.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-04-26  8:49 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 [this message]
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
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=20220426174749.b5372c5769af7bf901649a05@kernel.org \
    --to=mhiramat@kernel.org \
    --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=mark.rutland@arm.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).