Separate 'continue' and 'step' handling from other remote packet handling

I'm working on getting GDB tracepoints working with KGDB.  For a
general explanation of tracepoints, see:

http://sources.redhat.com/gdb/talks/esc-west-1999/

When the kernel hits a tracepoint, KGDB logs some data of the user's
choosing, and then immediately continues, without talking to GDB. 
This means that KGDB itself has to handle the process of stepping off
a breakpoint (i.e., remove breakpoint; single-step; replace
breakpoint; continue); in the past, KGDB simply let GDB take care of
all that.

Anyway, it's a pain that the 'single step' and 'continue'
functionality is wrapped up in kgdb_arch_handle_exception.  To do a
single-step, I'd have to fake up an 's' packet and pass it to
kgdb_arch_handle_exception, which seems dopey.  And then there's the
fact that you've got the 'c' and 's' packet parsing replicated in
every per-arch patch.

So would something generally like the below be welcome?  This is just
a sketch, it's untested; I wanted to see what folks thought before I
invested much in it.

Similar changes to those shown for the i386 would be needed for the
other architectures.  Or, if you want to avoid retrofitting all the
architectures, I could make kgdb_continue optional as well, and fall
back to letting kgdb_arch_handle_exception handle 's' and 'c' packets.
 But I don't mind doing the retrofitting, if someone can help me test
them.

