| Message ID | 20231003182407.32198-1-alexghiti@rivosinc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a87e7d3e8832271ecb7d5eaaabc5b49fe25a469b |
| Headers | show |
| Series | [-fixes] riscv: Fix ftrace syscall handling which are now prefixed with __riscv_ | expand |
@Conor Dooley This fails checkpatch but the documentation here states that this is how to do: https://elixir.bootlin.com/linux/latest/source/Documentation/trace/ftrace-design.rst#L246 On Tue, Oct 3, 2023 at 8:24 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote: > > ftrace creates entries for each syscall in the tracefs but has failed > since commit 08d0ce30e0e4 ("riscv: Implement syscall wrappers") which > prefixes all riscv syscalls with __riscv_. > > So fix this by implementing arch_syscall_match_sym_name() which allows us > to ignore this prefix. > > And also ignore compat syscalls like x86/arm64 by implementing > arch_trace_is_compat_syscall(). > > Fixes: 08d0ce30e0e4 ("riscv: Implement syscall wrappers") > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > --- > arch/riscv/include/asm/ftrace.h | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > index 740a979171e5..2b2f5df7ef2c 100644 > --- a/arch/riscv/include/asm/ftrace.h > +++ b/arch/riscv/include/asm/ftrace.h > @@ -31,6 +31,27 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr) > return addr; > } > > +/* > + * Let's do like x86/arm64 and ignore the compat syscalls. > + */ > +#define ARCH_TRACE_IGNORE_COMPAT_SYSCALLS > +static inline bool arch_trace_is_compat_syscall(struct pt_regs *regs) > +{ > + return is_compat_task(); > +} > + > +#define ARCH_HAS_SYSCALL_MATCH_SYM_NAME > +static inline bool arch_syscall_match_sym_name(const char *sym, > + const char *name) > +{ > + /* > + * Since all syscall functions have __riscv_ prefix, we must skip it. > + * However, as we described above, we decided to ignore compat > + * syscalls, so we don't care about __riscv_compat_ prefix here. > + */ > + return !strcmp(sym + 8, name); > +} > + > struct dyn_arch_ftrace { > }; > #endif > -- > 2.39.2 >
On Tue, Oct 3, 2023 at 11:24 AM Alexandre Ghiti <alexghiti@rivosinc.com> wrote: > > ftrace creates entries for each syscall in the tracefs but has failed > since commit 08d0ce30e0e4 ("riscv: Implement syscall wrappers") which > prefixes all riscv syscalls with __riscv_. > > So fix this by implementing arch_syscall_match_sym_name() which allows us > to ignore this prefix. > > And also ignore compat syscalls like x86/arm64 by implementing > arch_trace_is_compat_syscall(). > > Fixes: 08d0ce30e0e4 ("riscv: Implement syscall wrappers") > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > --- > arch/riscv/include/asm/ftrace.h | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > index 740a979171e5..2b2f5df7ef2c 100644 > --- a/arch/riscv/include/asm/ftrace.h > +++ b/arch/riscv/include/asm/ftrace.h > @@ -31,6 +31,27 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr) > return addr; > } > > +/* > + * Let's do like x86/arm64 and ignore the compat syscalls. > + */ > +#define ARCH_TRACE_IGNORE_COMPAT_SYSCALLS > +static inline bool arch_trace_is_compat_syscall(struct pt_regs *regs) > +{ > + return is_compat_task(); > +} > + > +#define ARCH_HAS_SYSCALL_MATCH_SYM_NAME > +static inline bool arch_syscall_match_sym_name(const char *sym, > + const char *name) > +{ > + /* > + * Since all syscall functions have __riscv_ prefix, we must skip it. > + * However, as we described above, we decided to ignore compat > + * syscalls, so we don't care about __riscv_compat_ prefix here. > + */ > + return !strcmp(sym + 8, name); > +} Good catch, thanks for fixing this! Reviewed-by: Sami Tolvanen <samitolvanen@google.com> Sami
On Tue, 3 Oct 2023 20:24:07 +0200 Alexandre Ghiti <alexghiti@rivosinc.com> wrote: > ftrace creates entries for each syscall in the tracefs but has failed > since commit 08d0ce30e0e4 ("riscv: Implement syscall wrappers") which > prefixes all riscv syscalls with __riscv_. > > So fix this by implementing arch_syscall_match_sym_name() which allows us > to ignore this prefix. > > And also ignore compat syscalls like x86/arm64 by implementing > arch_trace_is_compat_syscall(). > Looks good to me, Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Thank you, > Fixes: 08d0ce30e0e4 ("riscv: Implement syscall wrappers") > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > --- > arch/riscv/include/asm/ftrace.h | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > index 740a979171e5..2b2f5df7ef2c 100644 > --- a/arch/riscv/include/asm/ftrace.h > +++ b/arch/riscv/include/asm/ftrace.h > @@ -31,6 +31,27 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr) > return addr; > } > > +/* > + * Let's do like x86/arm64 and ignore the compat syscalls. > + */ > +#define ARCH_TRACE_IGNORE_COMPAT_SYSCALLS > +static inline bool arch_trace_is_compat_syscall(struct pt_regs *regs) > +{ > + return is_compat_task(); > +} > + > +#define ARCH_HAS_SYSCALL_MATCH_SYM_NAME > +static inline bool arch_syscall_match_sym_name(const char *sym, > + const char *name) > +{ > + /* > + * Since all syscall functions have __riscv_ prefix, we must skip it. > + * However, as we described above, we decided to ignore compat > + * syscalls, so we don't care about __riscv_compat_ prefix here. > + */ > + return !strcmp(sym + 8, name); > +} > + > struct dyn_arch_ftrace { > }; > #endif > -- > 2.39.2 >
Alexandre Ghiti <alexghiti@rivosinc.com> writes: > @Conor Dooley This fails checkpatch but the documentation here states > that this is how to do: > https://elixir.bootlin.com/linux/latest/source/Documentation/trace/ftrace-design.rst#L246 FWIW, I'll relax the PW CI checkpatch test going forward. It's way too strict...
Alexandre Ghiti <alexghiti@rivosinc.com> writes: > ftrace creates entries for each syscall in the tracefs but has failed > since commit 08d0ce30e0e4 ("riscv: Implement syscall wrappers") which > prefixes all riscv syscalls with __riscv_. > > So fix this by implementing arch_syscall_match_sym_name() which allows us > to ignore this prefix. > > And also ignore compat syscalls like x86/arm64 by implementing > arch_trace_is_compat_syscall(). > > Fixes: 08d0ce30e0e4 ("riscv: Implement syscall wrappers") > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> This fix does the BPF test suite happier! Tested-by: Björn Töpel <bjorn@rivosinc.com>
Hello: This patch was applied to riscv/linux.git (fixes) by Palmer Dabbelt <palmer@rivosinc.com>: On Tue, 3 Oct 2023 20:24:07 +0200 you wrote: > ftrace creates entries for each syscall in the tracefs but has failed > since commit 08d0ce30e0e4 ("riscv: Implement syscall wrappers") which > prefixes all riscv syscalls with __riscv_. > > So fix this by implementing arch_syscall_match_sym_name() which allows us > to ignore this prefix. > > [...] Here is the summary with links: - [-fixes] riscv: Fix ftrace syscall handling which are now prefixed with __riscv_ https://git.kernel.org/riscv/c/a87e7d3e8832 You are awesome, thank you!
diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h index 740a979171e5..2b2f5df7ef2c 100644 --- a/arch/riscv/include/asm/ftrace.h +++ b/arch/riscv/include/asm/ftrace.h @@ -31,6 +31,27 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr) return addr; } +/* + * Let's do like x86/arm64 and ignore the compat syscalls. + */ +#define ARCH_TRACE_IGNORE_COMPAT_SYSCALLS +static inline bool arch_trace_is_compat_syscall(struct pt_regs *regs) +{ + return is_compat_task(); +} + +#define ARCH_HAS_SYSCALL_MATCH_SYM_NAME +static inline bool arch_syscall_match_sym_name(const char *sym, + const char *name) +{ + /* + * Since all syscall functions have __riscv_ prefix, we must skip it. + * However, as we described above, we decided to ignore compat + * syscalls, so we don't care about __riscv_compat_ prefix here. + */ + return !strcmp(sym + 8, name); +} + struct dyn_arch_ftrace { }; #endif
ftrace creates entries for each syscall in the tracefs but has failed since commit 08d0ce30e0e4 ("riscv: Implement syscall wrappers") which prefixes all riscv syscalls with __riscv_. So fix this by implementing arch_syscall_match_sym_name() which allows us to ignore this prefix. And also ignore compat syscalls like x86/arm64 by implementing arch_trace_is_compat_syscall(). Fixes: 08d0ce30e0e4 ("riscv: Implement syscall wrappers") Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> --- arch/riscv/include/asm/ftrace.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)