[Qemu-devel] [PATCH v2 00/24] [uq/master] Patch queue, part II

[Qemu-devel] [PATCH v2 00/24] [uq/master] Patch queue, part II

[Jan Kiszka]

Version 2 of part II. Changes:
 - Fixed "Unconditionally reenter kernel after IO exits" to take
   self-INIT into account
 - Fixed misplaced hunk in "Fix race between timer signals and vcpu
   entry under !IOTHREAD" (rebase artifact)
 - Factor out block_synchronous_signals (analogue to block_io_signals)
 - Additional fix to break out of SMP VCPU loop on pending IO event
 - Fork qemu_kvm_init_cpu_signals over CONFIG_IOTHREAD
 - Additional cleanup, flattening the main loop

Hope I addressed all review comments (except for passing env to
qemu_cpu_kick_self which I think is better as it is).

Thanks,
Jan

CC: Alexander Graf <agraf@suse.de>
CC: Gleb Natapov <gleb@redhat.com>
CC: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
CC: Huang Ying <ying.huang@intel.com>
CC: Jin Dongming <jin.dongming@np.css.fujitsu.com>
CC:  Paolo Bonzini <pbonzini@redhat.com>
CC: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Jan Kiszka (24):
  kvm: x86: Fix build in absence of KVM_CAP_ASYNC_PF
  Prevent abortion on multiple VCPU kicks
  Stop current VCPU on synchronous reset requests
  Process vmstop requests in IO thread
  Trigger exit from cpu_exec_all on pending IO events
  Leave inner main_loop faster on pending requests
  Flatten the main loop
  kvm: Report proper error on GET_VCPU_MMAP_SIZE failures
  kvm: Drop redundant kvm_enabled from kvm_cpu_thread_fn
  kvm: Handle kvm_init_vcpu errors
  kvm: Provide sigbus services arch-independently
  Refactor signal setup functions in cpus.c
  kvm: Set up signal mask also for !CONFIG_IOTHREAD
  kvm: Refactor qemu_kvm_eat_signals
  kvm: Call qemu_kvm_eat_signals also under !CONFIG_IOTHREAD
  Set up signalfd under !CONFIG_IOTHREAD
  kvm: Fix race between timer signals and vcpu entry under !IOTHREAD
  kvm: Add MCE signal support for !CONFIG_IOTHREAD
  Introduce VCPU self-signaling service
  kvm: Unconditionally reenter kernel after IO exits
  kvm: Remove static return code of kvm_handle_io
  kvm: Leave kvm_cpu_exec directly after KVM_EXIT_SHUTDOWN
  Refactor kvm&tcg function names in cpus.c
  Fix a few coding style violations in cpus.c

 Makefile.objs      |    2 +-
 configure          |    6 +
 cpu-defs.h         |    1 +
 cpus.c             |  662 ++++++++++++++++++++++++++++++++--------------------
 cpus.h             |    1 +
 kvm-all.c          |   60 +++--
 kvm-stub.c         |    5 +
 kvm.h              |    7 +-
 qemu-common.h      |    1 +
 target-i386/kvm.c  |   11 +-
 target-ppc/kvm.c   |   10 +
 target-s390x/kvm.c |   10 +
 vl.c               |   40 ++--
 13 files changed, 514 insertions(+), 302 deletions(-)

[Qemu-devel] [PATCH v2 01/24] kvm: x86: Fix build in absence of KVM_CAP_ASYNC_PF

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Reported by Stefan Hajnoczi.

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

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 8e8880a..05010bb 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -167,7 +167,9 @@ static int get_para_features(CPUState *env)
             features |= (1 << para_features[i].feature);
         }
     }
+#ifdef KVM_CAP_ASYNC_PF
     has_msr_async_pf_en = features & (1 << KVM_FEATURE_ASYNC_PF);
+#endif
     return features;
 }
 #endif
-- 
1.7.1

[Qemu-devel] [PATCH v2 02/24] Prevent abortion on multiple VCPU kicks

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

If we call qemu_cpu_kick more than once before the target was able to
process the signal, pthread_kill will fail, and qemu will abort. Prevent
this by avoiding the redundant signal.

This logic can be found in qemu-kvm as well.

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

diff --git a/cpu-defs.h b/cpu-defs.h
index 8d4bf86..db809ed 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -205,6 +205,7 @@ typedef struct CPUWatchpoint {
     uint32_t stopped; /* Artificially stopped */                        \
     struct QemuThread *thread;                                          \
     struct QemuCond *halt_cond;                                         \
+    int thread_kicked;                                                  \
     struct qemu_work_item *queued_work_first, *queued_work_last;        \
     const char *cpu_model_str;                                          \
     struct KVMState *kvm_state;                                         \
diff --git a/cpus.c b/cpus.c
index 4c9928e..ab6e40e 100644
--- a/cpus.c
+++ b/cpus.c
@@ -481,6 +481,7 @@ static void qemu_wait_io_event_common(CPUState *env)
         qemu_cond_signal(&qemu_pause_cond);
     }
     flush_queued_work(env);
+    env->thread_kicked = false;
 }
 
 static void qemu_tcg_wait_io_event(void)
@@ -648,7 +649,10 @@ void qemu_cpu_kick(void *_env)
 {
     CPUState *env = _env;
     qemu_cond_broadcast(env->halt_cond);
-    qemu_thread_signal(env->thread, SIG_IPI);
+    if (!env->thread_kicked) {
+        qemu_thread_signal(env->thread, SIG_IPI);
+        env->thread_kicked = true;
+    }
 }
 
 int qemu_cpu_self(void *_env)
-- 
1.7.1

[Qemu-devel] [PATCH v2 03/24] Stop current VCPU on synchronous reset requests

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

If some I/O operation ends up calling qemu_system_reset_request in VCPU
context, we record this and inform the io-thread, but we do not
terminate the VCPU loop. This can lead to fairly unexpected behavior if
the triggering reset operation is supposed to work synchronously.

Fix this for TCG (when run in deterministic I/O mode) by setting the
VCPU on stop and issuing a cpu_exit. KVM requires some more work on its
VCPU loop.

[ ported from qemu-kvm ]

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

diff --git a/cpus.c b/cpus.c
index ab6e40e..ceb3a83 100644
--- a/cpus.c
+++ b/cpus.c
@@ -99,6 +99,14 @@ void cpu_synchronize_all_post_init(void)
     }
 }
 
+void cpu_stop_current(void)
+{
+    if (cpu_single_env) {
+        cpu_single_env->stopped = 1;
+        cpu_exit(cpu_single_env);
+    }
+}
+
 int cpu_is_stopped(CPUState *env)
 {
     return !vm_running || env->stopped;
@@ -863,10 +871,7 @@ void vm_stop(int reason)
          * FIXME: should not return to device code in case
          * vm_stop() has been requested.
          */
-        if (cpu_single_env) {
-            cpu_exit(cpu_single_env);
-            cpu_single_env->stop = 1;
-        }
+        cpu_stop_current();
         return;
     }
     do_vm_stop(reason);
diff --git a/cpus.h b/cpus.h
index bf4d9bb..4cadb64 100644
--- a/cpus.h
+++ b/cpus.h
@@ -6,6 +6,7 @@ int qemu_init_main_loop(void);
 void qemu_main_loop_start(void);
 void resume_all_vcpus(void);
 void pause_all_vcpus(void);
+void cpu_stop_current(void);
 
 /* vl.c */
 extern int smp_cores;
diff --git a/vl.c b/vl.c
index 33f844f..db24a05 100644
--- a/vl.c
+++ b/vl.c
@@ -1278,6 +1278,7 @@ void qemu_system_reset_request(void)
     } else {
         reset_requested = 1;
     }
+    cpu_stop_current();
     qemu_notify_event();
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH v2 04/24] Process vmstop requests in IO thread

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

A pending vmstop request is also a reason to leave the inner main loop.
So far we ignored it, and pending stop requests issued over VCPU threads
were simply ignored.

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

diff --git a/vl.c b/vl.c
index db24a05..5fad700 100644
--- a/vl.c
+++ b/vl.c
@@ -1373,15 +1373,11 @@ void main_loop_wait(int nonblocking)
 
 static int vm_can_run(void)
 {
-    if (powerdown_requested)
-        return 0;
-    if (reset_requested)
-        return 0;
-    if (shutdown_requested)
-        return 0;
-    if (debug_requested)
-        return 0;
-    return 1;
+    return !(powerdown_requested ||
+             reset_requested ||
+             shutdown_requested ||
+             debug_requested ||
+             vmstop_requested);
 }
 
 qemu_irq qemu_system_powerdown;
-- 
1.7.1

[Qemu-devel] [PATCH v2 05/24] Trigger exit from cpu_exec_all on pending IO events

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Except for timer events, we currently do not leave the loop over all
VCPUs if an IO event was filed. That may cause unexpected IO latencies
under !CONFIG_IOTHREAD in SMP scenarios. Fix it by setting the global
exit_request which breaks the loop.

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 ceb3a83..5dfc54e 100644
--- a/cpus.c
+++ b/cpus.c
@@ -315,6 +315,7 @@ void qemu_notify_event(void)
     if (next_cpu && env != next_cpu) {
         cpu_exit(next_cpu);
     }
+    exit_request = 1;
 }
 
 void qemu_mutex_lock_iothread(void) {}
-- 
1.7.1

[Qemu-devel] [PATCH v2 06/24] Leave inner main_loop faster on pending requests

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

If there is any pending request that requires us to leave the inner loop
if main_loop, makes sure we do this as soon as possible by enforcing
non-blocking IO processing.

At this change, move variable definitions out of the inner loop to
improve readability.

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

