riscv: ftrace: Fixup ftrace detour code
Subsystems (3)
cover riscv: ftrace: Fixup ftrace detour code
From: Guo Ren <[email protected]>
The previous ftrace detour implementation fc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of MCOUNT") contain three problems. The most horrible bug is preemption panic which found by Andy [1]. I think we could disable preemption for ftrace first, and Andy could continue the ftrace preemption work.
@Andy, as Steven said, improving stop_machine mechinism also could prevent preemption problem, but it would reduce the speed of ftrace_modify. How do you think about the stop_machine solution, would you give out the patch for this?
[1]: https://lpc.events/event/16/contributions/1171/
Andy Chiu (1):
riscv: ftrace: Fixup panic by disabling preemption
Guo Ren (2):
riscv: ftrace: Remove wasted nops for !RISCV_ISA_C
riscv: ftrace: Reduce the detour code size to half
arch/riscv/Kconfig | 2 +- arch/riscv/Makefile | 6 ++- arch/riscv/include/asm/ftrace.h | 46 ++++++++++++++++++----- arch/riscv/kernel/ftrace.c | 65 ++++++++++----------------------- arch/riscv/kernel/mcount-dyn.S | 43 +++++++++------------- 5 files changed, 78 insertions(+), 84 deletions(-)
Signature · 7 lines
2.36.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
PATCH 1/3 riscv: ftrace: Fixup panic by disabling preemption
From: Andy Chiu <[email protected]>
In RISCV, we must use an AUIPC + JALR pair to encode an immediate, forming a jump that jumps to an address over 4K. This may cause errors if we want to enable kernel preemption and remove dependency from patching code with stop_machine(). For example, if a task was switched out on auipc. And, if we changed the ftrace function before it was switched back, then it would jump to an address that has updated 11:0 bits mixing with previous XLEN:12 part.
p: patched area performed by dynamic ftraceftrace_prologue:
p| REG_S ra, -SZREG(sp) p| auipc ra, 0x? ------------> preempted ... change ftrace function ... p| jalr -?(ra) <------------- switched back p| REG_L ra, -SZREG(sp)
func:
xxx
ret
---
@Andy, could you give the patch a Signed-off-by? I just copy your most important comment, so the first author should be you. First, let's fix the problem caused by my previous patch, and you can continue your ftrace preemption work.
--- arch/riscv/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/riscv/Kconfig +1 -1
@@ -272,7 +272,7 @@ config ARCH_RV64I select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL select HAVE_FUNCTION_GRAPH_TRACER - select HAVE_FUNCTION_TRACER if !XIP_KERNEL + select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION select SWIOTLB if MMU endchoice
Signature · 7 lines
2.36.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH 1/3] riscv: ftrace: Fixup panic by disabling preemption
Yes, by disabling preemption and ensuring all sub-functions called by the busy waiting loop of stop_machine, which happens to be true on non-preemptive kernels, solve the problem from the original implementation.
Andy Chiu <[email protected]>
Re: [PATCH 1/3] riscv: ftrace: Fixup panic by disabling preemption
Thanks! I would add your Signed-off-by: Andy Chiu
<[email protected]> as the first author, next.
Quoted 7 lines · Andy Chiu
Yes, by disabling preemption and ensuring all sub-functions called by the busy waiting loop of
stop_machine, which happens to be true on non-preemptive kernels, solve the problem from the original implementation.Andy Chiu <[email protected]>
Signature · 2 lines
Best Regards Guo Ren
PATCH 2/3 riscv: ftrace: Remove wasted nops for !RISCV_ISA_C
In reply to [email protected]From: Guo Ren <[email protected]>
When CONFIG_RISCV_ISA_C=n, -fpatchable-function-entry=8 would generate more nops than we expect. Because it treat nop opcode as 0x00000013 instead of 0x0001.
Dump of assembler code for function dw_pcie_free_msi:
0xffffffff806fce94 <+0>: sd ra,-8(sp) 0xffffffff806fce98 <+4>: auipc ra,0xff90f 0xffffffff806fce9c <+8>: jalr -684(ra) # 0xffffffff8000bbec <ftrace_caller> 0xffffffff806fcea0 <+12>: ld ra,-8(sp) 0xffffffff806fcea4 <+16>: nop /* wasted */ 0xffffffff806fcea8 <+20>: nop /* wasted */ 0xffffffff806fceac <+24>: nop /* wasted */ 0xffffffff806fceb0 <+28>: nop /* wasted */ 0xffffffff806fceb4 <+0>: addi sp,sp,-48 0xffffffff806fceb8 <+4>: sd s0,32(sp) 0xffffffff806fcebc <+8>: sd s1,24(sp) 0xffffffff806fcec0 <+12>: sd s2,16(sp) 0xffffffff806fcec4 <+16>: sd s3,8(sp) 0xffffffff806fcec8 <+20>: sd ra,40(sp) 0xffffffff806fcecc <+24>: addi s0,sp,48
--- arch/riscv/Makefile | 4 ++++ 1 file changed, 4 insertions(+)
arch/riscv/Makefile +4 -0
@@ -11,7 +11,11 @@ LDFLAGS_vmlinux := ifeq ($(CONFIG_DYNAMIC_FTRACE),y) LDFLAGS_vmlinux := --no-relax KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY +ifeq ($(CONFIG_RISCV_ISA_C),y) CC_FLAGS_FTRACE := -fpatchable-function-entry=8 +else + CC_FLAGS_FTRACE := -fpatchable-function-entry=4 +endif endif ifeq ($(CONFIG_CMODEL_MEDLOW),y)
Signature · 7 lines
2.36.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
PATCH 3/3 riscv: ftrace: Reduce the detour code size to half
In reply to [email protected]From: Guo Ren <[email protected]>
Use a temporary register to reduce the size of detour code from 16B to 8B.
Before optimization (16 byte):
<func_prolog>:
0: REG_S ra, -SZREG(sp) 4: auipc ra, ? 8: jalr ?(ra) 12: REG_L ra, -SZREG(sp)
(func_boddy)
The previous implementation is from afc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of MCOUNT")
After optimization (8 byte):
<func_prolog>:
0: auipc t0, ?
4: jalr t0, ?(t0)
(func_boddy)
--- arch/riscv/Makefile | 4 +- arch/riscv/include/asm/ftrace.h | 46 ++++++++++++++++++----- arch/riscv/kernel/ftrace.c | 65 ++++++++++----------------------- arch/riscv/kernel/mcount-dyn.S | 43 +++++++++------------- 4 files changed, 74 insertions(+), 84 deletions(-)
arch/riscv/Makefile +2 -2
@@ -12,9 +12,9 @@ ifeq ($(CONFIG_DYNAMIC_FTRACE),y) LDFLAGS_vmlinux := --no-relax KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY ifeq ($(CONFIG_RISCV_ISA_C),y) - CC_FLAGS_FTRACE := -fpatchable-function-entry=8 -else CC_FLAGS_FTRACE := -fpatchable-function-entry=4 +else + CC_FLAGS_FTRACE := -fpatchable-function-entry=2 endif endif
arch/riscv/include/asm/ftrace.h +36 -10
67 lines not on this page · raw message
arch/riscv/kernel/ftrace.c +19 -46
138 lines not on this page · raw message
arch/riscv/kernel/mcount-dyn.S +17 -26
131 lines not on this page · raw message
Signature · 7 lines
2.36.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv