From: Steven Rostedt <rostedt@goodmis.org>
To: Miroslav Benes <mbenes@suse.cz>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
X86 ML <x86@kernel.org>, Nadav Amit <nadav.amit@gmail.com>,
Andy Lutomirski <luto@kernel.org>,
Dave Hansen <dave.hansen@linux.intel.com>,
Song Liu <songliubraving@fb.com>,
Masami Hiramatsu <mhiramat@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Daniel Bristot de Oliveira <bristot@redhat.com>,
Alexei Starovoitov <alexei.starovoitov@gmail.com>,
Josh Poimboeuf <jpoimboe@redhat.com>
Subject: Re: [PATCH 09/10] ftrace/x86: Add register_ftrace_direct() for custom trampolines
Date: Thu, 14 Nov 2019 11:19:52 -0500 [thread overview]
Message-ID: <20191114111952.3eb1a011@gandalf.local.home> (raw)
In-Reply-To: <alpine.LSU.2.21.1911141627300.20723@pobox.suse.cz>
On Thu, 14 Nov 2019 16:34:58 +0100 (CET)
Miroslav Benes <mbenes@suse.cz> wrote:
> On Fri, 8 Nov 2019, Steven Rostedt wrote:
>
> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> >
> > Enable x86 to allow for register_ftrace_direct(), where a custom trampoline
> > may be called directly from an ftrace mcount/fentry location.
> >
> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>
> [...]
>
> > +++ b/arch/x86/kernel/ftrace_64.S
> > @@ -88,6 +88,7 @@ EXPORT_SYMBOL(__fentry__)
> > movq %rdi, RDI(%rsp)
> > movq %r8, R8(%rsp)
> > movq %r9, R9(%rsp)
> > + movq $0, ORIG_RAX(%rsp)
> > /*
> > * Save the original RBP. Even though the mcount ABI does not
> > * require this, it helps out callers.
> > @@ -114,7 +115,8 @@ EXPORT_SYMBOL(__fentry__)
> > subq $MCOUNT_INSN_SIZE, %rdi
> > .endm
> >
> > -.macro restore_mcount_regs
> > +.macro restore_mcount_regs save=0
> > +
> > movq R9(%rsp), %r9
> > movq R8(%rsp), %r8
> > movq RDI(%rsp), %rdi
> > @@ -123,10 +125,7 @@ EXPORT_SYMBOL(__fentry__)
> > movq RCX(%rsp), %rcx
> > movq RAX(%rsp), %rax
> >
> > - /* ftrace_regs_caller can modify %rbp */
> > - movq RBP(%rsp), %rbp
> > -
> > - addq $MCOUNT_REG_SIZE, %rsp
> > + addq $MCOUNT_REG_SIZE-\save, %rsp
> >
> > .endm
> >
> > @@ -228,10 +227,30 @@ GLOBAL(ftrace_regs_call)
> > movq R10(%rsp), %r10
> > movq RBX(%rsp), %rbx
> >
> > - restore_mcount_regs
> > + movq RBP(%rsp), %rbp
> > +
> > + movq ORIG_RAX(%rsp), %rax
> > + movq %rax, MCOUNT_REG_SIZE-8(%rsp)
> > +
> > + /* If ORIG_RAX is anything but zero, make this a call to that */
> > + movq ORIG_RAX(%rsp), %rax
> > + cmpq $0, %rax
> > + je 1f
> > +
> > + /* Swap the flags with orig_rax */
> > + movq MCOUNT_REG_SIZE(%rsp), %rdi
> > + movq %rdi, MCOUNT_REG_SIZE-8(%rsp)
> > + movq %rax, MCOUNT_REG_SIZE(%rsp)
> > +
> > + restore_mcount_regs 8
> > +
> > + jmp 2f
> > +
> > +1: restore_mcount_regs
> > +
> >
> > /* Restore flags */
> > - popfq
> > +2: popfq
>
> If I am reading the code correctly (and I was confused couple of times, so
> maybe I am not), this is what makes the direct fops incompatible with
> ipmodify and livepatching for now. Is that correct?
Actually, it's the fact that the return goes to some unknown trampoline
that may do something else as well.
>
> What are your plans regarding this?
I wanted to see what the eBPF folks were doing, and then perhaps allow
the ip modify occur too. I could let it happen as well now, and then we
can see what the fallout is later ;-)
>
> Moreover, we could replace ftrace_regs_caller with direct fops for live
> patching when this is merged with all arch support we need. After all, all
Note, direct call is currently only available for x86_64.
> we need is to change the rip, which we could do easily in the direct
> trampoline. On the other hand, it would exclude coexistence of a live
> patch and a BPF filter (both direct now) on one function.
It may also end up being more complex, and not much of a performance
benefit. I believe the BPF is injecting programs into the start of
functions, but your trampoline for live patching may be not much
different than what ftrace gives you today.
-- Steve
next prev parent reply other threads:[~2019-11-14 16:19 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-08 21:28 [PATCH 00/10] ftrace: Add register_ftrace_direct() Steven Rostedt
2019-11-08 21:28 ` [PATCH 01/10] ftrace: Separate out the copying of a ftrace_hash from __ftrace_hash_move() Steven Rostedt
2019-11-08 21:28 ` [PATCH 02/10] ftrace: Separate out functionality from ftrace_location_range() Steven Rostedt
2019-11-08 21:28 ` [PATCH 03/10] ftrace: Add register_ftrace_direct() Steven Rostedt
2019-11-09 2:29 ` Alexei Starovoitov
2019-11-09 12:33 ` Steven Rostedt
2019-11-14 18:29 ` Steven Rostedt
2019-11-14 18:34 ` Alexei Starovoitov
2019-11-14 18:48 ` Steven Rostedt
2019-11-14 19:05 ` Steven Rostedt
2019-11-13 14:13 ` Miroslav Benes
2019-11-13 14:34 ` Steven Rostedt
2019-11-08 21:28 ` [PATCH 04/10] ftrace: Add ftrace_find_direct_func() Steven Rostedt
2019-11-08 21:28 ` [PATCH 05/10] ftrace: Add sample module that uses register_ftrace_direct() Steven Rostedt
2019-11-08 21:28 ` [PATCH 06/10] ftrace/selftest: Add tests to test register_ftrace_direct() Steven Rostedt
2019-11-08 21:28 ` [PATCH 07/10] ftrace: Add another example of register_ftrace_direct() use case Steven Rostedt
2019-11-08 21:28 ` [PATCH 08/10] ftrace/selftests: Update the direct call selftests to test two direct calls Steven Rostedt
2019-11-08 21:28 ` [PATCH 09/10] ftrace/x86: Add register_ftrace_direct() for custom trampolines Steven Rostedt
2019-11-14 15:34 ` Miroslav Benes
2019-11-14 16:19 ` Steven Rostedt [this message]
2019-11-15 9:32 ` Miroslav Benes
2019-11-08 21:28 ` [PATCH 10/10] ftrace/x86: Add a counter to test function_graph with direct Steven Rostedt
2019-11-08 22:51 ` [PATCH 00/10] ftrace: Add register_ftrace_direct() Josh Poimboeuf
2019-11-09 1:00 ` Steven Rostedt
2019-11-11 8:47 ` Peter Zijlstra
2019-11-11 14:15 ` Josh Poimboeuf
2019-11-13 15:10 ` Miroslav Benes
2019-11-13 16:31 ` Steven Rostedt
2019-11-14 9:05 ` Miroslav Benes
2019-11-14 14:36 ` Steven Rostedt
2019-11-14 15:42 ` Miroslav Benes
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=20191114111952.3eb1a011@gandalf.local.home \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=alexei.starovoitov@gmail.com \
--cc=bristot@redhat.com \
--cc=dave.hansen@linux.intel.com \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mbenes@suse.cz \
--cc=mhiramat@kernel.org \
--cc=mingo@kernel.org \
--cc=nadav.amit@gmail.com \
--cc=peterz@infradead.org \
--cc=songliubraving@fb.com \
--cc=x86@kernel.org \
/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).