linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] arm64: Changes to register and stack dumps
@ 2017-10-19 15:55 Will Deacon
  2017-10-19 15:55 ` [PATCH 1/4] arm64: consistently log ESR and page table Will Deacon
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Will Deacon @ 2017-10-19 15:55 UTC (permalink / raw)
  To: linux-arm-kernel

Hi all,

This handful of patches makes some significant changes to how we print
register/stack dumps and brings us inline with recent changes to x86.
The main changes are that we no longer print out the 'Exception stack:'
section and PC/LR values are resolved to a symbol+offset format where
possible.

Feedback welcome.

Will

--->8

Mark Rutland (1):
  arm64: consistently log ESR and page table

Will Deacon (3):
  arm64: traps: Don't print stack or raw PC/LR values in backtraces
  arm64: traps: Pretty-print pstate in register dumps
  arm64: uapi: Remove PSR_Q_BIT

 arch/arm64/include/uapi/asm/ptrace.h |  1 -
 arch/arm64/kernel/process.c          | 24 ++++++++++---
 arch/arm64/kernel/traps.c            | 65 ++----------------------------------
 arch/arm64/mm/fault.c                |  7 ++--
 4 files changed, 28 insertions(+), 69 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] arm64: consistently log ESR and page table
  2017-10-19 15:55 [PATCH 0/4] arm64: Changes to register and stack dumps Will Deacon
@ 2017-10-19 15:55 ` Will Deacon
  2017-10-19 15:55 ` [PATCH 2/4] arm64: traps: Don't print stack or raw PC/LR values in backtraces Will Deacon
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Will Deacon @ 2017-10-19 15:55 UTC (permalink / raw)
  To: linux-arm-kernel

From: Mark Rutland <mark.rutland@arm.com>

When we take a fault we can't handle, we try to dump some relevant
information, but we're not consistent about doing so.

In do_mem_abort(), we log the full ESR, but don't dump a page table
walk. In __do_kernel_fault, we dump an attempted decoding of the ESR
(but not the ESR itself) along with a page table walk.

Let's try to make things more consistent by dumping the full ESR in
mem_abort_decode(), and having do_mem_abort dump a page table walk. The
existing dump of the ESR in do_mem_abort() is rendered redundant, and
removed.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Julien Thierry <julien.thierry@arm.com>
Cc: Kristina Martsenko <kristina.martsenko@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/mm/fault.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 02d92bdf1c49..eec09c623d8e 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -112,6 +112,7 @@ static void mem_abort_decode(unsigned int esr)
 {
 	pr_alert("Mem abort info:\n");
 
+	pr_alert("  ESR = 0x%08x\n", esr);
 	pr_alert("  Exception class = %s, IL = %u bits\n",
 		 esr_get_class_string(esr),
 		 (esr & ESR_ELx_IL) ? 32 : 16);
@@ -745,11 +746,13 @@ asmlinkage void __exception do_mem_abort(unsigned long addr, unsigned int esr,
 	if (!inf->fn(addr, esr, regs))
 		return;
 
-	pr_alert("Unhandled fault: %s (0x%08x) at 0x%016lx\n",
-		 inf->name, esr, addr);
+	pr_alert("Unhandled fault: %s at 0x%016lx\n",
+		 inf->name, addr);
 
 	mem_abort_decode(esr);
 
+	show_pte(addr);
+
 	info.si_signo = inf->sig;
 	info.si_errno = 0;
 	info.si_code  = inf->code;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] arm64: traps: Don't print stack or raw PC/LR values in backtraces
  2017-10-19 15:55 [PATCH 0/4] arm64: Changes to register and stack dumps Will Deacon
  2017-10-19 15:55 ` [PATCH 1/4] arm64: consistently log ESR and page table Will Deacon
@ 2017-10-19 15:55 ` Will Deacon
  2017-10-19 15:55 ` [PATCH 3/4] arm64: traps: Pretty-print pstate in register dumps Will Deacon
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Will Deacon @ 2017-10-19 15:55 UTC (permalink / raw)
  To: linux-arm-kernel

