Messages in this thread | | | Date | Thu, 14 Nov 2019 16:34:58 +0100 (CET) | From | Miroslav Benes <> | Subject | Re: [PATCH 09/10] ftrace/x86: Add register_ftrace_direct() for custom trampolines |
| |
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?
What are your plans regarding this?
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 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.
I may have missed something though.
Thanks Miroslav
| |