linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] ARM: hw_breakpoint: updates for 3.2
@ 2011-08-15 17:41 Will Deacon
  2011-08-15 17:41 ` [PATCH v2 1/5] ARM: hw_breakpoint: add initial Cortex-A15 (debug v7.1) support Will Deacon
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Will Deacon @ 2011-08-15 17:41 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

This is version 2 of the patch series originally posted here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2011-August/060330.html

Changes since v1 include:

  * We no longer pretend to support the v7 memory-mapped interface
  * Swapped patch 4 and patch 5
  * Based on -rc2

I'd appreciate any testing with this lot applied (even if it's just boot
testing with CONFIG_HAVE_HW_BREAKPOINT enabled).

Thanks,

Will


Will Deacon (5):
  ARM: hw_breakpoint: add initial Cortex-A15 (debug v7.1) support
  ARM: hw_breakpoint: reserve one breakpoint for watchpoint stepping
  ARM: hw_breakpoint: add support for multiple watchpoints
  ARM: hw_breakpoint: trap undef instruction exceptions in
    reset_ctrl_regs
  ARM: hw_breakpoint: reduce the number of WARN_ONCE invocations

 arch/arm/include/asm/hw_breakpoint.h |    2 +
 arch/arm/kernel/hw_breakpoint.c      |  270 +++++++++++++++++++++-------------
 2 files changed, 167 insertions(+), 105 deletions(-)

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

* [PATCH v2 1/5] ARM: hw_breakpoint: add initial Cortex-A15 (debug v7.1) support
  2011-08-15 17:41 [PATCH v2 0/5] ARM: hw_breakpoint: updates for 3.2 Will Deacon
@ 2011-08-15 17:41 ` Will Deacon
  2011-08-15 17:41 ` [PATCH v2 2/5] ARM: hw_breakpoint: reserve one breakpoint for watchpoint stepping Will Deacon
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2011-08-15 17:41 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds initial support for Cortex-A15 (debug architecture v7.1)
to the hw_breakpoint ARM backend.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/hw_breakpoint.h |    1 +
 arch/arm/kernel/hw_breakpoint.c      |   55 ++++++++++++++++++++++------------
 2 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/arch/arm/include/asm/hw_breakpoint.h b/arch/arm/include/asm/hw_breakpoint.h
index f389b27..0ac141a 100644
--- a/arch/arm/include/asm/hw_breakpoint.h
+++ b/arch/arm/include/asm/hw_breakpoint.h
@@ -50,6 +50,7 @@ static inline void decode_ctrl_reg(u32 reg,
 #define ARM_DEBUG_ARCH_V6_1	2
 #define ARM_DEBUG_ARCH_V7_ECP14	3
 #define ARM_DEBUG_ARCH_V7_MM	4
+#define ARM_DEBUG_ARCH_V7_1	5
 
 /* Breakpoint */
 #define ARM_BREAKPOINT_EXECUTE	0
diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
index a927ca1..b6ddbfa 100644
--- a/arch/arm/kernel/hw_breakpoint.c
+++ b/arch/arm/kernel/hw_breakpoint.c
@@ -154,7 +154,10 @@ u8 arch_get_debug_arch(void)
 static int debug_arch_supported(void)
 {
 	u8 arch = get_debug_arch();
-	return arch >= ARM_DEBUG_ARCH_V6 && arch <= ARM_DEBUG_ARCH_V7_ECP14;
+
+	/* We don't support the memory-mapped interface. */
+	return (arch >= ARM_DEBUG_ARCH_V6 && arch <= ARM_DEBUG_ARCH_V7_ECP14) ||
+		arch >= ARM_DEBUG_ARCH_V7_1;
 }
 
 /* Determine number of BRP register available. */
@@ -255,6 +258,7 @@ static int enable_monitor_mode(void)
 		ARM_DBG_WRITE(c1, 0, (dscr | ARM_DSCR_MDBGEN));
 		break;
 	case ARM_DEBUG_ARCH_V7_ECP14:
+	case ARM_DEBUG_ARCH_V7_1:
 		ARM_DBG_WRITE(c2, 2, (dscr | ARM_DSCR_MDBGEN));
 		break;
 	default:
@@ -836,7 +840,7 @@ static int hw_breakpoint_pending(unsigned long addr, unsigned int fsr,
  */
 static void reset_ctrl_regs(void *info)
 {
-	int i, cpu = smp_processor_id();
+	int i, err = 0, cpu = smp_processor_id();
 	u32 dbg_power;
 	cpumask_t *cpumask = info;
 
@@ -848,33 +852,46 @@ static void reset_ctrl_regs(void *info)
 	 * Access Register to avoid taking undefined instruction exceptions
 	 * later on.
 	 */
-	if (debug_arch >= ARM_DEBUG_ARCH_V7_ECP14) {
+	switch (debug_arch) {
+	case ARM_DEBUG_ARCH_V7_ECP14:
 		/*
 		 * Ensure sticky power-down is clear (i.e. debug logic is
 		 * powered up).
 		 */
 		asm volatile("mrc p14, 0, %0, c1, c5, 4" : "=r" (dbg_power));
-		if ((dbg_power & 0x1) == 0) {
-			pr_warning("CPU %d debug is powered down!\n", cpu);
-			cpumask_or(cpumask, cpumask, cpumask_of(cpu));
-			return;
-		}
-
+		if ((dbg_power & 0x1) == 0)
+			err = -EPERM;
+		break;
+	case ARM_DEBUG_ARCH_V7_1:
 		/*
-		 * Unconditionally clear the lock by writing a value
-		 * other than 0xC5ACCE55 to the access register.
+		 * Ensure the OS double lock is clear.
 		 */
-		asm volatile("mcr p14, 0, %0, c1, c0, 4" : : "r" (0));
-		isb();
+		asm volatile("mrc p14, 0, %0, c1, c3, 4" : "=r" (dbg_power));
+		if ((dbg_power & 0x1) == 1)
+			err = -EPERM;
+		break;
+	}
 
-		/*
-		 * Clear any configured vector-catch events before
-		 * enabling monitor mode.
-		 */
-		asm volatile("mcr p14, 0, %0, c0, c7, 0" : : "r" (0));
-		isb();
+	if (err) {
+		pr_warning("CPU %d debug is powered down!\n", cpu);
+		cpumask_or(cpumask, cpumask, cpumask_of(cpu));
+		return;
 	}
 
+	/*
+	 * Unconditionally clear the lock by writing a value
+	 * other than 0xC5ACCE55 to the access register.
+	 */
+	asm volatile("mcr p14, 0, %0, c1, c0, 4" : : "r" (0));
+	isb();
+
+	/*
+	 * Clear any configured vector-catch events before
+	 * enabling monitor mode.
+	 */
+	asm volatile("mcr p14, 0, %0, c0, c7, 0" : : "r" (0));
+	isb();
+
 	if (enable_monitor_mode())
 		return;
 
-- 
1.7.0.4

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

* [PATCH v2 2/5] ARM: hw_breakpoint: reserve one breakpoint for watchpoint stepping
  2011-08-15 17:41 [PATCH v2 0/5] ARM: hw_breakpoint: updates for 3.2 Will Deacon
  2011-08-15 17:41 ` [PATCH v2 1/5] ARM: hw_breakpoint: add initial Cortex-A15 (debug v7.1) support Will Deacon
