| Message ID | 1456324115-21144-3-git-send-email-mpe@ellerman.id.au (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 25/02/16 01:28, Michael Ellerman wrote: > In order to support the new -mprofile-kernel ABI, we need to be able to > call from the module back to ftrace_caller() (in the kernel) without > using the module's r2. That is because the function in this module which > is calling ftrace_caller() may not have setup r2, if it doesn't > otherwise need it (ie. it accesses no globals). > > To make that work we add a new stub which is used for calling > ftrace_caller(), which uses the kernel toc instead of the module toc. > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > --- > arch/powerpc/kernel/module_64.c | 48 ++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 47 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c > index 9629966e614b..e711d40a3b8f 100644 > --- a/arch/powerpc/kernel/module_64.c > +++ b/arch/powerpc/kernel/module_64.c > @@ -671,10 +671,56 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, > } > > #ifdef CONFIG_DYNAMIC_FTRACE > + > +#define PACATOC offsetof(struct paca_struct, kernel_toc) > + > +static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs, struct module *me) > +{ > + struct ppc64_stub_entry *entry; > + unsigned int i, num_stubs; How about some comments on r2 r2 is still pointing to the module's toc, will be saved by ftrace_caller and restored by the instruction following bl ftrace_caller (after patching _mcount/nop) > + static u32 stub_insns[] = { > + 0xe98d0000 | PACATOC, /* ld r12,PACATOC(r13) */ > + 0x3d8c0000, /* addis r12,r12,<high> */ > + 0x398c0000, /* addi r12,r12,<low> */ > + 0x7d8903a6, /* mtctr r12 */ > + 0x4e800420, /* bctr */ > + }; > + long reladdr; > + > + num_stubs = sechdrs[me->arch.stubs_section].sh_size / sizeof(*entry); > + > + /* Find the next available stub entry */ > + entry = (void *)sechdrs[me->arch.stubs_section].sh_addr; > + for (i = 0; i < num_stubs && stub_func_addr(entry->funcdata); i++, entry++); > + > + if (i >= num_stubs) { > + pr_err("%s: Unable to find a free slot for ftrace stub.\n", me->name); > + return 0; > + } > + > + memcpy(entry->jump, stub_insns, sizeof(stub_insns)); > + > + /* Stub uses address relative to kernel_toc */ > + reladdr = (unsigned long)ftrace_caller - get_paca()->kernel_toc; > + if (reladdr > 0x7FFFFFFF || reladdr < -(0x80000000L)) { > + pr_err("%s: Address of ftrace_caller out of range of kernel_toc.\n", me->name); > + return 0; > + } > + > + entry->jump[1] |= PPC_HA(reladdr); > + entry->jump[2] |= PPC_LO(reladdr); > + > + /* Eventhough we don't use funcdata in the stub, it's needed elsewhere. */ > + entry->funcdata = func_desc((unsigned long)ftrace_caller); > + entry->magic = STUB_MAGIC; > + > + return (unsigned long)entry; > +} > + > int module_finalize_ftrace(struct module *mod, const Elf_Shdr *sechdrs) > { > mod->arch.toc = my_r2(sechdrs, mod); > - mod->arch.tramp = stub_for_addr(sechdrs, (unsigned long)ftrace_caller, mod); > + mod->arch.tramp = create_ftrace_stub(sechdrs, mod); > > if (!mod->arch.tramp) > return -ENOENT; Reviewed-by: Balbir Singh <bsingharora@gmail.com>
On Thu, 2016-02-25 at 11:08 +1100, Balbir Singh wrote: > > On 25/02/16 01:28, Michael Ellerman wrote: > > In order to support the new -mprofile-kernel ABI, we need to be able to > > call from the module back to ftrace_caller() (in the kernel) without > > using the module's r2. That is because the function in this module which > > is calling ftrace_caller() may not have setup r2, if it doesn't > > otherwise need it (ie. it accesses no globals). > > > > To make that work we add a new stub which is used for calling > > ftrace_caller(), which uses the kernel toc instead of the module toc. > > > > diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c > > index 9629966e614b..e711d40a3b8f 100644 > > --- a/arch/powerpc/kernel/module_64.c > > +++ b/arch/powerpc/kernel/module_64.c > > @@ -671,10 +671,56 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, > > } > > > > #ifdef CONFIG_DYNAMIC_FTRACE > > + > > +#define PACATOC offsetof(struct paca_struct, kernel_toc) > > + > > +static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs, struct module *me) > > +{ > > + struct ppc64_stub_entry *entry; > > + unsigned int i, num_stubs; > How about some comments on r2 > r2 is still pointing to the module's toc, will be saved by ftrace_caller and > restored by the instruction following bl ftrace_caller (after patching > _mcount/nop) Yeah I'll add some commentary. I think the change log describes it fairly well but a comment is also good. cheers
On Thu, Feb 25, 2016 at 11:08:54AM +1100, Balbir Singh wrote: > How about some comments on r2 > r2 is still pointing to the module's toc, will be saved by ftrace_caller and restored by the instruction following bl ftrace_caller (after patching _mcount/nop) To be precise: ftrace_caller needs to save _and_ restore r2 in case of -mprofile-kernel. > > + /* Stub uses address relative to kernel_toc */ > > + reladdr = (unsigned long)ftrace_caller - get_paca()->kernel_toc; kernel_toc is a compile time constant; do you really want to look it up in memory at runtime each time? It's a bit tricky to get the +- 0x8000 right OTOH... I wrote: extern unsigned long __toc_start; reladdr = addr - ((unsigned long)(&__toc_start) + 0x8000UL); looks a bit odd, but evaluates to a constant for ftrace_caller. Either way is fine with me: Signed-off-by: Torsten Duwe <duwe@suse.de> Reviewed-by: Torsten Duwe <duwe@suse.de> > Reviewed-by: Balbir Singh <bsingharora@gmail.com> Torsten
On Thu, 2016-02-25 at 14:31 +0100, Torsten Duwe wrote: > On Thu, Feb 25, 2016 at 11:08:54AM +1100, Balbir Singh wrote: > > How about some comments on r2 > > r2 is still pointing to the module's toc, will be saved by ftrace_caller and restored by the instruction following bl ftrace_caller (after patching _mcount/nop) > > To be precise: ftrace_caller needs to save _and_ restore r2 in case of -mprofile-kernel. > > > + /* Stub uses address relative to kernel_toc */ > > > + reladdr = (unsigned long)ftrace_caller - get_paca()->kernel_toc; Yeah true. I originally wrote it with the address of ftrace_caller passed as an argument, so it had to be computed at runtime, and getting it from the paca is ~= to getting it out of the GOT. > kernel_toc is a compile time constant; do you really want to look it up in > memory at runtime each time? It's a bit tricky to get the +- 0x8000 right > OTOH... > > I wrote: > extern unsigned long __toc_start; > reladdr = addr - ((unsigned long)(&__toc_start) + 0x8000UL); > > looks a bit odd, but evaluates to a constant for ftrace_caller. Yeah that makes sense. I'll add a helper to do the + 32k. cheers
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c index 9629966e614b..e711d40a3b8f 100644 --- a/arch/powerpc/kernel/module_64.c +++ b/arch/powerpc/kernel/module_64.c @@ -671,10 +671,56 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, } #ifdef CONFIG_DYNAMIC_FTRACE + +#define PACATOC offsetof(struct paca_struct, kernel_toc) + +static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs, struct module *me) +{ + struct ppc64_stub_entry *entry; + unsigned int i, num_stubs; + static u32 stub_insns[] = { + 0xe98d0000 | PACATOC, /* ld r12,PACATOC(r13) */ + 0x3d8c0000, /* addis r12,r12,<high> */ + 0x398c0000, /* addi r12,r12,<low> */ + 0x7d8903a6, /* mtctr r12 */ + 0x4e800420, /* bctr */ + }; + long reladdr; + + num_stubs = sechdrs[me->arch.stubs_section].sh_size / sizeof(*entry); + + /* Find the next available stub entry */ + entry = (void *)sechdrs[me->arch.stubs_section].sh_addr; + for (i = 0; i < num_stubs && stub_func_addr(entry->funcdata); i++, entry++); + + if (i >= num_stubs) { + pr_err("%s: Unable to find a free slot for ftrace stub.\n", me->name); + return 0; + } + + memcpy(entry->jump, stub_insns, sizeof(stub_insns)); + + /* Stub uses address relative to kernel_toc */ + reladdr = (unsigned long)ftrace_caller - get_paca()->kernel_toc; + if (reladdr > 0x7FFFFFFF || reladdr < -(0x80000000L)) { + pr_err("%s: Address of ftrace_caller out of range of kernel_toc.\n", me->name); + return 0; + } + + entry->jump[1] |= PPC_HA(reladdr); + entry->jump[2] |= PPC_LO(reladdr); + + /* Eventhough we don't use funcdata in the stub, it's needed elsewhere. */ + entry->funcdata = func_desc((unsigned long)ftrace_caller); + entry->magic = STUB_MAGIC; + + return (unsigned long)entry; +} + int module_finalize_ftrace(struct module *mod, const Elf_Shdr *sechdrs) { mod->arch.toc = my_r2(sechdrs, mod); - mod->arch.tramp = stub_for_addr(sechdrs, (unsigned long)ftrace_caller, mod); + mod->arch.tramp = create_ftrace_stub(sechdrs, mod); if (!mod->arch.tramp) return -ENOENT;
In order to support the new -mprofile-kernel ABI, we need to be able to call from the module back to ftrace_caller() (in the kernel) without using the module's r2. That is because the function in this module which is calling ftrace_caller() may not have setup r2, if it doesn't otherwise need it (ie. it accesses no globals). To make that work we add a new stub which is used for calling ftrace_caller(), which uses the kernel toc instead of the module toc. Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> --- arch/powerpc/kernel/module_64.c | 48 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-)