Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1317059 > unrolled thread

[PATCH v6 0/9] ftrace with regs + live patching for ppc64 LE (ABI v2)

Back to article view | Back to linux.kernel

Contents

  [PATCH v6 0/9] ftrace with regs + live patching for ppc64 LE (ABI v2) Torsten Duwe <duwe@lst.de> - 2016-01-25 18:10 +0100
    [PATCH v6 5/9] ppc64 ftrace_with_regs: spare early boot and low level Torsten Duwe <duwe@lst.de> - 2016-01-25 18:10 +0100
    [PATCH v6 8/9] Implement kernel live patching for ppc64le (ABIv2) Torsten Duwe <duwe@lst.de> - 2016-01-25 18:10 +0100
      Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le  (ABIv2) Miroslav Benes <mbenes@suse.cz> - 2016-01-26 12:00 +0100
        Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le (ABIv2) Petr Mladek <pmladek@suse.com> - 2016-01-26 13:50 +0100
          Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le  (ABIv2) Torsten Duwe <duwe@lst.de> - 2016-01-26 15:00 +0100
          Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le (ABIv2) Petr Mladek <pmladek@suse.com> - 2016-02-02 13:20 +0100
            Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le  (ABIv2) Torsten Duwe <duwe@lst.de> - 2016-02-02 16:50 +0100
              Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le (ABIv2) Petr Mladek <pmladek@suse.com> - 2016-02-02 17:50 +0100
                Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le  (ABIv2) Jiri Kosina <jikos@kernel.org> - 2016-02-02 21:40 +0100
        Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le  (ABIv2) Torsten Duwe <duwe@lst.de> - 2016-01-26 15:10 +0100
          Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le  (ABIv2) Miroslav Benes <mbenes@suse.cz> - 2016-01-26 15:20 +0100
            Re: Implement kernel live patching for ppc64le (ABIv2) Jessica Yu <jeyu@redhat.com> - 2016-01-27 03:00 +0100
      Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le (ABIv2) Denis Kirjanov <kda@linux-powerpc.org> - 2016-02-02 14:50 +0100
    [PATCH v6 2/9] ppc64le FTRACE_WITH_REGS implementation Torsten Duwe <duwe@lst.de> - 2016-01-25 18:10 +0100
    [PATCH v6 7/9] ppc64 ftrace: disable profiling for some files Torsten Duwe <duwe@lst.de> - 2016-01-25 18:20 +0100
    Re: [PATCH v6 0/9] ftrace with regs + live patching for ppc64 LE  (ABI v2) Balbir Singh <bsingharora@gmail.com> - 2016-01-27 12:00 +0100
      Re: [PATCH v6 0/9] ftrace with regs + live patching for ppc64 LE  (ABI v2) Torsten Duwe <duwe@lst.de> - 2016-01-27 13:20 +0100
        Re: [PATCH v6 0/9] ftrace with regs + live patching for ppc64 LE  (ABI v2) Balbir Singh <bsingharora@gmail.com> - 2016-01-28 03:50 +0100
          Re: [PATCH v6 0/9] ftrace with regs + live patching for ppc64 LE  (ABI v2) Michael Ellerman <mpe@ellerman.id.au> - 2016-01-28 04:40 +0100
            Re: [PATCH v6 0/9] ftrace with regs + live patching for ppc64 LE  (ABI v2) Torsten Duwe <duwe@lst.de> - 2016-01-28 12:20 +0100

Page 1 of 2  [1] 2  Next page →

#1317059 — [PATCH v6 0/9] ftrace with regs + live patching for ppc64 LE (ABI v2)

Changes since v5:
  * extra "std r0,LRSAVE(r1)" for gcc-6
    This makes the code compiler-agnostic.
  * Follow Petr Mladek's suggestion to avoid
    redefinition of HAVE_LIVEPATCH

Changes since v4:
  * change comment style in entry_64.S to C89
    (nobody is using assembler syntax comments there).
  * the bool function restore_r2 shouldn't return 2,
    that's a little confusing.
  * Test whether the compiler supports -mprofile-kernel
    and only then define CC_USING_MPROFILE_KERNEL
  * also make the return value of klp_check_compiler_support
    depend on that.

Major changes since v3:
  * the graph tracer works now.
    It turned out the stack frame it tried to manipulate does not
    exist at that point.
  * changes only needed in order to support -mprofile-kernel are now
    in a separate patch, prepended.
  * Kconfig cleanup so this is only selectable on ppc64le.


Torsten Duwe (9):
  ppc64 (le): prepare for -mprofile-kernel
  ppc64le FTRACE_WITH_REGS implementation
  ppc use ftrace_modify_all_code default
  ppc64 ftrace_with_regs configuration variables
  ppc64 ftrace_with_regs: spare early boot and low level
  ppc64 ftrace: disable profiling for some functions
  ppc64 ftrace: disable profiling for some files
  Implement kernel live patching for ppc64le (ABIv2)
  Enable LIVEPATCH to be configured on ppc64le and add livepatch.o if it
    is selected.

 arch/powerpc/Kconfig                 |   7 ++
 arch/powerpc/Makefile                |  10 +++
 arch/powerpc/include/asm/ftrace.h    |   5 ++
 arch/powerpc/include/asm/livepatch.h |  45 ++++++++++
 arch/powerpc/kernel/Makefile         |  13 +--
 arch/powerpc/kernel/entry_64.S       | 166 ++++++++++++++++++++++++++++++++++-
 arch/powerpc/kernel/ftrace.c         |  88 ++++++++++++++-----
 arch/powerpc/kernel/livepatch.c      |  38 ++++++++
 arch/powerpc/kernel/module_64.c      |  39 +++++++-
 arch/powerpc/kernel/process.c        |   2 +-
 arch/powerpc/lib/Makefile            |   4 +-
 arch/powerpc/mm/fault.c              |   2 +-
 arch/powerpc/mm/hash_utils_64.c      |  18 ++--
 arch/powerpc/mm/hugetlbpage-hash64.c |   2 +-
 arch/powerpc/mm/hugetlbpage.c        |   4 +-
 arch/powerpc/mm/mem.c                |   2 +-
 arch/powerpc/mm/pgtable_64.c         |   2 +-
 arch/powerpc/mm/slb.c                |   6 +-
 arch/powerpc/mm/slice.c              |   8 +-
 kernel/trace/Kconfig                 |   5 ++
 20 files changed, 412 insertions(+), 54 deletions(-)
 create mode 100644 arch/powerpc/include/asm/livepatch.h
 create mode 100644 arch/powerpc/kernel/livepatch.c

-- 
1.8.5.6

[toc] | [next] | [standalone]

#1317060 — [PATCH v6 5/9] ppc64 ftrace_with_regs: spare early boot and low level

Using -mprofile-kernel on early boot code not only confuses the
checker but is also useless, as the infrastructure is not yet in
place. Proceed like with -pg (remove it from CFLAGS), equally with
time.o and ftrace itself.

  * arch/powerpc/kernel/Makefile:
    - remove -mprofile-kernel from low level and boot code objects'
      CFLAGS for FUNCTION_TRACER configurations.

