Xen devel - Sep 2006 - [PATCH][XEN] __trap_to_gdb should return something different

--text follows this line--

This patch allows the caller to find out if the gdbstub actually did
anything.

Signed-off-by: Jimi Xenidis <jimix@watson.ibm.com>

---

diff -r e0bb62683805 xen/common/gdbstub.c
--- a/xen/common/gdbstub.c	Fri Sep 22 11:02:47 2006 -0400
+++ b/xen/common/gdbstub.c	Fri Sep 22 11:07:16 2006 -0400
@@ -481,7 +481,7 @@ __trap_to_gdb(struct cpu_user_regs *regs
     if ( gdb_ctx->serhnd < 0 )
     {
         dbg_printk("Debugger not ready yet.\n");
-        return 0;
+        return -1;
     }
 
     /* We rely on our caller to ensure we''re only on one processor
@@ -500,7 +500,7 @@ __trap_to_gdb(struct cpu_user_regs *regs
     {
         printk("WARNING WARNING WARNING: Avoiding recursive gdb.\n");
         atomic_inc(&gdb_ctx->running);
-        return 0;
+        return -1;
     }
 
     if ( !gdb_ctx->connected )

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

On 22/9/06 4:07 pm, "Jimi Xenidis" <jimix@watson.ibm.com> wrote:
> --text follows this line--
> 
> This patch allows the caller to find out if the gdbstub actually did
> anything.
> 
> Signed-off-by: Jimi Xenidis <jimix@watson.ibm.com>
What different actions do you envisage the caller will take? If they are
reasonable, perhaps they should be explicitly specified in a return-code
enumeration?

 -- Keir



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

On Sep 23, 2006, at 1:18 PM, Keir Fraser wrote:
> On 22/9/06 4:07 pm, "Jimi Xenidis" <jimix@watson.ibm.com>
wrote:
>
>> --text follows this line--
>>
>> This patch allows the caller to find out if the gdbstub actually did
>> anything.
>>
>> Signed-off-by: Jimi Xenidis <jimix@watson.ibm.com>
>
> What different actions do you envisage the caller will take? If  
> they are
> reasonable, perhaps they should be explicitly specified in a return- 
> code
> enumeration?
Currently there are two failure cases:
   1) GDB had no transport available for its use (UART or otherwise)
   2) "unexpected trap", usually another trap occurs while gdb is in  
control

I suppose we could have (1) -EIO and (2) -EBUSY.
My logic was that either gdb was available or not and that I needed  
to continue as if gdb was not enabled at all.

-JX


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

On 25/9/06 14:27, "Jimi Xenidis" <jimix@watson.ibm.com> wrote:
> Currently there are two failure cases:
>    1) GDB had no transport available for its use (UART or otherwise)
>    2) "unexpected trap", usually another trap occurs while gdb is
in
> control
> 
> I suppose we could have (1) -EIO and (2) -EBUSY.
> My logic was that either gdb was available or not and that I needed
> to continue as if gdb was not enabled at all.
How would your behaviour differ if gdbstub actually ran and eventually
returned control to you, versus the trap into the debugger being a simple
no-op? Surely the caller doesn''t really care either way?

 -- Keir



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

On Sep 25, 2006, at 9:34 AM, Keir Fraser wrote:
>
>
>
> On 25/9/06 14:27, "Jimi Xenidis" <jimix@watson.ibm.com>
wrote:
>
>> Currently there are two failure cases:
>>    1) GDB had no transport available for its use (UART or otherwise)
>>    2) "unexpected trap", usually another trap occurs while
gdb is in
>> control
>>
>> I suppose we could have (1) -EIO and (2) -EBUSY.
>> My logic was that either gdb was available or not and that I needed
>> to continue as if gdb was not enabled at all.
>
> How would your behaviour differ if gdbstub actually ran and eventually
> returned control to you, versus the trap into the debugger being a  
> simple
> no-op?
__trap_to_gdb() primary usage is to "respond" to the
"program"
exception that GDB caused because it inserted a trap in the text to  
serve as a breakpoint.
> Surely the caller doesn''t really care either way?
Uh oh, are we gonna have that "recovery" conversation again? :D
We find it _extremely_valuable_ to use GDB as a debugger/diagnostic  
and not just a post-mortem tool.
We program in breakpoints and we even use do_debug_key() (which uses  
the poorly named debugger_trap_fatal()) to trap into the debugger,  
take a look around, continue execution and do not consider this as  
"fatal".

I understand the _broken_is_broken_ position and even chanted the "my  
debugger is printk()" several times myself, but this is a powerful  
tool, for those who wish to use it, and requires little support in  
the common code.

Here is the code that cares:

#ifdef CRASH_DEBUG
     if (__trap_to_gdb(regs, cookie) == 0)
         return;
#endif /* CRASH_DEBUG */

     printk("%s: type: 0x%lx\n", __func__, cookie);
     show_backtrace_regs(regs);
     machine_halt();

NOTE: we will revisit the call to machine_halt() later, we can''t use  
panic() because right now we''ll end up back in this fuction.

-JX


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

On 25/9/06 15:09, "Jimi Xenidis" <jimix@watson.ibm.com> wrote:
> Here is the code that cares:
> 
> #ifdef CRASH_DEBUG
>      if (__trap_to_gdb(regs, cookie) == 0)
>          return;
> #endif /* CRASH_DEBUG */
> 
>      printk("%s: type: 0x%lx\n", __func__, cookie);
>      show_backtrace_regs(regs);
>      machine_halt();
> 
> NOTE: we will revisit the call to machine_halt() later, we can''t
use
> panic() because right now we''ll end up back in this fuction.
Yeah, ok this is quite reasonable.

 -- Keir



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