This is the arm64 version of ftrace-based kprobes to avoid the overhead
with regular kprobes, by using the ftrace infrastructure.
Signed-off-by: Jianlin Lv <[email protected]>
---
.../debug/kprobes-on-ftrace/arch-support.txt | 2 +-
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/probes/Makefile | 1 +
arch/arm64/kernel/probes/kprobes-ftrace.c | 81 +++++++++++++++++++
include/linux/kprobes.h | 2 +
kernel/kprobes.c | 4 +-
6 files changed, 88 insertions(+), 3 deletions(-)
create mode 100644 arch/arm64/kernel/probes/kprobes-ftrace.c
diff --git a/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt b/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
index b3697f4c806e..fadbd4513c91 100644
--- a/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
+++ b/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
@@ -9,7 +9,7 @@
| alpha: | TODO |
| arc: | TODO |
| arm: | TODO |
- | arm64: | TODO |
+ | arm64: | ok |
| csky: | ok |
| hexagon: | TODO |
| ia64: | TODO |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1652a9800ebe..7271d97e035a 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -204,6 +204,7 @@ config ARM64
select HAVE_STACKPROTECTOR
select HAVE_SYSCALL_TRACEPOINTS
select HAVE_KPROBES
+ select HAVE_KPROBES_ON_FTRACE
select HAVE_KRETPROBES
select HAVE_GENERIC_VDSO
select IOMMU_DMA if IOMMU_SUPPORT
diff --git a/arch/arm64/kernel/probes/Makefile b/arch/arm64/kernel/probes/Makefile
index 8e4be92e25b1..dab2fa4dba81 100644
--- a/arch/arm64/kernel/probes/Makefile
+++ b/arch/arm64/kernel/probes/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_KPROBES) += kprobes.o decode-insn.o \
simulate-insn.o
obj-$(CONFIG_UPROBES) += uprobes.o decode-insn.o \
simulate-insn.o
+obj-$(CONFIG_KPROBES_ON_FTRACE) += kprobes-ftrace.o
diff --git a/arch/arm64/kernel/probes/kprobes-ftrace.c b/arch/arm64/kernel/probes/kprobes-ftrace.c
new file mode 100644
index 000000000000..fcfa6b6533a0
--- /dev/null
+++ b/arch/arm64/kernel/probes/kprobes-ftrace.c
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Dynamic Ftrace based Kprobes Optimization
+ *
+ * Copyright (C) Hitachi Ltd., 2012
+ */
+#include <linux/kprobes.h>
+
+int arch_check_ftrace_location(struct kprobe *p)
+{
+ unsigned long addr = (unsigned long)p->addr;
+
+ /* ftrace location at the BL in the callsite (<function-entry> + AARCH64_INSN_SIZE) */
+ if (ftrace_location(addr) == (addr + AARCH64_INSN_SIZE)) {
+#ifdef CONFIG_KPROBES_ON_FTRACE
+ p->flags |= KPROBE_FLAG_FTRACE;
+#else /* !CONFIG_KPROBES_ON_FTRACE */
+ return -EINVAL;
+#endif
+ }
+ return 0;
+}
+
+/* Ftrace callback handler for kprobes */
+void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *ops, struct ftrace_regs *fregs)
+{
+ struct pt_regs *regs;
+ struct kprobe *p;
+ struct kprobe_ctlblk *kcb;
+ int bit;
+
+ bit = ftrace_test_recursion_trylock(ip, parent_ip);
+ if (bit < 0)
+ return;
+
+ regs = ftrace_get_regs(fregs);
+ p = get_kprobe((kprobe_opcode_t *) (ip - AARCH64_INSN_SIZE));
+ if (unlikely(!p) || kprobe_disabled(p))
+ goto out;
+
+ kcb = get_kprobe_ctlblk();
+ if (kprobe_running()) {
+ kprobes_inc_nmissed_count(p);
+ } else {
+ unsigned long orig_ip = instruction_pointer(regs);
+
+ instruction_pointer_set(regs, ip - AARCH64_INSN_SIZE);
+
+ __this_cpu_write(current_kprobe, p);
+ kcb->kprobe_status = KPROBE_HIT_ACTIVE;
+ if (!p->pre_handler || !p->pre_handler(p, regs)) {
+ /*
+ * Emulate singlestep (and also recover regs->pc)
+ * as if there is a 4byte nop
+ */
+ instruction_pointer_set(regs, ip);
+ if (unlikely(p->post_handler)) {
+ kcb->kprobe_status = KPROBE_HIT_SSDONE;
+ p->post_handler(p, regs, 0);
+ }
+ instruction_pointer_set(regs, orig_ip);
+ }
+
+ /*
+ * If pre_handler returns !0, it changes regs->pc. We have to
+ * skip emulating post_handler.
+ */
+ __this_cpu_write(current_kprobe, NULL);
+ }
+out:
+ ftrace_test_recursion_unlock(bit);
+}
+NOKPROBE_SYMBOL(kprobe_ftrace_handler);
+
+int arch_prepare_kprobe_ftrace(struct kprobe *p)
+{
+ p->ainsn.api.insn = NULL;
+ return 0;
+}
+
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 55041d2f884d..83a479908029 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -392,6 +392,8 @@ static inline int arch_prepare_kprobe_ftrace(struct kprobe *p)
}
#endif /* CONFIG_KPROBES_ON_FTRACE */
+int arch_check_ftrace_location(struct kprobe *p);
+
/* Get the kprobe at this addr (if any) - called with preemption disabled */
struct kprobe *get_kprobe(void *addr);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index f214f8c088ed..34754c2fe14e 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1534,7 +1534,7 @@ static inline int warn_kprobe_rereg(struct kprobe *p)
return ret;
}
-static int check_ftrace_location(struct kprobe *p)
+int __weak arch_check_ftrace_location(struct kprobe *p)
{
unsigned long addr = (unsigned long)p->addr;
@@ -1553,7 +1553,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
{
int ret;
- ret = check_ftrace_location(p);
+ ret = arch_check_ftrace_location(p);
if (ret)
return ret;
jump_label_lock();
--
2.25.1
On Mon, Nov 07, 2022 at 02:49:31PM +0000, Will Deacon wrote:
> [+Mark R]
>
> On Thu, Jul 28, 2022 at 02:02:50AM +0000, Jianlin Lv wrote:
> > This is the arm64 version of ftrace-based kprobes to avoid the overhead
> > with regular kprobes, by using the ftrace infrastructure.
> >
> > Signed-off-by: Jianlin Lv <[email protected]>
> > ---
> > .../debug/kprobes-on-ftrace/arch-support.txt | 2 +-
> > arch/arm64/Kconfig | 1 +
> > arch/arm64/kernel/probes/Makefile | 1 +
> > arch/arm64/kernel/probes/kprobes-ftrace.c | 81 +++++++++++++++++++
> > include/linux/kprobes.h | 2 +
> > kernel/kprobes.c | 4 +-
> > 6 files changed, 88 insertions(+), 3 deletions(-)
> > create mode 100644 arch/arm64/kernel/probes/kprobes-ftrace.c
>
> Sorry for the slow reply on this, but I think this deserved to be split
> into two patches: the first one reworking the core check_ftrace_location()
> logic to work properly with branch-and-link style architectures, and the
> second one adding support for arm64.
I'd prefer we don't do this at all; there a bunch of issues with kprobes *not*
taking an exception, since we get a dodgy not-quite-real pt_regs, and to clean
up the existing issues the plan is:
1) Move ftrace over to ftrace_regs
2) Implement fprobes using ftrace_regs
3) Remove kretprobes
... and regular kprobes will need to take an exception (via BRK) to get a real
pt_regs, so that can't be optimized to use ftrace.
Thanks,
Mark.
On Mon, Nov 07, 2022 at 03:32:24PM +0000, Mark Rutland wrote:
> On Mon, Nov 07, 2022 at 02:49:31PM +0000, Will Deacon wrote:
> > [+Mark R]
> >
> > On Thu, Jul 28, 2022 at 02:02:50AM +0000, Jianlin Lv wrote:
> > > This is the arm64 version of ftrace-based kprobes to avoid the overhead
> > > with regular kprobes, by using the ftrace infrastructure.
> > >
> > > Signed-off-by: Jianlin Lv <[email protected]>
> > > ---
> > > .../debug/kprobes-on-ftrace/arch-support.txt | 2 +-
> > > arch/arm64/Kconfig | 1 +
> > > arch/arm64/kernel/probes/Makefile | 1 +
> > > arch/arm64/kernel/probes/kprobes-ftrace.c | 81 +++++++++++++++++++
> > > include/linux/kprobes.h | 2 +
> > > kernel/kprobes.c | 4 +-
> > > 6 files changed, 88 insertions(+), 3 deletions(-)
> > > create mode 100644 arch/arm64/kernel/probes/kprobes-ftrace.c
> >
> > Sorry for the slow reply on this, but I think this deserved to be split
> > into two patches: the first one reworking the core check_ftrace_location()
> > logic to work properly with branch-and-link style architectures, and the
> > second one adding support for arm64.
>
> I'd prefer we don't do this at all; there a bunch of issues with kprobes *not*
> taking an exception, since we get a dodgy not-quite-real pt_regs, and to clean
> up the existing issues the plan is:
>
> 1) Move ftrace over to ftrace_regs
> 2) Implement fprobes using ftrace_regs
> 3) Remove kretprobes
>
> ... and regular kprobes will need to take an exception (via BRK) to get a real
> pt_regs, so that can't be optimized to use ftrace.
OKey doke. Does that mean that other architectures will follow the same
approach of taking an exception, or do they somehow work by magic?
Will
[+Mark R]
On Thu, Jul 28, 2022 at 02:02:50AM +0000, Jianlin Lv wrote:
> This is the arm64 version of ftrace-based kprobes to avoid the overhead
> with regular kprobes, by using the ftrace infrastructure.
>
> Signed-off-by: Jianlin Lv <[email protected]>
> ---
> .../debug/kprobes-on-ftrace/arch-support.txt | 2 +-
> arch/arm64/Kconfig | 1 +
> arch/arm64/kernel/probes/Makefile | 1 +
> arch/arm64/kernel/probes/kprobes-ftrace.c | 81 +++++++++++++++++++
> include/linux/kprobes.h | 2 +
> kernel/kprobes.c | 4 +-
> 6 files changed, 88 insertions(+), 3 deletions(-)
> create mode 100644 arch/arm64/kernel/probes/kprobes-ftrace.c
Sorry for the slow reply on this, but I think this deserved to be split
into two patches: the first one reworking the core check_ftrace_location()
logic to work properly with branch-and-link style architectures, and the
second one adding support for arm64.
I'd also prefer that we don't just punt the whole of check_ftrace_location()
to the arch code using weak symbols. I'd have thought it would be cleaner
for architectures to specify the offset which needs to be applied to the
PC instead.
Having said that, how do architectures such as PowerPC and Risc-V handle
this today without changing the core code?
Will
On Mon, Nov 07, 2022 at 03:35:07PM +0000, Will Deacon wrote:
> On Mon, Nov 07, 2022 at 03:32:24PM +0000, Mark Rutland wrote:
> > On Mon, Nov 07, 2022 at 02:49:31PM +0000, Will Deacon wrote:
> > > [+Mark R]
> > >
> > > On Thu, Jul 28, 2022 at 02:02:50AM +0000, Jianlin Lv wrote:
> > > > This is the arm64 version of ftrace-based kprobes to avoid the overhead
> > > > with regular kprobes, by using the ftrace infrastructure.
> > > >
> > > > Signed-off-by: Jianlin Lv <[email protected]>
> > > > ---
> > > > .../debug/kprobes-on-ftrace/arch-support.txt | 2 +-
> > > > arch/arm64/Kconfig | 1 +
> > > > arch/arm64/kernel/probes/Makefile | 1 +
> > > > arch/arm64/kernel/probes/kprobes-ftrace.c | 81 +++++++++++++++++++
> > > > include/linux/kprobes.h | 2 +
> > > > kernel/kprobes.c | 4 +-
> > > > 6 files changed, 88 insertions(+), 3 deletions(-)
> > > > create mode 100644 arch/arm64/kernel/probes/kprobes-ftrace.c
> > >
> > > Sorry for the slow reply on this, but I think this deserved to be split
> > > into two patches: the first one reworking the core check_ftrace_location()
> > > logic to work properly with branch-and-link style architectures, and the
> > > second one adding support for arm64.
> >
> > I'd prefer we don't do this at all; there a bunch of issues with kprobes *not*
> > taking an exception, since we get a dodgy not-quite-real pt_regs, and to clean
> > up the existing issues the plan is:
> >
> > 1) Move ftrace over to ftrace_regs
> > 2) Implement fprobes using ftrace_regs
> > 3) Remove kretprobes
> >
> > ... and regular kprobes will need to take an exception (via BRK) to get a real
> > pt_regs, so that can't be optimized to use ftrace.
>
> OKey doke. Does that mean that other architectures will follow the same
> approach of taking an exception,
I think once everyone has FPROBE, KPROBES_ON_FTRACE becomes redundant, and
could be removed (leaving kprobes to always follow a take-an-exception flow on
all architectures).
> or do they somehow work by magic?
Some architectures don't need to take an exception to be able to create a full
pt_regs (e.g. x86's flags are accessible in a way arm64's PSTATE isn't), but
that needs to be generated / restored differently to exception entry/return,
and so even where it's possible it can be painful to maintain (and slower than
using ftrace_regs), so I suspect KPROBES_ON_FTRACE would be removed.
So different constaints more than magic.
Thanks,
Mark.
On Mon, 7 Nov 2022 16:06:26 +0000
Mark Rutland <[email protected]> wrote:
> On Mon, Nov 07, 2022 at 03:35:07PM +0000, Will Deacon wrote:
> > On Mon, Nov 07, 2022 at 03:32:24PM +0000, Mark Rutland wrote:
> > > On Mon, Nov 07, 2022 at 02:49:31PM +0000, Will Deacon wrote:
> > > > [+Mark R]
> > > >
> > > > On Thu, Jul 28, 2022 at 02:02:50AM +0000, Jianlin Lv wrote:
> > > > > This is the arm64 version of ftrace-based kprobes to avoid the overhead
> > > > > with regular kprobes, by using the ftrace infrastructure.
> > > > >
> > > > > Signed-off-by: Jianlin Lv <[email protected]>
> > > > > ---
> > > > > .../debug/kprobes-on-ftrace/arch-support.txt | 2 +-
> > > > > arch/arm64/Kconfig | 1 +
> > > > > arch/arm64/kernel/probes/Makefile | 1 +
> > > > > arch/arm64/kernel/probes/kprobes-ftrace.c | 81 +++++++++++++++++++
> > > > > include/linux/kprobes.h | 2 +
> > > > > kernel/kprobes.c | 4 +-
> > > > > 6 files changed, 88 insertions(+), 3 deletions(-)
> > > > > create mode 100644 arch/arm64/kernel/probes/kprobes-ftrace.c
> > > >
> > > > Sorry for the slow reply on this, but I think this deserved to be split
> > > > into two patches: the first one reworking the core check_ftrace_location()
> > > > logic to work properly with branch-and-link style architectures, and the
> > > > second one adding support for arm64.
> > >
> > > I'd prefer we don't do this at all; there a bunch of issues with kprobes *not*
> > > taking an exception, since we get a dodgy not-quite-real pt_regs, and to clean
> > > up the existing issues the plan is:
> > >
> > > 1) Move ftrace over to ftrace_regs
> > > 2) Implement fprobes using ftrace_regs
> > > 3) Remove kretprobes
Yes, that is what we agreed at the tracing summit.
When we finished to move on the fprobe and fprobe-events, kretprobes
(and kprobe-on-ftrace if CONFIG_HAVE_DYNAMIC_FTRACE_WITH_REGS is not
supported) are not needed from the user viewpoint.
So we can mark the kretprobe API obsolete.
> > >
> > > ... and regular kprobes will need to take an exception (via BRK) to get a real
> > > pt_regs, so that can't be optimized to use ftrace.
> >
> > OKey doke. Does that mean that other architectures will follow the same
> > approach of taking an exception,
>
> I think once everyone has FPROBE, KPROBES_ON_FTRACE becomes redundant, and
> could be removed (leaving kprobes to always follow a take-an-exception flow on
> all architectures).
Anyway I will give some window to transit to the fprobe with ftrace_regs.
>
> > or do they somehow work by magic?
>
> Some architectures don't need to take an exception to be able to create a full
> pt_regs (e.g. x86's flags are accessible in a way arm64's PSTATE isn't), but
> that needs to be generated / restored differently to exception entry/return,
> and so even where it's possible it can be painful to maintain (and slower than
> using ftrace_regs), so I suspect KPROBES_ON_FTRACE would be removed.
I think KPROBES_ON_FTRACE support depends on CONFIG_HAVE_DYNAMIC_FTRACE_WITH_REGS.
When the all architecture removed it, I remove it. But it means that if function
tracer is enabled, we can not put any kprobes on the entry of functions on x86
because there is no space to put a software breakpoint at the function entry
on x86.
For the ftrace (tracefs) user, I will minimize the effect of this change, but
the tools developers (including eBPF developer) this change may be critical.
Thank you,
>
> So different constaints more than magic.
>
> Thanks,
> Mark.
--
Masami Hiramatsu (Google) <[email protected]>
On Tue, Nov 8, 2022 at 1:10 PM Masami Hiramatsu <[email protected]> wrote:
>
> On Mon, 7 Nov 2022 16:06:26 +0000
> Mark Rutland <[email protected]> wrote:
>
> > On Mon, Nov 07, 2022 at 03:35:07PM +0000, Will Deacon wrote:
> > > On Mon, Nov 07, 2022 at 03:32:24PM +0000, Mark Rutland wrote:
> > > > On Mon, Nov 07, 2022 at 02:49:31PM +0000, Will Deacon wrote:
> > > > > [+Mark R]
> > > > >
> > > > > On Thu, Jul 28, 2022 at 02:02:50AM +0000, Jianlin Lv wrote:
> > > > > > This is the arm64 version of ftrace-based kprobes to avoid the overhead
> > > > > > with regular kprobes, by using the ftrace infrastructure.
> > > > > >
> > > > > > Signed-off-by: Jianlin Lv <[email protected]>
> > > > > > ---
> > > > > > .../debug/kprobes-on-ftrace/arch-support.txt | 2 +-
> > > > > > arch/arm64/Kconfig | 1 +
> > > > > > arch/arm64/kernel/probes/Makefile | 1 +
> > > > > > arch/arm64/kernel/probes/kprobes-ftrace.c | 81 +++++++++++++++++++
> > > > > > include/linux/kprobes.h | 2 +
> > > > > > kernel/kprobes.c | 4 +-
> > > > > > 6 files changed, 88 insertions(+), 3 deletions(-)
> > > > > > create mode 100644 arch/arm64/kernel/probes/kprobes-ftrace.c
> > > > >
> > > > > Sorry for the slow reply on this, but I think this deserved to be split
> > > > > into two patches: the first one reworking the core check_ftrace_location()
> > > > > logic to work properly with branch-and-link style architectures, and the
> > > > > second one adding support for arm64.
> > > >
> > > > I'd prefer we don't do this at all; there a bunch of issues with kprobes *not*
> > > > taking an exception, since we get a dodgy not-quite-real pt_regs, and to clean
> > > > up the existing issues the plan is:
> > > >
> > > > 1) Move ftrace over to ftrace_regs
> > > > 2) Implement fprobes using ftrace_regs
> > > > 3) Remove kretprobes
>
> Yes, that is what we agreed at the tracing summit.
Sorry to miss TS2022.
Are there documents that can be shared to document the conclusions
reached at the tracing summit?
This will be helpful to understand what changes may be coming to tracing.
>
> When we finished to move on the fprobe and fprobe-events, kretprobes
> (and kprobe-on-ftrace if CONFIG_HAVE_DYNAMIC_FTRACE_WITH_REGS is not
> supported) are not needed from the user viewpoint.
> So we can mark the kretprobe API obsolete.
>
> > > >
> > > > ... and regular kprobes will need to take an exception (via BRK) to get a real
> > > > pt_regs, so that can't be optimized to use ftrace.
> > >
> > > OKey doke. Does that mean that other architectures will follow the same
> > > approach of taking an exception,
> >
> > I think once everyone has FPROBE, KPROBES_ON_FTRACE becomes redundant, and
> > could be removed (leaving kprobes to always follow a take-an-exception flow on
> > all architectures).
>
> Anyway I will give some window to transit to the fprobe with ftrace_regs.
>
> >
> > > or do they somehow work by magic?
> >
> > Some architectures don't need to take an exception to be able to create a full
> > pt_regs (e.g. x86's flags are accessible in a way arm64's PSTATE isn't), but
> > that needs to be generated / restored differently to exception entry/return,
> > and so even where it's possible it can be painful to maintain (and slower than
> > using ftrace_regs), so I suspect KPROBES_ON_FTRACE would be removed.
>
> I think KPROBES_ON_FTRACE support depends on CONFIG_HAVE_DYNAMIC_FTRACE_WITH_REGS.
> When the all architecture removed it, I remove it. But it means that if function
> tracer is enabled, we can not put any kprobes on the entry of functions on x86
> because there is no space to put a software breakpoint at the function entry
> on x86.
>
> For the ftrace (tracefs) user, I will minimize the effect of this change, but
> the tools developers (including eBPF developer) this change may be critical.
>
> Thank you,
>
>
> >
> > So different constaints more than magic.
> >
> > Thanks,
> > Mark.
>
>
> --
> Masami Hiramatsu (Google) <[email protected]>