Signed-off-by: Torsten Duwe <duwe@suse.de>
---
 arch/powerpc/kernel/Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index ba33693..0f417d5 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -16,14 +16,14 @@ endif
 
 ifdef CONFIG_FUNCTION_TRACER
 # Do not trace early boot code
-CFLAGS_REMOVE_cputable.o = -pg -mno-sched-epilog
-CFLAGS_REMOVE_prom_init.o = -pg -mno-sched-epilog
-CFLAGS_REMOVE_btext.o = -pg -mno-sched-epilog
-CFLAGS_REMOVE_prom.o = -pg -mno-sched-epilog
+CFLAGS_REMOVE_cputable.o = -pg -mno-sched-epilog -mprofile-kernel
+CFLAGS_REMOVE_prom_init.o = -pg -mno-sched-epilog -mprofile-kernel
+CFLAGS_REMOVE_btext.o = -pg -mno-sched-epilog -mprofile-kernel
+CFLAGS_REMOVE_prom.o = -pg -mno-sched-epilog -mprofile-kernel
 # do not trace tracer code
-CFLAGS_REMOVE_ftrace.o = -pg -mno-sched-epilog
+CFLAGS_REMOVE_ftrace.o = -pg -mno-sched-epilog -mprofile-kernel
 # timers used by tracing
-CFLAGS_REMOVE_time.o = -pg -mno-sched-epilog
+CFLAGS_REMOVE_time.o = -pg -mno-sched-epilog -mprofile-kernel
 endif
 
 obj-y				:= cputable.o ptrace.o syscalls.o \
-- 
1.8.5.6

[toc] | [prev] | [next] | [standalone]

#1317061 — [PATCH v6 8/9] Implement kernel live patching for ppc64le (ABIv2)

  * create the appropriate files+functions
    arch/powerpc/include/asm/livepatch.h
        klp_check_compiler_support,
        klp_arch_set_pc
    arch/powerpc/kernel/livepatch.c with a stub for
        klp_write_module_reloc
    This is architecture-independent work in progress.
  * introduce a fixup in arch/powerpc/kernel/entry_64.S
    for local calls that are becoming global due to live patching.
    And of course do the main KLP thing: return to a maybe different
    address, possibly altered by the live patching ftrace op.

Signed-off-by: Torsten Duwe <duwe@suse.de>
---
 arch/powerpc/include/asm/livepatch.h | 45 +++++++++++++++++++++++++++++++
 arch/powerpc/kernel/entry_64.S       | 51 +++++++++++++++++++++++++++++++++---
 arch/powerpc/kernel/livepatch.c      | 38 +++++++++++++++++++++++++++
 3 files changed, 130 insertions(+), 4 deletions(-)
 create mode 100644 arch/powerpc/include/asm/livepatch.h
 create mode 100644 arch/powerpc/kernel/livepatch.c

diff --git a/arch/powerpc/include/asm/livepatch.h b/arch/powerpc/include/asm/livepatch.h
new file mode 100644
index 0000000..44e8a2d
--- /dev/null
+++ b/arch/powerpc/include/asm/livepatch.h
@@ -0,0 +1,45 @@
+/*
+ * livepatch.h - powerpc-specific Kernel Live Patching Core
+ *
+ * Copyright (C) 2015 SUSE
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef _ASM_POWERPC64_LIVEPATCH_H
+#define _ASM_POWERPC64_LIVEPATCH_H
+
+#include <linux/module.h>
+#include <linux/ftrace.h>
+
+#ifdef CONFIG_LIVEPATCH
+static inline int klp_check_compiler_support(void)
+{
+#if !defined(_CALL_ELF) || _CALL_ELF != 2 || !defined(CC_USING_MPROFILE_KERNEL)
+	return 1;
+#endif
+	return 0;
+}
+
+extern int klp_write_module_reloc(struct module *mod, unsigned long type,
+				   unsigned long loc, unsigned long value);
+
+static inline void klp_arch_set_pc(struct pt_regs *regs, unsigned long ip)
+{
+	regs->nip = ip;
+}
+#else
+#error Live patching support is disabled; check CONFIG_LIVEPATCH
+#endif
+
+#endif /* _ASM_POWERPC64_LIVEPATCH_H */
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 9e98aa1..f6e3ee7 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -1267,6 +1267,9 @@ _GLOBAL(ftrace_caller)
 	mflr    r3
 	std     r3, _NIP(r1)
 	std	r3, 16(r1)
+#ifdef CONFIG_LIVEPATCH
+	mr	r14,r3		/* remember old NIP */
+#endif
 	subi    r3, r3, MCOUNT_INSN_SIZE
 	mfmsr   r4
 	std     r4, _MSR(r1)
@@ -1283,7 +1286,10 @@ ftrace_call:
 	nop
 
 	ld	r3, _NIP(r1)
-	mtlr	r3
+	mtctr	r3		/* prepare to jump there */
+#ifdef CONFIG_LIVEPATCH
+	cmpd	r14,r3		/* has NIP been altered? */
+#endif
 
 	REST_8GPRS(0,r1)
 	REST_8GPRS(8,r1)
@@ -1296,6 +1302,27 @@ ftrace_call:
 	mtlr	r12
 	mr	r2,r0		/* restore callee's TOC */
 
+#ifdef CONFIG_LIVEPATCH
+	beq+	4f		/* likely(old_NIP == new_NIP) */
+
+	/* For a local call, restore this TOC after calling the patch function.
+	 * For a global call, it does not matter what we restore here,
+	 * since the global caller does its own restore right afterwards,
+	 * anyway. Just insert a KLP_return_helper frame in any case,
+	 * so a patch function can always count on the changed stack offsets.
+	 */
+	stdu	r1,-32(r1)	/* open new mini stack frame */
+	std	r0,24(r1)	/* save TOC now, unconditionally. */
+	bl	5f
+5:	mflr	r12
+	addi	r12,r12,(KLP_return_helper+4-.)@l
+	std	r12,LRSAVE(r1)
+	mtlr	r12
+	mfctr	r12		/* allow for TOC calculation in newfunc */
+	bctr
+4:
+#endif
+
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 	stdu	r1, -112(r1)
 .globl ftrace_graph_call
@@ -1305,15 +1332,31 @@ _GLOBAL(ftrace_graph_stub)
 	addi	r1, r1, 112
 #endif
 
-	mflr	r0		/* move this LR to CTR */
-	mtctr	r0
-
 	ld	r0,LRSAVE(r1)	/* restore callee's lr at _mcount site */
 	mtlr	r0
 	bctr			/* jump after _mcount site */
 #endif /* CC_USING_MPROFILE_KERNEL */
 _GLOBAL(ftrace_stub)
 	blr