Index: linux-2.6.11/include/linux/kgdb.h
===================================================================
*** linux-2.6.11.orig/include/linux/kgdb.h
--- linux-2.6.11/include/linux/kgdb.h
*************** extern void sleeping_thread_to_gdb_regs(
*** 106,111 ****
--- 106,129 ----
   */
  extern void gdb_regs_to_regs(unsigned long *gdb_regs, struct pt_regs *regs);

+
+ /**     kgdb_continue - set process up for continuing
+  *      @regs - registers for the process
+  *      @at_addr - true if address to continue at is given
+  *      @addr - if @at_addr is true, address to continue at
+  *
+  *      Make any changes to the processor state in @regs needed to
+  *      resume execution of that thread.  (Clear single-step flag
+  *      bits, etc.)  If @at_addr is zero, continue at whatever
+  *      location the PC in @regs suggests.  If @at_addr is non-zero,
+  *      however, continue at @addr.  (We could use @addr == 0 to
+  *      indicate "continue at present location", but it's nice to
+  *      allow GDB to actually say "continue at address zero".)
+  */
+ extern void kgdb_continue (struct pt_regs *regs,
+                            int at_addr,
+                            unsigned long addr);
+
  /**
   *    kgdb_arch_handle_exception - Handle architecture specific GDB packets.
   *    @vector: The error vector of the exception that happened.
*************** extern struct task_struct *kgdb_get_shad
*** 170,175 ****
--- 188,214 ----
  extern struct pt_regs *kgdb_shadow_regs(struct pt_regs *regs, int threadid);

  /**
+  *      kgdb_single_step - set process up for single-stepping
+  *      @regs - registers for the process
+  *      @at_addr - true if address to step at is given
+  *      @addr - if @at_addr is true, address to step at
+  *
+  *      Make any changes to the processor state in @regs needed to
+  *      step a single instruction.  (Set single-step flag bits, etc.)
+  *      If @at_addr is zero, continue at whatever location the PC in
+  *      @regs suggests.  If @at_addr is non-zero, however, continue at
+  *      @addr.  (We could use @addr == 0 to indicate "continue at
+  *      present location", but it's nice to allow GDB to actually say
+  *      "continue at address zero".)
+  *
+  *      On success, return a positive value.  If the current target
+  *      doesn't implement single-stepping, return -EINVAL.
+  */
+ extern int kgdb_single_step (struct pt_regs *regs,
+                              int at_addr,
+                              unsigned long addr);
+
+ /**
   * struct kgdb_arch - Desribe architecture specific values.
   * @gdb_bpt_instr: The instruction to trigger a breakpoint.
   * @flags: Flags for the breakpoint, currently just %KGDB_HW_BREAKPOINT.
Index: linux-2.6.11/kernel/kgdb.c
===================================================================
*** linux-2.6.11.orig/kernel/kgdb.c
--- linux-2.6.11/kernel/kgdb.c
*************** struct pt_regs __attribute__ ((weak))
*** 288,293 ****
--- 288,299 ----
        return NULL;
  }

+ extern int __attribute__ ((weak))
+     kgdb_single_step (struct pt_regs *regs, int at_addr, unsigned long addr)
+ {
+         return -EINVAL;
+ }
+
  static int hex(char ch)
  {
        if ((ch >= 'a') && (ch <= 'f'))
*************** int kgdb_handle_exception(int ex_vector,
*** 1388,1394 ****
                                break;
                        }

!                       /* Followthrough to default processing */
                default:
                      default_handle:
                        error = kgdb_arch_handle_exception(ex_vector, signo,
--- 1394,1422 ----
                                break;
                        }

!                         /* try to read optional parameter, pc
!                          * unchanged if no parm */
!                         ptr = &remcom_in_buffer[1];
!                         length = kgdb_hex2long(&ptr, &addr);
!                         if (*ptr != '\0') {
!                                 error_packet (remcom_out_buffer, -EINVAL);
!                                 break;
!                         }
!
!                         if (remcom_in_buffer[0] == 'c')
!                                 kgdb_continue (linux_regs, length, addr);
!                         else if ((error = kgdb_single_step
(linux_regs, length,
!                                                             addr))
!                                  < 0) {
!                                 if (error != -EINVAL)
!                                         error_packet
(remcom_out_buffer, error);
!                                 /* Otherwise, leave it blank, which
!                                  * means "unrecognized".  */
!                                 break;
!                         }
!
!                         goto kgdb_exit;
!
                default:
                      default_handle:
                        error = kgdb_arch_handle_exception(ex_vector, signo,
Index: linux-2.6.11/arch/i386/kernel/kgdb.c
===================================================================
*** linux-2.6.11.orig/arch/i386/kernel/kgdb.c
--- linux-2.6.11/arch/i386/kernel/kgdb.c
*************** int kgdb_arch_handle_exception(int e_vec
*** 255,307 ****
                               char *remcom_out_buffer,
                               struct pt_regs *linux_regs)
  {
-       long addr;
-       char *ptr;
-       int newPC, dr6;
-
-       switch (remcom_in_buffer[0]) {
-       case 'c':
-       case 's':
-               /* try to read optional parameter, pc unchanged if no parm */
-               ptr = &remcom_in_buffer[1];
-               if (kgdb_hex2long(&ptr, &addr))
-                       linux_regs->eip = addr;
-               newPC = linux_regs->eip;
-
-               /* clear the trace bit */
-               linux_regs->eflags &= ~TF_MASK;
-               atomic_set(&cpu_doing_single_step, -1);
-
-               /* set the trace bit if we're stepping */
-               if (remcom_in_buffer[0] == 's') {
-                       linux_regs->eflags |= TF_MASK;
-                       debugger_step = 1;
-                       if (kgdb_contthread)
-                               atomic_set(&cpu_doing_single_step,
-                                          smp_processor_id());
-               }
-
-               asm volatile ("movl %%db6, %0\n":"=r" (dr6));
-               if (!(dr6 & 0x4000)) {
-                       long breakno;
-                       for (breakno = 0; breakno < 4; ++breakno) {
-                               if (dr6 & (1 << breakno) &&
-                                   breakinfo[breakno].type == 0) {
-                                       /* Set restore flag */
-                                       linux_regs->eflags |= X86_EFLAGS_RF;
-                                       break;
-                               }
-                       }
-               }
-               kgdb_correct_hw_break();
-               asm volatile ("movl %0, %%db6\n"::"r" (0));
-
-               return (0);
-       }                       /* switch */
        /* this means that we do not want to exit from the handler */
        return -1;
  }

  /* Register KGDB with the i386die_chain so that we hook into all of the right
   * spots. */
  static int kgdb_notify(struct notifier_block *self, unsigned long cmd,
--- 255,320 ----
                               char *remcom_out_buffer,
                               struct pt_regs *linux_regs)
  {
        /* this means that we do not want to exit from the handler */
        return -1;
  }

+ static void check_for_restore (struct pt_regs *regs)
+ {
+         int dr6;
+
+         asm volatile ("movl %%db6, %0\n":"=r" (dr6));
+         if (!(dr6 & 0x4000)) {
+                 long breakno;
+                 for (breakno = 0; breakno < 4; ++breakno) {
+                         if (dr6 & (1 << breakno) &&
+                             breakinfo[breakno].type == 0) {
+                                 /* Set restore flag */
+                                 regs->eflags |= X86_EFLAGS_RF;
+                                 break;
+                         }
+                 }
+         }
+ }
+
+ void kgdb_continue(struct pt_regs *regs,
+                    int at_addr,
+                    unsigned long addr)
+ {
+         if (at_addr)
+                 regs->eip = addr;
+
+         /* clear the trace bit */
+         regs->eflags &= ~TF_MASK;
+         atomic_set(&cpu_doing_single_step, -1);
+
+         check_for_restore (regs);
+         kgdb_correct_hw_break();
+         asm volatile ("movl %0, %%db6\n"::"r" (0));
+ }
+
+ int kgdb_single_step (struct pt_regs *regs,
+                       int at_addr,
+                       unsigned long addr)
+ {
+         /* set the trace bit */
+         regs->eflags |= TF_MASK;
+         debugger_step = 1;
+
+         if (kgdb_contthread)
+                 atomic_set(&cpu_doing_single_step, smp_processor_id());
+         else
+                 atomic_set(&cpu_doing_single_step, -1);
+
+         check_for_restore (regs);
+         kgdb_correct_hw_break();
+         asm volatile ("movl %0, %%db6\n"::"r" (0));
+
+         return 0;
+ }
+
+
+
  /* Register KGDB with the i386die_chain so that we hook into all of the right
   * spots. */
  static int kgdb_notify(struct notifier_block *self, unsigned long cmd,


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid3432&bid#0486&dat1642