WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-ia64-devel

Re: [Xen-devel] [PATCH][3 of 3] GDB serial port debugging: Changes to ad

To: Dan Doucette <doucette.daniel@xxxxxxxxx>
Subject: 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.
From: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
Date: Thu, 20 Dec 2007 10:36:27 +0900
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Wed, 19 Dec 2007 17:37:12 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <9dd432e50712191450h67f92cfaq160eca7f3e534f2a@xxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <9dd432e50712191450h67f92cfaq160eca7f3e534f2a@xxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.4.2.1i
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

<Prev in Thread] Current Thread [Next in Thread>