Skip to contentSkip to conversation

ARM: hw_breakpoint: fix reset sequence for debug arch v7.1

PATCH Superseded Will Deacon 2 participants 15 messages Oct 17, 2012 – Oct 25, 2012

Subsystems (2)
Arm (32-bit) Arm PMU profiling and debugging

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Will Deacon

author
#0 lore raw

cover ARM: hw_breakpoint: fix reset sequence for debug arch v7.1

Hello,

This set of patches fixes the hw_breakpoint debug register reset sequence for ARM debug architecture v7.1. This involves:

- Probing of the save/restore registers on v7
- Avoiding the use of DBGDSCRint.HDBGen, which is now UNKNOWN
- Only attempt to enable monitor mode during boot, then check whether it is still enabled during breakpoint validation
- Some minor changes to remain compatible with v6 processors

All feedback welcome,

Will

Dietmar Eggemann (1):
ARM: hw_breakpoint: use CRn as argument for debug reg accessor macros

Will Deacon (6):
ARM: hw_breakpoint: only clear OS lock when implemented on v7
ARM: hw_breakpoint: fix monitor mode detection with v7.1
ARM: hw_breakpoint: fix ordering of debug register reset sequence
ARM: hw_breakpoint: don't try to clear v6 debug registers during boot
ARM: hw_breakpoint: make boot quieter without CPUID feature registers
ARM: hw_breakpoint: check if monitor mode is enabled during
validation

 arch/arm/include/asm/hw_breakpoint.h |    8 +-
 arch/arm/kernel/hw_breakpoint.c      |  146 ++++++++++++++++++----------------
 2 files changed, 82 insertions(+), 72 deletions(-)
Signature · 1 line
1.7.4.1
author
#1 lore raw

PATCH 1/7 ARM: hw_breakpoint: only clear OS lock when implemented on v7

The OS save and restore register are optional in debug architecture v7, so check the status register before attempting to clear the OS lock.

Signed-off-by Will Deacon <[email protected]>
---
 arch/arm/kernel/hw_breakpoint.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)
arch/arm/kernel/hw_breakpoint.c +9 -1
index 281bf33..ec16ada 100644
@@ -929,6 +929,13 @@ static void reset_ctrl_regs(void *unused)
 		asm volatile("mrc p14, 0, %0, c1, c5, 4" : "=r" (dbg_power));
 		if ((dbg_power & 0x1) == 0)
 			err = -EPERM;
+
+		/*
+		 * Check whether we implement OS save and restore.
+		 */
+		asm volatile("mrc p14, 0, %0, c1, c1, 4" : "=r" (dbg_power));
+		if ((dbg_power & 0x9) == 0)
+			goto clear_vcr;
 		break;
 	case ARM_DEBUG_ARCH_V7_1:
 		/*

@@ -947,7 +954,7 @@ static void reset_ctrl_regs(void *unused)
 	}
 
 	/*
-	 * Unconditionally clear the lock by writing a value
+	 * Unconditionally clear the OS lock by writing a value
 	 * other than 0xC5ACCE55 to the access register.
 	 */
 	asm volatile("mcr p14, 0, %0, c1, c0, 4" : : "r" (0));

@@ -957,6 +964,7 @@ static void reset_ctrl_regs(void *unused)
 	 * Clear any configured vector-catch events before
 	 * enabling monitor mode.
 	 */
+clear_vcr:
 	asm volatile("mcr p14, 0, %0, c0, c7, 0" : : "r" (0));
 	isb();
Signature · 1 line
1.7.4.1
#2 lore raw

PATCH 1/7 ARM: hw_breakpoint: only clear OS lock when implemented on v7

Quoted 4 lines · Will Deacon

The OS save and restore register are optional in debug architecture v7, so check the status register before attempting to clear the OS lock.

Signed-off-by Will Deacon <[email protected]>
Tested-by Stephen Boyd <[email protected]>
Quoted 9 lines
---
 arch/arm/kernel/hw_breakpoint.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)
