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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home