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

[Greg Ungerer]

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?
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;
> +	}
> +}