Re: [PATCH] lib/sys_info: add a simple timer based memory corruption detector

[Petr Mladek <pmladek@suse.com>]

Added few more people into Cc.

On Wed 2026-05-27 11:43:24, Feng Tang wrote:
> During debugging some bios/hardware related nasty memory corruption
> issues, we found using periodic timer to monitor specific dram/mmio
> physical address is very useful for debugging, which acts like
> a basic software watchpoint.
>
> For those bugs,  who (and when) change(corrupt) those dram or mmio
> register is hard to trace, and sometimes even hardware jtag debugger
> can't help (say the physical address watchpoint doesn't work).

It seems that this approach helped you to debug a nasty problem.
I am not sure why the other ways did not work.

Could you please provide some more information about the use case?

Ideally, please describe one particular situation where it helped.
What was the bug, how it manifested, and how the crash dump helped
to analyze it. Feel free to use generic names, like graphics card,
or ssd disk, instead of exact producer names, ...

> The biggest shortcoming is it can never capture the exact point like
> a hardware watchpoint, no matter how small the timer interval is set,
> the idea is trying to approach the point, hoping the caught context
> have enough debug info (which did help us in solving bios/hardware
> bugs)
>
> The working flow is simple: after suspected address is identified,
> start periodic timer polling it to catch if its value is changed to
> target 'magic' value, then halt the cpu (better limit to have only
> one cpu online), or panic, or print out system information, so that
> the error environment is frozen for further check , or let
> kexec/kdump to record the vmore, etc.
> 
> All the settings are module parameters:
> 
>   watch_interval_ms: SW watchpoint check interval in ms
>   paddr_dram_to_watch: Physical dram address to monitor.
>   target_dram_val: Expected value at the dram address that triggers the watchpoint.
>   paddr_mmio_to_watch: Physical mmio address to monitor. Must be 32-bit aligned.
>   target_mmio_val: Expected value at the mmio address that triggers the watchpoint.
>   panic_on_hit: Trigger kernel panic when watchpoint condition hits.
>   hang_on_hit: halt the CPU (wait for HW debugger)
> 
> This RFC is trying to show the idea and get feedback, and there are
> some todos:
>   * merge the dram/mmio interface to auto detect it's dram or mmio
>   * support runtime changing the address
>   * move the starting point earlier in boot phase
>   * currently is monitoring 'changing to a value', add support
>     for 'changing from a value'

Sashiko AI has pointed out several possible problems, see
https://sashiko.dev/#/patchset/20260527034324.51136-1-feng.tang%40linux.alibaba.com

> --- a/lib/sys_info.c
> +++ b/lib/sys_info.c

I we agreed that this feature would be useful then it would deserve
its own source file.

IMHO, it fits into the watchdog category. I would put it into
kernel/watch_mem or so.

Best Regards,
Petr

> @@ -164,3 +164,107 @@ void sys_info(unsigned long si_mask)
>  {
>  	__sys_info(si_mask ? : kernel_si_mask);
>  }
> +
> +#ifdef CONFIG_SW_WATCHPOINT
> +
> +/* default 100 ms interval */
> +static unsigned long watch_interval_ms = 100;
> +module_param(watch_interval_ms, ulong, 0644);
> +MODULE_PARM_DESC(watch_interval_ms, "SW watchpoint check interval in ms");
> +
> +static unsigned long paddr_dram_to_watch;
> +module_param(paddr_dram_to_watch, ulong, 0644);
> +MODULE_PARM_DESC(paddr_dram_to_watch, "Physical DRAM address to watch");
> +
> +static unsigned long *vaddr_dram;
> +
> +static unsigned long target_dram_val;
> +module_param(target_dram_val, ulong, 0644);
> +MODULE_PARM_DESC(target_dram_val, "Target DRAM value to trigger watchpoint");
> +
> +/* The MMIO address should be 32b aligned */
> +static unsigned long paddr_mmio_to_watch;
> +module_param(paddr_mmio_to_watch, ulong, 0644);
> +MODULE_PARM_DESC(paddr_mmio_to_watch, "Physical MMIO address to watch (32bit aligned)");
> +
> +static unsigned int *vaddr_mmio;
> +
> +static unsigned int target_mmio_val;
> +module_param(target_mmio_val, uint, 0644);
> +MODULE_PARM_DESC(target_mmio_val, "Target MMIO value to trigger watchpoint");
> +
> +static bool panic_on_hit;
> +module_param(panic_on_hit, bool, 0644);
> +MODULE_PARM_DESC(panic_on_hit, "Panic when watchpoint hits");
> +
> +static bool hang_on_hit;
> +module_param(hang_on_hit, bool, 0644);
> +MODULE_PARM_DESC(hang_on_hit, "Hang when watchpoint hits");
> +
> +/* Stop the watchpoint timer after first hit */
> +static bool check_once = true;
> +module_param(check_once, bool, 0644);
> +MODULE_PARM_DESC(check_once, "Stop watching after first hit");
> +
> +static struct timer_list sw_watchpoint_timer;
> +
> +static void sw_watchpoint_timer_fn(struct timer_list *unused)
> +{
> +	bool hit = false;
> +
> +	if (vaddr_mmio && (*vaddr_mmio == target_mmio_val)) {
> +		pr_info("MMIO [@0x%lx] hit the target value [0x%x]!\n",
> +			paddr_mmio_to_watch, target_mmio_val);
> +		hit = true;
> +	}
> +
> +	if (vaddr_dram && (*vaddr_dram == target_dram_val)) {
> +		pr_info("DRAM [@0x%lx] hit the target value [0x%lx]!\n",
> +			paddr_dram_to_watch, target_dram_val);
> +		hit = true;
> +	}
> +
> +	if (hit) {
> +		sys_info(0);
> +
> +		/* Useful for attaching HW debugger */
> +		if (hang_on_hit) {
> +			pr_warn("Will dead loop on this CPU\n");
> +			while (1);
> +		}
> +
> +		/* Could be used to trigger kexec/kdump */
> +		if (panic_on_hit)
> +			panic("SW watchpoint hit!");
> +
> +		if (check_once)
> +			return;
> +	}
> +
> +	mod_timer(&sw_watchpoint_timer, jiffies + msecs_to_jiffies(watch_interval_ms));
> +}
> +
> +static int __init sw_watchpoint_timer_init(void)
> +{
> +	if (paddr_mmio_to_watch) {
> +		vaddr_mmio = ioremap(paddr_mmio_to_watch & PAGE_MASK, PAGE_SIZE);
> +		if (!vaddr_mmio)
> +			return -ENOMEM;
> +
> +		vaddr_mmio += (paddr_mmio_to_watch % PAGE_SIZE) / 4;
> +	}
> +
> +	if (paddr_dram_to_watch) {
> +		vaddr_dram = phys_to_virt(paddr_dram_to_watch);
> +		if (!vaddr_dram)
> +			return -ENOMEM;
> +	}
> +
> +	timer_setup(&sw_watchpoint_timer, sw_watchpoint_timer_fn, 0);
> +	sw_watchpoint_timer.expires = jiffies + msecs_to_jiffies(watch_interval_ms);
> +	add_timer(&sw_watchpoint_timer);
> +
> +	return 0;
> +}
> +core_initcall(sw_watchpoint_timer_init);
> +#endif
> 
> base-commit: e7ae89a0c97ce2b68b0983cd01eda67cf373517d
> -- 
> 2.39.5 (Apple Git-154)