Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH 1/4] Support the new BookE ptrace interface
@ 2009-12-24  0:31 Thiago Jung Bauermann
  2009-12-29  6:12 ` Joel Brobecker
  2010-01-04 17:33 ` Thiago Jung Bauermann
  0 siblings, 2 replies; 14+ messages in thread
From: Thiago Jung Bauermann @ 2009-12-24  0:31 UTC (permalink / raw)
  To: gdb-patches; +Cc: Luis Machado, Matt Tyrlik

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

This patch adapts ppc-linux-nat.c to the new interface, without changing
anything else. As a bonus, with the new interface it supports hardware 
breakpoints.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


2009-12-23  Sergio Durigan Junior  <sergiodj@linux.vnet.ibm.com>
	    Thiago Jung Bauermann  <bauerman@br.ibm.com>

	* ppc-linux-nat.c (PTRACE_GET_DEBUGREG): Update comment.
	(PPC_PTRACE_GETWDBGINFO, PPC_PTRACE_SETHWDEBUG, PPC_PTRACE_DELHWDEBUG,
	ppc_debug_info, PPC_DEBUG_FEATURE_INSN_BP_RANGE,
	PPC_DEBUG_FEATURE_INSN_BP_MASK, PPC_DEBUG_FEATURE_DATA_BP_RANGE,
	PPC_DEBUG_FEATURE_DATA_BP_MASK, ppc_hw_breakpoint,
	PPC_BREAKPOINT_TRIGGER_EXECUTE, PPC_BREAKPOINT_TRIGGER READ,
	PPC_BREAKPOINT_TRIGGER_WRITE, PPC_BREAKPOINT_TRIGGER_RW,
	PPC_BREAKPOINT_MODE_EXACT PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE,
	PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE, PPC_BREAKPOINT_MODE_MASK,
	PPC_BREAKPOINT_CONDITION_NONE, PPC_BREAKPOINT_CONDITION_AND,
	PPC_BREAKPOINT_CONDITION_EXACT, PPC_BREAKPOINT_CONDITION_OR,
	PPC_BREAKPOINT_CONDITION_AND_OR, PPC_BREAKPOINT_CONDITION_BE_ALL,
	PPC_BREAKPOINT_CONDITION_BE_SHIFT, PPC_BREAKPOINT_CONDITION_BE):
	Define, in case <ptrace.h> doesn't provide it.
	(have_ptrace_new_debug_booke): New flag.
	(ppc_linux_check_watch_resources): Renamed to ...
	(ppc_linux_can_use_hw_breakpoint): ... this.  Handle BookE processors.
	(booke_debug_info): New variable.
	(max_slots_number): Ditto.
	(struct hw_break_tuple): New struct.
	(struct thread_points): Ditto.
	(ppc_threads): New variable.
	(PPC_DEBUG_CURRENT_VERSION): New define.
	(ppc_linux_region_ok_for_hw_watchpoint): Handle BookE processors.
	(booke_cmp_hw_point): New function.
	(booke_find_thread_points_by_tid): Ditto.
	(booke_insert_point): Ditto.
	(booke_remove_point): Ditto.
	(ppc_linux_insert_hw_breakpoint): Ditto.
	(ppc_linux_remove_hw_breakpoint): Ditto.
	(HW_WATCH_RW_TRIGGER): New define.
	(ppc_linux_insert_watchpoint): Handle BookE processors.
	(ppc_linux_remove_watchpoint): Ditto.
	(ppc_linux_new_thread): Ditto.
	(ppc_linux_stopped_data_address): Ditto.
	(ppc_linux_watchpoint_addr_within_range): Ditto.
	with and without the new kernel interface.
	(ppc_linux_read_description): Query the target to know if it
	supports the new kernel BookE interface.
	(_initialize_ppc_linux_nat): Initialize to_insert_hw_breakpoint and
	to_remove_hw_breakpoint fields of the target operations struct.


[-- Attachment #2: ppc476-new-interface.diff --]
[-- Type: text/x-patch, Size: 21945 bytes --]

Index: gdb/gdb/ppc-linux-nat.c
===================================================================
--- gdb.orig/gdb/ppc-linux-nat.c	2009-12-23 13:36:14.000000000 -0200
+++ gdb/gdb/ppc-linux-nat.c	2009-12-23 15:15:13.000000000 -0200
@@ -100,7 +100,8 @@
 #define PTRACE_SETEVRREGS 21
 #endif
 
-/* Similarly for the hardware watchpoint support.  */
+/* Similarly for the hardware watchpoint support.  These requests are used
+   when the BookE kernel interface is not available.  */
 #ifndef PTRACE_GET_DEBUGREG
 #define PTRACE_GET_DEBUGREG    25
 #endif
@@ -111,6 +112,79 @@
 #define PTRACE_GETSIGINFO    0x4202
 #endif
 
+/* These requests are used when the BookE kernel interface is available.
+   It exposes the additional debug features of BookE processors, such as
+   ranged breakpoints and watchpoints and hardware-accelerated condition
+   evaluation.  */
+#ifndef PPC_PTRACE_GETHWDBGINFO
+
+/* Not having PPC_PTRACE_GETHWDBGINFO defined means that the new BookE
+   interface is not present in ptrace.h, so we'll have to pretty much include
+   it here so that the code at least compiles on older systems.  */
+#define PPC_PTRACE_GETHWDBGINFO 0x89
+#define PPC_PTRACE_SETHWDEBUG   0x88
+#define PPC_PTRACE_DELHWDEBUG   0x87
+
+struct ppc_debug_info {
+        uint32_t version;               /* Only version 1 exists to date */
+        uint32_t num_instruction_bps;
+        uint32_t num_data_bps;
+        uint32_t num_condition_regs;
+        uint32_t data_bp_alignment;
+        uint32_t sizeof_condition;      /* size of the DVC register */
+        uint64_t features;
+};
+
+/*
+ * features will have bits indication whether there is support for:
+ */
+#define PPC_DEBUG_FEATURE_INSN_BP_RANGE         0x1
+#define PPC_DEBUG_FEATURE_INSN_BP_MASK          0x2
+#define PPC_DEBUG_FEATURE_DATA_BP_RANGE         0x4
+#define PPC_DEBUG_FEATURE_DATA_BP_MASK          0x8
+
+struct ppc_hw_breakpoint {
+        uint32_t version;               /* currently, version must be 1 */
+        uint32_t trigger_type;          /* only some combinations allowed */
+        uint32_t addr_mode;             /* address match mode */
+        uint32_t condition_mode;        /* break/watchpoint condition flags */
+        uint64_t addr;                  /* break/watchpoint address */
+        uint64_t addr2;                 /* range end or mask */
+        uint64_t condition_value;       /* contents of the DVC register */
+};
+
+/*
+ * Trigger Type
+ */
+#define PPC_BREAKPOINT_TRIGGER_EXECUTE  0x1
+#define PPC_BREAKPOINT_TRIGGER_READ     0x2
+#define PPC_BREAKPOINT_TRIGGER_WRITE    0x4
+#define PPC_BREAKPOINT_TRIGGER_RW       0x6
+
+/*
+ * Address Mode
+ */
+#define PPC_BREAKPOINT_MODE_EXACT               0x0
+#define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE     0x1
+#define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE     0x2
+#define PPC_BREAKPOINT_MODE_MASK                0x3
+
+/*
+ * Condition Mode
+ */
+#define PPC_BREAKPOINT_CONDITION_NONE   0x0
+#define PPC_BREAKPOINT_CONDITION_AND    0x1
+#define PPC_BREAKPOINT_CONDITION_EXACT  0x1
+#define PPC_BREAKPOINT_CONDITION_OR     0x2
+#define PPC_BREAKPOINT_CONDITION_AND_OR 0x3
+#define PPC_BREAKPOINT_CONDITION_BE_ALL 0x00ff0000
+#define PPC_BREAKPOINT_CONDITION_BE_SHIFT       16
+#define PPC_BREAKPOINT_CONDITION_BE(n)  \
+        (1<<((n)+PPC_BREAKPOINT_CONDITION_BE_SHIFT))
+#endif /* PPC_PTRACE_GETHWDBGINFO */
+
+
+
 /* Similarly for the general-purpose (gp0 -- gp31)
    and floating-point registers (fp0 -- fp31).  */
 #ifndef PTRACE_GETREGS
@@ -248,6 +322,10 @@ int have_ptrace_getsetregs = 1;
    them and gotten an error.  */
 int have_ptrace_getsetfpregs = 1;
 
+/* Non-zero if we support the new Kernel interface that enables
+   new BOOKE debugging resources.  */
+int have_ptrace_new_debug_booke = 1;
+
 /* *INDENT-OFF* */
 /* registers layout, as presented by the ptrace interface:
 PT_R0, PT_R1, PT_R2, PT_R3, PT_R4, PT_R5, PT_R6, PT_R7,
@@ -1270,30 +1348,6 @@ store_ppc_registers (const struct regcac
     store_spe_register (regcache, tid, -1);
 }
 
-static 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 variants.
-     Some variants 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 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);
-
-  if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
-    return 0;
-  return 1;
-}
-
 /* Fetch the AT_HWCAP entry from the aux vector.  */
 unsigned long ppc_linux_get_hwcap (void)
 {
@@ -1305,6 +1359,98 @@ unsigned long ppc_linux_get_hwcap (void)
   return 0;
 }
 
+/* The cached DABR value, to install in new threads.
+   This variable is used when we are dealing with non-BookE
+   processors.  */
+static long saved_dabr_value;
+
+/* Global structure that will store information about the available
+   features on this BookE processor.  */
+static struct ppc_debug_info booke_debug_info;
+
+/* Global variable that holds the maximum number of slots that the
+   kernel will use.  This is only used when the processor is BookE.  */
+static size_t max_slots_number = 0;
+
+struct hw_break_tuple {
+  long slot;
+  struct ppc_hw_breakpoint *hw_break;
+};
+
+/* This is an internal VEC created to store information about *points inserted
+   for each thread.  This is used for BookE processors.  */
+typedef struct thread_points
+  {
+    /* The TID to which this *point relates.  */
+    int tid;
+    /* Information about the *point, such as its address, type, etc.
+
+       Each element inside this vector corresponds to a hardware
+       breakpoint or watchpoint in the thread represented by TID.  The maximum
+       size of these vector is MAX_SLOTS_NUMBER.  If the hw_break element of
+       the tuple is NULL, then the position in the vector is free.  */
+    struct hw_break_tuple *hw_breaks;
+  } *thread_points_p;
+DEF_VEC_P (thread_points_p);
+
+VEC(thread_points_p) *ppc_threads = NULL;
+
+/* The version of the kernel interface that we will use if the processor is
+   BookE.  */
+#define PPC_DEBUG_CURRENT_VERSION 1
+
+static int
+ppc_linux_can_use_hw_breakpoint (int type, int cnt, int ot)
+{
+  int total_hw_wp, total_hw_bp;
+
+  if (have_ptrace_new_debug_booke)
+    {
+      /* For PPC BookE processors, the number of available hardware
+         watchpoints and breakpoints is stored at the booke_debug_info
+	 struct.  */
+      total_hw_bp = booke_debug_info.num_instruction_bps;
+      total_hw_wp = booke_debug_info.num_data_bps;
+    }
+  else
+    {
+      /* For PPC server processors, we accept 1 hardware watchpoint and 0
+	 hardware breakpoints.  */
+      total_hw_bp = 0;
+      total_hw_wp = 1;
+    }
+
+  if (type == bp_hardware_watchpoint || type == bp_read_watchpoint
+      || type == bp_access_watchpoint || type == bp_watchpoint)
+    {
+      if (cnt > total_hw_wp)
+	return -1;
+    }
+  else if (type == bp_hardware_breakpoint)
+    {
+      if (cnt > total_hw_bp)
+	return -1;
+    }
+
+  if (!have_ptrace_new_debug_booke)
+    {
+      ptid_t ptid;
+      int tid;
+
+      /* 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);
+
+      if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
+	return 0;
+    }
+
+  return 1;
+}
+
 static int
 ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
 {
@@ -1312,89 +1458,352 @@ ppc_linux_region_ok_for_hw_watchpoint (C
   if (len <= 0)
     return 0;
 
+  /* The new BookE ptrace interface tells if there are alignment restrictions
+     for watchpoints in the processors.  In that case, we use that information
+     to determine the hardcoded watchable region for watchpoints.  */
+  if (have_ptrace_new_debug_booke)
+    {
+      if (booke_debug_info.data_bp_alignment
+	  && (addr + len > (addr & ~(booke_debug_info.data_bp_alignment - 1))
+	      + booke_debug_info.data_bp_alignment))
+	  return 0;
+    }
   /* addr+len must fall in the 8 byte watchable region for DABR-based
-     processors.  DAC-based processors, like the PowerPC 440, will use
+     processors (i.e., server processors).  Without the new BookE ptrace
+     interface, DAC-based processors (i.e., embedded processors) will use
      addresses aligned to 4-bytes due to the way the read/write flags are
-     passed at the moment.  */
-  if (((ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
-      && (addr + len) > (addr & ~3) + 4)
-      || (addr + len) > (addr & ~7) + 8)
+     passed in the old ptrace interface.  */
+  else if (((ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+	   && (addr + len) > (addr & ~3) + 4)
+	   || (addr + len) > (addr & ~7) + 8)
     return 0;
 
   return 1;
 }
 
-/* The cached DABR value, to install in new threads.  */
-static long saved_dabr_value;
+/* This function compares two ppc_hw_breakpoint structs field-by-field.  */
+static inline int
+booke_cmp_hw_point (struct ppc_hw_breakpoint *a, struct ppc_hw_breakpoint *b)
+{
+  return (a->trigger_type       == b->trigger_type
+	  && a->addr_mode       == b->addr_mode
+	  && a->condition_mode  == b->condition_mode
+	  && a->addr            == b->addr
+	  && a->addr2           == b->addr2
+	  && a->condition_value == b->condition_value);
+}
+
+/* This function can be used to retrieve a thread_points by
+   the TID of the related process/thread.  If nothing has been
+   found, it returns NULL.  */
+static struct thread_points *
+booke_find_thread_points_by_tid (int tid, int alloc_new)
+{
+  int i;
+  struct thread_points *t;
+
+  for (i = 0; VEC_iterate (thread_points_p, ppc_threads, i, t); i++)
+    if (t->tid == tid)
+      return t;
+
+  t = NULL;
+
+  /* Do we need to allocate a new point_item
+     if the wanted one does not exist?  */
+  if (alloc_new)
+    {
+      t = xmalloc (sizeof (struct thread_points));
+      t->hw_breaks = xzalloc (max_slots_number * sizeof (struct hw_break_tuple));
+      t->tid = tid;
+      VEC_safe_push (thread_points_p, ppc_threads, t);
+    }
+
+  return t;
+}
+
+/* This function is a generic wrapper that is responsible for inserting a
+   *point (i.e., calling `ptrace' in order to issue the request to the
+   kernel) and registering it internally in GDB.  */
+static void
+booke_insert_point (struct ppc_hw_breakpoint *b, int tid)
+{
+  int i;
+  long slot;
+  struct ppc_hw_breakpoint *p = xmalloc (sizeof (struct ppc_hw_breakpoint));
+  struct hw_break_tuple *hw_breaks;
+  struct cleanup *c = make_cleanup (xfree, p);
+  struct thread_points *t;
+  struct hw_break_tuple *tuple;
+
+  memcpy (p, b, sizeof (struct ppc_hw_breakpoint));
+
+  errno = 0;
+  if ((slot = ptrace (PPC_PTRACE_SETHWDEBUG, tid, 0, p)) < 0)
+    perror_with_name (_("\n\
+Please review your watchpoints/breakpoints and make sure the mask, ranges \n\
+and addresses are within acceptable limits."));
+
+  /* Everything went fine, so we have to register this *point.  */
+  t = booke_find_thread_points_by_tid (tid, 1);
+  gdb_assert (t != NULL);
+  hw_breaks = t->hw_breaks;
+
+  /* Find a free element in the hw_breaks vector.  */
+  for (i = 0; i < max_slots_number; i++)
+    if (hw_breaks[i].hw_break == NULL)
+      {
+	hw_breaks[i].slot = slot;
+	hw_breaks[i].hw_break = p;
+	break;
+      }
+
+  gdb_assert (i != max_slots_number);
+
+  discard_cleanups (c);
+}
+
+/* This function is a generic wrapper that is responsible for removing a
+   *point (i.e., calling `ptrace' in order to issue the request to the
+   kernel), and unregistering it internally at GDB.  */
+static void
+booke_remove_point (struct ppc_hw_breakpoint *b, int tid)
+{
+  int i;
+  long slot;
+  struct hw_break_tuple *hw_breaks;
+  struct thread_points *t;
+
+  t = booke_find_thread_points_by_tid (tid, 0);
+  gdb_assert (t != NULL);
+  hw_breaks = t->hw_breaks;
+
+  for (i = 0; i < max_slots_number; i++)
+    if (hw_breaks[i].hw_break && booke_cmp_hw_point (hw_breaks[i].hw_break, b))
+      break;
+
+  gdb_assert (i != max_slots_number);
+
+  errno = 0;
+  if (ptrace (PPC_PTRACE_DELHWDEBUG, tid, 0, hw_breaks[i].slot) < 0)
+    perror_with_name (_("\n\
+Could not delete the breakpoint/watchpoint."));
+
+  xfree (hw_breaks[i].hw_break);
+  hw_breaks[i].hw_break = NULL;
+}
 
-/* Set a watchpoint of type TYPE at address ADDR.  */
 static int
-ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+ppc_linux_insert_hw_breakpoint (struct gdbarch *gdbarch,
+				  struct bp_target_info *bp_tgt)
 {
   struct lwp_info *lp;
   ptid_t ptid;
-  long dabr_value;
-  long read_mode, write_mode;
 
-  if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
-  {
-  /* PowerPC 440 requires only the read/write flags to be passed
-     to the kernel.  */
-    read_mode  = 1;
-    write_mode = 2;
-  }
-  else
-  {
-  /* PowerPC 970 and other DABR-based processors are required to pass
-     the Breakpoint Translation bit together with the flags.  */
-    read_mode  = 5;
-    write_mode = 6;
-  }
-
-  dabr_value = addr & ~(read_mode | write_mode);
-  switch (rw)
-    {
-    case hw_read:
-      /* Set read and translate bits.  */
-      dabr_value |= read_mode;
-      break;
-    case hw_write:
-      /* Set write and translate bits.  */
-      dabr_value |= write_mode;
-      break;
-    case hw_access:
-      /* Set read, write and translate bits.  */
-      dabr_value |= read_mode | write_mode;
-      break;
+  if (!have_ptrace_new_debug_booke)
+    return -1;
+
+  ALL_LWPS (lp, ptid)
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) bp_tgt->placed_address;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      booke_insert_point (&p, TIDGET (ptid));
     }
 
-  saved_dabr_value = dabr_value;
+  return 0;
+}
+
+static int
+ppc_linux_remove_hw_breakpoint (struct gdbarch *gdbarch,
+				  struct bp_target_info *bp_tgt)
+{
+  struct lwp_info *lp;
+  ptid_t ptid;
+
+  if (!have_ptrace_new_debug_booke)
+    return -1;
 
   ALL_LWPS (lp, ptid)
-    if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
-      return -1;
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) bp_tgt->placed_address;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      booke_remove_point (&p, TIDGET (ptid));
+    }
 
   return 0;
 }
 
+#define HW_WATCH_RW_TRIGGER(t, rw) \
+  if (rw == hw_read) \
+    t = PPC_BREAKPOINT_TRIGGER_READ; \
+  else if (rw == hw_write) \
+    t = PPC_BREAKPOINT_TRIGGER_WRITE; \
+  else \
+    t = PPC_BREAKPOINT_TRIGGER_READ | PPC_BREAKPOINT_TRIGGER_WRITE;
+
+static int
+ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+{
+  struct lwp_info *lp;
+  ptid_t ptid;
+  int ret = -1;
+
+  if (have_ptrace_new_debug_booke)
+    {
+      if (len <= 4)
+	ALL_LWPS (lp, ptid)
+	  {
+	    struct ppc_hw_breakpoint p;
+	    uint32_t trigger;
+
+	    HW_WATCH_RW_TRIGGER (trigger, rw);
+
+	    p.version         = PPC_DEBUG_CURRENT_VERSION;
+	    p.trigger_type    = trigger;
+	    p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+	    p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+	    p.addr            = (uint64_t) addr;
+	    p.addr2           = 0;
+	    p.condition_value = 0;
+
+	    booke_insert_point (&p, TIDGET (ptid));
+	  }
+
+      ret = 0;
+    }
+  else
+    {
+      long dabr_value;
+      long read_mode, write_mode;
+
+      if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+	{
+	  /* PowerPC 440 requires only the read/write flags to be passed
+	     to the kernel.  */
+	  read_mode  = 1;
+	  write_mode = 2;
+	}
+      else
+	{
+	  /* PowerPC 970 and other DABR-based processors are required to pass
+	     the Breakpoint Translation bit together with the flags.  */
+	  read_mode  = 5;
+	  write_mode = 6;
+	}
+
+      dabr_value = addr & ~(read_mode | write_mode);
+      switch (rw)
+	{
+	  case hw_read:
+	    /* Set read and translate bits.  */
+	    dabr_value |= read_mode;
+	    break;
+	  case hw_write:
+	    /* Set write and translate bits.  */
+	    dabr_value |= write_mode;
+	    break;
+	  case hw_access:
+	    /* Set read, write and translate bits.  */
+	    dabr_value |= read_mode | write_mode;
+	    break;
+	}
+
+      saved_dabr_value = dabr_value;
+
+      ALL_LWPS (lp, ptid)
+	if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
+	  return -1;
+
+      ret = 0;
+    }
+
+  return ret;
+}
+
 static int
 ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw)
 {
   struct lwp_info *lp;
   ptid_t ptid;
-  long dabr_value = 0;
+  int ret = -1;
 
-  saved_dabr_value = 0;
-  ALL_LWPS (lp, ptid)
-    if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
-      return -1;
-  return 0;
+  if (have_ptrace_new_debug_booke)
+    {
+      if (len <= 4)
+	ALL_LWPS (lp, ptid)
+	  {
+	    struct ppc_hw_breakpoint p;
+	    uint32_t trigger;
+
+	    HW_WATCH_RW_TRIGGER (trigger, rw);
+
+	    p.version         = PPC_DEBUG_CURRENT_VERSION;
+	    p.trigger_type    = trigger;
+	    p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+	    p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+	    p.addr            = (uint64_t) addr;
+	    p.addr2           = 0;
+	    p.condition_value = 0;
+
+	    booke_remove_point (&p, TIDGET (ptid));
+	  }
+
+      ret = 0;
+    }
+  else
+    {
+      struct lwp_info *lp;
+      ptid_t ptid;
+      long dabr_value = 0;
+
+      saved_dabr_value = 0;
+      ALL_LWPS (lp, ptid)
+	if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
+	  return -1;
+
+      ret = 0;
+    }
+
+  return ret;
 }
 
 static void
 ppc_linux_new_thread (ptid_t ptid)
 {
-  ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value);
+  int tid = TIDGET (ptid);
+
+  if (have_ptrace_new_debug_booke)
+    {
+      int i;
+      struct thread_points *p;
+      struct hw_break_tuple *hw_breaks;
+
+      /* Get a list of breakpoints from any thread. */
+      p = VEC_last (thread_points_p, ppc_threads);
+      hw_breaks = p->hw_breaks;
+
+      /* Copy that thread's breakpoints and watchpoints to the new thread. */
+      for (i = 0; i < max_slots_number; i++)
+	if (hw_breaks[i].hw_break)
+	  booke_insert_point (hw_breaks[i].hw_break, tid);
+    }
+  else
+    ptrace (PTRACE_SET_DEBUGREG, tid, 0, saved_dabr_value);
 }
 
 static int
@@ -1408,6 +1817,29 @@ ppc_linux_stopped_data_address (struct t
       || (siginfo_p->si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */)
     return 0;
 
+  if (have_ptrace_new_debug_booke)
+    {
+      int i;
+      struct thread_points *t;
+      struct hw_break_tuple *hw_breaks;
+      /* The index (or slot) of the *point is passed in the si_errno field.  */
+      int slot = siginfo_p->si_errno;
+
+      t = booke_find_thread_points_by_tid (TIDGET (inferior_ptid), 0);
+
+      /* Find out if this *point is a hardware breakpoint.
+	 If so, we should return 0.  */
+      if (t)
+	{
+	  hw_breaks = t->hw_breaks;
+	  for (i = 0; i < max_slots_number; i++)
+	   if (hw_breaks[i].hw_break && hw_breaks[i].slot == slot
+	       && hw_breaks[i].hw_break->trigger_type
+		    == PPC_BREAKPOINT_TRIGGER_EXECUTE)
+	     return 0;
+	}
+    }
+
   *addr_p = (CORE_ADDR) (uintptr_t) siginfo_p->si_addr;
   return 1;
 }
@@ -1426,7 +1858,10 @@ ppc_linux_watchpoint_addr_within_range (
 {
   int mask;
 
-  if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+  if (have_ptrace_new_debug_booke
+      && ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+    return start <= addr && start + length >= addr;
+  else if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
     mask = 3;
   else
     mask = 7;
@@ -1595,6 +2030,20 @@ ppc_linux_read_description (struct targe
 	perror_with_name (_("Unable to fetch AltiVec registers"));
     }
 
+  /* Check for kernel support for new BOOKE debugging registers.  */
+  if (ptrace (PPC_PTRACE_GETHWDBGINFO, tid, 0, &booke_debug_info) >= 0)
+    {
+      have_ptrace_new_debug_booke = 1;
+      max_slots_number = booke_debug_info.num_instruction_bps
+	+ booke_debug_info.num_data_bps + booke_debug_info.num_condition_regs;
+    }
+  else
+    {
+      /* Old school interface and no new BOOKE registers support.  */
+      have_ptrace_new_debug_booke = 0;
+      memset (&booke_debug_info, 0, sizeof (struct ppc_debug_info));
+    }
+
   /* Power ISA 2.05 (implemented by Power 6 and newer processors) increases
      the FPSCR from 32 bits to 64 bits. Even though Power 7 supports this
      ISA version, it doesn't have PPC_FEATURE_ARCH_2_05 set, only
@@ -1643,8 +2092,10 @@ _initialize_ppc_linux_nat (void)
   t->to_fetch_registers = ppc_linux_fetch_inferior_registers;
   t->to_store_registers = ppc_linux_store_inferior_registers;
 
-  /* Add our watchpoint methods.  */
-  t->to_can_use_hw_breakpoint = ppc_linux_check_watch_resources;
+  /* Add our breakpoint/watchpoint methods.  */
+  t->to_can_use_hw_breakpoint = ppc_linux_can_use_hw_breakpoint;
+  t->to_insert_hw_breakpoint = ppc_linux_insert_hw_breakpoint;
+  t->to_remove_hw_breakpoint = ppc_linux_remove_hw_breakpoint;
   t->to_region_ok_for_hw_watchpoint = ppc_linux_region_ok_for_hw_watchpoint;
   t->to_insert_watchpoint = ppc_linux_insert_watchpoint;
   t->to_remove_watchpoint = ppc_linux_remove_watchpoint;

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Support the new BookE ptrace interface
  2009-12-24  0:31 [PATCH 1/4] Support the new BookE ptrace interface Thiago Jung Bauermann
@ 2009-12-29  6:12 ` Joel Brobecker
  2009-12-30 19:51   ` Thiago Jung Bauermann
  2010-01-04 17:33 ` Thiago Jung Bauermann
  1 sibling, 1 reply; 14+ messages in thread
From: Joel Brobecker @ 2009-12-29  6:12 UTC (permalink / raw)
  To: Thiago Jung Bauermann; +Cc: gdb-patches, Luis Machado, Matt Tyrlik

> 2009-12-23  Sergio Durigan Junior  <sergiodj@linux.vnet.ibm.com>
> 	    Thiago Jung Bauermann  <bauerman@br.ibm.com>
> 
> 	* ppc-linux-nat.c (PTRACE_GET_DEBUGREG): Update comment.
> 	(PPC_PTRACE_GETWDBGINFO, PPC_PTRACE_SETHWDEBUG, PPC_PTRACE_DELHWDEBUG,
> 	ppc_debug_info, PPC_DEBUG_FEATURE_INSN_BP_RANGE,
> 	PPC_DEBUG_FEATURE_INSN_BP_MASK, PPC_DEBUG_FEATURE_DATA_BP_RANGE,
> 	PPC_DEBUG_FEATURE_DATA_BP_MASK, ppc_hw_breakpoint,
> 	PPC_BREAKPOINT_TRIGGER_EXECUTE, PPC_BREAKPOINT_TRIGGER READ,
> 	PPC_BREAKPOINT_TRIGGER_WRITE, PPC_BREAKPOINT_TRIGGER_RW,
> 	PPC_BREAKPOINT_MODE_EXACT PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE,
> 	PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE, PPC_BREAKPOINT_MODE_MASK,
> 	PPC_BREAKPOINT_CONDITION_NONE, PPC_BREAKPOINT_CONDITION_AND,
> 	PPC_BREAKPOINT_CONDITION_EXACT, PPC_BREAKPOINT_CONDITION_OR,
> 	PPC_BREAKPOINT_CONDITION_AND_OR, PPC_BREAKPOINT_CONDITION_BE_ALL,
> 	PPC_BREAKPOINT_CONDITION_BE_SHIFT, PPC_BREAKPOINT_CONDITION_BE):
> 	Define, in case <ptrace.h> doesn't provide it.
> 	(have_ptrace_new_debug_booke): New flag.
> 	(ppc_linux_check_watch_resources): Renamed to ...
> 	(ppc_linux_can_use_hw_breakpoint): ... this.  Handle BookE processors.
> 	(booke_debug_info): New variable.
> 	(max_slots_number): Ditto.
> 	(struct hw_break_tuple): New struct.
> 	(struct thread_points): Ditto.
> 	(ppc_threads): New variable.
> 	(PPC_DEBUG_CURRENT_VERSION): New define.
> 	(ppc_linux_region_ok_for_hw_watchpoint): Handle BookE processors.
> 	(booke_cmp_hw_point): New function.
> 	(booke_find_thread_points_by_tid): Ditto.
> 	(booke_insert_point): Ditto.
> 	(booke_remove_point): Ditto.
> 	(ppc_linux_insert_hw_breakpoint): Ditto.
> 	(ppc_linux_remove_hw_breakpoint): Ditto.
> 	(HW_WATCH_RW_TRIGGER): New define.
> 	(ppc_linux_insert_watchpoint): Handle BookE processors.
> 	(ppc_linux_remove_watchpoint): Ditto.
> 	(ppc_linux_new_thread): Ditto.
> 	(ppc_linux_stopped_data_address): Ditto.
> 	(ppc_linux_watchpoint_addr_within_range): Ditto.
> 	with and without the new kernel interface.
> 	(ppc_linux_read_description): Query the target to know if it
> 	supports the new kernel BookE interface.
> 	(_initialize_ppc_linux_nat): Initialize to_insert_hw_breakpoint and
> 	to_remove_hw_breakpoint fields of the target operations struct.

Overall, this looks OK to me.  A few general, non-blocking comments:

  (1) That's a lot of definitions to add at the beginning of the file,
      and I was going to suggest that we move them to a separate include.
      But I see that we've already started doing that with other
      definitions, so this is fine.

  (2) Again, non-blocking, but I'm left wondering if the "old" case
      (when we don't have the new interface available), could be made
      a special case of the new one.  For instance, you maintain two
      sets of globals, one used for booke, and one used when non-booke.
      Would it be possible to maintain just one set that works for booke,
      and then infer the non-booke ones if necessary? Or said another
      way, in the non-booke case, we can still maintain the same data
      structures as in the booke case, and we can then convert that
      data to whatever we need in the non-booke case.  The only time
      I think this would be needed is during insertion/deletion.
      I think that this might simplify a bit some of the code, avoid
      maintaining two exclusive sets of globals, and this might also
      help with the transition away from the "old" interface when we
      decide not to support it anymore.

Has Ulrich had a chance to look at these patches? As I said, they look
OK to me, and are pre-approved pending the little nits I mentioned below.
But this is a rather large change, and it would be nice if someone more
knowledgeable with the ppc than me also had a look too.  The comments
above can be discussed as followup patches if you like.

> +/*
> + * features will have bits indication whether there is support for:
> + */

Small nit: We avoid the star ('*') at the beginning of the line (and
the sentence should start with a capital letter):

/* Features will have bits indication whether there is support for:  */

The sentence itself does not sound too English to me, but I'm OK with it
anyways. I think we get the idea :).

(there are a few more instances of the extra stars in that same section).

> +struct hw_break_tuple {
> +  long slot;
> +  struct ppc_hw_breakpoint *hw_break;
> +};

Another small formatting nit:

struct hw_break_tuple
{
  long slot;
  struct ppc_hw_breakpoint *hw_break;
};

> +  if (!have_ptrace_new_debug_booke)
> +    {
> +      ptid_t ptid;
> +      int tid;
> +
> +      /* 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);
> +
> +      if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
> +	return 0;

I'm ok with the implementation as is, but I'm wondering if this is
the type of data that we'd want to cache. Maybe this makes things
more complicated than it is worth?

> +/* This function can be used to retrieve a thread_points by
> +   the TID of the related process/thread.  If nothing has been
> +   found, it returns NULL.  */

Can you document the behavior when ALLOC_NEW is non-zero if nothing
has been found?

> +  if ((slot = ptrace (PPC_PTRACE_SETHWDEBUG, tid, 0, p)) < 0)

Hmmm, Let's avoid assignments inside conditions. It's harder to read, IMO.

    slot = ptrace (PPC_PTRACE_SETHWDEBUG, tid, 0, p);
    if (slot < 0)

> +    perror_with_name (_("\n\
> +Please review your watchpoints/breakpoints and make sure the mask, ranges \n\
> +and addresses are within acceptable limits."));

Is this something that we could catch earlier, when the *point is
created by the user?

> +    perror_with_name (_("\n\
> +Could not delete the breakpoint/watchpoint."));

All the other calls to perror_with_name do not insert a newline
at the beginning of the error string.  The previous one was rather long,
so I thought it made sense. But this one, it seems more consistent to
not have it. WDYT?

> +      struct ppc_hw_breakpoint p;
> +
> +      p.version         = PPC_DEBUG_CURRENT_VERSION;
> +      p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
> +      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
> +      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
> +      p.addr            = (uint64_t) bp_tgt->placed_address;
> +      p.addr2           = 0;
> +      p.condition_value = 0;
> +
> +      booke_remove_point (&p, TIDGET (ptid));

It seemed a bit strange that you're using the entire ppc_hw_breakpoint
as a key to your hw_breaks_structure. But thinking more about it, I
don't know if you have many options or not. At first, I thought it would
be a simple matter of remembering the slot where the point was inserted,
but then you'd need a map between the breakpoint and the slot.  The
current interface is too awkward for that (we only get passed the
bp_target_info). I have the same problem on VxWorks, where created
a map to be able to store some target-specific private date alongside
each breakpoint.

Oh well...

> +#define HW_WATCH_RW_TRIGGER(t, rw) \
> +  if (rw == hw_read) \
> +    t = PPC_BREAKPOINT_TRIGGER_READ; \
> +  else if (rw == hw_write) \
> +    t = PPC_BREAKPOINT_TRIGGER_WRITE; \
> +  else \
> +    t = PPC_BREAKPOINT_TRIGGER_READ | PPC_BREAKPOINT_TRIGGER_WRITE;

Can we use a function instead. I don't mind macros when it's a litteral
constant, or a simple arithmetic expression, but this is really complex
enough that a function would be better (particularly during debugging).

> +  if (have_ptrace_new_debug_booke)
> +    {
> +      if (len <= 4)
> +	ALL_LWPS (lp, ptid)

What happens if len > 4? Is that even possible? Right now, it looks
like we're returning zero as if the watchpoint has been inserted,
even though it hasn't...

Same for ppc_linux_remove_watchpoint.

-- 
Joel


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Support the new BookE ptrace interface
  2009-12-29  6:12 ` Joel Brobecker
@ 2009-12-30 19:51   ` Thiago Jung Bauermann
  0 siblings, 0 replies; 14+ messages in thread
From: Thiago Jung Bauermann @ 2009-12-30 19:51 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches, Luis Machado, Matt Tyrlik

On Tue 29 Dec 2009 04:11:41 Joel Brobecker wrote:
> Overall, this looks OK to me.  A few general, non-blocking comments:
>   (2) Again, non-blocking, but I'm left wondering if the "old" case
>       (when we don't have the new interface available), could be made
>       a special case of the new one.  For instance, you maintain two
>       sets of globals, one used for booke, and one used when non-booke.
>       Would it be possible to maintain just one set that works for booke,
>       and then infer the non-booke ones if necessary? Or said another
>       way, in the non-booke case, we can still maintain the same data
>       structures as in the booke case, and we can then convert that
>       data to whatever we need in the non-booke case.  The only time
>       I think this would be needed is during insertion/deletion.
>       I think that this might simplify a bit some of the code, avoid
>       maintaining two exclusive sets of globals, and this might also
>       help with the transition away from the "old" interface when we
>       decide not to support it anymore.

Well, the only global that the old interface uses is saved_dabr_value. :-)
But I see your point. I'll play with it and see what I get.

> Has Ulrich had a chance to look at these patches? As I said, they look
> OK to me, and are pre-approved pending the little nits I mentioned below.
> But this is a rather large change, and it would be nice if someone more
> knowledgeable with the ppc than me also had a look too.

Unfortunately Ulrich didn't have a chance to look at them yet, and I know he's 
on vacation so it might take a while... :-/

> The comments above can be discussed as followup patches if you like.

Since you mention it, I'd like to address the old-interface-as-special-case-
of-new-one suggestion in a followup patch.

> > +/*
> > + * features will have bits indication whether there is support for:
> > + */
> 
> Small nit: We avoid the star ('*') at the beginning of the line (and
> the sentence should start with a capital letter):

Right. Fixed.

> /* Features will have bits indication whether there is support for:  */
> 
> The sentence itself does not sound too English to me, but I'm OK with it
> anyways. I think we get the idea :).

I corrected from "indication" to "indicating".

> (there are a few more instances of the extra stars in that same section).

Fixed those too.

> > +struct hw_break_tuple {
> > +  long slot;
> > +  struct ppc_hw_breakpoint *hw_break;
> > +};
> 
> Another small formatting nit:
> 
> struct hw_break_tuple
> {
>   long slot;
>   struct ppc_hw_breakpoint *hw_break;
> };

Oops. Fixed.

> > +  if (!have_ptrace_new_debug_booke)
> > +    {
> > +      ptid_t ptid;
> > +      int tid;
> > +
> > +      /* 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);
> > +
> > +      if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
> > +	return 0;
> 
> I'm ok with the implementation as is, but I'm wondering if this is
> the type of data that we'd want to cache. Maybe this makes things
> more complicated than it is worth?

That is just how GDB does things now. I'd like to change the current old 
interface code as little as possible for now, since we are making a big change 
overall.

> > +/* This function can be used to retrieve a thread_points by
> > +   the TID of the related process/thread.  If nothing has been
> > +   found, it returns NULL.  */
> 
> Can you document the behavior when ALLOC_NEW is non-zero if nothing
> has been found?

Done:

/* This function can be used to retrieve a thread_points by the TID of the
   related process/thread.  If nothing has been found, and ALLOC_NEW is 0,
   it returns NULL.  If ALLOC_NEW is non-zero, a new thread_points for the
   provided TID will be created and returned.  */

> > +  if ((slot = ptrace (PPC_PTRACE_SETHWDEBUG, tid, 0, p)) < 0)
> 
> Hmmm, Let's avoid assignments inside conditions. It's harder to read, IMO.
> 
>     slot = ptrace (PPC_PTRACE_SETHWDEBUG, tid, 0, p);
>     if (slot < 0)

Right. Fixed.

> > +    perror_with_name (_("\n\
> > +Please review your watchpoints/breakpoints and make sure the mask,
> > ranges \n\ +and addresses are within acceptable limits."));
> 
> Is this something that we could catch earlier, when the *point is
> created by the user?

That message is really just in case the ptrace call fails. The breakpoints and 
watchpoints should be correct by now. I changed the message to:

  perror_with_name (_("Unexpected error setting breakpoint or watchpoint"));

> > +    perror_with_name (_("\n\
> > +Could not delete the breakpoint/watchpoint."));
> 
> All the other calls to perror_with_name do not insert a newline
> at the beginning of the error string.  The previous one was rather long,
> so I thought it made sense. But this one, it seems more consistent to
> not have it. WDYT?

Agreed. I changed it to a similar message as the previous one:

  perror_with_name (_("Unexpected error deleting breakpoint or watchpoint"));

> > +      struct ppc_hw_breakpoint p;
> > +
> > +      p.version         = PPC_DEBUG_CURRENT_VERSION;
> > +      p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
> > +      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
> > +      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
> > +      p.addr            = (uint64_t) bp_tgt->placed_address;
> > +      p.addr2           = 0;
> > +      p.condition_value = 0;
> > +
> > +      booke_remove_point (&p, TIDGET (ptid));
> 
> It seemed a bit strange that you're using the entire ppc_hw_breakpoint
> as a key to your hw_breaks_structure. But thinking more about it, I
> don't know if you have many options or not. At first, I thought it would
> be a simple matter of remembering the slot where the point was inserted,
> but then you'd need a map between the breakpoint and the slot.  The
> current interface is too awkward for that (we only get passed the
> bp_target_info). I have the same problem on VxWorks, where created
> a map to be able to store some target-specific private date alongside
> each breakpoint.
> 
> Oh well...

I agree that the data structures are a bit convoluted, but I can't come up 
with anything better for now. If someone has suggestions, I'l be glad to 
consider them. :-)

> > +#define HW_WATCH_RW_TRIGGER(t, rw) \
> > +  if (rw == hw_read) \
> > +    t = PPC_BREAKPOINT_TRIGGER_READ; \
> > +  else if (rw == hw_write) \
> > +    t = PPC_BREAKPOINT_TRIGGER_WRITE; \
> > +  else \
> > +    t = PPC_BREAKPOINT_TRIGGER_READ | PPC_BREAKPOINT_TRIGGER_WRITE;
> 
> Can we use a function instead. I don't mind macros when it's a litteral
> constant, or a simple arithmetic expression, but this is really complex
> enough that a function would be better (particularly during debugging).

Ok, changed.

> > +  if (have_ptrace_new_debug_booke)
> > +    {
> > +      if (len <= 4)
> > +	ALL_LWPS (lp, ptid)
> 
> What happens if len > 4? Is that even possible? Right now, it looks
> like we're returning zero as if the watchpoint has been inserted,
> even though it hasn't...

Oh, that if should be in the patch for ranged watchpoint support. There, 
ppc_linux_insert_watchpoint assumes that a watchpoint larger than 4 bytes 
should be converted to a ranged watchpoint.

But I don't think that's a good idea anymore, and instead added a 
target_insert_ranged_watchpoint method for GDB to explicitly ask the target to 
create a ranged watchpoint when the user asks for one, or when an array or 
struct is to be watched. The watchpoint length is not used for that anymore.

I'll post a new version of these patches with that change, and the fixes from 
the review comments I got so far. I'll submit them either tomorrow or on 
monday...
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Support the new BookE ptrace interface
  2009-12-24  0:31 [PATCH 1/4] Support the new BookE ptrace interface Thiago Jung Bauermann
  2009-12-29  6:12 ` Joel Brobecker
@ 2010-01-04 17:33 ` Thiago Jung Bauermann
  2010-01-05  4:51   ` Joel Brobecker
  1 sibling, 1 reply; 14+ messages in thread
From: Thiago Jung Bauermann @ 2010-01-04 17:33 UTC (permalink / raw)
  To: gdb-patches; +Cc: Luis Machado, Matt Tyrlik

[-- Attachment #1: Type: Text/Plain, Size: 2416 bytes --]

On Wed 23 Dec 2009 22:30:56 Thiago Jung Bauermann wrote:
> This patch adapts ppc-linux-nat.c to the new interface, without changing
> anything else. As a bonus, with the new interface it supports hardware
> breakpoints.

This version incorporates all the feedback received so far.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


2009-12-31  Sergio Durigan Junior  <sergiodj@linux.vnet.ibm.com>
	    Thiago Jung Bauermann  <bauerman@br.ibm.com>

	* ppc-linux-nat.c (PTRACE_GET_DEBUGREG): Update comment.
	(PPC_PTRACE_GETWDBGINFO, PPC_PTRACE_SETHWDEBUG, PPC_PTRACE_DELHWDEBUG,
	ppc_debug_info, PPC_DEBUG_FEATURE_INSN_BP_RANGE,
	PPC_DEBUG_FEATURE_INSN_BP_MASK, PPC_DEBUG_FEATURE_DATA_BP_RANGE,
	PPC_DEBUG_FEATURE_DATA_BP_MASK, ppc_hw_breakpoint,
	PPC_BREAKPOINT_TRIGGER_EXECUTE, PPC_BREAKPOINT_TRIGGER READ,
	PPC_BREAKPOINT_TRIGGER_WRITE, PPC_BREAKPOINT_TRIGGER_RW,
	PPC_BREAKPOINT_MODE_EXACT PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE,
	PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE, PPC_BREAKPOINT_MODE_MASK,
	PPC_BREAKPOINT_CONDITION_NONE, PPC_BREAKPOINT_CONDITION_AND,
	PPC_BREAKPOINT_CONDITION_EXACT, PPC_BREAKPOINT_CONDITION_OR,
	PPC_BREAKPOINT_CONDITION_AND_OR, PPC_BREAKPOINT_CONDITION_BE_ALL,
	PPC_BREAKPOINT_CONDITION_BE_SHIFT, PPC_BREAKPOINT_CONDITION_BE):
	Define, in case <ptrace.h> doesn't provide it.
	(have_ptrace_new_debug_booke): New global.
	(ppc_linux_check_watch_resources): Renamed to ...
	(ppc_linux_can_use_hw_breakpoint): ... this.  Handle BookE processors.
	(booke_debug_info): New variable.
	(max_slots_number): Ditto.
	(hw_break_tuple): New struct.
	(thread_points): Ditto.
	(ppc_threads): New variable.
	(PPC_DEBUG_CURRENT_VERSION): New define.
	(ppc_linux_region_ok_for_hw_watchpoint): Handle BookE processors.
	(booke_cmp_hw_point): New function.
	(booke_find_thread_points_by_tid): Ditto.
	(booke_insert_point): Ditto.
	(booke_remove_point): Ditto.
	(ppc_linux_insert_hw_breakpoint): Ditto.
	(ppc_linux_remove_hw_breakpoint): Ditto.
	(ppc_linux_insert_watchpoint): Handle BookE processors.
	(ppc_linux_remove_watchpoint): Ditto.
	(ppc_linux_new_thread): Ditto.
	(ppc_linux_stopped_data_address): Ditto.
	(ppc_linux_watchpoint_addr_within_range): Ditto.
	(ppc_linux_read_description): Query the target to know if it
	supports the new kernel BookE interface.
	(_initialize_ppc_linux_nat): Initialize to_insert_hw_breakpoint and
	to_remove_hw_breakpoint fields of the target operations struct.

[-- Attachment #2: ppc476-new-interface.diff --]
[-- Type: text/x-patch, Size: 21810 bytes --]

diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 47b74ad..2c930c9 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -100,7 +100,8 @@
 #define PTRACE_SETEVRREGS 21
 #endif
 
-/* Similarly for the hardware watchpoint support.  */
+/* Similarly for the hardware watchpoint support.  These requests are used
+   when the BookE kernel interface is not available.  */
 #ifndef PTRACE_GET_DEBUGREG
 #define PTRACE_GET_DEBUGREG    25
 #endif
@@ -111,6 +112,71 @@
 #define PTRACE_GETSIGINFO    0x4202
 #endif
 
+/* These requests are used when the BookE kernel interface is available.
+   It exposes the additional debug features of BookE processors, such as
+   ranged breakpoints and watchpoints and hardware-accelerated condition
+   evaluation.  */
+#ifndef PPC_PTRACE_GETHWDBGINFO
+
+/* Not having PPC_PTRACE_GETHWDBGINFO defined means that the new BookE
+   interface is not present in ptrace.h, so we'll have to pretty much include
+   it all here so that the code at least compiles on older systems.  */
+#define PPC_PTRACE_GETHWDBGINFO 0x89
+#define PPC_PTRACE_SETHWDEBUG   0x88
+#define PPC_PTRACE_DELHWDEBUG   0x87
+
+struct ppc_debug_info {
+        uint32_t version;               /* Only version 1 exists to date */
+        uint32_t num_instruction_bps;
+        uint32_t num_data_bps;
+        uint32_t num_condition_regs;
+        uint32_t data_bp_alignment;
+        uint32_t sizeof_condition;      /* size of the DVC register */
+        uint64_t features;
+};
+
+/* Features will have bits indicating whether there is support for:  */
+#define PPC_DEBUG_FEATURE_INSN_BP_RANGE         0x1
+#define PPC_DEBUG_FEATURE_INSN_BP_MASK          0x2
+#define PPC_DEBUG_FEATURE_DATA_BP_RANGE         0x4
+#define PPC_DEBUG_FEATURE_DATA_BP_MASK          0x8
+
+struct ppc_hw_breakpoint {
+        uint32_t version;               /* currently, version must be 1 */
+        uint32_t trigger_type;          /* only some combinations allowed */
+        uint32_t addr_mode;             /* address match mode */
+        uint32_t condition_mode;        /* break/watchpoint condition flags */
+        uint64_t addr;                  /* break/watchpoint address */
+        uint64_t addr2;                 /* range end or mask */
+        uint64_t condition_value;       /* contents of the DVC register */
+};
+
+/* Trigger type.  */
+#define PPC_BREAKPOINT_TRIGGER_EXECUTE  0x1
+#define PPC_BREAKPOINT_TRIGGER_READ     0x2
+#define PPC_BREAKPOINT_TRIGGER_WRITE    0x4
+#define PPC_BREAKPOINT_TRIGGER_RW       0x6
+
+/* Address mode.  */
+#define PPC_BREAKPOINT_MODE_EXACT               0x0
+#define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE     0x1
+#define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE     0x2
+#define PPC_BREAKPOINT_MODE_MASK                0x3
+
+/* Condition mode.  */
+#define PPC_BREAKPOINT_CONDITION_NONE   0x0
+#define PPC_BREAKPOINT_CONDITION_AND    0x1
+#define PPC_BREAKPOINT_CONDITION_EXACT  0x1
+#define PPC_BREAKPOINT_CONDITION_OR     0x2
+#define PPC_BREAKPOINT_CONDITION_AND_OR 0x3
+#define PPC_BREAKPOINT_CONDITION_BE_ALL 0x00ff0000
+#define PPC_BREAKPOINT_CONDITION_BE_SHIFT       16
+#define PPC_BREAKPOINT_CONDITION_BE(n)  \
+        (1<<((n)+PPC_BREAKPOINT_CONDITION_BE_SHIFT))
+#endif /* PPC_PTRACE_GETHWDBGINFO */
+
+
+
 /* Similarly for the general-purpose (gp0 -- gp31)
    and floating-point registers (fp0 -- fp31).  */
 #ifndef PTRACE_GETREGS
@@ -248,6 +314,10 @@ int have_ptrace_getsetregs = 1;
    them and gotten an error.  */
 int have_ptrace_getsetfpregs = 1;
 
+/* Non-zero if we support the new Kernel interface that enables
+   new BOOKE debugging resources.  */
+int have_ptrace_new_debug_booke = 1;
+
 /* *INDENT-OFF* */
 /* registers layout, as presented by the ptrace interface:
 PT_R0, PT_R1, PT_R2, PT_R3, PT_R4, PT_R5, PT_R6, PT_R7,
@@ -1270,30 +1340,6 @@ store_ppc_registers (const struct regcache *regcache, int tid)
     store_spe_register (regcache, tid, -1);
 }
 
-static 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 variants.
-     Some variants 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 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);
-
-  if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
-    return 0;
-  return 1;
-}
-
 /* Fetch the AT_HWCAP entry from the aux vector.  */
 unsigned long ppc_linux_get_hwcap (void)
 {
@@ -1305,6 +1351,99 @@ unsigned long ppc_linux_get_hwcap (void)
   return 0;
 }
 
+/* The cached DABR value, to install in new threads.
+   This variable is used when we are dealing with non-BookE
+   processors.  */
+static long saved_dabr_value;
+
+/* Global structure that will store information about the available
+   features on this BookE processor.  */
+static struct ppc_debug_info booke_debug_info;
+
+/* Global variable that holds the maximum number of slots that the
+   kernel will use.  This is only used when the processor is BookE.  */
+static size_t max_slots_number = 0;
+
+struct hw_break_tuple
+{
+  long slot;
+  struct ppc_hw_breakpoint *hw_break;
+};
+
+/* This is an internal VEC created to store information about *points inserted
+   for each thread.  This is used for BookE processors.  */
+typedef struct thread_points
+  {
+    /* The TID to which this *point relates.  */
+    int tid;
+    /* Information about the *point, such as its address, type, etc.
+
+       Each element inside this vector corresponds to a hardware
+       breakpoint or watchpoint in the thread represented by TID.  The maximum
+       size of these vector is MAX_SLOTS_NUMBER.  If the hw_break element of
+       the tuple is NULL, then the position in the vector is free.  */
+    struct hw_break_tuple *hw_breaks;
+  } *thread_points_p;
+DEF_VEC_P (thread_points_p);
+
+VEC(thread_points_p) *ppc_threads = NULL;
+
+/* The version of the kernel interface that we will use if the processor is
+   BookE.  */
+#define PPC_DEBUG_CURRENT_VERSION 1
+
+static int
+ppc_linux_can_use_hw_breakpoint (int type, int cnt, int ot)
+{
+  int total_hw_wp, total_hw_bp;
+
+  if (have_ptrace_new_debug_booke)
+    {
+      /* For PPC BookE processors, the number of available hardware
+         watchpoints and breakpoints is stored at the booke_debug_info
+	 struct.  */
+      total_hw_bp = booke_debug_info.num_instruction_bps;
+      total_hw_wp = booke_debug_info.num_data_bps;
+    }
+  else
+    {
+      /* For PPC server processors, we accept 1 hardware watchpoint and 0
+	 hardware breakpoints.  */
+      total_hw_bp = 0;
+      total_hw_wp = 1;
+    }
+
+  if (type == bp_hardware_watchpoint || type == bp_read_watchpoint
+      || type == bp_access_watchpoint || type == bp_watchpoint)
+    {
+      if (cnt > total_hw_wp)
+	return -1;
+    }
+  else if (type == bp_hardware_breakpoint)
+    {
+      if (cnt > total_hw_bp)
+	return -1;
+    }
+
+  if (!have_ptrace_new_debug_booke)
+    {
+      int tid;
+      ptid_t ptid = inferior_ptid;
+
+      /* 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);
+
+      if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
+	return 0;
+    }
+
+  return 1;
+}
+
 static int
 ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
 {
@@ -1312,89 +1451,341 @@ ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
   if (len <= 0)
     return 0;
 
+  /* The new BookE ptrace interface tells if there are alignment restrictions
+     for watchpoints in the processors.  In that case, we use that information
+     to determine the hardcoded watchable region for watchpoints.  */
+  if (have_ptrace_new_debug_booke)
+    {
+      if (booke_debug_info.data_bp_alignment
+	  && (addr + len > (addr & ~(booke_debug_info.data_bp_alignment - 1))
+	      + booke_debug_info.data_bp_alignment))
+	  return 0;
+    }
   /* addr+len must fall in the 8 byte watchable region for DABR-based
-     processors.  DAC-based processors, like the PowerPC 440, will use
+     processors (i.e., server processors).  Without the new BookE ptrace
+     interface, DAC-based processors (i.e., embedded processors) will use
      addresses aligned to 4-bytes due to the way the read/write flags are
-     passed at the moment.  */
-  if (((ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
-      && (addr + len) > (addr & ~3) + 4)
-      || (addr + len) > (addr & ~7) + 8)
+     passed in the old ptrace interface.  */
+  else if (((ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+	   && (addr + len) > (addr & ~3) + 4)
+	   || (addr + len) > (addr & ~7) + 8)
     return 0;
 
   return 1;
 }
 
-/* The cached DABR value, to install in new threads.  */
-static long saved_dabr_value;
+/* This function compares two ppc_hw_breakpoint structs field-by-field.  */
+static inline int
+booke_cmp_hw_point (struct ppc_hw_breakpoint *a, struct ppc_hw_breakpoint *b)
+{
+  return (a->trigger_type       == b->trigger_type
+	  && a->addr_mode       == b->addr_mode
+	  && a->condition_mode  == b->condition_mode
+	  && a->addr            == b->addr
+	  && a->addr2           == b->addr2
+	  && a->condition_value == b->condition_value);
+}
+
+/* This function can be used to retrieve a thread_points by the TID of the
+   related process/thread.  If nothing has been found, and ALLOC_NEW is 0,
+   it returns NULL.  If ALLOC_NEW is non-zero, a new thread_points for the
+   provided TID will be created and returned.  */
+static struct thread_points *
+booke_find_thread_points_by_tid (int tid, int alloc_new)
+{
+  int i;
+  struct thread_points *t;
+
+  for (i = 0; VEC_iterate (thread_points_p, ppc_threads, i, t); i++)
+    if (t->tid == tid)
+      return t;
+
+  t = NULL;
+
+  /* Do we need to allocate a new point_item
+     if the wanted one does not exist?  */
+  if (alloc_new)
+    {
+      t = xmalloc (sizeof (struct thread_points));
+      t->hw_breaks = xzalloc (max_slots_number * sizeof (struct hw_break_tuple));
+      t->tid = tid;
+      VEC_safe_push (thread_points_p, ppc_threads, t);
+    }
+
+  return t;
+}
+
+/* This function is a generic wrapper that is responsible for inserting a
+   *point (i.e., calling `ptrace' in order to issue the request to the
+   kernel) and registering it internally in GDB.  */
+static void
+booke_insert_point (struct ppc_hw_breakpoint *b, int tid)
+{
+  int i;
+  long slot;
+  struct ppc_hw_breakpoint *p = xmalloc (sizeof (struct ppc_hw_breakpoint));
+  struct hw_break_tuple *hw_breaks;
+  struct cleanup *c = make_cleanup (xfree, p);
+  struct thread_points *t;
+  struct hw_break_tuple *tuple;
+
+  memcpy (p, b, sizeof (struct ppc_hw_breakpoint));
+
+  errno = 0;
+  slot = ptrace (PPC_PTRACE_SETHWDEBUG, tid, 0, p);
+  if (slot < 0)
+    perror_with_name (_("Unexpected error setting breakpoint or watchpoint"));
+
+  /* Everything went fine, so we have to register this *point.  */
+  t = booke_find_thread_points_by_tid (tid, 1);
+  gdb_assert (t != NULL);
+  hw_breaks = t->hw_breaks;
+
+  /* Find a free element in the hw_breaks vector.  */
+  for (i = 0; i < max_slots_number; i++)
+    if (hw_breaks[i].hw_break == NULL)
+      {
+	hw_breaks[i].slot = slot;
+	hw_breaks[i].hw_break = p;
+	break;
+      }
+
+  gdb_assert (i != max_slots_number);
+
+  discard_cleanups (c);
+}
+
+/* This function is a generic wrapper that is responsible for removing a
+   *point (i.e., calling `ptrace' in order to issue the request to the
+   kernel), and unregistering it internally at GDB.  */
+static void
+booke_remove_point (struct ppc_hw_breakpoint *b, int tid)
+{
+  int i;
+  long slot;
+  struct hw_break_tuple *hw_breaks;
+  struct thread_points *t;
+
+  t = booke_find_thread_points_by_tid (tid, 0);
+  gdb_assert (t != NULL);
+  hw_breaks = t->hw_breaks;
+
+  for (i = 0; i < max_slots_number; i++)
+    if (hw_breaks[i].hw_break && booke_cmp_hw_point (hw_breaks[i].hw_break, b))
+      break;
+
+  gdb_assert (i != max_slots_number);
+
+  errno = 0;
+  if (ptrace (PPC_PTRACE_DELHWDEBUG, tid, 0, hw_breaks[i].slot) < 0)
+    perror_with_name (_("Unexpected error deleting breakpoint or watchpoint"));
+
+  xfree (hw_breaks[i].hw_break);
+  hw_breaks[i].hw_break = NULL;
+}
 
-/* Set a watchpoint of type TYPE at address ADDR.  */
 static int
-ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+ppc_linux_insert_hw_breakpoint (struct gdbarch *gdbarch,
+				  struct bp_target_info *bp_tgt)
 {
   struct lwp_info *lp;
   ptid_t ptid;
-  long dabr_value;
-  long read_mode, write_mode;
 
-  if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
-  {
-  /* PowerPC 440 requires only the read/write flags to be passed
-     to the kernel.  */
-    read_mode  = 1;
-    write_mode = 2;
-  }
-  else
-  {
-  /* PowerPC 970 and other DABR-based processors are required to pass
-     the Breakpoint Translation bit together with the flags.  */
-    read_mode  = 5;
-    write_mode = 6;
-  }
-
-  dabr_value = addr & ~(read_mode | write_mode);
-  switch (rw)
+  if (!have_ptrace_new_debug_booke)
+    return -1;
+
+  ALL_LWPS (lp, ptid)
     {
-    case hw_read:
-      /* Set read and translate bits.  */
-      dabr_value |= read_mode;
-      break;
-    case hw_write:
-      /* Set write and translate bits.  */
-      dabr_value |= write_mode;
-      break;
-    case hw_access:
-      /* Set read, write and translate bits.  */
-      dabr_value |= read_mode | write_mode;
-      break;
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) bp_tgt->placed_address;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      booke_insert_point (&p, TIDGET (ptid));
     }
 
-  saved_dabr_value = dabr_value;
+  return 0;
+}
+
+static int
+ppc_linux_remove_hw_breakpoint (struct gdbarch *gdbarch,
+				  struct bp_target_info *bp_tgt)
+{
+  struct lwp_info *lp;
+  ptid_t ptid;
+
+  if (!have_ptrace_new_debug_booke)
+    return -1;
 
   ALL_LWPS (lp, ptid)
-    if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
-      return -1;
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) bp_tgt->placed_address;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      booke_remove_point (&p, TIDGET (ptid));
+    }
 
   return 0;
 }
 
+static int get_trigger_type (int rw)
+{
+  int t;
+
+  if (rw == hw_read)
+    t = PPC_BREAKPOINT_TRIGGER_READ;
+  else if (rw == hw_write)
+    t = PPC_BREAKPOINT_TRIGGER_WRITE;
+  else
+    t = PPC_BREAKPOINT_TRIGGER_READ | PPC_BREAKPOINT_TRIGGER_WRITE;
+
+  return t;
+}
+
+static int
+ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+{
+  struct lwp_info *lp;
+  ptid_t ptid;
+  int ret = -1;
+
+  if (have_ptrace_new_debug_booke)
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = get_trigger_type (rw);
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) addr;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      ALL_LWPS (lp, ptid)
+	booke_insert_point (&p, TIDGET (ptid));
+
+      ret = 0;
+    }
+  else
+    {
+      long dabr_value;
+      long read_mode, write_mode;
+
+      if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+	{
+	  /* PowerPC 440 requires only the read/write flags to be passed
+	     to the kernel.  */
+	  read_mode  = 1;
+	  write_mode = 2;
+	}
+      else
+	{
+	  /* PowerPC 970 and other DABR-based processors are required to pass
+	     the Breakpoint Translation bit together with the flags.  */
+	  read_mode  = 5;
+	  write_mode = 6;
+	}
+
+      dabr_value = addr & ~(read_mode | write_mode);
+      switch (rw)
+	{
+	  case hw_read:
+	    /* Set read and translate bits.  */
+	    dabr_value |= read_mode;
+	    break;
+	  case hw_write:
+	    /* Set write and translate bits.  */
+	    dabr_value |= write_mode;
+	    break;
+	  case hw_access:
+	    /* Set read, write and translate bits.  */
+	    dabr_value |= read_mode | write_mode;
+	    break;
+	}
+
+      saved_dabr_value = dabr_value;
+
+      ALL_LWPS (lp, ptid)
+	if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
+	  return -1;
+
+      ret = 0;
+    }
+
+  return ret;
+}
+
 static int
 ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw)
 {
   struct lwp_info *lp;
   ptid_t ptid;
-  long dabr_value = 0;
+  int ret = -1;
 
-  saved_dabr_value = 0;
-  ALL_LWPS (lp, ptid)
-    if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
-      return -1;
-  return 0;
+  if (have_ptrace_new_debug_booke)
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = get_trigger_type (rw);
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) addr;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      ALL_LWPS (lp, ptid)
+	booke_remove_point (&p, TIDGET (ptid));
+
+      ret = 0;
+    }
+  else
+    {
+      saved_dabr_value = 0;
+      ALL_LWPS (lp, ptid)
+	if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
+	  return -1;
+
+      ret = 0;
+    }
+
+  return ret;
 }
 
 static void
 ppc_linux_new_thread (ptid_t ptid)
 {
-  ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value);
+  int tid = TIDGET (ptid);
+
+  if (have_ptrace_new_debug_booke)
+    {
+      int i;
+      struct thread_points *p;
+      struct hw_break_tuple *hw_breaks;
+
+      /* Get a list of breakpoints from any thread. */
+      p = VEC_last (thread_points_p, ppc_threads);
+      hw_breaks = p->hw_breaks;
+
+      /* Copy that thread's breakpoints and watchpoints to the new thread. */
+      for (i = 0; i < max_slots_number; i++)
+	if (hw_breaks[i].hw_break)
+	  booke_insert_point (hw_breaks[i].hw_break, tid);
+    }
+  else
+    ptrace (PTRACE_SET_DEBUGREG, tid, 0, saved_dabr_value);
 }
 
 static int
@@ -1408,6 +1799,29 @@ ppc_linux_stopped_data_address (struct target_ops *target, CORE_ADDR *addr_p)
       || (siginfo_p->si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */)
     return 0;
 
+  if (have_ptrace_new_debug_booke)
+    {
+      int i;
+      struct thread_points *t;
+      struct hw_break_tuple *hw_breaks;
+      /* The index (or slot) of the *point is passed in the si_errno field.  */
+      int slot = siginfo_p->si_errno;
+
+      t = booke_find_thread_points_by_tid (TIDGET (inferior_ptid), 0);
+
+      /* Find out if this *point is a hardware breakpoint.
+	 If so, we should return 0.  */
+      if (t)
+	{
+	  hw_breaks = t->hw_breaks;
+	  for (i = 0; i < max_slots_number; i++)
+	   if (hw_breaks[i].hw_break && hw_breaks[i].slot == slot
+	       && hw_breaks[i].hw_break->trigger_type
+		    == PPC_BREAKPOINT_TRIGGER_EXECUTE)
+	     return 0;
+	}
+    }
+
   *addr_p = (CORE_ADDR) (uintptr_t) siginfo_p->si_addr;
   return 1;
 }
@@ -1426,7 +1840,10 @@ ppc_linux_watchpoint_addr_within_range (struct target_ops *target,
 {
   int mask;
 
-  if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+  if (have_ptrace_new_debug_booke
+      && ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+    return start <= addr && start + length >= addr;
+  else if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
     mask = 3;
   else
     mask = 7;
@@ -1595,6 +2012,20 @@ ppc_linux_read_description (struct target_ops *ops)
 	perror_with_name (_("Unable to fetch AltiVec registers"));
     }
 
+  /* Check for kernel support for new BOOKE debugging registers.  */
+  if (ptrace (PPC_PTRACE_GETHWDBGINFO, tid, 0, &booke_debug_info) >= 0)
+    {
+      have_ptrace_new_debug_booke = 1;
+      max_slots_number = booke_debug_info.num_instruction_bps
+	+ booke_debug_info.num_data_bps + booke_debug_info.num_condition_regs;
+    }
+  else
+    {
+      /* Old school interface and no new BOOKE registers support.  */
+      have_ptrace_new_debug_booke = 0;
+      memset (&booke_debug_info, 0, sizeof (struct ppc_debug_info));
+    }
+
   /* Power ISA 2.05 (implemented by Power 6 and newer processors) increases
      the FPSCR from 32 bits to 64 bits. Even though Power 7 supports this
      ISA version, it doesn't have PPC_FEATURE_ARCH_2_05 set, only
@@ -1643,8 +2074,10 @@ _initialize_ppc_linux_nat (void)
   t->to_fetch_registers = ppc_linux_fetch_inferior_registers;
   t->to_store_registers = ppc_linux_store_inferior_registers;
 
-  /* Add our watchpoint methods.  */
-  t->to_can_use_hw_breakpoint = ppc_linux_check_watch_resources;
+  /* Add our breakpoint/watchpoint methods.  */
+  t->to_can_use_hw_breakpoint = ppc_linux_can_use_hw_breakpoint;
+  t->to_insert_hw_breakpoint = ppc_linux_insert_hw_breakpoint;
+  t->to_remove_hw_breakpoint = ppc_linux_remove_hw_breakpoint;
   t->to_region_ok_for_hw_watchpoint = ppc_linux_region_ok_for_hw_watchpoint;
   t->to_insert_watchpoint = ppc_linux_insert_watchpoint;
   t->to_remove_watchpoint = ppc_linux_remove_watchpoint;

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Support the new BookE ptrace interface
  2010-01-04 17:33 ` Thiago Jung Bauermann
@ 2010-01-05  4:51   ` Joel Brobecker
  2010-01-15 17:41     ` Luis Machado
                       ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Joel Brobecker @ 2010-01-05  4:51 UTC (permalink / raw)
  To: Thiago Jung Bauermann; +Cc: gdb-patches, Luis Machado, Matt Tyrlik

> 2009-12-31  Sergio Durigan Junior  <sergiodj@linux.vnet.ibm.com>
> 	    Thiago Jung Bauermann  <bauerman@br.ibm.com>
> 
> 	* ppc-linux-nat.c (PTRACE_GET_DEBUGREG): Update comment.
> 	(PPC_PTRACE_GETWDBGINFO, PPC_PTRACE_SETHWDEBUG, PPC_PTRACE_DELHWDEBUG,
> 	ppc_debug_info, PPC_DEBUG_FEATURE_INSN_BP_RANGE,
> 	PPC_DEBUG_FEATURE_INSN_BP_MASK, PPC_DEBUG_FEATURE_DATA_BP_RANGE,
> 	PPC_DEBUG_FEATURE_DATA_BP_MASK, ppc_hw_breakpoint,
> 	PPC_BREAKPOINT_TRIGGER_EXECUTE, PPC_BREAKPOINT_TRIGGER READ,
> 	PPC_BREAKPOINT_TRIGGER_WRITE, PPC_BREAKPOINT_TRIGGER_RW,
> 	PPC_BREAKPOINT_MODE_EXACT PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE,
> 	PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE, PPC_BREAKPOINT_MODE_MASK,
> 	PPC_BREAKPOINT_CONDITION_NONE, PPC_BREAKPOINT_CONDITION_AND,
> 	PPC_BREAKPOINT_CONDITION_EXACT, PPC_BREAKPOINT_CONDITION_OR,
> 	PPC_BREAKPOINT_CONDITION_AND_OR, PPC_BREAKPOINT_CONDITION_BE_ALL,
> 	PPC_BREAKPOINT_CONDITION_BE_SHIFT, PPC_BREAKPOINT_CONDITION_BE):
> 	Define, in case <ptrace.h> doesn't provide it.
> 	(have_ptrace_new_debug_booke): New global.
> 	(ppc_linux_check_watch_resources): Renamed to ...
> 	(ppc_linux_can_use_hw_breakpoint): ... this.  Handle BookE processors.
> 	(booke_debug_info): New variable.
> 	(max_slots_number): Ditto.
> 	(hw_break_tuple): New struct.
> 	(thread_points): Ditto.
> 	(ppc_threads): New variable.
> 	(PPC_DEBUG_CURRENT_VERSION): New define.
> 	(ppc_linux_region_ok_for_hw_watchpoint): Handle BookE processors.
> 	(booke_cmp_hw_point): New function.
> 	(booke_find_thread_points_by_tid): Ditto.
> 	(booke_insert_point): Ditto.
> 	(booke_remove_point): Ditto.
> 	(ppc_linux_insert_hw_breakpoint): Ditto.
> 	(ppc_linux_remove_hw_breakpoint): Ditto.
> 	(ppc_linux_insert_watchpoint): Handle BookE processors.
> 	(ppc_linux_remove_watchpoint): Ditto.
> 	(ppc_linux_new_thread): Ditto.
> 	(ppc_linux_stopped_data_address): Ditto.
> 	(ppc_linux_watchpoint_addr_within_range): Ditto.
> 	(ppc_linux_read_description): Query the target to know if it
> 	supports the new kernel BookE interface.
> 	(_initialize_ppc_linux_nat): Initialize to_insert_hw_breakpoint and
> 	to_remove_hw_breakpoint fields of the target operations struct.

Still looks good to me :). Just a couple of nits, but otherwise approved. 
It would still be nice if Ulrich had a chance to take a quick look ;-).

> +struct ppc_debug_info {
> +        uint32_t version;               /* Only version 1 exists to date */

The curly brace should be at the beginning of the next line.
And the indentation (probably a copy-paste from the kernel headers?)
needs to be adjusted.

Not that it really matters a whole lot. For this section, if it makes
things simpler to preserve the indentation of the original, I could
be convinced to leave the code untouched.

> +static int get_trigger_type (int rw)

Formatting nit as well:

   static int
   get_trigger_type (int rw)

-- 
Joel


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Support the new BookE ptrace interface
  2010-01-05  4:51   ` Joel Brobecker
@ 2010-01-15 17:41     ` Luis Machado
  2010-01-16  3:09     ` Luis Machado
  2010-02-11 17:06     ` Ulrich Weigand
  2 siblings, 0 replies; 14+ messages in thread
From: Luis Machado @ 2010-01-15 17:41 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Thiago Jung Bauermann, gdb-patches, Matt Tyrlik

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

Patch updated with the fixes.

Regards,
Luis

> Still looks good to me :). Just a couple of nits, but otherwise approved. 
> It would still be nice if Ulrich had a chance to take a quick look ;-).
> 
> > +struct ppc_debug_info {
> > +        uint32_t version;               /* Only version 1 exists to date */
> 
> The curly brace should be at the beginning of the next line.
> And the indentation (probably a copy-paste from the kernel headers?)
> needs to be adjusted.
> 
> Not that it really matters a whole lot. For this section, if it makes
> things simpler to preserve the indentation of the original, I could
> be convinced to leave the code untouched.
> 
> > +static int get_trigger_type (int rw)
> 
> Formatting nit as well:
> 
>    static int
>    get_trigger_type (int rw)
> 


[-- Attachment #2: ppc476-new-interface.diff --]
[-- Type: text/x-patch, Size: 23652 bytes --]

2009-12-23  Sergio Durigan Junior  <sergiodj@linux.vnet.ibm.com>
	    Thiago Jung Bauermann  <bauerman@br.ibm.com>

	* ppc-linux-nat.c (PTRACE_GET_DEBUGREG): Update comment.
	(PPC_PTRACE_GETWDBGINFO, PPC_PTRACE_SETHWDEBUG, PPC_PTRACE_DELHWDEBUG,
	ppc_debug_info, PPC_DEBUG_FEATURE_INSN_BP_RANGE,
	PPC_DEBUG_FEATURE_INSN_BP_MASK, PPC_DEBUG_FEATURE_DATA_BP_RANGE,
	PPC_DEBUG_FEATURE_DATA_BP_MASK, ppc_hw_breakpoint,
	PPC_BREAKPOINT_TRIGGER_EXECUTE, PPC_BREAKPOINT_TRIGGER READ,
	PPC_BREAKPOINT_TRIGGER_WRITE, PPC_BREAKPOINT_TRIGGER_RW,
	PPC_BREAKPOINT_MODE_EXACT PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE,
	PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE, PPC_BREAKPOINT_MODE_MASK,
	PPC_BREAKPOINT_CONDITION_NONE, PPC_BREAKPOINT_CONDITION_AND,
	PPC_BREAKPOINT_CONDITION_EXACT, PPC_BREAKPOINT_CONDITION_OR,
	PPC_BREAKPOINT_CONDITION_AND_OR, PPC_BREAKPOINT_CONDITION_BE_ALL,
	PPC_BREAKPOINT_CONDITION_BE_SHIFT, PPC_BREAKPOINT_CONDITION_BE):
	Define, in case <ptrace.h> doesn't provide it.
	(have_ptrace_new_debug_booke): New flag.
	(ppc_linux_check_watch_resources): Renamed to ...
	(ppc_linux_can_use_hw_breakpoint): ... this.  Handle BookE processors.
	(booke_debug_info): New variable.
	(max_slots_number): Ditto.
	(struct hw_break_tuple): New struct.
	(struct thread_points): Ditto.
	(ppc_threads): New variable.
	(PPC_DEBUG_CURRENT_VERSION): New define.
	(ppc_linux_region_ok_for_hw_watchpoint): Handle BookE processors.
	(booke_cmp_hw_point): New function.
	(booke_find_thread_points_by_tid): Ditto.
	(booke_insert_point): Ditto.
	(booke_remove_point): Ditto.
	(ppc_linux_insert_hw_breakpoint): Ditto.
	(ppc_linux_remove_hw_breakpoint): Ditto.
	(HW_WATCH_RW_TRIGGER): New define.
	(ppc_linux_insert_watchpoint): Handle BookE processors.
	(ppc_linux_remove_watchpoint): Ditto.
	(ppc_linux_new_thread): Ditto.
	(ppc_linux_stopped_data_address): Ditto.
	(ppc_linux_watchpoint_addr_within_range): Ditto.
	with and without the new kernel interface.
	(ppc_linux_read_description): Query the target to know if it
	supports the new kernel BookE interface.
	(_initialize_ppc_linux_nat): Initialize to_insert_hw_breakpoint and
	to_remove_hw_breakpoint fields of the target operations struct.

Index: gdb/gdb/ppc-linux-nat.c
===================================================================
--- gdb.orig/gdb/ppc-linux-nat.c	2010-01-15 15:12:21.698527461 -0200
+++ gdb/gdb/ppc-linux-nat.c	2010-01-15 15:36:21.935565550 -0200
@@ -100,7 +100,8 @@
 #define PTRACE_SETEVRREGS 21
 #endif
 
-/* Similarly for the hardware watchpoint support.  */
+/* Similarly for the hardware watchpoint support.  These requests are used
+   when the BookE kernel interface is not available.  */
 #ifndef PTRACE_GET_DEBUGREG
 #define PTRACE_GET_DEBUGREG    25
 #endif
@@ -111,6 +112,73 @@
 #define PTRACE_GETSIGINFO    0x4202
 #endif
 
+/* These requests are used when the BookE kernel interface is available.
+   It exposes the additional debug features of BookE processors, such as
+   ranged breakpoints and watchpoints and hardware-accelerated condition
+   evaluation.  */
+#ifndef PPC_PTRACE_GETHWDBGINFO
+
+/* Not having PPC_PTRACE_GETHWDBGINFO defined means that the new BookE
+   interface is not present in ptrace.h, so we'll have to pretty much include
+   it all here so that the code at least compiles on older systems.  */
+#define PPC_PTRACE_GETHWDBGINFO 0x89
+#define PPC_PTRACE_SETHWDEBUG   0x88
+#define PPC_PTRACE_DELHWDEBUG   0x87
+
+struct ppc_debug_info
+{
+        uint32_t version;               /* Only version 1 exists to date */
+        uint32_t num_instruction_bps;
+        uint32_t num_data_bps;
+        uint32_t num_condition_regs;
+        uint32_t data_bp_alignment;
+        uint32_t sizeof_condition;      /* size of the DVC register */
+        uint64_t features;
+};
+
+/* Features will have bits indicating whether there is support for:  */
+#define PPC_DEBUG_FEATURE_INSN_BP_RANGE         0x1
+#define PPC_DEBUG_FEATURE_INSN_BP_MASK          0x2
+#define PPC_DEBUG_FEATURE_DATA_BP_RANGE         0x4
+#define PPC_DEBUG_FEATURE_DATA_BP_MASK          0x8
+
+struct ppc_hw_breakpoint
+{
+        uint32_t version;               /* currently, version must be 1 */
+        uint32_t trigger_type;          /* only some combinations allowed */
+        uint32_t addr_mode;             /* address match mode */
+        uint32_t condition_mode;        /* break/watchpoint condition flags */
+        uint64_t addr;                  /* break/watchpoint address */
+        uint64_t addr2;                 /* range end or mask */
+        uint64_t condition_value;       /* contents of the DVC register */
+};
+
+/* Trigger type.  */
+#define PPC_BREAKPOINT_TRIGGER_EXECUTE  0x1
+#define PPC_BREAKPOINT_TRIGGER_READ     0x2
+#define PPC_BREAKPOINT_TRIGGER_WRITE    0x4
+#define PPC_BREAKPOINT_TRIGGER_RW       0x6
+
+/* Address mode.  */
+#define PPC_BREAKPOINT_MODE_EXACT               0x0
+#define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE     0x1
+#define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE     0x2
+#define PPC_BREAKPOINT_MODE_MASK                0x3
+
+/* Condition mode.  */
+#define PPC_BREAKPOINT_CONDITION_NONE   0x0
+#define PPC_BREAKPOINT_CONDITION_AND    0x1
+#define PPC_BREAKPOINT_CONDITION_EXACT  0x1
+#define PPC_BREAKPOINT_CONDITION_OR     0x2
+#define PPC_BREAKPOINT_CONDITION_AND_OR 0x3
+#define PPC_BREAKPOINT_CONDITION_BE_ALL 0x00ff0000
+#define PPC_BREAKPOINT_CONDITION_BE_SHIFT       16
+#define PPC_BREAKPOINT_CONDITION_BE(n)  \
+        (1<<((n)+PPC_BREAKPOINT_CONDITION_BE_SHIFT))
+#endif /* PPC_PTRACE_GETHWDBGINFO */
+
+
+
 /* Similarly for the general-purpose (gp0 -- gp31)
    and floating-point registers (fp0 -- fp31).  */
 #ifndef PTRACE_GETREGS
@@ -248,6 +316,10 @@
    them and gotten an error.  */
 int have_ptrace_getsetfpregs = 1;
 
+/* Non-zero if we support the new Kernel interface that enables
+   new BOOKE debugging resources.  */
+int have_ptrace_new_debug_booke = 1;
+
 /* *INDENT-OFF* */
 /* registers layout, as presented by the ptrace interface:
 PT_R0, PT_R1, PT_R2, PT_R3, PT_R4, PT_R5, PT_R6, PT_R7,
@@ -1270,30 +1342,6 @@
     store_spe_register (regcache, tid, -1);
 }
 
-static 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 variants.
-     Some variants 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 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);
-
-  if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
-    return 0;
-  return 1;
-}
-
 /* Fetch the AT_HWCAP entry from the aux vector.  */
 unsigned long ppc_linux_get_hwcap (void)
 {
@@ -1305,6 +1353,99 @@
   return 0;
 }
 
+/* The cached DABR value, to install in new threads.
+   This variable is used when we are dealing with non-BookE
+   processors.  */
+static long saved_dabr_value;
+
+/* Global structure that will store information about the available
+   features on this BookE processor.  */
+static struct ppc_debug_info booke_debug_info;
+
+/* Global variable that holds the maximum number of slots that the
+   kernel will use.  This is only used when the processor is BookE.  */
+static size_t max_slots_number = 0;
+
+struct hw_break_tuple
+{
+  long slot;
+  struct ppc_hw_breakpoint *hw_break;
+};
+
+/* This is an internal VEC created to store information about *points inserted
+   for each thread.  This is used for BookE processors.  */
+typedef struct thread_points
+  {
+    /* The TID to which this *point relates.  */
+    int tid;
+    /* Information about the *point, such as its address, type, etc.
+
+       Each element inside this vector corresponds to a hardware
+       breakpoint or watchpoint in the thread represented by TID.  The maximum
+       size of these vector is MAX_SLOTS_NUMBER.  If the hw_break element of
+       the tuple is NULL, then the position in the vector is free.  */
+    struct hw_break_tuple *hw_breaks;
+  } *thread_points_p;
+DEF_VEC_P (thread_points_p);
+
+VEC(thread_points_p) *ppc_threads = NULL;
+
+/* The version of the kernel interface that we will use if the processor is
+   BookE.  */
+#define PPC_DEBUG_CURRENT_VERSION 1
+
+static int
+ppc_linux_can_use_hw_breakpoint (int type, int cnt, int ot)
+{
+  int total_hw_wp, total_hw_bp;
+
+  if (have_ptrace_new_debug_booke)
+    {
+      /* For PPC BookE processors, the number of available hardware
+         watchpoints and breakpoints is stored at the booke_debug_info
+	 struct.  */
+      total_hw_bp = booke_debug_info.num_instruction_bps;
+      total_hw_wp = booke_debug_info.num_data_bps;
+    }
+  else
+    {
+      /* For PPC server processors, we accept 1 hardware watchpoint and 0
+	 hardware breakpoints.  */
+      total_hw_bp = 0;
+      total_hw_wp = 1;
+    }
+
+  if (type == bp_hardware_watchpoint || type == bp_read_watchpoint
+      || type == bp_access_watchpoint || type == bp_watchpoint)
+    {
+      if (cnt > total_hw_wp)
+	return -1;
+    }
+  else if (type == bp_hardware_breakpoint)
+    {
+      if (cnt > total_hw_bp)
+	return -1;
+    }
+
+  if (!have_ptrace_new_debug_booke)
+    {
+      int tid;
+      ptid_t ptid = inferior_ptid;
+
+      /* 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);
+
+      if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
+	return 0;
+    }
+
+  return 1;
+}
+
 static int
 ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
 {
@@ -1312,89 +1453,342 @@
   if (len <= 0)
     return 0;
 
+  /* The new BookE ptrace interface tells if there are alignment restrictions
+     for watchpoints in the processors.  In that case, we use that information
+     to determine the hardcoded watchable region for watchpoints.  */
+  if (have_ptrace_new_debug_booke)
+    {
+      if (booke_debug_info.data_bp_alignment
+	  && (addr + len > (addr & ~(booke_debug_info.data_bp_alignment - 1))
+	      + booke_debug_info.data_bp_alignment))
+	  return 0;
+    }
   /* addr+len must fall in the 8 byte watchable region for DABR-based
-     processors.  DAC-based processors, like the PowerPC 440, will use
+     processors (i.e., server processors).  Without the new BookE ptrace
+     interface, DAC-based processors (i.e., embedded processors) will use
      addresses aligned to 4-bytes due to the way the read/write flags are
-     passed at the moment.  */
-  if (((ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
-      && (addr + len) > (addr & ~3) + 4)
-      || (addr + len) > (addr & ~7) + 8)
+     passed in the old ptrace interface.  */
+  else if (((ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+	   && (addr + len) > (addr & ~3) + 4)
+	   || (addr + len) > (addr & ~7) + 8)
     return 0;
 
   return 1;
 }
 
-/* The cached DABR value, to install in new threads.  */
-static long saved_dabr_value;
+/* This function compares two ppc_hw_breakpoint structs field-by-field.  */
+static inline int
+booke_cmp_hw_point (struct ppc_hw_breakpoint *a, struct ppc_hw_breakpoint *b)
+{
+  return (a->trigger_type       == b->trigger_type
+	  && a->addr_mode       == b->addr_mode
+	  && a->condition_mode  == b->condition_mode
+	  && a->addr            == b->addr
+	  && a->addr2           == b->addr2
+	  && a->condition_value == b->condition_value);
+}
+
+/* This function can be used to retrieve a thread_points by the TID of the
+   related process/thread.  If nothing has been found, and ALLOC_NEW is 0,
+   it returns NULL.  If ALLOC_NEW is non-zero, a new thread_points for the
+   provided TID will be created and returned.  */
+static struct thread_points *
+booke_find_thread_points_by_tid (int tid, int alloc_new)
+{
+  int i;
+  struct thread_points *t;
+
+  for (i = 0; VEC_iterate (thread_points_p, ppc_threads, i, t); i++)
+    if (t->tid == tid)
+      return t;
+
+  t = NULL;
+
+  /* Do we need to allocate a new point_item
+     if the wanted one does not exist?  */
+  if (alloc_new)
+    {
+      t = xmalloc (sizeof (struct thread_points));
+      t->hw_breaks = xzalloc (max_slots_number * sizeof (struct hw_break_tuple));
+      t->tid = tid;
+      VEC_safe_push (thread_points_p, ppc_threads, t);
+    }
+
+  return t;
+}
+
+/* This function is a generic wrapper that is responsible for inserting a
+   *point (i.e., calling `ptrace' in order to issue the request to the
+   kernel) and registering it internally in GDB.  */
+static void
+booke_insert_point (struct ppc_hw_breakpoint *b, int tid)
+{
+  int i;
+  long slot;
+  struct ppc_hw_breakpoint *p = xmalloc (sizeof (struct ppc_hw_breakpoint));
+  struct hw_break_tuple *hw_breaks;
+  struct cleanup *c = make_cleanup (xfree, p);
+  struct thread_points *t;
+  struct hw_break_tuple *tuple;
+
+  memcpy (p, b, sizeof (struct ppc_hw_breakpoint));
+
+  errno = 0;
+  slot = ptrace (PPC_PTRACE_SETHWDEBUG, tid, 0, p);
+  if (slot < 0)
+    perror_with_name (_("Unexpected error setting breakpoint or watchpoint"));
+
+  /* Everything went fine, so we have to register this *point.  */
+  t = booke_find_thread_points_by_tid (tid, 1);
+  gdb_assert (t != NULL);
+  hw_breaks = t->hw_breaks;
+
+  /* Find a free element in the hw_breaks vector.  */
+  for (i = 0; i < max_slots_number; i++)
+    if (hw_breaks[i].hw_break == NULL)
+      {
+	hw_breaks[i].slot = slot;
+	hw_breaks[i].hw_break = p;
+	break;
+      }
+
+  gdb_assert (i != max_slots_number);
+
+  discard_cleanups (c);
+}
+
+/* This function is a generic wrapper that is responsible for removing a
+   *point (i.e., calling `ptrace' in order to issue the request to the
+   kernel), and unregistering it internally at GDB.  */
+static void
+booke_remove_point (struct ppc_hw_breakpoint *b, int tid)
+{
+  int i;
+  long slot;
+  struct hw_break_tuple *hw_breaks;
+  struct thread_points *t;
+
+  t = booke_find_thread_points_by_tid (tid, 0);
+  gdb_assert (t != NULL);
+  hw_breaks = t->hw_breaks;
+
+  for (i = 0; i < max_slots_number; i++)
+    if (hw_breaks[i].hw_break && booke_cmp_hw_point (hw_breaks[i].hw_break, b))
+      break;
+
+  gdb_assert (i != max_slots_number);
+
+  errno = 0;
+  if (ptrace (PPC_PTRACE_DELHWDEBUG, tid, 0, hw_breaks[i].slot) < 0)
+    perror_with_name (_("Unexpected error deleting breakpoint or watchpoint"));
+
+  xfree (hw_breaks[i].hw_break);
+  hw_breaks[i].hw_break = NULL;
+}
 
-/* Set a watchpoint of type TYPE at address ADDR.  */
 static int
-ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+ppc_linux_insert_hw_breakpoint (struct gdbarch *gdbarch,
+				  struct bp_target_info *bp_tgt)
 {
   struct lwp_info *lp;
   ptid_t ptid;
-  long dabr_value;
-  long read_mode, write_mode;
 
-  if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
-  {
-  /* PowerPC 440 requires only the read/write flags to be passed
-     to the kernel.  */
-    read_mode  = 1;
-    write_mode = 2;
-  }
-  else
-  {
-  /* PowerPC 970 and other DABR-based processors are required to pass
-     the Breakpoint Translation bit together with the flags.  */
-    read_mode  = 5;
-    write_mode = 6;
-  }
-
-  dabr_value = addr & ~(read_mode | write_mode);
-  switch (rw)
-    {
-    case hw_read:
-      /* Set read and translate bits.  */
-      dabr_value |= read_mode;
-      break;
-    case hw_write:
-      /* Set write and translate bits.  */
-      dabr_value |= write_mode;
-      break;
-    case hw_access:
-      /* Set read, write and translate bits.  */
-      dabr_value |= read_mode | write_mode;
-      break;
+  if (!have_ptrace_new_debug_booke)
+    return -1;
+
+  ALL_LWPS (lp, ptid)
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) bp_tgt->placed_address;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      booke_insert_point (&p, TIDGET (ptid));
     }
 
-  saved_dabr_value = dabr_value;
+  return 0;
+}
+
+static int
+ppc_linux_remove_hw_breakpoint (struct gdbarch *gdbarch,
+				  struct bp_target_info *bp_tgt)
+{
+  struct lwp_info *lp;
+  ptid_t ptid;
+
+  if (!have_ptrace_new_debug_booke)
+    return -1;
 
   ALL_LWPS (lp, ptid)
-    if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
-      return -1;
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) bp_tgt->placed_address;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      booke_remove_point (&p, TIDGET (ptid));
+    }
 
   return 0;
 }
 
 static int
+get_trigger_type (int rw)
+{
+  int t;
+
+  if (rw == hw_read)
+    t = PPC_BREAKPOINT_TRIGGER_READ;
+  else if (rw == hw_write)
+    t = PPC_BREAKPOINT_TRIGGER_WRITE;
+  else
+    t = PPC_BREAKPOINT_TRIGGER_READ | PPC_BREAKPOINT_TRIGGER_WRITE;
+
+  return t;
+}
+
+static int
+ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+{
+  struct lwp_info *lp;
+  ptid_t ptid;
+  int ret = -1;
+
+  if (have_ptrace_new_debug_booke)
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = get_trigger_type (rw);
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) addr;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      ALL_LWPS (lp, ptid)
+	booke_insert_point (&p, TIDGET (ptid));
+
+      ret = 0;
+    }
+  else
+    {
+      long dabr_value;
+      long read_mode, write_mode;
+
+      if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+	{
+	  /* PowerPC 440 requires only the read/write flags to be passed
+	     to the kernel.  */
+	  read_mode  = 1;
+	  write_mode = 2;
+	}
+      else
+	{
+	  /* PowerPC 970 and other DABR-based processors are required to pass
+	     the Breakpoint Translation bit together with the flags.  */
+	  read_mode  = 5;
+	  write_mode = 6;
+	}
+
+      dabr_value = addr & ~(read_mode | write_mode);
+      switch (rw)
+	{
+	  case hw_read:
+	    /* Set read and translate bits.  */
+	    dabr_value |= read_mode;
+	    break;
+	  case hw_write:
+	    /* Set write and translate bits.  */
+	    dabr_value |= write_mode;
+	    break;
+	  case hw_access:
+	    /* Set read, write and translate bits.  */
+	    dabr_value |= read_mode | write_mode;
+	    break;
+	}
+
+      saved_dabr_value = dabr_value;
+
+      ALL_LWPS (lp, ptid)
+	if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
+	  return -1;
+
+      ret = 0;
+    }
+
+  return ret;
+}
+
+static int
 ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw)
 {
   struct lwp_info *lp;
   ptid_t ptid;
-  long dabr_value = 0;
+  int ret = -1;
 
-  saved_dabr_value = 0;
-  ALL_LWPS (lp, ptid)
-    if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
-      return -1;
-  return 0;
+  if (have_ptrace_new_debug_booke)
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = get_trigger_type (rw);
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) addr;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      ALL_LWPS (lp, ptid)
+	booke_remove_point (&p, TIDGET (ptid));
+
+      ret = 0;
+    }
+  else
+    {
+      saved_dabr_value = 0;
+      ALL_LWPS (lp, ptid)
+	if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
+	  return -1;
+
+      ret = 0;
+    }
+
+  return ret;
 }
 
 static void
 ppc_linux_new_thread (ptid_t ptid)
 {
-  ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value);
+  int tid = TIDGET (ptid);
+
+  if (have_ptrace_new_debug_booke)
+    {
+      int i;
+      struct thread_points *p;
+      struct hw_break_tuple *hw_breaks;
+
+      /* Get a list of breakpoints from any thread. */
+      p = VEC_last (thread_points_p, ppc_threads);
+      hw_breaks = p->hw_breaks;
+
+      /* Copy that thread's breakpoints and watchpoints to the new thread. */
+      for (i = 0; i < max_slots_number; i++)
+	if (hw_breaks[i].hw_break)
+	  booke_insert_point (hw_breaks[i].hw_break, tid);
+    }
+  else
+    ptrace (PTRACE_SET_DEBUGREG, tid, 0, saved_dabr_value);
 }
 
 static int
@@ -1408,6 +1802,29 @@
       || (siginfo_p->si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */)
     return 0;
 
+  if (have_ptrace_new_debug_booke)
+    {
+      int i;
+      struct thread_points *t;
+      struct hw_break_tuple *hw_breaks;
+      /* The index (or slot) of the *point is passed in the si_errno field.  */
+      int slot = siginfo_p->si_errno;
+
+      t = booke_find_thread_points_by_tid (TIDGET (inferior_ptid), 0);
+
+      /* Find out if this *point is a hardware breakpoint.
+	 If so, we should return 0.  */
+      if (t)
+	{
+	  hw_breaks = t->hw_breaks;
+	  for (i = 0; i < max_slots_number; i++)
+	   if (hw_breaks[i].hw_break && hw_breaks[i].slot == slot
+	       && hw_breaks[i].hw_break->trigger_type
+		    == PPC_BREAKPOINT_TRIGGER_EXECUTE)
+	     return 0;
+	}
+    }
+
   *addr_p = (CORE_ADDR) (uintptr_t) siginfo_p->si_addr;
   return 1;
 }
@@ -1426,7 +1843,10 @@
 {
   int mask;
 
-  if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+  if (have_ptrace_new_debug_booke
+      && ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+    return start <= addr && start + length >= addr;
+  else if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
     mask = 3;
   else
     mask = 7;
@@ -1595,6 +2015,20 @@
 	perror_with_name (_("Unable to fetch AltiVec registers"));
     }
 
+  /* Check for kernel support for new BOOKE debugging registers.  */
+  if (ptrace (PPC_PTRACE_GETHWDBGINFO, tid, 0, &booke_debug_info) >= 0)
+    {
+      have_ptrace_new_debug_booke = 1;
+      max_slots_number = booke_debug_info.num_instruction_bps
+	+ booke_debug_info.num_data_bps + booke_debug_info.num_condition_regs;
+    }
+  else
+    {
+      /* Old school interface and no new BOOKE registers support.  */
+      have_ptrace_new_debug_booke = 0;
+      memset (&booke_debug_info, 0, sizeof (struct ppc_debug_info));
+    }
+
   /* Power ISA 2.05 (implemented by Power 6 and newer processors) increases
      the FPSCR from 32 bits to 64 bits. Even though Power 7 supports this
      ISA version, it doesn't have PPC_FEATURE_ARCH_2_05 set, only
@@ -1643,8 +2077,10 @@
   t->to_fetch_registers = ppc_linux_fetch_inferior_registers;
   t->to_store_registers = ppc_linux_store_inferior_registers;
 
-  /* Add our watchpoint methods.  */
-  t->to_can_use_hw_breakpoint = ppc_linux_check_watch_resources;
+  /* Add our breakpoint/watchpoint methods.  */
+  t->to_can_use_hw_breakpoint = ppc_linux_can_use_hw_breakpoint;
+  t->to_insert_hw_breakpoint = ppc_linux_insert_hw_breakpoint;
+  t->to_remove_hw_breakpoint = ppc_linux_remove_hw_breakpoint;
   t->to_region_ok_for_hw_watchpoint = ppc_linux_region_ok_for_hw_watchpoint;
   t->to_insert_watchpoint = ppc_linux_insert_watchpoint;
   t->to_remove_watchpoint = ppc_linux_remove_watchpoint;

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Support the new BookE ptrace interface
  2010-01-05  4:51   ` Joel Brobecker
  2010-01-15 17:41     ` Luis Machado
@ 2010-01-16  3:09     ` Luis Machado
  2010-01-18  4:16       ` Joel Brobecker
  2010-02-11 17:06     ` Ulrich Weigand
  2 siblings, 1 reply; 14+ messages in thread
From: Luis Machado @ 2010-01-16  3:09 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Thiago Jung Bauermann, gdb-patches, Matt Tyrlik

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

Oops...

Follows the updated patch with the suggestions... Please ignore the
previous patch, i was working with an outdated version.

I'll send the other pieces next.

Regards,
Luis

[-- Attachment #2: ppc476-new-interface.diff --]
[-- Type: text/x-patch, Size: 23840 bytes --]

2010-01-16  Sergio Durigan Junior  <sergiodj@linux.vnet.ibm.com>
            Thiago Jung Bauermann  <bauerman@br.ibm.com>

        * ppc-linux-nat.c (PTRACE_GET_DEBUGREG): Update comment.
        (PPC_PTRACE_GETWDBGINFO, PPC_PTRACE_SETHWDEBUG, PPC_PTRACE_DELHWDEBUG,
        ppc_debug_info, PPC_DEBUG_FEATURE_INSN_BP_RANGE,
        PPC_DEBUG_FEATURE_INSN_BP_MASK, PPC_DEBUG_FEATURE_DATA_BP_RANGE,
        PPC_DEBUG_FEATURE_DATA_BP_MASK, ppc_hw_breakpoint,
        PPC_BREAKPOINT_TRIGGER_EXECUTE, PPC_BREAKPOINT_TRIGGER READ,
        PPC_BREAKPOINT_TRIGGER_WRITE, PPC_BREAKPOINT_TRIGGER_RW,
        PPC_BREAKPOINT_MODE_EXACT PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE,
        PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE, PPC_BREAKPOINT_MODE_MASK,
        PPC_BREAKPOINT_CONDITION_NONE, PPC_BREAKPOINT_CONDITION_AND,
        PPC_BREAKPOINT_CONDITION_EXACT, PPC_BREAKPOINT_CONDITION_OR,
        PPC_BREAKPOINT_CONDITION_AND_OR, PPC_BREAKPOINT_CONDITION_BE_ALL,
        PPC_BREAKPOINT_CONDITION_BE_SHIFT, PPC_BREAKPOINT_CONDITION_BE):
        Define, in case <ptrace.h> doesn't provide it.
        (have_ptrace_new_debug_booke): New global.
        (ppc_linux_check_watch_resources): Renamed to ...
        (ppc_linux_can_use_hw_breakpoint): ... this.  Handle BookE processors.
        (booke_debug_info): New variable.
        (max_slots_number): Ditto.
        (hw_break_tuple): New struct.
        (thread_points): Ditto.
        (ppc_threads): New variable.
        (PPC_DEBUG_CURRENT_VERSION): New define.
        (ppc_linux_region_ok_for_hw_watchpoint): Handle BookE processors.
        (booke_cmp_hw_point): New function.
        (booke_find_thread_points_by_tid): Ditto.
        (booke_insert_point): Ditto.
        (booke_remove_point): Ditto.
        (ppc_linux_insert_hw_breakpoint): Ditto.
        (ppc_linux_remove_hw_breakpoint): Ditto.
        (ppc_linux_insert_watchpoint): Handle BookE processors.
        (ppc_linux_remove_watchpoint): Ditto.
        (ppc_linux_new_thread): Ditto.
        (ppc_linux_stopped_data_address): Ditto.
        (ppc_linux_watchpoint_addr_within_range): Ditto.
        (ppc_linux_read_description): Query the target to know if it
        supports the new kernel BookE interface.
        (_initialize_ppc_linux_nat): Initialize to_insert_hw_breakpoint and
        to_remove_hw_breakpoint fields of the target operations struct.

Index: gdb/gdb/ppc-linux-nat.c
===================================================================
--- gdb.orig/gdb/ppc-linux-nat.c	2010-01-15 21:44:42.242528023 -0200
+++ gdb/gdb/ppc-linux-nat.c	2010-01-16 00:23:29.070555968 -0200
@@ -100,7 +100,8 @@
 #define PTRACE_SETEVRREGS 21
 #endif
 
-/* Similarly for the hardware watchpoint support.  */
+/* Similarly for the hardware watchpoint support.  These requests are used
+   when the BookE kernel interface is not available.  */
 #ifndef PTRACE_GET_DEBUGREG
 #define PTRACE_GET_DEBUGREG    25
 #endif
@@ -111,6 +112,73 @@
 #define PTRACE_GETSIGINFO    0x4202
 #endif
 
+/* These requests are used when the BookE kernel interface is available.
+   It exposes the additional debug features of BookE processors, such as
+   ranged breakpoints and watchpoints and hardware-accelerated condition
+   evaluation.  */
+#ifndef PPC_PTRACE_GETHWDBGINFO
+
+/* Not having PPC_PTRACE_GETHWDBGINFO defined means that the new BookE
+   interface is not present in ptrace.h, so we'll have to pretty much include
+   it all here so that the code at least compiles on older systems.  */
+#define PPC_PTRACE_GETHWDBGINFO 0x89
+#define PPC_PTRACE_SETHWDEBUG   0x88
+#define PPC_PTRACE_DELHWDEBUG   0x87
+
+struct ppc_debug_info
+{
+        uint32_t version;               /* Only version 1 exists to date */
+        uint32_t num_instruction_bps;
+        uint32_t num_data_bps;
+        uint32_t num_condition_regs;
+        uint32_t data_bp_alignment;
+        uint32_t sizeof_condition;      /* size of the DVC register */
+        uint64_t features;
+};
+
+/* Features will have bits indicating whether there is support for:  */
+#define PPC_DEBUG_FEATURE_INSN_BP_RANGE         0x1
+#define PPC_DEBUG_FEATURE_INSN_BP_MASK          0x2
+#define PPC_DEBUG_FEATURE_DATA_BP_RANGE         0x4
+#define PPC_DEBUG_FEATURE_DATA_BP_MASK          0x8
+
+struct ppc_hw_breakpoint
+{
+        uint32_t version;               /* currently, version must be 1 */
+        uint32_t trigger_type;          /* only some combinations allowed */
+        uint32_t addr_mode;             /* address match mode */
+        uint32_t condition_mode;        /* break/watchpoint condition flags */
+        uint64_t addr;                  /* break/watchpoint address */
+        uint64_t addr2;                 /* range end or mask */
+        uint64_t condition_value;       /* contents of the DVC register */
+};
+
+/* Trigger type.  */
+#define PPC_BREAKPOINT_TRIGGER_EXECUTE  0x1
+#define PPC_BREAKPOINT_TRIGGER_READ     0x2
+#define PPC_BREAKPOINT_TRIGGER_WRITE    0x4
+#define PPC_BREAKPOINT_TRIGGER_RW       0x6
+
+/* Address mode.  */
+#define PPC_BREAKPOINT_MODE_EXACT               0x0
+#define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE     0x1
+#define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE     0x2
+#define PPC_BREAKPOINT_MODE_MASK                0x3
+
+/* Condition mode.  */
+#define PPC_BREAKPOINT_CONDITION_NONE   0x0
+#define PPC_BREAKPOINT_CONDITION_AND    0x1
+#define PPC_BREAKPOINT_CONDITION_EXACT  0x1
+#define PPC_BREAKPOINT_CONDITION_OR     0x2
+#define PPC_BREAKPOINT_CONDITION_AND_OR 0x3
+#define PPC_BREAKPOINT_CONDITION_BE_ALL 0x00ff0000
+#define PPC_BREAKPOINT_CONDITION_BE_SHIFT       16
+#define PPC_BREAKPOINT_CONDITION_BE(n)  \
+        (1<<((n)+PPC_BREAKPOINT_CONDITION_BE_SHIFT))
+#endif /* PPC_PTRACE_GETHWDBGINFO */
+
+
+
 /* Similarly for the general-purpose (gp0 -- gp31)
    and floating-point registers (fp0 -- fp31).  */
 #ifndef PTRACE_GETREGS
@@ -248,6 +316,10 @@
    them and gotten an error.  */
 int have_ptrace_getsetfpregs = 1;
 
+/* Non-zero if we support the new Kernel interface that enables
+   new BOOKE debugging resources.  */
+int have_ptrace_new_debug_booke = 1;
+
 /* *INDENT-OFF* */
 /* registers layout, as presented by the ptrace interface:
 PT_R0, PT_R1, PT_R2, PT_R3, PT_R4, PT_R5, PT_R6, PT_R7,
@@ -1270,30 +1342,6 @@
     store_spe_register (regcache, tid, -1);
 }
 
-static 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 variants.
-     Some variants 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 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);
-
-  if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
-    return 0;
-  return 1;
-}
-
 /* Fetch the AT_HWCAP entry from the aux vector.  */
 unsigned long ppc_linux_get_hwcap (void)
 {
@@ -1305,6 +1353,99 @@
   return 0;
 }
 
+/* The cached DABR value, to install in new threads.
+   This variable is used when we are dealing with non-BookE
+   processors.  */
+static long saved_dabr_value;
+
+/* Global structure that will store information about the available
+   features on this BookE processor.  */
+static struct ppc_debug_info booke_debug_info;
+
+/* Global variable that holds the maximum number of slots that the
+   kernel will use.  This is only used when the processor is BookE.  */
+static size_t max_slots_number = 0;
+
+struct hw_break_tuple
+{
+  long slot;
+  struct ppc_hw_breakpoint *hw_break;
+};
+
+/* This is an internal VEC created to store information about *points inserted
+   for each thread.  This is used for BookE processors.  */
+typedef struct thread_points
+  {
+    /* The TID to which this *point relates.  */
+    int tid;
+    /* Information about the *point, such as its address, type, etc.
+
+       Each element inside this vector corresponds to a hardware
+       breakpoint or watchpoint in the thread represented by TID.  The maximum
+       size of these vector is MAX_SLOTS_NUMBER.  If the hw_break element of
+       the tuple is NULL, then the position in the vector is free.  */
+    struct hw_break_tuple *hw_breaks;
+  } *thread_points_p;
+DEF_VEC_P (thread_points_p);
+
+VEC(thread_points_p) *ppc_threads = NULL;
+
+/* The version of the kernel interface that we will use if the processor is
+   BookE.  */
+#define PPC_DEBUG_CURRENT_VERSION 1
+
+static int
+ppc_linux_can_use_hw_breakpoint (int type, int cnt, int ot)
+{
+  int total_hw_wp, total_hw_bp;
+
+  if (have_ptrace_new_debug_booke)
+    {
+      /* For PPC BookE processors, the number of available hardware
+         watchpoints and breakpoints is stored at the booke_debug_info
+	 struct.  */
+      total_hw_bp = booke_debug_info.num_instruction_bps;
+      total_hw_wp = booke_debug_info.num_data_bps;
+    }
+  else
+    {
+      /* For PPC server processors, we accept 1 hardware watchpoint and 0
+	 hardware breakpoints.  */
+      total_hw_bp = 0;
+      total_hw_wp = 1;
+    }
+
+  if (type == bp_hardware_watchpoint || type == bp_read_watchpoint
+      || type == bp_access_watchpoint || type == bp_watchpoint)
+    {
+      if (cnt > total_hw_wp)
+	return -1;
+    }
+  else if (type == bp_hardware_breakpoint)
+    {
+      if (cnt > total_hw_bp)
+	return -1;
+    }
+
+  if (!have_ptrace_new_debug_booke)
+    {
+      int tid;
+      ptid_t ptid = inferior_ptid;
+
+      /* 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);
+
+      if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
+	return 0;
+    }
+
+  return 1;
+}
+
 static int
 ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
 {
@@ -1312,89 +1453,342 @@
   if (len <= 0)
     return 0;
 
+  /* The new BookE ptrace interface tells if there are alignment restrictions
+     for watchpoints in the processors.  In that case, we use that information
+     to determine the hardcoded watchable region for watchpoints.  */
+  if (have_ptrace_new_debug_booke)
+    {
+      if (booke_debug_info.data_bp_alignment
+	  && (addr + len > (addr & ~(booke_debug_info.data_bp_alignment - 1))
+	      + booke_debug_info.data_bp_alignment))
+	  return 0;
+    }
   /* addr+len must fall in the 8 byte watchable region for DABR-based
-     processors.  DAC-based processors, like the PowerPC 440, will use
+     processors (i.e., server processors).  Without the new BookE ptrace
+     interface, DAC-based processors (i.e., embedded processors) will use
      addresses aligned to 4-bytes due to the way the read/write flags are
-     passed at the moment.  */
-  if (((ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
-      && (addr + len) > (addr & ~3) + 4)
-      || (addr + len) > (addr & ~7) + 8)
+     passed in the old ptrace interface.  */
+  else if (((ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+	   && (addr + len) > (addr & ~3) + 4)
+	   || (addr + len) > (addr & ~7) + 8)
     return 0;
 
   return 1;
 }
 
-/* The cached DABR value, to install in new threads.  */
-static long saved_dabr_value;
+/* This function compares two ppc_hw_breakpoint structs field-by-field.  */
+static inline int
+booke_cmp_hw_point (struct ppc_hw_breakpoint *a, struct ppc_hw_breakpoint *b)
+{
+  return (a->trigger_type       == b->trigger_type
+	  && a->addr_mode       == b->addr_mode
+	  && a->condition_mode  == b->condition_mode
+	  && a->addr            == b->addr
+	  && a->addr2           == b->addr2
+	  && a->condition_value == b->condition_value);
+}
+
+/* This function can be used to retrieve a thread_points by the TID of the
+   related process/thread.  If nothing has been found, and ALLOC_NEW is 0,
+   it returns NULL.  If ALLOC_NEW is non-zero, a new thread_points for the
+   provided TID will be created and returned.  */
+static struct thread_points *
+booke_find_thread_points_by_tid (int tid, int alloc_new)
+{
+  int i;
+  struct thread_points *t;
+
+  for (i = 0; VEC_iterate (thread_points_p, ppc_threads, i, t); i++)
+    if (t->tid == tid)
+      return t;
+
+  t = NULL;
+
+  /* Do we need to allocate a new point_item
+     if the wanted one does not exist?  */
+  if (alloc_new)
+    {
+      t = xmalloc (sizeof (struct thread_points));
+      t->hw_breaks = xzalloc (max_slots_number * sizeof (struct hw_break_tuple));
+      t->tid = tid;
+      VEC_safe_push (thread_points_p, ppc_threads, t);
+    }
+
+  return t;
+}
+
+/* This function is a generic wrapper that is responsible for inserting a
+   *point (i.e., calling `ptrace' in order to issue the request to the
+   kernel) and registering it internally in GDB.  */
+static void
+booke_insert_point (struct ppc_hw_breakpoint *b, int tid)
+{
+  int i;
+  long slot;
+  struct ppc_hw_breakpoint *p = xmalloc (sizeof (struct ppc_hw_breakpoint));
+  struct hw_break_tuple *hw_breaks;
+  struct cleanup *c = make_cleanup (xfree, p);
+  struct thread_points *t;
+  struct hw_break_tuple *tuple;
+
+  memcpy (p, b, sizeof (struct ppc_hw_breakpoint));
+
+  errno = 0;
+  slot = ptrace (PPC_PTRACE_SETHWDEBUG, tid, 0, p);
+  if (slot < 0)
+    perror_with_name (_("Unexpected error setting breakpoint or watchpoint"));
+
+  /* Everything went fine, so we have to register this *point.  */
+  t = booke_find_thread_points_by_tid (tid, 1);
+  gdb_assert (t != NULL);
+  hw_breaks = t->hw_breaks;
+
+  /* Find a free element in the hw_breaks vector.  */
+  for (i = 0; i < max_slots_number; i++)
+    if (hw_breaks[i].hw_break == NULL)
+      {
+	hw_breaks[i].slot = slot;
+	hw_breaks[i].hw_break = p;
+	break;
+      }
+
+  gdb_assert (i != max_slots_number);
+
+  discard_cleanups (c);
+}
+
+/* This function is a generic wrapper that is responsible for removing a
+   *point (i.e., calling `ptrace' in order to issue the request to the
+   kernel), and unregistering it internally at GDB.  */
+static void
+booke_remove_point (struct ppc_hw_breakpoint *b, int tid)
+{
+  int i;
+  long slot;
+  struct hw_break_tuple *hw_breaks;
+  struct thread_points *t;
+
+  t = booke_find_thread_points_by_tid (tid, 0);
+  gdb_assert (t != NULL);
+  hw_breaks = t->hw_breaks;
+
+  for (i = 0; i < max_slots_number; i++)
+    if (hw_breaks[i].hw_break && booke_cmp_hw_point (hw_breaks[i].hw_break, b))
+      break;
+
+  gdb_assert (i != max_slots_number);
+
+  errno = 0;
+  if (ptrace (PPC_PTRACE_DELHWDEBUG, tid, 0, hw_breaks[i].slot) < 0)
+    perror_with_name (_("Unexpected error deleting breakpoint or watchpoint"));
+
+  xfree (hw_breaks[i].hw_break);
+  hw_breaks[i].hw_break = NULL;
+}
 
-/* Set a watchpoint of type TYPE at address ADDR.  */
 static int
-ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+ppc_linux_insert_hw_breakpoint (struct gdbarch *gdbarch,
+				  struct bp_target_info *bp_tgt)
 {
   struct lwp_info *lp;
   ptid_t ptid;
-  long dabr_value;
-  long read_mode, write_mode;
 
-  if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
-  {
-  /* PowerPC 440 requires only the read/write flags to be passed
-     to the kernel.  */
-    read_mode  = 1;
-    write_mode = 2;
-  }
-  else
-  {
-  /* PowerPC 970 and other DABR-based processors are required to pass
-     the Breakpoint Translation bit together with the flags.  */
-    read_mode  = 5;
-    write_mode = 6;
-  }
-
-  dabr_value = addr & ~(read_mode | write_mode);
-  switch (rw)
-    {
-    case hw_read:
-      /* Set read and translate bits.  */
-      dabr_value |= read_mode;
-      break;
-    case hw_write:
-      /* Set write and translate bits.  */
-      dabr_value |= write_mode;
-      break;
-    case hw_access:
-      /* Set read, write and translate bits.  */
-      dabr_value |= read_mode | write_mode;
-      break;
+  if (!have_ptrace_new_debug_booke)
+    return -1;
+
+  ALL_LWPS (lp, ptid)
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) bp_tgt->placed_address;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      booke_insert_point (&p, TIDGET (ptid));
     }
 
-  saved_dabr_value = dabr_value;
+  return 0;
+}
+
+static int
+ppc_linux_remove_hw_breakpoint (struct gdbarch *gdbarch,
+				  struct bp_target_info *bp_tgt)
+{
+  struct lwp_info *lp;
+  ptid_t ptid;
+
+  if (!have_ptrace_new_debug_booke)
+    return -1;
 
   ALL_LWPS (lp, ptid)
-    if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
-      return -1;
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) bp_tgt->placed_address;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      booke_remove_point (&p, TIDGET (ptid));
+    }
 
   return 0;
 }
 
 static int
+get_trigger_type (int rw)
+{
+  int t;
+
+  if (rw == hw_read)
+    t = PPC_BREAKPOINT_TRIGGER_READ;
+  else if (rw == hw_write)
+    t = PPC_BREAKPOINT_TRIGGER_WRITE;
+  else
+    t = PPC_BREAKPOINT_TRIGGER_READ | PPC_BREAKPOINT_TRIGGER_WRITE;
+
+  return t;
+}
+
+static int
+ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+{
+  struct lwp_info *lp;
+  ptid_t ptid;
+  int ret = -1;
+
+  if (have_ptrace_new_debug_booke)
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = get_trigger_type (rw);
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) addr;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      ALL_LWPS (lp, ptid)
+	booke_insert_point (&p, TIDGET (ptid));
+
+      ret = 0;
+    }
+  else
+    {
+      long dabr_value;
+      long read_mode, write_mode;
+
+      if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+	{
+	  /* PowerPC 440 requires only the read/write flags to be passed
+	     to the kernel.  */
+	  read_mode  = 1;
+	  write_mode = 2;
+	}
+      else
+	{
+	  /* PowerPC 970 and other DABR-based processors are required to pass
+	     the Breakpoint Translation bit together with the flags.  */
+	  read_mode  = 5;
+	  write_mode = 6;
+	}
+
+      dabr_value = addr & ~(read_mode | write_mode);
+      switch (rw)
+	{
+	  case hw_read:
+	    /* Set read and translate bits.  */
+	    dabr_value |= read_mode;
+	    break;
+	  case hw_write:
+	    /* Set write and translate bits.  */
+	    dabr_value |= write_mode;
+	    break;
+	  case hw_access:
+	    /* Set read, write and translate bits.  */
+	    dabr_value |= read_mode | write_mode;
+	    break;
+	}
+
+      saved_dabr_value = dabr_value;
+
+      ALL_LWPS (lp, ptid)
+	if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
+	  return -1;
+
+      ret = 0;
+    }
+
+  return ret;
+}
+
+static int
 ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw)
 {
   struct lwp_info *lp;
   ptid_t ptid;
-  long dabr_value = 0;
+  int ret = -1;
 
-  saved_dabr_value = 0;
-  ALL_LWPS (lp, ptid)
-    if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
-      return -1;
-  return 0;
+  if (have_ptrace_new_debug_booke)
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = get_trigger_type (rw);
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) addr;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      ALL_LWPS (lp, ptid)
+	booke_remove_point (&p, TIDGET (ptid));
+
+      ret = 0;
+    }
+  else
+    {
+      saved_dabr_value = 0;
+      ALL_LWPS (lp, ptid)
+	if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
+	  return -1;
+
+      ret = 0;
+    }
+
+  return ret;
 }
 
 static void
 ppc_linux_new_thread (ptid_t ptid)
 {
-  ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value);
+  int tid = TIDGET (ptid);
+
+  if (have_ptrace_new_debug_booke)
+    {
+      int i;
+      struct thread_points *p;
+      struct hw_break_tuple *hw_breaks;
+
+      /* Get a list of breakpoints from any thread. */
+      p = VEC_last (thread_points_p, ppc_threads);
+      hw_breaks = p->hw_breaks;
+
+      /* Copy that thread's breakpoints and watchpoints to the new thread. */
+      for (i = 0; i < max_slots_number; i++)
+	if (hw_breaks[i].hw_break)
+	  booke_insert_point (hw_breaks[i].hw_break, tid);
+    }
+  else
+    ptrace (PTRACE_SET_DEBUGREG, tid, 0, saved_dabr_value);
 }
 
 static int
@@ -1408,6 +1802,29 @@
       || (siginfo_p->si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */)
     return 0;
 
+  if (have_ptrace_new_debug_booke)
+    {
+      int i;
+      struct thread_points *t;
+      struct hw_break_tuple *hw_breaks;
+      /* The index (or slot) of the *point is passed in the si_errno field.  */
+      int slot = siginfo_p->si_errno;
+
+      t = booke_find_thread_points_by_tid (TIDGET (inferior_ptid), 0);
+
+      /* Find out if this *point is a hardware breakpoint.
+	 If so, we should return 0.  */
+      if (t)
+	{
+	  hw_breaks = t->hw_breaks;
+	  for (i = 0; i < max_slots_number; i++)
+	   if (hw_breaks[i].hw_break && hw_breaks[i].slot == slot
+	       && hw_breaks[i].hw_break->trigger_type
+		    == PPC_BREAKPOINT_TRIGGER_EXECUTE)
+	     return 0;
+	}
+    }
+
   *addr_p = (CORE_ADDR) (uintptr_t) siginfo_p->si_addr;
   return 1;
 }
@@ -1426,7 +1843,10 @@
 {
   int mask;
 
-  if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+  if (have_ptrace_new_debug_booke
+      && ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+    return start <= addr && start + length >= addr;
+  else if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
     mask = 3;
   else
     mask = 7;
@@ -1595,6 +2015,20 @@
 	perror_with_name (_("Unable to fetch AltiVec registers"));
     }
 
+  /* Check for kernel support for new BOOKE debugging registers.  */
+  if (ptrace (PPC_PTRACE_GETHWDBGINFO, tid, 0, &booke_debug_info) >= 0)
+    {
+      have_ptrace_new_debug_booke = 1;
+      max_slots_number = booke_debug_info.num_instruction_bps
+	+ booke_debug_info.num_data_bps + booke_debug_info.num_condition_regs;
+    }
+  else
+    {
+      /* Old school interface and no new BOOKE registers support.  */
+      have_ptrace_new_debug_booke = 0;
+      memset (&booke_debug_info, 0, sizeof (struct ppc_debug_info));
+    }
+
   /* Power ISA 2.05 (implemented by Power 6 and newer processors) increases
      the FPSCR from 32 bits to 64 bits. Even though Power 7 supports this
      ISA version, it doesn't have PPC_FEATURE_ARCH_2_05 set, only
@@ -1643,8 +2077,10 @@
   t->to_fetch_registers = ppc_linux_fetch_inferior_registers;
   t->to_store_registers = ppc_linux_store_inferior_registers;
 
-  /* Add our watchpoint methods.  */
-  t->to_can_use_hw_breakpoint = ppc_linux_check_watch_resources;
+  /* Add our breakpoint/watchpoint methods.  */
+  t->to_can_use_hw_breakpoint = ppc_linux_can_use_hw_breakpoint;
+  t->to_insert_hw_breakpoint = ppc_linux_insert_hw_breakpoint;
+  t->to_remove_hw_breakpoint = ppc_linux_remove_hw_breakpoint;
   t->to_region_ok_for_hw_watchpoint = ppc_linux_region_ok_for_hw_watchpoint;
   t->to_insert_watchpoint = ppc_linux_insert_watchpoint;
   t->to_remove_watchpoint = ppc_linux_remove_watchpoint;

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Support the new BookE ptrace interface
  2010-01-16  3:09     ` Luis Machado
@ 2010-01-18  4:16       ` Joel Brobecker
  0 siblings, 0 replies; 14+ messages in thread
From: Joel Brobecker @ 2010-01-18  4:16 UTC (permalink / raw)
  To: Luis Machado; +Cc: Thiago Jung Bauermann, gdb-patches, Matt Tyrlik

> 2010-01-16  Sergio Durigan Junior  <sergiodj@linux.vnet.ibm.com>
>             Thiago Jung Bauermann  <bauerman@br.ibm.com>
> 
>         * ppc-linux-nat.c (PTRACE_GET_DEBUGREG): Update comment.
>         (PPC_PTRACE_GETWDBGINFO, PPC_PTRACE_SETHWDEBUG, PPC_PTRACE_DELHWDEBUG,
>         ppc_debug_info, PPC_DEBUG_FEATURE_INSN_BP_RANGE,
>         PPC_DEBUG_FEATURE_INSN_BP_MASK, PPC_DEBUG_FEATURE_DATA_BP_RANGE,
>         PPC_DEBUG_FEATURE_DATA_BP_MASK, ppc_hw_breakpoint,
>         PPC_BREAKPOINT_TRIGGER_EXECUTE, PPC_BREAKPOINT_TRIGGER READ,
>         PPC_BREAKPOINT_TRIGGER_WRITE, PPC_BREAKPOINT_TRIGGER_RW,
>         PPC_BREAKPOINT_MODE_EXACT PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE,
>         PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE, PPC_BREAKPOINT_MODE_MASK,
>         PPC_BREAKPOINT_CONDITION_NONE, PPC_BREAKPOINT_CONDITION_AND,
>         PPC_BREAKPOINT_CONDITION_EXACT, PPC_BREAKPOINT_CONDITION_OR,
>         PPC_BREAKPOINT_CONDITION_AND_OR, PPC_BREAKPOINT_CONDITION_BE_ALL,
>         PPC_BREAKPOINT_CONDITION_BE_SHIFT, PPC_BREAKPOINT_CONDITION_BE):
>         Define, in case <ptrace.h> doesn't provide it.
>         (have_ptrace_new_debug_booke): New global.
>         (ppc_linux_check_watch_resources): Renamed to ...
>         (ppc_linux_can_use_hw_breakpoint): ... this.  Handle BookE processors.
>         (booke_debug_info): New variable.
>         (max_slots_number): Ditto.
>         (hw_break_tuple): New struct.
>         (thread_points): Ditto.
>         (ppc_threads): New variable.
>         (PPC_DEBUG_CURRENT_VERSION): New define.
>         (ppc_linux_region_ok_for_hw_watchpoint): Handle BookE processors.
>         (booke_cmp_hw_point): New function.
>         (booke_find_thread_points_by_tid): Ditto.
>         (booke_insert_point): Ditto.
>         (booke_remove_point): Ditto.
>         (ppc_linux_insert_hw_breakpoint): Ditto.
>         (ppc_linux_remove_hw_breakpoint): Ditto.
>         (ppc_linux_insert_watchpoint): Handle BookE processors.
>         (ppc_linux_remove_watchpoint): Ditto.
>         (ppc_linux_new_thread): Ditto.
>         (ppc_linux_stopped_data_address): Ditto.
>         (ppc_linux_watchpoint_addr_within_range): Ditto.
>         (ppc_linux_read_description): Query the target to know if it
>         supports the new kernel BookE interface.
>         (_initialize_ppc_linux_nat): Initialize to_insert_hw_breakpoint and
>         to_remove_hw_breakpoint fields of the target operations struct.

Approved :).

Now the *real* fun can begin, right? (designing the support for h/w
accelerated watchpoint conditions)

;-)

-- 
Joel


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Support the new BookE ptrace interface
  2010-01-05  4:51   ` Joel Brobecker
  2010-01-15 17:41     ` Luis Machado
  2010-01-16  3:09     ` Luis Machado
@ 2010-02-11 17:06     ` Ulrich Weigand
  2010-02-22 20:27       ` Thiago Jung Bauermann
  2 siblings, 1 reply; 14+ messages in thread
From: Ulrich Weigand @ 2010-02-11 17:06 UTC (permalink / raw)
  To: Joel Brobecker
  Cc: Thiago Jung Bauermann, gdb-patches, Luis Machado, Matt Tyrlik

Joel Brobecker wrote:
> > 2009-12-31  Sergio Durigan Junior  <sergiodj@linux.vnet.ibm.com>
> > 	    Thiago Jung Bauermann  <bauerman@br.ibm.com>
> > 
> > 	* ppc-linux-nat.c (PTRACE_GET_DEBUGREG): Update comment.
> > 	(PPC_PTRACE_GETWDBGINFO, PPC_PTRACE_SETHWDEBUG, PPC_PTRACE_DELHWDEBUG,
> > 	ppc_debug_info, PPC_DEBUG_FEATURE_INSN_BP_RANGE,
> > 	PPC_DEBUG_FEATURE_INSN_BP_MASK, PPC_DEBUG_FEATURE_DATA_BP_RANGE,
> > 	PPC_DEBUG_FEATURE_DATA_BP_MASK, ppc_hw_breakpoint,
> > 	PPC_BREAKPOINT_TRIGGER_EXECUTE, PPC_BREAKPOINT_TRIGGER READ,
> > 	PPC_BREAKPOINT_TRIGGER_WRITE, PPC_BREAKPOINT_TRIGGER_RW,
> > 	PPC_BREAKPOINT_MODE_EXACT PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE,
> > 	PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE, PPC_BREAKPOINT_MODE_MASK,
> > 	PPC_BREAKPOINT_CONDITION_NONE, PPC_BREAKPOINT_CONDITION_AND,
> > 	PPC_BREAKPOINT_CONDITION_EXACT, PPC_BREAKPOINT_CONDITION_OR,
> > 	PPC_BREAKPOINT_CONDITION_AND_OR, PPC_BREAKPOINT_CONDITION_BE_ALL,
> > 	PPC_BREAKPOINT_CONDITION_BE_SHIFT, PPC_BREAKPOINT_CONDITION_BE):
> > 	Define, in case <ptrace.h> doesn't provide it.
> > 	(have_ptrace_new_debug_booke): New global.
> > 	(ppc_linux_check_watch_resources): Renamed to ...
> > 	(ppc_linux_can_use_hw_breakpoint): ... this.  Handle BookE processors.
> > 	(booke_debug_info): New variable.
> > 	(max_slots_number): Ditto.
> > 	(hw_break_tuple): New struct.
> > 	(thread_points): Ditto.
> > 	(ppc_threads): New variable.
> > 	(PPC_DEBUG_CURRENT_VERSION): New define.
> > 	(ppc_linux_region_ok_for_hw_watchpoint): Handle BookE processors.
> > 	(booke_cmp_hw_point): New function.
> > 	(booke_find_thread_points_by_tid): Ditto.
> > 	(booke_insert_point): Ditto.
> > 	(booke_remove_point): Ditto.
> > 	(ppc_linux_insert_hw_breakpoint): Ditto.
> > 	(ppc_linux_remove_hw_breakpoint): Ditto.
> > 	(ppc_linux_insert_watchpoint): Handle BookE processors.
> > 	(ppc_linux_remove_watchpoint): Ditto.
> > 	(ppc_linux_new_thread): Ditto.
> > 	(ppc_linux_stopped_data_address): Ditto.
> > 	(ppc_linux_watchpoint_addr_within_range): Ditto.
> > 	(ppc_linux_read_description): Query the target to know if it
> > 	supports the new kernel BookE interface.
> > 	(_initialize_ppc_linux_nat): Initialize to_insert_hw_breakpoint and
> > 	to_remove_hw_breakpoint fields of the target operations struct.
> 
> Still looks good to me :). Just a couple of nits, but otherwise approved. 
> It would still be nice if Ulrich had a chance to take a quick look ;-).

Looks good to me as well.  The only nit I've found is this:

@@ -1595,6 +2012,20 @@ ppc_linux_read_description (struct target_ops *ops)
 	perror_with_name (_("Unable to fetch AltiVec registers"));
     }
 
+  /* Check for kernel support for new BOOKE debugging registers.  */
+  if (ptrace (PPC_PTRACE_GETHWDBGINFO, tid, 0, &booke_debug_info) >= 0)
+    {
+      have_ptrace_new_debug_booke = 1;
+      max_slots_number = booke_debug_info.num_instruction_bps
+	+ booke_debug_info.num_data_bps + booke_debug_info.num_condition_regs;
+    }
+  else
+    {
+      /* Old school interface and no new BOOKE registers support.  */
+      have_ptrace_new_debug_booke = 0;
+      memset (&booke_debug_info, 0, sizeof (struct ppc_debug_info));
+    }
+
   /* Power ISA 2.05 (implemented by Power 6 and newer processors) increases
      the FPSCR from 32 bits to 64 bits. Even though Power 7 supports this
      ISA version, it doesn't have PPC_FEATURE_ARCH_2_05 set, only


Setting global variables like have_ptrace_new_debug_booke as an implicit
side effect of ppc_linux_read_description isn't really nice; it makes
assumptions about the sequence in which these routines are called that
may not be guaranteed after potential future changes ...

Why not use a routine to retrieve the info that is called whereever
necessary (the routine can internally cache the data and the fact
that it useless to try as kernel support is missing)?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Support the new BookE ptrace interface
  2010-02-11 17:06     ` Ulrich Weigand
@ 2010-02-22 20:27       ` Thiago Jung Bauermann
  2010-02-23 19:07         ` Ulrich Weigand
  0 siblings, 1 reply; 14+ messages in thread
From: Thiago Jung Bauermann @ 2010-02-22 20:27 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: Joel Brobecker, gdb-patches, Luis Machado, Matt Tyrlik

[-- Attachment #1: Type: Text/Plain, Size: 4282 bytes --]

On Thu 11 Feb 2010 15:06:34 Ulrich Weigand wrote:
> Joel Brobecker wrote:
> > Still looks good to me :). Just a couple of nits, but otherwise approved.
> > It would still be nice if Ulrich had a chance to take a quick look ;-).
> 
> Looks good to me as well.  The only nit I've found is this:
> 
> @@ -1595,6 +2012,20 @@ ppc_linux_read_description (struct target_ops *ops)
>  	perror_with_name (_("Unable to fetch AltiVec registers"));
>      }
> 
> +  /* Check for kernel support for new BOOKE debugging registers.  */
> +  if (ptrace (PPC_PTRACE_GETHWDBGINFO, tid, 0, &booke_debug_info) >= 0)
> +    {
> +      have_ptrace_new_debug_booke = 1;
> +      max_slots_number = booke_debug_info.num_instruction_bps
> +	+ booke_debug_info.num_data_bps + booke_debug_info.num_condition_regs;
> +    }
> +  else
> +    {
> +      /* Old school interface and no new BOOKE registers support.  */
> +      have_ptrace_new_debug_booke = 0;
> +      memset (&booke_debug_info, 0, sizeof (struct ppc_debug_info));
> +    }
> +
>    /* Power ISA 2.05 (implemented by Power 6 and newer processors)
>  increases the FPSCR from 32 bits to 64 bits. Even though Power 7 supports
>  this ISA version, it doesn't have PPC_FEATURE_ARCH_2_05 set, only
> 
> 
> Setting global variables like have_ptrace_new_debug_booke as an implicit
> side effect of ppc_linux_read_description isn't really nice; it makes
> assumptions about the sequence in which these routines are called that
> may not be guaranteed after potential future changes ...
> 
> Why not use a routine to retrieve the info that is called whereever
> necessary (the routine can internally cache the data and the fact
> that it useless to try as kernel support is missing)?

Good point. This version adds a have_ptrace_booke_interface function that
is called whenever we need to check for support of the booke ptrace
interface.

This patch was approved before the 7.1 branch was created, but I didn't
commit it then because the kernel support wasn't committed upstream yet.
It is in the linux-next tree as of last friday, and will appear in 2.6.34.
Can I commit this patch both for the 7.1 branch and HEAD?

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


2010-02-22  Sergio Durigan Junior  <sergiodj@linux.vnet.ibm.com>
	    Thiago Jung Bauermann  <bauerman@br.ibm.com>

	* ppc-linux-nat.c (PTRACE_GET_DEBUGREG): Update comment.
	(PPC_PTRACE_GETWDBGINFO, PPC_PTRACE_SETHWDEBUG, PPC_PTRACE_DELHWDEBUG,
	ppc_debug_info, PPC_DEBUG_FEATURE_INSN_BP_RANGE,
	PPC_DEBUG_FEATURE_INSN_BP_MASK, PPC_DEBUG_FEATURE_DATA_BP_RANGE,
	PPC_DEBUG_FEATURE_DATA_BP_MASK, ppc_hw_breakpoint,
	PPC_BREAKPOINT_TRIGGER_EXECUTE, PPC_BREAKPOINT_TRIGGER READ,
	PPC_BREAKPOINT_TRIGGER_WRITE, PPC_BREAKPOINT_TRIGGER_RW,
	PPC_BREAKPOINT_MODE_EXACT PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE,
	PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE, PPC_BREAKPOINT_MODE_MASK,
	PPC_BREAKPOINT_CONDITION_NONE, PPC_BREAKPOINT_CONDITION_AND,
	PPC_BREAKPOINT_CONDITION_EXACT, PPC_BREAKPOINT_CONDITION_OR,
	PPC_BREAKPOINT_CONDITION_AND_OR, PPC_BREAKPOINT_CONDITION_BE_ALL,
	PPC_BREAKPOINT_CONDITION_BE_SHIFT, PPC_BREAKPOINT_CONDITION_BE):
	Define, in case <ptrace.h> doesn't provide it.
	(have_ptrace_booke_interface): New function.
	(ppc_linux_check_watch_resources): Renamed to ...
	(ppc_linux_can_use_hw_breakpoint): ... this.  Handle BookE processors.
	(booke_debug_info): New variable.
	(max_slots_number): Ditto.
	(hw_break_tuple): New struct.
	(thread_points): Ditto.
	(ppc_threads): New variable.
	(PPC_DEBUG_CURRENT_VERSION): New define.
	(ppc_linux_region_ok_for_hw_watchpoint): Handle BookE processors.
	(booke_cmp_hw_point): New function.
	(booke_find_thread_points_by_tid): Ditto.
	(booke_insert_point): Ditto.
	(booke_remove_point): Ditto.
	(ppc_linux_insert_hw_breakpoint): Ditto.
	(ppc_linux_remove_hw_breakpoint): Ditto.
	(ppc_linux_insert_watchpoint): Handle BookE processors.
	(ppc_linux_remove_watchpoint): Ditto.
	(ppc_linux_new_thread): Ditto.
	(ppc_linux_stopped_data_address): Ditto.
	(ppc_linux_watchpoint_addr_within_range): Ditto.
	(ppc_linux_read_description): Query the target to know if it
	supports the new kernel BookE interface.
	(_initialize_ppc_linux_nat): Initialize to_insert_hw_breakpoint and
	to_remove_hw_breakpoint fields of the target operations struct.

[-- Attachment #2: ppc476-new-interface-2010-02-22.diff --]
[-- Type: text/x-patch, Size: 21814 bytes --]

commit 804c5998959d22f7aed1f063fb26181872bee99c
Author: Thiago Jung Bauermann <bauermann@hactar.cps.virtua.com.br>
Date:   Mon Dec 28 14:23:16 2009 -0200

    Support the new BookE ptrace interface.

diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 10ff73d..397406b 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -100,7 +100,8 @@
 #define PTRACE_SETEVRREGS 21
 #endif
 
-/* Similarly for the hardware watchpoint support.  */
+/* Similarly for the hardware watchpoint support.  These requests are used
+   when the BookE kernel interface is not available.  */
 #ifndef PTRACE_GET_DEBUGREG
 #define PTRACE_GET_DEBUGREG    25
 #endif
@@ -111,6 +112,73 @@
 #define PTRACE_GETSIGINFO    0x4202
 #endif
 
+/* These requests are used when the BookE kernel interface is available.
+   It exposes the additional debug features of BookE processors, such as
+   ranged breakpoints and watchpoints and hardware-accelerated condition
+   evaluation.  */
+#ifndef PPC_PTRACE_GETHWDBGINFO
+
+/* Not having PPC_PTRACE_GETHWDBGINFO defined means that the new BookE
+   interface is not present in ptrace.h, so we'll have to pretty much include
+   it all here so that the code at least compiles on older systems.  */
+#define PPC_PTRACE_GETHWDBGINFO 0x89
+#define PPC_PTRACE_SETHWDEBUG   0x88
+#define PPC_PTRACE_DELHWDEBUG   0x87
+
+struct ppc_debug_info
+{
+        uint32_t version;               /* Only version 1 exists to date */
+        uint32_t num_instruction_bps;
+        uint32_t num_data_bps;
+        uint32_t num_condition_regs;
+        uint32_t data_bp_alignment;
+        uint32_t sizeof_condition;      /* size of the DVC register */
+        uint64_t features;
+};
+
+/* Features will have bits indicating whether there is support for:  */
+#define PPC_DEBUG_FEATURE_INSN_BP_RANGE         0x1
+#define PPC_DEBUG_FEATURE_INSN_BP_MASK          0x2
+#define PPC_DEBUG_FEATURE_DATA_BP_RANGE         0x4
+#define PPC_DEBUG_FEATURE_DATA_BP_MASK          0x8
+
+struct ppc_hw_breakpoint
+{
+        uint32_t version;               /* currently, version must be 1 */
+        uint32_t trigger_type;          /* only some combinations allowed */
+        uint32_t addr_mode;             /* address match mode */
+        uint32_t condition_mode;        /* break/watchpoint condition flags */
+        uint64_t addr;                  /* break/watchpoint address */
+        uint64_t addr2;                 /* range end or mask */
+        uint64_t condition_value;       /* contents of the DVC register */
+};
+
+/* Trigger type.  */
+#define PPC_BREAKPOINT_TRIGGER_EXECUTE  0x1
+#define PPC_BREAKPOINT_TRIGGER_READ     0x2
+#define PPC_BREAKPOINT_TRIGGER_WRITE    0x4
+#define PPC_BREAKPOINT_TRIGGER_RW       0x6
+
+/* Address mode.  */
+#define PPC_BREAKPOINT_MODE_EXACT               0x0
+#define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE     0x1
+#define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE     0x2
+#define PPC_BREAKPOINT_MODE_MASK                0x3
+
+/* Condition mode.  */
+#define PPC_BREAKPOINT_CONDITION_NONE   0x0
+#define PPC_BREAKPOINT_CONDITION_AND    0x1
+#define PPC_BREAKPOINT_CONDITION_EXACT  0x1
+#define PPC_BREAKPOINT_CONDITION_OR     0x2
+#define PPC_BREAKPOINT_CONDITION_AND_OR 0x3
+#define PPC_BREAKPOINT_CONDITION_BE_ALL 0x00ff0000
+#define PPC_BREAKPOINT_CONDITION_BE_SHIFT       16
+#define PPC_BREAKPOINT_CONDITION_BE(n)  \
+        (1<<((n)+PPC_BREAKPOINT_CONDITION_BE_SHIFT))
+#endif /* PPC_PTRACE_GETHWDBGINFO */
+
+
+
 /* Similarly for the general-purpose (gp0 -- gp31)
    and floating-point registers (fp0 -- fp31).  */
 #ifndef PTRACE_GETREGS
@@ -1270,39 +1338,140 @@ store_ppc_registers (const struct regcache *regcache, int tid)
     store_spe_register (regcache, tid, -1);
 }
 
+/* Fetch the AT_HWCAP entry from the aux vector.  */
+unsigned long ppc_linux_get_hwcap (void)
+{
+  CORE_ADDR field;
+
+  if (target_auxv_search (&current_target, AT_HWCAP, &field))
+    return (unsigned long) field;
+
+  return 0;
+}
+
+/* The cached DABR value, to install in new threads.
+   This variable is used when we are dealing with non-BookE
+   processors.  */
+static long saved_dabr_value;
+
+/* Global structure that will store information about the available
+   features on this BookE processor.  */
+static struct ppc_debug_info booke_debug_info;
+
+/* Global variable that holds the maximum number of slots that the
+   kernel will use.  This is only used when the processor is BookE.  */
+static size_t max_slots_number = 0;
+
+struct hw_break_tuple
+{
+  long slot;
+  struct ppc_hw_breakpoint *hw_break;
+};
+
+/* This is an internal VEC created to store information about *points inserted
+   for each thread.  This is used for BookE processors.  */
+typedef struct thread_points
+  {
+    /* The TID to which this *point relates.  */
+    int tid;
+    /* Information about the *point, such as its address, type, etc.
+
+       Each element inside this vector corresponds to a hardware
+       breakpoint or watchpoint in the thread represented by TID.  The maximum
+       size of these vector is MAX_SLOTS_NUMBER.  If the hw_break element of
+       the tuple is NULL, then the position in the vector is free.  */
+    struct hw_break_tuple *hw_breaks;
+  } *thread_points_p;
+DEF_VEC_P (thread_points_p);
+
+VEC(thread_points_p) *ppc_threads = NULL;
+
+/* The version of the kernel interface that we will use if the processor is
+   BookE.  */
+#define PPC_DEBUG_CURRENT_VERSION 1
+
+/* Returns non-zero if we support the ptrace interface which enables
+   booke debugging resources.  */
 static int
-ppc_linux_check_watch_resources (int type, int cnt, int ot)
+have_ptrace_booke_interface (void)
 {
+  static int have_ptrace_booke_interface = -1;
   int tid;
-  ptid_t ptid = inferior_ptid;
 
-  /* 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.  */
-  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);
+  tid = TIDGET (inferior_ptid);
   if (tid == 0)
-    tid = PIDGET (ptid);
+    tid = PIDGET (inferior_ptid);
 
-  if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
-    return 0;
-  return 1;
+  if (have_ptrace_booke_interface == -1)
+    {
+      /* Check for kernel support for BOOKE debug registers.  */
+      if (ptrace (PPC_PTRACE_GETHWDBGINFO, tid, 0, &booke_debug_info) >= 0)
+	{
+	  have_ptrace_booke_interface = 1;
+	  max_slots_number = booke_debug_info.num_instruction_bps
+	    + booke_debug_info.num_data_bps + booke_debug_info.num_condition_regs;
+	}
+      else
+	{
+	  /* Old school interface and no BOOKE debug registers support.  */
+	  have_ptrace_booke_interface = 0;
+	  memset (&booke_debug_info, 0, sizeof (struct ppc_debug_info));
+	}
+    }
+
+  return have_ptrace_booke_interface;
 }
 
-/* Fetch the AT_HWCAP entry from the aux vector.  */
-unsigned long ppc_linux_get_hwcap (void)
+static int
+ppc_linux_can_use_hw_breakpoint (int type, int cnt, int ot)
 {
-  CORE_ADDR field;
+  int total_hw_wp, total_hw_bp;
 
-  if (target_auxv_search (&current_target, AT_HWCAP, &field))
-    return (unsigned long) field;
+  if (have_ptrace_booke_interface ())
+    {
+      /* For PPC BookE processors, the number of available hardware
+         watchpoints and breakpoints is stored at the booke_debug_info
+	 struct.  */
+      total_hw_bp = booke_debug_info.num_instruction_bps;
+      total_hw_wp = booke_debug_info.num_data_bps;
+    }
+  else
+    {
+      /* For PPC server processors, we accept 1 hardware watchpoint and 0
+	 hardware breakpoints.  */
+      total_hw_bp = 0;
+      total_hw_wp = 1;
+    }
 
-  return 0;
+  if (type == bp_hardware_watchpoint || type == bp_read_watchpoint
+      || type == bp_access_watchpoint || type == bp_watchpoint)
+    {
+      if (cnt > total_hw_wp)
+	return -1;
+    }
+  else if (type == bp_hardware_breakpoint)
+    {
+      if (cnt > total_hw_bp)
+	return -1;
+    }
+
+  if (!have_ptrace_booke_interface ())
+    {
+      int tid;
+      ptid_t ptid = inferior_ptid;
+
+      /* 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);
+
+      if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
+	return 0;
+    }
+
+  return 1;
 }
 
 static int
@@ -1312,89 +1481,341 @@ ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
   if (len <= 0)
     return 0;
 
+  /* The new BookE ptrace interface tells if there are alignment restrictions
+     for watchpoints in the processors.  In that case, we use that information
+     to determine the hardcoded watchable region for watchpoints.  */
+  if (have_ptrace_booke_interface ())
+    {
+      if (booke_debug_info.data_bp_alignment
+	  && (addr + len > (addr & ~(booke_debug_info.data_bp_alignment - 1))
+	      + booke_debug_info.data_bp_alignment))
+	  return 0;
+    }
   /* addr+len must fall in the 8 byte watchable region for DABR-based
-     processors.  DAC-based processors, like the PowerPC 440, will use
+     processors (i.e., server processors).  Without the new BookE ptrace
+     interface, DAC-based processors (i.e., embedded processors) will use
      addresses aligned to 4-bytes due to the way the read/write flags are
-     passed at the moment.  */
-  if (((ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
-      && (addr + len) > (addr & ~3) + 4)
-      || (addr + len) > (addr & ~7) + 8)
+     passed in the old ptrace interface.  */
+  else if (((ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+	   && (addr + len) > (addr & ~3) + 4)
+	   || (addr + len) > (addr & ~7) + 8)
     return 0;
 
   return 1;
 }
 
-/* The cached DABR value, to install in new threads.  */
-static long saved_dabr_value;
+/* This function compares two ppc_hw_breakpoint structs field-by-field.  */
+static inline int
+booke_cmp_hw_point (struct ppc_hw_breakpoint *a, struct ppc_hw_breakpoint *b)
+{
+  return (a->trigger_type       == b->trigger_type
+	  && a->addr_mode       == b->addr_mode
+	  && a->condition_mode  == b->condition_mode
+	  && a->addr            == b->addr
+	  && a->addr2           == b->addr2
+	  && a->condition_value == b->condition_value);
+}
+
+/* This function can be used to retrieve a thread_points by the TID of the
+   related process/thread.  If nothing has been found, and ALLOC_NEW is 0,
+   it returns NULL.  If ALLOC_NEW is non-zero, a new thread_points for the
+   provided TID will be created and returned.  */
+static struct thread_points *
+booke_find_thread_points_by_tid (int tid, int alloc_new)
+{
+  int i;
+  struct thread_points *t;
+
+  for (i = 0; VEC_iterate (thread_points_p, ppc_threads, i, t); i++)
+    if (t->tid == tid)
+      return t;
+
+  t = NULL;
+
+  /* Do we need to allocate a new point_item
+     if the wanted one does not exist?  */
+  if (alloc_new)
+    {
+      t = xmalloc (sizeof (struct thread_points));
+      t->hw_breaks = xzalloc (max_slots_number * sizeof (struct hw_break_tuple));
+      t->tid = tid;
+      VEC_safe_push (thread_points_p, ppc_threads, t);
+    }
+
+  return t;
+}
+
+/* This function is a generic wrapper that is responsible for inserting a
+   *point (i.e., calling `ptrace' in order to issue the request to the
+   kernel) and registering it internally in GDB.  */
+static void
+booke_insert_point (struct ppc_hw_breakpoint *b, int tid)
+{
+  int i;
+  long slot;
+  struct ppc_hw_breakpoint *p = xmalloc (sizeof (struct ppc_hw_breakpoint));
+  struct hw_break_tuple *hw_breaks;
+  struct cleanup *c = make_cleanup (xfree, p);
+  struct thread_points *t;
+  struct hw_break_tuple *tuple;
+
+  memcpy (p, b, sizeof (struct ppc_hw_breakpoint));
+
+  errno = 0;
+  slot = ptrace (PPC_PTRACE_SETHWDEBUG, tid, 0, p);
+  if (slot < 0)
+    perror_with_name (_("Unexpected error setting breakpoint or watchpoint"));
+
+  /* Everything went fine, so we have to register this *point.  */
+  t = booke_find_thread_points_by_tid (tid, 1);
+  gdb_assert (t != NULL);
+  hw_breaks = t->hw_breaks;
+
+  /* Find a free element in the hw_breaks vector.  */
+  for (i = 0; i < max_slots_number; i++)
+    if (hw_breaks[i].hw_break == NULL)
+      {
+	hw_breaks[i].slot = slot;
+	hw_breaks[i].hw_break = p;
+	break;
+      }
+
+  gdb_assert (i != max_slots_number);
+
+  discard_cleanups (c);
+}
+
+/* This function is a generic wrapper that is responsible for removing a
+   *point (i.e., calling `ptrace' in order to issue the request to the
+   kernel), and unregistering it internally at GDB.  */
+static void
+booke_remove_point (struct ppc_hw_breakpoint *b, int tid)
+{
+  int i;
+  struct hw_break_tuple *hw_breaks;
+  struct thread_points *t;
+
+  t = booke_find_thread_points_by_tid (tid, 0);
+  gdb_assert (t != NULL);
+  hw_breaks = t->hw_breaks;
+
+  for (i = 0; i < max_slots_number; i++)
+    if (hw_breaks[i].hw_break && booke_cmp_hw_point (hw_breaks[i].hw_break, b))
+      break;
+
+  gdb_assert (i != max_slots_number);
+
+  errno = 0;
+  if (ptrace (PPC_PTRACE_DELHWDEBUG, tid, 0, hw_breaks[i].slot) < 0)
+    perror_with_name (_("Unexpected error deleting breakpoint or watchpoint"));
+
+  xfree (hw_breaks[i].hw_break);
+  hw_breaks[i].hw_break = NULL;
+}
 
-/* Set a watchpoint of type TYPE at address ADDR.  */
 static int
-ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+ppc_linux_insert_hw_breakpoint (struct gdbarch *gdbarch,
+				  struct bp_target_info *bp_tgt)
 {
   struct lwp_info *lp;
   ptid_t ptid;
-  long dabr_value;
-  long read_mode, write_mode;
 
-  if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
-  {
-  /* PowerPC 440 requires only the read/write flags to be passed
-     to the kernel.  */
-    read_mode  = 1;
-    write_mode = 2;
-  }
-  else
-  {
-  /* PowerPC 970 and other DABR-based processors are required to pass
-     the Breakpoint Translation bit together with the flags.  */
-    read_mode  = 5;
-    write_mode = 6;
-  }
-
-  dabr_value = addr & ~(read_mode | write_mode);
-  switch (rw)
+  if (!have_ptrace_booke_interface ())
+    return -1;
+
+  ALL_LWPS (lp, ptid)
     {
-    case hw_read:
-      /* Set read and translate bits.  */
-      dabr_value |= read_mode;
-      break;
-    case hw_write:
-      /* Set write and translate bits.  */
-      dabr_value |= write_mode;
-      break;
-    case hw_access:
-      /* Set read, write and translate bits.  */
-      dabr_value |= read_mode | write_mode;
-      break;
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) bp_tgt->placed_address;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      booke_insert_point (&p, TIDGET (ptid));
     }
 
-  saved_dabr_value = dabr_value;
+  return 0;
+}
+
+static int
+ppc_linux_remove_hw_breakpoint (struct gdbarch *gdbarch,
+				  struct bp_target_info *bp_tgt)
+{
+  struct lwp_info *lp;
+  ptid_t ptid;
+
+  if (!have_ptrace_booke_interface ())
+    return -1;
 
   ALL_LWPS (lp, ptid)
-    if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
-      return -1;
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) bp_tgt->placed_address;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      booke_remove_point (&p, TIDGET (ptid));
+    }
 
   return 0;
 }
 
 static int
+get_trigger_type (int rw)
+{
+  int t;
+
+  if (rw == hw_read)
+    t = PPC_BREAKPOINT_TRIGGER_READ;
+  else if (rw == hw_write)
+    t = PPC_BREAKPOINT_TRIGGER_WRITE;
+  else
+    t = PPC_BREAKPOINT_TRIGGER_READ | PPC_BREAKPOINT_TRIGGER_WRITE;
+
+  return t;
+}
+
+static int
+ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+{
+  struct lwp_info *lp;
+  ptid_t ptid;
+  int ret = -1;
+
+  if (have_ptrace_booke_interface ())
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = get_trigger_type (rw);
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) addr;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      ALL_LWPS (lp, ptid)
+	booke_insert_point (&p, TIDGET (ptid));
+
+      ret = 0;
+    }
+  else
+    {
+      long dabr_value;
+      long read_mode, write_mode;
+
+      if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+	{
+	  /* PowerPC 440 requires only the read/write flags to be passed
+	     to the kernel.  */
+	  read_mode  = 1;
+	  write_mode = 2;
+	}
+      else
+	{
+	  /* PowerPC 970 and other DABR-based processors are required to pass
+	     the Breakpoint Translation bit together with the flags.  */
+	  read_mode  = 5;
+	  write_mode = 6;
+	}
+
+      dabr_value = addr & ~(read_mode | write_mode);
+      switch (rw)
+	{
+	  case hw_read:
+	    /* Set read and translate bits.  */
+	    dabr_value |= read_mode;
+	    break;
+	  case hw_write:
+	    /* Set write and translate bits.  */
+	    dabr_value |= write_mode;
+	    break;
+	  case hw_access:
+	    /* Set read, write and translate bits.  */
+	    dabr_value |= read_mode | write_mode;
+	    break;
+	}
+
+      saved_dabr_value = dabr_value;
+
+      ALL_LWPS (lp, ptid)
+	if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
+	  return -1;
+
+      ret = 0;
+    }
+
+  return ret;
+}
+
+static int
 ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw)
 {
   struct lwp_info *lp;
   ptid_t ptid;
-  long dabr_value = 0;
+  int ret = -1;
 
-  saved_dabr_value = 0;
-  ALL_LWPS (lp, ptid)
-    if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
-      return -1;
-  return 0;
+  if (have_ptrace_booke_interface ())
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = get_trigger_type (rw);
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) addr;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      ALL_LWPS (lp, ptid)
+	booke_remove_point (&p, TIDGET (ptid));
+
+      ret = 0;
+    }
+  else
+    {
+      saved_dabr_value = 0;
+      ALL_LWPS (lp, ptid)
+	if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
+	  return -1;
+
+      ret = 0;
+    }
+
+  return ret;
 }
 
 static void
 ppc_linux_new_thread (ptid_t ptid)
 {
-  ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value);
+  int tid = TIDGET (ptid);
+
+  if (have_ptrace_booke_interface ())
+    {
+      int i;
+      struct thread_points *p;
+      struct hw_break_tuple *hw_breaks;
+
+      /* Get a list of breakpoints from any thread. */
+      p = VEC_last (thread_points_p, ppc_threads);
+      hw_breaks = p->hw_breaks;
+
+      /* Copy that thread's breakpoints and watchpoints to the new thread. */
+      for (i = 0; i < max_slots_number; i++)
+	if (hw_breaks[i].hw_break)
+	  booke_insert_point (hw_breaks[i].hw_break, tid);
+    }
+  else
+    ptrace (PTRACE_SET_DEBUGREG, tid, 0, saved_dabr_value);
 }
 
 static int
@@ -1408,6 +1829,29 @@ ppc_linux_stopped_data_address (struct target_ops *target, CORE_ADDR *addr_p)
       || (siginfo_p->si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */)
     return 0;
 
+  if (have_ptrace_booke_interface ())
+    {
+      int i;
+      struct thread_points *t;
+      struct hw_break_tuple *hw_breaks;
+      /* The index (or slot) of the *point is passed in the si_errno field.  */
+      int slot = siginfo_p->si_errno;
+
+      t = booke_find_thread_points_by_tid (TIDGET (inferior_ptid), 0);
+
+      /* Find out if this *point is a hardware breakpoint.
+	 If so, we should return 0.  */
+      if (t)
+	{
+	  hw_breaks = t->hw_breaks;
+	  for (i = 0; i < max_slots_number; i++)
+	   if (hw_breaks[i].hw_break && hw_breaks[i].slot == slot
+	       && hw_breaks[i].hw_break->trigger_type
+		    == PPC_BREAKPOINT_TRIGGER_EXECUTE)
+	     return 0;
+	}
+    }
+
   *addr_p = (CORE_ADDR) (uintptr_t) siginfo_p->si_addr;
   return 1;
 }
@@ -1426,7 +1870,10 @@ ppc_linux_watchpoint_addr_within_range (struct target_ops *target,
 {
   int mask;
 
-  if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+  if (have_ptrace_booke_interface ()
+      && ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+    return start <= addr && start + length >= addr;
+  else if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
     mask = 3;
   else
     mask = 7;
@@ -1643,8 +2090,10 @@ _initialize_ppc_linux_nat (void)
   t->to_fetch_registers = ppc_linux_fetch_inferior_registers;
   t->to_store_registers = ppc_linux_store_inferior_registers;
 
-  /* Add our watchpoint methods.  */
-  t->to_can_use_hw_breakpoint = ppc_linux_check_watch_resources;
+  /* Add our breakpoint/watchpoint methods.  */
+  t->to_can_use_hw_breakpoint = ppc_linux_can_use_hw_breakpoint;
+  t->to_insert_hw_breakpoint = ppc_linux_insert_hw_breakpoint;
+  t->to_remove_hw_breakpoint = ppc_linux_remove_hw_breakpoint;
   t->to_region_ok_for_hw_watchpoint = ppc_linux_region_ok_for_hw_watchpoint;
   t->to_insert_watchpoint = ppc_linux_insert_watchpoint;
   t->to_remove_watchpoint = ppc_linux_remove_watchpoint;

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Support the new BookE ptrace interface
  2010-02-22 20:27       ` Thiago Jung Bauermann
@ 2010-02-23 19:07         ` Ulrich Weigand
  2010-02-23 23:42           ` Joel Brobecker
  2010-04-22 22:53           ` Thiago Jung Bauermann
  0 siblings, 2 replies; 14+ messages in thread
From: Ulrich Weigand @ 2010-02-23 19:07 UTC (permalink / raw)
  To: Thiago Jung Bauermann
  Cc: Joel Brobecker, gdb-patches, Luis Machado, Matt Tyrlik

Thiago Jung Bauermann wrote:

> Good point. This version adds a have_ptrace_booke_interface function that
> is called whenever we need to check for support of the booke ptrace
> interface.

Thanks!
 
> This patch was approved before the 7.1 branch was created, but I didn't
> commit it then because the kernel support wasn't committed upstream yet.
> It is in the linux-next tree as of last friday, and will appear in 2.6.34.
> Can I commit this patch both for the 7.1 branch and HEAD?

Another problem just occurred to me, sorry for not noticing on the last
review: It seems the memory allocated in ppc_linux_new_thread is never
freed, or is it?   Re-running the same inferior (with all different TID
values) would then leak a bit of memory ...

I guess this memory should be freed in a thread_exit notifier.

With this change, the patch would be OK for HEAD.
For the branch, Joel has the last word ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Support the new BookE ptrace interface
  2010-02-23 19:07         ` Ulrich Weigand
@ 2010-02-23 23:42           ` Joel Brobecker
  2010-02-25 18:16             ` Ulrich Weigand
  2010-04-22 22:53           ` Thiago Jung Bauermann
  1 sibling, 1 reply; 14+ messages in thread
From: Joel Brobecker @ 2010-02-23 23:42 UTC (permalink / raw)
  To: Ulrich Weigand
  Cc: Thiago Jung Bauermann, gdb-patches, Luis Machado, Matt Tyrlik

> With this change, the patch would be OK for HEAD.
> For the branch, Joel has the last word ...

The patch was approved pre 7.1 branching, but I feel a little uncomfortable
applying a patch of this size that late, so I'd like a second opinion.
If Ulrich is OK, then the patch can be applied to the branch as well.

-- 
Joel


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Support the new BookE ptrace interface
  2010-02-23 23:42           ` Joel Brobecker
@ 2010-02-25 18:16             ` Ulrich Weigand
  0 siblings, 0 replies; 14+ messages in thread
From: Ulrich Weigand @ 2010-02-25 18:16 UTC (permalink / raw)
  To: Joel Brobecker
  Cc: Thiago Jung Bauermann, gdb-patches, Luis Machado, Matt Tyrlik

Joel Brobecker wrote:
> > With this change, the patch would be OK for HEAD.
> > For the branch, Joel has the last word ...
> 
> The patch was approved pre 7.1 branching, but I feel a little uncomfortable
> applying a patch of this size that late, so I'd like a second opinion.
> If Ulrich is OK, then the patch can be applied to the branch as well.

I've discussed this with Thiago off-line, and we've agreed that this feature
is not strictly necessary for 7.1 at this point.  Therefore I agree that this
should not go into the branch, so as to avoid potential instability.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Support the new BookE ptrace interface
  2010-02-23 19:07         ` Ulrich Weigand
  2010-02-23 23:42           ` Joel Brobecker
@ 2010-04-22 22:53           ` Thiago Jung Bauermann
  1 sibling, 0 replies; 14+ messages in thread
From: Thiago Jung Bauermann @ 2010-04-22 22:53 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: Joel Brobecker, gdb-patches, Luis Machado

On Tue 23 Feb 2010 16:07:37 Ulrich Weigand wrote:
> Thiago Jung Bauermann wrote:
> > This patch was approved before the 7.1 branch was created, but I didn't
> > commit it then because the kernel support wasn't committed upstream yet.
> > It is in the linux-next tree as of last friday, and will appear in
> > 2.6.34. Can I commit this patch both for the 7.1 branch and HEAD?
> 
> Another problem just occurred to me, sorry for not noticing on the last
> review: It seems the memory allocated in ppc_linux_new_thread is never
> freed, or is it?   Re-running the same inferior (with all different TID
> values) would then leak a bit of memory ...
> 
> I guess this memory should be freed in a thread_exit notifier.
> 
> With this change, the patch would be OK for HEAD.
> For the branch, Joel has the last word ...

It's been a while, but I finally checked in this version of the patch.
It adds a thread_exit observer to fix the memory leak. It also fixes a
couple of bugs we found along the way.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


2010-04-14  Sergio Durigan Junior  <sergiodj@linux.vnet.ibm.com>
	    Thiago Jung Bauermann  <bauerman@br.ibm.com>

	* ppc-linux-nat.c (PTRACE_GET_DEBUGREG): Update comment.
	(PPC_PTRACE_GETWDBGINFO, PPC_PTRACE_SETHWDEBUG, PPC_PTRACE_DELHWDEBUG,
	ppc_debug_info, PPC_DEBUG_FEATURE_INSN_BP_RANGE,
	PPC_DEBUG_FEATURE_INSN_BP_MASK, PPC_DEBUG_FEATURE_DATA_BP_RANGE,
	PPC_DEBUG_FEATURE_DATA_BP_MASK, ppc_hw_breakpoint,
	PPC_BREAKPOINT_TRIGGER_EXECUTE, PPC_BREAKPOINT_TRIGGER READ,
	PPC_BREAKPOINT_TRIGGER_WRITE, PPC_BREAKPOINT_TRIGGER_RW,
	PPC_BREAKPOINT_MODE_EXACT PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE,
	PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE, PPC_BREAKPOINT_MODE_MASK,
	PPC_BREAKPOINT_CONDITION_NONE, PPC_BREAKPOINT_CONDITION_AND,
	PPC_BREAKPOINT_CONDITION_EXACT, PPC_BREAKPOINT_CONDITION_OR,
	PPC_BREAKPOINT_CONDITION_AND_OR, PPC_BREAKPOINT_CONDITION_BE_ALL,
	PPC_BREAKPOINT_CONDITION_BE_SHIFT, PPC_BREAKPOINT_CONDITION_BE):
	Define, in case <ptrace.h> doesn't provide it.
	(booke_debug_info): New variable.
	(max_slots_number): Ditto.
	(hw_break_tuple): New struct.
	(thread_points): Ditto.
	(ppc_threads): New variable.
	(PPC_DEBUG_CURRENT_VERSION): New define.
	(have_ptrace_new_debug_booke): New function.
	(ppc_linux_check_watch_resources): Renamed to ...
	(ppc_linux_can_use_hw_breakpoint): ... this.  Handle BookE processors.
	(ppc_linux_region_ok_for_hw_watchpoint): Handle BookE processors.
	(booke_cmp_hw_point): New function.
	(booke_find_thread_points_by_tid): Ditto.
	(booke_insert_point): Ditto.
	(booke_remove_point): Ditto.
	(ppc_linux_insert_hw_breakpoint): Ditto.
	(ppc_linux_remove_hw_breakpoint): Ditto.
	(get_trigger_type): Ditto.
	(ppc_linux_insert_watchpoint): Handle BookE processors.
	(ppc_linux_remove_watchpoint): Ditto.
	(ppc_linux_new_thread): Ditto.
	(ppc_linux_thread_exit): New function..
	(ppc_linux_stopped_data_address): Handle BookE processors.
	(ppc_linux_watchpoint_addr_within_range): Ditto.
	(_initialize_ppc_linux_nat): Initialize to_insert_hw_breakpoint and
	to_remove_hw_breakpoint fields of the target operations struct.
	Add observe for the thread_exit event.

diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 10ff73d..f7c5bf5 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -20,8 +20,10 @@
 
 #include "defs.h"
 #include "gdb_string.h"
+#include "observer.h"
 #include "frame.h"
 #include "inferior.h"
+#include "gdbthread.h"
 #include "gdbcore.h"
 #include "regcache.h"
 #include "gdb_assert.h"
@@ -100,7 +102,8 @@
 #define PTRACE_SETEVRREGS 21
 #endif
 
-/* Similarly for the hardware watchpoint support.  */
+/* Similarly for the hardware watchpoint support.  These requests are used
+   when the BookE kernel interface is not available.  */
 #ifndef PTRACE_GET_DEBUGREG
 #define PTRACE_GET_DEBUGREG    25
 #endif
@@ -111,6 +114,73 @@
 #define PTRACE_GETSIGINFO    0x4202
 #endif
 
+/* These requests are used when the BookE kernel interface is available.
+   It exposes the additional debug features of BookE processors, such as
+   ranged breakpoints and watchpoints and hardware-accelerated condition
+   evaluation.  */
+#ifndef PPC_PTRACE_GETHWDBGINFO
+
+/* Not having PPC_PTRACE_GETHWDBGINFO defined means that the new BookE
+   interface is not present in ptrace.h, so we'll have to pretty much include
+   it all here so that the code at least compiles on older systems.  */
+#define PPC_PTRACE_GETHWDBGINFO 0x89
+#define PPC_PTRACE_SETHWDEBUG   0x88
+#define PPC_PTRACE_DELHWDEBUG   0x87
+
+struct ppc_debug_info
+{
+        uint32_t version;               /* Only version 1 exists to date */
+        uint32_t num_instruction_bps;
+        uint32_t num_data_bps;
+        uint32_t num_condition_regs;
+        uint32_t data_bp_alignment;
+        uint32_t sizeof_condition;      /* size of the DVC register */
+        uint64_t features;
+};
+
+/* Features will have bits indicating whether there is support for:  */
+#define PPC_DEBUG_FEATURE_INSN_BP_RANGE         0x1
+#define PPC_DEBUG_FEATURE_INSN_BP_MASK          0x2
+#define PPC_DEBUG_FEATURE_DATA_BP_RANGE         0x4
+#define PPC_DEBUG_FEATURE_DATA_BP_MASK          0x8
+
+struct ppc_hw_breakpoint
+{
+        uint32_t version;               /* currently, version must be 1 */
+        uint32_t trigger_type;          /* only some combinations allowed */
+        uint32_t addr_mode;             /* address match mode */
+        uint32_t condition_mode;        /* break/watchpoint condition flags */
+        uint64_t addr;                  /* break/watchpoint address */
+        uint64_t addr2;                 /* range end or mask */
+        uint64_t condition_value;       /* contents of the DVC register */
+};
+
+/* Trigger type.  */
+#define PPC_BREAKPOINT_TRIGGER_EXECUTE  0x1
+#define PPC_BREAKPOINT_TRIGGER_READ     0x2
+#define PPC_BREAKPOINT_TRIGGER_WRITE    0x4
+#define PPC_BREAKPOINT_TRIGGER_RW       0x6
+
+/* Address mode.  */
+#define PPC_BREAKPOINT_MODE_EXACT               0x0
+#define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE     0x1
+#define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE     0x2
+#define PPC_BREAKPOINT_MODE_MASK                0x3
+
+/* Condition mode.  */
+#define PPC_BREAKPOINT_CONDITION_NONE   0x0
+#define PPC_BREAKPOINT_CONDITION_AND    0x1
+#define PPC_BREAKPOINT_CONDITION_EXACT  0x1
+#define PPC_BREAKPOINT_CONDITION_OR     0x2
+#define PPC_BREAKPOINT_CONDITION_AND_OR 0x3
+#define PPC_BREAKPOINT_CONDITION_BE_ALL 0x00ff0000
+#define PPC_BREAKPOINT_CONDITION_BE_SHIFT       16
+#define PPC_BREAKPOINT_CONDITION_BE(n)  \
+        (1<<((n)+PPC_BREAKPOINT_CONDITION_BE_SHIFT))
+#endif /* PPC_PTRACE_GETHWDBGINFO */
+
+
+
 /* Similarly for the general-purpose (gp0 -- gp31)
    and floating-point registers (fp0 -- fp31).  */
 #ifndef PTRACE_GETREGS
@@ -1270,39 +1340,141 @@ store_ppc_registers (const struct regcache *regcache, int tid)
     store_spe_register (regcache, tid, -1);
 }
 
+/* Fetch the AT_HWCAP entry from the aux vector.  */
+unsigned long ppc_linux_get_hwcap (void)
+{
+  CORE_ADDR field;
+
+  if (target_auxv_search (&current_target, AT_HWCAP, &field))
+    return (unsigned long) field;
+
+  return 0;
+}
+
+/* The cached DABR value, to install in new threads.
+   This variable is used when we are dealing with non-BookE
+   processors.  */
+static long saved_dabr_value;
+
+/* Global structure that will store information about the available
+   features on this BookE processor.  */
+static struct ppc_debug_info booke_debug_info;
+
+/* Global variable that holds the maximum number of slots that the
+   kernel will use.  This is only used when the processor is BookE.  */
+static size_t max_slots_number = 0;
+
+struct hw_break_tuple
+{
+  long slot;
+  struct ppc_hw_breakpoint *hw_break;
+};
+
+/* This is an internal VEC created to store information about *points inserted
+   for each thread.  This is used for BookE processors.  */
+typedef struct thread_points
+  {
+    /* The TID to which this *point relates.  */
+    int tid;
+    /* Information about the *point, such as its address, type, etc.
+
+       Each element inside this vector corresponds to a hardware
+       breakpoint or watchpoint in the thread represented by TID.  The maximum
+       size of these vector is MAX_SLOTS_NUMBER.  If the hw_break element of
+       the tuple is NULL, then the position in the vector is free.  */
+    struct hw_break_tuple *hw_breaks;
+  } *thread_points_p;
+DEF_VEC_P (thread_points_p);
+
+VEC(thread_points_p) *ppc_threads = NULL;
+
+/* The version of the kernel interface that we will use if the processor is
+   BookE.  */
+#define PPC_DEBUG_CURRENT_VERSION 1
+
+/* Returns non-zero if we support the ptrace interface which enables
+   booke debugging resources.  */
 static int
-ppc_linux_check_watch_resources (int type, int cnt, int ot)
+have_ptrace_booke_interface (void)
 {
-  int tid;
-  ptid_t ptid = inferior_ptid;
+  static int have_ptrace_booke_interface = -1;
 
-  /* 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.  */
-  if (cnt > 1)
-    return 0;
+  if (have_ptrace_booke_interface == -1)
+    {
+      int tid;
 
-  /* 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);
+      tid = TIDGET (inferior_ptid);
+      if (tid == 0)
+	tid = PIDGET (inferior_ptid);
 
-  if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
-    return 0;
-  return 1;
+      /* Check for kernel support for BOOKE debug registers.  */
+      if (ptrace (PPC_PTRACE_GETHWDBGINFO, tid, 0, &booke_debug_info) >= 0)
+	{
+	  have_ptrace_booke_interface = 1;
+	  max_slots_number = booke_debug_info.num_instruction_bps
+	    + booke_debug_info.num_data_bps + booke_debug_info.num_condition_regs;
+	}
+      else
+	{
+	  /* Old school interface and no BOOKE debug registers support.  */
+	  have_ptrace_booke_interface = 0;
+	  memset (&booke_debug_info, 0, sizeof (struct ppc_debug_info));
+	}
+    }
+
+  return have_ptrace_booke_interface;
 }
 
-/* Fetch the AT_HWCAP entry from the aux vector.  */
-unsigned long ppc_linux_get_hwcap (void)
+static int
+ppc_linux_can_use_hw_breakpoint (int type, int cnt, int ot)
 {
-  CORE_ADDR field;
+  int total_hw_wp, total_hw_bp;
 
-  if (target_auxv_search (&current_target, AT_HWCAP, &field))
-    return (unsigned long) field;
+  if (have_ptrace_booke_interface ())
+    {
+      /* For PPC BookE processors, the number of available hardware
+         watchpoints and breakpoints is stored at the booke_debug_info
+	 struct.  */
+      total_hw_bp = booke_debug_info.num_instruction_bps;
+      total_hw_wp = booke_debug_info.num_data_bps;
+    }
+  else
+    {
+      /* For PPC server processors, we accept 1 hardware watchpoint and 0
+	 hardware breakpoints.  */
+      total_hw_bp = 0;
+      total_hw_wp = 1;
+    }
 
-  return 0;
+  if (type == bp_hardware_watchpoint || type == bp_read_watchpoint
+      || type == bp_access_watchpoint || type == bp_watchpoint)
+    {
+      if (cnt > total_hw_wp)
+	return -1;
+    }
+  else if (type == bp_hardware_breakpoint)
+    {
+      if (cnt > total_hw_bp)
+	return -1;
+    }
+
+  if (!have_ptrace_booke_interface ())
+    {
+      int tid;
+      ptid_t ptid = inferior_ptid;
+
+      /* 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);
+
+      if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
+	return 0;
+    }
+
+  return 1;
 }
 
 static int
@@ -1312,69 +1484,278 @@ ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
   if (len <= 0)
     return 0;
 
+  /* The new BookE ptrace interface tells if there are alignment restrictions
+     for watchpoints in the processors.  In that case, we use that information
+     to determine the hardcoded watchable region for watchpoints.  */
+  if (have_ptrace_booke_interface ())
+    {
+      if (booke_debug_info.data_bp_alignment
+	  && (addr + len > (addr & ~(booke_debug_info.data_bp_alignment - 1))
+	      + booke_debug_info.data_bp_alignment))
+	  return 0;
+    }
   /* addr+len must fall in the 8 byte watchable region for DABR-based
-     processors.  DAC-based processors, like the PowerPC 440, will use
+     processors (i.e., server processors).  Without the new BookE ptrace
+     interface, DAC-based processors (i.e., embedded processors) will use
      addresses aligned to 4-bytes due to the way the read/write flags are
-     passed at the moment.  */
-  if (((ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
-      && (addr + len) > (addr & ~3) + 4)
-      || (addr + len) > (addr & ~7) + 8)
+     passed in the old ptrace interface.  */
+  else if (((ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+	   && (addr + len) > (addr & ~3) + 4)
+	   || (addr + len) > (addr & ~7) + 8)
     return 0;
 
   return 1;
 }
 
-/* The cached DABR value, to install in new threads.  */
-static long saved_dabr_value;
+/* This function compares two ppc_hw_breakpoint structs field-by-field.  */
+static inline int
+booke_cmp_hw_point (struct ppc_hw_breakpoint *a, struct ppc_hw_breakpoint *b)
+{
+  return (a->trigger_type       == b->trigger_type
+	  && a->addr_mode       == b->addr_mode
+	  && a->condition_mode  == b->condition_mode
+	  && a->addr            == b->addr
+	  && a->addr2           == b->addr2
+	  && a->condition_value == b->condition_value);
+}
+
+/* This function can be used to retrieve a thread_points by the TID of the
+   related process/thread.  If nothing has been found, and ALLOC_NEW is 0,
+   it returns NULL.  If ALLOC_NEW is non-zero, a new thread_points for the
+   provided TID will be created and returned.  */
+static struct thread_points *
+booke_find_thread_points_by_tid (int tid, int alloc_new)
+{
+  int i;
+  struct thread_points *t;
+
+  for (i = 0; VEC_iterate (thread_points_p, ppc_threads, i, t); i++)
+    if (t->tid == tid)
+      return t;
+
+  t = NULL;
+
+  /* Do we need to allocate a new point_item
+     if the wanted one does not exist?  */
+  if (alloc_new)
+    {
+      t = xmalloc (sizeof (struct thread_points));
+      t->hw_breaks = xzalloc (max_slots_number * sizeof (struct hw_break_tuple));
+      t->tid = tid;
+      VEC_safe_push (thread_points_p, ppc_threads, t);
+    }
+
+  return t;
+}
+
+/* This function is a generic wrapper that is responsible for inserting a
+   *point (i.e., calling `ptrace' in order to issue the request to the
+   kernel) and registering it internally in GDB.  */
+static void
+booke_insert_point (struct ppc_hw_breakpoint *b, int tid)
+{
+  int i;
+  long slot;
+  struct ppc_hw_breakpoint *p = xmalloc (sizeof (struct ppc_hw_breakpoint));
+  struct hw_break_tuple *hw_breaks;
+  struct cleanup *c = make_cleanup (xfree, p);
+  struct thread_points *t;
+  struct hw_break_tuple *tuple;
+
+  memcpy (p, b, sizeof (struct ppc_hw_breakpoint));
+
+  errno = 0;
+  slot = ptrace (PPC_PTRACE_SETHWDEBUG, tid, 0, p);
+  if (slot < 0)
+    perror_with_name (_("Unexpected error setting breakpoint or watchpoint"));
+
+  /* Everything went fine, so we have to register this *point.  */
+  t = booke_find_thread_points_by_tid (tid, 1);
+  gdb_assert (t != NULL);
+  hw_breaks = t->hw_breaks;
+
+  /* Find a free element in the hw_breaks vector.  */
+  for (i = 0; i < max_slots_number; i++)
+    if (hw_breaks[i].hw_break == NULL)
+      {
+	hw_breaks[i].slot = slot;
+	hw_breaks[i].hw_break = p;
+	break;
+      }
+
+  gdb_assert (i != max_slots_number);
+
+  discard_cleanups (c);
+}
+
+/* This function is a generic wrapper that is responsible for removing a
+   *point (i.e., calling `ptrace' in order to issue the request to the
+   kernel), and unregistering it internally at GDB.  */
+static void
+booke_remove_point (struct ppc_hw_breakpoint *b, int tid)
+{
+  int i;
+  struct hw_break_tuple *hw_breaks;
+  struct thread_points *t;
+
+  t = booke_find_thread_points_by_tid (tid, 0);
+  gdb_assert (t != NULL);
+  hw_breaks = t->hw_breaks;
+
+  for (i = 0; i < max_slots_number; i++)
+    if (hw_breaks[i].hw_break && booke_cmp_hw_point (hw_breaks[i].hw_break, b))
+      break;
+
+  gdb_assert (i != max_slots_number);
+
+  /* We have to ignore ENOENT errors because the kernel implements hardware
+     breakpoints/watchpoints as "one-shot", that is, they are automatically
+     deleted when hit.  */
+  errno = 0;
+  if (ptrace (PPC_PTRACE_DELHWDEBUG, tid, 0, hw_breaks[i].slot) < 0)
+    if (errno != ENOENT)
+      perror_with_name (_("Unexpected error deleting breakpoint or watchpoint"));
+
+  xfree (hw_breaks[i].hw_break);
+  hw_breaks[i].hw_break = NULL;
+}
 
-/* Set a watchpoint of type TYPE at address ADDR.  */
 static int
-ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+ppc_linux_insert_hw_breakpoint (struct gdbarch *gdbarch,
+				  struct bp_target_info *bp_tgt)
 {
+  ptid_t ptid;
   struct lwp_info *lp;
+  struct ppc_hw_breakpoint p;
+
+  if (!have_ptrace_booke_interface ())
+    return -1;
+
+  p.version         = PPC_DEBUG_CURRENT_VERSION;
+  p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
+  p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+  p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+  p.addr            = (uint64_t) bp_tgt->placed_address;
+  p.addr2           = 0;
+  p.condition_value = 0;
+
+  ALL_LWPS (lp, ptid)
+    booke_insert_point (&p, TIDGET (ptid));
+
+  return 0;
+}
+
+static int
+ppc_linux_remove_hw_breakpoint (struct gdbarch *gdbarch,
+				  struct bp_target_info *bp_tgt)
+{
   ptid_t ptid;
-  long dabr_value;
-  long read_mode, write_mode;
+  struct lwp_info *lp;
+  struct ppc_hw_breakpoint p;
 
-  if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
-  {
-  /* PowerPC 440 requires only the read/write flags to be passed
-     to the kernel.  */
-    read_mode  = 1;
-    write_mode = 2;
-  }
+  if (!have_ptrace_booke_interface ())
+    return -1;
+
+  p.version         = PPC_DEBUG_CURRENT_VERSION;
+  p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
+  p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+  p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+  p.addr            = (uint64_t) bp_tgt->placed_address;
+  p.addr2           = 0;
+  p.condition_value = 0;
+
+  ALL_LWPS (lp, ptid)
+    booke_remove_point (&p, TIDGET (ptid));
+
+  return 0;
+}
+
+static int
+get_trigger_type (int rw)
+{
+  int t;
+
+  if (rw == hw_read)
+    t = PPC_BREAKPOINT_TRIGGER_READ;
+  else if (rw == hw_write)
+    t = PPC_BREAKPOINT_TRIGGER_WRITE;
   else
-  {
-  /* PowerPC 970 and other DABR-based processors are required to pass
-     the Breakpoint Translation bit together with the flags.  */
-    read_mode  = 5;
-    write_mode = 6;
-  }
-
-  dabr_value = addr & ~(read_mode | write_mode);
-  switch (rw)
+    t = PPC_BREAKPOINT_TRIGGER_READ | PPC_BREAKPOINT_TRIGGER_WRITE;
+
+  return t;
+}
+
+static int
+ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+{
+  struct lwp_info *lp;
+  ptid_t ptid;
+  int ret = -1;
+
+  if (have_ptrace_booke_interface ())
     {
-    case hw_read:
-      /* Set read and translate bits.  */
-      dabr_value |= read_mode;
-      break;
-    case hw_write:
-      /* Set write and translate bits.  */
-      dabr_value |= write_mode;
-      break;
-    case hw_access:
-      /* Set read, write and translate bits.  */
-      dabr_value |= read_mode | write_mode;
-      break;
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = get_trigger_type (rw);
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) addr;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      ALL_LWPS (lp, ptid)
+	booke_insert_point (&p, TIDGET (ptid));
+
+      ret = 0;
     }
+  else
+    {
+      long dabr_value;
+      long read_mode, write_mode;
 
-  saved_dabr_value = dabr_value;
+      if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+	{
+	  /* PowerPC 440 requires only the read/write flags to be passed
+	     to the kernel.  */
+	  read_mode  = 1;
+	  write_mode = 2;
+	}
+      else
+	{
+	  /* PowerPC 970 and other DABR-based processors are required to pass
+	     the Breakpoint Translation bit together with the flags.  */
+	  read_mode  = 5;
+	  write_mode = 6;
+	}
 
-  ALL_LWPS (lp, ptid)
-    if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
-      return -1;
+      dabr_value = addr & ~(read_mode | write_mode);
+      switch (rw)
+	{
+	  case hw_read:
+	    /* Set read and translate bits.  */
+	    dabr_value |= read_mode;
+	    break;
+	  case hw_write:
+	    /* Set write and translate bits.  */
+	    dabr_value |= write_mode;
+	    break;
+	  case hw_access:
+	    /* Set read, write and translate bits.  */
+	    dabr_value |= read_mode | write_mode;
+	    break;
+	}
 
-  return 0;
+      saved_dabr_value = dabr_value;
+
+      ALL_LWPS (lp, ptid)
+	if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
+	  return -1;
+
+      ret = 0;
+    }
+
+  return ret;
 }
 
 static int
@@ -1382,19 +1763,96 @@ ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw)
 {
   struct lwp_info *lp;
   ptid_t ptid;
-  long dabr_value = 0;
+  int ret = -1;
 
-  saved_dabr_value = 0;
-  ALL_LWPS (lp, ptid)
-    if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
-      return -1;
-  return 0;
+  if (have_ptrace_booke_interface ())
+    {
+      struct ppc_hw_breakpoint p;
+
+      p.version         = PPC_DEBUG_CURRENT_VERSION;
+      p.trigger_type    = get_trigger_type (rw);
+      p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+      p.addr            = (uint64_t) addr;
+      p.addr2           = 0;
+      p.condition_value = 0;
+
+      ALL_LWPS (lp, ptid)
+	booke_remove_point (&p, TIDGET (ptid));
+
+      ret = 0;
+    }
+  else
+    {
+      saved_dabr_value = 0;
+      ALL_LWPS (lp, ptid)
+	if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
+	  return -1;
+
+      ret = 0;
+    }
+
+  return ret;
 }
 
 static void
 ppc_linux_new_thread (ptid_t ptid)
 {
-  ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value);
+  int tid = TIDGET (ptid);
+
+  if (have_ptrace_booke_interface ())
+    {
+      int i;
+      struct thread_points *p;
+      struct hw_break_tuple *hw_breaks;
+
+      if (VEC_empty (thread_points_p, ppc_threads))
+	return;
+
+      /* Get a list of breakpoints from any thread. */
+      p = VEC_last (thread_points_p, ppc_threads);
+      hw_breaks = p->hw_breaks;
+
+      /* Copy that thread's breakpoints and watchpoints to the new thread. */
+      for (i = 0; i < max_slots_number; i++)
+	if (hw_breaks[i].hw_break)
+	  booke_insert_point (hw_breaks[i].hw_break, tid);
+    }
+  else
+    ptrace (PTRACE_SET_DEBUGREG, tid, 0, saved_dabr_value);
+}
+
+static void
+ppc_linux_thread_exit (struct thread_info *tp, int silent)
+{
+  int i;
+  int tid = TIDGET (tp->ptid);
+  struct hw_break_tuple *hw_breaks;
+  struct thread_points *t = NULL, *p;
+
+  if (!have_ptrace_booke_interface ())
+    return;
+
+  for (i = 0; VEC_iterate (thread_points_p, ppc_threads, i, p); i++)
+    if (p->tid == tid)
+      {
+	t = p;
+	break;
+      }
+
+  if (t == NULL)
+    return;
+
+  VEC_unordered_remove (thread_points_p, ppc_threads, i);
+
+  hw_breaks = t->hw_breaks;
+
+  for (i = 0; i < max_slots_number; i++)
+    if (hw_breaks[i].hw_break)
+      xfree (hw_breaks[i].hw_break);
+
+  xfree (t->hw_breaks);
+  xfree (t);
 }
 
 static int
@@ -1408,6 +1866,29 @@ ppc_linux_stopped_data_address (struct target_ops *target, CORE_ADDR *addr_p)
       || (siginfo_p->si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */)
     return 0;
 
+  if (have_ptrace_booke_interface ())
+    {
+      int i;
+      struct thread_points *t;
+      struct hw_break_tuple *hw_breaks;
+      /* The index (or slot) of the *point is passed in the si_errno field.  */
+      int slot = siginfo_p->si_errno;
+
+      t = booke_find_thread_points_by_tid (TIDGET (inferior_ptid), 0);
+
+      /* Find out if this *point is a hardware breakpoint.
+	 If so, we should return 0.  */
+      if (t)
+	{
+	  hw_breaks = t->hw_breaks;
+	  for (i = 0; i < max_slots_number; i++)
+	   if (hw_breaks[i].hw_break && hw_breaks[i].slot == slot
+	       && hw_breaks[i].hw_break->trigger_type
+		    == PPC_BREAKPOINT_TRIGGER_EXECUTE)
+	     return 0;
+	}
+    }
+
   *addr_p = (CORE_ADDR) (uintptr_t) siginfo_p->si_addr;
   return 1;
 }
@@ -1426,7 +1907,10 @@ ppc_linux_watchpoint_addr_within_range (struct target_ops *target,
 {
   int mask;
 
-  if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+  if (have_ptrace_booke_interface ()
+      && ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+    return start <= addr && start + length >= addr;
+  else if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
     mask = 3;
   else
     mask = 7;
@@ -1643,8 +2127,10 @@ _initialize_ppc_linux_nat (void)
   t->to_fetch_registers = ppc_linux_fetch_inferior_registers;
   t->to_store_registers = ppc_linux_store_inferior_registers;
 
-  /* Add our watchpoint methods.  */
-  t->to_can_use_hw_breakpoint = ppc_linux_check_watch_resources;
+  /* Add our breakpoint/watchpoint methods.  */
+  t->to_can_use_hw_breakpoint = ppc_linux_can_use_hw_breakpoint;
+  t->to_insert_hw_breakpoint = ppc_linux_insert_hw_breakpoint;
+  t->to_remove_hw_breakpoint = ppc_linux_remove_hw_breakpoint;
   t->to_region_ok_for_hw_watchpoint = ppc_linux_region_ok_for_hw_watchpoint;
   t->to_insert_watchpoint = ppc_linux_insert_watchpoint;
   t->to_remove_watchpoint = ppc_linux_remove_watchpoint;
@@ -1655,6 +2141,8 @@ _initialize_ppc_linux_nat (void)
   t->to_read_description = ppc_linux_read_description;
   t->to_auxv_parse = ppc_linux_auxv_parse;
 
+  observer_attach_thread_exit (ppc_linux_thread_exit);
+
   /* Register the target.  */
   linux_nat_add_target (t);
   linux_nat_set_new_thread (t, ppc_linux_new_thread);


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2010-04-22 22:53 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-12-24  0:31 [PATCH 1/4] Support the new BookE ptrace interface Thiago Jung Bauermann
2009-12-29  6:12 ` Joel Brobecker
2009-12-30 19:51   ` Thiago Jung Bauermann
2010-01-04 17:33 ` Thiago Jung Bauermann
2010-01-05  4:51   ` Joel Brobecker
2010-01-15 17:41     ` Luis Machado
2010-01-16  3:09     ` Luis Machado
2010-01-18  4:16       ` Joel Brobecker
2010-02-11 17:06     ` Ulrich Weigand
2010-02-22 20:27       ` Thiago Jung Bauermann
2010-02-23 19:07         ` Ulrich Weigand
2010-02-23 23:42           ` Joel Brobecker
2010-02-25 18:16             ` Ulrich Weigand
2010-04-22 22:53           ` Thiago Jung Bauermann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox