Re: [PATCH 6/9] LoongArch: modules/ftrace: Initialize PLT at load time

[Jinyang He]

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@xxxxxxxxxxx>
> ---
>   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.


>   	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.

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;
> +}