[RFC PATCH 00/14] powerpc64: Add support for ftrace direct calls

[RFC PATCH 00/14] powerpc64: Add support for ftrace direct calls

[Naveen N. Rao]

This series adds support for DYNAMIC_FTRACE_WITH_DIRECT_CALLS for 
powerpc64le.

This is mostly working fine for me, except for a soft lockup I see with 
the ftrace direct selftest. It happens when irqsoff tracer is being 
tested with the ftrace direct modules. This appears to be an existing 
upstream issue since I am able to reproduce the lockup without these 
patches. I will be looking into that to see if I can figure out the 
cause of those lockups.

In the meantime, I would appreciate a review of these patches.


- Naveen


Naveen N. Rao (14):
  ftrace: Fix updating FTRACE_FL_TRAMP
  ftrace: Fix DYNAMIC_FTRACE_WITH_DIRECT_CALLS dependency
  ftrace: Fix cleanup in error path of register_ftrace_direct()
  ftrace: Remove ftrace_find_direct_func()
  ftrace: Add architectural helpers for [un]register_ftrace_direct()
  powerpc: Add support for CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
  powerpc/ftrace: Remove dead code
  powerpc/ftrace: Use FTRACE_REGS_ADDR to identify the correct ftrace
    trampoline
  powerpc/ftrace: Use a hash table for tracking ftrace stubs
  powerpc/ftrace: Drop assumptions about ftrace trampoline target
  powerpc/ftrace: Use GPR save/restore macros in ftrace_graph_caller()
  powerpc/ftrace: Drop saving LR to stack save area for -mprofile-kernel
  powerpc/ftrace: Add support for register_ftrace_direct() for
    MPROFILE_KERNEL
  samples/ftrace: Add powerpc support for ftrace direct samples

 arch/powerpc/Kconfig                          |   2 +
 arch/powerpc/include/asm/ftrace.h             |  14 +
 arch/powerpc/include/asm/ptrace.h             |  31 ++
 arch/powerpc/kernel/trace/ftrace.c            | 314 +++++++++++++-----
 .../powerpc/kernel/trace/ftrace_64_mprofile.S |  70 ++--
 include/linux/ftrace.h                        |   7 +-
 kernel/trace/Kconfig                          |   2 +-
 kernel/trace/ftrace.c                         | 130 +++-----
 samples/Kconfig                               |   2 +-
 samples/ftrace/ftrace-direct-modify.c         |  58 ++++
 samples/ftrace/ftrace-direct-too.c            |  48 ++-
 samples/ftrace/ftrace-direct.c                |  45 ++-
 12 files changed, 519 insertions(+), 204 deletions(-)


base-commit: 4c202167192a77481310a3cacae9f12618b92216
-- 
2.25.4

[RFC PATCH 01/14] ftrace: Fix updating FTRACE_FL_TRAMP

[Naveen N. Rao]

On powerpc, kprobe-direct.tc triggered FTRACE_WARN_ON() in
ftrace_get_addr_new() followed by the below message:
  Bad trampoline accounting at: 000000004222522f (wake_up_process+0xc/0x20) (f0000001)

The set of steps leading to this involved:
- modprobe ftrace-direct-too
- enable_probe
- modprobe ftrace-direct
- rmmod ftrace-direct <-- trigger

The problem turned out to be that we were not updating flags in the
ftrace record properly. From the above message about the trampoline
accounting being bad, it can be seen that the ftrace record still has
FTRACE_FL_TRAMP set though ftrace-direct module is going away. This
happens because we are checking if any ftrace_ops has the
FTRACE_FL_TRAMP flag set _before_ updating the filter hash.

The fix for this is to look for any _other_ ftrace_ops that also needs
FTRACE_FL_TRAMP.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 kernel/trace/ftrace.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 8185f7240095f4..9c1bba8cc51b03 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1629,6 +1629,8 @@ static bool test_rec_ops_needs_regs(struct dyn_ftrace *rec)
 static struct ftrace_ops *
 ftrace_find_tramp_ops_any(struct dyn_ftrace *rec);
 static struct ftrace_ops *
+ftrace_find_tramp_ops_any_other(struct dyn_ftrace *rec, struct ftrace_ops *op_exclude);
+static struct ftrace_ops *
 ftrace_find_tramp_ops_next(struct dyn_ftrace *rec, struct ftrace_ops *ops);
 
 static bool __ftrace_hash_rec_update(struct ftrace_ops *ops,
@@ -1778,7 +1780,7 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops,
 			 * to it.
 			 */
 			if (ftrace_rec_count(rec) == 1 &&
-			    ftrace_find_tramp_ops_any(rec))
+			    ftrace_find_tramp_ops_any_other(rec, ops))
 				rec->flags |= FTRACE_FL_TRAMP;
 			else
 				rec->flags &= ~FTRACE_FL_TRAMP;
@@ -2244,6 +2246,24 @@ ftrace_find_tramp_ops_any(struct dyn_ftrace *rec)
 	return NULL;
 }
 
+static struct ftrace_ops *
+ftrace_find_tramp_ops_any_other(struct dyn_ftrace *rec, struct ftrace_ops *op_exclude)
+{
+	struct ftrace_ops *op;
+	unsigned long ip = rec->ip;
+
+	do_for_each_ftrace_op(op, ftrace_ops_list) {
+
+		if (op == op_exclude || !op->trampoline)
+			continue;
+
+		if (hash_contains_ip(ip, op->func_hash))
+			return op;
+	} while_for_each_ftrace_op(op);
+
+	return NULL;
+}
+
 static struct ftrace_ops *
 ftrace_find_tramp_ops_next(struct dyn_ftrace *rec,
 			   struct ftrace_ops *op)
-- 
2.25.4

[RFC PATCH 02/14] ftrace: Fix DYNAMIC_FTRACE_WITH_DIRECT_CALLS dependency

[Naveen N. Rao]

DYNAMIC_FTRACE_WITH_DIRECT_CALLS should depend on
DYNAMIC_FTRACE_WITH_REGS since we need ftrace_regs_caller().

Fixes: 763e34e74bb7d5c ("ftrace: Add register_ftrace_direct()")
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 kernel/trace/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index a4020c0b4508c9..e1bf5228fb692a 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -202,7 +202,7 @@ config DYNAMIC_FTRACE_WITH_REGS
 
 config DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 	def_bool y
-	depends on DYNAMIC_FTRACE
+	depends on DYNAMIC_FTRACE_WITH_REGS
 	depends on HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 
 config FUNCTION_PROFILER
-- 
2.25.4

[RFC PATCH 03/14] ftrace: Fix cleanup in error path of register_ftrace_direct()

[Naveen N. Rao]

We need to remove hash entry if register_ftrace_function() fails.
Consolidate the cleanup to be done after register_ftrace_function() at
the end.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 kernel/trace/ftrace.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 9c1bba8cc51b03..3844a4a1346a9c 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5136,8 +5136,6 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
 	__add_hash_entry(direct_functions, entry);
 
 	ret = ftrace_set_filter_ip(&direct_ops, ip, 0, 0);
-	if (ret)
-		remove_hash_entry(direct_functions, entry);
 
 	if (!ret && !(direct_ops.flags & FTRACE_OPS_FL_ENABLED)) {
 		ret = register_ftrace_function(&direct_ops);
@@ -5146,6 +5144,7 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
 	}
 
 	if (ret) {
+		remove_hash_entry(direct_functions, entry);
 		kfree(entry);
 		if (!direct->count) {
 			list_del_rcu(&direct->next);
-- 
2.25.4

[RFC PATCH 04/14] ftrace: Remove ftrace_find_direct_func()

[Naveen N. Rao]

This is a revert of commit 013bf0da047481 ("ftrace: Add
ftrace_find_direct_func()")

ftrace_find_direct_func() was meant for use in the function graph tracer
by architecture specific code. However, commit ff205766dbbee0 ("ftrace:
Fix function_graph tracer interaction with BPF trampoline") disabled
function graph tracer for direct calls leaving this without any users.

In addition, modify_ftrace_direct() allowed redirecting the direct call
to a different trampoline that was never registered through
register_ftrace_direct(). This meant that ftrace_direct_funcs didn't
capture all trampolines.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 include/linux/ftrace.h |  5 ---
 kernel/trace/ftrace.c  | 84 ++----------------------------------------
 2 files changed, 4 insertions(+), 85 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 1bd3a0356ae478..46b4b7ee28c41f 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -285,7 +285,6 @@ extern int ftrace_direct_func_count;
 int register_ftrace_direct(unsigned long ip, unsigned long addr);
 int unregister_ftrace_direct(unsigned long ip, unsigned long addr);
 int modify_ftrace_direct(unsigned long ip, unsigned long old_addr, unsigned long new_addr);
-struct ftrace_direct_func *ftrace_find_direct_func(unsigned long addr);
 int ftrace_modify_direct_caller(struct ftrace_func_entry *entry,
 				struct dyn_ftrace *rec,
 				unsigned long old_addr,
@@ -306,10 +305,6 @@ static inline int modify_ftrace_direct(unsigned long ip,
 {
 	return -ENOTSUPP;
 }
-static inline struct ftrace_direct_func *ftrace_find_direct_func(unsigned long addr)
-{
-	return NULL;
-}
 static inline int ftrace_modify_direct_caller(struct ftrace_func_entry *entry,
 					      struct dyn_ftrace *rec,
 					      unsigned long old_addr,
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 3844a4a1346a9c..7476f2458b6d95 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5005,46 +5005,6 @@ ftrace_set_addr(struct ftrace_ops *ops, unsigned long ip, int remove,
 }
 
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
-
-struct ftrace_direct_func {
-	struct list_head	next;
-	unsigned long		addr;
-	int			count;
-};
-
-static LIST_HEAD(ftrace_direct_funcs);
-
-/**
- * ftrace_find_direct_func - test an address if it is a registered direct caller
- * @addr: The address of a registered direct caller
- *
- * This searches to see if a ftrace direct caller has been registered
- * at a specific address, and if so, it returns a descriptor for it.
- *
- * This can be used by architecture code to see if an address is
- * a direct caller (trampoline) attached to a fentry/mcount location.
- * This is useful for the function_graph tracer, as it may need to
- * do adjustments if it traced a location that also has a direct
- * trampoline attached to it.
- */
-struct ftrace_direct_func *ftrace_find_direct_func(unsigned long addr)
-{
-	struct ftrace_direct_func *entry;
-	bool found = false;
-
-	/* May be called by fgraph trampoline (protected by rcu tasks) */
-	list_for_each_entry_rcu(entry, &ftrace_direct_funcs, next) {
-		if (entry->addr == addr) {
-			found = true;
-			break;
-		}
-	}
-	if (found)
-		return entry;
-
-	return NULL;
-}
-
 /**
  * register_ftrace_direct - Call a custom trampoline directly
  * @ip: The address of the nop at the beginning of a function
@@ -5064,7 +5024,6 @@ struct ftrace_direct_func *ftrace_find_direct_func(unsigned long addr)
  */
 int register_ftrace_direct(unsigned long ip, unsigned long addr)
 {
-	struct ftrace_direct_func *direct;
 	struct ftrace_func_entry *entry;
 	struct ftrace_hash *free_hash = NULL;
 	struct dyn_ftrace *rec;
@@ -5118,19 +5077,7 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
 	if (!entry)
 		goto out_unlock;
 
-	direct = ftrace_find_direct_func(addr);
-	if (!direct) {
-		direct = kmalloc(sizeof(*direct), GFP_KERNEL);
-		if (!direct) {
-			kfree(entry);
-			goto out_unlock;
-		}
-		direct->addr = addr;
-		direct->count = 0;
-		list_add_rcu(&direct->next, &ftrace_direct_funcs);
-		ftrace_direct_func_count++;
-	}
-
+	ftrace_direct_func_count++;
 	entry->ip = ip;
 	entry->direct = addr;
 	__add_hash_entry(direct_functions, entry);
@@ -5145,18 +5092,8 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
 
 	if (ret) {
 		remove_hash_entry(direct_functions, entry);
+		ftrace_direct_func_count--;
 		kfree(entry);
-		if (!direct->count) {
-			list_del_rcu(&direct->next);
-			synchronize_rcu_tasks();
-			kfree(direct);
-			if (free_hash)
-				free_ftrace_hash(free_hash);
-			free_hash = NULL;
-			ftrace_direct_func_count--;
-		}
-	} else {
-		direct->count++;
 	}
  out_unlock:
 	mutex_unlock(&direct_mutex);
@@ -5199,7 +5136,6 @@ static struct ftrace_func_entry *find_direct_entry(unsigned long *ip,
 
 int unregister_ftrace_direct(unsigned long ip, unsigned long addr)
 {
-	struct ftrace_direct_func *direct;
 	struct ftrace_func_entry *entry;
 	int ret = -ENODEV;
 
@@ -5217,20 +5153,8 @@ int unregister_ftrace_direct(unsigned long ip, unsigned long addr)
 	WARN_ON(ret);
 
 	remove_hash_entry(direct_functions, entry);
-
-	direct = ftrace_find_direct_func(addr);
-	if (!WARN_ON(!direct)) {
-		/* This is the good path (see the ! before WARN) */
-		direct->count--;
-		WARN_ON(direct->count < 0);
-		if (!direct->count) {
-			list_del_rcu(&direct->next);
-			synchronize_rcu_tasks();
-			kfree(direct);
-			kfree(entry);
-			ftrace_direct_func_count--;
-		}
-	}
+	ftrace_direct_func_count--;
+	kfree(entry);
  out_unlock:
 	mutex_unlock(&direct_mutex);
 
-- 
2.25.4

[RFC PATCH 05/14] ftrace: Add architectural helpers for [un]register_ftrace_direct()

[Naveen N. Rao]

Architectures may want to do some validation (such as to ensure that the
trampoline code is reachable from the provided ftrace location) before
accepting ftrace direct registration. Add helpers for the same.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 include/linux/ftrace.h |  2 ++
 kernel/trace/ftrace.c  | 27 +++++++++++++++++++++------
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 46b4b7ee28c41f..3fdcb4c513bc2d 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -290,6 +290,8 @@ int ftrace_modify_direct_caller(struct ftrace_func_entry *entry,
 				unsigned long old_addr,
 				unsigned long new_addr);
 unsigned long ftrace_find_rec_direct(unsigned long ip);
+int arch_register_ftrace_direct(unsigned long ip, unsigned long addr);
+void arch_unregister_ftrace_direct(unsigned long ip, unsigned long addr);
 #else
 # define ftrace_direct_func_count 0
 static inline int register_ftrace_direct(unsigned long ip, unsigned long addr)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 7476f2458b6d95..0e259b90527722 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5005,6 +5005,13 @@ ftrace_set_addr(struct ftrace_ops *ops, unsigned long ip, int remove,
 }
 
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+int __weak arch_register_ftrace_direct(unsigned long ip, unsigned long addr)
+{
+	return 0;
+}
+
+void __weak arch_unregister_ftrace_direct(unsigned long ip, unsigned long addr) { }
+
 /**
  * register_ftrace_direct - Call a custom trampoline directly
  * @ip: The address of the nop at the beginning of a function
@@ -5028,6 +5035,7 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
 	struct ftrace_hash *free_hash = NULL;
 	struct dyn_ftrace *rec;
 	int ret = -EBUSY;
+	int arch_ret;
 
 	mutex_lock(&direct_mutex);
 
@@ -5082,18 +5090,24 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
 	entry->direct = addr;
 	__add_hash_entry(direct_functions, entry);
 
-	ret = ftrace_set_filter_ip(&direct_ops, ip, 0, 0);
+	arch_ret = arch_register_ftrace_direct(ip, addr);
 
-	if (!ret && !(direct_ops.flags & FTRACE_OPS_FL_ENABLED)) {
-		ret = register_ftrace_function(&direct_ops);
-		if (ret)
-			ftrace_set_filter_ip(&direct_ops, ip, 1, 0);
+	if (!arch_ret) {
+		ret = ftrace_set_filter_ip(&direct_ops, ip, 0, 0);
+
+		if (!ret && !(direct_ops.flags & FTRACE_OPS_FL_ENABLED)) {
+			ret = register_ftrace_function(&direct_ops);
+			if (ret)
+				ftrace_set_filter_ip(&direct_ops, ip, 1, 0);
+		}
 	}
 
-	if (ret) {
+	if (arch_ret || ret) {
 		remove_hash_entry(direct_functions, entry);
 		ftrace_direct_func_count--;
 		kfree(entry);
+		if (!arch_ret)
+			arch_unregister_ftrace_direct(ip, addr);
 	}
  out_unlock:
 	mutex_unlock(&direct_mutex);
@@ -5155,6 +5169,7 @@ int unregister_ftrace_direct(unsigned long ip, unsigned long addr)
 	remove_hash_entry(direct_functions, entry);
 	ftrace_direct_func_count--;
 	kfree(entry);
+	arch_unregister_ftrace_direct(ip, addr);
  out_unlock:
 	mutex_unlock(&direct_mutex);
 
-- 
2.25.4

[RFC PATCH 06/14] powerpc: Add support for CONFIG_HAVE_FUNCTION_ARG_ACCESS_API

[Naveen N. Rao]

Add register_get_kernel_argument() for a rudimentary way to access
kernel function arguments.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/Kconfig              |  1 +
 arch/powerpc/include/asm/ptrace.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index e9f13fe084929b..cfc6dd787f532c 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -202,6 +202,7 @@ config PPC
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS	if !(CPU_LITTLE_ENDIAN && POWER7_CPU)
 	select HAVE_FAST_GUP
 	select HAVE_FTRACE_MCOUNT_RECORD
+	select HAVE_FUNCTION_ARG_ACCESS_API
 	select HAVE_FUNCTION_ERROR_INJECTION
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_TRACER
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index e2c778c176a3a6..956828c07abd70 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -62,6 +62,8 @@ struct pt_regs
 };
 #endif
 
+#define NR_REG_ARGUMENTS 8
+
 #ifdef __powerpc64__
 
 /*
@@ -85,8 +87,10 @@ struct pt_regs
 
 #ifdef PPC64_ELF_ABI_v2
 #define STACK_FRAME_MIN_SIZE	32
+#define STACK_FRAME_PARM_SAVE	32
 #else
 #define STACK_FRAME_MIN_SIZE	STACK_FRAME_OVERHEAD
+#define STACK_FRAME_PARM_SAVE	48
 #endif
 
 /* Size of dummy stack frame allocated when calling signal handler. */
@@ -103,6 +107,7 @@ struct pt_regs
 #define STACK_INT_FRAME_SIZE	(sizeof(struct pt_regs) + STACK_FRAME_OVERHEAD)
 #define STACK_FRAME_MARKER	2
 #define STACK_FRAME_MIN_SIZE	STACK_FRAME_OVERHEAD
+#define STACK_FRAME_PARM_SAVE	8
 
 /* Size of stack frame allocated when calling signal handler. */
 #define __SIGNAL_FRAMESIZE	64
@@ -309,6 +314,32 @@ static inline unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs,
 		return 0;
 }
 
+/**
+ * regs_get_kernel_argument() - get Nth function argument in kernel
+ * @regs:	pt_regs of that context
+ * @n:		function argument number (start from 0)
+ *
+ * regs_get_argument() returns @n th argument of the function call.
+ * Note that this chooses most probable assignment, and is incorrect
+ * in scenarios where double or fp/vector parameters are involved.
+ * This also doesn't take into account stack alignment requirements.
+ *
+ * This is expected to be called from kprobes or ftrace with regs
+ * at function entry, so the current function has not setup its stack.
+ */
+static inline unsigned long regs_get_kernel_argument(struct pt_regs *regs,
+						     unsigned int n)
+{
+	if (n >= NR_REG_ARGUMENTS) {
+#ifndef __powerpc64__
+		n -= NR_REG_ARGUMENTS;
+#endif
+		n += STACK_FRAME_PARM_SAVE / sizeof(unsigned long);
+		return regs_get_kernel_stack_nth(regs, n);
+	} else {
+		return regs_get_register(regs, offsetof(struct pt_regs, gpr[n + 3]));
+	}
+}
 #endif /* __ASSEMBLY__ */
 
 #ifndef __powerpc64__
-- 
2.25.4

[RFC PATCH 07/14] powerpc/ftrace: Remove dead code

[Naveen N. Rao]

ftrace_plt_tramps[] was intended to speed up skipping plt branches, but
the code wasn't completed. It is also not significantly better than
reading and decoding the instruction. Remove the same.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 42761ebec9f755..4fe5f373172fd2 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -332,7 +332,6 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 	struct ppc_inst op;
 	unsigned long ptr;
 	struct ppc_inst instr;
-	static unsigned long ftrace_plt_tramps[NUM_FTRACE_TRAMPS];
 
 	/* Is this a known long jump tramp? */
 	for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
@@ -341,13 +340,6 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 		else if (ftrace_tramps[i] == tramp)
 			return 0;
 
-	/* Is this a known plt tramp? */
-	for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
-		if (!ftrace_plt_tramps[i])
-			break;
-		else if (ftrace_plt_tramps[i] == tramp)
-			return -1;
-
 	/* New trampoline -- read where this goes */
 	if (probe_kernel_read_inst(&op, (void *)tramp)) {
 		pr_debug("Fetching opcode failed.\n");
-- 
2.25.4

[RFC PATCH 08/14] powerpc/ftrace: Use FTRACE_REGS_ADDR to identify the correct ftrace trampoline

[Naveen N. Rao]

Use FTRACE_REGS_ADDR instead of keying off
CONFIG_DYNAMIC_FTRACE_WITH_REGS to identify the proper ftrace trampoline
address to use.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 4fe5f373172fd2..14b39f7797d455 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -361,11 +361,7 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 	}
 
 	/* Let's re-write the tramp to go to ftrace_[regs_]caller */
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-	ptr = ppc_global_function_entry((void *)ftrace_regs_caller);
-#else
-	ptr = ppc_global_function_entry((void *)ftrace_caller);
-#endif
+	ptr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 	if (create_branch(&instr, (void *)tramp, ptr, 0)) {
 		pr_debug("%ps is not reachable from existing mcount tramp\n",
 				(void *)ptr);
@@ -885,11 +881,7 @@ int __init ftrace_dyn_arch_init(void)
 		0x7d8903a6,		/* mtctr   r12			*/
 		0x4e800420,		/* bctr				*/
 	};
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-	unsigned long addr = ppc_global_function_entry((void *)ftrace_regs_caller);
-#else
-	unsigned long addr = ppc_global_function_entry((void *)ftrace_caller);
-#endif
+	unsigned long addr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 	long reladdr = addr - kernel_toc_addr();
 
 	if (reladdr > 0x7FFFFFFF || reladdr < -(0x80000000L)) {
-- 
2.25.4

[RFC PATCH 09/14] powerpc/ftrace: Use a hash table for tracking ftrace stubs

[Naveen N. Rao]

In preparation for having to deal with large number of ftrace stubs in
support of ftrace direct calls, convert existing stubs to use a hash
table. The hash table is key'ed off the target address for the stubs
since there could be multiple stubs for the same target to cover the
full kernel text.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace.c | 75 +++++++++++++-----------------
 1 file changed, 33 insertions(+), 42 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 14b39f7797d455..7ddb6e4b527c39 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -13,6 +13,7 @@
 
 #define pr_fmt(fmt) "ftrace-powerpc: " fmt
 
+#include <linux/hashtable.h>
 #include <linux/spinlock.h>
 #include <linux/hardirq.h>
 #include <linux/uaccess.h>
@@ -32,14 +33,12 @@
 
 #ifdef CONFIG_DYNAMIC_FTRACE
 
-/*
- * We generally only have a single long_branch tramp and at most 2 or 3 plt
- * tramps generated. But, we don't use the plt tramps currently. We also allot
- * 2 tramps after .text and .init.text. So, we only end up with around 3 usable
- * tramps in total. Set aside 8 just to be sure.
- */
-#define	NUM_FTRACE_TRAMPS	8
-static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
+static DEFINE_HASHTABLE(ppc_ftrace_stubs, 8);
+struct ppc_ftrace_stub_data {
+	unsigned long addr;
+	unsigned long target;
+	struct hlist_node hentry;
+};
 
 static struct ppc_inst
 ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
@@ -288,36 +287,31 @@ __ftrace_make_nop(struct module *mod,
 #endif /* PPC64 */
 #endif /* CONFIG_MODULES */
 
-static unsigned long find_ftrace_tramp(unsigned long ip)
+static unsigned long find_ftrace_tramp(unsigned long ip, unsigned long target)
 {
-	int i;
+	struct ppc_ftrace_stub_data *stub;
 	struct ppc_inst instr;
 
-	/*
-	 * We have the compiler generated long_branch tramps at the end
-	 * and we prefer those
-	 */
-	for (i = NUM_FTRACE_TRAMPS - 1; i >= 0; i--)
-		if (!ftrace_tramps[i])
-			continue;
-		else if (create_branch(&instr, (void *)ip,
-				       ftrace_tramps[i], 0) == 0)
-			return ftrace_tramps[i];
+	hash_for_each_possible(ppc_ftrace_stubs, stub, hentry, target)
+		if (stub->target == target && !create_branch(&instr, (void *)ip, stub->addr, 0))
+			return stub->addr;
 
 	return 0;
 }
 
-static int add_ftrace_tramp(unsigned long tramp)
+static int add_ftrace_tramp(unsigned long tramp, unsigned long target)
 {
-	int i;
+	struct ppc_ftrace_stub_data *stub;
 
-	for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
-		if (!ftrace_tramps[i]) {
-			ftrace_tramps[i] = tramp;
-			return 0;
-		}
+	stub = kmalloc(sizeof(*stub), GFP_KERNEL);
+	if (!stub)
+		return -1;
 
-	return -1;
+	stub->addr = tramp;
+	stub->target = target;
+	hash_add(ppc_ftrace_stubs, &stub->hentry, target);
+
+	return 0;
 }
 
 /*
@@ -328,16 +322,14 @@ static int add_ftrace_tramp(unsigned long tramp)
  */
 static int setup_mcount_compiler_tramp(unsigned long tramp)
 {
-	int i;
 	struct ppc_inst op;
-	unsigned long ptr;
 	struct ppc_inst instr;
+	struct ppc_ftrace_stub_data *stub;
+	unsigned long ptr, ftrace_target = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 
 	/* Is this a known long jump tramp? */
-	for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
-		if (!ftrace_tramps[i])
-			break;
-		else if (ftrace_tramps[i] == tramp)
+	hash_for_each_possible(ppc_ftrace_stubs, stub, hentry, ftrace_target)
+		if (stub->target == ftrace_target && stub->addr == tramp)
 			return 0;
 
 	/* New trampoline -- read where this goes */
@@ -361,19 +353,18 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 	}
 
 	/* Let's re-write the tramp to go to ftrace_[regs_]caller */
-	ptr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
-	if (create_branch(&instr, (void *)tramp, ptr, 0)) {
+	if (create_branch(&instr, (void *)tramp, ftrace_target, 0)) {
 		pr_debug("%ps is not reachable from existing mcount tramp\n",
-				(void *)ptr);
+				(void *)ftrace_target);
 		return -1;
 	}
 
-	if (patch_branch((struct ppc_inst *)tramp, ptr, 0)) {
+	if (patch_branch((struct ppc_inst *)tramp, ftrace_target, 0)) {
 		pr_debug("REL24 out of range!\n");
 		return -1;
 	}
 
-	if (add_ftrace_tramp(tramp)) {
+	if (add_ftrace_tramp(tramp, ftrace_target)) {
 		pr_debug("No tramp locations left\n");
 		return -1;
 	}
@@ -405,7 +396,7 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned long addr)
 
 	if (setup_mcount_compiler_tramp(tramp)) {
 		/* Are other trampolines reachable? */
-		if (!find_ftrace_tramp(ip)) {
+		if (!find_ftrace_tramp(ip, FTRACE_REGS_ADDR)) {
 			pr_err("No ftrace trampolines reachable from %ps\n",
 					(void *)ip);
 			return -EINVAL;
@@ -646,7 +637,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
 		return -EINVAL;
 	}
 
-	tramp = find_ftrace_tramp((unsigned long)ip);
+	tramp = find_ftrace_tramp((unsigned long)ip, FTRACE_REGS_ADDR);
 	if (!tramp) {
 		pr_err("No ftrace trampolines reachable from %ps\n", ip);
 		return -EINVAL;
@@ -894,7 +885,7 @@ int __init ftrace_dyn_arch_init(void)
 		memcpy(tramp[i], stub_insns, sizeof(stub_insns));
 		tramp[i][1] |= PPC_HA(reladdr);
 		tramp[i][2] |= PPC_LO(reladdr);
-		add_ftrace_tramp((unsigned long)tramp[i]);
+		add_ftrace_tramp((unsigned long)tramp[i], addr);
 	}
 
 	return 0;
-- 
2.25.4

[RFC PATCH 10/14] powerpc/ftrace: Drop assumptions about ftrace trampoline target

[Naveen N. Rao]

We currently assume that ftrace locations are patched to go to either
ftrace_caller or ftrace_regs_caller. Drop this assumption in preparation
for supporting ftrace direct calls.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace.c | 107 +++++++++++++++++++++++------
 1 file changed, 86 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 7ddb6e4b527c39..fcb21a9756e456 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -322,14 +322,15 @@ static int add_ftrace_tramp(unsigned long tramp, unsigned long target)
  */
 static int setup_mcount_compiler_tramp(unsigned long tramp)
 {
+	int i;
 	struct ppc_inst op;
 	struct ppc_inst instr;
 	struct ppc_ftrace_stub_data *stub;
 	unsigned long ptr, ftrace_target = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 
-	/* Is this a known long jump tramp? */
-	hash_for_each_possible(ppc_ftrace_stubs, stub, hentry, ftrace_target)
-		if (stub->target == ftrace_target && stub->addr == tramp)
+	/* Is this a known tramp? */
+	hash_for_each(ppc_ftrace_stubs, i, stub, hentry)
+		if (stub->addr == tramp)
 			return 0;
 
 	/* New trampoline -- read where this goes */
@@ -608,23 +609,16 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
 {
 	struct ppc_inst op;
 	void *ip = (void *)rec->ip;
-	unsigned long tramp, entry, ptr;
+	unsigned long tramp, ptr;
 
-	/* Make sure we're being asked to patch branch to a known ftrace addr */
-	entry = ppc_global_function_entry((void *)ftrace_caller);
 	ptr = ppc_global_function_entry((void *)addr);
 
-	if (ptr != entry) {
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-		entry = ppc_global_function_entry((void *)ftrace_regs_caller);
-		if (ptr != entry) {
+	/* Make sure we branch to ftrace_regs_caller since we only setup stubs for that */
+	tramp = ppc_global_function_entry((void *)ftrace_caller);
+	if (ptr == tramp)
+		ptr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 #endif
-			pr_err("Unknown ftrace addr to patch: %ps\n", (void *)ptr);
-			return -EINVAL;
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-		}
-#endif
-	}
 
 	/* Make sure we have a nop */
 	if (probe_kernel_read_inst(&op, ip)) {
@@ -637,7 +631,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
 		return -EINVAL;
 	}
 
-	tramp = find_ftrace_tramp((unsigned long)ip, FTRACE_REGS_ADDR);
+	tramp = find_ftrace_tramp((unsigned long)ip, ptr);
 	if (!tramp) {
 		pr_err("No ftrace trampolines reachable from %ps\n", ip);
 		return -EINVAL;
@@ -783,6 +777,81 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 }
 #endif
 
+static int
+__ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsigned long addr)
+{
+	struct ppc_inst op;
+	unsigned long ip = rec->ip;
+	unsigned long entry, ptr, tramp;
+
+	/* read where this goes */
+	if (probe_kernel_read_inst(&op, (void *)ip)) {
+		pr_err("Fetching opcode failed.\n");
+		return -EFAULT;
+	}
+
+	/* Make sure that this is still a 24bit jump */
+	if (!is_bl_op(op)) {
+		pr_err("Not expected bl: opcode is %s\n", ppc_inst_as_str(op));
+		return -EINVAL;
+	}
+
+	/* lets find where the pointer goes */
+	tramp = find_bl_target(ip, op);
+	entry = ppc_global_function_entry((void *)old_addr);
+
+	pr_devel("ip:%lx jumps to %lx", ip, tramp);
+
+	if (tramp != entry) {
+		/* old_addr is not within range, so we must have used a trampoline */
+		struct ppc_ftrace_stub_data *stub;
+
+		hash_for_each_possible(ppc_ftrace_stubs, stub, hentry, entry)
+			if (stub->target == entry && stub->addr == tramp)
+				break;
+
+		if (stub->target != entry || stub->addr != tramp) {
+			pr_err("we don't know about the tramp at %lx!\n", tramp);
+			return -EFAULT;
+		}
+	}
+
+	/* The new target may be within range */
+	if (test_24bit_addr(ip, addr)) {
+		/* within range */
+		if (patch_branch((struct ppc_inst *)ip, addr, BRANCH_SET_LINK)) {
+			pr_err("REL24 out of range!\n");
+			return -EINVAL;
+		}
+
+		return 0;
+	}
+
+	ptr = ppc_global_function_entry((void *)addr);
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	/* Make sure we branch to ftrace_regs_caller since we only setup stubs for that */
+	entry = ppc_global_function_entry((void *)ftrace_caller);
+	if (ptr == entry)
+		ptr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
+#endif
+
+	tramp = find_ftrace_tramp(ip, ptr);
+
+	if (!tramp) {
+		pr_err("Couldn't find a trampoline\n");
+		return -EFAULT;
+	}
+
+	pr_devel("trampoline %lx target %lx", tramp, ptr);
+
+	if (patch_branch((struct ppc_inst *)ip, tramp, BRANCH_SET_LINK)) {
+		pr_err("REL24 out of range!\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
 int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 			unsigned long addr)
 {
@@ -800,11 +869,7 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 		new = ftrace_call_replace(ip, addr, 1);
 		return ftrace_modify_code(ip, old, new);
 	} else if (core_kernel_text(ip)) {
-		/*
-		 * We always patch out of range locations to go to the regs
-		 * variant, so there is nothing to do here
-		 */
-		return 0;
+		return __ftrace_modify_call_kernel(rec, old_addr, addr);
 	}
 
 #ifdef CONFIG_MODULES
-- 
2.25.4

[RFC PATCH 11/14] powerpc/ftrace: Use GPR save/restore macros in ftrace_graph_caller()

[Naveen N. Rao]

Use SAVE_8GPRS(), REST_8GPRS() and _NIP(), along with using the standard
SWITCH_FRAME_SIZE for the stack frame in ftrace_graph_caller() to
simplify code. This increases the stack frame size, but it is unlikely
to be an issue since ftrace_[regs_]caller() have just used a similar
stack frame size, and it isn't evident that the graph caller has too
deep a call stack to cause issues.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 .../powerpc/kernel/trace/ftrace_64_mprofile.S | 28 +++++--------------
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
index f9fd5f743eba34..bbe871b47ade58 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
@@ -279,24 +279,17 @@ livepatch_handler:
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 _GLOBAL(ftrace_graph_caller)
-	stdu	r1, -112(r1)
+	stdu	r1,-SWITCH_FRAME_SIZE(r1)
 	/* with -mprofile-kernel, parameter regs are still alive at _mcount */
-	std	r10, 104(r1)
-	std	r9, 96(r1)
-	std	r8, 88(r1)
-	std	r7, 80(r1)
-	std	r6, 72(r1)
-	std	r5, 64(r1)
-	std	r4, 56(r1)
-	std	r3, 48(r1)
+	SAVE_8GPRS(3, r1)
 
 	/* Save callee's TOC in the ABI compliant location */
 	std	r2, 24(r1)
 	ld	r2, PACATOC(r13)	/* get kernel TOC in r2 */
 
-	addi	r5, r1, 112
+	addi	r5, r1, SWITCH_FRAME_SIZE
 	mfctr	r4		/* ftrace_caller has moved local addr here */
-	std	r4, 40(r1)
+	std	r4, _NIP(r1)
 	mflr	r3		/* ftrace_caller has restored LR from stack */
 	subi	r4, r4, MCOUNT_INSN_SIZE
 
@@ -309,21 +302,14 @@ _GLOBAL(ftrace_graph_caller)
 	 */
 	mtlr	r3
 
-	ld	r0, 40(r1)
+	ld	r0, _NIP(r1)
 	mtctr	r0
-	ld	r10, 104(r1)
-	ld	r9, 96(r1)
-	ld	r8, 88(r1)
-	ld	r7, 80(r1)
-	ld	r6, 72(r1)
-	ld	r5, 64(r1)
-	ld	r4, 56(r1)
-	ld	r3, 48(r1)
+	REST_8GPRS(3, r1)
 
 	/* Restore callee's TOC */
 	ld	r2, 24(r1)
 
-	addi	r1, r1, 112
+	addi	r1, r1, SWITCH_FRAME_SIZE
 	mflr	r0
 	std	r0, LRSAVE(r1)
 	bctr
-- 
2.25.4

[RFC PATCH 12/14] powerpc/ftrace: Drop saving LR to stack save area for -mprofile-kernel

[Naveen N. Rao]

In -mprofile-kernel variant of ftrace_graph_caller(), we save the
optionally-updated LR address into the stack save area at the end. This
is likely an offshoot of the initial -mprofile-kernel implementation in
gcc emitting the same as part of the -mprofile-kernel instruction
sequence. However, this is not required. Drop it.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace_64_mprofile.S | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
index bbe871b47ade58..c5602e9b07faa3 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
@@ -310,7 +310,5 @@ _GLOBAL(ftrace_graph_caller)
 	ld	r2, 24(r1)
 
 	addi	r1, r1, SWITCH_FRAME_SIZE
-	mflr	r0
-	std	r0, LRSAVE(r1)
 	bctr
 #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-- 
2.25.4

[RFC PATCH 13/14] powerpc/ftrace: Add support for register_ftrace_direct() for MPROFILE_KERNEL

[Naveen N. Rao]

Add support for register_ftrace_direct() for MPROFILE_KERNEL, as it
depends on DYNAMIC_FTRACE_WITH_REGS.

Since powerpc only provides a branch range of 32MB, we set aside a 64k
area within kernel text for creating stubs that can be used to branch to
the provided trampoline, which can be located in the module area. This
is limited to kernel text, and as such, ftrace direct calls are not
supported for functions in kernel modules at this time.

We use orig_gpr3 to stash the address of the direct call trampoline in
arch_ftrace_set_direct_caller().  ftrace_regs_caller() is updated to
check for this to determine if we need to redirect to a direct call
trampoline. As the direct call trampoline has to work as an alternative
for the ftrace trampoline, we setup LR and r0 appropriately, and update
ctr to the trampoline address.  Finally, ftrace_graph_caller() is
updated to save/restore r0.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/Kconfig                          |   1 +
 arch/powerpc/include/asm/ftrace.h             |  14 ++
 arch/powerpc/kernel/trace/ftrace.c            | 140 +++++++++++++++++-
 .../powerpc/kernel/trace/ftrace_64_mprofile.S |  40 ++++-
 4 files changed, 182 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index cfc6dd787f532c..a87ac2e403196e 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -197,6 +197,7 @@ config PPC
 	select HAVE_DEBUG_KMEMLEAK
 	select HAVE_DEBUG_STACKOVERFLOW
 	select HAVE_DYNAMIC_FTRACE
+	select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS	if MPROFILE_KERNEL
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS	if MPROFILE_KERNEL
 	select HAVE_EBPF_JIT			if PPC64
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS	if !(CPU_LITTLE_ENDIAN && POWER7_CPU)
diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index bc76970b6ee532..2f1c46e9f5d416 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -10,6 +10,8 @@
 
 #define HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
 
+#define FTRACE_STUBS_SIZE	65536
+
 #ifdef __ASSEMBLY__
 
 /* Based off of objdump optput from glibc */
@@ -59,6 +61,18 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
 struct dyn_arch_ftrace {
 	struct module *mod;
 };
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+/*
+ * When there is a direct caller registered, we use regs->orig_gpr3 (similar to
+ * how x86 uses orig_ax) to let ftrace_{regs_}_caller know that we should go
+ * there instead of returning to the function
+ */
+static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
+{
+	regs->orig_gpr3 = addr;
+}
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
 #endif /* __ASSEMBLY__ */
 
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index fcb21a9756e456..815b14ae45a71f 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -37,6 +37,7 @@ static DEFINE_HASHTABLE(ppc_ftrace_stubs, 8);
 struct ppc_ftrace_stub_data {
 	unsigned long addr;
 	unsigned long target;
+	refcount_t refs;
 	struct hlist_node hentry;
 };
 
@@ -299,7 +300,7 @@ static unsigned long find_ftrace_tramp(unsigned long ip, unsigned long target)
 	return 0;
 }
 
-static int add_ftrace_tramp(unsigned long tramp, unsigned long target)
+static int add_ftrace_tramp(unsigned long tramp, unsigned long target, int lock)
 {
 	struct ppc_ftrace_stub_data *stub;
 
@@ -309,11 +310,123 @@ static int add_ftrace_tramp(unsigned long tramp, unsigned long target)
 
 	stub->addr = tramp;
 	stub->target = target;
+	refcount_set(&stub->refs, 1);
+	if (lock)
+		refcount_inc(&stub->refs);
 	hash_add(ppc_ftrace_stubs, &stub->hentry, target);
 
 	return 0;
 }
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+static u32 ftrace_direct_stub_insns[] = {
+	PPC_RAW_LIS(12, 0),
+	PPC_RAW_ORI(12, 12, 0),
+	PPC_RAW_SLDI(12, 12, 32),
+	PPC_RAW_ORIS(12, 12, 0),
+	PPC_RAW_ORI(12, 12, 0),
+	PPC_RAW_MTCTR(12),
+	PPC_RAW_BCTR(),
+};
+#define FTRACE_NUM_STUBS	(FTRACE_STUBS_SIZE / sizeof(ftrace_direct_stub_insns))
+static DECLARE_BITMAP(stubs_bitmap, FTRACE_NUM_STUBS);
+extern unsigned int ftrace_stubs[];
+
+static unsigned long get_ftrace_tramp(unsigned long ip, unsigned long target)
+{
+	struct ppc_ftrace_stub_data *stub_data;
+	struct ppc_inst instr;
+	unsigned int *stub;
+	int index;
+
+	hash_for_each_possible(ppc_ftrace_stubs, stub_data, hentry, target) {
+		if (stub_data->target == target &&
+				!create_branch(&instr, (void *)ip, stub_data->addr, 0)) {
+			refcount_inc(&stub_data->refs);
+			return stub_data->addr;
+		}
+	}
+
+	/* Allocate a stub */
+	do {
+		index = find_first_zero_bit(stubs_bitmap, FTRACE_NUM_STUBS);
+		if (index >= FTRACE_NUM_STUBS) {
+			pr_err("No stubs available\n");
+			return 0;
+		}
+	} while (test_and_set_bit(index, stubs_bitmap));
+	stub = &ftrace_stubs[index * sizeof(ftrace_direct_stub_insns) / 4];
+
+	if (create_branch(&instr, (void *)ip, (unsigned long)stub, 0)) {
+		/* Stub is not reachable from the ftrace location */
+		clear_bit(index, stubs_bitmap);
+		return 0;
+	}
+
+	memcpy(stub, ftrace_direct_stub_insns, sizeof(ftrace_direct_stub_insns));
+	stub[0] |= IMM_L(target >> 48);
+	stub[1] |= IMM_L(target >> 32);
+	stub[3] |= IMM_L(target >> 16);
+	stub[4] |= IMM_L(target);
+	if (add_ftrace_tramp((unsigned long)stub, target, 0)) {
+		pr_err("Error allocating ftrace stub");
+		clear_bit(index, stubs_bitmap);
+		return 0;
+	}
+
+	return (unsigned long)stub;
+}
+
+static void remove_ftrace_tramp(unsigned long ip, unsigned long target, unsigned long stub_addr)
+{
+	struct ppc_ftrace_stub_data *stub;
+	unsigned long tramp = 0;
+	struct ppc_inst instr;
+	int index;
+
+	hash_for_each_possible(ppc_ftrace_stubs, stub, hentry, target) {
+		if (stub->target == target && stub->addr == stub_addr &&
+				!create_branch(&instr, (void *)ip, stub->addr, 0)) {
+			if (refcount_dec_and_test(&stub->refs)) {
+				tramp = stub->addr;
+				hash_del(&stub->hentry);
+				kfree(stub);
+				break;
+			}
+			return;
+		}
+	}
+
+	if (tramp) {
+		synchronize_rcu_tasks();
+		index = (tramp - (unsigned long)ftrace_stubs) / sizeof(ftrace_direct_stub_insns);
+		clear_bit(index, stubs_bitmap);
+	}
+}
+
+int arch_register_ftrace_direct(unsigned long ip, unsigned long addr)
+{
+	if (addr & 0x03) {
+		pr_err("Target address is not at instruction boundary: 0x%lx\n", addr);
+		return -EINVAL;
+	}
+
+	if (is_module_text_address(ip)) {
+		pr_err("Kernel modules are not supported for direct calls\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+#else
+static unsigned long get_ftrace_tramp(unsigned long ip, unsigned long target)
+{
+	return find_ftrace_tramp(ip, target);
+}
+
+static void remove_ftrace_tramp(unsigned long ip, unsigned long target, unsigned long stub_addr) { }
+#endif
+
 /*
  * If this is a compiler generated long_branch trampoline (essentially, a
  * trampoline that has a branch to _mcount()), we re-write the branch to
@@ -365,7 +478,7 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 		return -1;
 	}
 
-	if (add_ftrace_tramp(tramp, ftrace_target)) {
+	if (add_ftrace_tramp(tramp, ftrace_target, 1)) {
 		pr_debug("No tramp locations left\n");
 		return -1;
 	}
@@ -409,6 +522,8 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned long addr)
 		return -EPERM;
 	}
 
+	remove_ftrace_tramp(ip, addr, tramp);
+
 	return 0;
 }
 
@@ -631,7 +746,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
 		return -EINVAL;
 	}
 
-	tramp = find_ftrace_tramp((unsigned long)ip, ptr);
+	tramp = get_ftrace_tramp((unsigned long)ip, ptr);
 	if (!tramp) {
 		pr_err("No ftrace trampolines reachable from %ps\n", ip);
 		return -EINVAL;
@@ -782,7 +897,7 @@ __ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsi
 {
 	struct ppc_inst op;
 	unsigned long ip = rec->ip;
-	unsigned long entry, ptr, tramp;
+	unsigned long entry, ptr, tramp, tramp_old = 0;
 
 	/* read where this goes */
 	if (probe_kernel_read_inst(&op, (void *)ip)) {
@@ -814,6 +929,8 @@ __ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsi
 			pr_err("we don't know about the tramp at %lx!\n", tramp);
 			return -EFAULT;
 		}
+
+		tramp_old = tramp;
 	}
 
 	/* The new target may be within range */
@@ -824,7 +941,7 @@ __ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsi
 			return -EINVAL;
 		}
 
-		return 0;
+		goto out;
 	}
 
 	ptr = ppc_global_function_entry((void *)addr);
@@ -836,7 +953,7 @@ __ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsi
 		ptr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 #endif
 
-	tramp = find_ftrace_tramp(ip, ptr);
+	tramp = get_ftrace_tramp(ip, ptr);
 
 	if (!tramp) {
 		pr_err("Couldn't find a trampoline\n");
@@ -850,8 +967,13 @@ __ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsi
 		return -EINVAL;
 	}
 
+out:
+	if (tramp_old)
+		remove_ftrace_tramp(ip, old_addr, tramp_old);
+
 	return 0;
 }
+
 int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 			unsigned long addr)
 {
@@ -950,9 +1072,13 @@ int __init ftrace_dyn_arch_init(void)
 		memcpy(tramp[i], stub_insns, sizeof(stub_insns));
 		tramp[i][1] |= PPC_HA(reladdr);
 		tramp[i][2] |= PPC_LO(reladdr);
-		add_ftrace_tramp((unsigned long)tramp[i], addr);
+		add_ftrace_tramp((unsigned long)tramp[i], addr, 1);
 	}
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+	bitmap_zero(stubs_bitmap, FTRACE_NUM_STUBS);
+#endif
+
 	return 0;
 }
 #else
diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
index c5602e9b07faa3..ffd2e33ff979bc 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
@@ -13,6 +13,13 @@
 #include <asm/bug.h>
 #include <asm/ptrace.h>
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+	.balign	4
+.global ftrace_stubs
+ftrace_stubs:
+	.space FTRACE_STUBS_SIZE
+#endif
+
 /*
  *
  * ftrace_caller()/ftrace_regs_caller() is the function that replaces _mcount()
@@ -91,6 +98,10 @@ _GLOBAL(ftrace_regs_caller)
 	std     r10, _XER(r1)
 	std     r11, _CCR(r1)
 
+	/* Clear out orig_gpr3 */
+	li	r6, 0
+	std	r6, ORIG_GPR3(r1)
+
 	/* Load &pt_regs in r6 for call below */
 	addi    r6, r1 ,STACK_FRAME_OVERHEAD
 
@@ -103,20 +114,34 @@ ftrace_regs_call:
 	/* Load ctr with the possibly modified NIP */
 	ld	r3, _NIP(r1)
 	mtctr	r3
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+	/* Check if we should go to a direct call next */
+	ld	r4, ORIG_GPR3(r1)
+	cmpdi	r4, 0
+	beq+	1f
+	/* r4 has the direct call target, setup LR and r0 as on our entry, reset cr0 */
+	mtctr	r4
+	mtlr	r3
+	ld	r0, _LINK(r1)
+	cmpd	r3, r3
+	b	2f
+#endif
+
+1:
 #ifdef CONFIG_LIVEPATCH
 	cmpd	r14, r3		/* has NIP been altered? */
 #endif
 
-	/* Restore gprs */
-	REST_GPR(0,r1)
-	REST_10GPRS(2,r1)
-	REST_10GPRS(12,r1)
-	REST_10GPRS(22,r1)
-
 	/* Restore possibly modified LR */
 	ld	r0, _LINK(r1)
 	mtlr	r0
 
+	/* Restore gprs */
+2:	REST_10GPRS(2,r1)
+	REST_10GPRS(12,r1)
+	REST_10GPRS(22,r1)
+
 	/* Restore callee's TOC */
 	ld	r2, 24(r1)
 
@@ -282,6 +307,7 @@ _GLOBAL(ftrace_graph_caller)
 	stdu	r1,-SWITCH_FRAME_SIZE(r1)
 	/* with -mprofile-kernel, parameter regs are still alive at _mcount */
 	SAVE_8GPRS(3, r1)
+	SAVE_GPR(0, r1)
 
 	/* Save callee's TOC in the ABI compliant location */
 	std	r2, 24(r1)
@@ -304,6 +330,8 @@ _GLOBAL(ftrace_graph_caller)
 
 	ld	r0, _NIP(r1)
 	mtctr	r0
+
+	REST_GPR(0, r1)
 	REST_8GPRS(3, r1)
 
 	/* Restore callee's TOC */
-- 
2.25.4

[RFC PATCH 14/14] samples/ftrace: Add powerpc support for ftrace direct samples

[Naveen N. Rao]

Add a simple powerpc trampoline to demonstrate use of ftrace direct on
powerpc.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 samples/Kconfig                       |  2 +-
 samples/ftrace/ftrace-direct-modify.c | 58 +++++++++++++++++++++++++++
 samples/ftrace/ftrace-direct-too.c    | 48 ++++++++++++++++++++--
 samples/ftrace/ftrace-direct.c        | 45 +++++++++++++++++++--
 4 files changed, 144 insertions(+), 9 deletions(-)

diff --git a/samples/Kconfig b/samples/Kconfig
index 0ed6e4d71d87b1..fdc9e44dba3b95 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -26,7 +26,7 @@ config SAMPLE_TRACE_PRINTK
 config SAMPLE_FTRACE_DIRECT
 	tristate "Build register_ftrace_direct() example"
 	depends on DYNAMIC_FTRACE_WITH_DIRECT_CALLS && m
-	depends on X86_64 # has x86_64 inlined asm
+	depends on X86_64 || PPC64 # has inlined asm
 	help
 	  This builds an ftrace direct function example
 	  that hooks to wake_up_process and prints the parameters.
diff --git a/samples/ftrace/ftrace-direct-modify.c b/samples/ftrace/ftrace-direct-modify.c
index c13a5bc5095bea..89d66a12d300e1 100644
--- a/samples/ftrace/ftrace-direct-modify.c
+++ b/samples/ftrace/ftrace-direct-modify.c
@@ -2,6 +2,7 @@
 #include <linux/module.h>
 #include <linux/kthread.h>
 #include <linux/ftrace.h>
+#include <linux/kprobes.h>
 
 void my_direct_func1(void)
 {
@@ -18,6 +19,7 @@ extern void my_tramp2(void *);
 
 static unsigned long my_ip = (unsigned long)schedule;
 
+#ifdef CONFIG_X86
 asm (
 "	.pushsection    .text, \"ax\", @progbits\n"
 "	.type		my_tramp1, @function\n"
@@ -38,6 +40,58 @@ asm (
 "	.size		my_tramp2, .-my_tramp2\n"
 "	.popsection\n"
 );
+#elif CONFIG_PPC64
+asm (
+"	.pushsection	.text, \"ax\", @progbits\n"
+"	.type		my_tramp1, @function\n"
+"	.global		my_tramp1\n"
+"   my_tramp1:\n"
+"	std	0, 16(1)\n"
+"	stdu	1, -480(1)\n"
+"	std	2, 24(1)\n"
+"	mflr	7\n"
+"	std	7, 368(1)\n"
+"	bcl	20, 31, 1f\n"
+"1:	mflr	12\n"
+"	ld	2, (3f - 1b)(12)\n"
+"	bl	my_direct_func1\n"
+"	nop\n"
+"	ld	2, 24(1)\n"
+"	ld	7, 368(1)\n"
+"	mtctr	7\n"
+"	addi	1, 1, 480\n"
+"	ld	0, 16(1)\n"
+"	mtlr	0\n"
+"	bctr\n"
+"	.size		my_tramp1, .-my_tramp1\n"
+"	nop\n"
+"	.type		my_tramp2, @function\n"
+"	.global		my_tramp2\n"
+"   my_tramp2:\n"
+"	std	0, 16(1)\n"
+"	stdu	1, -480(1)\n"
+"	std	2, 24(1)\n"
+"	mflr	7\n"
+"	std	7, 368(1)\n"
+"	bcl	20, 31, 2f\n"
+"2:	mflr	12\n"
+"	ld	2, (3f - 2b)(12)\n"
+"	bl	my_direct_func2\n"
+"	nop\n"
+"	ld	2, 24(1)\n"
+"	ld	7, 368(1)\n"
+"	mtctr	7\n"
+"	addi	1, 1, 480\n"
+"	ld	0, 16(1)\n"
+"	mtlr	0\n"
+"	bctr\n"
+"	.size		my_tramp2, .-my_tramp2\n"
+"3:\n"
+"	.quad		.TOC.@tocbase\n"
+"	.popsection\n"
+);
+#endif
+
 
 static unsigned long my_tramp = (unsigned long)my_tramp1;
 static unsigned long tramps[2] = {
@@ -72,6 +126,10 @@ static int __init ftrace_direct_init(void)
 {
 	int ret;
 
+#ifdef CONFIG_PPC64
+	my_ip = ppc_function_entry((void *)my_ip) + 4;
+#endif
+
 	ret = register_ftrace_direct(my_ip, my_tramp);
 	if (!ret)
 		simple_tsk = kthread_run(simple_thread, NULL, "event-sample-fn");
diff --git a/samples/ftrace/ftrace-direct-too.c b/samples/ftrace/ftrace-direct-too.c
index d5c5022be66429..9a82abecbe0dcc 100644
--- a/samples/ftrace/ftrace-direct-too.c
+++ b/samples/ftrace/ftrace-direct-too.c
@@ -3,6 +3,7 @@
 
 #include <linux/mm.h> /* for handle_mm_fault() */
 #include <linux/ftrace.h>
+#include <linux/kprobes.h>
 
 void my_direct_func(struct vm_area_struct *vma,
 			unsigned long address, unsigned int flags)
@@ -13,6 +14,9 @@ void my_direct_func(struct vm_area_struct *vma,
 
 extern void my_tramp(void *);
 
+static unsigned long my_ip = (unsigned long)handle_mm_fault;
+
+#ifdef CONFIG_X86
 asm (
 "	.pushsection    .text, \"ax\", @progbits\n"
 "	.type		my_tramp, @function\n"
@@ -31,18 +35,54 @@ asm (
 "	.size		my_tramp, .-my_tramp\n"
 "	.popsection\n"
 );
+#elif CONFIG_PPC64
+asm (
+"	.pushsection	.text, \"ax\", @progbits\n"
+"	.type		my_tramp, @function\n"
+"	.global		my_tramp\n"
+"   my_tramp:\n"
+"	std	0, 16(1)\n"
+"	stdu	1, -480(1)\n"
+"	std	2, 24(1)\n"
+"	std	3, 136(1)\n"
+"	std	4, 144(1)\n"
+"	std	5, 152(1)\n"
+"	mflr	7\n"
+"	std	7, 368(1)\n"
+"	bcl	20, 31, 1f\n"
+"1:	mflr	12\n"
+"	ld	2, (2f - 1b)(12)\n"
+"	bl	my_direct_func\n"
+"	nop\n"
+"	ld	5, 152(1)\n"
+"	ld	4, 144(1)\n"
+"	ld	3, 136(1)\n"
+"	ld	2, 24(1)\n"
+"	ld	7, 368(1)\n"
+"	mtctr	7\n"
+"	addi	1, 1, 480\n"
+"	ld	0, 16(1)\n"
+"	mtlr	0\n"
+"	bctr\n"
+"	.size		my_tramp, .-my_tramp\n"
+"2:\n"
+"	.quad		.TOC.@tocbase\n"
+"	.popsection\n"
+);
+#endif
 
 
 static int __init ftrace_direct_init(void)
 {
-	return register_ftrace_direct((unsigned long)handle_mm_fault,
-				     (unsigned long)my_tramp);
+#ifdef CONFIG_PPC64
+	my_ip = ppc_function_entry((void *)my_ip) + 4;
+#endif
+	return register_ftrace_direct(my_ip, (unsigned long)my_tramp);
 }
 
 static void __exit ftrace_direct_exit(void)
 {
-	unregister_ftrace_direct((unsigned long)handle_mm_fault,
-				 (unsigned long)my_tramp);
+	unregister_ftrace_direct(my_ip, (unsigned long)my_tramp);
 }
 
 module_init(ftrace_direct_init);
diff --git a/samples/ftrace/ftrace-direct.c b/samples/ftrace/ftrace-direct.c
index 63ca06d42c803f..da67b6217f91d2 100644
--- a/samples/ftrace/ftrace-direct.c
+++ b/samples/ftrace/ftrace-direct.c
@@ -3,6 +3,7 @@
 
 #include <linux/sched.h> /* for wake_up_process() */
 #include <linux/ftrace.h>
+#include <linux/kprobes.h> /* for ppc_function_entry() */
 
 void my_direct_func(struct task_struct *p)
 {
@@ -11,6 +12,9 @@ void my_direct_func(struct task_struct *p)
 
 extern void my_tramp(void *);
 
+static unsigned long my_ip = (unsigned long)wake_up_process;
+
+#ifdef CONFIG_X86
 asm (
 "	.pushsection    .text, \"ax\", @progbits\n"
 "	.type		my_tramp, @function\n"
@@ -25,18 +29,51 @@ asm (
 "	.size		my_tramp, .-my_tramp\n"
 "	.popsection\n"
 );
+#elif CONFIG_PPC64
+asm (
+"	.pushsection	.text, \"ax\", @progbits\n"
+"	.type		my_tramp, @function\n"
+"	.global		my_tramp\n"
+"   my_tramp:\n"
+"	std	0, 16(1)\n"
+"	stdu	1, -480(1)\n"
+"	std	2, 24(1)\n"
+"	std	3, 136(1)\n"
+"	mflr	7\n"
+"	std	7, 368(1)\n"
+"	bcl	20, 31, 1f\n"
+"1:	mflr	12\n"
+"	ld	2, (2f - 1b)(12)\n"
+"	bl	my_direct_func\n"
+"	nop\n"
+"	ld	3, 136(1)\n"
+"	ld	2, 24(1)\n"
+"	ld	7, 368(1)\n"
+"	mtctr	7\n"
+"	addi	1, 1, 480\n"
+"	ld	0, 16(1)\n"
+"	mtlr	0\n"
+"	bctr\n"
+"	.size		my_tramp, .-my_tramp\n"
+"2:\n"
+"	.quad		.TOC.@tocbase\n"
+"	.popsection\n"
+);
+#endif
 
 
 static int __init ftrace_direct_init(void)
 {
-	return register_ftrace_direct((unsigned long)wake_up_process,
-				     (unsigned long)my_tramp);
+#ifdef CONFIG_PPC64
+	/* Ftrace location is (usually) the second instruction at a function's local entry point */
+	my_ip = ppc_function_entry((void *)my_ip) + 4;
+#endif
+	return register_ftrace_direct(my_ip, (unsigned long)my_tramp);
 }
 
 static void __exit ftrace_direct_exit(void)
 {
-	unregister_ftrace_direct((unsigned long)wake_up_process,
-				 (unsigned long)my_tramp);
+	unregister_ftrace_direct(my_ip, (unsigned long)my_tramp);
 }
 
 module_init(ftrace_direct_init);
-- 
2.25.4

Re: [RFC PATCH 01/14] ftrace: Fix updating FTRACE_FL_TRAMP

[Steven Rostedt]

On Thu, 26 Nov 2020 23:38:38 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> On powerpc, kprobe-direct.tc triggered FTRACE_WARN_ON() in
> ftrace_get_addr_new() followed by the below message:
>   Bad trampoline accounting at: 000000004222522f (wake_up_process+0xc/0x20) (f0000001)
> 
> The set of steps leading to this involved:
> - modprobe ftrace-direct-too
> - enable_probe
> - modprobe ftrace-direct
> - rmmod ftrace-direct <-- trigger
> 
> The problem turned out to be that we were not updating flags in the
> ftrace record properly. From the above message about the trampoline
> accounting being bad, it can be seen that the ftrace record still has
> FTRACE_FL_TRAMP set though ftrace-direct module is going away. This
> happens because we are checking if any ftrace_ops has the
> FTRACE_FL_TRAMP flag set _before_ updating the filter hash.
> 
> The fix for this is to look for any _other_ ftrace_ops that also needs
> FTRACE_FL_TRAMP.

I'm applying this now and sending this for -rc and stable.

The code worked on x86 because x86 has a way to make all users use
trampolines, so this was never an issue (everything has a trampoline).
I modified the kernel so that x86 would not create its own trampoline
(see the weak function arch_ftrace_update_trampoline(), and I was able
to reproduce the bug.

I'm adding:

Cc: stable@vger.kernel.org
Fixes: a124692b698b0 ("ftrace: Enable trampoline when rec count returns back to one")

Thanks!

-- Steve


> 
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
>  kernel/trace/ftrace.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 8185f7240095f4..9c1bba8cc51b03 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -1629,6 +1629,8 @@ static bool test_rec_ops_needs_regs(struct dyn_ftrace *rec)
>  static struct ftrace_ops *
>  ftrace_find_tramp_ops_any(struct dyn_ftrace *rec);
>  static struct ftrace_ops *
> +ftrace_find_tramp_ops_any_other(struct dyn_ftrace *rec, struct ftrace_ops *op_exclude);
> +static struct ftrace_ops *
>  ftrace_find_tramp_ops_next(struct dyn_ftrace *rec, struct ftrace_ops *ops);
>  
>  static bool __ftrace_hash_rec_update(struct ftrace_ops *ops,
> @@ -1778,7 +1780,7 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops,
>  			 * to it.
>  			 */
>  			if (ftrace_rec_count(rec) == 1 &&
> -			    ftrace_find_tramp_ops_any(rec))
> +			    ftrace_find_tramp_ops_any_other(rec, ops))
>  				rec->flags |= FTRACE_FL_TRAMP;
>  			else
>  				rec->flags &= ~FTRACE_FL_TRAMP;
> @@ -2244,6 +2246,24 @@ ftrace_find_tramp_ops_any(struct dyn_ftrace *rec)
>  	return NULL;
>  }
>  
> +static struct ftrace_ops *
> +ftrace_find_tramp_ops_any_other(struct dyn_ftrace *rec, struct ftrace_ops *op_exclude)
> +{
> +	struct ftrace_ops *op;
> +	unsigned long ip = rec->ip;
> +
> +	do_for_each_ftrace_op(op, ftrace_ops_list) {
> +
> +		if (op == op_exclude || !op->trampoline)
> +			continue;
> +
> +		if (hash_contains_ip(ip, op->func_hash))
> +			return op;
> +	} while_for_each_ftrace_op(op);
> +
> +	return NULL;
> +}
> +
>  static struct ftrace_ops *
>  ftrace_find_tramp_ops_next(struct dyn_ftrace *rec,
>  			   struct ftrace_ops *op)

Re: [RFC PATCH 02/14] ftrace: Fix DYNAMIC_FTRACE_WITH_DIRECT_CALLS dependency

[Steven Rostedt]

On Thu, 26 Nov 2020 23:38:39 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> DYNAMIC_FTRACE_WITH_DIRECT_CALLS should depend on
> DYNAMIC_FTRACE_WITH_REGS since we need ftrace_regs_caller().
> 
> Fixes: 763e34e74bb7d5c ("ftrace: Add register_ftrace_direct()")
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

I'm pulling this in for stable as well.

-- Steve

> ---
>  kernel/trace/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index a4020c0b4508c9..e1bf5228fb692a 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -202,7 +202,7 @@ config DYNAMIC_FTRACE_WITH_REGS
>  
>  config DYNAMIC_FTRACE_WITH_DIRECT_CALLS
>  	def_bool y
> -	depends on DYNAMIC_FTRACE
> +	depends on DYNAMIC_FTRACE_WITH_REGS
>  	depends on HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
>  
>  config FUNCTION_PROFILER

Re: [RFC PATCH 03/14] ftrace: Fix cleanup in error path of register_ftrace_direct()

[Steven Rostedt]

On Thu, 26 Nov 2020 23:38:40 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> We need to remove hash entry if register_ftrace_function() fails.
> Consolidate the cleanup to be done after register_ftrace_function() at
> the end.

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

> 
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
>  kernel/trace/ftrace.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 9c1bba8cc51b03..3844a4a1346a9c 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5136,8 +5136,6 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
>  	__add_hash_entry(direct_functions, entry);
>  
>  	ret = ftrace_set_filter_ip(&direct_ops, ip, 0, 0);
> -	if (ret)
> -		remove_hash_entry(direct_functions, entry);
>  
>  	if (!ret && !(direct_ops.flags & FTRACE_OPS_FL_ENABLED)) {
>  		ret = register_ftrace_function(&direct_ops);
> @@ -5146,6 +5144,7 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
>  	}
>  
>  	if (ret) {
> +		remove_hash_entry(direct_functions, entry);
>  		kfree(entry);
>  		if (!direct->count) {
>  			list_del_rcu(&direct->next);

Re: [RFC PATCH 01/14] ftrace: Fix updating FTRACE_FL_TRAMP

[Naveen N. Rao]

Steven Rostedt wrote:
> On Thu, 26 Nov 2020 23:38:38 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> 
>> On powerpc, kprobe-direct.tc triggered FTRACE_WARN_ON() in
>> ftrace_get_addr_new() followed by the below message:
>>   Bad trampoline accounting at: 000000004222522f (wake_up_process+0xc/0x20) (f0000001)
>> 
>> The set of steps leading to this involved:
>> - modprobe ftrace-direct-too
>> - enable_probe
>> - modprobe ftrace-direct
>> - rmmod ftrace-direct <-- trigger
>> 
>> The problem turned out to be that we were not updating flags in the
>> ftrace record properly. From the above message about the trampoline
>> accounting being bad, it can be seen that the ftrace record still has
>> FTRACE_FL_TRAMP set though ftrace-direct module is going away. This
>> happens because we are checking if any ftrace_ops has the
>> FTRACE_FL_TRAMP flag set _before_ updating the filter hash.
>> 
>> The fix for this is to look for any _other_ ftrace_ops that also needs
>> FTRACE_FL_TRAMP.
> 
> I'm applying this now and sending this for -rc and stable.
> 
> The code worked on x86 because x86 has a way to make all users use
> trampolines, so this was never an issue (everything has a trampoline).
> I modified the kernel so that x86 would not create its own trampoline
> (see the weak function arch_ftrace_update_trampoline(), and I was able
> to reproduce the bug.

Good to know that you were able to reproduce this.

> 
> I'm adding:
> 
> Cc: stable@vger.kernel.org
> Fixes: a124692b698b0 ("ftrace: Enable trampoline when rec count returns back to one")

That looks good to me. Thanks for picking the two patches and for your 
review on the others!


- Naveen