| Message ID | 172904034052.36809.10990962223606196850.stgit@devnote2 (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
| bpf/vmtest-bpf-PR | fail | merge-conflict |
On Wed, Oct 16, 2024 at 09:59:00AM +0900, Masami Hiramatsu (Google) wrote: > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Add ftrace_partial_regs() which converts the ftrace_regs to pt_regs. > This is for the eBPF which needs this to keep the same pt_regs interface > to access registers. > Thus when replacing the pt_regs with ftrace_regs in fprobes (which is > used by kprobe_multi eBPF event), this will be used. > > If the architecture defines its own ftrace_regs, this copies partial > registers to pt_regs and returns it. If not, ftrace_regs is the same as > pt_regs and ftrace_partial_regs() will return ftrace_regs::regs. > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > Acked-by: Florent Revest <revest@chromium.org> > Cc: Steven Rostedt <rostedt@goodmis.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > Cc: Paul Walmsley <paul.walmsley@sifive.com> > Cc: Palmer Dabbelt <palmer@dabbelt.com> > Cc: Albert Ou <aou@eecs.berkeley.edu> > > --- > Changes in v14: > - Add riscv change. > Changes in v8: > - Add the reason why this required in changelog. > Changes from previous series: NOTHING, just forward ported. > --- > arch/arm64/include/asm/ftrace.h | 11 +++++++++++ > arch/riscv/include/asm/ftrace.h | 14 ++++++++++++++ > include/linux/ftrace.h | 17 +++++++++++++++++ > 3 files changed, 42 insertions(+) > > diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h > index b5fa57b61378..d344c69eb01e 100644 > --- a/arch/arm64/include/asm/ftrace.h > +++ b/arch/arm64/include/asm/ftrace.h > @@ -135,6 +135,17 @@ ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs) > return arch_ftrace_regs(fregs)->fp; > } > > +static __always_inline struct pt_regs * > +ftrace_partial_regs(const struct ftrace_regs *fregs, struct pt_regs *regs) > +{ > + memcpy(regs->regs, arch_ftrace_regs(fregs)->regs, sizeof(u64) * 9); Since ftrace_regs::regs is an 'unsigned long regs[9]' can we just use sizeof() on that instead of hard-coding the length of the array here? Will
On Mon, 21 Oct 2024 17:46:19 +0100 Will Deacon <will@kernel.org> wrote: > On Wed, Oct 16, 2024 at 09:59:00AM +0900, Masami Hiramatsu (Google) wrote: > > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > > Add ftrace_partial_regs() which converts the ftrace_regs to pt_regs. > > This is for the eBPF which needs this to keep the same pt_regs interface > > to access registers. > > Thus when replacing the pt_regs with ftrace_regs in fprobes (which is > > used by kprobe_multi eBPF event), this will be used. > > > > If the architecture defines its own ftrace_regs, this copies partial > > registers to pt_regs and returns it. If not, ftrace_regs is the same as > > pt_regs and ftrace_partial_regs() will return ftrace_regs::regs. > > > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Acked-by: Florent Revest <revest@chromium.org> > > Cc: Steven Rostedt <rostedt@goodmis.org> > > Cc: Mark Rutland <mark.rutland@arm.com> > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > Cc: Will Deacon <will@kernel.org> > > Cc: Paul Walmsley <paul.walmsley@sifive.com> > > Cc: Palmer Dabbelt <palmer@dabbelt.com> > > Cc: Albert Ou <aou@eecs.berkeley.edu> > > > > --- > > Changes in v14: > > - Add riscv change. > > Changes in v8: > > - Add the reason why this required in changelog. > > Changes from previous series: NOTHING, just forward ported. > > --- > > arch/arm64/include/asm/ftrace.h | 11 +++++++++++ > > arch/riscv/include/asm/ftrace.h | 14 ++++++++++++++ > > include/linux/ftrace.h | 17 +++++++++++++++++ > > 3 files changed, 42 insertions(+) > > > > diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h > > index b5fa57b61378..d344c69eb01e 100644 > > --- a/arch/arm64/include/asm/ftrace.h > > +++ b/arch/arm64/include/asm/ftrace.h > > @@ -135,6 +135,17 @@ ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs) > > return arch_ftrace_regs(fregs)->fp; > > } > > > > +static __always_inline struct pt_regs * > > +ftrace_partial_regs(const struct ftrace_regs *fregs, struct pt_regs *regs) > > +{ > > + memcpy(regs->regs, arch_ftrace_regs(fregs)->regs, sizeof(u64) * 9); > > Since ftrace_regs::regs is an 'unsigned long regs[9]' can we just use > sizeof() on that instead of hard-coding the length of the array here? Good catch! I will update it. Thank you! > > Will
diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h index b5fa57b61378..d344c69eb01e 100644 --- a/arch/arm64/include/asm/ftrace.h +++ b/arch/arm64/include/asm/ftrace.h @@ -135,6 +135,17 @@ ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs) return arch_ftrace_regs(fregs)->fp; } +static __always_inline struct pt_regs * +ftrace_partial_regs(const struct ftrace_regs *fregs, struct pt_regs *regs) +{ + memcpy(regs->regs, arch_ftrace_regs(fregs)->regs, sizeof(u64) * 9); + regs->sp = arch_ftrace_regs(fregs)->sp; + regs->pc = arch_ftrace_regs(fregs)->pc; + regs->regs[29] = arch_ftrace_regs(fregs)->fp; + regs->regs[30] = arch_ftrace_regs(fregs)->lr; + return regs; +} + int ftrace_regs_query_register_offset(const char *name); int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec); diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h index 9372f8d7036f..17e0a13cc386 100644 --- a/arch/riscv/include/asm/ftrace.h +++ b/arch/riscv/include/asm/ftrace.h @@ -197,6 +197,20 @@ static __always_inline void ftrace_override_function_with_return(struct ftrace_r arch_ftrace_regs(fregs)->epc = arch_ftrace_regs(fregs)->ra; } +static __always_inline struct pt_regs * +ftrace_partial_regs(const struct ftrace_regs *fregs, struct pt_regs *regs) +{ + struct __arch_ftrace_regs *afregs = arch_ftrace_regs(fregs); + + memcpy(®s->a0, afregs->args, sizeof(u64) * 8); + regs->epc = afregs->epc; + regs->ra = afregs->ra; + regs->sp = afregs->sp; + regs->s0 = afregs->s0; + regs->t1 = afregs->t1; + return regs; +} + int ftrace_regs_query_register_offset(const char *name); void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index bf8bb6c10553..ad2b46e1d5b0 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -190,6 +190,23 @@ static __always_inline struct pt_regs *ftrace_get_regs(struct ftrace_regs *fregs return arch_ftrace_get_regs(fregs); } +#if !defined(CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS) || \ + defined(CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS) + +static __always_inline struct pt_regs * +ftrace_partial_regs(struct ftrace_regs *fregs, struct pt_regs *regs) +{ + /* + * If CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS=y, ftrace_regs memory + * layout is including pt_regs. So always returns that address. + * Since arch_ftrace_get_regs() will check some members and may return + * NULL, we can not use it. + */ + return &arch_ftrace_regs(fregs)->regs; +} + +#endif /* !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS || CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS */ + /* * When true, the ftrace_regs_{get,set}_*() functions may be used on fregs. * Note: this can be true even when ftrace_get_regs() cannot provide a pt_regs.