Re: [PATCH v4 2/2] arch: m68k: Add STACKTRACE support

[Jean-Michel Hautbois]

Hi Greg,

On 22/12/2024 14:14, Greg Ungerer wrote:
> Hi JM,
>
> On 4/12/24 17:14, Jean-Michel Hautbois wrote:
> > In order to use tracing, implement a basic arch_stack_walk() based on
> > the one in PowerPC.
> > Tested on a M54418 coldfire.
> >
> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@xxxxxxxxxx>
> > ---
> >   arch/m68k/Kconfig             |  5 ++++
> >   arch/m68k/kernel/Makefile     |  1 +
> >   arch/m68k/kernel/stacktrace.c | 70 +++++++++++++++++++++++++++++++++ 
> > ++++++++++
> >   3 files changed, 76 insertions(+)
> >
> > diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
> > index 
> > 793ab1e2762609725bbf793f6dffecfa3ecfff0f..3ad8596aab71190807f8c11dd5876aa1198760f3 100644
> > --- a/arch/m68k/Kconfig
> > +++ b/arch/m68k/Kconfig
> > @@ -106,6 +106,11 @@ config BOOTINFO_PROC
> >         Say Y to export the bootinfo used to boot the kernel in a
> >         "bootinfo" file in procfs.  This is useful with kexec.
> > +config STACKTRACE_SUPPORT
> > +    def_bool MMU_COLDFIRE
>
> Modulo testing is there any reason this is enabled only for MMU ColdFire 
> targets?

You said it, modulo testing (and lack of knowledge on my side to be 
certain it is not coldfire specific) :-).

If it can be tested on other machines, I would be glad to change it !

Thanks,
JM

> There doesn't look to be anything that is ColdFire specific in these 
> changes,
> looks like it should work on any m68k machine.
>
> Regards
> Greg
>
>
>
> > +    select ARCH_STACKWALK
> > +    select ARCH_WANT_FRAME_POINTERS
> > +
> >   menu "Platform setup"
> >   source "arch/m68k/Kconfig.cpu"
> > diff --git a/arch/m68k/kernel/Makefile b/arch/m68k/kernel/Makefile
> > index 
> > 6c732ed3998b714a4842ee29c977550a61979779..cb02bcfe04c6b265fa97db9237395a262e649989 100644
> > --- a/arch/m68k/kernel/Makefile
> > +++ b/arch/m68k/kernel/Makefile
> > @@ -23,3 +23,4 @@ obj-$(CONFIG_UBOOT)        += uboot.o
> >   obj-$(CONFIG_EARLY_PRINTK)    += early_printk.o
> > +obj-y    += stacktrace.o
> > diff --git a/arch/m68k/kernel/stacktrace.c b/arch/m68k/kernel/ 
> > stacktrace.c
> > new file mode 100644
> > index 
> > 0000000000000000000000000000000000000000..4c2fb6b0cf675ee5a3a21393a50603fea98a1b03
> > --- /dev/null
> > +++ b/arch/m68k/kernel/stacktrace.c
> > @@ -0,0 +1,70 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/*
> > + * Stack trace utility functions etc.
> > + *
> > + * Copyright 2024 Jean-Michel Hautbois, Yoseli SAS.
> > + */
> > +
> > +#include <asm/current.h>
> > +#include <asm/ptrace.h>
> > +#include <linux/sched.h>
> > +#include <linux/sched/task_stack.h>
> > +#include <linux/stacktrace.h>
> > +
> > +static inline unsigned long current_stack_frame(void)
> > +{
> > +    unsigned long sp;
> > +
> > +    asm volatile("movl %%fp, %0" : "=r"(sp));
> > +    return sp;
> > +}
> > +
> > +static inline int validate_sp(unsigned long sp, struct task_struct 
> > *task)
> > +{
> > +    unsigned long stack_start, stack_end;
> > +
> > +    if (task == current)
> > +        stack_start = (unsigned long)task_stack_page(task);
> > +    else
> > +        stack_start = (unsigned long)task->thread.esp0;
> > +
> > +    stack_end = stack_start + THREAD_SIZE;
> > +
> > +    if (sp < stack_start || sp >= stack_end)
> > +        return 0;
> > +
> > +    return 1;
> > +}
> > +
> > +void __no_sanitize_address arch_stack_walk(stack_trace_consume_fn 
> > consume_entry, void *cookie,
> > +                       struct task_struct *task, struct pt_regs *regs)
> > +{
> > +    unsigned long sp;
> > +
> > +    if (regs && !consume_entry(cookie, regs->pc))
> > +        return;
> > +
> > +    if (regs)
> > +        sp = (unsigned long) regs;
> > +    else if (task == current)
> > +        sp = current_stack_frame();
> > +    else
> > +        sp = task->thread.ksp;
> > +
> > +    for (;;) {
> > +        unsigned long *stack = (unsigned long *) sp;
> > +        unsigned long newsp, ip;
> > +
> > +        if (!validate_sp(sp, task))
> > +            return;
> > +
> > +        newsp = stack[0];
> > +        ip = stack[1];
> > +
> > +        if (!consume_entry(cookie, ip))
> > +            return;
> > +
> > +        sp = newsp;
> > +    }
> > +}