[Qemu-devel] [PATCH 0/8] Fix various IO-thread breakages

[Qemu-devel] [PATCH 0/8] Fix various IO-thread breakages

[Jan Kiszka]

This series unbreaks -smp >1 and guest debugging in CONFIG_IOTHREAD
mode. I still find the SMP scheduling in cpu_exec_all suboptimal, but
at least it works now.

Dependencies are:
http://thread.gmane.org/gmane.comp.emulators.kvm.devel/52718 (kvm queue)
http://thread.gmane.org/gmane.comp.emulators.qemu/75087

The full series can be found at

	git://git.kiszka.org/qemu.git queues/iothread

Jan Kiszka (8):
  Introduce proper compiler barrier
  Fix cpu_unlink_tb race
  Init qemu_system_cond
  Fix cpu_exit for tcp_cpu_exec
  Fix qemu_wait_io_event processing in io-thread mode
  Drop redundant global cur_cpu variable
  Rename tcg_cpu_exec and tcg_has_work
  Rework debug exception processing for gdb use

 cpu-exec.c     |   15 +++++++------
 cpus.c         |   58 +++++++++++++++++++++++++++++++++----------------------
 cpus.h         |    2 +-
 kvm-all.c      |    2 -
 qemu-barrier.h |    3 ++
 vl.c           |    2 +-
 6 files changed, 48 insertions(+), 34 deletions(-)

[Qemu-devel] [PATCH 1/8] Introduce proper compiler barrier

[Jan Kiszka]

Define barrier() as optimization barrier and replace (potentially
unreliable) asm("") fences.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpu-exec.c     |    5 +++--
 qemu-barrier.h |    3 +++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 026980a..525b3b4 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -21,6 +21,7 @@
 #include "disas.h"
 #include "tcg.h"
 #include "kvm.h"
+#include "qemu-barrier.h"
 
 #if !defined(CONFIG_SOFTMMU)
 #undef EAX
@@ -233,7 +234,7 @@ int cpu_exec(CPUState *env1)
        use it.  */
     QEMU_BUILD_BUG_ON (sizeof (saved_env_reg) != sizeof (env));
     saved_env_reg = (host_reg_t) env;
-    asm("");
+    barrier();
     env = env1;
 
     if (exit_request) {
@@ -669,7 +670,7 @@ int cpu_exec(CPUState *env1)
 #endif
 
     /* restore global registers */
-    asm("");
+    barrier();
     env = (void *) saved_env_reg;
 
     /* fail safe : never use cpu_single_env outside cpu_exec() */
diff --git a/qemu-barrier.h b/qemu-barrier.h
index 3bd1075..b77fce2 100644
--- a/qemu-barrier.h
+++ b/qemu-barrier.h
@@ -4,4 +4,7 @@
 /* FIXME: arch dependant, x86 version */
 #define smp_wmb()   asm volatile("" ::: "memory")
 
+/* Compiler barrier */
+#define barrier()   asm volatile("" ::: "memory")
+
 #endif
-- 
1.7.1

[Qemu-devel] [PATCH 2/8] Fix cpu_unlink_tb race

[Jan Kiszka]

If a signal hit after the env->exit_request check but before cpu_exec
updated env->current_tb, cpu_unlink_tb called from the signal hander
will not unlink the current TB. This may leave us stuck in a guest loop
if no further unlink is invoked.

Fix this by reordering current_tb update and exit_request check,
additionally enforcing the correct order via a compiler barrier.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpu-exec.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 525b3b4..5f88f3f 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -600,8 +600,9 @@ int cpu_exec(CPUState *env1)
                    TB, but before it is linked into a potentially
                    infinite loop and becomes env->current_tb. Avoid
                    starting execution if there is a pending interrupt. */
-                if (!unlikely (env->exit_request)) {
-                    env->current_tb = tb;
+                env->current_tb = tb;
+                barrier();
+                if (likely(!env->exit_request)) {
                     tc_ptr = tb->tc_ptr;
                 /* execute the generated code */
 #if defined(__sparc__) && !defined(CONFIG_SOLARIS)
@@ -610,7 +611,6 @@ int cpu_exec(CPUState *env1)
 #define env cpu_single_env
 #endif
                     next_tb = tcg_qemu_tb_exec(tc_ptr);
-                    env->current_tb = NULL;
                     if ((next_tb & 3) == 2) {
                         /* Instruction counter expired.  */
                         int insns_left;
@@ -639,6 +639,7 @@ int cpu_exec(CPUState *env1)
                         }
                     }
                 }
+                env->current_tb = NULL;
                 /* reset soft MMU for next block (it can currently
                    only be set by a memory fault) */
             } /* for(;;) */
-- 
1.7.1

[Qemu-devel] [PATCH 3/8] Init qemu_system_cond

[Jan Kiszka]

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/cpus.c b/cpus.c
index fcd0f09..37e6b33 100644
--- a/cpus.c
+++ b/cpus.c
@@ -331,6 +331,7 @@ int qemu_init_main_loop(void)
         return ret;
 
     qemu_cond_init(&qemu_pause_cond);
+    qemu_cond_init(&qemu_system_cond);
     qemu_mutex_init(&qemu_fair_mutex);
     qemu_mutex_init(&qemu_global_mutex);
     qemu_mutex_lock(&qemu_global_mutex);
-- 
1.7.1

[Qemu-devel] [PATCH 4/8] Fix cpu_exit for tcp_cpu_exec

[Jan Kiszka]

If a cpu_exit request is pending, ensure that we leave the CPU loop
quickly. For this purpose, keep the global exit_request pending until
we are about to leave tcg_cpu_exec. Also, immediately break out of the
SMP loop if the request is set, do not run till the end of the chain.
This preserves the VCPU scheduling order in SMP mode.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpu-exec.c |    3 +--
 cpus.c     |    3 ++-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 5f88f3f..d170566 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -237,9 +237,8 @@ int cpu_exec(CPUState *env1)
     barrier();
     env = env1;
 
-    if (exit_request) {
+    if (unlikely(exit_request)) {
         env->exit_request = 1;
-        exit_request = 0;
     }
 
 #if defined(TARGET_I386)
diff --git a/cpus.c b/cpus.c
index 37e6b33..ff5e804 100644
--- a/cpus.c
+++ b/cpus.c
@@ -769,7 +769,7 @@ bool tcg_cpu_exec(void)
 
     if (next_cpu == NULL)
         next_cpu = first_cpu;
-    for (; next_cpu != NULL; next_cpu = next_cpu->next_cpu) {
+    for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
         CPUState *env = cur_cpu = next_cpu;
 
         qemu_clock_enable(vm_clock,
@@ -788,6 +788,7 @@ bool tcg_cpu_exec(void)
             break;
         }
     }
+    exit_request = 0;
     return tcg_has_work();
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH 5/8] Fix qemu_wait_io_event processing in io-thread mode

[Jan Kiszka]

When checking for I/O events in the tcg CPU loop, make sure that we
call qemu_wait_io_event_common for all CPUs, not only the current one.
Otherwise pause_all_vcpus may lock up or run_on_cpu requests may starve.

Rename qemu_wait_io_event to qemu_tcg_wait_io_event at this chance and
purge its argument list as it has no use for it.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/cpus.c b/cpus.c
index ff5e804..aef92cd 100644
--- a/cpus.c
+++ b/cpus.c
@@ -402,10 +402,12 @@ static void qemu_wait_io_event_common(CPUState *env)
     flush_queued_work(env);
 }
 
-static void qemu_wait_io_event(CPUState *env)
+static void qemu_tcg_wait_io_event(void)
 {
+    CPUState *env;
+
     while (!tcg_has_work())
-        qemu_cond_timedwait(env->halt_cond, &qemu_global_mutex, 1000);
+        qemu_cond_timedwait(tcg_halt_cond, &qemu_global_mutex, 1000);
 
     qemu_mutex_unlock(&qemu_global_mutex);
 
@@ -418,7 +420,10 @@ static void qemu_wait_io_event(CPUState *env)
     qemu_mutex_unlock(&qemu_fair_mutex);
 
     qemu_mutex_lock(&qemu_global_mutex);
-    qemu_wait_io_event_common(env);
+
+    for (env = first_cpu; env != NULL; env = env->next_cpu) {
+        qemu_wait_io_event_common(env);
+    }
 }
 
 static void qemu_kvm_eat_signal(CPUState *env, int timeout)
@@ -503,7 +508,7 @@ static void *tcg_cpu_thread_fn(void *arg)
 
     while (1) {
         tcg_cpu_exec();
-        qemu_wait_io_event(cur_cpu);
+        qemu_tcg_wait_io_event();
     }
 
     return NULL;
-- 
1.7.1

[Qemu-devel] [PATCH 6/8] Drop redundant global cur_cpu variable

