All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/9] arm64: Use BRK instruction for generic BUG traps
@ 2015-07-24 15:37 Dave Martin
  2015-07-24 15:37 ` [PATCH v3 1/9] arm64/debug: Eliminate magic number for size of BRK instruction Dave Martin
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Dave Martin @ 2015-07-24 15:37 UTC (permalink / raw)
  To: linux-arm-kernel

Final repost to address outstanding minor comments.

Note that I have dropped tags from the following non-trivially changed
patches:

 * patch 7/9 arm64/debug: Add missing #includes

   (Merged in remnants of dropped patch "arm64: esr.h type fixes and
   cleanup")

 * patch 6/9 arm64/debug: Simplify BRK insn opcode declarations

   (Added truncating type casts for ESR value assignment, to silence
   compiler warnings.)


Changes since v2:

 * Get rid of broad-brush type annotation changes in <asm/esr.h>.
   They are replaced with a couple of explicit casts where the affected
   constants are used.  The only purpose of this is to silence harmless
   compiler warnings.


Changes since v1:

 * Modified BRK immediate for BUG so that it doesn't overlap the range
   allocated for KGDB.

 * Typo fixes.

 * Don't assume that BUG() kills the thread, so that catching BUGs in
   kgdb works again.

 * Separate header for the BRK immediates removed, at Will's request.
   I've retained the other refactoring since it contains useful tidy-
   ups, but some of that could go away if desired.


Original cover letter:

Currently, the minimal default BUG() implementation from asm-generic is
used for arm64.

This series uses the BRK software breakpoint instruction to generate a
trap instead, similarly to most other arches, with the generic BUG code
generating the dmesg boilerplate.  This eliminates a fair amount of
inlined code at BUG() and WARN() sites.


This work makes it look increasingly desirable to collect BRK immediates
together in one place.  Patches 1-7 do some refactoring to prepare for
this, and patch 8 moves the definitions to a fresh header, <asm/brk.h>.

Patch 9 provides the BRK-based GENERIC_BUG support for arm64.

A side-effect of this change is that WARNs are now generated via a
different bit of generic code (lib/bug.c:report_bug()) that no longer
prints a backtrace (compare kernel/panic.c:warn_slowpath_common()).)  I
will post a separate mini-RFC series to address that in the generic
code.  Patch 10 hacks a backtrace back into the arm64 arch code in the
meantime.

[...]


Dave Martin (9):
  arm64/debug: Eliminate magic number for size of BRK instruction
  arm64/debug: Mask off all reserved bits from generated ESR values
  arm64/debug: Eliminate magic number from ESR template definition
  arm64/debug: More consistent naming for the BRK ESR template macro
  arm64/debug: Move BRK ESR template macro into <asm/esr.h>
  arm64/debug: Simplify BRK insn opcode declarations
  arm64/debug: Add missing #includes
  arm64/BUG: Use BRK instruction for generic BUG traps
  arm64/BUG: Show explicit backtrace for WARNs

 arch/arm64/Kconfig                      |    8 ++++
 arch/arm64/include/asm/bug.h            |   64 +++++++++++++++++++++++++++++++
 arch/arm64/include/asm/debug-monitors.h |   38 +++++++-----------
 arch/arm64/include/asm/esr.h            |    9 +++++
 arch/arm64/kernel/kgdb.c                |   12 +++---
 arch/arm64/kernel/traps.c               |   61 ++++++++++++++++++++++++++++-
 arch/arm64/mm/fault.c                   |   12 +++++-
 7 files changed, 170 insertions(+), 34 deletions(-)
 create mode 100644 arch/arm64/include/asm/bug.h

-- 
1.7.10.4

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

* [PATCH v3 1/9] arm64/debug: Eliminate magic number for size of BRK instruction
  2015-07-24 15:37 [PATCH v3 0/9] arm64: Use BRK instruction for generic BUG traps Dave Martin
@ 2015-07-24 15:37 ` Dave Martin
  2015-07-24 15:37 ` [PATCH v3 2/9] arm64/debug: Mask off all reserved bits from generated ESR values Dave Martin
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Dave Martin @ 2015-07-24 15:37 UTC (permalink / raw)
  To: linux-arm-kernel

The size of an A64 BRK instruction is the same as the size of all other
A64 instructions, because all A64 instructions are the same size.

BREAK_INSTR_SIZE is retained for readibility, but it should not be
an independent constant from AARCH64_INSN_SIZE.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/debug-monitors.h |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index 40ec68a..f3d2dbd 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -18,6 +18,8 @@
 
 #ifdef __KERNEL__
 
+#include <asm/insn.h>
+
 /* Low-level stepping controls. */
 #define DBG_MDSCR_SS		(1 << 0)
 #define DBG_SPSR_SS		(1 << 21)
@@ -38,7 +40,7 @@
 /*
  * Break point instruction encoding
  */
-#define BREAK_INSTR_SIZE		4
+#define BREAK_INSTR_SIZE		AARCH64_INSN_SIZE
 
 /*
  * ESR values expected for dynamic and compile time BRK instruction
-- 
1.7.10.4

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

* [PATCH v3 2/9] arm64/debug: Mask off all reserved bits from generated ESR values
  2015-07-24 15:37 [PATCH v3 0/9] arm64: Use BRK instruction for generic BUG traps Dave Martin
  2015-07-24 15:37 ` [PATCH v3 1/9] arm64/debug: Eliminate magic number for size of BRK instruction Dave Martin
@ 2015-07-24 15:37 ` Dave Martin
  2015-07-24 15:37 ` [PATCH v3 3/9] arm64/debug: Eliminate magic number from ESR template definition Dave Martin
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Dave Martin @ 2015-07-24 15:37 UTC (permalink / raw)
  To: linux-arm-kernel

There are only 16 comment bits in a BRK instruction, which
correspond to ESR bits 15:0.  Bits 24:16 of the ESR are RES0,
and might have weird meanings in the future.

This code inserts 16 bits of comment in the ESR value instead of
20 (almost certainly a typo in the original code).

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/debug-monitors.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index f3d2dbd..ab7d5a8 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -45,7 +45,7 @@
 /*
  * ESR values expected for dynamic and compile time BRK instruction
  */
-#define DBG_ESR_VAL_BRK(x)	(0xf2000000 | ((x) & 0xfffff))
+#define DBG_ESR_VAL_BRK(x)	(0xf2000000 | ((x) & 0xffff))
 
 /*
  * #imm16 values used for BRK instruction generation
-- 
1.7.10.4

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

* [PATCH v3 3/9] arm64/debug: Eliminate magic number from ESR template definition
  2015-07-24 15:37 [PATCH v3 0/9] arm64: Use BRK instruction for generic BUG traps Dave Martin
  2015-07-24 15:37 ` [PATCH v3 1/9] arm64/debug: Eliminate magic number for size of BRK instruction Dave Martin
  2015-07-24 15:37 ` [PATCH v3 2/9] arm64/debug: Mask off all reserved bits from generated ESR values Dave Martin
@ 2015-07-24 15:37 ` Dave Martin
  2015-07-24 15:37 ` [PATCH v3 4/9] arm64/debug: More consistent naming for the BRK ESR template macro Dave Martin
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Dave Martin @ 2015-07-24 15:37 UTC (permalink / raw)
  To: linux-arm-kernel

<asm/esr.h> has perfectly good constants for defining ESR values
already.  Let's use them.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/debug-monitors.h |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index ab7d5a8..ff09058 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -18,6 +18,7 @@
 
 #ifdef __KERNEL__
 
+#include <asm/esr.h>
 #include <asm/insn.h>
 
 /* Low-level stepping controls. */
@@ -45,7 +46,8 @@
 /*
  * ESR values expected for dynamic and compile time BRK instruction
  */
-#define DBG_ESR_VAL_BRK(x)	(0xf2000000 | ((x) & 0xffff))
+#define DBG_ESR_VAL_BRK(x) \
+	((ESR_ELx_EC_BRK64 << ESR_ELx_EC_SHIFT) | ESR_ELx_IL | ((x) & 0xffff))
 
 /*
  * #imm16 values used for BRK instruction generation
-- 
1.7.10.4

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

* [PATCH v3 4/9] arm64/debug: More consistent naming for the BRK ESR template macro
  2015-07-24 15:37 [PATCH v3 0/9] arm64: Use BRK instruction for generic BUG traps Dave Martin
                   ` (2 preceding siblings ...)
  2015-07-24 15:37 ` [PATCH v3 3/9] arm64/debug: Eliminate magic number from ESR template definition Dave Martin
@ 2015-07-24 15:37 ` Dave Martin
  2015-07-24 15:37 ` [PATCH v3 5/9] arm64/debug: Move BRK ESR template macro into <asm/esr.h> Dave Martin
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Dave Martin @ 2015-07-24 15:37 UTC (permalink / raw)
  To: linux-arm-kernel

The naming of DBG_ESR_VAL_BRK is inconsistent with the way other
similar macros are named.

This patch makes the naming more consistent, and appends "64"
as a reminder that this ESR pattern only matches from AArch64
state.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/debug-monitors.h |    5 +++--
 arch/arm64/kernel/kgdb.c                |    4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index ff09058..bb97e9d 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -46,8 +46,9 @@
 /*
  * ESR values expected for dynamic and compile time BRK instruction
  */
-#define DBG_ESR_VAL_BRK(x) \
-	((ESR_ELx_EC_BRK64 << ESR_ELx_EC_SHIFT) | ESR_ELx_IL | ((x) & 0xffff))
+#define ESR_ELx_VAL_BRK64(imm)					\
+	((ESR_ELx_EC_BRK64 << ESR_ELx_EC_SHIFT) | ESR_ELx_IL |	\
+	 ((imm) & 0xffff))
 
 /*
  * #imm16 values used for BRK instruction generation
diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index a0d10c5..a5a838e 100644
--- a/arch/arm64/kernel/kgdb.c
+++ b/arch/arm64/kernel/kgdb.c
@@ -235,13 +235,13 @@ static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned int esr)
 
 static struct break_hook kgdb_brkpt_hook = {
 	.esr_mask	= 0xffffffff,
-	.esr_val	= DBG_ESR_VAL_BRK(KGDB_DYN_DBG_BRK_IMM),
+	.esr_val	= ESR_ELx_VAL_BRK64(KGDB_DYN_DBG_BRK_IMM),
 	.fn		= kgdb_brk_fn
 };
 
 static struct break_hook kgdb_compiled_brkpt_hook = {
 	.esr_mask	= 0xffffffff,
-	.esr_val	= DBG_ESR_VAL_BRK(KGDB_COMPILED_DBG_BRK_IMM),
+	.esr_val	= ESR_ELx_VAL_BRK64(KGDB_COMPILED_DBG_BRK_IMM),
 	.fn		= kgdb_compiled_brk_fn
 };
 
-- 
1.7.10.4

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

* [PATCH v3 5/9] arm64/debug: Move BRK ESR template macro into <asm/esr.h>
  2015-07-24 15:37 [PATCH v3 0/9] arm64: Use BRK instruction for generic BUG traps Dave Martin
                   ` (3 preceding siblings ...)
  2015-07-24 15:37 ` [PATCH v3 4/9] arm64/debug: More consistent naming for the BRK ESR template macro Dave Martin
@ 2015-07-24 15:37 ` Dave Martin
  2015-07-24 15:37 ` [PATCH v3 6/9] arm64/debug: Simplify BRK insn opcode declarations Dave Martin
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Dave Martin @ 2015-07-24 15:37 UTC (permalink / raw)
  To: linux-arm-kernel

It makes sense to keep all the architectural exception syndrome
definitions in the same place.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/debug-monitors.h |    7 -------
 arch/arm64/include/asm/esr.h            |    7 +++++++
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index bb97e9d..e28b1dd 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -44,13 +44,6 @@
 #define BREAK_INSTR_SIZE		AARCH64_INSN_SIZE
 
 /*
- * ESR values expected for dynamic and compile time BRK instruction
- */
-#define ESR_ELx_VAL_BRK64(imm)					\
-	((ESR_ELx_EC_BRK64 << ESR_ELx_EC_SHIFT) | ESR_ELx_IL |	\
-	 ((imm) & 0xffff))
-
-/*
  * #imm16 values used for BRK instruction generation
  * Allowed values for kgbd are 0x400 - 0x7ff
  * 0x100: for triggering a fault on purpose (reserved)
diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index 7052245..1b44cf6 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -99,6 +99,13 @@
 #define ESR_ELx_WFx_ISS_WFE	(UL(1) << 0)
 #define ESR_ELx_xVC_IMM_MASK	((1UL << 16) - 1)
 
+/* ESR value templates for specific events */
+
+/* BRK instruction trap from AArch64 state */
+#define ESR_ELx_VAL_BRK64(imm)					\
+	((ESR_ELx_EC_BRK64 << ESR_ELx_EC_SHIFT) | ESR_ELx_IL |	\
+	 ((imm) & 0xffff))
+
 #ifndef __ASSEMBLY__
 #include <asm/types.h>
 
-- 
1.7.10.4

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

* [PATCH v3 6/9] arm64/debug: Simplify BRK insn opcode declarations
  2015-07-24 15:37 [PATCH v3 0/9] arm64: Use BRK instruction for generic BUG traps Dave Martin
                   ` (4 preceding siblings ...)
  2015-07-24 15:37 ` [PATCH v3 5/9] arm64/debug: Move BRK ESR template macro into <asm/esr.h> Dave Martin
@ 2015-07-24 15:37 ` Dave Martin
  2015-07-24 15:37 ` [PATCH v3 7/9] arm64/debug: Add missing #includes Dave Martin
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Dave Martin @ 2015-07-24 15:37 UTC (permalink / raw)
  To: linux-arm-kernel

The way the KGDB_DYN_BRK_INS_BYTEx macros are declared is more
complex than it needs to be.  Also, the macros are only used in one
place, which is arch-specific anyway.

This patch refactors the macros to simplify them, and exposes an
argument so that we can have a single macro instead of 4.

As a side effect, this patch also fixes some anomalous spellings of
"KGDB".

These changes alter the compile types of some integer constants
that are harmless but trigger truncation warnings in gcc when
assigning to 32-bit variables.  This patch adds an explicit cast
for the affected cases.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/include/asm/debug-monitors.h |   23 ++++-------------------
 arch/arm64/kernel/kgdb.c                |   12 ++++++------
 2 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index e28b1dd..6a17fb8 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -66,25 +66,10 @@
  */
 #define AARCH64_BREAK_FAULT	(AARCH64_BREAK_MON | (FAULT_BRK_IMM << 5))
 
-/*
- * Extract byte from BRK instruction
- */
-#define KGDB_DYN_DBG_BRK_INS_BYTE(x) \
-	((((AARCH64_BREAK_MON) & 0xffe0001f) >> (x * 8)) & 0xff)
-
-/*
- * Extract byte from BRK #imm16
- */
-#define KGBD_DYN_DBG_BRK_IMM_BYTE(x) \
-	(((((KGDB_DYN_DBG_BRK_IMM) & 0xffff) << 5) >> (x * 8)) & 0xff)
-
-#define KGDB_DYN_DBG_BRK_BYTE(x) \
-	(KGDB_DYN_DBG_BRK_INS_BYTE(x) | KGBD_DYN_DBG_BRK_IMM_BYTE(x))
-
-#define  KGDB_DYN_BRK_INS_BYTE0  KGDB_DYN_DBG_BRK_BYTE(0)
-#define  KGDB_DYN_BRK_INS_BYTE1  KGDB_DYN_DBG_BRK_BYTE(1)
-#define  KGDB_DYN_BRK_INS_BYTE2  KGDB_DYN_DBG_BRK_BYTE(2)
-#define  KGDB_DYN_BRK_INS_BYTE3  KGDB_DYN_DBG_BRK_BYTE(3)
+#define AARCH64_BREAK_KGDB_DYN_DBG	\
+	(AARCH64_BREAK_MON | (KGDB_DYN_DBG_BRK_IMM << 5))
+#define KGDB_DYN_BRK_INS_BYTE(x)	\
+	((AARCH64_BREAK_KGDB_DYN_DBG >> (8 * (x))) & 0xff)
 
 #define CACHE_FLUSH_IS_SAFE		1
 
diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index a5a838e..bcac81e 100644
--- a/arch/arm64/kernel/kgdb.c
+++ b/arch/arm64/kernel/kgdb.c
@@ -235,13 +235,13 @@ static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned int esr)
 
 static struct break_hook kgdb_brkpt_hook = {
 	.esr_mask	= 0xffffffff,
-	.esr_val	= ESR_ELx_VAL_BRK64(KGDB_DYN_DBG_BRK_IMM),
+	.esr_val	= (u32)ESR_ELx_VAL_BRK64(KGDB_DYN_DBG_BRK_IMM),
 	.fn		= kgdb_brk_fn
 };
 
 static struct break_hook kgdb_compiled_brkpt_hook = {
 	.esr_mask	= 0xffffffff,
-	.esr_val	= ESR_ELx_VAL_BRK64(KGDB_COMPILED_DBG_BRK_IMM),
+	.esr_val	= (u32)ESR_ELx_VAL_BRK64(KGDB_COMPILED_DBG_BRK_IMM),
 	.fn		= kgdb_compiled_brk_fn
 };
 
@@ -328,9 +328,9 @@ void kgdb_arch_exit(void)
  */
 struct kgdb_arch arch_kgdb_ops = {
 	.gdb_bpt_instr = {
-		KGDB_DYN_BRK_INS_BYTE0,
-		KGDB_DYN_BRK_INS_BYTE1,
-		KGDB_DYN_BRK_INS_BYTE2,
-		KGDB_DYN_BRK_INS_BYTE3,
+		KGDB_DYN_BRK_INS_BYTE(0),
+		KGDB_DYN_BRK_INS_BYTE(1),
+		KGDB_DYN_BRK_INS_BYTE(2),
+		KGDB_DYN_BRK_INS_BYTE(3),
 	}
 };
-- 
1.7.10.4

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

* [PATCH v3 7/9] arm64/debug: Add missing #includes
  2015-07-24 15:37 [PATCH v3 0/9] arm64: Use BRK instruction for generic BUG traps Dave Martin
                   ` (5 preceding siblings ...)
  2015-07-24 15:37 ` [PATCH v3 6/9] arm64/debug: Simplify BRK insn opcode declarations Dave Martin
@ 2015-07-24 15:37 ` Dave Martin
  2015-07-24 15:37 ` [PATCH v3 8/9] arm64/BUG: Use BRK instruction for generic BUG traps Dave Martin
  2015-07-24 15:37 ` [PATCH v3 9/9] arm64/BUG: Show explicit backtrace for WARNs Dave Martin
  8 siblings, 0 replies; 10+ messages in thread
From: Dave Martin @ 2015-07-24 15:37 UTC (permalink / raw)
  To: linux-arm-kernel

<asm/debug-monitors.h> relies on <asm/ptrace.h>, but doesn't
declare this dependency.  This becomes a problem once
debug-monitors.h starts getting included all over the place to get
the BRK immedates.

The missing include of <asm/memory.h> (for UL()) in <asm/esr.h> is
also added.  The series no longer relies on this, but I spotted it
during development and it may as well get fixed.

No functional change.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/include/asm/debug-monitors.h |    3 +++
 arch/arm64/include/asm/esr.h            |    2 ++
 2 files changed, 5 insertions(+)

diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index 6a17fb8..777c36a 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -18,8 +18,11 @@
 
 #ifdef __KERNEL__
 
+#include <linux/errno.h>
+#include <linux/types.h>
 #include <asm/esr.h>
 #include <asm/insn.h>
+#include <asm/ptrace.h>
 
 /* Low-level stepping controls. */
 #define DBG_MDSCR_SS		(1 << 0)
diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index 1b44cf6..77eeb2c 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -18,6 +18,8 @@
 #ifndef __ASM_ESR_H
 #define __ASM_ESR_H
 
+#include <asm/memory.h>
+
 #define ESR_ELx_EC_UNKNOWN	(0x00)
 #define ESR_ELx_EC_WFx		(0x01)
 /* Unallocated EC: 0x02 */
-- 
1.7.10.4

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

* [PATCH v3 8/9] arm64/BUG: Use BRK instruction for generic BUG traps
  2015-07-24 15:37 [PATCH v3 0/9] arm64: Use BRK instruction for generic BUG traps Dave Martin
                   ` (6 preceding siblings ...)
  2015-07-24 15:37 ` [PATCH v3 7/9] arm64/debug: Add missing #includes Dave Martin
@ 2015-07-24 15:37 ` Dave Martin
  2015-07-24 15:37 ` [PATCH v3 9/9] arm64/BUG: Show explicit backtrace for WARNs Dave Martin
  8 siblings, 0 replies; 10+ messages in thread
From: Dave Martin @ 2015-07-24 15:37 UTC (permalink / raw)
  To: linux-arm-kernel

Currently, the minimal default BUG() implementation from asm-
generic is used for arm64.

This patch uses the BRK software breakpoint instruction to generate
a trap instead, similarly to most other arches, with the generic
BUG code generating the dmesg boilerplate.

This allows bug metadata to be moved to a separate table and
reduces the amount of inline code at BUG and WARN sites.  This also
avoids clobbering any registers before they can be dumped.

To mitigate the size of the bug table further, this patch makes
use of the existing infrastructure for encoding addresses within
the bug table as 32-bit offsets instead of absolute pointers.
(Note that this limits the kernel size to 2GB.)

Traps are registered at arch_initcall time for aarch64, but BUG
has minimal real dependencies and it is desirable to be able to
generate bug splats as early as possible.  This patch redirects
all debug exceptions caused by BRK directly to bug_handler() until
the full debug exception support has been initialised.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/Kconfig                      |    8 ++++
 arch/arm64/include/asm/bug.h            |   64 +++++++++++++++++++++++++++++++
 arch/arm64/include/asm/debug-monitors.h |    2 +
 arch/arm64/kernel/traps.c               |   59 +++++++++++++++++++++++++++-
 arch/arm64/mm/fault.c                   |   12 +++++-
 5 files changed, 142 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm64/include/asm/bug.h

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 318175f..b918286 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -113,6 +113,14 @@ config TRACE_IRQFLAGS_SUPPORT
 config RWSEM_XCHGADD_ALGORITHM
 	def_bool y
 
+config GENERIC_BUG
+	def_bool y
+	depends on BUG
+
+config GENERIC_BUG_RELATIVE_POINTERS
+	def_bool y
+	depends on GENERIC_BUG
+
 config GENERIC_HWEIGHT
 	def_bool y
 
diff --git a/arch/arm64/include/asm/bug.h b/arch/arm64/include/asm/bug.h
new file mode 100644
index 0000000..4a748ce
--- /dev/null
+++ b/arch/arm64/include/asm/bug.h
@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 2015  ARM Limited
+ * Author: Dave Martin <Dave.Martin@arm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _ARCH_ARM64_ASM_BUG_H
+#define _ARCH_ARM64_ASM_BUG_H
+
+#include <asm/debug-monitors.h>
+
+#ifdef CONFIG_GENERIC_BUG
+#define HAVE_ARCH_BUG
+
+#ifdef CONFIG_DEBUG_BUGVERBOSE
+#define _BUGVERBOSE_LOCATION(file, line) __BUGVERBOSE_LOCATION(file, line)
+#define __BUGVERBOSE_LOCATION(file, line)				\
+		".pushsection .rodata.str,\"aMS\", at progbits,1\n"	\
+	"2:	.string \"" file "\"\n\t"				\
+		".popsection\n\t"					\
+									\
+		".long 2b - 0b\n\t"					\
+		".short " #line "\n\t"
+#else
+#define _BUGVERBOSE_LOCATION(file, line)
+#endif
+
+#define _BUG_FLAGS(flags) __BUG_FLAGS(flags)
+
+#define __BUG_FLAGS(flags) asm volatile (		\
+		".pushsection __bug_table,\"a\"\n\t"	\
+		".align 2\n\t"				\
+	"0:	.long 1f - 0b\n\t"			\
+_BUGVERBOSE_LOCATION(__FILE__, __LINE__)		\
+		".short " #flags "\n\t"			\
+		".popsection\n"				\
+							\
+	"1:	brk %[imm]"				\
+		:: [imm] "i" (BUG_BRK_IMM)		\
+)
+
+#define BUG() do {				\
+	_BUG_FLAGS(0);				\
+	unreachable();				\
+} while (0)
+
+#define __WARN_TAINT(taint) _BUG_FLAGS(BUGFLAG_TAINT(taint))
+
+#endif /* ! CONFIG_GENERIC_BUG */
+
+#include <asm-generic/bug.h>
+
+#endif /* ! _ARCH_ARM64_ASM_BUG_H */
diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index 777c36a..e3f2bad 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -52,10 +52,12 @@
  * 0x100: for triggering a fault on purpose (reserved)
  * 0x400: for dynamic BRK instruction
  * 0x401: for compile time BRK instruction
+ * 0x800: kernel-mode BUG() and WARN() traps
  */
 #define FAULT_BRK_IMM			0x100
 #define KGDB_DYN_DBG_BRK_IMM		0x400
 #define KGDB_COMPILED_DBG_BRK_IMM	0x401
+#define BUG_BRK_IMM			0x800
 
 /*
  * BRK instruction encoding
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 566bc4c..b10f4bf 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -17,6 +17,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/bug.h>
 #include <linux/signal.h>
 #include <linux/personality.h>
 #include <linux/kallsyms.h>
@@ -32,8 +33,10 @@
 #include <linux/syscalls.h>
 
 #include <asm/atomic.h>
+#include <asm/bug.h>
 #include <asm/debug-monitors.h>
 #include <asm/esr.h>
+#include <asm/insn.h>
 #include <asm/traps.h>
 #include <asm/stacktrace.h>
 #include <asm/exception.h>
@@ -459,7 +462,61 @@ void __pgd_error(const char *file, int line, unsigned long val)
 	pr_crit("%s:%d: bad pgd %016lx.\n", file, line, val);
 }
 
+/* GENERIC_BUG traps */
+
+int is_valid_bugaddr(unsigned long addr)
+{
+	/*
+	 * bug_handler() only called for BRK #BUG_BRK_IMM.
+	 * So the answer is trivial -- any spurious instances with no
+	 * bug table entry will be rejected by report_bug() and passed
+	 * back to the debug-monitors code and handled as a fatal
+	 * unexpected debug exception.
+	 */
+	return 1;
+}
+
+static int bug_handler(struct pt_regs *regs, unsigned int esr)
+{
+	if (user_mode(regs))
+		return DBG_HOOK_ERROR;
+
+	switch (report_bug(regs->pc, regs)) {
+	case BUG_TRAP_TYPE_BUG:
+		die("Oops - BUG", regs, 0);
+		break;
+
+	case BUG_TRAP_TYPE_WARN:
+		break;
+
+	default:
+		/* unknown/unrecognised bug trap type */
+		return DBG_HOOK_ERROR;
+	}
+
+	/* If thread survives, skip over the BUG instruction and continue: */
+	regs->pc += AARCH64_INSN_SIZE;	/* skip BRK and resume */
+	return DBG_HOOK_HANDLED;
+}
+
+static struct break_hook bug_break_hook = {
+	.esr_val = 0xf2000000 | BUG_BRK_IMM,
+	.esr_mask = 0xffffffff,
+	.fn = bug_handler,
+};
+
+/*
+ * Initial handler for AArch64 BRK exceptions
+ * This handler only used until debug_traps_init().
+ */
+int __init early_brk64(unsigned long addr, unsigned int esr,
+		struct pt_regs *regs)
+{
+	return bug_handler(regs, esr) != DBG_HOOK_HANDLED;
+}
+
+/* This registration must happen early, before debug_traps_init(). */
 void __init trap_init(void)
 {
-	return;
+	register_break_hook(&bug_break_hook);
 }
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 94d98cd..49da50d 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -492,14 +492,22 @@ asmlinkage void __exception do_sp_pc_abort(unsigned long addr,
 	arm64_notify_die("Oops - SP/PC alignment exception", regs, &info, esr);
 }
 
-static struct fault_info debug_fault_info[] = {
+int __init early_brk64(unsigned long addr, unsigned int esr,
+		       struct pt_regs *regs);
+
+/*
+ * __refdata because early_brk64 is __init, but the reference to it is
+ * clobbered at arch_initcall time.
+ * See traps.c and debug-monitors.c:debug_traps_init().
+ */
+static struct fault_info __refdata debug_fault_info[] = {
 	{ do_bad,	SIGTRAP,	TRAP_HWBKPT,	"hardware breakpoint"	},
 	{ do_bad,	SIGTRAP,	TRAP_HWBKPT,	"hardware single-step"	},
 	{ do_bad,	SIGTRAP,	TRAP_HWBKPT,	"hardware watchpoint"	},
 	{ do_bad,	SIGBUS,		0,		"unknown 3"		},
 	{ do_bad,	SIGTRAP,	TRAP_BRKPT,	"aarch32 BKPT"		},
 	{ do_bad,	SIGTRAP,	0,		"aarch32 vector catch"	},
-	{ do_bad,	SIGTRAP,	TRAP_BRKPT,	"aarch64 BRK"		},
+	{ early_brk64,	SIGTRAP,	TRAP_BRKPT,	"aarch64 BRK"		},
 	{ do_bad,	SIGBUS,		0,		"unknown 7"		},
 };
 
-- 
1.7.10.4

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

* [PATCH v3 9/9] arm64/BUG: Show explicit backtrace for WARNs
  2015-07-24 15:37 [PATCH v3 0/9] arm64: Use BRK instruction for generic BUG traps Dave Martin
                   ` (7 preceding siblings ...)
  2015-07-24 15:37 ` [PATCH v3 8/9] arm64/BUG: Use BRK instruction for generic BUG traps Dave Martin
@ 2015-07-24 15:37 ` Dave Martin
  8 siblings, 0 replies; 10+ messages in thread
From: Dave Martin @ 2015-07-24 15:37 UTC (permalink / raw)
  To: linux-arm-kernel

The generic slowpath WARN implementation prints a backtrace, but
the report_bug() based implementation does not, opting to print the
registers instead which is generally not as useful.

Ideally, report_bug() should be fixed to make the behaviour more
consistent, but in the meantime this patch generates a backtrace
directly from the arm64 backend instead so that this functionality
is not lost with the migration to report_bug().

As a side-effect, the backtrace will be outside the oops end
marker, but that's hard to avoid without modifying generic code.

This patch can go away if report_bug() grows the ability in the
future to generate a backtrace directly or call an arch hook at the
appropriate time.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/kernel/traps.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index b10f4bf..bc3dfc5 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -487,6 +487,8 @@ static int bug_handler(struct pt_regs *regs, unsigned int esr)
 		break;
 
 	case BUG_TRAP_TYPE_WARN:
+		/* Ideally, report_bug() should backtrace for us... but no. */
+		dump_backtrace(regs, NULL);
 		break;
 
 	default:
-- 
1.7.10.4

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

end of thread, other threads:[~2015-07-24 15:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-24 15:37 [PATCH v3 0/9] arm64: Use BRK instruction for generic BUG traps Dave Martin
2015-07-24 15:37 ` [PATCH v3 1/9] arm64/debug: Eliminate magic number for size of BRK instruction Dave Martin
2015-07-24 15:37 ` [PATCH v3 2/9] arm64/debug: Mask off all reserved bits from generated ESR values Dave Martin
2015-07-24 15:37 ` [PATCH v3 3/9] arm64/debug: Eliminate magic number from ESR template definition Dave Martin
2015-07-24 15:37 ` [PATCH v3 4/9] arm64/debug: More consistent naming for the BRK ESR template macro Dave Martin
2015-07-24 15:37 ` [PATCH v3 5/9] arm64/debug: Move BRK ESR template macro into <asm/esr.h> Dave Martin
2015-07-24 15:37 ` [PATCH v3 6/9] arm64/debug: Simplify BRK insn opcode declarations Dave Martin
2015-07-24 15:37 ` [PATCH v3 7/9] arm64/debug: Add missing #includes Dave Martin
2015-07-24 15:37 ` [PATCH v3 8/9] arm64/BUG: Use BRK instruction for generic BUG traps Dave Martin
2015-07-24 15:37 ` [PATCH v3 9/9] arm64/BUG: Show explicit backtrace for WARNs Dave Martin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.