From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16101 invoked by alias); 23 Jun 2006 00:44:47 -0000 Received: (qmail 16093 invoked by uid 22791); 23 Jun 2006 00:44:46 -0000 X-Spam-Check-By: sourceware.org Received: from ausmtp05.au.ibm.com (HELO ausmtp05.au.ibm.com) (202.81.18.154) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 23 Jun 2006 00:44:43 +0000 Received: from sd0208e0.au.ibm.com (d23rh904.au.ibm.com [202.81.18.202]) by ausmtp05.au.ibm.com (8.13.6/8.13.6) with ESMTP id k5N0le338486984 for ; Fri, 23 Jun 2006 10:47:40 +1000 Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.250.237]) by sd0208e0.au.ibm.com (8.12.10/NCO/VER6.8) with ESMTP id k5N0m0Ou201046 for ; Fri, 23 Jun 2006 10:48:01 +1000 Received: from d23av04.au.ibm.com (loopback [127.0.0.1]) by d23av04.au.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id k5N0idjQ010654 for ; Fri, 23 Jun 2006 10:44:39 +1000 Received: from [9.47.22.62] (dyn9047022062.beaverton.ibm.com [9.47.22.62]) by d23av04.au.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id k5N0ibqj010617 for ; Fri, 23 Jun 2006 10:44:38 +1000 Date: Fri, 23 Jun 2006 00:44:00 -0000 From: Wu Zhou To: gdb-patches@sourceware.org Subject: [RFC]: h/w watchpoint (r,w,a) for ppc440 In-Reply-To: Message-ID: References: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-06/txt/msg00344.txt.bz2 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 > > * 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 >