diff --git a/vl.c b/vl.c
index 5fad700..2ebc55b 100644
--- a/vl.c
+++ b/vl.c
@@ -1384,18 +1384,21 @@ qemu_irq qemu_system_powerdown;
 
 static void main_loop(void)
 {
+    bool nonblocking = false;
+#ifdef CONFIG_PROFILER
+    int64_t ti;
+#endif
     int r;
 
     qemu_main_loop_start();
 
     for (;;) {
         do {
-            bool nonblocking = false;
-#ifdef CONFIG_PROFILER
-            int64_t ti;
-#endif
 #ifndef CONFIG_IOTHREAD
             nonblocking = cpu_exec_all();
+            if (!vm_can_run()) {
+                nonblocking = true;
+            }
 #endif
 #ifdef CONFIG_PROFILER
             ti = profile_getclock();
-- 
1.7.1

[Qemu-devel] [PATCH v2 07/24] Flatten the main loop

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

First of all, vm_can_run is a misnomer, it actually means "no request
pending". Moreover, there is no need to check all pending requests
twice, the first time via the inner loop check and then again when
actually processing the requests. We can simply remove the inner loop
and do the checks directly.

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

diff --git a/vl.c b/vl.c
index 2ebc55b..f5dec09 100644
--- a/vl.c
+++ b/vl.c
@@ -1371,14 +1371,16 @@ void main_loop_wait(int nonblocking)
 
 }
 
-static int vm_can_run(void)
+#ifndef CONFIG_IOTHREAD
+static int vm_request_pending(void)
 {
-    return !(powerdown_requested ||
-             reset_requested ||
-             shutdown_requested ||
-             debug_requested ||
-             vmstop_requested);
+    return powerdown_requested ||
+           reset_requested ||
+           shutdown_requested ||
+           debug_requested ||
+           vmstop_requested;
 }
+#endif
 
 qemu_irq qemu_system_powerdown;
 
@@ -1393,21 +1395,19 @@ static void main_loop(void)
     qemu_main_loop_start();
 
     for (;;) {
-        do {
 #ifndef CONFIG_IOTHREAD
-            nonblocking = cpu_exec_all();
-            if (!vm_can_run()) {
-                nonblocking = true;
-            }
+        nonblocking = cpu_exec_all();
+        if (vm_request_pending()) {
+            nonblocking = true;
+        }
 #endif
 #ifdef CONFIG_PROFILER
-            ti = profile_getclock();
+        ti = profile_getclock();
 #endif
-            main_loop_wait(nonblocking);
+        main_loop_wait(nonblocking);
 #ifdef CONFIG_PROFILER
-            dev_time += profile_getclock() - ti;
+        dev_time += profile_getclock() - ti;
 #endif
-        } while (vm_can_run());
 
         if ((r = qemu_debug_requested())) {
             vm_stop(r);
-- 
1.7.1

[Qemu-devel] [PATCH v2 08/24] kvm: Report proper error on GET_VCPU_MMAP_SIZE failures

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

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

diff --git a/kvm-all.c b/kvm-all.c
index 9976762..1a55a10 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -219,6 +219,7 @@ int kvm_init_vcpu(CPUState *env)
 
     mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
     if (mmap_size < 0) {
+        ret = mmap_size;
         DPRINTF("KVM_GET_VCPU_MMAP_SIZE failed\n");
         goto err;
     }
-- 
1.7.1

[Qemu-devel] [PATCH v2 09/24] kvm: Drop redundant kvm_enabled from kvm_cpu_thread_fn

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

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

diff --git a/cpus.c b/cpus.c
index 5dfc54e..312c7a2 100644
--- a/cpus.c
+++ b/cpus.c
@@ -607,8 +607,8 @@ static void *kvm_cpu_thread_fn(void *arg)
 
     qemu_mutex_lock(&qemu_global_mutex);
     qemu_thread_self(env->thread);
-    if (kvm_enabled())
-        kvm_init_vcpu(env);
+
+    kvm_init_vcpu(env);
 
     kvm_init_ipi(env);
 
-- 
1.7.1

[Qemu-devel] [PATCH v2 10/24] kvm: Handle kvm_init_vcpu errors

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Do not ignore errors of kvm_init_vcpu, they are fatal.

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

diff --git a/cpus.c b/cpus.c
index 312c7a2..8475757 100644
--- a/cpus.c
+++ b/cpus.c
@@ -273,12 +273,18 @@ void qemu_main_loop_start(void)
 void qemu_init_vcpu(void *_env)
 {
     CPUState *env = _env;
+    int r;
 
     env->nr_cores = smp_cores;
     env->nr_threads = smp_threads;
-    if (kvm_enabled())
-        kvm_init_vcpu(env);
-    return;
+
+    if (kvm_enabled()) {
+        r = kvm_init_vcpu(env);
+        if (r < 0) {
+            fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r));
+            exit(1);
+        }
+    }
 }
 
 int qemu_cpu_self(void *env)
@@ -604,11 +610,16 @@ static int qemu_cpu_exec(CPUState *env);
 static void *kvm_cpu_thread_fn(void *arg)
 {
     CPUState *env = arg;
+    int r;
 
     qemu_mutex_lock(&qemu_global_mutex);
     qemu_thread_self(env->thread);
 
-    kvm_init_vcpu(env);
+    r = kvm_init_vcpu(env);
+    if (r < 0) {
+        fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r));
+        exit(1);
+    }
 
     kvm_init_ipi(env);
 
-- 
1.7.1

[Qemu-devel] [PATCH v2 11/24] kvm: Provide sigbus services arch-independently

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Provide arch-independent kvm_on_sigbus* stubs to remove the #ifdef'ery
from cpus.c. This patch also fixes --disable-kvm build by providing the
missing kvm_on_sigbus_vcpu kvm-stub.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Acked-by: Alexander Graf <agraf@suse.de>
---
 cpus.c             |   10 ++++------
 kvm-all.c          |   10 ++++++++++
 kvm-stub.c         |    5 +++++
 kvm.h              |    7 +++++--
 target-i386/kvm.c  |    4 ++--
 target-ppc/kvm.c   |   10 ++++++++++
 target-s390x/kvm.c |   10 ++++++++++
 7 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/cpus.c b/cpus.c
index 8475757..3a32828 100644
--- a/cpus.c
+++ b/cpus.c
@@ -543,10 +543,9 @@ static void sigbus_reraise(void)
 static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
                            void *ctx)
 {
-#if defined(TARGET_I386)
-    if (kvm_on_sigbus(siginfo->ssi_code, (void *)(intptr_t)siginfo->ssi_addr))
-#endif
+    if (kvm_on_sigbus(siginfo->ssi_code, (void *)(intptr_t)siginfo->ssi_addr)) {
         sigbus_reraise();
+    }
 }
 
 static void qemu_kvm_eat_signal(CPUState *env, int timeout)
@@ -579,10 +578,9 @@ static void qemu_kvm_eat_signal(CPUState *env, int timeout)
 
         switch (r) {
         case SIGBUS:
-#ifdef TARGET_I386
-            if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr))
-#endif
+            if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr)) {
                 sigbus_reraise();
+            }
             break;
         default:
             break;
diff --git a/kvm-all.c b/kvm-all.c
index 1a55a10..5bfa8c0 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1362,3 +1362,13 @@ int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t val, bool assign)
     return -ENOSYS;
 #endif
 }
+
+int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+{
+    return kvm_arch_on_sigbus_vcpu(env, code, addr);
+}
+
+int kvm_on_sigbus(int code, void *addr)
+{
+    return kvm_arch_on_sigbus(code, addr);
+}
diff --git a/kvm-stub.c b/kvm-stub.c
index 88682f2..d6b6c8e 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -147,6 +147,11 @@ int kvm_set_ioeventfd_mmio_long(int fd, uint32_t adr, uint32_t val, bool assign)
     return -ENOSYS;
 }
 
+int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+{
+    return 1;
+}
+
 int kvm_on_sigbus(int code, void *addr)
 {
     return 1;
diff --git a/kvm.h b/kvm.h
index ca57517..b2fb5c6 100644
--- a/kvm.h
+++ b/kvm.h
@@ -81,6 +81,9 @@ int kvm_set_signal_mask(CPUState *env, const sigset_t *sigset);
 int kvm_pit_in_kernel(void);
 int kvm_irqchip_in_kernel(void);
 
+int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr);
+int kvm_on_sigbus(int code, void *addr);
+
 /* internal API */
 
 struct KVMState;
@@ -121,8 +124,8 @@ int kvm_arch_init_vcpu(CPUState *env);
 
 void kvm_arch_reset_vcpu(CPUState *env);
 
-int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr);
-int kvm_on_sigbus(int code, void *addr);
+int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr);
+int kvm_arch_on_sigbus(int code, void *addr);
 
 struct kvm_guest_debug;
 struct kvm_debug_exit_arch;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 05010bb..9df8ff8 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1839,7 +1839,7 @@ static void kvm_mce_inj_srao_memscrub2(CPUState *env, target_phys_addr_t paddr)
 
 #endif
 
-int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr)
 {
 #if defined(KVM_CAP_MCE)
     void *vaddr;
@@ -1889,7 +1889,7 @@ int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
     return 0;
 }
 
-int kvm_on_sigbus(int code, void *addr)
+int kvm_arch_on_sigbus(int code, void *addr)
 {
 #if defined(KVM_CAP_MCE)
     if ((first_cpu->mcg_cap & MCG_SER_P) && addr && code == BUS_MCEERR_AO) {
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 710eca1..93ecc57 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -404,3 +404,13 @@ bool kvm_arch_stop_on_emulation_error(CPUState *env)
 {
     return true;
 }
+
+int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+{
+    return 1;
+}
+
+int kvm_arch_on_sigbus(int code, void *addr)
+{
+    return 1;
+}
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 38823f5..1702c46 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -505,3 +505,13 @@ bool kvm_arch_stop_on_emulation_error(CPUState *env)
 {
     return true;
 }
+
+int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+{
+    return 1;
+}
+
+int kvm_arch_on_sigbus(int code, void *addr)
+{
+    return 1;
+}
-- 
1.7.1

[Qemu-devel] [PATCH v2 12/24] Refactor signal setup functions in cpus.c

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Move {tcg,kvm}_init_ipi and block_io_signals to avoid prototypes, rename
the former two to clarify that they deal with more than SIG_IPI. No
functional changes - except for the tiny fixup of strerror usage.

The forward declaration of sigbus_handler is just temporarily, it will
be moved in a succeeding patch. dummy_signal is moved into the !_WIN32
block as we will soon need it also for !CONFIG_IOTHREAD.

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

diff --git a/cpus.c b/cpus.c
index 3a32828..42717ba 100644
--- a/cpus.c
+++ b/cpus.c
@@ -230,7 +230,15 @@ fail:
     close(fds[1]);
     return err;
 }
-#else
+
+#ifdef CONFIG_IOTHREAD
+static void dummy_signal(int sig)
+{
+}
+#endif
+
+#else /* _WIN32 */
+
 HANDLE qemu_event_handle;
 
 static void dummy_event_handler(void *opaque)
@@ -256,7 +264,7 @@ static void qemu_event_increment(void)
         exit (1);
     }
 }
-#endif
+#endif /* _WIN32 */
 
 #ifndef CONFIG_IOTHREAD
 int qemu_init_main_loop(void)
@@ -352,10 +360,6 @@ static QemuCond qemu_system_cond;
 static QemuCond qemu_pause_cond;
 static QemuCond qemu_work_cond;
 
-static void tcg_init_ipi(void);
-static void kvm_init_ipi(CPUState *env);
-static sigset_t block_io_signals(void);
-
 /* If we have signalfd, we mask out the signals we want to handle and then
  * use signalfd to listen for them.  We rely on whatever the current signal
  * handler is to dispatch the signals when we receive them.
@@ -391,6 +395,77 @@ static void sigfd_handler(void *opaque)
     }
 }
 
+static void cpu_signal(int sig)
+{
+    if (cpu_single_env) {
+        cpu_exit(cpu_single_env);
+    }
+    exit_request = 1;
+}
+
+static void qemu_kvm_init_cpu_signals(CPUState *env)
+{
+    int r;
+    sigset_t set;
+    struct sigaction sigact;
+
+    memset(&sigact, 0, sizeof(sigact));
+    sigact.sa_handler = dummy_signal;
+    sigaction(SIG_IPI, &sigact, NULL);
+
+    pthread_sigmask(SIG_BLOCK, NULL, &set);
+    sigdelset(&set, SIG_IPI);
+    sigdelset(&set, SIGBUS);
+    r = kvm_set_signal_mask(env, &set);
+    if (r) {
+        fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r));
+        exit(1);
+    }
+}
+
+static void qemu_tcg_init_cpu_signals(void)
+{
+    sigset_t set;
+    struct sigaction sigact;
+
+    memset(&sigact, 0, sizeof(sigact));
+    sigact.sa_handler = cpu_signal;
+    sigaction(SIG_IPI, &sigact, NULL);
+
+    sigemptyset(&set);
+    sigaddset(&set, SIG_IPI);
+    pthread_sigmask(SIG_UNBLOCK, &set, NULL);
+}
+
+static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
+                           void *ctx);
+
+static sigset_t block_io_signals(void)
+{
+    sigset_t set;
+    struct sigaction action;
+
+    /* SIGUSR2 used by posix-aio-compat.c */
+    sigemptyset(&set);
+    sigaddset(&set, SIGUSR2);
+    pthread_sigmask(SIG_UNBLOCK, &set, NULL);
+
+    sigemptyset(&set);
+    sigaddset(&set, SIGIO);
+    sigaddset(&set, SIGALRM);
+    sigaddset(&set, SIG_IPI);
+    sigaddset(&set, SIGBUS);
+    pthread_sigmask(SIG_BLOCK, &set, NULL);
+
+    memset(&action, 0, sizeof(action));
+    action.sa_flags = SA_SIGINFO;
+    action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler;
+    sigaction(SIGBUS, &action, NULL);
+    prctl(PR_MCE_KILL, 1, 1, 0, 0);
+
+    return set;
+}
+
 static int qemu_signalfd_init(sigset_t mask)
 {
     int sigfd;
@@ -619,7 +694,7 @@ static void *kvm_cpu_thread_fn(void *arg)
         exit(1);
     }
 
-    kvm_init_ipi(env);
+    qemu_kvm_init_cpu_signals(env);
 
     /* signal CPU creation */
     env->created = 1;
@@ -642,7 +717,7 @@ static void *tcg_cpu_thread_fn(void *arg)
 {
     CPUState *env = arg;
 
-    tcg_init_ipi();
+    qemu_tcg_init_cpu_signals();
     qemu_thread_self(env->thread);
 
     /* signal CPU creation */
@@ -683,77 +758,6 @@ int qemu_cpu_self(void *_env)
     return qemu_thread_equal(&this, env->thread);
 }
 
-static void cpu_signal(int sig)
-{
-    if (cpu_single_env)
-        cpu_exit(cpu_single_env);
-    exit_request = 1;
-}
-
-static void tcg_init_ipi(void)
-{
-    sigset_t set;
-    struct sigaction sigact;
-
-    memset(&sigact, 0, sizeof(sigact));
-    sigact.sa_handler = cpu_signal;
-    sigaction(SIG_IPI, &sigact, NULL);
-
-    sigemptyset(&set);
-    sigaddset(&set, SIG_IPI);
-    pthread_sigmask(SIG_UNBLOCK, &set, NULL);
-}
-
-static void dummy_signal(int sig)
-{
-}
-
-static void kvm_init_ipi(CPUState *env)
-{
-    int r;
-    sigset_t set;
-    struct sigaction sigact;
-
-    memset(&sigact, 0, sizeof(sigact));
-    sigact.sa_handler = dummy_signal;
-    sigaction(SIG_IPI, &sigact, NULL);
-
-    pthread_sigmask(SIG_BLOCK, NULL, &set);
-    sigdelset(&set, SIG_IPI);
-    sigdelset(&set, SIGBUS);
-    r = kvm_set_signal_mask(env, &set);
-    if (r) {
-        fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(r));
-        exit(1);
-    }
-}
-
-static sigset_t block_io_signals(void)
-{
-    sigset_t set;
-    struct sigaction action;
-
-    /* SIGUSR2 used by posix-aio-compat.c */
-    sigemptyset(&set);
-    sigaddset(&set, SIGUSR2);
-    pthread_sigmask(SIG_UNBLOCK, &set, NULL);
-
-    sigemptyset(&set);
-    sigaddset(&set, SIGIO);
-    sigaddset(&set, SIGALRM);
-    sigaddset(&set, SIG_IPI);
-    sigaddset(&set, SIGBUS);
-    pthread_sigmask(SIG_BLOCK, &set, NULL);
-
-    memset(&action, 0, sizeof(action));
-    action.sa_flags = SA_SIGINFO;
-    action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler;
-    sigaction(SIGBUS, &action, NULL);
-    prctl(PR_MCE_KILL, 1, 1, 0, 0);
-
-    return set;
-}
-
 void qemu_mutex_lock_iothread(void)
 {
     if (kvm_enabled()) {
-- 
1.7.1

[Qemu-devel] [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode.
It's unused so far, but this infrastructure will be required for
self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As
Windows doesn't support signal services, we need to provide a stub for
the init function.

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

diff --git a/cpus.c b/cpus.c
index 42717ba..a33e470 100644
--- a/cpus.c
+++ b/cpus.c
@@ -231,11 +231,9 @@ fail:
     return err;
 }
 
-#ifdef CONFIG_IOTHREAD
 static void dummy_signal(int sig)
 {
 }
-#endif
 
 #else /* _WIN32 */
 
@@ -267,6 +265,32 @@ static void qemu_event_increment(void)
 #endif /* _WIN32 */
 
 #ifndef CONFIG_IOTHREAD
+static void qemu_kvm_init_cpu_signals(CPUState *env)
+{
+#ifndef _WIN32
+    int r;
+    sigset_t set;
+    struct sigaction sigact;
+
+    memset(&sigact, 0, sizeof(sigact));
+    sigact.sa_handler = dummy_signal;
+    sigaction(SIG_IPI, &sigact, NULL);
+
+    sigemptyset(&set);
+    sigaddset(&set, SIG_IPI);
+    pthread_sigmask(SIG_BLOCK, &set, NULL);
+
+    pthread_sigmask(SIG_BLOCK, NULL, &set);
+    sigdelset(&set, SIG_IPI);
+    sigdelset(&set, SIGBUS);
+    r = kvm_set_signal_mask(env, &set);
+    if (r) {
+        fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r));
+        exit(1);
+    }
+#endif
+}
+
 int qemu_init_main_loop(void)
 {
     cpu_set_debug_excp_handler(cpu_debug_handler);
@@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env)
             fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r));
             exit(1);
         }
+        qemu_kvm_init_cpu_signals(env);
     }
 }
 
-- 
1.7.1

[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD

[Avi Kivity]

On 02/01/2011 11:15 PM, Jan Kiszka wrote:
> From: Jan Kiszka<jan.kiszka@siemens.com>
>
> Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode.
> It's unused so far, but this infrastructure will be required for
> self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As
> Windows doesn't support signal services, we need to provide a stub for
> the init function.
>

This patch breaks qemu-kvm after merging.  The symptoms are that Windows 
XP x64 does not respond when netcat connects to some server in it, via 
-net user,hostfwd.  The vcpu thread loops indefinitely on KVM_EXIT_INTR, 
which is consistent with signals being messed up.

I verified that 981085dd465c1 merged with ff48eb5fe79ad works,
while 981085dd465c1 merged with ff48eb5fe79ad^ fails.


> diff --git a/cpus.c b/cpus.c
> index 42717ba..a33e470 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -231,11 +231,9 @@ fail:
>       return err;
>   }
>
> -#ifdef CONFIG_IOTHREAD
>   static void dummy_signal(int sig)
>   {
>   }
> -#endif
>
>   #else /* _WIN32 */
>
> @@ -267,6 +265,32 @@ static void qemu_event_increment(void)
>   #endif /* _WIN32 */
>
>   #ifndef CONFIG_IOTHREAD
> +static void qemu_kvm_init_cpu_signals(CPUState *env)
> +{
> +#ifndef _WIN32
> +    int r;
> +    sigset_t set;
> +    struct sigaction sigact;
> +
> +    memset(&sigact, 0, sizeof(sigact));
> +    sigact.sa_handler = dummy_signal;
> +    sigaction(SIG_IPI,&sigact, NULL);
> +
> +    sigemptyset(&set);
> +    sigaddset(&set, SIG_IPI);
> +    pthread_sigmask(SIG_BLOCK,&set, NULL);
> +
> +    pthread_sigmask(SIG_BLOCK, NULL,&set);
> +    sigdelset(&set, SIG_IPI);
> +    sigdelset(&set, SIGBUS);
> +    r = kvm_set_signal_mask(env,&set);
> +    if (r) {
> +        fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r));
> +        exit(1);
> +    }
> +#endif
> +}
> +
>   int qemu_init_main_loop(void)
>   {
>       cpu_set_debug_excp_handler(cpu_debug_handler);
> @@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env)
>               fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r));
>               exit(1);
>           }
> +        qemu_kvm_init_cpu_signals(env);
>       }
>   }
>


-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD

[Jan Kiszka]

On 2011-02-28 16:55, Avi Kivity wrote:
> On 02/01/2011 11:15 PM, Jan Kiszka wrote:
>> From: Jan Kiszka<jan.kiszka@siemens.com>
>>
>> Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode.
>> It's unused so far, but this infrastructure will be required for
>> self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As
>> Windows doesn't support signal services, we need to provide a stub for
>> the init function.
>>
> 
> This patch breaks qemu-kvm after merging.  The symptoms are that Windows
> XP x64 does not respond when netcat connects to some server in it, via
> -net user,hostfwd.  The vcpu thread loops indefinitely on KVM_EXIT_INTR,
> which is consistent with signals being messed up.

Does the same test case work with qemu, iothread on and off? Just to
ensure we are not hunting an issue with the patch itself but of the merge.

Will have a look as well.

Jan

> 
> I verified that 981085dd465c1 merged with ff48eb5fe79ad works,
> while 981085dd465c1 merged with ff48eb5fe79ad^ fails.
> 
> 
>> diff --git a/cpus.c b/cpus.c
>> index 42717ba..a33e470 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -231,11 +231,9 @@ fail:
>>       return err;
>>   }
>>
>> -#ifdef CONFIG_IOTHREAD
>>   static void dummy_signal(int sig)
>>   {
>>   }
>> -#endif
>>
>>   #else /* _WIN32 */
>>
>> @@ -267,6 +265,32 @@ static void qemu_event_increment(void)
>>   #endif /* _WIN32 */
>>
>>   #ifndef CONFIG_IOTHREAD
>> +static void qemu_kvm_init_cpu_signals(CPUState *env)
>> +{
>> +#ifndef _WIN32
>> +    int r;
>> +    sigset_t set;
>> +    struct sigaction sigact;
>> +
>> +    memset(&sigact, 0, sizeof(sigact));
>> +    sigact.sa_handler = dummy_signal;
>> +    sigaction(SIG_IPI,&sigact, NULL);
>> +
>> +    sigemptyset(&set);
>> +    sigaddset(&set, SIG_IPI);
>> +    pthread_sigmask(SIG_BLOCK,&set, NULL);
>> +
>> +    pthread_sigmask(SIG_BLOCK, NULL,&set);
>> +    sigdelset(&set, SIG_IPI);
>> +    sigdelset(&set, SIGBUS);
>> +    r = kvm_set_signal_mask(env,&set);
>> +    if (r) {
>> +        fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r));
>> +        exit(1);
>> +    }
>> +#endif
>> +}
>> +
>>   int qemu_init_main_loop(void)
>>   {
>>       cpu_set_debug_excp_handler(cpu_debug_handler);
>> @@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env)
>>               fprintf(stderr, "kvm_init_vcpu failed: %s\n",
>> strerror(-r));
>>               exit(1);
>>           }
>> +        qemu_kvm_init_cpu_signals(env);
>>       }
>>   }
>>
> 
> 

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD

[Jan Kiszka]

On 2011-02-28 17:02, Jan Kiszka wrote:
> On 2011-02-28 16:55, Avi Kivity wrote:
>> On 02/01/2011 11:15 PM, Jan Kiszka wrote:
>>> From: Jan Kiszka<jan.kiszka@siemens.com>
>>>
>>> Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode.
>>> It's unused so far, but this infrastructure will be required for
>>> self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As
>>> Windows doesn't support signal services, we need to provide a stub for
>>> the init function.
>>>
>>
>> This patch breaks qemu-kvm after merging.  The symptoms are that Windows
>> XP x64 does not respond when netcat connects to some server in it, via
>> -net user,hostfwd.  The vcpu thread loops indefinitely on KVM_EXIT_INTR,
>> which is consistent with signals being messed up.
> 
> Does the same test case work with qemu, iothread on and off? Just to

Err, "iothread on" makes no sense here, of course.

> ensure we are not hunting an issue with the patch itself but of the merge.
> 
> Will have a look as well.
> 

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD

[Jan Kiszka]

On 2011-02-28 16:55, Avi Kivity wrote:
> On 02/01/2011 11:15 PM, Jan Kiszka wrote:
>> From: Jan Kiszka<jan.kiszka@siemens.com>
>>
>> Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode.
>> It's unused so far, but this infrastructure will be required for
>> self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As
>> Windows doesn't support signal services, we need to provide a stub for
>> the init function.
>>
> 
> This patch breaks qemu-kvm after merging.  The symptoms are that Windows
> XP x64 does not respond when netcat connects to some server in it, via
> -net user,hostfwd.  The vcpu thread loops indefinitely on KVM_EXIT_INTR,
> which is consistent with signals being messed up.
> 
> I verified that 981085dd465c1 merged with ff48eb5fe79ad works,
> while 981085dd465c1 merged with ff48eb5fe79ad^ fails.
> 
> 
>> diff --git a/cpus.c b/cpus.c
>> index 42717ba..a33e470 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -231,11 +231,9 @@ fail:
>>       return err;
>>   }
>>
>> -#ifdef CONFIG_IOTHREAD
>>   static void dummy_signal(int sig)
>>   {
>>   }
>> -#endif
>>
>>   #else /* _WIN32 */
>>
>> @@ -267,6 +265,32 @@ static void qemu_event_increment(void)
>>   #endif /* _WIN32 */
>>
>>   #ifndef CONFIG_IOTHREAD
>> +static void qemu_kvm_init_cpu_signals(CPUState *env)
>> +{
>> +#ifndef _WIN32
>> +    int r;
>> +    sigset_t set;
>> +    struct sigaction sigact;
>> +
>> +    memset(&sigact, 0, sizeof(sigact));
>> +    sigact.sa_handler = dummy_signal;
>> +    sigaction(SIG_IPI,&sigact, NULL);
>> +
>> +    sigemptyset(&set);
>> +    sigaddset(&set, SIG_IPI);
>> +    pthread_sigmask(SIG_BLOCK,&set, NULL);
>> +
>> +    pthread_sigmask(SIG_BLOCK, NULL,&set);
>> +    sigdelset(&set, SIG_IPI);
>> +    sigdelset(&set, SIGBUS);
>> +    r = kvm_set_signal_mask(env,&set);
>> +    if (r) {
>> +        fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r));
>> +        exit(1);
>> +    }
>> +#endif
>> +}
>> +
>>   int qemu_init_main_loop(void)
>>   {
>>       cpu_set_debug_excp_handler(cpu_debug_handler);
>> @@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env)
>>               fprintf(stderr, "kvm_init_vcpu failed: %s\n",
>> strerror(-r));
>>               exit(1);
>>           }
>> +        qemu_kvm_init_cpu_signals(env);

Just comment that out as long as qemu-kvm is (mis-)using !IOTHREAD mode.
I thought it would run before setup_kernel_sigmask, but it's the other
way around, and then the wrong non-iothread signal setup is applied.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD

[Avi Kivity]

On 02/28/2011 06:16 PM, Jan Kiszka wrote:
> On 2011-02-28 16:55, Avi Kivity wrote:
> >  On 02/01/2011 11:15 PM, Jan Kiszka wrote:
> >>  From: Jan Kiszka<jan.kiszka@siemens.com>
> >>
> >>  Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode.
> >>  It's unused so far, but this infrastructure will be required for
> >>  self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As
> >>  Windows doesn't support signal services, we need to provide a stub for
> >>  the init function.
> >>
> >
> >  This patch breaks qemu-kvm after merging.  The symptoms are that Windows
> >  XP x64 does not respond when netcat connects to some server in it, via
> >  -net user,hostfwd.  The vcpu thread loops indefinitely on KVM_EXIT_INTR,
> >  which is consistent with signals being messed up.
> >
> >  I verified that 981085dd465c1 merged with ff48eb5fe79ad works,
> >  while 981085dd465c1 merged with ff48eb5fe79ad^ fails.
> >
> >
> >>  diff --git a/cpus.c b/cpus.c
> >>  index 42717ba..a33e470 100644
> >>  --- a/cpus.c
> >>  +++ b/cpus.c
> >>  @@ -231,11 +231,9 @@ fail:
> >>        return err;
> >>    }
> >>
> >>  -#ifdef CONFIG_IOTHREAD
> >>    static void dummy_signal(int sig)
> >>    {
> >>    }
> >>  -#endif
> >>
> >>    #else /* _WIN32 */
> >>
> >>  @@ -267,6 +265,32 @@ static void qemu_event_increment(void)
> >>    #endif /* _WIN32 */
> >>
> >>    #ifndef CONFIG_IOTHREAD
> >>  +static void qemu_kvm_init_cpu_signals(CPUState *env)
> >>  +{
> >>  +#ifndef _WIN32
> >>  +    int r;
> >>  +    sigset_t set;
> >>  +    struct sigaction sigact;
> >>  +
> >>  +    memset(&sigact, 0, sizeof(sigact));
> >>  +    sigact.sa_handler = dummy_signal;
> >>  +    sigaction(SIG_IPI,&sigact, NULL);
> >>  +
> >>  +    sigemptyset(&set);
> >>  +    sigaddset(&set, SIG_IPI);
> >>  +    pthread_sigmask(SIG_BLOCK,&set, NULL);
> >>  +
> >>  +    pthread_sigmask(SIG_BLOCK, NULL,&set);
> >>  +    sigdelset(&set, SIG_IPI);
> >>  +    sigdelset(&set, SIGBUS);
> >>  +    r = kvm_set_signal_mask(env,&set);
> >>  +    if (r) {
> >>  +        fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r));
> >>  +        exit(1);
> >>  +    }
> >>  +#endif
> >>  +}
> >>  +
> >>    int qemu_init_main_loop(void)
> >>    {
> >>        cpu_set_debug_excp_handler(cpu_debug_handler);
> >>  @@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env)
> >>                fprintf(stderr, "kvm_init_vcpu failed: %s\n",
> >>  strerror(-r));
> >>                exit(1);
> >>            }
> >>  +        qemu_kvm_init_cpu_signals(env);
>
> Just comment that out as long as qemu-kvm is (mis-)using !IOTHREAD mode.
> I thought it would run before setup_kernel_sigmask, but it's the other
> way around, and then the wrong non-iothread signal setup is applied.

That's what I tried, and it didn't work?!  Maybe I forgot to compile or 
something.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD

[Avi Kivity]

On 02/28/2011 06:45 PM, Avi Kivity wrote:
>
> That's what I tried, and it didn't work?!  Maybe I forgot to compile 
> or something.
>

I misspelled #ifdef.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD

[Jan Kiszka]

On 2011-02-28 17:45, Avi Kivity wrote:
> On 02/28/2011 06:16 PM, Jan Kiszka wrote:
>> On 2011-02-28 16:55, Avi Kivity wrote:
>>>  On 02/01/2011 11:15 PM, Jan Kiszka wrote:
>>>>  From: Jan Kiszka<jan.kiszka@siemens.com>
>>>>
>>>>  Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode.
>>>>  It's unused so far, but this infrastructure will be required for
>>>>  self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As
>>>>  Windows doesn't support signal services, we need to provide a stub for
>>>>  the init function.
>>>>
>>>
>>>  This patch breaks qemu-kvm after merging.  The symptoms are that Windows
>>>  XP x64 does not respond when netcat connects to some server in it, via
>>>  -net user,hostfwd.  The vcpu thread loops indefinitely on KVM_EXIT_INTR,
>>>  which is consistent with signals being messed up.
>>>
>>>  I verified that 981085dd465c1 merged with ff48eb5fe79ad works,
>>>  while 981085dd465c1 merged with ff48eb5fe79ad^ fails.
>>>
>>>
>>>>  diff --git a/cpus.c b/cpus.c
>>>>  index 42717ba..a33e470 100644
>>>>  --- a/cpus.c
>>>>  +++ b/cpus.c
>>>>  @@ -231,11 +231,9 @@ fail:
>>>>        return err;
>>>>    }
>>>>
>>>>  -#ifdef CONFIG_IOTHREAD
>>>>    static void dummy_signal(int sig)
>>>>    {
>>>>    }
>>>>  -#endif
>>>>
>>>>    #else /* _WIN32 */
>>>>
>>>>  @@ -267,6 +265,32 @@ static void qemu_event_increment(void)
>>>>    #endif /* _WIN32 */
>>>>
>>>>    #ifndef CONFIG_IOTHREAD
>>>>  +static void qemu_kvm_init_cpu_signals(CPUState *env)
>>>>  +{
>>>>  +#ifndef _WIN32
>>>>  +    int r;
>>>>  +    sigset_t set;
>>>>  +    struct sigaction sigact;
>>>>  +
>>>>  +    memset(&sigact, 0, sizeof(sigact));
>>>>  +    sigact.sa_handler = dummy_signal;
>>>>  +    sigaction(SIG_IPI,&sigact, NULL);
>>>>  +
>>>>  +    sigemptyset(&set);
>>>>  +    sigaddset(&set, SIG_IPI);
>>>>  +    pthread_sigmask(SIG_BLOCK,&set, NULL);
>>>>  +
>>>>  +    pthread_sigmask(SIG_BLOCK, NULL,&set);
>>>>  +    sigdelset(&set, SIG_IPI);
>>>>  +    sigdelset(&set, SIGBUS);
>>>>  +    r = kvm_set_signal_mask(env,&set);
>>>>  +    if (r) {
>>>>  +        fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r));
>>>>  +        exit(1);
>>>>  +    }
>>>>  +#endif
>>>>  +}
>>>>  +
>>>>    int qemu_init_main_loop(void)
>>>>    {
>>>>        cpu_set_debug_excp_handler(cpu_debug_handler);
>>>>  @@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env)
>>>>                fprintf(stderr, "kvm_init_vcpu failed: %s\n",
>>>>  strerror(-r));
>>>>                exit(1);
>>>>            }
>>>>  +        qemu_kvm_init_cpu_signals(env);
>>
>> Just comment that out as long as qemu-kvm is (mis-)using !IOTHREAD mode.
>> I thought it would run before setup_kernel_sigmask, but it's the other
>> way around, and then the wrong non-iothread signal setup is applied.
> 
> That's what I tried, and it didn't work?!  Maybe I forgot to compile or 
> something.

Well, it maybe failed to build as qemu_kvm_init_cpu_signals became
unused and the compiler should have bailed out? Probably it's better to
disable it directly in the function.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD

[Avi Kivity]

On 02/28/2011 06:49 PM, Jan Kiszka wrote:
> >
> >  That's what I tried, and it didn't work?!  Maybe I forgot to compile or
> >  something.
>
> Well, it maybe failed to build as qemu_kvm_init_cpu_signals became
> unused and the compiler should have bailed out? Probably it's better to
> disable it directly in the function.
>

That's what I did, with #ifdefs, but brokenly (#ifndef).


-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD

[Avi Kivity]

On 02/28/2011 06:54 PM, Avi Kivity wrote:
> On 02/28/2011 06:49 PM, Jan Kiszka wrote:
>> >
>> >  That's what I tried, and it didn't work?!  Maybe I forgot to 
>> compile or
>> >  something.
>>
>> Well, it maybe failed to build as qemu_kvm_init_cpu_signals became
>> unused and the compiler should have bailed out? Probably it's better to
>> disable it directly in the function.
>>
>
> That's what I did, with #ifdefs, but brokenly (#ifndef).
>
>

Well it fails even with the correct #ifdef.  Maybe some later patch adds 
to the breakage.

This is really strange - the same test (migrate.tcp) works for Fedora 
and Windows XP x86.  Installation and setup of Windows XP x64 work 
fine.  It is only migration.tcp (when using netcat to connect to the 
guest) that fails.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD

[Jan Kiszka]

On 2011-03-01 09:39, Avi Kivity wrote:
> On 02/28/2011 06:54 PM, Avi Kivity wrote:
>> On 02/28/2011 06:49 PM, Jan Kiszka wrote:
>>>>
>>>>  That's what I tried, and it didn't work?!  Maybe I forgot to 
>>> compile or
>>>>  something.
>>>
>>> Well, it maybe failed to build as qemu_kvm_init_cpu_signals became
>>> unused and the compiler should have bailed out? Probably it's better to
>>> disable it directly in the function.
>>>
>>
>> That's what I did, with #ifdefs, but brokenly (#ifndef).
>>
>>
> 
> Well it fails even with the correct #ifdef.  Maybe some later patch adds 
> to the breakage.

But when ifdef'ed out, this patch should be a nop for qemu-kvm. Indeed
strange.

> 
> This is really strange - the same test (migrate.tcp) works for Fedora 
> and Windows XP x86.  Installation and setup of Windows XP x64 work 
> fine.  It is only migration.tcp (when using netcat to connect to the 
> guest) that fails.
> 

Guess this has to be classically debugged. :-/ Let me know if I can help
(though not today).

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD

[Avi Kivity]

On 03/01/2011 10:58 AM, Jan Kiszka wrote:
> On 2011-03-01 09:39, Avi Kivity wrote:
> >  On 02/28/2011 06:54 PM, Avi Kivity wrote:
> >>  On 02/28/2011 06:49 PM, Jan Kiszka wrote:
> >>>>
> >>>>   That's what I tried, and it didn't work?!  Maybe I forgot to
> >>>  compile or
> >>>>   something.
> >>>
> >>>  Well, it maybe failed to build as qemu_kvm_init_cpu_signals became
> >>>  unused and the compiler should have bailed out? Probably it's better to
> >>>  disable it directly in the function.
> >>>
> >>
> >>  That's what I did, with #ifdefs, but brokenly (#ifndef).
> >>
> >>
> >
> >  Well it fails even with the correct #ifdef.  Maybe some later patch adds
> >  to the breakage.
>
> But when ifdef'ed out, this patch should be a nop for qemu-kvm. Indeed
> strange.

Well, there are two functions in cpus.c named 
qemu_kvm_init_cpu_signals() (an intriguing coincidence).  I #ifdefed the 
wrong one.  With the right #ifdef it works correctly.

> >
> >  This is really strange - the same test (migrate.tcp) works for Fedora
> >  and Windows XP x86.  Installation and setup of Windows XP x64 work
> >  fine.  It is only migration.tcp (when using netcat to connect to the
> >  guest) that fails.
> >
>
> Guess this has to be classically debugged. :-/ Let me know if I can help
> (though not today).

Still has to be debugged, but at least the tree is alive now.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] [PATCH v2 14/24] kvm: Refactor qemu_kvm_eat_signals

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

We do not use the timeout, so drop its logic. As we always poll our
signals, we do not need to drop the global lock. Removing those calls
allows some further simplifications. Also fix the error processing of
sigpending at this chance.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |   23 +++++++----------------
 1 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/cpus.c b/cpus.c
index a33e470..04138ba 100644
--- a/cpus.c
+++ b/cpus.c
@@ -648,31 +648,22 @@ static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
     }
 }
 
-static void qemu_kvm_eat_signal(CPUState *env, int timeout)
+static void qemu_kvm_eat_signals(CPUState *env)
 {
-    struct timespec ts;
-    int r, e;
+    struct timespec ts = { 0, 0 };
     siginfo_t siginfo;
     sigset_t waitset;
     sigset_t chkset;
-
-    ts.tv_sec = timeout / 1000;
-    ts.tv_nsec = (timeout % 1000) * 1000000;
+    int r;
 
     sigemptyset(&waitset);
     sigaddset(&waitset, SIG_IPI);
     sigaddset(&waitset, SIGBUS);
 
     do {
-        qemu_mutex_unlock(&qemu_global_mutex);
-
         r = sigtimedwait(&waitset, &siginfo, &ts);
-        e = errno;
-
-        qemu_mutex_lock(&qemu_global_mutex);
-
-        if (r == -1 && !(e == EAGAIN || e == EINTR)) {
-            fprintf(stderr, "sigtimedwait: %s\n", strerror(e));
+        if (r == -1 && !(errno == EAGAIN || errno == EINTR)) {
+            perror("sigtimedwait");
             exit(1);
         }
 
@@ -688,7 +679,7 @@ static void qemu_kvm_eat_signal(CPUState *env, int timeout)
 
         r = sigpending(&chkset);
         if (r == -1) {
-            fprintf(stderr, "sigpending: %s\n", strerror(e));
+            perror("sigpending");
             exit(1);
         }
     } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS));
@@ -699,7 +690,7 @@ static void qemu_kvm_wait_io_event(CPUState *env)
     while (!cpu_has_work(env))
         qemu_cond_timedwait(env->halt_cond, &qemu_global_mutex, 1000);
 
-    qemu_kvm_eat_signal(env, 0);
+    qemu_kvm_eat_signals(env);
     qemu_wait_io_event_common(env);
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH v2 15/24] kvm: Call qemu_kvm_eat_signals also under !CONFIG_IOTHREAD

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Move qemu_kvm_eat_signals around and call it also when the IO-thread is
not used. Do not yet process SIGBUS, will be armed in a separate step.

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

diff --git a/cpus.c b/cpus.c
index 04138ba..861e270 100644
--- a/cpus.c
+++ b/cpus.c
@@ -235,6 +235,47 @@ static void dummy_signal(int sig)
 {
 }
 
+static void sigbus_reraise(void);
+
+static void qemu_kvm_eat_signals(CPUState *env)
+{
+    struct timespec ts = { 0, 0 };
+    siginfo_t siginfo;
+    sigset_t waitset;
+    sigset_t chkset;
+    int r;
+
+    sigemptyset(&waitset);
+    sigaddset(&waitset, SIG_IPI);
+    sigaddset(&waitset, SIGBUS);
+
+    do {
+        r = sigtimedwait(&waitset, &siginfo, &ts);
+        if (r == -1 && !(errno == EAGAIN || errno == EINTR)) {
+            perror("sigtimedwait");
+            exit(1);
+        }
+
+        switch (r) {
+#ifdef CONFIG_IOTHREAD
+        case SIGBUS:
+            if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr)) {
+                sigbus_reraise();
+            }
+            break;
+#endif
+        default:
+            break;
+        }
+
+        r = sigpending(&chkset);
+        if (r == -1) {
+            perror("sigpending");
+            exit(1);
+        }
+    } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS));
+}
+
 #else /* _WIN32 */
 
 HANDLE qemu_event_handle;
@@ -262,6 +303,10 @@ static void qemu_event_increment(void)
         exit (1);
     }
 }
+
+static void qemu_kvm_eat_signals(CPUState *env)
+{
+}
 #endif /* _WIN32 */
 
 #ifndef CONFIG_IOTHREAD
@@ -648,43 +693,6 @@ static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
     }
 }
 
