lkml.org 
[lkml]   [2024]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] riscv: add CALLER_ADDRx support
From
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

\
 
 \ /
  Last update: 2024-05-27 14:43    [W:0.068 / U:0.192 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site