Re: [PATCH v2 1/2] ftrace: Make ftrace_regs abstract from direct use

From: Google
Date: Wed Oct 09 2024 - 01:19:06 EST

On Tue, 08 Oct 2024 19:05:28 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> From: Steven Rostedt <rostedt@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxxx/
>

This looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>

For x86 and generic part.

Thank you,

> Acked-by: Catalin Marinas <catalin.marinas@xxxxxxx>
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
> ---
> Changes since v1: https://lore.kernel.org/20241007204743.41314f1d@xxxxxxxxxxxxxxxxxx
>
> - 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 *)&regs->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@xxxxxxxxxx>