2023-04-27 14:10:01

by Florent Revest

[permalink] [raw]
Subject: [PATCH 0/2] Ftrace direct call samples improvements

This series is a subset of [1] that didn't go through the arm64 tree.

- The first patch fixes a small bug when a direct call sample is loaded on x86
- The second patch adds arm64 support to all direct calls samples

They are sent together because the second one depends on the first one.

This series applies cleanly on Linus's master branch. It needs the first two
patches of [1] which, at the time of writing, don't seem to have made it to the
trace/linux-trace tree but I suppose they could be pulled from Linus's master

1: https://lore.kernel.org/bpf/[email protected]/

Florent Revest (2):
samples: ftrace: Save required argument registers in sample
trampolines
arm64: ftrace: Add direct call trampoline samples support

arch/arm64/Kconfig | 2 ++
samples/ftrace/ftrace-direct-modify.c | 34 ++++++++++++++++++
samples/ftrace/ftrace-direct-multi-modify.c | 40 +++++++++++++++++++++
samples/ftrace/ftrace-direct-multi.c | 24 +++++++++++++
samples/ftrace/ftrace-direct-too.c | 40 +++++++++++++++++----
samples/ftrace/ftrace-direct.c | 24 +++++++++++++
6 files changed, 158 insertions(+), 6 deletions(-)

--
2.40.1.495.gc816e09b53d-goog


2023-04-27 14:10:08

by Florent Revest

[permalink] [raw]
Subject: [PATCH 1/2] samples: ftrace: Save required argument registers in sample trampolines

The ftrace-direct-too sample traces the handle_mm_fault function whose
signature changed since the introduction of the sample. Since:
commit bce617edecad ("mm: do page fault accounting in handle_mm_fault")
handle_mm_fault now has 4 arguments. Therefore, the sample trampoline
should save 4 argument registers.

s390 saves all argument registers already so it does not need a change
but x86_64 needs an extra push and pop.

This also evolves the signature of the tracing function to make it
mirror the signature of the traced function.

Cc: [email protected]
Fixes: bce617edecad ("mm: do page fault accounting in handle_mm_fault")
Reviewed-by: Steven Rostedt (Google) <[email protected]>
Reviewed-by: Mark Rutland <[email protected]>
Signed-off-by: Florent Revest <[email protected]>
---
samples/ftrace/ftrace-direct-too.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/samples/ftrace/ftrace-direct-too.c b/samples/ftrace/ftrace-direct-too.c
index f28e7b99840f..71ed4ee8cb4a 100644
--- a/samples/ftrace/ftrace-direct-too.c
+++ b/samples/ftrace/ftrace-direct-too.c
@@ -5,14 +5,14 @@
#include <linux/ftrace.h>
#include <asm/asm-offsets.h>

-extern void my_direct_func(struct vm_area_struct *vma,
- unsigned long address, unsigned int flags);
+extern void my_direct_func(struct vm_area_struct *vma, unsigned long address,
+ unsigned int flags, struct pt_regs *regs);

-void my_direct_func(struct vm_area_struct *vma,
- unsigned long address, unsigned int flags)
+void my_direct_func(struct vm_area_struct *vma, unsigned long address,
+ unsigned int flags, struct pt_regs *regs)
{
- trace_printk("handle mm fault vma=%p address=%lx flags=%x\n",
- vma, address, flags);
+ trace_printk("handle mm fault vma=%p address=%lx flags=%x regs=%p\n",
+ vma, address, flags, regs);
}

extern void my_tramp(void *);
@@ -34,7 +34,9 @@ asm (
" pushq %rdi\n"
" pushq %rsi\n"
" pushq %rdx\n"
+" pushq %rcx\n"
" call my_direct_func\n"
+" popq %rcx\n"
" popq %rdx\n"
" popq %rsi\n"
" popq %rdi\n"
--
2.40.1.495.gc816e09b53d-goog

2023-04-27 14:11:36

by Florent Revest

[permalink] [raw]
Subject: [PATCH 2/2] arm64: ftrace: Add direct call trampoline samples support

The ftrace samples need per-architecture trampoline implementations
to save and restore argument registers around the calls to
my_direct_func* and to restore polluted registers (eg: x30).

These samples also include <asm/asm-offsets.h> which, on arm64, is not
necessary and redefines previously defined macros (resulting in
warnings) so these includes are guarded by !CONFIG_ARM64.

Reviewed-by: Mark Rutland <[email protected]>
Tested-by: Mark Rutland <[email protected]>
Signed-off-by: Florent Revest <[email protected]>
---
arch/arm64/Kconfig | 2 ++
samples/ftrace/ftrace-direct-modify.c | 34 ++++++++++++++++++
samples/ftrace/ftrace-direct-multi-modify.c | 40 +++++++++++++++++++++
samples/ftrace/ftrace-direct-multi.c | 24 +++++++++++++
samples/ftrace/ftrace-direct-too.c | 26 ++++++++++++++
samples/ftrace/ftrace-direct.c | 24 +++++++++++++
6 files changed, 150 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 3f5bf55050e8..e98d6b7845c4 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -195,6 +195,8 @@ config ARM64
!CC_OPTIMIZE_FOR_SIZE)
select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY \
if DYNAMIC_FTRACE_WITH_ARGS
+ select HAVE_SAMPLE_FTRACE_DIRECT
+ select HAVE_SAMPLE_FTRACE_DIRECT_MULTI
select HAVE_EFFICIENT_UNALIGNED_ACCESS
select HAVE_FAST_GUP
select HAVE_FTRACE_MCOUNT_RECORD
diff --git a/samples/ftrace/ftrace-direct-modify.c b/samples/ftrace/ftrace-direct-modify.c
index 25fba66f61c0..98d1b7385f08 100644
--- a/samples/ftrace/ftrace-direct-modify.c
+++ b/samples/ftrace/ftrace-direct-modify.c
@@ -2,7 +2,9 @@
#include <linux/module.h>
#include <linux/kthread.h>
#include <linux/ftrace.h>
+#ifndef CONFIG_ARM64
#include <asm/asm-offsets.h>
+#endif

extern void my_direct_func1(void);
extern void my_direct_func2(void);
@@ -96,6 +98,38 @@ asm (

#endif /* CONFIG_S390 */

+#ifdef CONFIG_ARM64
+
+asm (
+" .pushsection .text, \"ax\", @progbits\n"
+" .type my_tramp1, @function\n"
+" .globl my_tramp1\n"
+" my_tramp1:"
+" bti c\n"
+" sub sp, sp, #16\n"
+" stp x9, x30, [sp]\n"
+" bl my_direct_func1\n"
+" ldp x30, x9, [sp]\n"
+" add sp, sp, #16\n"
+" ret x9\n"
+" .size my_tramp1, .-my_tramp1\n"
+
+" .type my_tramp2, @function\n"
+" .globl my_tramp2\n"
+" my_tramp2:"
+" bti c\n"
+" sub sp, sp, #16\n"
+" stp x9, x30, [sp]\n"
+" bl my_direct_func2\n"
+" ldp x30, x9, [sp]\n"
+" add sp, sp, #16\n"
+" ret x9\n"
+" .size my_tramp2, .-my_tramp2\n"
+" .popsection\n"
+);
+
+#endif /* CONFIG_ARM64 */
+
static struct ftrace_ops direct;

static unsigned long my_tramp = (unsigned long)my_tramp1;
diff --git a/samples/ftrace/ftrace-direct-multi-modify.c b/samples/ftrace/ftrace-direct-multi-modify.c
index f72623899602..26956c8fc513 100644
--- a/samples/ftrace/ftrace-direct-multi-modify.c
+++ b/samples/ftrace/ftrace-direct-multi-modify.c
@@ -2,7 +2,9 @@
#include <linux/module.h>
#include <linux/kthread.h>
#include <linux/ftrace.h>
+#ifndef CONFIG_ARM64
#include <asm/asm-offsets.h>
+#endif

extern void my_direct_func1(unsigned long ip);
extern void my_direct_func2(unsigned long ip);
@@ -103,6 +105,44 @@ asm (

#endif /* CONFIG_S390 */

+#ifdef CONFIG_ARM64
+
+asm (
+" .pushsection .text, \"ax\", @progbits\n"
+" .type my_tramp1, @function\n"
+" .globl my_tramp1\n"
+" my_tramp1:"
+" bti c\n"
+" sub sp, sp, #32\n"
+" stp x9, x30, [sp]\n"
+" str x0, [sp, #16]\n"
+" mov x0, x30\n"
+" bl my_direct_func1\n"
+" ldp x30, x9, [sp]\n"
+" ldr x0, [sp, #16]\n"
+" add sp, sp, #32\n"
+" ret x9\n"
+" .size my_tramp1, .-my_tramp1\n"
+
+" .type my_tramp2, @function\n"
+" .globl my_tramp2\n"
+" my_tramp2:"
+" bti c\n"
+" sub sp, sp, #32\n"
+" stp x9, x30, [sp]\n"
+" str x0, [sp, #16]\n"
+" mov x0, x30\n"
+" bl my_direct_func2\n"
+" ldp x30, x9, [sp]\n"
+" ldr x0, [sp, #16]\n"
+" add sp, sp, #32\n"
+" ret x9\n"
+" .size my_tramp2, .-my_tramp2\n"
+" .popsection\n"
+);
+
+#endif /* CONFIG_ARM64 */
+
static unsigned long my_tramp = (unsigned long)my_tramp1;
static unsigned long tramps[2] = {
(unsigned long)my_tramp1,
diff --git a/samples/ftrace/ftrace-direct-multi.c b/samples/ftrace/ftrace-direct-multi.c
index 1547c2c6be02..b2ac90e0c02e 100644
--- a/samples/ftrace/ftrace-direct-multi.c
+++ b/samples/ftrace/ftrace-direct-multi.c
@@ -4,7 +4,9 @@
#include <linux/mm.h> /* for handle_mm_fault() */
#include <linux/ftrace.h>
#include <linux/sched/stat.h>
+#ifndef CONFIG_ARM64
#include <asm/asm-offsets.h>
+#endif

extern void my_direct_func(unsigned long ip);

@@ -66,6 +68,28 @@ asm (

#endif /* CONFIG_S390 */

+#ifdef CONFIG_ARM64
+
+asm (
+" .pushsection .text, \"ax\", @progbits\n"
+" .type my_tramp, @function\n"
+" .globl my_tramp\n"
+" my_tramp:"
+" bti c\n"
+" sub sp, sp, #32\n"
+" stp x9, x30, [sp]\n"
+" str x0, [sp, #16]\n"
+" mov x0, x30\n"
+" bl my_direct_func\n"
+" ldp x30, x9, [sp]\n"
+" ldr x0, [sp, #16]\n"
+" add sp, sp, #32\n"
+" ret x9\n"
+" .size my_tramp, .-my_tramp\n"
+" .popsection\n"
+);
+
+#endif /* CONFIG_ARM64 */
static struct ftrace_ops direct;

static int __init ftrace_direct_multi_init(void)
diff --git a/samples/ftrace/ftrace-direct-too.c b/samples/ftrace/ftrace-direct-too.c
index 71ed4ee8cb4a..38f6f677f913 100644
--- a/samples/ftrace/ftrace-direct-too.c
+++ b/samples/ftrace/ftrace-direct-too.c
@@ -3,7 +3,9 @@

#include <linux/mm.h> /* for handle_mm_fault() */
#include <linux/ftrace.h>
+#ifndef CONFIG_ARM64
#include <asm/asm-offsets.h>
+#endif

extern void my_direct_func(struct vm_area_struct *vma, unsigned long address,
unsigned int flags, struct pt_regs *regs);
@@ -72,6 +74,30 @@ asm (

#endif /* CONFIG_S390 */

+#ifdef CONFIG_ARM64
+
+asm (
+" .pushsection .text, \"ax\", @progbits\n"
+" .type my_tramp, @function\n"
+" .globl my_tramp\n"
+" my_tramp:"
+" bti c\n"
+" sub sp, sp, #48\n"
+" stp x9, x30, [sp]\n"
+" stp x0, x1, [sp, #16]\n"
+" stp x2, x3, [sp, #32]\n"
+" bl my_direct_func\n"
+" ldp x30, x9, [sp]\n"
+" ldp x0, x1, [sp, #16]\n"
+" ldp x2, x3, [sp, #32]\n"
+" add sp, sp, #48\n"
+" ret x9\n"
+" .size my_tramp, .-my_tramp\n"
+" .popsection\n"
+);
+
+#endif /* CONFIG_ARM64 */
+
static struct ftrace_ops direct;

static int __init ftrace_direct_init(void)
diff --git a/samples/ftrace/ftrace-direct.c b/samples/ftrace/ftrace-direct.c
index d81a9473b585..e5312f9c15d3 100644
--- a/samples/ftrace/ftrace-direct.c
+++ b/samples/ftrace/ftrace-direct.c
@@ -3,7 +3,9 @@

#include <linux/sched.h> /* for wake_up_process() */
#include <linux/ftrace.h>
+#ifndef CONFIG_ARM64
#include <asm/asm-offsets.h>
+#endif

extern void my_direct_func(struct task_struct *p);

@@ -63,6 +65,28 @@ asm (

#endif /* CONFIG_S390 */

+#ifdef CONFIG_ARM64
+
+asm (
+" .pushsection .text, \"ax\", @progbits\n"
+" .type my_tramp, @function\n"
+" .globl my_tramp\n"
+" my_tramp:"
+" bti c\n"
+" sub sp, sp, #32\n"
+" stp x9, x30, [sp]\n"
+" str x0, [sp, #16]\n"
+" bl my_direct_func\n"
+" ldp x30, x9, [sp]\n"
+" ldr x0, [sp, #16]\n"
+" add sp, sp, #32\n"
+" ret x9\n"
+" .size my_tramp, .-my_tramp\n"
+" .popsection\n"
+);
+
+#endif /* CONFIG_ARM64 */
+
static struct ftrace_ops direct;

static int __init ftrace_direct_init(void)
--
2.40.1.495.gc816e09b53d-goog

2023-06-09 20:45:56

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 0/2] Ftrace direct call samples improvements

On Thu, 27 Apr 2023 16:06:58 +0200
Florent Revest <[email protected]> wrote:

> This series is a subset of [1] that didn't go through the arm64 tree.
>
> - The first patch fixes a small bug when a direct call sample is loaded on x86
> - The second patch adds arm64 support to all direct calls samples
>
> They are sent together because the second one depends on the first one.
>
> This series applies cleanly on Linus's master branch. It needs the first two
> patches of [1] which, at the time of writing, don't seem to have made it to the
> trace/linux-trace tree but I suppose they could be pulled from Linus's master
>
> 1: https://lore.kernel.org/bpf/[email protected]/
>
> Florent Revest (2):
> samples: ftrace: Save required argument registers in sample
> trampolines
> arm64: ftrace: Add direct call trampoline samples support

Is this going through the arm64 tree, or should it go through mine?

-- Steve


>
> arch/arm64/Kconfig | 2 ++
> samples/ftrace/ftrace-direct-modify.c | 34 ++++++++++++++++++
> samples/ftrace/ftrace-direct-multi-modify.c | 40 +++++++++++++++++++++
> samples/ftrace/ftrace-direct-multi.c | 24 +++++++++++++
> samples/ftrace/ftrace-direct-too.c | 40 +++++++++++++++++----
> samples/ftrace/ftrace-direct.c | 24 +++++++++++++
> 6 files changed, 158 insertions(+), 6 deletions(-)
>


2023-06-10 08:34:07

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH 0/2] Ftrace direct call samples improvements

On Fri, Jun 09, 2023 at 04:20:46PM -0400, Steven Rostedt wrote:
> On Thu, 27 Apr 2023 16:06:58 +0200
> Florent Revest <[email protected]> wrote:
>
> > This series is a subset of [1] that didn't go through the arm64 tree.
> >
> > - The first patch fixes a small bug when a direct call sample is loaded on x86
> > - The second patch adds arm64 support to all direct calls samples
> >
> > They are sent together because the second one depends on the first one.
> >
> > This series applies cleanly on Linus's master branch. It needs the first two
> > patches of [1] which, at the time of writing, don't seem to have made it to the
> > trace/linux-trace tree but I suppose they could be pulled from Linus's master
> >
> > 1: https://lore.kernel.org/bpf/[email protected]/
> >
> > Florent Revest (2):
> > samples: ftrace: Save required argument registers in sample
> > trampolines
> > arm64: ftrace: Add direct call trampoline samples support
>
> Is this going through the arm64 tree, or should it go through mine?

Feel free to take it through your tree.

Acked-by: Catalin Marinas <[email protected]>

2023-07-10 22:17:11

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 0/2] Ftrace direct call samples improvements

On Sat, 10 Jun 2023 09:28:36 +0100
Catalin Marinas <[email protected]> wrote:

> > Is this going through the arm64 tree, or should it go through mine?
>
> Feel free to take it through your tree.
>
> Acked-by: Catalin Marinas <[email protected]>

I think I missed your email and let this drop. I'll add it now. As it's
sample code, it should still be OK to add to the rc releases.

-- Steve