* [PATCH v2 0/2] ftrace: Make ftrace_regs abstract and consolidate code
@ 2024-10-08 23:05 Steven Rostedt
2024-10-08 23:05 ` [PATCH v2 1/2] ftrace: Make ftrace_regs abstract from direct use Steven Rostedt
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Steven Rostedt @ 2024-10-08 23:05 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel, linux-arm-kernel, loongarch,
linuxppc-dev, linux-riscv, linux-s390, linux-arch@vger.kernel.org,
x86@kernel.org
Cc: Masami Hiramatsu, Mathieu Desnoyers, Mark Rutland,
Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Madhavan Srinivasan, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen
This is based on:
https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git/
ftrace/for-next
ftrace_regs was created to hold registers that store information to save
function parameters, return value and stack. Since it is a subset of
pt_regs, it should only be used by its accessor functions. But because
pt_regs can easily be taken from ftrace_regs (on most archs), it is
tempting to use it directly. But when running on other architectures, it
may fail to build or worse, build but crash the kernel!
Instead, make struct ftrace_regs an empty structure and have the
architectures define __arch_ftrace_regs and all the accessor functions
will typecast to it to get to the actual fields. This will help avoid
usage of ftrace_regs directly.
I again compiled all the affected architectures (except for 32bit ppc).
I got s390 built when disabling bcachefs.
Changes since v1: https://lore.kernel.org/all/20241007204743.41314f1d@gandalf.local.home/
- Moved the non ftrace args code from asm-generic/ftrace.h to linux/ftrace.h
those archs have their own asm/ftrace.h and are not using asm-generic.
The default has to be in linux/ftrace.h
- simplified arch_ftrace_get_regs() and made it a static inline function
- Added a second patch that consolidates a lot of the duplicate code
when an architecture has pt_regs embedded in the ftrace_regs.
Steven Rostedt (2):
ftrace: Make ftrace_regs abstract from direct use
ftrace: Consolidate ftrace_regs accessor functions for archs using pt_regs
----
arch/arm64/include/asm/ftrace.h | 21 +++++++++--------
arch/arm64/kernel/asm-offsets.c | 22 +++++++++---------
arch/arm64/kernel/ftrace.c | 10 ++++----
arch/loongarch/include/asm/ftrace.h | 29 ++++--------------------
arch/loongarch/kernel/ftrace_dyn.c | 2 +-
arch/powerpc/include/asm/ftrace.h | 27 +++-------------------
arch/powerpc/kernel/trace/ftrace.c | 4 ++--
arch/powerpc/kernel/trace/ftrace_64_pg.c | 2 +-
arch/riscv/include/asm/ftrace.h | 22 ++++++++++--------
arch/riscv/kernel/asm-offsets.c | 28 +++++++++++------------
arch/riscv/kernel/ftrace.c | 2 +-
arch/s390/include/asm/ftrace.h | 29 ++++--------------------
arch/s390/kernel/asm-offsets.c | 4 ++--
arch/s390/kernel/ftrace.c | 2 +-
arch/s390/lib/test_unwind.c | 4 ++--
arch/x86/include/asm/ftrace.h | 30 ++++++------------------
arch/x86/kernel/ftrace.c | 2 +-
include/linux/ftrace.h | 39 +++++++++++++++-----------------
include/linux/ftrace_regs.h | 36 +++++++++++++++++++++++++++++
kernel/trace/ftrace.c | 2 +-
20 files changed, 139 insertions(+), 178 deletions(-)
create mode 100644 include/linux/ftrace_regs.h
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v2 1/2] ftrace: Make ftrace_regs abstract from direct use 2024-10-08 23:05 [PATCH v2 0/2] ftrace: Make ftrace_regs abstract and consolidate code Steven Rostedt @ 2024-10-08 23:05 ` Steven Rostedt 2024-10-09 5:18 ` Masami Hiramatsu 2024-10-09 9:36 ` Heiko Carstens 2024-10-08 23:05 ` [PATCH v2 2/2] ftrace: Consolidate ftrace_regs accessor functions for archs using pt_regs Steven Rostedt ` (2 subsequent siblings) 3 siblings, 2 replies; 11+ messages in thread From: Steven Rostedt @ 2024-10-08 23:05 UTC (permalink / raw) To: linux-kernel, linux-trace-kernel, linux-arm-kernel, loongarch, linuxppc-dev, linux-riscv, linux-s390, linux-arch@vger.kernel.org, x86@kernel.org Cc: Masami Hiramatsu, Mathieu Desnoyers, Mark Rutland, Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui, Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao, Madhavan Srinivasan, Paul Walmsley, Palmer Dabbelt, Albert Ou, Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Christian Borntraeger, Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen From: Steven Rostedt <rostedt@goodmis.org> ftrace_regs was created to hold registers that store information to save function parameters, return value and stack. Since it is a subset of pt_regs, it should only be used by its accessor functions. But because pt_regs can easily be taken from ftrace_regs (on most archs), it is tempting to use it directly. But when running on other architectures, it may fail to build or worse, build but crash the kernel! Instead, make struct ftrace_regs an empty structure and have the architectures define __arch_ftrace_regs and all the accessor functions will typecast to it to get to the actual fields. This will help avoid usage of ftrace_regs directly. Link: https://lore.kernel.org/all/20241007171027.629bdafd@gandalf.local.home/ Acked-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> --- Changes since v1: https://lore.kernel.org/20241007204743.41314f1d@gandalf.local.home - Moved the non ftrace args code from asm-generic/ftrace.h to linux/ftrace.h those archs have their own asm/ftrace.h and are not using asm-generic. The default has to be in linux/ftrace.h - simplified arch_ftrace_get_regs() and made it a static inline function arch/arm64/include/asm/ftrace.h | 20 +++++++++-------- arch/arm64/kernel/asm-offsets.c | 22 +++++++++---------- arch/arm64/kernel/ftrace.c | 10 ++++----- arch/loongarch/include/asm/ftrace.h | 22 ++++++++++--------- arch/loongarch/kernel/ftrace_dyn.c | 2 +- arch/powerpc/include/asm/ftrace.h | 21 ++++++++++-------- arch/powerpc/kernel/trace/ftrace.c | 4 ++-- arch/powerpc/kernel/trace/ftrace_64_pg.c | 2 +- arch/riscv/include/asm/ftrace.h | 21 ++++++++++-------- arch/riscv/kernel/asm-offsets.c | 28 ++++++++++++------------ arch/riscv/kernel/ftrace.c | 2 +- arch/s390/include/asm/ftrace.h | 23 ++++++++++--------- arch/s390/kernel/asm-offsets.c | 4 ++-- arch/s390/kernel/ftrace.c | 2 +- arch/s390/lib/test_unwind.c | 4 ++-- arch/x86/include/asm/ftrace.h | 25 +++++++++++---------- arch/x86/kernel/ftrace.c | 2 +- include/linux/ftrace.h | 21 +++++++++++++++--- kernel/trace/ftrace.c | 2 +- 19 files changed, 134 insertions(+), 103 deletions(-) diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h index dc9cf0bd2a4c..bbb69c7751b9 100644 --- a/arch/arm64/include/asm/ftrace.h +++ b/arch/arm64/include/asm/ftrace.h @@ -56,6 +56,8 @@ unsigned long ftrace_call_adjust(unsigned long addr); #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS struct dyn_ftrace; struct ftrace_ops; +struct ftrace_regs; +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) #define arch_ftrace_get_regs(regs) NULL @@ -63,7 +65,7 @@ struct ftrace_ops; * Note: sizeof(struct ftrace_regs) must be a multiple of 16 to ensure correct * stack alignment */ -struct ftrace_regs { +struct __arch_ftrace_regs { /* x0 - x8 */ unsigned long regs[9]; @@ -83,47 +85,47 @@ struct ftrace_regs { static __always_inline unsigned long ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs) { - return fregs->pc; + return arch_ftrace_regs(fregs)->pc; } static __always_inline void ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long pc) { - fregs->pc = pc; + arch_ftrace_regs(fregs)->pc = pc; } static __always_inline unsigned long ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs) { - return fregs->sp; + return arch_ftrace_regs(fregs)->sp; } static __always_inline unsigned long ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n) { if (n < 8) - return fregs->regs[n]; + return arch_ftrace_regs(fregs)->regs[n]; return 0; } static __always_inline unsigned long ftrace_regs_get_return_value(const struct ftrace_regs *fregs) { - return fregs->regs[0]; + return arch_ftrace_regs(fregs)->regs[0]; } static __always_inline void ftrace_regs_set_return_value(struct ftrace_regs *fregs, unsigned long ret) { - fregs->regs[0] = ret; + arch_ftrace_regs(fregs)->regs[0] = ret; } static __always_inline void ftrace_override_function_with_return(struct ftrace_regs *fregs) { - fregs->pc = fregs->lr; + arch_ftrace_regs(fregs)->pc = arch_ftrace_regs(fregs)->lr; } int ftrace_regs_query_register_offset(const char *name); @@ -143,7 +145,7 @@ static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, * The ftrace trampoline will return to this address instead of the * instrumented function. */ - fregs->direct_tramp = addr; + arch_ftrace_regs(fregs)->direct_tramp = addr; } #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */ diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c index 27de1dddb0ab..a5de57f68219 100644 --- a/arch/arm64/kernel/asm-offsets.c +++ b/arch/arm64/kernel/asm-offsets.c @@ -84,19 +84,19 @@ int main(void) DEFINE(PT_REGS_SIZE, sizeof(struct pt_regs)); BLANK(); #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS - DEFINE(FREGS_X0, offsetof(struct ftrace_regs, regs[0])); - DEFINE(FREGS_X2, offsetof(struct ftrace_regs, regs[2])); - DEFINE(FREGS_X4, offsetof(struct ftrace_regs, regs[4])); - DEFINE(FREGS_X6, offsetof(struct ftrace_regs, regs[6])); - DEFINE(FREGS_X8, offsetof(struct ftrace_regs, regs[8])); - DEFINE(FREGS_FP, offsetof(struct ftrace_regs, fp)); - DEFINE(FREGS_LR, offsetof(struct ftrace_regs, lr)); - DEFINE(FREGS_SP, offsetof(struct ftrace_regs, sp)); - DEFINE(FREGS_PC, offsetof(struct ftrace_regs, pc)); + DEFINE(FREGS_X0, offsetof(struct __arch_ftrace_regs, regs[0])); + DEFINE(FREGS_X2, offsetof(struct __arch_ftrace_regs, regs[2])); + DEFINE(FREGS_X4, offsetof(struct __arch_ftrace_regs, regs[4])); + DEFINE(FREGS_X6, offsetof(struct __arch_ftrace_regs, regs[6])); + DEFINE(FREGS_X8, offsetof(struct __arch_ftrace_regs, regs[8])); + DEFINE(FREGS_FP, offsetof(struct __arch_ftrace_regs, fp)); + DEFINE(FREGS_LR, offsetof(struct __arch_ftrace_regs, lr)); + DEFINE(FREGS_SP, offsetof(struct __arch_ftrace_regs, sp)); + DEFINE(FREGS_PC, offsetof(struct __arch_ftrace_regs, pc)); #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS - DEFINE(FREGS_DIRECT_TRAMP, offsetof(struct ftrace_regs, direct_tramp)); + DEFINE(FREGS_DIRECT_TRAMP, offsetof(struct __arch_ftrace_regs, direct_tramp)); #endif - DEFINE(FREGS_SIZE, sizeof(struct ftrace_regs)); + DEFINE(FREGS_SIZE, sizeof(struct __arch_ftrace_regs)); BLANK(); #endif #ifdef CONFIG_COMPAT diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c index a650f5e11fc5..b2d947175cbe 100644 --- a/arch/arm64/kernel/ftrace.c +++ b/arch/arm64/kernel/ftrace.c @@ -23,10 +23,10 @@ struct fregs_offset { int offset; }; -#define FREGS_OFFSET(n, field) \ -{ \ - .name = n, \ - .offset = offsetof(struct ftrace_regs, field), \ +#define FREGS_OFFSET(n, field) \ +{ \ + .name = n, \ + .offset = offsetof(struct __arch_ftrace_regs, field), \ } static const struct fregs_offset fregs_offsets[] = { @@ -481,7 +481,7 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent, void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op, struct ftrace_regs *fregs) { - prepare_ftrace_return(ip, &fregs->lr, fregs->fp); + prepare_ftrace_return(ip, &arch_ftrace_regs(fregs)->lr, arch_ftrace_regs(fregs)->fp); } #else /* diff --git a/arch/loongarch/include/asm/ftrace.h b/arch/loongarch/include/asm/ftrace.h index c0a682808e07..0e15d36ce251 100644 --- a/arch/loongarch/include/asm/ftrace.h +++ b/arch/loongarch/include/asm/ftrace.h @@ -43,38 +43,40 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent); #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS struct ftrace_ops; +struct ftrace_regs; +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) -struct ftrace_regs { +struct __arch_ftrace_regs { struct pt_regs regs; }; static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) { - return &fregs->regs; + return &arch_ftrace_regs(fregs)->regs; } static __always_inline unsigned long ftrace_regs_get_instruction_pointer(struct ftrace_regs *fregs) { - return instruction_pointer(&fregs->regs); + return instruction_pointer(&arch_ftrace_regs(fregs)->regs); } static __always_inline void ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long ip) { - instruction_pointer_set(&fregs->regs, ip); + instruction_pointer_set(&arch_ftrace_regs(fregs)->regs, ip); } #define ftrace_regs_get_argument(fregs, n) \ - regs_get_kernel_argument(&(fregs)->regs, n) + regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n) #define ftrace_regs_get_stack_pointer(fregs) \ - kernel_stack_pointer(&(fregs)->regs) + kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs) #define ftrace_regs_return_value(fregs) \ - regs_return_value(&(fregs)->regs) + regs_return_value(&arch_ftrace_regs(fregs)->regs) #define ftrace_regs_set_return_value(fregs, ret) \ - regs_set_return_value(&(fregs)->regs, ret) + regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret) #define ftrace_override_function_with_return(fregs) \ - override_function_with_return(&(fregs)->regs) + override_function_with_return(&arch_ftrace_regs(fregs)->regs) #define ftrace_regs_query_register_offset(name) \ regs_query_register_offset(name) @@ -90,7 +92,7 @@ __arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr) } #define arch_ftrace_set_direct_caller(fregs, addr) \ - __arch_ftrace_set_direct_caller(&(fregs)->regs, addr) + __arch_ftrace_set_direct_caller(&arch_ftrace_regs(fregs)->regs, addr) #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */ #endif diff --git a/arch/loongarch/kernel/ftrace_dyn.c b/arch/loongarch/kernel/ftrace_dyn.c index bff058317062..18056229e22e 100644 --- a/arch/loongarch/kernel/ftrace_dyn.c +++ b/arch/loongarch/kernel/ftrace_dyn.c @@ -241,7 +241,7 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent) void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op, struct ftrace_regs *fregs) { - struct pt_regs *regs = &fregs->regs; + struct pt_regs *regs = &arch_ftrace_regs(fregs)->regs; unsigned long *parent = (unsigned long *)®s->regs[1]; prepare_ftrace_return(ip, (unsigned long *)parent); diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h index 559560286e6d..e299fd47d201 100644 --- a/arch/powerpc/include/asm/ftrace.h +++ b/arch/powerpc/include/asm/ftrace.h @@ -32,39 +32,42 @@ struct dyn_arch_ftrace { int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec); #define ftrace_init_nop ftrace_init_nop -struct ftrace_regs { +struct ftrace_regs; +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) + +struct __arch_ftrace_regs { struct pt_regs regs; }; static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) { /* We clear regs.msr in ftrace_call */ - return fregs->regs.msr ? &fregs->regs : NULL; + return arch_ftrace_regs(fregs)->regs.msr ? &arch_ftrace_regs(fregs)->regs : NULL; } static __always_inline void ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long ip) { - regs_set_return_ip(&fregs->regs, ip); + regs_set_return_ip(&arch_ftrace_regs(fregs)->regs, ip); } static __always_inline unsigned long ftrace_regs_get_instruction_pointer(struct ftrace_regs *fregs) { - return instruction_pointer(&fregs->regs); + return instruction_pointer(&arch_ftrace_regs(fregs)->regs); } #define ftrace_regs_get_argument(fregs, n) \ - regs_get_kernel_argument(&(fregs)->regs, n) + regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n) #define ftrace_regs_get_stack_pointer(fregs) \ - kernel_stack_pointer(&(fregs)->regs) + kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs) #define ftrace_regs_return_value(fregs) \ - regs_return_value(&(fregs)->regs) + regs_return_value(&arch_ftrace_regs(fregs)->regs) #define ftrace_regs_set_return_value(fregs, ret) \ - regs_set_return_value(&(fregs)->regs, ret) + regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret) #define ftrace_override_function_with_return(fregs) \ - override_function_with_return(&(fregs)->regs) + override_function_with_return(&arch_ftrace_regs(fregs)->regs) #define ftrace_regs_query_register_offset(name) \ regs_query_register_offset(name) diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c index d8d6b4fd9a14..df41f4a7c738 100644 --- a/arch/powerpc/kernel/trace/ftrace.c +++ b/arch/powerpc/kernel/trace/ftrace.c @@ -421,7 +421,7 @@ int __init ftrace_dyn_arch_init(void) void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op, struct ftrace_regs *fregs) { - unsigned long sp = fregs->regs.gpr[1]; + unsigned long sp = arch_ftrace_regs(fregs)->regs.gpr[1]; int bit; if (unlikely(ftrace_graph_is_dead())) @@ -439,6 +439,6 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, ftrace_test_recursion_unlock(bit); out: - fregs->regs.link = parent_ip; + arch_ftrace_regs(fregs)->regs.link = parent_ip; } #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ diff --git a/arch/powerpc/kernel/trace/ftrace_64_pg.c b/arch/powerpc/kernel/trace/ftrace_64_pg.c index 12fab1803bcf..d3c5552e4984 100644 --- a/arch/powerpc/kernel/trace/ftrace_64_pg.c +++ b/arch/powerpc/kernel/trace/ftrace_64_pg.c @@ -829,7 +829,7 @@ __prepare_ftrace_return(unsigned long parent, unsigned long ip, unsigned long sp void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op, struct ftrace_regs *fregs) { - fregs->regs.link = __prepare_ftrace_return(parent_ip, ip, fregs->regs.gpr[1]); + arch_ftrace_regs(fregs)->regs.link = __prepare_ftrace_return(parent_ip, ip, arch_ftrace_regs(fregs)->regs.gpr[1]); } #else unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip, diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h index 2cddd79ff21b..c6bcdff105b5 100644 --- a/arch/riscv/include/asm/ftrace.h +++ b/arch/riscv/include/asm/ftrace.h @@ -126,7 +126,10 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec); #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS #define arch_ftrace_get_regs(regs) NULL struct ftrace_ops; -struct ftrace_regs { +struct ftrace_regs; +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) + +struct __arch_ftrace_regs { unsigned long epc; unsigned long ra; unsigned long sp; @@ -150,42 +153,42 @@ struct ftrace_regs { static __always_inline unsigned long ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs) { - return fregs->epc; + return arch_ftrace_regs(fregs)->epc; } static __always_inline void ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long pc) { - fregs->epc = pc; + arch_ftrace_regs(fregs)->epc = pc; } static __always_inline unsigned long ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs) { - return fregs->sp; + return arch_ftrace_regs(fregs)->sp; } static __always_inline unsigned long ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n) { if (n < 8) - return fregs->args[n]; + return arch_ftrace_regs(fregs)->args[n]; return 0; } static __always_inline unsigned long ftrace_regs_get_return_value(const struct ftrace_regs *fregs) { - return fregs->a0; + return arch_ftrace_regs(fregs)->a0; } static __always_inline void ftrace_regs_set_return_value(struct ftrace_regs *fregs, unsigned long ret) { - fregs->a0 = ret; + arch_ftrace_regs(fregs)->a0 = ret; } static __always_inline void ftrace_override_function_with_return(struct ftrace_regs *fregs) { - fregs->epc = fregs->ra; + arch_ftrace_regs(fregs)->epc = arch_ftrace_regs(fregs)->ra; } int ftrace_regs_query_register_offset(const char *name); @@ -196,7 +199,7 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, unsigned long addr) { - fregs->t1 = addr; + arch_ftrace_regs(fregs)->t1 = addr; } #endif /* CONFIG_DYNAMIC_FTRACE_WITH_ARGS */ diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c index e94180ba432f..f6f5a277ba9d 100644 --- a/arch/riscv/kernel/asm-offsets.c +++ b/arch/riscv/kernel/asm-offsets.c @@ -498,19 +498,19 @@ void asm_offsets(void) OFFSET(STACKFRAME_RA, stackframe, ra); #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS - DEFINE(FREGS_SIZE_ON_STACK, ALIGN(sizeof(struct ftrace_regs), STACK_ALIGN)); - DEFINE(FREGS_EPC, offsetof(struct ftrace_regs, epc)); - DEFINE(FREGS_RA, offsetof(struct ftrace_regs, ra)); - DEFINE(FREGS_SP, offsetof(struct ftrace_regs, sp)); - DEFINE(FREGS_S0, offsetof(struct ftrace_regs, s0)); - DEFINE(FREGS_T1, offsetof(struct ftrace_regs, t1)); - DEFINE(FREGS_A0, offsetof(struct ftrace_regs, a0)); - DEFINE(FREGS_A1, offsetof(struct ftrace_regs, a1)); - DEFINE(FREGS_A2, offsetof(struct ftrace_regs, a2)); - DEFINE(FREGS_A3, offsetof(struct ftrace_regs, a3)); - DEFINE(FREGS_A4, offsetof(struct ftrace_regs, a4)); - DEFINE(FREGS_A5, offsetof(struct ftrace_regs, a5)); - DEFINE(FREGS_A6, offsetof(struct ftrace_regs, a6)); - DEFINE(FREGS_A7, offsetof(struct ftrace_regs, a7)); + DEFINE(FREGS_SIZE_ON_STACK, ALIGN(sizeof(struct __arch_ftrace_regs), STACK_ALIGN)); + DEFINE(FREGS_EPC, offsetof(struct __arch_ftrace_regs, epc)); + DEFINE(FREGS_RA, offsetof(struct __arch_ftrace_regs, ra)); + DEFINE(FREGS_SP, offsetof(struct __arch_ftrace_regs, sp)); + DEFINE(FREGS_S0, offsetof(struct __arch_ftrace_regs, s0)); + DEFINE(FREGS_T1, offsetof(struct __arch_ftrace_regs, t1)); + DEFINE(FREGS_A0, offsetof(struct __arch_ftrace_regs, a0)); + DEFINE(FREGS_A1, offsetof(struct __arch_ftrace_regs, a1)); + DEFINE(FREGS_A2, offsetof(struct __arch_ftrace_regs, a2)); + DEFINE(FREGS_A3, offsetof(struct __arch_ftrace_regs, a3)); + DEFINE(FREGS_A4, offsetof(struct __arch_ftrace_regs, a4)); + DEFINE(FREGS_A5, offsetof(struct __arch_ftrace_regs, a5)); + DEFINE(FREGS_A6, offsetof(struct __arch_ftrace_regs, a6)); + DEFINE(FREGS_A7, offsetof(struct __arch_ftrace_regs, a7)); #endif } diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c index 4b95c574fd04..5081ad886841 100644 --- a/arch/riscv/kernel/ftrace.c +++ b/arch/riscv/kernel/ftrace.c @@ -214,7 +214,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr, void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op, struct ftrace_regs *fregs) { - prepare_ftrace_return(&fregs->ra, ip, fregs->s0); + prepare_ftrace_return(&arch_ftrace_regs(fregs)->ra, ip, arch_ftrace_regs(fregs)->s0); } #else /* CONFIG_DYNAMIC_FTRACE_WITH_ARGS */ extern void ftrace_graph_call(void); diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h index 406746666eb7..1498d0a9c762 100644 --- a/arch/s390/include/asm/ftrace.h +++ b/arch/s390/include/asm/ftrace.h @@ -51,13 +51,16 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr) return addr; } -struct ftrace_regs { +struct ftrace_regs; +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) + +struct __arch_ftrace_regs { struct pt_regs regs; }; static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) { - struct pt_regs *regs = &fregs->regs; + struct pt_regs *regs = &arch_ftrace_regs(fregs)->regs; if (test_pt_regs_flag(regs, PIF_FTRACE_FULL_REGS)) return regs; @@ -84,26 +87,26 @@ static __always_inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph static __always_inline unsigned long ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs) { - return fregs->regs.psw.addr; + return arch_ftrace_regs(fregs)->regs.psw.addr; } static __always_inline void ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long ip) { - fregs->regs.psw.addr = ip; + arch_ftrace_regs(fregs)->regs.psw.addr = ip; } #define ftrace_regs_get_argument(fregs, n) \ - regs_get_kernel_argument(&(fregs)->regs, n) + regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n) #define ftrace_regs_get_stack_pointer(fregs) \ - kernel_stack_pointer(&(fregs)->regs) + kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs) #define ftrace_regs_return_value(fregs) \ - regs_return_value(&(fregs)->regs) + regs_return_value(&arch_ftrace_regs(fregs)->regs) #define ftrace_regs_set_return_value(fregs, ret) \ - regs_set_return_value(&(fregs)->regs, ret) + regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret) #define ftrace_override_function_with_return(fregs) \ - override_function_with_return(&(fregs)->regs) + override_function_with_return(&arch_ftrace_regs(fregs)->regs) #define ftrace_regs_query_register_offset(name) \ regs_query_register_offset(name) @@ -117,7 +120,7 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, */ static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, unsigned long addr) { - struct pt_regs *regs = &fregs->regs; + struct pt_regs *regs = &arch_ftrace_regs(fregs)->regs; regs->orig_gpr2 = addr; } #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */ diff --git a/arch/s390/kernel/asm-offsets.c b/arch/s390/kernel/asm-offsets.c index 5529248d84fb..db9659980175 100644 --- a/arch/s390/kernel/asm-offsets.c +++ b/arch/s390/kernel/asm-offsets.c @@ -184,8 +184,8 @@ int main(void) OFFSET(__FGRAPH_RET_FP, fgraph_ret_regs, fp); DEFINE(__FGRAPH_RET_SIZE, sizeof(struct fgraph_ret_regs)); #endif - OFFSET(__FTRACE_REGS_PT_REGS, ftrace_regs, regs); - DEFINE(__FTRACE_REGS_SIZE, sizeof(struct ftrace_regs)); + OFFSET(__FTRACE_REGS_PT_REGS, __arch_ftrace_regs, regs); + DEFINE(__FTRACE_REGS_SIZE, sizeof(struct __arch_ftrace_regs)); OFFSET(__PCPU_FLAGS, pcpu, flags); return 0; diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c index 0b6e62d1d8b8..51439a71e392 100644 --- a/arch/s390/kernel/ftrace.c +++ b/arch/s390/kernel/ftrace.c @@ -318,7 +318,7 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip, if (bit < 0) return; - kmsan_unpoison_memory(fregs, sizeof(*fregs)); + kmsan_unpoison_memory(fregs, ftrace_regs_size()); regs = ftrace_get_regs(fregs); p = get_kprobe((kprobe_opcode_t *)ip); if (!regs || unlikely(!p) || kprobe_disabled(p)) diff --git a/arch/s390/lib/test_unwind.c b/arch/s390/lib/test_unwind.c index 8b7f981e6f34..6e42100875e7 100644 --- a/arch/s390/lib/test_unwind.c +++ b/arch/s390/lib/test_unwind.c @@ -270,9 +270,9 @@ static void notrace __used test_unwind_ftrace_handler(unsigned long ip, struct ftrace_ops *fops, struct ftrace_regs *fregs) { - struct unwindme *u = (struct unwindme *)fregs->regs.gprs[2]; + struct unwindme *u = (struct unwindme *)arch_ftrace_regs(fregs)->regs.gprs[2]; - u->ret = test_unwind(NULL, (u->flags & UWM_REGS) ? &fregs->regs : NULL, + u->ret = test_unwind(NULL, (u->flags & UWM_REGS) ? &arch_ftrace_regs(fregs)->regs : NULL, (u->flags & UWM_SP) ? u->sp : 0); } diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h index 0152a81d9b4a..87943f7a299b 100644 --- a/arch/x86/include/asm/ftrace.h +++ b/arch/x86/include/asm/ftrace.h @@ -33,7 +33,10 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr) } #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS -struct ftrace_regs { +struct ftrace_regs; +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) + +struct __arch_ftrace_regs { struct pt_regs regs; }; @@ -41,27 +44,27 @@ static __always_inline struct pt_regs * arch_ftrace_get_regs(struct ftrace_regs *fregs) { /* Only when FL_SAVE_REGS is set, cs will be non zero */ - if (!fregs->regs.cs) + if (!arch_ftrace_regs(fregs)->regs.cs) return NULL; - return &fregs->regs; + return &arch_ftrace_regs(fregs)->regs; } #define ftrace_regs_set_instruction_pointer(fregs, _ip) \ - do { (fregs)->regs.ip = (_ip); } while (0) + do { arch_ftrace_regs(fregs)->regs.ip = (_ip); } while (0) #define ftrace_regs_get_instruction_pointer(fregs) \ - ((fregs)->regs.ip) + arch_ftrace_regs(fregs)->regs.ip) #define ftrace_regs_get_argument(fregs, n) \ - regs_get_kernel_argument(&(fregs)->regs, n) + regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n) #define ftrace_regs_get_stack_pointer(fregs) \ - kernel_stack_pointer(&(fregs)->regs) + kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs) #define ftrace_regs_return_value(fregs) \ - regs_return_value(&(fregs)->regs) + regs_return_value(&arch_ftrace_regs(fregs)->regs) #define ftrace_regs_set_return_value(fregs, ret) \ - regs_set_return_value(&(fregs)->regs, ret) + regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret) #define ftrace_override_function_with_return(fregs) \ - override_function_with_return(&(fregs)->regs) + override_function_with_return(&arch_ftrace_regs(fregs)->regs) #define ftrace_regs_query_register_offset(name) \ regs_query_register_offset(name) @@ -88,7 +91,7 @@ __arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr) regs->orig_ax = addr; } #define arch_ftrace_set_direct_caller(fregs, addr) \ - __arch_ftrace_set_direct_caller(&(fregs)->regs, addr) + __arch_ftrace_set_direct_caller(&arch_ftrace_regs(fregs)->regs, addr) #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */ #ifdef CONFIG_DYNAMIC_FTRACE diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index 8da0e66ca22d..adb09f78edb2 100644 --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -647,7 +647,7 @@ void prepare_ftrace_return(unsigned long ip, unsigned long *parent, void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op, struct ftrace_regs *fregs) { - struct pt_regs *regs = &fregs->regs; + struct pt_regs *regs = &arch_ftrace_regs(fregs)->regs; unsigned long *stack = (unsigned long *)kernel_stack_pointer(regs); prepare_ftrace_return(ip, (unsigned long *)stack, 0); diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index 2ac3b3b53cd0..f7d4f152f84d 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -115,8 +115,6 @@ static inline int ftrace_mod_get_kallsym(unsigned int symnum, unsigned long *val extern int ftrace_enabled; -#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS - /** * ftrace_regs - ftrace partial/optimal register set * @@ -142,11 +140,28 @@ extern int ftrace_enabled; * * NOTE: user *must not* access regs directly, only do it via APIs, because * the member can be changed according to the architecture. + * This is why the structure is empty here, so that nothing accesses + * the ftrace_regs directly. */ struct ftrace_regs { + /* Nothing to see here, use the accessor functions! */ +}; + +#define ftrace_regs_size() sizeof(struct __arch_ftrace_regs) + +#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS + +struct __arch_ftrace_regs { struct pt_regs regs; }; -#define arch_ftrace_get_regs(fregs) (&(fregs)->regs) + +struct ftrace_regs; +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) + +static inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) +{ + return &arch_ftrace_regs(fregs)->regs; +} /* * ftrace_regs_set_instruction_pointer() is to be defined by the architecture diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 5d87dac83b80..1e6251174530 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -7944,7 +7944,7 @@ __ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip, void arch_ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op, struct ftrace_regs *fregs) { - kmsan_unpoison_memory(fregs, sizeof(*fregs)); + kmsan_unpoison_memory(fregs, ftrace_regs_size()); __ftrace_ops_list_func(ip, parent_ip, NULL, fregs); } #else -- 2.45.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] ftrace: Make ftrace_regs abstract from direct use 2024-10-08 23:05 ` [PATCH v2 1/2] ftrace: Make ftrace_regs abstract from direct use Steven Rostedt @ 2024-10-09 5:18 ` Masami Hiramatsu 2024-10-09 9:36 ` Heiko Carstens 1 sibling, 0 replies; 11+ messages in thread From: Masami Hiramatsu @ 2024-10-09 5:18 UTC (permalink / raw) To: Steven Rostedt Cc: linux-kernel, linux-trace-kernel, linux-arm-kernel, loongarch, linuxppc-dev, linux-riscv, linux-s390, linux-arch@vger.kernel.org, x86@kernel.org, Masami Hiramatsu, Mathieu Desnoyers, Mark Rutland, Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui, Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao, Madhavan Srinivasan, Paul Walmsley, Palmer Dabbelt, Albert Ou, Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Christian Borntraeger, Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen On Tue, 08 Oct 2024 19:05:28 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > From: Steven Rostedt <rostedt@goodmis.org> > > ftrace_regs was created to hold registers that store information to save > function parameters, return value and stack. Since it is a subset of > pt_regs, it should only be used by its accessor functions. But because > pt_regs can easily be taken from ftrace_regs (on most archs), it is > tempting to use it directly. But when running on other architectures, it > may fail to build or worse, build but crash the kernel! > > Instead, make struct ftrace_regs an empty structure and have the > architectures define __arch_ftrace_regs and all the accessor functions > will typecast to it to get to the actual fields. This will help avoid > usage of ftrace_regs directly. > > Link: https://lore.kernel.org/all/20241007171027.629bdafd@gandalf.local.home/ > This looks good to me. Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> For x86 and generic part. Thank you, > Acked-by: Catalin Marinas <catalin.marinas@arm.com> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > Changes since v1: https://lore.kernel.org/20241007204743.41314f1d@gandalf.local.home > > - Moved the non ftrace args code from asm-generic/ftrace.h to linux/ftrace.h > those archs have their own asm/ftrace.h and are not using asm-generic. > The default has to be in linux/ftrace.h > > - simplified arch_ftrace_get_regs() and made it a static inline function > > arch/arm64/include/asm/ftrace.h | 20 +++++++++-------- > arch/arm64/kernel/asm-offsets.c | 22 +++++++++---------- > arch/arm64/kernel/ftrace.c | 10 ++++----- > arch/loongarch/include/asm/ftrace.h | 22 ++++++++++--------- > arch/loongarch/kernel/ftrace_dyn.c | 2 +- > arch/powerpc/include/asm/ftrace.h | 21 ++++++++++-------- > arch/powerpc/kernel/trace/ftrace.c | 4 ++-- > arch/powerpc/kernel/trace/ftrace_64_pg.c | 2 +- > arch/riscv/include/asm/ftrace.h | 21 ++++++++++-------- > arch/riscv/kernel/asm-offsets.c | 28 ++++++++++++------------ > arch/riscv/kernel/ftrace.c | 2 +- > arch/s390/include/asm/ftrace.h | 23 ++++++++++--------- > arch/s390/kernel/asm-offsets.c | 4 ++-- > arch/s390/kernel/ftrace.c | 2 +- > arch/s390/lib/test_unwind.c | 4 ++-- > arch/x86/include/asm/ftrace.h | 25 +++++++++++---------- > arch/x86/kernel/ftrace.c | 2 +- > include/linux/ftrace.h | 21 +++++++++++++++--- > kernel/trace/ftrace.c | 2 +- > 19 files changed, 134 insertions(+), 103 deletions(-) > > diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h > index dc9cf0bd2a4c..bbb69c7751b9 100644 > --- a/arch/arm64/include/asm/ftrace.h > +++ b/arch/arm64/include/asm/ftrace.h > @@ -56,6 +56,8 @@ unsigned long ftrace_call_adjust(unsigned long addr); > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS > struct dyn_ftrace; > struct ftrace_ops; > +struct ftrace_regs; > +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > > #define arch_ftrace_get_regs(regs) NULL > > @@ -63,7 +65,7 @@ struct ftrace_ops; > * Note: sizeof(struct ftrace_regs) must be a multiple of 16 to ensure correct > * stack alignment > */ > -struct ftrace_regs { > +struct __arch_ftrace_regs { > /* x0 - x8 */ > unsigned long regs[9]; > > @@ -83,47 +85,47 @@ struct ftrace_regs { > static __always_inline unsigned long > ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs) > { > - return fregs->pc; > + return arch_ftrace_regs(fregs)->pc; > } > > static __always_inline void > ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, > unsigned long pc) > { > - fregs->pc = pc; > + arch_ftrace_regs(fregs)->pc = pc; > } > > static __always_inline unsigned long > ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs) > { > - return fregs->sp; > + return arch_ftrace_regs(fregs)->sp; > } > > static __always_inline unsigned long > ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n) > { > if (n < 8) > - return fregs->regs[n]; > + return arch_ftrace_regs(fregs)->regs[n]; > return 0; > } > > static __always_inline unsigned long > ftrace_regs_get_return_value(const struct ftrace_regs *fregs) > { > - return fregs->regs[0]; > + return arch_ftrace_regs(fregs)->regs[0]; > } > > static __always_inline void > ftrace_regs_set_return_value(struct ftrace_regs *fregs, > unsigned long ret) > { > - fregs->regs[0] = ret; > + arch_ftrace_regs(fregs)->regs[0] = ret; > } > > static __always_inline void > ftrace_override_function_with_return(struct ftrace_regs *fregs) > { > - fregs->pc = fregs->lr; > + arch_ftrace_regs(fregs)->pc = arch_ftrace_regs(fregs)->lr; > } > > int ftrace_regs_query_register_offset(const char *name); > @@ -143,7 +145,7 @@ static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, > * The ftrace trampoline will return to this address instead of the > * instrumented function. > */ > - fregs->direct_tramp = addr; > + arch_ftrace_regs(fregs)->direct_tramp = addr; > } > #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */ > > diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c > index 27de1dddb0ab..a5de57f68219 100644 > --- a/arch/arm64/kernel/asm-offsets.c > +++ b/arch/arm64/kernel/asm-offsets.c > @@ -84,19 +84,19 @@ int main(void) > DEFINE(PT_REGS_SIZE, sizeof(struct pt_regs)); > BLANK(); > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS > - DEFINE(FREGS_X0, offsetof(struct ftrace_regs, regs[0])); > - DEFINE(FREGS_X2, offsetof(struct ftrace_regs, regs[2])); > - DEFINE(FREGS_X4, offsetof(struct ftrace_regs, regs[4])); > - DEFINE(FREGS_X6, offsetof(struct ftrace_regs, regs[6])); > - DEFINE(FREGS_X8, offsetof(struct ftrace_regs, regs[8])); > - DEFINE(FREGS_FP, offsetof(struct ftrace_regs, fp)); > - DEFINE(FREGS_LR, offsetof(struct ftrace_regs, lr)); > - DEFINE(FREGS_SP, offsetof(struct ftrace_regs, sp)); > - DEFINE(FREGS_PC, offsetof(struct ftrace_regs, pc)); > + DEFINE(FREGS_X0, offsetof(struct __arch_ftrace_regs, regs[0])); > + DEFINE(FREGS_X2, offsetof(struct __arch_ftrace_regs, regs[2])); > + DEFINE(FREGS_X4, offsetof(struct __arch_ftrace_regs, regs[4])); > + DEFINE(FREGS_X6, offsetof(struct __arch_ftrace_regs, regs[6])); > + DEFINE(FREGS_X8, offsetof(struct __arch_ftrace_regs, regs[8])); > + DEFINE(FREGS_FP, offsetof(struct __arch_ftrace_regs, fp)); > + DEFINE(FREGS_LR, offsetof(struct __arch_ftrace_regs, lr)); > + DEFINE(FREGS_SP, offsetof(struct __arch_ftrace_regs, sp)); > + DEFINE(FREGS_PC, offsetof(struct __arch_ftrace_regs, pc)); > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > - DEFINE(FREGS_DIRECT_TRAMP, offsetof(struct ftrace_regs, direct_tramp)); > + DEFINE(FREGS_DIRECT_TRAMP, offsetof(struct __arch_ftrace_regs, direct_tramp)); > #endif > - DEFINE(FREGS_SIZE, sizeof(struct ftrace_regs)); > + DEFINE(FREGS_SIZE, sizeof(struct __arch_ftrace_regs)); > BLANK(); > #endif > #ifdef CONFIG_COMPAT > diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c > index a650f5e11fc5..b2d947175cbe 100644 > --- a/arch/arm64/kernel/ftrace.c > +++ b/arch/arm64/kernel/ftrace.c > @@ -23,10 +23,10 @@ struct fregs_offset { > int offset; > }; > > -#define FREGS_OFFSET(n, field) \ > -{ \ > - .name = n, \ > - .offset = offsetof(struct ftrace_regs, field), \ > +#define FREGS_OFFSET(n, field) \ > +{ \ > + .name = n, \ > + .offset = offsetof(struct __arch_ftrace_regs, field), \ > } > > static const struct fregs_offset fregs_offsets[] = { > @@ -481,7 +481,7 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent, > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct ftrace_regs *fregs) > { > - prepare_ftrace_return(ip, &fregs->lr, fregs->fp); > + prepare_ftrace_return(ip, &arch_ftrace_regs(fregs)->lr, arch_ftrace_regs(fregs)->fp); > } > #else > /* > diff --git a/arch/loongarch/include/asm/ftrace.h b/arch/loongarch/include/asm/ftrace.h > index c0a682808e07..0e15d36ce251 100644 > --- a/arch/loongarch/include/asm/ftrace.h > +++ b/arch/loongarch/include/asm/ftrace.h > @@ -43,38 +43,40 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent); > > #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS > struct ftrace_ops; > +struct ftrace_regs; > +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > > -struct ftrace_regs { > +struct __arch_ftrace_regs { > struct pt_regs regs; > }; > > static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) > { > - return &fregs->regs; > + return &arch_ftrace_regs(fregs)->regs; > } > > static __always_inline unsigned long > ftrace_regs_get_instruction_pointer(struct ftrace_regs *fregs) > { > - return instruction_pointer(&fregs->regs); > + return instruction_pointer(&arch_ftrace_regs(fregs)->regs); > } > > static __always_inline void > ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long ip) > { > - instruction_pointer_set(&fregs->regs, ip); > + instruction_pointer_set(&arch_ftrace_regs(fregs)->regs, ip); > } > > #define ftrace_regs_get_argument(fregs, n) \ > - regs_get_kernel_argument(&(fregs)->regs, n) > + regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n) > #define ftrace_regs_get_stack_pointer(fregs) \ > - kernel_stack_pointer(&(fregs)->regs) > + kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs) > #define ftrace_regs_return_value(fregs) \ > - regs_return_value(&(fregs)->regs) > + regs_return_value(&arch_ftrace_regs(fregs)->regs) > #define ftrace_regs_set_return_value(fregs, ret) \ > - regs_set_return_value(&(fregs)->regs, ret) > + regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret) > #define ftrace_override_function_with_return(fregs) \ > - override_function_with_return(&(fregs)->regs) > + override_function_with_return(&arch_ftrace_regs(fregs)->regs) > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > > @@ -90,7 +92,7 @@ __arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr) > } > > #define arch_ftrace_set_direct_caller(fregs, addr) \ > - __arch_ftrace_set_direct_caller(&(fregs)->regs, addr) > + __arch_ftrace_set_direct_caller(&arch_ftrace_regs(fregs)->regs, addr) > #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */ > > #endif > diff --git a/arch/loongarch/kernel/ftrace_dyn.c b/arch/loongarch/kernel/ftrace_dyn.c > index bff058317062..18056229e22e 100644 > --- a/arch/loongarch/kernel/ftrace_dyn.c > +++ b/arch/loongarch/kernel/ftrace_dyn.c > @@ -241,7 +241,7 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent) > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct ftrace_regs *fregs) > { > - struct pt_regs *regs = &fregs->regs; > + struct pt_regs *regs = &arch_ftrace_regs(fregs)->regs; > unsigned long *parent = (unsigned long *)®s->regs[1]; > > prepare_ftrace_return(ip, (unsigned long *)parent); > diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h > index 559560286e6d..e299fd47d201 100644 > --- a/arch/powerpc/include/asm/ftrace.h > +++ b/arch/powerpc/include/asm/ftrace.h > @@ -32,39 +32,42 @@ struct dyn_arch_ftrace { > int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec); > #define ftrace_init_nop ftrace_init_nop > > -struct ftrace_regs { > +struct ftrace_regs; > +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > + > +struct __arch_ftrace_regs { > struct pt_regs regs; > }; > > static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) > { > /* We clear regs.msr in ftrace_call */ > - return fregs->regs.msr ? &fregs->regs : NULL; > + return arch_ftrace_regs(fregs)->regs.msr ? &arch_ftrace_regs(fregs)->regs : NULL; > } > > static __always_inline void > ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, > unsigned long ip) > { > - regs_set_return_ip(&fregs->regs, ip); > + regs_set_return_ip(&arch_ftrace_regs(fregs)->regs, ip); > } > > static __always_inline unsigned long > ftrace_regs_get_instruction_pointer(struct ftrace_regs *fregs) > { > - return instruction_pointer(&fregs->regs); > + return instruction_pointer(&arch_ftrace_regs(fregs)->regs); > } > > #define ftrace_regs_get_argument(fregs, n) \ > - regs_get_kernel_argument(&(fregs)->regs, n) > + regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n) > #define ftrace_regs_get_stack_pointer(fregs) \ > - kernel_stack_pointer(&(fregs)->regs) > + kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs) > #define ftrace_regs_return_value(fregs) \ > - regs_return_value(&(fregs)->regs) > + regs_return_value(&arch_ftrace_regs(fregs)->regs) > #define ftrace_regs_set_return_value(fregs, ret) \ > - regs_set_return_value(&(fregs)->regs, ret) > + regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret) > #define ftrace_override_function_with_return(fregs) \ > - override_function_with_return(&(fregs)->regs) > + override_function_with_return(&arch_ftrace_regs(fregs)->regs) > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > > diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c > index d8d6b4fd9a14..df41f4a7c738 100644 > --- a/arch/powerpc/kernel/trace/ftrace.c > +++ b/arch/powerpc/kernel/trace/ftrace.c > @@ -421,7 +421,7 @@ int __init ftrace_dyn_arch_init(void) > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct ftrace_regs *fregs) > { > - unsigned long sp = fregs->regs.gpr[1]; > + unsigned long sp = arch_ftrace_regs(fregs)->regs.gpr[1]; > int bit; > > if (unlikely(ftrace_graph_is_dead())) > @@ -439,6 +439,6 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > > ftrace_test_recursion_unlock(bit); > out: > - fregs->regs.link = parent_ip; > + arch_ftrace_regs(fregs)->regs.link = parent_ip; > } > #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ > diff --git a/arch/powerpc/kernel/trace/ftrace_64_pg.c b/arch/powerpc/kernel/trace/ftrace_64_pg.c > index 12fab1803bcf..d3c5552e4984 100644 > --- a/arch/powerpc/kernel/trace/ftrace_64_pg.c > +++ b/arch/powerpc/kernel/trace/ftrace_64_pg.c > @@ -829,7 +829,7 @@ __prepare_ftrace_return(unsigned long parent, unsigned long ip, unsigned long sp > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct ftrace_regs *fregs) > { > - fregs->regs.link = __prepare_ftrace_return(parent_ip, ip, fregs->regs.gpr[1]); > + arch_ftrace_regs(fregs)->regs.link = __prepare_ftrace_return(parent_ip, ip, arch_ftrace_regs(fregs)->regs.gpr[1]); > } > #else > unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip, > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > index 2cddd79ff21b..c6bcdff105b5 100644 > --- a/arch/riscv/include/asm/ftrace.h > +++ b/arch/riscv/include/asm/ftrace.h > @@ -126,7 +126,10 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec); > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS > #define arch_ftrace_get_regs(regs) NULL > struct ftrace_ops; > -struct ftrace_regs { > +struct ftrace_regs; > +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > + > +struct __arch_ftrace_regs { > unsigned long epc; > unsigned long ra; > unsigned long sp; > @@ -150,42 +153,42 @@ struct ftrace_regs { > static __always_inline unsigned long ftrace_regs_get_instruction_pointer(const struct ftrace_regs > *fregs) > { > - return fregs->epc; > + return arch_ftrace_regs(fregs)->epc; > } > > static __always_inline void ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, > unsigned long pc) > { > - fregs->epc = pc; > + arch_ftrace_regs(fregs)->epc = pc; > } > > static __always_inline unsigned long ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs) > { > - return fregs->sp; > + return arch_ftrace_regs(fregs)->sp; > } > > static __always_inline unsigned long ftrace_regs_get_argument(struct ftrace_regs *fregs, > unsigned int n) > { > if (n < 8) > - return fregs->args[n]; > + return arch_ftrace_regs(fregs)->args[n]; > return 0; > } > > static __always_inline unsigned long ftrace_regs_get_return_value(const struct ftrace_regs *fregs) > { > - return fregs->a0; > + return arch_ftrace_regs(fregs)->a0; > } > > static __always_inline void ftrace_regs_set_return_value(struct ftrace_regs *fregs, > unsigned long ret) > { > - fregs->a0 = ret; > + arch_ftrace_regs(fregs)->a0 = ret; > } > > static __always_inline void ftrace_override_function_with_return(struct ftrace_regs *fregs) > { > - fregs->epc = fregs->ra; > + arch_ftrace_regs(fregs)->epc = arch_ftrace_regs(fregs)->ra; > } > > int ftrace_regs_query_register_offset(const char *name); > @@ -196,7 +199,7 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > > static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, unsigned long addr) > { > - fregs->t1 = addr; > + arch_ftrace_regs(fregs)->t1 = addr; > } > #endif /* CONFIG_DYNAMIC_FTRACE_WITH_ARGS */ > > diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c > index e94180ba432f..f6f5a277ba9d 100644 > --- a/arch/riscv/kernel/asm-offsets.c > +++ b/arch/riscv/kernel/asm-offsets.c > @@ -498,19 +498,19 @@ void asm_offsets(void) > OFFSET(STACKFRAME_RA, stackframe, ra); > > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS > - DEFINE(FREGS_SIZE_ON_STACK, ALIGN(sizeof(struct ftrace_regs), STACK_ALIGN)); > - DEFINE(FREGS_EPC, offsetof(struct ftrace_regs, epc)); > - DEFINE(FREGS_RA, offsetof(struct ftrace_regs, ra)); > - DEFINE(FREGS_SP, offsetof(struct ftrace_regs, sp)); > - DEFINE(FREGS_S0, offsetof(struct ftrace_regs, s0)); > - DEFINE(FREGS_T1, offsetof(struct ftrace_regs, t1)); > - DEFINE(FREGS_A0, offsetof(struct ftrace_regs, a0)); > - DEFINE(FREGS_A1, offsetof(struct ftrace_regs, a1)); > - DEFINE(FREGS_A2, offsetof(struct ftrace_regs, a2)); > - DEFINE(FREGS_A3, offsetof(struct ftrace_regs, a3)); > - DEFINE(FREGS_A4, offsetof(struct ftrace_regs, a4)); > - DEFINE(FREGS_A5, offsetof(struct ftrace_regs, a5)); > - DEFINE(FREGS_A6, offsetof(struct ftrace_regs, a6)); > - DEFINE(FREGS_A7, offsetof(struct ftrace_regs, a7)); > + DEFINE(FREGS_SIZE_ON_STACK, ALIGN(sizeof(struct __arch_ftrace_regs), STACK_ALIGN)); > + DEFINE(FREGS_EPC, offsetof(struct __arch_ftrace_regs, epc)); > + DEFINE(FREGS_RA, offsetof(struct __arch_ftrace_regs, ra)); > + DEFINE(FREGS_SP, offsetof(struct __arch_ftrace_regs, sp)); > + DEFINE(FREGS_S0, offsetof(struct __arch_ftrace_regs, s0)); > + DEFINE(FREGS_T1, offsetof(struct __arch_ftrace_regs, t1)); > + DEFINE(FREGS_A0, offsetof(struct __arch_ftrace_regs, a0)); > + DEFINE(FREGS_A1, offsetof(struct __arch_ftrace_regs, a1)); > + DEFINE(FREGS_A2, offsetof(struct __arch_ftrace_regs, a2)); > + DEFINE(FREGS_A3, offsetof(struct __arch_ftrace_regs, a3)); > + DEFINE(FREGS_A4, offsetof(struct __arch_ftrace_regs, a4)); > + DEFINE(FREGS_A5, offsetof(struct __arch_ftrace_regs, a5)); > + DEFINE(FREGS_A6, offsetof(struct __arch_ftrace_regs, a6)); > + DEFINE(FREGS_A7, offsetof(struct __arch_ftrace_regs, a7)); > #endif > } > diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c > index 4b95c574fd04..5081ad886841 100644 > --- a/arch/riscv/kernel/ftrace.c > +++ b/arch/riscv/kernel/ftrace.c > @@ -214,7 +214,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr, > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct ftrace_regs *fregs) > { > - prepare_ftrace_return(&fregs->ra, ip, fregs->s0); > + prepare_ftrace_return(&arch_ftrace_regs(fregs)->ra, ip, arch_ftrace_regs(fregs)->s0); > } > #else /* CONFIG_DYNAMIC_FTRACE_WITH_ARGS */ > extern void ftrace_graph_call(void); > diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h > index 406746666eb7..1498d0a9c762 100644 > --- a/arch/s390/include/asm/ftrace.h > +++ b/arch/s390/include/asm/ftrace.h > @@ -51,13 +51,16 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr) > return addr; > } > > -struct ftrace_regs { > +struct ftrace_regs; > +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > + > +struct __arch_ftrace_regs { > struct pt_regs regs; > }; > > static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) > { > - struct pt_regs *regs = &fregs->regs; > + struct pt_regs *regs = &arch_ftrace_regs(fregs)->regs; > > if (test_pt_regs_flag(regs, PIF_FTRACE_FULL_REGS)) > return regs; > @@ -84,26 +87,26 @@ static __always_inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph > static __always_inline unsigned long > ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs) > { > - return fregs->regs.psw.addr; > + return arch_ftrace_regs(fregs)->regs.psw.addr; > } > > static __always_inline void > ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, > unsigned long ip) > { > - fregs->regs.psw.addr = ip; > + arch_ftrace_regs(fregs)->regs.psw.addr = ip; > } > > #define ftrace_regs_get_argument(fregs, n) \ > - regs_get_kernel_argument(&(fregs)->regs, n) > + regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n) > #define ftrace_regs_get_stack_pointer(fregs) \ > - kernel_stack_pointer(&(fregs)->regs) > + kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs) > #define ftrace_regs_return_value(fregs) \ > - regs_return_value(&(fregs)->regs) > + regs_return_value(&arch_ftrace_regs(fregs)->regs) > #define ftrace_regs_set_return_value(fregs, ret) \ > - regs_set_return_value(&(fregs)->regs, ret) > + regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret) > #define ftrace_override_function_with_return(fregs) \ > - override_function_with_return(&(fregs)->regs) > + override_function_with_return(&arch_ftrace_regs(fregs)->regs) > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > > @@ -117,7 +120,7 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, > */ > static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, unsigned long addr) > { > - struct pt_regs *regs = &fregs->regs; > + struct pt_regs *regs = &arch_ftrace_regs(fregs)->regs; > regs->orig_gpr2 = addr; > } > #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */ > diff --git a/arch/s390/kernel/asm-offsets.c b/arch/s390/kernel/asm-offsets.c > index 5529248d84fb..db9659980175 100644 > --- a/arch/s390/kernel/asm-offsets.c > +++ b/arch/s390/kernel/asm-offsets.c > @@ -184,8 +184,8 @@ int main(void) > OFFSET(__FGRAPH_RET_FP, fgraph_ret_regs, fp); > DEFINE(__FGRAPH_RET_SIZE, sizeof(struct fgraph_ret_regs)); > #endif > - OFFSET(__FTRACE_REGS_PT_REGS, ftrace_regs, regs); > - DEFINE(__FTRACE_REGS_SIZE, sizeof(struct ftrace_regs)); > + OFFSET(__FTRACE_REGS_PT_REGS, __arch_ftrace_regs, regs); > + DEFINE(__FTRACE_REGS_SIZE, sizeof(struct __arch_ftrace_regs)); > > OFFSET(__PCPU_FLAGS, pcpu, flags); > return 0; > diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c > index 0b6e62d1d8b8..51439a71e392 100644 > --- a/arch/s390/kernel/ftrace.c > +++ b/arch/s390/kernel/ftrace.c > @@ -318,7 +318,7 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip, > if (bit < 0) > return; > > - kmsan_unpoison_memory(fregs, sizeof(*fregs)); > + kmsan_unpoison_memory(fregs, ftrace_regs_size()); > regs = ftrace_get_regs(fregs); > p = get_kprobe((kprobe_opcode_t *)ip); > if (!regs || unlikely(!p) || kprobe_disabled(p)) > diff --git a/arch/s390/lib/test_unwind.c b/arch/s390/lib/test_unwind.c > index 8b7f981e6f34..6e42100875e7 100644 > --- a/arch/s390/lib/test_unwind.c > +++ b/arch/s390/lib/test_unwind.c > @@ -270,9 +270,9 @@ static void notrace __used test_unwind_ftrace_handler(unsigned long ip, > struct ftrace_ops *fops, > struct ftrace_regs *fregs) > { > - struct unwindme *u = (struct unwindme *)fregs->regs.gprs[2]; > + struct unwindme *u = (struct unwindme *)arch_ftrace_regs(fregs)->regs.gprs[2]; > > - u->ret = test_unwind(NULL, (u->flags & UWM_REGS) ? &fregs->regs : NULL, > + u->ret = test_unwind(NULL, (u->flags & UWM_REGS) ? &arch_ftrace_regs(fregs)->regs : NULL, > (u->flags & UWM_SP) ? u->sp : 0); > } > > diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h > index 0152a81d9b4a..87943f7a299b 100644 > --- a/arch/x86/include/asm/ftrace.h > +++ b/arch/x86/include/asm/ftrace.h > @@ -33,7 +33,10 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr) > } > > #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS > -struct ftrace_regs { > +struct ftrace_regs; > +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > + > +struct __arch_ftrace_regs { > struct pt_regs regs; > }; > > @@ -41,27 +44,27 @@ static __always_inline struct pt_regs * > arch_ftrace_get_regs(struct ftrace_regs *fregs) > { > /* Only when FL_SAVE_REGS is set, cs will be non zero */ > - if (!fregs->regs.cs) > + if (!arch_ftrace_regs(fregs)->regs.cs) > return NULL; > - return &fregs->regs; > + return &arch_ftrace_regs(fregs)->regs; > } > > #define ftrace_regs_set_instruction_pointer(fregs, _ip) \ > - do { (fregs)->regs.ip = (_ip); } while (0) > + do { arch_ftrace_regs(fregs)->regs.ip = (_ip); } while (0) > > #define ftrace_regs_get_instruction_pointer(fregs) \ > - ((fregs)->regs.ip) > + arch_ftrace_regs(fregs)->regs.ip) > > #define ftrace_regs_get_argument(fregs, n) \ > - regs_get_kernel_argument(&(fregs)->regs, n) > + regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n) > #define ftrace_regs_get_stack_pointer(fregs) \ > - kernel_stack_pointer(&(fregs)->regs) > + kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs) > #define ftrace_regs_return_value(fregs) \ > - regs_return_value(&(fregs)->regs) > + regs_return_value(&arch_ftrace_regs(fregs)->regs) > #define ftrace_regs_set_return_value(fregs, ret) \ > - regs_set_return_value(&(fregs)->regs, ret) > + regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret) > #define ftrace_override_function_with_return(fregs) \ > - override_function_with_return(&(fregs)->regs) > + override_function_with_return(&arch_ftrace_regs(fregs)->regs) > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > > @@ -88,7 +91,7 @@ __arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr) > regs->orig_ax = addr; > } > #define arch_ftrace_set_direct_caller(fregs, addr) \ > - __arch_ftrace_set_direct_caller(&(fregs)->regs, addr) > + __arch_ftrace_set_direct_caller(&arch_ftrace_regs(fregs)->regs, addr) > #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */ > > #ifdef CONFIG_DYNAMIC_FTRACE > diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c > index 8da0e66ca22d..adb09f78edb2 100644 > --- a/arch/x86/kernel/ftrace.c > +++ b/arch/x86/kernel/ftrace.c > @@ -647,7 +647,7 @@ void prepare_ftrace_return(unsigned long ip, unsigned long *parent, > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct ftrace_regs *fregs) > { > - struct pt_regs *regs = &fregs->regs; > + struct pt_regs *regs = &arch_ftrace_regs(fregs)->regs; > unsigned long *stack = (unsigned long *)kernel_stack_pointer(regs); > > prepare_ftrace_return(ip, (unsigned long *)stack, 0); > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > index 2ac3b3b53cd0..f7d4f152f84d 100644 > --- a/include/linux/ftrace.h > +++ b/include/linux/ftrace.h > @@ -115,8 +115,6 @@ static inline int ftrace_mod_get_kallsym(unsigned int symnum, unsigned long *val > > extern int ftrace_enabled; > > -#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS > - > /** > * ftrace_regs - ftrace partial/optimal register set > * > @@ -142,11 +140,28 @@ extern int ftrace_enabled; > * > * NOTE: user *must not* access regs directly, only do it via APIs, because > * the member can be changed according to the architecture. > + * This is why the structure is empty here, so that nothing accesses > + * the ftrace_regs directly. > */ > struct ftrace_regs { > + /* Nothing to see here, use the accessor functions! */ > +}; > + > +#define ftrace_regs_size() sizeof(struct __arch_ftrace_regs) > + > +#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS > + > +struct __arch_ftrace_regs { > struct pt_regs regs; > }; > -#define arch_ftrace_get_regs(fregs) (&(fregs)->regs) > + > +struct ftrace_regs; > +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > + > +static inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) > +{ > + return &arch_ftrace_regs(fregs)->regs; > +} > > /* > * ftrace_regs_set_instruction_pointer() is to be defined by the architecture > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 5d87dac83b80..1e6251174530 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -7944,7 +7944,7 @@ __ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip, > void arch_ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct ftrace_regs *fregs) > { > - kmsan_unpoison_memory(fregs, sizeof(*fregs)); > + kmsan_unpoison_memory(fregs, ftrace_regs_size()); > __ftrace_ops_list_func(ip, parent_ip, NULL, fregs); > } > #else > -- > 2.45.2 > > -- Masami Hiramatsu (Google) <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] ftrace: Make ftrace_regs abstract from direct use 2024-10-08 23:05 ` [PATCH v2 1/2] ftrace: Make ftrace_regs abstract from direct use Steven Rostedt 2024-10-09 5:18 ` Masami Hiramatsu @ 2024-10-09 9:36 ` Heiko Carstens 1 sibling, 0 replies; 11+ messages in thread From: Heiko Carstens @ 2024-10-09 9:36 UTC (permalink / raw) To: Steven Rostedt Cc: linux-kernel, linux-trace-kernel, linux-arm-kernel, loongarch, linuxppc-dev, linux-riscv, linux-s390, linux-arch@vger.kernel.org, x86@kernel.org, Masami Hiramatsu, Mathieu Desnoyers, Mark Rutland, Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui, Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao, Madhavan Srinivasan, Paul Walmsley, Palmer Dabbelt, Albert Ou, Vasily Gorbik, Alexander Gordeev, Christian Borntraeger, Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen On Tue, Oct 08, 2024 at 07:05:28PM -0400, Steven Rostedt wrote: > From: Steven Rostedt <rostedt@goodmis.org> > > ftrace_regs was created to hold registers that store information to save > function parameters, return value and stack. Since it is a subset of > pt_regs, it should only be used by its accessor functions. But because > pt_regs can easily be taken from ftrace_regs (on most archs), it is > tempting to use it directly. But when running on other architectures, it > may fail to build or worse, build but crash the kernel! > > Instead, make struct ftrace_regs an empty structure and have the > architectures define __arch_ftrace_regs and all the accessor functions > will typecast to it to get to the actual fields. This will help avoid > usage of ftrace_regs directly. > > Link: https://lore.kernel.org/all/20241007171027.629bdafd@gandalf.local.home/ > > Acked-by: Catalin Marinas <catalin.marinas@arm.com> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- ... > arch/s390/include/asm/ftrace.h | 23 ++++++++++--------- > arch/s390/kernel/asm-offsets.c | 4 ++-- > arch/s390/kernel/ftrace.c | 2 +- > arch/s390/lib/test_unwind.c | 4 ++-- Acked-by: Heiko Carstens <hca@linux.ibm.com> # s390 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] ftrace: Consolidate ftrace_regs accessor functions for archs using pt_regs 2024-10-08 23:05 [PATCH v2 0/2] ftrace: Make ftrace_regs abstract and consolidate code Steven Rostedt 2024-10-08 23:05 ` [PATCH v2 1/2] ftrace: Make ftrace_regs abstract from direct use Steven Rostedt @ 2024-10-08 23:05 ` Steven Rostedt 2024-10-09 4:47 ` Masami Hiramatsu ` (2 more replies) 2024-10-09 9:36 ` [PATCH v2 0/2] ftrace: Make ftrace_regs abstract and consolidate code Heiko Carstens 2024-12-11 22:33 ` patchwork-bot+linux-riscv 3 siblings, 3 replies; 11+ messages in thread From: Steven Rostedt @ 2024-10-08 23:05 UTC (permalink / raw) To: linux-kernel, linux-trace-kernel, linux-arm-kernel, loongarch, linuxppc-dev, linux-riscv, linux-s390, linux-arch@vger.kernel.org, x86@kernel.org Cc: Masami Hiramatsu, Mathieu Desnoyers, Mark Rutland, Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui, Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao, Madhavan Srinivasan, Paul Walmsley, Palmer Dabbelt, Albert Ou, Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Christian Borntraeger, Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen From: Steven Rostedt <rostedt@goodmis.org> Most architectures use pt_regs within ftrace_regs making a lot of the accessor functions just calls to the pt_regs internally. Instead of duplication this effort, use a HAVE_ARCH_FTRACE_REGS for architectures that have their own ftrace_regs that is not based on pt_regs and will define all the accessor functions, and for the architectures that just use pt_regs, it will leave it undefined, and the default accessor functions will be used. Note, this will also make it easier to add new accessor functions to ftrace_regs as it will mean having to touch less architectures. Suggested-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> --- arch/arm64/include/asm/ftrace.h | 1 + arch/loongarch/include/asm/ftrace.h | 25 +------------------- arch/powerpc/include/asm/ftrace.h | 26 +-------------------- arch/riscv/include/asm/ftrace.h | 1 + arch/s390/include/asm/ftrace.h | 26 +-------------------- arch/x86/include/asm/ftrace.h | 21 +---------------- include/linux/ftrace.h | 32 ++++++------------------- include/linux/ftrace_regs.h | 36 +++++++++++++++++++++++++++++ 8 files changed, 49 insertions(+), 119 deletions(-) create mode 100644 include/linux/ftrace_regs.h diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h index bbb69c7751b9..5ccff4de7f09 100644 --- a/arch/arm64/include/asm/ftrace.h +++ b/arch/arm64/include/asm/ftrace.h @@ -54,6 +54,7 @@ extern void return_to_handler(void); unsigned long ftrace_call_adjust(unsigned long addr); #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS +#define HAVE_ARCH_FTRACE_REGS struct dyn_ftrace; struct ftrace_ops; struct ftrace_regs; diff --git a/arch/loongarch/include/asm/ftrace.h b/arch/loongarch/include/asm/ftrace.h index 0e15d36ce251..8f13eaeaa325 100644 --- a/arch/loongarch/include/asm/ftrace.h +++ b/arch/loongarch/include/asm/ftrace.h @@ -43,43 +43,20 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent); #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS struct ftrace_ops; -struct ftrace_regs; -#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) -struct __arch_ftrace_regs { - struct pt_regs regs; -}; +#include <linux/ftrace_regs.h> static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) { return &arch_ftrace_regs(fregs)->regs; } -static __always_inline unsigned long -ftrace_regs_get_instruction_pointer(struct ftrace_regs *fregs) -{ - return instruction_pointer(&arch_ftrace_regs(fregs)->regs); -} - static __always_inline void ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long ip) { instruction_pointer_set(&arch_ftrace_regs(fregs)->regs, ip); } -#define ftrace_regs_get_argument(fregs, n) \ - regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n) -#define ftrace_regs_get_stack_pointer(fregs) \ - kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs) -#define ftrace_regs_return_value(fregs) \ - regs_return_value(&arch_ftrace_regs(fregs)->regs) -#define ftrace_regs_set_return_value(fregs, ret) \ - regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret) -#define ftrace_override_function_with_return(fregs) \ - override_function_with_return(&arch_ftrace_regs(fregs)->regs) -#define ftrace_regs_query_register_offset(name) \ - regs_query_register_offset(name) - #define ftrace_graph_func ftrace_graph_func void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op, struct ftrace_regs *fregs); diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h index e299fd47d201..0edfb874eb02 100644 --- a/arch/powerpc/include/asm/ftrace.h +++ b/arch/powerpc/include/asm/ftrace.h @@ -32,12 +32,7 @@ struct dyn_arch_ftrace { int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec); #define ftrace_init_nop ftrace_init_nop -struct ftrace_regs; -#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) - -struct __arch_ftrace_regs { - struct pt_regs regs; -}; +#include <linux/ftrace_regs.h> static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) { @@ -52,25 +47,6 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, regs_set_return_ip(&arch_ftrace_regs(fregs)->regs, ip); } -static __always_inline unsigned long -ftrace_regs_get_instruction_pointer(struct ftrace_regs *fregs) -{ - return instruction_pointer(&arch_ftrace_regs(fregs)->regs); -} - -#define ftrace_regs_get_argument(fregs, n) \ - regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n) -#define ftrace_regs_get_stack_pointer(fregs) \ - kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs) -#define ftrace_regs_return_value(fregs) \ - regs_return_value(&arch_ftrace_regs(fregs)->regs) -#define ftrace_regs_set_return_value(fregs, ret) \ - regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret) -#define ftrace_override_function_with_return(fregs) \ - override_function_with_return(&arch_ftrace_regs(fregs)->regs) -#define ftrace_regs_query_register_offset(name) \ - regs_query_register_offset(name) - struct ftrace_ops; #define ftrace_graph_func ftrace_graph_func diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h index c6bcdff105b5..3d66437a1029 100644 --- a/arch/riscv/include/asm/ftrace.h +++ b/arch/riscv/include/asm/ftrace.h @@ -125,6 +125,7 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec); #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS #define arch_ftrace_get_regs(regs) NULL +#define HAVE_ARCH_FTRACE_REGS struct ftrace_ops; struct ftrace_regs; #define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h index 1498d0a9c762..fc97d75dc752 100644 --- a/arch/s390/include/asm/ftrace.h +++ b/arch/s390/include/asm/ftrace.h @@ -51,12 +51,7 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr) return addr; } -struct ftrace_regs; -#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) - -struct __arch_ftrace_regs { - struct pt_regs regs; -}; +#include <linux/ftrace_regs.h> static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) { @@ -84,12 +79,6 @@ static __always_inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph } #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ -static __always_inline unsigned long -ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs) -{ - return arch_ftrace_regs(fregs)->regs.psw.addr; -} - static __always_inline void ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long ip) @@ -97,19 +86,6 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, arch_ftrace_regs(fregs)->regs.psw.addr = ip; } -#define ftrace_regs_get_argument(fregs, n) \ - regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n) -#define ftrace_regs_get_stack_pointer(fregs) \ - kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs) -#define ftrace_regs_return_value(fregs) \ - regs_return_value(&arch_ftrace_regs(fregs)->regs) -#define ftrace_regs_set_return_value(fregs, ret) \ - regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret) -#define ftrace_override_function_with_return(fregs) \ - override_function_with_return(&arch_ftrace_regs(fregs)->regs) -#define ftrace_regs_query_register_offset(name) \ - regs_query_register_offset(name) - #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS /* * When an ftrace registered caller is tracing a function that is diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h index 87943f7a299b..8f02d28c571a 100644 --- a/arch/x86/include/asm/ftrace.h +++ b/arch/x86/include/asm/ftrace.h @@ -33,12 +33,8 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr) } #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS -struct ftrace_regs; -#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) -struct __arch_ftrace_regs { - struct pt_regs regs; -}; +#include <linux/ftrace_regs.h> static __always_inline struct pt_regs * arch_ftrace_get_regs(struct ftrace_regs *fregs) @@ -52,21 +48,6 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs) #define ftrace_regs_set_instruction_pointer(fregs, _ip) \ do { arch_ftrace_regs(fregs)->regs.ip = (_ip); } while (0) -#define ftrace_regs_get_instruction_pointer(fregs) \ - arch_ftrace_regs(fregs)->regs.ip) - -#define ftrace_regs_get_argument(fregs, n) \ - regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n) -#define ftrace_regs_get_stack_pointer(fregs) \ - kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs) -#define ftrace_regs_return_value(fregs) \ - regs_return_value(&arch_ftrace_regs(fregs)->regs) -#define ftrace_regs_set_return_value(fregs, ret) \ - regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret) -#define ftrace_override_function_with_return(fregs) \ - override_function_with_return(&arch_ftrace_regs(fregs)->regs) -#define ftrace_regs_query_register_offset(name) \ - regs_query_register_offset(name) struct ftrace_ops; #define ftrace_graph_func ftrace_graph_func diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index f7d4f152f84d..c96f9b0eb86e 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -113,6 +113,8 @@ static inline int ftrace_mod_get_kallsym(unsigned int symnum, unsigned long *val #ifdef CONFIG_FUNCTION_TRACER +#include <linux/ftrace_regs.h> + extern int ftrace_enabled; /** @@ -150,14 +152,11 @@ struct ftrace_regs { #define ftrace_regs_size() sizeof(struct __arch_ftrace_regs) #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS - -struct __arch_ftrace_regs { - struct pt_regs regs; -}; - -struct ftrace_regs; -#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) - +/* + * Architectures that define HAVE_DYNAMIC_FTRACE_WITH_ARGS must define their own + * arch_ftrace_get_regs() where it only returns pt_regs *if* it is fully + * populated. It should return NULL otherwise. + */ static inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) { return &arch_ftrace_regs(fregs)->regs; @@ -191,23 +190,6 @@ static __always_inline bool ftrace_regs_has_args(struct ftrace_regs *fregs) return ftrace_get_regs(fregs) != NULL; } -#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS -#define ftrace_regs_get_instruction_pointer(fregs) \ - instruction_pointer(ftrace_get_regs(fregs)) -#define ftrace_regs_get_argument(fregs, n) \ - regs_get_kernel_argument(ftrace_get_regs(fregs), n) -#define ftrace_regs_get_stack_pointer(fregs) \ - kernel_stack_pointer(ftrace_get_regs(fregs)) -#define ftrace_regs_return_value(fregs) \ - regs_return_value(ftrace_get_regs(fregs)) -#define ftrace_regs_set_return_value(fregs, ret) \ - regs_set_return_value(ftrace_get_regs(fregs), ret) -#define ftrace_override_function_with_return(fregs) \ - override_function_with_return(ftrace_get_regs(fregs)) -#define ftrace_regs_query_register_offset(name) \ - regs_query_register_offset(name) -#endif - typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op, struct ftrace_regs *fregs); diff --git a/include/linux/ftrace_regs.h b/include/linux/ftrace_regs.h new file mode 100644 index 000000000000..1cdd4bfa440b --- /dev/null +++ b/include/linux/ftrace_regs.h @@ -0,0 +1,36 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_FTRACE_TYPES_H +#define _LINUX_FTRACE_TYPES_H + +/* + * For archs that just copy pt_regs in ftrace regs, it can use this default. + * If an architecture does not use pt_regs, it must define all the below + * accessor functions. + */ +#ifndef HAVE_ARCH_FTRACE_REGS +struct __arch_ftrace_regs { + struct pt_regs regs; +}; + +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) + +struct ftrace_regs; + +#define ftrace_regs_get_instruction_pointer(fregs) \ + instruction_pointer(arch_ftrace_get_regs(fregs)) +#define ftrace_regs_get_argument(fregs, n) \ + regs_get_kernel_argument(arch_ftrace_get_regs(fregs), n) +#define ftrace_regs_get_stack_pointer(fregs) \ + kernel_stack_pointer(arch_ftrace_get_regs(fregs)) +#define ftrace_regs_return_value(fregs) \ + regs_return_value(arch_ftrace_get_regs(fregs)) +#define ftrace_regs_set_return_value(fregs, ret) \ + regs_set_return_value(arch_ftrace_get_regs(fregs), ret) +#define ftrace_override_function_with_return(fregs) \ + override_function_with_return(arch_ftrace_get_regs(fregs)) +#define ftrace_regs_query_register_offset(name) \ + regs_query_register_offset(name) + +#endif /* HAVE_ARCH_FTRACE_REGS */ + +#endif /* _LINUX_FTRACE_TYPES_H */ -- 2.45.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] ftrace: Consolidate ftrace_regs accessor functions for archs using pt_regs 2024-10-08 23:05 ` [PATCH v2 2/2] ftrace: Consolidate ftrace_regs accessor functions for archs using pt_regs Steven Rostedt @ 2024-10-09 4:47 ` Masami Hiramatsu 2024-10-09 13:43 ` Steven Rostedt 2024-10-09 9:37 ` Heiko Carstens 2024-10-09 15:31 ` Steven Rostedt 2 siblings, 1 reply; 11+ messages in thread From: Masami Hiramatsu @ 2024-10-09 4:47 UTC (permalink / raw) To: Steven Rostedt Cc: linux-kernel, linux-trace-kernel, linux-arm-kernel, loongarch, linuxppc-dev, linux-riscv, linux-s390, linux-arch@vger.kernel.org, x86@kernel.org, Masami Hiramatsu, Mathieu Desnoyers, Mark Rutland, Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui, Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao, Madhavan Srinivasan, Paul Walmsley, Palmer Dabbelt, Albert Ou, Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Christian Borntraeger, Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen On Tue, 08 Oct 2024 19:05:29 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > From: Steven Rostedt <rostedt@goodmis.org> > > Most architectures use pt_regs within ftrace_regs making a lot of the > accessor functions just calls to the pt_regs internally. Instead of > duplication this effort, use a HAVE_ARCH_FTRACE_REGS for architectures > that have their own ftrace_regs that is not based on pt_regs and will > define all the accessor functions, and for the architectures that just use > pt_regs, it will leave it undefined, and the default accessor functions > will be used. > > Note, this will also make it easier to add new accessor functions to > ftrace_regs as it will mean having to touch less architectures. > > Suggested-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > arch/arm64/include/asm/ftrace.h | 1 + > arch/loongarch/include/asm/ftrace.h | 25 +------------------- > arch/powerpc/include/asm/ftrace.h | 26 +-------------------- > arch/riscv/include/asm/ftrace.h | 1 + > arch/s390/include/asm/ftrace.h | 26 +-------------------- > arch/x86/include/asm/ftrace.h | 21 +---------------- > include/linux/ftrace.h | 32 ++++++------------------- > include/linux/ftrace_regs.h | 36 +++++++++++++++++++++++++++++ > 8 files changed, 49 insertions(+), 119 deletions(-) > create mode 100644 include/linux/ftrace_regs.h > > diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h > index bbb69c7751b9..5ccff4de7f09 100644 > --- a/arch/arm64/include/asm/ftrace.h > +++ b/arch/arm64/include/asm/ftrace.h > @@ -54,6 +54,7 @@ extern void return_to_handler(void); > unsigned long ftrace_call_adjust(unsigned long addr); > > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS > +#define HAVE_ARCH_FTRACE_REGS > struct dyn_ftrace; > struct ftrace_ops; > struct ftrace_regs; > diff --git a/arch/loongarch/include/asm/ftrace.h b/arch/loongarch/include/asm/ftrace.h > index 0e15d36ce251..8f13eaeaa325 100644 > --- a/arch/loongarch/include/asm/ftrace.h > +++ b/arch/loongarch/include/asm/ftrace.h > @@ -43,43 +43,20 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent); > > #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS > struct ftrace_ops; > -struct ftrace_regs; > -#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > > -struct __arch_ftrace_regs { > - struct pt_regs regs; > -}; > +#include <linux/ftrace_regs.h> > > static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) > { > return &arch_ftrace_regs(fregs)->regs; > } > > -static __always_inline unsigned long > -ftrace_regs_get_instruction_pointer(struct ftrace_regs *fregs) > -{ > - return instruction_pointer(&arch_ftrace_regs(fregs)->regs); > -} > - > static __always_inline void > ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long ip) > { > instruction_pointer_set(&arch_ftrace_regs(fregs)->regs, ip); > } > > -#define ftrace_regs_get_argument(fregs, n) \ > - regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n) > -#define ftrace_regs_get_stack_pointer(fregs) \ > - kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs) > -#define ftrace_regs_return_value(fregs) \ > - regs_return_value(&arch_ftrace_regs(fregs)->regs) > -#define ftrace_regs_set_return_value(fregs, ret) \ > - regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret) > -#define ftrace_override_function_with_return(fregs) \ > - override_function_with_return(&arch_ftrace_regs(fregs)->regs) > -#define ftrace_regs_query_register_offset(name) \ > - regs_query_register_offset(name) > - > #define ftrace_graph_func ftrace_graph_func > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct ftrace_regs *fregs); > diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h > index e299fd47d201..0edfb874eb02 100644 > --- a/arch/powerpc/include/asm/ftrace.h > +++ b/arch/powerpc/include/asm/ftrace.h > @@ -32,12 +32,7 @@ struct dyn_arch_ftrace { > int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec); > #define ftrace_init_nop ftrace_init_nop > > -struct ftrace_regs; > -#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > - > -struct __arch_ftrace_regs { > - struct pt_regs regs; > -}; > +#include <linux/ftrace_regs.h> > > static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) > { > @@ -52,25 +47,6 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, > regs_set_return_ip(&arch_ftrace_regs(fregs)->regs, ip); > } > > -static __always_inline unsigned long > -ftrace_regs_get_instruction_pointer(struct ftrace_regs *fregs) > -{ > - return instruction_pointer(&arch_ftrace_regs(fregs)->regs); > -} > - > -#define ftrace_regs_get_argument(fregs, n) \ > - regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n) > -#define ftrace_regs_get_stack_pointer(fregs) \ > - kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs) > -#define ftrace_regs_return_value(fregs) \ > - regs_return_value(&arch_ftrace_regs(fregs)->regs) > -#define ftrace_regs_set_return_value(fregs, ret) \ > - regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret) > -#define ftrace_override_function_with_return(fregs) \ > - override_function_with_return(&arch_ftrace_regs(fregs)->regs) > -#define ftrace_regs_query_register_offset(name) \ > - regs_query_register_offset(name) > - > struct ftrace_ops; > > #define ftrace_graph_func ftrace_graph_func > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > index c6bcdff105b5..3d66437a1029 100644 > --- a/arch/riscv/include/asm/ftrace.h > +++ b/arch/riscv/include/asm/ftrace.h > @@ -125,6 +125,7 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec); > > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS > #define arch_ftrace_get_regs(regs) NULL > +#define HAVE_ARCH_FTRACE_REGS > struct ftrace_ops; > struct ftrace_regs; > #define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h > index 1498d0a9c762..fc97d75dc752 100644 > --- a/arch/s390/include/asm/ftrace.h > +++ b/arch/s390/include/asm/ftrace.h > @@ -51,12 +51,7 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr) > return addr; > } > > -struct ftrace_regs; > -#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > - > -struct __arch_ftrace_regs { > - struct pt_regs regs; > -}; > +#include <linux/ftrace_regs.h> > > static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) > { > @@ -84,12 +79,6 @@ static __always_inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph > } > #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ > > -static __always_inline unsigned long > -ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs) > -{ > - return arch_ftrace_regs(fregs)->regs.psw.addr; > -} > - > static __always_inline void > ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, > unsigned long ip) > @@ -97,19 +86,6 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, > arch_ftrace_regs(fregs)->regs.psw.addr = ip; > } > > -#define ftrace_regs_get_argument(fregs, n) \ > - regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n) > -#define ftrace_regs_get_stack_pointer(fregs) \ > - kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs) > -#define ftrace_regs_return_value(fregs) \ > - regs_return_value(&arch_ftrace_regs(fregs)->regs) > -#define ftrace_regs_set_return_value(fregs, ret) \ > - regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret) > -#define ftrace_override_function_with_return(fregs) \ > - override_function_with_return(&arch_ftrace_regs(fregs)->regs) > -#define ftrace_regs_query_register_offset(name) \ > - regs_query_register_offset(name) > - > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > /* > * When an ftrace registered caller is tracing a function that is > diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h > index 87943f7a299b..8f02d28c571a 100644 > --- a/arch/x86/include/asm/ftrace.h > +++ b/arch/x86/include/asm/ftrace.h > @@ -33,12 +33,8 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr) > } > > #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS > -struct ftrace_regs; > -#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > > -struct __arch_ftrace_regs { > - struct pt_regs regs; > -}; > +#include <linux/ftrace_regs.h> > > static __always_inline struct pt_regs * > arch_ftrace_get_regs(struct ftrace_regs *fregs) > @@ -52,21 +48,6 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs) > #define ftrace_regs_set_instruction_pointer(fregs, _ip) \ > do { arch_ftrace_regs(fregs)->regs.ip = (_ip); } while (0) > > -#define ftrace_regs_get_instruction_pointer(fregs) \ > - arch_ftrace_regs(fregs)->regs.ip) > - > -#define ftrace_regs_get_argument(fregs, n) \ > - regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n) > -#define ftrace_regs_get_stack_pointer(fregs) \ > - kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs) > -#define ftrace_regs_return_value(fregs) \ > - regs_return_value(&arch_ftrace_regs(fregs)->regs) > -#define ftrace_regs_set_return_value(fregs, ret) \ > - regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret) > -#define ftrace_override_function_with_return(fregs) \ > - override_function_with_return(&arch_ftrace_regs(fregs)->regs) > -#define ftrace_regs_query_register_offset(name) \ > - regs_query_register_offset(name) > > struct ftrace_ops; > #define ftrace_graph_func ftrace_graph_func > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > index f7d4f152f84d..c96f9b0eb86e 100644 > --- a/include/linux/ftrace.h > +++ b/include/linux/ftrace.h > @@ -113,6 +113,8 @@ static inline int ftrace_mod_get_kallsym(unsigned int symnum, unsigned long *val > > #ifdef CONFIG_FUNCTION_TRACER > > +#include <linux/ftrace_regs.h> > + > extern int ftrace_enabled; > > /** > @@ -150,14 +152,11 @@ struct ftrace_regs { > #define ftrace_regs_size() sizeof(struct __arch_ftrace_regs) > > #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS > - > -struct __arch_ftrace_regs { > - struct pt_regs regs; > -}; > - > -struct ftrace_regs; > -#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > - > +/* > + * Architectures that define HAVE_DYNAMIC_FTRACE_WITH_ARGS must define their own > + * arch_ftrace_get_regs() where it only returns pt_regs *if* it is fully > + * populated. It should return NULL otherwise. > + */ > static inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) > { > return &arch_ftrace_regs(fregs)->regs; > @@ -191,23 +190,6 @@ static __always_inline bool ftrace_regs_has_args(struct ftrace_regs *fregs) > return ftrace_get_regs(fregs) != NULL; > } > > -#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS > -#define ftrace_regs_get_instruction_pointer(fregs) \ > - instruction_pointer(ftrace_get_regs(fregs)) > -#define ftrace_regs_get_argument(fregs, n) \ > - regs_get_kernel_argument(ftrace_get_regs(fregs), n) > -#define ftrace_regs_get_stack_pointer(fregs) \ > - kernel_stack_pointer(ftrace_get_regs(fregs)) > -#define ftrace_regs_return_value(fregs) \ > - regs_return_value(ftrace_get_regs(fregs)) > -#define ftrace_regs_set_return_value(fregs, ret) \ > - regs_set_return_value(ftrace_get_regs(fregs), ret) > -#define ftrace_override_function_with_return(fregs) \ > - override_function_with_return(ftrace_get_regs(fregs)) > -#define ftrace_regs_query_register_offset(name) \ > - regs_query_register_offset(name) > -#endif > - > typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct ftrace_regs *fregs); > > diff --git a/include/linux/ftrace_regs.h b/include/linux/ftrace_regs.h > new file mode 100644 > index 000000000000..1cdd4bfa440b > --- /dev/null > +++ b/include/linux/ftrace_regs.h > @@ -0,0 +1,36 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LINUX_FTRACE_TYPES_H > +#define _LINUX_FTRACE_TYPES_H ^^^^^^^^^^^^^^^^ Is this _LINUX_FTRACE_REGS_H? > + > +/* > + * For archs that just copy pt_regs in ftrace regs, it can use this default. > + * If an architecture does not use pt_regs, it must define all the below > + * accessor functions. > + */ > +#ifndef HAVE_ARCH_FTRACE_REGS > +struct __arch_ftrace_regs { > + struct pt_regs regs; > +}; > + > +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > + > +struct ftrace_regs; > + > +#define ftrace_regs_get_instruction_pointer(fregs) \ > + instruction_pointer(arch_ftrace_get_regs(fregs)) > +#define ftrace_regs_get_argument(fregs, n) \ > + regs_get_kernel_argument(arch_ftrace_get_regs(fregs), n) > +#define ftrace_regs_get_stack_pointer(fregs) \ > + kernel_stack_pointer(arch_ftrace_get_regs(fregs)) > +#define ftrace_regs_return_value(fregs) \ > + regs_return_value(arch_ftrace_get_regs(fregs)) > +#define ftrace_regs_set_return_value(fregs, ret) \ > + regs_set_return_value(arch_ftrace_get_regs(fregs), ret) > +#define ftrace_override_function_with_return(fregs) \ > + override_function_with_return(arch_ftrace_get_regs(fregs)) > +#define ftrace_regs_query_register_offset(name) \ > + regs_query_register_offset(name) > + > +#endif /* HAVE_ARCH_FTRACE_REGS */ > + > +#endif /* _LINUX_FTRACE_TYPES_H */ Ditto. Others looks good to me. Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Thank you! > -- > 2.45.2 > > -- Masami Hiramatsu (Google) <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] ftrace: Consolidate ftrace_regs accessor functions for archs using pt_regs 2024-10-09 4:47 ` Masami Hiramatsu @ 2024-10-09 13:43 ` Steven Rostedt 0 siblings, 0 replies; 11+ messages in thread From: Steven Rostedt @ 2024-10-09 13:43 UTC (permalink / raw) To: Masami Hiramatsu (Google) Cc: linux-kernel, linux-trace-kernel, linux-arm-kernel, loongarch, linuxppc-dev, linux-riscv, linux-s390, linux-arch@vger.kernel.org, x86@kernel.org, Mathieu Desnoyers, Mark Rutland, Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui, Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao, Madhavan Srinivasan, Paul Walmsley, Palmer Dabbelt, Albert Ou, Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Christian Borntraeger, Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen On Wed, 9 Oct 2024 13:47:23 +0900 Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > > --- /dev/null > > +++ b/include/linux/ftrace_regs.h > > @@ -0,0 +1,36 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef _LINUX_FTRACE_TYPES_H > > +#define _LINUX_FTRACE_TYPES_H > ^^^^^^^^^^^^^^^^ Is this _LINUX_FTRACE_REGS_H? Ah, I originally called it ftrace_types.h, but later decided that name didn't really fit. I changed all references to it but this one. Thanks for catching this. > > > > + > > +/* > > + * For archs that just copy pt_regs in ftrace regs, it can use this default. > > + * If an architecture does not use pt_regs, it must define all the below > > + * accessor functions. > > + */ > > +#ifndef HAVE_ARCH_FTRACE_REGS > > +struct __arch_ftrace_regs { > > + struct pt_regs regs; > > +}; > > + > > +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > > + > > +struct ftrace_regs; > > + > > +#define ftrace_regs_get_instruction_pointer(fregs) \ > > + instruction_pointer(arch_ftrace_get_regs(fregs)) > > +#define ftrace_regs_get_argument(fregs, n) \ > > + regs_get_kernel_argument(arch_ftrace_get_regs(fregs), n) > > +#define ftrace_regs_get_stack_pointer(fregs) \ > > + kernel_stack_pointer(arch_ftrace_get_regs(fregs)) > > +#define ftrace_regs_return_value(fregs) \ > > + regs_return_value(arch_ftrace_get_regs(fregs)) > > +#define ftrace_regs_set_return_value(fregs, ret) \ > > + regs_set_return_value(arch_ftrace_get_regs(fregs), ret) > > +#define ftrace_override_function_with_return(fregs) \ > > + override_function_with_return(arch_ftrace_get_regs(fregs)) > > +#define ftrace_regs_query_register_offset(name) \ > > + regs_query_register_offset(name) > > + > > +#endif /* HAVE_ARCH_FTRACE_REGS */ > > + > > +#endif /* _LINUX_FTRACE_TYPES_H */ > > Ditto. > > Others looks good to me. > > Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Thanks, I'll send a v2 with this update. -- Steve ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] ftrace: Consolidate ftrace_regs accessor functions for archs using pt_regs 2024-10-08 23:05 ` [PATCH v2 2/2] ftrace: Consolidate ftrace_regs accessor functions for archs using pt_regs Steven Rostedt 2024-10-09 4:47 ` Masami Hiramatsu @ 2024-10-09 9:37 ` Heiko Carstens 2024-10-09 15:31 ` Steven Rostedt 2 siblings, 0 replies; 11+ messages in thread From: Heiko Carstens @ 2024-10-09 9:37 UTC (permalink / raw) To: Steven Rostedt Cc: linux-kernel, linux-trace-kernel, linux-arm-kernel, loongarch, linuxppc-dev, linux-riscv, linux-s390, linux-arch@vger.kernel.org, x86@kernel.org, Masami Hiramatsu, Mathieu Desnoyers, Mark Rutland, Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui, Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao, Madhavan Srinivasan, Paul Walmsley, Palmer Dabbelt, Albert Ou, Vasily Gorbik, Alexander Gordeev, Christian Borntraeger, Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen On Tue, Oct 08, 2024 at 07:05:29PM -0400, Steven Rostedt wrote: > From: Steven Rostedt <rostedt@goodmis.org> > > Most architectures use pt_regs within ftrace_regs making a lot of the > accessor functions just calls to the pt_regs internally. Instead of > duplication this effort, use a HAVE_ARCH_FTRACE_REGS for architectures > that have their own ftrace_regs that is not based on pt_regs and will > define all the accessor functions, and for the architectures that just use > pt_regs, it will leave it undefined, and the default accessor functions > will be used. > > Note, this will also make it easier to add new accessor functions to > ftrace_regs as it will mean having to touch less architectures. > > Suggested-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> ... > arch/s390/include/asm/ftrace.h | 26 +-------------------- Acked-by: Heiko Carstens <hca@linux.ibm.com> # s390 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] ftrace: Consolidate ftrace_regs accessor functions for archs using pt_regs 2024-10-08 23:05 ` [PATCH v2 2/2] ftrace: Consolidate ftrace_regs accessor functions for archs using pt_regs Steven Rostedt 2024-10-09 4:47 ` Masami Hiramatsu 2024-10-09 9:37 ` Heiko Carstens @ 2024-10-09 15:31 ` Steven Rostedt 2 siblings, 0 replies; 11+ messages in thread From: Steven Rostedt @ 2024-10-09 15:31 UTC (permalink / raw) To: linux-kernel, linux-trace-kernel, linux-arm-kernel, loongarch, linuxppc-dev, linux-riscv, linux-s390, linux-arch@vger.kernel.org, x86@kernel.org Cc: Masami Hiramatsu, Mathieu Desnoyers, Mark Rutland, Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui, Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao, Madhavan Srinivasan, Paul Walmsley, Palmer Dabbelt, Albert Ou, Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Christian Borntraeger, Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen Loongarch maintainers, please note the below comments! On Tue, 08 Oct 2024 19:05:29 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h > index bbb69c7751b9..5ccff4de7f09 100644 > --- a/arch/arm64/include/asm/ftrace.h > +++ b/arch/arm64/include/asm/ftrace.h > @@ -54,6 +54,7 @@ extern void return_to_handler(void); > unsigned long ftrace_call_adjust(unsigned long addr); > > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS > +#define HAVE_ARCH_FTRACE_REGS > struct dyn_ftrace; > struct ftrace_ops; > struct ftrace_regs; > diff --git a/arch/loongarch/include/asm/ftrace.h b/arch/loongarch/include/asm/ftrace.h > index 0e15d36ce251..8f13eaeaa325 100644 > --- a/arch/loongarch/include/asm/ftrace.h > +++ b/arch/loongarch/include/asm/ftrace.h > @@ -43,43 +43,20 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent); > > #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS > struct ftrace_ops; > -struct ftrace_regs; > -#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > > -struct __arch_ftrace_regs { > - struct pt_regs regs; > -}; > +#include <linux/ftrace_regs.h> > > static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) > { > return &arch_ftrace_regs(fregs)->regs; > } The above function is incorrect. I know I just added the comment about how it is to work below, but if pt_regs is not fully filled, then arch_ftrace_get_regs() must return NULL. This is because if a callback is registered with ftrace, and forgets to add the FTRACE_OPS_FL_SAVE_REGS flag, then when it does: regs = ftrace_get_regs(fregs); it should get NULL and not a partially filled pt_regs set. Because the API is that ftrace_get_regs() will return either a full pt_regs (where the caller can know that it has all the correct registers) or NULL where it does not have any registers. It's an all or nothing approach. You can see x86 has: static __always_inline struct pt_regs * arch_ftrace_get_regs(struct ftrace_regs *fregs) { /* Only when FL_SAVE_REGS is set, cs will be non zero */ if (!arch_ftrace_regs(fregs)->regs.cs) return NULL; return &arch_ftrace_regs(fregs)->regs; } Where it checks if regs.cs is set to determine if it has all the regs or not. Please do something similar for your architecture. > > -static __always_inline unsigned long > -ftrace_regs_get_instruction_pointer(struct ftrace_regs *fregs) > -{ > - return instruction_pointer(&arch_ftrace_regs(fregs)->regs); > -} > - > static __always_inline void > ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long ip) > { > instruction_pointer_set(&arch_ftrace_regs(fregs)->regs, ip); > } > > > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > index f7d4f152f84d..c96f9b0eb86e 100644 > --- a/include/linux/ftrace.h > +++ b/include/linux/ftrace.h > @@ -113,6 +113,8 @@ static inline int ftrace_mod_get_kallsym(unsigned int symnum, unsigned long *val > > #ifdef CONFIG_FUNCTION_TRACER > > +#include <linux/ftrace_regs.h> > + > extern int ftrace_enabled; > > /** > @@ -150,14 +152,11 @@ struct ftrace_regs { > #define ftrace_regs_size() sizeof(struct __arch_ftrace_regs) > > #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS > - > -struct __arch_ftrace_regs { > - struct pt_regs regs; > -}; > - > -struct ftrace_regs; > -#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs)) > - > +/* > + * Architectures that define HAVE_DYNAMIC_FTRACE_WITH_ARGS must define their own > + * arch_ftrace_get_regs() where it only returns pt_regs *if* it is fully > + * populated. It should return NULL otherwise. > + */ I'm adding the above comment to help other architectures know of this requirement. > static inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) > { > return &arch_ftrace_regs(fregs)->regs; -- Steve ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/2] ftrace: Make ftrace_regs abstract and consolidate code 2024-10-08 23:05 [PATCH v2 0/2] ftrace: Make ftrace_regs abstract and consolidate code Steven Rostedt 2024-10-08 23:05 ` [PATCH v2 1/2] ftrace: Make ftrace_regs abstract from direct use Steven Rostedt 2024-10-08 23:05 ` [PATCH v2 2/2] ftrace: Consolidate ftrace_regs accessor functions for archs using pt_regs Steven Rostedt @ 2024-10-09 9:36 ` Heiko Carstens 2024-12-11 22:33 ` patchwork-bot+linux-riscv 3 siblings, 0 replies; 11+ messages in thread From: Heiko Carstens @ 2024-10-09 9:36 UTC (permalink / raw) To: Steven Rostedt Cc: linux-kernel, linux-trace-kernel, linux-arm-kernel, loongarch, linuxppc-dev, linux-riscv, linux-s390, linux-arch@vger.kernel.org, x86@kernel.org, Masami Hiramatsu, Mathieu Desnoyers, Mark Rutland, Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui, Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao, Madhavan Srinivasan, Paul Walmsley, Palmer Dabbelt, Albert Ou, Vasily Gorbik, Alexander Gordeev, Christian Borntraeger, Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen On Tue, Oct 08, 2024 at 07:05:27PM -0400, Steven Rostedt wrote: > > This is based on: > > https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git/ > ftrace/for-next > > ftrace_regs was created to hold registers that store information to save > function parameters, return value and stack. Since it is a subset of > pt_regs, it should only be used by its accessor functions. But because > pt_regs can easily be taken from ftrace_regs (on most archs), it is > tempting to use it directly. But when running on other architectures, it > may fail to build or worse, build but crash the kernel! > > Instead, make struct ftrace_regs an empty structure and have the > architectures define __arch_ftrace_regs and all the accessor functions > will typecast to it to get to the actual fields. This will help avoid > usage of ftrace_regs directly. > > I again compiled all the affected architectures (except for 32bit ppc). > I got s390 built when disabling bcachefs. Build fix for this problem is commit 2007d28ec009 ("bcachefs: rename version -> bversion for big endian builds"). ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/2] ftrace: Make ftrace_regs abstract and consolidate code 2024-10-08 23:05 [PATCH v2 0/2] ftrace: Make ftrace_regs abstract and consolidate code Steven Rostedt ` (2 preceding siblings ...) 2024-10-09 9:36 ` [PATCH v2 0/2] ftrace: Make ftrace_regs abstract and consolidate code Heiko Carstens @ 2024-12-11 22:33 ` patchwork-bot+linux-riscv 3 siblings, 0 replies; 11+ messages in thread From: patchwork-bot+linux-riscv @ 2024-12-11 22:33 UTC (permalink / raw) To: Steven Rostedt Cc: linux-riscv, linux-kernel, linux-trace-kernel, linux-arm-kernel, loongarch, linuxppc-dev, linux-s390, linux-arch, x86, mhiramat, mathieu.desnoyers, mark.rutland, catalin.marinas, will, chenhuacai, kernel, mpe, npiggin, christophe.leroy, naveen, maddy, paul.walmsley, palmer, aou, hca, gor, agordeev, borntraeger, svens, tglx, mingo, bp, dave.hansen Hello: This series was applied to riscv/linux.git (fixes) by Steven Rostedt (Google) <rostedt@goodmis.org>: On Tue, 08 Oct 2024 19:05:27 -0400 you wrote: > This is based on: > > https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git/ > ftrace/for-next > > ftrace_regs was created to hold registers that store information to save > function parameters, return value and stack. Since it is a subset of > pt_regs, it should only be used by its accessor functions. But because > pt_regs can easily be taken from ftrace_regs (on most archs), it is > tempting to use it directly. But when running on other architectures, it > may fail to build or worse, build but crash the kernel! > > [...] Here is the summary with links: - [v2,1/2] ftrace: Make ftrace_regs abstract from direct use https://git.kernel.org/riscv/c/7888af4166d4 - [v2,2/2] ftrace: Consolidate ftrace_regs accessor functions for archs using pt_regs (no matching commit) You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-12-11 22:33 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-08 23:05 [PATCH v2 0/2] ftrace: Make ftrace_regs abstract and consolidate code Steven Rostedt 2024-10-08 23:05 ` [PATCH v2 1/2] ftrace: Make ftrace_regs abstract from direct use Steven Rostedt 2024-10-09 5:18 ` Masami Hiramatsu 2024-10-09 9:36 ` Heiko Carstens 2024-10-08 23:05 ` [PATCH v2 2/2] ftrace: Consolidate ftrace_regs accessor functions for archs using pt_regs Steven Rostedt 2024-10-09 4:47 ` Masami Hiramatsu 2024-10-09 13:43 ` Steven Rostedt 2024-10-09 9:37 ` Heiko Carstens 2024-10-09 15:31 ` Steven Rostedt 2024-10-09 9:36 ` [PATCH v2 0/2] ftrace: Make ftrace_regs abstract and consolidate code Heiko Carstens 2024-12-11 22:33 ` patchwork-bot+linux-riscv
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).