* [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS
@ 2022-11-03 17:05 ` Mark Rutland
0 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2022-11-03 17:05 UTC (permalink / raw)
To: linux-kernel
Cc: catalin.marinas, linux-arm-kernel, mark.rutland, mhiramat,
revest, rostedt, will
This series replaces arm64's support for FTRACE_WITH_REGS with support
for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
removes some latent issues with inconsistent presentation of struct
pt_regs (which can only be reliably saved/restored at exception
boundaries).
The existing FTRACE_WITH_REGS support was added for two major reasons:
(1) To make it possible to use the ftrace graph tracer with pointer
authentication, where it's necessary to snapshot/manipulate the LR
before it is signed by the instrumented function.
(2) To make it possible to implement LIVEPATCH in future, where we need
to hook function entry before an instrumented function manipulates
the stack or argument registers. Practically speaking, we need to
preserve the argument/return registers, PC, LR, and SP.
Neither of these requires the full set of pt_regs, and only requires us
to save/restore a subset of registers used for passing
arguments/return-values and context/return information (which is the
minimum set we always need to save/restore today).
As there is no longer a need to save different sets of registers for
different features, we no longer need distinct `ftrace_caller` and
`ftrace_regs_caller` trampolines. This allows the trampoline assembly to
be simpler, and simplifies code which previously had to handle the two
trampolines.
I've tested this with the ftrace selftests, where there are no
unexpected failures.
I plan to build atop this with subsequent patches to add per-callsite
ftrace_ops, and I'm sending these patches on their own as I think they
make sense regardless.
Since v1 [1]:
* Change ifdeferry per Steve's request
* Add ftrace_regs_query_register_offset() per Masami's request
* Fix a bunch of typos
[1] https://lore.kernel.org/lkml/20221024140846.3555435-1-mark.rutland@arm.com
This series can be found in my 'arm64/ftrace/minimal-regs' branch on
kernel.org:
https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/
git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
This version is tagged as:
arm64-ftrace-minimal-regs-20221103
Thanks,
Mark.
Mark Rutland (4):
ftrace: pass fregs to arch_ftrace_set_direct_caller()
ftrace: rename ftrace_instruction_pointer_set() ->
ftrace_regs_set_instruction_pointer()
ftrace: abstract DYNAMIC_FTRACE_WITH_ARGS accesses
ftrace: arm64: move from REGS to ARGS
arch/arm64/Kconfig | 18 +++--
arch/arm64/Makefile | 2 +-
arch/arm64/include/asm/ftrace.h | 72 ++++++++++++++++--
arch/arm64/kernel/asm-offsets.c | 13 ++++
arch/arm64/kernel/entry-ftrace.S | 117 ++++++++++++------------------
arch/arm64/kernel/ftrace.c | 82 ++++++++++++---------
arch/arm64/kernel/module.c | 3 -
arch/powerpc/include/asm/ftrace.h | 24 +++++-
arch/s390/include/asm/ftrace.h | 29 +++++++-
arch/x86/include/asm/ftrace.h | 49 +++++++++----
include/linux/ftrace.h | 47 +++++++++---
kernel/livepatch/patch.c | 2 +-
kernel/trace/Kconfig | 6 +-
kernel/trace/ftrace.c | 3 +-
14 files changed, 309 insertions(+), 158 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS
@ 2022-11-03 17:05 ` Mark Rutland
0 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2022-11-03 17:05 UTC (permalink / raw)
To: linux-kernel
Cc: catalin.marinas, linux-arm-kernel, mark.rutland, mhiramat,
revest, rostedt, will
This series replaces arm64's support for FTRACE_WITH_REGS with support
for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
removes some latent issues with inconsistent presentation of struct
pt_regs (which can only be reliably saved/restored at exception
boundaries).
The existing FTRACE_WITH_REGS support was added for two major reasons:
(1) To make it possible to use the ftrace graph tracer with pointer
authentication, where it's necessary to snapshot/manipulate the LR
before it is signed by the instrumented function.
(2) To make it possible to implement LIVEPATCH in future, where we need
to hook function entry before an instrumented function manipulates
the stack or argument registers. Practically speaking, we need to
preserve the argument/return registers, PC, LR, and SP.
Neither of these requires the full set of pt_regs, and only requires us
to save/restore a subset of registers used for passing
arguments/return-values and context/return information (which is the
minimum set we always need to save/restore today).
As there is no longer a need to save different sets of registers for
different features, we no longer need distinct `ftrace_caller` and
`ftrace_regs_caller` trampolines. This allows the trampoline assembly to
be simpler, and simplifies code which previously had to handle the two
trampolines.
I've tested this with the ftrace selftests, where there are no
unexpected failures.
I plan to build atop this with subsequent patches to add per-callsite
ftrace_ops, and I'm sending these patches on their own as I think they
make sense regardless.
Since v1 [1]:
* Change ifdeferry per Steve's request
* Add ftrace_regs_query_register_offset() per Masami's request
* Fix a bunch of typos
[1] https://lore.kernel.org/lkml/20221024140846.3555435-1-mark.rutland@arm.com
This series can be found in my 'arm64/ftrace/minimal-regs' branch on
kernel.org:
https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/
git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
This version is tagged as:
arm64-ftrace-minimal-regs-20221103
Thanks,
Mark.
Mark Rutland (4):
ftrace: pass fregs to arch_ftrace_set_direct_caller()
ftrace: rename ftrace_instruction_pointer_set() ->
ftrace_regs_set_instruction_pointer()
ftrace: abstract DYNAMIC_FTRACE_WITH_ARGS accesses
ftrace: arm64: move from REGS to ARGS
arch/arm64/Kconfig | 18 +++--
arch/arm64/Makefile | 2 +-
arch/arm64/include/asm/ftrace.h | 72 ++++++++++++++++--
arch/arm64/kernel/asm-offsets.c | 13 ++++
arch/arm64/kernel/entry-ftrace.S | 117 ++++++++++++------------------
arch/arm64/kernel/ftrace.c | 82 ++++++++++++---------
arch/arm64/kernel/module.c | 3 -
arch/powerpc/include/asm/ftrace.h | 24 +++++-
arch/s390/include/asm/ftrace.h | 29 +++++++-
arch/x86/include/asm/ftrace.h | 49 +++++++++----
include/linux/ftrace.h | 47 +++++++++---
kernel/livepatch/patch.c | 2 +-
kernel/trace/Kconfig | 6 +-
kernel/trace/ftrace.c | 3 +-
14 files changed, 309 insertions(+), 158 deletions(-)
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 1/4] ftrace: pass fregs to arch_ftrace_set_direct_caller()
2022-11-03 17:05 ` Mark Rutland
@ 2022-11-03 17:05 ` Mark Rutland
-1 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2022-11-03 17:05 UTC (permalink / raw)
To: linux-kernel
Cc: catalin.marinas, linux-arm-kernel, mark.rutland, mhiramat,
revest, rostedt, will
In subsequent patches we'll arrange for architectures to have an
ftrace_regs which is entirely distinct from pt_regs. In preparation for
this, we need to minimize the use of pt_regs to where strictly
necessary in the core ftrace code.
This patch changes the prototype of arch_ftrace_set_direct_caller() to
take ftrace_regs rather than pt_regs, and moves the extraction of the
pt_regs into arch_ftrace_set_direct_caller().
On x86, arch_ftrace_set_direct_caller() can be used even when
CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS=n, and <linux/ftrace.h> defines
struct ftrace_regs. Due to this, it's necessary to define
arch_ftrace_set_direct_caller() as a macro to avoid using an incomplete
type. I've also moved the body of arch_ftrace_set_direct_caller() after
the CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS=y defineidion of struct
ftrace_regs.
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Florent Revest <revest@chromium.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
arch/s390/include/asm/ftrace.h | 5 ++++-
arch/x86/include/asm/ftrace.h | 31 ++++++++++++++++++-------------
include/linux/ftrace.h | 9 ++++-----
kernel/trace/ftrace.c | 3 +--
4 files changed, 27 insertions(+), 21 deletions(-)
diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
index 6f80ec9c04be9..85bc26ccdb872 100644
--- a/arch/s390/include/asm/ftrace.h
+++ b/arch/s390/include/asm/ftrace.h
@@ -60,6 +60,7 @@ static __always_inline void ftrace_instruction_pointer_set(struct ftrace_regs *f
fregs->regs.psw.addr = ip;
}
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
/*
* When an ftrace registered caller is tracing a function that is
* also set by a register_ftrace_direct() call, it needs to be
@@ -67,10 +68,12 @@ static __always_inline void ftrace_instruction_pointer_set(struct ftrace_regs *f
* place the direct caller in the ORIG_GPR2 part of pt_regs. This
* tells the ftrace_caller that there's a direct caller.
*/
-static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
+static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, unsigned long addr)
{
+ struct pt_regs *regs = &fregs->regs;
regs->orig_gpr2 = addr;
}
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
/*
* Even though the system call numbers are identical for s390/s390x a
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index 908d99b127d31..d2350a8351fa4 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -34,19 +34,6 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
return addr;
}
-/*
- * When a ftrace registered caller is tracing a function that is
- * also set by a register_ftrace_direct() call, it needs to be
- * differentiated in the ftrace_caller trampoline. To do this, we
- * place the direct caller in the ORIG_AX part of pt_regs. This
- * tells the ftrace_caller that there's a direct caller.
- */
-static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
-{
- /* Emulate a call */
- regs->orig_ax = addr;
-}
-
#ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
struct ftrace_regs {
struct pt_regs regs;
@@ -72,6 +59,24 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
#define FTRACE_GRAPH_TRAMP_ADDR FTRACE_GRAPH_ADDR
#endif
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+/*
+ * When a ftrace registered caller is tracing a function that is
+ * also set by a register_ftrace_direct() call, it needs to be
+ * differentiated in the ftrace_caller trampoline. To do this, we
+ * place the direct caller in the ORIG_AX part of pt_regs. This
+ * tells the ftrace_caller that there's a direct caller.
+ */
+static inline void
+__arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
+{
+ /* Emulate a call */
+ regs->orig_ax = addr;
+}
+#define arch_ftrace_set_direct_caller(fregs, addr) \
+ __arch_ftrace_set_direct_caller(&(fregs)->regs, addr)
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
+
#ifdef CONFIG_DYNAMIC_FTRACE
struct dyn_arch_ftrace {
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 62557d4bffc2b..f201fcbfffb07 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -37,9 +37,10 @@ extern void ftrace_boot_snapshot(void);
static inline void ftrace_boot_snapshot(void) { }
#endif
-#ifdef CONFIG_FUNCTION_TRACER
struct ftrace_ops;
struct ftrace_regs;
+
+#ifdef CONFIG_FUNCTION_TRACER
/*
* If the arch's mcount caller does not support all of ftrace's
* features, then it must call an indirect function that
@@ -427,9 +428,7 @@ static inline int modify_ftrace_direct_multi_nolock(struct ftrace_ops *ops, unsi
{
return -ENODEV;
}
-#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
-#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
/*
* This must be implemented by the architecture.
* It is the way the ftrace direct_ops helper, when called
@@ -443,9 +442,9 @@ static inline int modify_ftrace_direct_multi_nolock(struct ftrace_ops *ops, unsi
* the return from the trampoline jump to the direct caller
* instead of going back to the function it just traced.
*/
-static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs,
+static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs,
unsigned long addr) { }
-#endif /* CONFIG_HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
#ifdef CONFIG_STACK_TRACER
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index fbf2543111c05..234c5414deeeb 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2487,14 +2487,13 @@ ftrace_add_rec_direct(unsigned long ip, unsigned long addr,
static void call_direct_funcs(unsigned long ip, unsigned long pip,
struct ftrace_ops *ops, struct ftrace_regs *fregs)
{
- struct pt_regs *regs = ftrace_get_regs(fregs);
unsigned long addr;
addr = ftrace_find_rec_direct(ip);
if (!addr)
return;
- arch_ftrace_set_direct_caller(regs, addr);
+ arch_ftrace_set_direct_caller(fregs, addr);
}
struct ftrace_ops direct_ops = {
--
2.30.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 1/4] ftrace: pass fregs to arch_ftrace_set_direct_caller()
@ 2022-11-03 17:05 ` Mark Rutland
0 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2022-11-03 17:05 UTC (permalink / raw)
To: linux-kernel
Cc: catalin.marinas, linux-arm-kernel, mark.rutland, mhiramat,
revest, rostedt, will
In subsequent patches we'll arrange for architectures to have an
ftrace_regs which is entirely distinct from pt_regs. In preparation for
this, we need to minimize the use of pt_regs to where strictly
necessary in the core ftrace code.
This patch changes the prototype of arch_ftrace_set_direct_caller() to
take ftrace_regs rather than pt_regs, and moves the extraction of the
pt_regs into arch_ftrace_set_direct_caller().
On x86, arch_ftrace_set_direct_caller() can be used even when
CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS=n, and <linux/ftrace.h> defines
struct ftrace_regs. Due to this, it's necessary to define
arch_ftrace_set_direct_caller() as a macro to avoid using an incomplete
type. I've also moved the body of arch_ftrace_set_direct_caller() after
the CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS=y defineidion of struct
ftrace_regs.
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Florent Revest <revest@chromium.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
arch/s390/include/asm/ftrace.h | 5 ++++-
arch/x86/include/asm/ftrace.h | 31 ++++++++++++++++++-------------
include/linux/ftrace.h | 9 ++++-----
kernel/trace/ftrace.c | 3 +--
4 files changed, 27 insertions(+), 21 deletions(-)
diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
index 6f80ec9c04be9..85bc26ccdb872 100644
--- a/arch/s390/include/asm/ftrace.h
+++ b/arch/s390/include/asm/ftrace.h
@@ -60,6 +60,7 @@ static __always_inline void ftrace_instruction_pointer_set(struct ftrace_regs *f
fregs->regs.psw.addr = ip;
}
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
/*
* When an ftrace registered caller is tracing a function that is
* also set by a register_ftrace_direct() call, it needs to be
@@ -67,10 +68,12 @@ static __always_inline void ftrace_instruction_pointer_set(struct ftrace_regs *f
* place the direct caller in the ORIG_GPR2 part of pt_regs. This
* tells the ftrace_caller that there's a direct caller.
*/
-static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
+static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, unsigned long addr)
{
+ struct pt_regs *regs = &fregs->regs;
regs->orig_gpr2 = addr;
}
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
/*
* Even though the system call numbers are identical for s390/s390x a
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index 908d99b127d31..d2350a8351fa4 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -34,19 +34,6 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
return addr;
}
-/*
- * When a ftrace registered caller is tracing a function that is
- * also set by a register_ftrace_direct() call, it needs to be
- * differentiated in the ftrace_caller trampoline. To do this, we
- * place the direct caller in the ORIG_AX part of pt_regs. This
- * tells the ftrace_caller that there's a direct caller.
- */
-static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
-{
- /* Emulate a call */
- regs->orig_ax = addr;
-}
-
#ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
struct ftrace_regs {
struct pt_regs regs;
@@ -72,6 +59,24 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
#define FTRACE_GRAPH_TRAMP_ADDR FTRACE_GRAPH_ADDR
#endif
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+/*
+ * When a ftrace registered caller is tracing a function that is
+ * also set by a register_ftrace_direct() call, it needs to be
+ * differentiated in the ftrace_caller trampoline. To do this, we
+ * place the direct caller in the ORIG_AX part of pt_regs. This
+ * tells the ftrace_caller that there's a direct caller.
+ */
+static inline void
+__arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
+{
+ /* Emulate a call */
+ regs->orig_ax = addr;
+}
+#define arch_ftrace_set_direct_caller(fregs, addr) \
+ __arch_ftrace_set_direct_caller(&(fregs)->regs, addr)
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
+
#ifdef CONFIG_DYNAMIC_FTRACE
struct dyn_arch_ftrace {
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 62557d4bffc2b..f201fcbfffb07 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -37,9 +37,10 @@ extern void ftrace_boot_snapshot(void);
static inline void ftrace_boot_snapshot(void) { }
#endif
-#ifdef CONFIG_FUNCTION_TRACER
struct ftrace_ops;
struct ftrace_regs;
+
+#ifdef CONFIG_FUNCTION_TRACER
/*
* If the arch's mcount caller does not support all of ftrace's
* features, then it must call an indirect function that
@@ -427,9 +428,7 @@ static inline int modify_ftrace_direct_multi_nolock(struct ftrace_ops *ops, unsi
{
return -ENODEV;
}
-#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
-#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
/*
* This must be implemented by the architecture.
* It is the way the ftrace direct_ops helper, when called
@@ -443,9 +442,9 @@ static inline int modify_ftrace_direct_multi_nolock(struct ftrace_ops *ops, unsi
* the return from the trampoline jump to the direct caller
* instead of going back to the function it just traced.
*/
-static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs,
+static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs,
unsigned long addr) { }
-#endif /* CONFIG_HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
#ifdef CONFIG_STACK_TRACER
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index fbf2543111c05..234c5414deeeb 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2487,14 +2487,13 @@ ftrace_add_rec_direct(unsigned long ip, unsigned long addr,
static void call_direct_funcs(unsigned long ip, unsigned long pip,
struct ftrace_ops *ops, struct ftrace_regs *fregs)
{
- struct pt_regs *regs = ftrace_get_regs(fregs);
unsigned long addr;
addr = ftrace_find_rec_direct(ip);
if (!addr)
return;
- arch_ftrace_set_direct_caller(regs, addr);
+ arch_ftrace_set_direct_caller(fregs, addr);
}
struct ftrace_ops direct_ops = {
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 2/4] ftrace: rename ftrace_instruction_pointer_set() -> ftrace_regs_set_instruction_pointer()
2022-11-03 17:05 ` Mark Rutland
@ 2022-11-03 17:05 ` Mark Rutland
-1 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2022-11-03 17:05 UTC (permalink / raw)
To: linux-kernel
Cc: catalin.marinas, linux-arm-kernel, mark.rutland, mhiramat,
revest, rostedt, will
In subsequent patches we'll add a sew of ftrace_regs_{get,set}_*()
helpers. In preparation, this patch renames
ftrace_instruction_pointer_set() to
ftrace_regs_set_instruction_pointer().
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Florent Revest <revest@chromium.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
arch/powerpc/include/asm/ftrace.h | 5 +++--
arch/s390/include/asm/ftrace.h | 5 +++--
arch/x86/include/asm/ftrace.h | 2 +-
include/linux/ftrace.h | 9 ++++-----
kernel/livepatch/patch.c | 2 +-
5 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index 3cee7115441b4..c3eb48f675669 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -37,8 +37,9 @@ static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *
return fregs->regs.msr ? &fregs->regs : NULL;
}
-static __always_inline void ftrace_instruction_pointer_set(struct ftrace_regs *fregs,
- unsigned long ip)
+static __always_inline void
+ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
+ unsigned long ip)
{
regs_set_return_ip(&fregs->regs, ip);
}
diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
index 85bc26ccdb872..113d194acea01 100644
--- a/arch/s390/include/asm/ftrace.h
+++ b/arch/s390/include/asm/ftrace.h
@@ -54,8 +54,9 @@ static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *
return NULL;
}
-static __always_inline void ftrace_instruction_pointer_set(struct ftrace_regs *fregs,
- unsigned long ip)
+static __always_inline void
+ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
+ unsigned long ip)
{
fregs->regs.psw.addr = ip;
}
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index d2350a8351fa4..b2925d342c650 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -48,7 +48,7 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs)
return &fregs->regs;
}
-#define ftrace_instruction_pointer_set(fregs, _ip) \
+#define ftrace_regs_set_instruction_pointer(fregs, _ip) \
do { (fregs)->regs.ip = (_ip); } while (0)
struct ftrace_ops;
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index f201fcbfffb07..ec3657a60f850 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -111,12 +111,11 @@ struct ftrace_regs {
#define arch_ftrace_get_regs(fregs) (&(fregs)->regs)
/*
- * ftrace_instruction_pointer_set() is to be defined by the architecture
- * if to allow setting of the instruction pointer from the ftrace_regs
- * when HAVE_DYNAMIC_FTRACE_WITH_ARGS is set and it supports
- * live kernel patching.
+ * ftrace_regs_set_instruction_pointer() is to be defined by the architecture
+ * if to allow setting of the instruction pointer from the ftrace_regs when
+ * HAVE_DYNAMIC_FTRACE_WITH_ARGS is set and it supports live kernel patching.
*/
-#define ftrace_instruction_pointer_set(fregs, ip) do { } while (0)
+#define ftrace_regs_set_instruction_pointer(fregs, ip) do { } while (0)
#endif /* CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS */
static __always_inline struct pt_regs *ftrace_get_regs(struct ftrace_regs *fregs)
diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
index 4c4f5a776d808..4152c71507e24 100644
--- a/kernel/livepatch/patch.c
+++ b/kernel/livepatch/patch.c
@@ -118,7 +118,7 @@ static void notrace klp_ftrace_handler(unsigned long ip,
if (func->nop)
goto unlock;
- ftrace_instruction_pointer_set(fregs, (unsigned long)func->new_func);
+ ftrace_regs_set_instruction_pointer(fregs, (unsigned long)func->new_func);
unlock:
ftrace_test_recursion_unlock(bit);
--
2.30.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 2/4] ftrace: rename ftrace_instruction_pointer_set() -> ftrace_regs_set_instruction_pointer()
@ 2022-11-03 17:05 ` Mark Rutland
0 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2022-11-03 17:05 UTC (permalink / raw)
To: linux-kernel
Cc: catalin.marinas, linux-arm-kernel, mark.rutland, mhiramat,
revest, rostedt, will
In subsequent patches we'll add a sew of ftrace_regs_{get,set}_*()
helpers. In preparation, this patch renames
ftrace_instruction_pointer_set() to
ftrace_regs_set_instruction_pointer().
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Florent Revest <revest@chromium.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
arch/powerpc/include/asm/ftrace.h | 5 +++--
arch/s390/include/asm/ftrace.h | 5 +++--
arch/x86/include/asm/ftrace.h | 2 +-
include/linux/ftrace.h | 9 ++++-----
kernel/livepatch/patch.c | 2 +-
5 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index 3cee7115441b4..c3eb48f675669 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -37,8 +37,9 @@ static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *
return fregs->regs.msr ? &fregs->regs : NULL;
}
-static __always_inline void ftrace_instruction_pointer_set(struct ftrace_regs *fregs,
- unsigned long ip)
+static __always_inline void
+ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
+ unsigned long ip)
{
regs_set_return_ip(&fregs->regs, ip);
}
diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
index 85bc26ccdb872..113d194acea01 100644
--- a/arch/s390/include/asm/ftrace.h
+++ b/arch/s390/include/asm/ftrace.h
@@ -54,8 +54,9 @@ static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *
return NULL;
}
-static __always_inline void ftrace_instruction_pointer_set(struct ftrace_regs *fregs,
- unsigned long ip)
+static __always_inline void
+ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
+ unsigned long ip)
{
fregs->regs.psw.addr = ip;
}
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index d2350a8351fa4..b2925d342c650 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -48,7 +48,7 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs)
return &fregs->regs;
}
-#define ftrace_instruction_pointer_set(fregs, _ip) \
+#define ftrace_regs_set_instruction_pointer(fregs, _ip) \
do { (fregs)->regs.ip = (_ip); } while (0)
struct ftrace_ops;
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index f201fcbfffb07..ec3657a60f850 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -111,12 +111,11 @@ struct ftrace_regs {
#define arch_ftrace_get_regs(fregs) (&(fregs)->regs)
/*
- * ftrace_instruction_pointer_set() is to be defined by the architecture
- * if to allow setting of the instruction pointer from the ftrace_regs
- * when HAVE_DYNAMIC_FTRACE_WITH_ARGS is set and it supports
- * live kernel patching.
+ * ftrace_regs_set_instruction_pointer() is to be defined by the architecture
+ * if to allow setting of the instruction pointer from the ftrace_regs when
+ * HAVE_DYNAMIC_FTRACE_WITH_ARGS is set and it supports live kernel patching.
*/
-#define ftrace_instruction_pointer_set(fregs, ip) do { } while (0)
+#define ftrace_regs_set_instruction_pointer(fregs, ip) do { } while (0)
#endif /* CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS */
static __always_inline struct pt_regs *ftrace_get_regs(struct ftrace_regs *fregs)
diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
index 4c4f5a776d808..4152c71507e24 100644
--- a/kernel/livepatch/patch.c
+++ b/kernel/livepatch/patch.c
@@ -118,7 +118,7 @@ static void notrace klp_ftrace_handler(unsigned long ip,
if (func->nop)
goto unlock;
- ftrace_instruction_pointer_set(fregs, (unsigned long)func->new_func);
+ ftrace_regs_set_instruction_pointer(fregs, (unsigned long)func->new_func);
unlock:
ftrace_test_recursion_unlock(bit);
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 3/4] ftrace: abstract DYNAMIC_FTRACE_WITH_ARGS accesses
2022-11-03 17:05 ` Mark Rutland
@ 2022-11-03 17:05 ` Mark Rutland
-1 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2022-11-03 17:05 UTC (permalink / raw)
To: linux-kernel
Cc: catalin.marinas, linux-arm-kernel, mark.rutland, mhiramat,
revest, rostedt, will
In subsequent patches we'll arrange for architectures to have an
ftrace_regs which is entirely distinct from pt_regs. In preparation for
this, we need to minimize the use of pt_regs to where strictly necessary
in the core ftrace code.
This patch adds new ftrace_regs_{get,set}_*() helpers which can be used
to manipulate ftrace_regs. When CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS=y,
these can always be used on any ftrace_regs, and when
CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS=n these can be used when regs are
available. A new ftrace_regs_has_args(fregs) helper is added which code
can use to check when these are usable.
Co-developed-by: Florent Revest <revest@chromium.org>
Signed-off-by: Florent Revest <revest@chromium.org>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
arch/powerpc/include/asm/ftrace.h | 19 +++++++++++++++++++
arch/s390/include/asm/ftrace.h | 19 +++++++++++++++++++
arch/x86/include/asm/ftrace.h | 16 ++++++++++++++++
include/linux/ftrace.h | 29 +++++++++++++++++++++++++++++
kernel/trace/Kconfig | 6 +++---
5 files changed, 86 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index c3eb48f675669..259b9dd5fe1c5 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -44,6 +44,25 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
regs_set_return_ip(&fregs->regs, ip);
}
+static __always_inline unsigned long
+ftrace_regs_get_instruction_pointer(struct ftrace_regs *fregs)
+{
+ return instruction_pointer(&fregs->regs);
+}
+
+#define ftrace_regs_get_argument(fregs, n) \
+ regs_get_kernel_argument(&(fregs)->regs, n)
+#define ftrace_regs_get_stack_pointer(fregs) \
+ kernel_stack_pointer(&(fregs)->regs)
+#define ftrace_regs_return_value(fregs) \
+ regs_return_value(&(fregs)->regs)
+#define ftrace_regs_set_return_value(fregs, ret) \
+ regs_set_return_value(&(fregs)->regs, ret)
+#define ftrace_override_function_with_return(fregs) \
+ override_function_with_return(&(fregs)->regs)
+#define ftrace_regs_query_register_offset(name) \
+ regs_query_register_offset(name)
+
struct ftrace_ops;
#define ftrace_graph_func ftrace_graph_func
diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
index 113d194acea01..e5c5cb1207e2c 100644
--- a/arch/s390/include/asm/ftrace.h
+++ b/arch/s390/include/asm/ftrace.h
@@ -54,6 +54,12 @@ static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *
return NULL;
}
+static __always_inline unsigned long
+ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs)
+{
+ return fregs->regs.psw.addr;
+}
+
static __always_inline void
ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
unsigned long ip)
@@ -61,6 +67,19 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
fregs->regs.psw.addr = ip;
}
+#define ftrace_regs_get_argument(fregs, n) \
+ regs_get_kernel_argument(&(fregs)->regs, n)
+#define ftrace_regs_get_stack_pointer(fregs) \
+ kernel_stack_pointer(&(fregs)->regs)
+#define ftrace_regs_return_value(fregs) \
+ regs_return_value(&(fregs)->regs)
+#define ftrace_regs_set_return_value(fregs, ret) \
+ regs_set_return_value(&(fregs)->regs, ret)
+#define ftrace_override_function_with_return(fregs) \
+ override_function_with_return(&(fregs)->regs)
+#define ftrace_regs_query_register_offset(name) \
+ regs_query_register_offset(name)
+
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
/*
* When an ftrace registered caller is tracing a function that is
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index b2925d342c650..5061ac98ffa16 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -51,6 +51,22 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs)
#define ftrace_regs_set_instruction_pointer(fregs, _ip) \
do { (fregs)->regs.ip = (_ip); } while (0)
+#define ftrace_regs_get_instruction_pointer(fregs) \
+ ((fregs)->regs.ip)
+
+#define ftrace_regs_get_argument(fregs, n) \
+ regs_get_kernel_argument(&(fregs)->regs, n)
+#define ftrace_regs_get_stack_pointer(fregs) \
+ kernel_stack_pointer(&(fregs)->regs)
+#define ftrace_regs_return_value(fregs) \
+ regs_return_value(&(fregs)->regs)
+#define ftrace_regs_set_return_value(fregs, ret) \
+ regs_set_return_value(&(fregs)->regs, ret)
+#define ftrace_override_function_with_return(fregs) \
+ override_function_with_return(&(fregs)->regs)
+#define ftrace_regs_query_register_offset(name) \
+ regs_query_register_offset(name)
+
struct ftrace_ops;
#define ftrace_graph_func ftrace_graph_func
void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index ec3657a60f850..99f1146614c04 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -126,6 +126,35 @@ static __always_inline struct pt_regs *ftrace_get_regs(struct ftrace_regs *fregs
return arch_ftrace_get_regs(fregs);
}
+/*
+ * When true, the ftrace_regs_{get,set}_*() functions may be used on fregs.
+ * Note: this can be true even when ftrace_get_regs() cannot provide a pt_regs.
+ */
+static __always_inline bool ftrace_regs_has_args(struct ftrace_regs *fregs)
+{
+ if (IS_ENABLED(CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS))
+ return true;
+
+ return ftrace_get_regs(fregs) != NULL;
+}
+
+#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
+#define ftrace_regs_get_instruction_pointer(fregs) \
+ instruction_pointer(ftrace_get_regs(fregs))
+#define ftrace_regs_get_argument(fregs, n) \
+ regs_get_kernel_argument(ftrace_get_regs(fregs), n)
+#define ftrace_regs_get_stack_pointer(fregs) \
+ kernel_stack_pointer(ftrace_get_regs(fregs))
+#define ftrace_regs_return_value(fregs) \
+ regs_return_value(ftrace_get_regs(fregs))
+#define ftrace_regs_set_return_value(fregs, ret) \
+ regs_set_return_value(ftrace_get_regs(fregs), ret)
+#define ftrace_override_function_with_return(fregs) \
+ override_function_with_return(ftrace_get_regs(fregs))
+#define ftrace_regs_query_register_offset(name) \
+ regs_query_register_offset(name)
+#endif
+
typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *op, struct ftrace_regs *fregs);
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index e9e95c790b8ee..2c6611c13f99e 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -46,10 +46,10 @@ config HAVE_DYNAMIC_FTRACE_WITH_ARGS
bool
help
If this is set, then arguments and stack can be found from
- the pt_regs passed into the function callback regs parameter
+ the ftrace_regs passed into the function callback regs parameter
by default, even without setting the REGS flag in the ftrace_ops.
- This allows for use of regs_get_kernel_argument() and
- kernel_stack_pointer().
+ This allows for use of ftrace_regs_get_argument() and
+ ftrace_regs_get_stack_pointer().
config HAVE_DYNAMIC_FTRACE_NO_PATCHABLE
bool
--
2.30.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 3/4] ftrace: abstract DYNAMIC_FTRACE_WITH_ARGS accesses
@ 2022-11-03 17:05 ` Mark Rutland
0 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2022-11-03 17:05 UTC (permalink / raw)
To: linux-kernel
Cc: catalin.marinas, linux-arm-kernel, mark.rutland, mhiramat,
revest, rostedt, will
In subsequent patches we'll arrange for architectures to have an
ftrace_regs which is entirely distinct from pt_regs. In preparation for
this, we need to minimize the use of pt_regs to where strictly necessary
in the core ftrace code.
This patch adds new ftrace_regs_{get,set}_*() helpers which can be used
to manipulate ftrace_regs. When CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS=y,
these can always be used on any ftrace_regs, and when
CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS=n these can be used when regs are
available. A new ftrace_regs_has_args(fregs) helper is added which code
can use to check when these are usable.
Co-developed-by: Florent Revest <revest@chromium.org>
Signed-off-by: Florent Revest <revest@chromium.org>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
arch/powerpc/include/asm/ftrace.h | 19 +++++++++++++++++++
arch/s390/include/asm/ftrace.h | 19 +++++++++++++++++++
arch/x86/include/asm/ftrace.h | 16 ++++++++++++++++
include/linux/ftrace.h | 29 +++++++++++++++++++++++++++++
kernel/trace/Kconfig | 6 +++---
5 files changed, 86 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index c3eb48f675669..259b9dd5fe1c5 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -44,6 +44,25 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
regs_set_return_ip(&fregs->regs, ip);
}
+static __always_inline unsigned long
+ftrace_regs_get_instruction_pointer(struct ftrace_regs *fregs)
+{
+ return instruction_pointer(&fregs->regs);
+}
+
+#define ftrace_regs_get_argument(fregs, n) \
+ regs_get_kernel_argument(&(fregs)->regs, n)
+#define ftrace_regs_get_stack_pointer(fregs) \
+ kernel_stack_pointer(&(fregs)->regs)
+#define ftrace_regs_return_value(fregs) \
+ regs_return_value(&(fregs)->regs)
+#define ftrace_regs_set_return_value(fregs, ret) \
+ regs_set_return_value(&(fregs)->regs, ret)
+#define ftrace_override_function_with_return(fregs) \
+ override_function_with_return(&(fregs)->regs)
+#define ftrace_regs_query_register_offset(name) \
+ regs_query_register_offset(name)
+
struct ftrace_ops;
#define ftrace_graph_func ftrace_graph_func
diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
index 113d194acea01..e5c5cb1207e2c 100644
--- a/arch/s390/include/asm/ftrace.h
+++ b/arch/s390/include/asm/ftrace.h
@@ -54,6 +54,12 @@ static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *
return NULL;
}
+static __always_inline unsigned long
+ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs)
+{
+ return fregs->regs.psw.addr;
+}
+
static __always_inline void
ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
unsigned long ip)
@@ -61,6 +67,19 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
fregs->regs.psw.addr = ip;
}
+#define ftrace_regs_get_argument(fregs, n) \
+ regs_get_kernel_argument(&(fregs)->regs, n)
+#define ftrace_regs_get_stack_pointer(fregs) \
+ kernel_stack_pointer(&(fregs)->regs)
+#define ftrace_regs_return_value(fregs) \
+ regs_return_value(&(fregs)->regs)
+#define ftrace_regs_set_return_value(fregs, ret) \
+ regs_set_return_value(&(fregs)->regs, ret)
+#define ftrace_override_function_with_return(fregs) \
+ override_function_with_return(&(fregs)->regs)
+#define ftrace_regs_query_register_offset(name) \
+ regs_query_register_offset(name)
+
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
/*
* When an ftrace registered caller is tracing a function that is
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index b2925d342c650..5061ac98ffa16 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -51,6 +51,22 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs)
#define ftrace_regs_set_instruction_pointer(fregs, _ip) \
do { (fregs)->regs.ip = (_ip); } while (0)
+#define ftrace_regs_get_instruction_pointer(fregs) \
+ ((fregs)->regs.ip)
+
+#define ftrace_regs_get_argument(fregs, n) \
+ regs_get_kernel_argument(&(fregs)->regs, n)
+#define ftrace_regs_get_stack_pointer(fregs) \
+ kernel_stack_pointer(&(fregs)->regs)
+#define ftrace_regs_return_value(fregs) \
+ regs_return_value(&(fregs)->regs)
+#define ftrace_regs_set_return_value(fregs, ret) \
+ regs_set_return_value(&(fregs)->regs, ret)
+#define ftrace_override_function_with_return(fregs) \
+ override_function_with_return(&(fregs)->regs)
+#define ftrace_regs_query_register_offset(name) \
+ regs_query_register_offset(name)
+
struct ftrace_ops;
#define ftrace_graph_func ftrace_graph_func
void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index ec3657a60f850..99f1146614c04 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -126,6 +126,35 @@ static __always_inline struct pt_regs *ftrace_get_regs(struct ftrace_regs *fregs
return arch_ftrace_get_regs(fregs);
}
+/*
+ * When true, the ftrace_regs_{get,set}_*() functions may be used on fregs.
+ * Note: this can be true even when ftrace_get_regs() cannot provide a pt_regs.
+ */
+static __always_inline bool ftrace_regs_has_args(struct ftrace_regs *fregs)
+{
+ if (IS_ENABLED(CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS))
+ return true;
+
+ return ftrace_get_regs(fregs) != NULL;
+}
+
+#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
+#define ftrace_regs_get_instruction_pointer(fregs) \
+ instruction_pointer(ftrace_get_regs(fregs))
+#define ftrace_regs_get_argument(fregs, n) \
+ regs_get_kernel_argument(ftrace_get_regs(fregs), n)
+#define ftrace_regs_get_stack_pointer(fregs) \
+ kernel_stack_pointer(ftrace_get_regs(fregs))
+#define ftrace_regs_return_value(fregs) \
+ regs_return_value(ftrace_get_regs(fregs))
+#define ftrace_regs_set_return_value(fregs, ret) \
+ regs_set_return_value(ftrace_get_regs(fregs), ret)
+#define ftrace_override_function_with_return(fregs) \
+ override_function_with_return(ftrace_get_regs(fregs))
+#define ftrace_regs_query_register_offset(name) \
+ regs_query_register_offset(name)
+#endif
+
typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *op, struct ftrace_regs *fregs);
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index e9e95c790b8ee..2c6611c13f99e 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -46,10 +46,10 @@ config HAVE_DYNAMIC_FTRACE_WITH_ARGS
bool
help
If this is set, then arguments and stack can be found from
- the pt_regs passed into the function callback regs parameter
+ the ftrace_regs passed into the function callback regs parameter
by default, even without setting the REGS flag in the ftrace_ops.
- This allows for use of regs_get_kernel_argument() and
- kernel_stack_pointer().
+ This allows for use of ftrace_regs_get_argument() and
+ ftrace_regs_get_stack_pointer().
config HAVE_DYNAMIC_FTRACE_NO_PATCHABLE
bool
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS
2022-11-03 17:05 ` Mark Rutland
@ 2022-11-03 17:05 ` Mark Rutland
-1 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2022-11-03 17:05 UTC (permalink / raw)
To: linux-kernel
Cc: catalin.marinas, linux-arm-kernel, mark.rutland, mhiramat,
revest, rostedt, will
This commit replaces arm64's support for FTRACE_WITH_REGS with support
for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
removes some latent issues with inconsistent presentation of struct
pt_regs (which can only be reliably saved/restored at exception
boundaries).
FTRACE_WITH_REGS has been supported on arm64 since commit:
3b23e4991fb66f6d ("arm64: implement ftrace with regs")
As noted in the commit message, the major reasons for implementing
FTRACE_WITH_REGS were:
(1) To make it possible to use the ftrace graph tracer with pointer
authentication, where it's necessary to snapshot/manipulate the LR
before it is signed by the instrumented function.
(2) To make it possible to implement LIVEPATCH in future, where we need
to hook function entry before an instrumented function manipulates
the stack or argument registers. Practically speaking, we need to
preserve the argument/return registers, PC, LR, and SP.
Neither of these need a struct pt_regs, and only require the set of
registers which are live at function call/return boundaries. Our calling
convention is defined by "Procedure Call Standard for the Arm® 64-bit
Architecture (AArch64)" (AKA "AAPCS64"), which can currently be found
at:
https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst
Per AAPCS64, all function call argument and return values are held in
the following GPRs:
* X0 - X7 : parameter / result registers
* X8 : indirect result location register
* SP : stack pointer (AKA SP)
Additionally, ad function call boundaries, the following GPRs hold
context/return information:
* X29 : frame pointer (AKA FP)
* X30 : link register (AKA LR)
... and for ftrace we need to capture the instrumented address:
* PC : program counter
No other GPRs are relevant, as none of the other arguments hold
parameters or return values:
* X9 - X17 : temporaries, may be clobbered
* X18 : shadow call stack pointer (or temorary)
* X19 - X28 : callee saved
This patch implements FTRACE_WITH_ARGS for arm64, only saving/restoring
the minimal set of registers necessary. This is always sufficient to
manipulate control flow (e.g. for live-patching) or to manipulate
function arguments and return values.
This reduces the necessary stack usage from 336 bytes for pt_regs down
to 112 bytes for ftrace_regs + 32 bytes for two frame records, freeing
up 188 bytes. This could be reduced further with changes to the
unwinder.
As there is no longer a need to save different sets of registers for
different features, we no longer need distinct `ftrace_caller` and
`ftrace_regs_caller` trampolines. This allows the trampoline assembly to
be simpler, and simplifies code which previously had to handle the two
trampolines.
I've tested this with the ftrace selftests, where there are no
unexpected failures.
Co-developed-by: Florent Revest <revest@chromium.org>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Florent Revest <revest@chromium.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/Kconfig | 18 ++---
arch/arm64/Makefile | 2 +-
arch/arm64/include/asm/ftrace.h | 72 +++++++++++++++++--
arch/arm64/kernel/asm-offsets.c | 13 ++++
arch/arm64/kernel/entry-ftrace.S | 117 ++++++++++++-------------------
arch/arm64/kernel/ftrace.c | 82 +++++++++++++---------
arch/arm64/kernel/module.c | 3 -
7 files changed, 184 insertions(+), 123 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 505c8a1ccbe0c..b6b3305ba7013 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -181,8 +181,10 @@ config ARM64
select HAVE_DEBUG_KMEMLEAK
select HAVE_DMA_CONTIGUOUS
select HAVE_DYNAMIC_FTRACE
+ select HAVE_DYNAMIC_FTRACE_WITH_ARGS \
+ if $(cc-option,-fpatchable-function-entry=2)
select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY \
- if DYNAMIC_FTRACE_WITH_REGS
+ if DYNAMIC_FTRACE_WITH_ARGS
select HAVE_EFFICIENT_UNALIGNED_ACCESS
select HAVE_FAST_GUP
select HAVE_FTRACE_MCOUNT_RECORD
@@ -233,16 +235,16 @@ config ARM64
help
ARM 64-bit (AArch64) Linux support.
-config CLANG_SUPPORTS_DYNAMIC_FTRACE_WITH_REGS
+config CLANG_SUPPORTS_DYNAMIC_FTRACE_WITH_ARGS
def_bool CC_IS_CLANG
# https://github.com/ClangBuiltLinux/linux/issues/1507
depends on AS_IS_GNU || (AS_IS_LLVM && (LD_IS_LLD || LD_VERSION >= 23600))
- select HAVE_DYNAMIC_FTRACE_WITH_REGS
+ select HAVE_DYNAMIC_FTRACE_WITH_ARGS
-config GCC_SUPPORTS_DYNAMIC_FTRACE_WITH_REGS
+config GCC_SUPPORTS_DYNAMIC_FTRACE_WITH_ARGS
def_bool CC_IS_GCC
depends on $(cc-option,-fpatchable-function-entry=2)
- select HAVE_DYNAMIC_FTRACE_WITH_REGS
+ select HAVE_DYNAMIC_FTRACE_WITH_ARGS
config 64BIT
def_bool y
@@ -1816,7 +1818,7 @@ config ARM64_PTR_AUTH_KERNEL
# which is only understood by binutils starting with version 2.33.1.
depends on LD_IS_LLD || LD_VERSION >= 23301 || (CC_IS_GCC && GCC_VERSION < 90100)
depends on !CC_IS_CLANG || AS_HAS_CFI_NEGATE_RA_STATE
- depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
+ depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_ARGS)
help
If the compiler supports the -mbranch-protection or
-msign-return-address flag (e.g. GCC 7 or later), then this option
@@ -1826,7 +1828,7 @@ config ARM64_PTR_AUTH_KERNEL
disabled with minimal loss of protection.
This feature works with FUNCTION_GRAPH_TRACER option only if
- DYNAMIC_FTRACE_WITH_REGS is enabled.
+ DYNAMIC_FTRACE_WITH_ARGS is enabled.
config CC_HAS_BRANCH_PROT_PAC_RET
# GCC 9 or later, clang 8 or later
@@ -1924,7 +1926,7 @@ config ARM64_BTI_KERNEL
depends on !CC_IS_GCC
# https://github.com/llvm/llvm-project/commit/a88c722e687e6780dcd6a58718350dc76fcc4cc9
depends on !CC_IS_CLANG || CLANG_VERSION >= 120000
- depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
+ depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_ARGS)
help
Build the kernel with Branch Target Identification annotations
and enable enforcement of this for kernel code. When this option
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 5e56d26a22398..b1202fa84baba 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -128,7 +128,7 @@ endif
CHECKFLAGS += -D__aarch64__
-ifeq ($(CONFIG_DYNAMIC_FTRACE_WITH_REGS),y)
+ifeq ($(CONFIG_DYNAMIC_FTRACE_WITH_ARGS),y)
KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY
CC_FLAGS_FTRACE := -fpatchable-function-entry=2
endif
diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
index 329dbbd4d50b6..5664729800ae1 100644
--- a/arch/arm64/include/asm/ftrace.h
+++ b/arch/arm64/include/asm/ftrace.h
@@ -23,7 +23,7 @@
*/
#define HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
#define ARCH_SUPPORTS_FTRACE_OPS 1
#else
#define MCOUNT_ADDR ((unsigned long)_mcount)
@@ -33,8 +33,7 @@
#define MCOUNT_INSN_SIZE AARCH64_INSN_SIZE
#define FTRACE_PLT_IDX 0
-#define FTRACE_REGS_PLT_IDX 1
-#define NR_FTRACE_PLTS 2
+#define NR_FTRACE_PLTS 1
/*
* Currently, gcc tends to save the link register after the local variables
@@ -69,7 +68,7 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
* Adjust addr to point at the BL in the callsite.
* See ftrace_init_nop() for the callsite sequence.
*/
- if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS))
+ if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_ARGS))
return addr + AARCH64_INSN_SIZE;
/*
* addr is the address of the mcount call instruction.
@@ -78,10 +77,71 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
return addr;
}
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
struct dyn_ftrace;
struct ftrace_ops;
-struct ftrace_regs;
+
+#define arch_ftrace_get_regs(regs) NULL
+
+struct ftrace_regs {
+ /* x0 - x8 */
+ unsigned long regs[9];
+ unsigned long __unused;
+
+ unsigned long fp;
+ unsigned long lr;
+
+ unsigned long sp;
+ unsigned long pc;
+};
+
+static __always_inline unsigned long
+ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs)
+{
+ return fregs->pc;
+}
+
+static __always_inline void
+ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
+ unsigned long pc)
+{
+ fregs->pc = pc;
+}
+
+static __always_inline unsigned long
+ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs)
+{
+ return fregs->sp;
+}
+
+static __always_inline unsigned long
+ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n)
+{
+ if (n < 8)
+ return fregs->regs[n];
+ return 0;
+}
+
+static __always_inline unsigned long
+ftrace_regs_get_return_value(const struct ftrace_regs *fregs)
+{
+ return fregs->regs[0];
+}
+
+static __always_inline void
+ftrace_regs_set_return_value(struct ftrace_regs *fregs,
+ unsigned long ret)
+{
+ fregs->regs[0] = ret;
+}
+
+static __always_inline void
+ftrace_override_function_with_return(struct ftrace_regs *fregs)
+{
+ fregs->pc = fregs->lr;
+}
+
+int ftrace_regs_query_register_offset(const char *name);
int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec);
#define ftrace_init_nop ftrace_init_nop
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 1197e7679882e..2234624536d95 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -82,6 +82,19 @@ int main(void)
DEFINE(S_STACKFRAME, offsetof(struct pt_regs, stackframe));
DEFINE(PT_REGS_SIZE, sizeof(struct pt_regs));
BLANK();
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
+ DEFINE(FREGS_X0, offsetof(struct ftrace_regs, regs[0]));
+ DEFINE(FREGS_X2, offsetof(struct ftrace_regs, regs[2]));
+ DEFINE(FREGS_X4, offsetof(struct ftrace_regs, regs[4]));
+ DEFINE(FREGS_X6, offsetof(struct ftrace_regs, regs[6]));
+ DEFINE(FREGS_X8, offsetof(struct ftrace_regs, regs[8]));
+ DEFINE(FREGS_FP, offsetof(struct ftrace_regs, fp));
+ DEFINE(FREGS_LR, offsetof(struct ftrace_regs, lr));
+ DEFINE(FREGS_SP, offsetof(struct ftrace_regs, sp));
+ DEFINE(FREGS_PC, offsetof(struct ftrace_regs, pc));
+ DEFINE(FREGS_SIZE, sizeof(struct ftrace_regs));
+ BLANK();
+#endif
#ifdef CONFIG_COMPAT
DEFINE(COMPAT_SIGFRAME_REGS_OFFSET, offsetof(struct compat_sigframe, uc.uc_mcontext.arm_r0));
DEFINE(COMPAT_RT_SIGFRAME_REGS_OFFSET, offsetof(struct compat_rt_sigframe, sig.uc.uc_mcontext.arm_r0));
diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S
index 795344ab4ec45..4d3050549aa6e 100644
--- a/arch/arm64/kernel/entry-ftrace.S
+++ b/arch/arm64/kernel/entry-ftrace.S
@@ -13,83 +13,58 @@
#include <asm/ftrace.h>
#include <asm/insn.h>
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
/*
* Due to -fpatchable-function-entry=2, the compiler has placed two NOPs before
* the regular function prologue. For an enabled callsite, ftrace_init_nop() and
* ftrace_make_call() have patched those NOPs to:
*
* MOV X9, LR
- * BL <entry>
- *
- * ... where <entry> is either ftrace_caller or ftrace_regs_caller.
+ * BL ftrace_caller
*
* Each instrumented function follows the AAPCS, so here x0-x8 and x18-x30 are
* live (x18 holds the Shadow Call Stack pointer), and x9-x17 are safe to
* clobber.
*
- * We save the callsite's context into a pt_regs before invoking any ftrace
- * callbacks. So that we can get a sensible backtrace, we create a stack record
- * for the callsite and the ftrace entry assembly. This is not sufficient for
- * reliable stacktrace: until we create the callsite stack record, its caller
- * is missing from the LR and existing chain of frame records.
+ * We save the callsite's context into a struct ftrace_regs before invoking any
+ * ftrace callbacks. So that we can get a sensible backtrace, we create frame
+ * records for the callsite and the ftrace entry assembly. This is not
+ * sufficient for reliable stacktrace: until we create the callsite stack
+ * record, its caller is missing from the LR and existing chain of frame
+ * records.
*/
- .macro ftrace_regs_entry, allregs=0
- /* Make room for pt_regs, plus a callee frame */
- sub sp, sp, #(PT_REGS_SIZE + 16)
-
- /* Save function arguments (and x9 for simplicity) */
- stp x0, x1, [sp, #S_X0]
- stp x2, x3, [sp, #S_X2]
- stp x4, x5, [sp, #S_X4]
- stp x6, x7, [sp, #S_X6]
- stp x8, x9, [sp, #S_X8]
-
- /* Optionally save the callee-saved registers, always save the FP */
- .if \allregs == 1
- stp x10, x11, [sp, #S_X10]
- stp x12, x13, [sp, #S_X12]
- stp x14, x15, [sp, #S_X14]
- stp x16, x17, [sp, #S_X16]
- stp x18, x19, [sp, #S_X18]
- stp x20, x21, [sp, #S_X20]
- stp x22, x23, [sp, #S_X22]
- stp x24, x25, [sp, #S_X24]
- stp x26, x27, [sp, #S_X26]
- stp x28, x29, [sp, #S_X28]
- .else
- str x29, [sp, #S_FP]
- .endif
-
- /* Save the callsite's SP and LR */
- add x10, sp, #(PT_REGS_SIZE + 16)
- stp x9, x10, [sp, #S_LR]
+SYM_CODE_START(ftrace_caller)
+ bti c
- /* Save the PC after the ftrace callsite */
- str x30, [sp, #S_PC]
+ /* Save original SP */
+ mov x10, sp
- /* Create a frame record for the callsite above pt_regs */
- stp x29, x9, [sp, #PT_REGS_SIZE]
- add x29, sp, #PT_REGS_SIZE
+ /* Make room for ftrace regs, plus two frame records */
+ sub sp, sp, #(FREGS_SIZE + 32)
- /* Create our frame record within pt_regs. */
- stp x29, x30, [sp, #S_STACKFRAME]
- add x29, sp, #S_STACKFRAME
- .endm
+ /* Save function arguments */
+ stp x0, x1, [sp, #FREGS_X0]
+ stp x2, x3, [sp, #FREGS_X2]
+ stp x4, x5, [sp, #FREGS_X4]
+ stp x6, x7, [sp, #FREGS_X6]
+ str x8, [sp, #FREGS_X8]
-SYM_CODE_START(ftrace_regs_caller)
- bti c
- ftrace_regs_entry 1
- b ftrace_common
-SYM_CODE_END(ftrace_regs_caller)
+ /* Save the callsite's FP, LR, SP */
+ str x29, [sp, #FREGS_FP]
+ str x9, [sp, #FREGS_LR]
+ str x10, [sp, #FREGS_SP]
-SYM_CODE_START(ftrace_caller)
- bti c
- ftrace_regs_entry 0
- b ftrace_common
-SYM_CODE_END(ftrace_caller)
+ /* Save the PC after the ftrace callsite */
+ str x30, [sp, #FREGS_PC]
+
+ /* Create a frame record for the callsite above the ftrace regs */
+ stp x29, x9, [sp, #FREGS_SIZE + 16]
+ add x29, sp, #FREGS_SIZE + 16
+
+ /* Create our frame record above the ftrace regs */
+ stp x29, x30, [sp, #FREGS_SIZE]
+ add x29, sp, #FREGS_SIZE
-SYM_CODE_START(ftrace_common)
sub x0, x30, #AARCH64_INSN_SIZE // ip (callsite's BL insn)
mov x1, x9 // parent_ip (callsite's LR)
ldr_l x2, function_trace_op // op
@@ -104,24 +79,24 @@ SYM_INNER_LABEL(ftrace_call, SYM_L_GLOBAL)
* to restore x0-x8, x29, and x30.
*/
/* Restore function arguments */
- ldp x0, x1, [sp]
- ldp x2, x3, [sp, #S_X2]
- ldp x4, x5, [sp, #S_X4]
- ldp x6, x7, [sp, #S_X6]
- ldr x8, [sp, #S_X8]
+ ldp x0, x1, [sp, #FREGS_X0]
+ ldp x2, x3, [sp, #FREGS_X2]
+ ldp x4, x5, [sp, #FREGS_X4]
+ ldp x6, x7, [sp, #FREGS_X6]
+ ldr x8, [sp, #FREGS_X8]
/* Restore the callsite's FP, LR, PC */
- ldr x29, [sp, #S_FP]
- ldr x30, [sp, #S_LR]
- ldr x9, [sp, #S_PC]
+ ldr x29, [sp, #FREGS_FP]
+ ldr x30, [sp, #FREGS_LR]
+ ldr x9, [sp, #FREGS_PC]
/* Restore the callsite's SP */
- add sp, sp, #PT_REGS_SIZE + 16
+ add sp, sp, #FREGS_SIZE + 32
ret x9
-SYM_CODE_END(ftrace_common)
+SYM_CODE_END(ftrace_caller)
-#else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
+#else /* CONFIG_DYNAMIC_FTRACE_WITH_ARGS */
/*
* Gcc with -pg will put the following code in the beginning of each function:
@@ -293,7 +268,7 @@ SYM_FUNC_START(ftrace_graph_caller)
mcount_exit
SYM_FUNC_END(ftrace_graph_caller)
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-#endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_ARGS */
SYM_TYPED_FUNC_START(ftrace_stub)
ret
diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index 8745175f4a754..5cf990d052ba8 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c
@@ -17,6 +17,49 @@
#include <asm/insn.h>
#include <asm/patching.h>
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
+struct fregs_offset {
+ const char *name;
+ int offset;
+};
+
+#define FREGS_OFFSET(n, field) \
+{ \
+ .name = n, \
+ .offset = offsetof(struct ftrace_regs, field), \
+}
+
+static const struct fregs_offset fregs_offsets[] = {
+ FREGS_OFFSET("x0", regs[0]),
+ FREGS_OFFSET("x1", regs[1]),
+ FREGS_OFFSET("x2", regs[2]),
+ FREGS_OFFSET("x3", regs[3]),
+ FREGS_OFFSET("x4", regs[4]),
+ FREGS_OFFSET("x5", regs[5]),
+ FREGS_OFFSET("x6", regs[6]),
+ FREGS_OFFSET("x7", regs[7]),
+ FREGS_OFFSET("x8", regs[8]),
+
+ FREGS_OFFSET("x29", fp),
+ FREGS_OFFSET("x30", lr),
+ FREGS_OFFSET("lr", lr),
+
+ FREGS_OFFSET("sp", sp),
+ FREGS_OFFSET("pc", pc),
+};
+
+int ftrace_regs_query_register_offset(const char *name)
+{
+ for (int i = 0; i < ARRAY_SIZE(fregs_offsets); i++) {
+ const struct fregs_offset *roff = &fregs_offsets[i];
+ if (!strcmp(roff->name, name))
+ return roff->offset;
+ }
+
+ return -EINVAL;
+}
+#endif
+
#ifdef CONFIG_DYNAMIC_FTRACE
/*
* Replace a single instruction, which may be a branch or NOP.
@@ -70,9 +113,6 @@ static struct plt_entry *get_ftrace_plt(struct module *mod, unsigned long addr)
if (addr == FTRACE_ADDR)
return &plt[FTRACE_PLT_IDX];
- if (addr == FTRACE_REGS_ADDR &&
- IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS))
- return &plt[FTRACE_REGS_PLT_IDX];
#endif
return NULL;
}
@@ -154,25 +194,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
return ftrace_modify_code(pc, old, new, true);
}
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
- unsigned long addr)
-{
- unsigned long pc = rec->ip;
- u32 old, new;
-
- if (!ftrace_find_callable_addr(rec, NULL, &old_addr))
- return -EINVAL;
- if (!ftrace_find_callable_addr(rec, NULL, &addr))
- return -EINVAL;
-
- old = aarch64_insn_gen_branch_imm(pc, old_addr,
- AARCH64_INSN_BRANCH_LINK);
- new = aarch64_insn_gen_branch_imm(pc, addr, AARCH64_INSN_BRANCH_LINK);
-
- return ftrace_modify_code(pc, old, new, true);
-}
-
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
/*
* The compiler has inserted two NOPs before the regular function prologue.
* All instrumented functions follow the AAPCS, so x0-x8 and x19-x30 are live,
@@ -228,7 +250,7 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
*
* Note: 'mod' is only set at module load time.
*/
- if (!IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS) &&
+ if (!IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_ARGS) &&
IS_ENABLED(CONFIG_ARM64_MODULE_PLTS) && mod) {
return aarch64_insn_patch_text_nosync((void *)pc, new);
}
@@ -279,19 +301,11 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent,
#ifdef CONFIG_DYNAMIC_FTRACE
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *op, struct ftrace_regs *fregs)
{
- /*
- * When DYNAMIC_FTRACE_WITH_REGS is selected, `fregs` can never be NULL
- * and arch_ftrace_get_regs(fregs) will always give a non-NULL pt_regs
- * in which we can safely modify the LR.
- */
- struct pt_regs *regs = arch_ftrace_get_regs(fregs);
- unsigned long *parent = (unsigned long *)&procedure_link_pointer(regs);
-
- prepare_ftrace_return(ip, parent, frame_pointer(regs));
+ prepare_ftrace_return(ip, &fregs->lr, fregs->fp);
}
#else
/*
@@ -323,6 +337,6 @@ int ftrace_disable_ftrace_graph_caller(void)
{
return ftrace_modify_graph_caller(false);
}
-#endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_ARGS */
#endif /* CONFIG_DYNAMIC_FTRACE */
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
index 76b41e4ca9fa3..acd0d883e9ca2 100644
--- a/arch/arm64/kernel/module.c
+++ b/arch/arm64/kernel/module.c
@@ -497,9 +497,6 @@ static int module_init_ftrace_plt(const Elf_Ehdr *hdr,
__init_plt(&plts[FTRACE_PLT_IDX], FTRACE_ADDR);
- if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS))
- __init_plt(&plts[FTRACE_REGS_PLT_IDX], FTRACE_REGS_ADDR);
-
mod->arch.ftrace_trampolines = plts;
#endif
return 0;
--
2.30.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS
@ 2022-11-03 17:05 ` Mark Rutland
0 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2022-11-03 17:05 UTC (permalink / raw)
To: linux-kernel
Cc: catalin.marinas, linux-arm-kernel, mark.rutland, mhiramat,
revest, rostedt, will
This commit replaces arm64's support for FTRACE_WITH_REGS with support
for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
removes some latent issues with inconsistent presentation of struct
pt_regs (which can only be reliably saved/restored at exception
boundaries).
FTRACE_WITH_REGS has been supported on arm64 since commit:
3b23e4991fb66f6d ("arm64: implement ftrace with regs")
As noted in the commit message, the major reasons for implementing
FTRACE_WITH_REGS were:
(1) To make it possible to use the ftrace graph tracer with pointer
authentication, where it's necessary to snapshot/manipulate the LR
before it is signed by the instrumented function.
(2) To make it possible to implement LIVEPATCH in future, where we need
to hook function entry before an instrumented function manipulates
the stack or argument registers. Practically speaking, we need to
preserve the argument/return registers, PC, LR, and SP.
Neither of these need a struct pt_regs, and only require the set of
registers which are live at function call/return boundaries. Our calling
convention is defined by "Procedure Call Standard for the Arm® 64-bit
Architecture (AArch64)" (AKA "AAPCS64"), which can currently be found
at:
https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst
Per AAPCS64, all function call argument and return values are held in
the following GPRs:
* X0 - X7 : parameter / result registers
* X8 : indirect result location register
* SP : stack pointer (AKA SP)
Additionally, ad function call boundaries, the following GPRs hold
context/return information:
* X29 : frame pointer (AKA FP)
* X30 : link register (AKA LR)
... and for ftrace we need to capture the instrumented address:
* PC : program counter
No other GPRs are relevant, as none of the other arguments hold
parameters or return values:
* X9 - X17 : temporaries, may be clobbered
* X18 : shadow call stack pointer (or temorary)
* X19 - X28 : callee saved
This patch implements FTRACE_WITH_ARGS for arm64, only saving/restoring
the minimal set of registers necessary. This is always sufficient to
manipulate control flow (e.g. for live-patching) or to manipulate
function arguments and return values.
This reduces the necessary stack usage from 336 bytes for pt_regs down
to 112 bytes for ftrace_regs + 32 bytes for two frame records, freeing
up 188 bytes. This could be reduced further with changes to the
unwinder.
As there is no longer a need to save different sets of registers for
different features, we no longer need distinct `ftrace_caller` and
`ftrace_regs_caller` trampolines. This allows the trampoline assembly to
be simpler, and simplifies code which previously had to handle the two
trampolines.
I've tested this with the ftrace selftests, where there are no
unexpected failures.
Co-developed-by: Florent Revest <revest@chromium.org>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Florent Revest <revest@chromium.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/Kconfig | 18 ++---
arch/arm64/Makefile | 2 +-
arch/arm64/include/asm/ftrace.h | 72 +++++++++++++++++--
arch/arm64/kernel/asm-offsets.c | 13 ++++
arch/arm64/kernel/entry-ftrace.S | 117 ++++++++++++-------------------
arch/arm64/kernel/ftrace.c | 82 +++++++++++++---------
arch/arm64/kernel/module.c | 3 -
7 files changed, 184 insertions(+), 123 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 505c8a1ccbe0c..b6b3305ba7013 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -181,8 +181,10 @@ config ARM64
select HAVE_DEBUG_KMEMLEAK
select HAVE_DMA_CONTIGUOUS
select HAVE_DYNAMIC_FTRACE
+ select HAVE_DYNAMIC_FTRACE_WITH_ARGS \
+ if $(cc-option,-fpatchable-function-entry=2)
select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY \
- if DYNAMIC_FTRACE_WITH_REGS
+ if DYNAMIC_FTRACE_WITH_ARGS
select HAVE_EFFICIENT_UNALIGNED_ACCESS
select HAVE_FAST_GUP
select HAVE_FTRACE_MCOUNT_RECORD
@@ -233,16 +235,16 @@ config ARM64
help
ARM 64-bit (AArch64) Linux support.
-config CLANG_SUPPORTS_DYNAMIC_FTRACE_WITH_REGS
+config CLANG_SUPPORTS_DYNAMIC_FTRACE_WITH_ARGS
def_bool CC_IS_CLANG
# https://github.com/ClangBuiltLinux/linux/issues/1507
depends on AS_IS_GNU || (AS_IS_LLVM && (LD_IS_LLD || LD_VERSION >= 23600))
- select HAVE_DYNAMIC_FTRACE_WITH_REGS
+ select HAVE_DYNAMIC_FTRACE_WITH_ARGS
-config GCC_SUPPORTS_DYNAMIC_FTRACE_WITH_REGS
+config GCC_SUPPORTS_DYNAMIC_FTRACE_WITH_ARGS
def_bool CC_IS_GCC
depends on $(cc-option,-fpatchable-function-entry=2)
- select HAVE_DYNAMIC_FTRACE_WITH_REGS
+ select HAVE_DYNAMIC_FTRACE_WITH_ARGS
config 64BIT
def_bool y
@@ -1816,7 +1818,7 @@ config ARM64_PTR_AUTH_KERNEL
# which is only understood by binutils starting with version 2.33.1.
depends on LD_IS_LLD || LD_VERSION >= 23301 || (CC_IS_GCC && GCC_VERSION < 90100)
depends on !CC_IS_CLANG || AS_HAS_CFI_NEGATE_RA_STATE
- depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
+ depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_ARGS)
help
If the compiler supports the -mbranch-protection or
-msign-return-address flag (e.g. GCC 7 or later), then this option
@@ -1826,7 +1828,7 @@ config ARM64_PTR_AUTH_KERNEL
disabled with minimal loss of protection.
This feature works with FUNCTION_GRAPH_TRACER option only if
- DYNAMIC_FTRACE_WITH_REGS is enabled.
+ DYNAMIC_FTRACE_WITH_ARGS is enabled.
config CC_HAS_BRANCH_PROT_PAC_RET
# GCC 9 or later, clang 8 or later
@@ -1924,7 +1926,7 @@ config ARM64_BTI_KERNEL
depends on !CC_IS_GCC
# https://github.com/llvm/llvm-project/commit/a88c722e687e6780dcd6a58718350dc76fcc4cc9
depends on !CC_IS_CLANG || CLANG_VERSION >= 120000
- depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
+ depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_ARGS)
help
Build the kernel with Branch Target Identification annotations
and enable enforcement of this for kernel code. When this option
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 5e56d26a22398..b1202fa84baba 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -128,7 +128,7 @@ endif
CHECKFLAGS += -D__aarch64__
-ifeq ($(CONFIG_DYNAMIC_FTRACE_WITH_REGS),y)
+ifeq ($(CONFIG_DYNAMIC_FTRACE_WITH_ARGS),y)
KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY
CC_FLAGS_FTRACE := -fpatchable-function-entry=2
endif
diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
index 329dbbd4d50b6..5664729800ae1 100644
--- a/arch/arm64/include/asm/ftrace.h
+++ b/arch/arm64/include/asm/ftrace.h
@@ -23,7 +23,7 @@
*/
#define HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
#define ARCH_SUPPORTS_FTRACE_OPS 1
#else
#define MCOUNT_ADDR ((unsigned long)_mcount)
@@ -33,8 +33,7 @@
#define MCOUNT_INSN_SIZE AARCH64_INSN_SIZE
#define FTRACE_PLT_IDX 0
-#define FTRACE_REGS_PLT_IDX 1
-#define NR_FTRACE_PLTS 2
+#define NR_FTRACE_PLTS 1
/*
* Currently, gcc tends to save the link register after the local variables
@@ -69,7 +68,7 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
* Adjust addr to point at the BL in the callsite.
* See ftrace_init_nop() for the callsite sequence.
*/
- if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS))
+ if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_ARGS))
return addr + AARCH64_INSN_SIZE;
/*
* addr is the address of the mcount call instruction.
@@ -78,10 +77,71 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
return addr;
}
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
struct dyn_ftrace;
struct ftrace_ops;
-struct ftrace_regs;
+
+#define arch_ftrace_get_regs(regs) NULL
+
+struct ftrace_regs {
+ /* x0 - x8 */
+ unsigned long regs[9];
+ unsigned long __unused;
+
+ unsigned long fp;
+ unsigned long lr;
+
+ unsigned long sp;
+ unsigned long pc;
+};
+
+static __always_inline unsigned long
+ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs)
+{
+ return fregs->pc;
+}
+
+static __always_inline void
+ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
+ unsigned long pc)
+{
+ fregs->pc = pc;
+}
+
+static __always_inline unsigned long
+ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs)
+{
+ return fregs->sp;
+}
+
+static __always_inline unsigned long
+ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n)
+{
+ if (n < 8)
+ return fregs->regs[n];
+ return 0;
+}
+
+static __always_inline unsigned long
+ftrace_regs_get_return_value(const struct ftrace_regs *fregs)
+{
+ return fregs->regs[0];
+}
+
+static __always_inline void
+ftrace_regs_set_return_value(struct ftrace_regs *fregs,
+ unsigned long ret)
+{
+ fregs->regs[0] = ret;
+}
+
+static __always_inline void
+ftrace_override_function_with_return(struct ftrace_regs *fregs)
+{
+ fregs->pc = fregs->lr;
+}
+
+int ftrace_regs_query_register_offset(const char *name);
int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec);
#define ftrace_init_nop ftrace_init_nop
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 1197e7679882e..2234624536d95 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -82,6 +82,19 @@ int main(void)
DEFINE(S_STACKFRAME, offsetof(struct pt_regs, stackframe));
DEFINE(PT_REGS_SIZE, sizeof(struct pt_regs));
BLANK();
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
+ DEFINE(FREGS_X0, offsetof(struct ftrace_regs, regs[0]));
+ DEFINE(FREGS_X2, offsetof(struct ftrace_regs, regs[2]));
+ DEFINE(FREGS_X4, offsetof(struct ftrace_regs, regs[4]));
+ DEFINE(FREGS_X6, offsetof(struct ftrace_regs, regs[6]));
+ DEFINE(FREGS_X8, offsetof(struct ftrace_regs, regs[8]));
+ DEFINE(FREGS_FP, offsetof(struct ftrace_regs, fp));
+ DEFINE(FREGS_LR, offsetof(struct ftrace_regs, lr));
+ DEFINE(FREGS_SP, offsetof(struct ftrace_regs, sp));
+ DEFINE(FREGS_PC, offsetof(struct ftrace_regs, pc));
+ DEFINE(FREGS_SIZE, sizeof(struct ftrace_regs));
+ BLANK();
+#endif
#ifdef CONFIG_COMPAT
DEFINE(COMPAT_SIGFRAME_REGS_OFFSET, offsetof(struct compat_sigframe, uc.uc_mcontext.arm_r0));
DEFINE(COMPAT_RT_SIGFRAME_REGS_OFFSET, offsetof(struct compat_rt_sigframe, sig.uc.uc_mcontext.arm_r0));
diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S
index 795344ab4ec45..4d3050549aa6e 100644
--- a/arch/arm64/kernel/entry-ftrace.S
+++ b/arch/arm64/kernel/entry-ftrace.S
@@ -13,83 +13,58 @@
#include <asm/ftrace.h>
#include <asm/insn.h>
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
/*
* Due to -fpatchable-function-entry=2, the compiler has placed two NOPs before
* the regular function prologue. For an enabled callsite, ftrace_init_nop() and
* ftrace_make_call() have patched those NOPs to:
*
* MOV X9, LR
- * BL <entry>
- *
- * ... where <entry> is either ftrace_caller or ftrace_regs_caller.
+ * BL ftrace_caller
*
* Each instrumented function follows the AAPCS, so here x0-x8 and x18-x30 are
* live (x18 holds the Shadow Call Stack pointer), and x9-x17 are safe to
* clobber.
*
- * We save the callsite's context into a pt_regs before invoking any ftrace
- * callbacks. So that we can get a sensible backtrace, we create a stack record
- * for the callsite and the ftrace entry assembly. This is not sufficient for
- * reliable stacktrace: until we create the callsite stack record, its caller
- * is missing from the LR and existing chain of frame records.
+ * We save the callsite's context into a struct ftrace_regs before invoking any
+ * ftrace callbacks. So that we can get a sensible backtrace, we create frame
+ * records for the callsite and the ftrace entry assembly. This is not
+ * sufficient for reliable stacktrace: until we create the callsite stack
+ * record, its caller is missing from the LR and existing chain of frame
+ * records.
*/
- .macro ftrace_regs_entry, allregs=0
- /* Make room for pt_regs, plus a callee frame */
- sub sp, sp, #(PT_REGS_SIZE + 16)
-
- /* Save function arguments (and x9 for simplicity) */
- stp x0, x1, [sp, #S_X0]
- stp x2, x3, [sp, #S_X2]
- stp x4, x5, [sp, #S_X4]
- stp x6, x7, [sp, #S_X6]
- stp x8, x9, [sp, #S_X8]
-
- /* Optionally save the callee-saved registers, always save the FP */
- .if \allregs == 1
- stp x10, x11, [sp, #S_X10]
- stp x12, x13, [sp, #S_X12]
- stp x14, x15, [sp, #S_X14]
- stp x16, x17, [sp, #S_X16]
- stp x18, x19, [sp, #S_X18]
- stp x20, x21, [sp, #S_X20]
- stp x22, x23, [sp, #S_X22]
- stp x24, x25, [sp, #S_X24]
- stp x26, x27, [sp, #S_X26]
- stp x28, x29, [sp, #S_X28]
- .else
- str x29, [sp, #S_FP]
- .endif
-
- /* Save the callsite's SP and LR */
- add x10, sp, #(PT_REGS_SIZE + 16)
- stp x9, x10, [sp, #S_LR]
+SYM_CODE_START(ftrace_caller)
+ bti c
- /* Save the PC after the ftrace callsite */
- str x30, [sp, #S_PC]
+ /* Save original SP */
+ mov x10, sp
- /* Create a frame record for the callsite above pt_regs */
- stp x29, x9, [sp, #PT_REGS_SIZE]
- add x29, sp, #PT_REGS_SIZE
+ /* Make room for ftrace regs, plus two frame records */
+ sub sp, sp, #(FREGS_SIZE + 32)
- /* Create our frame record within pt_regs. */
- stp x29, x30, [sp, #S_STACKFRAME]
- add x29, sp, #S_STACKFRAME
- .endm
+ /* Save function arguments */
+ stp x0, x1, [sp, #FREGS_X0]
+ stp x2, x3, [sp, #FREGS_X2]
+ stp x4, x5, [sp, #FREGS_X4]
+ stp x6, x7, [sp, #FREGS_X6]
+ str x8, [sp, #FREGS_X8]
-SYM_CODE_START(ftrace_regs_caller)
- bti c
- ftrace_regs_entry 1
- b ftrace_common
-SYM_CODE_END(ftrace_regs_caller)
+ /* Save the callsite's FP, LR, SP */
+ str x29, [sp, #FREGS_FP]
+ str x9, [sp, #FREGS_LR]
+ str x10, [sp, #FREGS_SP]
-SYM_CODE_START(ftrace_caller)
- bti c
- ftrace_regs_entry 0
- b ftrace_common
-SYM_CODE_END(ftrace_caller)
+ /* Save the PC after the ftrace callsite */
+ str x30, [sp, #FREGS_PC]
+
+ /* Create a frame record for the callsite above the ftrace regs */
+ stp x29, x9, [sp, #FREGS_SIZE + 16]
+ add x29, sp, #FREGS_SIZE + 16
+
+ /* Create our frame record above the ftrace regs */
+ stp x29, x30, [sp, #FREGS_SIZE]
+ add x29, sp, #FREGS_SIZE
-SYM_CODE_START(ftrace_common)
sub x0, x30, #AARCH64_INSN_SIZE // ip (callsite's BL insn)
mov x1, x9 // parent_ip (callsite's LR)
ldr_l x2, function_trace_op // op
@@ -104,24 +79,24 @@ SYM_INNER_LABEL(ftrace_call, SYM_L_GLOBAL)
* to restore x0-x8, x29, and x30.
*/
/* Restore function arguments */
- ldp x0, x1, [sp]
- ldp x2, x3, [sp, #S_X2]
- ldp x4, x5, [sp, #S_X4]
- ldp x6, x7, [sp, #S_X6]
- ldr x8, [sp, #S_X8]
+ ldp x0, x1, [sp, #FREGS_X0]
+ ldp x2, x3, [sp, #FREGS_X2]
+ ldp x4, x5, [sp, #FREGS_X4]
+ ldp x6, x7, [sp, #FREGS_X6]
+ ldr x8, [sp, #FREGS_X8]
/* Restore the callsite's FP, LR, PC */
- ldr x29, [sp, #S_FP]
- ldr x30, [sp, #S_LR]
- ldr x9, [sp, #S_PC]
+ ldr x29, [sp, #FREGS_FP]
+ ldr x30, [sp, #FREGS_LR]
+ ldr x9, [sp, #FREGS_PC]
/* Restore the callsite's SP */
- add sp, sp, #PT_REGS_SIZE + 16
+ add sp, sp, #FREGS_SIZE + 32
ret x9
-SYM_CODE_END(ftrace_common)
+SYM_CODE_END(ftrace_caller)
-#else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
+#else /* CONFIG_DYNAMIC_FTRACE_WITH_ARGS */
/*
* Gcc with -pg will put the following code in the beginning of each function:
@@ -293,7 +268,7 @@ SYM_FUNC_START(ftrace_graph_caller)
mcount_exit
SYM_FUNC_END(ftrace_graph_caller)
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-#endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_ARGS */
SYM_TYPED_FUNC_START(ftrace_stub)
ret
diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index 8745175f4a754..5cf990d052ba8 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c
@@ -17,6 +17,49 @@
#include <asm/insn.h>
#include <asm/patching.h>
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
+struct fregs_offset {
+ const char *name;
+ int offset;
+};
+
+#define FREGS_OFFSET(n, field) \
+{ \
+ .name = n, \
+ .offset = offsetof(struct ftrace_regs, field), \
+}
+
+static const struct fregs_offset fregs_offsets[] = {
+ FREGS_OFFSET("x0", regs[0]),
+ FREGS_OFFSET("x1", regs[1]),
+ FREGS_OFFSET("x2", regs[2]),
+ FREGS_OFFSET("x3", regs[3]),
+ FREGS_OFFSET("x4", regs[4]),
+ FREGS_OFFSET("x5", regs[5]),
+ FREGS_OFFSET("x6", regs[6]),
+ FREGS_OFFSET("x7", regs[7]),
+ FREGS_OFFSET("x8", regs[8]),
+
+ FREGS_OFFSET("x29", fp),
+ FREGS_OFFSET("x30", lr),
+ FREGS_OFFSET("lr", lr),
+
+ FREGS_OFFSET("sp", sp),
+ FREGS_OFFSET("pc", pc),
+};
+
+int ftrace_regs_query_register_offset(const char *name)
+{
+ for (int i = 0; i < ARRAY_SIZE(fregs_offsets); i++) {
+ const struct fregs_offset *roff = &fregs_offsets[i];
+ if (!strcmp(roff->name, name))
+ return roff->offset;
+ }
+
+ return -EINVAL;
+}
+#endif
+
#ifdef CONFIG_DYNAMIC_FTRACE
/*
* Replace a single instruction, which may be a branch or NOP.
@@ -70,9 +113,6 @@ static struct plt_entry *get_ftrace_plt(struct module *mod, unsigned long addr)
if (addr == FTRACE_ADDR)
return &plt[FTRACE_PLT_IDX];
- if (addr == FTRACE_REGS_ADDR &&
- IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS))
- return &plt[FTRACE_REGS_PLT_IDX];
#endif
return NULL;
}
@@ -154,25 +194,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
return ftrace_modify_code(pc, old, new, true);
}
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
- unsigned long addr)
-{
- unsigned long pc = rec->ip;
- u32 old, new;
-
- if (!ftrace_find_callable_addr(rec, NULL, &old_addr))
- return -EINVAL;
- if (!ftrace_find_callable_addr(rec, NULL, &addr))
- return -EINVAL;
-
- old = aarch64_insn_gen_branch_imm(pc, old_addr,
- AARCH64_INSN_BRANCH_LINK);
- new = aarch64_insn_gen_branch_imm(pc, addr, AARCH64_INSN_BRANCH_LINK);
-
- return ftrace_modify_code(pc, old, new, true);
-}
-
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
/*
* The compiler has inserted two NOPs before the regular function prologue.
* All instrumented functions follow the AAPCS, so x0-x8 and x19-x30 are live,
@@ -228,7 +250,7 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
*
* Note: 'mod' is only set at module load time.
*/
- if (!IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS) &&
+ if (!IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_ARGS) &&
IS_ENABLED(CONFIG_ARM64_MODULE_PLTS) && mod) {
return aarch64_insn_patch_text_nosync((void *)pc, new);
}
@@ -279,19 +301,11 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent,
#ifdef CONFIG_DYNAMIC_FTRACE
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *op, struct ftrace_regs *fregs)
{
- /*
- * When DYNAMIC_FTRACE_WITH_REGS is selected, `fregs` can never be NULL
- * and arch_ftrace_get_regs(fregs) will always give a non-NULL pt_regs
- * in which we can safely modify the LR.
- */
- struct pt_regs *regs = arch_ftrace_get_regs(fregs);
- unsigned long *parent = (unsigned long *)&procedure_link_pointer(regs);
-
- prepare_ftrace_return(ip, parent, frame_pointer(regs));
+ prepare_ftrace_return(ip, &fregs->lr, fregs->fp);
}
#else
/*
@@ -323,6 +337,6 @@ int ftrace_disable_ftrace_graph_caller(void)
{
return ftrace_modify_graph_caller(false);
}
-#endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_ARGS */
#endif /* CONFIG_DYNAMIC_FTRACE */
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
index 76b41e4ca9fa3..acd0d883e9ca2 100644
--- a/arch/arm64/kernel/module.c
+++ b/arch/arm64/kernel/module.c
@@ -497,9 +497,6 @@ static int module_init_ftrace_plt(const Elf_Ehdr *hdr,
__init_plt(&plts[FTRACE_PLT_IDX], FTRACE_ADDR);
- if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS))
- __init_plt(&plts[FTRACE_REGS_PLT_IDX], FTRACE_REGS_ADDR);
-
mod->arch.ftrace_trampolines = plts;
#endif
return 0;
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS
2022-11-03 17:05 ` Mark Rutland
@ 2022-11-15 11:27 ` Will Deacon
-1 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2022-11-15 11:27 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel, catalin.marinas, linux-arm-kernel, mhiramat,
revest, rostedt
On Thu, Nov 03, 2022 at 05:05:20PM +0000, Mark Rutland wrote:
> This commit replaces arm64's support for FTRACE_WITH_REGS with support
> for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> removes some latent issues with inconsistent presentation of struct
> pt_regs (which can only be reliably saved/restored at exception
> boundaries).
[...]
> @@ -78,10 +77,71 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
> return addr;
> }
>
> -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
> struct dyn_ftrace;
> struct ftrace_ops;
> -struct ftrace_regs;
> +
> +#define arch_ftrace_get_regs(regs) NULL
> +
> +struct ftrace_regs {
> + /* x0 - x8 */
> + unsigned long regs[9];
> + unsigned long __unused;
> +
> + unsigned long fp;
> + unsigned long lr;
> +
> + unsigned long sp;
> + unsigned long pc;
> +};
> +
> +static __always_inline unsigned long
> +ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs)
> +{
> + return fregs->pc;
> +}
> +
> +static __always_inline void
> +ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
> + unsigned long pc)
> +{
> + fregs->pc = pc;
> +}
> +
> +static __always_inline unsigned long
> +ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs)
> +{
> + return fregs->sp;
> +}
> +
> +static __always_inline unsigned long
> +ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n)
> +{
> + if (n < 8)
> + return fregs->regs[n];
Where does this '8' come from?
Will
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS
@ 2022-11-15 11:27 ` Will Deacon
0 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2022-11-15 11:27 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel, catalin.marinas, linux-arm-kernel, mhiramat,
revest, rostedt
On Thu, Nov 03, 2022 at 05:05:20PM +0000, Mark Rutland wrote:
> This commit replaces arm64's support for FTRACE_WITH_REGS with support
> for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> removes some latent issues with inconsistent presentation of struct
> pt_regs (which can only be reliably saved/restored at exception
> boundaries).
[...]
> @@ -78,10 +77,71 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
> return addr;
> }
>
> -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
> struct dyn_ftrace;
> struct ftrace_ops;
> -struct ftrace_regs;
> +
> +#define arch_ftrace_get_regs(regs) NULL
> +
> +struct ftrace_regs {
> + /* x0 - x8 */
> + unsigned long regs[9];
> + unsigned long __unused;
> +
> + unsigned long fp;
> + unsigned long lr;
> +
> + unsigned long sp;
> + unsigned long pc;
> +};
> +
> +static __always_inline unsigned long
> +ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs)
> +{
> + return fregs->pc;
> +}
> +
> +static __always_inline void
> +ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
> + unsigned long pc)
> +{
> + fregs->pc = pc;
> +}
> +
> +static __always_inline unsigned long
> +ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs)
> +{
> + return fregs->sp;
> +}
> +
> +static __always_inline unsigned long
> +ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n)
> +{
> + if (n < 8)
> + return fregs->regs[n];
Where does this '8' come from?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS
2022-11-03 17:05 ` Mark Rutland
@ 2022-11-15 15:01 ` Steven Rostedt
-1 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2022-11-15 15:01 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel, catalin.marinas, linux-arm-kernel, mhiramat, revest, will
On Thu, 3 Nov 2022 17:05:16 +0000
Mark Rutland <mark.rutland@arm.com> wrote:
> This series replaces arm64's support for FTRACE_WITH_REGS with support
> for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> removes some latent issues with inconsistent presentation of struct
> pt_regs (which can only be reliably saved/restored at exception
> boundaries).
>
> The existing FTRACE_WITH_REGS support was added for two major reasons:
>
> (1) To make it possible to use the ftrace graph tracer with pointer
> authentication, where it's necessary to snapshot/manipulate the LR
> before it is signed by the instrumented function.
>
> (2) To make it possible to implement LIVEPATCH in future, where we need
> to hook function entry before an instrumented function manipulates
> the stack or argument registers. Practically speaking, we need to
> preserve the argument/return registers, PC, LR, and SP.
>
> Neither of these requires the full set of pt_regs, and only requires us
> to save/restore a subset of registers used for passing
> arguments/return-values and context/return information (which is the
> minimum set we always need to save/restore today).
>
> As there is no longer a need to save different sets of registers for
> different features, we no longer need distinct `ftrace_caller` and
> `ftrace_regs_caller` trampolines. This allows the trampoline assembly to
> be simpler, and simplifies code which previously had to handle the two
> trampolines.
>
> I've tested this with the ftrace selftests, where there are no
> unexpected failures.
Were there any "expected" failures?
>
> I plan to build atop this with subsequent patches to add per-callsite
> ftrace_ops, and I'm sending these patches on their own as I think they
> make sense regardless.
>
> Since v1 [1]:
> * Change ifdeferry per Steve's request
> * Add ftrace_regs_query_register_offset() per Masami's request
> * Fix a bunch of typos
>
> [1] https://lore.kernel.org/lkml/20221024140846.3555435-1-mark.rutland@arm.com
>
> This series can be found in my 'arm64/ftrace/minimal-regs' branch on
> kernel.org:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/
> git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
>
> This version is tagged as:
>
> arm64-ftrace-minimal-regs-20221103
So I ran this on top of my code through all my ftrace tests (for x86) and
it didn't cause any regressions.
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS
@ 2022-11-15 15:01 ` Steven Rostedt
0 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2022-11-15 15:01 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel, catalin.marinas, linux-arm-kernel, mhiramat, revest, will
On Thu, 3 Nov 2022 17:05:16 +0000
Mark Rutland <mark.rutland@arm.com> wrote:
> This series replaces arm64's support for FTRACE_WITH_REGS with support
> for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> removes some latent issues with inconsistent presentation of struct
> pt_regs (which can only be reliably saved/restored at exception
> boundaries).
>
> The existing FTRACE_WITH_REGS support was added for two major reasons:
>
> (1) To make it possible to use the ftrace graph tracer with pointer
> authentication, where it's necessary to snapshot/manipulate the LR
> before it is signed by the instrumented function.
>
> (2) To make it possible to implement LIVEPATCH in future, where we need
> to hook function entry before an instrumented function manipulates
> the stack or argument registers. Practically speaking, we need to
> preserve the argument/return registers, PC, LR, and SP.
>
> Neither of these requires the full set of pt_regs, and only requires us
> to save/restore a subset of registers used for passing
> arguments/return-values and context/return information (which is the
> minimum set we always need to save/restore today).
>
> As there is no longer a need to save different sets of registers for
> different features, we no longer need distinct `ftrace_caller` and
> `ftrace_regs_caller` trampolines. This allows the trampoline assembly to
> be simpler, and simplifies code which previously had to handle the two
> trampolines.
>
> I've tested this with the ftrace selftests, where there are no
> unexpected failures.
Were there any "expected" failures?
>
> I plan to build atop this with subsequent patches to add per-callsite
> ftrace_ops, and I'm sending these patches on their own as I think they
> make sense regardless.
>
> Since v1 [1]:
> * Change ifdeferry per Steve's request
> * Add ftrace_regs_query_register_offset() per Masami's request
> * Fix a bunch of typos
>
> [1] https://lore.kernel.org/lkml/20221024140846.3555435-1-mark.rutland@arm.com
>
> This series can be found in my 'arm64/ftrace/minimal-regs' branch on
> kernel.org:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/
> git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
>
> This version is tagged as:
>
> arm64-ftrace-minimal-regs-20221103
So I ran this on top of my code through all my ftrace tests (for x86) and
it didn't cause any regressions.
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS
2022-11-15 15:01 ` Steven Rostedt
@ 2022-11-15 15:48 ` Mark Rutland
-1 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2022-11-15 15:48 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, catalin.marinas, linux-arm-kernel, mhiramat, revest, will
On Tue, Nov 15, 2022 at 10:01:48AM -0500, Steven Rostedt wrote:
> On Thu, 3 Nov 2022 17:05:16 +0000
> Mark Rutland <mark.rutland@arm.com> wrote:
>
> > This series replaces arm64's support for FTRACE_WITH_REGS with support
> > for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> > removes some latent issues with inconsistent presentation of struct
> > pt_regs (which can only be reliably saved/restored at exception
> > boundaries).
> >
> > The existing FTRACE_WITH_REGS support was added for two major reasons:
> >
> > (1) To make it possible to use the ftrace graph tracer with pointer
> > authentication, where it's necessary to snapshot/manipulate the LR
> > before it is signed by the instrumented function.
> >
> > (2) To make it possible to implement LIVEPATCH in future, where we need
> > to hook function entry before an instrumented function manipulates
> > the stack or argument registers. Practically speaking, we need to
> > preserve the argument/return registers, PC, LR, and SP.
> >
> > Neither of these requires the full set of pt_regs, and only requires us
> > to save/restore a subset of registers used for passing
> > arguments/return-values and context/return information (which is the
> > minimum set we always need to save/restore today).
> >
> > As there is no longer a need to save different sets of registers for
> > different features, we no longer need distinct `ftrace_caller` and
> > `ftrace_regs_caller` trampolines. This allows the trampoline assembly to
> > be simpler, and simplifies code which previously had to handle the two
> > trampolines.
> >
> > I've tested this with the ftrace selftests, where there are no
> > unexpected failures.
>
> Were there any "expected" failures?
Ah; sorry, I had meant to include the results here.
With this series applied atop v6.1-rc4 and using the ftrace selftests from that
tree, my results were the same as with baseline v6.1-rc4:
| # of passed: 104
| # of failed: 0
| # of unresolved: 7
| # of untested: 0
| # of unsupported: 2
| # of xfailed: 1
| # of undefined(test bug): 0
Where the non-passing tests were:
| [8] Test ftrace direct functions against tracers [UNRESOLVED]
| [9] Test ftrace direct functions against kprobes [UNRESOLVED]
... as direct functions aren't supported on arm64 (both before and after this
series).
| [16] Generic dynamic event - check if duplicate events are caught [UNSUPPORTED]
| [74] event trigger - test inter-event histogram trigger eprobe on synthetic event [UNSUPPORTED]
... which are due to a bug in the tests fixed by:
https://lore.kernel.org/all/20221010074207.714077-1-svens@linux.ibm.com/
... and they both pass with that applied.
| [22] Test trace_printk from module [UNRESOLVED]
| [31] ftrace - function trace on module [UNRESOLVED]
| [51] Kprobe dynamic event - probing module [UNRESOLVED]
| [61] test for the preemptirqsoff tracer [UNRESOLVED]
... which are because my test environment didn't have modules.
| [62] Meta-selftest: Checkbashisms [UNRESOLVED]
... which is irrelevant for this series.
| [65] event trigger - test inter-event histogram trigger expected fail actions [XFAIL]
... which is expected.
[...]
> So I ran this on top of my code through all my ftrace tests (for x86) and
> it didn't cause any regressions.
>
> Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Thanks!
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS
@ 2022-11-15 15:48 ` Mark Rutland
0 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2022-11-15 15:48 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, catalin.marinas, linux-arm-kernel, mhiramat, revest, will
On Tue, Nov 15, 2022 at 10:01:48AM -0500, Steven Rostedt wrote:
> On Thu, 3 Nov 2022 17:05:16 +0000
> Mark Rutland <mark.rutland@arm.com> wrote:
>
> > This series replaces arm64's support for FTRACE_WITH_REGS with support
> > for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> > removes some latent issues with inconsistent presentation of struct
> > pt_regs (which can only be reliably saved/restored at exception
> > boundaries).
> >
> > The existing FTRACE_WITH_REGS support was added for two major reasons:
> >
> > (1) To make it possible to use the ftrace graph tracer with pointer
> > authentication, where it's necessary to snapshot/manipulate the LR
> > before it is signed by the instrumented function.
> >
> > (2) To make it possible to implement LIVEPATCH in future, where we need
> > to hook function entry before an instrumented function manipulates
> > the stack or argument registers. Practically speaking, we need to
> > preserve the argument/return registers, PC, LR, and SP.
> >
> > Neither of these requires the full set of pt_regs, and only requires us
> > to save/restore a subset of registers used for passing
> > arguments/return-values and context/return information (which is the
> > minimum set we always need to save/restore today).
> >
> > As there is no longer a need to save different sets of registers for
> > different features, we no longer need distinct `ftrace_caller` and
> > `ftrace_regs_caller` trampolines. This allows the trampoline assembly to
> > be simpler, and simplifies code which previously had to handle the two
> > trampolines.
> >
> > I've tested this with the ftrace selftests, where there are no
> > unexpected failures.
>
> Were there any "expected" failures?
Ah; sorry, I had meant to include the results here.
With this series applied atop v6.1-rc4 and using the ftrace selftests from that
tree, my results were the same as with baseline v6.1-rc4:
| # of passed: 104
| # of failed: 0
| # of unresolved: 7
| # of untested: 0
| # of unsupported: 2
| # of xfailed: 1
| # of undefined(test bug): 0
Where the non-passing tests were:
| [8] Test ftrace direct functions against tracers [UNRESOLVED]
| [9] Test ftrace direct functions against kprobes [UNRESOLVED]
... as direct functions aren't supported on arm64 (both before and after this
series).
| [16] Generic dynamic event - check if duplicate events are caught [UNSUPPORTED]
| [74] event trigger - test inter-event histogram trigger eprobe on synthetic event [UNSUPPORTED]
... which are due to a bug in the tests fixed by:
https://lore.kernel.org/all/20221010074207.714077-1-svens@linux.ibm.com/
... and they both pass with that applied.
| [22] Test trace_printk from module [UNRESOLVED]
| [31] ftrace - function trace on module [UNRESOLVED]
| [51] Kprobe dynamic event - probing module [UNRESOLVED]
| [61] test for the preemptirqsoff tracer [UNRESOLVED]
... which are because my test environment didn't have modules.
| [62] Meta-selftest: Checkbashisms [UNRESOLVED]
... which is irrelevant for this series.
| [65] event trigger - test inter-event histogram trigger expected fail actions [XFAIL]
... which is expected.
[...]
> So I ran this on top of my code through all my ftrace tests (for x86) and
> it didn't cause any regressions.
>
> Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Thanks!
Mark.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS
2022-11-15 11:27 ` Will Deacon
@ 2022-11-17 10:52 ` Mark Rutland
-1 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2022-11-17 10:52 UTC (permalink / raw)
To: Will Deacon
Cc: linux-kernel, catalin.marinas, linux-arm-kernel, mhiramat,
revest, rostedt
On Tue, Nov 15, 2022 at 11:27:03AM +0000, Will Deacon wrote:
> On Thu, Nov 03, 2022 at 05:05:20PM +0000, Mark Rutland wrote:
> > This commit replaces arm64's support for FTRACE_WITH_REGS with support
> > for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> > removes some latent issues with inconsistent presentation of struct
> > pt_regs (which can only be reliably saved/restored at exception
> > boundaries).
>
> [...]
>
> > @@ -78,10 +77,71 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
> > return addr;
> > }
> >
> > -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
> > struct dyn_ftrace;
> > struct ftrace_ops;
> > -struct ftrace_regs;
> > +
> > +#define arch_ftrace_get_regs(regs) NULL
> > +
> > +struct ftrace_regs {
> > + /* x0 - x8 */
> > + unsigned long regs[9];
> > + unsigned long __unused;
> > +
> > + unsigned long fp;
> > + unsigned long lr;
> > +
> > + unsigned long sp;
> > + unsigned long pc;
> > +};
> > +
> > +static __always_inline unsigned long
> > +ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs)
> > +{
> > + return fregs->pc;
> > +}
> > +
> > +static __always_inline void
> > +ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
> > + unsigned long pc)
> > +{
> > + fregs->pc = pc;
> > +}
> > +
> > +static __always_inline unsigned long
> > +ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs)
> > +{
> > + return fregs->sp;
> > +}
> > +
> > +static __always_inline unsigned long
> > +ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n)
> > +{
> > + if (n < 8)
> > + return fregs->regs[n];
>
> Where does this '8' come from?
Because in AAPCS64 the arguments are in x0 to x7, as mentioned in the commit
message:
| Per AAPCS64, all function call argument and return values are held in
| the following GPRs:
|
| * X0 - X7 : parameter / result registers
| * X8 : indirect result location register
| * SP : stack pointer (AKA SP)
The 'indirect result location register' would be used when returning large
structures, and isn't a function argument as such.
The logic is the same as in regs_get_kernel_argument() for pt_regs.
I can add a comment here to explain that, if that would help?
The rest of the registers are as described in the commit message (and I now
spot a typo that I'll go fix):
| Additionally, ad function call boundaries, the following GPRs hold
| context/return information:
|
| * X29 : frame pointer (AKA FP)
| * X30 : link register (AKA LR)
|
| ... and for ftrace we need to capture the instrumented address:
|
| * PC : program counter
|
| No other GPRs are relevant, as none of the other arguments hold
| parameters or return values:
|
| * X9 - X17 : temporaries, may be clobbered
| * X18 : shadow call stack pointer (or temorary)
| * X19 - X28 : callee saved
Thanks,
Mark.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS
@ 2022-11-17 10:52 ` Mark Rutland
0 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2022-11-17 10:52 UTC (permalink / raw)
To: Will Deacon
Cc: linux-kernel, catalin.marinas, linux-arm-kernel, mhiramat,
revest, rostedt
On Tue, Nov 15, 2022 at 11:27:03AM +0000, Will Deacon wrote:
> On Thu, Nov 03, 2022 at 05:05:20PM +0000, Mark Rutland wrote:
> > This commit replaces arm64's support for FTRACE_WITH_REGS with support
> > for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> > removes some latent issues with inconsistent presentation of struct
> > pt_regs (which can only be reliably saved/restored at exception
> > boundaries).
>
> [...]
>
> > @@ -78,10 +77,71 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
> > return addr;
> > }
> >
> > -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
> > struct dyn_ftrace;
> > struct ftrace_ops;
> > -struct ftrace_regs;
> > +
> > +#define arch_ftrace_get_regs(regs) NULL
> > +
> > +struct ftrace_regs {
> > + /* x0 - x8 */
> > + unsigned long regs[9];
> > + unsigned long __unused;
> > +
> > + unsigned long fp;
> > + unsigned long lr;
> > +
> > + unsigned long sp;
> > + unsigned long pc;
> > +};
> > +
> > +static __always_inline unsigned long
> > +ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs)
> > +{
> > + return fregs->pc;
> > +}
> > +
> > +static __always_inline void
> > +ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
> > + unsigned long pc)
> > +{
> > + fregs->pc = pc;
> > +}
> > +
> > +static __always_inline unsigned long
> > +ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs)
> > +{
> > + return fregs->sp;
> > +}
> > +
> > +static __always_inline unsigned long
> > +ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n)
> > +{
> > + if (n < 8)
> > + return fregs->regs[n];
>
> Where does this '8' come from?
Because in AAPCS64 the arguments are in x0 to x7, as mentioned in the commit
message:
| Per AAPCS64, all function call argument and return values are held in
| the following GPRs:
|
| * X0 - X7 : parameter / result registers
| * X8 : indirect result location register
| * SP : stack pointer (AKA SP)
The 'indirect result location register' would be used when returning large
structures, and isn't a function argument as such.
The logic is the same as in regs_get_kernel_argument() for pt_regs.
I can add a comment here to explain that, if that would help?
The rest of the registers are as described in the commit message (and I now
spot a typo that I'll go fix):
| Additionally, ad function call boundaries, the following GPRs hold
| context/return information:
|
| * X29 : frame pointer (AKA FP)
| * X30 : link register (AKA LR)
|
| ... and for ftrace we need to capture the instrumented address:
|
| * PC : program counter
|
| No other GPRs are relevant, as none of the other arguments hold
| parameters or return values:
|
| * X9 - X17 : temporaries, may be clobbered
| * X18 : shadow call stack pointer (or temorary)
| * X19 - X28 : callee saved
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS
2022-11-03 17:05 ` Mark Rutland
@ 2022-11-18 0:04 ` Masami Hiramatsu
-1 siblings, 0 replies; 28+ messages in thread
From: Masami Hiramatsu @ 2022-11-18 0:04 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel, catalin.marinas, linux-arm-kernel, mhiramat,
revest, rostedt, will
On Thu, 3 Nov 2022 17:05:16 +0000
Mark Rutland <mark.rutland@arm.com> wrote:
> This series replaces arm64's support for FTRACE_WITH_REGS with support
> for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> removes some latent issues with inconsistent presentation of struct
> pt_regs (which can only be reliably saved/restored at exception
> boundaries).
>
> The existing FTRACE_WITH_REGS support was added for two major reasons:
>
> (1) To make it possible to use the ftrace graph tracer with pointer
> authentication, where it's necessary to snapshot/manipulate the LR
> before it is signed by the instrumented function.
>
> (2) To make it possible to implement LIVEPATCH in future, where we need
> to hook function entry before an instrumented function manipulates
> the stack or argument registers. Practically speaking, we need to
> preserve the argument/return registers, PC, LR, and SP.
>
> Neither of these requires the full set of pt_regs, and only requires us
> to save/restore a subset of registers used for passing
> arguments/return-values and context/return information (which is the
> minimum set we always need to save/restore today).
>
> As there is no longer a need to save different sets of registers for
> different features, we no longer need distinct `ftrace_caller` and
> `ftrace_regs_caller` trampolines. This allows the trampoline assembly to
> be simpler, and simplifies code which previously had to handle the two
> trampolines.
>
> I've tested this with the ftrace selftests, where there are no
> unexpected failures.
>
> I plan to build atop this with subsequent patches to add per-callsite
> ftrace_ops, and I'm sending these patches on their own as I think they
> make sense regardless.
Thanks! this series looks good to me.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
So it is the good time to rewrite the new fprobe event handler
based on these interfaces :)
>
> Since v1 [1]:
> * Change ifdeferry per Steve's request
> * Add ftrace_regs_query_register_offset() per Masami's request
> * Fix a bunch of typos
>
> [1] https://lore.kernel.org/lkml/20221024140846.3555435-1-mark.rutland@arm.com
>
> This series can be found in my 'arm64/ftrace/minimal-regs' branch on
> kernel.org:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/
> git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
>
> This version is tagged as:
>
> arm64-ftrace-minimal-regs-20221103
>
> Thanks,
> Mark.
>
> Mark Rutland (4):
> ftrace: pass fregs to arch_ftrace_set_direct_caller()
> ftrace: rename ftrace_instruction_pointer_set() ->
> ftrace_regs_set_instruction_pointer()
> ftrace: abstract DYNAMIC_FTRACE_WITH_ARGS accesses
> ftrace: arm64: move from REGS to ARGS
>
> arch/arm64/Kconfig | 18 +++--
> arch/arm64/Makefile | 2 +-
> arch/arm64/include/asm/ftrace.h | 72 ++++++++++++++++--
> arch/arm64/kernel/asm-offsets.c | 13 ++++
> arch/arm64/kernel/entry-ftrace.S | 117 ++++++++++++------------------
> arch/arm64/kernel/ftrace.c | 82 ++++++++++++---------
> arch/arm64/kernel/module.c | 3 -
> arch/powerpc/include/asm/ftrace.h | 24 +++++-
> arch/s390/include/asm/ftrace.h | 29 +++++++-
> arch/x86/include/asm/ftrace.h | 49 +++++++++----
> include/linux/ftrace.h | 47 +++++++++---
> kernel/livepatch/patch.c | 2 +-
> kernel/trace/Kconfig | 6 +-
> kernel/trace/ftrace.c | 3 +-
> 14 files changed, 309 insertions(+), 158 deletions(-)
>
> --
> 2.30.2
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS
@ 2022-11-18 0:04 ` Masami Hiramatsu
0 siblings, 0 replies; 28+ messages in thread
From: Masami Hiramatsu @ 2022-11-18 0:04 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel, catalin.marinas, linux-arm-kernel, mhiramat,
revest, rostedt, will
On Thu, 3 Nov 2022 17:05:16 +0000
Mark Rutland <mark.rutland@arm.com> wrote:
> This series replaces arm64's support for FTRACE_WITH_REGS with support
> for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> removes some latent issues with inconsistent presentation of struct
> pt_regs (which can only be reliably saved/restored at exception
> boundaries).
>
> The existing FTRACE_WITH_REGS support was added for two major reasons:
>
> (1) To make it possible to use the ftrace graph tracer with pointer
> authentication, where it's necessary to snapshot/manipulate the LR
> before it is signed by the instrumented function.
>
> (2) To make it possible to implement LIVEPATCH in future, where we need
> to hook function entry before an instrumented function manipulates
> the stack or argument registers. Practically speaking, we need to
> preserve the argument/return registers, PC, LR, and SP.
>
> Neither of these requires the full set of pt_regs, and only requires us
> to save/restore a subset of registers used for passing
> arguments/return-values and context/return information (which is the
> minimum set we always need to save/restore today).
>
> As there is no longer a need to save different sets of registers for
> different features, we no longer need distinct `ftrace_caller` and
> `ftrace_regs_caller` trampolines. This allows the trampoline assembly to
> be simpler, and simplifies code which previously had to handle the two
> trampolines.
>
> I've tested this with the ftrace selftests, where there are no
> unexpected failures.
>
> I plan to build atop this with subsequent patches to add per-callsite
> ftrace_ops, and I'm sending these patches on their own as I think they
> make sense regardless.
Thanks! this series looks good to me.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
So it is the good time to rewrite the new fprobe event handler
based on these interfaces :)
>
> Since v1 [1]:
> * Change ifdeferry per Steve's request
> * Add ftrace_regs_query_register_offset() per Masami's request
> * Fix a bunch of typos
>
> [1] https://lore.kernel.org/lkml/20221024140846.3555435-1-mark.rutland@arm.com
>
> This series can be found in my 'arm64/ftrace/minimal-regs' branch on
> kernel.org:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/
> git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
>
> This version is tagged as:
>
> arm64-ftrace-minimal-regs-20221103
>
> Thanks,
> Mark.
>
> Mark Rutland (4):
> ftrace: pass fregs to arch_ftrace_set_direct_caller()
> ftrace: rename ftrace_instruction_pointer_set() ->
> ftrace_regs_set_instruction_pointer()
> ftrace: abstract DYNAMIC_FTRACE_WITH_ARGS accesses
> ftrace: arm64: move from REGS to ARGS
>
> arch/arm64/Kconfig | 18 +++--
> arch/arm64/Makefile | 2 +-
> arch/arm64/include/asm/ftrace.h | 72 ++++++++++++++++--
> arch/arm64/kernel/asm-offsets.c | 13 ++++
> arch/arm64/kernel/entry-ftrace.S | 117 ++++++++++++------------------
> arch/arm64/kernel/ftrace.c | 82 ++++++++++++---------
> arch/arm64/kernel/module.c | 3 -
> arch/powerpc/include/asm/ftrace.h | 24 +++++-
> arch/s390/include/asm/ftrace.h | 29 +++++++-
> arch/x86/include/asm/ftrace.h | 49 +++++++++----
> include/linux/ftrace.h | 47 +++++++++---
> kernel/livepatch/patch.c | 2 +-
> kernel/trace/Kconfig | 6 +-
> kernel/trace/ftrace.c | 3 +-
> 14 files changed, 309 insertions(+), 158 deletions(-)
>
> --
> 2.30.2
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS
2022-11-17 10:52 ` Mark Rutland
@ 2022-11-18 12:31 ` Will Deacon
-1 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2022-11-18 12:31 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel, catalin.marinas, linux-arm-kernel, mhiramat,
revest, rostedt
On Thu, Nov 17, 2022 at 10:52:15AM +0000, Mark Rutland wrote:
> On Tue, Nov 15, 2022 at 11:27:03AM +0000, Will Deacon wrote:
> > On Thu, Nov 03, 2022 at 05:05:20PM +0000, Mark Rutland wrote:
> > > This commit replaces arm64's support for FTRACE_WITH_REGS with support
> > > for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> > > removes some latent issues with inconsistent presentation of struct
> > > pt_regs (which can only be reliably saved/restored at exception
> > > boundaries).
> >
> > [...]
> >
> > > @@ -78,10 +77,71 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
> > > return addr;
> > > }
> > >
> > > -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> > > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
> > > struct dyn_ftrace;
> > > struct ftrace_ops;
> > > -struct ftrace_regs;
> > > +
> > > +#define arch_ftrace_get_regs(regs) NULL
> > > +
> > > +struct ftrace_regs {
> > > + /* x0 - x8 */
> > > + unsigned long regs[9];
> > > + unsigned long __unused;
> > > +
> > > + unsigned long fp;
> > > + unsigned long lr;
> > > +
> > > + unsigned long sp;
> > > + unsigned long pc;
> > > +};
> > > +
> > > +static __always_inline unsigned long
> > > +ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs)
> > > +{
> > > + return fregs->pc;
> > > +}
> > > +
> > > +static __always_inline void
> > > +ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
> > > + unsigned long pc)
> > > +{
> > > + fregs->pc = pc;
> > > +}
> > > +
> > > +static __always_inline unsigned long
> > > +ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs)
> > > +{
> > > + return fregs->sp;
> > > +}
> > > +
> > > +static __always_inline unsigned long
> > > +ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n)
> > > +{
> > > + if (n < 8)
> > > + return fregs->regs[n];
> >
> > Where does this '8' come from?
>
> Because in AAPCS64 the arguments are in x0 to x7, as mentioned in the commit
> message:
>
> | Per AAPCS64, all function call argument and return values are held in
> | the following GPRs:
> |
> | * X0 - X7 : parameter / result registers
> | * X8 : indirect result location register
> | * SP : stack pointer (AKA SP)
>
> The 'indirect result location register' would be used when returning large
> structures, and isn't a function argument as such.
Ah gotcha, I was mainly wondering about the role of x8 in 'struct
ftrace_regs', but now I see that the FETCH_OP_REG might want to get at that.
Will
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS
@ 2022-11-18 12:31 ` Will Deacon
0 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2022-11-18 12:31 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel, catalin.marinas, linux-arm-kernel, mhiramat,
revest, rostedt
On Thu, Nov 17, 2022 at 10:52:15AM +0000, Mark Rutland wrote:
> On Tue, Nov 15, 2022 at 11:27:03AM +0000, Will Deacon wrote:
> > On Thu, Nov 03, 2022 at 05:05:20PM +0000, Mark Rutland wrote:
> > > This commit replaces arm64's support for FTRACE_WITH_REGS with support
> > > for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> > > removes some latent issues with inconsistent presentation of struct
> > > pt_regs (which can only be reliably saved/restored at exception
> > > boundaries).
> >
> > [...]
> >
> > > @@ -78,10 +77,71 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
> > > return addr;
> > > }
> > >
> > > -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> > > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
> > > struct dyn_ftrace;
> > > struct ftrace_ops;
> > > -struct ftrace_regs;
> > > +
> > > +#define arch_ftrace_get_regs(regs) NULL
> > > +
> > > +struct ftrace_regs {
> > > + /* x0 - x8 */
> > > + unsigned long regs[9];
> > > + unsigned long __unused;
> > > +
> > > + unsigned long fp;
> > > + unsigned long lr;
> > > +
> > > + unsigned long sp;
> > > + unsigned long pc;
> > > +};
> > > +
> > > +static __always_inline unsigned long
> > > +ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs)
> > > +{
> > > + return fregs->pc;
> > > +}
> > > +
> > > +static __always_inline void
> > > +ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
> > > + unsigned long pc)
> > > +{
> > > + fregs->pc = pc;
> > > +}
> > > +
> > > +static __always_inline unsigned long
> > > +ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs)
> > > +{
> > > + return fregs->sp;
> > > +}
> > > +
> > > +static __always_inline unsigned long
> > > +ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n)
> > > +{
> > > + if (n < 8)
> > > + return fregs->regs[n];
> >
> > Where does this '8' come from?
>
> Because in AAPCS64 the arguments are in x0 to x7, as mentioned in the commit
> message:
>
> | Per AAPCS64, all function call argument and return values are held in
> | the following GPRs:
> |
> | * X0 - X7 : parameter / result registers
> | * X8 : indirect result location register
> | * SP : stack pointer (AKA SP)
>
> The 'indirect result location register' would be used when returning large
> structures, and isn't a function argument as such.
Ah gotcha, I was mainly wondering about the role of x8 in 'struct
ftrace_regs', but now I see that the FETCH_OP_REG might want to get at that.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS
2022-11-18 12:31 ` Will Deacon
@ 2022-11-18 13:57 ` Mark Rutland
-1 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2022-11-18 13:57 UTC (permalink / raw)
To: Will Deacon
Cc: linux-kernel, catalin.marinas, linux-arm-kernel, mhiramat,
revest, rostedt
On Fri, Nov 18, 2022 at 12:31:50PM +0000, Will Deacon wrote:
> On Thu, Nov 17, 2022 at 10:52:15AM +0000, Mark Rutland wrote:
> > On Tue, Nov 15, 2022 at 11:27:03AM +0000, Will Deacon wrote:
> > > On Thu, Nov 03, 2022 at 05:05:20PM +0000, Mark Rutland wrote:
> > > > This commit replaces arm64's support for FTRACE_WITH_REGS with support
> > > > for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> > > > removes some latent issues with inconsistent presentation of struct
> > > > pt_regs (which can only be reliably saved/restored at exception
> > > > boundaries).
> > >
> > > [...]
> > >
> > > > @@ -78,10 +77,71 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
> > > > return addr;
> > > > }
> > > >
> > > > -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> > > > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
> > > > struct dyn_ftrace;
> > > > struct ftrace_ops;
> > > > -struct ftrace_regs;
> > > > +
> > > > +#define arch_ftrace_get_regs(regs) NULL
> > > > +
> > > > +struct ftrace_regs {
> > > > + /* x0 - x8 */
> > > > + unsigned long regs[9];
> > > > + unsigned long __unused;
> > > > +
> > > > + unsigned long fp;
> > > > + unsigned long lr;
> > > > +
> > > > + unsigned long sp;
> > > > + unsigned long pc;
> > > > +};
> > > > +
> > > > +static __always_inline unsigned long
> > > > +ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs)
> > > > +{
> > > > + return fregs->pc;
> > > > +}
> > > > +
> > > > +static __always_inline void
> > > > +ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
> > > > + unsigned long pc)
> > > > +{
> > > > + fregs->pc = pc;
> > > > +}
> > > > +
> > > > +static __always_inline unsigned long
> > > > +ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs)
> > > > +{
> > > > + return fregs->sp;
> > > > +}
> > > > +
> > > > +static __always_inline unsigned long
> > > > +ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n)
> > > > +{
> > > > + if (n < 8)
> > > > + return fregs->regs[n];
> > >
> > > Where does this '8' come from?
> >
> > Because in AAPCS64 the arguments are in x0 to x7, as mentioned in the commit
> > message:
> >
> > | Per AAPCS64, all function call argument and return values are held in
> > | the following GPRs:
> > |
> > | * X0 - X7 : parameter / result registers
> > | * X8 : indirect result location register
> > | * SP : stack pointer (AKA SP)
> >
> > The 'indirect result location register' would be used when returning large
> > structures, and isn't a function argument as such.
>
> Ah gotcha, I was mainly wondering about the role of x8 in 'struct
> ftrace_regs', but now I see that the FETCH_OP_REG might want to get at that.
Ah, I see. Should I just add the bits above from the commit message into a
comment block above the definition of struct ftrace_regs ?
Thanks,
Mark.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS
@ 2022-11-18 13:57 ` Mark Rutland
0 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2022-11-18 13:57 UTC (permalink / raw)
To: Will Deacon
Cc: linux-kernel, catalin.marinas, linux-arm-kernel, mhiramat,
revest, rostedt
On Fri, Nov 18, 2022 at 12:31:50PM +0000, Will Deacon wrote:
> On Thu, Nov 17, 2022 at 10:52:15AM +0000, Mark Rutland wrote:
> > On Tue, Nov 15, 2022 at 11:27:03AM +0000, Will Deacon wrote:
> > > On Thu, Nov 03, 2022 at 05:05:20PM +0000, Mark Rutland wrote:
> > > > This commit replaces arm64's support for FTRACE_WITH_REGS with support
> > > > for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> > > > removes some latent issues with inconsistent presentation of struct
> > > > pt_regs (which can only be reliably saved/restored at exception
> > > > boundaries).
> > >
> > > [...]
> > >
> > > > @@ -78,10 +77,71 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
> > > > return addr;
> > > > }
> > > >
> > > > -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> > > > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
> > > > struct dyn_ftrace;
> > > > struct ftrace_ops;
> > > > -struct ftrace_regs;
> > > > +
> > > > +#define arch_ftrace_get_regs(regs) NULL
> > > > +
> > > > +struct ftrace_regs {
> > > > + /* x0 - x8 */
> > > > + unsigned long regs[9];
> > > > + unsigned long __unused;
> > > > +
> > > > + unsigned long fp;
> > > > + unsigned long lr;
> > > > +
> > > > + unsigned long sp;
> > > > + unsigned long pc;
> > > > +};
> > > > +
> > > > +static __always_inline unsigned long
> > > > +ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs)
> > > > +{
> > > > + return fregs->pc;
> > > > +}
> > > > +
> > > > +static __always_inline void
> > > > +ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
> > > > + unsigned long pc)
> > > > +{
> > > > + fregs->pc = pc;
> > > > +}
> > > > +
> > > > +static __always_inline unsigned long
> > > > +ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs)
> > > > +{
> > > > + return fregs->sp;
> > > > +}
> > > > +
> > > > +static __always_inline unsigned long
> > > > +ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n)
> > > > +{
> > > > + if (n < 8)
> > > > + return fregs->regs[n];
> > >
> > > Where does this '8' come from?
> >
> > Because in AAPCS64 the arguments are in x0 to x7, as mentioned in the commit
> > message:
> >
> > | Per AAPCS64, all function call argument and return values are held in
> > | the following GPRs:
> > |
> > | * X0 - X7 : parameter / result registers
> > | * X8 : indirect result location register
> > | * SP : stack pointer (AKA SP)
> >
> > The 'indirect result location register' would be used when returning large
> > structures, and isn't a function argument as such.
>
> Ah gotcha, I was mainly wondering about the role of x8 in 'struct
> ftrace_regs', but now I see that the FETCH_OP_REG might want to get at that.
Ah, I see. Should I just add the bits above from the commit message into a
comment block above the definition of struct ftrace_regs ?
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS
2022-11-18 13:57 ` Mark Rutland
@ 2022-11-18 14:09 ` Will Deacon
-1 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2022-11-18 14:09 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel, catalin.marinas, linux-arm-kernel, mhiramat,
revest, rostedt
On Fri, Nov 18, 2022 at 01:57:52PM +0000, Mark Rutland wrote:
> On Fri, Nov 18, 2022 at 12:31:50PM +0000, Will Deacon wrote:
> > On Thu, Nov 17, 2022 at 10:52:15AM +0000, Mark Rutland wrote:
> > > On Tue, Nov 15, 2022 at 11:27:03AM +0000, Will Deacon wrote:
> > > > On Thu, Nov 03, 2022 at 05:05:20PM +0000, Mark Rutland wrote:
> > > > > +static __always_inline unsigned long
> > > > > +ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n)
> > > > > +{
> > > > > + if (n < 8)
> > > > > + return fregs->regs[n];
> > > >
> > > > Where does this '8' come from?
> > >
> > > Because in AAPCS64 the arguments are in x0 to x7, as mentioned in the commit
> > > message:
> > >
> > > | Per AAPCS64, all function call argument and return values are held in
> > > | the following GPRs:
> > > |
> > > | * X0 - X7 : parameter / result registers
> > > | * X8 : indirect result location register
> > > | * SP : stack pointer (AKA SP)
> > >
> > > The 'indirect result location register' would be used when returning large
> > > structures, and isn't a function argument as such.
> >
> > Ah gotcha, I was mainly wondering about the role of x8 in 'struct
> > ftrace_regs', but now I see that the FETCH_OP_REG might want to get at that.
>
> Ah, I see. Should I just add the bits above from the commit message into a
> comment block above the definition of struct ftrace_regs ?
Nah, it's ok, mainly just me learning what this is doing and I've queued it
locally now.
Will
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS
@ 2022-11-18 14:09 ` Will Deacon
0 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2022-11-18 14:09 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel, catalin.marinas, linux-arm-kernel, mhiramat,
revest, rostedt
On Fri, Nov 18, 2022 at 01:57:52PM +0000, Mark Rutland wrote:
> On Fri, Nov 18, 2022 at 12:31:50PM +0000, Will Deacon wrote:
> > On Thu, Nov 17, 2022 at 10:52:15AM +0000, Mark Rutland wrote:
> > > On Tue, Nov 15, 2022 at 11:27:03AM +0000, Will Deacon wrote:
> > > > On Thu, Nov 03, 2022 at 05:05:20PM +0000, Mark Rutland wrote:
> > > > > +static __always_inline unsigned long
> > > > > +ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n)
> > > > > +{
> > > > > + if (n < 8)
> > > > > + return fregs->regs[n];
> > > >
> > > > Where does this '8' come from?
> > >
> > > Because in AAPCS64 the arguments are in x0 to x7, as mentioned in the commit
> > > message:
> > >
> > > | Per AAPCS64, all function call argument and return values are held in
> > > | the following GPRs:
> > > |
> > > | * X0 - X7 : parameter / result registers
> > > | * X8 : indirect result location register
> > > | * SP : stack pointer (AKA SP)
> > >
> > > The 'indirect result location register' would be used when returning large
> > > structures, and isn't a function argument as such.
> >
> > Ah gotcha, I was mainly wondering about the role of x8 in 'struct
> > ftrace_regs', but now I see that the FETCH_OP_REG might want to get at that.
>
> Ah, I see. Should I just add the bits above from the commit message into a
> comment block above the definition of struct ftrace_regs ?
Nah, it's ok, mainly just me learning what this is doing and I've queued it
locally now.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS
2022-11-03 17:05 ` Mark Rutland
@ 2022-11-18 19:40 ` Will Deacon
-1 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2022-11-18 19:40 UTC (permalink / raw)
To: Mark Rutland, linux-kernel
Cc: catalin.marinas, kernel-team, Will Deacon, linux-arm-kernel,
rostedt, revest, mhiramat
On Thu, 3 Nov 2022 17:05:16 +0000, Mark Rutland wrote:
> This series replaces arm64's support for FTRACE_WITH_REGS with support
> for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> removes some latent issues with inconsistent presentation of struct
> pt_regs (which can only be reliably saved/restored at exception
> boundaries).
>
> The existing FTRACE_WITH_REGS support was added for two major reasons:
>
> [...]
Applied to arm64 (for-next/ftrace), thanks!
[1/4] ftrace: pass fregs to arch_ftrace_set_direct_caller()
https://git.kernel.org/arm64/c/9705bc709604
[2/4] ftrace: rename ftrace_instruction_pointer_set() -> ftrace_regs_set_instruction_pointer()
https://git.kernel.org/arm64/c/0ef86097f127
[3/4] ftrace: abstract DYNAMIC_FTRACE_WITH_ARGS accesses
https://git.kernel.org/arm64/c/94d095ffa0e1
[4/4] ftrace: arm64: move from REGS to ARGS
https://git.kernel.org/arm64/c/26299b3f6ba2
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS
@ 2022-11-18 19:40 ` Will Deacon
0 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2022-11-18 19:40 UTC (permalink / raw)
To: Mark Rutland, linux-kernel
Cc: catalin.marinas, kernel-team, Will Deacon, linux-arm-kernel,
rostedt, revest, mhiramat
On Thu, 3 Nov 2022 17:05:16 +0000, Mark Rutland wrote:
> This series replaces arm64's support for FTRACE_WITH_REGS with support
> for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> removes some latent issues with inconsistent presentation of struct
> pt_regs (which can only be reliably saved/restored at exception
> boundaries).
>
> The existing FTRACE_WITH_REGS support was added for two major reasons:
>
> [...]
Applied to arm64 (for-next/ftrace), thanks!
[1/4] ftrace: pass fregs to arch_ftrace_set_direct_caller()
https://git.kernel.org/arm64/c/9705bc709604
[2/4] ftrace: rename ftrace_instruction_pointer_set() -> ftrace_regs_set_instruction_pointer()
https://git.kernel.org/arm64/c/0ef86097f127
[3/4] ftrace: abstract DYNAMIC_FTRACE_WITH_ARGS accesses
https://git.kernel.org/arm64/c/94d095ffa0e1
[4/4] ftrace: arm64: move from REGS to ARGS
https://git.kernel.org/arm64/c/26299b3f6ba2
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2022-11-18 19:44 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-03 17:05 [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS Mark Rutland
2022-11-03 17:05 ` Mark Rutland
2022-11-03 17:05 ` [PATCH v2 1/4] ftrace: pass fregs to arch_ftrace_set_direct_caller() Mark Rutland
2022-11-03 17:05 ` Mark Rutland
2022-11-03 17:05 ` [PATCH v2 2/4] ftrace: rename ftrace_instruction_pointer_set() -> ftrace_regs_set_instruction_pointer() Mark Rutland
2022-11-03 17:05 ` Mark Rutland
2022-11-03 17:05 ` [PATCH v2 3/4] ftrace: abstract DYNAMIC_FTRACE_WITH_ARGS accesses Mark Rutland
2022-11-03 17:05 ` Mark Rutland
2022-11-03 17:05 ` [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS Mark Rutland
2022-11-03 17:05 ` Mark Rutland
2022-11-15 11:27 ` Will Deacon
2022-11-15 11:27 ` Will Deacon
2022-11-17 10:52 ` Mark Rutland
2022-11-17 10:52 ` Mark Rutland
2022-11-18 12:31 ` Will Deacon
2022-11-18 12:31 ` Will Deacon
2022-11-18 13:57 ` Mark Rutland
2022-11-18 13:57 ` Mark Rutland
2022-11-18 14:09 ` Will Deacon
2022-11-18 14:09 ` Will Deacon
2022-11-15 15:01 ` [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS Steven Rostedt
2022-11-15 15:01 ` Steven Rostedt
2022-11-15 15:48 ` Mark Rutland
2022-11-15 15:48 ` Mark Rutland
2022-11-18 0:04 ` Masami Hiramatsu
2022-11-18 0:04 ` Masami Hiramatsu
2022-11-18 19:40 ` Will Deacon
2022-11-18 19:40 ` Will Deacon
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.