[Qemu-devel] [PATCH uq/master 00/21] Win32 iothread support

[Qemu-devel] [PATCH uq/master 00/21] Win32 iothread support

[Paolo Bonzini]

After gathering the comments about the two series I sent separately,
here is the full series for Win32 iothread support, ready to be
applied to uq/master.

Patches 1 to 5 are generic Win32 improvements, including the qemu-thread
implementation.  Because of complex dependencies, I think it's better
if this part is also routed through uq/master.

Patches 6 to 8 are generic threading improvements, including using
PTHREAD_MUTEX_ERRORCHECK as suggested by Jan.

Patches 9 to 16 eliminate polling, replacing condition variable
timedwait with wait.

Patch 17 removes a redundant condition from the TCG cpu_exec_all
function.

Patches 18 to 20 add all necessary stubs to make iothread compile
with Win32, except the IPI calls.  These are provided by patch 21.

Tested on Wine and Linux, not on "real" Windows.  The series introduces
a dependency on Windows 2K or newer.  I don't think either 95/98/ME
or Windows NT 3.x are reasonable host systems for QEMU, anyway.

I incorporated all suggestions from Jan.

Paolo Bonzini (21):
  unlock iothread during WaitForMultipleObjects
  implement win32 dynticks timer
  use win32 timer queues
  replace qemu_thread_equal API with qemu_thread_is_current
  add win32 qemu-thread implementation
  include qemu-thread.h early
  add assertions on the owner of a QemuMutex
  remove CONFIG_THREAD
  inline cpu_halted into sole caller
  always qemu_cpu_kick after unhalting a cpu
  exit round-robin vcpu loop if cpu->stopped is true
  always signal pause_cond after stopping a VCPU
  do not use timedwait on qemu_halt_cond
  do not use timedwait on qemu_system_cond
  do not use timedwait on qemu_pause_cond
  do not use timedwait on qemu_cpu_cond
  iothread stops the vcpu thread via IPI
  move blocking of signals to qemu_signalfd_init
  provide dummy signal init functions for win32
  protect qemu_cpu_kick_self for Win32
  add Win32 IPI service

 Makefile.objs                        |    4 +-
 configure                            |    2 -
 cpu-defs.h                           |    1 +
 cpu-exec.c                           |   17 ++-
 cpus.c                               |  288 +++++++++++++++++----------------
 hw/apic.c                            |    1 +
 hw/ppc.c                             |    2 +
 hw/sun4m.c                           |   10 +-
 hw/sun4u.c                           |    4 +-
 os-win32.c                           |    2 +
 qemu-thread.c => qemu-thread-posix.c |   40 +++--
 qemu-thread-posix.h                  |   18 ++
 qemu-thread-win32.c                  |  261 ++++++++++++++++++++++++++++++
 qemu-thread-win32.h                  |   21 +++
 qemu-thread.h                        |   29 ++--
 qemu-timer.c                         |   88 +++++------
 target-alpha/exec.h                  |   11 --
 target-arm/exec.h                    |   13 --
 target-cris/exec.h                   |   11 --
 target-i386/exec.h                   |   12 --
 target-m68k/exec.h                   |   10 --
 target-microblaze/exec.h             |   11 --
 target-mips/exec.h                   |   11 --
 target-ppc/exec.h                    |   11 --
 target-s390x/exec.h                  |   12 --
 target-s390x/kvm.c                   |    1 +
 target-sh4/cpu.h                     |    1 -
 target-sh4/exec.h                    |   11 --
 target-sparc/exec.h                  |   10 --
 29 files changed, 559 insertions(+), 353 deletions(-)
 rename qemu-thread.c => qemu-thread-posix.c (78%)
 create mode 100644 qemu-thread-posix.h
 create mode 100644 qemu-thread-win32.c
 create mode 100644 qemu-thread-win32.h

-- 
1.7.3.5

[Qemu-devel] [PATCH 01/21] unlock iothread during WaitForMultipleObjects

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 os-win32.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/os-win32.c b/os-win32.c
index b214e6a..c971d92 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -140,7 +140,9 @@ void os_host_main_loop_wait(int *timeout)
         int err;
         WaitObjects *w = &wait_objects;
 
+        qemu_mutex_unlock_iothread();
         ret = WaitForMultipleObjects(w->num, w->events, FALSE, *timeout);
