[PATCH 1/23] Make register values available to panic notifiers

[PATCH 1/23] Make register values available to panic notifiers

[David VomLehn]

This patch makes panic() and die() registers available to, for example,
panic notifier functions.  Panic notifier functions are quite useful
for recording crash information, but they don't get passed the register
values. This makes it hard to print register contents, do stack
backtraces, etc. The changes in this patch save the register state when
panic() is called and introduce a function for die() to call that allows
it to pass in the registers it was passed.

Following this patch are more patches, one per architecture. These include
two types of changes:
o  A save_ptregs() function for the processor. I've taken a whack at
   doing this for all of the processors. I have tested x86 and MIPS
   versions. I was able to find cross compilers for ARM, ... and the
   code compiles cleanly. Everything else, well, what you see is sheer
   fantasy. You are welcome to chortle with merriment.
o  When I could figure it out, I replaced the calls to panic() in
   exception handling functions with calls to panic_with_regs() so
   that everyone can leverage these changes without much effort. Again,
   not all the code was transparent, so there are likely some places
   that should have additional work done.

Note that the pointer to the struct pt_regs may be NULL. This is to
accomodate those processors which don't have a working save_ptregs(). I'd
love to eliminate this case by providing a save_ptregs() for all
architectures, but I'll need help to so.

Signed-off-by: David VomLehn <dvomlehn@cisco.com>
---
 include/linux/kernel.h |    5 ++
 include/linux/ptrace.h |    9 ++++
 include/linux/ptreg.h  |   62 +++++++++++++++++++++++++++++++
 kernel/panic.c         |   96 +++++++++++++++++++++++++++++++++++++++++------
 4 files changed, 159 insertions(+), 13 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 328bca6..d73ae5c 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -162,6 +162,11 @@ extern struct atomic_notifier_head panic_notifier_list;
 extern long (*panic_blink)(long time);
 NORET_TYPE void panic(const char * fmt, ...)
 	__attribute__ ((NORET_AND format (printf, 1, 2))) __cold;
+NORET_TYPE void panic_with_regs(const struct pt_regs *regs,
+	const char *fmt, ...)
+	__attribute__ ((NORET_AND format (printf, 2, 3))) __cold;
+const struct pt_regs *get_panic_regs(void);
+extern const struct pt_regs *set_panic_regs(const struct pt_regs *regs);
 extern void oops_enter(void);
 extern void oops_exit(void);
 extern int oops_may_print(void);
diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 56f2d63..8f477ff 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -336,6 +336,15 @@ extern int task_current_syscall(struct task_struct *target, long *callno,
 				unsigned long args[6], unsigned int maxargs,
 				unsigned long *sp, unsigned long *pc);
 
+#ifndef arch_has_save_ptregs
+#warning save_ptregs undefined for this architecture; please define it soon
+
+static const struct pt_regs *save_ptregs(const struct pt_regs *regs)
+{
+	return NULL;
+}
+#endif
+
 #endif
 
 #endif
diff --git a/include/linux/ptreg.h b/include/linux/ptreg.h
new file mode 100644
index 0000000..f8cc2ea
--- /dev/null
+++ b/include/linux/ptreg.h
@@ -0,0 +1,62 @@
+/*
+ * Define macros for saving registers in a struct pt_regs structure
+ *
+ * Copyright (C) 2009 Cisco Systems, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef _LINUX_PTREG_H_
+#define _LINUX_PTREG_H_
+/*
+ * Define macros useful for writing save_ptregs(). Each architecture will
+ * need to define some macros, and some architectures have special cases
+ * that are most easily done as a special case. Macros that must be defined
+ * per architecture are:
+ *
+ * PTREG_SAVE - Save a register in a named location
+ * @r:		Name of the register to save
+ * @name:	%[name] of the location
+ *
+ * This is used in cases where the entry in the struct pt_regs is a simple
+ * string that doesn't end with a digit, e.g. pc.
+ *
+ * PTREG_INDIRECT_SAVE - move register to another register for saving
+ * @tmp_r:	Name of a temporary register
+ * @r:		Name of the register to be saved
+ * @name:	%[name] of the location
+ *
+ * This is used for cases where the register can't be saved directly,
+ * but must first be moved to another register that can be saved
+ * directly. The second register must already have been saved
+ * by the time this macro is used.
+ */
+
+/* Register save macro where the pt_regs item is an array, e.g. r[0] */
+#define _PTREG_SAVE_IDX(r, name, i)	PTREG_SAVE(r##i, name##i)
+
+/* Save register where the pt_regs item ends with a digit, e.g. r0. */
+#define _PTREG_SAVE_I(r, name, i)	PTREG_SAVE(r##i, name##i)
+
+/* Save register where it must first be copied to another register and where
+ * the pt_regs element ends with a digit, e.g. a2 */
+#define _PTREG_INDIRECT_SAVE_I(tmp, r, name, i) \
+					PTREG_INDIRECT_SAVE(tmp, r##i, name##i)
+
+/* Output specification */
+#define PTREG_OUT(rp, r, name)		[name] "=m" (rp->r)
+#define _PTREG_OUT_IDX(rp, r, name, i)	PTREG_OUT(rp, r[i], name##i)
+#define _PTREG_OUT_I(rp, r, name, i)	PTREG_OUT(rp, r##i, name##i)
+#endif
diff --git a/kernel/panic.c b/kernel/panic.c
index c787333..ef500c7 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -46,28 +46,22 @@ long (*panic_blink)(long time);
 EXPORT_SYMBOL(panic_blink);
 
 /**
- *	panic - halt the system
- *	@fmt: The text string to print
- *
- *	Display a message, then perform cleanups.
- *
- *	This function never returns.
+ * vpanic_with_regs - panic with specific register values and a argument array
+ * @regs:	Pointer to a &struct pt_reg with the register values
+ * @fmt:	Format for the panic message
+ * @args:	Arguments for the panic message as a va_list
  */
-NORET_TYPE void panic(const char * fmt, ...)
+static NORET_TYPE void vpanic_with_regs(const struct pt_regs *regs,
+	const char *fmt, va_list args)
 {
 	static char buf[1024];
-	va_list args;
 	long i;
-
 	/*
 	 * It's possible to come here directly from a panic-assertion and
 	 * not have preempt disabled. Some functions called from here want
 	 * preempt to be disabled. No point enabling it later though...
 	 */
-	preempt_disable();
-
-	bust_spinlocks(1);
-	va_start(args, fmt);
+	set_panic_regs(regs);
 	vsnprintf(buf, sizeof(buf), fmt, args);
 	va_end(args);
 	printk(KERN_EMERG "Kernel panic - not syncing: %s\n",buf);
@@ -143,8 +137,84 @@ NORET_TYPE void panic(const char * fmt, ...)
 	}
 }
 
+/* Registers stored in calls to panic() */
+static DEFINE_PER_CPU(struct pt_regs, panic_panic_regs);
+static DEFINE_PER_CPU(const struct pt_regs *, panic_regs);
+
+/**
+ * get_panic_regs - return the current pointer to panic register values
+ */
+const struct pt_regs *get_panic_regs()
+{
+	return __get_cpu_var(panic_regs);
+}
+EXPORT_SYMBOL(get_panic_regs);
+
+/**
+ * set_panic_regs - Set a pointer to the values of registers on panic()
+ * @new_regs:	Pointer to register values
+ *
+ * Returns: Pointer to the previous panic registers, if any.
+ */
+const struct pt_regs *set_panic_regs(const struct pt_regs *new_regs)
+{
+	const struct pt_regs *old_regs, **pp_regs;
+
+	pp_regs = &__get_cpu_var(panic_regs);
+	old_regs = *pp_regs;
+	*pp_regs = new_regs;
+	return old_regs;
+}
+
+/**
+ *	panic - halt the system
+ *	@fmt: The text string to print
+ *
+ *	Display a message, then perform cleanups.
+ *
+ *	This function never returns.
+ */
+NORET_TYPE void panic(const char *fmt, ...)
+{
+	va_list args;
+	const struct pt_regs *regs;
+	int i;
+
+	preempt_disable();
+	bust_spinlocks(1);
+	regs = save_ptregs(&__get_cpu_var(panic_panic_regs));
+	va_start(args, fmt);
+	vpanic_with_regs(regs, fmt, args);
+	/* Since vpanic_with_regs doesn't return, we skip va_end() */
+	/* Infinite loop so compiler doesn't complain about this returning */
+	for (i = 0; ; )
+		mdelay(1);
+}
 EXPORT_SYMBOL(panic);
 