@ 2011-08-15 17:41 ` Will Deacon
  2011-08-15 17:41 ` [PATCH v2 3/5] ARM: hw_breakpoint: add support for multiple watchpoints Will Deacon
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2011-08-15 17:41 UTC (permalink / raw)
  To: linux-arm-kernel

The current hw_breakpoint code on ARM reserves 1 breakpoint for each
watchpoint that is available. Since debug architectures prior to 7.1
are restricted to 1 watchpoint anyway, only one breakpoint was ever
reserved.

This patch changes the reservation strategy so that a single breakpoint
is reserved, regardless of the number of watchpoints. This is in
preparation for multiple-watchpoint support on debug architectures
from 7.1 onwards.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/hw_breakpoint.c |   63 +++++++++++++++------------------------
 1 files changed, 24 insertions(+), 39 deletions(-)

diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
index b6ddbfa..156b8af 100644
--- a/arch/arm/kernel/hw_breakpoint.c
+++ b/arch/arm/kernel/hw_breakpoint.c
@@ -45,7 +45,6 @@ static DEFINE_PER_CPU(struct perf_event *, wp_on_reg[ARM_MAX_WRP]);
 
 /* Number of BRP/WRP registers on this CPU. */
 static int core_num_brps;
-static int core_num_reserved_brps;
 static int core_num_wrps;
 
 /* Debug architecture version. */
@@ -160,7 +159,15 @@ static int debug_arch_supported(void)
 		arch >= ARM_DEBUG_ARCH_V7_1;
 }
 
-/* Determine number of BRP register available. */
+/* Determine number of WRP registers available. */
+static int get_num_wrp_resources(void)
+{
+	u32 didr;
+	ARM_DBG_READ(c0, 0, didr);
+	return ((didr >> 28) & 0xf) + 1;
+}
+
+/* Determine number of BRP registers available. */
 static int get_num_brp_resources(void)
 {
 	u32 didr;
@@ -179,9 +186,10 @@ static int core_has_mismatch_brps(void)
 static int get_num_wrps(void)
 {
 	/*
-	 * FIXME: When a watchpoint fires, the only way to work out which
-	 * watchpoint it was is by disassembling the faulting instruction
-	 * and working out the address of the memory access.
+	 * On debug architectures prior to 7.1, when a watchpoint fires, the
+	 * only way to work out which watchpoint it was is by disassembling
+	 * the faulting instruction and working out the address of the memory
+	 * access.
 	 *
 	 * Furthermore, we can only do this if the watchpoint was precise
 	 * since imprecise watchpoints prevent us from calculating register
@@ -195,36 +203,17 @@ static int get_num_wrps(void)
 	 * [the ARM ARM states that the DFAR is UNKNOWN, but experience shows
 	 * that it is set on some implementations].
 	 */
+	if (get_debug_arch() < ARM_DEBUG_ARCH_V7_1)
+		return 1;
 
-#if 0
-	int wrps;
-	u32 didr;
-	ARM_DBG_READ(c0, 0, didr);
-	wrps = ((didr >> 28) & 0xf) + 1;
-#endif
-	int wrps = 1;
-
-	if (core_has_mismatch_brps() && wrps >= get_num_brp_resources())
-		wrps = get_num_brp_resources() - 1;
-
-	return wrps;
-}
-
-/* We reserve one breakpoint for each watchpoint. */
-static int get_num_reserved_brps(void)
-{
-	if (core_has_mismatch_brps())
-		return get_num_wrps();
-	return 0;
+	return get_num_wrp_resources();
 }
 
 /* Determine number of usable BRPs available. */
 static int get_num_brps(void)
 {
 	int brps = get_num_brp_resources();
-	if (core_has_mismatch_brps())
-		brps -= get_num_reserved_brps();
-	return brps;
+	return core_has_mismatch_brps() ? brps - 1 : brps;
 }
 
 /*
@@ -721,7 +710,7 @@ static void watchpoint_single_step_handler(unsigned long pc)
 
 	slots = (struct perf_event **)__get_cpu_var(wp_on_reg);
 
-	for (i = 0; i < core_num_reserved_brps; ++i) {
+	for (i = 0; i < core_num_wrps; ++i) {
 		rcu_read_lock();
 
 		wp = slots[i];
@@ -840,7 +829,7 @@ static int hw_breakpoint_pending(unsigned long addr, unsigned int fsr,
  */
 static void reset_ctrl_regs(void *info)
 {
-	int i, err = 0, cpu = smp_processor_id();
+	int i, raw_num_brps, err = 0, cpu = smp_processor_id();
 	u32 dbg_power;
 	cpumask_t *cpumask = info;
 
@@ -896,7 +885,8 @@ static void reset_ctrl_regs(void *info)
 		return;
 
 	/* We must also reset any reserved registers. */
-	for (i = 0; i < core_num_brps + core_num_reserved_brps; ++i) {
+	raw_num_brps = get_num_brp_resources();
+	for (i = 0; i < raw_num_brps; ++i) {
 		write_wb_reg(ARM_BASE_BCR + i, 0UL);
 		write_wb_reg(ARM_BASE_BVR + i, 0UL);
 	}
@@ -933,15 +923,11 @@ static int __init arch_hw_breakpoint_init(void)
 
 	/* Determine how many BRPs/WRPs are available. */
 	core_num_brps = get_num_brps();
-	core_num_reserved_brps = get_num_reserved_brps();
 	core_num_wrps = get_num_wrps();
 
-	pr_info("found %d breakpoint and %d watchpoint registers.\n",
-		core_num_brps + core_num_reserved_brps, core_num_wrps);
-
-	if (core_num_reserved_brps)
-		pr_info("%d breakpoint(s) reserved for watchpoint "
-				"single-step.\n", core_num_reserved_brps);
+	pr_info("found %d " "%s" "breakpoint and %d watchpoint registers.\n",
+		core_num_brps, core_has_mismatch_brps() ? "(+1 reserved) " :
+		"", core_num_wrps);
 
 	/*
 	 * Reset the breakpoint resources. We assume that a halting
@@ -950,7 +936,6 @@ static int __init arch_hw_breakpoint_init(void)
 	on_each_cpu(reset_ctrl_regs, &cpumask, 1);
 	if (!cpumask_empty(&cpumask)) {
 		core_num_brps = 0;
-		core_num_reserved_brps = 0;
 		core_num_wrps = 0;
 		return 0;
 	}
-- 
1.7.0.4

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

* [PATCH v2 3/5] ARM: hw_breakpoint: add support for multiple watchpoints
  2011-08-15 17:41 [PATCH v2 0/5] ARM: hw_breakpoint: updates for 3.2 Will Deacon
  2011-08-15 17:41 ` [PATCH v2 1/5] ARM: hw_breakpoint: add initial Cortex-A15 (debug v7.1) support Will Deacon
  2011-08-15 17:41 ` [PATCH v2 2/5] ARM: hw_breakpoint: reserve one breakpoint for watchpoint stepping Will Deacon
@ 2011-08-15 17:41 ` Will Deacon
  2011-08-15 17:41 ` [PATCH v2 4/5] ARM: hw_breakpoint: trap undef instruction exceptions in reset_ctrl_regs Will Deacon
  2011-08-15 17:41 ` [PATCH v2 5/5] ARM: hw_breakpoint: reduce the number of WARN_ONCE invocations Will Deacon
  4 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2011-08-15 17:41 UTC (permalink / raw)
  To: linux-arm-kernel

ARM debug architecture 7.1 mandates that the DFAR is updated on a
watchpoint debug exception to contain the faulting virtual address
of the memory access. This allows us to determine which watchpoints
have fired and therefore report useful information to userspace.

This patch adds support for using the DFAR in the watchpoint handler,
which allows us to support multiple watchpoints on CPUs implementing
the new debug architecture.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/hw_breakpoint.h |    1 +
 arch/arm/kernel/hw_breakpoint.c      |  100 ++++++++++++++++++++++------------
 2 files changed, 66 insertions(+), 35 deletions(-)

diff --git a/arch/arm/include/asm/hw_breakpoint.h b/arch/arm/include/asm/hw_breakpoint.h
index 0ac141a..c190bc9 100644
--- a/arch/arm/include/asm/hw_breakpoint.h
+++ b/arch/arm/include/asm/hw_breakpoint.h
@@ -58,6 +58,7 @@ static inline void decode_ctrl_reg(u32 reg,
 /* Watchpoints */
 #define ARM_BREAKPOINT_LOAD	1
 #define ARM_BREAKPOINT_STORE	2
+#define ARM_FSR_ACCESS_MASK	(1 << 11)
 
 /* Privilege Levels */
 #define ARM_BREAKPOINT_PRIV	1
diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
index 156b8af..448143e 100644
--- a/arch/arm/kernel/hw_breakpoint.c
+++ b/arch/arm/kernel/hw_breakpoint.c
@@ -339,24 +339,10 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
 		val_base = ARM_BASE_BVR;
 		slots = (struct perf_event **)__get_cpu_var(bp_on_reg);
 		max_slots = core_num_brps;
-		if (info->step_ctrl.enabled) {
-			/* Override the breakpoint data with the step data. */
-			addr = info->trigger & ~0x3;
-			ctrl = encode_ctrl_reg(info->step_ctrl);
-		}
 	} else {
 		/* Watchpoint */
-		if (info->step_ctrl.enabled) {
-			/* Install into the reserved breakpoint region. */
-			ctrl_base = ARM_BASE_BCR + core_num_brps;
-			val_base = ARM_BASE_BVR + core_num_brps;
-			/* Override the watchpoint data with the step data. */
-			addr = info->trigger & ~0x3;
-			ctrl = encode_ctrl_reg(info->step_ctrl);
-		} else {
-			ctrl_base = ARM_BASE_WCR;
-			val_base = ARM_BASE_WVR;
-		}
+		ctrl_base = ARM_BASE_WCR;
+		val_base = ARM_BASE_WVR;
 		slots = (struct perf_event **)__get_cpu_var(wp_on_reg);
 		max_slots = core_num_wrps;
 	}
@@ -375,6 +361,17 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
 		goto out;
 	}
 
+	/* Override the breakpoint data with the step data. */
+	if (info->step_ctrl.enabled) {
+		addr = info->trigger & ~0x3;
+		ctrl = encode_ctrl_reg(info->step_ctrl);
+		if (info->ctrl.type != ARM_BREAKPOINT_EXECUTE) {
+			i = 0;
+			ctrl_base = ARM_BASE_BCR + core_num_brps;
+			val_base = ARM_BASE_BVR + core_num_brps;
+		}
+	}
+
 	/* Setup the address register. */
 	write_wb_reg(val_base + i, addr);
 
@@ -398,10 +395,7 @@ void arch_uninstall_hw_breakpoint(struct perf_event *bp)
 		max_slots = core_num_brps;
 	} else {
 		/* Watchpoint */
-		if (info->step_ctrl.enabled)
-			base = ARM_BASE_BCR + core_num_brps;
-		else
-			base = ARM_BASE_WCR;
+		base = ARM_BASE_WCR;
 		slots = (struct perf_event **)__get_cpu_var(wp_on_reg);
 		max_slots = core_num_wrps;
 	}
@@ -419,6 +413,13 @@ void arch_uninstall_hw_breakpoint(struct perf_event *bp)
 	if (WARN_ONCE(i == max_slots, "Can't find any breakpoint slot\n"))
 		return;
 
+	/* Ensure that we disable the mismatch breakpoint. */
+	if (info->ctrl.type != ARM_BREAKPOINT_EXECUTE &&
+	    info->step_ctrl.enabled) {
+		i = 0;
+		base = ARM_BASE_BCR + core_num_brps;
+	}
+
 	/* Reset the control register. */
 	write_wb_reg(base + i, 0);
 }
@@ -659,34 +660,62 @@ static void disable_single_step(struct perf_event *bp)
 	arch_install_hw_breakpoint(bp);
 }
 
-static void watchpoint_handler(unsigned long unknown, struct pt_regs *regs)
+static void watchpoint_handler(unsigned long addr, unsigned int fsr,
+			       struct pt_regs *regs)
 {
-	int i;
+	int i, access;
+	u32 val, ctrl_reg, alignment_mask;
 	struct perf_event *wp, **slots;
 	struct arch_hw_breakpoint *info;
+	struct arch_hw_breakpoint_ctrl ctrl;
 
 	slots = (struct perf_event **)__get_cpu_var(wp_on_reg);
 
-	/* Without a disassembler, we can only handle 1 watchpoint. */
-	BUG_ON(core_num_wrps > 1);
-
 	for (i = 0; i < core_num_wrps; ++i) {
 		rcu_read_lock();
 
 		wp = slots[i];
 
-		if (wp == NULL) {
-			rcu_read_unlock();
-			continue;
-		}
+		if (wp == NULL)
+			goto unlock;
 
+		info = counter_arch_bp(wp);
 		/*
-		 * The DFAR is an unknown value. Since we only allow a
-		 * single watchpoint, we can set the trigger to the lowest
-		 * possible faulting address.
+		 * The DFAR is an unknown value on debug architectures prior
+		 * to 7.1. Since we only allow a single watchpoint on these
+		 * older CPUs, we can set the trigger to the lowest possible
+		 * faulting address.
 		 */
-		info = counter_arch_bp(wp);
-		info->trigger = wp->attr.bp_addr;
+		if (debug_arch < ARM_DEBUG_ARCH_V7_1) {
+			BUG_ON(i > 0);
+			info->trigger = wp->attr.bp_addr;
+		} else {
+			if (info->ctrl.len == ARM_BREAKPOINT_LEN_8)
+				alignment_mask = 0x7;
+			else
+				alignment_mask = 0x3;
+
+			/* Check if the watchpoint value matches. */
+			val = read_wb_reg(ARM_BASE_WVR + i);
+			if (val != (addr & ~alignment_mask))
+				goto unlock;
+
+			/* Possible match, check the byte address select. */
+			ctrl_reg = read_wb_reg(ARM_BASE_WCR + i);
+			decode_ctrl_reg(ctrl_reg, &ctrl);
+			if (!((1 << (addr & alignment_mask)) & ctrl.len))
+				goto unlock;
+
+			/* Check that the access type matches. */
+			access = (fsr & ARM_FSR_ACCESS_MASK) ? HW_BREAKPOINT_W :
+				 HW_BREAKPOINT_R;
+			if (!(access & hw_breakpoint_type(wp)))
+				goto unlock;
+
+			/* We have a winner. */
+			info->trigger = addr;
+		}
+
 		pr_debug("watchpoint fired: address = 0x%x\n", info->trigger);
 		perf_bp_event(wp, regs);
 
@@ -698,6 +727,7 @@ static void watchpoint_handler(unsigned long unknown, struct pt_regs *regs)
 		if (!wp->overflow_handler)
 			enable_single_step(wp, instruction_pointer(regs));
 
+unlock:
 		rcu_read_unlock();
 	}
 }
