All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Naveen N Rao <naveen@kernel.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH 10/17] powerpc/ftrace: Add separate ftrace_init_nop() with additional validation
Date: Fri, 23 Jun 2023 05:29:27 +0000	[thread overview]
Message-ID: <327c6d9d-8de4-080d-9996-f573d531b308@csgroup.eu> (raw)
In-Reply-To: <f373684081e8e98be09b7f44d2d93069768324dc.1687166935.git.naveen@kernel.org>



Le 19/06/2023 à 11:47, Naveen N Rao a écrit :
> Currently, we validate instructions around the ftrace location every
> time we have to enable/disable ftrace. Introduce ftrace_init_nop() to
> instead perform all the validation during ftrace initialization. This
> allows us to simply patch the necessary instructions during
> enabling/disabling ftrace.
> 
> Signed-off-by: Naveen N Rao <naveen@kernel.org>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
>   arch/powerpc/include/asm/ftrace.h  |  6 +++
>   arch/powerpc/kernel/trace/ftrace.c | 71 ++++++++++++++++++++++++++++++
>   2 files changed, 77 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
> index 702aaf2efa966c..ef9f0b97670d1c 100644
> --- a/arch/powerpc/include/asm/ftrace.h
> +++ b/arch/powerpc/include/asm/ftrace.h
> @@ -29,11 +29,17 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
>   unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip,
>   				    unsigned long sp);
>   
> +struct module;
> +struct dyn_ftrace;
>   struct dyn_arch_ftrace {
>   	struct module *mod;
>   };
>   
>   #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
> +#define ftrace_need_init_nop()	(true)
> +int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec);
> +#define ftrace_init_nop ftrace_init_nop
> +
>   struct ftrace_regs {
>   	struct pt_regs regs;
>   };
> diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
> index 278bf8e52b6e89..98bd099c428ee0 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -31,6 +31,16 @@
>   #define	NUM_FTRACE_TRAMPS	2
>   static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
>   
> +static ppc_inst_t ftrace_create_branch_inst(unsigned long ip, unsigned long addr, int link)
> +{
> +	ppc_inst_t op;
> +
> +	WARN_ON(!is_offset_in_branch_range(addr - ip));
> +	create_branch(&op, (u32 *)ip, addr, link ? BRANCH_SET_LINK : 0);
> +
> +	return op;
> +}
> +
>   static ppc_inst_t
>   ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
>   {
> @@ -597,6 +607,67 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
>   }
>   #endif
>   
> +int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec)
> +{
> +	unsigned long addr, ip = rec->ip;
> +	ppc_inst_t old, new;
> +	int ret = 0;
> +
> +	/* Verify instructions surrounding the ftrace location */
> +	if (IS_ENABLED(CONFIG_PPC32)) {
> +		/* 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)));
> +	} 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)));
> +		}
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	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;
> +	if (is_offset_in_branch_range(addr - ip)) {
> +		/* Within range */
> +		old = ftrace_create_branch_inst(ip, addr, 1);
> +		ret = ftrace_modify_code(ip, old, new);
> +	} else if (core_kernel_text(ip) || (IS_ENABLED(CONFIG_MODULES) && mod)) {
> +		/*
> +		 * We would be branching to a linker-generated stub, or to the module _mcount
> +		 * stub. Let's just confirm we have a 'bl' here.
> +		 */
> +		ret = ftrace_read_inst(ip, &old);
> +		if (ret)
> +			return ret;
> +		if (!is_bl_op(old)) {
> +			pr_err("0x%lx: expected (bl) != found (%08lx)\n", ip, ppc_inst_as_ulong(old));
> +			return -EINVAL;
> +		}
> +		ret = patch_instruction((u32 *)ip, new);
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
>   int ftrace_update_ftrace_func(ftrace_func_t func)
>   {
>   	unsigned long ip = (unsigned long)(&ftrace_call);

  reply	other threads:[~2023-06-23  5:30 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-19  9:47 [PATCH 00/17] powerpc/ftrace: refactor and add support for -fpatchable-function-entry Naveen N Rao
2023-06-19  9:47 ` [PATCH 01/17] powerpc/ftrace: Fix dropping weak symbols with older toolchains Naveen N Rao
2023-06-23  5:10   ` Christophe Leroy
2023-06-19  9:47 ` [PATCH 02/17] powerpc/module: Remove unused .ftrace.tramp section Naveen N Rao
2023-06-23  5:12   ` Christophe Leroy
2023-06-19  9:47 ` [PATCH 03/17] powerpc64/ftrace: Move ELFv1 and -pg support code into a separate file Naveen N Rao
2023-06-23  5:13   ` Christophe Leroy
2023-06-19  9:47 ` [PATCH 04/17] powerpc/ftrace: Simplify function_graph support in ftrace.c Naveen N Rao
2023-06-23  5:14   ` Christophe Leroy
2023-06-19  9:47 ` [PATCH 05/17] powerpc/ftrace: Use FTRACE_REGS_ADDR to identify the correct ftrace trampoline Naveen N Rao
2023-06-23  5:15   ` Christophe Leroy
2023-06-19  9:47 ` [PATCH 06/17] powerpc/ftrace: Extend ftrace support for large kernels to ppc32 Naveen N Rao
2023-06-23  5:21   ` Christophe Leroy
2023-06-19  9:47 ` [PATCH 07/17] powerpc/ftrace: Consolidate ftrace support into fewer files Naveen N Rao
2023-06-23  5:25   ` Christophe Leroy
2023-06-28  7:32     ` Naveen N Rao
2023-06-19  9:47 ` [PATCH 08/17] powerpc/ftrace: Refactor ftrace_modify_code() Naveen N Rao
2023-06-23  5:27   ` Christophe Leroy
2023-06-19  9:47 ` [PATCH 09/17] powerpc/ftrace: Stop re-purposing linker generated long branches for ftrace Naveen N Rao
2023-06-23  5:28   ` Christophe Leroy
2023-06-19  9:47 ` [PATCH 10/17] powerpc/ftrace: Add separate ftrace_init_nop() with additional validation Naveen N Rao
2023-06-23  5:29   ` Christophe Leroy [this message]
2023-06-19  9:47 ` [PATCH 11/17] powerpc/ftrace: Simplify ftrace_make_nop() Naveen N Rao
2023-06-23  5:30   ` Christophe Leroy
2023-06-19  9:47 ` [PATCH 12/17] powerpc/ftrace: Simplify ftrace_make_call() Naveen N Rao
2023-06-23  5:30   ` Christophe Leroy
2023-06-19  9:47 ` [PATCH 13/17] powerpc/ftrace: Simplify ftrace_modify_call() Naveen N Rao
2023-06-23  5:31   ` Christophe Leroy
2023-06-19  9:47 ` [PATCH 14/17] powerpc/ftrace: Replace use of ftrace_call_replace() with ftrace_create_branch_inst() Naveen N Rao
2023-06-23  5:32   ` Christophe Leroy
2023-06-19  9:47 ` [PATCH 15/17] powerpc/ftrace: Implement ftrace_replace_code() Naveen N Rao
2023-06-23  5:32   ` Christophe Leroy
2023-06-19  9:47 ` [PATCH 16/17] powerpc/ftrace: Add support for -fpatchable-function-entry Naveen N Rao
2023-06-23  5:37   ` Christophe Leroy
2023-06-28  7:40     ` Naveen N Rao
2023-06-19  9:47 ` [PATCH 17/17] powerpc/ftrace: Create a dummy stackframe to fix stack unwind Naveen N Rao
2023-06-23  5:40   ` Christophe Leroy
2023-06-28  7:43     ` Naveen N Rao
2023-08-23 11:55 ` [PATCH 00/17] powerpc/ftrace: refactor and add support for -fpatchable-function-entry Michael Ellerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=327c6d9d-8de4-080d-9996-f573d531b308@csgroup.eu \
    --to=christophe.leroy@csgroup.eu \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=naveen@kernel.org \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.