+        qemu_mutex_lock_iothread();
         if (WAIT_OBJECT_0 + 0 <= ret && ret <= WAIT_OBJECT_0 + w->num - 1) {
             if (w->func[ret - WAIT_OBJECT_0])
                 w->func[ret - WAIT_OBJECT_0](w->opaque[ret - WAIT_OBJECT_0]);
-- 
1.7.3.5

[Qemu-devel] [PATCH 02/21] implement win32 dynticks timer

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-timer.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index b0db780..e6b926b 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -1006,6 +1006,7 @@ static void win32_stop_timer(struct qemu_alarm_timer *t)
 static void win32_rearm_timer(struct qemu_alarm_timer *t)
 {
     struct qemu_alarm_win32 *data = t->priv;
+    int nearest_delta_ms;
 
     assert(alarm_has_dynticks(t));
     if (!active_timers[QEMU_CLOCK_REALTIME] &&
@@ -1015,7 +1016,11 @@ static void win32_rearm_timer(struct qemu_alarm_timer *t)
 
     timeKillEvent(data->timerId);
 
-    data->timerId = timeSetEvent(1,
+    nearest_delta_ms = (qemu_next_alarm_deadline() + 999999) / 1000000;
+    if (nearest_delta_ms < 1) {
+        nearest_delta_ms = 1;
+    }
+    data->timerId = timeSetEvent(nearest_delta_ms,
                         data->period,
                         host_alarm_handler,
                         (DWORD)t,
-- 
1.7.3.5

[Qemu-devel] [PATCH 03/21] use win32 timer queues

[Paolo Bonzini]

Multimedia timers are only useful for compatibility with Windows NT 4.0
and earlier.  Plus, the implementation in Wine is extremely heavyweight.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-timer.c |   86 +++++++++++++++++++++++----------------------------------
 1 files changed, 35 insertions(+), 51 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index e6b926b..185abb1 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -202,11 +202,6 @@ static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t)
 
 #ifdef _WIN32
 
-struct qemu_alarm_win32 {
-    MMRESULT timerId;
-    unsigned int period;
-} alarm_win32_data = {0, 0};
-
 static int win32_start_timer(struct qemu_alarm_timer *t);
 static void win32_stop_timer(struct qemu_alarm_timer *t);
 static void win32_rearm_timer(struct qemu_alarm_timer *t);
@@ -300,9 +295,9 @@ static struct qemu_alarm_timer alarm_timers[] = {
     {"unix", unix_start_timer, unix_stop_timer, NULL, NULL},
 #else
     {"dynticks", win32_start_timer,
-     win32_stop_timer, win32_rearm_timer, &alarm_win32_data},
+     win32_stop_timer, win32_rearm_timer, NULL},
     {"win32", win32_start_timer,
-     win32_stop_timer, NULL, &alarm_win32_data},
+     win32_stop_timer, NULL, NULL},
 #endif
     {NULL, }
 };
@@ -638,9 +633,7 @@ void qemu_run_all_timers(void)
 static int64_t qemu_next_alarm_deadline(void);
 
 #ifdef _WIN32
-static void CALLBACK host_alarm_handler(UINT uTimerID, UINT uMsg,
-                                        DWORD_PTR dwUser, DWORD_PTR dw1,
-                                        DWORD_PTR dw2)
+static void CALLBACK host_alarm_handler(PVOID lpParam, BOOLEAN unused)
 #else
 static void host_alarm_handler(int host_signum)
 #endif
@@ -963,50 +956,45 @@ static void unix_stop_timer(struct qemu_alarm_timer *t)
 
 static int win32_start_timer(struct qemu_alarm_timer *t)
 {
-    TIMECAPS tc;
-    struct qemu_alarm_win32 *data = t->priv;
-    UINT flags;
-
-    memset(&tc, 0, sizeof(tc));
-    timeGetDevCaps(&tc, sizeof(tc));
-
-    data->period = tc.wPeriodMin;
-    timeBeginPeriod(data->period);
-
-    flags = TIME_CALLBACK_FUNCTION;
-    if (alarm_has_dynticks(t))
-        flags |= TIME_ONESHOT;
-    else
-        flags |= TIME_PERIODIC;
-
-    data->timerId = timeSetEvent(1,         // interval (ms)
-                        data->period,       // resolution
-                        host_alarm_handler, // function
-                        (DWORD)t,           // parameter
-                        flags);
-
-    if (!data->timerId) {
+    HANDLE hTimer;
+    BOOLEAN success;
+
+    /* If you call ChangeTimerQueueTimer on a one-shot timer (its period
+       is zero) that has already expired, the timer is not updated.  Since
+       creating a new timer is relatively expensive, set a bogus one-hour
+       interval in the dynticks case.  */
+    success = CreateTimerQueueTimer(&hTimer,
+                          NULL,
+                          host_alarm_handler,
+                          t,
+                          1,
+                          alarm_has_dynticks(t) ? 3600000 : 1,
+                          WT_EXECUTEINTIMERTHREAD);
+
+    if (!success) {
         fprintf(stderr, "Failed to initialize win32 alarm timer: %ld\n",
                 GetLastError());
-        timeEndPeriod(data->period);
         return -1;
     }
 
+    t->priv = (PVOID) hTimer;
     return 0;
 }
 
 static void win32_stop_timer(struct qemu_alarm_timer *t)
 {
-    struct qemu_alarm_win32 *data = t->priv;
+    HANDLE hTimer = t->priv;
 
-    timeKillEvent(data->timerId);
-    timeEndPeriod(data->period);
+    if (hTimer) {
+        DeleteTimerQueueTimer(NULL, hTimer, NULL);
+    }
 }
 
 static void win32_rearm_timer(struct qemu_alarm_timer *t)
 {
-    struct qemu_alarm_win32 *data = t->priv;
+    HANDLE hTimer = t->priv;
     int nearest_delta_ms;
+    BOOLEAN success;
 
     assert(alarm_has_dynticks(t));
     if (!active_timers[QEMU_CLOCK_REALTIME] &&
@@ -1014,25 +1002,21 @@ static void win32_rearm_timer(struct qemu_alarm_timer *t)
         !active_timers[QEMU_CLOCK_HOST])
         return;
 
-    timeKillEvent(data->timerId);
-
     nearest_delta_ms = (qemu_next_alarm_deadline() + 999999) / 1000000;
     if (nearest_delta_ms < 1) {
         nearest_delta_ms = 1;
     }
-    data->timerId = timeSetEvent(nearest_delta_ms,
-                        data->period,
-                        host_alarm_handler,
-                        (DWORD)t,
-                        TIME_ONESHOT | TIME_CALLBACK_FUNCTION);
-
-    if (!data->timerId) {
-        fprintf(stderr, "Failed to re-arm win32 alarm timer %ld\n",
-                GetLastError());
+    success = ChangeTimerQueueTimer(NULL,
+                                    hTimer,
+                                    nearest_delta_ms,
+                                    3600000);
 
-        timeEndPeriod(data->period);
-        exit(1);
+    if (!success) {
+        fprintf(stderr, "Failed to rearm win32 alarm timer: %ld\n",
+                GetLastError());
+        exit(-1);
     }
+
 }
 
 #endif /* _WIN32 */
-- 
1.7.3.5

[Qemu-devel] [PATCH 04/21] replace qemu_thread_equal API with qemu_thread_is_current

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c        |   10 ++--------
 qemu-thread.c |    4 ++--
 qemu-thread.h |    2 +-
 3 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/cpus.c b/cpus.c
index 0f33945..aa30474 100644
--- a/cpus.c
+++ b/cpus.c
@@ -891,11 +891,8 @@ void qemu_cpu_kick_self(void)
 int qemu_cpu_self(void *_env)
 {
     CPUState *env = _env;
-    QemuThread this;
 
-    qemu_thread_self(&this);
-
-    return qemu_thread_equal(&this, env->thread);
+    return qemu_thread_is_current(env->thread);
 }
 
 void qemu_mutex_lock_iothread(void)
@@ -1023,10 +1020,7 @@ void cpu_stop_current(void)
 
 void vm_stop(int reason)
 {
-    QemuThread me;
-    qemu_thread_self(&me);
-
-    if (!qemu_thread_equal(&me, &io_thread)) {
+    if (!qemu_thread_is_current(&io_thread)) {
         qemu_system_vmstop_request(reason);
         /*
          * FIXME: should not return to device code in case
diff --git a/qemu-thread.c b/qemu-thread.c
index fbc78fe..28b3f80 100644
--- a/qemu-thread.c
+++ b/qemu-thread.c
@@ -181,9 +181,9 @@ void qemu_thread_self(QemuThread *thread)
     thread->thread = pthread_self();
 }
 
-int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2)
+int qemu_thread_is_current(QemuThread *thread)
 {
-   return pthread_equal(thread1->thread, thread2->thread);
+   return pthread_equal(pthread_self(), thread->thread);
 }
 
 void qemu_thread_exit(void *retval)
diff --git a/qemu-thread.h b/qemu-thread.h
index 19bb30c..a7e35b4 100644
--- a/qemu-thread.h
+++ b/qemu-thread.h
@@ -38,7 +38,7 @@ void qemu_thread_create(QemuThread *thread,
                        void *arg);
 void qemu_thread_signal(QemuThread *thread, int sig);
 void qemu_thread_self(QemuThread *thread);
-int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2);
+int qemu_thread_is_current(QemuThread *thread);
 void qemu_thread_exit(void *retval);
 
 #endif
-- 
1.7.3.5

[Qemu-devel] [PATCH 05/21] add win32 qemu-thread implementation

[Paolo Bonzini]

For now, qemu_cond_timedwait and qemu_mutex_timedlock are left as
POSIX-only functions.  They can be removed later, once the patches
that remove their uses are in.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.objs                        |    4 +-
 qemu-thread.c => qemu-thread-posix.c |    0
 qemu-thread-posix.h                  |   18 +++
 qemu-thread-win32.c                  |  261 ++++++++++++++++++++++++++++++++++
 qemu-thread-win32.h                  |   21 +++
 qemu-thread.h                        |   27 ++--
 6 files changed, 314 insertions(+), 17 deletions(-)
 rename qemu-thread.c => qemu-thread-posix.c (100%)
 create mode 100644 qemu-thread-posix.h
 create mode 100644 qemu-thread-win32.c
 create mode 100644 qemu-thread-win32.h

diff --git a/Makefile.objs b/Makefile.objs
index c144df1..d47a364 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -140,8 +140,8 @@ endif
 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_POSIX) += compatfd.o
+common-obj-$(CONFIG_POSIX) += qemu-thread-posix.o compatfd.o
+common-obj-$(CONFIG_WIN32) += qemu-thread-win32.o
 common-obj-y += notify.o event_notifier.o
 common-obj-y += qemu-timer.o qemu-timer-common.o
 
diff --git a/qemu-thread.c b/qemu-thread-posix.c
similarity index 100%
rename from qemu-thread.c
rename to qemu-thread-posix.c
diff --git a/qemu-thread-posix.h b/qemu-thread-posix.h
new file mode 100644
index 0000000..7af371c
--- /dev/null
+++ b/qemu-thread-posix.h
@@ -0,0 +1,18 @@
+#ifndef __QEMU_THREAD_POSIX_H
+#define __QEMU_THREAD_POSIX_H 1
+#include "pthread.h"
+
+struct QemuMutex {
+    pthread_mutex_t lock;
+};
+
+struct QemuCond {
+    pthread_cond_t cond;
+};
+
+struct QemuThread {
+    pthread_t thread;
+};
+
+void qemu_thread_signal(QemuThread *thread, int sig);
+#endif
diff --git a/qemu-thread-win32.c b/qemu-thread-win32.c
new file mode 100644
index 0000000..0755695
--- /dev/null
+++ b/qemu-thread-win32.c
@@ -0,0 +1,261 @@
+/*
+ * Win32 implementation for mutex/cond/thread functions
+ *
+ * Copyright Red Hat, Inc. 2010
+ *
+ * Author:
+ *  Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include "qemu-common.h"
+#include "qemu-thread.h"
+#include <process.h>
+#include <assert.h>
+#include <limits.h>
+
+static void error_exit(int err, const char *msg)
+{
+    char *pstr;
+
+    FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ALLOCATE_BUFFER,
+                  NULL, err, 0, (LPTSTR)&pstr, 2, NULL);
+    fprintf(stderr, "qemu: %s: %s\n", msg, pstr);
+    LocalFree(pstr);
+    exit(1);
+}
+
+void qemu_mutex_init(QemuMutex *mutex)
+{
+    mutex->owner = 0;
+    InitializeCriticalSection(&mutex->lock);
+}
+
+void qemu_mutex_lock(QemuMutex *mutex)
+{
+    EnterCriticalSection(&mutex->lock);
+
+    /* Win32 CRITICAL_SECTIONs are recursive.  Assert that we're not
+     * using them as such.
+     */
+    assert(mutex->owner == 0);
+    mutex->owner = GetCurrentThreadId();
+}
+
+int qemu_mutex_trylock(QemuMutex *mutex)
+{
+    int owned;
+
+    owned = TryEnterCriticalSection(&mutex->lock);
+    if (owned) {
+        assert(mutex->owner == 0);
+        mutex->owner = GetCurrentThreadId();
+    }
+    return !owned;
+}
+
+void qemu_mutex_unlock(QemuMutex *mutex)
+{
+    assert(mutex->owner == GetCurrentThreadId());
+    mutex->owner = 0;
+    LeaveCriticalSection(&mutex->lock);
+}
+
+void qemu_cond_init(QemuCond *cond)
+{
+    memset(cond, 0, sizeof(*cond));
+
+    cond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);
+    if (!cond->sema) {
+        error_exit(GetLastError(), __func__);
+    }
+    cond->continue_event = CreateEvent(NULL,    /* security */
+                                       FALSE,   /* auto-reset */
+                                       FALSE,   /* not signaled */
+                                       NULL);   /* name */
+    if (!cond->continue_event) {
+        error_exit(GetLastError(), __func__);
+    }
+}
+
+void qemu_cond_signal(QemuCond *cond)
+{
+    DWORD result;
+
+    /*
+     * Signal only when there are waiters.  cond->waiters is
+     * incremented by pthread_cond_wait under the external lock,
+     * so we are safe about that.
+     */
+    if (cond->waiters == 0) {
+        return;
+    }
+
+    /*
+     * Waiting threads decrement it outside the external lock, but
+     * only if another thread is executing pthread_cond_broadcast and
+     * has the mutex.  So, it also cannot be decremented concurrently
+     * with this particular access.
+     */
+    cond->target = cond->waiters - 1;
+    result = SignalObjectAndWait(cond->sema, cond->continue_event,
+                                 INFINITE, FALSE);
+    if (result == WAIT_ABANDONED || result == WAIT_FAILED) {
+        error_exit(GetLastError(), __func__);
+    }
+}
+
+void qemu_cond_broadcast(QemuCond *cond)
+{
+    BOOLEAN result;
+    /*
+     * As in pthread_cond_signal, access to cond->waiters and
+     * cond->target is locked via the external mutex.
+     */
+    if (cond->waiters == 0) {
+        return;
+    }
+
+    cond->target = 0;
+    result = ReleaseSemaphore(cond->sema, cond->waiters, NULL);
+    if (!result) {
+        error_exit(GetLastError(), __func__);
+    }
+
+    /*
+     * At this point all waiters continue. Each one takes its
+     * slice of the semaphore. Now it's our turn to wait: Since
+     * the external mutex is held, no thread can leave cond_wait,
+     * yet. For this reason, we can be sure that no thread gets
+     * a chance to eat *more* than one slice. OTOH, it means
+     * that the last waiter must send us a wake-up.
+     */
+    WaitForSingleObject(cond->continue_event, INFINITE);
+}
+
+void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex)
+{
+    /*
+     * This access is protected under the mutex.
+     */
+    cond->waiters++;
+
+    /*
+     * Unlock external mutex and wait for signal.
+     * NOTE: we've held mutex locked long enough to increment
+     * waiters count above, so there's no problem with
+     * leaving mutex unlocked before we wait on semaphore.
+     */
+    qemu_mutex_unlock(mutex);
+    WaitForSingleObject(cond->sema, INFINITE);
+
+    /* Now waiters must rendez-vous with the signaling thread and
+     * let it continue.  For cond_broadcast this has heavy contention
+     * and triggers thundering herd.  So goes life.
+     *
+     * Decrease waiters count.  The mutex is not taken, so we have
+     * to do this atomically.
+     *
+     * All waiters contend for the mutex at the end of this function
+     * until the signaling thread relinquishes it.  To ensure
+     * each waiter consumes exactly one slice of the semaphore,
+     * the signaling thread stops until it is told by the last
+     * waiter that it can go on.
+     */
+    if (InterlockedDecrement(&cond->waiters) == cond->target) {
+        SetEvent(cond->continue_event);
+    }
+
+    qemu_mutex_lock(mutex);
+}
+
+struct qemu_thread_data {
+    QemuThread *thread;
+    void *(*start_routine)(void *);
+    void *arg;
+};
+
+static int qemu_thread_tls_index = TLS_OUT_OF_INDEXES;
+
+static unsigned __stdcall win32_start_routine(void *arg)
+{
+    struct qemu_thread_data data = *(struct qemu_thread_data *) arg;
+    QemuThread *thread = data.thread;
+
+    free(arg);
+    TlsSetValue(qemu_thread_tls_index, thread);
+
+    /*
+     * Use DuplicateHandle instead of assigning thread->thread in the
+     * creating thread to avoid races.  It's simpler this way than with
+     * synchronization.
+     */
+    DuplicateHandle(GetCurrentProcess(), GetCurrentThread(),
+                    GetCurrentProcess(), &thread->thread,
+                    0, FALSE, DUPLICATE_SAME_ACCESS);
+
+    qemu_thread_exit(data.start_routine(data.arg));
+    abort();
+}
+
+void qemu_thread_exit(void *arg)
+{
+    QemuThread *thread = TlsGetValue(qemu_thread_tls_index);
+    thread->ret = arg;
+    CloseHandle(thread->thread);
+    thread->thread = NULL;
+    ExitThread(0);
+}
+
+static inline void qemu_thread_init(void)
+{
+    if (qemu_thread_tls_index == TLS_OUT_OF_INDEXES) {
+        qemu_thread_tls_index = TlsAlloc();
+        if (qemu_thread_tls_index == TLS_OUT_OF_INDEXES) {
+            error_exit(ERROR_NO_SYSTEM_RESOURCES, __func__);
+        }
+    }
+}
+
+
+void qemu_thread_create(QemuThread *thread,
+                       void *(*start_routine)(void *),
+                       void *arg)
+{
+    HANDLE hThread;
+
+    struct qemu_thread_data *data;
+    qemu_thread_init();
+    data = qemu_malloc(sizeof *data);
+    data->thread = thread;
+    data->start_routine = start_routine;
+    data->arg = arg;
+
+    hThread = (HANDLE) _beginthreadex(NULL, 0, win32_start_routine,
+                                      data, 0, NULL);
+    if (!hThread) {
+        error_exit(GetLastError(), __func__);
+    }
+    CloseHandle(hThread);
+}
+
+void qemu_thread_self(QemuThread *thread)
+{
+    if (!thread->thread) {
+        /* In the main thread of the process.  Initialize the QemuThread
+           pointer in TLS, and use the dummy GetCurrentThread handle as
+           the identifier for qemu_thread_is_current.  */
+        assert(qemu_thread_tls_index == TLS_OUT_OF_INDEXES);
+        qemu_thread_init();
+        TlsSetValue(qemu_thread_tls_index, thread);
+        thread->thread = GetCurrentThread();
+    }
+}
+
+int qemu_thread_is_current(QemuThread *thread)
+{
+    QemuThread *this_thread = TlsGetValue(qemu_thread_tls_index);
+    return this_thread->thread == thread->thread;
+}
diff --git a/qemu-thread-win32.h b/qemu-thread-win32.h
new file mode 100644
index 0000000..878f86a
--- /dev/null
+++ b/qemu-thread-win32.h
@@ -0,0 +1,21 @@
+#ifndef __QEMU_THREAD_WIN32_H
+#define __QEMU_THREAD_WIN32_H 1
+#include "windows.h"
+
+struct QemuMutex {
+    CRITICAL_SECTION lock;
+    LONG owner;
+};
+
+struct QemuCond {
+    LONG waiters, target;
+    HANDLE sema;
+    HANDLE continue_event;
+};
+
+struct QemuThread {
+    HANDLE thread;
+    void *ret;
+};
+
+#endif
diff --git a/qemu-thread.h b/qemu-thread.h
index a7e35b4..f6f6b06 100644
--- a/qemu-thread.h
+++ b/qemu-thread.h
@@ -1,24 +1,16 @@
 #ifndef __QEMU_THREAD_H
 #define __QEMU_THREAD_H 1
-#include "semaphore.h"
-#include "pthread.h"
-
-struct QemuMutex {
-    pthread_mutex_t lock;
-};
-
-struct QemuCond {
-    pthread_cond_t cond;
-};
-
-struct QemuThread {
-    pthread_t thread;
-};
 
 typedef struct QemuMutex QemuMutex;
 typedef struct QemuCond QemuCond;
 typedef struct QemuThread QemuThread;
 
+#ifdef _WIN32
+#include "qemu-thread-win32.h"
+#else
+#include "qemu-thread-posix.h"
+#endif
+
 void qemu_mutex_init(QemuMutex *mutex);
 void qemu_mutex_destroy(QemuMutex *mutex);
 void qemu_mutex_lock(QemuMutex *mutex);
@@ -28,6 +20,12 @@ void qemu_mutex_unlock(QemuMutex *mutex);
 
 void qemu_cond_init(QemuCond *cond);
 void qemu_cond_destroy(QemuCond *cond);
+
+/*
+ * IMPORTANT: The implementation does not guarantee that qemu_cond_signal
+ * and qemu_cond_broadcast can be called except while the same mutex is
+ * held as in the corresponding qemu_cond_wait calls!
+ */
 void qemu_cond_signal(QemuCond *cond);
 void qemu_cond_broadcast(QemuCond *cond);
 void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex);
@@ -36,7 +34,6 @@ int qemu_cond_timedwait(QemuCond *cond, QemuMutex *mutex, uint64_t msecs);
 void qemu_thread_create(QemuThread *thread,
                        void *(*start_routine)(void*),
                        void *arg);
-void qemu_thread_signal(QemuThread *thread, int sig);
 void qemu_thread_self(QemuThread *thread);
 int qemu_thread_is_current(QemuThread *thread);
 void qemu_thread_exit(void *retval);
-- 
1.7.3.5

[Qemu-devel] [PATCH 06/21] include qemu-thread.h early

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index aa30474..7a9440f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -32,6 +32,7 @@
 #include "kvm.h"
 #include "exec-all.h"
 
+#include "qemu-thread.h"
 #include "cpus.h"
 #include "compatfd.h"
 
@@ -592,8 +593,6 @@ void vm_stop(int reason)
 
 #else /* CONFIG_IOTHREAD */
 
-#include "qemu-thread.h"
-
 QemuMutex qemu_global_mutex;
 static QemuMutex qemu_fair_mutex;
 
-- 
1.7.3.5

[Qemu-devel] [PATCH 07/21] add assertions on the owner of a QemuMutex

[Paolo Bonzini]

These are already present in the Win32 implementation, add them to
the pthread wrappers as well.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-thread-posix.c |   27 +++++++++++++++++++++++++--
 qemu-thread-posix.h |    1 +
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c
index 28b3f80..2176f81 100644
--- a/qemu-thread-posix.c
+++ b/qemu-thread-posix.c
@@ -16,9 +16,12 @@
 #include <time.h>
 #include <signal.h>
 #include <stdint.h>
+#include <assert.h>
 #include <string.h>
 #include "qemu-thread.h"
 
+static pthread_t pthread_null;
+
 static void error_exit(int err, const char *msg)
 {
     fprintf(stderr, "qemu: %s: %s\n", msg, strerror(err));
@@ -28,8 +31,13 @@ static void error_exit(int err, const char *msg)
 void qemu_mutex_init(QemuMutex *mutex)
 {
     int err;
+    pthread_mutexattr_t mutexattr;
 
-    err = pthread_mutex_init(&mutex->lock, NULL);
+    mutex->owner = pthread_null;
+    pthread_mutexattr_init(&mutexattr);
+    pthread_mutexattr_settype(&mutexattr, PTHREAD_MUTEX_ERRORCHECK);
+    err = pthread_mutex_init(&mutex->lock, &mutexattr);
+    pthread_mutexattr_destroy(&mutexattr);
     if (err)
         error_exit(err, __func__);
 }
@@ -48,13 +56,20 @@ void qemu_mutex_lock(QemuMutex *mutex)
     int err;
 
     err = pthread_mutex_lock(&mutex->lock);
+    mutex->owner = pthread_self();
     if (err)
         error_exit(err, __func__);
 }
 
 int qemu_mutex_trylock(QemuMutex *mutex)
 {
-    return pthread_mutex_trylock(&mutex->lock);
+    int err;
+    err = pthread_mutex_trylock(&mutex->lock);
+    if (err == 0) {
+        mutex->owner = pthread_self();
+    }
+
+    return !!err;
 }
 
 static void timespec_add_ms(struct timespec *ts, uint64_t msecs)
@@ -85,6 +100,11 @@ void qemu_mutex_unlock(QemuMutex *mutex)
 {
     int err;
 
+    /* An EDEADLOCK would arrive after we reset the owner.  So this
+       assert is for ease of debugging (it lets you see what is the
+       actual owner.  */
+    assert(pthread_equal(mutex->owner, pthread_self()));
+    mutex->owner = pthread_null;
     err = pthread_mutex_unlock(&mutex->lock);
     if (err)
         error_exit(err, __func__);
@@ -130,7 +150,10 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex)
 {
     int err;
 
+    assert(pthread_equal(mutex->owner, pthread_self()));
+    mutex->owner = pthread_null;
     err = pthread_cond_wait(&cond->cond, &mutex->lock);
+    mutex->owner = pthread_self();
     if (err)
         error_exit(err, __func__);
 }
diff --git a/qemu-thread-posix.h b/qemu-thread-posix.h
index 7af371c..11978db 100644
--- a/qemu-thread-posix.h
+++ b/qemu-thread-posix.h
@@ -4,6 +4,7 @@
 
 struct QemuMutex {
     pthread_mutex_t lock;
+    pthread_t owner;
 };
 
 struct QemuCond {
-- 
1.7.3.5

[Qemu-devel] [PATCH 08/21] remove CONFIG_THREAD

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 791b71d..45a5440 100755
--- a/configure
+++ b/configure
@@ -2660,7 +2660,6 @@ if test "$vnc_png" != "no" ; then
 fi
 if test "$vnc_thread" != "no" ; then
   echo "CONFIG_VNC_THREAD=y" >> $config_host_mak
-  echo "CONFIG_THREAD=y" >> $config_host_mak
 fi
 if test "$fnmatch" = "yes" ; then
   echo "CONFIG_FNMATCH=y" >> $config_host_mak
@@ -2756,7 +2755,6 @@ if test "$xen" = "yes" ; then
 fi
 if test "$io_thread" = "yes" ; then
   echo "CONFIG_IOTHREAD=y" >> $config_host_mak
-  echo "CONFIG_THREAD=y" >> $config_host_mak
 fi
 if test "$linux_aio" = "yes" ; then
   echo "CONFIG_LINUX_AIO=y" >> $config_host_mak
-- 
1.7.3.5

[Qemu-devel] [PATCH 09/21] inline cpu_halted into sole caller

[Paolo Bonzini]

All implementations are now the same except SH, which can fit in
the default implementation easily.  The newly added flag will not make
much sense on non-SH platforms, but I left it anyway.  You could
be moved to common code.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
 cpu-defs.h               |    1 +
 cpu-exec.c               |   17 +++++++++++++++--
 target-alpha/exec.h      |   11 -----------
 target-arm/exec.h        |   13 -------------
 target-cris/exec.h       |   11 -----------
 target-i386/exec.h       |   12 ------------
 target-m68k/exec.h       |   10 ----------
 target-microblaze/exec.h |   11 -----------
 target-mips/exec.h       |   11 -----------
 target-ppc/exec.h        |   11 -----------
 target-s390x/exec.h      |   12 ------------
 target-sh4/cpu.h         |    1 -
 target-sh4/exec.h        |   11 -----------
 target-sparc/exec.h      |   10 ----------
 14 files changed, 16 insertions(+), 126 deletions(-)

diff --git a/cpu-defs.h b/cpu-defs.h
index 2b59fa6..a444665 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -165,6 +165,7 @@ typedef struct CPUWatchpoint {
     target_ulong mem_io_vaddr; /* target virtual addr at which the      \
                                      memory was accessed */             \
     uint32_t halted; /* Nonzero if the CPU is in suspend state */       \
+    uint32_t intr_at_halt; /* Nonzero if an irq woke CPU from halted state */ \
     uint32_t interrupt_request;                                         \
     volatile sig_atomic_t exit_request;                                 \
     CPU_COMMON_TLB                                                      \
diff --git a/cpu-exec.c b/cpu-exec.c
index b03b3a7..f161f57 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -208,8 +208,21 @@ int cpu_exec(CPUState *env1)
     uint8_t *tc_ptr;
     unsigned long next_tb;
 
-    if (cpu_halted(env1) == EXCP_HALTED)
-        return EXCP_HALTED;
+    if (env1->halted) {
+        if (!cpu_has_work(env1)) {
+            return EXCP_HALTED;
+        }
+
+        /* Tell the target that an IRQ woke the CPU from halted state.
+           If they care, they are supposed to bring back the flag to zero
+           as soon as they process the interrupt.
+
+           FIXME: it seems to me that *we* could clear the flag after
+           do_interrupt.  However, the logic in the SH target is a bit
+           more complex and I'm wary of touching it.  */
+        env1->intr_at_halt = 1;
+        env1->halted = 0;
+    }
 
     cpu_single_env = env1;
 
diff --git a/target-alpha/exec.h b/target-alpha/exec.h
index a8a38d2..6ae96d1 100644
--- a/target-alpha/exec.h
+++ b/target-alpha/exec.h
@@ -42,17 +42,6 @@ static inline int cpu_has_work(CPUState *env)
     return (env->interrupt_request & CPU_INTERRUPT_HARD);
 }
 
-static inline int cpu_halted(CPUState *env)
-{
-    if (!env->halted)
-        return 0;
-    if (cpu_has_work(env)) {
-        env->halted = 0;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
 {
     env->pc = tb->pc;
diff --git a/target-arm/exec.h b/target-arm/exec.h
index e4c35a3..44e1b55 100644
--- a/target-arm/exec.h
+++ b/target-arm/exec.h
@@ -32,19 +32,6 @@ static inline int cpu_has_work(CPUState *env)
             (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_EXITTB));
 }
 
-static inline int cpu_halted(CPUState *env) {
-    if (!env->halted)
-        return 0;
-    /* An interrupt wakes the CPU even if the I and F CPSR bits are
-       set.  We use EXITTB to silently wake CPU without causing an
-       actual interrupt.  */
-    if (cpu_has_work(env)) {
-        env->halted = 0;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 #if !defined(CONFIG_USER_ONLY)
 #include "softmmu_exec.h"
 #endif
diff --git a/target-cris/exec.h b/target-cris/exec.h
index 34c0132..2d5d297 100644
--- a/target-cris/exec.h
+++ b/target-cris/exec.h
@@ -33,17 +33,6 @@ static inline int cpu_has_work(CPUState *env)
     return (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI));
 }
 
-static inline int cpu_halted(CPUState *env) {
-	if (!env->halted)
-		return 0;
-
-	if (cpu_has_work(env)) {
-		env->halted = 0;
-		return 0;
-	}
-	return EXCP_HALTED;
-}
-
 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
 {
     env->pc = tb->pc;
diff --git a/target-i386/exec.h b/target-i386/exec.h
index fc8945b..3e7386e 100644
--- a/target-i386/exec.h
+++ b/target-i386/exec.h
@@ -304,18 +304,6 @@ static inline int cpu_has_work(CPUState *env)
     return work;
 }
 
-static inline int cpu_halted(CPUState *env) {
-    /* handle exit of HALTED state */
-    if (!env->halted)
-        return 0;
-    /* disable halt condition */
-    if (cpu_has_work(env)) {
-        env->halted = 0;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 /* load efer and update the corresponding hflags. XXX: do consistency
    checks with cpuid bits ? */
 static inline void cpu_load_efer(CPUState *env, uint64_t val)
diff --git a/target-m68k/exec.h b/target-m68k/exec.h
index f31e06e..91daa6b 100644
--- a/target-m68k/exec.h
+++ b/target-m68k/exec.h
@@ -33,16 +33,6 @@ static inline int cpu_has_work(CPUState *env)
     return (env->interrupt_request & (CPU_INTERRUPT_HARD));
 }
 
-static inline int cpu_halted(CPUState *env) {
-    if (!env->halted)
-        return 0;
-    if (cpu_has_work(env)) {
-        env->halted = 0;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
 {
     env->pc = tb->pc;
diff --git a/target-microblaze/exec.h b/target-microblaze/exec.h
index ab19828..1efff30 100644
--- a/target-microblaze/exec.h
+++ b/target-microblaze/exec.h
@@ -32,17 +32,6 @@ static inline int cpu_has_work(CPUState *env)
     return (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI));
 }
 
-static inline int cpu_halted(CPUState *env) {
-	if (!env->halted)
-		return 0;
-
-	if (cpu_has_work(env)) {
-		env->halted = 0;
-		return 0;
-	}
-	return EXCP_HALTED;
-}
-
 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
 {
     env->sregs[SR_PC] = tb->pc;
diff --git a/target-mips/exec.h b/target-mips/exec.h
index 1273654..b3c5a13 100644
--- a/target-mips/exec.h
+++ b/target-mips/exec.h
@@ -36,17 +36,6 @@ static inline int cpu_has_work(CPUState *env)
     return has_work;
 }
 
-static inline int cpu_halted(CPUState *env)
-{
-    if (!env->halted)
-        return 0;
-    if (cpu_has_work(env)) {
-        env->halted = 0;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 static inline void compute_hflags(CPUState *env)
 {
     env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
diff --git a/target-ppc/exec.h b/target-ppc/exec.h
index 4688ef5..f87847a 100644
--- a/target-ppc/exec.h
+++ b/target-ppc/exec.h
@@ -38,17 +38,6 @@ static inline int cpu_has_work(CPUState *env)
 }
 
 
-static inline int cpu_halted(CPUState *env)
-{
-    if (!env->halted)
-        return 0;
-    if (cpu_has_work(env)) {
-        env->halted = 0;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
 {
     env->nip = tb->pc;
diff --git a/target-s390x/exec.h b/target-s390x/exec.h
index bf3f264..f7893f3 100644
--- a/target-s390x/exec.h
+++ b/target-s390x/exec.h
@@ -34,18 +34,6 @@ static inline int cpu_has_work(CPUState *env)
     return env->interrupt_request & CPU_INTERRUPT_HARD; // guess
 }
 
-static inline int cpu_halted(CPUState *env)
-{
-    if (!env->halted) {
-       return 0;
-    }
-    if (cpu_has_work(env)) {
-        env->halted = 0;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock* tb)
 {
     env->psw.addr = tb->pc;
diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h
index 789d188..7188c4d 100644
--- a/target-sh4/cpu.h
+++ b/target-sh4/cpu.h
@@ -184,7 +184,6 @@ typedef struct CPUSH4State {
     uint32_t cvr;		/* Cache Version Register */
 
     void *intc_handle;
-    int intr_at_halt;		/* SR_BL ignored during sleep */
     memory_content *movcal_backup;
     memory_content **movcal_backup_tail;
 } CPUSH4State;
diff --git a/target-sh4/exec.h b/target-sh4/exec.h
index 2999c02..9f1c1f6 100644
--- a/target-sh4/exec.h
+++ b/target-sh4/exec.h
@@ -32,17 +32,6 @@ static inline int cpu_has_work(CPUState *env)
     return (env->interrupt_request & CPU_INTERRUPT_HARD);
 }
 
-static inline int cpu_halted(CPUState *env) {
-    if (!env->halted)
-        return 0;
-    if (cpu_has_work(env)) {
-        env->halted = 0;
-        env->intr_at_halt = 1;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 #ifndef CONFIG_USER_ONLY
 #include "softmmu_exec.h"
 #endif
diff --git a/target-sparc/exec.h b/target-sparc/exec.h
index f811571..f5c221e 100644
--- a/target-sparc/exec.h
+++ b/target-sparc/exec.h
@@ -22,16 +22,6 @@ static inline int cpu_has_work(CPUState *env1)
 }
 
 
-static inline int cpu_halted(CPUState *env1) {
-    if (!env1->halted)
-        return 0;
-    if (cpu_has_work(env1)) {
-        env1->halted = 0;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
 {
     env->pc = tb->pc;
-- 
1.7.3.5

[Qemu-devel] [PATCH 10/21] always qemu_cpu_kick after unhalting a cpu

[Paolo Bonzini]

This ensures env->halt_cond is broadcast, and the loop in
qemu_tcg_wait_io_event and qemu_kvm_wait_io_event is exited
naturally rather than through a timeout.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/apic.c          |    1 +
 hw/ppc.c           |    2 ++
 hw/sun4m.c         |   10 ++++++++--
 hw/sun4u.c         |    4 ++--
 target-s390x/kvm.c |    1 +
 5 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/hw/apic.c b/hw/apic.c
index 218d1bb..09fc61b 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -544,6 +544,7 @@ void apic_sipi(DeviceState *d)
         return;
     cpu_x86_load_seg_cache_sipi(s->cpu_env, s->sipi_vector);
     s->wait_for_sipi = 0;
+    qemu_cpu_kick(s->cpu_env);
 }
 
 static void apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode,
diff --git a/hw/ppc.c b/hw/ppc.c
index 968aec1..de02d33 100644
--- a/hw/ppc.c
+++ b/hw/ppc.c
@@ -208,6 +208,7 @@ static void ppc970_set_irq (void *opaque, int pin, int level)
             } else {
                 LOG_IRQ("%s: restart the CPU\n", __func__);
                 env->halted = 0;
+                qemu_cpu_kick(env);
             }
             break;
         case PPC970_INPUT_HRESET:
@@ -300,6 +301,7 @@ static void ppc40x_set_irq (void *opaque, int pin, int level)
             } else {
                 LOG_IRQ("%s: restart the CPU\n", __func__);
                 env->halted = 0;
+                qemu_cpu_kick(env);
             }
             break;
         case PPC40x_INPUT_DEBUG:
diff --git a/hw/sun4m.c b/hw/sun4m.c
index 30e8a21..df3aa32 100644
--- a/hw/sun4m.c
+++ b/hw/sun4m.c
@@ -253,15 +253,21 @@ void cpu_check_irqs(CPUState *env)
     }
 }
 
+static void cpu_kick_irq(CPUState *env)
+{
+    env->halted = 0;
+    cpu_check_irqs(env);
+    qemu_cpu_kick(env);
+}
+
 static void cpu_set_irq(void *opaque, int irq, int level)
 {
     CPUState *env = opaque;
 
     if (level) {
         trace_sun4m_cpu_set_irq_raise(irq);
-        env->halted = 0;
         env->pil_in |= 1 << irq;
-        cpu_check_irqs(env);
+        cpu_kick_irq(env);
     } else {
         trace_sun4m_cpu_set_irq_lower(irq);
         env->pil_in &= ~(1 << irq);
diff --git a/hw/sun4u.c b/hw/sun4u.c
index 90b1ce2..d282324 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -298,6 +298,7 @@ static void cpu_kick_irq(CPUState *env)
 {
     env->halted = 0;
     cpu_check_irqs(env);
+    qemu_cpu_kick(env);
 }
 
 static void cpu_set_irq(void *opaque, int irq, int level)
@@ -306,9 +307,8 @@ static void cpu_set_irq(void *opaque, int irq, int level)
 
     if (level) {
         CPUIRQ_DPRINTF("Raise CPU IRQ %d\n", irq);
-        env->halted = 0;
         env->pil_in |= 1 << irq;
-        cpu_check_irqs(env);
+        cpu_kick_irq(env);
     } else {
         CPUIRQ_DPRINTF("Lower CPU IRQ %d\n", irq);
         env->pil_in &= ~(1 << irq);
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index b349812..6e94274 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -194,6 +194,7 @@ static void kvm_s390_interrupt_internal(CPUState *env, int type, uint32_t parm,
 
     env->halted = 0;
     env->exception_index = -1;
+    qemu_cpu_kick(env);
 
     kvmint.type = type;
     kvmint.parm = parm;
-- 
1.7.3.5

[Qemu-devel] [PATCH 11/21] exit round-robin vcpu loop if cpu->stopped is true

[Paolo Bonzini]

Sometimes vcpus are stopped directly without going through ->stop = 1.
Exit the VCPU execution loop in this case as well.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/cpus.c b/cpus.c
index 7a9440f..912e0d8 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1098,7 +1098,7 @@ bool cpu_exec_all(void)
                 cpu_handle_debug_exception(env);
                 break;
             }
-        } else if (env->stop) {
+        } else if (env->stop || env->stopped) {
             break;
         }
     }
-- 
1.7.3.5

[Qemu-devel] [PATCH 12/21] always signal pause_cond after stopping a VCPU

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/cpus.c b/cpus.c
index 912e0d8..670a42d 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1012,8 +1012,10 @@ void qemu_notify_event(void)
 void cpu_stop_current(void)
 {
     if (cpu_single_env) {
+        cpu_single_env->stop = 0;
         cpu_single_env->stopped = 1;
         cpu_exit(cpu_single_env);
+        qemu_cond_signal(&qemu_pause_cond);
     }
 }
 
-- 
1.7.3.5

[Qemu-devel] [PATCH 13/21] do not use timedwait on qemu_halt_cond

[Paolo Bonzini]

The following conditions can cause cpu_has_work(env) to become true:

* env->queued_work_first: run_on_cpu is already kicking the VCPU

* env->stop = 1: pause_all_vcpus is already kicking the VCPU

* env->stopped = 0: resume_all_vcpus is already kicking the VCPU

* vm_running = 1: vm_start is calling resume_all_vcpus

* env->halted = 0: see previous patch

* qemu_cpu_has_work(env): when it becomes true, board code should set
env->halted = 0 too.  cpu_interrupt kicks the VCPU, too.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index 670a42d..868857d 100644
--- a/cpus.c
+++ b/cpus.c
@@ -771,7 +771,7 @@ static void qemu_tcg_wait_io_event(void)
     CPUState *env;
 
     while (all_cpu_threads_idle()) {
-        qemu_cond_timedwait(tcg_halt_cond, &qemu_global_mutex, 1000);
+        qemu_cond_wait(tcg_halt_cond, &qemu_global_mutex);
     }
 
     qemu_mutex_unlock(&qemu_global_mutex);
@@ -794,7 +794,7 @@ static void qemu_tcg_wait_io_event(void)
 static void qemu_kvm_wait_io_event(CPUState *env)
 {
     while (cpu_thread_is_idle(env)) {
-        qemu_cond_timedwait(env->halt_cond, &qemu_global_mutex, 1000);
+        qemu_cond_wait(env->halt_cond, &qemu_global_mutex);
     }
 
     qemu_kvm_eat_signals(env);
-- 
1.7.3.5

[Qemu-devel] [PATCH 14/21] do not use timedwait on qemu_system_cond

[Paolo Bonzini]

qemu_main_loop_start is the only place where qemu_system_ready is set
to 1.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index 868857d..8c147bc 100644
--- a/cpus.c
+++ b/cpus.c
@@ -823,7 +823,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
 
     /* and wait for machine initialization */
     while (!qemu_system_ready) {
-        qemu_cond_timedwait(&qemu_system_cond, &qemu_global_mutex, 100);
+        qemu_cond_wait(&qemu_system_cond, &qemu_global_mutex);
     }
 
     while (1) {
@@ -855,7 +855,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 
     /* and wait for machine initialization */
     while (!qemu_system_ready) {
-        qemu_cond_timedwait(&qemu_system_cond, &qemu_global_mutex, 100);
+        qemu_cond_wait(&qemu_system_cond, &qemu_global_mutex);
     }
 
     while (1) {
-- 
1.7.3.5

[Qemu-devel] [PATCH 15/21] do not use timedwait on qemu_pause_cond

[Paolo Bonzini]

all_vcpus_paused can start returning true after penv->stopped changes
from 0 to 1.  When this is done, qemu_pause_cond is always signaled.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/cpus.c b/cpus.c
index 8c147bc..66fd384 100644
--- a/cpus.c
+++ b/cpus.c
@@ -938,7 +938,7 @@ void pause_all_vcpus(void)
     }
 
     while (!all_vcpus_paused()) {
-        qemu_cond_timedwait(&qemu_pause_cond, &qemu_global_mutex, 100);
+        qemu_cond_wait(&qemu_pause_cond, &qemu_global_mutex);
         penv = first_cpu;
         while (penv) {
             qemu_cpu_kick(penv);
-- 
1.7.3.5

[Qemu-devel] [PATCH 16/21] do not use timedwait on qemu_cpu_cond

[Paolo Bonzini]

Whenever env->created becomes true, qemu_cpu_cond is signaled by
{kvm,tcg}_cpu_thread_fn.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index 66fd384..ee9999b 100644
--- a/cpus.c
+++ b/cpus.c
@@ -970,7 +970,7 @@ static void qemu_tcg_init_vcpu(void *_env)
         qemu_cond_init(env->halt_cond);
         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);
+            qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
         }
         tcg_cpu_thread = env->thread;
         tcg_halt_cond = env->halt_cond;
@@ -987,7 +987,7 @@ static void qemu_kvm_start_vcpu(CPUState *env)
     qemu_cond_init(env->halt_cond);
     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);
+        qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
     }
 }
 
-- 
1.7.3.5

[Qemu-devel] [PATCH 17/21] iothread stops the vcpu thread via IPI

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/cpus.c b/cpus.c
index ee9999b..1b6893d 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1086,9 +1086,11 @@ bool cpu_exec_all(void)
         qemu_clock_enable(vm_clock,
                           (env->singlestep_enabled & SSTEP_NOTIMER) == 0);
 
+#ifndef CONFIG_IOTHREAD
         if (qemu_alarm_pending()) {
             break;
         }
+#endif
         if (cpu_can_run(env)) {
             if (kvm_enabled()) {
                 r = kvm_cpu_exec(env);
-- 
1.7.3.5

[Qemu-devel] [PATCH 18/21] move blocking of signals to qemu_signalfd_init

[Paolo Bonzini]

This makes it easier to add a Win32 stub.  The patch does that, too.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |   87 ++++++++++++++++++++++++++-------------------------------------
 1 files changed, 36 insertions(+), 51 deletions(-)

diff --git a/cpus.c b/cpus.c
index 1b6893d..5bb95d8 100644
--- a/cpus.c
+++ b/cpus.c
@@ -346,11 +346,37 @@ static void sigfd_handler(void *opaque)
     }
 }
 
-static int qemu_signalfd_init(sigset_t mask)
+static int qemu_signalfd_init(void)
 {
     int sigfd;
+    sigset_t set;
 
-    sigfd = qemu_signalfd(&mask);
+#ifdef CONFIG_IOTHREAD
+    /* 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);
+#else
+    sigemptyset(&set);
+    sigaddset(&set, SIGBUS);
+    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);
+    }
+#endif
+
+    sigfd = qemu_signalfd(&set);
     if (sigfd == -1) {
         fprintf(stderr, "failed to create signalfd\n");
         return -errno;
@@ -438,6 +464,12 @@ static void qemu_event_increment(void)
 static void qemu_kvm_eat_signals(CPUState *env)
 {
 }
+
+static int qemu_signalfd_init(void)
+{
+    return 0;
+}
+
 #endif /* _WIN32 */
 
 #ifndef CONFIG_IOTHREAD
@@ -471,39 +503,14 @@ static void qemu_kvm_init_cpu_signals(CPUState *env)
 #endif
 }
 
-#ifndef _WIN32
-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
-         * 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;
 
-    blocked_signals = block_synchronous_signals();
-
-    ret = qemu_signalfd_init(blocked_signals);
+    ret = qemu_signalfd_init();
     if (ret) {
         return ret;
     }
-#endif
 
     qemu_init_sigbus();
 
@@ -651,35 +658,13 @@ static void qemu_tcg_init_cpu_signals(void)
     pthread_sigmask(SIG_UNBLOCK, &set, NULL);
 }
 
-static sigset_t block_io_signals(void)
-{
-    sigset_t set;
-
-    /* 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);
-
-    return set;
-}
-
 int qemu_init_main_loop(void)
 {
     int ret;
-    sigset_t blocked_signals;
 
     qemu_init_sigbus();
 
-    blocked_signals = block_io_signals();
-
-    ret = qemu_signalfd_init(blocked_signals);
+    ret = qemu_signalfd_init();
     if (ret) {
         return ret;
     }
-- 
1.7.3.5

[Qemu-devel] [PATCH 19/21] provide dummy signal init functions for win32

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |  143 ++++++++++++++++++++++++++++++++-------------------------------
 1 files changed, 73 insertions(+), 70 deletions(-)

diff --git a/cpus.c b/cpus.c
index 5bb95d8..c2c139c 100644
--- a/cpus.c
+++ b/cpus.c
@@ -196,6 +196,16 @@ static void cpu_handle_debug_exception(CPUState *env)
 #endif
 }
 
+#ifdef CONFIG_IOTHREAD
+static void cpu_signal(int sig)
+{
+    if (cpu_single_env) {
+        cpu_exit(cpu_single_env);
+    }
+    exit_request = 1;
+}
+#endif
+
 #ifdef CONFIG_LINUX
 static void sigbus_reraise(void)
 {
@@ -390,6 +400,61 @@ static int qemu_signalfd_init(void)
     return 0;
 }
 
+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);
+
+#ifdef CONFIG_IOTHREAD
+    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);
+    }
+#else
+    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, SIGIO);
+    sigdelset(&set, SIGALRM);
+#endif
+    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)
+{
+#ifdef CONFIG_IOTHREAD
+    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);
+#endif
+}
+
 static void qemu_kvm_eat_signals(CPUState *env)
 {
     struct timespec ts = { 0, 0 };
@@ -470,39 +535,17 @@ static int qemu_signalfd_init(void)
     return 0;
 }
 
-#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);
-    sigaddset(&set, SIGIO);
-    sigaddset(&set, SIGALRM);
-    pthread_sigmask(SIG_BLOCK, &set, NULL);
+    abort();
+}
 
-    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));
-        exit(1);
-    }
-#endif
+static void qemu_tcg_init_cpu_signals(void)
+{
 }
+#endif /* _WIN32 */
 
+#ifndef CONFIG_IOTHREAD
 int qemu_init_main_loop(void)
 {
     int ret;
@@ -536,6 +579,8 @@ void qemu_init_vcpu(void *_env)
             exit(1);
         }
         qemu_kvm_init_cpu_signals(env);
+    } else {
+        qemu_tcg_init_cpu_signals();
     }
 }
 
@@ -616,48 +661,6 @@ static QemuCond qemu_system_cond;
 static QemuCond qemu_pause_cond;
 static QemuCond qemu_work_cond;
 
-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);
-}
-
 int qemu_init_main_loop(void)
 {
     int ret;
-- 
1.7.3.5

[Qemu-devel] [PATCH 20/21] protect qemu_cpu_kick_self for Win32

[Paolo Bonzini]

The non-iothread version is already protected.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/cpus.c b/cpus.c
index c2c139c..fd791a3 100644
--- a/cpus.c
+++ b/cpus.c
@@ -867,12 +867,16 @@ void qemu_cpu_kick(void *_env)
 
 void qemu_cpu_kick_self(void)
 {
+#ifndef _WIN32
     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;
     }
+#else
+    abort();
+#endif
 }
 
 int qemu_cpu_self(void *_env)
-- 
1.7.3.5

[Qemu-devel] [PATCH 21/21] add Win32 IPI service

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c              |   23 ++++++++++++++++++++---
 qemu-thread-posix.c |    9 ---------
 qemu-thread-posix.h |    1 -
 3 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/cpus.c b/cpus.c
index fd791a3..869ed1a 100644
--- a/cpus.c
+++ b/cpus.c
@@ -854,13 +854,30 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
     return NULL;
 }
 
+static void qemu_thread_kick(QemuThread *thread)
+{
+#ifndef _WIN32
+    int err;
+
+    err = pthread_kill(thread->thread, SIG_IPI);
+    if (err) {
+        fprintf(stderr, "qemu:%s: %s", __func__, strerror(err));
+        exit(1);
+    }
+#else /* _WIN32 */
+    SuspendThread(thread->thread);
+    cpu_signal(0);
+    ResumeThread(thread->thread);
+#endif
+}
+
 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);
+        qemu_thread_kick(env->thread);
         env->thread_kicked = true;
     }
 }
@@ -871,7 +888,7 @@ 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);
+        qemu_thread_kick(cpu_single_env->thread);
         cpu_single_env->thread_kicked = true;
     }
 #else
@@ -893,7 +910,7 @@ void qemu_mutex_lock_iothread(void)
     } else {
         qemu_mutex_lock(&qemu_fair_mutex);
         if (qemu_mutex_trylock(&qemu_global_mutex)) {
-            qemu_thread_signal(tcg_cpu_thread, SIG_IPI);
+            qemu_thread_kick(tcg_cpu_thread);
             qemu_mutex_lock(&qemu_global_mutex);
         }
         qemu_mutex_unlock(&qemu_fair_mutex);
diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c
index 2176f81..5fdf16b 100644
--- a/qemu-thread-posix.c
+++ b/qemu-thread-posix.c
@@ -190,15 +190,6 @@ void qemu_thread_create(QemuThread *thread,
     pthread_sigmask(SIG_SETMASK, &oldset, NULL);
 }
 
-void qemu_thread_signal(QemuThread *thread, int sig)
-{
-    int err;
-
-    err = pthread_kill(thread->thread, sig);
-    if (err)
-        error_exit(err, __func__);
-}
-
 void qemu_thread_self(QemuThread *thread)
 {
     thread->thread = pthread_self();
diff --git a/qemu-thread-posix.h b/qemu-thread-posix.h
index 11978db..35e0a8b 100644
--- a/qemu-thread-posix.h
+++ b/qemu-thread-posix.h
@@ -15,5 +15,4 @@ struct QemuThread {
     pthread_t thread;
 };
 
-void qemu_thread_signal(QemuThread *thread, int sig);
 #endif
-- 
1.7.3.5

[Qemu-devel] Re: [PATCH 04/21] replace qemu_thread_equal API with qemu_thread_is_current

[Jan Kiszka]

On 2011-02-21 09:43, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  cpus.c        |   10 ++--------
>  qemu-thread.c |    4 ++--
>  qemu-thread.h |    2 +-
>  3 files changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 0f33945..aa30474 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -891,11 +891,8 @@ void qemu_cpu_kick_self(void)
>  int qemu_cpu_self(void *_env)
>  {
>      CPUState *env = _env;
> -    QemuThread this;
>  
> -    qemu_thread_self(&this);
> -
> -    return qemu_thread_equal(&this, env->thread);
> +    return qemu_thread_is_current(env->thread);
>  }
>  
>  void qemu_mutex_lock_iothread(void)
> @@ -1023,10 +1020,7 @@ void cpu_stop_current(void)
>  
>  void vm_stop(int reason)
>  {
> -    QemuThread me;
> -    qemu_thread_self(&me);
> -
> -    if (!qemu_thread_equal(&me, &io_thread)) {
> +    if (!qemu_thread_is_current(&io_thread)) {
>          qemu_system_vmstop_request(reason);
>          /*
>           * FIXME: should not return to device code in case
> diff --git a/qemu-thread.c b/qemu-thread.c
> index fbc78fe..28b3f80 100644
> --- a/qemu-thread.c
> +++ b/qemu-thread.c
> @@ -181,9 +181,9 @@ void qemu_thread_self(QemuThread *thread)
>      thread->thread = pthread_self();
>  }
>  
> -int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2)
> +int qemu_thread_is_current(QemuThread *thread)
>  {
> -   return pthread_equal(thread1->thread, thread2->thread);
> +   return pthread_equal(pthread_self(), thread->thread);
>  }
>  
>  void qemu_thread_exit(void *retval)
> diff --git a/qemu-thread.h b/qemu-thread.h
> index 19bb30c..a7e35b4 100644
> --- a/qemu-thread.h
> +++ b/qemu-thread.h
> @@ -38,7 +38,7 @@ void qemu_thread_create(QemuThread *thread,
>                         void *arg);
>  void qemu_thread_signal(QemuThread *thread, int sig);
>  void qemu_thread_self(QemuThread *thread);
> -int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2);
> +int qemu_thread_is_current(QemuThread *thread);
>  void qemu_thread_exit(void *retval);
>  
>  #endif

I'm now in favor of an even more consistent refactoring:
qemu_thread_is_self, qemu_thread_get_self, and qemu_cpu_is_self. See [1].

Jan

[1]
http://git.kiszka.org/?p=qemu-kvm.git;a=commitdiff;h=f5b278f5aec06fe4d140f68caf9b1bf17b4809b2

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

[Qemu-devel] Re: [PATCH 07/21] add assertions on the owner of a QemuMutex

[Jan Kiszka]

On 2011-02-21 09:43, Paolo Bonzini wrote:
> These are already present in the Win32 implementation, add them to
> the pthread wrappers as well.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-thread-posix.c |   27 +++++++++++++++++++++++++--
>  qemu-thread-posix.h |    1 +
>  2 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c
> index 28b3f80..2176f81 100644
> --- a/qemu-thread-posix.c
> +++ b/qemu-thread-posix.c
> @@ -16,9 +16,12 @@
>  #include <time.h>
>  #include <signal.h>
>  #include <stdint.h>
> +#include <assert.h>
>  #include <string.h>
>  #include "qemu-thread.h"
>  
> +static pthread_t pthread_null;
> +
>  static void error_exit(int err, const char *msg)
>  {
>      fprintf(stderr, "qemu: %s: %s\n", msg, strerror(err));
> @@ -28,8 +31,13 @@ static void error_exit(int err, const char *msg)
>  void qemu_mutex_init(QemuMutex *mutex)
>  {
>      int err;
> +    pthread_mutexattr_t mutexattr;
>  
> -    err = pthread_mutex_init(&mutex->lock, NULL);
> +    mutex->owner = pthread_null;
> +    pthread_mutexattr_init(&mutexattr);
> +    pthread_mutexattr_settype(&mutexattr, PTHREAD_MUTEX_ERRORCHECK);
> +    err = pthread_mutex_init(&mutex->lock, &mutexattr);
> +    pthread_mutexattr_destroy(&mutexattr);
>      if (err)
>          error_exit(err, __func__);
>  }
> @@ -48,13 +56,20 @@ void qemu_mutex_lock(QemuMutex *mutex)
>      int err;
>  
>      err = pthread_mutex_lock(&mutex->lock);
> +    mutex->owner = pthread_self();
>      if (err)
>          error_exit(err, __func__);
>  }
>  
>  int qemu_mutex_trylock(QemuMutex *mutex)
>  {
> -    return pthread_mutex_trylock(&mutex->lock);
> +    int err;
> +    err = pthread_mutex_trylock(&mutex->lock);
> +    if (err == 0) {
> +        mutex->owner = pthread_self();
> +    }
> +
> +    return !!err;
>  }
>  
>  static void timespec_add_ms(struct timespec *ts, uint64_t msecs)
> @@ -85,6 +100,11 @@ void qemu_mutex_unlock(QemuMutex *mutex)
>  {
>      int err;
>  
> +    /* An EDEADLOCK would arrive after we reset the owner.  So this
> +       assert is for ease of debugging (it lets you see what is the
> +       actual owner.  */

Don't get this. Why do you want to avoid the proper error detection of
pthread?

> +    assert(pthread_equal(mutex->owner, pthread_self()));
> +    mutex->owner = pthread_null;
>      err = pthread_mutex_unlock(&mutex->lock);
>      if (err)
>          error_exit(err, __func__);
> @@ -130,7 +150,10 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex)
>  {
>      int err;
>  
> +    assert(pthread_equal(mutex->owner, pthread_self()));
> +    mutex->owner = pthread_null;
>      err = pthread_cond_wait(&cond->cond, &mutex->lock);
> +    mutex->owner = pthread_self();
>      if (err)
>          error_exit(err, __func__);
>  }
> diff --git a/qemu-thread-posix.h b/qemu-thread-posix.h
> index 7af371c..11978db 100644
> --- a/qemu-thread-posix.h
> +++ b/qemu-thread-posix.h
> @@ -4,6 +4,7 @@
>  
>  struct QemuMutex {
>      pthread_mutex_t lock;
> +    pthread_t owner;
>  };
>  
>  struct QemuCond {

You said that you want to add owner tracking for assertion in
cond_signal/broadcast. That's OK. But the assertions in the mutex layer
are redundant for PTHREAD_MUTEX_ERRORCHECK - unless I'm missing
something now.

Jan

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

[Qemu-devel] Re: [PATCH 10/21] always qemu_cpu_kick after unhalting a cpu

[Jan Kiszka]

On 2011-02-21 09:43, Paolo Bonzini wrote:
> This ensures env->halt_cond is broadcast, and the loop in
> qemu_tcg_wait_io_event and qemu_kvm_wait_io_event is exited
> naturally rather than through a timeout.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/apic.c          |    1 +
>  hw/ppc.c           |    2 ++
>  hw/sun4m.c         |   10 ++++++++--
>  hw/sun4u.c         |    4 ++--
>  target-s390x/kvm.c |    1 +
>  5 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/apic.c b/hw/apic.c
> index 218d1bb..09fc61b 100644
> --- a/hw/apic.c
> +++ b/hw/apic.c
> @@ -544,6 +544,7 @@ void apic_sipi(DeviceState *d)
>          return;
>      cpu_x86_load_seg_cache_sipi(s->cpu_env, s->sipi_vector);
>      s->wait_for_sipi = 0;
> +    qemu_cpu_kick(s->cpu_env);

Meanwhile I realized that this call is also unneeded. apic_sipi is
invoked only synchronously over the respective VCPU. So please remove.

>  }
>  
>  static void apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode,
> diff --git a/hw/ppc.c b/hw/ppc.c
> index 968aec1..de02d33 100644
> --- a/hw/ppc.c
> +++ b/hw/ppc.c
> @@ -208,6 +208,7 @@ static void ppc970_set_irq (void *opaque, int pin, int level)
>              } else {
>                  LOG_IRQ("%s: restart the CPU\n", __func__);
>                  env->halted = 0;
> +                qemu_cpu_kick(env);
>              }
>              break;
>          case PPC970_INPUT_HRESET:
> @@ -300,6 +301,7 @@ static void ppc40x_set_irq (void *opaque, int pin, int level)
>              } else {
>                  LOG_IRQ("%s: restart the CPU\n", __func__);
>                  env->halted = 0;
> +                qemu_cpu_kick(env);
>              }
>              break;
>          case PPC40x_INPUT_DEBUG:
> diff --git a/hw/sun4m.c b/hw/sun4m.c
> index 30e8a21..df3aa32 100644
> --- a/hw/sun4m.c
> +++ b/hw/sun4m.c
> @@ -253,15 +253,21 @@ void cpu_check_irqs(CPUState *env)
>      }
>  }
>  
> +static void cpu_kick_irq(CPUState *env)
> +{
> +    env->halted = 0;
> +    cpu_check_irqs(env);
> +    qemu_cpu_kick(env);
> +}
> +
>  static void cpu_set_irq(void *opaque, int irq, int level)
>  {
>      CPUState *env = opaque;
>  
>      if (level) {
>          trace_sun4m_cpu_set_irq_raise(irq);
> -        env->halted = 0;
>          env->pil_in |= 1 << irq;
> -        cpu_check_irqs(env);
> +        cpu_kick_irq(env);
>      } else {
>          trace_sun4m_cpu_set_irq_lower(irq);
>          env->pil_in &= ~(1 << irq);
> diff --git a/hw/sun4u.c b/hw/sun4u.c
> index 90b1ce2..d282324 100644
> --- a/hw/sun4u.c
> +++ b/hw/sun4u.c
> @@ -298,6 +298,7 @@ static void cpu_kick_irq(CPUState *env)
>  {
>      env->halted = 0;
>      cpu_check_irqs(env);
> +    qemu_cpu_kick(env);
>  }
>  
>  static void cpu_set_irq(void *opaque, int irq, int level)
> @@ -306,9 +307,8 @@ static void cpu_set_irq(void *opaque, int irq, int level)
>  
>      if (level) {
>          CPUIRQ_DPRINTF("Raise CPU IRQ %d\n", irq);
> -        env->halted = 0;
>          env->pil_in |= 1 << irq;
> -        cpu_check_irqs(env);
> +        cpu_kick_irq(env);
>      } else {
>          CPUIRQ_DPRINTF("Lower CPU IRQ %d\n", irq);
>          env->pil_in &= ~(1 << irq);
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index b349812..6e94274 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -194,6 +194,7 @@ static void kvm_s390_interrupt_internal(CPUState *env, int type, uint32_t parm,
>  
>      env->halted = 0;
>      env->exception_index = -1;
> +    qemu_cpu_kick(env);
>  
>      kvmint.type = type;
>      kvmint.parm = parm;

Otherwise, it looks good.

Jan

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

[Qemu-devel] Re: [PATCH 18/21] move blocking of signals to qemu_signalfd_init

[Jan Kiszka]

On 2011-02-21 09:43, Paolo Bonzini wrote:
> This makes it easier to add a Win32 stub.  The patch does that, too.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  cpus.c |   87 ++++++++++++++++++++++++++-------------------------------------
>  1 files changed, 36 insertions(+), 51 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 1b6893d..5bb95d8 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -346,11 +346,37 @@ static void sigfd_handler(void *opaque)
>      }
>  }
>  
> -static int qemu_signalfd_init(sigset_t mask)
> +static int qemu_signalfd_init(void)
>  {
>      int sigfd;
> +    sigset_t set;
>  
> -    sigfd = qemu_signalfd(&mask);
> +#ifdef CONFIG_IOTHREAD
> +    /* SIGUSR2 used by posix-aio-compat.c */
> +    sigemptyset(&set);
> +    sigaddset(&set, SIGUSR2);
> +    pthread_sigmask(SIG_UNBLOCK, &set, NULL);

These lines are actually not related to signalfd setup. So you may want
to rename this function or move everything that does not block signals
and then pass the very same set to qemu_signalfd elsewhere.

> +
> +    sigemptyset(&set);
> +    sigaddset(&set, SIGIO);
> +    sigaddset(&set, SIGALRM);
> +    sigaddset(&set, SIG_IPI);
> +    sigaddset(&set, SIGBUS);
> +    pthread_sigmask(SIG_BLOCK, &set, NULL);
> +#else
> +    sigemptyset(&set);
> +    sigaddset(&set, SIGBUS);
> +    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);
> +    }
> +#endif
> +
> +    sigfd = qemu_signalfd(&set);
>      if (sigfd == -1) {
>          fprintf(stderr, "failed to create signalfd\n");
>          return -errno;
> @@ -438,6 +464,12 @@ static void qemu_event_increment(void)
>  static void qemu_kvm_eat_signals(CPUState *env)
>  {
>  }
> +
> +static int qemu_signalfd_init(void)
> +{
> +    return 0;
> +}
> +
>  #endif /* _WIN32 */
>  
>  #ifndef CONFIG_IOTHREAD
> @@ -471,39 +503,14 @@ static void qemu_kvm_init_cpu_signals(CPUState *env)
>  #endif
>  }
>  
> -#ifndef _WIN32
> -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
> -         * 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;
>  
> -    blocked_signals = block_synchronous_signals();
> -
> -    ret = qemu_signalfd_init(blocked_signals);
> +    ret = qemu_signalfd_init();
>      if (ret) {
>          return ret;
>      }
> -#endif
>  
>      qemu_init_sigbus();
>  
> @@ -651,35 +658,13 @@ static void qemu_tcg_init_cpu_signals(void)
>      pthread_sigmask(SIG_UNBLOCK, &set, NULL);
>  }
>  
> -static sigset_t block_io_signals(void)
> -{
> -    sigset_t set;
> -
> -    /* 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);
> -
> -    return set;
> -}
> -
>  int qemu_init_main_loop(void)
>  {
>      int ret;
> -    sigset_t blocked_signals;
>  
>      qemu_init_sigbus();
>  
> -    blocked_signals = block_io_signals();
> -
> -    ret = qemu_signalfd_init(blocked_signals);
> +    ret = qemu_signalfd_init();
>      if (ret) {
>          return ret;
>      }

For consistency reasons, you should also refactor the !IOTHREAD case.

Jan

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

[Qemu-devel] Re: [PATCH 07/21] add assertions on the owner of a QemuMutex

[Paolo Bonzini]

On 02/21/2011 10:50 AM, Jan Kiszka wrote:
>> >  +    /* An EDEADLOCK would arrive after we reset the owner.  So this
>> >  +       assert is for ease of debugging (it lets you see what is the
>> >  +       actual owner.  */
>
> Don't get this. Why do you want to avoid the proper error detection of
> pthread?

Because by the time you get to error_exit mutex->owner has been NULL-ed 
out already.  So it doesn't help to put a breakpoint on error_exit, you 
cannot find out which thread was the owner.

> But the assertions in the mutex layer
> are redundant for PTHREAD_MUTEX_ERRORCHECK - unless I'm missing
> something now.

Yes, but tracking the mutex's owner gives a bit more specific 
information when an error happens even for mutexes.  I removed them from 
lock/trylock, but for unlock it's already too late when the error happens.

Paolo

[Qemu-devel] Re: [PATCH 18/21] move blocking of signals to qemu_signalfd_init

[Paolo Bonzini]

On 02/21/2011 11:12 AM, Jan Kiszka wrote:
> On 2011-02-21 09:43, Paolo Bonzini wrote:
>> This makes it easier to add a Win32 stub.  The patch does that, too.
>>
>> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
>> ---
>>   cpus.c |   87 ++++++++++++++++++++++++++-------------------------------------
>>   1 files changed, 36 insertions(+), 51 deletions(-)
>>
>> diff --git a/cpus.c b/cpus.c
>> index 1b6893d..5bb95d8 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -346,11 +346,37 @@ static void sigfd_handler(void *opaque)
>>       }
>>   }
>>
>> -static int qemu_signalfd_init(sigset_t mask)
>> +static int qemu_signalfd_init(void)
>>   {
>>       int sigfd;
>> +    sigset_t set;
>>
>> -    sigfd = qemu_signalfd(&mask);
>> +#ifdef CONFIG_IOTHREAD
>> +    /* SIGUSR2 used by posix-aio-compat.c */
>> +    sigemptyset(&set);
>> +    sigaddset(&set, SIGUSR2);
>> +    pthread_sigmask(SIG_UNBLOCK,&set, NULL);
>
> These lines are actually not related to signalfd setup. So you may want
> to rename this function or move everything that does not block signals
> and then pass the very same set to qemu_signalfd elsewhere.

Ok, I'll just rename qemu_signalfd_init to qemu_init_iothread_signals.

> For consistency reasons, you should also refactor the !IOTHREAD case.

I think I did?

non-iothread:

>>   int qemu_init_main_loop(void)
>>   {
>> -#ifndef _WIN32
>> -    sigset_t blocked_signals;
>>       int ret;
>>
>> -    blocked_signals = block_synchronous_signals();
>> -
>> -    ret = qemu_signalfd_init(blocked_signals);
>> +    ret = qemu_signalfd_init();
>>       if (ret) {
>>           return ret;
>>       }
>> -#endif

iothread:

>>   int qemu_init_main_loop(void)
>>   {
>>       int ret;
>> -    sigset_t blocked_signals;
>>
>>       qemu_init_sigbus();
>>
>> -    blocked_signals = block_io_signals();
>> -
>> -    ret = qemu_signalfd_init(blocked_signals);
>> +    ret = qemu_signalfd_init();
>>       if (ret) {
>>           return ret;
>>       }

Paolo

[Qemu-devel] Re: [PATCH 07/21] add assertions on the owner of a QemuMutex

[Jan Kiszka]

On 2011-02-21 11:15, Paolo Bonzini wrote:
> On 02/21/2011 10:50 AM, Jan Kiszka wrote:
>>>>  +    /* An EDEADLOCK would arrive after we reset the owner.  So this
>>>>  +       assert is for ease of debugging (it lets you see what is the
>>>>  +       actual owner.  */
>>
>> Don't get this. Why do you want to avoid the proper error detection of
>> pthread?
> 
> Because by the time you get to error_exit mutex->owner has been NULL-ed 
> out already.  So it doesn't help to put a breakpoint on error_exit, you 
> cannot find out which thread was the owner.

That's easy, "p <my_mutex>" will tell (the structure contains the
owner's tid).

And for debugging invalid mutex_unlock calls, it's more interesting to
track the call path of that thread which incorrectly claimed to hold the
lock.

Jan

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

[Qemu-devel] Re: [PATCH 07/21] add assertions on the owner of a QemuMutex

[Paolo Bonzini]

On 02/21/2011 11:22 AM, Jan Kiszka wrote:
> That's easy, "p<my_mutex>" will tell (the structure contains the
> owner's tid).
>
> And for debugging invalid mutex_unlock calls, it's more interesting to
> track the call path of that thread which incorrectly claimed to hold the
> lock.

Ok, will remove.

Paolo

[Qemu-devel] Re: [PATCH 18/21] move blocking of signals to qemu_signalfd_init

[Jan Kiszka]

On 2011-02-21 11:20, Paolo Bonzini wrote:
> On 02/21/2011 11:12 AM, Jan Kiszka wrote:
>> On 2011-02-21 09:43, Paolo Bonzini wrote:
>>> This makes it easier to add a Win32 stub.  The patch does that, too.
>>>
>>> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
>>> ---
>>>   cpus.c |   87 ++++++++++++++++++++++++++-------------------------------------
>>>   1 files changed, 36 insertions(+), 51 deletions(-)
>>>
>>> diff --git a/cpus.c b/cpus.c
>>> index 1b6893d..5bb95d8 100644
>>> --- a/cpus.c
>>> +++ b/cpus.c
>>> @@ -346,11 +346,37 @@ static void sigfd_handler(void *opaque)
>>>       }
>>>   }
>>>
>>> -static int qemu_signalfd_init(sigset_t mask)
>>> +static int qemu_signalfd_init(void)
>>>   {
>>>       int sigfd;
>>> +    sigset_t set;
>>>
>>> -    sigfd = qemu_signalfd(&mask);
>>> +#ifdef CONFIG_IOTHREAD
>>> +    /* SIGUSR2 used by posix-aio-compat.c */
>>> +    sigemptyset(&set);
>>> +    sigaddset(&set, SIGUSR2);
>>> +    pthread_sigmask(SIG_UNBLOCK,&set, NULL);
>>
>> These lines are actually not related to signalfd setup. So you may want
>> to rename this function or move everything that does not block signals
>> and then pass the very same set to qemu_signalfd elsewhere.
> 
> Ok, I'll just rename qemu_signalfd_init to qemu_init_iothread_signals.
> 
>> For consistency reasons, you should also refactor the !IOTHREAD case.
> 
> I think I did?

Yeah, I must have missed that qemu_signalfd_init was already refactored
by whoever :) to be independent of CONFIG_IOTHREAD.

Jan

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

[Qemu-devel] Re: [PATCH 04/21] replace qemu_thread_equal API with qemu_thread_is_current

[Paolo Bonzini]

On 02/21/2011 10:41 AM, Jan Kiszka wrote:
> I'm now in favor of an even more consistent refactoring:
> qemu_thread_is_self, qemu_thread_get_self, and qemu_cpu_is_self. See [1].
>
> Jan
>
> [1]
> http://git.kiszka.org/?p=qemu-kvm.git;a=commitdiff;h=f5b278f5aec06fe4d140f68caf9b1bf17b4809b2

Ok, I'll replace this patch with yours.

Paolo

[Qemu-devel] Re: [PATCH 21/21] add Win32 IPI service

[Jan Kiszka]

On 2011-02-21 09:43, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  cpus.c              |   23 ++++++++++++++++++++---
>  qemu-thread-posix.c |    9 ---------
>  qemu-thread-posix.h |    1 -
>  3 files changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index fd791a3..869ed1a 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -854,13 +854,30 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
>      return NULL;
>  }
>  
> +static void qemu_thread_kick(QemuThread *thread)
> +{
> +#ifndef _WIN32
> +    int err;
> +
> +    err = pthread_kill(thread->thread, SIG_IPI);
> +    if (err) {
> +        fprintf(stderr, "qemu:%s: %s", __func__, strerror(err));
> +        exit(1);
> +    }
> +#else /* _WIN32 */
> +    SuspendThread(thread->thread);
> +    cpu_signal(0);
> +    ResumeThread(thread->thread);
> +#endif
> +}
> +

qemu_thread_* services do not truly belong here.

What about defining SIG_IPI as QEMU_SIG_KICK which would be provided by
qemu-thread-posix.h? Then the qemu-thread-posix.c could implement
qemu_thread_kick() again.

>  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);
> +        qemu_thread_kick(env->thread);
>          env->thread_kicked = true;
>      }
>  }
> @@ -871,7 +888,7 @@ 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);
> +        qemu_thread_kick(cpu_single_env->thread);
>          cpu_single_env->thread_kicked = true;
>      }
>  #else
> @@ -893,7 +910,7 @@ void qemu_mutex_lock_iothread(void)
>      } else {
>          qemu_mutex_lock(&qemu_fair_mutex);
>          if (qemu_mutex_trylock(&qemu_global_mutex)) {
> -            qemu_thread_signal(tcg_cpu_thread, SIG_IPI);
> +            qemu_thread_kick(tcg_cpu_thread);
>              qemu_mutex_lock(&qemu_global_mutex);
>          }
>          qemu_mutex_unlock(&qemu_fair_mutex);
> diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c
> index 2176f81..5fdf16b 100644
> --- a/qemu-thread-posix.c
> +++ b/qemu-thread-posix.c
> @@ -190,15 +190,6 @@ void qemu_thread_create(QemuThread *thread,
>      pthread_sigmask(SIG_SETMASK, &oldset, NULL);
>  }
>  
> -void qemu_thread_signal(QemuThread *thread, int sig)
> -{
> -    int err;
> -
> -    err = pthread_kill(thread->thread, sig);
> -    if (err)
> -        error_exit(err, __func__);
> -}
> -
>  void qemu_thread_self(QemuThread *thread)
>  {
>      thread->thread = pthread_self();
> diff --git a/qemu-thread-posix.h b/qemu-thread-posix.h
> index 11978db..35e0a8b 100644
> --- a/qemu-thread-posix.h
> +++ b/qemu-thread-posix.h
> @@ -15,5 +15,4 @@ struct QemuThread {
>      pthread_t thread;
>  };
>  
> -void qemu_thread_signal(QemuThread *thread, int sig);
>  #endif

Jan

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

[Qemu-devel] Re: [PATCH 21/21] add Win32 IPI service

[Paolo Bonzini]

On 02/21/2011 01:00 PM, Jan Kiszka wrote:
> qemu_thread_* services do not truly belong here.
>
> What about defining SIG_IPI as QEMU_SIG_KICK which would be provided by
> qemu-thread-posix.h? Then the qemu-thread-posix.c could implement
> qemu_thread_kick() again.

The function is really specific to VCPU threads.  It is simply a wrapper 
for the common bit of qemu_kick_vcpu and qemu_mutex_lock_iothread.  I'll 
rename it to qemu_cpu_kick_thread and make it accept a CPUState.

Paolo

[Qemu-devel] Re: [PATCH 21/21] add Win32 IPI service

[Jan Kiszka]

On 2011-02-21 13:15, Paolo Bonzini wrote:
> On 02/21/2011 01:00 PM, Jan Kiszka wrote:
>> qemu_thread_* services do not truly belong here.
>>
>> What about defining SIG_IPI as QEMU_SIG_KICK which would be provided by
>> qemu-thread-posix.h? Then the qemu-thread-posix.c could implement
>> qemu_thread_kick() again.
> 
> The function is really specific to VCPU threads.  It is simply a wrapper 
> for the common bit of qemu_kick_vcpu and qemu_mutex_lock_iothread.  I'll 
> rename it to qemu_cpu_kick_thread and make it accept a CPUState.

...or that way. We should just try to respect the abstraction layers.

Jan

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

Re: [Qemu-devel] [PATCH uq/master 00/21] Win32 iothread support

[Anthony Liguori]

On 02/21/2011 02:43 AM, Paolo Bonzini wrote:
> After gathering the comments about the two series I sent separately,
> here is the full series for Win32 iothread support, ready to be
> applied to uq/master.
>
> Patches 1 to 5 are generic Win32 improvements, including the qemu-thread
> implementation.  Because of complex dependencies, I think it's better
> if this part is also routed through uq/master.
>
> Patches 6 to 8 are generic threading improvements, including using
> PTHREAD_MUTEX_ERRORCHECK as suggested by Jan.
>
> Patches 9 to 16 eliminate polling, replacing condition variable
> timedwait with wait.
>
> Patch 17 removes a redundant condition from the TCG cpu_exec_all
> function.
>
> Patches 18 to 20 add all necessary stubs to make iothread compile
> with Win32, except the IPI calls.  These are provided by patch 21.
>
> Tested on Wine and Linux, not on "real" Windows.  The series introduces
> a dependency on Windows 2K or newer.  I don't think either 95/98/ME
> or Windows NT 3.x are reasonable host systems for QEMU, anyway.
>
> I incorporated all suggestions from Jan.
>    

So why reinvent this instead of just using glib for thread support?

Regards,

Anthony Liguori

> Paolo Bonzini (21):
>    unlock iothread during WaitForMultipleObjects
>    implement win32 dynticks timer
>    use win32 timer queues
>    replace qemu_thread_equal API with qemu_thread_is_current
>    add win32 qemu-thread implementation
>    include qemu-thread.h early
>    add assertions on the owner of a QemuMutex
>    remove CONFIG_THREAD
>    inline cpu_halted into sole caller
>    always qemu_cpu_kick after unhalting a cpu
>    exit round-robin vcpu loop if cpu->stopped is true
>    always signal pause_cond after stopping a VCPU
>    do not use timedwait on qemu_halt_cond
>    do not use timedwait on qemu_system_cond
>    do not use timedwait on qemu_pause_cond
>    do not use timedwait on qemu_cpu_cond
>    iothread stops the vcpu thread via IPI
>    move blocking of signals to qemu_signalfd_init
>    provide dummy signal init functions for win32
>    protect qemu_cpu_kick_self for Win32
>    add Win32 IPI service
>
>   Makefile.objs                        |    4 +-
>   configure                            |    2 -
>   cpu-defs.h                           |    1 +
>   cpu-exec.c                           |   17 ++-
>   cpus.c                               |  288 +++++++++++++++++----------------
>   hw/apic.c                            |    1 +
>   hw/ppc.c                             |    2 +
>   hw/sun4m.c                           |   10 +-
>   hw/sun4u.c                           |    4 +-
>   os-win32.c                           |    2 +
>   qemu-thread.c =>  qemu-thread-posix.c |   40 +++--
>   qemu-thread-posix.h                  |   18 ++
>   qemu-thread-win32.c                  |  261 ++++++++++++++++++++++++++++++
>   qemu-thread-win32.h                  |   21 +++
>   qemu-thread.h                        |   29 ++--
>   qemu-timer.c                         |   88 +++++------
>   target-alpha/exec.h                  |   11 --
>   target-arm/exec.h                    |   13 --
>   target-cris/exec.h                   |   11 --
>   target-i386/exec.h                   |   12 --
>   target-m68k/exec.h                   |   10 --
>   target-microblaze/exec.h             |   11 --
>   target-mips/exec.h                   |   11 --
>   target-ppc/exec.h                    |   11 --
>   target-s390x/exec.h                  |   12 --
>   target-s390x/kvm.c                   |    1 +
>   target-sh4/cpu.h                     |    1 -
>   target-sh4/exec.h                    |   11 --
>   target-sparc/exec.h                  |   10 --
>   29 files changed, 559 insertions(+), 353 deletions(-)
>   rename qemu-thread.c =>  qemu-thread-posix.c (78%)
>   create mode 100644 qemu-thread-posix.h
>   create mode 100644 qemu-thread-win32.c
>   create mode 100644 qemu-thread-win32.h
>
>    

Re: [Qemu-devel] [PATCH uq/master 00/21] Win32 iothread support

[Paolo Bonzini]

On 02/22/2011 09:35 PM, Anthony Liguori wrote:
>
> So why reinvent this instead of just using glib for thread support?

Because Win32 thread support is one patch out of 21.

Paolo