@@ -813,7 +843,7 @@ static int hw_breakpoint_pending(unsigned long addr, unsigned int fsr,
 	case ARM_ENTRY_ASYNC_WATCHPOINT:
 		WARN(1, "Asynchronous watchpoint exception taken. Debugging results may be unreliable\n");
 	case ARM_ENTRY_SYNC_WATCHPOINT:
-		watchpoint_handler(addr, regs);
+		watchpoint_handler(addr, fsr, regs);
 		break;
 	default:
 		ret = 1; /* Unhandled fault. */
-- 
1.7.0.4

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

* [PATCH v2 4/5] ARM: hw_breakpoint: trap undef instruction exceptions in reset_ctrl_regs
  2011-08-15 17:41 [PATCH v2 0/5] ARM: hw_breakpoint: updates for 3.2 Will Deacon
                   ` (2 preceding siblings ...)
  2011-08-15 17:41 ` [PATCH v2 3/5] ARM: hw_breakpoint: add support for multiple watchpoints Will Deacon
@ 2011-08-15 17:41 ` Will Deacon
  2011-08-15 17:41 ` [PATCH v2 5/5] ARM: hw_breakpoint: reduce the number of WARN_ONCE invocations Will Deacon
  4 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2011-08-15 17:41 UTC (permalink / raw)
  To: linux-arm-kernel

The ARM debug registers can only be accessed if the DBGSWENABLE signal
to the core is driven HIGH by the DAP. The architecture does not provide
a way to detect the value of this signal, so the best we can do is
register an undef_hook to trap debug register co-processor accesses and
then fail if the trap is taken.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/hw_breakpoint.c |   46 +++++++++++++++++++++++++++++++-------
 1 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
index 448143e..64ac5c6 100644
--- a/arch/arm/kernel/hw_breakpoint.c
+++ b/arch/arm/kernel/hw_breakpoint.c
@@ -857,11 +857,31 @@ static int hw_breakpoint_pending(unsigned long addr, unsigned int fsr,
 /*
  * One-time initialisation.
  */
-static void reset_ctrl_regs(void *info)
+static cpumask_t debug_err_mask;
+
+static int debug_reg_trap(struct pt_regs *regs, unsigned int instr)
+{
+	int cpu = smp_processor_id();
+
+	pr_warning("Debug register access (0x%x) caused undefined instruction on CPU %d\n",
+		   instr, cpu);
+
+	/* Set the error flag for this CPU and skip the faulting instruction. */
+	cpumask_set_cpu(cpu, &debug_err_mask);
+	instruction_pointer(regs) += 4;
+	return 0;
+}
+
+static struct undef_hook debug_reg_hook = {
+	.instr_mask	= 0x0fe80f10,
+	.instr_val	= 0x0e000e10,
+	.fn		= debug_reg_trap,
+};
+
+static void reset_ctrl_regs(void *unused)
 {
 	int i, raw_num_brps, err = 0, cpu = smp_processor_id();
 	u32 dbg_power;
-	cpumask_t *cpumask = info;
 
 	/*
 	 * v7 debug contains save and restore registers so that debug state
@@ -893,7 +913,7 @@ static void reset_ctrl_regs(void *info)
 
 	if (err) {
 		pr_warning("CPU %d debug is powered down!\n", cpu);
-		cpumask_or(cpumask, cpumask, cpumask_of(cpu));
+		cpumask_or(&debug_err_mask, &debug_err_mask, cpumask_of(cpu));
 		return;
 	}
 
@@ -932,6 +952,7 @@ static int __cpuinit dbg_reset_notify(struct notifier_block *self,
 {
 	if (action == CPU_ONLINE)
 		smp_call_function_single((int)cpu, reset_ctrl_regs, NULL, 1);
+
 	return NOTIFY_OK;
 }
 
@@ -942,7 +963,6 @@ static struct notifier_block __cpuinitdata dbg_reset_nb = {
 static int __init arch_hw_breakpoint_init(void)
 {
 	u32 dscr;
-	cpumask_t cpumask = { CPU_BITS_NONE };
 
 	debug_arch = get_debug_arch();
 
@@ -955,21 +975,29 @@ static int __init arch_hw_breakpoint_init(void)
 	core_num_brps = get_num_brps();
 	core_num_wrps = get_num_wrps();
 
-	pr_info("found %d " "%s" "breakpoint and %d watchpoint registers.\n",
-		core_num_brps, core_has_mismatch_brps() ? "(+1 reserved) " :
-		"", core_num_wrps);
+	/*
+	 * We need to tread carefully here because DBGSWENABLE may be
+	 * driven low on this core and there isn't an architected way to
+	 * determine that.
+	 */
+	register_undef_hook(&debug_reg_hook);
 
 	/*
 	 * Reset the breakpoint resources. We assume that a halting
 	 * debugger will leave the world in a nice state for us.
 	 */
-	on_each_cpu(reset_ctrl_regs, &cpumask, 1);
-	if (!cpumask_empty(&cpumask)) {
+	on_each_cpu(reset_ctrl_regs, NULL, 1);
+	unregister_undef_hook(&debug_reg_hook);
+	if (!cpumask_empty(&debug_err_mask)) {
 		core_num_brps = 0;
 		core_num_wrps = 0;
 		return 0;
 	}
 
+	pr_info("found %d " "%s" "breakpoint and %d watchpoint registers.\n",
+		core_num_brps, core_has_mismatch_brps() ? "(+1 reserved) " :
+		"", core_num_wrps);
+
 	ARM_DBG_READ(c1, 0, dscr);
 	if (dscr & ARM_DSCR_HDBGEN) {
 		max_watchpoint_len = 4;
-- 
1.7.0.4

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

* [PATCH v2 5/5] ARM: hw_breakpoint: reduce the number of WARN_ONCE invocations
  2011-08-15 17:41 [PATCH v2 0/5] ARM: hw_breakpoint: updates for 3.2 Will Deacon
                   ` (3 preceding siblings ...)
  2011-08-15 17:41 ` [PATCH v2 4/5] ARM: hw_breakpoint: trap undef instruction exceptions in reset_ctrl_regs Will Deacon
@ 2011-08-15 17:41 ` Will Deacon
  4 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2011-08-15 17:41 UTC (permalink / raw)
  To: linux-arm-kernel

The ARM hw_breakpoint backend is currently a bit too noisy when things
start to go awry.

This patch removes a couple of over-zealous WARN_ONCE invocations and
replaces then with pr_warnings instead.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/hw_breakpoint.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
index 64ac5c6..5a46225 100644
--- a/arch/arm/kernel/hw_breakpoint.c
+++ b/arch/arm/kernel/hw_breakpoint.c
@@ -136,10 +136,11 @@ static u8 get_debug_arch(void)
 	u32 didr;
 
 	/* Do we implement the extended CPUID interface? */
-	if (WARN_ONCE((((read_cpuid_id() >> 16) & 0xf) != 0xf),
-	    "CPUID feature registers not supported. "
-	    "Assuming v6 debug is present.\n"))
+	if (((read_cpuid_id() >> 16) & 0xf) != 0xf) {
+		pr_warning("CPUID feature registers not supported. "
+			   "Assuming v6 debug is present.\n");
 		return ARM_DEBUG_ARCH_V6;
+	}
 
 	ARM_DBG_READ(c0, 0, didr);
 	return (didr >> 16) & 0xf;
@@ -231,7 +232,7 @@ static int enable_monitor_mode(void)
 
 	/* Ensure that halting mode is disabled. */
 	if (WARN_ONCE(dscr & ARM_DSCR_HDBGEN,
-			"halting debug mode enabled. Unable to access hardware resources.\n")) {
+		"halting debug mode enabled. Unable to access hardware resources.\n")) {
 		ret = -EPERM;
 		goto out;
 	}
@@ -626,10 +627,9 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp)
 	 * we can use the mismatch feature as a poor-man's hardware
 	 * single-step, but this only works for per-task breakpoints.
 	 */
-	if (WARN_ONCE(!bp->overflow_handler &&
-		(arch_check_bp_in_kernelspace(bp) || !core_has_mismatch_brps()
-		 || !bp->hw.bp_target),
-			"overflow handler required but none found\n")) {
+	if (!bp->overflow_handler && (arch_check_bp_in_kernelspace(bp) ||
+	    !core_has_mismatch_brps() || !bp->hw.bp_target)) {
+		pr_warning("overflow handler required but none found\n");
 		ret = -EINVAL;
 	}
 out:
-- 
1.7.0.4

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

end of thread, other threads:[~2011-08-15 17:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-15 17:41 [PATCH v2 0/5] ARM: hw_breakpoint: updates for 3.2 Will Deacon
2011-08-15 17:41 ` [PATCH v2 1/5] ARM: hw_breakpoint: add initial Cortex-A15 (debug v7.1) support Will Deacon
2011-08-15 17:41 ` [PATCH v2 2/5] ARM: hw_breakpoint: reserve one breakpoint for watchpoint stepping Will Deacon
2011-08-15 17:41 ` [PATCH v2 3/5] ARM: hw_breakpoint: add support for multiple watchpoints Will Deacon
2011-08-15 17:41 ` [PATCH v2 4/5] ARM: hw_breakpoint: trap undef instruction exceptions in reset_ctrl_regs Will Deacon
2011-08-15 17:41 ` [PATCH v2 5/5] ARM: hw_breakpoint: reduce the number of WARN_ONCE invocations 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).