Messages in this thread | | | Subject | Re: [PATCH 6/9] LoongArch: modules/ftrace: Initialize PLT at load time | From | Qing Zhang <> | Date | Sat, 20 Aug 2022 09:30:52 +0800 |
| |
On 2022/8/19 下午6:43, Jinyang He wrote: > On 2022/8/19 16:16, Qing Zhang wrote: > >> To Implement ftrace trampiones through plt entry. >> >> Tested by forcing ftrace_make_call() to use the module PLT, and then >> loading up a module after setting up ftrace with: >> >> | echo ":mod:<module-name>" > set_ftrace_filter; >> | echo function > current_tracer; >> | modprobe <module-name> >> >> Since FTRACE_ADDR/FTRACE_REGS_ADDR is only defined when >> CONFIG_DYNAMIC_FTRACE >> is selected, we wrap its use along with most of >> module_init_ftrace_plt() with >> ifdeffery rather than using IS_ENABLED(). >> >> Signed-off-by: Qing Zhang <zhangqing@loongson.cn> >> --- >> arch/loongarch/include/asm/ftrace.h | 4 ++ >> arch/loongarch/include/asm/inst.h | 2 + >> arch/loongarch/include/asm/module.h | 14 +++-- >> arch/loongarch/include/asm/module.lds.h | 1 + >> arch/loongarch/kernel/ftrace_dyn.c | 79 +++++++++++++++++++++++++ >> arch/loongarch/kernel/inst.c | 12 ++++ >> arch/loongarch/kernel/module-sections.c | 11 ++++ >> arch/loongarch/kernel/module.c | 48 +++++++++++++++ >> 8 files changed, 166 insertions(+), 5 deletions(-) > [...] >> @@ -36,14 +39,15 @@ Elf_Addr module_emit_plt_entry(struct module *mod, >> unsigned long val); >> static inline struct plt_entry emit_plt_entry(unsigned long val) >> { >> - u32 lu12iw, lu32id, lu52id, jirl; >> + u32 addu16id, lu32id, lu52id, jirl; >> - lu12iw = (lu12iw_op << 25 | (((val >> 12) & 0xfffff) << 5) | >> LOONGARCH_GPR_T1); >> + addu16id = larch_insn_gen_addu16id(LOONGARCH_GPR_T1, >> LOONGARCH_GPR_ZERO, >> + ADDR_IMM(val, ADDU16ID)); > > Hi, Qing, > > Why change lu12iw to addu16id? They have same effect here. > Hi. Jinyang
Yes, the effect is the same. I will modify it to lu12iw in V2, which can be consistent with the user's plt style.
Thanks, - Qing
> >> lu32id = larch_insn_gen_lu32id(LOONGARCH_GPR_T1, ADDR_IMM(val, >> LU32ID)); >> lu52id = larch_insn_gen_lu52id(LOONGARCH_GPR_T1, >> LOONGARCH_GPR_T1, ADDR_IMM(val, LU52ID)); >> - jirl = larch_insn_gen_jirl(0, LOONGARCH_GPR_T1, 0, (val & 0xfff)); >> + jirl = larch_insn_gen_jirl(0, LOONGARCH_GPR_T1, 0, (val & 0xffff)); >> - return (struct plt_entry) { lu12iw, lu32id, lu52id, jirl }; >> + return (struct plt_entry) { addu16id, lu32id, lu52id, jirl }; >> } > [...] >> +static int module_init_ftrace_plt(const Elf_Ehdr *hdr, const Elf_Shdr >> *sechdrs, >> + struct module *mod) >> +{ >> +#ifdef CONFIG_DYNAMIC_FTRACE >> + const Elf_Shdr *s; >> + struct plt_entry *ftrace_plts; >> + >> + s = find_section(hdr, sechdrs, ".ftrace_trampoline"); >> + if (!s) >> + return -ENOEXEC; > > Return value is not used later, may we do something or drop returning > value. > Yes, Should return module_init_ftrace_plt(hdr, sechdrs, mod) here.
Thanks, -Qing > Thanks, > > Jinyang > >> + >> + ftrace_plts = (void *)s->sh_addr; >> + >> + ftrace_plts[FTRACE_PLT_IDX] = emit_plt_entry(FTRACE_ADDR); >> + >> + if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS)) >> + ftrace_plts[FTRACE_REGS_PLT_IDX] = >> emit_plt_entry(FTRACE_REGS_ADDR); >> + >> + mod->arch.ftrace_trampolines = ftrace_plts; >> +#endif >> + return 0; >> +} >> + >> +int module_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs, >> struct module *mod) >> +{ >> + module_init_ftrace_plt(hdr, sechdrs, mod); >> + >> + return 0; >> +} >
| |