Messages in this thread | | | Date | Wed, 31 Jan 2024 15:46:27 +0100 | Subject | Re: [PATCH] riscv: add CALLER_ADDRx support | From | Alexandre Ghiti <> |
| |
Hi Zong,
On 31/01/2024 15:24, Zong Li wrote: > 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
I'm not Palmer but I'll take a look at your patch today :)
Thanks,
Alex
> > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
| |