This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, ppc] Fix hw *points for embedded ppc in a threaded environment.


Hi Jan,

On 08/06/2012 02:04 PM, Jan Kratochvil wrote:
On Mon, 06 Aug 2012 16:33:58 +0200, Luis Gustavo wrote:
With the somewhat recent booke kernel interface, more hw
watchpoints/breakpoints are available to GDB.

Which hardware can be BookE tested on? Red Hat has available for example PPC970FX, POWER5, POWER7 etc. but those AFAIK are not BookE. Or does it depend just on kernel?

The ones you listed are server processors, and they use the old kernel interface. Examples of BookE's are the 4xx family (405, 440, 460, 476 etc...)


The logic of replicating the existing process' debug state to the
new thread is still there though, but the new booke interface in the
kernel already replicates that state. More precisely, the kernel
gives the new thread the debug state of its parent thread.

I have been testing it recently for fork() (processes, no threads) and my results were:

kernel-3.3.4-5.fc17.ppc64 clears debug info in both parent (!) and child
kernel-2.6.18-308.el5.ppc64 copies debug info to child and keeps it in parent

Interesting. The kernel i was trying things on was 2.6.3x.


The behavior you mention in 3.3.4 sounds odd to me. It would be nice to hear from the kernel folks if this is expected.

Also, the ppc server kernel may differ from the booke one.



It's still unclear if the kernel is supposed to do this and i'm
chasing answers with the ppc linux kernel folks (https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-August/100083.html).
Nonetheless, the kernel is out and it has such behavior.

Yes, I agree GDB should cope with all these kinds of behavior.



--- gdb_head.orig/gdb/ppc-linux-nat.c	2012-08-06 11:02:12.538532628 -0300
+++ gdb_head/gdb/ppc-linux-nat.c	2012-08-06 11:04:38.486536320 -0300
@@ -2179,7 +2179,21 @@ ppc_linux_new_thread (struct lwp_info *l
        /* 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);
+	  {
+	    /* The ppc Linux kernel causes a thread to inherit its parent
+	       thread's debug state, and that includes any hardware
+	       watchpoints or breakpoints that the parent thread may have set.
+
+	       For this reason, the debug state of the new thread is cleared
+	       before trying to replicate any hardware watchpoints or
+	       breakpoints contained in other threads.  */
+
+	    /* The ppc debug resource accounting is done through "slots".
+	       Ask the kernel the deallocate this specific *point's slot.  */
+	    ptrace (PPC_PTRACE_DELHWDEBUG, tid, 0, hw_breaks[i].slot);
+
+	    booke_insert_point (hw_breaks[i].hw_break, tid);

I agree with the patch.


I was considering to call PPC_PTRACE_DELHWDEBUG unconditionally but that would
be probably an unneeded overhead.

Did you test that kernel really does not reorder the [i] slots during
inheritance into the new thread?

The tests i did indicate that the child thread inherits whatever slot is in use by the parent thread, and no reordering appears to happen. Otherwise we would need to do a complete cleanup of the debug state in GDB.


What i saw with hardware that has 2 watchpoint slots (440):

* Thread 1 creates a hw watch using slot 5 (first DAC register).
* Thread 1 creates Thread 2.
* GDB proceeds to replicate the hw watch from Thread 1 in Thread 2.
* The kernel replies with slot 6, which indicates that a new DAC has been used (and also indicates slot 5 is currently in use)
* Thread 2 creates Thread 3
* GDB proceeds to replicate both hw watches from Threads 1 and 2 in Thread 3.
* Kernel replies -1, indicating there are no more slots left to create a new hw watchpoint. This shows slots 5 and 6 are in use in Thread 3.




+	  }
      }
    else
      ptrace (PTRACE_SET_DEBUGREG, tid, 0, saved_dabr_value);


This would be nice to include before 7.5, as it's an annoying problem.

Does this fix affect existing testcases on BookE? Otherwise a new testcase would be appropriate.

I don't think it does. Hardware watchpoints haven't been reliably tested for embedded ppc's due to the variaty of debugging features between cpu's. Additionaly, this only shows up when using hw watchpoints with threaded inferiors.


An appropriate test would be to run many threads and create the maximum amount of hw watchpoints and see if that works fine, but GDB's debug resource accounting is still a little awkward

Luis


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]