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

On 1/25/06, Tom Rini <trini@xxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, Jan 24, 2006 at 10:30:13PM -0800, Jim Blandy wrote:
>
> > Index: linux-2.6.11/include/linux/kgdb.h
>
> Any chance you can speak unified diff? Please?  Thanks!

I have difficulties reading unified diffs, because they mix the
context with both old and new text.  If there are interesting control
flow changes, I get lost.

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
@@ -106,6 +106,24 @@ extern void sleeping_thread_to_gdb_regs(
  */
 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.
@@ -170,6 +188,27 @@ extern struct task_struct *kgdb_get_shad
 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 architecture
+ *      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
@@ -288,6 +288,12 @@ struct pt_regs __attribute__ ((weak))
        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'))
@@ -1388,7 +1394,29 @@ int kgdb_handle_exception(int ex_vector,
                                break;
                        }

-                       /* Followthrough to default processing */
+                        /* 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
@@ -255,53 +255,66 @@ int kgdb_arch_handle_exception(int e_vec
                               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;
 }

+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