Patchwork [8/8] Rework debug exception processing for gdb use

login
register
mail settings
Submitter Jan Kiszka
Date June 25, 2010, 2:56 p.m.
Message ID <c3827edf1aafc1731d82b82886dbedd45216c0c9.1277477810.git.jan.kiszka@siemens.com>
Download mbox | patch
Permalink /patch/56966/
State New
Headers show

Comments

Jan Kiszka - June 25, 2010, 2:56 p.m.
Guest debugging is currently broken under CONFIG_IOTHREAD. The reason is
inconsistent or even lacking signaling the debug events from the source
VCPU to the main loop and the gdbstub.

This patch addresses the issue by pushing this signaling into a
CPUDebugExcpHandler: cpu_debug_handler is registered as first handler,
thus will be executed last after potential breakpoint emulation
handlers. It sets informs the gdbstub about the debug event source,
requests a debug exit of the main loop and stops the current VCPU. This
mechanism works both for TCG and KVM, with and without IO-thread.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c    |   26 ++++++++++++++++----------
 kvm-all.c |    2 --
 2 files changed, 16 insertions(+), 12 deletions(-)
TeLeMan - July 23, 2010, 4:58 a.m.
On Fri, Jun 25, 2010 at 22:56, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Guest debugging is currently broken under CONFIG_IOTHREAD. The reason is
> inconsistent or even lacking signaling the debug events from the source
> VCPU to the main loop and the gdbstub.
>
> This patch addresses the issue by pushing this signaling into a
> CPUDebugExcpHandler: cpu_debug_handler is registered as first handler,
> thus will be executed last after potential breakpoint emulation
> handlers. It sets informs the gdbstub about the debug event source,
> requests a debug exit of the main loop and stops the current VCPU. This
> mechanism works both for TCG and KVM, with and without IO-thread.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  cpus.c    |   26 ++++++++++++++++----------
>  kvm-all.c |    2 --
>  2 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index c024421..a607d9a 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -140,6 +140,13 @@ static int any_cpu_has_work(void)
>     return 0;
>  }
>
> +static void cpu_debug_handler(CPUState *env)
> +{
> +    gdb_set_stop_cpu(env);
> +    debug_requested = EXCP_DEBUG;
> +    vm_stop(EXCP_DEBUG);
> +}

Is debug_requested or vm_stop() redundant?
Jun Koi - July 23, 2010, 5:44 a.m.
On Fri, Jul 23, 2010 at 1:58 PM, TeLeMan <geleman@gmail.com> wrote:
> On Fri, Jun 25, 2010 at 22:56, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> Guest debugging is currently broken under CONFIG_IOTHREAD. The reason is
>> inconsistent or even lacking signaling the debug events from the source
>> VCPU to the main loop and the gdbstub.
>>
>> This patch addresses the issue by pushing this signaling into a
>> CPUDebugExcpHandler: cpu_debug_handler is registered as first handler,
>> thus will be executed last after potential breakpoint emulation
>> handlers. It sets informs the gdbstub about the debug event source,
>> requests a debug exit of the main loop and stops the current VCPU. This
>> mechanism works both for TCG and KVM, with and without IO-thread.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  cpus.c    |   26 ++++++++++++++++----------
>>  kvm-all.c |    2 --
>>  2 files changed, 16 insertions(+), 12 deletions(-)
>>
>> diff --git a/cpus.c b/cpus.c
>> index c024421..a607d9a 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -140,6 +140,13 @@ static int any_cpu_has_work(void)
>>     return 0;
>>  }
>>
>> +static void cpu_debug_handler(CPUState *env)
>> +{
>> +    gdb_set_stop_cpu(env);
>> +    debug_requested = EXCP_DEBUG;
>> +    vm_stop(EXCP_DEBUG);
>> +}
>
> Is debug_requested or vm_stop() redundant?
>

certainly that debug_requested should only take value of 0 or 1.

thanks,
J
Jan Kiszka - July 23, 2010, 7:57 a.m.
Jun Koi wrote:
> On Fri, Jul 23, 2010 at 1:58 PM, TeLeMan <geleman@gmail.com> wrote:
>> On Fri, Jun 25, 2010 at 22:56, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> Guest debugging is currently broken under CONFIG_IOTHREAD. The reason is
>>> inconsistent or even lacking signaling the debug events from the source
>>> VCPU to the main loop and the gdbstub.
>>>
>>> This patch addresses the issue by pushing this signaling into a
>>> CPUDebugExcpHandler: cpu_debug_handler is registered as first handler,
>>> thus will be executed last after potential breakpoint emulation
>>> handlers. It sets informs the gdbstub about the debug event source,
>>> requests a debug exit of the main loop and stops the current VCPU. This
>>> mechanism works both for TCG and KVM, with and without IO-thread.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>  cpus.c    |   26 ++++++++++++++++----------
>>>  kvm-all.c |    2 --
>>>  2 files changed, 16 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/cpus.c b/cpus.c
>>> index c024421..a607d9a 100644
>>> --- a/cpus.c
>>> +++ b/cpus.c
>>> @@ -140,6 +140,13 @@ static int any_cpu_has_work(void)
>>>     return 0;
>>>  }
>>>
>>> +static void cpu_debug_handler(CPUState *env)
>>> +{
>>> +    gdb_set_stop_cpu(env);
>>> +    debug_requested = EXCP_DEBUG;
>>> +    vm_stop(EXCP_DEBUG);
>>> +}
>> Is debug_requested or vm_stop() redundant?
>>
> 
> certainly that debug_requested should only take value of 0 or 1.

This works analogously to vmstop_requested: The stop reason code is
transfered along the request. Granted, there is only one code used here
so far.

This whole thing could probably be simplified if we did not have to
support both single- and multi-threaded QEMU execution models. But
that's the situation ATM.

Jan

Patch

diff --git a/cpus.c b/cpus.c
index c024421..a607d9a 100644
--- a/cpus.c
+++ b/cpus.c
@@ -140,6 +140,13 @@  static int any_cpu_has_work(void)
     return 0;
 }
 
+static void cpu_debug_handler(CPUState *env)
+{
+    gdb_set_stop_cpu(env);
+    debug_requested = EXCP_DEBUG;
+    vm_stop(EXCP_DEBUG);
+}
+
 #ifndef _WIN32
 static int io_thread_fd = -1;
 
@@ -235,6 +242,8 @@  static void qemu_event_increment(void)
 #ifndef CONFIG_IOTHREAD
 int qemu_init_main_loop(void)
 {
+    cpu_set_debug_excp_handler(cpu_debug_handler);
+
     return qemu_event_init();
 }
 
@@ -325,6 +334,8 @@  int qemu_init_main_loop(void)
 {
     int ret;
 
+    cpu_set_debug_excp_handler(cpu_debug_handler);
+
     ret = qemu_event_init();
     if (ret)
         return ret;
@@ -769,8 +780,6 @@  static int qemu_cpu_exec(CPUState *env)
 
 bool cpu_exec_all(void)
 {
-    int ret = 0;
-
     if (next_cpu == NULL)
         next_cpu = first_cpu;
     for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
@@ -781,14 +790,11 @@  bool cpu_exec_all(void)
 
         if (qemu_alarm_pending())
             break;
-        if (cpu_can_run(env))
-            ret = qemu_cpu_exec(env);
-        else if (env->stop)
-            break;
-
-        if (ret == EXCP_DEBUG) {
-            gdb_set_stop_cpu(env);
-            debug_requested = EXCP_DEBUG;
+        if (cpu_can_run(env)) {
+            if (qemu_cpu_exec(env) == EXCP_DEBUG) {
+                break;
+            }
+        } else if (env->stop) {
             break;
         }
     }
diff --git a/kvm-all.c b/kvm-all.c
index 5684e51..cb8ae9a 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -913,8 +913,6 @@  int kvm_cpu_exec(CPUState *env)
             DPRINTF("kvm_exit_debug\n");
 #ifdef KVM_CAP_SET_GUEST_DEBUG
             if (kvm_arch_debug(&run->debug.arch)) {
-                gdb_set_stop_cpu(env);
-                vm_stop(EXCP_DEBUG);
                 env->exception_index = EXCP_DEBUG;
                 return 0;
             }