[PATCH] fix the bug of gdb which debugs xen.

[PATCH] fix the bug of gdb which debugs xen.

[Caz Yokoyama]

[-- Attachment #1.1: Type: text/plain, Size: 2239 bytes --]

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
/XenDebugger-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 #1.2: Type: text/html, Size: 19052 bytes --]

[-- Attachment #2: XenGdb.patch --]
[-- Type: application/octet-stream, Size: 1436 bytes --]

--- 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
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':

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH] fix the bug of gdb which debugs xen.

[Keir Fraser]

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@gmail.com> 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).
>  
> 

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

[Caz Yokoyama]

[-- Attachment #1: Type: text/plain, Size: 3227 bytes --]

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@eu.citrix.com] 
Sent: Wednesday, August 12, 2009 12:58 AM
To: Caz Yokoyama; xen-devel@lists.xensource.com
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@gmail.com> 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 #2: XenGdb.patch --]
[-- Type: application/octet-stream, Size: 1845 bytes --]

diff -r 13fe7f07df15 xen/arch/x86/traps.c
--- a/xen/arch/x86/traps.c	Thu Aug 06 13:27:53 2009 +0100
+++ b/xen/arch/x86/traps.c	Wed Aug 12 05:43:34 2009 -0700
@@ -2976,6 +2976,9 @@
     {
         if ( regs->eflags & EF_TF )
         {
+#ifdef CRASH_DEBUG
+            debugger_trap_fatal(TRAP_debug, regs);
+#else
 #ifdef __x86_64__
             void sysenter_entry(void);
             void sysenter_eflags_saved(void);
@@ -2987,6 +2990,7 @@
 #else
             WARN_ON(1);
 #endif
+#endif /* CRASH_DEBUG */
             regs->eflags &= ~EF_TF;
         }
         else
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	Wed Aug 12 05:43:34 2009 -0700
@@ -271,6 +271,35 @@
 #endif
 }
 
+#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)))
+
+static unsigned long
+gdb_value_str_to_ulong(const char *str, int bytes)
+{
+    unsigned long x;
+
+	x = str2ulong(str, bytes);
+#ifdef __LITTLE_ENDIAN
+    switch ( bytes )
+    {
+    case sizeof(u32):
+		x = swap32(x);
+		break;
+    case sizeof(u64):
+		x = swap64(x);
+        break;
+    default:
+        dbg_printk("WARNING: %s x: 0x%lx int_size: %d\n",
+                   __func__, x, int_size);
+        break;
+    }
+#endif
+
+	return x;
+}
+
 static int
 gdb_check_ack(struct gdb_context *ctx)
 {
@@ -488,7 +517,7 @@
             return 0;
         }
         ptr++;
-        val = str2ulong(ptr, sizeof(unsigned long));
+        val = gdb_value_str_to_ulong(ptr, sizeof(unsigned long));
         gdb_arch_write_reg(addr, val, regs, ctx);
         break;
     case 'D':

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH] fix the bug of gdb which debugs xen.

[Keir Fraser]

I rewrote it some more. Take a look at changeset 20054.

 -- Keir

On 12/08/2009 13:52, "Caz Yokoyama" <cazyokoyama@gmail.com> 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@eu.citrix.com]
> Sent: Wednesday, August 12, 2009 12:58 AM
> To: Caz Yokoyama; xen-devel@lists.xensource.com
> 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@gmail.com> 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).
>>  
>> 
> 

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

[Caz Yokoyama]

[-- Attachment #1: Type: text/plain, Size: 4160 bytes --]

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@eu.citrix.com] 
Sent: Wednesday, August 12, 2009 6:30 AM
To: Caz Yokoyama; xen-devel@lists.xensource.com
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@gmail.com> 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@eu.citrix.com]
> Sent: Wednesday, August 12, 2009 12:58 AM
> To: Caz Yokoyama; xen-devel@lists.xensource.com
> 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@gmail.com> 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 #2: str_to_native_ulong.patch --]
[-- Type: application/octet-stream, Size: 413 bytes --]

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

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

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

[Caz Yokoyama]