+
+#ifdef CONFIG_LIVEPATCH
+/* Helper function for local calls that are becoming global
+   due to live patching.
+   We can't simply patch the NOP after the original call,
+   because, depending on the consistency model, some kernel
+   threads may still have called the original, local function
+   *without* saving their TOC in the respective stack frame slot,
+   so the decision is made per-thread during function return by
+   maybe inserting a KLP_return_helper frame or not.
+*/
+KLP_return_helper:
+	ld	r2,24(r1)	/* restore TOC (saved by ftrace_caller) */
+	addi r1, r1, 32		/* destroy mini stack frame */
+	ld	r0,LRSAVE(r1)	/* get the real return address */
+	mtlr	r0
+	blr
+#endif
+
 #else
 _GLOBAL_TOC(_mcount)
 	/* Taken from output of objdump from lib64/glibc */
diff --git a/arch/powerpc/kernel/livepatch.c b/arch/powerpc/kernel/livepatch.c
new file mode 100644
index 0000000..564eafa
--- /dev/null
+++ b/arch/powerpc/kernel/livepatch.c
@@ -0,0 +1,38 @@
+/*
+ * livepatch.c - powerpc-specific Kernel Live Patching Core
+ *
+ * Copyright (C) 2015 SUSE
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/module.h>
+#include <asm/livepatch.h>
+
+/**
+ * klp_write_module_reloc() - write a relocation in a module
+ * @mod:       module in which the section to be modified is found
+ * @type:      ELF relocation type (see asm/elf.h)
+ * @loc:       address that the relocation should be written to
+ * @value:     relocation value (sym address + addend)
+ *
+ * This function writes a relocation to the specified location for
+ * a particular module.
+ */
+int klp_write_module_reloc(struct module *mod, unsigned long type,
+			    unsigned long loc, unsigned long value)
+{
+	/* This requires infrastructure changes; we need the loadinfos. */
+	pr_err("lpc_write_module_reloc not yet supported\n");
+	return -ENOSYS;
+}
-- 
1.8.5.6

[toc] | [prev] | [next] | [standalone]

#1317734 — Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le (ABIv2)

[ added Petr to CC list ]

On Mon, 25 Jan 2016, Torsten Duwe wrote:

>   * create the appropriate files+functions
>     arch/powerpc/include/asm/livepatch.h
>         klp_check_compiler_support,
>         klp_arch_set_pc
>     arch/powerpc/kernel/livepatch.c with a stub for
>         klp_write_module_reloc
>     This is architecture-independent work in progress.
>   * introduce a fixup in arch/powerpc/kernel/entry_64.S
>     for local calls that are becoming global due to live patching.
>     And of course do the main KLP thing: return to a maybe different
>     address, possibly altered by the live patching ftrace op.
> 
> Signed-off-by: Torsten Duwe <duwe@suse.de>

Hi,

I have a few questions...

We still need Petr's patch from [1] to make livepatch work, right? Could 
you, please, add it to this patch set to make it self-sufficient?

Second, what is the situation with mcount prologue between gcc < 6 and 
gcc-6? Are there only 12 bytes in gcc-6 prologue? If yes, we need to 
change Petr's patch to make it more general and to be able to cope with 
different prologues. This is unfortunate. Either way, please mention it 
somewhere in a changelog.

I haven't reviewed the patch properly yet, but there is a comment below.

[1] http://lkml.kernel.org/g/20151203160004.GE8047@pathway.suse.cz

> +/**
> + * klp_write_module_reloc() - write a relocation in a module
> + * @mod:       module in which the section to be modified is found
> + * @type:      ELF relocation type (see asm/elf.h)
> + * @loc:       address that the relocation should be written to
> + * @value:     relocation value (sym address + addend)
> + *
> + * This function writes a relocation to the specified location for
> + * a particular module.
> + */
> +int klp_write_module_reloc(struct module *mod, unsigned long type,
> +			    unsigned long loc, unsigned long value)
> +{
> +	/* This requires infrastructure changes; we need the loadinfos. */
> +	pr_err("lpc_write_module_reloc not yet supported\n");

This is a nit, but there is no lpc_write_module_reloc. It should be 
klp_write_module_reloc.

Thanks,
Miroslav

[toc] | [prev] | [next] | [standalone]

#1317845 — Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le (ABIv2)

On Tue 2016-01-26 11:50:25, Miroslav Benes wrote:
> 
> [ added Petr to CC list ]
> 
> On Mon, 25 Jan 2016, Torsten Duwe wrote:
> 
> >   * create the appropriate files+functions
> >     arch/powerpc/include/asm/livepatch.h
> >         klp_check_compiler_support,
> >         klp_arch_set_pc
> >     arch/powerpc/kernel/livepatch.c with a stub for
> >         klp_write_module_reloc
> >     This is architecture-independent work in progress.
> >   * introduce a fixup in arch/powerpc/kernel/entry_64.S
> >     for local calls that are becoming global due to live patching.
> >     And of course do the main KLP thing: return to a maybe different
> >     address, possibly altered by the live patching ftrace op.
> > 
> > Signed-off-by: Torsten Duwe <duwe@suse.de>
> 
> Hi,
> 
> I have a few questions...
> 
> We still need Petr's patch from [1] to make livepatch work, right? Could 
> you, please, add it to this patch set to make it self-sufficient?
> 
> Second, what is the situation with mcount prologue between gcc < 6 and 
> gcc-6? Are there only 12 bytes in gcc-6 prologue? If yes, we need to 
> change Petr's patch to make it more general and to be able to cope with 
> different prologues. This is unfortunate. Either way, please mention it 
> somewhere in a changelog.

I am going to update the extra patch. There is an idea to detect the
offset during build by scrips/recordmcount. This tool looks for the
ftrace locations. The offset should always be a constant that depends
on the used architecture, compiler, and compiler flags.

The tool is called post build. We might need to pass the constant
as a symbol added to the binary. The tool already adds some symbols.


Best Regards,
Petr

[toc] | [prev] | [next] | [standalone]

#1317928 — Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le (ABIv2)

On Tue, Jan 26, 2016 at 01:48:53PM +0100, Petr Mladek wrote:
> On Tue 2016-01-26 11:50:25, Miroslav Benes wrote:
> > 
> > We still need Petr's patch from [1] to make livepatch work, right? Could 
> > you, please, add it to this patch set to make it self-sufficient?

It's Petr's patch, I don't want to decide how to best tackle this, see below.
I think Michael is already aware that it is needed, too.

> > Second, what is the situation with mcount prologue between gcc < 6 and 
> > gcc-6? Are there only 12 bytes in gcc-6 prologue? If yes, we need to 

Precisely, it's commit e95d0248daced44 (in http://repo.or.cz/official-gcc.git)
or svn trunk change 222352 "No need for -mprofile-kernel to save LR to stack."
It's efficient, I like it.

> I am going to update the extra patch. There is an idea to detect the
> offset during build by scrips/recordmcount. This tool looks for the
> ftrace locations. The offset should always be a constant that depends
> on the used architecture, compiler, and compiler flags.

My first idea was to check for compiler version defines, but some vendors
are rumoured to patch their compilers ;-)

> The tool is called post build. We might need to pass the constant
> as a symbol added to the binary. The tool already adds some symbols.

That's even better.
Thanks!
	Torsten

[toc] | [prev] | [next] | [standalone]

#1324023 — Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le (ABIv2)

