From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25572 invoked by alias); 6 Dec 2005 06:09:04 -0000 Received: (qmail 25563 invoked by uid 22791); 6 Dec 2005 06:09:03 -0000 X-Spam-Check-By: sourceware.org Received: from ausmtp03.au.ibm.com (HELO ausmtp03.au.ibm.com) (202.81.18.151) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 06 Dec 2005 06:08:59 +0000 Received: from sd0112e0.au.ibm.com (d23rh903.au.ibm.com [202.81.18.201]) by ausmtp03.au.ibm.com (8.12.10/8.12.10) with ESMTP id jB66BkK2084204 for ; Tue, 6 Dec 2005 17:11:47 +1100 Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.250.244]) by sd0112e0.au.ibm.com (8.12.10/NCO/VERS6.8) with ESMTP id jB66BYlL149830 for ; Tue, 6 Dec 2005 17:11:34 +1100 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.12.11/8.13.3) with ESMTP id jB668VWj016462 for ; Tue, 6 Dec 2005 17:08:31 +1100 Received: from [9.181.133.252] ([9.181.133.252]) by d23av03.au.ibm.com (8.12.11/8.12.11) with ESMTP id jB668Ohi016259; Tue, 6 Dec 2005 17:08:26 +1100 Date: Tue, 06 Dec 2005 19:54:00 -0000 From: Wu Zhou To: gdb-patches@sources.redhat.com cc: drow@false.org, eliz@gnu.org, mark.kettenis@xs4all.nl Subject: [RFC] GDB patches for hw watchpoints - revised Message-ID: 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: 2005-12/txt/msg00109.txt.bz2 Hello, maintainers I am taking over Manoj's work of submitting Ben Elliston's ppc64 h/w watchpoint patch. When Manoj submitted that patch a few monthes ago, some of you (Daniel, Eli and Mark...) expressed some concerns about that patch. We are now making some changes to that patch, and wish that this could address (or partially address) your concerns. Appended is the revised patch. And here is some explanation about it. 1. Daniel ever express the concern that some PPC variations don't have any DABR. Yes, that is true. So I added a runtime test to check this. If the arch don't have DABR or that the kernel don't support PTRACE_SET_DEBUGREG, then ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) will return -1. We will return 0 for to_can_use_hardware_watchpoint. See the revised implementation of ppc_linux_check_watch_resources 2. PTRACE_SET_DEBUGREG and PTRACE_GETSIGINFO for ppc64 is now in kernel 2.6.14.3 from kernel.org. You can also check http://patchwork.ozlabs.org/linuxppc64/patch?id=2341 and related thread on the patch. 3. Eli ever expressed a concern that the PPC doesn't have a way to return the data address that triggered the watchpoint? As far as I think, the reason is that PPC will only have one DABR (if it does have). So maybe we don't need to have such a method. 4. Mark said that we can't add anything more to nm.h. The revised patch changed that and add most of them to target vector. But we had to find a way to set TARGET_REGION_OK_FOR_HW_WATCHPOINT for ppc64 arch. I am now setting that in nm-ppc64-linux.h. Is that okay? Maybe we need to set TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT instead? 5. I had built a 2.6.14.3 kernel and test the following patch on a P630 box. It works. Test with gdb.base/recurse.exp report 12 PASS and 7 FAIL, which is better than 6 PASS and 13 FAIL with un-patched GDB. What is more, The 7 failure is around the second watchpoint, which is a software one. So I am believing that this is a progress. I also tried gdb.base/watchpoint.exp, no regression is found. The third test I made is against Paul's testcase to verify "rs6000_in_function_epilogue_p". Hardware watchpoint didn't encounter the problem software watchpoint met. Any suggestion for more test to be taken? Thanks in advance. Here goes the patch. Please review and comment. diff -cpr gdb-6.3.90/gdb/config/powerpc/nm-ppc64-linux.h gdb-6.3.90.new/gdb/config/powerpc/nm-ppc64-linux.h *** gdb-6.3.90/gdb/config/powerpc/nm-ppc64-linux.h 2003-06-12 19:30:39.000000000 -0400 --- gdb-6.3.90.new/gdb/config/powerpc/nm-ppc64-linux.h 2005-11-30 18:13:04.000000000 -0500 *************** Foundation, Inc., 675 Mass Ave, Cambridg *** 24,27 **** --- 24,31 ---- #define PTRACE_ARG3_TYPE void * #define PTRACE_XFER_TYPE long + /* Return true for region ok for hardware watchpoint. */ + + #define TARGET_REGION_OK_FOR_HW_WATCHPOINT(addr, len) 1 + #endif /* NM_PPC64_LINUX_H */ diff -cpr gdb-6.3.90/gdb/ppc-linux-nat.c gdb-6.3.90.new/gdb/ppc-linux-nat.c *** gdb-6.3.90/gdb/ppc-linux-nat.c 2005-09-10 14:11:04.000000000 -0400 --- gdb-6.3.90.new/gdb/ppc-linux-nat.c 2005-12-06 18:36:07.000000000 -0500 *************** *** 81,86 **** --- 81,96 ---- #define PTRACE_SETEVRREGS 21 #endif + /* Similarly for the hardware watchpoint support. */ + #ifndef PTRACE_GET_DEBUGREG + #define PTRACE_GET_DEBUGREG 25 + #endif + #ifndef PTRACE_SET_DEBUGREG + #define PTRACE_SET_DEBUGREG 26 + #endif + #ifndef PTRACE_GETSIGINFO + #define PTRACE_GETSIGINFO 0x4202 + #endif /* This oddity is because the Linux kernel defines elf_vrregset_t as an array of 33 16 bytes long elements. I.e. it leaves out vrsave. *************** store_ppc_registers (int tid) *** 777,782 **** --- 787,890 ---- store_spe_register (tid, -1); } + int + ppc_linux_check_watch_resources (int type, int cnt, int ot) + { + int tid; + ptid_t ptid = inferior_ptid; + + /* DABR (data address breakpoint register) is optional for PPC variations. + Some variation have one DABR, others have none. So CNT can't be larger + than 1. */ + if (cnt > 1) + return 0; + + /* We need to know whether ptrace syscall support PTRACE_SET_DEBUGREG and + whether the ppc arch have DABR. If either answer is no, the ptrace call + will return -1. Return 0 for that. */ + tid = TIDGET (ptid); + if (tid == 0) + tid = PIDGET (ptid); + + if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1) + return 0; + return 1; + } + + /* Set a watchpoint of type TYPE at address ADDR. */ + long + ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw) + { + int tid; + long dabr_value; + ptid_t ptid = inferior_ptid; + + /* Handle sub-8-byte quantities. */ + if (len <= 0) + return -1; + + /* addr+len must fall in the 8 byte watchable region. */ + if ((addr + len) > (addr & ~7) + 8) + return -1; + + 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; + } + + tid = TIDGET (ptid); + if (tid == 0) + tid = PIDGET (ptid); + + return ptrace (PTRACE_SET_DEBUGREG, tid, 0, dabr_value); + } + + long + ppc_linux_remove_watchpoint (CORE_ADDR addr, int len) + { + int tid; + ptid_t ptid = inferior_ptid; + + tid = TIDGET (ptid); + if (tid == 0) + tid = PIDGET (ptid); + + return ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0); + } + + int + ppc_linux_stopped_by_watchpoint (void) + { + int tid; + struct siginfo siginfo; + ptid_t ptid = inferior_ptid; + + tid = TIDGET(ptid); + if (tid == 0) + tid = PIDGET (ptid); + + errno = 0; + ptrace (PTRACE_GETSIGINFO, tid, (PTRACE_TYPE_ARG3) 0, &siginfo); + + if (errno != 0 || siginfo.si_signo != SIGTRAP || + (siginfo.si_code & 0xffff) != 0x0004) + return 0; + + return 1; + } + static void ppc_linux_store_inferior_registers (int regno) { *************** _initialize_ppc_linux_nat (void) *** 897,904 **** --- 1005,1017 ---- t = linux_target (); /* Add our register access methods. */ + t->to_can_use_hw_breakpoint = ppc_linux_check_watch_resources; t->to_fetch_registers = ppc_linux_fetch_inferior_registers; + t->to_insert_watchpoint = ppc_linux_insert_watchpoint; + t->to_remove_watchpoint = ppc_linux_remove_watchpoint; t->to_store_registers = ppc_linux_store_inferior_registers; + t->to_stopped_by_watchpoint = ppc_linux_stopped_by_watchpoint; + /* Register the target. */ add_target (t); diff -cpr gdb-6.3.90/gdb/rs6000-tdep.c gdb-6.3.90.new/gdb/rs6000-tdep.c *** gdb-6.3.90/gdb/rs6000-tdep.c 2005-10-05 20:22:57.000000000 -0400 --- gdb-6.3.90.new/gdb/rs6000-tdep.c 2005-12-06 20:24:43.000000000 -0500 *************** rs6000_gdbarch_init (struct gdbarch_info *** 3277,3282 **** --- 3277,3290 ---- _("rs6000_gdbarch_init: " "received unexpected BFD 'arch' value")); + /* If the MACH is p630, set have_nonsteppable_watchpoint to 1. + + FIXME: not quite sure if all ppc64 mach support nonsteppable watchpoint. + But p630 do support nonsteppable h/w watchpoint. */ + + if (bfd_mach_ppc_630) + set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 1); + /* Sanity check on registers. */ gdb_assert (strcmp (tdep->regs[tdep->ppc_gp0_regnum].name, "r0") == 0); Regards - Wu Zhou