-static void qemu_kvm_eat_signals(CPUState *env)
-{
-    struct timespec ts = { 0, 0 };
-    siginfo_t siginfo;
-    sigset_t waitset;
-    sigset_t chkset;
-    int r;
-
-    sigemptyset(&waitset);
-    sigaddset(&waitset, SIG_IPI);
-    sigaddset(&waitset, SIGBUS);
-
-    do {
-        r = sigtimedwait(&waitset, &siginfo, &ts);
-        if (r == -1 && !(errno == EAGAIN || errno == EINTR)) {
-            perror("sigtimedwait");
-            exit(1);
-        }
-
-        switch (r) {
-        case SIGBUS:
-            if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr)) {
-                sigbus_reraise();
-            }
-            break;
-        default:
-            break;
-        }
-
-        r = sigpending(&chkset);
-        if (r == -1) {
-            perror("sigpending");
-            exit(1);
-        }
-    } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS));
-}
-
 static void qemu_kvm_wait_io_event(CPUState *env)
 {
     while (!cpu_has_work(env))
@@ -949,6 +957,8 @@ static int qemu_cpu_exec(CPUState *env)
 
 bool cpu_exec_all(void)
 {
+    int r;
+
     if (next_cpu == NULL)
         next_cpu = first_cpu;
     for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
@@ -960,7 +970,11 @@ bool cpu_exec_all(void)
         if (qemu_alarm_pending())
             break;
         if (cpu_can_run(env)) {
-            if (qemu_cpu_exec(env) == EXCP_DEBUG) {
+            r = qemu_cpu_exec(env);
+            if (kvm_enabled()) {
+                qemu_kvm_eat_signals(env);
+            }
+            if (r == EXCP_DEBUG) {
                 break;
             }
         } else if (env->stop) {
-- 
1.7.1

[Qemu-devel] [PATCH v2 16/24] Set up signalfd under !CONFIG_IOTHREAD

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Will be required for SIGBUS handling. For obvious reasons, this will
remain a nop on Windows hosts.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.objs |    2 +-
 cpus.c        |  117 +++++++++++++++++++++++++++++++--------------------------
 2 files changed, 65 insertions(+), 54 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index c3e52c5..81b9a5b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -141,7 +141,7 @@ common-obj-y += $(addprefix ui/, $(ui-obj-y))
 
 common-obj-y += iov.o acl.o
 common-obj-$(CONFIG_THREAD) += qemu-thread.o
-common-obj-$(CONFIG_IOTHREAD) += compatfd.o
+common-obj-$(CONFIG_POSIX) += compatfd.o
 common-obj-y += notify.o event_notifier.o
 common-obj-y += qemu-timer.o qemu-timer-common.o
 
diff --git a/cpus.c b/cpus.c
index 861e270..359361f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -235,6 +235,59 @@ static void dummy_signal(int sig)
 {
 }
 
+/* If we have signalfd, we mask out the signals we want to handle and then
+ * use signalfd to listen for them.  We rely on whatever the current signal
+ * handler is to dispatch the signals when we receive them.
+ */
+static void sigfd_handler(void *opaque)
+{
+    int fd = (unsigned long) opaque;
+    struct qemu_signalfd_siginfo info;
+    struct sigaction action;
+    ssize_t len;
+
+    while (1) {
+        do {
+            len = read(fd, &info, sizeof(info));
+        } while (len == -1 && errno == EINTR);
+
+        if (len == -1 && errno == EAGAIN) {
+            break;
+        }
+
+        if (len != sizeof(info)) {
+            printf("read from sigfd returned %zd: %m\n", len);
+            return;
+        }
+
+        sigaction(info.ssi_signo, NULL, &action);
+        if ((action.sa_flags & SA_SIGINFO) && action.sa_sigaction) {
+            action.sa_sigaction(info.ssi_signo,
+                                (siginfo_t *)&info, NULL);
+        } else if (action.sa_handler) {
+            action.sa_handler(info.ssi_signo);
+        }
+    }
+}
+
+static int qemu_signalfd_init(sigset_t mask)
+{
+    int sigfd;
+
+    sigfd = qemu_signalfd(&mask);
+    if (sigfd == -1) {
+        fprintf(stderr, "failed to create signalfd\n");
+        return -errno;
+    }
+
+    fcntl_setfl(sigfd, O_NONBLOCK);
+
+    qemu_set_fd_handler2(sigfd, NULL, sigfd_handler, NULL,
+                         (void *)(unsigned long) sigfd);
+
+    return 0;
+}
+
 static void sigbus_reraise(void);
 
 static void qemu_kvm_eat_signals(CPUState *env)
@@ -338,6 +391,17 @@ static void qemu_kvm_init_cpu_signals(CPUState *env)
 
 int qemu_init_main_loop(void)
 {
+#ifndef _WIN32
+    sigset_t blocked_signals;
+    int ret;
+
+    sigemptyset(&blocked_signals);
+
+    ret = qemu_signalfd_init(blocked_signals);
+    if (ret) {
+        return ret;
+    }
+#endif
     cpu_set_debug_excp_handler(cpu_debug_handler);
 
     return qemu_event_init();
@@ -430,41 +494,6 @@ static QemuCond qemu_system_cond;
 static QemuCond qemu_pause_cond;
 static QemuCond qemu_work_cond;
 
-/* If we have signalfd, we mask out the signals we want to handle and then
- * use signalfd to listen for them.  We rely on whatever the current signal
- * handler is to dispatch the signals when we receive them.
- */
-static void sigfd_handler(void *opaque)
-{
-    int fd = (unsigned long) opaque;
-    struct qemu_signalfd_siginfo info;
-    struct sigaction action;
-    ssize_t len;
-
-    while (1) {
-        do {
-            len = read(fd, &info, sizeof(info));
-        } while (len == -1 && errno == EINTR);
-
-        if (len == -1 && errno == EAGAIN) {
-            break;
-        }
-
-        if (len != sizeof(info)) {
-            printf("read from sigfd returned %zd: %m\n", len);
-            return;
-        }
-
-        sigaction(info.ssi_signo, NULL, &action);
-        if ((action.sa_flags & SA_SIGINFO) && action.sa_sigaction) {
-            action.sa_sigaction(info.ssi_signo,
-                                (siginfo_t *)&info, NULL);
-        } else if (action.sa_handler) {
-            action.sa_handler(info.ssi_signo);
-        }
-    }
-}
-
 static void cpu_signal(int sig)
 {
     if (cpu_single_env) {
@@ -536,24 +565,6 @@ static sigset_t block_io_signals(void)
     return set;
 }
 
-static int qemu_signalfd_init(sigset_t mask)
-{
-    int sigfd;
-
-    sigfd = qemu_signalfd(&mask);
-    if (sigfd == -1) {
-        fprintf(stderr, "failed to create signalfd\n");
-        return -errno;
-    }
-
-    fcntl_setfl(sigfd, O_NONBLOCK);
-
-    qemu_set_fd_handler2(sigfd, NULL, sigfd_handler, NULL,
-                         (void *)(unsigned long) sigfd);
-
-    return 0;
-}
-
 int qemu_init_main_loop(void)
 {
     int ret;
-- 
1.7.1

[Qemu-devel] [PATCH v2 17/24] kvm: Fix race between timer signals and vcpu entry under !IOTHREAD

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Found by Stefan Hajnoczi: There is a race in kvm_cpu_exec between
checking for exit_request on vcpu entry and timer signals arriving
before KVM starts to catch them. Plug it by blocking both timer related
signals also on !CONFIG_IOTHREAD and process those via signalfd.

As this fix depends on real signalfd support (otherwise the timer
signals only kick the compat helper thread, and the main thread hangs),
we need to detect the invalid constellation and abort configure.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
CC: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 configure |    6 ++++++
 cpus.c    |   31 ++++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 4673bf0..368ca8a 100755
--- a/configure
+++ b/configure
@@ -2056,6 +2056,12 @@ EOF
 
 if compile_prog "" "" ; then
   signalfd=yes
+elif test "$kvm" = "yes" -a "$io_thread" != "yes"; then
+  echo
+  echo "ERROR: Host kernel lacks signalfd() support,"
+  echo "but KVM depends on it when the IO thread is disabled."
+  echo
+  exit 1
 fi
 
 # check if eventfd is supported
diff --git a/cpus.c b/cpus.c
index 359361f..18caf47 100644
--- a/cpus.c
+++ b/cpus.c
@@ -327,6 +327,12 @@ static void qemu_kvm_eat_signals(CPUState *env)
             exit(1);
         }
     } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS));
+
+#ifndef CONFIG_IOTHREAD
+    if (sigismember(&chkset, SIGIO) || sigismember(&chkset, SIGALRM)) {
+        qemu_notify_event();
+    }
+#endif
 }
 
 #else /* _WIN32 */
@@ -376,11 +382,15 @@ static void qemu_kvm_init_cpu_signals(CPUState *env)
 
     sigemptyset(&set);
     sigaddset(&set, SIG_IPI);
+    sigaddset(&set, SIGIO);
+    sigaddset(&set, SIGALRM);
     pthread_sigmask(SIG_BLOCK, &set, NULL);
 
     pthread_sigmask(SIG_BLOCK, NULL, &set);
     sigdelset(&set, SIG_IPI);
     sigdelset(&set, SIGBUS);
+    sigdelset(&set, SIGIO);
+    sigdelset(&set, SIGALRM);
     r = kvm_set_signal_mask(env, &set);
     if (r) {
         fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r));
@@ -389,13 +399,32 @@ static void qemu_kvm_init_cpu_signals(CPUState *env)
 #endif
 }
 
+#ifndef _WIN32
+static sigset_t block_synchronous_signals(void)
+{
+    sigset_t set;
+
+    sigemptyset(&set);
+    if (kvm_enabled()) {
+        /*
+         * We need to process timer signals synchronously to avoid a race
+         * between exit_request check and KVM vcpu entry.
+         */
+        sigaddset(&set, SIGIO);
+        sigaddset(&set, SIGALRM);
+    }
+
+    return set;
+}
+#endif
+
 int qemu_init_main_loop(void)
 {
 #ifndef _WIN32
     sigset_t blocked_signals;
     int ret;
 
-    sigemptyset(&blocked_signals);
+    blocked_signals = block_synchronous_signals();
 
     ret = qemu_signalfd_init(blocked_signals);
     if (ret) {
-- 
1.7.1

[Qemu-devel] [PATCH v2 18/24] kvm: Add MCE signal support for !CONFIG_IOTHREAD

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Currently, we only configure and process MCE-related SIGBUS events if
CONFIG_IOTHREAD is enabled. The groundwork is laid, we just need to
factor out the required handler registration and system configuration.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
CC: Huang Ying <ying.huang@intel.com>
CC: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
CC: Jin Dongming <jin.dongming@np.css.fujitsu.com>
---
 cpus.c |  107 +++++++++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 65 insertions(+), 42 deletions(-)

diff --git a/cpus.c b/cpus.c
index 18caf47..c4c5914 100644
--- a/cpus.c
+++ b/cpus.c
@@ -34,9 +34,6 @@
 
 #include "cpus.h"
 #include "compatfd.h"
-#ifdef CONFIG_LINUX
-#include <sys/prctl.h>
-#endif
 
 #ifdef SIGRTMIN
 #define SIG_IPI (SIGRTMIN+4)
@@ -44,10 +41,24 @@
 #define SIG_IPI SIGUSR1
 #endif
 
+#ifdef CONFIG_LINUX
+
+#include <sys/prctl.h>
+
 #ifndef PR_MCE_KILL
 #define PR_MCE_KILL 33
 #endif
 
+#ifndef PR_MCE_KILL_SET
+#define PR_MCE_KILL_SET 1
+#endif
+
+#ifndef PR_MCE_KILL_EARLY
+#define PR_MCE_KILL_EARLY 1
+#endif
+
+#endif /* CONFIG_LINUX */
+
 static CPUState *next_cpu;
 
 /***********************************************************/
@@ -166,6 +177,52 @@ static void cpu_debug_handler(CPUState *env)
     vm_stop(EXCP_DEBUG);
 }
 
+#ifdef CONFIG_LINUX
+static void sigbus_reraise(void)
+{
+    sigset_t set;
+    struct sigaction action;
+
+    memset(&action, 0, sizeof(action));
+    action.sa_handler = SIG_DFL;
+    if (!sigaction(SIGBUS, &action, NULL)) {
+        raise(SIGBUS);
+        sigemptyset(&set);
+        sigaddset(&set, SIGBUS);
+        sigprocmask(SIG_UNBLOCK, &set, NULL);
+    }
+    perror("Failed to re-raise SIGBUS!\n");
+    abort();
+}
+
+static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
+                           void *ctx)
+{
+    if (kvm_on_sigbus(siginfo->ssi_code,
+                      (void *)(intptr_t)siginfo->ssi_addr)) {
+        sigbus_reraise();
+    }
+}
+
+static void qemu_init_sigbus(void)
+{
+    struct sigaction action;
+
+    memset(&action, 0, sizeof(action));
+    action.sa_flags = SA_SIGINFO;
+    action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler;
+    sigaction(SIGBUS, &action, NULL);
+
+    prctl(PR_MCE_KILL, PR_MCE_KILL_SET, PR_MCE_KILL_EARLY, 0, 0);
+}
+
+#else /* !CONFIG_LINUX */
+
+static void qemu_init_sigbus(void)
+{
+}
+#endif /* !CONFIG_LINUX */
+
 #ifndef _WIN32
 static int io_thread_fd = -1;
 
@@ -288,8 +345,6 @@ static int qemu_signalfd_init(sigset_t mask)
     return 0;
 }
 
-static void sigbus_reraise(void);
-
 static void qemu_kvm_eat_signals(CPUState *env)
 {
     struct timespec ts = { 0, 0 };
@@ -310,13 +365,11 @@ static void qemu_kvm_eat_signals(CPUState *env)
         }
 
         switch (r) {
-#ifdef CONFIG_IOTHREAD
         case SIGBUS:
             if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr)) {
                 sigbus_reraise();
             }
             break;
-#endif
         default:
             break;
         }
@@ -405,6 +458,7 @@ static sigset_t block_synchronous_signals(void)
     sigset_t set;
 
     sigemptyset(&set);
+    sigaddset(&set, SIGBUS);
     if (kvm_enabled()) {
         /*
          * We need to process timer signals synchronously to avoid a race
@@ -433,6 +487,8 @@ int qemu_init_main_loop(void)
 #endif
     cpu_set_debug_excp_handler(cpu_debug_handler);
 
+    qemu_init_sigbus();
+
     return qemu_event_init();
 }
 
@@ -565,13 +621,9 @@ static void qemu_tcg_init_cpu_signals(void)
     pthread_sigmask(SIG_UNBLOCK, &set, NULL);
 }
 
-static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
-                           void *ctx);
-
 static sigset_t block_io_signals(void)
 {
     sigset_t set;
-    struct sigaction action;
 
     /* SIGUSR2 used by posix-aio-compat.c */
     sigemptyset(&set);
@@ -585,12 +637,6 @@ static sigset_t block_io_signals(void)
     sigaddset(&set, SIGBUS);
     pthread_sigmask(SIG_BLOCK, &set, NULL);
 
-    memset(&action, 0, sizeof(action));
-    action.sa_flags = SA_SIGINFO;
-    action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler;
-    sigaction(SIGBUS, &action, NULL);
-    prctl(PR_MCE_KILL, 1, 1, 0, 0);
-
     return set;
 }
 
@@ -601,6 +647,8 @@ int qemu_init_main_loop(void)
 
     cpu_set_debug_excp_handler(cpu_debug_handler);
 
+    qemu_init_sigbus();
+
     blocked_signals = block_io_signals();
 
     ret = qemu_signalfd_init(blocked_signals);
@@ -708,31 +756,6 @@ static void qemu_tcg_wait_io_event(void)
     }
 }
 
-static void sigbus_reraise(void)
-{
-    sigset_t set;
-    struct sigaction action;
-
-    memset(&action, 0, sizeof(action));
-    action.sa_handler = SIG_DFL;
-    if (!sigaction(SIGBUS, &action, NULL)) {
-        raise(SIGBUS);
-        sigemptyset(&set);
-        sigaddset(&set, SIGBUS);
-        sigprocmask(SIG_UNBLOCK, &set, NULL);
-    }
-    perror("Failed to re-raise SIGBUS!\n");
-    abort();
-}
-
-static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
-                           void *ctx)
-{
-    if (kvm_on_sigbus(siginfo->ssi_code, (void *)(intptr_t)siginfo->ssi_addr)) {
-        sigbus_reraise();
-    }
-}
-
 static void qemu_kvm_wait_io_event(CPUState *env)
 {
     while (!cpu_has_work(env))
-- 
1.7.1

[Qemu-devel] [PATCH v2 19/24] Introduce VCPU self-signaling service

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Introduce qemu_cpu_kick_self to send SIG_IPI to the calling VCPU
context. First user will be kvm.

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

diff --git a/cpus.c b/cpus.c
index c4c5914..9c50a34 100644
--- a/cpus.c
+++ b/cpus.c
@@ -537,6 +537,17 @@ void qemu_cpu_kick(void *env)
     return;
 }
 
+void qemu_cpu_kick_self(void)
+{
+#ifndef _WIN32
+    assert(cpu_single_env);
+
+    raise(SIG_IPI);
+#else
+    abort();
+#endif
+}
+
 void qemu_notify_event(void)
 {
     CPUState *env = cpu_single_env;
@@ -835,6 +846,16 @@ void qemu_cpu_kick(void *_env)
     }
 }
 
+void qemu_cpu_kick_self(void)
+{
+    assert(cpu_single_env);
+
+    if (!cpu_single_env->thread_kicked) {
+        qemu_thread_signal(cpu_single_env->thread, SIG_IPI);
+        cpu_single_env->thread_kicked = true;
+    }
+}
+
 int qemu_cpu_self(void *_env)
 {
     CPUState *env = _env;
diff --git a/qemu-common.h b/qemu-common.h
index 63d9943..220c8c8 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -287,6 +287,7 @@ void qemu_notify_event(void);
 
 /* Unblock cpu */
 void qemu_cpu_kick(void *env);
+void qemu_cpu_kick_self(void);
 int qemu_cpu_self(void *env);
 
 /* work queue */
-- 
1.7.1

[Qemu-devel] [PATCH v2 20/24] kvm: Unconditionally reenter kernel after IO exits

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

KVM requires to reenter the kernel after IO exits in order to complete
instruction emulation. Failing to do so will leave the kernel state
inconsistently behind. To ensure that we will get back ASAP, we issue a
self-signal that will cause KVM_RUN to return once the pending
operations are completed.

We can move kvm_arch_process_irqchip_events out of the inner VCPU loop.
The only state that mattered at its old place was a pending INIT
request. Catch it in kvm_arch_pre_run and also trigger a self-signal to
process the request on next kvm_cpu_exec.

This patch also fixes the missing exit_request check in kvm_cpu_exec in
the CONFIG_IOTHREAD case.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
CC: Gleb Natapov <gleb@redhat.com>
---
 kvm-all.c         |   31 +++++++++++++++++--------------
 target-i386/kvm.c |    5 +++++
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 5bfa8c0..d961697 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -199,7 +199,6 @@ int kvm_pit_in_kernel(void)
     return kvm_state->pit_in_kernel;
 }
 
-
 int kvm_init_vcpu(CPUState *env)
 {
     KVMState *s = kvm_state;
@@ -892,29 +891,33 @@ int kvm_cpu_exec(CPUState *env)
 
     DPRINTF("kvm_cpu_exec()\n");
 
-    do {
-#ifndef CONFIG_IOTHREAD
-        if (env->exit_request) {
-            DPRINTF("interrupt exit requested\n");
-            ret = 0;
-            break;
-        }
-#endif
-
-        if (kvm_arch_process_irqchip_events(env)) {
-            ret = 0;
-            break;
-        }
+    if (kvm_arch_process_irqchip_events(env)) {
+        env->exit_request = 0;
+        env->exception_index = EXCP_HLT;
+        return 0;
+    }
 
+    do {
         if (env->kvm_vcpu_dirty) {
             kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE);
             env->kvm_vcpu_dirty = 0;
         }
 
         kvm_arch_pre_run(env, run);
+        if (env->exit_request) {
+            DPRINTF("interrupt exit requested\n");
+            /*
+             * KVM requires us to reenter the kernel after IO exits to complete
+             * instruction emulation. This self-signal will ensure that we
+             * leave ASAP again.
+             */
+            qemu_cpu_kick_self();
+        }
         cpu_single_env = NULL;
         qemu_mutex_unlock_iothread();
+
         ret = kvm_vcpu_ioctl(env, KVM_RUN, 0);
+
         qemu_mutex_lock_iothread();
         cpu_single_env = env;
         kvm_arch_post_run(env, run);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 9df8ff8..8a87244 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1426,6 +1426,11 @@ int kvm_arch_get_registers(CPUState *env)
 
 int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
 {
+    /* Force the VCPU out of its inner loop to process the INIT request */
+    if (env->interrupt_request & CPU_INTERRUPT_INIT) {
+        env->exit_request = 1;
+    }
+
     /* Inject NMI */
     if (env->interrupt_request & CPU_INTERRUPT_NMI) {
         env->interrupt_request &= ~CPU_INTERRUPT_NMI;
-- 
1.7.1

[Qemu-devel] [PATCH v2 21/24] kvm: Remove static return code of kvm_handle_io

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Improve the readability of the exit dispatcher by moving the static
return value of kvm_handle_io to its caller.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index d961697..cf54256 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -770,8 +770,8 @@ err:
     return ret;
 }
 
-static int kvm_handle_io(uint16_t port, void *data, int direction, int size,
-                         uint32_t count)
+static void kvm_handle_io(uint16_t port, void *data, int direction, int size,
+                          uint32_t count)
 {
     int i;
     uint8_t *ptr = data;
@@ -805,8 +805,6 @@ static int kvm_handle_io(uint16_t port, void *data, int direction, int size,
 
         ptr += size;
     }
-
-    return 1;
 }
 
 #ifdef KVM_CAP_INTERNAL_ERROR_DATA
@@ -940,11 +938,12 @@ int kvm_cpu_exec(CPUState *env)
         switch (run->exit_reason) {
         case KVM_EXIT_IO:
             DPRINTF("handle_io\n");
-            ret = kvm_handle_io(run->io.port,
-                                (uint8_t *)run + run->io.data_offset,
-                                run->io.direction,
-                                run->io.size,
-                                run->io.count);
+            kvm_handle_io(run->io.port,
+                          (uint8_t *)run + run->io.data_offset,
+                          run->io.direction,
+                          run->io.size,
+                          run->io.count);
+            ret = 1;
             break;
         case KVM_EXIT_MMIO:
             DPRINTF("handle_mmio\n");
-- 
1.7.1

[Qemu-devel] [PATCH v2 22/24] kvm: Leave kvm_cpu_exec directly after KVM_EXIT_SHUTDOWN

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

The reset we issue on KVM_EXIT_SHUTDOWN implies that we should also
leave the VCPU loop. As we now check for exit_request which is set by
qemu_system_reset_request, this bug is no longer critical. Still it's an
unneeded extra turn.

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

diff --git a/kvm-all.c b/kvm-all.c
index cf54256..35860df 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -959,7 +959,6 @@ int kvm_cpu_exec(CPUState *env)
         case KVM_EXIT_SHUTDOWN:
             DPRINTF("shutdown\n");
             qemu_system_reset_request();
-            ret = 1;
             break;
         case KVM_EXIT_UNKNOWN:
             fprintf(stderr, "KVM: unknown exit, hardware reason %" PRIx64 "\n",
-- 
1.7.1

[Qemu-devel] [PATCH v2 23/24] Refactor kvm&tcg function names in cpus.c

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Pure interface cosmetics: Ensure that only kvm core services (as
declared in kvm.h) start with "kvm_". Prepend "qemu_" to those that
violate this rule in cpus.c. Also rename the corresponding tcg functions
for the sake of consistency.

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

diff --git a/cpus.c b/cpus.c
index 9c50a34..0d11a20 100644
--- a/cpus.c
+++ b/cpus.c
@@ -778,7 +778,7 @@ static void qemu_kvm_wait_io_event(CPUState *env)
 
 static int qemu_cpu_exec(CPUState *env);
 
-static void *kvm_cpu_thread_fn(void *arg)
+static void *qemu_kvm_cpu_thread_fn(void *arg)
 {
     CPUState *env = arg;
     int r;
@@ -811,7 +811,7 @@ static void *kvm_cpu_thread_fn(void *arg)
     return NULL;
 }
 
-static void *tcg_cpu_thread_fn(void *arg)
+static void *qemu_tcg_cpu_thread_fn(void *arg)
 {
     CPUState *env = arg;
 
@@ -930,7 +930,7 @@ void resume_all_vcpus(void)
     }
 }
 
-static void tcg_init_vcpu(void *_env)
+static void qemu_tcg_init_vcpu(void *_env)
 {
     CPUState *env = _env;
     /* share a single thread for all cpus with TCG */
@@ -938,7 +938,7 @@ static void tcg_init_vcpu(void *_env)
         env->thread = qemu_mallocz(sizeof(QemuThread));
         env->halt_cond = qemu_mallocz(sizeof(QemuCond));
         qemu_cond_init(env->halt_cond);
-        qemu_thread_create(env->thread, tcg_cpu_thread_fn, env);
+        qemu_thread_create(env->thread, qemu_tcg_cpu_thread_fn, env);
         while (env->created == 0)
             qemu_cond_timedwait(&qemu_cpu_cond, &qemu_global_mutex, 100);
         tcg_cpu_thread = env->thread;
@@ -949,12 +949,12 @@ static void tcg_init_vcpu(void *_env)
     }
 }
 
-static void kvm_start_vcpu(CPUState *env)
+static void qemu_kvm_start_vcpu(CPUState *env)
 {
     env->thread = qemu_mallocz(sizeof(QemuThread));
     env->halt_cond = qemu_mallocz(sizeof(QemuCond));
     qemu_cond_init(env->halt_cond);
-    qemu_thread_create(env->thread, kvm_cpu_thread_fn, env);
+    qemu_thread_create(env->thread, qemu_kvm_cpu_thread_fn, env);
     while (env->created == 0)
         qemu_cond_timedwait(&qemu_cpu_cond, &qemu_global_mutex, 100);
 }
@@ -966,9 +966,9 @@ void qemu_init_vcpu(void *_env)
     env->nr_cores = smp_cores;
     env->nr_threads = smp_threads;
     if (kvm_enabled())
-        kvm_start_vcpu(env);
+        qemu_kvm_start_vcpu(env);
     else
-        tcg_init_vcpu(env);
+        qemu_tcg_init_vcpu(env);
 }
 
 void qemu_notify_event(void)
-- 
1.7.1

[Qemu-devel] [PATCH v2 24/24] Fix a few coding style violations in cpus.c

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

No functional changes.

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

diff --git a/cpus.c b/cpus.c
index 0d11a20..dd24fe8 100644
--- a/cpus.c
+++ b/cpus.c
@@ -138,25 +138,26 @@ static void do_vm_stop(int reason)
 
 static int cpu_can_run(CPUState *env)
 {
-    if (env->stop)
+    if (env->stop) {
         return 0;
-    if (env->stopped || !vm_running)
+    }
+    if (env->stopped || !vm_running) {
         return 0;
+    }
     return 1;
 }
 
 static int cpu_has_work(CPUState *env)
 {
-    if (env->stop)
+    if (env->stop || env->queued_work_first) {
         return 1;
-    if (env->queued_work_first)
-        return 1;
-    if (env->stopped || !vm_running)
+    }
+    if (env->stopped || !vm_running) {
         return 0;
-    if (!env->halted)
-        return 1;
-    if (qemu_cpu_has_work(env))
+    }
+    if (!env->halted || qemu_cpu_has_work(env)) {
         return 1;
+    }
     return 0;
 }
 
@@ -164,9 +165,11 @@ static int any_cpu_has_work(void)
 {
     CPUState *env;
 
-    for (env = first_cpu; env != NULL; env = env->next_cpu)
-        if (cpu_has_work(env))
+    for (env = first_cpu; env != NULL; env = env->next_cpu) {
+        if (cpu_has_work(env)) {
             return 1;
+        }
+    }
     return 0;
 }
 
@@ -232,9 +235,9 @@ static void qemu_event_increment(void)
     static const uint64_t val = 1;
     ssize_t ret;
 
-    if (io_thread_fd == -1)
+    if (io_thread_fd == -1) {
         return;
-
+    }
     do {
         ret = write(io_thread_fd, &val, sizeof(val));
     } while (ret < 0 && errno == EINTR);
@@ -265,17 +268,17 @@ static int qemu_event_init(void)
     int fds[2];
 
     err = qemu_eventfd(fds);
-    if (err == -1)
+    if (err == -1) {
         return -errno;
-
+    }
     err = fcntl_setfl(fds[0], O_NONBLOCK);
-    if (err < 0)
+    if (err < 0) {
         goto fail;
-
+    }
     err = fcntl_setfl(fds[1], O_NONBLOCK);
-    if (err < 0)
+    if (err < 0) {
         goto fail;
-
+    }
     qemu_set_fd_handler2(fds[0], NULL, qemu_event_read, NULL,
                          (void *)(unsigned long)fds[0]);
 
@@ -534,7 +537,6 @@ void pause_all_vcpus(void)
 
 void qemu_cpu_kick(void *env)
 {
-    return;
 }
 
 void qemu_cpu_kick_self(void)
@@ -663,13 +665,15 @@ int qemu_init_main_loop(void)
     blocked_signals = block_io_signals();
 
     ret = qemu_signalfd_init(blocked_signals);
-    if (ret)
+    if (ret) {
         return ret;
+    }
 
     /* Note eventfd must be drained before signalfd handlers run */
     ret = qemu_event_init();
-    if (ret)
+    if (ret) {
         return ret;
+    }
 
     qemu_cond_init(&qemu_pause_cond);
     qemu_cond_init(&qemu_system_cond);
@@ -699,10 +703,11 @@ void run_on_cpu(CPUState *env, void (*func)(void *data), void *data)
 
     wi.func = func;
     wi.data = data;
-    if (!env->queued_work_first)
+    if (!env->queued_work_first) {
         env->queued_work_first = &wi;
-    else
+    } else {
         env->queued_work_last->next = &wi;
+    }
     env->queued_work_last = &wi;
     wi.next = NULL;
     wi.done = false;
@@ -720,8 +725,9 @@ static void flush_queued_work(CPUState *env)
 {
     struct qemu_work_item *wi;
 
-    if (!env->queued_work_first)
+    if (!env->queued_work_first) {
         return;
+    }
 
     while ((wi = env->queued_work_first)) {
         env->queued_work_first = wi->next;
@@ -747,8 +753,9 @@ static void qemu_tcg_wait_io_event(void)
 {
     CPUState *env;
 
-    while (!any_cpu_has_work())
+    while (!any_cpu_has_work()) {
         qemu_cond_timedwait(tcg_halt_cond, &qemu_global_mutex, 1000);
+    }
 
     qemu_mutex_unlock(&qemu_global_mutex);
 
@@ -769,9 +776,9 @@ static void qemu_tcg_wait_io_event(void)
 
 static void qemu_kvm_wait_io_event(CPUState *env)
 {
-    while (!cpu_has_work(env))
+    while (!cpu_has_work(env)) {
         qemu_cond_timedwait(env->halt_cond, &qemu_global_mutex, 1000);
-
+    }
     qemu_kvm_eat_signals(env);
     qemu_wait_io_event_common(env);
 }
@@ -799,12 +806,14 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
     qemu_cond_signal(&qemu_cpu_cond);
 
     /* and wait for machine initialization */
-    while (!qemu_system_ready)
+    while (!qemu_system_ready) {
         qemu_cond_timedwait(&qemu_system_cond, &qemu_global_mutex, 100);
+    }
 
     while (1) {
-        if (cpu_can_run(env))
+        if (cpu_can_run(env)) {
             qemu_cpu_exec(env);
+        }
         qemu_kvm_wait_io_event(env);
     }
 
@@ -820,13 +829,15 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 
     /* signal CPU creation */
     qemu_mutex_lock(&qemu_global_mutex);
-    for (env = first_cpu; env != NULL; env = env->next_cpu)
+    for (env = first_cpu; env != NULL; env = env->next_cpu) {
         env->created = 1;
+    }
     qemu_cond_signal(&qemu_cpu_cond);
 
     /* and wait for machine initialization */
-    while (!qemu_system_ready)
+    while (!qemu_system_ready) {
         qemu_cond_timedwait(&qemu_system_cond, &qemu_global_mutex, 100);
+    }
 
     while (1) {
         cpu_exec_all();
@@ -839,6 +850,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 void qemu_cpu_kick(void *_env)
 {
     CPUState *env = _env;
+
     qemu_cond_broadcast(env->halt_cond);
     if (!env->thread_kicked) {
         qemu_thread_signal(env->thread, SIG_IPI);
@@ -890,8 +902,9 @@ static int all_vcpus_paused(void)
     CPUState *penv = first_cpu;
 
     while (penv) {
-        if (!penv->stopped)
+        if (!penv->stopped) {
             return 0;
+        }
         penv = (CPUState *)penv->next_cpu;
     }
 
@@ -933,14 +946,16 @@ void resume_all_vcpus(void)
 static void qemu_tcg_init_vcpu(void *_env)
 {
     CPUState *env = _env;
+
     /* share a single thread for all cpus with TCG */
     if (!tcg_cpu_thread) {
         env->thread = qemu_mallocz(sizeof(QemuThread));
         env->halt_cond = qemu_mallocz(sizeof(QemuCond));
         qemu_cond_init(env->halt_cond);
         qemu_thread_create(env->thread, qemu_tcg_cpu_thread_fn, env);
-        while (env->created == 0)
+        while (env->created == 0) {
             qemu_cond_timedwait(&qemu_cpu_cond, &qemu_global_mutex, 100);
+        }
         tcg_cpu_thread = env->thread;
         tcg_halt_cond = env->halt_cond;
     } else {
@@ -955,8 +970,9 @@ static void qemu_kvm_start_vcpu(CPUState *env)
     env->halt_cond = qemu_mallocz(sizeof(QemuCond));
     qemu_cond_init(env->halt_cond);
     qemu_thread_create(env->thread, qemu_kvm_cpu_thread_fn, env);
-    while (env->created == 0)
+    while (env->created == 0) {
         qemu_cond_timedwait(&qemu_cpu_cond, &qemu_global_mutex, 100);
+    }
 }
 
 void qemu_init_vcpu(void *_env)
@@ -965,10 +981,11 @@ void qemu_init_vcpu(void *_env)
 
     env->nr_cores = smp_cores;
     env->nr_threads = smp_threads;
-    if (kvm_enabled())
+    if (kvm_enabled()) {
         qemu_kvm_start_vcpu(env);
-    else
+    } else {
         qemu_tcg_init_vcpu(env);
+    }
 }
 
 void qemu_notify_event(void)
@@ -1043,16 +1060,18 @@ bool cpu_exec_all(void)
 {
     int r;
 
-    if (next_cpu == NULL)
+    if (next_cpu == NULL) {
         next_cpu = first_cpu;
+    }
     for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
         CPUState *env = next_cpu;
 
         qemu_clock_enable(vm_clock,
                           (env->singlestep_enabled & SSTEP_NOTIMER) == 0);
 
-        if (qemu_alarm_pending())
+        if (qemu_alarm_pending()) {
             break;
+        }
         if (cpu_can_run(env)) {
             r = qemu_cpu_exec(env);
             if (kvm_enabled()) {
-- 
1.7.1

[Qemu-devel] Re: [PATCH v2 00/24] [uq/master] Patch queue, part II

[Marcelo Tosatti]

On Tue, Feb 01, 2011 at 10:15:40PM +0100, Jan Kiszka wrote:
> Version 2 of part II. Changes:
>  - Fixed "Unconditionally reenter kernel after IO exits" to take
>    self-INIT into account
>  - Fixed misplaced hunk in "Fix race between timer signals and vcpu
>    entry under !IOTHREAD" (rebase artifact)
>  - Factor out block_synchronous_signals (analogue to block_io_signals)
>  - Additional fix to break out of SMP VCPU loop on pending IO event
>  - Fork qemu_kvm_init_cpu_signals over CONFIG_IOTHREAD
>  - Additional cleanup, flattening the main loop
> 
> Hope I addressed all review comments (except for passing env to
> qemu_cpu_kick_self which I think is better as it is).

1 and 24 rejected, applied the remaining ones, thanks.

[Qemu-devel] Re: [PATCH v2 00/24] [uq/master] Patch queue, part II

[Jan Kiszka]

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

On 2011-02-04 14:54, Marcelo Tosatti wrote:
> On Tue, Feb 01, 2011 at 10:15:40PM +0100, Jan Kiszka wrote:
>> Version 2 of part II. Changes:
>>  - Fixed "Unconditionally reenter kernel after IO exits" to take
>>    self-INIT into account
>>  - Fixed misplaced hunk in "Fix race between timer signals and vcpu
>>    entry under !IOTHREAD" (rebase artifact)
>>  - Factor out block_synchronous_signals (analogue to block_io_signals)
>>  - Additional fix to break out of SMP VCPU loop on pending IO event
>>  - Fork qemu_kvm_init_cpu_signals over CONFIG_IOTHREAD
>>  - Additional cleanup, flattening the main loop
>>
>> Hope I addressed all review comments (except for passing env to
>> qemu_cpu_kick_self which I think is better as it is).
> 
> 1 and 24 rejected, applied the remaining ones, thanks.

23 is missing as well. You want me to post the last two directly?

Jan


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

[Qemu-devel] Re: [PATCH v2 00/24] [uq/master] Patch queue, part II

[Marcelo Tosatti]

On Fri, Feb 04, 2011 at 05:29:00PM +0100, Jan Kiszka wrote:
> On 2011-02-04 14:54, Marcelo Tosatti wrote:
> > On Tue, Feb 01, 2011 at 10:15:40PM +0100, Jan Kiszka wrote:
> >> Version 2 of part II. Changes:
> >>  - Fixed "Unconditionally reenter kernel after IO exits" to take
> >>    self-INIT into account
> >>  - Fixed misplaced hunk in "Fix race between timer signals and vcpu
> >>    entry under !IOTHREAD" (rebase artifact)
> >>  - Factor out block_synchronous_signals (analogue to block_io_signals)
> >>  - Additional fix to break out of SMP VCPU loop on pending IO event
> >>  - Fork qemu_kvm_init_cpu_signals over CONFIG_IOTHREAD
> >>  - Additional cleanup, flattening the main loop
> >>
> >> Hope I addressed all review comments (except for passing env to
> >> qemu_cpu_kick_self which I think is better as it is).
> > 
> > 1 and 24 rejected, applied the remaining ones, thanks.
> 
> 23 is missing as well. You want me to post the last two directly?
> 
> Jan

Better include in the next round to make Anthony's life easier (they're
not critical anyway).

BTW, Anthony, patches are being sent twice to the list, once by author
and again by pull request. It is that way by your request.