On Tue 2016-01-26 13:48:53, Petr Mladek wrote:
> On Tue 2016-01-26 11:50:25, Miroslav Benes wrote:
> > 
> > [ added Petr to CC list ]
> > 
> > On Mon, 25 Jan 2016, Torsten Duwe wrote:
> > 
> > >   * create the appropriate files+functions
> > >     arch/powerpc/include/asm/livepatch.h
> > >         klp_check_compiler_support,
> > >         klp_arch_set_pc
> > >     arch/powerpc/kernel/livepatch.c with a stub for
> > >         klp_write_module_reloc
> > >     This is architecture-independent work in progress.
> > >   * introduce a fixup in arch/powerpc/kernel/entry_64.S
> > >     for local calls that are becoming global due to live patching.
> > >     And of course do the main KLP thing: return to a maybe different
> > >     address, possibly altered by the live patching ftrace op.
> > > 
> > > Signed-off-by: Torsten Duwe <duwe@suse.de>
> > 
> > Hi,
> > 
> > I have a few questions...
> > 
> > We still need Petr's patch from [1] to make livepatch work, right? Could 
> > you, please, add it to this patch set to make it self-sufficient?
> > 
> > Second, what is the situation with mcount prologue between gcc < 6 and 
> > gcc-6? Are there only 12 bytes in gcc-6 prologue? If yes, we need to 
> > change Petr's patch to make it more general and to be able to cope with 
> > different prologues. This is unfortunate. Either way, please mention it 
> > somewhere in a changelog.
> 
> I am going to update the extra patch. There is an idea to detect the
> offset during build by scrips/recordmcount. This tool looks for the
> ftrace locations. The offset should always be a constant that depends
> on the used architecture, compiler, and compiler flags.
> 
> The tool is called post build. We might need to pass the constant
> as a symbol added to the binary. The tool already adds some symbols.

Hmm, the size of the offset is not a constant. In particular, leaf
functions do not set TOC before the mcount location.

For example, the code generated for int_to_scsilun() looks like:


00000000000002d0 <int_to_scsilun>:
 2d0:   a6 02 08 7c     mflr    r0
 2d4:   10 00 01 f8     std     r0,16(r1)
 2d8:   01 00 00 48     bl      2d8 <int_to_scsilun+0x8>
                        2d8: R_PPC64_REL24      _mcount
 2dc:   a6 02 08 7c     mflr    r0
 2e0:   10 00 01 f8     std     r0,16(r1)
 2e4:   e1 ff 21 f8     stdu    r1,-32(r1)
 2e8:   00 00 20 39     li      r9,0
 2ec:   00 00 24 f9     std     r9,0(r4)
 2f0:   04 00 20 39     li      r9,4
 2f4:   a6 03 29 7d     mtctr   r9
 2f8:   00 00 40 39     li      r10,0
 2fc:   02 c2 68 78     rldicl  r8,r3,56,8
 300:   78 23 89 7c     mr      r9,r4
 304:   ee 51 09 7d     stbux   r8,r9,r10
 308:   02 00 4a 39     addi    r10,r10,2
 30c:   01 00 69 98     stb     r3,1(r9)
 310:   02 84 63 78     rldicl  r3,r3,48,16
 314:   e8 ff 00 42     bdnz    2fc <int_to_scsilun+0x2c>
 318:   20 00 21 38     addi    r1,r1,32
 31c:   10 00 01 e8     ld      r0,16(r1)
 320:   a6 03 08 7c     mtlr    r0
 324:   20 00 80 4e     blr
 328:   00 00 00 60     nop
 32c:   00 00 42 60     ori     r2,r2,0


Note that non-leaf functions starts with

0000000000000330 <scsi_set_sense_information>:
 330:   00 00 4c 3c     addis   r2,r12,0
                        330: R_PPC64_REL16_HA   .TOC.
 334:   00 00 42 38     addi    r2,r2,0
                        334: R_PPC64_REL16_LO   .TOC.+0x4
 338:   a6 02 08 7c     mflr    r0
 33c:   10 00 01 f8     std     r0,16(r1)
 340:   01 00 00 48     bl      340 <scsi_set_sense_information+0x10>
                        340: R_PPC64_REL24      _mcount


The above code is generated from kernel-4.5-rc1 sources using

$> gcc --version
gcc (SUSE Linux) 4.8.5
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.


But I get similar code also with

$> gcc-6 --version
gcc-6 (SUSE Linux) 6.0.0 20160121 (experimental) [trunk revision 232670]
Copyright (C) 2016 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.



The result is that kernel crashes when trying to trace leaf function
from modules. The mcount location is replaced with a call (branch)
that does not work without the TOC stuff.

By other words, it seems that the code generated with -mprofile-kernel
option has been buggy in all gcc versions.

I am curious that nobody found this earlier. Do I something wrong,
please?


Best Regards,
Petr

[toc] | [prev] | [next] | [standalone]

#1324216 — Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le (ABIv2)

On Tue, Feb 02, 2016 at 01:12:24PM +0100, Petr Mladek wrote:
> 
> Hmm, the size of the offset is not a constant. In particular, leaf
> functions do not set TOC before the mcount location.

To be slightly more precise, a leaf function that additionally uses
no global data. No global function calls, no global data access =>
no need to load the TOC.

> For example, the code generated for int_to_scsilun() looks like:
> 
> 
> 00000000000002d0 <int_to_scsilun>:
>  2d0:   a6 02 08 7c     mflr    r0
>  2d4:   10 00 01 f8     std     r0,16(r1)
>  2d8:   01 00 00 48     bl      2d8 <int_to_scsilun+0x8>
>                         2d8: R_PPC64_REL24      _mcount
[...]
> The above code is generated from kernel-4.5-rc1 sources using
> 
> $> gcc --version
> gcc (SUSE Linux) 4.8.5
> 
> But I get similar code also with
> 
> $> gcc-6 --version
> gcc-6 (SUSE Linux) 6.0.0 20160121 (experimental) [trunk revision 232670]
> 
> 
> The result is that kernel crashes when trying to trace leaf function

The trampoline *requires* a proper TOC pointer to find the remote function
entry point. If you jump onto the trampoline with the TOC from the caller's
caller you'll grab some address from somewhere and jump into nirvana.

> By other words, it seems that the code generated with -mprofile-kernel
> option has been buggy in all gcc versions.

Either that or we need bigger trampolines for everybody.

Michael, should we grow every module trampoline to always load R2,
or fix GCC to recognise the generated bl _mcount as a global function call?
Anton, what do you think?

	Torsten

[toc] | [prev] | [next] | [standalone]

#1324275 — Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le (ABIv2)

On Tue 2016-02-02 16:45:23, Torsten Duwe wrote:
> On Tue, Feb 02, 2016 at 01:12:24PM +0100, Petr Mladek wrote:
> > 
> > Hmm, the size of the offset is not a constant. In particular, leaf
> > functions do not set TOC before the mcount location.
> 
> To be slightly more precise, a leaf function that additionally uses
> no global data. No global function calls, no global data access =>
> no need to load the TOC.

Thanks for explanation.
 