Printing raw pointer values in backtraces has potential security
implications and are of questionable value anyway.

This patch follows x86's lead and removes the "Exception stack:" dump
from kernel backtraces, as well as converting PC/LR values to symbols
such as "sysrq_handle_crash+0x20/0x30".

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/process.c |  8 +++---
 arch/arm64/kernel/traps.c   | 65 +++------------------------------------------
 2 files changed, 6 insertions(+), 67 deletions(-)

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 2dc0f8482210..c20896b8fb2d 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -186,11 +186,9 @@ void __show_regs(struct pt_regs *regs)
 	}
 
 	show_regs_print_info(KERN_DEFAULT);
-	print_symbol("PC is at %s\n", instruction_pointer(regs));
-	print_symbol("LR is at %s\n", lr);
-	printk("pc : [<%016llx>] lr : [<%016llx>] pstate: %08llx\n",
-	       regs->pc, lr, regs->pstate);
-	printk("sp : %016llx\n", sp);
+	print_symbol("pc : %s\n", regs->pc);
+	print_symbol("lr : %s\n", lr);
+	printk("sp : %016llx pstate : %08llx\n", sp, regs->pstate);
 
 	i = top_reg;
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 5ea4b85aee0e..57794773863c 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -58,55 +58,9 @@ static const char *handler[]= {
 
 int show_unhandled_signals = 1;
 
-/*
- * Dump out the contents of some kernel memory nicely...
- */
-static void dump_mem(const char *lvl, const char *str, unsigned long bottom,
-		     unsigned long top)
-{
-	unsigned long first;
-	mm_segment_t fs;
-	int i;
-
-	/*
-	 * We need to switch to kernel mode so that we can use __get_user
-	 * to safely read from kernel space.
-	 */
-	fs = get_fs();
-	set_fs(KERNEL_DS);
-
-	printk("%s%s(0x%016lx to 0x%016lx)\n", lvl, str, bottom, top);
-
-	for (first = bottom & ~31; first < top; first += 32) {
-		unsigned long p;
-		char str[sizeof(" 12345678") * 8 + 1];
-
-		memset(str, ' ', sizeof(str));
-		str[sizeof(str) - 1] = '\0';
-
-		for (p = first, i = 0; i < (32 / 8)
-					&& p < top; i++, p += 8) {
-			if (p >= bottom && p < top) {
-				unsigned long val;
-
-				if (__get_user(val, (unsigned long *)p) == 0)
-					sprintf(str + i * 17, " %016lx", val);
-				else
-					sprintf(str + i * 17, " ????????????????");
-			}
-		}
-		printk("%s%04lx:%s\n", lvl, first & 0xffff, str);
-	}
-
-	set_fs(fs);
-}
-
 static void dump_backtrace_entry(unsigned long where)
 {
-	/*
-	 * Note that 'where' can have a physical address, but it's not handled.
-	 */
-	print_ip_sym(where);
+	printk(" %pS\n", (void *)where);
 }
 
 static void __dump_instr(const char *lvl, struct pt_regs *regs)
@@ -171,10 +125,7 @@ void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
 
 	skip = !!regs;
 	printk("Call trace:\n");
-	while (1) {
-		unsigned long stack;
-		int ret;
-
+	do {
 		/* skip until specified stack frame */
 		if (!skip) {
 			dump_backtrace_entry(frame.pc);
@@ -189,17 +140,7 @@ void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
 			 */
 			dump_backtrace_entry(regs->pc);
 		}
-		ret = unwind_frame(tsk, &frame);
-		if (ret < 0)
-			break;
-		if (in_entry_text(frame.pc)) {
-			stack = frame.fp - offsetof(struct pt_regs, stackframe);
-
-			if (on_accessible_stack(tsk, stack))
-				dump_mem("", "Exception stack", stack,
-					 stack + sizeof(struct pt_regs));
-		}
-	}
+	} while (!unwind_frame(tsk, &frame));
 
 	put_task_stack(tsk);
 }
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] arm64: traps: Pretty-print pstate in register dumps
  2017-10-19 15:55 [PATCH 0/4] arm64: Changes to register and stack dumps Will Deacon
  2017-10-19 15:55 ` [PATCH 1/4] arm64: consistently log ESR and page table Will Deacon
  2017-10-19 15:55 ` [PATCH 2/4] arm64: traps: Don't print stack or raw PC/LR values in backtraces Will Deacon
