RE: [Xen-devel] [PATCH] fix the bug of gdb which debugs xen.

[Caz Yokoyama <cazyokoyama@xxxxxxxxx>]

Hello Keir,
Having multiple "Waiting for GDB to attach...\n" on single step is too
noisy.
diff -r 5619bed51ec4 xen/common/gdbstub.c
--- a/xen/common/gdbstub.c      Fri Aug 14 17:26:23 2009 +0100
+++ b/xen/common/gdbstub.c      Fri Aug 14 12:31:42 2009 -0700
@@ -608,9 +608,11 @@
     watchdog_disable();
     console_start_sync();
 
+#if 0
     /* Shouldn't really do this, but otherwise we stop for no
        obvious reason, which is Bad */
     printk("Waiting for GDB to attach...\n");
+#endif
 
     gdb_arch_enter(regs);
     gdb_ctx->signum = gdb_arch_signal_num(regs, cookie);
-caz

-----Original Message-----
From: Caz Yokoyama [mailto:caz@xxxxxxxxxxx] 
Sent: Thursday, August 13, 2009 8:52 PM
To: 'Keir Fraser'; 'xen-devel@xxxxxxxxxxxxxxxxxxx'
Subject: RE: [Xen-devel] [PATCH] fix the bug of gdb which debugs xen.

Hello Keir,
This patch fixes the problem of single step command.

In debugger_trap_fatal(), EF_TF bit is turned-on on single step command.
Debug trap (single step exception) is not produced when it is off.
diff -r da620c454916 xen/arch/x86/traps.c
--- a/xen/arch/x86/traps.c      Thu Aug 13 08:40:39 2009 +0100
+++ b/xen/arch/x86/traps.c      Thu Aug 13 20:34:34 2009 -0700
@@ -3014,7 +3014,9 @@
             if ( !debugger_trap_fatal(TRAP_debug, regs) )
                 WARN_ON(1);
 #endif
+#if 0
             regs->eflags &= ~EF_TF;
+#endif
         }
         else
         {
Because EF_TF bit is turned off when not on single step command (i.e.
continue command) in debugger_trap_fatal(), just deleting the line is good
enough. But I respect your modification.
BTW, why you have EF_TF and X86_EFLAGS_TF? Both are same. Is there any
reason why you have both? Thank you.
-caz
-----Original Message-----
From: Keir Fraser [mailto:keir.fraser@xxxxxxxxxxxxx] 
Sent: Thursday, August 13, 2009 12:32 AM
To: Caz Yokoyama; xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: Re: [Xen-devel] [PATCH] fix the bug of gdb which debugs xen.

Sorry, my test build worked because I didn't set crash_debug=y. I'll test a
fix.

 -- Keir


On 13/08/2009 03:54, "Caz Yokoyama" <cazyokoyama@xxxxxxxxx> wrote:

> Hello Keir,
> You code does not work. When each byte string is more than 0x7f, sign bit
is
> extended and produce wrong value.
> 
> diff -r 8a9f81672c76 xen/common/gdbstub.c
> --- a/xen/common/gdbstub.c Wed Aug 12 14:27:52 2009 +0100
> +++ b/xen/common/gdbstub.c Wed Aug 12 19:49:12 2009 -0700
> @@ -93,7 +93,7 @@
>      return -1;
>  }
>  
> -char
> +unsigned char
>  str2hex(const char *str)
>  {
>      return (char2hex(str[0]) << 4) | char2hex(str[1]);
> @@ -127,7 +127,7 @@
>          x <<= 8;
>          x += str2hex(*str);
>  #elif defined(__LITTLE_ENDIAN)
> -        x += (unsigned long)str2hex(*str) << (i*8);
> +        x += (unsigned long)str2hex(str) << (i*8);
>  #else
>  # error unknown endian
>  #endif
> diff -r 8a9f81672c76 xen/include/xen/gdbstub.h
> --- a/xen/include/xen/gdbstub.h Wed Aug 12 14:27:52 2009 +0100
> +++ b/xen/include/xen/gdbstub.h Wed Aug 12 19:49:12 2009 -0700
> @@ -29,7 +29,7 @@
>  /* value <-> char (de)serialzers for arch specific gdb backends */
>  char hex2char(unsigned long x);
>  int char2hex(unsigned char c);
> -char str2hex(const char *str);
> +unsigned char str2hex(const char *str);
>  unsigned long str2ulong(const char *str, unsigned long bytes);
>  
>  struct gdb_context {
> -caz
> 
> -----Original Message-----
> From: Caz Yokoyama [mailto:caz@xxxxxxxxxxx]
> Sent: Wednesday, August 12, 2009 2:03 PM
> To: 'Keir Fraser'; 'xen-devel@xxxxxxxxxxxxxxxxxxx'
> Subject: RE: [Xen-devel] [PATCH] fix the bug of gdb which debugs xen.
> 
> Probably, you mean.
> 
> diff -r 8a9f81672c76 xen/common/gdbstub.c
> --- a/xen/common/gdbstub.c Wed Aug 12 14:27:52 2009 +0100
> +++ b/xen/common/gdbstub.c Wed Aug 12 14:00:20 2009 -0700
> @@ -127,7 +127,7 @@
>          x <<= 8;
>          x += str2hex(*str);
>  #elif defined(__LITTLE_ENDIAN)
> -        x += (unsigned long)str2hex(*str) << (i*8);
> +        x += (unsigned long)str2hex(str) << (i*8);
>  #else
>  # error unknown endian
>  #endif
> -caz
> -----Original Message-----
> From: Keir Fraser [mailto:keir.fraser@xxxxxxxxxxxxx]
> Sent: Wednesday, August 12, 2009 6:30 AM
> To: Caz Yokoyama; xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH] fix the bug of gdb which debugs xen.
> 
> I rewrote it some more. Take a look at changeset 20054.
> 
>  -- Keir
> 
> On 12/08/2009 13:52, "Caz Yokoyama" <cazyokoyama@xxxxxxxxx> wrote:
> 
>> Hello Keir,
>> You are right and good points. Thank you. I did not aware them because
> only
>> environment I test is x86_64. How about this? I am glad if you check in.
I
>> regularly update my local source code by "hg pull; hg up".
>> -caz
>> 
>> -----Original Message-----
>> From: Keir Fraser [mailto:keir.fraser@xxxxxxxxxxxxx]
>> Sent: Wednesday, August 12, 2009 12:58 AM
>> To: Caz Yokoyama; xen-devel@xxxxxxxxxxxxxxxxxxx
>> Subject: Re: [Xen-devel] [PATCH] fix the bug of gdb which debugs xen.
>> 
>> I don't know whether you intend this for immediate checkin, but anyway:
>> 
>>  * Don't delete the sysenter logic in traps.c, leave it.
>>  * I'm not sure about swap64() in gdbstub.c. The value may not be 64 bits
>> (e.g., running on i386), or the system may not be little endian. Might be
>> better to define an alternative to str2ulong() which is endian aware,
like
>> gdb_write_to_packet_hex().
>> 
>>  -- Keir
>> 
>> On 12/08/2009 03:15, "Caz Yokoyama" <cazyokoyama@xxxxxxxxx> wrote:
>> 
>>> Hello,
>>> This patch fixes the bug of gdb which debugs Xen hypervisor, i.e. not
>> domU. As
>>> Emre Can Sezer reported in
>>>
http://lists.xensource.com/archives/html/xen-devel/2009-01/msg00885.html,
>> once
>>> break point is hit, continue command produces SIGTRAP at
>> restore_all_xen().
>>> This patch makes continue command resume Xen running. I still see other
>> bugs
>>> like backtrace command does not show function name. But I hope this
helps
>> your
>>> debug.
>>> FYI, related postings.
>>> http://lists.xensource.com/archives/html/xen-devel/2007-12/msg00678.html
>>> 
>> 
>
http://www.filewatcher.com/p/xen_2.0.6.orig.tar.gz.2456215/xen-2.0/docs/misc
>> /X
>>> enDebugger-HOWTO.html
>>>  
>>> connect gdb on step command
>>> --- a/xen/arch/x86/traps.c      Thu Aug 06 13:27:53 2009 +0100
>>> +++ b/xen/arch/x86/traps.c      Tue Aug 11 18:15:25 2009 -0700
>>> @@ -2977,13 +2977,7 @@
>>>         if ( regs->eflags & EF_TF )
>>>         {
>>>  #ifdef __x86_64__
>>> -           void sysenter_entry(void);
>>> -           void sysenter_eflags_saved(void);
>>> -           /* In SYSENTER entry path we can't zap TF until EFLAGS is
>> saved.
>>> */
>>> -           if ( (regs->rip >= (unsigned long)sysenter_entry) &&
>>> -                (regs->rip < (unsigned long)sysenter_eflags_saved) )
>>> -               goto out;
>>> -           WARN_ON(regs->rip != (unsigned long)sysenter_eflags_saved);
>>> +           debugger_trap_fatal(TRAP_debug, regs);
>>>  #else
>>>             WARN_ON(1);
>>>  #endif
>>>  
>>> Value of gdb command is little endian.
>>> diff -r 13fe7f07df15 xen/common/gdbstub.c
>>> --- a/xen/common/gdbstub.c      Thu Aug 06 13:27:53 2009 +0100
>>> +++ b/xen/common/gdbstub.c      Tue Aug 11 18:15:25 2009 -0700
>>> @@ -53,6 +53,10 @@
>>>  
>>>  #define GDB_RETRY_MAX   10
>>>  
>>> +#define swap16(_v) ((((u16)(_v)>>8)&0xff)|(((u16)(_v)&0xff)<<8))
>>> +#define swap32(_v)
>>> (((u32)swap16((u16)(_v))<<16)|(u32)swap16((u32)((_v)>>16)))
>>> +#define swap64(_v)
>>> (((u64)swap32((u32)(_v))<<32)|(u64)swap32((u32)((_v)>>32)))
>>> +
>>>  struct gdb_cpu_info
>>>  {
>>>      atomic_t paused;
>>> @@ -489,6 +493,7 @@
>>>         }
>>>         ptr++;
>>>         val = str2ulong(ptr, sizeof(unsigned long));
>>> +       val = swap64(val);
>>>         gdb_arch_write_reg(addr, val, regs, ctx);
>>>         break;
>>>     case 'D':
>>>  
>>> Thank you.
>>> -Caz Yokoyama, caz at caztech dot com. 503-804-1028(m).
>>>  
>>> 
>> 
>

Attachment: QuietGdb.patch
Description: Binary data

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel