Le 19/06/2023 à 11:47, Naveen N Rao a écrit :
> Commit 67361cf8071286 ("powerpc/ftrace: Handle large kernel configs")
> added ftrace support for ppc64 kernel images with a text section larger
> than 32MB. The patch did two things:
> 1. Add stubs at the end of .text to branch into ftrace_[regs_]caller for
> functions that were out of branch range.
> 2. Re-purpose linker-generated long branches to _mcount to instead branch
> to ftrace_[regs_]caller.
>
> Before that, we only supported kernel .text up to ~32MB. With the above,
> we now support up to ~96MB:
> - The first 32MB of kernel text can branch directly into
> ftrace_[regs_]caller since that symbol is usually at the beginning.
> - The modified long_branch from (2) above is used by the next 32MB of
> kernel text.
> - The next 32MB of kernel text can use the stub at the end of text to
> branch back to ftrace_[regs_]caller.
>
> While re-purposing the long branch works in practice, it still restricts
> ftrace to kernel text up to ~96MB. The stub at the end of kernel text
> from (1) already enables us to extend ftrace support for kernel text
> up to 64MB, which fulfils the original requirement. Further, once we
> switch to -fpatchable-function-entry, there will not be a long branch
> that we can use.
>
> Stop re-purposing the linker-generated long branches for ftrace to
> simplify the code. If there are good reasons to support ftrace on
> kernels beyond 64MB, we can consider adding support by using
> -fpatchable-function-entry.
>
> Signed-off-by: Naveen N Rao <nav...@kernel.org>
Reviewed-by: Christophe Leroy <christophe.le...@csgroup.eu>
> ---
> arch/powerpc/kernel/trace/ftrace.c | 110 +++++------------------------
> 1 file changed, 17 insertions(+), 93 deletions(-)
>
> diff --git a/arch/powerpc/kernel/trace/ftrace.c
> b/arch/powerpc/kernel/trace/ftrace.c
> index ef4e49c2c37781..278bf8e52b6e89 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -28,13 +28,7 @@
> #include <asm/syscall.h>
> #include <asm/inst.h>
>
> -/*
> - * We generally only have a single long_branch tramp and at most 2 or 3 plt
> - * tramps generated. But, we don't use the plt tramps currently. We also
> allot
> - * 2 tramps after .text and .init.text. So, we only end up with around 3
> usable
> - * tramps in total. Set aside 8 just to be sure.
> - */
> -#define NUM_FTRACE_TRAMPS 8
> +#define NUM_FTRACE_TRAMPS 2
> static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
>
> static ppc_inst_t
> @@ -100,11 +94,6 @@ static int is_bl_op(ppc_inst_t op)
> return (ppc_inst_val(op) & ~PPC_LI_MASK) == PPC_RAW_BL(0);
> }
>
> -static int is_b_op(ppc_inst_t op)
> -{
> - return (ppc_inst_val(op) & ~PPC_LI_MASK) == PPC_RAW_BRANCH(0);
> -}
> -
> static unsigned long find_bl_target(unsigned long ip, ppc_inst_t op)
> {
> int offset;
> @@ -227,11 +216,7 @@ static unsigned long find_ftrace_tramp(unsigned long ip)
> {
> int i;
>
> - /*
> - * We have the compiler generated long_branch tramps at the end
> - * and we prefer those
> - */
> - for (i = NUM_FTRACE_TRAMPS - 1; i >= 0; i--)
> + for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
> if (!ftrace_tramps[i])
> continue;
> else if (is_offset_in_branch_range(ftrace_tramps[i] - ip))
> @@ -240,75 +225,6 @@ static unsigned long find_ftrace_tramp(unsigned long ip)
> return 0;
> }
>
> -static int add_ftrace_tramp(unsigned long tramp)
> -{
> - int i;
> -
> - for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
> - if (!ftrace_tramps[i]) {
> - ftrace_tramps[i] = tramp;
> - return 0;
> - }
> -
> - return -1;
> -}
> -
> -/*
> - * If this is a compiler generated long_branch trampoline (essentially, a
> - * trampoline that has a branch to _mcount()), we re-write the branch to
> - * instead go to ftrace_[regs_]caller() and note down the location of this
> - * trampoline.
> - */
> -static int setup_mcount_compiler_tramp(unsigned long tramp)
> -{
> - int i;
> - ppc_inst_t op;
> - unsigned long ptr;
> -
> - /* Is this a known long jump tramp? */
> - for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
> - if (ftrace_tramps[i] == tramp)
> - return 0;
> -
> - /* New trampoline -- read where this goes */
> - if (copy_inst_from_kernel_nofault(&op, (void *)tramp)) {
> - pr_debug("Fetching opcode failed.\n");
> - return -1;
> - }
> -
> - /* Is this a 24 bit branch? */
> - if (!is_b_op(op)) {
> - pr_debug("Trampoline is not a long branch tramp.\n");
> - return -1;
> - }
> -
> - /* lets find where the pointer goes */
> - ptr = find_bl_target(tramp, op);
> -
> - if (ptr != ppc_global_function_entry((void *)_mcount)) {
> - pr_debug("Trampoline target %p is not _mcount\n", (void *)ptr);
> - return -1;
> - }
> -
> - /* Let's re-write the tramp to go to ftrace_[regs_]caller */
> - if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS))
> - ptr = ppc_global_function_entry((void *)ftrace_regs_caller);
> - else
> - ptr = ppc_global_function_entry((void *)ftrace_caller);
> -
> - if (patch_branch((u32 *)tramp, ptr, 0)) {
> - pr_debug("REL24 out of range!\n");
> - return -1;
> - }
> -
> - if (add_ftrace_tramp(tramp)) {
> - pr_debug("No tramp locations left\n");
> - return -1;
> - }
> -
> - return 0;
> -}
> -
> static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned long
> addr)
> {
> unsigned long tramp, ip = rec->ip;
> @@ -331,13 +247,10 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace
> *rec, unsigned long addr)
>
> pr_devel("ip:%lx jumps to %lx", ip, tramp);
>
> - if (setup_mcount_compiler_tramp(tramp)) {
> - /* Are other trampolines reachable? */
> - if (!find_ftrace_tramp(ip)) {
> - pr_err("No ftrace trampolines reachable from %ps\n",
> - (void *)ip);
> - return -EINVAL;
> - }
> + /* Are ftrace trampolines reachable? */
> + if (!find_ftrace_tramp(ip)) {
> + pr_err("No ftrace trampolines reachable from %ps\n", (void
> *)ip);
> + return -EINVAL;
> }
>
> if (patch_instruction((u32 *)ip, ppc_inst(PPC_RAW_NOP()))) {
> @@ -725,6 +638,17 @@ void ftrace_free_init_tramp(void)
> }
> }
>
> +static void __init add_ftrace_tramp(unsigned long tramp)
> +{
> + int i;
> +
> + for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
> + if (!ftrace_tramps[i]) {
> + ftrace_tramps[i] = tramp;
> + return;
> + }
> +}
> +
> int __init ftrace_dyn_arch_init(void)
> {
> unsigned int *tramp[] = { ftrace_tramp_text, ftrace_tramp_init };
