lkml.org 
[lkml]   [2020]   [Jul]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 2/2] kernel/trace: Remove function callback casts
On Sun, 26 Jul 2020 17:52:42 +0200
Oscar Carter <oscar.carter@gmx.com> wrote:

> > If I try to do this I will need some help. Some info that point me to the
> > right direction would be greatly appreciated. Some advice about what
> > functions I will need to implement would be really helpfull. Or point me
> > to the right piece of code that I can pick as base point.
>
> I've been searching and reading the code as much as possible. I've found
> two patches that I think can be useful to guide me. One [1] adds support
> for ftrace_ops to the riscv architecture. The other one [2] adds support
> for ftrace_ops to the parisc architecture.
>
> [1] commit 71e736a7d655 ("riscv/ftrace: Add ARCH_SUPPORTS_FTRACE_OPS support")
> [2] commit d562aca37a54 ("parisc/ftrace: Add ARCH_SUPPORTS_FTRACE_OPS support")
>
> Due to powerpc arch calls the needed functions from assembly, I based my
> idea on the patch for the RISCV arch.
>
> Can something like the following work?
>
> diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
> index bc76970b6ee5..1c51ff5afae1 100644
> --- a/arch/powerpc/include/asm/ftrace.h
> +++ b/arch/powerpc/include/asm/ftrace.h
> @@ -61,9 +61,8 @@ struct dyn_arch_ftrace {
> };
> #endif /* __ASSEMBLY__ */
>
> -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> #define ARCH_SUPPORTS_FTRACE_OPS 1
> -#endif
> +
> #endif /* CONFIG_FUNCTION_TRACER */
>
> #ifndef __ASSEMBLY__
> diff --git a/arch/powerpc/kernel/trace/ftrace_32.S b/arch/powerpc/kernel/trace/ftrace_32.S
> index e023ae59c429..e69a4e945986 100644
> --- a/arch/powerpc/kernel/trace/ftrace_32.S
> +++ b/arch/powerpc/kernel/trace/ftrace_32.S
> @@ -29,6 +29,10 @@ _GLOBAL(ftrace_caller)
> MCOUNT_SAVE_FRAME
> /* r3 ends up with link register */
> subi r3, r3, MCOUNT_INSN_SIZE
> +
> + /* Set ftrace_ops (r5) to the global variable function_trace_op */
> + /* Set pt_regs (r6) to NULL */
> +
> .globl ftrace_call
> ftrace_call:
> bl ftrace_stub
> diff --git a/arch/powerpc/kernel/trace/ftrace_64_pg.S b/arch/powerpc/kernel/trace/ftrace_64_pg.S
> index 6708e24db0ab..a741448b1df9 100644
> --- a/arch/powerpc/kernel/trace/ftrace_64_pg.S
> +++ b/arch/powerpc/kernel/trace/ftrace_64_pg.S
> @@ -22,6 +22,10 @@ _GLOBAL_TOC(ftrace_caller)
> std r3, 128(r1)
> ld r4, 16(r11)
> subi r3, r3, MCOUNT_INSN_SIZE
> +
> + /* Set ftrace_ops (r5) to the global variable function_trace_op */
> + /* Set pt_regs (r6) to NULL */

I'm guessing you are going to do the above here. If so, this looks correct.

> +
> .globl ftrace_call
> ftrace_call:
> bl ftrace_stub
>
> To add support for ftrace_ops to the powerpc architecture is only necessary
> to fill the r5 and r6 registers before the call to ftrace_stub in all the
> cases. The register r5 is a pointer to ftrace_ops struct and the register
> r6 is a pointer to pt_regs struct. These registers are the third and fourth
> parameters of a function with the following prototype. The first and second
> ones are yet set.

I guess you mean that the first and second ones are already set. But,
yeah, you are on the correct path here!

>
> void func(unsigned long ip, unsigned long parent_ip,
> struct ftrace_ops *ops, struct pt_regs *regs);
>
> Am I in the right direction? or am I totally wrong?

No, you don't look wrong. But the true test is to try it out :-)

Don't forget to update ftrace_32.S as well.


>
> Thanks for your time and patience.

My pleasure. Thanks for doing this. The more people that understand all
this, the better!

-- Steve

\
 
 \ /
  Last update: 2020-07-27 15:53    [W:0.088 / U:0.844 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site