Re: [Patch 2/2] MIPS hardware watchpoint support.

[David Daney <ddaney@caviumnetworks.com>]

[-- Attachment #1: Type: text/plain, Size: 15190 bytes --]

Pedro Alves wrote:
> On Monday 06 April 2009 08:27:07, David Daney wrote:
>> Support for hardware watchpoints for mips-linux was merged into the 
>> Linux kernel for version 2.6.28, with some bug fixes in 2.6.29.  This 
>> patch adds the corresponding gdb support.  It has been tested on both 
>> mipsel-linux (32-bit kernel) and mips64-linux (64-bit kernel).
> 
> Thanks!  I'll pretend to know a thing about mips from here on, and
> hope that it doesn't show too much that I don't.
> 
> In general, this looks very good to me, although I didn't
> make an effort to understand all the bit juggling going on.  Minor
> comments to the code below.  Then comments to the tests follow that.
> 
>> 2009-04-05  David Daney  <ddaney@caviumnetworks.com>
> 
>>         (get_irw_mask, get_reg_mask, get_num_valid, get_watchlo,
>>         set_watchlo, get_watchhi, set_watchhi, mips_show_dr,
> 
> Please split these context lines like so instead, as per the
> coding conventions:
> 
>>         (get_irw_mask, get_reg_mask, get_num_valid, get_watchlo)
>>         (set_watchlo, get_watchhi, set_watchhi, mips_show_dr)
> 
> 

Done.  All these years and there is still more to learn about the coding 
conventions.

>> +/* Assuming usable watch registers, return the irw_mask.  */
>> +static uint32_t
> 
> Please add an empty line between comment and function.  Here
> and elsewhere.

Done.

> 
>> +get_irw_mask (struct pt_watch_regs *regs, int set)
>> +{
>> +  switch (regs->style)
>> +    {
>> +    case pt_watch_style_mips32:
>> +      return regs->mips32.watch_masks[set] & IRW_MASK;
>> +    case pt_watch_style_mips64:
>> +      return regs->mips64.watch_masks[set] & IRW_MASK;
>> +    default:
>> +      gdb_assert (0);
> 
> Please don't write 'gdb_assert (0)', instead make that a
> more bit more friendly `internal_error'.  Here and
> elsewhere.
> 

Done.

>> +    printf_unfiltered (" (addr=%lx, len=%d, type=%s)",
>> +                      /* This code is for mips, so casting CORE_ADDR
>> +                         to unsigned long should be okay.  */
>> +                      (unsigned long)addr, len,
> 
> Why bother to explain that instead of using `paddr'?  If there's
> a reason, then it would be nice if it was spelled out in the
> comment.

Comment deleted, and types casted to (unsigned long long).


>> +  for (i = 0; i < MAX_DEBUG_REGISTER; i++)
>> +    {
> 
> Unneeded braces.  You have a couple more in the patch.

Fixed.

> 
>> +      printf_unfiltered ("\tDR%d: lo=0x%s, hi=0x%s\n",
>> +                        i, paddr (get_watchlo (&watch_mirror, i)),
>> +                        paddr (get_watchhi (&watch_mirror, i)));
>> +    }
>> +}
>> +

>> +       {
>> +         perror_with_name (_("Couldn't write debug register"));
>> +         return -1;
> 
> perror doesn't return, no need for `return -1'.

Fixed.


>> +
>> +  watch_mirror = watch_readback;
>> +  populate_regs_from_watches (&watch_mirror);
>> +
>> +  retval = write_watchpoint_regs();
> 
>                                    ^ missing space.


All instances fixed.


>> +  deprecated_add_set_cmd ("show-debug-regs", class_maintenance,
>> +                         var_boolean, (char *) &maint_show_dr, _("\
>> +Set whether to show variables that mirror the mips debug registers.\n\
>> +Use \"on\" to enable, \"off\" to disable.\n\
>> +If enabled, the debug registers values are shown when GDB inserts\n\
>> +or removes a hardware breakpoint or watchpoint, and when the inferior\n\
>> +triggers a breakpoint or watchpoint."),
>> +                         &maintenancelist);
> 
> Argh.  I've posted a patch that removes deprecated_add_set_cmd, but
> never got around to committing it.
> 
>  http://sourceware.org/ml/gdb-patches/2009-01/msg00119.html
> 
> When I get around to finish off that patch, I'll adjust this
> file too --- I assume that is OK with you.
> 
> However, reusing the x86 command does avoid needing to document
> it, but, could you remove the reference to x86 in the current
> docs?
> 
>  @kindex maint show-debug-regs
>  @cindex x86 hardware debug registers
>  @item maint show-debug-regs
>  Control whether to show variables that mirror the x86 hardware debug
>  registers.  Use @code{ON} to enable, @code{OFF} to disable.  If
>  enabled, the debug registers values are shown when @value{GDBN} inserts or
>  removes a hardware breakpoint or watchpoint, and when the inferior
>  triggers a hardware-assisted breakpoint or watchpoint.
> 
> ... since it now applies to mips as well.

Done.


>> For this submission I tested on a mipsel-linux system with these results 
>> (with comments):
>>
>> --- ./gdb/testsuite/gdb.sum    2009-04-05 20:53:20.000000000 -0700
>> +++ gdb.sum    2009-04-05 17:53:05.000000000 -0700
>> @@ -1,4 +1,4 @@
>> -Test Run By root on Sun Apr  5 18:08:05 2009
>> +Test Run By root on Sun Apr  5 14:45:02 2009
>>  Native configuration is mipsel-unknown-linux-gnu
>>  
>>          === gdb tests ===
>> @@ -5717,15 +5717,15 @@
>>  PASS: gdb.base/recurse.exp: continue to recurse (a = 5)
>>  PASS: gdb.base/recurse.exp: next over b = 0 in second instance
>>  PASS: gdb.base/recurse.exp: set second instance watchpoint
>> -PASS: gdb.base/recurse.exp: continue to second instance watchpoint, 
>> first time
>> -PASS: gdb.base/recurse.exp: continue to recurse (a = 4)
>> -PASS: gdb.base/recurse.exp: continue to recurse (a = 3)
>> -PASS: gdb.base/recurse.exp: continue to recurse (a = 2)
>> -PASS: gdb.base/recurse.exp: continue to recurse (a = 1)
>> -PASS: gdb.base/recurse.exp: continue to second instance watchpoint, 
>> second time
>> -PASS: gdb.base/recurse.exp: second instance watchpoint deleted when 
>> leaving scope
>> -PASS: gdb.base/recurse.exp: continue to first instance watchpoint, 
>> second time
>> -PASS: gdb.base/recurse.exp: first instance watchpoint deleted when 
>> leaving scope
>> +FAIL: gdb.base/recurse.exp: continue to second instance watchpoint, 
>> first time
>> +FAIL: gdb.base/recurse.exp: continue to recurse (a = 4)
>> +FAIL: gdb.base/recurse.exp: continue to recurse (a = 3)
>> +FAIL: gdb.base/recurse.exp: continue to recurse (a = 2)
>> +FAIL: gdb.base/recurse.exp: continue to recurse (a = 1)
>> +FAIL: gdb.base/recurse.exp: continue to second instance watchpoint, 
>> second time
>> +FAIL: gdb.base/recurse.exp: second instance watchpoint deleted when 
>> leaving scope
>> +FAIL: gdb.base/recurse.exp: continue to first instance watchpoint, 
>> second time
>> +FAIL: gdb.base/recurse.exp: first instance watchpoint deleted when 
>> leaving scope
>>
>> These failures are caused by the fact that the test assumes more than a 
>> the single watch register supported by the test system.  Although this 
>> patch supports up to eight registers, the test platform only has one.  
>> We return true from mips_linux_can_use_hw_breakpoint for multiple 
>> requests as it is not possible to know how many watchpoints are 
>> enabled.  Later gdb tries to insert more than one watchpoint and all but 
>> the first fail.
> 
> Could we detect that, and fail the rest of the test gracefully?
> What error message do you get?
> 

I slightly modified the patch so that it makes more conservative 
estimates about how many hardware watches it can handle.  This fixes 
these failures.


> 
>> -PASS: gdb.mi/mi-watch.exp: hw: watchpoint trigger
>> +FAIL: gdb.mi/mi-watch.exp: hw: watchpoint trigger (unknown output after 
>> running)
>>
>> Reported instruction location of the watchpoint trigger is one 
>> instruction later and one line later.
> 
> Why is that?

I was mistaken, it is earlier, not later.  With hardware watch points, 
$pc points to the faulting instruction, with software watch points $pc 
points to the following instruction.  In a couple of tests, this results 
in the reported line number being different than the expected value.



Here is the revised patch and its test results:

--- pre/gdb.sum	2009-04-15 12:12:33.000000000 -0700
+++ post/gdb.sum	2009-04-15 10:22:37.000000000 -0700
@@ -1,4 +1,4 @@
-Test Run By root on Wed Apr 15 10:42:35 2009
+Test Run By root on Tue Apr 14 16:28:52 2009
  Native configuration is mips64-unknown-linux-gnu

  		=== gdb tests ===
@@ -5684,7 +5684,7 @@
  PASS: gdb.base/recurse.exp: continue to recurse (a = 1)
  PASS: gdb.base/recurse.exp: continue to second instance watchpoint, 
second time
  PASS: gdb.base/recurse.exp: second instance watchpoint deleted when 
leaving scope
-PASS: gdb.base/recurse.exp: continue to first instance watchpoint, 
second time
+FAIL: gdb.base/recurse.exp: continue to first instance watchpoint, 
second time

Different line number as explained above.


  PASS: gdb.base/recurse.exp: first instance watchpoint deleted when 
leaving scope
  Running ../../../src/gdb/testsuite/gdb.base/regs.exp ...
  Running ../../../src/gdb/testsuite/gdb.base/relational.exp ...
@@ -8272,26 +8272,26 @@
  PASS: gdb.base/watch_thread_num.exp: Disable breakpoint 2
  PASS: gdb.base/watch_thread_num.exp: Watchpoint on shared variable
  PASS: gdb.base/watch_thread_num.exp: info breakpoint 3
-FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 1 
(timeout)
-FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 1 (timeout)
-FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 2 
(timeout)
-FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 2 (timeout)
-FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 3 
(timeout)
-FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 3 (timeout)
-FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 4 
(timeout)
-FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 4 (timeout)
-FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 5 
(timeout)
-FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 5 (timeout)
-FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 6 
(timeout)
-FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 6 (timeout)
-FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 7 
(timeout)
-FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 7 (timeout)
-FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 8 
(timeout)
-FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 8 (timeout)
-FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 9 
(timeout)
-FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 9 (timeout)
-FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 10 
(timeout)
-FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 10 (timeout)
+PASS: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 1
+PASS: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 1
+FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 2
+FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 2
+FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 3
+PASS: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 3
+FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 4
+FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 4
+FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 5
+FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 5
+FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 6
+FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 6
+FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 7
+FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 7
+FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 8
+PASS: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 8
+FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 9
+FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 9
+FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 10
+FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered 
iteration 10
  Running ../../../src/gdb/testsuite/gdb.base/watchpoint-hw.exp ...
  Running ../../../src/gdb/testsuite/gdb.base/watchpoint-solib.exp ...
  PASS: gdb.base/watchpoint-solib.exp: set pending breakpoint
@@ -8476,7 +8476,7 @@
  PASS: gdb.cp/annota2.exp: breakpoint at main
  PASS: gdb.cp/annota2.exp: run until main breakpoint
  PASS: gdb.cp/annota2.exp: set watch on a.x
-PASS: gdb.cp/annota2.exp: watch triggered on a.x
+FAIL: gdb.cp/annota2.exp: watch triggered on a.x
  PASS: gdb.cp/annota2.exp: annotate-quit
  Running ../../../src/gdb/testsuite/gdb.cp/annota3.exp ...
  PASS: gdb.cp/annota3.exp: breakpoint main
@@ -8488,7 +8488,7 @@
  PASS: gdb.cp/annota3.exp: break at main
  PASS: gdb.cp/annota3.exp: second run until main breakpoint
  PASS: gdb.cp/annota3.exp: set watch on a.x
-PASS: gdb.cp/annota3.exp: watch triggered on a.x
+FAIL: gdb.cp/annota3.exp: watch triggered on a.x

The two annotation failures are explained in the

  PASS: gdb.cp/annota3.exp: annotate-quit
  Running ../../../src/gdb/testsuite/gdb.cp/anon-union.exp ...
  PASS: gdb.cp/anon-union.exp: next 1
@@ -11976,7 +11976,7 @@
  PASS: gdb.mi/mi-watch.exp: hw: mi runto callee4
  PASS: gdb.mi/mi-watch.exp: hw: break-watch operation
  PASS: gdb.mi/mi-watch.exp: hw: list of watchpoints
-PASS: gdb.mi/mi-watch.exp: hw: watchpoint trigger
+FAIL: gdb.mi/mi-watch.exp: hw: watchpoint trigger (unknown output after 
running)

Different line number as explained above.

  PASS: gdb.mi/mi-watch.exp: hw: watchpoint trigger
  Running ../../../src/gdb/testsuite/gdb.mi/mi2-basics.exp ...
  PASS: gdb.mi/mi2-basics.exp: acceptance of MI operations
@@ -12609,7 +12609,7 @@
  PASS: gdb.mi/mi2-watch.exp: hw: mi runto callee4
  PASS: gdb.mi/mi2-watch.exp: hw: break-watch operation
  PASS: gdb.mi/mi2-watch.exp: hw: list of watchpoints
-PASS: gdb.mi/mi2-watch.exp: hw: watchpoint trigger
+FAIL: gdb.mi/mi2-watch.exp: hw: watchpoint trigger (unknown output 
after running)

Different line number as explained above.

  PASS: gdb.mi/mi2-watch.exp: hw: watchpoint trigger
  Running ../../../src/gdb/testsuite/gdb.modula2/unbounded-array.exp ...
  PASS: gdb.modula2/unbounded-array.exp: switch to modula-2
@@ -13351,10 +13351,10 @@
  PASS: gdb.threads/tls.exp: info address a_thread_local
  Running ../../../src/gdb/testsuite/gdb.threads/watchthreads.exp ...
  PASS: gdb.threads/watchthreads.exp: successfully compiled posix 
threads test case
-FAIL: gdb.threads/watchthreads.exp: watch args[0]
+PASS: gdb.threads/watchthreads.exp: watch args[0]
  FAIL: gdb.threads/watchthreads.exp: watch args[1]
  FAIL: gdb.threads/watchthreads.exp: threaded watch loop
-FAIL: gdb.threads/watchthreads.exp: first watchpoint on args[0] hit
+PASS: gdb.threads/watchthreads.exp: first watchpoint on args[0] hit
  FAIL: gdb.threads/watchthreads.exp: first watchpoint on args[1] hit
  FAIL: gdb.threads/watchthreads.exp: watchpoint on args[0] hit in thread
  FAIL: gdb.threads/watchthreads.exp: watchpoint on args[1] hit in thread



OK to commit?


[-- Attachment #2: mips-watch.patch --]
[-- Type: text/plain, Size: 22095 bytes --]

gdb/
2009-04-16  David Daney  <ddaney@caviumnetworks.com>

	* mips-linux-nat.c (command.h, gdbcmd.h, gdb_assert.h): New #includes.
	(maint_show_dr, super_close): New variables.
	(super_fetch_registers, super_store_registers): Make static.
	(PTRACE_GET_WATCH_REGS, PTRACE_SET_WATCH_REGS, W_BIT, R_BIT, I_BIT)
	(W_MASK, R_MASK, I_MASK, IRW_MASK, MAX_DEBUG_REGISTER): Define.
	(pt_watch_style): Define new enum.
	(mips32_watch_regs, mips64_watch_regs, pt_watch_regs, mips_watchpoint):
	Define new structs.
	(watch_readback_valid, watch_readback, current_watches,	watch_mirror):
	New variables.
	(get_irw_mask, get_reg_mask, get_num_valid, get_watchlo)
	(set_watchlo, get_watchhi, set_watchhi, mips_show_dr)
	(mips_linux_read_watch_registers, mips_linux_can_use_hw_breakpoint)
	(mips_linux_stopped_by_watchpoint, mips_linux_stopped_data_address)
	(type_to_irw, fill_mask, try_one_watch)
	(mips_linux_region_ok_for_hw_watchpoint, write_watchpoint_regs)
	(mips_linux_new_thread, populate_regs_from_watches)
	(mips_linux_insert_watchpoint, mips_linux_remove_watchpoint)
	(mips_linux_close): New functions.
	(_initialize_mips_linux_nat): Register watchpoint functions with
	the target_ops.  Add show-debug-regs maintenance command.


gdb/doc/
2009-04-16  David Daney  <ddaney@caviumnetworks.com>

	* gdb.texinfo (maint show-debug-regs): Remove mention of x86.

	

Index: mips-linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-linux-nat.c,v
retrieving revision 1.32
diff -u -p -r1.32 mips-linux-nat.c
--- mips-linux-nat.c	26 Feb 2009 19:44:39 -0000	1.32
+++ mips-linux-nat.c	16 Apr 2009 16:34:56 -0000
@@ -19,6 +19,9 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "command.h"
+#include "gdbcmd.h"
+#include "gdb_assert.h"
 #include "inferior.h"
 #include "mips-tdep.h"
 #include "target.h"
@@ -44,11 +47,19 @@
    we'll clear this and use PTRACE_PEEKUSER instead.  */
 static int have_ptrace_regsets = 1;
 
+/* Whether or not to print the mirrored debug registers.  */
+
+static int maint_show_dr;
+
 /* Saved function pointers to fetch and store a single register using
    PTRACE_PEEKUSER and PTRACE_POKEUSER.  */
 
-void (*super_fetch_registers) (struct target_ops *, struct regcache *, int);
-void (*super_store_registers) (struct target_ops *, struct regcache *, int);
+static void (*super_fetch_registers) (struct target_ops *,
+				      struct regcache *, int);
+static void (*super_store_registers) (struct target_ops *,
+				      struct regcache *, int);
+
+static void (*super_close) (int);
 
 /* Map gdb internal register number to ptrace ``address''.
    These ``addresses'' are normally defined in <asm/ptrace.h>. 
@@ -357,12 +368,697 @@ mips_linux_read_description (struct targ
     return tdesc_mips64_linux;
 }
 
+#ifndef PTRACE_GET_WATCH_REGS
+#  define PTRACE_GET_WATCH_REGS	0xd0
+#endif
+
+#ifndef PTRACE_SET_WATCH_REGS
+#  define PTRACE_SET_WATCH_REGS	0xd1
+#endif
+
+#define W_BIT 0
+#define R_BIT 1
+#define I_BIT 2
+
+#define W_MASK (1 << W_BIT)
+#define R_MASK (1 << R_BIT)
+#define I_MASK (1 << I_BIT)
+
+#define IRW_MASK (I_MASK | R_MASK | W_MASK)
+
+enum pt_watch_style {
+  pt_watch_style_mips32,
+  pt_watch_style_mips64
+};
+
+#define MAX_DEBUG_REGISTER 8
+
+/* A value of zero in a watchlo indicates that it is available. */
+
+struct mips32_watch_regs
+{
+  uint32_t watchlo[MAX_DEBUG_REGISTER];
+  /* Lower 16 bits of watchhi. */
+  uint16_t watchhi[MAX_DEBUG_REGISTER];
+  /* Valid mask and I R W bits.
+   * bit 0 -- 1 if W bit is usable.
+   * bit 1 -- 1 if R bit is usable.
+   * bit 2 -- 1 if I bit is usable.
+   * bits 3 - 11 -- Valid watchhi mask bits.
+   */
+  uint16_t watch_masks[MAX_DEBUG_REGISTER];
+  /* The number of valid watch register pairs.  */
+  uint32_t num_valid;
+  /* There is confusion across gcc versions about structure alignment,
+     so we force 8 byte alignment for these structures so they match
+     the kernel even if it was build with a different gcc version.  */
+} __attribute__ ((aligned (8)));
+
+struct mips64_watch_regs
+{
+  uint64_t watchlo[MAX_DEBUG_REGISTER];
+  uint16_t watchhi[MAX_DEBUG_REGISTER];
+  uint16_t watch_masks[MAX_DEBUG_REGISTER];
+  uint32_t num_valid;
+} __attribute__ ((aligned (8)));
+
+struct pt_watch_regs
+{
+  enum pt_watch_style style;
+  union
+  {
+    struct mips32_watch_regs mips32;
+    struct mips64_watch_regs mips64;
+  };
+};
+
+/* -1 if the kernel and/or CPU do not support watch registers.
+    1 if watch_readback is valid and we can read style, num_valid
+      and the masks.
+    0 if we need to read the watch_readback.  */
+
+static int watch_readback_valid;
+
+/* Cached watch register read values.  */
+
+static struct pt_watch_regs watch_readback;
+
+/* We keep list of all watchpoints we should install and calculate the
+   watch register values each time the list changes.  This allows for
+   easy sharing of watch registers for more than one watchpoint.  */
+
+struct mips_watchpoint
+{
+  CORE_ADDR addr;
+  int len;
+  int type;
+  struct mips_watchpoint *next;
+};
+
+static struct mips_watchpoint *current_watches;
+
+/*  The current set of watch register values for writing the
+    registers.  */
+
+static struct pt_watch_regs watch_mirror;
+
+/* Assuming usable watch registers, return the irw_mask.  */
+
+static uint32_t
+get_irw_mask (struct pt_watch_regs *regs, int set)
+{
+  switch (regs->style)
+    {
+    case pt_watch_style_mips32:
+      return regs->mips32.watch_masks[set] & IRW_MASK;
+    case pt_watch_style_mips64:
+      return regs->mips64.watch_masks[set] & IRW_MASK;
+    default:
+      internal_error (__FILE__, __LINE__,
+		      _("Unrecognized watch register style"));
+    }
+}
+
+/* Assuming usable watch registers, return the reg_mask.  */
+
+static uint32_t
+get_reg_mask (struct pt_watch_regs *regs, int set)
+{
+  switch (regs->style)
+    {
+    case pt_watch_style_mips32:
+      return regs->mips32.watch_masks[set] & ~IRW_MASK;
+    case pt_watch_style_mips64:
+      return regs->mips64.watch_masks[set] & ~IRW_MASK;
+    default:
+      internal_error (__FILE__, __LINE__,
+		      _("Unrecognized watch register style"));
+    }
+}
+
+/* Assuming usable watch registers, return the num_valid.  */
+
+static uint32_t
+get_num_valid (struct pt_watch_regs *regs)
+{
+  switch (regs->style)
+    {
+    case pt_watch_style_mips32:
+      return regs->mips32.num_valid;
+    case pt_watch_style_mips64:
+      return regs->mips64.num_valid;
+    default:
+      internal_error (__FILE__, __LINE__,
+		      _("Unrecognized watch register style"));
+    }
+}
+
+/* Assuming usable watch registers, return the watchlo.  */
+
+static CORE_ADDR
+get_watchlo (struct pt_watch_regs *regs, int set)
+{
+  switch (regs->style)
+    {
+    case pt_watch_style_mips32:
+      return regs->mips32.watchlo[set];
+    case pt_watch_style_mips64:
+      return regs->mips64.watchlo[set];
+    default:
+      internal_error (__FILE__, __LINE__,
+		      _("Unrecognized watch register style"));
+    }
+}
+
+/* Assuming usable watch registers, set a watchlo value.  */
+
+static void
+set_watchlo (struct pt_watch_regs *regs, int set, CORE_ADDR value)
+{
+  switch (regs->style)
+    {
+    case pt_watch_style_mips32:
+      /*  The cast will never throw away bits as 64 bit addresses can
+	  never be used on a 32 bit kernel.  */
+      regs->mips32.watchlo[set] = (uint32_t)value;
+      break;
+    case pt_watch_style_mips64:
+      regs->mips64.watchlo[set] = value;
+      break;
+    default:
+      internal_error (__FILE__, __LINE__,
+		      _("Unrecognized watch register style"));
+    }
+}
+
+/* Assuming usable watch registers, return the watchhi.  */
+
+static uint32_t
+get_watchhi (struct pt_watch_regs *regs, int n)
+{
+  switch (regs->style)
+    {
+    case pt_watch_style_mips32:
+      return regs->mips32.watchhi[n];
+    case pt_watch_style_mips64:
+      return regs->mips64.watchhi[n];
+    default:
+      internal_error (__FILE__, __LINE__,
+		      _("Unrecognized watch register style"));
+    }
+}
+
+/* Assuming usable watch registers, set a watchhi value.  */
+
+static void
+set_watchhi (struct pt_watch_regs *regs, int n, uint16_t value)
+{
+  switch (regs->style)
+    {
+    case pt_watch_style_mips32:
+      regs->mips32.watchhi[n] = value;
+      break;
+    case pt_watch_style_mips64:
+      regs->mips64.watchhi[n] = value;
+      break;
+    default:
+      internal_error (__FILE__, __LINE__,
+		      _("Unrecognized watch register style"));
+    }
+}
+
+static void
+mips_show_dr (const char *func, CORE_ADDR addr,
+	      int len, enum target_hw_bp_type type)
+{
+  int i;
+
+  puts_unfiltered (func);
+  if (addr || len)
+    printf_unfiltered (" (addr=%llx, len=%d, type=%s)",
+		       (unsigned long long)addr, len,
+		       type == hw_write ? "data-write"
+		       : (type == hw_read ? "data-read"
+			  : (type == hw_access ? "data-read/write"
+			     : (type == hw_execute ? "instruction-execute"
+				: "??unknown??"))));
+  puts_unfiltered (":\n");
+
+  for (i = 0; i < MAX_DEBUG_REGISTER; i++)
+    printf_unfiltered ("\tDR%d: lo=0x%s, hi=0x%s\n",
+		       i, paddr (get_watchlo (&watch_mirror, i)),
+		       paddr (get_watchhi (&watch_mirror, i)));
+}
+
+/* Return 1 if watch registers are usable.  Cached information is used
+   unless force is true.  */
+
+static int
+mips_linux_read_watch_registers (int force)
+{
+  int tid;
+
+  if (force || watch_readback_valid == 0)
+    {
+      tid = ptid_get_lwp (inferior_ptid);
+      if (ptrace (PTRACE_GET_WATCH_REGS, tid, &watch_readback) == -1)
+	{
+	  watch_readback_valid = -1;
+	  return 0;
+	}
+      switch (watch_readback.style)
+	{
+	case pt_watch_style_mips32:
+	  if (watch_readback.mips32.num_valid == 0)
+	    {
+	      watch_readback_valid = -1;
+	      return 0;
+	    }
+	  break;
+	case pt_watch_style_mips64:
+	  if (watch_readback.mips64.num_valid == 0)
+	    {
+	      watch_readback_valid = -1;
+	      return 0;
+	    }
+	  break;
+	default:
+	  watch_readback_valid = -1;
+	  return 0;
+	}
+      /* Watch registers appear to be usable.  */
+      watch_readback_valid = 1;
+    }
+  return (watch_readback_valid == 1) ? 1 : 0;
+}
+
+/* Convert GDB's type to an IRW mask.  */
+
+static unsigned
+type_to_irw (int type)
+{
+  switch (type)
+    {
+    case hw_write:
+      return W_MASK;
+    case hw_read:
+      return R_MASK;
+    case hw_access:
+      return (W_MASK | R_MASK);
+    default:
+      return 0;
+    }
+}
+
+/* Target to_can_use_hw_breakpoint implementation.  Return 1 if we can
+   handle the specified watch type.  */
+
+static int
+mips_linux_can_use_hw_breakpoint (int type, int cnt, int ot)
+{
+  int i;
+  uint32_t wanted_mask, irw_mask;
+
+  if (!mips_linux_read_watch_registers (0))
+    return 0;
+
+   switch (type)
+    {
+    case bp_hardware_watchpoint:
+      wanted_mask = W_MASK;
+      break;
+    case bp_read_watchpoint:
+      wanted_mask = R_MASK;
+      break;
+    case bp_access_watchpoint:
+      wanted_mask = R_MASK | W_MASK;
+      break;
+    default:
+      return 0;
+    }
+ 
+  for (i = 0; i < get_num_valid (&watch_readback) && cnt; i++)
+    {
+      irw_mask = get_irw_mask (&watch_readback, i);
+      if ((irw_mask & wanted_mask) == wanted_mask)
+	cnt--;
+    }
+  return (cnt == 0) ? 1 : 0;
+}
+
+/* Target to_stopped_by_watchpoint implementation.  Return 1 if
+   stopped by watchpoint.  The watchhi R and W bits indicate the watch
+   register triggered. */
+
+static int
+mips_linux_stopped_by_watchpoint (void)
+{
+  int n;
+  int num_valid;
+
+  if (!mips_linux_read_watch_registers (1))
+    return 0;
+
+  num_valid = get_num_valid (&watch_readback);
+
+  for (n = 0; n < MAX_DEBUG_REGISTER && n < num_valid; n++)
+    if (get_watchhi (&watch_readback, n) & (R_MASK | W_MASK))
+      return 1;
+
+  return 0;
+}
+
+/* Target to_stopped_data_address implementation.  Set the address
+   where the watch triggered (if known).  Return 1 if the address was
+   known.  */
+
+static int
+mips_linux_stopped_data_address (struct target_ops *t, CORE_ADDR *paddr)
+{
+  /* On mips we don't know the low order 3 bits of the data address,
+     so we must return false.  */
+  return 0;
+}
+
+/* Set any low order bits in mask that are not set.  */
+
+static CORE_ADDR
+fill_mask (CORE_ADDR mask)
+{
+  CORE_ADDR f = 1;
+  while (f && f < mask)
+    {
+      mask |= f;
+      f <<= 1;
+    }
+  return mask;
+}
+
+/* Try to add a single watch to the specified registers.  Return 1 on
+   success, 0 on failure.  */
+
+static int
+try_one_watch (struct pt_watch_regs *regs, CORE_ADDR addr,
+	       int len, unsigned irw)
+{
+  CORE_ADDR base_addr, last_byte, break_addr, segment_len;
+  CORE_ADDR mask_bits, t_low, t_low_end;
+  uint16_t t_hi;
+  int i, free_watches;
+  struct pt_watch_regs regs_copy;
+
+  if (len <= 0)
+    return 0;
+
+  last_byte = addr + len - 1;
+  mask_bits = fill_mask (addr ^ last_byte) | IRW_MASK;
+  base_addr = addr & ~mask_bits;
+
+  /* Check to see if it is covered by current registers.  */
+  for (i = 0; i < get_num_valid (regs); i++)
+    {
+      t_low = get_watchlo (regs, i);
+      if (t_low != 0 && irw == ((unsigned)t_low & irw))
+	{
+	  t_hi = get_watchhi (regs, i) | IRW_MASK;
+	  t_low &= ~(CORE_ADDR)t_hi;
+	  if (addr >= t_low && last_byte <= (t_low + t_hi))
+	    return 1;
+	}
+    }
+  /* Try to find an empty register.  */
+  free_watches = 0;
+  for (i = 0; i < get_num_valid (regs); i++)
+    {
+      t_low = get_watchlo (regs, i);
+      if (t_low == 0 && irw == (get_irw_mask (regs, i) & irw))
+	{
+	  if (mask_bits <= (get_reg_mask (regs, i) | IRW_MASK))
+	    {
+	      /* It fits, we'll take it.  */
+	      set_watchlo (regs, i, base_addr | irw);
+	      set_watchhi (regs, i, mask_bits & ~IRW_MASK);
+	      return 1;
+	    }
+	  else
+	    {
+	      /* It doesn't fit, but has the proper IRW capabilities.  */
+	      free_watches++;
+	    }
+	}
+    }
+  if (free_watches > 1)
+    {
+      /* Try to split it across several registers.  */
+      regs_copy = *regs;
+      for (i = 0; i < get_num_valid (&regs_copy); i++)
+	{
+	  t_low = get_watchlo (&regs_copy, i);
+	  t_hi = get_reg_mask (&regs_copy, i) | IRW_MASK;
+	  if (t_low == 0 && irw == (t_hi & irw))
+	    {
+	      t_low = addr & ~(CORE_ADDR)t_hi;
+	      break_addr = t_low + t_hi + 1;
+	      if (break_addr >= addr + len)
+		segment_len = len;
+	      else
+		segment_len = break_addr - addr;
+	      mask_bits = fill_mask (addr ^ (addr + segment_len - 1));
+	      set_watchlo (&regs_copy, i, (addr & ~mask_bits) | irw);
+	      set_watchhi (&regs_copy, i, mask_bits & ~IRW_MASK);
+	      if (break_addr >= addr + len)
+		{
+		  *regs = regs_copy;
+		  return 1;
+		}
+	      len = addr + len - break_addr;
+	      addr = break_addr;
+	    }
+	}
+    }
+  /* It didn't fit anywhere, we failed.  */
+  return 0;
+}
+
+/* Target to_region_ok_for_hw_watchpoint implementation.  Return 1 if
+   the specified region can be covered by the watch registers.  */
+
+static int
+mips_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
+{
+  struct pt_watch_regs dummy_regs;
+  int i;
+
+  if (!mips_linux_read_watch_registers (0))
+    return 0;
+
+  dummy_regs = watch_readback;
+  /* Clear them out.  */
+  for (i = 0; i < get_num_valid (&dummy_regs); i++)
+    set_watchlo (&dummy_regs, i, 0);
+  return try_one_watch (&dummy_regs, addr, len, 0);
+}
+
+
+/* Write the mirrored watch register values for each thread.  */
+
+static int
+write_watchpoint_regs (void)
+{
+  struct lwp_info *lp;
+  ptid_t ptid;
+  int tid;
+
+  ALL_LWPS (lp, ptid)
+    {
+      tid = ptid_get_lwp (ptid);
+      if (ptrace (PTRACE_SET_WATCH_REGS, tid, &watch_mirror) == -1)
+	perror_with_name (_("Couldn't write debug register"));
+    }
+  return 0;
+}
+
+/* linux_nat new_thread implementation.  Write the mirrored watch
+ register values for the new thread.  */
+
+static void
+mips_linux_new_thread (ptid_t ptid)
+{
+  int tid;
+
+  if (!mips_linux_read_watch_registers (0))
+    return;
+
+  tid = ptid_get_lwp (ptid);
+  if (ptrace (PTRACE_SET_WATCH_REGS, tid, &watch_mirror) == -1)
+    perror_with_name (_("Couldn't write debug register"));
+}
+
+/* Fill in the watch registers with the currently cached watches.  */
+
+static void
+populate_regs_from_watches (struct pt_watch_regs *regs)
+{
+  struct mips_watchpoint *w;
+  int i;
+
+  /* Clear them out.  */
+  for (i = 0; i < get_num_valid (regs); i++)
+    {
+      set_watchlo (regs, i, 0);
+      set_watchhi (regs, i, 0);
+    }
+
+  w = current_watches;
+  while (w)
+    {
+      i = try_one_watch (regs, w->addr, w->len, type_to_irw (w->type));
+      /* They must all fit, because we previously calculated that they
+	 would.  */
+      gdb_assert (i);
+      w = w->next;
+    }
+}
+
+/* Target to_insert_watchpoint implementation.  Try to insert a new
+   watch.  Return zero on success.  */
+
+static int
+mips_linux_insert_watchpoint (CORE_ADDR addr, int len, int type)
+{
+  struct pt_watch_regs regs;
+  struct mips_watchpoint *new_watch;
+  struct mips_watchpoint **pw;
+
+  int i;
+  int retval;
+
+  if (!mips_linux_read_watch_registers (0))
+    return -1;
+
+  if (len <= 0)
+    return -1;
+
+  regs = watch_readback;
+  /* Add the current watches.  */
+  populate_regs_from_watches (&regs);
+
+  /* Now try to add the new watch.  */
+  if (!try_one_watch (&regs, addr, len, type_to_irw (type)))
+    return -1;
+
+  /* It fit.  Stick it on the end of the list.  */
+  new_watch = (struct mips_watchpoint *)
+    xmalloc (sizeof (struct mips_watchpoint));
+  new_watch->addr = addr;
+  new_watch->len = len;
+  new_watch->type = type;
+  new_watch->next = NULL;
+
+  pw = &current_watches;
+  while (*pw != NULL)
+    pw = &(*pw)->next;
+  *pw = new_watch;
+
+  watch_mirror = regs;
+  retval = write_watchpoint_regs ();
+
+  if (maint_show_dr)
+    mips_show_dr ("insert_watchpoint", addr, len, type);
+
+  return retval;
+}
+
+/* Target to_remove_watchpoint implementation.  Try to remove a watch.
+   Return zero on success.  */
+
+static int
+mips_linux_remove_watchpoint (CORE_ADDR addr, int len, int type)
+{
+  int retval;
+  int deleted_one;
+
+  struct mips_watchpoint **pw;
+  struct mips_watchpoint *w;
+
+  /* Search for a known watch that matches.  Then unlink and free
+     it.  */
+  deleted_one = 0;
+  pw = &current_watches;
+  while ((w = *pw))
+    {
+      if (w->addr == addr && w->len == len && w->type == type)
+	{
+	  *pw = w->next;
+	  xfree (w);
+	  deleted_one = 1;
+	  break;
+	}
+      pw = &(w->next);
+    }
+
+  if (!deleted_one)
+    return -1;  /* We don't know about it, fail doing nothing.  */
+
+  /* At this point watch_readback is known to be valid because we
+     could not have added the watch without reading it.  */
+  gdb_assert (watch_readback_valid == 1);
+
+  watch_mirror = watch_readback;
+  populate_regs_from_watches (&watch_mirror);
+
+  retval = write_watchpoint_regs ();
+
+  if (maint_show_dr)
+    mips_show_dr ("remove_watchpoint", addr, len, type);
+
+  return retval;
+}
+
+/* Target to_close implementation.  Free any watches and call the
+   super implementation.  */
+
+static void
+mips_linux_close (int quitting)
+{
+  struct mips_watchpoint *w;
+  struct mips_watchpoint *nw;
+
+  /* Clean out the current_watches list.  */
+  w = current_watches;
+  while (w)
+    {
+      nw = w->next;
+      xfree (w);
+      w = nw;
+    }
+  current_watches = NULL;
+
+  if (super_close)
+    super_close (quitting);
+}
+
 void _initialize_mips_linux_nat (void);
 
 void
 _initialize_mips_linux_nat (void)
 {
-  struct target_ops *t = linux_trad_target (mips_linux_register_u_offset);
+  struct target_ops *t;
+
+  deprecated_add_set_cmd ("show-debug-regs", class_maintenance,
+			  var_boolean, (char *) &maint_show_dr, _("\
+Set whether to show variables that mirror the mips debug registers.\n\
+Use \"on\" to enable, \"off\" to disable.\n\
+If enabled, the debug registers values are shown when GDB inserts\n\
+or removes a hardware breakpoint or watchpoint, and when the inferior\n\
+triggers a breakpoint or watchpoint."),
+			  &maintenancelist);
+
+
+  t = linux_trad_target (mips_linux_register_u_offset);
+
+  super_close = t->to_close;
+  t->to_close = mips_linux_close;
 
   super_fetch_registers = t->to_fetch_registers;
   super_store_registers = t->to_store_registers;
@@ -370,9 +1066,17 @@ _initialize_mips_linux_nat (void)
   t->to_fetch_registers = mips64_linux_fetch_registers;
   t->to_store_registers = mips64_linux_store_registers;
 
+  t->to_can_use_hw_breakpoint = mips_linux_can_use_hw_breakpoint;
+  t->to_remove_watchpoint = mips_linux_remove_watchpoint;
+  t->to_insert_watchpoint = mips_linux_insert_watchpoint;
+  t->to_stopped_by_watchpoint = mips_linux_stopped_by_watchpoint;
+  t->to_stopped_data_address = mips_linux_stopped_data_address;
+  t->to_region_ok_for_hw_watchpoint = mips_linux_region_ok_for_hw_watchpoint;
+
   t->to_read_description = mips_linux_read_description;
 
   linux_nat_add_target (t);
+  linux_nat_set_new_thread (t, mips_linux_new_thread);
 
   /* Initialize the standard target descriptions.  */
   initialize_tdesc_mips_linux ();
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.579
diff -u -p -r1.579 gdb.texinfo
--- doc/gdb.texinfo	9 Apr 2009 20:11:57 -0000	1.579
+++ doc/gdb.texinfo	16 Apr 2009 16:34:59 -0000
@@ -25715,9 +25715,9 @@ Configuring with @samp{--enable-profilin
 compiled with the @samp{-pg} compiler option.
 
 @kindex maint show-debug-regs
-@cindex x86 hardware debug registers
+@cindex hardware debug registers
 @item maint show-debug-regs
-Control whether to show variables that mirror the x86 hardware debug
+Control whether to show variables that mirror the hardware debug
 registers.  Use @code{ON} to enable, @code{OFF} to disable.  If
 enabled, the debug registers values are shown when @value{GDBN} inserts or
 removes a hardware breakpoint or watchpoint, and when the inferior