[Jan Kiszka]

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/cpus.c b/cpus.c
index aef92cd..2ce839d 100644
--- a/cpus.c
+++ b/cpus.c
@@ -39,7 +39,6 @@
 #define SIG_IPI SIGUSR1
 #endif
 
-static CPUState *cur_cpu;
 static CPUState *next_cpu;
 
 /***********************************************************/
@@ -775,10 +774,10 @@ bool tcg_cpu_exec(void)
     if (next_cpu == NULL)
         next_cpu = first_cpu;
     for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
-        CPUState *env = cur_cpu = next_cpu;
+        CPUState *env = next_cpu;
 
         qemu_clock_enable(vm_clock,
-                          (cur_cpu->singlestep_enabled & SSTEP_NOTIMER) == 0);
+                          (env->singlestep_enabled & SSTEP_NOTIMER) == 0);
 
         if (qemu_alarm_pending())
             break;
-- 
1.7.1

[Qemu-devel] [PATCH 7/8] Rename tcg_cpu_exec and tcg_has_work

[Jan Kiszka]

These functions are also used for kvm under !CONFIG_IOTHREAD, having
'tcg' in their name is just misleading.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |   10 +++++-----
 cpus.h |    2 +-
 vl.c   |    2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/cpus.c b/cpus.c
index 2ce839d..c024421 100644
--- a/cpus.c
+++ b/cpus.c
@@ -130,7 +130,7 @@ static int cpu_has_work(CPUState *env)
     return 0;
 }
 
-static int tcg_has_work(void)
+static int any_cpu_has_work(void)
 {
     CPUState *env;
 
@@ -405,7 +405,7 @@ static void qemu_tcg_wait_io_event(void)
 {
     CPUState *env;
 
-    while (!tcg_has_work())
+    while (!any_cpu_has_work())
         qemu_cond_timedwait(tcg_halt_cond, &qemu_global_mutex, 1000);
 
     qemu_mutex_unlock(&qemu_global_mutex);
@@ -506,7 +506,7 @@ static void *tcg_cpu_thread_fn(void *arg)
         qemu_cond_timedwait(&qemu_system_cond, &qemu_global_mutex, 100);
 
     while (1) {
-        tcg_cpu_exec();
+        cpu_exec_all();
         qemu_tcg_wait_io_event();
     }
 
@@ -767,7 +767,7 @@ static int qemu_cpu_exec(CPUState *env)
     return ret;
 }
 
-bool tcg_cpu_exec(void)
+bool cpu_exec_all(void)
 {
     int ret = 0;
 
@@ -793,7 +793,7 @@ bool tcg_cpu_exec(void)
         }
     }
     exit_request = 0;
-    return tcg_has_work();
+    return any_cpu_has_work();
 }
 
 void set_numa_modes(void)
diff --git a/cpus.h b/cpus.h
index 774150a..af267ea 100644
--- a/cpus.h
+++ b/cpus.h
@@ -13,7 +13,7 @@ extern int smp_threads;
 extern int debug_requested;
 extern int vmstop_requested;
 void vm_state_notify(int running, int reason);
-bool tcg_cpu_exec(void);
+bool cpu_exec_all(void);
 void set_numa_modes(void);
 void set_cpu_log(const char *optarg);
 void list_cpus(FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
diff --git a/vl.c b/vl.c
index 920717a..0938656 100644
--- a/vl.c
+++ b/vl.c
@@ -1325,7 +1325,7 @@ static void main_loop(void)
             int64_t ti;
 #endif
 #ifndef CONFIG_IOTHREAD
-            nonblocking = tcg_cpu_exec();
+            nonblocking = cpu_exec_all();
 #endif
 #ifdef CONFIG_PROFILER
             ti = profile_getclock();
-- 
1.7.1

[Qemu-devel] [PATCH 8/8] Rework debug exception processing for gdb use

[Jan Kiszka]

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);
+}
+
 #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;
             }
-- 
1.7.1

[Qemu-devel] Re: [PATCH 0/8] Fix various IO-thread breakages

[Paolo Bonzini]

On 06/25/2010 04:56 PM, Jan Kiszka wrote:
> This series unbreaks -smp>1 and guest debugging in CONFIG_IOTHREAD
> mode. I still find the SMP scheduling in cpu_exec_all suboptimal, but
> at least it works now.
>
> Dependencies are:
> http://thread.gmane.org/gmane.comp.emulators.kvm.devel/52718 (kvm queue)
> http://thread.gmane.org/gmane.comp.emulators.qemu/75087
>
> The full series can be found at
>
> 	git://git.kiszka.org/qemu.git queues/iothread
>
> Jan Kiszka (8):
>    Introduce proper compiler barrier
>    Fix cpu_unlink_tb race
>    Init qemu_system_cond
>    Fix cpu_exit for tcp_cpu_exec
>    Fix qemu_wait_io_event processing in io-thread mode
>    Drop redundant global cur_cpu variable
>    Rename tcg_cpu_exec and tcg_has_work
>    Rework debug exception processing for gdb use
>
>   cpu-exec.c     |   15 +++++++------
>   cpus.c         |   58 +++++++++++++++++++++++++++++++++----------------------
>   cpus.h         |    2 +-
>   kvm-all.c      |    2 -
>   qemu-barrier.h |    3 ++
>   vl.c           |    2 +-
>   6 files changed, 48 insertions(+), 34 deletions(-)

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

[Qemu-devel] Re: [PATCH 0/8] Fix various IO-thread breakages

[Marcelo Tosatti]

On Fri, Jun 25, 2010 at 04:56:48PM +0200, Jan Kiszka wrote:
> This series unbreaks -smp >1 and guest debugging in CONFIG_IOTHREAD
> mode. I still find the SMP scheduling in cpu_exec_all suboptimal, but
> at least it works now.
> 
> Dependencies are:
> http://thread.gmane.org/gmane.comp.emulators.kvm.devel/52718 (kvm queue)
> http://thread.gmane.org/gmane.comp.emulators.qemu/75087
> 
> The full series can be found at
> 
> 	git://git.kiszka.org/qemu.git queues/iothread
> 
> Jan Kiszka (8):
>   Introduce proper compiler barrier
>   Fix cpu_unlink_tb race
>   Init qemu_system_cond
>   Fix cpu_exit for tcp_cpu_exec
>   Fix qemu_wait_io_event processing in io-thread mode
>   Drop redundant global cur_cpu variable
>   Rename tcg_cpu_exec and tcg_has_work
>   Rework debug exception processing for gdb use
> 
>  cpu-exec.c     |   15 +++++++------
>  cpus.c         |   58 +++++++++++++++++++++++++++++++++----------------------
>  cpus.h         |    2 +-
>  kvm-all.c      |    2 -
>  qemu-barrier.h |    3 ++
>  vl.c           |    2 +-
>  6 files changed, 48 insertions(+), 34 deletions(-)

Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>

I'll be sending the uq/master queue shortly (but this can go in
separately).

Re: [Qemu-devel] [PATCH 0/8] Fix various IO-thread breakages

[Aurelien Jarno]

On Fri, Jun 25, 2010 at 04:56:48PM +0200, Jan Kiszka wrote:
> This series unbreaks -smp >1 and guest debugging in CONFIG_IOTHREAD
> mode. I still find the SMP scheduling in cpu_exec_all suboptimal, but
> at least it works now.
> 
> Dependencies are:
> http://thread.gmane.org/gmane.comp.emulators.kvm.devel/52718 (kvm queue)
> http://thread.gmane.org/gmane.comp.emulators.qemu/75087
> 
> The full series can be found at
> 
> 	git://git.kiszka.org/qemu.git queues/iothread
> 
> Jan Kiszka (8):
>   Introduce proper compiler barrier
>   Fix cpu_unlink_tb race
>   Init qemu_system_cond
>   Fix cpu_exit for tcp_cpu_exec
>   Fix qemu_wait_io_event processing in io-thread mode
>   Drop redundant global cur_cpu variable
>   Rename tcg_cpu_exec and tcg_has_work
>   Rework debug exception processing for gdb use
> 
>  cpu-exec.c     |   15 +++++++------
>  cpus.c         |   58 +++++++++++++++++++++++++++++++++----------------------
>  cpus.h         |    2 +-
>  kvm-all.c      |    2 -
>  qemu-barrier.h |    3 ++
>  vl.c           |    2 +-
>  6 files changed, 48 insertions(+), 34 deletions(-)
> 

Thanks, all applied.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 8/8] Rework debug exception processing for gdb use

[TeLeMan]

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?

Re: [Qemu-devel] [PATCH 8/8] Rework debug exception processing for gdb use

[Jun Koi]

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

[Qemu-devel] Re: [PATCH 8/8] Rework debug exception processing for gdb use

[Jan Kiszka]

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

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]