> > The result is that kernel crashes when trying to trace leaf function
> 
> The trampoline *requires* a proper TOC pointer to find the remote function
> entry point. If you jump onto the trampoline with the TOC from the caller's
> caller you'll grab some address from somewhere and jump into nirvana.

The dmesg messages suggested someting like this.


> > By other words, it seems that the code generated with -mprofile-kernel
> > option has been buggy in all gcc versions.
> 
> Either that or we need bigger trampolines for everybody.
> 
> Michael, should we grow every module trampoline to always load R2,
> or fix GCC to recognise the generated bl _mcount as a global function call?
> Anton, what do you think?

BTW: Is the trampoline used also for classic probes? If not, we might need
a trampoline for them as well.

Note that TOC is not set only when the problematic functions are
compiled with --mprofile-kernel. I still see the TOC stuff when
compiling only with -pg.


Best Regards,
Petr

[toc] | [prev] | [next] | [standalone]

#1324606 — Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le (ABIv2)

On Tue, 2 Feb 2016, Petr Mladek wrote:

> Note that TOC is not set only when the problematic functions are 
> compiled with --mprofile-kernel. I still see the TOC stuff when 
> compiling only with -pg.

I don't see how this wouldn't be a gcc bug.

No matter whether it's plain profiling call (-pg) or kernel profiling call 
(-mprofile-kernel), gcc must always assume that global function (that will 
typically have just one instance for the whole address space) will be 
called.

-- 
Jiri Kosina
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1317941 — Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le (ABIv2)

On Tue, Jan 26, 2016 at 11:50:25AM +0100, Miroslav Benes wrote:
> > + */
> > +int klp_write_module_reloc(struct module *mod, unsigned long type,
> > +			    unsigned long loc, unsigned long value)
> > +{
> > +	/* This requires infrastructure changes; we need the loadinfos. */
> > +	pr_err("lpc_write_module_reloc not yet supported\n");
> 
> This is a nit, but there is no lpc_write_module_reloc. It should be 
> klp_write_module_reloc.

Indeed. Michael, feel free to fix this on the fly or not. It needs to
disappear anyway and be replaced with functionality.

	Torsten

[toc] | [prev] | [next] | [standalone]

#1317956 — Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le (ABIv2)

[ Jessica added to CC list so she is aware that there are plans to 
implement livepatch on ppc64le ]

On Tue, 26 Jan 2016, Torsten Duwe wrote:

> On Tue, Jan 26, 2016 at 11:50:25AM +0100, Miroslav Benes wrote:
> > > + */
> > > +int klp_write_module_reloc(struct module *mod, unsigned long type,
> > > +			    unsigned long loc, unsigned long value)
> > > +{
> > > +	/* This requires infrastructure changes; we need the loadinfos. */
> > > +	pr_err("lpc_write_module_reloc not yet supported\n");
> > 
> > This is a nit, but there is no lpc_write_module_reloc. It should be 
> > klp_write_module_reloc.
> 
> Indeed. Michael, feel free to fix this on the fly or not. It needs to
> disappear anyway and be replaced with functionality.

Or not at all thanks to Jessica's effort [1].

Miroslav

[1] http://lkml.kernel.org/g/1452281304-28618-1-git-send-email-jeyu@redhat.com

[toc] | [prev] | [next] | [standalone]

#1318543 — Re: Implement kernel live patching for ppc64le (ABIv2)

+++ Miroslav Benes [26/01/16 15:14 +0100]:
>
>[ Jessica added to CC list so she is aware that there are plans to
>implement livepatch on ppc64le ]
>
>On Tue, 26 Jan 2016, Torsten Duwe wrote:
>
>> On Tue, Jan 26, 2016 at 11:50:25AM +0100, Miroslav Benes wrote:
>> > > + */
>> > > +int klp_write_module_reloc(struct module *mod, unsigned long type,
>> > > +			    unsigned long loc, unsigned long value)
>> > > +{
>> > > +	/* This requires infrastructure changes; we need the loadinfos. */
>> > > +	pr_err("lpc_write_module_reloc not yet supported\n");
>> >
>> > This is a nit, but there is no lpc_write_module_reloc. It should be
>> > klp_write_module_reloc.
>>
>> Indeed. Michael, feel free to fix this on the fly or not. It needs to
>> disappear anyway and be replaced with functionality.
>
>Or not at all thanks to Jessica's effort [1].
>
>Miroslav
>
>[1] http://lkml.kernel.org/g/1452281304-28618-1-git-send-email-jeyu@redhat.com

Miroslav, thanks for the CC. Indeed, if things go well, there may be
no need to implement klp_write_module_reloc() anymore in the near
future. :-)

Jessica

[toc] | [prev] | [next] | [standalone]

#1324100 — Re: [PATCH v6 8/9] Implement kernel live patching for ppc64le (ABIv2)

On 1/25/16, Torsten Duwe <duwe@lst.de> wrote:
>   * create the appropriate files+functions
>     arch/powerpc/include/asm/livepatch.h
>         klp_check_compiler_support,
>         klp_arch_set_pc
>     arch/powerpc/kernel/livepatch.c with a stub for
>         klp_write_module_reloc
>     This is architecture-independent work in progress.
>   * introduce a fixup in arch/powerpc/kernel/entry_64.S
>     for local calls that are becoming global due to live patching.
>     And of course do the main KLP thing: return to a maybe different
>     address, possibly altered by the live patching ftrace op.
>
> Signed-off-by: Torsten Duwe <duwe@suse.de>
> ---
>  arch/powerpc/include/asm/livepatch.h | 45 +++++++++++++++++++++++++++++++
>  arch/powerpc/kernel/entry_64.S       | 51
> +++++++++++++++++++++++++++++++++---
>  arch/powerpc/kernel/livepatch.c      | 38 +++++++++++++++++++++++++++
>  3 files changed, 130 insertions(+), 4 deletions(-)
>  create mode 100644 arch/powerpc/include/asm/livepatch.h
>  create mode 100644 arch/powerpc/kernel/livepatch.c
>
> diff --git a/arch/powerpc/include/asm/livepatch.h
> b/arch/powerpc/include/asm/livepatch.h
> new file mode 100644
> index 0000000..44e8a2d
> --- /dev/null
> +++ b/arch/powerpc/include/asm/livepatch.h
> @@ -0,0 +1,45 @@
> +/*
> + * livepatch.h - powerpc-specific Kernel Live Patching Core
> + *
> + * Copyright (C) 2015 SUSE
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef _ASM_POWERPC64_LIVEPATCH_H
> +#define _ASM_POWERPC64_LIVEPATCH_H
> +
> +#include <linux/module.h>
> +#include <linux/ftrace.h>
> +
> +#ifdef CONFIG_LIVEPATCH
> +static inline int klp_check_compiler_support(void)
> +{
> +#if !defined(_CALL_ELF) || _CALL_ELF != 2 ||
> !defined(CC_USING_MPROFILE_KERNEL)
> +	return 1;
> +#endif
> +	return 0;
> +}
This function can be boolean.
> +
> +extern int klp_write_module_reloc(struct module *mod, unsigned long type,
> +				   unsigned long loc, unsigned long value);
> +
> +static inline void klp_arch_set_pc(struct pt_regs *regs, unsigned long ip)
> +{
> +	regs->nip = ip;
> +}
> +#else
> +#error Live patching support is disabled; check CONFIG_LIVEPATCH
> +#endif
> +
> +#endif /* _ASM_POWERPC64_LIVEPATCH_H */
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 9e98aa1..f6e3ee7 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -1267,6 +1267,9 @@ _GLOBAL(ftrace_caller)
>  	mflr    r3
>  	std     r3, _NIP(r1)
>  	std	r3, 16(r1)
> +#ifdef CONFIG_LIVEPATCH
> +	mr	r14,r3		/* remember old NIP */
> +#endif
>  	subi    r3, r3, MCOUNT_INSN_SIZE
>  	mfmsr   r4
>  	std     r4, _MSR(r1)
> @@ -1283,7 +1286,10 @@ ftrace_call:
>  	nop
>
>  	ld	r3, _NIP(r1)
> -	mtlr	r3
> +	mtctr	r3		/* prepare to jump there */
> +#ifdef CONFIG_LIVEPATCH
> +	cmpd	r14,r3		/* has NIP been altered? */
> +#endif
>
>  	REST_8GPRS(0,r1)
>  	REST_8GPRS(8,r1)
> @@ -1296,6 +1302,27 @@ ftrace_call:
>  	mtlr	r12
>  	mr	r2,r0		/* restore callee's TOC */
>
> +#ifdef CONFIG_LIVEPATCH
> +	beq+	4f		/* likely(old_NIP == new_NIP) */
> +
> +	/* For a local call, restore this TOC after calling the patch function.
> +	 * For a global call, it does not matter what we restore here,
> +	 * since the global caller does its own restore right afterwards,
> +	 * anyway. Just insert a KLP_return_helper frame in any case,
> +	 * so a patch function can always count on the changed stack offsets.
> +	 */
> +	stdu	r1,-32(r1)	/* open new mini stack frame */
> +	std	r0,24(r1)	/* save TOC now, unconditionally. */
> +	bl	5f
> +5:	mflr	r12
> +	addi	r12,r12,(KLP_return_helper+4-.)@l
> +	std	r12,LRSAVE(r1)
> +	mtlr	r12
> +	mfctr	r12		/* allow for TOC calculation in newfunc */
> +	bctr
> +4:
> +#endif
> +
>  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
>  	stdu	r1, -112(r1)
>  .globl ftrace_graph_call
> @@ -1305,15 +1332,31 @@ _GLOBAL(ftrace_graph_stub)
>  	addi	r1, r1, 112
>  #endif
>
> -	mflr	r0		/* move this LR to CTR */
> -	mtctr	r0
> -
>  	ld	r0,LRSAVE(r1)	/* restore callee's lr at _mcount site */
>  	mtlr	r0
>  	bctr			/* jump after _mcount site */
>  #endif /* CC_USING_MPROFILE_KERNEL */
>  _GLOBAL(ftrace_stub)
>  	blr
> +
> +#ifdef CONFIG_LIVEPATCH
> +/* Helper function for local calls that are becoming global
> +   due to live patching.
> +   We can't simply patch the NOP after the original call,
> +   because, depending on the consistency model, some kernel
> +   threads may still have called the original, local function
> +   *without* saving their TOC in the respective stack frame slot,
> +   so the decision is made per-thread during function return by
> +   maybe inserting a KLP_return_helper frame or not.
> +*/
> +KLP_return_helper:
> +	ld	r2,24(r1)	/* restore TOC (saved by ftrace_caller) */
> +	addi r1, r1, 32		/* destroy mini stack frame */
> +	ld	r0,LRSAVE(r1)	/* get the real return address */
> +	mtlr	r0
> +	blr
> +#endif
> +
>  #else
>  _GLOBAL_TOC(_mcount)
>  	/* Taken from output of objdump from lib64/glibc */
> diff --git a/arch/powerpc/kernel/livepatch.c
> b/arch/powerpc/kernel/livepatch.c
> new file mode 100644
> index 0000000..564eafa
> --- /dev/null
> +++ b/arch/powerpc/kernel/livepatch.c
> @@ -0,0 +1,38 @@
> +/*
> + * livepatch.c - powerpc-specific Kernel Live Patching Core
> + *
> + * Copyright (C) 2015 SUSE
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/module.h>
> +#include <asm/livepatch.h>
> +
> +/**
> + * klp_write_module_reloc() - write a relocation in a module
> + * @mod:       module in which the section to be modified is found
> + * @type:      ELF relocation type (see asm/elf.h)
> + * @loc:       address that the relocation should be written to
> + * @value:     relocation value (sym address + addend)
> + *
> + * This function writes a relocation to the specified location for
> + * a particular module.
> + */
> +int klp_write_module_reloc(struct module *mod, unsigned long type,
> +			    unsigned long loc, unsigned long value)
> +{
> +	/* This requires infrastructure changes; we need the loadinfos. */
> +	pr_err("lpc_write_module_reloc not yet supported\n");
> +	return -ENOSYS;
> +}
> --
> 1.8.5.6
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

[toc] | [prev] | [next] | [standalone]

#1317062 — [PATCH v6 2/9] ppc64le FTRACE_WITH_REGS implementation

Implement FTRACE_WITH_REGS for powerpc64, on ELF ABI v2.
Initial work started by Vojtech Pavlik, used with permission.

  * arch/powerpc/kernel/entry_64.S:
    - Implement an effective ftrace_caller that works from
      within the kernel binary as well as from modules.
  * arch/powerpc/kernel/ftrace.c:
    - be prepared to deal with ppc64 ELF ABI v2, especially
      calls to _mcount that result from gcc -mprofile-kernel
    - a little more error verbosity
  * arch/powerpc/kernel/module_64.c:
    - do not save the TOC pointer on the trampoline when the
      destination is ftrace_caller. This trampoline jump happens from
      a function prologue before a new stack frame is set up, so bad
      things may happen otherwise...
    - relax is_module_trampoline() to recognise the modified
      trampoline.

Signed-off-by: Torsten Duwe <duwe@suse.de>
---
 arch/powerpc/include/asm/ftrace.h |  5 +++
 arch/powerpc/kernel/entry_64.S    | 78 +++++++++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/ftrace.c      | 60 +++++++++++++++++++++++++++---
 arch/powerpc/kernel/module_64.c   | 25 ++++++++++++-
 4 files changed, 161 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index ef89b14..50ca758 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -46,6 +46,8 @@
 extern void _mcount(void);
 
 #ifdef CONFIG_DYNAMIC_FTRACE
+# define FTRACE_ADDR ((unsigned long)ftrace_caller)
+# define FTRACE_REGS_ADDR FTRACE_ADDR
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
 {
        /* reloction of mcount call site is the same as the address */
@@ -58,6 +60,9 @@ struct dyn_arch_ftrace {
 #endif /*  CONFIG_DYNAMIC_FTRACE */
 #endif /* __ASSEMBLY__ */
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#define ARCH_SUPPORTS_FTRACE_OPS 1
+#endif
 #endif
 
 #if defined(CONFIG_FTRACE_SYSCALLS) && defined(CONFIG_PPC64) && !defined(__ASSEMBLY__)
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index e7cd043..9e98aa1 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -1213,6 +1213,7 @@ _GLOBAL(_mcount)
 	mtlr	r0
 	bctr
 
+#ifndef CC_USING_MPROFILE_KERNEL
 _GLOBAL_TOC(ftrace_caller)
 	/* Taken from output of objdump from lib64/glibc */
 	mflr	r3
@@ -1234,6 +1235,83 @@ _GLOBAL(ftrace_graph_stub)
 	ld	r0, 128(r1)
 	mtlr	r0
 	addi	r1, r1, 112
+#else
+_GLOBAL(ftrace_caller)
+	std	r0,LRSAVE(r1)
+#if defined(_CALL_ELF) && _CALL_ELF == 2
+	mflr	r0
+	bl	2f
+2:	mflr	r12
+	mtlr	r0
+	mr      r0,r2   /* save callee's TOC */
+	addis	r2,r12,(.TOC.-ftrace_caller-12)@ha
+	addi    r2,r2,(.TOC.-ftrace_caller-12)@l
+#else
+	mr	r0,r2
+#endif
+	ld	r12,LRSAVE(r1)	/* get caller's address */
+
+	stdu	r1,-SWITCH_FRAME_SIZE(r1)
+
+	std     r12, _LINK(r1)
+	SAVE_8GPRS(0,r1)
+	std	r0, 24(r1)	/* save TOC */
+	SAVE_8GPRS(8,r1)
+	SAVE_8GPRS(16,r1)
+	SAVE_8GPRS(24,r1)
+
+	addis	r3,r2,function_trace_op@toc@ha
+	addi	r3,r3,function_trace_op@toc@l
+	ld	r5,0(r3)
+
+	mflr    r3
+	std     r3, _NIP(r1)
+	std	r3, 16(r1)
+	subi    r3, r3, MCOUNT_INSN_SIZE
+	mfmsr   r4
+	std     r4, _MSR(r1)
+	mfctr   r4
+	std     r4, _CTR(r1)
+	mfxer   r4
+	std     r4, _XER(r1)
+	mr	r4, r12
+	addi    r6, r1 ,STACK_FRAME_OVERHEAD
+
+.globl ftrace_call
+ftrace_call:
+	bl	ftrace_stub
+	nop
+
+	ld	r3, _NIP(r1)
+	mtlr	r3
+
+	REST_8GPRS(0,r1)
+	REST_8GPRS(8,r1)
+	REST_8GPRS(16,r1)
+	REST_8GPRS(24,r1)
+
+	addi r1, r1, SWITCH_FRAME_SIZE
+
+	ld	r12, LRSAVE(r1)  /* get caller's address */
+	mtlr	r12
+	mr	r2,r0		/* restore callee's TOC */
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+	stdu	r1, -112(r1)
+.globl ftrace_graph_call
+ftrace_graph_call:
+	b	ftrace_graph_stub
+_GLOBAL(ftrace_graph_stub)
+	addi	r1, r1, 112
+#endif
+
+	mflr	r0		/* move this LR to CTR */
+	mtctr	r0
+
+	ld	r0,LRSAVE(r1)	/* restore callee's lr at _mcount site */
+	mtlr	r0
+	bctr			/* jump after _mcount site */
+#endif /* CC_USING_MPROFILE_KERNEL */
 _GLOBAL(ftrace_stub)
 	blr
 #else
diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index 080c525..310137f 100644
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -61,8 +61,11 @@ ftrace_modify_code(unsigned long ip, unsigned int old, unsigned int new)
 		return -EFAULT;
 
 	/* Make sure it is what we expect it to be */
-	if (replaced != old)
+	if (replaced != old) {
+		pr_err("%p: replaced (%#x) != old (%#x)",
+		(void *)ip, replaced, old);
 		return -EINVAL;
+	}
 
 	/* replace the text with the new text */
 	if (patch_instruction((unsigned int *)ip, new))
@@ -106,14 +109,16 @@ static int
 __ftrace_make_nop(struct module *mod,
 		  struct dyn_ftrace *rec, unsigned long addr)
 {
-	unsigned int op;
+	unsigned int op, op0, op1, pop;
 	unsigned long entry, ptr;
 	unsigned long ip = rec->ip;
 	void *tramp;
 
 	/* read where this goes */
-	if (probe_kernel_read(&op, (void *)ip, sizeof(int)))
+	if (probe_kernel_read(&op, (void *)ip, sizeof(int))) {
+		pr_err("Fetching opcode failed.\n");
 		return -EFAULT;
+	}
 
 	/* Make sure that that this is still a 24bit jump */
 	if (!is_bl_op(op)) {
@@ -158,10 +163,46 @@ __ftrace_make_nop(struct module *mod,
 	 *
 	 * Use a b +8 to jump over the load.
 	 */
-	op = 0x48000008;	/* b +8 */
 
-	if (patch_instruction((unsigned int *)ip, op))
+	pop = 0x48000008;	/* b +8 */
+
+	/*
+	 * Check what is in the next instruction. We can see ld r2,40(r1), but
+	 * on first pass after boot we will see mflr r0.
+	 */
+	if (probe_kernel_read(&op, (void *)(ip+4), MCOUNT_INSN_SIZE)) {
+		pr_err("Fetching op failed.\n");
+		return -EFAULT;
+	}
+
+	if (op != 0xe8410028) { /* ld r2,STACK_OFFSET(r1) */
+
+		if (probe_kernel_read(&op0, (void *)(ip-8), MCOUNT_INSN_SIZE)) {
+			pr_err("Fetching op0 failed.\n");
+			return -EFAULT;
+		}
+
+		if (probe_kernel_read(&op1, (void *)(ip-4), MCOUNT_INSN_SIZE)) {
+			pr_err("Fetching op1 failed.\n");
+			return -EFAULT;
+		}
+
+		/* mflr r0 ; std r0,LRSAVE(r1) */
+		if (op0 != 0x7c0802a6 && op1 != 0xf8010010) {
+			pr_err("Unexpected instructions around bl\n"
+				"when enabling dynamic ftrace!\t"
+				"(%08x,%08x,bl,%08x)\n", op0, op1, op);
+			return -EINVAL;
+		}
+
+		/* When using -mkernel_profile there is no load to jump over */
+		pop = PPC_INST_NOP;
+	}
+
+	if (patch_instruction((unsigned int *)ip, pop)) {
+		pr_err("Patching NOP failed.\n");
 		return -EPERM;
+	}
 
 	return 0;
 }
@@ -287,6 +328,13 @@ int ftrace_make_nop(struct module *mod,
 
 #ifdef CONFIG_MODULES
 #ifdef CONFIG_PPC64
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
+			unsigned long addr)
+{
+	return ftrace_make_call(rec, addr);
+}
+#endif
 static int
 __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
@@ -338,7 +386,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 
 	return 0;
 }
-#else
+#else  /* !CONFIG_PPC64: */
 static int
 __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 30f6be1..a8facb6 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -138,12 +138,25 @@ static u32 ppc64_stub_insns[] = {
 	0x4e800420			/* bctr */
 };
 
+#ifdef CC_USING_MPROFILE_KERNEL
+/* In case of _mcount calls or dynamic ftracing, Do not save the
+ * current callee's TOC (in R2) again into the original caller's stack
+ * frame during this trampoline hop. The stack frame already holds
+ * that of the original caller.  _mcount and ftrace_caller will take
+ * care of this TOC value themselves.
+ */
+#define SQUASH_TOC_SAVE_INSN(trampoline_addr) \
+	(((struct ppc64_stub_entry *)(trampoline_addr))->jump[2] = PPC_INST_NOP)
+#else
+#define SQUASH_TOC_SAVE_INSN(trampoline_addr)
+#endif
+
 #ifdef CONFIG_DYNAMIC_FTRACE
 
 static u32 ppc64_stub_mask[] = {
 	0xffff0000,
 	0xffff0000,
-	0xffffffff,
+	0x00000000,
 	0xffffffff,
 #if !defined(_CALL_ELF) || _CALL_ELF != 2
 	0xffffffff,
@@ -170,6 +183,9 @@ bool is_module_trampoline(u32 *p)
 		if ((insna & mask) != (insnb & mask))
 			return false;
 	}
+	if (insns[2] != ppc64_stub_insns[2] &&
+	    insns[2] != PPC_INST_NOP)
+		return false;
 
 	return true;
 }
@@ -619,6 +635,9 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
 					return -ENOENT;
 				if (!restore_r2((u32 *)location + 1, me))
 					return -ENOEXEC;
+				/* Squash the TOC saver for profiler calls */
+				if (!strcmp("_mcount", strtab+sym->st_name))
+					SQUASH_TOC_SAVE_INSN(value);
 			} else
 				value += local_entry_offset(sym);
 
@@ -679,6 +698,10 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
 	me->arch.tramp = stub_for_addr(sechdrs,
 				       (unsigned long)ftrace_caller,
 				       me);
+	/* ftrace_caller will take care of the TOC;
+	 * do not clobber original caller's value.
+	 */
+	SQUASH_TOC_SAVE_INSN(me->arch.tramp);
 #endif
 
 	return 0;
-- 
1.8.5.6

[toc] | [prev] | [next] | [standalone]

#1317073 — [PATCH v6 7/9] ppc64 ftrace: disable profiling for some files

This patch complements the "notrace" attribute for selected functions.
It adds -mprofile-kernel to the cc flags to be stripped from the command
line for code-patching.o and feature-fixups.o, in addition to "-pg"

Signed-off-by: Torsten Duwe <duwe@suse.de>
---
 arch/powerpc/lib/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index a47e142..98e22b2 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -6,8 +6,8 @@ subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
 
 ccflags-$(CONFIG_PPC64)	:= $(NO_MINIMAL_TOC)
 
-CFLAGS_REMOVE_code-patching.o = -pg
-CFLAGS_REMOVE_feature-fixups.o = -pg
+CFLAGS_REMOVE_code-patching.o = -pg -mprofile-kernel
+CFLAGS_REMOVE_feature-fixups.o = -pg -mprofile-kernel
 
 obj-y += string.o alloc.o crtsavres.o ppc_ksyms.o code-patching.o \
 	 feature-fixups.o
-- 
1.8.5.6

[toc] | [prev] | [next] | [standalone]

#1318854 — Re: [PATCH v6 0/9] ftrace with regs + live patching for ppc64 LE (ABI v2)

On Mon, 25 Jan 2016 16:38:48 +0100
Torsten Duwe <duwe@lst.de> wrote:

> Changes since v5:
>   * extra "std r0,LRSAVE(r1)" for gcc-6
>     This makes the code compiler-agnostic.
>   * Follow Petr Mladek's suggestion to avoid
>     redefinition of HAVE_LIVEPATCH

I looked at the patches - well mostly patches 1 and 2, some quick questions

1. I know -mprofile-kernel is a big optimization win, do we need it or can
we incrementally add it?
2. Some of the hardcoded checks for opcode are hard to review, I know they've
been there in similar forms for a while. May be as an iterative step we should
give the numbers some meaning and use proper helpers for it.

I am going to give the patches a spin

Balbir Singh.

[toc] | [prev] | [next] | [standalone]

#1318911 — Re: [PATCH v6 0/9] ftrace with regs + live patching for ppc64 LE (ABI v2)

On Wed, Jan 27, 2016 at 09:51:12PM +1100, Balbir Singh wrote:
> On Mon, 25 Jan 2016 16:38:48 +0100
> Torsten Duwe <duwe@lst.de> wrote:
> 
> > Changes since v5:
> >   * extra "std r0,LRSAVE(r1)" for gcc-6
> >     This makes the code compiler-agnostic.
> >   * Follow Petr Mladek's suggestion to avoid
> >     redefinition of HAVE_LIVEPATCH
> 
> I looked at the patches - well mostly patches 1 and 2, some quick questions
> 
> 1. I know -mprofile-kernel is a big optimization win, do we need it or can
> we incrementally add it?

There's a reason why these are first ;-)
The following ones assume -mprofile-kernel is used.
The disadvantage is all relevant registers need to be saved before calling
further C code in between functions. On the Pro side, no stack frame has been
created at that point. These are assumptions made all over the ftrace-with-regs
and live patching code here.

> 2. Some of the hardcoded checks for opcode are hard to review, I know they've
> been there in similar forms for a while. May be as an iterative step we should
> give the numbers some meaning and use proper helpers for it.

Yes, Michael has already criticised that. No further literal hex constants, I promise.

> I am going to give the patches a spin

Thanks! Make sure you use a compiler that can disable -mprofile-kernel with "notrace".

	Torsten

[toc] | [prev] | [next] | [standalone]

#1320239 — Re: [PATCH v6 0/9] ftrace with regs + live patching for ppc64 LE (ABI v2)

On Wed, 27 Jan 2016 13:19:04 +0100
Torsten Duwe <duwe@lst.de> wrote:

> Thanks! Make sure you use a compiler that can disable -mprofile-kernel with "notrace".

gcc-6? I have gcc-5.2.1

Balbir Singh.

[toc] | [prev] | [next] | [standalone]

#1320270 — Re: [PATCH v6 0/9] ftrace with regs + live patching for ppc64 LE (ABI v2)

On Thu, 2016-01-28 at 13:41 +1100, Balbir Singh wrote:
> On Wed, 27 Jan 2016 13:19:04 +0100
> Torsten Duwe <duwe@lst.de> wrote:
>
> > Thanks! Make sure you use a compiler that can disable -mprofile-kernel with "notrace".
> 
> gcc-6? I have gcc-5.2.1

That should work.

But that's a good point.

We need to have some Makefile logic to only enable -mprofile-kernel when it's
known to work, ie. for versions where the notrace fix is in.

Looking at GCC history it looks like the fix is in 4.9.0 and anything later.

But a version check doesn't work with patched distro/vendor toolchains. So we
probably need some sort of runtime check.

cheers

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web