Messages in this thread | | | From | Zong Li <> | Date | Wed, 31 Jan 2024 22:24:58 +0800 | Subject | Re: [PATCH] riscv: add CALLER_ADDRx support |
| |
On Thu, Jan 18, 2024 at 8:50 AM Zong Li <zong.li@sifive.com> wrote: > > On Wed, Jan 10, 2024 at 11:33 AM Zong Li <zong.li@sifive.com> wrote: > > > > On Fri, Dec 29, 2023 at 2:34 PM Zong Li <zong.li@sifive.com> wrote: > > > > > > On Tue, Dec 5, 2023 at 5:00 PM Zong Li <zong.li@sifive.com> wrote: > > > > > > > > CALLER_ADDRx returns caller's address at specified level, they are used > > > > for several tracers. These macros eventually use > > > > __builtin_return_address(n) to get the caller's address if arch doesn't > > > > define their own implementation. > > > > > > > > In RISC-V, __builtin_return_address(n) only works when n == 0, we need > > > > to walk the stack frame to get the caller's address at specified level. > > > > > > > > data.level started from 'level + 3' due to the call flow of getting > > > > caller's address in RISC-V implementation. If we don't have additional > > > > three iteration, the level is corresponding to follows: > > > > > > > > callsite -> return_address -> arch_stack_walk -> walk_stackframe > > > > | | | | > > > > level 3 level 2 level 1 level 0 > > > > > > > > Signed-off-by: Zong Li <zong.li@sifive.com> > > > > --- > > > > arch/riscv/include/asm/ftrace.h | 5 ++++ > > > > arch/riscv/kernel/Makefile | 2 ++ > > > > arch/riscv/kernel/return_address.c | 48 ++++++++++++++++++++++++++++++ > > > > 3 files changed, 55 insertions(+) > > > > create mode 100644 arch/riscv/kernel/return_address.c > > > > > > > > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > > > > index 2b2f5df7ef2c..42777f91a9c5 100644 > > > > --- a/arch/riscv/include/asm/ftrace.h > > > > +++ b/arch/riscv/include/asm/ftrace.h > > > > @@ -25,6 +25,11 @@ > > > > > > > > #define ARCH_SUPPORTS_FTRACE_OPS 1 > > > > #ifndef __ASSEMBLY__ > > > > + > > > > +extern void *return_address(unsigned int level); > > > > + > > > > +#define ftrace_return_address(n) return_address(n) > > > > + > > > > void MCOUNT_NAME(void); > > > > static inline unsigned long ftrace_call_adjust(unsigned long addr) > > > > { > > > > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile > > > > index fee22a3d1b53..40d054939ae2 100644 > > > > --- a/arch/riscv/kernel/Makefile > > > > +++ b/arch/riscv/kernel/Makefile > > > > @@ -7,6 +7,7 @@ ifdef CONFIG_FTRACE > > > > CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE) > > > > CFLAGS_REMOVE_patch.o = $(CC_FLAGS_FTRACE) > > > > CFLAGS_REMOVE_sbi.o = $(CC_FLAGS_FTRACE) > > > > +CFLAGS_REMOVE_return_address.o = $(CC_FLAGS_FTRACE) > > > > endif > > > > CFLAGS_syscall_table.o += $(call cc-option,-Wno-override-init,) > > > > CFLAGS_compat_syscall_table.o += $(call cc-option,-Wno-override-init,) > > > > @@ -46,6 +47,7 @@ obj-y += irq.o > > > > obj-y += process.o > > > > obj-y += ptrace.o > > > > obj-y += reset.o > > > > +obj-y += return_address.o > > > > obj-y += setup.o > > > > obj-y += signal.o > > > > obj-y += syscall_table.o > > > > diff --git a/arch/riscv/kernel/return_address.c b/arch/riscv/kernel/return_address.c > > > > new file mode 100644 > > > > index 000000000000..c2008d4aa6e5 > > > > --- /dev/null > > > > +++ b/arch/riscv/kernel/return_address.c > > > > @@ -0,0 +1,48 @@ > > > > +// SPDX-License-Identifier: GPL-2.0-only > > > > +/* > > > > + * This code come from arch/arm64/kernel/return_address.c > > > > + * > > > > + * Copyright (C) 2023 SiFive. > > > > + */ > > > > + > > > > +#include <linux/export.h> > > > > +#include <linux/kprobes.h> > > > > +#include <linux/stacktrace.h> > > > > + > > > > +struct return_address_data { > > > > + unsigned int level; > > > > + void *addr; > > > > +}; > > > > + > > > > +static bool save_return_addr(void *d, unsigned long pc) > > > > +{ > > > > + struct return_address_data *data = d; > > > > + > > > > + if (!data->level) { > > > > + data->addr = (void *)pc; > > > > + return false; > > > > + } > > > > + > > > > + --data->level; > > > > + > > > > + return true; > > > > +} > > > > +NOKPROBE_SYMBOL(save_return_addr); > > > > + > > > > +void *return_address(unsigned int level) > > > > +{ > > > > + struct return_address_data data; > > > > + > > > > + data.level = level + 3; > > > > + data.addr = NULL; > > > > + > > > > + arch_stack_walk(save_return_addr, &data, current, NULL); > > > > + > > > > + if (!data.level) > > > > + return data.addr; > > > > + else > > > > + return NULL; > > > > + > > > > +} > > > > +EXPORT_SYMBOL_GPL(return_address); > > > > +NOKPROBE_SYMBOL(return_address); > > > > -- > > > > 2.17.1 > > > > > > > > > > Hi Palmer and all, > > > I was wondering whether this patch is good for everyone? Thanks > > > > Hi Palmer, > > Is there any chance to include this patch in 6.8-rc1? Thanks > > Hi Palmer, > I'm not sure if this patch is a feature or bug fix, would you consider > it for 6.8-rcX? Thanks
Hi Palmer, Sorry for pinging you again. I'm not sure if you saw this patch. The irqsoff and wakeup tracer will use CALLER_ADDR1 and CALLER_ADDR2 to obtain the caller's return address, but we are currently encountering an issue in the RISC-V port where the wrong caller is identified. I guess you can easily reproduce the situation using ftrace. Could you share your thoughts on this when you have the time to take a look? Thanks
| |