[RFC]: h/w watchpoint (r,w,a) for ppc440

[Wu Zhou <woodzltc at cn dot ibm dot com>]

I am also working on providing basic h/w watchpoint functionality to 
ppc440 chip.  It is requested that read/write/access watchpoints are all 
supported.  The bug talked about below is also found in this process. So I 
choose to merge them together.

Appended is the patch.  Please note that I am now using /proc/cpuinfo to 
know what chip we are running on.  I see that some code in gdb are using 
bfd_mach to differentiate between various chips in one cpu family.  But 
there is no ppc_440 (we have ppc_403, ppc_505, ppc_603 and so on) for 
bfd_mach yet.  So if you prefer this kind of consistent mechanism, we 
might need to convince bfd maintainers to get out a bfd_mach_ppc_440 
first. 

I had tested this on ppc440 and power4 machine with the testcase posted at 
http://sources.redhat.com/ml/gdb-patches/2006-06/msg00169.html, all 44 
tests pass.  Please review and comment.  Thanks.

diff -uNr src/gdb/ppc-linux-nat.c src.440/gdb/ppc-linux-nat.c
--- src/gdb/ppc-linux-nat.c	2006-04-11 03:53:24.000000000 +0800
+++ src.440/gdb/ppc-linux-nat.c	2006-06-22 08:35:20.947752072 +0800
@@ -796,16 +796,20 @@
   /* DABR (data address breakpoint register) is optional for PPC variants.
      Some variants have one DABR, others have none.  So CNT can't be larger
      than 1.  */
+  /* XXX: There are two watchpoint registers in ppc440.  But currrent kernel
+     only support one.  To support two watchpoint registers, both the kernel 
+     and gdb code need to make some change.  */
   if (cnt > 1)
     return 0;
 
-  /* We need to know whether ptrace supports PTRACE_SET_DEBUGREG and whether
-     the target has DABR.  If either answer is no, the ptrace call will
-     return -1.  Fail in that case.  */
   tid = TIDGET (ptid);
   if (tid == 0)
     tid = PIDGET (ptid);
 
+  /* We need to know whether ptrace supports PTRACE_SET_DEBUGREG and whether
+     the target has DABR or DAC (ppc440 uses DAC register to store watchpoint
+     address).  If either answer is no, the ptrace call will return -1.  Fail
+     in that case.  */
   if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
     return 0;
   return 1;
@@ -825,6 +829,37 @@
   return 1;
 }
 
+#define MAXLINE 256
+
+static char *
+get_cpu_name ()
+{
+  char *cpuinfo = "/proc/cpuinfo";
+  FILE *fp = fopen (cpuinfo, "r");
+  char *buf = malloc (MAXLINE);
+  char *saved;
+
+  if (fp == NULL)
+    {
+      perror ("file open failed:");
+      return (NULL);
+    }
+
+  while (fgets (buf, MAXLINE, fp) != NULL)
+    {
+      if (strstr (buf, "cpu") != NULL)
+        {
+//          printf ("cpu hit: %s\n", buf);
+          char *next = strtok_r (buf, ": ", &saved);
+          fclose (fp);
+          return (saved);
+        }
+    }
+
+  fclose (fp);
+  return (NULL);
+}
+
 /* Set a watchpoint of type TYPE at address ADDR.  */
 static int
 ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
@@ -833,22 +868,10 @@
   long dabr_value;
   ptid_t ptid = inferior_ptid;
 
-  dabr_value = addr & ~7;
-  switch (rw)
-    {
-    case hw_read:
-      /* Set read and translate bits.  */
-      dabr_value |= 5;
-      break;
-    case hw_write:
-      /* Set write and translate bits.  */
-      dabr_value |= 6;
-      break;
-    case hw_access:
-      /* Set read, write and translate bits.  */
-      dabr_value |= 7;
-      break;
-    }
+  if (strstr (get_cpu_name (), "440GP") != NULL)
+    dabr_value = addr | 3;
+  else
+    dabr_value = addr | 7;
 
   tid = TIDGET (ptid);
   if (tid == 0)


Regards
- Wu Zhou

On Fri, 9 Jun 2006, Wu Zhou wrote:

> Hello all,
> 
> I found a bug in the current ppc-linux h/w watchpoint implementation:  
> when we set read watchpoint to some expression, if there are any write 
> operation to it before a read operation is hit, watchpoint_check will see 
> that its value is changed. So user won't see the watchpoint is hit.
> 
> I make one change to the SET_DEBUGREG operation: even if it is only 
> read or write watchpoint, we still set access flag.  Then, no matter 
> what operation is on the watched address, a SIGTRAP will be triggered. 
> The gdb code itself can determine if it is a write operation or read 
> operation.  If it is write, watchpoint_check routine can update the 
> bs->value to the latest.  
> 
> Here is the patch.  Thanks for reviewing.
> 
> 2006-06-09  Wu Zhou  <woodzltc@cn.ibm.com>
> 
> 	* ppc-linux-nat.c (ppc_linux_insert_watchpoint): Set access flag for
> 	all hardware watchpoint.
> 
> --- ppc-linux-nat.c.orig	2006-06-09 14:53:35.000000000 +0800
> +++ ppc-linux-nat.c	2006-06-09 15:04:12.000000000 +0800
> @@ -821,22 +821,7 @@ ppc_linux_insert_watchpoint (CORE_ADDR a
>    long dabr_value;
>    ptid_t ptid = inferior_ptid;
>  
> -  dabr_value = addr & ~7;
> -  switch (rw)
> -    {
> -    case hw_read:
> -      /* Set read and translate bits.  */
> -      dabr_value |= 5;
> -      break;
> -    case hw_write:
> -      /* Set write and translate bits.  */
> -      dabr_value |= 6;
> -      break;
> -    case hw_access:
> -      /* Set read, write and translate bits.  */
> -      dabr_value |= 7;
> -      break;
> -    }
> +  dabr_value = addr | 7;
>  
>    tid = TIDGET (ptid);
>    if (tid == 0)
> 
> 
> :ADDPATCH ppc:
> 
> Regards
> - Wu Zhou
>