+/**
+ * panic_with_regs - panic with specific register values
+ * @regs:	Pointer to a &struct pt_reg with the register values
+ * @fmt:	Format for the panic message
+ * @...:	Arguments for the panic message
+ */
+NORET_TYPE void panic_with_regs(const struct pt_regs *regs, const char *fmt,
+	...)
+{
+	va_list args;
+	int i;
+
+	preempt_disable();
+	bust_spinlocks(1);
+	va_start(args, fmt);
+	vpanic_with_regs(regs, fmt, args);
+	/* Since vpanic_with_regs doesn't return, we skip va_end() */
+	/* Infinite loop so compiler doesn't complain about this returning */
+	for (i = 0; ; )
+		mdelay(1);
+}
+EXPORT_SYMBOL(panic_with_regs);
+
 
 struct tnt {
 	u8	bit;

[PATCH 1/23] Make register values available to panic notifiers

[David VomLehn]

This patch makes panic() and die() registers available to, for example,
panic notifier functions.  Panic notifier functions are quite useful
for recording crash information, but they don't get passed the register
values. This makes it hard to print register contents, do stack
backtraces, etc. The changes in this patch save the register state when
panic() is called and introduce a function for die() to call that allows
it to pass in the registers it was passed.

Following this patch are more patches, one per architecture. These include
two types of changes:
o  A save_ptregs() function for the processor. I've taken a whack at
   doing this for all of the processors. I have tested x86 and MIPS
   versions. I was able to find cross compilers for ARM, ... and the
   code compiles cleanly. Everything else, well, what you see is sheer
   fantasy. You are welcome to chortle with merriment.
o  When I could figure it out, I replaced the calls to panic() in
   exception handling functions with calls to panic_with_regs() so
   that everyone can leverage these changes without much effort. Again,
   not all the code was transparent, so there are likely some places
   that should have additional work done.

Note that the pointer to the struct pt_regs may be NULL. This is to
accomodate those processors which don't have a working save_ptregs(). I'd
love to eliminate this case by providing a save_ptregs() for all
architectures, but I'll need help to so.

Signed-off-by: David VomLehn <dvomlehn@cisco.com>
---
 include/linux/kernel.h |    5 ++
 include/linux/ptrace.h |    9 ++++
 include/linux/ptreg.h  |   62 +++++++++++++++++++++++++++++++
 kernel/panic.c         |   96 +++++++++++++++++++++++++++++++++++++++++------
 4 files changed, 159 insertions(+), 13 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 328bca6..d73ae5c 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -162,6 +162,11 @@ extern struct atomic_notifier_head panic_notifier_list;
 extern long (*panic_blink)(long time);
 NORET_TYPE void panic(const char * fmt, ...)
 	__attribute__ ((NORET_AND format (printf, 1, 2))) __cold;
+NORET_TYPE void panic_with_regs(const struct pt_regs *regs,
+	const char *fmt, ...)
+	__attribute__ ((NORET_AND format (printf, 2, 3))) __cold;
+const struct pt_regs *get_panic_regs(void);
+extern const struct pt_regs *set_panic_regs(const struct pt_regs *regs);
 extern void oops_enter(void);
 extern void oops_exit(void);
 extern int oops_may_print(void);
diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 56f2d63..8f477ff 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -336,6 +336,15 @@ extern int task_current_syscall(struct task_struct *target, long *callno,
 				unsigned long args[6], unsigned int maxargs,
 				unsigned long *sp, unsigned long *pc);
 
+#ifndef arch_has_save_ptregs
+#warning save_ptregs undefined for this architecture; please define it soon
+
+static const struct pt_regs *save_ptregs(const struct pt_regs *regs)
+{
+	return NULL;
+}
+#endif
+
 #endif
 
 #endif
diff --git a/include/linux/ptreg.h b/include/linux/ptreg.h
new file mode 100644
index 0000000..f8cc2ea
--- /dev/null
+++ b/include/linux/ptreg.h
@@ -0,0 +1,62 @@
+/*
+ * Define macros for saving registers in a struct pt_regs structure
+ *
+ * Copyright (C) 2009 Cisco Systems, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef _LINUX_PTREG_H_
+#define _LINUX_PTREG_H_
+/*
+ * Define macros useful for writing save_ptregs(). Each architecture will
+ * need to define some macros, and some architectures have special cases
+ * that are most easily done as a special case. Macros that must be defined
+ * per architecture are:
+ *
+ * PTREG_SAVE - Save a register in a named location
+ * @r:		Name of the register to save
+ * @name:	%[name] of the location
+ *
+ * This is used in cases where the entry in the struct pt_regs is a simple
+ * string that doesn't end with a digit, e.g. pc.
+ *
+ * PTREG_INDIRECT_SAVE - move register to another register for saving
+ * @tmp_r:	Name of a temporary register
+ * @r:		Name of the register to be saved
+ * @name:	%[name] of the location
+ *
+ * This is used for cases where the register can't be saved directly,
+ * but must first be moved to another register that can be saved
+ * directly. The second register must already have been saved
+ * by the time this macro is used.
+ */
+
+/* Register save macro where the pt_regs item is an array, e.g. r[0] */
+#define _PTREG_SAVE_IDX(r, name, i)	PTREG_SAVE(r##i, name##i)
+
+/* Save register where the pt_regs item ends with a digit, e.g. r0. */
+#define _PTREG_SAVE_I(r, name, i)	PTREG_SAVE(r##i, name##i)
+
+/* Save register where it must first be copied to another register and where
+ * the pt_regs element ends with a digit, e.g. a2 */
+#define _PTREG_INDIRECT_SAVE_I(tmp, r, name, i) \
+					PTREG_INDIRECT_SAVE(tmp, r##i, name##i)
+
+/* Output specification */
+#define PTREG_OUT(rp, r, name)		[name] "=m" (rp->r)
+#define _PTREG_OUT_IDX(rp, r, name, i)	PTREG_OUT(rp, r[i], name##i)
+#define _PTREG_OUT_I(rp, r, name, i)	PTREG_OUT(rp, r##i, name##i)
+#endif
diff --git a/kernel/panic.c b/kernel/panic.c
index c787333..ef500c7 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -46,28 +46,22 @@ long (*panic_blink)(long time);
 EXPORT_SYMBOL(panic_blink);
 
 /**
- *	panic - halt the system
- *	@fmt: The text string to print
- *
- *	Display a message, then perform cleanups.
- *
- *	This function never returns.
+ * vpanic_with_regs - panic with specific register values and a argument array
+ * @regs:	Pointer to a &struct pt_reg with the register values
+ * @fmt:	Format for the panic message
+ * @args:	Arguments for the panic message as a va_list
  */
-NORET_TYPE void panic(const char * fmt, ...)
+static NORET_TYPE void vpanic_with_regs(const struct pt_regs *regs,
+	const char *fmt, va_list args)
 {
 	static char buf[1024];
-	va_list args;
 	long i;
-
 	/*
 	 * It's possible to come here directly from a panic-assertion and
 	 * not have preempt disabled. Some functions called from here want
 	 * preempt to be disabled. No point enabling it later though...
 	 */
-	preempt_disable();
-
-	bust_spinlocks(1);
-	va_start(args, fmt);
+	set_panic_regs(regs);
 	vsnprintf(buf, sizeof(buf), fmt, args);
 	va_end(args);
 	printk(KERN_EMERG "Kernel panic - not syncing: %s\n",buf);
@@ -143,8 +137,84 @@ NORET_TYPE void panic(const char * fmt, ...)
 	}
 }
 
+/* Registers stored in calls to panic() */
+static DEFINE_PER_CPU(struct pt_regs, panic_panic_regs);
+static DEFINE_PER_CPU(const struct pt_regs *, panic_regs);
+
+/**
+ * get_panic_regs - return the current pointer to panic register values
+ */
+const struct pt_regs *get_panic_regs()
+{
+	return __get_cpu_var(panic_regs);
+}
+EXPORT_SYMBOL(get_panic_regs);
+
+/**
+ * set_panic_regs - Set a pointer to the values of registers on panic()
+ * @new_regs:	Pointer to register values
+ *
+ * Returns: Pointer to the previous panic registers, if any.
+ */
+const struct pt_regs *set_panic_regs(const struct pt_regs *new_regs)
+{
+	const struct pt_regs *old_regs, **pp_regs;
+
+	pp_regs = &__get_cpu_var(panic_regs);
+	old_regs = *pp_regs;
+	*pp_regs = new_regs;
+	return old_regs;
+}
+
+/**
+ *	panic - halt the system
+ *	@fmt: The text string to print
+ *
+ *	Display a message, then perform cleanups.
+ *
+ *	This function never returns.
+ */
+NORET_TYPE void panic(const char *fmt, ...)
+{
+	va_list args;
+	const struct pt_regs *regs;
+	int i;
+
+	preempt_disable();
+	bust_spinlocks(1);
+	regs = save_ptregs(&__get_cpu_var(panic_panic_regs));
+	va_start(args, fmt);
+	vpanic_with_regs(regs, fmt, args);
+	/* Since vpanic_with_regs doesn't return, we skip va_end() */
+	/* Infinite loop so compiler doesn't complain about this returning */
+	for (i = 0; ; )
+		mdelay(1);
+}
 EXPORT_SYMBOL(panic);
 
+/**
+ * panic_with_regs - panic with specific register values
+ * @regs:	Pointer to a &struct pt_reg with the register values
+ * @fmt:	Format for the panic message
+ * @...:	Arguments for the panic message
+ */
+NORET_TYPE void panic_with_regs(const struct pt_regs *regs, const char *fmt,
+	...)
+{
+	va_list args;
+	int i;
+
+	preempt_disable();
+	bust_spinlocks(1);
+	va_start(args, fmt);
+	vpanic_with_regs(regs, fmt, args);
+	/* Since vpanic_with_regs doesn't return, we skip va_end() */
+	/* Infinite loop so compiler doesn't complain about this returning */
+	for (i = 0; ; )
+		mdelay(1);
+}
+EXPORT_SYMBOL(panic_with_regs);
+
 
 struct tnt {
 	u8	bit;

Re: [PATCH 1/23] Make register values available to panic notifiers

[Mike Frysinger]

On Mon, Apr 12, 2010 at 02:06, David VomLehn wrote:
> This patch makes panic() and die() registers available to, for example,
> panic notifier functions.  Panic notifier functions are quite useful
> for recording crash information, but they don't get passed the register
> values. This makes it hard to print register contents, do stack
> backtraces, etc. The changes in this patch save the register state when
> panic() is called and introduce a function for die() to call that allows
> it to pass in the registers it was passed.
>
> Following this patch are more patches, one per architecture. These include
> two types of changes:
> o  A save_ptregs() function for the processor. I've taken a whack at
>   doing this for all of the processors. I have tested x86 and MIPS
>   versions. I was able to find cross compilers for ARM, ... and the
>   code compiles cleanly. Everything else, well, what you see is sheer
>   fantasy. You are welcome to chortle with merriment.

could you post a sample module that you're using to test with here ?
presumably you have some simple code that registers a notify handler
and then calls panic_with_regs() ...
-mike

Re: [PATCH 1/23] Make register values available to panic notifiers

[Mike Frysinger]

On Mon, Apr 12, 2010 at 02:06, David VomLehn wrote:
> This patch makes panic() and die() registers available to, for example,
> panic notifier functions.  Panic notifier functions are quite useful
> for recording crash information, but they don't get passed the register
> values. This makes it hard to print register contents, do stack
> backtraces, etc. The changes in this patch save the register state when
> panic() is called and introduce a function for die() to call that allows
> it to pass in the registers it was passed.
>
> Following this patch are more patches, one per architecture. These include
> two types of changes:
> o  A save_ptregs() function for the processor. I've taken a whack at
>   doing this for all of the processors. I have tested x86 and MIPS
>   versions. I was able to find cross compilers for ARM, ... and the
>   code compiles cleanly. Everything else, well, what you see is sheer
>   fantasy. You are welcome to chortle with merriment.

could you post a sample module that you're using to test with here ?
presumably you have some simple code that registers a notify handler
and then calls panic_with_regs() ...
-mike

Re: [PATCH 1/23] Make register values available to panic notifiers

[David Howells]

David VomLehn <dvomlehn@cisco.com> wrote:

> +NORET_TYPE void panic(const char *fmt, ...)
> +{
> +	va_list args;
> +	const struct pt_regs *regs;
> +	int i;
> +
> +	preempt_disable();
> +	bust_spinlocks(1);
> +	regs = save_ptregs(&__get_cpu_var(panic_panic_regs));
> +	va_start(args, fmt);
> +	vpanic_with_regs(regs, fmt, args);
> +	/* Since vpanic_with_regs doesn't return, we skip va_end() */
> +	/* Infinite loop so compiler doesn't complain about this returning */
> +	for (i = 0; ; )
> +		mdelay(1);
> +}

Can the use of va_start() clobber lots of registers, thereby rendering the
exercise pointless on some arches?

Also, can the save_ptregs() function be out of line asm?  The FRV constructed
inline statement is huge (and wrong).

David

Re: [PATCH 1/23] Make register values available to panic notifiers

[David Howells]

David VomLehn <dvomlehn@cisco.com> wrote:

> +NORET_TYPE void panic(const char *fmt, ...)
> +{
> +	va_list args;
> +	const struct pt_regs *regs;
> +	int i;
> +
> +	preempt_disable();
> +	bust_spinlocks(1);
> +	regs = save_ptregs(&__get_cpu_var(panic_panic_regs));
> +	va_start(args, fmt);
> +	vpanic_with_regs(regs, fmt, args);
> +	/* Since vpanic_with_regs doesn't return, we skip va_end() */
> +	/* Infinite loop so compiler doesn't complain about this returning */
> +	for (i = 0; ; )
> +		mdelay(1);
> +}

Can the use of va_start() clobber lots of registers, thereby rendering the
exercise pointless on some arches?

Also, can the save_ptregs() function be out of line asm?  The FRV constructed
inline statement is huge (and wrong).

David

Re: [PATCH 1/23] Make register values available to panic notifiers

[Heiko Carstens]

On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
> This patch makes panic() and die() registers available to, for example,
> panic notifier functions.  Panic notifier functions are quite useful
> for recording crash information, but they don't get passed the register
> values. This makes it hard to print register contents, do stack
> backtraces, etc. The changes in this patch save the register state when
> panic() is called and introduce a function for die() to call that allows
> it to pass in the registers it was passed.
> 
> Following this patch are more patches, one per architecture. These include
> two types of changes:
> o  A save_ptregs() function for the processor. I've taken a whack at
>    doing this for all of the processors. I have tested x86 and MIPS
>    versions. I was able to find cross compilers for ARM, ... and the
>    code compiles cleanly. Everything else, well, what you see is sheer
>    fantasy. You are welcome to chortle with merriment.
> o  When I could figure it out, I replaced the calls to panic() in
>    exception handling functions with calls to panic_with_regs() so
>    that everyone can leverage these changes without much effort. Again,
>    not all the code was transparent, so there are likely some places
>    that should have additional work done.
> 
> Note that the pointer to the struct pt_regs may be NULL. This is to
> accomodate those processors which don't have a working save_ptregs(). I'd
> love to eliminate this case by providing a save_ptregs() for all
> architectures, but I'll need help to so.

Wouldn't it be much easier to implement panic with an illegal op and let
the exception handler set up the pt regs structure instead? Just like some
architectures do that already for warnings.
Have a look at lib/bug.c and at various arch/<...>/include/asm/bug.h.
BUG_FLAG_PANIC would do the trick.

But I'm still wondering what the use case would be. You haven't posted any
code that would actually use this.

Re: [PATCH 1/23] Make register values available to panic notifiers

[Heiko Carstens]

On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
> This patch makes panic() and die() registers available to, for example,
> panic notifier functions.  Panic notifier functions are quite useful
> for recording crash information, but they don't get passed the register
> values. This makes it hard to print register contents, do stack
> backtraces, etc. The changes in this patch save the register state when
> panic() is called and introduce a function for die() to call that allows
> it to pass in the registers it was passed.
> 
> Following this patch are more patches, one per architecture. These include
> two types of changes:
> o  A save_ptregs() function for the processor. I've taken a whack at
>    doing this for all of the processors. I have tested x86 and MIPS
>    versions. I was able to find cross compilers for ARM, ... and the
>    code compiles cleanly. Everything else, well, what you see is sheer
>    fantasy. You are welcome to chortle with merriment.
> o  When I could figure it out, I replaced the calls to panic() in
>    exception handling functions with calls to panic_with_regs() so
>    that everyone can leverage these changes without much effort. Again,
>    not all the code was transparent, so there are likely some places
>    that should have additional work done.
> 
> Note that the pointer to the struct pt_regs may be NULL. This is to
> accomodate those processors which don't have a working save_ptregs(). I'd
> love to eliminate this case by providing a save_ptregs() for all
> architectures, but I'll need help to so.

Wouldn't it be much easier to implement panic with an illegal op and let
the exception handler set up the pt regs structure instead? Just like some
architectures do that already for warnings.
Have a look at lib/bug.c and at various arch/<...>/include/asm/bug.h.
BUG_FLAG_PANIC would do the trick.

But I'm still wondering what the use case would be. You haven't posted any
code that would actually use this.

Re: [PATCH 1/23] Make register values available to panic notifiers

[David Howells]

Heiko Carstens <heiko.carstens@de.ibm.com> wrote:

> Wouldn't it be much easier to implement panic with an illegal op and let
> the exception handler set up the pt regs structure instead? Just like some
> architectures do that already for warnings.

No, not necessarily.  panic() is varargs.

David

Re: [PATCH 1/23] Make register values available to panic notifiers

[David Howells]

Heiko Carstens <heiko.carstens@de.ibm.com> wrote:

> Wouldn't it be much easier to implement panic with an illegal op and let
> the exception handler set up the pt regs structure instead? Just like some
> architectures do that already for warnings.

No, not necessarily.  panic() is varargs.

David

Re: [PATCH 1/23] Make register values available to panic notifiers

[Russell King]

On Mon, Apr 12, 2010 at 02:03:30PM +0200, Heiko Carstens wrote:
> Wouldn't it be much easier to implement panic with an illegal op and let
> the exception handler set up the pt regs structure instead? Just like some
> architectures do that already for warnings.
> Have a look at lib/bug.c and at various arch/<...>/include/asm/bug.h.
> BUG_FLAG_PANIC would do the trick.

You also have the problem that there are panic() statements before
exception vectors are setup.

Eg, using lmb, you might want to allocate a page for a L2 page table
so you can setup the exception vectors, but the lmb allocator has a
panic statement which will be used on failure to allocate a page.

The result is that you don't know why you didn't boot since there's no
diagnostics from the kernel.

At least with the current setup, merely (re)directing the kernel printk
output results in something you can read.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH 1/23] Make register values available to panic notifiers

[Russell King]

On Mon, Apr 12, 2010 at 02:03:30PM +0200, Heiko Carstens wrote:
> Wouldn't it be much easier to implement panic with an illegal op and let
> the exception handler set up the pt regs structure instead? Just like some
> architectures do that already for warnings.
> Have a look at lib/bug.c and at various arch/<...>/include/asm/bug.h.
> BUG_FLAG_PANIC would do the trick.

You also have the problem that there are panic() statements before
exception vectors are setup.

Eg, using lmb, you might want to allocate a page for a L2 page table
so you can setup the exception vectors, but the lmb allocator has a
panic statement which will be used on failure to allocate a page.

The result is that you don't know why you didn't boot since there's no
diagnostics from the kernel.

At least with the current setup, merely (re)directing the kernel printk
output results in something you can read.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH 1/23] Make register values available to panic notifiers

[Russell King]

On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
> This patch makes panic() and die() registers available to, for example,
> panic notifier functions.  Panic notifier functions are quite useful
> for recording crash information, but they don't get passed the register
> values. This makes it hard to print register contents, do stack
> backtraces, etc. The changes in this patch save the register state when
> panic() is called and introduce a function for die() to call that allows
> it to pass in the registers it was passed.

Can you explain why you want this?

I'm wondering about the value of saving the registers; normally when a panic
occurs, it's because of a well defined reason, and not because something
went wrong in some CPU register; to put it another way, a panic() is a
more controlled exception than a BUG() or a bad pointer dereference.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH 1/23] Make register values available to panic notifiers

[Russell King]

On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
> This patch makes panic() and die() registers available to, for example,
> panic notifier functions.  Panic notifier functions are quite useful
> for recording crash information, but they don't get passed the register
> values. This makes it hard to print register contents, do stack
> backtraces, etc. The changes in this patch save the register state when
> panic() is called and introduce a function for die() to call that allows
> it to pass in the registers it was passed.

Can you explain why you want this?

I'm wondering about the value of saving the registers; normally when a panic
occurs, it's because of a well defined reason, and not because something
went wrong in some CPU register; to put it another way, a panic() is a
more controlled exception than a BUG() or a bad pointer dereference.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH 1/23] Make register values available to panic notifiers

[David Howells]

Russell King <rmk@arm.linux.org.uk> wrote:

> Can you explain why you want this?
> 
> I'm wondering about the value of saving the registers; normally when a panic
> occurs, it's because of a well defined reason, and not because something
> went wrong in some CPU register; to put it another way, a panic() is a
> more controlled exception than a BUG() or a bad pointer dereference.

+1.

I found in FS-Cache and CacheFiles that often the things I most wanted to know
when I had something of the form:

	if (A == B)
		BUG();

was a and b, so I made the following macro:

	#define ASSERTCMP(X, OP, Y)					\
	do {								\
		if (unlikely(!((X) OP (Y)))) {				\
			printk(KERN_ERR "\n");				\
			printk(KERN_ERR "AFS: Assertion failed\n");	\
			printk(KERN_ERR "%lu " #OP " %lu is false\n",	\
			       (unsigned long)(X), (unsigned long)(Y));	\
			printk(KERN_ERR "0x%lx " #OP " 0x%lx is false\n", \
			       (unsigned long)(X), (unsigned long)(Y));	\
			BUG();						\
		}							\
	} while(0)

which I could then call like this:

	ASSERTCMP(A, ==, B);

and if the assertion failed, it prints A and B explicitly.  This is much
easier than trying to pick the values out of a register dump, especially as
the compiler may be free to clobber A or B immediately after testing them.

David

Re: [PATCH 1/23] Make register values available to panic notifiers

[Martin Schwidefsky]

On Mon, 12 Apr 2010 13:27:45 +0100
Russell King <rmk@arm.linux.org.uk> wrote:

> On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
> > This patch makes panic() and die() registers available to, for example,
> > panic notifier functions.  Panic notifier functions are quite useful
> > for recording crash information, but they don't get passed the register
> > values. This makes it hard to print register contents, do stack
> > backtraces, etc. The changes in this patch save the register state when
> > panic() is called and introduce a function for die() to call that allows
> > it to pass in the registers it was passed.
> 
> Can you explain why you want this?
> 
> I'm wondering about the value of saving the registers; normally when a panic
> occurs, it's because of a well defined reason, and not because something
> went wrong in some CPU register; to put it another way, a panic() is a
> more controlled exception than a BUG() or a bad pointer dereference.

I'm curious about the potential use case as well. So far I only wanted
to know the registers if the panic has been triggered due to an
unexpected fault with panic_on_oops=1 or in_interrupt()==1. If that
happens the die() handler prints the registers. An open coded panic is
easy to analyze, imho no need for the registers.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH 1/23] Make register values available to panic notifiers

[Martin Schwidefsky]

On Mon, 12 Apr 2010 13:27:45 +0100
Russell King <rmk@arm.linux.org.uk> wrote:

> On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
> > This patch makes panic() and die() registers available to, for example,
> > panic notifier functions.  Panic notifier functions are quite useful
> > for recording crash information, but they don't get passed the register
> > values. This makes it hard to print register contents, do stack
> > backtraces, etc. The changes in this patch save the register state when
> > panic() is called and introduce a function for die() to call that allows
> > it to pass in the registers it was passed.
> 
> Can you explain why you want this?
> 
> I'm wondering about the value of saving the registers; normally when a panic
> occurs, it's because of a well defined reason, and not because something
> went wrong in some CPU register; to put it another way, a panic() is a
> more controlled exception than a BUG() or a bad pointer dereference.

I'm curious about the potential use case as well. So far I only wanted
to know the registers if the panic has been triggered due to an
unexpected fault with panic_on_oops=1 or in_interrupt()==1. If that
happens the die() handler prints the registers. An open coded panic is
easy to analyze, imho no need for the registers.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH 1/23] Make register values available to panic notifiers

[David VomLehn]

David Howells wrote:
> David VomLehn <dvomlehn@cisco.com> wrote:
>
>   
>> +NORET_TYPE void panic(const char *fmt, ...)
>> +{
>> +	va_list args;
>> +	const struct pt_regs *regs;
>> +	int i;
>> +
>> +	preempt_disable();
>> +	bust_spinlocks(1);
>> +	regs = save_ptregs(&__get_cpu_var(panic_panic_regs));
>> +	va_start(args, fmt);
>> +	vpanic_with_regs(regs, fmt, args);
>> +	/* Since vpanic_with_regs doesn't return, we skip va_end() */
>> +	/* Infinite loop so compiler doesn't complain about this returning */
>> +	for (i = 0; ; )
>> +		mdelay(1);
>> +}
>>     
>
> Can the use of va_start() clobber lots of registers, thereby rendering the
> exercise pointless on some arches?
>   
The implementations I'm familiar with only need one or two registers. 
What it
*does* do is to force the contents of registers being used to pass 
argument values
onto the stack. This is roughly what gcc does for asm() statements when you
tell it registers are clobbered.
> Also, can the save_ptregs() function be out of line asm?  The FRV constructed
> inline statement is huge (and wrong).
>   

With this implementation it has to be inline. One use of the saved 
registers is to
backtrace the stack. If  you call a function to save the registers, the 
stack pointer
and program counter would be those of the called function, which will not be
valid after it returns. I expect that you could come up with an alternative
out-of-line function--on every processor I know, you could backtrace one 
frame
to get reasonable values for those registers,. Unfortunately, you would 
run the
risk of clobbering other registers by doing the function call. The more you
change register values from those in the function that calls panic(), 
the less
useful this becomes. In this case, I think an inline function is worth the
effort to get working. (I'd be interested in know more details about how
tshings are broken in the FRV)
> David
>   

Re: [PATCH 1/23] Make register values available to panic notifiers

[David VomLehn]

David Howells wrote:
> David VomLehn <dvomlehn@cisco.com> wrote:
>
>   
>> +NORET_TYPE void panic(const char *fmt, ...)
>> +{
>> +	va_list args;
>> +	const struct pt_regs *regs;
>> +	int i;
>> +
>> +	preempt_disable();
>> +	bust_spinlocks(1);
>> +	regs = save_ptregs(&__get_cpu_var(panic_panic_regs));
>> +	va_start(args, fmt);
>> +	vpanic_with_regs(regs, fmt, args);
>> +	/* Since vpanic_with_regs doesn't return, we skip va_end() */
>> +	/* Infinite loop so compiler doesn't complain about this returning */
>> +	for (i = 0; ; )
>> +		mdelay(1);
>> +}
>>     
>
> Can the use of va_start() clobber lots of registers, thereby rendering the
> exercise pointless on some arches?
>   
The implementations I'm familiar with only need one or two registers. 
What it
*does* do is to force the contents of registers being used to pass 
argument values
onto the stack. This is roughly what gcc does for asm() statements when you
tell it registers are clobbered.
> Also, can the save_ptregs() function be out of line asm?  The FRV constructed
> inline statement is huge (and wrong).
>   

With this implementation it has to be inline. One use of the saved 
registers is to
backtrace the stack. If  you call a function to save the registers, the 
stack pointer
and program counter would be those of the called function, which will not be
valid after it returns. I expect that you could come up with an alternative
out-of-line function--on every processor I know, you could backtrace one 
frame
to get reasonable values for those registers,. Unfortunately, you would 
run the
risk of clobbering other registers by doing the function call. The more you
change register values from those in the function that calls panic(), 
the less
useful this becomes. In this case, I think an inline function is worth the
effort to get working. (I'd be interested in know more details about how
tshings are broken in the FRV)
> David
>   

Re: [PATCH 1/23] Make register values available to panic notifiers

[David VomLehn]

Heiko Carstens wrote:
> On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
>   
>> This patch makes panic() and die() registers available to, for example,
>> panic notifier functions.  Panic notifier functions are quite useful
>> for recording crash information, but they don't get passed the register
>> values. This makes it hard to print register contents, do stack
>> backtraces, etc. The changes in this patch save the register state when
>> panic() is called and introduce a function for die() to call that allows
>> it to pass in the registers it was passed.
>>
>> Following this patch are more patches, one per architecture. These include
>> two types of changes:
>> o  A save_ptregs() function for the processor. I've taken a whack at
>>    doing this for all of the processors. I have tested x86 and MIPS
>>    versions. I was able to find cross compilers for ARM, ... and the
>>    code compiles cleanly. Everything else, well, what you see is sheer
>>    fantasy. You are welcome to chortle with merriment.
>> o  When I could figure it out, I replaced the calls to panic() in
>>    exception handling functions with calls to panic_with_regs() so
>>    that everyone can leverage these changes without much effort. Again,
>>    not all the code was transparent, so there are likely some places
>>    that should have additional work done.
>>
>> Note that the pointer to the struct pt_regs may be NULL. This is to
>> accomodate those processors which don't have a working save_ptregs(). I'd
>> love to eliminate this case by providing a save_ptregs() for all
>> architectures, but I'll need help to so.
>>     
>
> Wouldn't it be much easier to implement panic with an illegal op and let
> the exception handler set up the pt regs structure instead? Just like some
> architectures do that already for warnings.
> Have a look at lib/bug.c and at various arch/<...>/include/asm/bug.h.
> BUG_FLAG_PANIC would do the trick.
>   
You could do this so long as your exception handler wasn't compromised. When
you get here, your system is already known to have failed, so the idea 
is to be
as gentle as possible.
> But I'm still wondering what the use case would be. You haven't posted any
> code that would actually use this.
>   
I'm working my way towards this, but the general idea is to be able to 
log all system
state from within a panic handler. So, you'd want to print the 
registers, the stack,
the window of memory around instructions, the stack trace, etc., all of 
which need
some set of the register values.

Re: [PATCH 1/23] Make register values available to panic notifiers

[David VomLehn]

Russell King wrote:
> On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
>   
>> This patch makes panic() and die() registers available to, for example,
>> panic notifier functions.  Panic notifier functions are quite useful
>> for recording crash information, but they don't get passed the register
>> values. This makes it hard to print register contents, do stack
>> backtraces, etc. The changes in this patch save the register state when
>> panic() is called and introduce a function for die() to call that allows
>> it to pass in the registers it was passed.
>>     
>
> Can you explain why you want this?
>
> I'm wondering about the value of saving the registers; normally when a panic
> occurs, it's because of a well defined reason, and not because something
> went wrong in some CPU register; to put it another way, a panic() is a
> more controlled exception than a BUG() or a bad pointer dereference.
>   
More context probably helps, starting with noting that the platform is 
an embedded
one. (I'm at the Eembedded Linux Conference, which is why my reply is so 
tardy).
In embedded systems we frequently (probably?) don't have the resources 
to create or
store a crash dump. A very common approach is to record a subset of the 
system
state that is likely to help diagnose the failure. The state information 
is then stored\
on the system or sent upstream to some network-connected node. When I 
gave a talk
about this at the ELC, I polled the audience and got at least half a 
dozen other
companies using the same approach. This is the first set of patches to 
allow this
common embedded community requirement to be met. My expectation that 
register
values will be wanted by everyone doing this kind of targeted state 
reporting.

I should also note that I regard this is more the beginning of a 
conversation on how
to diagnose kernel (and, possibly, application) failures on crash 
dump-less systems.
I would expect to see some number of patches, some of which will 
ultimately be
dropped in favor of other approaches.

Re: [PATCH 1/23] Make register values available to panic notifiers

[David VomLehn]

David Howells wrote:
> Russell King <rmk@arm.linux.org.uk> wrote:
>
>   
>> Can you explain why you want this?
>>
>> I'm wondering about the value of saving the registers; normally when a panic
>> occurs, it's because of a well defined reason, and not because something
>> went wrong in some CPU register; to put it another way, a panic() is a
>> more controlled exception than a BUG() or a bad pointer dereference.
>>     
>
> +1.
>
> I found in FS-Cache and CacheFiles that often the things I most wanted to know
> when I had something of the form:
>
> 	if (A == B)
> 		BUG();
>
> was a and b, so I made the following macro:
>
> 	#define ASSERTCMP(X, OP, Y)					\
> 	do {								\
> 		if (unlikely(!((X) OP (Y)))) {				\
> 			printk(KERN_ERR "\n");				\
> 			printk(KERN_ERR "AFS: Assertion failed\n");	\
> 			printk(KERN_ERR "%lu " #OP " %lu is false\n",	\
> 			       (unsigned long)(X), (unsigned long)(Y));	\
> 			printk(KERN_ERR "0x%lx " #OP " 0x%lx is false\n", \
> 			       (unsigned long)(X), (unsigned long)(Y));	\
> 			BUG();						\
> 		}							\
> 	} while(0)
>
> which I could then call like this:
>
> 	ASSERTCMP(A, ==, B);
>
> and if the assertion failed, it prints A and B explicitly.  This is much
> easier than trying to pick the values out of a register dump, especially as
> the compiler may be free to clobber A or B immediately after testing them.
>   
This is great if you'r in a development environment, and can focus on a 
single, well
characterized case. Unfortunately, I'm staring at hundreds of thousands 
of systems
in the field, all which which have a large number of panic() statements 
for which this
approach has not been taken. So, I have no alternative but to pick the 
value out of
a register dump.
> David
>   

Re: [PATCH 1/23] Make register values available to panic notifiers

[David VomLehn]

Martin Schwidefsky wrote:
> On Mon, 12 Apr 2010 13:27:45 +0100
> Russell King <rmk@arm.linux.org.uk> wrote:
>
>   
>> On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
>>     
>>> This patch makes panic() and die() registers available to, for example,
>>> panic notifier functions.  Panic notifier functions are quite useful
>>> for recording crash information, but they don't get passed the register
>>> values. This makes it hard to print register contents, do stack
>>> backtraces, etc. The changes in this patch save the register state when
>>> panic() is called and introduce a function for die() to call that allows
>>> it to pass in the registers it was passed.
>>>       
>> Can you explain why you want this?
>>
>> I'm wondering about the value of saving the registers; normally when a panic
>> occurs, it's because of a well defined reason, and not because something
>> went wrong in some CPU register; to put it another way, a panic() is a
>> more controlled exception than a BUG() or a bad pointer dereference.
>>     
>
> I'm curious about the potential use case as well. So far I only wanted
> to know the registers if the panic has been triggered due to an
> unexpected fault with panic_on_oops=1 or in_interrupt()==1. If that
> happens the die() handler prints the registers. An open coded panic is
> easy to analyze, imho no need for the registers
>   

Good example, because helps focus the issue. In recording a subset of 
kernel state
information from an embedded system for collection at a central point. 
The register
values printed by die() are printed to the console, where they 
disappear. One of the
things in this patch involves passed a pointer to those die() registers 
to a register
panic notifier handler. So, there is a path to where panic handlers are 
called from
die() and another one from panic() and this patch makes register values 
available in
both cases.

Re: [PATCH 1/23] Make register values available to panic notifiers

[David Howells]

David VomLehn <dvomlehn@cisco.com> wrote:

> > Can the use of va_start() clobber lots of registers, thereby rendering the
> > exercise pointless on some arches?
> >   
>
> The implementations I'm familiar with only need one or two registers. What
> it *does* do is to force the contents of registers being used to pass
> argument values onto the stack. This is roughly what gcc does for asm()
> statements when you tell it registers are clobbered.

How about something like Sparc, where you can pass up to 8 arguments (if I
remember correctly) in registers.  I'm not sure how Sparc handles varargs
functions, though.

> > Also, can the save_ptregs() function be out of line asm?  The FRV
> > constructed inline statement is huge (and wrong).
> 
> With this implementation it has to be inline. One use of the saved registers
> is to backtrace the stack.

Indeed, but you've probably already lost that by using va_start().

> If you call a function to save the registers, the stack pointer and program
> counter would be those of the called function, which will not be valid after
> it returns.

Well, they'll be the context of panic() in your implementation.

> I expect that you could come up with an alternative out-of-line function

Yes.  The easiest way might be to write the saver in assembly and call it from
within an inline asm statement.

> --on every processor I know, you could backtrace one frame to get reasonable
> values for those registers,. Unfortunately, you would run the risk of
> clobbering other registers by doing the function call. The more you change
> register values from those in the function that calls panic(), the less
> useful this becomes.

Indeed, but the more things panic() does, the more likely it is to clobber
registers anyway.

Note, also: panic() is __attribute__((noreturn)), which means that the
compiler calling it is not required to save the return address or registers
before jumping to panic() or even in panic() itself.

> In this case, I think an inline function is worth the effort to get working.

As I mentioned above, you can use asm for this.  For instance, you can write
an inline asm statement that saves onto the stack the registers that need to
be clobbered to make a jump, then make the jump, and then have the saver
routine retrieve the register values from the stack and place them in the
storage area.

