| Message ID | 2cd04be69e90adc34bcf98d405ab6b21f268cb6a.1718908016.git.naveen@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc: Add support for ftrace direct and BPF trampolines | expand |
On Fri Jun 21, 2024 at 4:54 AM AEST, Naveen N Rao wrote: > Rather than hard-coding the offset into a function to be used to > determine if a kprobe is at function entry, use ftrace_location() to > determine the ftrace location within the function and categorize all > instructions till that offset to be function entry. > > For functions that cannot be traced, we fall back to using a fixed > offset of 8 (two instructions) to categorize a probe as being at > function entry for 64-bit elfv2, unless we are using pcrel. > > Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > Signed-off-by: Naveen N Rao <naveen@kernel.org> > --- > arch/powerpc/kernel/kprobes.c | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > > diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c > index 14c5ddec3056..ca204f4f21c1 100644 > --- a/arch/powerpc/kernel/kprobes.c > +++ b/arch/powerpc/kernel/kprobes.c > @@ -105,24 +105,22 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset) > return addr; > } > > -static bool arch_kprobe_on_func_entry(unsigned long offset) > +static bool arch_kprobe_on_func_entry(unsigned long addr, unsigned long offset) > { > -#ifdef CONFIG_PPC64_ELF_ABI_V2 > -#ifdef CONFIG_KPROBES_ON_FTRACE > - return offset <= 16; > -#else > - return offset <= 8; > -#endif > -#else > + unsigned long ip = ftrace_location(addr); > + > + if (ip) > + return offset <= (ip - addr); > + if (IS_ENABLED(CONFIG_PPC64_ELF_ABI_V2) && !IS_ENABLED(CONFIG_PPC_KERNEL_PCREL)) > + return offset <= 8; If it is PCREL, why not offset == 0 as well? Thanks, Nick
Hi Nick, Thanks for the reviews! On Mon, Jul 01, 2024 at 06:40:50PM GMT, Nicholas Piggin wrote: > On Fri Jun 21, 2024 at 4:54 AM AEST, Naveen N Rao wrote: > > Rather than hard-coding the offset into a function to be used to > > determine if a kprobe is at function entry, use ftrace_location() to > > determine the ftrace location within the function and categorize all > > instructions till that offset to be function entry. > > > > For functions that cannot be traced, we fall back to using a fixed > > offset of 8 (two instructions) to categorize a probe as being at > > function entry for 64-bit elfv2, unless we are using pcrel. > > > > Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Signed-off-by: Naveen N Rao <naveen@kernel.org> > > --- > > arch/powerpc/kernel/kprobes.c | 18 ++++++++---------- > > 1 file changed, 8 insertions(+), 10 deletions(-) > > > > diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c > > index 14c5ddec3056..ca204f4f21c1 100644 > > --- a/arch/powerpc/kernel/kprobes.c > > +++ b/arch/powerpc/kernel/kprobes.c > > @@ -105,24 +105,22 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset) > > return addr; > > } > > > > -static bool arch_kprobe_on_func_entry(unsigned long offset) > > +static bool arch_kprobe_on_func_entry(unsigned long addr, unsigned long offset) > > { > > -#ifdef CONFIG_PPC64_ELF_ABI_V2 > > -#ifdef CONFIG_KPROBES_ON_FTRACE > > - return offset <= 16; > > -#else > > - return offset <= 8; > > -#endif > > -#else > > + unsigned long ip = ftrace_location(addr); > > + > > + if (ip) > > + return offset <= (ip - addr); > > + if (IS_ENABLED(CONFIG_PPC64_ELF_ABI_V2) && !IS_ENABLED(CONFIG_PPC_KERNEL_PCREL)) > > + return offset <= 8; > > If it is PCREL, why not offset == 0 as well? That's handled by the fallback code that is after the above line: return !offset; That addresses both pcrel, as well as 32-bit powerpc. Thanks, Naveen
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c index 14c5ddec3056..ca204f4f21c1 100644 --- a/arch/powerpc/kernel/kprobes.c +++ b/arch/powerpc/kernel/kprobes.c @@ -105,24 +105,22 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset) return addr; } -static bool arch_kprobe_on_func_entry(unsigned long offset) +static bool arch_kprobe_on_func_entry(unsigned long addr, unsigned long offset) { -#ifdef CONFIG_PPC64_ELF_ABI_V2 -#ifdef CONFIG_KPROBES_ON_FTRACE - return offset <= 16; -#else - return offset <= 8; -#endif -#else + unsigned long ip = ftrace_location(addr); + + if (ip) + return offset <= (ip - addr); + if (IS_ENABLED(CONFIG_PPC64_ELF_ABI_V2) && !IS_ENABLED(CONFIG_PPC_KERNEL_PCREL)) + return offset <= 8; return !offset; -#endif } /* XXX try and fold the magic of kprobe_lookup_name() in this */ kprobe_opcode_t *arch_adjust_kprobe_addr(unsigned long addr, unsigned long offset, bool *on_func_entry) { - *on_func_entry = arch_kprobe_on_func_entry(offset); + *on_func_entry = arch_kprobe_on_func_entry(addr, offset); return (kprobe_opcode_t *)(addr + offset); }