All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH V2]: add DYNAMC_FTRACE_WITH_REGS and
@ 2021-03-13  6:41 Huang Pei
  2021-03-13  6:41 ` [PATCH 1/6] MIPS: replace -pg with CC_FLAGS_FTRACE Huang Pei
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Huang Pei @ 2021-03-13  6:41 UTC (permalink / raw)
  To: Thomas Bogendoerfer, ambrosehua
  Cc: Bibo Mao, linux-mips, linux-arch, linux-mm, Jiaxun Yang,
	Paul Burton, Li Xuefeng, Yang Tiezhu, Gao Juxin, Huacai Chen,
	Jinyang He, Maciej W . Rozycki, Steven Rostedt, Jisheng Zhang,
	Masami Hiramatsu

V2:
+. fix ftrace regs test failure

+. fix kprobe test failure with adding KPROBES_ON_FTRACE


This series add DYNAMC_FTRACE_WITH_REGS and KPROBES_ON_FTRACE support 
without depending on _mcount and -pg, and try to address following issue

+. _mcount stub size is 3 insns in vmlinux  and  4 insns in .ko, too much

+. complex handing MIPS32 and MIPS64 in _mcount, especially sp pointer in
MIPS32

+. stub is called with sp adjusted in Callee(the traced function), which
is hard for livepatch to restore the original sp pointer

Remaining Issues
################

+. reserve three nops or four nops for <= MIPS R5 ?

Without direct call, three nops is enough. With direct call, we need to 
hack ftrace to save the first instruction somewhere. Four nops is enough 
for all cases

MIPS R6 only need three nops without hacking

+. MIPS32 support, working on it

+. checking for gcc version, can previous two bug back porting to gcc 8.5?
We should check gcc's version

+. stack backstrace

GCC
#########

+. GCC 8 add -fpatchable-function-entry=N[, M] support to insert N 
nops before real start, for more info, see gcc 8 manual

+. GCC/MIPS has two bug: 93242 (fixed in gcc 10), 99217 (with a fix, but
not accepted) about this option. With fixes applyed in gcc 8.3, vmlinux is OK


Design
#########

+. Caller A calls Callee B, with -fpatchable-function-entry=3, B has 
three nops at its entry,

------------
	::

		A:

		......
			jal	B
			nop
		......

		B:
			nop
			nop
			nop

		#B: real start 
			INSN_B_first

+. With ftrace initialized or module loaded, this three nop got
replaced,

------------
	::

		A:

		......
			jal	B
			nop
		......

		B:
			lui	at, %hi(ftrace_regs_caller)
			nop
			li	t0, 0

		#B: real start 
			INSN_B_first

Obviously, ftrace_regs_caller is 64KB aligned, thanks He Jinyang
<hejinyang@loongson.cn>
	
+. To enable tracing , take nop into "jalr at, at“, 

PS: 

"jalr at, at" jump and link into addr in "at", and save the return address
in "at";

With this, no touching parent return address in ra

------------
	::

		A:

		......
			jal	B
			nop
		......

		B:
			lui	at, %hi(ftrace_regs_caller)
			jalr	at, at
			li	t0, 0

		#B: real start 
			INSN_B_first
	

+. To disable tracing, take "jalr at, at" into nop

------------
	::

		A:
		......

			jal	B
			nop
		......

		B:
			lui	at, %hi(ftrace_regs_caller)
			nop
			li	t0, 0

		#B: real start 
			INSN_B_first
	
+. when tracing without regs, replace "li t0, 0' with "li t0, 1"

------------
	::

		A:
		......

			jal	B
			nop
		......
		B:
			lui	at, %hi(ftrace_regs_caller)
			jalr	at, at
			li	t0, 1
		#B: real start 
			INSN_B_first

With only one instruction modified, it is atomic and no sync needed (
_mcount need sync between two writes) on both MIPS32 and MIPS64, I got 
this from ARM64.

we need transfrom from tracing disabled into tracing without regs, first
replace "li t0, 0" with "li t0, 1", then "nop" with "jalr at, at", still
no sync between

------------
	::

		A:

		......
			jal	B
			nop
		......
		B:
			lui	at, %hi(ftrace_regs_caller)
			jalr	at, at
			li	t0, 1

		#B: real start 
			INSN_B_first


PS:

In mcount-based ftrace of MIPS32 vmlinux, the _mcount calling sequence
like this:

------------
	::

		A:
		......

			jal	B
			nop
		......
		B:
			move	at, ra
			jal	_mcount
			addiu	sp, sp, -32
		#B: real start 
			INSN_B_first

------------
	::

		A:
		......

			jal	B
			nop
		......
		B:
			move	at, ra
			nop
			nop
		#B: real start 
			INSN_B_first

no matter disabing and enabling tracing, we can not atomically change
both "jalr" and "addiu"(sync does not help here), on MIP32/SMP, whether
this 

------------
	::

		A:

		......
			jal	B
			nop
		......
		B:
			move	at, ra
			nop
			addiu	sp, sp, -32
		#B: real start 
			INSN_B_first

or this

------------
	::

		A:
		......
			jal	B
			nop
		......
		B:
			move	at, ra
			nop
			addiu	sp, sp, -32
		#B: real start 
			INSN_B_first

would wreck the ftrace

+. When B is ok to be patched, replace first four instruction with new 
function B'

------------
	::

		A:
		......
			jal	B
			nop
		......
		B:
			lui	at, %hi(B')	// second, fill new B'high
			addiu	at, %lo(B')	// first, fill nop
						// third, fill new B' low
			jr	at		// at last, fill jr
		#B: real start 
			nop			//forth, fill nop
						//Watch Out! 
						//first instruction 
						// clobbered. we
						//need to save it somewhere
						//or we must use four nops

if tracing enabled, we need to disable tracing first, and we need sync 
before fill "jr"
	
***Or use 4 nops for stub***

Patches
###########

Patch 1 - Patch 3 

This prepares new MIPS/ftrace with DYNAMIC_FTRACE_WITH_REGS and KPROBES_ON_FTACE 
in parallel with old MIPS/Ftrace 

NO function changed, these three patches can be merge into one patch


Patch 4 - Patch 5

this is needed for all RISC


Patch 6

Add DYNAMC_FTRACE_WITH_REGS and KPROBES_ON_FTACE support 




^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/6] MIPS: replace -pg with CC_FLAGS_FTRACE
  2021-03-13  6:41 [RFC PATCH V2]: add DYNAMC_FTRACE_WITH_REGS and Huang Pei
@ 2021-03-13  6:41 ` Huang Pei
  2021-03-25 19:38   ` Steven Rostedt
  2021-03-13  6:41 ` [PATCH 2/6] MIPS: move FTRACE_SYSCALLS from ftrace.c into syscall.c Huang Pei
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Huang Pei @ 2021-03-13  6:41 UTC (permalink / raw)
  To: Thomas Bogendoerfer, ambrosehua
  Cc: Bibo Mao, linux-mips, linux-arch, linux-mm, Jiaxun Yang,
	Paul Burton, Li Xuefeng, Yang Tiezhu, Gao Juxin, Huacai Chen,
	Jinyang He, Maciej W . Rozycki, Steven Rostedt, Jisheng Zhang,
	Masami Hiramatsu

Signed-off-by: Huang Pei <huangpei@loongson.cn>
---
 arch/mips/boot/compressed/Makefile | 2 +-
 arch/mips/kernel/Makefile          | 8 ++++----
 arch/mips/vdso/Makefile            | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/mips/boot/compressed/Makefile b/arch/mips/boot/compressed/Makefile
index d66511825fe1..8fc9ceeec709 100644
--- a/arch/mips/boot/compressed/Makefile
+++ b/arch/mips/boot/compressed/Makefile
@@ -18,7 +18,7 @@ include $(srctree)/arch/mips/Kbuild.platforms
 BOOT_HEAP_SIZE := 0x400000
 
 # Disable Function Tracer
-KBUILD_CFLAGS := $(filter-out -pg, $(KBUILD_CFLAGS))
+KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE), $(KBUILD_CFLAGS))
 
 KBUILD_CFLAGS := $(filter-out -fstack-protector, $(KBUILD_CFLAGS))
 
diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
index 2a05b923f579..33e31ea10234 100644
--- a/arch/mips/kernel/Makefile
+++ b/arch/mips/kernel/Makefile
@@ -17,10 +17,10 @@ obj-y		+= cpu-probe.o
 endif
 
 ifdef CONFIG_FUNCTION_TRACER
-CFLAGS_REMOVE_ftrace.o = -pg
-CFLAGS_REMOVE_early_printk.o = -pg
-CFLAGS_REMOVE_perf_event.o = -pg
-CFLAGS_REMOVE_perf_event_mipsxx.o = -pg
+CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_early_printk.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_perf_event.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_perf_event_mipsxx.o = $(CC_FLAGS_FTRACE)
 endif
 
 obj-$(CONFIG_CEVT_BCM1480)	+= cevt-bcm1480.o
diff --git a/arch/mips/vdso/Makefile b/arch/mips/vdso/Makefile
index 5810cc12bc1d..f21cf88f7ae3 100644
--- a/arch/mips/vdso/Makefile
+++ b/arch/mips/vdso/Makefile
@@ -49,7 +49,7 @@ CFLAGS_vgettimeofday-o32.o = -include $(srctree)/$(src)/config-n32-o32-env.c -in
 CFLAGS_vgettimeofday-n32.o = -include $(srctree)/$(src)/config-n32-o32-env.c -include $(c-gettimeofday-y)
 endif
 
-CFLAGS_REMOVE_vgettimeofday.o = -pg
+CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE)
 
 ifdef CONFIG_MIPS_DISABLE_VDSO
   ifndef CONFIG_MIPS_LD_CAN_LINK_VDSO
@@ -63,7 +63,7 @@ ldflags-y := -Bsymbolic --no-undefined -soname=linux-vdso.so.1 \
 	$(filter -E%,$(KBUILD_CFLAGS)) -nostdlib -shared \
 	-G 0 --eh-frame-hdr --hash-style=sysv --build-id=sha1 -T
 
