From: Alan Stern <[email protected]>
This patch introduces virtual debug registers to used by the per-thread
structure ad wrapper routines to manage debug registers by process-related
functions.
[K.Prasad: Split-out from the bigger patch and minor changes following
re-basing]
Signed-off-by: K.Prasad <[email protected]>
Signed-off-by: Alan Stern <[email protected]>
---
arch/x86/include/asm/debugreg.h | 27 +++++++++++++++++++++++++++
arch/x86/include/asm/processor.h | 10 +++-------
2 files changed, 30 insertions(+), 7 deletions(-)
Index: linux-2.6-tip.hbkpt/arch/x86/include/asm/debugreg.h
===================================================================
--- linux-2.6-tip.hbkpt.orig/arch/x86/include/asm/debugreg.h
+++ linux-2.6-tip.hbkpt/arch/x86/include/asm/debugreg.h
@@ -49,6 +49,8 @@
#define DR_LOCAL_ENABLE_SHIFT 0 /* Extra shift to the local enable bit */
#define DR_GLOBAL_ENABLE_SHIFT 1 /* Extra shift to the global enable bit */
+#define DR_LOCAL_ENABLE (0x1) /* Local enable for reg 0 */
+#define DR_GLOBAL_ENABLE (0x2) /* Global enable for reg 0 */
#define DR_ENABLE_SIZE 2 /* 2 enable bits per register */
#define DR_LOCAL_ENABLE_MASK (0x55) /* Set local bits for all 4 regs */
@@ -67,4 +69,29 @@
#define DR_LOCAL_SLOWDOWN (0x100) /* Local slow the pipeline */
#define DR_GLOBAL_SLOWDOWN (0x200) /* Global slow the pipeline */
+/*
+ * HW breakpoint additions
+ */
+#ifdef __KERNEL__
+
+/* For process management */
+void flush_thread_hw_breakpoint(struct task_struct *tsk);
+int copy_thread_hw_breakpoint(struct task_struct *tsk,
+ struct task_struct *child, unsigned long clone_flags);
+void dump_thread_hw_breakpoint(struct task_struct *tsk, int u_debugreg[8]);
+void switch_to_thread_hw_breakpoint(struct task_struct *tsk);
+
+/* For CPU management */
+void load_debug_registers(void);
+static inline void disable_debug_registers(void)
+{
+ set_debugreg(0UL, 7);
+}
+
+/* For use by ptrace */
+unsigned long thread_get_debugreg(struct task_struct *tsk, int n);
+int thread_set_debugreg(struct task_struct *tsk, int n, unsigned long val);
+
+#endif /* __KERNEL__ */
+
#endif /* _ASM_X86_DEBUGREG_H */
Index: linux-2.6-tip.hbkpt/arch/x86/include/asm/processor.h
===================================================================
--- linux-2.6-tip.hbkpt.orig/arch/x86/include/asm/processor.h
+++ linux-2.6-tip.hbkpt/arch/x86/include/asm/processor.h
@@ -427,13 +427,9 @@ struct thread_struct {
unsigned long ip;
unsigned long fs;
unsigned long gs;
- /* Hardware debugging registers: */
- unsigned long debugreg0;
- unsigned long debugreg1;
- unsigned long debugreg2;
- unsigned long debugreg3;
- unsigned long debugreg6;
- unsigned long debugreg7;
+ /* Hardware breakpoint info */
+ unsigned long vdr6;
+ struct thread_hw_breakpoint *hw_breakpoint_info;
/* Fault info: */
unsigned long cr2;
unsigned long trap_no;
* [email protected] <[email protected]> wrote:
> This patch introduces virtual debug registers to used by the
> per-thread structure ad wrapper routines to manage debug
> registers by process-related functions.
this is somewhat confusing. It would be much clearer to name it
'user debug registers'.
and why is this:
> @@ -427,13 +427,9 @@ struct thread_struct {
> unsigned long ip;
> unsigned long fs;
> unsigned long gs;
> - /* Hardware debugging registers: */
> - unsigned long debugreg0;
> - unsigned long debugreg1;
> - unsigned long debugreg2;
> - unsigned long debugreg3;
> - unsigned long debugreg6;
> - unsigned long debugreg7;
> + /* Hardware breakpoint info */
> + unsigned long vdr6;
> + struct thread_hw_breakpoint *hw_breakpoint_info;
detached from thread_struct? There's a lot of complications
(alloc/free, locking, etc.) from this for no good reason - the
hardware-breakpoints info structure is alway per thread and is
quite small, so there's no reason not to embedd it directly
inside thread_struct.
That way we get its allocation and freeing logic for free in
essence.
Ingo
On Tue, 10 Mar 2009, Ingo Molnar wrote:
> and why is this:
>
> > @@ -427,13 +427,9 @@ struct thread_struct {
> > unsigned long ip;
> > unsigned long fs;
> > unsigned long gs;
> > - /* Hardware debugging registers: */
> > - unsigned long debugreg0;
> > - unsigned long debugreg1;
> > - unsigned long debugreg2;
> > - unsigned long debugreg3;
> > - unsigned long debugreg6;
> > - unsigned long debugreg7;
> > + /* Hardware breakpoint info */
> > + unsigned long vdr6;
> > + struct thread_hw_breakpoint *hw_breakpoint_info;
>
> detached from thread_struct? There's a lot of complications
> (alloc/free, locking, etc.) from this for no good reason - the
> hardware-breakpoints info structure is alway per thread and is
> quite small, so there's no reason not to embedd it directly
> inside thread_struct.
The only reason for separating it out was to avoid bogging down the
vast majority of threads which aren't debugged. If you think the extra
overhead isn't worth worrying about then the hw-breakpoint info
structure can be embedded.
Alan Stern
* Alan Stern <[email protected]> wrote:
> On Tue, 10 Mar 2009, Ingo Molnar wrote:
>
> > and why is this:
> >
> > > @@ -427,13 +427,9 @@ struct thread_struct {
> > > unsigned long ip;
> > > unsigned long fs;
> > > unsigned long gs;
> > > - /* Hardware debugging registers: */
> > > - unsigned long debugreg0;
> > > - unsigned long debugreg1;
> > > - unsigned long debugreg2;
> > > - unsigned long debugreg3;
> > > - unsigned long debugreg6;
> > > - unsigned long debugreg7;
> > > + /* Hardware breakpoint info */
> > > + unsigned long vdr6;
> > > + struct thread_hw_breakpoint *hw_breakpoint_info;
> >
> > detached from thread_struct? There's a lot of complications
> > (alloc/free, locking, etc.) from this for no good reason - the
> > hardware-breakpoints info structure is alway per thread and is
> > quite small, so there's no reason not to embedd it directly
> > inside thread_struct.
>
> The only reason for separating it out was to avoid bogging
> down the vast majority of threads which aren't debugged. If
> you think the extra overhead isn't worth worrying about then
> the hw-breakpoint info structure can be embedded.
yeah. This new facility is barely used, and such things should
always strive for 100% dumb simplicity. If the overhead of that
structure is ever a problem we can allocate it dynamically. (but
generally it's just not worth the pain)
Ingo
> detached from thread_struct? There's a lot of complications
> (alloc/free, locking, etc.) from this for no good reason - the
> hardware-breakpoints info structure is alway per thread and is
> quite small, so there's no reason not to embedd it directly
> inside thread_struct.
I do certainly think it's worthwhile to use a coherent struct type here
rather than many fields in thread_struct, independent of the allocation for
it being direct or indirect (not that you objected to that). That makes
the code read cleaner, and should make it a minor change to most of the
code later if the allocation plan changes.
I think in the original effort another motivating factor was concern about
bloating the size of thread_struct. The struct thread_hw_breakpoint is at
least a few times the size of the set old fields it replaces. We were
concerned that inflating every task's thread_struct for the benefit of
these rarely-used fancy new features might meet resistance from arch
maintainers like you. If that issue doesn't hold you back from taking the
new code, then I think we are more than happy to start with thread_struct
directly containing a struct thread_hw_breakpoint member.
I do think we'll want to make it a pointer with later incremental changes.
(But those may not need to come very soon.) Firstly, the size reduction to
task_struct is fairly compelling since it's for the vast majority of tasks
which never need to allocate it. The hair potential is really not very
much at least to begin with, if you just make it allocate on first setup
and never free (no locking et al, just if (thread->hwbkpt) ...). Second,
eventually we'd like to have the possibility of sharing the struct among
threads. This will come later on when we have higher-level things that
would like to set common watchpoints on a whole group of threads (what
debuggers usually really do). Such APIs are far improved and optimized by
updating many threads together, and when a big app has thousands of threads
to which all the same watchpoints apply, sharing at low level makes many of
those intraprocess context switches quicker. As I said, all in the future.
But it's far from being entirely baseless to think a pointer makes good sense.
Thanks,
Roland