@ 2017-10-19 15:55 ` Will Deacon
  2017-10-19 16:17   ` Mark Rutland
  2017-10-19 15:55 ` [PATCH 4/4] arm64: uapi: Remove PSR_Q_BIT Will Deacon
  2017-10-20 22:53 ` [PATCH 0/4] arm64: Changes to register and stack dumps Laura Abbott
  4 siblings, 1 reply; 10+ messages in thread
From: Will Deacon @ 2017-10-19 15:55 UTC (permalink / raw)
  To: linux-arm-kernel

We can decode the PSTATE easily enough, so pretty-print it in register
dumps.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/process.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index c20896b8fb2d..be3cf9cd7dec 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -170,6 +170,23 @@ void machine_restart(char *cmd)
 	while (1);
 }
 
+static void print_pstate(struct pt_regs *regs)
+{
+	u64 pstate = regs->pstate;
+	printk("pstate: %08llx (%c%c%c%c %c%c%c%c %cPAN %cUAO)\n",
+		pstate,
+		pstate & PSR_N_BIT ? 'N' : 'n',
+		pstate & PSR_Z_BIT ? 'Z' : 'z',
+		pstate & PSR_C_BIT ? 'C' : 'c',
+		pstate & PSR_V_BIT ? 'V' : 'v',
+		pstate & PSR_D_BIT ? 'D' : 'd',
+		pstate & PSR_A_BIT ? 'A' : 'a',
+		pstate & PSR_I_BIT ? 'I' : 'i',
+		pstate & PSR_F_BIT ? 'F' : 'f',
+		pstate & PSR_PAN_BIT ? '+' : '-',
+		pstate & PSR_UAO_BIT ? '+' : '-');
+}
+
 void __show_regs(struct pt_regs *regs)
 {
 	int i, top_reg;
@@ -186,9 +203,10 @@ void __show_regs(struct pt_regs *regs)
 	}
 
 	show_regs_print_info(KERN_DEFAULT);
+	print_pstate(regs);
 	print_symbol("pc : %s\n", regs->pc);
 	print_symbol("lr : %s\n", lr);
-	printk("sp : %016llx pstate : %08llx\n", sp, regs->pstate);
+	printk("sp : %016llx\n", sp);
 
 	i = top_reg;
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] arm64: uapi: Remove PSR_Q_BIT
  2017-10-19 15:55 [PATCH 0/4] arm64: Changes to register and stack dumps Will Deacon
                   ` (2 preceding siblings ...)
  2017-10-19 15:55 ` [PATCH 3/4] arm64: traps: Pretty-print pstate in register dumps Will Deacon
@ 2017-10-19 15:55 ` Will Deacon
  2017-10-20 22:53 ` [PATCH 0/4] arm64: Changes to register and stack dumps Laura Abbott
  4 siblings, 0 replies; 10+ messages in thread
From: Will Deacon @ 2017-10-19 15:55 UTC (permalink / raw)
  To: linux-arm-kernel

PSTATE.Q only exists for AArch32, which can be referred to using
COMPAT_PSR_Q_BIT. Remove PSR_Q_BIT, since the native bit doesn't exist
in the architecture

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/uapi/asm/ptrace.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
index d1ff83dfe5de..3697d95ba0a1 100644
--- a/arch/arm64/include/uapi/asm/ptrace.h
+++ b/arch/arm64/include/uapi/asm/ptrace.h
@@ -46,7 +46,6 @@
 #define PSR_D_BIT	0x00000200
 #define PSR_PAN_BIT	0x00400000
 #define PSR_UAO_BIT	0x00800000
-#define PSR_Q_BIT	0x08000000
 #define PSR_V_BIT	0x10000000
 #define PSR_C_BIT	0x20000000
 #define PSR_Z_BIT	0x40000000
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] arm64: traps: Pretty-print pstate in register dumps
  2017-10-19 15:55 ` [PATCH 3/4] arm64: traps: Pretty-print pstate in register dumps Will Deacon
@ 2017-10-19 16:17   ` Mark Rutland
  2017-10-20 15:58     ` Will Deacon
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Rutland @ 2017-10-19 16:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Oct 19, 2017 at 04:55:33PM +0100, Will Deacon wrote:
> We can decode the PSTATE easily enough, so pretty-print it in register
> dumps.
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/kernel/process.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index c20896b8fb2d..be3cf9cd7dec 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -170,6 +170,23 @@ void machine_restart(char *cmd)
>  	while (1);
>  }
>  
> +static void print_pstate(struct pt_regs *regs)
> +{
> +	u64 pstate = regs->pstate;
> +	printk("pstate: %08llx (%c%c%c%c %c%c%c%c %cPAN %cUAO)\n",
> +		pstate,
> +		pstate & PSR_N_BIT ? 'N' : 'n',
> +		pstate & PSR_Z_BIT ? 'Z' : 'z',
> +		pstate & PSR_C_BIT ? 'C' : 'c',
> +		pstate & PSR_V_BIT ? 'V' : 'v',
> +		pstate & PSR_D_BIT ? 'D' : 'd',
> +		pstate & PSR_A_BIT ? 'A' : 'a',
> +		pstate & PSR_I_BIT ? 'I' : 'i',
> +		pstate & PSR_F_BIT ? 'F' : 'f',
> +		pstate & PSR_PAN_BIT ? '+' : '-',
> +		pstate & PSR_UAO_BIT ? '+' : '-');
> +}

Given we use __show_regs for user state, I think we need to check
compat_user_mode() regs, and adjust the decoding accordingly.

e.g. AArch32 has an E bit where the D in DAIF is.

Thanks,
Mark.

> +
>  void __show_regs(struct pt_regs *regs)
>  {
>  	int i, top_reg;
> @@ -186,9 +203,10 @@ void __show_regs(struct pt_regs *regs)
>  	}
>  
>  	show_regs_print_info(KERN_DEFAULT);
> +	print_pstate(regs);
>  	print_symbol("pc : %s\n", regs->pc);
>  	print_symbol("lr : %s\n", lr);
> -	printk("sp : %016llx pstate : %08llx\n", sp, regs->pstate);
> +	printk("sp : %016llx\n", sp);
>  
>  	i = top_reg;
>  
> -- 
> 2.1.4
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/4] arm64: traps: Pretty-print pstate in register dumps
  2017-10-19 16:17   ` Mark Rutland
@ 2017-10-20 15:58     ` Will Deacon
  2017-10-20 16:08       ` Mark Rutland
  0 siblings, 1 reply; 10+ messages in thread
From: Will Deacon @ 2017-10-20 15:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Oct 19, 2017 at 05:17:55PM +0100, Mark Rutland wrote:
> On Thu, Oct 19, 2017 at 04:55:33PM +0100, Will Deacon wrote:
> > We can decode the PSTATE easily enough, so pretty-print it in register
> > dumps.
> > 
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> >  arch/arm64/kernel/process.c | 20 +++++++++++++++++++-
> >  1 file changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> > index c20896b8fb2d..be3cf9cd7dec 100644
> > --- a/arch/arm64/kernel/process.c
> > +++ b/arch/arm64/kernel/process.c
> > @@ -170,6 +170,23 @@ void machine_restart(char *cmd)
> >  	while (1);
> >  }
> >  
> > +static void print_pstate(struct pt_regs *regs)
> > +{
> > +	u64 pstate = regs->pstate;
> > +	printk("pstate: %08llx (%c%c%c%c %c%c%c%c %cPAN %cUAO)\n",
> > +		pstate,
> > +		pstate & PSR_N_BIT ? 'N' : 'n',
> > +		pstate & PSR_Z_BIT ? 'Z' : 'z',
> > +		pstate & PSR_C_BIT ? 'C' : 'c',
> > +		pstate & PSR_V_BIT ? 'V' : 'v',
> > +		pstate & PSR_D_BIT ? 'D' : 'd',
> > +		pstate & PSR_A_BIT ? 'A' : 'a',
> > +		pstate & PSR_I_BIT ? 'I' : 'i',
> > +		pstate & PSR_F_BIT ? 'F' : 'f',
> > +		pstate & PSR_PAN_BIT ? '+' : '-',
> > +		pstate & PSR_UAO_BIT ? '+' : '-');
> > +}
> 
> Given we use __show_regs for user state, I think we need to check
> compat_user_mode() regs, and adjust the decoding accordingly.
> 
> e.g. AArch32 has an E bit where the D in DAIF is.

Yes, you're right. There's lots of exciting bits in the AArch32 pstate
too. I ended up with the diff below.

Will

--->8

+static void print_pstate(struct pt_regs *regs)
+{
+	u64 pstate = regs->pstate;
+
+	if (compat_user_mode(regs))
+		printk("pstate: %08llx (%c%c%c%c %c %s %s-endian %c%c%c)\n",
+			pstate,
+			pstate & COMPAT_PSR_N_BIT ? 'N' : 'n',
+			pstate & COMPAT_PSR_Z_BIT ? 'Z' : 'z',
+			pstate & COMPAT_PSR_C_BIT ? 'C' : 'c',
+			pstate & COMPAT_PSR_V_BIT ? 'V' : 'v',
+			pstate & COMPAT_PSR_Q_BIT ? 'Q' : 'q',
+			pstate & COMPAT_PSR_T_BIT ? "T32" : "A32",
+			pstate & COMPAT_PSR_E_BIT ? "big" : "little",
+			pstate & COMPAT_PSR_A_BIT ? 'A' : 'a',
+			pstate & COMPAT_PSR_I_BIT ? 'I' : 'i',
+			pstate & COMPAT_PSR_F_BIT ? 'F' : 'f');
+	else
+		printk("pstate: %08llx (%c%c%c%c %c%c%c%c %cPAN %cUAO)\n",
+			pstate,
+			pstate & PSR_N_BIT ? 'N' : 'n',
+			pstate & PSR_Z_BIT ? 'Z' : 'z',
+			pstate & PSR_C_BIT ? 'C' : 'c',
+			pstate & PSR_V_BIT ? 'V' : 'v',
+			pstate & PSR_D_BIT ? 'D' : 'd',
+			pstate & PSR_A_BIT ? 'A' : 'a',
+			pstate & PSR_I_BIT ? 'I' : 'i',
+			pstate & PSR_F_BIT ? 'F' : 'f',
+			pstate & PSR_PAN_BIT ? '+' : '-',
+			pstate & PSR_UAO_BIT ? '+' : '-');
+}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/4] arm64: traps: Pretty-print pstate in register dumps
  2017-10-20 15:58     ` Will Deacon
@ 2017-10-20 16:08       ` Mark Rutland
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2017-10-20 16:08 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Oct 20, 2017 at 04:58:37PM +0100, Will Deacon wrote:
> On Thu, Oct 19, 2017 at 05:17:55PM +0100, Mark Rutland wrote:
> > On Thu, Oct 19, 2017 at 04:55:33PM +0100, Will Deacon wrote:
> > > We can decode the PSTATE easily enough, so pretty-print it in register
> > > dumps.
> > > 
> > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > ---
> > >  arch/arm64/kernel/process.c | 20 +++++++++++++++++++-
> > >  1 file changed, 19 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> > > index c20896b8fb2d..be3cf9cd7dec 100644
> > > --- a/arch/arm64/kernel/process.c
> > > +++ b/arch/arm64/kernel/process.c
> > > @@ -170,6 +170,23 @@ void machine_restart(char *cmd)
> > >  	while (1);
> > >  }
> > >  
> > > +static void print_pstate(struct pt_regs *regs)
> > > +{
> > > +	u64 pstate = regs->pstate;
> > > +	printk("pstate: %08llx (%c%c%c%c %c%c%c%c %cPAN %cUAO)\n",
> > > +		pstate,
> > > +		pstate & PSR_N_BIT ? 'N' : 'n',
> > > +		pstate & PSR_Z_BIT ? 'Z' : 'z',
> > > +		pstate & PSR_C_BIT ? 'C' : 'c',
> > > +		pstate & PSR_V_BIT ? 'V' : 'v',
> > > +		pstate & PSR_D_BIT ? 'D' : 'd',
> > > +		pstate & PSR_A_BIT ? 'A' : 'a',
> > > +		pstate & PSR_I_BIT ? 'I' : 'i',
> > > +		pstate & PSR_F_BIT ? 'F' : 'f',
> > > +		pstate & PSR_PAN_BIT ? '+' : '-',
> > > +		pstate & PSR_UAO_BIT ? '+' : '-');
> > > +}
> > 
> > Given we use __show_regs for user state, I think we need to check
> > compat_user_mode() regs, and adjust the decoding accordingly.
> > 
> > e.g. AArch32 has an E bit where the D in DAIF is.
> 
> Yes, you're right. There's lots of exciting bits in the AArch32 pstate
> too. I ended up with the diff below.
> 
> Will
> 
> --->8
> 
> +static void print_pstate(struct pt_regs *regs)
> +{
> +	u64 pstate = regs->pstate;
> +
> +	if (compat_user_mode(regs))
> +		printk("pstate: %08llx (%c%c%c%c %c %s %s-endian %c%c%c)\n",
> +			pstate,
> +			pstate & COMPAT_PSR_N_BIT ? 'N' : 'n',
> +			pstate & COMPAT_PSR_Z_BIT ? 'Z' : 'z',
> +			pstate & COMPAT_PSR_C_BIT ? 'C' : 'c',
> +			pstate & COMPAT_PSR_V_BIT ? 'V' : 'v',
> +			pstate & COMPAT_PSR_Q_BIT ? 'Q' : 'q',
> +			pstate & COMPAT_PSR_T_BIT ? "T32" : "A32",
> +			pstate & COMPAT_PSR_E_BIT ? "big" : "little",

We could say BE/LE for these so as to keep the line short.

> +			pstate & COMPAT_PSR_A_BIT ? 'A' : 'a',
> +			pstate & COMPAT_PSR_I_BIT ? 'I' : 'i',
> +			pstate & COMPAT_PSR_F_BIT ? 'F' : 'f');
> +	else

Given the either side of the if-else is several lines, could we add
braces for clarity?

> +		printk("pstate: %08llx (%c%c%c%c %c%c%c%c %cPAN %cUAO)\n",
> +			pstate,
> +			pstate & PSR_N_BIT ? 'N' : 'n',
> +			pstate & PSR_Z_BIT ? 'Z' : 'z',
> +			pstate & PSR_C_BIT ? 'C' : 'c',
> +			pstate & PSR_V_BIT ? 'V' : 'v',
> +			pstate & PSR_D_BIT ? 'D' : 'd',
> +			pstate & PSR_A_BIT ? 'A' : 'a',
> +			pstate & PSR_I_BIT ? 'I' : 'i',
> +			pstate & PSR_F_BIT ? 'F' : 'f',
> +			pstate & PSR_PAN_BIT ? '+' : '-',
> +			pstate & PSR_UAO_BIT ? '+' : '-');
> +}

