* [RFC PATCH v2 0/5] powerpc/ftrace: Move ftrace sequence out of line
@ 2024-06-10 8:38 Naveen N Rao
2024-06-10 8:38 ` [RFC PATCH v2 1/5] powerpc/kprobes: Use ftrace to determine if a probe is at function entry Naveen N Rao
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Naveen N Rao @ 2024-06-10 8:38 UTC (permalink / raw)
To: linuxppc-dev
Cc: Mark Rutland, Masahiro Yamada, Steven Rostedt, Nicholas Piggin,
Masami Hiramatsu
This is v2 of the series posted here:
http://lkml.kernel.org/r/cover.1702045299.git.naveen@kernel.org
Since v2, the primary change is that the entire ftrace sequence is moved
out of line and this is now restricted to 64-bit powerpc by default.
Patch 5 has the details.
I have dropped patches to enable DYNAMIC_FTRACE_WITH_CALL_OPS and ftrace
direct support so that this approach can be finalized.
This series depends on Benjamin Gray's series adding support for
patch_ulong():
http://lkml.kernel.org/r/20240515024445.236364-1-bgray@linux.ibm.com
Appreciate feedback on the approach.
Thanks,
Naveen
Naveen N Rao (5):
powerpc/kprobes: Use ftrace to determine if a probe is at function
entry
powerpc/ftrace: Remove pointer to struct module from dyn_arch_ftrace
powerpc/ftrace: Unify 32-bit and 64-bit ftrace entry code
kbuild: Add generic hook for architectures to use before the final
vmlinux link
powerpc64/ftrace: Move ftrace sequence out of line
arch/Kconfig | 3 +
arch/powerpc/Kconfig | 4 +
arch/powerpc/Makefile | 4 +
arch/powerpc/include/asm/ftrace.h | 11 +-
arch/powerpc/include/asm/module.h | 5 +
arch/powerpc/kernel/asm-offsets.c | 4 +
arch/powerpc/kernel/kprobes.c | 18 +--
arch/powerpc/kernel/module_64.c | 67 +++++++-
arch/powerpc/kernel/trace/ftrace.c | 196 ++++++++++++++++++++---
arch/powerpc/kernel/trace/ftrace_64_pg.c | 73 ++++-----
arch/powerpc/kernel/trace/ftrace_entry.S | 75 ++++++---
arch/powerpc/kernel/vmlinux.lds.S | 3 +-
arch/powerpc/tools/vmlinux_o.sh | 47 ++++++
scripts/link-vmlinux.sh | 18 ++-
14 files changed, 419 insertions(+), 109 deletions(-)
create mode 100755 arch/powerpc/tools/vmlinux_o.sh
base-commit: 2c644f2847c188b4fa545e602e4a1d4db55e8c8d
prerequisite-patch-id: a1d50e589288239d5a8b1c1f354cd4737057c9d3
prerequisite-patch-id: da4142d56880861bd0ad7ad7087c9e2670a2ee54
prerequisite-patch-id: 609d292e054b2396b603890522a940fa0bdfb6d8
prerequisite-patch-id: 6f7213fb77b1260defbf43be0e47bff9c80054cc
prerequisite-patch-id: ad3b71bf071ae4ba1bee5b087e61a2055772a74f
--
2.45.2
^ permalink raw reply [flat|nested] 16+ messages in thread* [RFC PATCH v2 1/5] powerpc/kprobes: Use ftrace to determine if a probe is at function entry 2024-06-10 8:38 [RFC PATCH v2 0/5] powerpc/ftrace: Move ftrace sequence out of line Naveen N Rao @ 2024-06-10 8:38 ` Naveen N Rao 2024-06-10 8:38 ` [RFC PATCH v2 2/5] powerpc/ftrace: Remove pointer to struct module from dyn_arch_ftrace Naveen N Rao ` (3 subsequent siblings) 4 siblings, 0 replies; 16+ messages in thread From: Naveen N Rao @ 2024-06-10 8:38 UTC (permalink / raw) To: linuxppc-dev Cc: Mark Rutland, Masahiro Yamada, Steven Rostedt, Nicholas Piggin, Masami Hiramatsu 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; 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); } -- 2.45.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC PATCH v2 2/5] powerpc/ftrace: Remove pointer to struct module from dyn_arch_ftrace 2024-06-10 8:38 [RFC PATCH v2 0/5] powerpc/ftrace: Move ftrace sequence out of line Naveen N Rao 2024-06-10 8:38 ` [RFC PATCH v2 1/5] powerpc/kprobes: Use ftrace to determine if a probe is at function entry Naveen N Rao @ 2024-06-10 8:38 ` Naveen N Rao 2024-06-10 20:03 ` Steven Rostedt 2024-06-10 8:38 ` [RFC PATCH v2 3/5] powerpc/ftrace: Unify 32-bit and 64-bit ftrace entry code Naveen N Rao ` (2 subsequent siblings) 4 siblings, 1 reply; 16+ messages in thread From: Naveen N Rao @ 2024-06-10 8:38 UTC (permalink / raw) To: linuxppc-dev Cc: Mark Rutland, Masahiro Yamada, Steven Rostedt, Nicholas Piggin, Masami Hiramatsu Pointer to struct module is only relevant for ftrace records belonging to kernel modules. Having this field in dyn_arch_ftrace wastes memory for all ftrace records belonging to the kernel. Remove the same in favour of looking up the module from the ftrace record address, similar to other architectures. Signed-off-by: Naveen N Rao <naveen@kernel.org> --- arch/powerpc/include/asm/ftrace.h | 1 - arch/powerpc/kernel/trace/ftrace.c | 47 ++++++++++----- arch/powerpc/kernel/trace/ftrace_64_pg.c | 73 +++++++++++------------- 3 files changed, 64 insertions(+), 57 deletions(-) diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h index 107fc5a48456..201f9d15430a 100644 --- a/arch/powerpc/include/asm/ftrace.h +++ b/arch/powerpc/include/asm/ftrace.h @@ -26,7 +26,6 @@ unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip, struct module; struct dyn_ftrace; struct dyn_arch_ftrace { - struct module *mod; }; #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c index d8d6b4fd9a14..041be965485e 100644 --- a/arch/powerpc/kernel/trace/ftrace.c +++ b/arch/powerpc/kernel/trace/ftrace.c @@ -106,20 +106,36 @@ static unsigned long find_ftrace_tramp(unsigned long ip) return 0; } +static struct module *ftrace_lookup_module(struct dyn_ftrace *rec) +{ + struct module *mod = NULL; + +#ifdef CONFIG_MODULES + /* + * NOTE: __module_text_address() must be called with preemption + * disabled, but we can rely on ftrace_lock to ensure that 'mod' + * retains its validity throughout the remainder of this code. + */ + preempt_disable(); + mod = __module_text_address(rec->ip); + preempt_enable(); + + if (!mod) + pr_err("No module loaded at addr=%lx\n", rec->ip); +#endif + + return mod; +} + static int ftrace_get_call_inst(struct dyn_ftrace *rec, unsigned long addr, ppc_inst_t *call_inst) { unsigned long ip = rec->ip; unsigned long stub; + struct module *mod; if (is_offset_in_branch_range(addr - ip)) { /* Within range */ stub = addr; -#ifdef CONFIG_MODULES - } else if (rec->arch.mod) { - /* Module code would be going to one of the module stubs */ - stub = (addr == (unsigned long)ftrace_caller ? rec->arch.mod->arch.tramp : - rec->arch.mod->arch.tramp_regs); -#endif } else if (core_kernel_text(ip)) { /* We would be branching to one of our ftrace stubs */ stub = find_ftrace_tramp(ip); @@ -128,7 +144,16 @@ static int ftrace_get_call_inst(struct dyn_ftrace *rec, unsigned long addr, ppc_ return -EINVAL; } } else { - return -EINVAL; + mod = ftrace_lookup_module(rec); + if (mod) { +#ifdef CONFIG_MODULES + /* Module code would be going to one of the module stubs */ + stub = (addr == (unsigned long)ftrace_caller ? mod->arch.tramp : + mod->arch.tramp_regs); +#endif + } else { + return -EINVAL; + } } *call_inst = ftrace_create_branch_inst(ip, stub, 1); @@ -256,14 +281,6 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec) if (ret) return ret; - if (!core_kernel_text(ip)) { - if (!mod) { - pr_err("0x%lx: No module provided for non-kernel address\n", ip); - return -EFAULT; - } - rec->arch.mod = mod; - } - /* Nop-out the ftrace location */ new = ppc_inst(PPC_RAW_NOP()); addr = MCOUNT_ADDR; diff --git a/arch/powerpc/kernel/trace/ftrace_64_pg.c b/arch/powerpc/kernel/trace/ftrace_64_pg.c index 12fab1803bcf..45bd8dcf9886 100644 --- a/arch/powerpc/kernel/trace/ftrace_64_pg.c +++ b/arch/powerpc/kernel/trace/ftrace_64_pg.c @@ -116,6 +116,24 @@ static unsigned long find_bl_target(unsigned long ip, ppc_inst_t op) } #ifdef CONFIG_MODULES +static struct module *ftrace_lookup_module(struct dyn_ftrace *rec) +{ + struct module *mod; + /* + * NOTE: __module_text_address() must be called with preemption + * disabled, but we can rely on ftrace_lock to ensure that 'mod' + * retains its validity throughout the remainder of this code. + */ + preempt_disable(); + mod = __module_text_address(rec->ip); + preempt_enable(); + + if (!mod) + pr_err("No module loaded at addr=%lx\n", rec->ip); + + return mod; +} + static int __ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, unsigned long addr) @@ -124,6 +142,12 @@ __ftrace_make_nop(struct module *mod, unsigned long ip = rec->ip; ppc_inst_t op, pop; + if (!mod) { + mod = ftrace_lookup_module(rec); + if (!mod) + return -EINVAL; + } + /* read where this goes */ if (copy_inst_from_kernel_nofault(&op, (void *)ip)) { pr_err("Fetching opcode failed.\n"); @@ -366,27 +390,6 @@ int ftrace_make_nop(struct module *mod, return -EINVAL; } - /* - * Out of range jumps are called from modules. - * We should either already have a pointer to the module - * or it has been passed in. - */ - if (!rec->arch.mod) { - if (!mod) { - pr_err("No module loaded addr=%lx\n", addr); - return -EFAULT; - } - rec->arch.mod = mod; - } else if (mod) { - if (mod != rec->arch.mod) { - pr_err("Record mod %p not equal to passed in mod %p\n", - rec->arch.mod, mod); - return -EINVAL; - } - /* nothing to do if mod == rec->arch.mod */ - } else - mod = rec->arch.mod; - return __ftrace_make_nop(mod, rec, addr); } @@ -411,7 +414,10 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) ppc_inst_t op[2]; void *ip = (void *)rec->ip; unsigned long entry, ptr, tramp; - struct module *mod = rec->arch.mod; + struct module *mod = ftrace_lookup_module(rec); + + if (!mod) + return -EINVAL; /* read where this goes */ if (copy_inst_from_kernel_nofault(op, ip)) @@ -533,16 +539,6 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) return -EINVAL; } - /* - * Out of range jumps are called from modules. - * Being that we are converting from nop, it had better - * already have a module defined. - */ - if (!rec->arch.mod) { - pr_err("No module loaded\n"); - return -EINVAL; - } - return __ftrace_make_call(rec, addr); } @@ -555,7 +551,10 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, ppc_inst_t op; unsigned long ip = rec->ip; unsigned long entry, ptr, tramp; - struct module *mod = rec->arch.mod; + struct module *mod = ftrace_lookup_module(rec); + + if (!mod) + return -EINVAL; /* If we never set up ftrace trampolines, then bail */ if (!mod->arch.tramp || !mod->arch.tramp_regs) { @@ -668,14 +667,6 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, return -EINVAL; } - /* - * Out of range jumps are called from modules. - */ - if (!rec->arch.mod) { - pr_err("No module loaded\n"); - return -EINVAL; - } - return __ftrace_modify_call(rec, old_addr, addr); } #endif -- 2.45.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [RFC PATCH v2 2/5] powerpc/ftrace: Remove pointer to struct module from dyn_arch_ftrace 2024-06-10 8:38 ` [RFC PATCH v2 2/5] powerpc/ftrace: Remove pointer to struct module from dyn_arch_ftrace Naveen N Rao @ 2024-06-10 20:03 ` Steven Rostedt 2024-06-11 14:45 ` Naveen N Rao 0 siblings, 1 reply; 16+ messages in thread From: Steven Rostedt @ 2024-06-10 20:03 UTC (permalink / raw) To: Naveen N Rao Cc: Mark Rutland, Masahiro Yamada, Nicholas Piggin, linuxppc-dev, Masami Hiramatsu On Mon, 10 Jun 2024 14:08:15 +0530 Naveen N Rao <naveen@kernel.org> wrote: > Pointer to struct module is only relevant for ftrace records belonging > to kernel modules. Having this field in dyn_arch_ftrace wastes memory > for all ftrace records belonging to the kernel. Remove the same in > favour of looking up the module from the ftrace record address, similar > to other architectures. > > Signed-off-by: Naveen N Rao <naveen@kernel.org> > --- > arch/powerpc/include/asm/ftrace.h | 1 - > arch/powerpc/kernel/trace/ftrace.c | 47 ++++++++++----- > arch/powerpc/kernel/trace/ftrace_64_pg.c | 73 +++++++++++------------- > 3 files changed, 64 insertions(+), 57 deletions(-) > > diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h > index 107fc5a48456..201f9d15430a 100644 > --- a/arch/powerpc/include/asm/ftrace.h > +++ b/arch/powerpc/include/asm/ftrace.h > @@ -26,7 +26,6 @@ unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip, > struct module; > struct dyn_ftrace; > struct dyn_arch_ftrace { > - struct module *mod; > }; Nice. I always hated that extra field. > > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS > diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c > index d8d6b4fd9a14..041be965485e 100644 > --- a/arch/powerpc/kernel/trace/ftrace.c > +++ b/arch/powerpc/kernel/trace/ftrace.c > @@ -106,20 +106,36 @@ static unsigned long find_ftrace_tramp(unsigned long ip) > return 0; > } > > +static struct module *ftrace_lookup_module(struct dyn_ftrace *rec) > +{ > + struct module *mod = NULL; > + > +#ifdef CONFIG_MODULES > + /* > + * NOTE: __module_text_address() must be called with preemption > + * disabled, but we can rely on ftrace_lock to ensure that 'mod' > + * retains its validity throughout the remainder of this code. > + */ > + preempt_disable(); > + mod = __module_text_address(rec->ip); > + preempt_enable(); > + > + if (!mod) > + pr_err("No module loaded at addr=%lx\n", rec->ip); > +#endif > + > + return mod; > +} It may look nicer to have: #ifdef CONFIG_MODULES static struct module *ftrace_lookup_module(struct dyn_ftrace *rec) { struct module *mod = NULL; /* * NOTE: __module_text_address() must be called with preemption * disabled, but we can rely on ftrace_lock to ensure that 'mod' * retains its validity throughout the remainder of this code. */ preempt_disable(); mod = __module_text_address(rec->ip); preempt_enable(); if (!mod) pr_err("No module loaded at addr=%lx\n", rec->ip); return mod; } #else static inline struct module *ftrace_lookup_module(struct dyn_ftrace *rec) { return NULL; } #endif > + > static int ftrace_get_call_inst(struct dyn_ftrace *rec, unsigned long addr, ppc_inst_t *call_inst) > { > unsigned long ip = rec->ip; > unsigned long stub; > + struct module *mod; > > if (is_offset_in_branch_range(addr - ip)) { > /* Within range */ > stub = addr; > -#ifdef CONFIG_MODULES > - } else if (rec->arch.mod) { > - /* Module code would be going to one of the module stubs */ > - stub = (addr == (unsigned long)ftrace_caller ? rec->arch.mod->arch.tramp : > - rec->arch.mod->arch.tramp_regs); > -#endif > } else if (core_kernel_text(ip)) { > /* We would be branching to one of our ftrace stubs */ > stub = find_ftrace_tramp(ip); > @@ -128,7 +144,16 @@ static int ftrace_get_call_inst(struct dyn_ftrace *rec, unsigned long addr, ppc_ > return -EINVAL; > } > } else { > - return -EINVAL; > + mod = ftrace_lookup_module(rec); > + if (mod) { > +#ifdef CONFIG_MODULES > + /* Module code would be going to one of the module stubs */ > + stub = (addr == (unsigned long)ftrace_caller ? mod->arch.tramp : > + mod->arch.tramp_regs); > +#endif You have CONFIG_MODULES here and in ftrace_lookup_module() above, which would always return NULL. Could you combine the above to be done in ftrace_lookup_module() so that there's no #ifdef CONFIG_MODULES here? > + } else { > + return -EINVAL; > + } > } > > *call_inst = ftrace_create_branch_inst(ip, stub, 1); > @@ -256,14 +281,6 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec) > if (ret) > return ret; > > - if (!core_kernel_text(ip)) { > - if (!mod) { > - pr_err("0x%lx: No module provided for non-kernel address\n", ip); > - return -EFAULT; > - } > - rec->arch.mod = mod; > - } > - > /* Nop-out the ftrace location */ > new = ppc_inst(PPC_RAW_NOP()); > addr = MCOUNT_ADDR; -- Steve ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH v2 2/5] powerpc/ftrace: Remove pointer to struct module from dyn_arch_ftrace 2024-06-10 20:03 ` Steven Rostedt @ 2024-06-11 14:45 ` Naveen N Rao 0 siblings, 0 replies; 16+ messages in thread From: Naveen N Rao @ 2024-06-11 14:45 UTC (permalink / raw) To: Steven Rostedt Cc: Mark Rutland, Masahiro Yamada, Nicholas Piggin, Christophe Leroy, linuxppc-dev, Masami Hiramatsu On Mon, Jun 10, 2024 at 04:03:56PM GMT, Steven Rostedt wrote: > On Mon, 10 Jun 2024 14:08:15 +0530 > Naveen N Rao <naveen@kernel.org> wrote: > > > Pointer to struct module is only relevant for ftrace records belonging > > to kernel modules. Having this field in dyn_arch_ftrace wastes memory > > for all ftrace records belonging to the kernel. Remove the same in > > favour of looking up the module from the ftrace record address, similar > > to other architectures. > > > > Signed-off-by: Naveen N Rao <naveen@kernel.org> > > --- > > arch/powerpc/include/asm/ftrace.h | 1 - > > arch/powerpc/kernel/trace/ftrace.c | 47 ++++++++++----- > > arch/powerpc/kernel/trace/ftrace_64_pg.c | 73 +++++++++++------------- > > 3 files changed, 64 insertions(+), 57 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h > > index 107fc5a48456..201f9d15430a 100644 > > --- a/arch/powerpc/include/asm/ftrace.h > > +++ b/arch/powerpc/include/asm/ftrace.h > > @@ -26,7 +26,6 @@ unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip, > > struct module; > > struct dyn_ftrace; > > struct dyn_arch_ftrace { > > - struct module *mod; > > }; > > Nice. I always hated that extra field. It was your complaint a while back that prompted this change :) Though I introduce a different pointer here in the next patch. /me ducks. > > > > > > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS > > diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c > > index d8d6b4fd9a14..041be965485e 100644 > > --- a/arch/powerpc/kernel/trace/ftrace.c > > +++ b/arch/powerpc/kernel/trace/ftrace.c > > @@ -106,20 +106,36 @@ static unsigned long find_ftrace_tramp(unsigned long ip) > > return 0; > > } > > > > +static struct module *ftrace_lookup_module(struct dyn_ftrace *rec) > > +{ > > + struct module *mod = NULL; > > + > > +#ifdef CONFIG_MODULES > > + /* > > + * NOTE: __module_text_address() must be called with preemption > > + * disabled, but we can rely on ftrace_lock to ensure that 'mod' > > + * retains its validity throughout the remainder of this code. > > + */ > > + preempt_disable(); > > + mod = __module_text_address(rec->ip); > > + preempt_enable(); > > + > > + if (!mod) > > + pr_err("No module loaded at addr=%lx\n", rec->ip); > > +#endif > > + > > + return mod; > > +} > > It may look nicer to have: > > #ifdef CONFIG_MODULES > static struct module *ftrace_lookup_module(struct dyn_ftrace *rec) > { > struct module *mod = NULL; > > /* > * NOTE: __module_text_address() must be called with preemption > * disabled, but we can rely on ftrace_lock to ensure that 'mod' > * retains its validity throughout the remainder of this code. > */ > preempt_disable(); > mod = __module_text_address(rec->ip); > preempt_enable(); > > if (!mod) > pr_err("No module loaded at addr=%lx\n", rec->ip); > > return mod; > } > #else > static inline struct module *ftrace_lookup_module(struct dyn_ftrace *rec) > { > return NULL; > } > #endif I wrote this, and then I thought it will be simpler to do the version I posted. I will move back to this since it looks to be the preferred way. > > > + > > static int ftrace_get_call_inst(struct dyn_ftrace *rec, unsigned long addr, ppc_inst_t *call_inst) > > { > > unsigned long ip = rec->ip; > > unsigned long stub; > > + struct module *mod; > > > > if (is_offset_in_branch_range(addr - ip)) { > > /* Within range */ > > stub = addr; > > -#ifdef CONFIG_MODULES > > - } else if (rec->arch.mod) { > > - /* Module code would be going to one of the module stubs */ > > - stub = (addr == (unsigned long)ftrace_caller ? rec->arch.mod->arch.tramp : > > - rec->arch.mod->arch.tramp_regs); > > -#endif > > } else if (core_kernel_text(ip)) { > > /* We would be branching to one of our ftrace stubs */ > > stub = find_ftrace_tramp(ip); > > @@ -128,7 +144,16 @@ static int ftrace_get_call_inst(struct dyn_ftrace *rec, unsigned long addr, ppc_ > > return -EINVAL; > > } > > } else { > > - return -EINVAL; > > + mod = ftrace_lookup_module(rec); > > + if (mod) { > > +#ifdef CONFIG_MODULES > > + /* Module code would be going to one of the module stubs */ > > + stub = (addr == (unsigned long)ftrace_caller ? mod->arch.tramp : > > + mod->arch.tramp_regs); > > +#endif > > You have CONFIG_MODULES here and in ftrace_lookup_module() above, which > would always return NULL. Could you combine the above to be done in > ftrace_lookup_module() so that there's no #ifdef CONFIG_MODULES here? Yes, indeed. That will look cleaner. Thanks, Naveen ^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC PATCH v2 3/5] powerpc/ftrace: Unify 32-bit and 64-bit ftrace entry code 2024-06-10 8:38 [RFC PATCH v2 0/5] powerpc/ftrace: Move ftrace sequence out of line Naveen N Rao 2024-06-10 8:38 ` [RFC PATCH v2 1/5] powerpc/kprobes: Use ftrace to determine if a probe is at function entry Naveen N Rao 2024-06-10 8:38 ` [RFC PATCH v2 2/5] powerpc/ftrace: Remove pointer to struct module from dyn_arch_ftrace Naveen N Rao @ 2024-06-10 8:38 ` Naveen N Rao 2024-06-10 20:06 ` Steven Rostedt 2024-06-10 8:38 ` [RFC PATCH v2 4/5] kbuild: Add generic hook for architectures to use before the final vmlinux link Naveen N Rao 2024-06-10 8:38 ` [RFC PATCH v2 5/5] powerpc64/ftrace: Move ftrace sequence out of line Naveen N Rao 4 siblings, 1 reply; 16+ messages in thread From: Naveen N Rao @ 2024-06-10 8:38 UTC (permalink / raw) To: linuxppc-dev Cc: Mark Rutland, Masahiro Yamada, Steven Rostedt, Nicholas Piggin, Masami Hiramatsu On 32-bit powerpc, gcc generates a three instruction sequence for function profiling: mflr r0 stw r0, 4(r1) bl _mcount On kernel boot, the call to _mcount() is nop-ed out, to be patched back in when ftrace is actually enabled. The 'stw' instruction therefore is not necessary unless ftrace is enabled. Nop it out during ftrace init. When ftrace is enabled, we want the 'stw' so that stack unwinding works properly. Perform the same within the ftrace handler, similar to 64-bit powerpc. For 64-bit powerpc, early versions of gcc used to emit a three instruction sequence for function profiling (with -mprofile-kernel) with a 'std' instruction to mimic the 'stw' above. Address that scenario also by nop-ing out the 'std' instruction during ftrace init. Signed-off-by: Naveen N Rao <naveen@kernel.org> --- arch/powerpc/kernel/trace/ftrace.c | 6 ++++-- arch/powerpc/kernel/trace/ftrace_entry.S | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c index 041be965485e..2e1667a578ff 100644 --- a/arch/powerpc/kernel/trace/ftrace.c +++ b/arch/powerpc/kernel/trace/ftrace.c @@ -266,13 +266,15 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec) /* Expected sequence: 'mflr r0', 'stw r0,4(r1)', 'bl _mcount' */ ret = ftrace_validate_inst(ip - 8, ppc_inst(PPC_RAW_MFLR(_R0))); if (!ret) - ret = ftrace_validate_inst(ip - 4, ppc_inst(PPC_RAW_STW(_R0, _R1, 4))); + ret = ftrace_modify_code(ip - 4, ppc_inst(PPC_RAW_STW(_R0, _R1, 4)), + ppc_inst(PPC_RAW_NOP())); } else if (IS_ENABLED(CONFIG_MPROFILE_KERNEL)) { /* Expected sequence: 'mflr r0', ['std r0,16(r1)'], 'bl _mcount' */ ret = ftrace_read_inst(ip - 4, &old); if (!ret && !ppc_inst_equal(old, ppc_inst(PPC_RAW_MFLR(_R0)))) { ret = ftrace_validate_inst(ip - 8, ppc_inst(PPC_RAW_MFLR(_R0))); - ret |= ftrace_validate_inst(ip - 4, ppc_inst(PPC_RAW_STD(_R0, _R1, 16))); + ret |= ftrace_modify_code(ip - 4, ppc_inst(PPC_RAW_STD(_R0, _R1, 16)), + ppc_inst(PPC_RAW_NOP())); } } else { return -EINVAL; diff --git a/arch/powerpc/kernel/trace/ftrace_entry.S b/arch/powerpc/kernel/trace/ftrace_entry.S index 76dbe9fd2c0f..244a1c7bb1e8 100644 --- a/arch/powerpc/kernel/trace/ftrace_entry.S +++ b/arch/powerpc/kernel/trace/ftrace_entry.S @@ -33,6 +33,8 @@ * and then arrange for the ftrace function to be called. */ .macro ftrace_regs_entry allregs + /* Save the original return address in A's stack frame */ + PPC_STL r0, LRSAVE(r1) /* Create a minimal stack frame for representing B */ PPC_STLU r1, -STACK_FRAME_MIN_SIZE(r1) @@ -44,8 +46,6 @@ SAVE_GPRS(3, 10, r1) #ifdef CONFIG_PPC64 - /* Save the original return address in A's stack frame */ - std r0, LRSAVE+SWITCH_FRAME_SIZE+STACK_FRAME_MIN_SIZE(r1) /* Ok to continue? */ lbz r3, PACA_FTRACE_ENABLED(r13) cmpdi r3, 0 -- 2.45.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [RFC PATCH v2 3/5] powerpc/ftrace: Unify 32-bit and 64-bit ftrace entry code 2024-06-10 8:38 ` [RFC PATCH v2 3/5] powerpc/ftrace: Unify 32-bit and 64-bit ftrace entry code Naveen N Rao @ 2024-06-10 20:06 ` Steven Rostedt 2024-06-11 14:47 ` Naveen N Rao 0 siblings, 1 reply; 16+ messages in thread From: Steven Rostedt @ 2024-06-10 20:06 UTC (permalink / raw) To: Naveen N Rao Cc: Mark Rutland, Masahiro Yamada, Nicholas Piggin, linuxppc-dev, Masami Hiramatsu On Mon, 10 Jun 2024 14:08:16 +0530 Naveen N Rao <naveen@kernel.org> wrote: > On 32-bit powerpc, gcc generates a three instruction sequence for > function profiling: > mflr r0 > stw r0, 4(r1) > bl _mcount > > On kernel boot, the call to _mcount() is nop-ed out, to be patched back > in when ftrace is actually enabled. The 'stw' instruction therefore is > not necessary unless ftrace is enabled. Nop it out during ftrace init. > > When ftrace is enabled, we want the 'stw' so that stack unwinding works > properly. Perform the same within the ftrace handler, similar to 64-bit > powerpc. > > For 64-bit powerpc, early versions of gcc used to emit a three > instruction sequence for function profiling (with -mprofile-kernel) with > a 'std' instruction to mimic the 'stw' above. Address that scenario also > by nop-ing out the 'std' instruction during ftrace init. > > Signed-off-by: Naveen N Rao <naveen@kernel.org> Isn't there still the race that there's a preemption between the: stw r0, 4(r1) and bl _mcount And if this breaks stack unwinding, couldn't this cause an issue for live kernel patching? I know it's very unlikely, but in theory, I think the race exists. -- Steve ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH v2 3/5] powerpc/ftrace: Unify 32-bit and 64-bit ftrace entry code 2024-06-10 20:06 ` Steven Rostedt @ 2024-06-11 14:47 ` Naveen N Rao 2024-06-11 15:14 ` Steven Rostedt 0 siblings, 1 reply; 16+ messages in thread From: Naveen N Rao @ 2024-06-11 14:47 UTC (permalink / raw) To: Steven Rostedt Cc: Mark Rutland, Masahiro Yamada, Nicholas Piggin, Christophe Leroy, linuxppc-dev, Masami Hiramatsu On Mon, Jun 10, 2024 at 04:06:32PM GMT, Steven Rostedt wrote: > On Mon, 10 Jun 2024 14:08:16 +0530 > Naveen N Rao <naveen@kernel.org> wrote: > > > On 32-bit powerpc, gcc generates a three instruction sequence for > > function profiling: > > mflr r0 > > stw r0, 4(r1) > > bl _mcount > > > > On kernel boot, the call to _mcount() is nop-ed out, to be patched back > > in when ftrace is actually enabled. The 'stw' instruction therefore is > > not necessary unless ftrace is enabled. Nop it out during ftrace init. > > > > When ftrace is enabled, we want the 'stw' so that stack unwinding works > > properly. Perform the same within the ftrace handler, similar to 64-bit > > powerpc. > > > > For 64-bit powerpc, early versions of gcc used to emit a three > > instruction sequence for function profiling (with -mprofile-kernel) with > > a 'std' instruction to mimic the 'stw' above. Address that scenario also > > by nop-ing out the 'std' instruction during ftrace init. > > > > Signed-off-by: Naveen N Rao <naveen@kernel.org> > > Isn't there still the race that there's a preemption between the: > > stw r0, 4(r1) > and > bl _mcount > > And if this breaks stack unwinding, couldn't this cause an issue for live > kernel patching? > > I know it's very unlikely, but in theory, I think the race exists. I *think* you are assuming that we will be patching back the 'stw' instruction here? So, there could be an issue if a cpu has executed the nop instead of 'stw' and then sees the call to _mcount(). But, we don't patch back the 'stw' instruction. That is instead done as part of ftrace_caller(), along with setting up an additional stack frame to ensure reliable stack unwinding. Commit 41a506ef71eb ("powerpc/ftrace: Create a dummy stackframe to fix stack unwind") has more details. The primary motivation for this patch is to address differences in the function profile sequence with various toolchains. Since commit 0f71dcfb4aef ("powerpc/ftrace: Add support for -fpatchable-function-entry"), we use the same two-instruction profile sequence across 32-bit and 64-bit powerpc: mflr r0 bl ftrace_caller This has also been true on 64-bit powerpc with -mprofile-kernel, except the very early versions of gcc that supported that option (gcc v5). On 32-bit powerpc, we used to use the three instruction sequence before support for -fpatchable-function-entry was introduced. In this patch, we move all toolchain variants to use the two-instruction sequence for consistency. Thanks, Naveen ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH v2 3/5] powerpc/ftrace: Unify 32-bit and 64-bit ftrace entry code 2024-06-11 14:47 ` Naveen N Rao @ 2024-06-11 15:14 ` Steven Rostedt 0 siblings, 0 replies; 16+ messages in thread From: Steven Rostedt @ 2024-06-11 15:14 UTC (permalink / raw) To: Naveen N Rao Cc: Mark Rutland, Masahiro Yamada, Nicholas Piggin, Christophe Leroy, linuxppc-dev, Masami Hiramatsu On Tue, 11 Jun 2024 20:17:19 +0530 Naveen N Rao <naveen@kernel.org> wrote: > > I know it's very unlikely, but in theory, I think the race exists. > > I *think* you are assuming that we will be patching back the 'stw' Yes, that was what I was assuming :-p > instruction here? So, there could be an issue if a cpu has executed the > nop instead of 'stw' and then sees the call to _mcount(). > > But, we don't patch back the 'stw' instruction. That is instead done as > part of ftrace_caller(), along with setting up an additional stack frame > to ensure reliable stack unwinding. Commit 41a506ef71eb > ("powerpc/ftrace: Create a dummy stackframe to fix stack unwind") has > more details. > > The primary motivation for this patch is to address differences in the > function profile sequence with various toolchains. Since commit > 0f71dcfb4aef ("powerpc/ftrace: Add support for > -fpatchable-function-entry"), we use the same two-instruction profile > sequence across 32-bit and 64-bit powerpc: > mflr r0 > bl ftrace_caller > > This has also been true on 64-bit powerpc with -mprofile-kernel, except > the very early versions of gcc that supported that option (gcc v5). > > On 32-bit powerpc, we used to use the three instruction sequence before > support for -fpatchable-function-entry was introduced. > > In this patch, we move all toolchain variants to use the two-instruction > sequence for consistency. OK, if you are not patching that back, then all should be good. -- Steve ^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC PATCH v2 4/5] kbuild: Add generic hook for architectures to use before the final vmlinux link 2024-06-10 8:38 [RFC PATCH v2 0/5] powerpc/ftrace: Move ftrace sequence out of line Naveen N Rao ` (2 preceding siblings ...) 2024-06-10 8:38 ` [RFC PATCH v2 3/5] powerpc/ftrace: Unify 32-bit and 64-bit ftrace entry code Naveen N Rao @ 2024-06-10 8:38 ` Naveen N Rao 2024-06-10 9:14 ` Masahiro Yamada 2024-06-10 8:38 ` [RFC PATCH v2 5/5] powerpc64/ftrace: Move ftrace sequence out of line Naveen N Rao 4 siblings, 1 reply; 16+ messages in thread From: Naveen N Rao @ 2024-06-10 8:38 UTC (permalink / raw) To: linuxppc-dev Cc: Mark Rutland, Masahiro Yamada, Steven Rostedt, Nicholas Piggin, Masami Hiramatsu On powerpc, we would like to be able to make a pass on vmlinux.o and generate a new object file to be linked into vmlinux. Add a generic pass in link-vmlinux.sh that architectures can use for this purpose. Architectures need to select CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX and must provide arch/<arch>/tools/vmlinux_o.sh, which will be invoked prior to the final vmlinux link step. Signed-off-by: Naveen N Rao <naveen@kernel.org> --- arch/Kconfig | 3 +++ scripts/link-vmlinux.sh | 18 +++++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index 975dd22a2dbd..649f0903e7ef 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -1643,4 +1643,7 @@ config CC_HAS_SANE_FUNCTION_ALIGNMENT config ARCH_NEED_CMPXCHG_1_EMU bool +config ARCH_WANTS_PRE_LINK_VMLINUX + def_bool n + endmenu diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index 46ce5d04dbeb..07f70e105d82 100755 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -122,7 +122,7 @@ gen_btf() return 1 fi - vmlinux_link ${1} + vmlinux_link ${1} ${arch_vmlinux_o} info "BTF" ${2} LLVM_OBJCOPY="${OBJCOPY}" ${PAHOLE} -J ${PAHOLE_FLAGS} ${1} @@ -178,7 +178,7 @@ kallsyms_step() kallsymso=${kallsyms_vmlinux}.o kallsyms_S=${kallsyms_vmlinux}.S - vmlinux_link ${kallsyms_vmlinux} "${kallsymso_prev}" ${btf_vmlinux_bin_o} + vmlinux_link ${kallsyms_vmlinux} "${kallsymso_prev}" ${btf_vmlinux_bin_o} ${arch_vmlinux_o} mksysmap ${kallsyms_vmlinux} ${kallsyms_vmlinux}.syms kallsyms ${kallsyms_vmlinux}.syms ${kallsyms_S} @@ -203,6 +203,7 @@ sorttable() cleanup() { + rm -f .arch.vmlinux.* rm -f .btf.* rm -f System.map rm -f vmlinux @@ -223,6 +224,17 @@ fi ${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init init/version-timestamp.o +arch_vmlinux_o="" +if is_enabled CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX; then + arch_vmlinux_o=.arch.vmlinux.o + info "ARCH" ${arch_vmlinux_o} + if ! ${srctree}/arch/${SRCARCH}/tools/vmlinux_o.sh ${arch_vmlinux_o} ; then + echo >&2 "Failed to generate ${arch_vmlinux_o}" + echo >&2 "Try to disable CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX" + exit 1 + fi +fi + btf_vmlinux_bin_o="" if is_enabled CONFIG_DEBUG_INFO_BTF; then btf_vmlinux_bin_o=.btf.vmlinux.bin.o @@ -273,7 +285,7 @@ if is_enabled CONFIG_KALLSYMS; then fi fi -vmlinux_link vmlinux "${kallsymso}" ${btf_vmlinux_bin_o} +vmlinux_link vmlinux "${kallsymso}" ${btf_vmlinux_bin_o} ${arch_vmlinux_o} # fill in BTF IDs if is_enabled CONFIG_DEBUG_INFO_BTF && is_enabled CONFIG_BPF; then -- 2.45.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [RFC PATCH v2 4/5] kbuild: Add generic hook for architectures to use before the final vmlinux link 2024-06-10 8:38 ` [RFC PATCH v2 4/5] kbuild: Add generic hook for architectures to use before the final vmlinux link Naveen N Rao @ 2024-06-10 9:14 ` Masahiro Yamada 2024-06-10 17:16 ` Naveen N Rao 0 siblings, 1 reply; 16+ messages in thread From: Masahiro Yamada @ 2024-06-10 9:14 UTC (permalink / raw) To: Naveen N Rao Cc: Mark Rutland, Steven Rostedt, Nicholas Piggin, linuxppc-dev, Masami Hiramatsu On Mon, Jun 10, 2024 at 5:39 PM Naveen N Rao <naveen@kernel.org> wrote: > > On powerpc, we would like to be able to make a pass on vmlinux.o and > generate a new object file to be linked into vmlinux. Add a generic pass > in link-vmlinux.sh that architectures can use for this purpose. > Architectures need to select CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX and must > provide arch/<arch>/tools/vmlinux_o.sh, which will be invoked prior to > the final vmlinux link step. > > Signed-off-by: Naveen N Rao <naveen@kernel.org> > --- > arch/Kconfig | 3 +++ > scripts/link-vmlinux.sh | 18 +++++++++++++++--- > 2 files changed, 18 insertions(+), 3 deletions(-) > > diff --git a/arch/Kconfig b/arch/Kconfig > index 975dd22a2dbd..649f0903e7ef 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -1643,4 +1643,7 @@ config CC_HAS_SANE_FUNCTION_ALIGNMENT > config ARCH_NEED_CMPXCHG_1_EMU > bool > > +config ARCH_WANTS_PRE_LINK_VMLINUX > + def_bool n > + > endmenu > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > index 46ce5d04dbeb..07f70e105d82 100755 > --- a/scripts/link-vmlinux.sh > +++ b/scripts/link-vmlinux.sh > @@ -122,7 +122,7 @@ gen_btf() > return 1 > fi > > - vmlinux_link ${1} > + vmlinux_link ${1} ${arch_vmlinux_o} > > info "BTF" ${2} > LLVM_OBJCOPY="${OBJCOPY}" ${PAHOLE} -J ${PAHOLE_FLAGS} ${1} > @@ -178,7 +178,7 @@ kallsyms_step() > kallsymso=${kallsyms_vmlinux}.o > kallsyms_S=${kallsyms_vmlinux}.S > > - vmlinux_link ${kallsyms_vmlinux} "${kallsymso_prev}" ${btf_vmlinux_bin_o} > + vmlinux_link ${kallsyms_vmlinux} "${kallsymso_prev}" ${btf_vmlinux_bin_o} ${arch_vmlinux_o} > mksysmap ${kallsyms_vmlinux} ${kallsyms_vmlinux}.syms > kallsyms ${kallsyms_vmlinux}.syms ${kallsyms_S} > > @@ -203,6 +203,7 @@ sorttable() > > cleanup() > { > + rm -f .arch.vmlinux.* > rm -f .btf.* > rm -f System.map > rm -f vmlinux > @@ -223,6 +224,17 @@ fi > > ${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init init/version-timestamp.o > > +arch_vmlinux_o="" > +if is_enabled CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX; then > + arch_vmlinux_o=.arch.vmlinux.o > + info "ARCH" ${arch_vmlinux_o} > + if ! ${srctree}/arch/${SRCARCH}/tools/vmlinux_o.sh ${arch_vmlinux_o} ; then > + echo >&2 "Failed to generate ${arch_vmlinux_o}" > + echo >&2 "Try to disable CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX" > + exit 1 > + fi > +fi This is wrong because scripts/link-vmlinux.sh is not triggered even when source files under arch/powerpc/tools/ are changed. Presumably, scripts/Makefile.vmlinux will be the right place. > + > btf_vmlinux_bin_o="" > if is_enabled CONFIG_DEBUG_INFO_BTF; then > btf_vmlinux_bin_o=.btf.vmlinux.bin.o > @@ -273,7 +285,7 @@ if is_enabled CONFIG_KALLSYMS; then > fi > fi > > -vmlinux_link vmlinux "${kallsymso}" ${btf_vmlinux_bin_o} > +vmlinux_link vmlinux "${kallsymso}" ${btf_vmlinux_bin_o} ${arch_vmlinux_o} > > # fill in BTF IDs > if is_enabled CONFIG_DEBUG_INFO_BTF && is_enabled CONFIG_BPF; then > -- > 2.45.2 > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH v2 4/5] kbuild: Add generic hook for architectures to use before the final vmlinux link 2024-06-10 9:14 ` Masahiro Yamada @ 2024-06-10 17:16 ` Naveen N Rao 2024-06-10 21:51 ` Masahiro Yamada 0 siblings, 1 reply; 16+ messages in thread From: Naveen N Rao @ 2024-06-10 17:16 UTC (permalink / raw) To: Masahiro Yamada Cc: Mark Rutland, Steven Rostedt, Nicholas Piggin, linuxppc-dev, Masami Hiramatsu On Mon, Jun 10, 2024 at 06:14:51PM GMT, Masahiro Yamada wrote: > On Mon, Jun 10, 2024 at 5:39 PM Naveen N Rao <naveen@kernel.org> wrote: > > > > On powerpc, we would like to be able to make a pass on vmlinux.o and > > generate a new object file to be linked into vmlinux. Add a generic pass > > in link-vmlinux.sh that architectures can use for this purpose. > > Architectures need to select CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX and must > > provide arch/<arch>/tools/vmlinux_o.sh, which will be invoked prior to > > the final vmlinux link step. > > > > Signed-off-by: Naveen N Rao <naveen@kernel.org> > > --- > > arch/Kconfig | 3 +++ > > scripts/link-vmlinux.sh | 18 +++++++++++++++--- > > 2 files changed, 18 insertions(+), 3 deletions(-) > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > index 975dd22a2dbd..649f0903e7ef 100644 > > --- a/arch/Kconfig > > +++ b/arch/Kconfig > > @@ -1643,4 +1643,7 @@ config CC_HAS_SANE_FUNCTION_ALIGNMENT > > config ARCH_NEED_CMPXCHG_1_EMU > > bool > > > > +config ARCH_WANTS_PRE_LINK_VMLINUX > > + def_bool n > > + > > endmenu > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > > index 46ce5d04dbeb..07f70e105d82 100755 > > --- a/scripts/link-vmlinux.sh > > +++ b/scripts/link-vmlinux.sh ... > > > > +arch_vmlinux_o="" > > +if is_enabled CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX; then > > + arch_vmlinux_o=.arch.vmlinux.o > > + info "ARCH" ${arch_vmlinux_o} > > + if ! ${srctree}/arch/${SRCARCH}/tools/vmlinux_o.sh ${arch_vmlinux_o} ; then > > + echo >&2 "Failed to generate ${arch_vmlinux_o}" > > + echo >&2 "Try to disable CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX" > > + exit 1 > > + fi > > +fi > > > > This is wrong because scripts/link-vmlinux.sh is not triggered > even when source files under arch/powerpc/tools/ are changed. > > Presumably, scripts/Makefile.vmlinux will be the right place. Ah, yes. Something like this? diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux index 49946cb96844..77d90b6ac53e 100644 --- a/scripts/Makefile.vmlinux +++ b/scripts/Makefile.vmlinux @@ -22,6 +22,10 @@ targets += .vmlinux.export.o vmlinux: .vmlinux.export.o endif +ifdef CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX +vmlinux: $(srctree)/arch/$(SRCARCH)/tools/vmlinux_o.sh +endif + ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) # Final link of vmlinux with optional arch pass after final link Thanks, Naveen ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [RFC PATCH v2 4/5] kbuild: Add generic hook for architectures to use before the final vmlinux link 2024-06-10 17:16 ` Naveen N Rao @ 2024-06-10 21:51 ` Masahiro Yamada 2024-06-11 17:35 ` Naveen N Rao 0 siblings, 1 reply; 16+ messages in thread From: Masahiro Yamada @ 2024-06-10 21:51 UTC (permalink / raw) To: Naveen N Rao Cc: Mark Rutland, Steven Rostedt, Nicholas Piggin, linuxppc-dev, Masami Hiramatsu On Tue, Jun 11, 2024 at 2:20 AM Naveen N Rao <naveen@kernel.org> wrote: > > On Mon, Jun 10, 2024 at 06:14:51PM GMT, Masahiro Yamada wrote: > > On Mon, Jun 10, 2024 at 5:39 PM Naveen N Rao <naveen@kernel.org> wrote: > > > > > > On powerpc, we would like to be able to make a pass on vmlinux.o and > > > generate a new object file to be linked into vmlinux. Add a generic pass > > > in link-vmlinux.sh that architectures can use for this purpose. > > > Architectures need to select CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX and must > > > provide arch/<arch>/tools/vmlinux_o.sh, which will be invoked prior to > > > the final vmlinux link step. > > > > > > Signed-off-by: Naveen N Rao <naveen@kernel.org> > > > --- > > > arch/Kconfig | 3 +++ > > > scripts/link-vmlinux.sh | 18 +++++++++++++++--- > > > 2 files changed, 18 insertions(+), 3 deletions(-) > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > index 975dd22a2dbd..649f0903e7ef 100644 > > > --- a/arch/Kconfig > > > +++ b/arch/Kconfig > > > @@ -1643,4 +1643,7 @@ config CC_HAS_SANE_FUNCTION_ALIGNMENT > > > config ARCH_NEED_CMPXCHG_1_EMU > > > bool > > > > > > +config ARCH_WANTS_PRE_LINK_VMLINUX > > > + def_bool n > > > + > > > endmenu > > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > > > index 46ce5d04dbeb..07f70e105d82 100755 > > > --- a/scripts/link-vmlinux.sh > > > +++ b/scripts/link-vmlinux.sh > ... > > > > > > +arch_vmlinux_o="" > > > +if is_enabled CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX; then > > > + arch_vmlinux_o=.arch.vmlinux.o > > > + info "ARCH" ${arch_vmlinux_o} > > > + if ! ${srctree}/arch/${SRCARCH}/tools/vmlinux_o.sh ${arch_vmlinux_o} ; then > > > + echo >&2 "Failed to generate ${arch_vmlinux_o}" > > > + echo >&2 "Try to disable CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX" > > > + exit 1 > > > + fi > > > +fi > > > > > > > > This is wrong because scripts/link-vmlinux.sh is not triggered > > even when source files under arch/powerpc/tools/ are changed. > > > > Presumably, scripts/Makefile.vmlinux will be the right place. > > Ah, yes. Something like this? > > diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux > index 49946cb96844..77d90b6ac53e 100644 > --- a/scripts/Makefile.vmlinux > +++ b/scripts/Makefile.vmlinux > @@ -22,6 +22,10 @@ targets += .vmlinux.export.o > vmlinux: .vmlinux.export.o > endif > > +ifdef CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX > +vmlinux: $(srctree)/arch/$(SRCARCH)/tools/vmlinux_o.sh > +endif > + > ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) > > # Final link of vmlinux with optional arch pass after final link > > > Thanks, > Naveen > No. Something like below. Then, you can do everything in Makefile, not a shell script. ifdef CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX vmlinux: .arch.vmlinux.o .arch.vmlinux.o: FORCE $(Q)$(MAKE) $(build)=arch/$(SRCARCH)/tools .arch.vmlinux.o endif I did not test it, though. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH v2 4/5] kbuild: Add generic hook for architectures to use before the final vmlinux link 2024-06-10 21:51 ` Masahiro Yamada @ 2024-06-11 17:35 ` Naveen N Rao 2024-06-12 10:52 ` Naveen N Rao 0 siblings, 1 reply; 16+ messages in thread From: Naveen N Rao @ 2024-06-11 17:35 UTC (permalink / raw) To: Masahiro Yamada Cc: Mark Rutland, Steven Rostedt, Nicholas Piggin, Christophe Leroy, linuxppc-dev, Masami Hiramatsu On Tue, Jun 11, 2024 at 06:51:51AM GMT, Masahiro Yamada wrote: > On Tue, Jun 11, 2024 at 2:20 AM Naveen N Rao <naveen@kernel.org> wrote: > > > > On Mon, Jun 10, 2024 at 06:14:51PM GMT, Masahiro Yamada wrote: > > > On Mon, Jun 10, 2024 at 5:39 PM Naveen N Rao <naveen@kernel.org> wrote: > > > > > > > > On powerpc, we would like to be able to make a pass on vmlinux.o and > > > > generate a new object file to be linked into vmlinux. Add a generic pass > > > > in link-vmlinux.sh that architectures can use for this purpose. > > > > Architectures need to select CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX and must > > > > provide arch/<arch>/tools/vmlinux_o.sh, which will be invoked prior to > > > > the final vmlinux link step. > > > > > > > > Signed-off-by: Naveen N Rao <naveen@kernel.org> > > > > --- > > > > arch/Kconfig | 3 +++ > > > > scripts/link-vmlinux.sh | 18 +++++++++++++++--- > > > > 2 files changed, 18 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > > index 975dd22a2dbd..649f0903e7ef 100644 > > > > --- a/arch/Kconfig > > > > +++ b/arch/Kconfig > > > > @@ -1643,4 +1643,7 @@ config CC_HAS_SANE_FUNCTION_ALIGNMENT > > > > config ARCH_NEED_CMPXCHG_1_EMU > > > > bool > > > > > > > > +config ARCH_WANTS_PRE_LINK_VMLINUX > > > > + def_bool n > > > > + > > > > endmenu > > > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > > > > index 46ce5d04dbeb..07f70e105d82 100755 > > > > --- a/scripts/link-vmlinux.sh > > > > +++ b/scripts/link-vmlinux.sh > > ... > > > > > > > > +arch_vmlinux_o="" > > > > +if is_enabled CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX; then > > > > + arch_vmlinux_o=.arch.vmlinux.o > > > > + info "ARCH" ${arch_vmlinux_o} > > > > + if ! ${srctree}/arch/${SRCARCH}/tools/vmlinux_o.sh ${arch_vmlinux_o} ; then > > > > + echo >&2 "Failed to generate ${arch_vmlinux_o}" > > > > + echo >&2 "Try to disable CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX" > > > > + exit 1 > > > > + fi > > > > +fi > > > > > > > > > > > > This is wrong because scripts/link-vmlinux.sh is not triggered > > > even when source files under arch/powerpc/tools/ are changed. > > > > > > Presumably, scripts/Makefile.vmlinux will be the right place. > > > > Ah, yes. Something like this? > > > > diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux > > index 49946cb96844..77d90b6ac53e 100644 > > --- a/scripts/Makefile.vmlinux > > +++ b/scripts/Makefile.vmlinux > > @@ -22,6 +22,10 @@ targets += .vmlinux.export.o > > vmlinux: .vmlinux.export.o > > endif > > > > +ifdef CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX > > +vmlinux: $(srctree)/arch/$(SRCARCH)/tools/vmlinux_o.sh > > +endif > > + > > ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) > > > > # Final link of vmlinux with optional arch pass after final link > > > > > > Thanks, > > Naveen > > > > > > No. > > Something like below. > > Then, you can do everything in Makefile, not a shell script. > > > > ifdef CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX > vmlinux: .arch.vmlinux.o > > .arch.vmlinux.o: FORCE > $(Q)$(MAKE) $(build)=arch/$(SRCARCH)/tools .arch.vmlinux.o > > endif > > > > I did not test it, though. Thanks for the pointer. I will try and build on that. Just to be completely sure, does the below incremetal diff on top of the existing patch capture your suggestion? --- diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux index 49946cb96844..04e088d7a1ca 100644 --- a/scripts/Makefile.vmlinux +++ b/scripts/Makefile.vmlinux @@ -22,6 +22,13 @@ targets += .vmlinux.export.o vmlinux: .vmlinux.export.o endif +ifdef CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX +vmlinux: .arch.vmlinux.o + +.arch.vmlinux.o: FORCE + $(Q)$(MAKE) $(build)=arch/$(SRCARCH)/tools .arch.vmlinux.o +endif + ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) # Final link of vmlinux with optional arch pass after final link diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index 07f70e105d82..f1b705b8cdca 100755 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -227,12 +227,6 @@ ${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init init/version-timestamp.o arch_vmlinux_o="" if is_enabled CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX; then arch_vmlinux_o=.arch.vmlinux.o - info "ARCH" ${arch_vmlinux_o} - if ! ${srctree}/arch/${SRCARCH}/tools/vmlinux_o.sh ${arch_vmlinux_o} ; then - echo >&2 "Failed to generate ${arch_vmlinux_o}" - echo >&2 "Try to disable CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX" - exit 1 - fi fi btf_vmlinux_bin_o="" Thanks, Naveen ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [RFC PATCH v2 4/5] kbuild: Add generic hook for architectures to use before the final vmlinux link 2024-06-11 17:35 ` Naveen N Rao @ 2024-06-12 10:52 ` Naveen N Rao 0 siblings, 0 replies; 16+ messages in thread From: Naveen N Rao @ 2024-06-12 10:52 UTC (permalink / raw) To: Masahiro Yamada Cc: Mark Rutland, Steven Rostedt, Nicholas Piggin, Christophe Leroy, linuxppc-dev, Masami Hiramatsu On Tue, Jun 11, 2024 at 11:05:56PM GMT, Naveen N Rao wrote: > On Tue, Jun 11, 2024 at 06:51:51AM GMT, Masahiro Yamada wrote: > > On Tue, Jun 11, 2024 at 2:20 AM Naveen N Rao <naveen@kernel.org> wrote: > > > > > > On Mon, Jun 10, 2024 at 06:14:51PM GMT, Masahiro Yamada wrote: > > > > On Mon, Jun 10, 2024 at 5:39 PM Naveen N Rao <naveen@kernel.org> wrote: > > > > > > > > > > +arch_vmlinux_o="" > > > > > +if is_enabled CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX; then > > > > > + arch_vmlinux_o=.arch.vmlinux.o > > > > > + info "ARCH" ${arch_vmlinux_o} > > > > > + if ! ${srctree}/arch/${SRCARCH}/tools/vmlinux_o.sh ${arch_vmlinux_o} ; then > > > > > + echo >&2 "Failed to generate ${arch_vmlinux_o}" > > > > > + echo >&2 "Try to disable CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX" > > > > > + exit 1 > > > > > + fi > > > > > +fi > > > > > > > > > > > > > > > > This is wrong because scripts/link-vmlinux.sh is not triggered > > > > even when source files under arch/powerpc/tools/ are changed. > > > > > > > > Presumably, scripts/Makefile.vmlinux will be the right place. > > > > > > Ah, yes. Something like this? > > > > > > diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux > > > index 49946cb96844..77d90b6ac53e 100644 > > > --- a/scripts/Makefile.vmlinux > > > +++ b/scripts/Makefile.vmlinux > > > @@ -22,6 +22,10 @@ targets += .vmlinux.export.o > > > vmlinux: .vmlinux.export.o > > > endif > > > > > > +ifdef CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX > > > +vmlinux: $(srctree)/arch/$(SRCARCH)/tools/vmlinux_o.sh > > > +endif > > > + > > > ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) > > > > > > # Final link of vmlinux with optional arch pass after final link > > > > > > > > > Thanks, > > > Naveen > > > > > > > > > > > No. > > > > Something like below. > > > > Then, you can do everything in Makefile, not a shell script. > > > > > > > > ifdef CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX > > vmlinux: .arch.vmlinux.o > > > > .arch.vmlinux.o: FORCE > > $(Q)$(MAKE) $(build)=arch/$(SRCARCH)/tools .arch.vmlinux.o > > > > endif > > > > > > > > I did not test it, though. > > Thanks for the pointer. I will try and build on that. > > Just to be completely sure, does the below incremetal diff on top of the > existing patch capture your suggestion? > > --- > diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux > index 49946cb96844..04e088d7a1ca 100644 > --- a/scripts/Makefile.vmlinux > +++ b/scripts/Makefile.vmlinux > @@ -22,6 +22,13 @@ targets += .vmlinux.export.o > vmlinux: .vmlinux.export.o > endif > > +ifdef CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX > +vmlinux: .arch.vmlinux.o > + > +.arch.vmlinux.o: FORCE > + $(Q)$(MAKE) $(build)=arch/$(SRCARCH)/tools .arch.vmlinux.o > +endif > + > ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) > > # Final link of vmlinux with optional arch pass after final link > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > index 07f70e105d82..f1b705b8cdca 100755 > --- a/scripts/link-vmlinux.sh > +++ b/scripts/link-vmlinux.sh > @@ -227,12 +227,6 @@ ${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init init/version-timestamp.o > arch_vmlinux_o="" > if is_enabled CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX; then > arch_vmlinux_o=.arch.vmlinux.o > - info "ARCH" ${arch_vmlinux_o} > - if ! ${srctree}/arch/${SRCARCH}/tools/vmlinux_o.sh ${arch_vmlinux_o} ; then > - echo >&2 "Failed to generate ${arch_vmlinux_o}" > - echo >&2 "Try to disable CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX" > - exit 1 > - fi > fi > > btf_vmlinux_bin_o="" This is what I ended up with: --- diff --git a/arch/Kconfig b/arch/Kconfig index 975dd22a2dbd..649f0903e7ef 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -1643,4 +1643,7 @@ config CC_HAS_SANE_FUNCTION_ALIGNMENT config ARCH_NEED_CMPXCHG_1_EMU bool +config ARCH_WANTS_PRE_LINK_VMLINUX + def_bool n + endmenu diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux index 49946cb96844..6410e0be7f52 100644 --- a/scripts/Makefile.vmlinux +++ b/scripts/Makefile.vmlinux @@ -22,6 +22,14 @@ targets += .vmlinux.export.o vmlinux: .vmlinux.export.o endif +ifdef CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX +targets += .arch.vmlinux.o +.arch.vmlinux.o: vmlinux.o FORCE + $(Q)$(MAKE) $(build)=arch/$(SRCARCH)/tools .arch.vmlinux.o + +vmlinux: .arch.vmlinux.o +endif + ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) # Final link of vmlinux with optional arch pass after final link diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index 46ce5d04dbeb..eb5fb71f5207 100755 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -122,7 +122,7 @@ gen_btf() return 1 fi - vmlinux_link ${1} + vmlinux_link ${1} ${arch_vmlinux_o} info "BTF" ${2} LLVM_OBJCOPY="${OBJCOPY}" ${PAHOLE} -J ${PAHOLE_FLAGS} ${1} @@ -178,7 +178,7 @@ kallsyms_step() kallsymso=${kallsyms_vmlinux}.o kallsyms_S=${kallsyms_vmlinux}.S - vmlinux_link ${kallsyms_vmlinux} "${kallsymso_prev}" ${btf_vmlinux_bin_o} + vmlinux_link ${kallsyms_vmlinux} "${kallsymso_prev}" ${btf_vmlinux_bin_o} ${arch_vmlinux_o} mksysmap ${kallsyms_vmlinux} ${kallsyms_vmlinux}.syms kallsyms ${kallsyms_vmlinux}.syms ${kallsyms_S} @@ -223,6 +223,11 @@ fi ${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init init/version-timestamp.o +arch_vmlinux_o="" +if is_enabled CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX; then + arch_vmlinux_o=.arch.vmlinux.o +fi + btf_vmlinux_bin_o="" if is_enabled CONFIG_DEBUG_INFO_BTF; then btf_vmlinux_bin_o=.btf.vmlinux.bin.o @@ -273,7 +278,7 @@ if is_enabled CONFIG_KALLSYMS; then fi fi -vmlinux_link vmlinux "${kallsymso}" ${btf_vmlinux_bin_o} +vmlinux_link vmlinux "${kallsymso}" ${btf_vmlinux_bin_o} ${arch_vmlinux_o} # fill in BTF IDs if is_enabled CONFIG_DEBUG_INFO_BTF && is_enabled CONFIG_BPF; then --- Along with the below Makefile, things seem to be working properly in my limited tests: diff --git a/arch/powerpc/tools/Makefile b/arch/powerpc/tools/Makefile new file mode 100644 index 000000000000..9e2ba9a85baa --- /dev/null +++ b/arch/powerpc/tools/Makefile @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: GPL-2.0-or-later + +quiet_cmd_gen_ftrace_pfe_stubs = FTRACE $@ + cmd_gen_ftrace_pfe_stubs = $< $(objtree)/vmlinux.o $@ + +targets += .arch.vmlinux.o +.arch.vmlinux.o: $(srctree)/arch/powerpc/tools/gen-ftrace-pfe-stubs.sh $(objtree)/vmlinux.o FORCE + $(call if_changed,gen_ftrace_pfe_stubs) + +clean-files += $(objtree)/.arch.vmlinux.S $(objtree)/.arch.vmlinux.o - Naveen ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC PATCH v2 5/5] powerpc64/ftrace: Move ftrace sequence out of line 2024-06-10 8:38 [RFC PATCH v2 0/5] powerpc/ftrace: Move ftrace sequence out of line Naveen N Rao ` (3 preceding siblings ...) 2024-06-10 8:38 ` [RFC PATCH v2 4/5] kbuild: Add generic hook for architectures to use before the final vmlinux link Naveen N Rao @ 2024-06-10 8:38 ` Naveen N Rao 4 siblings, 0 replies; 16+ messages in thread From: Naveen N Rao @ 2024-06-10 8:38 UTC (permalink / raw) To: linuxppc-dev Cc: Mark Rutland, Masahiro Yamada, Steven Rostedt, Nicholas Piggin, Masami Hiramatsu Function profile sequence on powerpc includes two instructions at the beginning of each function: mflr r0 bl ftrace_caller The call to ftrace_caller() gets nop'ed out during kernel boot and is patched in when ftrace is enabled. Given the sequence, we cannot return from ftrace_caller with 'blr' as we need to keep LR and r0 intact. This results in link stack imbalance when ftrace is enabled. To address that, we would like to use a three instruction sequence: mflr r0 bl ftrace_caller mtlr r0 Further more, to support DYNAMIC_FTRACE_WITH_CALL_OPS, we need to reserve two instruction slots before the function. This results in a total of five instruction slots to be reserved for ftrace use on each function that is traced. Move the function profile sequence out-of-line to minimize its impact. To do this, we reserve a single nop at function entry using -fpatchable-function-entry=1 and add a pass on vmlinux.o to determine the total number of functions that can be traced. This is then used to generate a .S file reserving the appropriate amount of space for use as ftrace stubs, which is built and linked into vmlinux. On bootup, the stub space is split into separate stubs per function and populated with the proper instruction sequence. A pointer to the associated stub is maintained in dyn_arch_ftrace. For modules, space for ftrace stubs is reserved from the generic module stub space. This is restricted to and enabled by default only on 64-bit powerpc. Signed-off-by: Naveen N Rao <naveen@kernel.org> --- arch/powerpc/Kconfig | 4 + arch/powerpc/Makefile | 4 + arch/powerpc/include/asm/ftrace.h | 10 ++ arch/powerpc/include/asm/module.h | 5 + arch/powerpc/kernel/asm-offsets.c | 4 + arch/powerpc/kernel/module_64.c | 67 +++++++++-- arch/powerpc/kernel/trace/ftrace.c | 145 +++++++++++++++++++++-- arch/powerpc/kernel/trace/ftrace_entry.S | 71 ++++++++--- arch/powerpc/kernel/vmlinux.lds.S | 3 +- arch/powerpc/tools/vmlinux_o.sh | 47 ++++++++ 10 files changed, 324 insertions(+), 36 deletions(-) create mode 100755 arch/powerpc/tools/vmlinux_o.sh diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index c88c6d46a5bc..c393daeaf643 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -568,6 +568,10 @@ config ARCH_USING_PATCHABLE_FUNCTION_ENTRY def_bool $(success,$(srctree)/arch/powerpc/tools/gcc-check-fpatchable-function-entry.sh $(CC) -mlittle-endian) if PPC64 && CPU_LITTLE_ENDIAN def_bool $(success,$(srctree)/arch/powerpc/tools/gcc-check-fpatchable-function-entry.sh $(CC) -mbig-endian) if PPC64 && CPU_BIG_ENDIAN +config FTRACE_PFE_OUT_OF_LINE + def_bool PPC64 && ARCH_USING_PATCHABLE_FUNCTION_ENTRY + select ARCH_WANTS_PRE_LINK_VMLINUX + config HOTPLUG_CPU bool "Support for enabling/disabling CPUs" depends on SMP && (PPC_PSERIES || \ diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile index a8479c881cac..bb920d48ec6e 100644 --- a/arch/powerpc/Makefile +++ b/arch/powerpc/Makefile @@ -155,7 +155,11 @@ CC_FLAGS_NO_FPU := $(call cc-option,-msoft-float) ifdef CONFIG_FUNCTION_TRACER ifdef CONFIG_ARCH_USING_PATCHABLE_FUNCTION_ENTRY KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY +ifdef CONFIG_FTRACE_PFE_OUT_OF_LINE +CC_FLAGS_FTRACE := -fpatchable-function-entry=1 +else CC_FLAGS_FTRACE := -fpatchable-function-entry=2 +endif else CC_FLAGS_FTRACE := -pg ifdef CONFIG_MPROFILE_KERNEL diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h index 201f9d15430a..9da1da0f87b4 100644 --- a/arch/powerpc/include/asm/ftrace.h +++ b/arch/powerpc/include/asm/ftrace.h @@ -26,6 +26,9 @@ unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip, struct module; struct dyn_ftrace; struct dyn_arch_ftrace { +#ifdef CONFIG_FTRACE_PFE_OUT_OF_LINE + unsigned long pfe_stub; +#endif }; #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS @@ -132,6 +135,13 @@ static inline u8 this_cpu_get_ftrace_enabled(void) { return 1; } #ifdef CONFIG_FUNCTION_TRACER extern unsigned int ftrace_tramp_text[], ftrace_tramp_init[]; +#ifdef CONFIG_FTRACE_PFE_OUT_OF_LINE +struct ftrace_pfe_stub { + u32 insn[4]; +}; +extern struct ftrace_pfe_stub ftrace_pfe_stub_text[], ftrace_pfe_stub_inittext[]; +extern unsigned long ftrace_pfe_stub_text_count, ftrace_pfe_stub_inittext_count; +#endif void ftrace_free_init_tramp(void); unsigned long ftrace_call_adjust(unsigned long addr); #else diff --git a/arch/powerpc/include/asm/module.h b/arch/powerpc/include/asm/module.h index 300c777cc307..28dbd1ec5593 100644 --- a/arch/powerpc/include/asm/module.h +++ b/arch/powerpc/include/asm/module.h @@ -47,6 +47,11 @@ struct mod_arch_specific { #ifdef CONFIG_DYNAMIC_FTRACE unsigned long tramp; unsigned long tramp_regs; +#ifdef CONFIG_FTRACE_PFE_OUT_OF_LINE + struct ftrace_pfe_stub *pfe_stubs; + unsigned int pfe_stub_count; + unsigned int pfe_stub_index; +#endif #endif }; diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c index f029755f9e69..5f1a411d714c 100644 --- a/arch/powerpc/kernel/asm-offsets.c +++ b/arch/powerpc/kernel/asm-offsets.c @@ -674,5 +674,9 @@ int main(void) DEFINE(BPT_SIZE, BPT_SIZE); #endif +#ifdef CONFIG_FTRACE_PFE_OUT_OF_LINE + DEFINE(FTRACE_PFE_STUB_SIZE, sizeof(struct ftrace_pfe_stub)); +#endif + return 0; } diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c index e9bab599d0c2..3388de5a32de 100644 --- a/arch/powerpc/kernel/module_64.c +++ b/arch/powerpc/kernel/module_64.c @@ -205,7 +205,9 @@ static int relacmp(const void *_x, const void *_y) /* Get size of potential trampolines required. */ static unsigned long get_stubs_size(const Elf64_Ehdr *hdr, - const Elf64_Shdr *sechdrs) + const Elf64_Shdr *sechdrs, + char *secstrings, + struct module *me) { /* One extra reloc so it's always 0-addr terminated */ unsigned long relocs = 1; @@ -241,13 +243,30 @@ static unsigned long get_stubs_size(const Elf64_Ehdr *hdr, } } -#ifdef CONFIG_DYNAMIC_FTRACE /* make the trampoline to the ftrace_caller */ - relocs++; -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS + if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE)) + relocs++; + /* an additional one for ftrace_regs_caller */ - relocs++; -#endif + if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS)) + relocs++; + +#ifdef CONFIG_FTRACE_PFE_OUT_OF_LINE + /* stubs for the function tracer */ + for (i = 1; i < hdr->e_shnum; i++) { + if (!strcmp(secstrings + sechdrs[i].sh_name, "__patchable_function_entries")) { + me->arch.pfe_stub_count = sechdrs[i].sh_size / sizeof(unsigned long); + me->arch.pfe_stub_index = 0; + relocs += roundup(me->arch.pfe_stub_count * sizeof(struct ftrace_pfe_stub), + sizeof(struct ppc64_stub_entry)) / + sizeof(struct ppc64_stub_entry); + break; + } + } + if (i == hdr->e_shnum) { + pr_err("%s: doesn't contain __patchable_function_entries.\n", me->name); + return -ENOEXEC; + } #endif pr_debug("Looks like a total of %lu stubs, max\n", relocs); @@ -460,7 +479,7 @@ int module_frob_arch_sections(Elf64_Ehdr *hdr, #endif /* Override the stubs size */ - sechdrs[me->arch.stubs_section].sh_size = get_stubs_size(hdr, sechdrs); + sechdrs[me->arch.stubs_section].sh_size = get_stubs_size(hdr, sechdrs, secstrings, me); return 0; } @@ -1085,6 +1104,37 @@ int module_trampoline_target(struct module *mod, unsigned long addr, return 0; } +static int setup_ftrace_pfe_stubs(const Elf64_Shdr *sechdrs, unsigned long addr, struct module *me) +{ +#ifdef CONFIG_FTRACE_PFE_OUT_OF_LINE + unsigned int i, total_stubs, num_stubs; + struct ppc64_stub_entry *stub; + + total_stubs = sechdrs[me->arch.stubs_section].sh_size / sizeof(*stub); + num_stubs = roundup(me->arch.pfe_stub_count * sizeof(struct ftrace_pfe_stub), + sizeof(struct ppc64_stub_entry)) / sizeof(struct ppc64_stub_entry); + + /* Find the next available entry */ + stub = (void *)sechdrs[me->arch.stubs_section].sh_addr; + for (i = 0; stub_func_addr(stub[i].funcdata); i++) + if (WARN_ON(i >= total_stubs)) + return -1; + + if (WARN_ON(i + num_stubs > total_stubs)) + return -1; + + stub += i; + me->arch.pfe_stubs = (struct ftrace_pfe_stub *)stub; + + /* reserve stubs */ + for (i = 0; i < num_stubs; i++) + if (patch_u32((void *)&stub->funcdata, PPC_RAW_NOP())) + return -1; +#endif + + return 0; +} + int module_finalize_ftrace(struct module *mod, const Elf_Shdr *sechdrs) { mod->arch.tramp = stub_for_addr(sechdrs, @@ -1103,6 +1153,9 @@ int module_finalize_ftrace(struct module *mod, const Elf_Shdr *sechdrs) if (!mod->arch.tramp) return -ENOENT; + if (setup_ftrace_pfe_stubs(sechdrs, mod->arch.tramp, mod)) + return -ENOENT; + return 0; } #endif diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c index 2e1667a578ff..c3ef8ab6e940 100644 --- a/arch/powerpc/kernel/trace/ftrace.c +++ b/arch/powerpc/kernel/trace/ftrace.c @@ -37,7 +37,7 @@ unsigned long ftrace_call_adjust(unsigned long addr) if (addr >= (unsigned long)__exittext_begin && addr < (unsigned long)__exittext_end) return 0; - if (IS_ENABLED(CONFIG_ARCH_USING_PATCHABLE_FUNCTION_ENTRY)) + if (IS_ENABLED(CONFIG_ARCH_USING_PATCHABLE_FUNCTION_ENTRY) && !IS_ENABLED(CONFIG_FTRACE_PFE_OUT_OF_LINE)) addr += MCOUNT_INSN_SIZE; return addr; @@ -82,7 +82,7 @@ static inline int ftrace_modify_code(unsigned long ip, ppc_inst_t old, ppc_inst_ { int ret = ftrace_validate_inst(ip, old); - if (!ret) + if (!ret && !ppc_inst_equal(old, new)) ret = patch_instruction((u32 *)ip, new); return ret; @@ -127,12 +127,24 @@ static struct module *ftrace_lookup_module(struct dyn_ftrace *rec) return mod; } +static unsigned long ftrace_get_pfe_stub(struct dyn_ftrace *rec) +{ +#ifdef CONFIG_FTRACE_PFE_OUT_OF_LINE + return rec->arch.pfe_stub; +#else + BUILD_BUG(); +#endif +} + static int ftrace_get_call_inst(struct dyn_ftrace *rec, unsigned long addr, ppc_inst_t *call_inst) { unsigned long ip = rec->ip; unsigned long stub; struct module *mod; + if (IS_ENABLED(CONFIG_FTRACE_PFE_OUT_OF_LINE)) + ip = ftrace_get_pfe_stub(rec) + MCOUNT_INSN_SIZE; /* second instruction in stub */ + if (is_offset_in_branch_range(addr - ip)) { /* Within range */ stub = addr; @@ -140,7 +152,7 @@ static int ftrace_get_call_inst(struct dyn_ftrace *rec, unsigned long addr, ppc_ /* We would be branching to one of our ftrace stubs */ stub = find_ftrace_tramp(ip); if (!stub) { - pr_err("0x%lx: No ftrace stubs reachable\n", ip); + pr_err("0x%lx (0x%lx): No ftrace stubs reachable\n", ip, rec->ip); return -EINVAL; } } else { @@ -160,6 +172,79 @@ static int ftrace_get_call_inst(struct dyn_ftrace *rec, unsigned long addr, ppc_ return 0; } +static int ftrace_init_pfe_stub(struct module *mod, struct dyn_ftrace *rec) +{ +#ifdef CONFIG_FTRACE_PFE_OUT_OF_LINE + static int pfe_stub_text_index = 0, pfe_stub_inittext_index = 0; + int ret = 0, pfe_stub_count, *pfe_stub_index; + ppc_inst_t inst; + struct ftrace_pfe_stub *pfe_stub, pfe_stub_template = { + .insn = { + PPC_RAW_MFLR(_R0), + PPC_RAW_NOP(), /* bl ftrace_caller */ + PPC_RAW_MTLR(_R0), + PPC_RAW_NOP() /* b rec->ip + 4 */ + } + }; + + WARN_ON(rec->arch.pfe_stub); + + if (is_kernel_inittext(rec->ip)) { + pfe_stub = ftrace_pfe_stub_inittext; + pfe_stub_index = &pfe_stub_inittext_index; + pfe_stub_count = ftrace_pfe_stub_inittext_count; + } else if (is_kernel_text(rec->ip)) { + pfe_stub = ftrace_pfe_stub_text; + pfe_stub_index = &pfe_stub_text_index; + pfe_stub_count = ftrace_pfe_stub_text_count; +#ifdef CONFIG_MODULES + } else if (mod) { + pfe_stub = mod->arch.pfe_stubs; + pfe_stub_index = &mod->arch.pfe_stub_index; + pfe_stub_count = mod->arch.pfe_stub_count; +#endif + } else { + return -EINVAL; + } + + pfe_stub += (*pfe_stub_index)++; + + if (WARN_ON(*pfe_stub_index > pfe_stub_count)) + return -EINVAL; + + if (!is_offset_in_branch_range((long)rec->ip - (long)&pfe_stub->insn[0]) || + !is_offset_in_branch_range((long)(rec->ip + MCOUNT_INSN_SIZE) - (long)&pfe_stub->insn[3])) { + pr_err("%s: ftrace pfe stub out of range (%p -> %p).\n", + __func__, (void *)rec->ip, (void *)&pfe_stub->insn[0]); + return -EINVAL; + } + + rec->arch.pfe_stub = (unsigned long)&pfe_stub->insn[0]; + + /* bl ftrace_caller */ + if (mod) + /* We can't lookup the module since it is not fully formed yet */ + inst = ftrace_create_branch_inst(ftrace_get_pfe_stub(rec) + MCOUNT_INSN_SIZE, + mod->arch.tramp, 1); + else + ret = ftrace_get_call_inst(rec, (unsigned long)ftrace_caller, &inst); + pfe_stub_template.insn[1] = ppc_inst_val(inst); + + /* b rec->ip + 4 */ + if (!ret && create_branch(&inst, &pfe_stub->insn[3], rec->ip + MCOUNT_INSN_SIZE, 0)) + return -EINVAL; + pfe_stub_template.insn[3] = ppc_inst_val(inst); + + if (!ret) + ret = patch_instructions((u32 *)pfe_stub, (u32 *)&pfe_stub_template, + sizeof(pfe_stub_template), false); + + return ret; +#else /* !CONFIG_FTRACE_PFE_OUT_OF_LINE */ + BUILD_BUG(); +#endif +} + #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, unsigned long addr) { @@ -172,18 +257,29 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, unsigned int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) { ppc_inst_t old, new; - int ret; + unsigned long ip = rec->ip; + int ret = 0; /* This can only ever be called during module load */ - if (WARN_ON(!IS_ENABLED(CONFIG_MODULES) || core_kernel_text(rec->ip))) + if (WARN_ON(!IS_ENABLED(CONFIG_MODULES) || core_kernel_text(ip))) return -EINVAL; old = ppc_inst(PPC_RAW_NOP()); - ret = ftrace_get_call_inst(rec, addr, &new); - if (ret) - return ret; + if (IS_ENABLED(CONFIG_FTRACE_PFE_OUT_OF_LINE)) { + ip = ftrace_get_pfe_stub(rec) + MCOUNT_INSN_SIZE; /* second instruction in stub */ + ret = ftrace_get_call_inst(rec, (unsigned long)ftrace_caller, &old); + } - return ftrace_modify_code(rec->ip, old, new); + ret |= ftrace_get_call_inst(rec, addr, &new); + + if (!ret) + ret = ftrace_modify_code(ip, old, new); + + if (!ret && IS_ENABLED(CONFIG_FTRACE_PFE_OUT_OF_LINE)) + ret = ftrace_modify_code(rec->ip, ppc_inst(PPC_RAW_NOP()), + ppc_inst(PPC_RAW_BRANCH((long)ftrace_get_pfe_stub(rec) - (long)rec->ip))); + + return ret; } int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, unsigned long addr) @@ -216,6 +312,13 @@ void ftrace_replace_code(int enable) new_addr = ftrace_get_addr_new(rec); update = ftrace_update_record(rec, enable); + if (IS_ENABLED(CONFIG_FTRACE_PFE_OUT_OF_LINE) && update != FTRACE_UPDATE_IGNORE) { + ip = ftrace_get_pfe_stub(rec) + MCOUNT_INSN_SIZE; + ret = ftrace_get_call_inst(rec, (unsigned long)ftrace_caller, &nop_inst); + if (ret) + goto out; + } + switch (update) { case FTRACE_UPDATE_IGNORE: default: @@ -240,6 +343,23 @@ void ftrace_replace_code(int enable) if (!ret) ret = ftrace_modify_code(ip, old, new); + + if (!ret && IS_ENABLED(CONFIG_FTRACE_PFE_OUT_OF_LINE) && + (update == FTRACE_UPDATE_MAKE_NOP || update == FTRACE_UPDATE_MAKE_CALL)) { + /* Update the actual ftrace location */ + call_inst = ppc_inst(PPC_RAW_BRANCH((long)ftrace_get_pfe_stub(rec) - (long)rec->ip)); + nop_inst = ppc_inst(PPC_RAW_NOP()); + ip = rec->ip; + + if (update == FTRACE_UPDATE_MAKE_NOP) + ret = ftrace_modify_code(ip, call_inst, nop_inst); + else + ret = ftrace_modify_code(ip, nop_inst, call_inst); + + if (ret) + goto out; + } + if (ret) goto out; } @@ -259,7 +379,8 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec) /* Verify instructions surrounding the ftrace location */ if (IS_ENABLED(CONFIG_ARCH_USING_PATCHABLE_FUNCTION_ENTRY)) { /* Expect nops */ - ret = ftrace_validate_inst(ip - 4, ppc_inst(PPC_RAW_NOP())); + if (!IS_ENABLED(CONFIG_FTRACE_PFE_OUT_OF_LINE)) + ret = ftrace_validate_inst(ip - 4, ppc_inst(PPC_RAW_NOP())); if (!ret) ret = ftrace_validate_inst(ip, ppc_inst(PPC_RAW_NOP())); } else if (IS_ENABLED(CONFIG_PPC32)) { @@ -283,6 +404,10 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec) if (ret) return ret; + /* Set up out-of-line stub */ + if (IS_ENABLED(CONFIG_FTRACE_PFE_OUT_OF_LINE)) + return ftrace_init_pfe_stub(mod, rec); + /* Nop-out the ftrace location */ new = ppc_inst(PPC_RAW_NOP()); addr = MCOUNT_ADDR; diff --git a/arch/powerpc/kernel/trace/ftrace_entry.S b/arch/powerpc/kernel/trace/ftrace_entry.S index 244a1c7bb1e8..2e9e000d1305 100644 --- a/arch/powerpc/kernel/trace/ftrace_entry.S +++ b/arch/powerpc/kernel/trace/ftrace_entry.S @@ -100,12 +100,6 @@ mr r14, r7 /* remember old NIP */ #endif - /* Calculate ip from nip-4 into r3 for call below */ - subi r3, r7, MCOUNT_INSN_SIZE - - /* Put the original return address in r4 as parent_ip */ - mr r4, r0 - /* Save special regs */ PPC_STL r8, _MSR(r1) .if \allregs == 1 @@ -114,6 +108,26 @@ PPC_STL r11, _CCR(r1) .endif +#ifdef CONFIG_FTRACE_PFE_OUT_OF_LINE + /* + * We want the ftrace location in the function, but our lr (in r7) + * points at the 'mtlr r0' instruction in the out of line stub. To + * recover the ftrace location, we read the branch instruction in the + * stub, and adjust our lr by the branch offset. + */ + lwz r8, MCOUNT_INSN_SIZE(r7) + li r9, 6 + slw r8, r8, r9 + sraw r8, r8, r9 + add r3, r7, r8 +#else + /* Calculate ip from nip-4 into r3 for call below */ + subi r3, r7, MCOUNT_INSN_SIZE +#endif + + /* Put the original return address in r4 as parent_ip */ + mr r4, r0 + /* Load &pt_regs in r6 for call below */ addi r6, r1, STACK_INT_FRAME_REGS .endm @@ -121,12 +135,17 @@ .macro ftrace_regs_exit allregs /* Load ctr with the possibly modified NIP */ PPC_LL r3, _NIP(r1) - mtctr r3 #ifdef CONFIG_LIVEPATCH_64 cmpd r14, r3 /* has NIP been altered? */ #endif +#ifdef CONFIG_FTRACE_PFE_OUT_OF_LINE + mtlr r3 +#else + mtctr r3 +#endif + /* Restore gprs */ .if \allregs == 1 REST_GPRS(2, 31, r1) @@ -139,7 +158,9 @@ /* Restore possibly modified LR */ PPC_LL r0, _LINK(r1) +#ifndef CONFIG_FTRACE_PFE_OUT_OF_LINE mtlr r0 +#endif #ifdef CONFIG_PPC64 /* Restore callee's TOC */ @@ -153,7 +174,12 @@ /* Based on the cmpd above, if the NIP was altered handle livepatch */ bne- livepatch_handler #endif - bctr /* jump after _mcount site */ + /* jump after _mcount site */ +#ifdef CONFIG_FTRACE_PFE_OUT_OF_LINE + blr +#else + bctr +#endif .endm _GLOBAL(ftrace_regs_caller) @@ -177,6 +203,11 @@ _GLOBAL(ftrace_stub) #ifdef CONFIG_PPC64 ftrace_no_trace: +#ifdef CONFIG_FTRACE_PFE_OUT_OF_LINE + REST_GPR(3, r1) + addi r1, r1, SWITCH_FRAME_SIZE+STACK_FRAME_MIN_SIZE + blr +#else mflr r3 mtctr r3 REST_GPR(3, r1) @@ -184,6 +215,7 @@ ftrace_no_trace: mtlr r0 bctr #endif +#endif #ifdef CONFIG_LIVEPATCH_64 /* @@ -196,9 +228,9 @@ ftrace_no_trace: * * On entry: * - we have no stack frame and can not allocate one - * - LR points back to the original caller (in A) - * - CTR holds the new NIP in C - * - r0, r11 & r12 are free + * - LR/r0 points back to the original caller (in A) + * - CTR/LR holds the new NIP in C + * - r11 & r12 are free */ livepatch_handler: ld r12, PACA_THREAD_INFO(r13) @@ -208,18 +240,23 @@ livepatch_handler: addi r11, r11, 24 std r11, TI_livepatch_sp(r12) - /* Save toc & real LR on livepatch stack */ - std r2, -24(r11) - mflr r12 - std r12, -16(r11) - /* Store stack end marker */ lis r12, STACK_END_MAGIC@h ori r12, r12, STACK_END_MAGIC@l std r12, -8(r11) - /* Put ctr in r12 for global entry and branch there */ + /* Save toc & real LR on livepatch stack */ + std r2, -24(r11) +#ifndef CONFIG_FTRACE_PFE_OUT_OF_LINE + mflr r12 + std r12, -16(r11) mfctr r12 +#else + std r0, -16(r11) + mflr r12 + /* Put ctr in r12 for global entry and branch there */ + mtctr r12 +#endif bctrl /* diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S index f420df7888a7..0aef9959f2cd 100644 --- a/arch/powerpc/kernel/vmlinux.lds.S +++ b/arch/powerpc/kernel/vmlinux.lds.S @@ -267,14 +267,13 @@ SECTIONS .init.text : AT(ADDR(.init.text) - LOAD_OFFSET) { _sinittext = .; INIT_TEXT - + *(.tramp.ftrace.init); /* *.init.text might be RO so we must ensure this section ends on * a page boundary. */ . = ALIGN(PAGE_SIZE); _einittext = .; - *(.tramp.ftrace.init); } :text /* .exit.text is discarded at runtime, not link time, diff --git a/arch/powerpc/tools/vmlinux_o.sh b/arch/powerpc/tools/vmlinux_o.sh new file mode 100755 index 000000000000..68e9f40e17c5 --- /dev/null +++ b/arch/powerpc/tools/vmlinux_o.sh @@ -0,0 +1,47 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0-or-later + +# Error out on error +set -e + +is_enabled() { + grep -q "^$1=y" include/config/auto.conf +} + +arch_vmlinux_o=${1} +vmlinux_o=${objtree}/vmlinux.o + +RELOCATION=R_PPC64_ADDR64 +if is_enabled CONFIG_PPC32; then + RELOCATION=R_PPC_ADDR32 +fi + +num_pfe_stubs_text=$(${CROSS_COMPILE}objdump -r -j __patchable_function_entries ${vmlinux_o} | + grep -v ".init.text" | grep "${RELOCATION}" | wc -l) +num_pfe_stubs_inittext=$(${CROSS_COMPILE}objdump -r -j __patchable_function_entries ${vmlinux_o} | + grep ".init.text" | grep "${RELOCATION}" | wc -l) + +cat > ${arch_vmlinux_o}.S <<EOF +#include <asm/asm-offsets.h> +#include <linux/linkage.h> + +.pushsection .tramp.ftrace.text,"aw" +SYM_DATA(ftrace_pfe_stub_text_count, .long ${num_pfe_stubs_text}) + +SYM_CODE_START(ftrace_pfe_stub_text) + .space ${num_pfe_stubs_text} * FTRACE_PFE_STUB_SIZE +SYM_CODE_END(ftrace_pfe_stub_text) +.popsection + +.pushsection .tramp.ftrace.init,"aw" +SYM_DATA(ftrace_pfe_stub_inittext_count, .long ${num_pfe_stubs_inittext}) + +SYM_CODE_START(ftrace_pfe_stub_inittext) + .space ${num_pfe_stubs_inittext} * FTRACE_PFE_STUB_SIZE +SYM_CODE_END(ftrace_pfe_stub_inittext) +.popsection +EOF + +${CC} ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS} \ + ${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL} \ + -c -o ${arch_vmlinux_o} ${arch_vmlinux_o}.S -- 2.45.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-06-12 10:54 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-06-10 8:38 [RFC PATCH v2 0/5] powerpc/ftrace: Move ftrace sequence out of line Naveen N Rao 2024-06-10 8:38 ` [RFC PATCH v2 1/5] powerpc/kprobes: Use ftrace to determine if a probe is at function entry Naveen N Rao 2024-06-10 8:38 ` [RFC PATCH v2 2/5] powerpc/ftrace: Remove pointer to struct module from dyn_arch_ftrace Naveen N Rao 2024-06-10 20:03 ` Steven Rostedt 2024-06-11 14:45 ` Naveen N Rao 2024-06-10 8:38 ` [RFC PATCH v2 3/5] powerpc/ftrace: Unify 32-bit and 64-bit ftrace entry code Naveen N Rao 2024-06-10 20:06 ` Steven Rostedt 2024-06-11 14:47 ` Naveen N Rao 2024-06-11 15:14 ` Steven Rostedt 2024-06-10 8:38 ` [RFC PATCH v2 4/5] kbuild: Add generic hook for architectures to use before the final vmlinux link Naveen N Rao 2024-06-10 9:14 ` Masahiro Yamada 2024-06-10 17:16 ` Naveen N Rao 2024-06-10 21:51 ` Masahiro Yamada 2024-06-11 17:35 ` Naveen N Rao 2024-06-12 10:52 ` Naveen N Rao 2024-06-10 8:38 ` [RFC PATCH v2 5/5] powerpc64/ftrace: Move ftrace sequence out of line Naveen N Rao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).