In fact, you could insert a prologue wrapper on panic() with a bit of asm to
save the registers, for example on FRV:

	panic:
		subi	sp,#-8,sp
		stdi.p	gr4,@(sp,#0)		# save GR4/GR5 on stack
		addi	sp,#8,gr5
		sethi.p	%hi(__panic_reg_save),gr4  # get the save space addr
		setlo	%lo(__panic_reg_save),gr4
		sti	gr5,@(gr4,#REG_SP))	# save orig stack pointer
		stdi	gr2,@(gr4,#REG_GR(2))	# save GR2/GR3
		ldi	@(sp,#0),gr5
		sti	gr5,@(gr4,#REG_GR(4))	# save orig GR4
		ldi	@(sp,#4),gr5
		sti	gr5,@(gr4,#REG_GR(5))	# save orig GR5
		stdi	gr6,@(gr4,#REG_GR(6))	# save GR6/GR7
		stdi	gr8,@(gr4,#REG_GR(8))	# save GR8/GR9
		...
		lddi.p	@(sp,#0),gr4		# restore GR4/GR5 from stack
		addi	sp,#8,sp
		bra	real_panic		# chain

Then real_panic() would be the original C panic function.

> (I'd be interested in know more details about how things are broken in the
> FRV)

Most load/store instructions come in two types, and you need to modify the
opcode according to the addressing mode and indicate that you're interpolating
a memory dereference argument rather than an address:

	asm("ldd%I1 %M1,%0"
	    : "=e"(counter)
	    : "m"(v->counter));

You're also trying to load data into GR0 which won't achieve anything.  GR0 is
hardwired to 0.  It's used as the target of instructions where you don't care
about the calculated result (eg: compare is implemented as subtract to GR0),
and as a source of 0.

David

Re: [PATCH 1/23] Make register values available to panic notifiers

[David Miller]

From: David Howells <dhowells@redhat.com>
Date: Thu, 15 Apr 2010 00:52:14 +0100

> David VomLehn <dvomlehn@cisco.com> wrote:
> 
>> > Can the use of va_start() clobber lots of registers, thereby rendering the
>> > exercise pointless on some arches?
>> >   
>>
>> The implementations I'm familiar with only need one or two registers. What
>> it *does* do is to force the contents of registers being used to pass
>> argument values onto the stack. This is roughly what gcc does for asm()
>> statements when you tell it registers are clobbered.
> 
> How about something like Sparc, where you can pass up to 8 arguments (if I
> remember correctly) in registers.  I'm not sure how Sparc handles varargs
> functions, though.

6 arguments, and all arguments get popped onto the stack into the
argument save area when doing varargs so you can access them as an
array.

Stack looks like:

	struct register_window	window;
	unsigned long args[...];

Re: [PATCH 1/23] Make register values available to panic notifiers

[Paul Mundt]

On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
> o  A save_ptregs() function for the processor. I've taken a whack at
>    doing this for all of the processors. I have tested x86 and MIPS
>    versions. I was able to find cross compilers for ARM, ... and the
>    code compiles cleanly. Everything else, well, what you see is sheer
>    fantasy. You are welcome to chortle with merriment.

Perhaps I'm missing something obvious, but is there some reason why you
can't just reuse the crash_setup_regs() code? MIPS doesn't implement it
presumably because it's lacking kexec crash kernel support, but it would
make sense to make current ptregs saving more generic if there are going
to be multiple users for it.

Re: [PATCH 1/23] Make register values available to panic notifiers

[David VomLehn]

Andrew Morton wrote:
> On Sun, 11 Apr 2010 23:03:38 -0700
> David VomLehn <dvomlehn@cisco.com> wrote:
>
>   
>> This patch makes panic() and die() registers available to, for example,
>> panic notifier functions.  Panic notifier functions are quite useful
>> for recording crash information, but they don't get passed the register
>> values. This makes it hard to print register contents, do stack
>> backtraces, etc. The changes in this patch save the register state when
>> panic() is called and introduce a function for die() to call that allows
>> it to pass in the registers it was passed.
>>
>> Following this patch are more patches, one per architecture. These include
>> two types of changes:
>> o  A save_ptregs() function for the processor. I've taken a whack at
>>    doing this for all of the processors. I have tested x86 and MIPS
>>    versions. I was able to find cross compilers for ARM, ... and the
>>    code compiles cleanly. Everything else, well, what you see is sheer
>>    fantasy. You are welcome to chortle with merriment.
>> o  When I could figure it out, I replaced the calls to panic() in
>>    exception handling functions with calls to panic_with_regs() so
>>    that everyone can leverage these changes without much effort. Again,
>>    not all the code was transparent, so there are likely some places
>>    that should have additional work done.
>>
>> Note that the pointer to the struct pt_regs may be NULL. This is to
>> accomodate those processors which don't have a working save_ptregs(). I'd
>> love to eliminate this case by providing a save_ptregs() for all
>> architectures, but I'll need help to so.
>>
>>     
>
> It would make life easier if you could describe (or send) a means by
> which arch maintainers can easily test these changes.
>   
Great idea. It should be pretty easy to brew up an LKM to do this.
>> --- a/kernel/panic.c
>> +++ b/kernel/panic.c
>>
>> ...
>>
>> +/* Registers stored in calls to panic() */
>> +static DEFINE_PER_CPU(struct pt_regs, panic_panic_regs);
>> +static DEFINE_PER_CPU(const struct pt_regs *, panic_regs);
>> +
>> +/**
>> + * get_panic_regs - return the current pointer to panic register values
>> + */
>> +const struct pt_regs *get_panic_regs()
>> +{
>> +	return __get_cpu_var(panic_regs);
>> +}
>> +EXPORT_SYMBOL(get_panic_regs);
>> +
>> +/**
>> + * set_panic_regs - Set a pointer to the values of registers on panic()
>> + * @new_regs:	Pointer to register values
>> + *
>> + * Returns: Pointer to the previous panic registers, if any.
>> + */
>> +const struct pt_regs *set_panic_regs(const struct pt_regs *new_regs)
>> +{
>> +	const struct pt_regs *old_regs, **pp_regs;
>> +
>> +	pp_regs = &__get_cpu_var(panic_regs);
>> +	old_regs = *pp_regs;
>> +	*pp_regs = new_regs;
>> +	return old_regs;
>> +}
>>     
>
> What's going on here?  We define storage for a set of pt_regs and also
> storage for a set of pt_regs pointers, and provide the ability for
> callers to rewrite the thing which the pt_regs*'s point at.
>   
It's a stack of pt_regs. It's not on the processor's stack since that 
would use a fair
amount of memory. In this way, it is possible to construct code to handle
nested exceptions. Since, on some processors, interrupts are handle the 
same as
other exceptions, so nested exceptions are fairly common. On the other hand,
if the consensus is that this is not going to be used, I'm find with 
just keeping
around a pointer.

It should be possible to have an interface which doesn't preclude 
pt_regs stacks
but which is simpler, so I'll shoot for that.
> Seems complex.  Why not simply provide a set of pt_regs and permit
> callers to copy their own pt_regs sets into that area?
>   
I was trying to avoid overflowing the panic-time stack with pt_regs. My 
32-bit
MIPS pt_regs is around 160 bytes. I think the 64-bit MIPS pt_regs is about
twice that. That's enough that it gave me pause. But other points of 
view would
be helpful.
> Secondly, this code implicitly assumes that the panicing code is pinned
> to the panicing CPU and cannot be preempted and migrated to a different
> CPU.  Is that true - do we take steps to ensure this anywhere?
>   
The get_panic_regs() functions is intended to be called only from panic 
notifier
functions. In the patch, these are now called from vpanic_with_regs, which
is called from panic() and panic_with_regs(), both of which disable
preemption, so I think the code won't slip off the right processor. Assuming
that's right, I can make it clearer that get_panic_regs() should only be
called from a panic notifier function. Enforcing such a restriction from
a panic notifier function seems fruitless--what would I do, panic?
> Thirdly and relatedly, the code assumes that callers have disabled
> preemption (otherwise __get_cpu_var->smp_processor_id() will whine). 
> Where does this get reliably assured?
>   
I think this is covered above, with the caveat that it really has to be 
plain
that you shouldn't call get_panic_regs() unless you are in a panic notifier
function.
--
David VL

Re: [PATCH 1/23] Make register values available to panic notifiers

[Andrew Morton]

On Sun, 11 Apr 2010 23:03:38 -0700
David VomLehn <dvomlehn@cisco.com> wrote:

> This patch makes panic() and die() registers available to, for example,
> panic notifier functions.  Panic notifier functions are quite useful
> for recording crash information, but they don't get passed the register
> values. This makes it hard to print register contents, do stack
> backtraces, etc. The changes in this patch save the register state when
> panic() is called and introduce a function for die() to call that allows
> it to pass in the registers it was passed.
> 
> Following this patch are more patches, one per architecture. These include
> two types of changes:
> o  A save_ptregs() function for the processor. I've taken a whack at
>    doing this for all of the processors. I have tested x86 and MIPS
>    versions. I was able to find cross compilers for ARM, ... and the
>    code compiles cleanly. Everything else, well, what you see is sheer
>    fantasy. You are welcome to chortle with merriment.
> o  When I could figure it out, I replaced the calls to panic() in
>    exception handling functions with calls to panic_with_regs() so
>    that everyone can leverage these changes without much effort. Again,
>    not all the code was transparent, so there are likely some places
>    that should have additional work done.
> 
> Note that the pointer to the struct pt_regs may be NULL. This is to
> accomodate those processors which don't have a working save_ptregs(). I'd
> love to eliminate this case by providing a save_ptregs() for all
> architectures, but I'll need help to so.
> 

It would make life easier if you could describe (or send) a means by
which arch maintainers can easily test these changes.

> --- a/kernel/panic.c
> +++ b/kernel/panic.c
>
> ...
>
> +/* Registers stored in calls to panic() */
> +static DEFINE_PER_CPU(struct pt_regs, panic_panic_regs);
> +static DEFINE_PER_CPU(const struct pt_regs *, panic_regs);
> +
> +/**
> + * get_panic_regs - return the current pointer to panic register values
> + */
> +const struct pt_regs *get_panic_regs()
> +{
> +	return __get_cpu_var(panic_regs);
> +}
> +EXPORT_SYMBOL(get_panic_regs);
> +
> +/**
> + * set_panic_regs - Set a pointer to the values of registers on panic()
> + * @new_regs:	Pointer to register values
> + *
> + * Returns: Pointer to the previous panic registers, if any.
> + */
> +const struct pt_regs *set_panic_regs(const struct pt_regs *new_regs)
> +{
> +	const struct pt_regs *old_regs, **pp_regs;
> +
> +	pp_regs = &__get_cpu_var(panic_regs);
> +	old_regs = *pp_regs;
> +	*pp_regs = new_regs;
> +	return old_regs;
> +}

What's going on here?  We define storage for a set of pt_regs and also
storage for a set of pt_regs pointers, and provide the ability for
callers to rewrite the thing which the pt_regs*'s point at.

Seems complex.  Why not simply provide a set of pt_regs and permit
callers to copy their own pt_regs sets into that area?

Secondly, this code implicitly assumes that the panicing code is pinned
to the panicing CPU and cannot be preempted and migrated to a different
CPU.  Is that true - do we take steps to ensure this anywhere?

Thirdly and relatedly, the code assumes that callers have disabled
preemption (otherwise __get_cpu_var->smp_processor_id() will whine). 
Where does this get reliably assured?

[PATCH 1/23] Make register values available to panic notifiers

[David VomLehn]

This patch makes panic() and die() registers available to, for example,
panic notifier functions.  Panic notifier functions are quite useful
for recording crash information, but they don't get passed the register
values. This makes it hard to print register contents, do stack
backtraces, etc. The changes in this patch save the register state when
panic() is called and introduce a function for die() to call that allows
it to pass in the registers it was passed.

Following this patch are more patches, one per architecture. These include
two types of changes:
o  A save_ptregs() function for the processor. I've taken a whack at
   doing this for all of the processors. I have tested x86 and MIPS
   versions. I was able to find cross compilers for ARM, ... and the
   code compiles cleanly. Everything else, well, what you see is sheer
   fantasy. You are welcome to chortle with merriment.
o  When I could figure it out, I replaced the calls to panic() in
   exception handling functions with calls to panic_with_regs() so
   that everyone can leverage these changes without much effort. Again,
   not all the code was transparent, so there are likely some places
   that should have additional work done.

Note that the pointer to the struct pt_regs may be NULL. This is to
accomodate those processors which don't have a working save_ptregs(). I'd
love to eliminate this case by providing a save_ptregs() for all
architectures, but I'll need help to so.

Signed-off-by: David VomLehn <dvomlehn@cisco.com>
---
 include/linux/kernel.h |    5 ++
 include/linux/ptrace.h |    9 ++++
 include/linux/ptreg.h  |   62 +++++++++++++++++++++++++++++++
 kernel/panic.c         |   96 +++++++++++++++++++++++++++++++++++++++++------
 4 files changed, 159 insertions(+), 13 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 328bca6..d73ae5c 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -162,6 +162,11 @@ extern struct atomic_notifier_head panic_notifier_list;
 extern long (*panic_blink)(long time);
 NORET_TYPE void panic(const char * fmt, ...)
 	__attribute__ ((NORET_AND format (printf, 1, 2))) __cold;
+NORET_TYPE void panic_with_regs(const struct pt_regs *regs,
+	const char *fmt, ...)
+	__attribute__ ((NORET_AND format (printf, 2, 3))) __cold;
+const struct pt_regs *get_panic_regs(void);
+extern const struct pt_regs *set_panic_regs(const struct pt_regs *regs);
 extern void oops_enter(void);
 extern void oops_exit(void);
 extern int oops_may_print(void);
diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 56f2d63..8f477ff 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -336,6 +336,15 @@ extern int task_current_syscall(struct task_struct *target, long *callno,
 				unsigned long args[6], unsigned int maxargs,
 				unsigned long *sp, unsigned long *pc);
 
+#ifndef arch_has_save_ptregs
+#warning save_ptregs undefined for this architecture; please define it soon
+
+static const struct pt_regs *save_ptregs(const struct pt_regs *regs)
+{
+	return NULL;
+}
+#endif
+
 #endif
 
 #endif
diff --git a/include/linux/ptreg.h b/include/linux/ptreg.h
new file mode 100644
index 0000000..f8cc2ea
--- /dev/null
+++ b/include/linux/ptreg.h
@@ -0,0 +1,62 @@
+/*
+ * Define macros for saving registers in a struct pt_regs structure
+ *
+ * Copyright (C) 2009 Cisco Systems, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef _LINUX_PTREG_H_
+#define _LINUX_PTREG_H_
+/*
+ * Define macros useful for writing save_ptregs(). Each architecture will
+ * need to define some macros, and some architectures have special cases
+ * that are most easily done as a special case. Macros that must be defined
+ * per architecture are:
+ *
+ * PTREG_SAVE - Save a register in a named location
+ * @r:		Name of the register to save
+ * @name:	%[name] of the location
+ *
+ * This is used in cases where the entry in the struct pt_regs is a simple
+ * string that doesn't end with a digit, e.g. pc.
+ *
+ * PTREG_INDIRECT_SAVE - move register to another register for saving
+ * @tmp_r:	Name of a temporary register
+ * @r:		Name of the register to be saved
+ * @name:	%[name] of the location
+ *
+ * This is used for cases where the register can't be saved directly,
+ * but must first be moved to another register that can be saved
+ * directly. The second register must already have been saved
+ * by the time this macro is used.
+ */
+
+/* Register save macro where the pt_regs item is an array, e.g. r[0] */
+#define _PTREG_SAVE_IDX(r, name, i)	PTREG_SAVE(r##i, name##i)
+
+/* Save register where the pt_regs item ends with a digit, e.g. r0. */
+#define _PTREG_SAVE_I(r, name, i)	PTREG_SAVE(r##i, name##i)
+
+/* Save register where it must first be copied to another register and where
+ * the pt_regs element ends with a digit, e.g. a2 */
+#define _PTREG_INDIRECT_SAVE_I(tmp, r, name, i) \
+					PTREG_INDIRECT_SAVE(tmp, r##i, name##i)
+
+/* Output specification */
+#define PTREG_OUT(rp, r, name)		[name] "=m" (rp->r)
+#define _PTREG_OUT_IDX(rp, r, name, i)	PTREG_OUT(rp, r[i], name##i)
+#define _PTREG_OUT_I(rp, r, name, i)	PTREG_OUT(rp, r##i, name##i)
+#endif
diff --git a/kernel/panic.c b/kernel/panic.c
index c787333..ef500c7 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -46,28 +46,22 @@ long (*panic_blink)(long time);
 EXPORT_SYMBOL(panic_blink);
 
 /**
- *	panic - halt the system
- *	@fmt: The text string to print
- *
- *	Display a message, then perform cleanups.
- *
- *	This function never returns.
+ * vpanic_with_regs - panic with specific register values and a argument array
+ * @regs:	Pointer to a &struct pt_reg with the register values
+ * @fmt:	Format for the panic message
+ * @args:	Arguments for the panic message as a va_list
  */
-NORET_TYPE void panic(const char * fmt, ...)
+static NORET_TYPE void vpanic_with_regs(const struct pt_regs *regs,
+	const char *fmt, va_list args)
 {
 	static char buf[1024];
-	va_list args;
 	long i;
-
 	/*
 	 * It's possible to come here directly from a panic-assertion and
 	 * not have preempt disabled. Some functions called from here want
 	 * preempt to be disabled. No point enabling it later though...
 	 */
-	preempt_disable();
-
-	bust_spinlocks(1);
-	va_start(args, fmt);
+	set_panic_regs(regs);
 	vsnprintf(buf, sizeof(buf), fmt, args);
 	va_end(args);
 	printk(KERN_EMERG "Kernel panic - not syncing: %s\n",buf);
@@ -143,8 +137,84 @@ NORET_TYPE void panic(const char * fmt, ...)
 	}
 }
 
+/* Registers stored in calls to panic() */
+static DEFINE_PER_CPU(struct pt_regs, panic_panic_regs);
+static DEFINE_PER_CPU(const struct pt_regs *, panic_regs);
+
+/**
+ * get_panic_regs - return the current pointer to panic register values
+ */
+const struct pt_regs *get_panic_regs()
+{
+	return __get_cpu_var(panic_regs);
+}
+EXPORT_SYMBOL(get_panic_regs);
+
+/**
+ * set_panic_regs - Set a pointer to the values of registers on panic()
+ * @new_regs:	Pointer to register values
+ *
+ * Returns: Pointer to the previous panic registers, if any.
+ */
+const struct pt_regs *set_panic_regs(const struct pt_regs *new_regs)
+{
+	const struct pt_regs *old_regs, **pp_regs;
+
+	pp_regs = &__get_cpu_var(panic_regs);
+	old_regs = *pp_regs;
+	*pp_regs = new_regs;
+	return old_regs;
+}
+
+/**
+ *	panic - halt the system
+ *	@fmt: The text string to print
+ *
+ *	Display a message, then perform cleanups.
+ *
+ *	This function never returns.
+ */
+NORET_TYPE void panic(const char *fmt, ...)
+{
+	va_list args;
+	const struct pt_regs *regs;
+	int i;
+
+	preempt_disable();
+	bust_spinlocks(1);
+	regs = save_ptregs(&__get_cpu_var(panic_panic_regs));
+	va_start(args, fmt);
+	vpanic_with_regs(regs, fmt, args);
+	/* Since vpanic_with_regs doesn't return, we skip va_end() */
+	/* Infinite loop so compiler doesn't complain about this returning */
+	for (i = 0; ; )
+		mdelay(1);
+}
 EXPORT_SYMBOL(panic);
 
+/**
+ * panic_with_regs - panic with specific register values
+ * @regs:	Pointer to a &struct pt_reg with the register values
+ * @fmt:	Format for the panic message
+ * @...:	Arguments for the panic message
+ */
+NORET_TYPE void panic_with_regs(const struct pt_regs *regs, const char *fmt,
+	...)
+{
+	va_list args;
+	int i;
+
+	preempt_disable();
+	bust_spinlocks(1);
+	va_start(args, fmt);
+	vpanic_with_regs(regs, fmt, args);
+	/* Since vpanic_with_regs doesn't return, we skip va_end() */
+	/* Infinite loop so compiler doesn't complain about this returning */
+	for (i = 0; ; )
+		mdelay(1);
+}
+EXPORT_SYMBOL(panic_with_regs);
+
 
 struct tnt {
 	u8	bit;