Messages in this thread | | | From | Björn Töpel <> | Subject | Re: [PATCH] ftrace: riscv: move from REGS to ARGS | Date | Wed, 24 Apr 2024 11:13:46 +0200 |
| |
Puranjay Mohan <puranjay@kernel.org> writes:
> This commit replaces riscv's support for FTRACE_WITH_REGS with support > for FTRACE_WITH_ARGS. This is required for the ongoing effort to stop > relying on stop_machine() for RISCV's implementation of ftrace. > > The main relevant benefit that this change will bring for the above > use-case is that now we don't have separate ftrace_caller and > ftrace_regs_caller trampolines. This will allow the callsite to call > ftrace_caller by modifying a single instruction. Now the callsite can > do something similar to: > > When not tracing: | When tracing: > > func: func: > auipc t0, ftrace_caller_top auipc t0, ftrace_caller_top > nop <=========<Enable/Disable>=========> jalr t0, ftrace_caller_bottom > [...] [...] > > The above assumes that we are dropping the support of calling a direct > trampoline from the callsite. We need to drop this as the callsite can't > change the target address to call, it can only enable/disable a call to > a preset target (ftrace_caller in the above diagram). We can later optimize > this by calling an intermediate dispatcher trampoline before ftrace_caller. > > Currently, ftrace_regs_caller saves all CPU registers in the format of > struct pt_regs and allows the tracer to modify them. We don't need to > save all of the CPU registers because at function entry only a subset of > pt_regs is live: > > |----------+----------+---------------------------------------------| > | Register | ABI Name | Description | > |----------+----------+---------------------------------------------| > | x1 | ra | Return address for traced function | > | x2 | sp | Stack pointer | > | x5 | t0 | Return address for ftrace_caller trampoline | > | x8 | s0/fp | Frame pointer | > | x10-11 | a0-1 | Function arguments/return values | > | x12-17 | a2-7 | Function arguments | > |----------+----------+---------------------------------------------| > > See RISCV calling convention[1] for the above table. > > Saving just the live registers decreases the amount of stack space > required from 288 Bytes to 112 Bytes. > > Basic testing was done with this on the VisionFive 2 development board. > > Note: > - Moving from REGS to ARGS will mean that RISCV will stop supporting > KPROBES_ON_FTRACE as it requires full pt_regs to be saved. > - KPROBES_ON_FTRACE will be supplanted by FPROBES see [2].
Puranjay,
Apologies for the slow response.
Restating from the RFC; This change is a prerequisite for moving towards a "FTRACE_WITH_CALL_OPS" model on riscv, and finally getting rid of the stop_machine() text patching.
I've tested this on QEMU, and on the VisionFive2. No regressions (FTRACE_STARTUP_*, and kselftest), other than that KPROBES_ON_FTRACE no longer works. (Which will be addressed by [2]).
Palmer, my preference is that this should go on for-next, rather than being part of the upcoming text patching support (worked on by Andy), but really up to you.
Tested-by: Björn Töpel <bjorn@rivosinc.com> Reviewed-by: Björn Töpel <bjorn@rivosinc.com>
| |