-CFLAGS_REMOVE_vdso.o = -pg
+CFLAGS_REMOVE_vdso.o = $(CC_FLAGS_FTRACE)
 
 GCOV_PROFILE := n
 UBSAN_SANITIZE := n
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/6] MIPS: move FTRACE_SYSCALLS from ftrace.c into syscall.c
  2021-03-13  6:41 [RFC PATCH V2]: add DYNAMC_FTRACE_WITH_REGS and Huang Pei
  2021-03-13  6:41 ` [PATCH 1/6] MIPS: replace -pg with CC_FLAGS_FTRACE Huang Pei
@ 2021-03-13  6:41 ` Huang Pei
  2021-03-25 19:38   ` Steven Rostedt
  2021-03-13  6:41 ` [PATCH 3/6] MIPS: prepare for new ftrace implementation Huang Pei
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Huang Pei @ 2021-03-13  6:41 UTC (permalink / raw)
  To: Thomas Bogendoerfer, ambrosehua
  Cc: Bibo Mao, linux-mips, linux-arch, linux-mm, Jiaxun Yang,
	Paul Burton, Li Xuefeng, Yang Tiezhu, Gao Juxin, Huacai Chen,
	Jinyang He, Maciej W . Rozycki, Steven Rostedt, Jisheng Zhang,
	Masami Hiramatsu

Signed-off-by: Huang Pei <huangpei@loongson.cn>
---
 arch/mips/kernel/Makefile  |  1 -
 arch/mips/kernel/ftrace.c  | 33 ---------------------------------
 arch/mips/kernel/syscall.c | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
index 33e31ea10234..5b2b551058ac 100644
--- a/arch/mips/kernel/Makefile
+++ b/arch/mips/kernel/Makefile
@@ -39,7 +39,6 @@ obj-$(CONFIG_DEBUG_FS)		+= segment.o
 obj-$(CONFIG_STACKTRACE)	+= stacktrace.o
 obj-$(CONFIG_MODULES)		+= module.o
 
-obj-$(CONFIG_FTRACE_SYSCALLS)	+= ftrace.o
 obj-$(CONFIG_FUNCTION_TRACER)	+= mcount.o ftrace.o
 
 sw-y				:= r4k_switch.o
diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
index f57e68f40a34..5156b2e54bfe 100644
--- a/arch/mips/kernel/ftrace.c
+++ b/arch/mips/kernel/ftrace.c
@@ -12,14 +12,11 @@
 #include <linux/uaccess.h>
 #include <linux/init.h>
 #include <linux/ftrace.h>
-#include <linux/syscalls.h>
 
 #include <asm/asm.h>
 #include <asm/asm-offsets.h>
 #include <asm/cacheflush.h>
-#include <asm/syscall.h>
 #include <asm/uasm.h>
-#include <asm/unistd.h>
 
 #include <asm-generic/sections.h>
 
@@ -382,33 +379,3 @@ void prepare_ftrace_return(unsigned long *parent_ra_addr, unsigned long self_ra,
 	WARN_ON(1);
 }
 #endif	/* CONFIG_FUNCTION_GRAPH_TRACER */
-
-#ifdef CONFIG_FTRACE_SYSCALLS
-
-#ifdef CONFIG_32BIT
-unsigned long __init arch_syscall_addr(int nr)
-{
-	return (unsigned long)sys_call_table[nr - __NR_O32_Linux];
-}
-#endif
-
-#ifdef CONFIG_64BIT
-
-unsigned long __init arch_syscall_addr(int nr)
-{
-#ifdef CONFIG_MIPS32_N32
-	if (nr >= __NR_N32_Linux && nr < __NR_N32_Linux + __NR_N32_Linux_syscalls)
-		return (unsigned long)sysn32_call_table[nr - __NR_N32_Linux];
-#endif
-	if (nr >= __NR_64_Linux  && nr < __NR_64_Linux + __NR_64_Linux_syscalls)
-		return (unsigned long)sys_call_table[nr - __NR_64_Linux];
-#ifdef CONFIG_MIPS32_O32
-	if (nr >= __NR_O32_Linux && nr < __NR_O32_Linux + __NR_O32_Linux_syscalls)
-		return (unsigned long)sys32_call_table[nr - __NR_O32_Linux];
-#endif
-
-	return (unsigned long) &sys_ni_syscall;
-}
-#endif
-
-#endif /* CONFIG_FTRACE_SYSCALLS */
diff --git a/arch/mips/kernel/syscall.c b/arch/mips/kernel/syscall.c
index 2afa3eef486a..797d9ce478da 100644
--- a/arch/mips/kernel/syscall.c
+++ b/arch/mips/kernel/syscall.c
@@ -39,7 +39,9 @@
 #include <asm/shmparam.h>
 #include <asm/sync.h>
 #include <asm/sysmips.h>
+#include <asm/syscall.h>
 #include <asm/switch_to.h>
+#include <asm/unistd.h>
 
 /*
  * For historic reasons the pipe(2) syscall on MIPS has an unusual calling
@@ -233,6 +235,36 @@ SYSCALL_DEFINE3(sysmips, long, cmd, long, arg1, long, arg2)
 	return -EINVAL;
 }
 
+#ifdef CONFIG_FTRACE_SYSCALLS
+
+#ifdef CONFIG_32BIT
+unsigned long __init arch_syscall_addr(int nr)
+{
+	return (unsigned long)sys_call_table[nr - __NR_O32_Linux];
+}
+#endif
+
+#ifdef CONFIG_64BIT
+
+unsigned long __init arch_syscall_addr(int nr)
+{
+#ifdef CONFIG_MIPS32_N32
+	if (nr >= __NR_N32_Linux && nr < __NR_N32_Linux + __NR_N32_Linux_syscalls)
+		return (unsigned long)sysn32_call_table[nr - __NR_N32_Linux];
+#endif
+	if (nr >= __NR_64_Linux  && nr < __NR_64_Linux + __NR_64_Linux_syscalls)
+		return (unsigned long)sys_call_table[nr - __NR_64_Linux];
+#ifdef CONFIG_MIPS32_O32
+	if (nr >= __NR_O32_Linux && nr < __NR_O32_Linux + __NR_O32_Linux_syscalls)
+		return (unsigned long)sys32_call_table[nr - __NR_O32_Linux];
+#endif
+
+	return (unsigned long) &sys_ni_syscall;
+}
+#endif
+
+#endif /* CONFIG_FTRACE_SYSCALLS */
+
 /*
  * No implemented yet ...
  */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/6] MIPS: prepare for new ftrace implementation
  2021-03-13  6:41 [RFC PATCH V2]: add DYNAMC_FTRACE_WITH_REGS and Huang Pei
  2021-03-13  6:41 ` [PATCH 1/6] MIPS: replace -pg with CC_FLAGS_FTRACE Huang Pei
  2021-03-13  6:41 ` [PATCH 2/6] MIPS: move FTRACE_SYSCALLS from ftrace.c into syscall.c Huang Pei
@ 2021-03-13  6:41 ` Huang Pei
  2021-03-25 19:39   ` Steven Rostedt
  2021-03-13  6:41 ` [PATCH 4/6] kprobes/ftrace: Use ftrace_location() when [dis]arming probes Huang Pei
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Huang Pei @ 2021-03-13  6:41 UTC (permalink / raw)
  To: Thomas Bogendoerfer, ambrosehua
  Cc: Bibo Mao, linux-mips, linux-arch, linux-mm, Jiaxun Yang,
	Paul Burton, Li Xuefeng, Yang Tiezhu, Gao Juxin, Huacai Chen,
	Jinyang He, Maciej W . Rozycki, Steven Rostedt, Jisheng Zhang,
	Masami Hiramatsu

No function change

Signed-off-by: Huang Pei <huangpei@loongson.cn>
---
 arch/mips/kernel/Makefile                      | 4 ++--
 arch/mips/kernel/{ftrace.c => ftrace-mcount.c} | 0
 2 files changed, 2 insertions(+), 2 deletions(-)
 rename arch/mips/kernel/{ftrace.c => ftrace-mcount.c} (100%)

diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
index 5b2b551058ac..3e7b0ee54cfb 100644
--- a/arch/mips/kernel/Makefile
+++ b/arch/mips/kernel/Makefile
@@ -17,7 +17,7 @@ obj-y		+= cpu-probe.o
 endif
 
 ifdef CONFIG_FUNCTION_TRACER
-CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_ftrace-mcount.o = $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_early_printk.o = $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_perf_event.o = $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_perf_event_mipsxx.o = $(CC_FLAGS_FTRACE)
@@ -39,7 +39,7 @@ obj-$(CONFIG_DEBUG_FS)		+= segment.o
 obj-$(CONFIG_STACKTRACE)	+= stacktrace.o
 obj-$(CONFIG_MODULES)		+= module.o
 
-obj-$(CONFIG_FUNCTION_TRACER)	+= mcount.o ftrace.o
+obj-$(CONFIG_FUNCTION_TRACER)	+= mcount.o ftrace-mcount.o
 
 sw-y				:= r4k_switch.o
 sw-$(CONFIG_CPU_R3000)		:= r2300_switch.o
diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace-mcount.c
similarity index 100%
rename from arch/mips/kernel/ftrace.c
rename to arch/mips/kernel/ftrace-mcount.c
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/6] kprobes/ftrace: Use ftrace_location() when [dis]arming probes
  2021-03-13  6:41 [RFC PATCH V2]: add DYNAMC_FTRACE_WITH_REGS and Huang Pei
                   ` (2 preceding siblings ...)
  2021-03-13  6:41 ` [PATCH 3/6] MIPS: prepare for new ftrace implementation Huang Pei
@ 2021-03-13  6:41 ` Huang Pei
  2021-03-25 19:44   ` Steven Rostedt
  2021-03-13  6:41 ` [PATCH 5/6] ftrace: introduce FTRACE_IP_EXTENSION Huang Pei
  2021-03-13  6:41 ` [PATCH 6/6] MIPS: add DYNAMIC_FTRACE_WITH_REGS and KPROBES_ON_FTACE Huang Pei
  5 siblings, 1 reply; 18+ messages in thread
From: Huang Pei @ 2021-03-13  6:41 UTC (permalink / raw)
  To: Thomas Bogendoerfer, ambrosehua
  Cc: Bibo Mao, linux-mips, linux-arch, linux-mm, Jiaxun Yang,
	Paul Burton, Li Xuefeng, Yang Tiezhu, Gao Juxin, Huacai Chen,
	Jinyang He, Maciej W . Rozycki, Steven Rostedt, Jisheng Zhang,
	Masami Hiramatsu, Naveen N . Rao

From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>

Ftrace location could include more than a single instruction in case
of some architectures (powerpc64, for now). In this case, kprobe is
permitted on any of those instructions, and uses ftrace infrastructure
for functioning.

However, [dis]arm_kprobe_ftrace() uses the kprobe address when setting
up ftrace filter IP. This won't work if the address points to any
instruction apart from the one that has a branch to _mcount(). To
resolve this, have [dis]arm_kprobe_ftrace() use ftrace_function() to
identify the filter IP.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/kprobes.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 41fdbb7953c6..66ee28b071c2 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1045,9 +1045,10 @@ static int prepare_kprobe(struct kprobe *p)
 static int __arm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
 			       int *cnt)
 {
+	unsigned long ftrace_ip = ftrace_location((unsigned long)p->addr);
 	int ret = 0;
 
-	ret = ftrace_set_filter_ip(ops, (unsigned long)p->addr, 0, 0);
+	ret = ftrace_set_filter_ip(ops, ftrace_ip, 0, 0);
 	if (ret) {
 		pr_debug("Failed to arm kprobe-ftrace at %pS (%d)\n",
 			 p->addr, ret);
@@ -1070,7 +1071,7 @@ static int __arm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
 	 * At this point, sinec ops is not registered, we should be sefe from
 	 * registering empty filter.
 	 */
-	ftrace_set_filter_ip(ops, (unsigned long)p->addr, 1, 0);
+	ftrace_set_filter_ip(ops, ftrace_ip, 1, 0);
 	return ret;
 }
 
@@ -1087,6 +1088,7 @@ static int arm_kprobe_ftrace(struct kprobe *p)
 static int __disarm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
 				  int *cnt)
 {
+	unsigned long ftrace_ip = ftrace_location((unsigned long)p->addr);
 	int ret = 0;
 
 	if (*cnt == 1) {
@@ -1097,7 +1099,7 @@ static int __disarm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
 
 	(*cnt)--;
 
-	ret = ftrace_set_filter_ip(ops, (unsigned long)p->addr, 1, 0);
+	ret = ftrace_set_filter_ip(ops, ftrace_ip, 1, 0);
 	WARN_ONCE(ret < 0, "Failed to disarm kprobe-ftrace at %pS (%d)\n",
 		  p->addr, ret);
 	return ret;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/6] ftrace: introduce FTRACE_IP_EXTENSION
  2021-03-13  6:41 [RFC PATCH V2]: add DYNAMC_FTRACE_WITH_REGS and Huang Pei
                   ` (3 preceding siblings ...)
  2021-03-13  6:41 ` [PATCH 4/6] kprobes/ftrace: Use ftrace_location() when [dis]arming probes Huang Pei
@ 2021-03-13  6:41 ` Huang Pei
  2021-03-13  9:26   ` Sergei Shtylyov
  2021-03-13  6:41 ` [PATCH 6/6] MIPS: add DYNAMIC_FTRACE_WITH_REGS and KPROBES_ON_FTACE Huang Pei
  5 siblings, 1 reply; 18+ messages in thread
From: Huang Pei @ 2021-03-13  6:41 UTC (permalink / raw)
  To: Thomas Bogendoerfer, ambrosehua
  Cc: Bibo Mao, linux-mips, linux-arch, linux-mm, Jiaxun Yang,
	Paul Burton, Li Xuefeng, Yang Tiezhu, Gao Juxin, Huacai Chen,
	Jinyang He, Maciej W . Rozycki, Steven Rostedt, Jisheng Zhang,
	Masami Hiramatsu

From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>

On some architectures, the DYNAMIC_FTRACE_WITH_REGS is implemented by
gcc's -fpatchable-function-entry option. Take arm64 for example, arm64
makes use of GCC -fpatchable-function-entry=2 option to insert two
nops. When the function is traced, the first nop will be modified to
the LR saver, then the second nop to "bl <ftrace-entry>". we need to
update ftrace_location() to recognise these two instructions  as being
part of ftrace. To do this, we introduce FTRACE_IP_EXTENSION to let
ftrace_location search IP, IP + FTRACE_IP_EXTENSION range.

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 include/linux/ftrace.h | 4 ++++
 kernel/trace/ftrace.c  | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 1bd3a0356ae4..c1e1fbde8a04 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -20,6 +20,10 @@
 
 #include <asm/ftrace.h>
 
+#ifndef FTRACE_IP_EXTENSION
+#define  FTRACE_IP_EXTENSION 0
+#endif
+
 /*
  * If the arch supports passing the variable contents of
  * function_trace_op as the third parameter back from the
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 9c1bba8cc51b..a6f0e3db2479 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1583,7 +1583,7 @@ unsigned long ftrace_location_range(unsigned long start, unsigned long end)
  */
 unsigned long ftrace_location(unsigned long ip)
 {
-	return ftrace_location_range(ip, ip);
+	return ftrace_location_range(ip, ip + FTRACE_IP_EXTENSION);
 }
 
 /**
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/6] MIPS: add DYNAMIC_FTRACE_WITH_REGS and KPROBES_ON_FTACE
  2021-03-13  6:41 [RFC PATCH V2]: add DYNAMC_FTRACE_WITH_REGS and Huang Pei
                   ` (4 preceding siblings ...)
  2021-03-13  6:41 ` [PATCH 5/6] ftrace: introduce FTRACE_IP_EXTENSION Huang Pei
@ 2021-03-13  6:41 ` Huang Pei
  2021-03-13 21:37     ` kernel test robot
  5 siblings, 1 reply; 18+ messages in thread
From: Huang Pei @ 2021-03-13  6:41 UTC (permalink / raw)
  To: Thomas Bogendoerfer, ambrosehua
  Cc: Bibo Mao, linux-mips, linux-arch, linux-mm, Jiaxun Yang,
	Paul Burton, Li Xuefeng, Yang Tiezhu, Gao Juxin, Huacai Chen,
	Jinyang He, Maciej W . Rozycki, Steven Rostedt, Jisheng Zhang,
	Masami Hiramatsu

Add DYNAMIC_FTRACE_WITH_REGS and KPROBES_ON_FTACE suppport with
another ftrace implementation in parallel with mcount-based ftrace

+. Depend on GCC with -fpatchable-function-entry.

+. Depend on DYNAMIC_FTRACE.

+. Use 3 nops for stub in module and vmlinux, smaller than old one.

+. Simplify ftrace_regs_caller/ftrace_caller handling, especially
on MIPS O32.

+. No adjustment on sp, so callee(the traced function) can get caller's
sp, very friendly to livepatch.

Signed-off-by: Huang Pei <huangpei@loongson.cn>
---
 arch/mips/Kconfig               |   4 +
 arch/mips/Makefile              |  16 +-
 arch/mips/include/asm/ftrace.h  |   9 ++
 arch/mips/kernel/Makefile       |   5 +-
 arch/mips/kernel/entry-ftrace.S | 188 ++++++++++++++++++++++
 arch/mips/kernel/ftrace.c       | 271 ++++++++++++++++++++++++++++++++
 6 files changed, 488 insertions(+), 5 deletions(-)
 create mode 100644 arch/mips/kernel/entry-ftrace.S
 create mode 100644 arch/mips/kernel/ftrace.c

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 5741dae35b74..dafc65b0226a 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -56,6 +56,10 @@ config MIPS
 	select HAVE_DEBUG_STACKOVERFLOW
 	select HAVE_DMA_CONTIGUOUS
 	select HAVE_DYNAMIC_FTRACE
+	select HAVE_DYNAMIC_FTRACE_WITH_REGS \
+		if $(cc-option, -fpatchable-function-entry=3)
+	select HAVE_KPROBES_ON_FTRACE \
+		if $(cc-option, -fpatchable-function-entry=3)
 	select HAVE_EBPF_JIT if 64BIT && !CPU_MICROMIPS && TARGET_ISA_REV >= 2
 	select HAVE_EXIT_THREAD
 	select HAVE_FAST_GUP
diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index 0d0f29d662c9..1d6553f0623e 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -56,13 +56,16 @@ ifneq ($(SUBARCH),$(ARCH))
   endif
 endif
 
-ifdef CONFIG_FUNCTION_GRAPH_TRACER
-  ifndef KBUILD_MCOUNT_RA_ADDRESS
-    ifeq ($(call cc-option-yn,-mmcount-ra-address), y)
-      cflags-y += -mmcount-ra-address -DKBUILD_MCOUNT_RA_ADDRESS
+ifndef CONFIG_DYNMAIC_FTRACE_WITH_REGS
+  ifdef CONFIG_FUNCTION_GRAPH_TRACER
+    ifndef KBUILD_MCOUNT_RA_ADDRESS
+      ifeq ($(call cc-option-yn,-mmcount-ra-address), y)
+        cflags-y += -mmcount-ra-address -DKBUILD_MCOUNT_RA_ADDRESS
+      endif
     endif
   endif
 endif
+
 cflags-y += $(call cc-option, -mno-check-zero-division)
 
 ifdef CONFIG_32BIT
@@ -293,6 +296,11 @@ ifdef CONFIG_64BIT
 bootvars-y	+= ADDR_BITS=64
 endif
 
+ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY
+	CC_FLAGS_FTRACE := -fpatchable-function-entry=3
+endif
+
 # This is required to get dwarf unwinding tables into .debug_frame
 # instead of .eh_frame so we don't discard them.
 KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
diff --git a/arch/mips/include/asm/ftrace.h b/arch/mips/include/asm/ftrace.h
index b463f2aa5a61..5508890566ea 100644
--- a/arch/mips/include/asm/ftrace.h
+++ b/arch/mips/include/asm/ftrace.h
@@ -14,6 +14,7 @@
 
 #define MCOUNT_ADDR ((unsigned long)(_mcount))
 #define MCOUNT_INSN_SIZE 4		/* sizeof mcount call */
+#define FTRACE_IP_EXTENSION	(2 * MCOUNT_INSN_SIZE)
 
 #ifndef __ASSEMBLY__
 extern void _mcount(void);
@@ -87,4 +88,12 @@ struct dyn_arch_ftrace {
 #endif /*  CONFIG_DYNAMIC_FTRACE */
 #endif /* __ASSEMBLY__ */
 #endif /* CONFIG_FUNCTION_TRACER */
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#define ARCH_SUPPORTS_FTRACE_OPS	1
+struct dyn_ftrace;
+int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec);
+#define ftrace_init_nop ftrace_init_nop
+#endif
+
 #endif /* _ASM_MIPS_FTRACE_H */
diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
index 3e7b0ee54cfb..7b07d80aadd5 100644
--- a/arch/mips/kernel/Makefile
+++ b/arch/mips/kernel/Makefile
@@ -21,6 +21,7 @@ CFLAGS_REMOVE_ftrace-mcount.o = $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_early_printk.o = $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_perf_event.o = $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_perf_event_mipsxx.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE)
 endif
 
 obj-$(CONFIG_CEVT_BCM1480)	+= cevt-bcm1480.o
@@ -39,7 +40,9 @@ obj-$(CONFIG_DEBUG_FS)		+= segment.o
 obj-$(CONFIG_STACKTRACE)	+= stacktrace.o
 obj-$(CONFIG_MODULES)		+= module.o
 
-obj-$(CONFIG_FUNCTION_TRACER)	+= mcount.o ftrace-mcount.o
+ft-$(CONFIG_FUNCTION_TRACER)	+= mcount.o ftrace-mcount.o
+ft-$(CONFIG_DYNAMIC_FTRACE_WITH_REGS)	:= entry-ftrace.o ftrace.o
+obj-y				+= $(ft-y)
 
 sw-y				:= r4k_switch.o
 sw-$(CONFIG_CPU_R3000)		:= r2300_switch.o
diff --git a/arch/mips/kernel/entry-ftrace.S b/arch/mips/kernel/entry-ftrace.S
new file mode 100644
index 000000000000..c920a568ce77
--- /dev/null
+++ b/arch/mips/kernel/entry-ftrace.S
@@ -0,0 +1,188 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * arch/mips/kernel/entry_ftrace.S
+ *
+ * Copyright (C) 2021 Loongson Corp
+ * Author: Huang Pei <huangpei@loongson.cn>
+ */
+
+#include <asm/export.h>
+#include <asm/regdef.h>
+#include <asm/stackframe.h>
+
+/*
+ * ftrace_regs_caller() is the function that replaces _mcount() when ftrace
+ * is active.
+ *
+ * we arrive here after a function A calls function B, and B is what we
+ * are tracing for. When we enter, sp points to A's stack frame, B has not
+ * yet had a chance to allocate one yet. (This is different from -pg case
+ * , in which the B's stack is allocated))
+
+ * when ftrace initialized, it replace three nops from all function with 
+ * "lui + nop + move"
+ * B:
+ *	lui	at, %hi(ftrace_regs_caller)
+ *	nop
+ *	li	t0, 0
+ * #	B's real start
+ *
+ * at B's entry, when tracing enabled, replace the 'nop' with 'jalr'
+ *
+ * #	B's entry, three nop for both in vmlinux and in kernel modules
+ * B:
+ *	lui	at, %hi(ftrace_regs_caller)
+ *	jalr	at, at
+ *	move	t0, zero
+ * #	B's real start
+ *
+ * if set t0 to 1, then calling ftrace_regs_caller with partial regs saved
+ *
+ * B:
+ *	lui	at, %hi(ftrace_regs_caller)
+ *	jalr	at, at
+ *	li	t0, 1
+ * #	B's real start
+ *
+ * we make ftrace_regs_caller 64KB aligned, when entring ftrace_regs_caller
+ * AT points to the return address to B, and ra points to return address
+ * to A,
+ *
+ * if patched to new funcition, then clobbered the first real instruction
+ *
+ * B:
+ *	lui	at, %hi(new_B)
+ *	addiu	at, at, %lo(new_B)
+ *	jr	at
+ * #	B's real start, now clobbered with zero
+ *	nop
+ *
+ */
+	.text
+	.set push
+	.set noreorder
+	.set noat
+	.align 16
+NESTED(ftrace_regs_caller, PT_SIZE, ra)
+	PTR_ADDIU	sp, sp, -PT_SIZE
+	.globl ftrace_caller
+ftrace_caller:
+#ifdef CONFIG_64BIT
+	PTR_S	a4, PT_R8(sp)
+	PTR_S	a5, PT_R9(sp)
+	PTR_S	a6, PT_R10(sp)
+	PTR_S	a7, PT_R11(sp)
+#endif
+	PTR_S	a0, PT_R4(sp)
+	PTR_S	a1, PT_R5(sp)
+	PTR_S	a2, PT_R6(sp)
+
+	bnez	t0, 1f
+	PTR_S	a3, PT_R7(sp)
+
+	PTR_S	t0, PT_R12(sp)
+	PTR_S	t1, PT_R13(sp)
+	PTR_S	t2, PT_R14(sp)
+	PTR_S	t3, PT_R15(sp)
+
+	PTR_S	s0, PT_R16(sp)
+	PTR_S	s1, PT_R17(sp)
+	PTR_S	s2, PT_R18(sp)
+	PTR_S	s3, PT_R19(sp)
+
+	PTR_S	s4, PT_R20(sp)
+	PTR_S	s5, PT_R21(sp)
+	PTR_S	s6, PT_R22(sp)
+	PTR_S	s7, PT_R23(sp)
+
+
+	PTR_S	t8, PT_R24(sp)
+	PTR_S	t9, PT_R25(sp)
+	PTR_S	s8, PT_R30(sp)
+	PTR_S	gp, PT_R28(sp)
+
+	PTR_S	AT, PT_R1(sp)
+1:
+	PTR_LA	t0, PT_SIZE(sp)
+	PTR_S	AT, PT_R0(sp)	//R0 for expected epc
+	PTR_S	t0, PT_R29(sp)
+
+	PTR_S	ra, PT_R31(sp)
+	PTR_S	AT, PT_EPC(sp)		//PT_EPC maybe changed by kprobe handler
+
+	END(ftrace_regs_caller)
+
+ftrace_common:
+	PTR_ADDIU	a0, AT, -12	//a0 points to B's entry address
+	move	a1, ra			//a1 points to return address to A
+	PTR_L	a2, function_trace_op	//a2 points to function_trace op
+
+	.globl	ftrace_call
+ftrace_call:
+	jal	ftrace_stub
+	move	a3, sp			//a3 point to pt_regs
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+	.globl ftrace_graph_call
+ftrace_graph_call:
+	nop
+	nop
+#endif
+
+ftrace_common_return:
+	PTR_L	AT, PT_R31(sp)
+ftrace_graph_return:
+	PTR_L	ra, PT_EPC(sp)
+	PTR_L	a0, PT_R4(sp)
+	PTR_L	a1, PT_R5(sp)
+	PTR_L	a2, PT_R6(sp)
+	PTR_L	a3, PT_R7(sp)
+#ifdef CONFIG_64BIT
+	PTR_L	a4, PT_R8(sp)
+	PTR_L	a5, PT_R9(sp)
+	PTR_L	a6, PT_R10(sp)
+	PTR_L	a7, PT_R11(sp)
+#endif
+	PTR_ADDIU	sp, sp, PT_SIZE	//retore stack frame
+	jr	ra
+	move	ra, AT
+
+
+	.globl ftrace_stub
+ftrace_stub:
+	jr	ra
+	nop
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+	.globl	ftrace_graph_caller
+ftrace_graph_caller:
+	PTR_L	a0, PT_R31(sp)
+	PTR_L	a1, PT_EPC(sp)
+	jal	prepare_ftrace_return
+	PTR_ADDIU	a2, sp, PT_SIZE
+
+	b	ftrace_graph_return
+	move	AT, v0
+
+
+	.align	2
+	.globl	return_to_handler
+return_to_handler:
+	PTR_SUBU	sp, PT_SIZE
+	PTR_S	v0, PT_R2(sp)
+
+	PTR_S	v1, PT_R3(sp)
+	jal	ftrace_return_to_handler
+	PTR_LA	a0, PT_SIZE(sp)
+
+	/* restore the real parent address: v0 -> ra */
+	move	ra, v0
+
+	PTR_L	v0, PT_R2(sp)
+	PTR_L	v1, PT_R3(sp)
+	jr	ra
+	 PTR_ADDIU	sp, PT_SIZE
+
+	.set at
+	.set reorder
+#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
new file mode 100644
index 000000000000..c3baa2e692c1
--- /dev/null
+++ b/arch/mips/kernel/ftrace.c
@@ -0,0 +1,271 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * arch/mips/kernel/ftrace.c
+ *
+ * Copyright (C) 2021 Loongson Limited Corp.
+ * Author: Huang Pei <huangpei@loongson.cn>
+ */
+
+#include <linux/ftrace.h>
+#include <linux/kprobes.h>
+#include <linux/module.h>
+#include <linux/swab.h>
+#include <linux/uaccess.h>
+
+#include <asm/ftrace.h>
+#include <asm/asm.h>
+#include <asm/asm-offsets.h>
+#include <asm/cacheflush.h>
+#include <asm/uasm.h>
+
+
+#define INSN_NOP	0x00000000	/* nop */
+#define INSN_JALR_AT2	0x00200809 	/* jalr at, at */
+#define INSN_LI_0	0x240c0000	/* li t0, 0 */
+#define INSN_LI_1	0x240c0001	/* li t0, 1 */
+#define FTRACE_CALL_IP	((unsigned long)(&ftrace_call))
+#define JAL		0x0c000000	/* jump & link: ip --> ra, jump to target */
+#define ADDR_MASK	0x03ffffff	/*  op_code|addr : 31...26|25 ....0 */
+#define INSN_JAL(addr)	\
+	((unsigned int)(JAL | (((addr) >> 2) & ADDR_MASK)))
+
+extern void ftrace_graph_call(void);
+
+static unsigned int insn_lui __read_mostly;
+
+/* Arch override because MIPS doesn't need to run this from stop_machine() */
+void arch_ftrace_update_code(int command)
+{
+	ftrace_modify_all_code(command);
+}
+
+static int ftrace_modify_code(unsigned long ip, unsigned int new_code)
+{
+	int faulted;
+	mm_segment_t old_fs;
+
+	/* *(unsigned int *)ip = new_code; */
+	safe_store_code(new_code, ip, faulted);
+
+	if (unlikely(faulted))
+		return -EFAULT;
+
+	old_fs = get_fs();
+	set_fs(KERNEL_DS);
+	flush_icache_range(ip, ip + 8);
+	set_fs(old_fs);
+
+	return 0;
+}
+
+int ftrace_update_ftrace_func(ftrace_func_t func)
+{
+	unsigned int new;
+
+	new = INSN_JAL((unsigned long)func);
+
+	return ftrace_modify_code(FTRACE_CALL_IP, new);
+}
+
+/*
+ * enable tracing by replacing the middle nop with jalr, like
+ *
+ * lui	at, %hi(ftrace_regs_all)
+ * jalr	at, at
+ * li	t0, 0
+ */
+int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
+{
+	unsigned long ip = rec->ip;
+
+	ftrace_modify_code(ip + 4, INSN_JALR_AT2);
+	return 0;
+}
+
+/*
+ * disable recording regs by replacing
+ *
+ *  li	t0, 0
+ *
+ * with
+ *
+ *  li, t0, 1
+ *
+ * or vesa
+ */
+int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
+			unsigned long addr)
+{
+	unsigned long ip = rec->ip;
+
+	if (abs(old_addr - addr) == 4) {
+		if (addr == (unsigned long)ftrace_regs_caller)
+			return ftrace_modify_code(ip + 4, INSN_LI_0);
+
+		if (addr == (unsigned long)ftrace_caller)
+			return ftrace_modify_code(ip + 4, INSN_LI_1);
+
+	}
+
+	/* we do not support direct call or trampoline now */
+
+	return -1;
+
+}
+
+/*
+ * replace all three nop at the entry with
+ *
+ * lui	at, %hi(ftrace_regs_all)
+ * nop
+ * li	t0, 1
+ */
+
+int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec)
+{
+	unsigned long ip = rec->ip;
+
+	ftrace_modify_code(ip, insn_lui);
+
+	ftrace_modify_code(ip + 8, INSN_LI_1);
+	return 0;
+}
+
+
+
+/*
+ * disable tracing by replacing
+ *
+ *  jalr at, at
+ *
+ * with
+ *
+ *  nop
+ *
+ */
+
+int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
+			unsigned long addr)
+
+{
+	unsigned int new = INSN_NOP;
+	unsigned long ip = rec->ip + 4;
+
+	return ftrace_modify_code(ip, new);
+}
+
+int __init ftrace_dyn_arch_init(void)
+{
+	u32 *buf;
+	int reg;
+
+	reg = 1;
+	/* lui at, %hi(ftrace_regs_all) */
+	buf = (u32*)&insn_lui;
+	uasm_i_lui(&buf, reg, uasm_rel_hi((long)ftrace_regs_caller));
+
+	return 0;
+}
+
+void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
+			   struct ftrace_ops *ops, struct pt_regs *regs)
+{
+	struct kprobe *p;
+	struct kprobe_ctlblk *kcb;
+	unsigned long orig_ip;
+
+	p = get_kprobe((kprobe_opcode_t *)ip);
+	if (unlikely(!p) || kprobe_disabled(p))
+		return;
+
+	kcb = get_kprobe_ctlblk();
+	if (kprobe_running()) {
+		kprobes_inc_nmissed_count(p);
+	} else {
+		/*
+		 * pre_handler need epc point to the kprobe
+		 *
+		 */
+		orig_ip = instruction_pointer(regs);
+		instruction_pointer_set(regs, (unsigned long)p->addr);
+		__this_cpu_write(current_kprobe, p);
+		kcb->kprobe_status = KPROBE_HIT_ACTIVE;
+		if (!p->pre_handler || !p->pre_handler(p, regs)) {
+			/*
+			 * Emulate singlestep (and also recover regs->nip)
+			 * as if there is a nop
+			 */
+			if (unlikely(p->post_handler)) {
+				kcb->kprobe_status = KPROBE_HIT_SSDONE;
+				if ((unsigned long)p->addr == (ip + 4))
+
+					ip = (unsigned long)p->addr + 8;
+				else
+					ip = (unsigned long)p->addr + 4;
+				instruction_pointer_set(regs, ip);
+				p->post_handler(p, regs, 0);
+			}
+		}
+		instruction_pointer_set(regs, orig_ip);
+		/*
+		 * If pre_handler returns !0, we have to
+		 * skip emulating post_handler.
+		 */
+		__this_cpu_write(current_kprobe, NULL);
+	}
+
+}
+
+int arch_prepare_kprobe_ftrace(struct kprobe *p)
+{
+	p->ainsn.insn = NULL;
+	return 0;
+}
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+unsigned long prepare_ftrace_return(unsigned long parent, unsigned long self_ra,
+			   unsigned long fp)
+{
+	unsigned long return_hooker = (unsigned long)&return_to_handler;
+
+	if (unlikely(ftrace_graph_is_dead()))
+		goto out;
+
+	if (unlikely(atomic_read(&current->tracing_graph_pause)))
+		goto out;
+
+	self_ra -=  8;
+	if (!function_graph_enter(parent, self_ra, fp, NULL))
+		parent = return_hooker;
+out:
+	return parent;
+}
+
+/*
+ * Turn on/off the call to ftrace_graph_caller() in ftrace_caller()
+ * depending on @enable.
+ */
+static int ftrace_modify_graph_caller(bool enable)
+{
+	unsigned long pc = (unsigned long)ftrace_graph_call;
+	unsigned new;
+
+	if (enable)
+		new = INSN_JAL((unsigned long)ftrace_graph_caller);
+	else
+		new = INSN_NOP;
+
+	return ftrace_modify_code(pc, new);
+}
+
+int ftrace_enable_ftrace_graph_caller(void)
+{
+	return ftrace_modify_graph_caller(true);
+}
+
+int ftrace_disable_ftrace_graph_caller(void)
+{
+	return ftrace_modify_graph_caller(false);
+}
+
+#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/6] ftrace: introduce FTRACE_IP_EXTENSION
  2021-03-13  6:41 ` [PATCH 5/6] ftrace: introduce FTRACE_IP_EXTENSION Huang Pei
@ 2021-03-13  9:26   ` Sergei Shtylyov
  0 siblings, 0 replies; 18+ messages in thread
From: Sergei Shtylyov @ 2021-03-13  9:26 UTC (permalink / raw)
  To: Huang Pei, Thomas Bogendoerfer, ambrosehua
  Cc: Bibo Mao, linux-mips, linux-arch, linux-mm, Jiaxun Yang,
	Paul Burton, Li Xuefeng, Yang Tiezhu, Gao Juxin, Huacai Chen,
	Jinyang He, Maciej W . Rozycki, Steven Rostedt, Jisheng Zhang,
	Masami Hiramatsu

Hello!

On 3/13/21 9:41 AM, Huang Pei wrote:

> From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> 
> On some architectures, the DYNAMIC_FTRACE_WITH_REGS is implemented by
> gcc's -fpatchable-function-entry option. Take arm64 for example, arm64
> makes use of GCC -fpatchable-function-entry=2 option to insert two
> nops. When the function is traced, the first nop will be modified to
> the LR saver, then the second nop to "bl <ftrace-entry>". we need to
> update ftrace_location() to recognise these two instructions  as being
> part of ftrace. To do this, we introduce FTRACE_IP_EXTENSION to let
> ftrace_location search IP, IP + FTRACE_IP_EXTENSION range.
> 
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  include/linux/ftrace.h | 4 ++++
>  kernel/trace/ftrace.c  | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 1bd3a0356ae4..c1e1fbde8a04 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -20,6 +20,10 @@
>  
>  #include <asm/ftrace.h>
>  
> +#ifndef FTRACE_IP_EXTENSION
> +#define  FTRACE_IP_EXTENSION 0

   Inconsistent spacing between #<directive> and the value?

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/6] MIPS: add DYNAMIC_FTRACE_WITH_REGS and KPROBES_ON_FTACE
  2021-03-13  6:41 ` [PATCH 6/6] MIPS: add DYNAMIC_FTRACE_WITH_REGS and KPROBES_ON_FTACE Huang Pei
@ 2021-03-13 21:37     ` kernel test robot
  0 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2021-03-13 21:37 UTC (permalink / raw)
  To: Huang Pei, Thomas Bogendoerfer, ambrosehua
  Cc: kbuild-all, Bibo Mao, linux-mips, linux-arch, linux-mm,
	Jiaxun Yang, Paul Burton, Li Xuefeng, Yang Tiezhu

[-- Attachment #1: Type: text/plain, Size: 3964 bytes --]

Hi Huang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on trace/for-next linus/master v5.12-rc2 next-20210312]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Huang-Pei/MIPS-replace-pg-with-CC_FLAGS_FTRACE/20210313-154234
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 8bcfdd7cad3dffdd340f9a79098cbf331eb2cd53
config: mips-allyesconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/c81a1cbcfd0d4b65f668fd824466b9bce02cee74
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Huang-Pei/MIPS-replace-pg-with-CC_FLAGS_FTRACE/20210313-154234
        git checkout c81a1cbcfd0d4b65f668fd824466b9bce02cee74
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> arch/mips/kernel/ftrace.c:170:6: error: conflicting types for 'kprobe_ftrace_handler'
     170 | void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
         |      ^~~~~~~~~~~~~~~~~~~~~
   In file included from arch/mips/kernel/ftrace.c:10:
   include/linux/kprobes.h:362:13: note: previous declaration of 'kprobe_ftrace_handler' was here
     362 | extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
         |             ^~~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/ftrace.c:226:15: warning: no previous prototype for 'prepare_ftrace_return' [-Wmissing-prototypes]
     226 | unsigned long prepare_ftrace_return(unsigned long parent, unsigned long self_ra,
         |               ^~~~~~~~~~~~~~~~~~~~~


vim +/kprobe_ftrace_handler +170 arch/mips/kernel/ftrace.c

   169	
 > 170	void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
   171				   struct ftrace_ops *ops, struct pt_regs *regs)
   172	{
   173		struct kprobe *p;
   174		struct kprobe_ctlblk *kcb;
   175		unsigned long orig_ip;
   176	
   177		p = get_kprobe((kprobe_opcode_t *)ip);
   178		if (unlikely(!p) || kprobe_disabled(p))
   179			return;
   180	
   181		kcb = get_kprobe_ctlblk();
   182		if (kprobe_running()) {
   183			kprobes_inc_nmissed_count(p);
   184		} else {
   185			/*
   186			 * pre_handler need epc point to the kprobe
   187			 *
   188			 */
   189			orig_ip = instruction_pointer(regs);
   190			instruction_pointer_set(regs, (unsigned long)p->addr);
   191			__this_cpu_write(current_kprobe, p);
   192			kcb->kprobe_status = KPROBE_HIT_ACTIVE;
   193			if (!p->pre_handler || !p->pre_handler(p, regs)) {
   194				/*
   195				 * Emulate singlestep (and also recover regs->nip)
   196				 * as if there is a nop
   197				 */
   198				if (unlikely(p->post_handler)) {
   199					kcb->kprobe_status = KPROBE_HIT_SSDONE;
   200					if ((unsigned long)p->addr == (ip + 4))
   201	
   202						ip = (unsigned long)p->addr + 8;
   203					else
   204						ip = (unsigned long)p->addr + 4;
   205					instruction_pointer_set(regs, ip);
   206					p->post_handler(p, regs, 0);
   207				}
   208			}
   209			instruction_pointer_set(regs, orig_ip);
   210			/*
   211			 * If pre_handler returns !0, we have to
   212			 * skip emulating post_handler.
   213			 */
   214			__this_cpu_write(current_kprobe, NULL);
   215		}
   216	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 69954 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/6] MIPS: add DYNAMIC_FTRACE_WITH_REGS and KPROBES_ON_FTACE
@ 2021-03-13 21:37     ` kernel test robot
  0 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2021-03-13 21:37 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4061 bytes --]

Hi Huang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on trace/for-next linus/master v5.12-rc2 next-20210312]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Huang-Pei/MIPS-replace-pg-with-CC_FLAGS_FTRACE/20210313-154234
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 8bcfdd7cad3dffdd340f9a79098cbf331eb2cd53
config: mips-allyesconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/c81a1cbcfd0d4b65f668fd824466b9bce02cee74
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Huang-Pei/MIPS-replace-pg-with-CC_FLAGS_FTRACE/20210313-154234
        git checkout c81a1cbcfd0d4b65f668fd824466b9bce02cee74
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> arch/mips/kernel/ftrace.c:170:6: error: conflicting types for 'kprobe_ftrace_handler'
     170 | void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
         |      ^~~~~~~~~~~~~~~~~~~~~
   In file included from arch/mips/kernel/ftrace.c:10:
   include/linux/kprobes.h:362:13: note: previous declaration of 'kprobe_ftrace_handler' was here
     362 | extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
         |             ^~~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/ftrace.c:226:15: warning: no previous prototype for 'prepare_ftrace_return' [-Wmissing-prototypes]
     226 | unsigned long prepare_ftrace_return(unsigned long parent, unsigned long self_ra,
         |               ^~~~~~~~~~~~~~~~~~~~~


vim +/kprobe_ftrace_handler +170 arch/mips/kernel/ftrace.c

   169	
 > 170	void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
   171				   struct ftrace_ops *ops, struct pt_regs *regs)
   172	{
   173		struct kprobe *p;
   174		struct kprobe_ctlblk *kcb;
   175		unsigned long orig_ip;
   176	
   177		p = get_kprobe((kprobe_opcode_t *)ip);
   178		if (unlikely(!p) || kprobe_disabled(p))
   179			return;
   180	
   181		kcb = get_kprobe_ctlblk();
   182		if (kprobe_running()) {
   183			kprobes_inc_nmissed_count(p);
   184		} else {
   185			/*
   186			 * pre_handler need epc point to the kprobe
   187			 *
   188			 */
   189			orig_ip = instruction_pointer(regs);
   190			instruction_pointer_set(regs, (unsigned long)p->addr);
   191			__this_cpu_write(current_kprobe, p);
   192			kcb->kprobe_status = KPROBE_HIT_ACTIVE;
   193			if (!p->pre_handler || !p->pre_handler(p, regs)) {
   194				/*
   195				 * Emulate singlestep (and also recover regs->nip)
   196				 * as if there is a nop
   197				 */
   198				if (unlikely(p->post_handler)) {
   199					kcb->kprobe_status = KPROBE_HIT_SSDONE;
   200					if ((unsigned long)p->addr == (ip + 4))
   201	
   202						ip = (unsigned long)p->addr + 8;
   203					else
   204						ip = (unsigned long)p->addr + 4;
   205					instruction_pointer_set(regs, ip);
   206					p->post_handler(p, regs, 0);
   207				}
   208			}
   209			instruction_pointer_set(regs, orig_ip);
   210			/*
   211			 * If pre_handler returns !0, we have to
   212			 * skip emulating post_handler.
   213			 */
   214			__this_cpu_write(current_kprobe, NULL);
   215		}
   216	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 69954 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/6] MIPS: replace -pg with CC_FLAGS_FTRACE
  2021-03-13  6:41 ` [PATCH 1/6] MIPS: replace -pg with CC_FLAGS_FTRACE Huang Pei
@ 2021-03-25 19:38   ` Steven Rostedt
  2021-03-26  1:48     ` Huang Pei
  0 siblings, 1 reply; 18+ messages in thread
From: Steven Rostedt @ 2021-03-25 19:38 UTC (permalink / raw)
  To: Huang Pei
  Cc: Thomas Bogendoerfer, ambrosehua, Bibo Mao, linux-mips,
	linux-arch, linux-mm, Jiaxun Yang, Paul Burton, Li Xuefeng,
	Yang Tiezhu, Gao Juxin, Huacai Chen, Jinyang He,
	Maciej W . Rozycki, Jisheng Zhang, Masami Hiramatsu

On Sat, 13 Mar 2021 14:41:44 +0800
Huang Pei <huangpei@loongson.cn> wrote:


Even simple changes require change logs. For example:

"Enabling ftrace may require more than just the -pg flags today. As ftrace
enables more flags, use the $(CC_FLAGS_FTRACE) in the make file instead of
hard coding "-pg"."

Other than that:

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

-- Steve


> Signed-off-by: Huang Pei <huangpei@loongson.cn>
> ---
>  arch/mips/boot/compressed/Makefile | 2 +-
>  arch/mips/kernel/Makefile          | 8 ++++----
>  arch/mips/vdso/Makefile            | 4 ++--
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/mips/boot/compressed/Makefile b/arch/mips/boot/compressed/Makefile
> index d66511825fe1..8fc9ceeec709 100644
> --- a/arch/mips/boot/compressed/Makefile
> +++ b/arch/mips/boot/compressed/Makefile
> @@ -18,7 +18,7 @@ include $(srctree)/arch/mips/Kbuild.platforms
>  BOOT_HEAP_SIZE := 0x400000
>  
>  # Disable Function Tracer
> -KBUILD_CFLAGS := $(filter-out -pg, $(KBUILD_CFLAGS))
> +KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE), $(KBUILD_CFLAGS))
>  
>  KBUILD_CFLAGS := $(filter-out -fstack-protector, $(KBUILD_CFLAGS))
>  
> diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
> index 2a05b923f579..33e31ea10234 100644
> --- a/arch/mips/kernel/Makefile
> +++ b/arch/mips/kernel/Makefile
> @@ -17,10 +17,10 @@ obj-y		+= cpu-probe.o
>  endif
>  
>  ifdef CONFIG_FUNCTION_TRACER
> -CFLAGS_REMOVE_ftrace.o = -pg
> -CFLAGS_REMOVE_early_printk.o = -pg
> -CFLAGS_REMOVE_perf_event.o = -pg
> -CFLAGS_REMOVE_perf_event_mipsxx.o = -pg
> +CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE)
> +CFLAGS_REMOVE_early_printk.o = $(CC_FLAGS_FTRACE)
> +CFLAGS_REMOVE_perf_event.o = $(CC_FLAGS_FTRACE)
> +CFLAGS_REMOVE_perf_event_mipsxx.o = $(CC_FLAGS_FTRACE)
>  endif
>  
>  obj-$(CONFIG_CEVT_BCM1480)	+= cevt-bcm1480.o
> diff --git a/arch/mips/vdso/Makefile b/arch/mips/vdso/Makefile
> index 5810cc12bc1d..f21cf88f7ae3 100644
> --- a/arch/mips/vdso/Makefile
> +++ b/arch/mips/vdso/Makefile
> @@ -49,7 +49,7 @@ CFLAGS_vgettimeofday-o32.o = -include $(srctree)/$(src)/config-n32-o32-env.c -in
>  CFLAGS_vgettimeofday-n32.o = -include $(srctree)/$(src)/config-n32-o32-env.c -include $(c-gettimeofday-y)
>  endif
>  
> -CFLAGS_REMOVE_vgettimeofday.o = -pg
> +CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE)
>  
>  ifdef CONFIG_MIPS_DISABLE_VDSO
>    ifndef CONFIG_MIPS_LD_CAN_LINK_VDSO
> @@ -63,7 +63,7 @@ ldflags-y := -Bsymbolic --no-undefined -soname=linux-vdso.so.1 \
>  	$(filter -E%,$(KBUILD_CFLAGS)) -nostdlib -shared \
>  	-G 0 --eh-frame-hdr --hash-style=sysv --build-id=sha1 -T
>  
> -CFLAGS_REMOVE_vdso.o = -pg
> +CFLAGS_REMOVE_vdso.o = $(CC_FLAGS_FTRACE)
>  
>  GCOV_PROFILE := n
>  UBSAN_SANITIZE := n


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] MIPS: move FTRACE_SYSCALLS from ftrace.c into syscall.c
  2021-03-13  6:41 ` [PATCH 2/6] MIPS: move FTRACE_SYSCALLS from ftrace.c into syscall.c Huang Pei
@ 2021-03-25 19:38   ` Steven Rostedt
  2021-03-26 14:01     ` Huang Pei
  0 siblings, 1 reply; 18+ messages in thread
From: Steven Rostedt @ 2021-03-25 19:38 UTC (permalink / raw)
  To: Huang Pei
  Cc: Thomas Bogendoerfer, ambrosehua, Bibo Mao, linux-mips,
	linux-arch, linux-mm, Jiaxun Yang, Paul Burton, Li Xuefeng,
	Yang Tiezhu, Gao Juxin, Huacai Chen, Jinyang He,
	Maciej W . Rozycki, Jisheng Zhang, Masami Hiramatsu

On Sat, 13 Mar 2021 14:41:45 +0800
Huang Pei <huangpei@loongson.cn> wrote:

Why?

-- Steve

> Signed-off-by: Huang Pei <huangpei@loongson.cn>
> ---
>  arch/mips/kernel/Makefile  |  1 -
>  arch/mips/kernel/ftrace.c  | 33 ---------------------------------
>  arch/mips/kernel/syscall.c | 32 ++++++++++++++++++++++++++++++++
>  3 files changed, 32 insertions(+), 34 deletions(-)


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/6] MIPS: prepare for new ftrace implementation
  2021-03-13  6:41 ` [PATCH 3/6] MIPS: prepare for new ftrace implementation Huang Pei
@ 2021-03-25 19:39   ` Steven Rostedt
  0 siblings, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2021-03-25 19:39 UTC (permalink / raw)
  To: Huang Pei
  Cc: Thomas Bogendoerfer, ambrosehua, Bibo Mao, linux-mips,
	linux-arch, linux-mm, Jiaxun Yang, Paul Burton, Li Xuefeng,
	Yang Tiezhu, Gao Juxin, Huacai Chen, Jinyang He,
	Maciej W . Rozycki, Jisheng Zhang, Masami Hiramatsu

On Sat, 13 Mar 2021 14:41:46 +0800
Huang Pei <huangpei@loongson.cn> wrote:

> No function change

Change logs require rationale for the change. Why is this being done? Just
saying "No function change" is not informative at all.

-- Steve


> 
> Signed-off-by: Huang Pei <huangpei@loongson.cn>
> ---
>  arch/mips/kernel/Makefile                      | 4 ++--
>  arch/mips/kernel/{ftrace.c => ftrace-mcount.c} | 0
>  2 files changed, 2 insertions(+), 2 deletions(-)
>  rename arch/mips/kernel/{ftrace.c => ftrace-mcount.c} (100%)
> 
> diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
> index 5b2b551058ac..3e7b0ee54cfb 100644
> --- a/arch/mips/kernel/Makefile
> +++ b/arch/mips/kernel/Makefile
> @@ -17,7 +17,7 @@ obj-y		+= cpu-probe.o
>  endif
>  
>  ifdef CONFIG_FUNCTION_TRACER
> -CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE)
> +CFLAGS_REMOVE_ftrace-mcount.o = $(CC_FLAGS_FTRACE)
>  CFLAGS_REMOVE_early_printk.o = $(CC_FLAGS_FTRACE)
>  CFLAGS_REMOVE_perf_event.o = $(CC_FLAGS_FTRACE)
>  CFLAGS_REMOVE_perf_event_mipsxx.o = $(CC_FLAGS_FTRACE)
> @@ -39,7 +39,7 @@ obj-$(CONFIG_DEBUG_FS)		+= segment.o
>  obj-$(CONFIG_STACKTRACE)	+= stacktrace.o
>  obj-$(CONFIG_MODULES)		+= module.o
>  
> -obj-$(CONFIG_FUNCTION_TRACER)	+= mcount.o ftrace.o
> +obj-$(CONFIG_FUNCTION_TRACER)	+= mcount.o ftrace-mcount.o
>  
>  sw-y				:= r4k_switch.o
>  sw-$(CONFIG_CPU_R3000)		:= r2300_switch.o
> diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace-mcount.c
> similarity index 100%
> rename from arch/mips/kernel/ftrace.c
> rename to arch/mips/kernel/ftrace-mcount.c


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/6] kprobes/ftrace: Use ftrace_location() when [dis]arming probes
  2021-03-13  6:41 ` [PATCH 4/6] kprobes/ftrace: Use ftrace_location() when [dis]arming probes Huang Pei
@ 2021-03-25 19:44   ` Steven Rostedt
  2021-03-26 14:12     ` Huang Pei
  0 siblings, 1 reply; 18+ messages in thread
From: Steven Rostedt @ 2021-03-25 19:44 UTC (permalink / raw)
  To: Huang Pei
  Cc: Thomas Bogendoerfer, ambrosehua, Bibo Mao, linux-mips,
	linux-arch, linux-mm, Jiaxun Yang, Paul Burton, Li Xuefeng,
	Yang Tiezhu, Gao Juxin, Huacai Chen, Jinyang He,
	Maciej W . Rozycki, Jisheng Zhang, Masami Hiramatsu,
	Naveen N . Rao

On Sat, 13 Mar 2021 14:41:47 +0800
Huang Pei <huangpei@loongson.cn> wrote:

> From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> 

Looks like this was sent before, but was missing the proper authorship
(which is not Jisheng).

   https://lore.kernel.org/linux-arm-kernel/20191225173219.4f9db436@xhacker.debian/

-- Steve


> Ftrace location could include more than a single instruction in case
> of some architectures (powerpc64, for now). In this case, kprobe is
> permitted on any of those instructions, and uses ftrace infrastructure
> for functioning.
> 
> However, [dis]arm_kprobe_ftrace() uses the kprobe address when setting
> up ftrace filter IP. This won't work if the address points to any
> instruction apart from the one that has a branch to _mcount(). To
> resolve this, have [dis]arm_kprobe_ftrace() use ftrace_function() to
> identify the filter IP.
> 
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  kernel/kprobes.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 41fdbb7953c6..66ee28b071c2 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1045,9 +1045,10 @@ static int prepare_kprobe(struct kprobe *p)
>  static int __arm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
>  			       int *cnt)
>  {
> +	unsigned long ftrace_ip = ftrace_location((unsigned long)p->addr);
>  	int ret = 0;
>  
> -	ret = ftrace_set_filter_ip(ops, (unsigned long)p->addr, 0, 0);
> +	ret = ftrace_set_filter_ip(ops, ftrace_ip, 0, 0);
>  	if (ret) {
>  		pr_debug("Failed to arm kprobe-ftrace at %pS (%d)\n",
>  			 p->addr, ret);
> @@ -1070,7 +1071,7 @@ static int __arm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
>  	 * At this point, sinec ops is not registered, we should be sefe from
>  	 * registering empty filter.
>  	 */
> -	ftrace_set_filter_ip(ops, (unsigned long)p->addr, 1, 0);
> +	ftrace_set_filter_ip(ops, ftrace_ip, 1, 0);
>  	return ret;
>  }
>  
> @@ -1087,6 +1088,7 @@ static int arm_kprobe_ftrace(struct kprobe *p)
>  static int __disarm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
>  				  int *cnt)
>  {
> +	unsigned long ftrace_ip = ftrace_location((unsigned long)p->addr);
>  	int ret = 0;
>  
>  	if (*cnt == 1) {
> @@ -1097,7 +1099,7 @@ static int __disarm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
>  
>  	(*cnt)--;
>  
> -	ret = ftrace_set_filter_ip(ops, (unsigned long)p->addr, 1, 0);
> +	ret = ftrace_set_filter_ip(ops, ftrace_ip, 1, 0);
>  	WARN_ONCE(ret < 0, "Failed to disarm kprobe-ftrace at %pS (%d)\n",
>  		  p->addr, ret);
>  	return ret;


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/6] MIPS: replace -pg with CC_FLAGS_FTRACE
  2021-03-25 19:38   ` Steven Rostedt
@ 2021-03-26  1:48     ` Huang Pei
  0 siblings, 0 replies; 18+ messages in thread
From: Huang Pei @ 2021-03-26  1:48 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Thomas Bogendoerfer, ambrosehua, Bibo Mao, linux-mips,
	linux-arch, linux-mm, Jiaxun Yang, Paul Burton, Li Xuefeng,
	Yang Tiezhu, Gao Juxin, Huacai Chen, Jinyang He,
	Maciej W . Rozycki, Jisheng Zhang, Masami Hiramatsu

On Thu, Mar 25, 2021 at 03:38:14PM -0400, Steven Rostedt wrote:
> On Sat, 13 Mar 2021 14:41:44 +0800
> Huang Pei <huangpei@loongson.cn> wrote:
> 
> 
> Even simple changes require change logs. For example:
> 
> "Enabling ftrace may require more than just the -pg flags today. As ftrace
> enables more flags, use the $(CC_FLAGS_FTRACE) in the make file instead of
> hard coding "-pg"."
> 
> Other than that:
> 
> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> 
> -- Steve
> 
Got it, much better than mine, thank you
> 
> > Signed-off-by: Huang Pei <huangpei@loongson.cn>
> > ---
> >  arch/mips/boot/compressed/Makefile | 2 +-
> >  arch/mips/kernel/Makefile          | 8 ++++----
> >  arch/mips/vdso/Makefile            | 4 ++--
> >  3 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/mips/boot/compressed/Makefile b/arch/mips/boot/compressed/Makefile
> > index d66511825fe1..8fc9ceeec709 100644
> > --- a/arch/mips/boot/compressed/Makefile
> > +++ b/arch/mips/boot/compressed/Makefile
> > @@ -18,7 +18,7 @@ include $(srctree)/arch/mips/Kbuild.platforms
> >  BOOT_HEAP_SIZE := 0x400000
> >  
> >  # Disable Function Tracer
> > -KBUILD_CFLAGS := $(filter-out -pg, $(KBUILD_CFLAGS))
> > +KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE), $(KBUILD_CFLAGS))
> >  
> >  KBUILD_CFLAGS := $(filter-out -fstack-protector, $(KBUILD_CFLAGS))
> >  
> > diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
> > index 2a05b923f579..33e31ea10234 100644
> > --- a/arch/mips/kernel/Makefile
> > +++ b/arch/mips/kernel/Makefile
> > @@ -17,10 +17,10 @@ obj-y		+= cpu-probe.o
> >  endif
> >  
> >  ifdef CONFIG_FUNCTION_TRACER
> > -CFLAGS_REMOVE_ftrace.o = -pg
> > -CFLAGS_REMOVE_early_printk.o = -pg
> > -CFLAGS_REMOVE_perf_event.o = -pg
> > -CFLAGS_REMOVE_perf_event_mipsxx.o = -pg
> > +CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE)
> > +CFLAGS_REMOVE_early_printk.o = $(CC_FLAGS_FTRACE)
> > +CFLAGS_REMOVE_perf_event.o = $(CC_FLAGS_FTRACE)
> > +CFLAGS_REMOVE_perf_event_mipsxx.o = $(CC_FLAGS_FTRACE)
> >  endif
> >  
> >  obj-$(CONFIG_CEVT_BCM1480)	+= cevt-bcm1480.o
> > diff --git a/arch/mips/vdso/Makefile b/arch/mips/vdso/Makefile
> > index 5810cc12bc1d..f21cf88f7ae3 100644
> > --- a/arch/mips/vdso/Makefile
> > +++ b/arch/mips/vdso/Makefile
> > @@ -49,7 +49,7 @@ CFLAGS_vgettimeofday-o32.o = -include $(srctree)/$(src)/config-n32-o32-env.c -in
> >  CFLAGS_vgettimeofday-n32.o = -include $(srctree)/$(src)/config-n32-o32-env.c -include $(c-gettimeofday-y)
> >  endif
> >  
> > -CFLAGS_REMOVE_vgettimeofday.o = -pg
> > +CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE)
> >  
> >  ifdef CONFIG_MIPS_DISABLE_VDSO
> >    ifndef CONFIG_MIPS_LD_CAN_LINK_VDSO
> > @@ -63,7 +63,7 @@ ldflags-y := -Bsymbolic --no-undefined -soname=linux-vdso.so.1 \
> >  	$(filter -E%,$(KBUILD_CFLAGS)) -nostdlib -shared \
> >  	-G 0 --eh-frame-hdr --hash-style=sysv --build-id=sha1 -T
> >  
> > -CFLAGS_REMOVE_vdso.o = -pg
> > +CFLAGS_REMOVE_vdso.o = $(CC_FLAGS_FTRACE)
> >  
> >  GCOV_PROFILE := n
> >  UBSAN_SANITIZE := n


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] MIPS: move FTRACE_SYSCALLS from ftrace.c into syscall.c
  2021-03-25 19:38   ` Steven Rostedt
@ 2021-03-26 14:01     ` Huang Pei
  0 siblings, 0 replies; 18+ messages in thread
From: Huang Pei @ 2021-03-26 14:01 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Thomas Bogendoerfer, ambrosehua, Bibo Mao, linux-mips,
	linux-arch, linux-mm, Jiaxun Yang, Paul Burton, Li Xuefeng,
	Yang Tiezhu, Gao Juxin, Huacai Chen, Jinyang He,
	Maciej W . Rozycki, Jisheng Zhang, Masami Hiramatsu

I was intended to add -fpatchable-function-entry based new ftrace implementation in parallel 
with old -pg based one, which is enabled by supported gcc version at build time, since 

+. the fix for -fpatchable-function-entry is not merge into upstream; 

+. the -fpatchable-function-entry does not support static function tracing which is rarely used today, 
which is only implemented in the old -pg based implementation;

Move FTRACE_SYSCALLS into syscall.c, so both new and old implementation can easily be switched
freely. When everything is all right, old one can be easily removed, and that is patch 1/2/3 intended for

> On Mar 26, 2021, at 3:38 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> On Sat, 13 Mar 2021 14:41:45 +0800
> Huang Pei <huangpei@loongson.cn> wrote:
> 
> Why?
> 
> -- Steve
> 
>> Signed-off-by: Huang Pei <huangpei@loongson.cn>
>> ---
>> arch/mips/kernel/Makefile  |  1 -
>> arch/mips/kernel/ftrace.c  | 33 ---------------------------------
>> arch/mips/kernel/syscall.c | 32 ++++++++++++++++++++++++++++++++
>> 3 files changed, 32 insertions(+), 34 deletions(-)


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/6] kprobes/ftrace: Use ftrace_location() when [dis]arming probes
  2021-03-25 19:44   ` Steven Rostedt
@ 2021-03-26 14:12     ` Huang Pei
  2021-03-26 14:23       ` Steven Rostedt
  0 siblings, 1 reply; 18+ messages in thread
From: Huang Pei @ 2021-03-26 14:12 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Thomas Bogendoerfer, ambrosehua, Bibo Mao, linux-mips,
	linux-arch, linux-mm, Jiaxun Yang, Paul Burton, Li Xuefeng,
	Yang Tiezhu, Gao Juxin, Huacai Chen, Jinyang He,
	Maciej W . Rozycki, Jisheng Zhang, Masami Hiramatsu,
	Naveen N . Rao

Patch 4/5 is from arm64’s KPROBES_ON_FTRACE,  I think which is needed by
all RISC with both KPROBES_ON_FTRACE and -fpatchable-function-entry.

But since V7, no further patches are released, what protocol should I follow if
I need these two patches?

> On Mar 26, 2021, at 3:44 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> On Sat, 13 Mar 2021 14:41:47 +0800
> Huang Pei <huangpei@loongson.cn> wrote:
> 
>> From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
>> 
> 
> Looks like this was sent before, but was missing the proper authorship
> (which is not Jisheng).
> 
>   https://lore.kernel.org/linux-arm-kernel/20191225173219.4f9db436@xhacker.debian/
> 
> -- Steve
> 
> 
>> Ftrace location could include more than a single instruction in case
>> of some architectures (powerpc64, for now). In this case, kprobe is
>> permitted on any of those instructions, and uses ftrace infrastructure
>> for functioning.
>> 
>> However, [dis]arm_kprobe_ftrace() uses the kprobe address when setting
>> up ftrace filter IP. This won't work if the address points to any
>> instruction apart from the one that has a branch to _mcount(). To
>> resolve this, have [dis]arm_kprobe_ftrace() use ftrace_function() to
>> identify the filter IP.
>> 
>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
>> ---
>> kernel/kprobes.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>> 
>> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
>> index 41fdbb7953c6..66ee28b071c2 100644
>> --- a/kernel/kprobes.c
>> +++ b/kernel/kprobes.c
>> @@ -1045,9 +1045,10 @@ static int prepare_kprobe(struct kprobe *p)
>> static int __arm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
>> 			       int *cnt)
>> {
>> +	unsigned long ftrace_ip = ftrace_location((unsigned long)p->addr);
>> 	int ret = 0;
>> 
>> -	ret = ftrace_set_filter_ip(ops, (unsigned long)p->addr, 0, 0);
>> +	ret = ftrace_set_filter_ip(ops, ftrace_ip, 0, 0);
>> 	if (ret) {
>> 		pr_debug("Failed to arm kprobe-ftrace at %pS (%d)\n",
>> 			 p->addr, ret);
>> @@ -1070,7 +1071,7 @@ static int __arm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
>> 	 * At this point, sinec ops is not registered, we should be sefe from
>> 	 * registering empty filter.
>> 	 */
>> -	ftrace_set_filter_ip(ops, (unsigned long)p->addr, 1, 0);
>> +	ftrace_set_filter_ip(ops, ftrace_ip, 1, 0);
>> 	return ret;
>> }
>> 
>> @@ -1087,6 +1088,7 @@ static int arm_kprobe_ftrace(struct kprobe *p)
>> static int __disarm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
>> 				  int *cnt)
>> {
>> +	unsigned long ftrace_ip = ftrace_location((unsigned long)p->addr);
>> 	int ret = 0;
>> 
>> 	if (*cnt == 1) {
>> @@ -1097,7 +1099,7 @@ static int __disarm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
>> 
>> 	(*cnt)--;
>> 
>> -	ret = ftrace_set_filter_ip(ops, (unsigned long)p->addr, 1, 0);
>> +	ret = ftrace_set_filter_ip(ops, ftrace_ip, 1, 0);
>> 	WARN_ONCE(ret < 0, "Failed to disarm kprobe-ftrace at %pS (%d)\n",
>> 		  p->addr, ret);
>> 	return ret;

Huang Pei
huangpei@loongson.cn




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/6] kprobes/ftrace: Use ftrace_location() when [dis]arming probes
  2021-03-26 14:12     ` Huang Pei
@ 2021-03-26 14:23       ` Steven Rostedt
  0 siblings, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2021-03-26 14:23 UTC (permalink / raw)
  To: Huang Pei
  Cc: Thomas Bogendoerfer, ambrosehua, Bibo Mao, linux-mips,
	linux-arch, linux-mm, Jiaxun Yang, Paul Burton, Li Xuefeng,
	Yang Tiezhu, Gao Juxin, Huacai Chen, Jinyang He,
	Maciej W . Rozycki, Jisheng Zhang, Masami Hiramatsu,
	Naveen N . Rao

On Fri, 26 Mar 2021 22:12:18 +0800
Huang Pei <huangpei@loongson.cn> wrote:

> Patch 4/5 is from arm64’s KPROBES_ON_FTRACE,  I think which is needed by
> all RISC with both KPROBES_ON_FTRACE and -fpatchable-function-entry.
> 
> But since V7, no further patches are released, what protocol should I follow if
> I need these two patches?
> 

If you need this patch, just resend it, but with the proper author. If you
look at the thread I linked to, Jisheng pointed out that the From line that
held the proper author was missing from the patch.

You'll need that.

-- Steve


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-03-26 14:24 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-13  6:41 [RFC PATCH V2]: add DYNAMC_FTRACE_WITH_REGS and Huang Pei
2021-03-13  6:41 ` [PATCH 1/6] MIPS: replace -pg with CC_FLAGS_FTRACE Huang Pei
2021-03-25 19:38   ` Steven Rostedt
2021-03-26  1:48     ` Huang Pei
2021-03-13  6:41 ` [PATCH 2/6] MIPS: move FTRACE_SYSCALLS from ftrace.c into syscall.c Huang Pei
2021-03-25 19:38   ` Steven Rostedt
2021-03-26 14:01     ` Huang Pei
2021-03-13  6:41 ` [PATCH 3/6] MIPS: prepare for new ftrace implementation Huang Pei
2021-03-25 19:39   ` Steven Rostedt
2021-03-13  6:41 ` [PATCH 4/6] kprobes/ftrace: Use ftrace_location() when [dis]arming probes Huang Pei
2021-03-25 19:44   ` Steven Rostedt
2021-03-26 14:12     ` Huang Pei
2021-03-26 14:23       ` Steven Rostedt
2021-03-13  6:41 ` [PATCH 5/6] ftrace: introduce FTRACE_IP_EXTENSION Huang Pei
2021-03-13  9:26   ` Sergei Shtylyov
2021-03-13  6:41 ` [PATCH 6/6] MIPS: add DYNAMIC_FTRACE_WITH_REGS and KPROBES_ON_FTACE Huang Pei
2021-03-13 21:37   ` kernel test robot
2021-03-13 21:37     ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.