[-- Attachment #1: Type: text/plain, Size: 5556 bytes --]

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@caztech.com] 
Sent: Wednesday, August 12, 2009 2:03 PM
To: 'Keir Fraser'; 'xen-devel@lists.xensource.com'
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@eu.citrix.com] 
Sent: Wednesday, August 12, 2009 6:30 AM
To: Caz Yokoyama; xen-devel@lists.xensource.com
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@gmail.com> 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@eu.citrix.com]
> Sent: Wednesday, August 12, 2009 12:58 AM
> To: Caz Yokoyama; xen-devel@lists.xensource.com
> 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@gmail.com> 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 #2: str_to_native_ulong.patch --]
[-- Type: application/octet-stream, Size: 1064 bytes --]

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 {

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH] fix the bug of gdb which debugs xen.

[Keir Fraser]

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@gmail.com> 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@caztech.com]
> Sent: Wednesday, August 12, 2009 2:03 PM
> To: 'Keir Fraser'; 'xen-devel@lists.xensource.com'
> 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@eu.citrix.com]
> Sent: Wednesday, August 12, 2009 6:30 AM
> To: Caz Yokoyama; xen-devel@lists.xensource.com
> 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@gmail.com> 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@eu.citrix.com]
>> Sent: Wednesday, August 12, 2009 12:58 AM
>> To: Caz Yokoyama; xen-devel@lists.xensource.com
>> 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@gmail.com> 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).
>>>  
>>> 
>> 
> 

Re: [PATCH] fix the bug of gdb which debugs xen.

[Paolo Bonzini]

On 08/12/2009 11:03 PM, Caz Yokoyama wrote:
> 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);

This has to be fixed too.

>   #elif defined(__LITTLE_ENDIAN)
> -        x += (unsigned long)str2hex(*str)<<  (i*8);
> +        x += (unsigned long)str2hex(str)<<  (i*8);

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

[Caz Yokoyama]

Keir did it on changeset 20055. Thank you for reporting.
-caz

-----Original Message-----
From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo
Bonzini
Sent: Thursday, August 13, 2009 6:30 AM
To: Caz Yokoyama
Cc: 'Keir Fraser'; xen-devel@lists.xensource.com
Subject: Re: [PATCH] fix the bug of gdb which debugs xen.

On 08/12/2009 11:03 PM, Caz Yokoyama wrote:
> 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);

This has to be fixed too.

>   #elif defined(__LITTLE_ENDIAN)
> -        x += (unsigned long)str2hex(*str)<<  (i*8);
> +        x += (unsigned long)str2hex(str)<<  (i*8);

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

[Caz Yokoyama]

[-- Attachment #1: Type: text/plain, Size: 7073 bytes --]

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@eu.citrix.com] 
Sent: Thursday, August 13, 2009 12:32 AM
To: Caz Yokoyama; xen-devel@lists.xensource.com
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@gmail.com> 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@caztech.com]
> Sent: Wednesday, August 12, 2009 2:03 PM
> To: 'Keir Fraser'; 'xen-devel@lists.xensource.com'
> 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@eu.citrix.com]
> Sent: Wednesday, August 12, 2009 6:30 AM
> To: Caz Yokoyama; xen-devel@lists.xensource.com
> 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@gmail.com> 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@eu.citrix.com]
>> Sent: Wednesday, August 12, 2009 12:58 AM
>> To: Caz Yokoyama; xen-devel@lists.xensource.com
>> 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@gmail.com> 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 #2: SingleStep.patch --]
[-- Type: application/octet-stream, Size: 377 bytes --]

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
         {

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

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

[Caz Yokoyama]

[-- Attachment #1: Type: text/plain, Size: 7888 bytes --]

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@caztech.com] 
Sent: Thursday, August 13, 2009 8:52 PM
To: 'Keir Fraser'; 'xen-devel@lists.xensource.com'
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@eu.citrix.com] 
Sent: Thursday, August 13, 2009 12:32 AM
To: Caz Yokoyama; xen-devel@lists.xensource.com
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@gmail.com> 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@caztech.com]
> Sent: Wednesday, August 12, 2009 2:03 PM
> To: 'Keir Fraser'; 'xen-devel@lists.xensource.com'
> 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@eu.citrix.com]
> Sent: Wednesday, August 12, 2009 6:30 AM
> To: Caz Yokoyama; xen-devel@lists.xensource.com
> 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@gmail.com> 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@eu.citrix.com]
>> Sent: Wednesday, August 12, 2009 12:58 AM
>> To: Caz Yokoyama; xen-devel@lists.xensource.com
>> 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@gmail.com> 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 #2: QuietGdb.patch --]
[-- Type: application/octet-stream, Size: 500 bytes --]

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);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

RE: [PATCH] break-in to Xen 1) when gdb is invoked and 2) ^C on gdb

[Caz Yokoyama]

[-- Attachment #1: Type: text/plain, Size: 2457 bytes --]

Hello Keir,
This patch makes gdb break-in to Xen when 1) it is invoked and 2) ^C is
typed on gdb or clicking interrupt on ddd.
diff -r 8b0f1f37e145 xen/common/keyhandler.c
--- a/xen/common/keyhandler.c	Sun Aug 16 08:46:08 2009 +0100
+++ b/xen/common/keyhandler.c	Mon Aug 17 10:07:04 2009 -0700
@@ -397,7 +397,11 @@
     .desc = "print all diagnostics"
 };
 
+#if 1
+void do_debug_key(unsigned char key, struct cpu_user_regs *regs)
+#else
 static void do_debug_key(unsigned char key, struct cpu_user_regs *regs)
+#endif
 {
     printk("'%c' pressed -> trapping into debugger\n", key);
     (void)debugger_trap_fatal(0xf001, regs);
diff -r 8b0f1f37e145 xen/drivers/char/ns16550.c
--- a/xen/drivers/char/ns16550.c	Sun Aug 16 08:46:08 2009 +0100
+++ b/xen/drivers/char/ns16550.c	Mon Aug 17 10:07:04 2009 -0700
@@ -16,6 +16,7 @@
 #include <xen/serial.h>
 #include <xen/iocap.h>
 #include <asm/io.h>
+#include <xen/keyhandler.h>
 
 /*
  * Configure serial port with a string:
@@ -125,10 +126,16 @@
 {
     struct serial_port *port = dev_id;
     struct ns16550 *uart = port->uart;
+    char lsr;
 
     while ( !(ns_read_reg(uart, IIR) & IIR_NOINT) )
     {
-        char lsr = ns_read_reg(uart, LSR);
+        lsr = ns_read_reg(uart, LSR);
+#if 1
+        if ( lsr & LSR_BI ) { /* on BREAK */
+            do_debug_key('g', regs); /* g is meaningless */
+        }
+#endif
         if ( lsr & LSR_THRE )
             serial_tx_interrupt(port, regs);
         if ( lsr & LSR_DR )
diff -r 8b0f1f37e145 xen/include/xen/keyhandler.h
--- a/xen/include/xen/keyhandler.h	Sun Aug 16 08:46:08 2009 +0100
+++ b/xen/include/xen/keyhandler.h	Mon Aug 17 10:07:04 2009 -0700
@@ -40,6 +40,10 @@
     char *desc;
 };
 
+#if 1
+extern void do_debug_key(unsigned char key, struct cpu_user_regs *regs);
+#endif
+
 /* Initialize keytable with default handlers */
 extern void initialize_keytable(void);

Xen expects BREAK and 'g' which is the same sequence of the one on Linux
kernel. I submitted a patch which sends BREAK and 'g' to gdb-patches on
08/07, but it is not approved yet.
You can't use ssplitd/nsplitd because it does not pass BREAK. Therefore, you
lose console output and corresponding ^A^A^A functions. My associated lines
for the patch are
menu.lst
        kernel /boot/xen-3.5-unstable.gz com1=115200,8n1 gdb=com1 
.gdbinit
	set remotebreak BREAK-g
	target remote /dev/ttyS0
 ddd --gdb --debugger "/home/caz/lightfleet/kgdb/gdb-KgdbLight/gdb/gdb"
xen/xen-syms
-caz


[-- Attachment #2: BREAK-g.patch --]
[-- Type: application/octet-stream, Size: 1811 bytes --]

diff -r 8b0f1f37e145 xen/common/keyhandler.c
--- a/xen/common/keyhandler.c	Sun Aug 16 08:46:08 2009 +0100
+++ b/xen/common/keyhandler.c	Mon Aug 17 10:07:04 2009 -0700
@@ -397,7 +397,11 @@
     .desc = "print all diagnostics"
 };
 
+#if 1
+void do_debug_key(unsigned char key, struct cpu_user_regs *regs)
+#else
 static void do_debug_key(unsigned char key, struct cpu_user_regs *regs)
+#endif
 {
     printk("'%c' pressed -> trapping into debugger\n", key);
     (void)debugger_trap_fatal(0xf001, regs);
diff -r 8b0f1f37e145 xen/drivers/char/ns16550.c
--- a/xen/drivers/char/ns16550.c	Sun Aug 16 08:46:08 2009 +0100
+++ b/xen/drivers/char/ns16550.c	Mon Aug 17 10:07:04 2009 -0700
@@ -16,6 +16,7 @@
 #include <xen/serial.h>
 #include <xen/iocap.h>
 #include <asm/io.h>
+#include <xen/keyhandler.h>
 
 /*
  * Configure serial port with a string:
@@ -125,10 +126,16 @@
 {
     struct serial_port *port = dev_id;
     struct ns16550 *uart = port->uart;
+    char lsr;
 
     while ( !(ns_read_reg(uart, IIR) & IIR_NOINT) )
     {
-        char lsr = ns_read_reg(uart, LSR);
+        lsr = ns_read_reg(uart, LSR);
+#if 1
+        if ( lsr & LSR_BI ) { /* on BREAK */
+            do_debug_key('g', regs); /* g is meaningless */
+        }
+#endif
         if ( lsr & LSR_THRE )
             serial_tx_interrupt(port, regs);
         if ( lsr & LSR_DR )
diff -r 8b0f1f37e145 xen/include/xen/keyhandler.h
--- a/xen/include/xen/keyhandler.h	Sun Aug 16 08:46:08 2009 +0100
+++ b/xen/include/xen/keyhandler.h	Mon Aug 17 10:07:04 2009 -0700
@@ -40,6 +40,10 @@
     char *desc;
 };
 
+#if 1
+extern void do_debug_key(unsigned char key, struct cpu_user_regs *regs);
+#endif
+
 /* Initialize keytable with default handlers */
 extern void initialize_keytable(void);
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH] break-in to Xen 1) when gdb is invoked and 2) ^C on gdb

[Simon Horman]

On Mon, Aug 17, 2009 at 01:56:38PM -0700, Caz Yokoyama wrote:
> Hello Keir,
> This patch makes gdb break-in to Xen when 1) it is invoked and 2) ^C is
> typed on gdb or clicking interrupt on ddd.
> diff -r 8b0f1f37e145 xen/common/keyhandler.c
> --- a/xen/common/keyhandler.c	Sun Aug 16 08:46:08 2009 +0100
> +++ b/xen/common/keyhandler.c	Mon Aug 17 10:07:04 2009 -0700
> @@ -397,7 +397,11 @@
>      .desc = "print all diagnostics"
>  };
>  
> +#if 1
> +void do_debug_key(unsigned char key, struct cpu_user_regs *regs)
> +#else
>  static void do_debug_key(unsigned char key, struct cpu_user_regs *regs)
> +#endif
>  {
>      printk("'%c' pressed -> trapping into debugger\n", key);
>      (void)debugger_trap_fatal(0xf001, regs);
> diff -r 8b0f1f37e145 xen/drivers/char/ns16550.c
> --- a/xen/drivers/char/ns16550.c	Sun Aug 16 08:46:08 2009 +0100
> +++ b/xen/drivers/char/ns16550.c	Mon Aug 17 10:07:04 2009 -0700
> @@ -16,6 +16,7 @@
>  #include <xen/serial.h>
>  #include <xen/iocap.h>
>  #include <asm/io.h>
> +#include <xen/keyhandler.h>
>  
>  /*
>   * Configure serial port with a string:
> @@ -125,10 +126,16 @@
>  {
>      struct serial_port *port = dev_id;
>      struct ns16550 *uart = port->uart;
> +    char lsr;
>  
>      while ( !(ns_read_reg(uart, IIR) & IIR_NOINT) )
>      {
> -        char lsr = ns_read_reg(uart, LSR);
> +        lsr = ns_read_reg(uart, LSR);
> +#if 1

Is there a reason for #if 1 here?

> +        if ( lsr & LSR_BI ) { /* on BREAK */
> +            do_debug_key('g', regs); /* g is meaningless */
> +        }
> +#endif
>          if ( lsr & LSR_THRE )
>              serial_tx_interrupt(port, regs);
>          if ( lsr & LSR_DR )
> diff -r 8b0f1f37e145 xen/include/xen/keyhandler.h
> --- a/xen/include/xen/keyhandler.h	Sun Aug 16 08:46:08 2009 +0100
> +++ b/xen/include/xen/keyhandler.h	Mon Aug 17 10:07:04 2009 -0700
> @@ -40,6 +40,10 @@
>      char *desc;
>  };
>  
> +#if 1
> +extern void do_debug_key(unsigned char key, struct cpu_user_regs *regs);
> +#endif
> +
>  /* Initialize keytable with default handlers */
>  extern void initialize_keytable(void);
> 
> Xen expects BREAK and 'g' which is the same sequence of the one on Linux
> kernel. I submitted a patch which sends BREAK and 'g' to gdb-patches on
> 08/07, but it is not approved yet.
> You can't use ssplitd/nsplitd because it does not pass BREAK. Therefore, you
> lose console output and corresponding ^A^A^A functions. My associated lines
> for the patch are
> menu.lst
>         kernel /boot/xen-3.5-unstable.gz com1=115200,8n1 gdb=com1 
> .gdbinit
> 	set remotebreak BREAK-g
> 	target remote /dev/ttyS0
>  ddd --gdb --debugger "/home/caz/lightfleet/kgdb/gdb-KgdbLight/gdb/gdb"
> xen/xen-syms
> -caz
> 


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

RE: [PATCH] break-in to Xen 1) when gdb is invoked and 2) ^C on gdb

[Caz Yokoyama]

Hello Simon,
I afraid I don't understand your question. I assume your question is why I
move the declaration of lsr from inside of while loop to outside. The reason
is that I expect lsr is allocated and de-allocated every time the body of
the while loop is executed. The speed of serial port is slow. Therefore,
there may be no difference. Also gcc may optimize allocation and
de-allocation moving toward outside of while loop.
Let me know if I haven't answered your question.
-caz

-----Original Message-----
From: Simon Horman [mailto:horms@verge.net.au] 
Sent: Thursday, August 20, 2009 5:24 PM
To: Caz Yokoyama
Cc: 'Keir Fraser'; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] [PATCH] break-in to Xen 1) when gdb is invoked and
2) ^C on gdb

On Mon, Aug 17, 2009 at 01:56:38PM -0700, Caz Yokoyama wrote:
> Hello Keir,
> This patch makes gdb break-in to Xen when 1) it is invoked and 2) ^C is
> typed on gdb or clicking interrupt on ddd.
> diff -r 8b0f1f37e145 xen/common/keyhandler.c
> --- a/xen/common/keyhandler.c	Sun Aug 16 08:46:08 2009 +0100
> +++ b/xen/common/keyhandler.c	Mon Aug 17 10:07:04 2009 -0700
> @@ -397,7 +397,11 @@
>      .desc = "print all diagnostics"
>  };
>  
> +#if 1
> +void do_debug_key(unsigned char key, struct cpu_user_regs *regs)
> +#else
>  static void do_debug_key(unsigned char key, struct cpu_user_regs *regs)
> +#endif
>  {
>      printk("'%c' pressed -> trapping into debugger\n", key);
>      (void)debugger_trap_fatal(0xf001, regs);
> diff -r 8b0f1f37e145 xen/drivers/char/ns16550.c
> --- a/xen/drivers/char/ns16550.c	Sun Aug 16 08:46:08 2009 +0100
> +++ b/xen/drivers/char/ns16550.c	Mon Aug 17 10:07:04 2009 -0700
> @@ -16,6 +16,7 @@
>  #include <xen/serial.h>
>  #include <xen/iocap.h>
>  #include <asm/io.h>
> +#include <xen/keyhandler.h>
>  
>  /*
>   * Configure serial port with a string:
> @@ -125,10 +126,16 @@
>  {
>      struct serial_port *port = dev_id;
>      struct ns16550 *uart = port->uart;
> +    char lsr;
>  
>      while ( !(ns_read_reg(uart, IIR) & IIR_NOINT) )
>      {
> -        char lsr = ns_read_reg(uart, LSR);
> +        lsr = ns_read_reg(uart, LSR);
> +#if 1

Is there a reason for #if 1 here?

> +        if ( lsr & LSR_BI ) { /* on BREAK */
> +            do_debug_key('g', regs); /* g is meaningless */
> +        }
> +#endif
>          if ( lsr & LSR_THRE )
>              serial_tx_interrupt(port, regs);
>          if ( lsr & LSR_DR )
> diff -r 8b0f1f37e145 xen/include/xen/keyhandler.h
> --- a/xen/include/xen/keyhandler.h	Sun Aug 16 08:46:08 2009 +0100
> +++ b/xen/include/xen/keyhandler.h	Mon Aug 17 10:07:04 2009 -0700
> @@ -40,6 +40,10 @@
>      char *desc;
>  };
>  
> +#if 1
> +extern void do_debug_key(unsigned char key, struct cpu_user_regs *regs);
> +#endif
> +
>  /* Initialize keytable with default handlers */
>  extern void initialize_keytable(void);
> 
> Xen expects BREAK and 'g' which is the same sequence of the one on Linux
> kernel. I submitted a patch which sends BREAK and 'g' to gdb-patches on
> 08/07, but it is not approved yet.
> You can't use ssplitd/nsplitd because it does not pass BREAK. Therefore,
you
> lose console output and corresponding ^A^A^A functions. My associated
lines
> for the patch are
> menu.lst
>         kernel /boot/xen-3.5-unstable.gz com1=115200,8n1 gdb=com1 
> .gdbinit
> 	set remotebreak BREAK-g
> 	target remote /dev/ttyS0
>  ddd --gdb --debugger "/home/caz/lightfleet/kgdb/gdb-KgdbLight/gdb/gdb"
> xen/xen-syms
> -caz
> 


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

Re: [PATCH] break-in to Xen 1) when gdb is invoked and 2) ^C on gdb

[Simon Horman]

On Thu, Aug 20, 2009 at 10:37:05PM -0700, Caz Yokoyama wrote:
> Hello Simon,
> I afraid I don't understand your question. I assume your question is why I
> move the declaration of lsr from inside of while loop to outside. The reason
> is that I expect lsr is allocated and de-allocated every time the body of
> the while loop is executed. The speed of serial port is slow. Therefore,
> there may be no difference. Also gcc may optimize allocation and
> de-allocation moving toward outside of while loop.
> Let me know if I haven't answered your question.
> -caz

My question is, why are the following two lines included in your patch.
They don't seem to be necessary.

#if 1
#endif

> -----Original Message-----
> From: Simon Horman [mailto:horms@verge.net.au] 
> Sent: Thursday, August 20, 2009 5:24 PM
> To: Caz Yokoyama
> Cc: 'Keir Fraser'; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH] break-in to Xen 1) when gdb is invoked and
> 2) ^C on gdb
> 
> On Mon, Aug 17, 2009 at 01:56:38PM -0700, Caz Yokoyama wrote:
> > Hello Keir,
> > This patch makes gdb break-in to Xen when 1) it is invoked and 2) ^C is
> > typed on gdb or clicking interrupt on ddd.
> > diff -r 8b0f1f37e145 xen/common/keyhandler.c
> > --- a/xen/common/keyhandler.c	Sun Aug 16 08:46:08 2009 +0100
> > +++ b/xen/common/keyhandler.c	Mon Aug 17 10:07:04 2009 -0700
> > @@ -397,7 +397,11 @@
> >      .desc = "print all diagnostics"
> >  };
> >  
> > +#if 1
> > +void do_debug_key(unsigned char key, struct cpu_user_regs *regs)
> > +#else
> >  static void do_debug_key(unsigned char key, struct cpu_user_regs *regs)
> > +#endif
> >  {
> >      printk("'%c' pressed -> trapping into debugger\n", key);
> >      (void)debugger_trap_fatal(0xf001, regs);
> > diff -r 8b0f1f37e145 xen/drivers/char/ns16550.c
> > --- a/xen/drivers/char/ns16550.c	Sun Aug 16 08:46:08 2009 +0100
> > +++ b/xen/drivers/char/ns16550.c	Mon Aug 17 10:07:04 2009 -0700
> > @@ -16,6 +16,7 @@
> >  #include <xen/serial.h>
> >  #include <xen/iocap.h>
> >  #include <asm/io.h>
> > +#include <xen/keyhandler.h>
> >  
> >  /*
> >   * Configure serial port with a string:
> > @@ -125,10 +126,16 @@
> >  {
> >      struct serial_port *port = dev_id;
> >      struct ns16550 *uart = port->uart;
> > +    char lsr;
> >  
> >      while ( !(ns_read_reg(uart, IIR) & IIR_NOINT) )
> >      {
> > -        char lsr = ns_read_reg(uart, LSR);
> > +        lsr = ns_read_reg(uart, LSR);
> > +#if 1
> 
> Is there a reason for #if 1 here?
> 
> > +        if ( lsr & LSR_BI ) { /* on BREAK */
> > +            do_debug_key('g', regs); /* g is meaningless */
> > +        }
> > +#endif
> >          if ( lsr & LSR_THRE )
> >              serial_tx_interrupt(port, regs);
> >          if ( lsr & LSR_DR )
> > diff -r 8b0f1f37e145 xen/include/xen/keyhandler.h
> > --- a/xen/include/xen/keyhandler.h	Sun Aug 16 08:46:08 2009 +0100
> > +++ b/xen/include/xen/keyhandler.h	Mon Aug 17 10:07:04 2009 -0700
> > @@ -40,6 +40,10 @@
> >      char *desc;
> >  };
> >  
> > +#if 1
> > +extern void do_debug_key(unsigned char key, struct cpu_user_regs *regs);
> > +#endif
> > +
> >  /* Initialize keytable with default handlers */
> >  extern void initialize_keytable(void);
> > 
> > Xen expects BREAK and 'g' which is the same sequence of the one on Linux
> > kernel. I submitted a patch which sends BREAK and 'g' to gdb-patches on
> > 08/07, but it is not approved yet.
> > You can't use ssplitd/nsplitd because it does not pass BREAK. Therefore,
> you
> > lose console output and corresponding ^A^A^A functions. My associated
> lines
> > for the patch are
> > menu.lst
> >         kernel /boot/xen-3.5-unstable.gz com1=115200,8n1 gdb=com1 
> > .gdbinit
> > 	set remotebreak BREAK-g
> > 	target remote /dev/ttyS0
> >  ddd --gdb --debugger "/home/caz/lightfleet/kgdb/gdb-KgdbLight/gdb/gdb"
> > xen/xen-syms
> > -caz
> > 
> 
> 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

RE: [PATCH] break-in to Xen 1) when gdb is invoked and 2) ^C on gdb

[Caz Yokoyama]

Hello Simon,
No, they are not needed. The reason is to emphasize where I modified. In
patch file, modification is quite remarkable.
-caz

-----Original Message-----
From: Simon Horman [mailto:horms@verge.net.au] 
Sent: Thursday, August 20, 2009 11:08 PM
To: Caz Yokoyama
Cc: xen-devel@lists.xensource.com; 'Keir Fraser'
Subject: Re: [Xen-devel] [PATCH] break-in to Xen 1) when gdb is invoked and
2) ^C on gdb

On Thu, Aug 20, 2009 at 10:37:05PM -0700, Caz Yokoyama wrote:
> Hello Simon,
> I afraid I don't understand your question. I assume your question is why I
> move the declaration of lsr from inside of while loop to outside. The
reason
> is that I expect lsr is allocated and de-allocated every time the body of
> the while loop is executed. The speed of serial port is slow. Therefore,
> there may be no difference. Also gcc may optimize allocation and
> de-allocation moving toward outside of while loop.
> Let me know if I haven't answered your question.
> -caz

My question is, why are the following two lines included in your patch.
They don't seem to be necessary.

#if 1
#endif

> -----Original Message-----
> From: Simon Horman [mailto:horms@verge.net.au] 
> Sent: Thursday, August 20, 2009 5:24 PM
> To: Caz Yokoyama
> Cc: 'Keir Fraser'; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH] break-in to Xen 1) when gdb is invoked
and
> 2) ^C on gdb
> 
> On Mon, Aug 17, 2009 at 01:56:38PM -0700, Caz Yokoyama wrote:
> > Hello Keir,
> > This patch makes gdb break-in to Xen when 1) it is invoked and 2) ^C is
> > typed on gdb or clicking interrupt on ddd.
> > diff -r 8b0f1f37e145 xen/common/keyhandler.c
> > --- a/xen/common/keyhandler.c	Sun Aug 16 08:46:08 2009 +0100
> > +++ b/xen/common/keyhandler.c	Mon Aug 17 10:07:04 2009 -0700
> > @@ -397,7 +397,11 @@
> >      .desc = "print all diagnostics"
> >  };
> >  
> > +#if 1
> > +void do_debug_key(unsigned char key, struct cpu_user_regs *regs)
> > +#else
> >  static void do_debug_key(unsigned char key, struct cpu_user_regs *regs)
> > +#endif
> >  {
> >      printk("'%c' pressed -> trapping into debugger\n", key);
> >      (void)debugger_trap_fatal(0xf001, regs);
> > diff -r 8b0f1f37e145 xen/drivers/char/ns16550.c
> > --- a/xen/drivers/char/ns16550.c	Sun Aug 16 08:46:08 2009 +0100
> > +++ b/xen/drivers/char/ns16550.c	Mon Aug 17 10:07:04 2009 -0700
> > @@ -16,6 +16,7 @@
> >  #include <xen/serial.h>
> >  #include <xen/iocap.h>
> >  #include <asm/io.h>
> > +#include <xen/keyhandler.h>
> >  
> >  /*
> >   * Configure serial port with a string:
> > @@ -125,10 +126,16 @@
> >  {
> >      struct serial_port *port = dev_id;
> >      struct ns16550 *uart = port->uart;
> > +    char lsr;
> >  
> >      while ( !(ns_read_reg(uart, IIR) & IIR_NOINT) )
> >      {
> > -        char lsr = ns_read_reg(uart, LSR);
> > +        lsr = ns_read_reg(uart, LSR);
> > +#if 1
> 
> Is there a reason for #if 1 here?
> 
> > +        if ( lsr & LSR_BI ) { /* on BREAK */
> > +            do_debug_key('g', regs); /* g is meaningless */
> > +        }
> > +#endif
> >          if ( lsr & LSR_THRE )
> >              serial_tx_interrupt(port, regs);
> >          if ( lsr & LSR_DR )
> > diff -r 8b0f1f37e145 xen/include/xen/keyhandler.h
> > --- a/xen/include/xen/keyhandler.h	Sun Aug 16 08:46:08 2009 +0100
> > +++ b/xen/include/xen/keyhandler.h	Mon Aug 17 10:07:04 2009 -0700
> > @@ -40,6 +40,10 @@
> >      char *desc;
> >  };
> >  
> > +#if 1
> > +extern void do_debug_key(unsigned char key, struct cpu_user_regs
*regs);
> > +#endif
> > +
> >  /* Initialize keytable with default handlers */
> >  extern void initialize_keytable(void);
> > 
> > Xen expects BREAK and 'g' which is the same sequence of the one on Linux
> > kernel. I submitted a patch which sends BREAK and 'g' to gdb-patches on
> > 08/07, but it is not approved yet.
> > You can't use ssplitd/nsplitd because it does not pass BREAK. Therefore,
> you
> > lose console output and corresponding ^A^A^A functions. My associated
> lines
> > for the patch are
> > menu.lst
> >         kernel /boot/xen-3.5-unstable.gz com1=115200,8n1 gdb=com1 
> > .gdbinit
> > 	set remotebreak BREAK-g
> > 	target remote /dev/ttyS0
> >  ddd --gdb --debugger "/home/caz/lightfleet/kgdb/gdb-KgdbLight/gdb/gdb"
> > xen/xen-syms
> > -caz
> > 
> 
> 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel