[PATCH 0/4] MIPS: Watchpoint fixes and cleanups

[PATCH 0/4] MIPS: Watchpoint fixes and cleanups

[James Hogan]

Patch 1 fixes watchpoint breakage since v4.3.

Patch 2 enables watchpoints in R6 kernels, where they are inadvertently
disabled.

Patches 3 and 4 are cosmetic cleanups, to replace magic numbers with
preprocessor definitions.

James Hogan (4):
  MIPS: Fix watchpoint restoration
  MIPS: Enable ptrace hw watchpoints on MIPS R6
  MIPS: Add and use CAUSEF_WP definition
  MIPS: Add and use watch register field definitions

 arch/mips/Kconfig                 |  2 +-
 arch/mips/include/asm/mipsregs.h  | 20 ++++++++++
 arch/mips/include/asm/switch_to.h |  2 +-
 arch/mips/include/asm/watch.h     | 10 ++---
 arch/mips/kernel/pm.c             |  2 +-
 arch/mips/kernel/ptrace.c         |  7 ++--
 arch/mips/kernel/traps.c          |  5 +--
 arch/mips/kernel/watch.c          | 79 +++++++++++++++++++++------------------
 8 files changed, 76 insertions(+), 51 deletions(-)

Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: linux-mips@linux-mips.org
-- 
2.4.10

[PATCH 0/4] MIPS: Watchpoint fixes and cleanups

[James Hogan]

Patch 1 fixes watchpoint breakage since v4.3.

Patch 2 enables watchpoints in R6 kernels, where they are inadvertently
disabled.

Patches 3 and 4 are cosmetic cleanups, to replace magic numbers with
preprocessor definitions.

James Hogan (4):
  MIPS: Fix watchpoint restoration
  MIPS: Enable ptrace hw watchpoints on MIPS R6
  MIPS: Add and use CAUSEF_WP definition
  MIPS: Add and use watch register field definitions

 arch/mips/Kconfig                 |  2 +-
 arch/mips/include/asm/mipsregs.h  | 20 ++++++++++
 arch/mips/include/asm/switch_to.h |  2 +-
 arch/mips/include/asm/watch.h     | 10 ++---
 arch/mips/kernel/pm.c             |  2 +-
 arch/mips/kernel/ptrace.c         |  7 ++--
 arch/mips/kernel/traps.c          |  5 +--
 arch/mips/kernel/watch.c          | 79 +++++++++++++++++++++------------------
 8 files changed, 76 insertions(+), 51 deletions(-)

Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: linux-mips@linux-mips.org
-- 
2.4.10

[PATCH 1/4] MIPS: Fix watchpoint restoration

[James Hogan]

Commit f51246efee2b ("MIPS: Get rid of finish_arch_switch().") moved the
__restore_watch() call from finish_arch_switch() (i.e. after resume()
returns) to before the resume() call in switch_to(). This results in
watchpoints only being restored when a task is descheduled, preventing
the watchpoints from being effective most of the time, except due to
chance before the watchpoints are lazily removed.

Fix the call sequence from switch_to() through to
mips_install_watch_registers() to pass the task_struct pointer of the
next task, instead of using current. This allows the watchpoints for the
next (non-current) task to be restored without reintroducing
finish_arch_switch().

Fixes: f51246efee2b ("MIPS: Get rid of finish_arch_switch().")
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: linux-mips@linux-mips.org
Cc: <stable@vger.kernel.org> # 4.3.x-
---
 arch/mips/include/asm/switch_to.h |  2 +-
 arch/mips/include/asm/watch.h     | 10 +++++-----
 arch/mips/kernel/pm.c             |  2 +-
 arch/mips/kernel/watch.c          |  5 ++---
 4 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/mips/include/asm/switch_to.h b/arch/mips/include/asm/switch_to.h
index 28b5d84a5022..ebb5c0f2f90d 100644
--- a/arch/mips/include/asm/switch_to.h
+++ b/arch/mips/include/asm/switch_to.h
@@ -105,7 +105,7 @@ do {									\
 	__clear_software_ll_bit();					\
 	if (cpu_has_userlocal)						\
 		write_c0_userlocal(task_thread_info(next)->tp_value);	\
-	__restore_watch();						\
+	__restore_watch(next);						\
 	(last) = resume(prev, next, task_thread_info(next));		\
 } while (0)
 
diff --git a/arch/mips/include/asm/watch.h b/arch/mips/include/asm/watch.h
index 20126ec79359..6ffe3eadf105 100644
--- a/arch/mips/include/asm/watch.h
+++ b/arch/mips/include/asm/watch.h
@@ -12,21 +12,21 @@
 
 #include <asm/mipsregs.h>
 
-void mips_install_watch_registers(void);
+void mips_install_watch_registers(struct task_struct *t);
 void mips_read_watch_registers(void);
 void mips_clear_watch_registers(void);
 void mips_probe_watch_registers(struct cpuinfo_mips *c);
 
 #ifdef CONFIG_HARDWARE_WATCHPOINTS
-#define __restore_watch() do {						\
+#define __restore_watch(task) do {					\
 	if (unlikely(test_bit(TIF_LOAD_WATCH,				\
-			      &current_thread_info()->flags))) {	\
-		mips_install_watch_registers();				\
+			      &task_thread_info(task)->flags))) {	\
+		mips_install_watch_registers(task);			\
 	}								\
 } while (0)
 
 #else
-#define __restore_watch() do {} while (0)
+#define __restore_watch(task) do {} while (0)
 #endif
 
 #endif /* _ASM_WATCH_H */
diff --git a/arch/mips/kernel/pm.c b/arch/mips/kernel/pm.c
index fefdf39d3df3..dc814892133c 100644
--- a/arch/mips/kernel/pm.c
+++ b/arch/mips/kernel/pm.c
@@ -56,7 +56,7 @@ static void mips_cpu_restore(void)
 		write_c0_userlocal(current_thread_info()->tp_value);
 
 	/* Restore watch registers */
-	__restore_watch();
+	__restore_watch(current);
 }
 
 /**
diff --git a/arch/mips/kernel/watch.c b/arch/mips/kernel/watch.c
index 2a03abb5bd2c..9b78e375118e 100644
--- a/arch/mips/kernel/watch.c
+++ b/arch/mips/kernel/watch.c
@@ -15,10 +15,9 @@
  * Install the watch registers for the current thread.	A maximum of
  * four registers are installed although the machine may have more.
  */
-void mips_install_watch_registers(void)
+void mips_install_watch_registers(struct task_struct *t)
 {
-	struct mips3264_watch_reg_state *watches =
-		&current->thread.watch.mips3264;
+	struct mips3264_watch_reg_state *watches = &t->thread.watch.mips3264;
 	switch (current_cpu_data.watch_reg_use_cnt) {
 	default:
 		BUG();
-- 
2.4.10

[PATCH 1/4] MIPS: Fix watchpoint restoration

[James Hogan]

Commit f51246efee2b ("MIPS: Get rid of finish_arch_switch().") moved the
__restore_watch() call from finish_arch_switch() (i.e. after resume()
returns) to before the resume() call in switch_to(). This results in
watchpoints only being restored when a task is descheduled, preventing
the watchpoints from being effective most of the time, except due to
chance before the watchpoints are lazily removed.

Fix the call sequence from switch_to() through to
mips_install_watch_registers() to pass the task_struct pointer of the
next task, instead of using current. This allows the watchpoints for the
next (non-current) task to be restored without reintroducing
finish_arch_switch().

Fixes: f51246efee2b ("MIPS: Get rid of finish_arch_switch().")
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: linux-mips@linux-mips.org
Cc: <stable@vger.kernel.org> # 4.3.x-
---
 arch/mips/include/asm/switch_to.h |  2 +-
 arch/mips/include/asm/watch.h     | 10 +++++-----
 arch/mips/kernel/pm.c             |  2 +-
 arch/mips/kernel/watch.c          |  5 ++---
 4 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/mips/include/asm/switch_to.h b/arch/mips/include/asm/switch_to.h
index 28b5d84a5022..ebb5c0f2f90d 100644
--- a/arch/mips/include/asm/switch_to.h
+++ b/arch/mips/include/asm/switch_to.h
@@ -105,7 +105,7 @@ do {									\
 	__clear_software_ll_bit();					\
 	if (cpu_has_userlocal)						\
 		write_c0_userlocal(task_thread_info(next)->tp_value);	\
-	__restore_watch();						\
+	__restore_watch(next);						\
 	(last) = resume(prev, next, task_thread_info(next));		\
 } while (0)
 
diff --git a/arch/mips/include/asm/watch.h b/arch/mips/include/asm/watch.h
index 20126ec79359..6ffe3eadf105 100644
--- a/arch/mips/include/asm/watch.h
+++ b/arch/mips/include/asm/watch.h
@@ -12,21 +12,21 @@
 
 #include <asm/mipsregs.h>
 
-void mips_install_watch_registers(void);
+void mips_install_watch_registers(struct task_struct *t);
 void mips_read_watch_registers(void);
 void mips_clear_watch_registers(void);
 void mips_probe_watch_registers(struct cpuinfo_mips *c);
 
 #ifdef CONFIG_HARDWARE_WATCHPOINTS
-#define __restore_watch() do {						\
+#define __restore_watch(task) do {					\
 	if (unlikely(test_bit(TIF_LOAD_WATCH,				\
-			      &current_thread_info()->flags))) {	\
-		mips_install_watch_registers();				\
+			      &task_thread_info(task)->flags))) {	\
+		mips_install_watch_registers(task);			\
 	}								\
 } while (0)
 
 #else
-#define __restore_watch() do {} while (0)
+#define __restore_watch(task) do {} while (0)
 #endif
 
 #endif /* _ASM_WATCH_H */
diff --git a/arch/mips/kernel/pm.c b/arch/mips/kernel/pm.c
index fefdf39d3df3..dc814892133c 100644
--- a/arch/mips/kernel/pm.c
+++ b/arch/mips/kernel/pm.c
@@ -56,7 +56,7 @@ static void mips_cpu_restore(void)
 		write_c0_userlocal(current_thread_info()->tp_value);
 
 	/* Restore watch registers */
-	__restore_watch();
+	__restore_watch(current);
 }
 
 /**
diff --git a/arch/mips/kernel/watch.c b/arch/mips/kernel/watch.c
index 2a03abb5bd2c..9b78e375118e 100644
--- a/arch/mips/kernel/watch.c
+++ b/arch/mips/kernel/watch.c
@@ -15,10 +15,9 @@
  * Install the watch registers for the current thread.	A maximum of
  * four registers are installed although the machine may have more.
  */
-void mips_install_watch_registers(void)
+void mips_install_watch_registers(struct task_struct *t)
 {
-	struct mips3264_watch_reg_state *watches =
-		&current->thread.watch.mips3264;
+	struct mips3264_watch_reg_state *watches = &t->thread.watch.mips3264;
 	switch (current_cpu_data.watch_reg_use_cnt) {
 	default:
 		BUG();
-- 
2.4.10

[PATCH 2/4] MIPS: Enable ptrace hw watchpoints on MIPS R6

[James Hogan]

HARDWARE_WATCHPOINTS isn't being enabled for CPU_MIPSR6, even though it
has an identical hardware watchpoint interface to CPU_MIPSR2, which
prevents ptrace watchpoints from being loaded when executing a ptraced
process even though the watchpoints are described in /proc/cpuinfo.

Enable HARDWARE_WATCHPOINTS for CPU_MIPSR6 too.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 74a3db92da1b..e9053ab1a847 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2000,7 +2000,7 @@ config MIPS_PGD_C0_CONTEXT
 #
 config HARDWARE_WATCHPOINTS
        bool
-       default y if CPU_MIPSR1 || CPU_MIPSR2
+       default y if CPU_MIPSR1 || CPU_MIPSR2 || CPU_MIPSR6
 
 menu "Kernel type"
 
-- 
2.4.10

[PATCH 2/4] MIPS: Enable ptrace hw watchpoints on MIPS R6

[James Hogan]

HARDWARE_WATCHPOINTS isn't being enabled for CPU_MIPSR6, even though it
has an identical hardware watchpoint interface to CPU_MIPSR2, which
prevents ptrace watchpoints from being loaded when executing a ptraced
process even though the watchpoints are described in /proc/cpuinfo.

Enable HARDWARE_WATCHPOINTS for CPU_MIPSR6 too.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 74a3db92da1b..e9053ab1a847 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2000,7 +2000,7 @@ config MIPS_PGD_C0_CONTEXT
 #
 config HARDWARE_WATCHPOINTS
        bool
-       default y if CPU_MIPSR1 || CPU_MIPSR2
+       default y if CPU_MIPSR1 || CPU_MIPSR2 || CPU_MIPSR6
 
 menu "Kernel type"
 
-- 
2.4.10

[PATCH 3/4] MIPS: Add and use CAUSEF_WP definition

[James Hogan]

do_watch() clears bit 22 of cause without using a CAUSEF_* definition
from mipsregs.h. Add a definition for this bit (CAUSEF_WP) and make use
of it. Also use clear_c0_cause() instead of manual read/modify/write.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/include/asm/mipsregs.h | 2 ++
 arch/mips/kernel/traps.c         | 5 +----
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index 3ad19ad04d8a..f7e3215ca2f6 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -390,6 +390,8 @@
 #define	 CAUSEF_IP7		(_ULCAST_(1)   << 15)
 #define CAUSEB_FDCI		21
 #define CAUSEF_FDCI		(_ULCAST_(1)   << 21)
+#define CAUSEB_WP		22
+#define CAUSEF_WP		(_ULCAST_(1)   << 22)
 #define CAUSEB_IV		23
 #define CAUSEF_IV		(_ULCAST_(1)   << 23)
 #define CAUSEB_PCI		26
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index ae790c575d4f..6ee2443c6fd5 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -1494,16 +1494,13 @@ asmlinkage void do_mdmx(struct pt_regs *regs)
 asmlinkage void do_watch(struct pt_regs *regs)
 {
 	enum ctx_state prev_state;
-	u32 cause;
 
 	prev_state = exception_enter();
 	/*
 	 * Clear WP (bit 22) bit of cause register so we don't loop
 	 * forever.
 	 */
-	cause = read_c0_cause();
-	cause &= ~(1 << 22);
-	write_c0_cause(cause);
+	clear_c0_cause(CAUSEF_WP);
 
 	/*
 	 * If the current thread has the watch registers loaded, save
-- 
2.4.10

[PATCH 3/4] MIPS: Add and use CAUSEF_WP definition

[James Hogan]

do_watch() clears bit 22 of cause without using a CAUSEF_* definition
from mipsregs.h. Add a definition for this bit (CAUSEF_WP) and make use
of it. Also use clear_c0_cause() instead of manual read/modify/write.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/include/asm/mipsregs.h | 2 ++
 arch/mips/kernel/traps.c         | 5 +----
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index 3ad19ad04d8a..f7e3215ca2f6 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -390,6 +390,8 @@
 #define	 CAUSEF_IP7		(_ULCAST_(1)   << 15)
 #define CAUSEB_FDCI		21
 #define CAUSEF_FDCI		(_ULCAST_(1)   << 21)
+#define CAUSEB_WP		22
+#define CAUSEF_WP		(_ULCAST_(1)   << 22)
 #define CAUSEB_IV		23
 #define CAUSEF_IV		(_ULCAST_(1)   << 23)
 #define CAUSEB_PCI		26
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index ae790c575d4f..6ee2443c6fd5 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -1494,16 +1494,13 @@ asmlinkage void do_mdmx(struct pt_regs *regs)
 asmlinkage void do_watch(struct pt_regs *regs)
 {
 	enum ctx_state prev_state;
-	u32 cause;
 
 	prev_state = exception_enter();
 	/*
 	 * Clear WP (bit 22) bit of cause register so we don't loop
 	 * forever.
 	 */
-	cause = read_c0_cause();
-	cause &= ~(1 << 22);
-	write_c0_cause(cause);
+	clear_c0_cause(CAUSEF_WP);
 
 	/*
 	 * If the current thread has the watch registers loaded, save
-- 
2.4.10

[PATCH 4/4] MIPS: Add and use watch register field definitions

[James Hogan]

The files watch.c and ptrace.c contain various magic masks for
WatchLo/WatchHi register fields. Add some definitions to mipsregs.h for
these registers and make use of them in both watch.c and ptrace.c,
hopefully making them more readable.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/include/asm/mipsregs.h | 18 ++++++++++
 arch/mips/kernel/ptrace.c        |  7 ++--
 arch/mips/kernel/watch.c         | 74 ++++++++++++++++++++++------------------
 3 files changed, 63 insertions(+), 36 deletions(-)

diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index f7e3215ca2f6..b04f80271f46 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -647,6 +647,24 @@
 /* FTLB probability bits for R6 */
 #define MIPS_CONF7_FTLBP_SHIFT	(18)
 
+/* WatchLo* register definitions */
+#define MIPS_WATCHLO_IRW	(_ULCAST_(0x7) << 0)
+
+/* WatchHi* register definitions */
+#define MIPS_WATCHHI_M		(_ULCAST_(1) << 31)
+#define MIPS_WATCHHI_G		(_ULCAST_(1) << 30)
+#define MIPS_WATCHHI_WM		(_ULCAST_(0x3) << 28)
+#define MIPS_WATCHHI_WM_R_RVA	(_ULCAST_(0) << 28)
+#define MIPS_WATCHHI_WM_R_GPA	(_ULCAST_(1) << 28)
+#define MIPS_WATCHHI_WM_G_GVA	(_ULCAST_(2) << 28)
+#define MIPS_WATCHHI_EAS	(_ULCAST_(0x3) << 24)
+#define MIPS_WATCHHI_ASID	(_ULCAST_(0xff) << 16)
+#define MIPS_WATCHHI_MASK	(_ULCAST_(0x1ff) << 3)
+#define MIPS_WATCHHI_I		(_ULCAST_(1) << 2)
+#define MIPS_WATCHHI_R		(_ULCAST_(1) << 1)
+#define MIPS_WATCHHI_W		(_ULCAST_(1) << 0)
+#define MIPS_WATCHHI_IRW	(_ULCAST_(0x7) << 0)
+
 /* MAAR bit definitions */
 #define MIPS_MAAR_ADDR		((BIT_ULL(BITS_PER_LONG - 12) - 1) << 12)
 #define MIPS_MAAR_ADDR_SHIFT	12
diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
index a5279b2f3198..48c0534c4d15 100644
--- a/arch/mips/kernel/ptrace.c
+++ b/arch/mips/kernel/ptrace.c
@@ -210,7 +210,8 @@ int ptrace_get_watch_regs(struct task_struct *child,
 	for (i = 0; i < boot_cpu_data.watch_reg_use_cnt; i++) {
 		__put_user(child->thread.watch.mips3264.watchlo[i],
 			   &addr->WATCH_STYLE.watchlo[i]);
-		__put_user(child->thread.watch.mips3264.watchhi[i] & 0xfff,
+		__put_user(child->thread.watch.mips3264.watchhi[i] &
+				(MIPS_WATCHHI_MASK | MIPS_WATCHHI_IRW),
 			   &addr->WATCH_STYLE.watchhi[i]);
 		__put_user(boot_cpu_data.watch_reg_masks[i],
 			   &addr->WATCH_STYLE.watch_masks[i]);
@@ -252,12 +253,12 @@ int ptrace_set_watch_regs(struct task_struct *child,
 		}
 #endif
 		__get_user(ht[i], &addr->WATCH_STYLE.watchhi[i]);
-		if (ht[i] & ~0xff8)
+		if (ht[i] & ~MIPS_WATCHHI_MASK)
 			return -EINVAL;
 	}
 	/* Install them. */
 	for (i = 0; i < boot_cpu_data.watch_reg_use_cnt; i++) {
-		if (lt[i] & 7)
+		if (lt[i] & MIPS_WATCHLO_IRW)
 			watch_active = 1;
 		child->thread.watch.mips3264.watchlo[i] = lt[i];
 		/* Set the G bit. */
diff --git a/arch/mips/kernel/watch.c b/arch/mips/kernel/watch.c
index 9b78e375118e..19fcab7348b1 100644
--- a/arch/mips/kernel/watch.c
+++ b/arch/mips/kernel/watch.c
@@ -25,16 +25,20 @@ void mips_install_watch_registers(struct task_struct *t)
 		write_c0_watchlo3(watches->watchlo[3]);
 		/* Write 1 to the I, R, and W bits to clear them, and
 		   1 to G so all ASIDs are trapped. */
-		write_c0_watchhi3(0x40000007 | watches->watchhi[3]);
+		write_c0_watchhi3(MIPS_WATCHHI_G | MIPS_WATCHHI_IRW |
+				  watches->watchhi[3]);
 	case 3:
 		write_c0_watchlo2(watches->watchlo[2]);
-		write_c0_watchhi2(0x40000007 | watches->watchhi[2]);
+		write_c0_watchhi2(MIPS_WATCHHI_G | MIPS_WATCHHI_IRW |
+				  watches->watchhi[2]);
 	case 2:
 		write_c0_watchlo1(watches->watchlo[1]);
-		write_c0_watchhi1(0x40000007 | watches->watchhi[1]);
+		write_c0_watchhi1(MIPS_WATCHHI_G | MIPS_WATCHHI_IRW |
+				  watches->watchhi[1]);
 	case 1:
 		write_c0_watchlo0(watches->watchlo[0]);
-		write_c0_watchhi0(0x40000007 | watches->watchhi[0]);
+		write_c0_watchhi0(MIPS_WATCHHI_G | MIPS_WATCHHI_IRW |
+				  watches->watchhi[0]);
 	}
 }
 
@@ -51,22 +55,26 @@ void mips_read_watch_registers(void)
 	default:
 		BUG();
 	case 4:
-		watches->watchhi[3] = (read_c0_watchhi3() & 0x0fff);
+		watches->watchhi[3] = (read_c0_watchhi3() &
+				       (MIPS_WATCHHI_MASK | MIPS_WATCHHI_IRW));
 	case 3:
-		watches->watchhi[2] = (read_c0_watchhi2() & 0x0fff);
+		watches->watchhi[2] = (read_c0_watchhi2() &
+				       (MIPS_WATCHHI_MASK | MIPS_WATCHHI_IRW));
 	case 2:
-		watches->watchhi[1] = (read_c0_watchhi1() & 0x0fff);
+		watches->watchhi[1] = (read_c0_watchhi1() &
+				       (MIPS_WATCHHI_MASK | MIPS_WATCHHI_IRW));
 	case 1:
-		watches->watchhi[0] = (read_c0_watchhi0() & 0x0fff);
+		watches->watchhi[0] = (read_c0_watchhi0() &
+				       (MIPS_WATCHHI_MASK | MIPS_WATCHHI_IRW));
 	}
 	if (current_cpu_data.watch_reg_use_cnt == 1 &&
-	    (watches->watchhi[0] & 7) == 0) {
+	    (watches->watchhi[0] & MIPS_WATCHHI_IRW) == 0) {
 		/* Pathological case of release 1 architecture that
 		 * doesn't set the condition bits.  We assume that
 		 * since we got here, the watch condition was met and
 		 * signal that the conditions requested in watchlo
 		 * were met.  */
-		watches->watchhi[0] |= (watches->watchlo[0] & 7);
+		watches->watchhi[0] |= (watches->watchlo[0] & MIPS_WATCHHI_IRW);
 	}
  }
 
@@ -109,86 +117,86 @@ void mips_probe_watch_registers(struct cpuinfo_mips *c)
 	 * Check which of the I,R and W bits are supported, then
 	 * disable the register.
 	 */
-	write_c0_watchlo0(7);
+	write_c0_watchlo0(MIPS_WATCHLO_IRW);
 	back_to_back_c0_hazard();
 	t = read_c0_watchlo0();
 	write_c0_watchlo0(0);
-	c->watch_reg_masks[0] = t & 7;
+	c->watch_reg_masks[0] = t & MIPS_WATCHLO_IRW;
 
 	/* Write the mask bits and read them back to determine which
 	 * can be used. */
 	c->watch_reg_count = 1;
 	c->watch_reg_use_cnt = 1;
 	t = read_c0_watchhi0();
-	write_c0_watchhi0(t | 0xff8);
+	write_c0_watchhi0(t | MIPS_WATCHHI_MASK);
 	back_to_back_c0_hazard();
 	t = read_c0_watchhi0();
-	c->watch_reg_masks[0] |= (t & 0xff8);
-	if ((t & 0x80000000) == 0)
+	c->watch_reg_masks[0] |= (t & MIPS_WATCHHI_MASK);
+	if ((t & MIPS_WATCHHI_M) == 0)
 		return;
 
-	write_c0_watchlo1(7);
+	write_c0_watchlo1(MIPS_WATCHLO_IRW);
 	back_to_back_c0_hazard();
 	t = read_c0_watchlo1();
 	write_c0_watchlo1(0);
-	c->watch_reg_masks[1] = t & 7;
+	c->watch_reg_masks[1] = t & MIPS_WATCHLO_IRW;
 
 	c->watch_reg_count = 2;
 	c->watch_reg_use_cnt = 2;
 	t = read_c0_watchhi1();
-	write_c0_watchhi1(t | 0xff8);
+	write_c0_watchhi1(t | MIPS_WATCHHI_MASK);
 	back_to_back_c0_hazard();
 	t = read_c0_watchhi1();
-	c->watch_reg_masks[1] |= (t & 0xff8);
-	if ((t & 0x80000000) == 0)
+	c->watch_reg_masks[1] |= (t & MIPS_WATCHHI_MASK);
+	if ((t & MIPS_WATCHHI_M) == 0)
 		return;
 
-	write_c0_watchlo2(7);
+	write_c0_watchlo2(MIPS_WATCHLO_IRW);
 	back_to_back_c0_hazard();
 	t = read_c0_watchlo2();
 	write_c0_watchlo2(0);
-	c->watch_reg_masks[2] = t & 7;
+	c->watch_reg_masks[2] = t & MIPS_WATCHLO_IRW;
 
 	c->watch_reg_count = 3;
 	c->watch_reg_use_cnt = 3;
 	t = read_c0_watchhi2();
-	write_c0_watchhi2(t | 0xff8);
+	write_c0_watchhi2(t | MIPS_WATCHHI_MASK);
 	back_to_back_c0_hazard();
 	t = read_c0_watchhi2();
-	c->watch_reg_masks[2] |= (t & 0xff8);
-	if ((t & 0x80000000) == 0)
+	c->watch_reg_masks[2] |= (t & MIPS_WATCHHI_MASK);
+	if ((t & MIPS_WATCHHI_M) == 0)
 		return;
 
-	write_c0_watchlo3(7);
+	write_c0_watchlo3(MIPS_WATCHLO_IRW);
 	back_to_back_c0_hazard();
 	t = read_c0_watchlo3();
 	write_c0_watchlo3(0);
-	c->watch_reg_masks[3] = t & 7;
+	c->watch_reg_masks[3] = t & MIPS_WATCHLO_IRW;
 
 	c->watch_reg_count = 4;
 	c->watch_reg_use_cnt = 4;
 	t = read_c0_watchhi3();
-	write_c0_watchhi3(t | 0xff8);
+	write_c0_watchhi3(t | MIPS_WATCHHI_MASK);
 	back_to_back_c0_hazard();
 	t = read_c0_watchhi3();
-	c->watch_reg_masks[3] |= (t & 0xff8);
-	if ((t & 0x80000000) == 0)
+	c->watch_reg_masks[3] |= (t & MIPS_WATCHHI_MASK);
+	if ((t & MIPS_WATCHHI_M) == 0)
 		return;
 
 	/* We use at most 4, but probe and report up to 8. */
 	c->watch_reg_count = 5;
 	t = read_c0_watchhi4();
-	if ((t & 0x80000000) == 0)
+	if ((t & MIPS_WATCHHI_M) == 0)
 		return;
 
 	c->watch_reg_count = 6;
 	t = read_c0_watchhi5();
-	if ((t & 0x80000000) == 0)
+	if ((t & MIPS_WATCHHI_M) == 0)
 		return;
 
 	c->watch_reg_count = 7;
 	t = read_c0_watchhi6();
-	if ((t & 0x80000000) == 0)
+	if ((t & MIPS_WATCHHI_M) == 0)
 		return;
 
 	c->watch_reg_count = 8;
-- 
2.4.10

[PATCH 4/4] MIPS: Add and use watch register field definitions

[James Hogan]

The files watch.c and ptrace.c contain various magic masks for
WatchLo/WatchHi register fields. Add some definitions to mipsregs.h for
these registers and make use of them in both watch.c and ptrace.c,
hopefully making them more readable.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/include/asm/mipsregs.h | 18 ++++++++++
 arch/mips/kernel/ptrace.c        |  7 ++--
 arch/mips/kernel/watch.c         | 74 ++++++++++++++++++++++------------------
 3 files changed, 63 insertions(+), 36 deletions(-)

diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index f7e3215ca2f6..b04f80271f46 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -647,6 +647,24 @@
 /* FTLB probability bits for R6 */
 #define MIPS_CONF7_FTLBP_SHIFT	(18)
 
+/* WatchLo* register definitions */
+#define MIPS_WATCHLO_IRW	(_ULCAST_(0x7) << 0)
+
+/* WatchHi* register definitions */
+#define MIPS_WATCHHI_M		(_ULCAST_(1) << 31)
+#define MIPS_WATCHHI_G		(_ULCAST_(1) << 30)
+#define MIPS_WATCHHI_WM		(_ULCAST_(0x3) << 28)
+#define MIPS_WATCHHI_WM_R_RVA	(_ULCAST_(0) << 28)
+#define MIPS_WATCHHI_WM_R_GPA	(_ULCAST_(1) << 28)
+#define MIPS_WATCHHI_WM_G_GVA	(_ULCAST_(2) << 28)
+#define MIPS_WATCHHI_EAS	(_ULCAST_(0x3) << 24)
+#define MIPS_WATCHHI_ASID	(_ULCAST_(0xff) << 16)
+#define MIPS_WATCHHI_MASK	(_ULCAST_(0x1ff) << 3)
+#define MIPS_WATCHHI_I		(_ULCAST_(1) << 2)
+#define MIPS_WATCHHI_R		(_ULCAST_(1) << 1)
+#define MIPS_WATCHHI_W		(_ULCAST_(1) << 0)
+#define MIPS_WATCHHI_IRW	(_ULCAST_(0x7) << 0)
+
 /* MAAR bit definitions */
 #define MIPS_MAAR_ADDR		((BIT_ULL(BITS_PER_LONG - 12) - 1) << 12)
 #define MIPS_MAAR_ADDR_SHIFT	12
diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
index a5279b2f3198..48c0534c4d15 100644
--- a/arch/mips/kernel/ptrace.c
+++ b/arch/mips/kernel/ptrace.c
@@ -210,7 +210,8 @@ int ptrace_get_watch_regs(struct task_struct *child,
 	for (i = 0; i < boot_cpu_data.watch_reg_use_cnt; i++) {
 		__put_user(child->thread.watch.mips3264.watchlo[i],
 			   &addr->WATCH_STYLE.watchlo[i]);
-		__put_user(child->thread.watch.mips3264.watchhi[i] & 0xfff,
+		__put_user(child->thread.watch.mips3264.watchhi[i] &
+				(MIPS_WATCHHI_MASK | MIPS_WATCHHI_IRW),
 			   &addr->WATCH_STYLE.watchhi[i]);
 		__put_user(boot_cpu_data.watch_reg_masks[i],
 			   &addr->WATCH_STYLE.watch_masks[i]);
@@ -252,12 +253,12 @@ int ptrace_set_watch_regs(struct task_struct *child,
 		}
 #endif
 		__get_user(ht[i], &addr->WATCH_STYLE.watchhi[i]);
-		if (ht[i] & ~0xff8)
+		if (ht[i] & ~MIPS_WATCHHI_MASK)
 			return -EINVAL;
 	}
 	/* Install them. */
 	for (i = 0; i < boot_cpu_data.watch_reg_use_cnt; i++) {
-		if (lt[i] & 7)
+		if (lt[i] & MIPS_WATCHLO_IRW)
 			watch_active = 1;
 		child->thread.watch.mips3264.watchlo[i] = lt[i];
 		/* Set the G bit. */
diff --git a/arch/mips/kernel/watch.c b/arch/mips/kernel/watch.c
index 9b78e375118e..19fcab7348b1 100644
--- a/arch/mips/kernel/watch.c
+++ b/arch/mips/kernel/watch.c
@@ -25,16 +25,20 @@ void mips_install_watch_registers(struct task_struct *t)
 		write_c0_watchlo3(watches->watchlo[3]);
 		/* Write 1 to the I, R, and W bits to clear them, and
 		   1 to G so all ASIDs are trapped. */
-		write_c0_watchhi3(0x40000007 | watches->watchhi[3]);
+		write_c0_watchhi3(MIPS_WATCHHI_G | MIPS_WATCHHI_IRW |
+				  watches->watchhi[3]);
 	case 3:
 		write_c0_watchlo2(watches->watchlo[2]);
-		write_c0_watchhi2(0x40000007 | watches->watchhi[2]);
+		write_c0_watchhi2(MIPS_WATCHHI_G | MIPS_WATCHHI_IRW |
+				  watches->watchhi[2]);
 	case 2:
 		write_c0_watchlo1(watches->watchlo[1]);
-		write_c0_watchhi1(0x40000007 | watches->watchhi[1]);
+		write_c0_watchhi1(MIPS_WATCHHI_G | MIPS_WATCHHI_IRW |
+				  watches->watchhi[1]);
 	case 1:
 		write_c0_watchlo0(watches->watchlo[0]);
-		write_c0_watchhi0(0x40000007 | watches->watchhi[0]);
+		write_c0_watchhi0(MIPS_WATCHHI_G | MIPS_WATCHHI_IRW |
+				  watches->watchhi[0]);
 	}
 }
 
@@ -51,22 +55,26 @@ void mips_read_watch_registers(void)
 	default:
 		BUG();
 	case 4:
-		watches->watchhi[3] = (read_c0_watchhi3() & 0x0fff);
+		watches->watchhi[3] = (read_c0_watchhi3() &
+				       (MIPS_WATCHHI_MASK | MIPS_WATCHHI_IRW));
 	case 3:
-		watches->watchhi[2] = (read_c0_watchhi2() & 0x0fff);
+		watches->watchhi[2] = (read_c0_watchhi2() &
+				       (MIPS_WATCHHI_MASK | MIPS_WATCHHI_IRW));
 	case 2:
-		watches->watchhi[1] = (read_c0_watchhi1() & 0x0fff);
+		watches->watchhi[1] = (read_c0_watchhi1() &
+				       (MIPS_WATCHHI_MASK | MIPS_WATCHHI_IRW));
 	case 1:
-		watches->watchhi[0] = (read_c0_watchhi0() & 0x0fff);
+		watches->watchhi[0] = (read_c0_watchhi0() &
+				       (MIPS_WATCHHI_MASK | MIPS_WATCHHI_IRW));
 	}
 	if (current_cpu_data.watch_reg_use_cnt == 1 &&
-	    (watches->watchhi[0] & 7) == 0) {
+	    (watches->watchhi[0] & MIPS_WATCHHI_IRW) == 0) {
 		/* Pathological case of release 1 architecture that
 		 * doesn't set the condition bits.  We assume that
 		 * since we got here, the watch condition was met and
 		 * signal that the conditions requested in watchlo
 		 * were met.  */
-		watches->watchhi[0] |= (watches->watchlo[0] & 7);
+		watches->watchhi[0] |= (watches->watchlo[0] & MIPS_WATCHHI_IRW);
 	}
  }
 
@@ -109,86 +117,86 @@ void mips_probe_watch_registers(struct cpuinfo_mips *c)
 	 * Check which of the I,R and W bits are supported, then
 	 * disable the register.
 	 */
-	write_c0_watchlo0(7);
+	write_c0_watchlo0(MIPS_WATCHLO_IRW);
 	back_to_back_c0_hazard();
 	t = read_c0_watchlo0();
 	write_c0_watchlo0(0);
-	c->watch_reg_masks[0] = t & 7;
+	c->watch_reg_masks[0] = t & MIPS_WATCHLO_IRW;
 
 	/* Write the mask bits and read them back to determine which
 	 * can be used. */
 	c->watch_reg_count = 1;
 	c->watch_reg_use_cnt = 1;
 	t = read_c0_watchhi0();
-	write_c0_watchhi0(t | 0xff8);
+	write_c0_watchhi0(t | MIPS_WATCHHI_MASK);
 	back_to_back_c0_hazard();
 	t = read_c0_watchhi0();
-	c->watch_reg_masks[0] |= (t & 0xff8);
-	if ((t & 0x80000000) == 0)
+	c->watch_reg_masks[0] |= (t & MIPS_WATCHHI_MASK);
+	if ((t & MIPS_WATCHHI_M) == 0)
 		return;
 
-	write_c0_watchlo1(7);
+	write_c0_watchlo1(MIPS_WATCHLO_IRW);
 	back_to_back_c0_hazard();
 	t = read_c0_watchlo1();
 	write_c0_watchlo1(0);
-	c->watch_reg_masks[1] = t & 7;
+	c->watch_reg_masks[1] = t & MIPS_WATCHLO_IRW;
 
 	c->watch_reg_count = 2;
 	c->watch_reg_use_cnt = 2;
 	t = read_c0_watchhi1();
-	write_c0_watchhi1(t | 0xff8);
+	write_c0_watchhi1(t | MIPS_WATCHHI_MASK);
 	back_to_back_c0_hazard();
 	t = read_c0_watchhi1();
-	c->watch_reg_masks[1] |= (t & 0xff8);
-	if ((t & 0x80000000) == 0)
+	c->watch_reg_masks[1] |= (t & MIPS_WATCHHI_MASK);
+	if ((t & MIPS_WATCHHI_M) == 0)
 		return;
 
-	write_c0_watchlo2(7);
+	write_c0_watchlo2(MIPS_WATCHLO_IRW);
 	back_to_back_c0_hazard();
 	t = read_c0_watchlo2();
 	write_c0_watchlo2(0);
-	c->watch_reg_masks[2] = t & 7;
+	c->watch_reg_masks[2] = t & MIPS_WATCHLO_IRW;
 
 	c->watch_reg_count = 3;
 	c->watch_reg_use_cnt = 3;
 	t = read_c0_watchhi2();
-	write_c0_watchhi2(t | 0xff8);
+	write_c0_watchhi2(t | MIPS_WATCHHI_MASK);
 	back_to_back_c0_hazard();
 	t = read_c0_watchhi2();
-	c->watch_reg_masks[2] |= (t & 0xff8);
-	if ((t & 0x80000000) == 0)
+	c->watch_reg_masks[2] |= (t & MIPS_WATCHHI_MASK);
+	if ((t & MIPS_WATCHHI_M) == 0)
 		return;
 
-	write_c0_watchlo3(7);
+	write_c0_watchlo3(MIPS_WATCHLO_IRW);
 	back_to_back_c0_hazard();
 	t = read_c0_watchlo3();
 	write_c0_watchlo3(0);
-	c->watch_reg_masks[3] = t & 7;
+	c->watch_reg_masks[3] = t & MIPS_WATCHLO_IRW;
 
 	c->watch_reg_count = 4;
 	c->watch_reg_use_cnt = 4;
 	t = read_c0_watchhi3();
-	write_c0_watchhi3(t | 0xff8);
+	write_c0_watchhi3(t | MIPS_WATCHHI_MASK);
 	back_to_back_c0_hazard();
 	t = read_c0_watchhi3();
-	c->watch_reg_masks[3] |= (t & 0xff8);
-	if ((t & 0x80000000) == 0)
+	c->watch_reg_masks[3] |= (t & MIPS_WATCHHI_MASK);
+	if ((t & MIPS_WATCHHI_M) == 0)
 		return;
 
 	/* We use at most 4, but probe and report up to 8. */
 	c->watch_reg_count = 5;
 	t = read_c0_watchhi4();
-	if ((t & 0x80000000) == 0)
+	if ((t & MIPS_WATCHHI_M) == 0)
 		return;
 
 	c->watch_reg_count = 6;
 	t = read_c0_watchhi5();
-	if ((t & 0x80000000) == 0)
+	if ((t & MIPS_WATCHHI_M) == 0)
 		return;
 
 	c->watch_reg_count = 7;
 	t = read_c0_watchhi6();
-	if ((t & 0x80000000) == 0)
+	if ((t & MIPS_WATCHHI_M) == 0)
 		return;
 
 	c->watch_reg_count = 8;
-- 
2.4.10

Re: [PATCH 4/4] MIPS: Add and use watch register field definitions

[David Daney]

On 03/01/2016 02:19 PM, James Hogan wrote:
> The files watch.c and ptrace.c contain various magic masks for
> WatchLo/WatchHi register fields. Add some definitions to mipsregs.h for
> these registers and make use of them in both watch.c and ptrace.c,
> hopefully making them more readable.
>
> Signed-off-by: James Hogan<james.hogan@imgtec.com>

Seems sane,

Reviewed-by: David Daney <david.daney@cavium.com>

> Cc: Ralf Baechle<ralf@linux-mips.org>
> Cc:linux-mips@linux-mips.org
> ---
>   arch/mips/include/asm/mipsregs.h | 18 ++++++++++
>   arch/mips/kernel/ptrace.c        |  7 ++--
>   arch/mips/kernel/watch.c         | 74 ++++++++++++++++++++++------------------
>   3 files changed, 63 insertions(+), 36 deletions(-)
>
[...]