Re: [for-linus][PATCH 3/3] ftrace/microblaze: Do not find "true_parent" for return address

[Al Viro]

On Fri, Dec 13, 2024 at 04:39:29PM +0100, Michal Simek wrote:

> > diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
> > index 74c353164ca1..a75d107a45f8 100644
> > --- a/kernel/trace/trace_functions.c
> > +++ b/kernel/trace/trace_functions.c
> > @@ -176,7 +176,8 @@ static void function_trace_start(struct trace_array *tr)
> >   	tracing_reset_online_cpus(&tr->array_buffer);
> >   }
> > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> > +/* Microblaze currently doesn't implement kernel_stack_pointer() */
> 
> Does it mean that this function should depends on
> ARCH_HAS_CURRENT_STACK_POINTER instead of name the architecture?

Nope.  ARCH_HAS_CURRENT_STACK_POINTER == "there's a current_stack_pointer variable"
(presumably something like register unsigned long current_stack_pointer asm("r1");
in case of microblaze).  kernel_stack_pointer() is "here's pt_regs, give me the
kernel stack pointer stored in it (assuming it _is_ stored there)".

And what ftrace code really want is "here's the structure formed by _mcount();
give me the kernel stack pointer at the time of _mcount() entry".  _IF_ that
structure is pt_regs (fairly common) and if there's kernel_stack_pointer(),
we get the default implementation of that helper in linux/ftrace_regs.h:

#define ftrace_regs_get_stack_pointer(fregs) \
        kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs)

If it's not pt_regs, you are expected to define HAVE_ARCH_FTRACE_REGS, define
struct __arch_ftrace_regs to match whatever layout you are using and provide
the set of ftrace_regs_...() helpers.

>From my reading of your mcount.S, the layout is, indeed, different and
r1 is not stored there at all - something like

struct __arch_ftrace_regs {
	unsigned long r2, r3, r4, r6;
	unsigned long r7, r8, r9, r10;
	unsigned long r11, r12, r13, r14;
	unsigned long r16, r17, r18, r19;
	unsigned long r20, r21, r22, r23;
	unsigned long r24, r25, r26, r27;
	unsigned long r28, r29, r30, r31;
	unsigned long r5;
}

static inline unsigned long ftrace_regs_get_stack_pointer(struct ftrace_regs *regs)
{
	return (unsigned long)regs + sizeof(struct __arch_ftrace_regs) + 4;
}