Otherwise, looks good!

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 0/4] arm64: Changes to register and stack dumps
  2017-10-19 15:55 [PATCH 0/4] arm64: Changes to register and stack dumps Will Deacon
                   ` (3 preceding siblings ...)
  2017-10-19 15:55 ` [PATCH 4/4] arm64: uapi: Remove PSR_Q_BIT Will Deacon
@ 2017-10-20 22:53 ` Laura Abbott
  2017-10-23  9:03   ` Will Deacon
  4 siblings, 1 reply; 10+ messages in thread
From: Laura Abbott @ 2017-10-20 22:53 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/19/2017 08:55 AM, Will Deacon wrote:
> Hi all,
> 
> This handful of patches makes some significant changes to how we print
> register/stack dumps and brings us inline with recent changes to x86.
> The main changes are that we no longer print out the 'Exception stack:'
> section and PC/LR values are resolved to a symbol+offset format where
> possible.
> 
> Feedback welcome.
> 

In very petty complaints, I can no longer just use the PC/LR with
addr2line. Yes I can use gdb or one of the scripts but that's a
few more steps when I'm doing debugging on my own. Like I said,
very minor and overall this is good for both security and alignment.

I ran this through several of the LKDTM tests and the backtraces
looked fine. You're welcome to add

Tested-by: Laura Abbott <labbott@redhat.com>

> Will
> 
> --->8
> 
> Mark Rutland (1):
>   arm64: consistently log ESR and page table
> 
> Will Deacon (3):
>   arm64: traps: Don't print stack or raw PC/LR values in backtraces
>   arm64: traps: Pretty-print pstate in register dumps
>   arm64: uapi: Remove PSR_Q_BIT
> 
>  arch/arm64/include/uapi/asm/ptrace.h |  1 -
>  arch/arm64/kernel/process.c          | 24 ++++++++++---
>  arch/arm64/kernel/traps.c            | 65 ++----------------------------------
>  arch/arm64/mm/fault.c                |  7 ++--
>  4 files changed, 28 insertions(+), 69 deletions(-)
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 0/4] arm64: Changes to register and stack dumps
  2017-10-20 22:53 ` [PATCH 0/4] arm64: Changes to register and stack dumps Laura Abbott
@ 2017-10-23  9:03   ` Will Deacon
  0 siblings, 0 replies; 10+ messages in thread
From: Will Deacon @ 2017-10-23  9:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Oct 20, 2017 at 03:53:50PM -0700, Laura Abbott wrote:
> On 10/19/2017 08:55 AM, Will Deacon wrote:
> > This handful of patches makes some significant changes to how we print
> > register/stack dumps and brings us inline with recent changes to x86.
> > The main changes are that we no longer print out the 'Exception stack:'
> > section and PC/LR values are resolved to a symbol+offset format where
> > possible.
> > 
> > Feedback welcome.
> > 
> 
> In very petty complaints, I can no longer just use the PC/LR with
> addr2line. Yes I can use gdb or one of the scripts but that's a
> few more steps when I'm doing debugging on my own. Like I said,
> very minor and overall this is good for both security and alignment.

Understood. scripts/faddr2line seems to do a reasonable job, but I agree
that it's a little annoying that you have to do some pointer arithmetic
if you want to e.g. locate disassembly in objdump. Setting KALLSYMS=n will
give you the addresses if you *really* want them.

> I ran this through several of the LKDTM tests and the backtraces
> looked fine. You're welcome to add
> 
> Tested-by: Laura Abbott <labbott@redhat.com>

Thanks, Laura.

Will

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-10-23  9:03 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-19 15:55 [PATCH 0/4] arm64: Changes to register and stack dumps Will Deacon
2017-10-19 15:55 ` [PATCH 1/4] arm64: consistently log ESR and page table Will Deacon
2017-10-19 15:55 ` [PATCH 2/4] arm64: traps: Don't print stack or raw PC/LR values in backtraces Will Deacon
2017-10-19 15:55 ` [PATCH 3/4] arm64: traps: Pretty-print pstate in register dumps Will Deacon
2017-10-19 16:17   ` Mark Rutland
2017-10-20 15:58     ` Will Deacon
2017-10-20 16:08       ` Mark Rutland
2017-10-19 15:55 ` [PATCH 4/4] arm64: uapi: Remove PSR_Q_BIT Will Deacon
2017-10-20 22:53 ` [PATCH 0/4] arm64: Changes to register and stack dumps Laura Abbott
2017-10-23  9:03   ` Will Deacon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).