arch/arm/kernel/hw_breakpoint.c +0 -0
index 281bf33..ec16ada 100644

arch/arm/kernel/hw_breakpoint.c static void reset_ctrl_regs(void *unused) · line 936

Quoted 8 lines
arch/arm/kernel/hw_breakpoint.c +5 -0
@@ -929,6 +929,13 @@ static void reset_ctrl_regs(void *unused)
 		asm volatile("mrc p14, 0, %0, c1, c5, 4" : "=r" (dbg_power));
 		if ((dbg_power & 0x1) == 0)
 			err = -EPERM;
+
+		/*
+		 * Check whether we implement OS save and restore.
+		 */
+		asm volatile("mrc p14, 0, %0, c1, c1, 4" : "=r" (dbg_power));

minor nit for this series. dbg_power has become a catch-all variable in this code. It would be nice if we named the variables used to hold the read register the same as the register or if we made the name of the variable generic like 'val'.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
author
#3 lore raw

PATCH 1/7 ARM: hw_breakpoint: only clear OS lock when implemented on v7

Quoted 22 lines · Stephen Boyd
Quoted 4 lines · Will Deacon

The OS save and restore register are optional in debug architecture v7, so check the status register before attempting to clear the OS lock.

Signed-off-by Will Deacon <[email protected]>
Tested-by Stephen Boyd <[email protected]>
Quoted 14 lines
---
 arch/arm/kernel/hw_breakpoint.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)
arch/arm/kernel/hw_breakpoint.c +2 -0
index 281bf33..ec16ada 100644
@@ -929,6 +929,13 @@ static void reset_ctrl_regs(void *unused)
 		asm volatile("mrc p14, 0, %0, c1, c5, 4" : "=r" (dbg_power));
 		if ((dbg_power & 0x1) == 0)
 			err = -EPERM;
+
+		/*
Quoted 8 lines

arch/arm/kernel/hw_breakpoint.c static void reset_ctrl_regs(void *unused) · line 936

Quoted 3 lines
arch/arm/kernel/hw_breakpoint.c +3 -0
+		 * Check whether we implement OS save and restore.
+		 */
+		asm volatile("mrc p14, 0, %0, c1, c1, 4" : "=r" (dbg_power));

minor nit for this series. dbg_power has become a catch-all variable in this code. It would be nice if we named the variables used to hold the read register the same as the register or if we made the name of the variable generic like 'val'.

Thanks for your feedback and tested-bys on this series Stephen, I really appreciate it.

I'll send out a v2 and update the branch I'm sending to -next.

Cheers,

Will

author
#4 lore raw

PATCH 2/7 ARM: hw_breakpoint: fix monitor mode detection with v7.1

In reply to Will Deacon

Detecting whether halting debug is enabled is no longer possible via the DBGDSCR in v7.1, returning an UNKNOWN value for the HDBGen bit via CP14 when the OS lock is clear.

This patch removes the halting mode check and ensures that accesses to the internal and external views of the DBGDSCR are serialised with an instruction barrier.

Signed-off-by Will Deacon <[email protected]>
---
 arch/arm/kernel/hw_breakpoint.c |   25 +++++--------------------
 1 files changed, 5 insertions(+), 20 deletions(-)
arch/arm/kernel/hw_breakpoint.c +5 -20
index ec16ada..169dcce 100644
@@ -235,13 +235,6 @@ static int enable_monitor_mode(void)
 
 	ARM_DBG_READ(c1, 0, dscr);
 
-	/* Ensure that halting mode is disabled. */
-	if (WARN_ONCE(dscr & ARM_DSCR_HDBGEN,
-		"halting debug mode enabled. Unable to access hardware resources.\n")) {
-		ret = -EPERM;
-		goto out;
-	}
-
 	/* If monitor mode is already enabled, just return. */
 	if (dscr & ARM_DSCR_MDBGEN)
 		goto out;

@@ -255,6 +248,7 @@ static int enable_monitor_mode(void)
 	case ARM_DEBUG_ARCH_V7_ECP14:
 	case ARM_DEBUG_ARCH_V7_1:
 		ARM_DBG_WRITE(c2, 2, (dscr | ARM_DSCR_MDBGEN));
+		isb();
 		break;
 	default:
 		ret = -ENODEV;

@@ -1000,8 +994,6 @@ static struct notifier_block __cpuinitdata dbg_reset_nb = {
 
 static int __init arch_hw_breakpoint_init(void)
 {
-	u32 dscr;
-
 	debug_arch = get_debug_arch();
 
 	if (!debug_arch_supported()) {

@@ -1036,17 +1028,10 @@ static int __init arch_hw_breakpoint_init(void)
 		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;
-		pr_warning("halting debug mode enabled. Assuming maximum watchpoint size of %u bytes.\n",
-			   max_watchpoint_len);
-	} else {
-		/* Work out the maximum supported watchpoint length. */
-		max_watchpoint_len = get_max_wp_len();
-		pr_info("maximum watchpoint size is %u bytes.\n",
-				max_watchpoint_len);
-	}
+	/* Work out the maximum supported watchpoint length. */
+	max_watchpoint_len = get_max_wp_len();
+	pr_info("maximum watchpoint size is %u bytes.\n",
+			max_watchpoint_len);
 
 	/* Register debug fault handler. */
 	hook_fault_code(FAULT_CODE_DEBUG, hw_breakpoint_pending, SIGTRAP,
Signature · 1 line
1.7.4.1
#5 lore raw

PATCH 2/7 ARM: hw_breakpoint: fix monitor mode detection with v7.1

Quoted 9 lines · Will Deacon

Detecting whether halting debug is enabled is no longer possible via the DBGDSCR in v7.1, returning an UNKNOWN value for the HDBGen bit via CP14 when the OS lock is clear.

This patch removes the halting mode check and ensures that accesses to the internal and external views of the DBGDSCR are serialised with an instruction barrier.

Signed-off-by Will Deacon <[email protected]>
Tested-by Stephen Boyd <[email protected]>
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
author
#6 lore raw

PATCH 3/7 ARM: hw_breakpoint: fix ordering of debug register reset sequence

In reply to Will Deacon

The debug register reset sequence for v7 and v7.1 is congruent with tap-dancing through a minefield.

Rather than wait until we've blown ourselves to pieces, this patch instead checks the debug_err_mask after each potentially faulting operation. We also move the enabling of monitor_mode to the end of the sequence in order to prevent spurious debug events generated by UNKNOWN register values.

Signed-off-by Will Deacon <[email protected]>
---
 arch/arm/kernel/hw_breakpoint.c |   36 ++++++++++++++++++++++++++----------
 1 files changed, 26 insertions(+), 10 deletions(-)
arch/arm/kernel/hw_breakpoint.c +26 -10
index 169dcce..e76cf1a 100644
@@ -231,8 +231,6 @@ static int get_num_brps(void)
 static int enable_monitor_mode(void)
 {
 	u32 dscr;
-	int ret = 0;
-
 	ARM_DBG_READ(c1, 0, dscr);
 
 	/* If monitor mode is already enabled, just return. */

@@ -251,17 +249,18 @@ static int enable_monitor_mode(void)
 		isb();
 		break;
 	default:
-		ret = -ENODEV;
-		goto out;
+		return -ENODEV;
 	}
 
 	/* Check that the write made it through. */
 	ARM_DBG_READ(c1, 0, dscr);
-	if (!(dscr & ARM_DSCR_MDBGEN))
-		ret = -EPERM;
+	if (WARN_ONCE(!(dscr & ARM_DSCR_MDBGEN),
+		"Failed to enable monitor mode on CPU %d.\n",
+		smp_processor_id()))
+		return -EPERM;
 
 out:
-	return ret;
+	return 0;
 }
 
 int hw_breakpoint_slots(int type)

@@ -962,11 +961,16 @@ clear_vcr:
 	asm volatile("mcr p14, 0, %0, c0, c7, 0" : : "r" (0));
 	isb();
 
-reset_regs:
-	if (enable_monitor_mode())
+	if (cpumask_intersects(&debug_err_mask, cpumask_of(cpu))) {
+		pr_warning("CPU %d failed to disable vector catch\n", cpu);
 		return;
+	}
 
-	/* We must also reset any reserved registers. */
+reset_regs:
+	/*
+	 * The control/value register pairs are UNKNOWN out of reset so
+	 * clear them to avoid spurious debug events.
+	 */
 	raw_num_brps = get_num_brp_resources();
 	for (i = 0; i < raw_num_brps; ++i) {
 		write_wb_reg(ARM_BASE_BCR + i, 0UL);

@@ -977,6 +981,18 @@ reset_regs:
 		write_wb_reg(ARM_BASE_WCR + i, 0UL);
 		write_wb_reg(ARM_BASE_WVR + i, 0UL);
 	}
+
+	if (cpumask_intersects(&debug_err_mask, cpumask_of(cpu))) {
+		pr_warning("CPU %d failed to clear debug register pairs\n", cpu);
+		return;
+	}
+
+	/*
+	 * Have a crack@enabling monitor mode. We don't actually need
+	 * it yet, but reporting an error early is useful if it fails.
+	 */
+	if (enable_monitor_mode())
+		cpumask_or(&debug_err_mask, &debug_err_mask, cpumask_of(cpu));
 }
 
 static int __cpuinit dbg_reset_notify(struct notifier_block *self,
Signature · 1 line
1.7.4.1
#7 lore raw

PATCH 3/7 ARM: hw_breakpoint: fix ordering of debug register reset sequence

Quoted 10 lines · Will Deacon

The debug register reset sequence for v7 and v7.1 is congruent with tap-dancing through a minefield.

Rather than wait until we've blown ourselves to pieces, this patch instead checks the debug_err_mask after each potentially faulting operation. We also move the enabling of monitor_mode to the end of the sequence in order to prevent spurious debug events generated by UNKNOWN register values.

Signed-off-by Will Deacon <[email protected]>
Reported-by Stephen Boyd <[email protected]>
Tested-by Stephen Boyd <[email protected]>
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
author
#8 lore raw

PATCH 4/7 ARM: hw_breakpoint: don't try to clear v6 debug registers during boot

In reply to Will Deacon

v6 cores do not provide a way to clear the debug registers without first enabling monitor mode, meaning that we could take spurious debug exceptions. Instead, rely on the registers being in a sane state when we boot as they are defined to be disabled out of reset anyway.

Signed-off-by Will Deacon <[email protected]>
---
 arch/arm/kernel/hw_breakpoint.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
arch/arm/kernel/hw_breakpoint.c +3 -3
index e76cf1a..b6f4aec 100644
@@ -912,8 +912,8 @@ static void reset_ctrl_regs(void *unused)
 	switch (debug_arch) {
 	case ARM_DEBUG_ARCH_V6:
 	case ARM_DEBUG_ARCH_V6_1:
-		/* ARMv6 cores just need to reset the registers. */
-		goto reset_regs;
+		/* ARMv6 cores clear the registers out of reset. */
+		goto out_mdbgen;
 	case ARM_DEBUG_ARCH_V7_ECP14:
 		/*
 		 * Ensure sticky power-down is clear (i.e. debug logic is

@@ -966,7 +966,6 @@ clear_vcr:
 		return;
 	}
 
-reset_regs:
 	/*
 	 * The control/value register pairs are UNKNOWN out of reset so
 	 * clear them to avoid spurious debug events.

@@ -991,6 +990,7 @@ reset_regs:
 	 * Have a crack at enabling monitor mode. We don't actually need
 	 * it yet, but reporting an error early is useful if it fails.
 	 */
+out_mdbgen:
 	if (enable_monitor_mode())
 		cpumask_or(&debug_err_mask, &debug_err_mask, cpumask_of(cpu));
 }
Signature · 1 line
1.7.4.1
#9 lore raw

PATCH 4/7 ARM: hw_breakpoint: don't try to clear v6 debug registers during boot

Quoted 6 lines · Will Deacon

v6 cores do not provide a way to clear the debug registers without first enabling monitor mode, meaning that we could take spurious debug exceptions. Instead, rely on the registers being in a sane state when we boot as they are defined to be disabled out of reset anyway.

Signed-off-by Will Deacon <[email protected]>
Tested-by Stephen Boyd <[email protected]>
Quoted 35 lines
---
 arch/arm/kernel/hw_breakpoint.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
arch/arm/kernel/hw_breakpoint.c +3 -3
index e76cf1a..b6f4aec 100644
@@ -912,8 +912,8 @@ static void reset_ctrl_regs(void *unused)
 	switch (debug_arch) {
 	case ARM_DEBUG_ARCH_V6:
 	case ARM_DEBUG_ARCH_V6_1:
-		/* ARMv6 cores just need to reset the registers. */
-		goto reset_regs;
+		/* ARMv6 cores clear the registers out of reset. */
+		goto out_mdbgen;
 	case ARM_DEBUG_ARCH_V7_ECP14:
 		/*
 		 * Ensure sticky power-down is clear (i.e. debug logic is

@@ -966,7 +966,6 @@ clear_vcr:
 		return;
 	}
 
-reset_regs:
 	/*
 	 * The control/value register pairs are UNKNOWN out of reset so
 	 * clear them to avoid spurious debug events.

@@ -991,6 +990,7 @@ reset_regs:
 	 * Have a crack at enabling monitor mode. We don't actually need
 	 * it yet, but reporting an error early is useful if it fails.
 	 */
+out_mdbgen:
 	if (enable_monitor_mode())
 		cpumask_or(&debug_err_mask, &debug_err_mask, cpumask_of(cpu));
 }
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
author
#10 lore raw

PATCH 5/7 ARM: hw_breakpoint: make boot quieter without CPUID feature registers

In reply to Will Deacon

Booting on a v6 core without the CPUID feature registers (e.g. 1136) leads to a noisy dmesg complaining about their absence.

This patch changes the pr_warning into a WARN_ONCE to keep the log quieter.

Signed-off-by Will Deacon <[email protected]>
---
 arch/arm/kernel/hw_breakpoint.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)
arch/arm/kernel/hw_breakpoint.c +3 -4
index b6f4aec..f267120 100644
@@ -135,11 +135,10 @@ static u8 get_debug_arch(void)
 	u32 didr;
 
 	/* Do we implement the extended CPUID interface? */
-	if (((read_cpuid_id() >> 16) & 0xf) != 0xf) {
-		pr_warning("CPUID feature registers not supported. "
-			   "Assuming v6 debug is present.\n");
+	if (WARN_ONCE(((read_cpuid_id() >> 16) & 0xf) != 0xf,
+		"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;
Signature · 1 line
1.7.4.1
#11 lore raw

PATCH 5/7 ARM: hw_breakpoint: make boot quieter without CPUID feature registers

Quoted 9 lines · Will Deacon

Booting on a v6 core without the CPUID feature registers (e.g. 1136) leads to a noisy dmesg complaining about their absence.

This patch changes the pr_warning into a WARN_ONCE to keep the log quieter.

Signed-off-by Will Deacon <[email protected]>

---

arch/arm/kernel/hw_breakpoint.c static u8 get_debug_arch(void) · line 140

Quoted 9 lines
arch/arm/kernel/hw_breakpoint.c +3 -3
@@ -135,11 +135,10 @@ static u8 get_debug_arch(void)
 	u32 didr;
 
 	/* Do we implement the extended CPUID interface? */
-	if (((read_cpuid_id() >> 16) & 0xf) != 0xf) {
-		pr_warning("CPUID feature registers not supported. "
-			   "Assuming v6 debug is present.\n");
+	if (WARN_ONCE(((read_cpuid_id() >> 16) & 0xf) != 0xf,
+		"CPUID feature registers not supported. "
+		"Assuming v6 debug is present.\n"))

Won't this print a big stack trace instead of a one liner? Perhaps you want pr_warn_once()?

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
author
#12 lore raw

PATCH 6/7 ARM: hw_breakpoint: check if monitor mode is enabled during validation

In reply to Will Deacon

Rather than attempt to enable monitor mode explicitly when scheduling in a breakpoint event (which could raise an undefined exception trap when accessing DBGDSCRext), instead check that DBGDSCRint.MDBGen is set during event validation and report an error to the caller if not.

Signed-off-by Will Deacon <[email protected]>
---
 arch/arm/kernel/hw_breakpoint.c |   28 +++++++++++++++-------------
 1 files changed, 15 insertions(+), 13 deletions(-)
arch/arm/kernel/hw_breakpoint.c +15 -13
index f267120..3127ae4 100644
@@ -227,6 +227,13 @@ static int get_num_brps(void)
  * be put into halting debug mode at any time by an external debugger
  * but there is nothing we can do to prevent that.
  */
+static int monitor_mode_enabled(void)
+{
+	u32 dscr;
+	ARM_DBG_READ(c1, 0, dscr);
+	return !!(dscr & ARM_DSCR_MDBGEN);
+}
+
 static int enable_monitor_mode(void)
 {
 	u32 dscr;

@@ -320,14 +327,9 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
 {
 	struct arch_hw_breakpoint *info = counter_arch_bp(bp);
 	struct perf_event **slot, **slots;
-	int i, max_slots, ctrl_base, val_base, ret = 0;
+	int i, max_slots, ctrl_base, val_base;
 	u32 addr, ctrl;
 
-	/* Ensure that we are in monitor mode and halting mode is disabled. */
-	ret = enable_monitor_mode();
-	if (ret)
-		goto out;
-
 	addr = info->address;
 	ctrl = encode_ctrl_reg(info->ctrl) | 0x1;
 

@@ -354,10 +356,8 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
 		}
 	}
 
-	if (WARN_ONCE(i == max_slots, "Can't find any breakpoint slot\n")) {
-		ret = -EBUSY;
-		goto out;
-	}
+	if (WARN_ONCE(i == max_slots, "Can't find any breakpoint slot\n"))
+		return -EBUSY;
 
 	/* Override the breakpoint data with the step data. */
 	if (info->step_ctrl.enabled) {

@@ -375,9 +375,7 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
 
 	/* Setup the control register. */
 	write_wb_reg(ctrl_base + i, ctrl);
-
-out:
-	return ret;
+	return 0;
 }
 
 void arch_uninstall_hw_breakpoint(struct perf_event *bp)

@@ -588,6 +586,10 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp)
 	int ret = 0;
 	u32 offset, alignment_mask = 0x3;
 
+	/* Ensure that we are in monitor debug mode. */
+	if (!monitor_mode_enabled())
+		return -ENODEV;
+
 	/* Build the arch_hw_breakpoint. */
 	ret = arch_build_bp_info(bp);
 	if (ret)
Signature · 1 line
1.7.4.1
author
#13 lore raw

PATCH 7/7 ARM: hw_breakpoint: use CRn as argument for debug reg accessor macros

In reply to Will Deacon

From: Dietmar Eggemann <[email protected]>

The coprocessor register CRn for accesses to the debug register can be a different one than C0. Take this into account for the ARM_DBG_READ and the ARM_DBG_WRITE macro.

The inline assembler calls which used a coprocessor register CRn other than C0 are replaced by the ARM_DBG_READ or ARM_DBG_WRITE macro.

Signed-off-by Dietmar Eggemann <[email protected]>
Signed-off-by Will Deacon <[email protected]>
---
 arch/arm/include/asm/hw_breakpoint.h |    8 +++---
 arch/arm/kernel/hw_breakpoint.c      |   40 +++++++++++++++++-----------------
 2 files changed, 24 insertions(+), 24 deletions(-)
arch/arm/include/asm/hw_breakpoint.h +4 -4
index c190bc9..01169dd 100644
@@ -98,12 +98,12 @@ static inline void decode_ctrl_reg(u32 reg,
 #define ARM_BASE_WCR		112
 
 /* Accessor macros for the debug registers. */
-#define ARM_DBG_READ(M, OP2, VAL) do {\
-	asm volatile("mrc p14, 0, %0, c0," #M ", " #OP2 : "=r" (VAL));\
+#define ARM_DBG_READ(N, M, OP2, VAL) do {\
+	asm volatile("mrc p14, 0, %0, " #N "," #M ", " #OP2 : "=r" (VAL));\
 } while (0)
 
-#define ARM_DBG_WRITE(M, OP2, VAL) do {\
-	asm volatile("mcr p14, 0, %0, c0," #M ", " #OP2 : : "r" (VAL));\
+#define ARM_DBG_WRITE(N, M, OP2, VAL) do {\
+	asm volatile("mcr p14, 0, %0, " #N "," #M ", " #OP2 : : "r" (VAL));\
 } while (0)
 
 struct notifier_block;
arch/arm/kernel/hw_breakpoint.c +20 -20
index 3127ae4..05fdd68 100644
@@ -52,14 +52,14 @@ static u8 debug_arch;
 /* Maximum supported watchpoint length. */
 static u8 max_watchpoint_len;
 
-#define READ_WB_REG_CASE(OP2, M, VAL)		\
-	case ((OP2 << 4) + M):			\
-		ARM_DBG_READ(c ## M, OP2, VAL); \
+#define READ_WB_REG_CASE(OP2, M, VAL)			\
+	case ((OP2 << 4) + M):				\
+		ARM_DBG_READ(c0, c ## M, OP2, VAL);	\
 		break
 
-#define WRITE_WB_REG_CASE(OP2, M, VAL)		\
-	case ((OP2 << 4) + M):			\
-		ARM_DBG_WRITE(c ## M, OP2, VAL);\
+#define WRITE_WB_REG_CASE(OP2, M, VAL)			\
+	case ((OP2 << 4) + M):				\
+		ARM_DBG_WRITE(c0, c ## M, OP2, VAL);	\
 		break
 
 #define GEN_READ_WB_REG_CASES(OP2, VAL)		\

@@ -140,7 +140,7 @@ static u8 get_debug_arch(void)
 		"Assuming v6 debug is present.\n"))
 		return ARM_DEBUG_ARCH_V6;
 
-	ARM_DBG_READ(c0, 0, didr);
+	ARM_DBG_READ(c0, c0, 0, didr);
 	return (didr >> 16) & 0xf;
 }
 

@@ -168,7 +168,7 @@ static int debug_exception_updates_fsr(void)
 static int get_num_wrp_resources(void)
 {
 	u32 didr;
-	ARM_DBG_READ(c0, 0, didr);
+	ARM_DBG_READ(c0, c0, 0, didr);
 	return ((didr >> 28) & 0xf) + 1;
 }
 

@@ -176,7 +176,7 @@ static int get_num_wrp_resources(void)
 static int get_num_brp_resources(void)
 {
 	u32 didr;
-	ARM_DBG_READ(c0, 0, didr);
+	ARM_DBG_READ(c0, c0, 0, didr);
 	return ((didr >> 24) & 0xf) + 1;
 }
 

@@ -230,14 +230,14 @@ static int get_num_brps(void)
 static int monitor_mode_enabled(void)
 {
 	u32 dscr;
-	ARM_DBG_READ(c1, 0, dscr);
+	ARM_DBG_READ(c0, c1, 0, dscr);
 	return !!(dscr & ARM_DSCR_MDBGEN);
 }
 
 static int enable_monitor_mode(void)
 {
 	u32 dscr;
-	ARM_DBG_READ(c1, 0, dscr);
+	ARM_DBG_READ(c0, c1, 0, dscr);
 
 	/* If monitor mode is already enabled, just return. */
 	if (dscr & ARM_DSCR_MDBGEN)

@@ -247,11 +247,11 @@ static int enable_monitor_mode(void)
 	switch (get_debug_arch()) {
 	case ARM_DEBUG_ARCH_V6:
 	case ARM_DEBUG_ARCH_V6_1:
-		ARM_DBG_WRITE(c1, 0, (dscr | ARM_DSCR_MDBGEN));
+		ARM_DBG_WRITE(c0, 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));
+		ARM_DBG_WRITE(c0, c2, 2, (dscr | ARM_DSCR_MDBGEN));
 		isb();
 		break;
 	default:

@@ -259,7 +259,7 @@ static int enable_monitor_mode(void)
 	}
 
 	/* Check that the write made it through. */
-	ARM_DBG_READ(c1, 0, dscr);
+	ARM_DBG_READ(c0, c1, 0, dscr);
 	if (WARN_ONCE(!(dscr & ARM_DSCR_MDBGEN),
 		"Failed to enable monitor mode on CPU %d.\n",
 		smp_processor_id()))

@@ -852,7 +852,7 @@ static int hw_breakpoint_pending(unsigned long addr, unsigned int fsr,
 		local_irq_enable();
 
 	/* We only handle watchpoints and hardware breakpoints. */
-	ARM_DBG_READ(c1, 0, dscr);
+	ARM_DBG_READ(c0, c1, 0, dscr);
 
 	/* Perform perf callbacks. */
 	switch (ARM_DSCR_MOE(dscr)) {

@@ -920,14 +920,14 @@ static void reset_ctrl_regs(void *unused)
 		 * 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));
+		ARM_DBG_READ(c1, c5, 4, dbg_power);
 		if ((dbg_power & 0x1) == 0)
 			err = -EPERM;
 
 		/*
 		 * Check whether we implement OS save and restore.
 		 */
-		asm volatile("mrc p14, 0, %0, c1, c1, 4" : "=r" (dbg_power));
+		ARM_DBG_READ(c1, c1, 4, dbg_power);
 		if ((dbg_power & 0x9) == 0)
 			goto clear_vcr;
 		break;

@@ -935,7 +935,7 @@ static void reset_ctrl_regs(void *unused)
 		/*
 		 * Ensure the OS double lock is clear.
 		 */
-		asm volatile("mrc p14, 0, %0, c1, c3, 4" : "=r" (dbg_power));
+		ARM_DBG_READ(c1, c3, 4, dbg_power);
 		if ((dbg_power & 0x1) == 1)
 			err = -EPERM;
 		break;

@@ -951,7 +951,7 @@ static void reset_ctrl_regs(void *unused)
 	 * Unconditionally clear the OS lock by writing a value
 	 * other than 0xC5ACCE55 to the access register.
 	 */
-	asm volatile("mcr p14, 0, %0, c1, c0, 4" : : "r" (0));
+	ARM_DBG_WRITE(c1, c0, 4, 0);
 	isb();
 
 	/*

@@ -959,7 +959,7 @@ static void reset_ctrl_regs(void *unused)
 	 * enabling monitor mode.
 	 */
 clear_vcr:
-	asm volatile("mcr p14, 0, %0, c0, c7, 0" : : "r" (0));
+	ARM_DBG_WRITE(c0, c7, 0, 0);
 	isb();
 
 	if (cpumask_intersects(&debug_err_mask, cpumask_of(cpu))) {
Signature · 1 line
1.7.4.1
#14 lore raw

PATCH 7/7 ARM: hw_breakpoint: use CRn as argument for debug reg accessor macros

Quoted 6 lines · Will Deacon

From: Dietmar Eggemann <[email protected]>

The coprocessor register CRn for accesses to the debug register can be a different one than C0. Take this into account for the ARM_DBG_READ and the ARM_DBG_WRITE macro.

Quoted 6 lines

The inline assembler calls which used a coprocessor register CRn other than C0 are replaced by the ARM_DBG_READ or ARM_DBG_WRITE macro.

Signed-off-by Dietmar Eggemann <[email protected]>
Signed-off-by Will Deacon <[email protected]>
Tested-by Stephen Boyd <[email protected]>
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation