Dan Doucette
2007-Dec-19 22:50 UTC
[Xen-devel] [PATCH][3 of 3] GDB serial port debugging: Changes to add SMP pausing, x86_64 register mappings for serial port GDB, and others.
Hello, This patch contains a number of changes to allow serial port debugging of Xen with GDB. Highlights: - Removed panics and smp stop calls in favour of an smp pause mechanism. - Added x86_64 register mapping for gdb serial protocol support. % diffstat gdb_patch arch/x86/gdbstub.c | 170 ++++++++++++++++++++++++++++++---------------- arch/x86/traps.c | 7 - arch/x86/x86_32/gdbstub.c | 75 ++++++++++++++++++++ arch/x86/x86_64/Makefile | 1 arch/x86/x86_64/gdbstub.c | 164 ++++++++++++++++++++++++++++++++++++++++++++ common/gdbstub.c | 46 +++++++----- common/keyhandler.c | 2 include/xen/gdbstub.h | 3 8 files changed, 386 insertions(+), 82 deletions(-) Signed-off-by: Dan Doucette doucette.daniel@gmail.com Dan. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Isaku Yamahata
2007-Dec-20 01:36 UTC
Re: [Xen-devel] [PATCH][3 of 3] GDB serial port debugging: Changes to add SMP pausing, x86_64 register mappings for serial port GDB, and others.
On Wed, Dec 19, 2007 at 02:50:45PM -0800, Dan Doucette wrote:> Hello,Hi.> diff -Nur --exclude=setup.c --exclude=nmi.c --exclude=''*.o'' --exclude=''*.s'' --exclude=''*.swp'' --exclude=compile.h --exclude=serial.c xen-unstable.hg/xen/arch/x86/gdbstub.c xen-unstable.hg.new/xen/arch/x86/gdbstub.c > --- xen-unstable.hg/xen/arch/x86/gdbstub.c 2007-12-19 14:18:45.000000000 -0800 > +++ xen-unstable.hg.new/xen/arch/x86/gdbstub.c 2007-12-19 09:21:29.000000000 -0800...> @@ -107,13 +67,107 @@ > void > gdb_arch_enter(struct cpu_user_regs *regs) > { > - /* nothing */ > + /* > + * We must pause all other CPUs. > + */ > + gdb_smp_pause(); > } > > void > gdb_arch_exit(struct cpu_user_regs *regs) > { > - /* nothing */ > + /* > + * Resume all CPUs. > + */ > + gdb_smp_resume(); > +} > + > +static void gdb_pause_this_cpu(void *unused) > +{ > + unsigned long flags; > + > + local_irq_save(flags); > + > + atomic_set(&gdb_cpu[smp_processor_id()].ack, 1); > + atomic_inc(&gdb_smp_paused_count); > + > + while (atomic_read(&gdb_cpu[smp_processor_id()].paused)) > + mdelay(1); > + > + atomic_dec(&gdb_smp_paused_count); > + atomic_set(&gdb_cpu[smp_processor_id()].ack, 0); > + > + /* Restore interrupts */ > + local_irq_restore(flags); > +} > + > +static void gdb_smp_pause(void) > +{ > + int timeout = 100; > + int cpu; > + > + for_each_online_cpu(cpu) > + { > + atomic_set(&gdb_cpu[cpu].ack, 0); > + atomic_set(&gdb_cpu[cpu].paused, 1); > + } > + > + atomic_set(&gdb_smp_paused_count, 0); > + > + smp_call_function(gdb_pause_this_cpu, NULL, /* dont wait! */0, 0); > + > + /* Wait 100ms for all other CPUs to enter pause loop */ > + while ( (atomic_read(&gdb_smp_paused_count) < (num_online_cpus() - 1)) > + && (timeout-- > 0) ) > + mdelay(1); > + > + if ( atomic_read(&gdb_smp_paused_count) < (num_online_cpus() - 1) ) > + { > + printk("GDB: Not all CPUs have paused, missing CPUs "); > + for_each_online_cpu(cpu) > + { > + if ( cpu == smp_processor_id() ) > + continue; > + > + if ( !atomic_read(&gdb_cpu[cpu].ack) ) > + { > + printk("%d ", cpu); > + } > + } > + printk("\n"); > + } > +} > + > +static void gdb_smp_resume(void) > +{ > + int cpu; > + int timeout = 100; > + > + for_each_online_cpu(cpu) > + { > + atomic_set(&gdb_cpu[cpu].paused, 0); > + } > + > + /* Make sure all CPUs resume */ > + while ( (atomic_read(&gdb_smp_paused_count) > 0) > + && (timeout-- > 0) ) > + mdelay(1); > + > + if ( atomic_read(&gdb_smp_paused_count) > 0 ) > + { > + printk("GDB: Not all CPUs have resumed execution, missing CPUs "); > + for_each_online_cpu(cpu) > + { > + if ( cpu == smp_processor_id() ) > + continue; > + > + if ( !atomic_read(&gdb_cpu[cpu].ack) ) > + { > + printk("%d ", cpu); > + } > + } > + printk("\n"); > + } > }Pausing/resuming cpus looks like arch generic. Could you move them to the common code?> diff -Nur --exclude=setup.c --exclude=nmi.c --exclude=''*.o'' --exclude=''*.s'' --exclude=''*.swp'' --exclude=compile.h --exclude=serial.c xen-unstable.hg/xen/common/keyhandler.c xen-unstable.hg.new/xen/common/keyhandler.c > --- xen-unstable.hg/xen/common/keyhandler.c 2007-12-19 14:18:45.000000000 -0800 > +++ xen-unstable.hg.new/xen/common/keyhandler.c 2007-12-19 09:21:29.000000000 -0800 > @@ -276,7 +276,7 @@ > static void do_debug_key(unsigned char key, struct cpu_user_regs *regs) > { > printk("''%c'' pressed -> trapping into debugger\n", key); > - (void)debugger_trap_fatal(0xf001, regs); > + (void)debugger_trap_fatal(TRAP_int3, regs); > nop(); /* Prevent the compiler doing tail call > optimisation, as that confuses xendbg a > bit. */TRAP_int3 is x86 specific. Please define a constant like TRAP_gdbstab in the arch header and use it.> diff -Nur --exclude=setup.c --exclude=nmi.c --exclude=''*.o'' --exclude=''*.s'' --exclude=''*.swp'' --exclude=compile.h --exclude=serial.c xen-unstable.hg/xen/include/xen/gdbstub.h xen-unstable.hg.new/xen/include/xen/gdbstub.h > --- xen-unstable.hg/xen/include/xen/gdbstub.h 2007-12-19 14:18:46.000000000 -0800 > +++ xen-unstable.hg.new/xen/include/xen/gdbstub.h 2007-12-19 09:21:29.000000000 -0800 > @@ -69,6 +69,9 @@ > struct cpu_user_regs *regs, const char* buf, struct gdb_context *ctx); > void gdb_arch_read_reg( > unsigned long regnum, struct cpu_user_regs *regs, struct gdb_context *ctx); > +void gdb_arch_write_reg( > + unsigned long regnum, unsigned long val, struct cpu_user_regs *regs, > + struct gdb_context *ctx); > unsigned int gdb_arch_copy_from_user( > void *dest, const void *src, unsigned len); > unsigned int gdb_arch_copy_to_user(Adding gdb_arch_write_reg() breaks other arch (IA64 and power). How about adding the default definition which just returns as a weak symbol? -- yamahata _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Dec-20 09:39 UTC
Re: [Xen-devel] [PATCH][3 of 3] GDB serial port debugging: Changes to add SMP pausing, x86_64 register mappings for serial port GDB, and others.
On 20/12/07 01:36, "Isaku Yamahata" <yamahata@valinux.co.jp> wrote:> Adding gdb_arch_write_reg() breaks other arch (IA64 and power). > How about adding the default definition which just returns > as a weak symbol?Just add the no-op implementation to arch/{ia64,powerpc}/gdbstub.c. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Dan Doucette
2007-Dec-20 20:58 UTC
Re: [Xen-devel] [PATCH][3 of 3] GDB serial port debugging: Changes to add SMP pausing, x86_64 register mappings for serial port GDB, and others.
Hello,>> Pausing/resuming cpus looks like arch generic. >> Could you move them to the common code?Yes. Moved to common/gdbstub.c.>> TRAP_int3 is x86 specific. >> Please define a constant like TRAP_gdbstab in the arch header and use it.The value of the vector is not used, yet. I just removed this change from the patch.>> Adding gdb_arch_write_reg() breaks other arch (IA64 and power). >> How about adding the default definition which just returns >> as a weak symbol?>>> Just add the no-op implementation to arch/{ia64,powerpc}/gdbstub.c.I added the no-op implementations to ia64 and powerpc. Dan. On Dec 20, 2007 1:39 AM, Keir Fraser <Keir.Fraser@cl.cam.ac.uk> wrote:> > > > On 20/12/07 01:36, "Isaku Yamahata" <yamahata@valinux.co.jp> wrote: > > > Adding gdb_arch_write_reg() breaks other arch (IA64 and power). > > How about adding the default definition which just returns > > as a weak symbol? > > Just add the no-op implementation to arch/{ia64,powerpc}/gdbstub.c. > > -- Keir > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel