Make cpu_single_env thread local (Linux only for now)

PATCH In review Dr. David Alan Gilbert 7 participants 20 messages Oct 3, 2011 – Oct 26, 2011

lore

Share & tools

lore

Dr. David Alan Gilbert

author

2011-10-03 16:34 UTC #0 lore raw

PATCH 1/1 Make cpu_single_env thread local (Linux only for now)

Make cpu_single_env thread local (Linux only for now)
* Fixes some user space threading issues (esp those triggered

    by bug 823902)

Against rev d11cf8cc..., tested on ARM user mode, and ARM Vexpress system mode (with Blue Swirl's fix from yesterday) - only tested on Linux host. Lets me run ARM userspace firefox.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

cpu-all.h +6 -0

index 42a5fa0..d895ee6 100644
@@ -334,7 +334,13 @@ void cpu_dump_statistics(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
 void QEMU_NORETURN cpu_abort(CPUState *env, const char *fmt, ...)
     GCC_FMT_ATTR(2, 3);
 extern CPUState *first_cpu;
+
+#ifdef __linux__
+/* DAG: Only tested thread local on Linux, feel free to add others */
+extern __thread CPUState *cpu_single_env;
+#else
 extern CPUState *cpu_single_env;
+#endif
 
 /* Flags for use in ENV->INTERRUPT_PENDING.

exec.c +5 -0

index d0cbf15..b82d8e4 100644
@@ -120,7 +120,12 @@ static MemoryRegion *system_io;
 CPUState *first_cpu;
 /* current CPU in the current thread. It is only valid inside
    cpu_exec() */
+#ifdef __linux__
+/* DAG: Only tested thread local on Linux, feel free to add others */
+__thread CPUState *cpu_single_env;
+#else
 CPUState *cpu_single_env;
+#endif
 /* 0 = Do not count executed instructions.
    1 = Precise instruction counting.
    2 = Adaptive rate instruction counting.  */

Jan Kiszka

2011-10-03 17:51 UTC #1 lore raw

Quoted 15 lines · Dr. David Alan Gilbert

Make cpu_single_env thread local (Linux only for now)
* Fixes some user space threading issues (esp those triggered
    by bug 823902)
Against rev d11cf8cc..., tested on ARM user mode, and ARM Vexpress system mode (with Blue Swirl's fix from yesterday) - only tested on Linux host. Lets me run ARM userspace firefox.
Signed-off-by Dr. David Alan Gilbert <[email protected]>
cpu-all.h +0 -0
index 42a5fa0..d895ee6 100644

cpu-all.h void cpu_dump_statistics(CPUState *env, FILE *f, fprintf_function cpu_fprintf, · line 343

Quoted 10 lines

cpu-all.h +6 -0

@@ -334,7 +334,13 @@ void cpu_dump_statistics(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
 void QEMU_NORETURN cpu_abort(CPUState *env, const char *fmt, ...)
     GCC_FMT_ATTR(2, 3);
 extern CPUState *first_cpu;
+
+#ifdef __linux__
+/* DAG: Only tested thread local on Linux, feel free to add others */
+extern __thread CPUState *cpu_single_env;
+#else
 extern CPUState *cpu_single_env;
+#endif

We need this for all platforms in order to skip qemu_global_mutex while manipulating some CPUState. And leaving some platforms with non-TLS will eventually break them when code is added that assumes TLS.

However, it's not unlikely that some weird platforms / ancient toolchains still have problems with __thread - even on Linux. We may want to play safe and use pthread_key on POSIX.

Jan

Lluís Vilanova

2011-10-04 15:10 UTC #2 lore raw

Quoted 31 lines · Jan Kiszka

Quoted 15 lines · Dr. David Alan Gilbert
Make cpu_single_env thread local (Linux only for now)
* Fixes some user space threading issues (esp those triggered by bug 823902)
Against rev d11cf8cc..., tested on ARM user mode, and ARM Vexpress system mode (with Blue Swirl's fix from yesterday) - only tested on Linux host. Lets me run ARM userspace firefox.
Signed-off-by Dr. David Alan Gilbert <[email protected]>
cpu-all.h +0 -0
index 42a5fa0..d895ee6 100644
cpu-all.h void cpu_dump_statistics(CPUState *env, FILE *f, fprintf_function cpu_fprintf, · line 343
Quoted 10 lines
cpu-all.h +6 -0
@@ -334,7 +334,13 @@ void cpu_dump_statistics(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
 void QEMU_NORETURN cpu_abort(CPUState *env, const char *fmt, ...)
 GCC_FMT_ATTR(2, 3);
 extern CPUState *first_cpu;
+
+#ifdef __linux__
+/* DAG: Only tested thread local on Linux, feel free to add others */
+extern __thread CPUState *cpu_single_env;
+#else
 extern CPUState *cpu_single_env;
+#endif
We need this for all platforms in order to skip qemu_global_mutex while manipulating some CPUState. And leaving some platforms with non-TLS will eventually break them when code is added that assumes TLS.

Quoted 3 lines

However, it's not unlikely that some weird platforms / ancient toolchains still have problems with __thread - even on Linux. We may want to play safe and use pthread_key on POSIX.

Why not make this kind of annotations available in qemu-commmon.h; or even better somewhere less... "generic".

#if defined(CONFIG_SUPPORTS_THREAD_KEYWORD)

// use __thread

#define QEMU_DECL_TLS(type, name) \
        extern __thread type name;
#define QEMU_DEF_TLS(type, name) \
        __thread type name;

#define QEMU_SET_TLS(name, value) \
do { name = value; } while (0)
#define QEMU_GET_TLS(name) \
name

#elif defined(CONFIG_SUPPORTS_PTHREAD_KEY)

// use pthread_key_t

#define QEMU_DECL_TLS(type, name) \
        extern type _type_##name; \
        extern pthread_key_t name;
#define QEMU_DEF_TLS(type, name) \
        pthread_key_t name; \
        void _init_##name (void) __attribute__((constructor)); \
        void _init_##name (void) { pthread_key_create(&name, NULL); }
#define QEMU_SET_TLS(name, value) \
        do { pthread_setspecific(name, (void*)value); } while (0)
#define QEMU_GET_TLS(name) \
        ((typeof(_type_##name))pthread_getspecific(name))

#else
#error Go home
#endif

Lluis

Signature · 4 lines

 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Jan Kiszka

2011-10-04 17:26 UTC #3 lore raw

Quoted 33 lines · Lluís Vilanova

Quoted 30 lines · Jan Kiszka
Quoted 15 lines · Dr. David Alan Gilbert
Make cpu_single_env thread local (Linux only for now)
* Fixes some user space threading issues (esp those triggered by bug 823902)
Against rev d11cf8cc..., tested on ARM user mode, and ARM Vexpress system mode (with Blue Swirl's fix from yesterday) - only tested on Linux host. Lets me run ARM userspace firefox.
Signed-off-by Dr. David Alan Gilbert <[email protected]>
cpu-all.h +0 -0
index 42a5fa0..d895ee6 100644
cpu-all.h void cpu_dump_statistics(CPUState *env, FILE *f, fprintf_function cpu_fprintf, · line 343
Quoted 10 lines
cpu-all.h +6 -0
@@ -334,7 +334,13 @@ void cpu_dump_statistics(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
 void QEMU_NORETURN cpu_abort(CPUState *env, const char *fmt, ...)
 GCC_FMT_ATTR(2, 3);
 extern CPUState *first_cpu;
+
+#ifdef __linux__
+/* DAG: Only tested thread local on Linux, feel free to add others */
+extern __thread CPUState *cpu_single_env;
+#else
 extern CPUState *cpu_single_env;
+#endif
We need this for all platforms in order to skip qemu_global_mutex while manipulating some CPUState. And leaving some platforms with non-TLS will eventually break them when code is added that assumes TLS.

Quoted 6 lines

Quoted 3 lines
However, it's not unlikely that some weird platforms / ancient toolchains still have problems with __thread - even on Linux. We may want to play safe and use pthread_key on POSIX.
Why not make this kind of annotations available in qemu-commmon.h; or even better somewhere less... "generic".

Yes, we will need some encapsulation for this.

Quoted 28 lines

#if defined(CONFIG_SUPPORTS_THREAD_KEYWORD)

// use __thread

#define QEMU_DECL_TLS(type, name) \
        extern __thread type name;
#define QEMU_DEF_TLS(type, name) \
        __thread type name;

#define QEMU_SET_TLS(name, value) \
do { name = value; } while (0)
#define QEMU_GET_TLS(name) \
name

#elif defined(CONFIG_SUPPORTS_PTHREAD_KEY)

// use pthread_key_t

#define QEMU_DECL_TLS(type, name) \
        extern type _type_##name; \
        extern pthread_key_t name;
#define QEMU_DEF_TLS(type, name) \
        pthread_key_t name; \
        void _init_##name (void) __attribute__((constructor)); \
        void _init_##name (void) { pthread_key_create(&name, NULL); }
#define QEMU_SET_TLS(name, value) \
        do { pthread_setspecific(name, (void*)value); } while (0)
#define QEMU_GET_TLS(name) \
        ((typeof(_type_##name))pthread_getspecific(name))

Quoted 3 lines

#else
#error Go home
#endif

Looks like a start. But I would avoid macros and go for (static inline) functions where possible. And initialization should be explicit (so that you can start using TLS already inside constructors).

Jan

Paolo Bonzini

2011-10-05 07:08 UTC #4 lore raw

Quoted 3 lines · Jan Kiszka

Looks like a start. But I would avoid macros and go for (static inline) functions where possible. And initialization should be explicit (so that you can start using TLS already inside constructors).

Here is the patch I wrote to do more or less the same thing, plus Windows support. It's a bit different in that I wrote a macro that can be used as lvalue.

It doesn't let you use TLS inside constructors, however, unless you use constructor priorities.

Paolo

Quoted 1 line

From 497ed0672f7fe08d9654a0e5c11b682bea43a59e Mon Sep 17 00:00:00 2001

From: Paolo Bonzini <[email protected]>
Date: Wed, 5 Oct 2011 08:29:39 +0200
Subject: [PATCH 0/3] *** SUBJECT HERE ***

*** BLURB HERE ***

Paolo Bonzini (3):
qemu-threads: add TLS wrappers
windows
tls

 configure           |   20 +++++++++++++++++
 coroutine-win32.c   |    7 ++++-
 cpu-all.h           |    4 ++-
 cpus.c              |   13 +++++++---
 exec.c              |    2 +-
 qemu-thread-posix.c |   42 ++++++++++++++++++++++++++++++++---
 qemu-thread-win32.c |   16 +++++++++++++
 qemu-tls-gcc.h      |   25 +++++++++++++++++++++
 qemu-tls-pthread.h  |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-tls-win32.h    |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
 10 files changed, 234 insertions(+), 12 deletions(-)
 create mode 100644 qemu-tls-gcc.h
 create mode 100644 qemu-tls-pthread.h
 create mode 100644 qemu-tls-win32.h

Signature · 1 line

1.7.6

Quoted 1 line

From d8c3c4e789f9b86a66042a9181333e1a096b6b93 Mon Sep 17 00:00:00 2001

From: Paolo Bonzini <[email protected]>
Date: Tue, 16 Aug 2011 10:37:44 -0700
Subject: [PATCH 1/3] qemu-threads: add TLS wrappers

Win32 emulated TLS is slow and is not available on all versions of GCC; some versions of Unix only have pthread_getspecific as a means to access TLS.

Actually, Win32 does have support for decent TLS, and GCC does not map __thread to it. But kind of unlike ELF TLS, it's perfectly possible to declare TLS variables with simple C code! For pthread_getspecific we similarly allocate a memory block; we have to compute all the offsets at load time, which is also cheaper than doing a pthread_key_create for each variable. Not optimal, but it works.

This patch adds wrappers to qemu-thread that will use __thread or pthread_getspecific on POSIX systems, and the .tls segment on Windows.

It does kinda uglify the declarations, but not too much.

Signed-off-by Paolo Bonzini <[email protected]>

---
 configure           |   20 +++++++++++++++++
 coroutine-win32.c   |    7 ++++-
 qemu-thread-posix.c |   42 ++++++++++++++++++++++++++++++++---
 qemu-thread-win32.c |   16 +++++++++++++
 qemu-tls-gcc.h      |   25 +++++++++++++++++++++
 qemu-tls-pthread.h  |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-tls-win32.h    |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 221 insertions(+), 6 deletions(-)
 create mode 100644 qemu-tls-gcc.h
 create mode 100644 qemu-tls-pthread.h
 create mode 100644 qemu-tls-win32.h

configure +20 -0

index 59b1494..50d7b54 100755
@@ -1215,6 +1215,23 @@ EOF
 fi
 
 ##########################################
+# __thread check
+
+if test "$mingw32" = "yes" ; then
+    tls_model=win32
+else
+    cat > $TMPC << EOF
+__thread int x;
+int main() { return x; }
+EOF
+    if compile_prog "" "" ; then
+        tls_model=gcc
+    else
+        tls_model=pthread
+    fi
+fi
+
+##########################################
 # zlib check
 
 if test "$zlib" != "no" ; then

@@ -2697,6 +2714,7 @@ echo "Documentation     $docs"
 [ ! -z "$uname_release" ] && \
 echo "uname -r          $uname_release"
 echo "NPTL support      $nptl"
+echo "TLS support       $tls_model"
 echo "GUEST_BASE        $guest_base"
 echo "PIE user targets  $user_pie"
 echo "vde support       $vde"

@@ -3580,6 +3598,8 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then
   esac
 fi
 
+symlink $source_path/qemu-tls-$tls_model.h qemu-tls.h
+
 # use included Linux headers
 if test "$linux" = "yes" ; then
   includes="-I\$(SRC_PATH)/linux-headers $includes"

coroutine-win32.c +5 -2

index 4179609..708e220 100644
@@ -24,6 +24,7 @@
 
 #include "qemu-common.h"
 #include "qemu-coroutine-int.h"
+#include "qemu-tls.h"
 
 typedef struct
 {

@@ -33,8 +34,10 @@ typedef struct
     CoroutineAction action;
 } CoroutineWin32;
 
-static __thread CoroutineWin32 leader;
-static __thread Coroutine *current;
+static DEFINE_TLS(CoroutineWin32, tls_leader);
+static DEFINE_TLS(Coroutine *, tls_current);
+#define leader get_tls(tls_leader)
+#define current get_tls(tls_current)
 
 CoroutineAction qemu_coroutine_switch(Coroutine *from_, Coroutine *to_,
                                       CoroutineAction action)

qemu-thread-posix.c +33 -4

index ac3c0c9..acd04ff 100644
@@ -18,6 +18,9 @@
 #include <stdint.h>
 #include <string.h>
 #include "qemu-thread.h"
+#include "qemu-common.h"
+#include "qemu-tls.h"
+#include "qemu-barrier.h"
 
 static void error_exit(int err, const char *msg)
 {

@@ -115,18 +118,44 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex)
         error_exit(err, __func__);
 }
 
+size_t tls_size;
+pthread_key_t tls_key;
+
+static void __attribute__((constructor(102))) tls_init_thread(void)
+{
+    /* It's easier to always create the key, even if using GCC tls.  */
+    pthread_key_create(&tls_key, g_free);
+    _tls_init_thread();
+}
+
+typedef struct QemuThreadData {
+    void *(*start_routine)(void *);
+    void *arg;
+} QemuThreadData;
+
+static void *start_routine_wrapper(void *arg)
+{
+    QemuThreadData args = *(QemuThreadData *) arg;
+    g_free(arg);
+    _tls_init_thread();
+    return args.start_routine(args.arg);
+}
+
 void qemu_thread_create(QemuThread *thread,
-                       void *(*start_routine)(void*),
+                       void *(*start_routine)(void *),
                        void *arg)
 {
+    sigset_t set, oldset;
+    QemuThreadData *args = g_malloc(sizeof(QemuThreadData));
     int err;
 
-    /* Leave signal handling to the iothread.  */
-    sigset_t set, oldset;
+    args->start_routine = start_routine;
+    args->arg = arg;
 
+    /* Leave signal handling to the iothread.  */
     sigfillset(&set);
     pthread_sigmask(SIG_SETMASK, &set, &oldset);
-    err = pthread_create(&thread->thread, NULL, start_routine, arg);
+    err = pthread_create(&thread->thread, NULL, start_routine_wrapper, args);
     if (err)
         error_exit(err, __func__);

qemu-thread-win32.c +16 -0

index db8e744..118d92f 100644
@@ -16,6 +16,22 @@
 #include <assert.h>
 #include <limits.h>
 
+/* TLS support.  */
+
+int __attribute__((section(".tls$000"))) _tls_start = 0;
+int __attribute__((section(".tls$ZZZ"))) _tls_end = 0;
+int _tls_index = 0;
+
+const IMAGE_TLS_DIRECTORY _tls_used __attribute__((used, section(".rdata$T"))) = {
+ (ULONG)(ULONG_PTR) &_tls_start, /* start of tls data */
+ (ULONG)(ULONG_PTR) &_tls_end,   /* end of tls data */
+ (ULONG)(ULONG_PTR) &_tls_index, /* address of tls_index */
+ (ULONG) 0,                      /* pointer to callbacks */
+ (ULONG) 0,                      /* size of tls zero fill */
+ (ULONG) 0                       /* characteristics */
+};
+
+
 static void error_exit(int err, const char *msg)
 {
     char *pstr;

qemu-tls-gcc.h new file +24 -0

new file mode 100644
index 0000000..8cff148
@@ -0,0 +1,24 @@
+/*
+ * TLS with __thread
+ *
+ * Copyright Red Hat, Inc. 2011
+ *
+ * Authors:
+ *  Paolo Bonzini   <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_TLS_GCC_H
+#define QEMU_TLS_GCC_H
+
+#define DECLARE_TLS(type, x) extern __thread type tls__##x
+#define DEFINE_TLS(type, x)  __thread type tls__##x
+#define get_tls(x)           tls__##x
+
+static inline size_t tls_init(size_t size, size_t alignment) { return 0; }
+static inline void _tls_init_thread(void) {}
+
+#endif

qemu-tls-pthread.h new file +57 -0

57 lines not on this page · raw message

qemu-tls-win32.h new file +59 -0

59 lines not on this page · raw message

Signature · 1 line

1.7.6

Quoted 1 line

From b10531473a833cf5e925f00461134b0bcd2295bb Mon Sep 17 00:00:00 2001

From: Paolo Bonzini <[email protected]>
Date: Mon, 29 Aug 2011 17:03:55 +0200
Subject: [PATCH 2/3] Prepare Windows port for thread-local cpu_single_env

Windows does not execute cpu_signal in VCPU-thread context, so it won't be able to use cpu_single_env there. However, it has the CPUState available, so nothing is lost.

Signed-off-by Paolo Bonzini <[email protected]>

---
 cpus.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

cpus.c +9 -4

index 8978779..822ce7a 100644
@@ -176,10 +176,10 @@ static void cpu_handle_guest_debug(CPUState *env)
     env->stopped = 1;
 }
 
-static void cpu_signal(int sig)
+static inline void do_cpu_kick(CPUState *env)
 {
-    if (cpu_single_env) {
-        cpu_exit(cpu_single_env);
+    if (env) {
+        cpu_exit(env);
     }
     exit_request = 1;
 }

@@ -437,6 +437,11 @@ static void qemu_kvm_init_cpu_signals(CPUState *env)
     }
 }
 
+static void cpu_signal(int sig)
+{
+    do_cpu_kick(cpu_single_env);
+}
+
 static void qemu_tcg_init_cpu_signals(void)
 {
     sigset_t set;

@@ -708,7 +713,7 @@ static void qemu_cpu_kick_thread(CPUState *env)
 #else /* _WIN32 */
     if (!qemu_cpu_is_self(env)) {
         SuspendThread(env->thread->thread);
-        cpu_signal(0);
+        do_cpu_kick(env);
         ResumeThread(env->thread->thread);
     }
 #endif

Signature · 1 line

1.7.6

Quoted 1 line

From 497ed0672f7fe08d9654a0e5c11b682bea43a59e Mon Sep 17 00:00:00 2001

From: Paolo Bonzini <[email protected]>
Date: Mon, 29 Aug 2011 17:04:01 +0200
Subject: [PATCH 3/3] Make cpu_single_env thread-local

Signed-off-by Paolo Bonzini <[email protected]>

---
 cpu-all.h |    4 +++-
 exec.c    |    2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

cpu-all.h +3 -1

index 42a5fa0..da457dc 100644
@@ -20,6 +20,7 @@
 #define CPU_ALL_H
 
 #include "qemu-common.h"
+#include "qemu-tls.h"
 #include "cpu-common.h"
 
 /* some important defines:

@@ -334,7 +335,8 @@ void cpu_dump_statistics(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
 void QEMU_NORETURN cpu_abort(CPUState *env, const char *fmt, ...)
     GCC_FMT_ATTR(2, 3);
 extern CPUState *first_cpu;
-extern CPUState *cpu_single_env;
+DECLARE_TLS(CPUState *,tls_cpu_single_env);
+#define cpu_single_env get_tls(tls_cpu_single_env)
 
 /* Flags for use in ENV->INTERRUPT_PENDING.

exec.c +1 -1

index d0cbf15..66b82db 100644
@@ -120,7 +120,7 @@ static MemoryRegion *system_io;
 CPUState *first_cpu;
 /* current CPU in the current thread. It is only valid inside
    cpu_exec() */
-CPUState *cpu_single_env;
+DEFINE_TLS(CPUState *,cpu_single_env);
 /* 0 = Do not count executed instructions.
    1 = Precise instruction counting.
    2 = Adaptive rate instruction counting.  */

Signature · 1 line

1.7.6

Jan Kiszka

2011-10-05 07:52 UTC #5 lore raw

Quoted 8 lines · Paolo Bonzini

Quoted 3 lines · Jan Kiszka
Looks like a start. But I would avoid macros and go for (static inline) functions where possible. And initialization should be explicit (so that you can start using TLS already inside constructors).
Here is the patch I wrote to do more or less the same thing, plus Windows support. It's a bit different in that I wrote a macro that can be used as lvalue.

Yeah, it probably makes sense to build the abstractions around __thread so that - one day - we can drop the legacy wrappers.

Just do not prepend "tls__" in the gcc model (there is also some inconsistency with prefixes in patch 3). And avoid leading "_" unless they are dictated by the platform. And patch 3 needs to update darwin-user/main.c as well.

Quoted 3 lines

It doesn't let you use TLS inside constructors, however, unless you use constructor priorities.

What is the default priority of constructors BTW? You picked the highest, will others that do not specify one have the same? Then we could also define a QEMU_CONSTRUCTOR wrapper with a lower priority.

Jan

Paolo Bonzini

2011-10-05 09:21 UTC #6 lore raw

Quoted 4 lines · Jan Kiszka

Yeah, it probably makes sense to build the abstractions around __thread so that - one day - we can drop the legacy wrappers.
Just do not prepend "tls__" in the gcc model

Actually I did that on purpose so that people would not forget get_tls. :)

Quoted 1 line

(there is also some inconsistency with prefixes in patch 3).

Yep, the attached v2 actually builds. I also needed a small change to avoid errors with -Wredundant-decls, and I changed it to support arrays with

DECLARE_TLS(int[10], array);

Quoted 2 lines

And avoid leading "_" unless
they are dictated by the platform.

Ok, I replaced tls_init_thread with tls_init_main_thread and _tls_init_thread with tls_init_thread.

Quoted 1 line

And patch 3 needs to update darwin-user/main.c as well.

I think the declaration can just be removed.

Quoted 2 lines

What is the default priority of constructors BTW? You picked the highest, will others that do not specify one have the same?

Looks like the prioritized constructors always run _before_ the others, which is good.

$ cat f.c
int f(void) __attribute__((constructor(101)));
int f(void) { write (1, "101\n", 4); }
int h(void) __attribute__((constructor));
int h(void) { write (1, "default\n", 8); }
int g(void) __attribute__((constructor(102)));
int g(void) { write (1, "102\n", 4); }
int main() { write(1, "main\n", 5); }
$ gcc f.c
$ ./a.out
101
102
default
main

If interested people can test the patches more and submit them more formally, I'd be very glad. I wrote it for RCU, but of course that one is not really going to be 1.0 material (even for 9p).

Paolo

Quoted 1 line

From 497ed0672f7fe08d9654a0e5c11b682bea43a59e Mon Sep 17 00:00:00 2001

From: Paolo Bonzini <[email protected]>
Date: Wed, 5 Oct 2011 08:29:39 +0200
Subject: [PATCH 0/3] *** SUBJECT HERE ***

*** BLURB HERE ***

Paolo Bonzini (3):
qemu-threads: add TLS wrappers
Prepare Windows port for thread-local cpu_single_env Make cpu_single_env thread-local

 configure           |   20 +++++++++++++++++
 coroutine-win32.c   |    7 ++++-
 cpu-all.h           |    4 ++-
 cpus.c              |   13 +++++++---
 exec.c              |    2 +-
 qemu-thread-posix.c |   42 ++++++++++++++++++++++++++++++++---
 qemu-thread-win32.c |   16 +++++++++++++
 qemu-tls-gcc.h      |   25 +++++++++++++++++++++
 qemu-tls-pthread.h  |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-tls-win32.h    |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
 10 files changed, 234 insertions(+), 12 deletions(-)
 create mode 100644 qemu-tls-gcc.h
 create mode 100644 qemu-tls-pthread.h
 create mode 100644 qemu-tls-win32.h

Signature · 1 line

1.7.6

Quoted 1 line

From d8c3c4e789f9b86a66042a9181333e1a096b6b93 Mon Sep 17 00:00:00 2001

From: Paolo Bonzini <[email protected]>
Date: Tue, 16 Aug 2011 10:37:44 -0700
Subject: [PATCH 1/3] qemu-threads: add TLS wrappers

Win32 emulated TLS is slow and is not available on all versions of GCC; some versions of Unix only have pthread_getspecific as a means to access TLS.

This patch adds wrappers to qemu-thread that will use __thread or pthread_getspecific on POSIX systems, and the .tls segment on Windows.

It does kinda uglify the declarations, but not too much.

Signed-off-by Paolo Bonzini <[email protected]>

---
 configure           |   20 +++++++++++++++++
 coroutine-win32.c   |    7 ++++-
 qemu-thread-posix.c |   42 ++++++++++++++++++++++++++++++++---
 qemu-thread-win32.c |   16 +++++++++++++
 qemu-tls-gcc.h      |   25 +++++++++++++++++++++
 qemu-tls-pthread.h  |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-tls-win32.h    |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 221 insertions(+), 6 deletions(-)
 create mode 100644 qemu-tls-gcc.h
 create mode 100644 qemu-tls-pthread.h
 create mode 100644 qemu-tls-win32.h

configure +20 -0

index 59b1494..50d7b54 100755
@@ -1215,6 +1215,23 @@ EOF
 fi
 
 ##########################################
+# __thread check
+
+if test "$mingw32" = "yes" ; then
+    tls_model=win32
+else
+    cat > $TMPC << EOF
+__thread int x;
+int main() { return x; }
+EOF
+    if compile_prog "" "" ; then
+        tls_model=gcc
+    else
+        tls_model=pthread
+    fi
+fi
+
+##########################################
 # zlib check
 
 if test "$zlib" != "no" ; then

@@ -2697,6 +2714,7 @@ echo "Documentation     $docs"
 [ ! -z "$uname_release" ] && \
 echo "uname -r          $uname_release"
 echo "NPTL support      $nptl"
+echo "TLS support       $tls_model"
 echo "GUEST_BASE        $guest_base"
 echo "PIE user targets  $user_pie"
 echo "vde support       $vde"

@@ -3580,6 +3598,8 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then
   esac
 fi
 
+symlink $source_path/qemu-tls-$tls_model.h qemu-tls.h
+
 # use included Linux headers
 if test "$linux" = "yes" ; then
   includes="-I\$(SRC_PATH)/linux-headers $includes"

coroutine-win32.c +5 -2

index 4179609..708e220 100644
@@ -24,6 +24,7 @@
 
 #include "qemu-common.h"
 #include "qemu-coroutine-int.h"
+#include "qemu-tls.h"
 
 typedef struct
 {

@@ -33,8 +34,10 @@ typedef struct
     CoroutineAction action;
 } CoroutineWin32;
 
-static __thread CoroutineWin32 leader;
-static __thread Coroutine *current;
+static DEFINE_TLS(CoroutineWin32, tls_leader);
+static DEFINE_TLS(Coroutine *, tls_current);
+#define leader get_tls(tls_leader)
+#define current get_tls(tls_current)
 
 CoroutineAction qemu_coroutine_switch(Coroutine *from_, Coroutine *to_,
                                       CoroutineAction action)

qemu-thread-posix.c +33 -4

index ac3c0c9..acd04ff 100644
@@ -18,6 +18,9 @@
 #include <stdint.h>
 #include <string.h>
 #include "qemu-thread.h"
+#include "qemu-common.h"
+#include "qemu-tls.h"
+#include "qemu-barrier.h"
 
 static void error_exit(int err, const char *msg)
 {

@@ -115,18 +118,44 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex)
         error_exit(err, __func__);
 }
 
+size_t tls_size;
+pthread_key_t tls_key;
+
+static void __attribute__((constructor(102))) tls_init_main_thread(void)
+{
+    /* It's easier to always create the key, even if using GCC tls.  */
+    pthread_key_create(&tls_key, g_free);
+    tls_init_thread();
+}
+
+typedef struct QemuThreadData {
+    void *(*start_routine)(void *);
+    void *arg;
+} QemuThreadData;
+
+static void *start_routine_wrapper(void *arg)
+{
+    QemuThreadData args = *(QemuThreadData *) arg;
+    g_free(arg);
+    tls_init_thread();
+    return args.start_routine(args.arg);
+}
+
 void qemu_thread_create(QemuThread *thread,
-                       void *(*start_routine)(void*),
+                       void *(*start_routine)(void *),
                        void *arg)
 {
+    sigset_t set, oldset;
+    QemuThreadData *args = g_malloc(sizeof(QemuThreadData));
     int err;
 
-    /* Leave signal handling to the iothread.  */
-    sigset_t set, oldset;
+    args->start_routine = start_routine;
+    args->arg = arg;
 
+    /* Leave signal handling to the iothread.  */
     sigfillset(&set);
     pthread_sigmask(SIG_SETMASK, &set, &oldset);
-    err = pthread_create(&thread->thread, NULL, start_routine, arg);
+    err = pthread_create(&thread->thread, NULL, start_routine_wrapper, args);
     if (err)
         error_exit(err, __func__);

qemu-thread-win32.c +16 -0

index db8e744..118d92f 100644
@@ -16,6 +16,22 @@
 #include <assert.h>
 #include <limits.h>
 
+/* TLS support.  */
+
+int __attribute__((section(".tls$000"))) _tls_start = 0;
+int __attribute__((section(".tls$ZZZ"))) _tls_end = 0;
+int _tls_index = 0;
+
+const IMAGE_TLS_DIRECTORY _tls_used __attribute__((used, section(".rdata$T"))) = {
+ (ULONG)(ULONG_PTR) &_tls_start, /* start of tls data */
+ (ULONG)(ULONG_PTR) &_tls_end,   /* end of tls data */
+ (ULONG)(ULONG_PTR) &_tls_index, /* address of tls_index */
+ (ULONG) 0,                      /* pointer to callbacks */
+ (ULONG) 0,                      /* size of tls zero fill */
+ (ULONG) 0                       /* characteristics */
+};
+
+
 static void error_exit(int err, const char *msg)
 {
     char *pstr;

qemu-tls-gcc.h new file +24 -0

new file mode 100644
index 0000000..8cff148
@@ -0,0 +1,24 @@
+/*
+ * TLS with __thread
+ *
+ * Copyright Red Hat, Inc. 2011
+ *
+ * Authors:
+ *  Paolo Bonzini   <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_TLS_GCC_H
+#define QEMU_TLS_GCC_H
+
+#define DECLARE_TLS(type, x) extern DEFINE_TLS(type, x)
+#define DEFINE_TLS(type, x)  __thread __typeof__(type) tls__##x
+#define get_tls(x)           tls__##x
+
+static inline size_t tls_init(size_t size, size_t alignment) { return 0; }
+static inline void tls_init_thread(void) {}
+
+#endif

qemu-tls-pthread.h new file +59 -0

59 lines not on this page · raw message

qemu-tls-win32.h new file +59 -0

59 lines not on this page · raw message

Signature · 1 line

1.7.6

Quoted 1 line

From b10531473a833cf5e925f00461134b0bcd2295bb Mon Sep 17 00:00:00 2001

From: Paolo Bonzini <[email protected]>
Date: Mon, 29 Aug 2011 17:03:55 +0200
Subject: [PATCH 2/3] Prepare Windows port for thread-local cpu_single_env

Windows does not execute cpu_signal in VCPU-thread context, so it won't be able to use cpu_single_env there. However, it has the CPUState available, so nothing is lost.

Signed-off-by Paolo Bonzini <[email protected]>

---
 cpus.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

cpus.c +9 -4

index 8978779..822ce7a 100644
@@ -176,10 +176,10 @@ static void cpu_handle_guest_debug(CPUState *env)
     env->stopped = 1;
 }
 
-static void cpu_signal(int sig)
+static inline void do_cpu_kick(CPUState *env)
 {
-    if (cpu_single_env) {
-        cpu_exit(cpu_single_env);
+    if (env) {
+        cpu_exit(env);
     }
     exit_request = 1;
 }

@@ -437,6 +437,11 @@ static void qemu_kvm_init_cpu_signals(CPUState *env)
     }
 }
 
+static void cpu_signal(int sig)
+{
+    do_cpu_kick(cpu_single_env);
+}
+
 static void qemu_tcg_init_cpu_signals(void)
 {
     sigset_t set;

@@ -708,7 +713,7 @@ static void qemu_cpu_kick_thread(CPUState *env)
 #else /* _WIN32 */
     if (!qemu_cpu_is_self(env)) {
         SuspendThread(env->thread->thread);
-        cpu_signal(0);
+        do_cpu_kick(env);
         ResumeThread(env->thread->thread);
     }
 #endif

Signature · 1 line

1.7.6

Quoted 1 line

From 6dd053d0acc0f0334432259d989329a4c688fe63 Mon Sep 17 00:00:00 2001

From: Paolo Bonzini <[email protected]>
Date: Mon, 29 Aug 2011 17:04:01 +0200
Subject: [PATCH 3/3] Make cpu_single_env thread-local

Signed-off-by Paolo Bonzini <[email protected]>

---
 cpu-all.h          |    4 +++-
 darwin-user/main.c |    2 --
 exec.c             |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

cpu-all.h +3 -1

index 42a5fa0..e37ebfc 100644
@@ -20,6 +20,7 @@
 #define CPU_ALL_H
 
 #include "qemu-common.h"
+#include "qemu-tls.h"
 #include "cpu-common.h"
 
 /* some important defines:

@@ -334,7 +335,8 @@ void cpu_dump_statistics(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
 void QEMU_NORETURN cpu_abort(CPUState *env, const char *fmt, ...)
     GCC_FMT_ATTR(2, 3);
 extern CPUState *first_cpu;
-extern CPUState *cpu_single_env;
+DECLARE_TLS(CPUState *,tls_cpu_single_env);
+#define cpu_single_env get_tls(tls_cpu_single_env)
 
 /* Flags for use in ENV->INTERRUPT_PENDING.

darwin-user/main.c +0 -2

index 1a881a0..c0f14f8 100644
@@ -729,8 +729,6 @@ static void usage(void)
 
 /* XXX: currently only used for async signals (see signal.c) */
 CPUState *global_env;
-/* used only if single thread */
-CPUState *cpu_single_env = NULL;
 
 /* used to free thread contexts */
 TaskState *first_task_state;

exec.c +1 -1

index d0cbf15..afc5fe3 100644
@@ -120,7 +120,7 @@ static MemoryRegion *system_io;
 CPUState *first_cpu;
 /* current CPU in the current thread. It is only valid inside
    cpu_exec() */
-CPUState *cpu_single_env;
+DEFINE_TLS(CPUState *,tls_cpu_single_env);
 /* 0 = Do not count executed instructions.
    1 = Precise instruction counting.
    2 = Adaptive rate instruction counting.  */

Signature · 1 line

1.7.6

David Gilbert

author

2011-10-07 17:29 UTC #7 lore raw

Quoted 3 lines · Paolo Bonzini

If interested people can test the patches more and submit them more formally, I'd be very glad. I wrote it for RCU, but of course that one is not really going to be 1.0 material (even for 9p).

Hmm this got a bit more complex than the original patch; still it covers a lot more bases.

Should this also replace the THREAD that's defined in linux-user/qemu.h and bsd-user/qemu.h (that is __thread if built with NPTL)?
It seems to only be there for 'thread_env' which is also a CPUState* (hmm - what state does that contain that cpu_single_env doesn't?)

Dave

Paolo Bonzini

2011-10-08 13:42 UTC #8 lore raw

Quoted 8 lines · David Gilbert

Hmm this got a bit more complex than the original patch; still it covers a lot more bases.
Should this also replace the THREAD that's defined in linux-user/qemu.h and bsd-user/qemu.h (that is __thread if built with NPTL)?
It seems to only be there for 'thread_env' which is also a CPUState* (hmm - what state does that contain that cpu_single_env doesn't?)

Yes, I wouldn't be surprised if thread_env can just go. That would be a separate cleanup, however.

Paolo

Peter Maydell

2011-10-26 14:03 UTC #9 lore raw

In reply to Paolo Bonzini

On 5 October 2011 10:21, Paolo Bonzini <[email protected]> wrote:
[tls patches]

Quoted 3 lines

If interested people can test the patches more and submit them more formally, I'd be very glad. I wrote it for RCU, but of course that one is not really going to be 1.0 material (even for 9p).

For the record (since I think we only talked about this on IRC):
* the POSIX TLS fallback code doesn't work on Linux hosts for linux-user emulation (the constructor is never called to set up the TLS for the main thread, probably something to do with our custom linker script, since it does work OK for system emulation)
* if I recall correctly from IRC it doesn't compile on OpenBSD

so for 1.0 perhaps we need to fall back to a simpler set of patches that just avoid the regression in thread support for linux-user ?

-- PMM

Paolo Bonzini

2011-10-26 14:39 UTC #10 lore raw

Quoted 9 lines · Peter Maydell

For the record (since I think we only talked about this on IRC):
* the POSIX TLS fallback code doesn't work on Linux hosts for linux-user emulation (the constructor is never called to set up the TLS for the main thread, probably something to do with our custom linker script, since it does work OK for system emulation)
* if I recall correctly from IRC it doesn't compile on OpenBSD
so for 1.0 perhaps we need to fall back to a simpler set of patches that just avoid the regression in thread support for linux-user ?

I agree.

Paolo

Peter Maydell

2011-10-26 14:54 UTC #11 lore raw

Quoted 9 lines · Paolo Bonzini

Quoted 8 lines · Peter Maydell
For the record (since I think we only talked about this on IRC):
* the POSIX TLS fallback code doesn't work on Linux hosts for linux-user emulation (the constructor is never called to set up the TLS for the main thread, probably something to do with our custom linker script, since it does work OK for system emulation)
* if I recall correctly from IRC it doesn't compile on OpenBSD

Quoted 4 lines

Quoted 2 lines
so for 1.0 perhaps we need to fall back to a simpler set of patches that just avoid the regression in thread support for linux-user ?
I agree.

I was thinking something like a trivial qemu-tls.h which does

#ifdef __linux__
#define DECLARE_TLS(type, x) extern DEFINE_TLS(type, x) #define DEFINE_TLS(type, x) __thread __typeof__(type) tls__##x
#define get_tls(x) tls__##x
#else
/* Dummy implementations -- we can get away with this because system
* mode is effectively single-threaded for our current limited use of
* TLS, and the only -user mode which supports multiple threads is
* linux-user.
* TODO: proper implementations via Win32 .tls sections and
* POSIX pthread_getspecific.
*/
#define DECLARE_TLS(type, x) extern DEFINE_TLS(type, x) #define DEFINE_TLS(type, x) __typeof__(type) tls__##x
#define get_tls(x) tls__##x
#endif

and then we can just use your "Make cpu_single_env thread-local" patch as-is:

http://git.linaro.org/gitweb?p=people/pmaydell/qemu-arm.git;a=commitdiff;h=b674559fc8a67ae7d30fe3ab3062d5855ac77d2d

(for that matter we could apply the "Prepare Windows port for thread-local cpu_single_env" patch too, but that's not a requirement.)

-- PMM

Paolo Bonzini

2011-10-26 14:59 UTC #12 lore raw

Quoted 34 lines · Peter Maydell

Quoted 9 lines · Paolo Bonzini
Quoted 8 lines · Peter Maydell
For the record (since I think we only talked about this on IRC):
* the POSIX TLS fallback code doesn't work on Linux hosts for linux-user emulation (the constructor is never called to set up the TLS for the main thread, probably something to do with our custom linker script, since it does work OK for system emulation)
* if I recall correctly from IRC it doesn't compile on OpenBSD
Quoted 4 lines
Quoted 2 lines
so for 1.0 perhaps we need to fall back to a simpler set of patches that just avoid the regression in thread support for linux-user ?
I agree.
I was thinking something like a trivial qemu-tls.h which does
#ifdef __linux__
#define DECLARE_TLS(type, x) extern DEFINE_TLS(type, x) #define DEFINE_TLS(type, x) __thread __typeof__(type) tls__##x
#define get_tls(x) tls__##x
#else
/* Dummy implementations -- we can get away with this because system
* mode is effectively single-threaded for our current limited use of
* TLS, and the only -user mode which supports multiple threads is
* linux-user.
* TODO: proper implementations via Win32 .tls sections and
* POSIX pthread_getspecific.
*/
#define DECLARE_TLS(type, x) extern DEFINE_TLS(type, x) #define DEFINE_TLS(type, x) __typeof__(type) tls__##x
#define get_tls(x) tls__##x
#endif

Quoted 7 lines

and then we can just use your "Make cpu_single_env thread-local" patch as-is:
http://git.linaro.org/gitweb?p=people/pmaydell/qemu-arm.git;a=commitdiff;h=b674559fc8a67ae7d30fe3ab3062d5855ac77d2d
(for that matter we could apply the "Prepare Windows port for thread-local cpu_single_env" patch too, but that's not a requirement.)

That would be fine.

Paolo

Jan Kiszka

2011-10-26 15:03 UTC #13 lore raw

In reply to Peter Maydell

Quoted 17 lines · Peter Maydell

Quoted 9 lines · Paolo Bonzini
Quoted 8 lines · Peter Maydell
For the record (since I think we only talked about this on IRC):
* the POSIX TLS fallback code doesn't work on Linux hosts for linux-user emulation (the constructor is never called to set up the TLS for the main thread, probably something to do with our custom linker script, since it does work OK for system emulation)
* if I recall correctly from IRC it doesn't compile on OpenBSD
Quoted 4 lines
Quoted 2 lines
so for 1.0 perhaps we need to fall back to a simpler set of patches that just avoid the regression in thread support for linux-user ?
I agree.
I was thinking something like a trivial qemu-tls.h which does

Quoted 9 lines

#ifdef __linux__
#define DECLARE_TLS(type, x) extern DEFINE_TLS(type, x) #define DEFINE_TLS(type, x) __thread __typeof__(type) tls__##x
#define get_tls(x) tls__##x
#else
/* Dummy implementations -- we can get away with this because system
* mode is effectively single-threaded for our current limited use of
* TLS, and the only -user mode which supports multiple threads is
* linux-user.

We will use it in system mode as well, e.g. when I'll push away the global lock from KVM entry/exits. So let's state that it is only interesting on Linux hosts for now.

Otherwise, this looks good.

Jan

Quoted 17 lines

* TODO: proper implementations via Win32 .tls sections and
* POSIX pthread_getspecific.
*/
#define DECLARE_TLS(type, x) extern DEFINE_TLS(type, x) #define DEFINE_TLS(type, x) __typeof__(type) tls__##x
#define get_tls(x) tls__##x
#endif
and then we can just use your "Make cpu_single_env thread-local" patch as-is:
http://git.linaro.org/gitweb?p=people/pmaydell/qemu-arm.git;a=commitdiff;h=b674559fc8a67ae7d30fe3ab3062d5855ac77d2d
(for that matter we could apply the "Prepare Windows port for thread-local cpu_single_env" patch too, but that's not a requirement.)
-- PMM

Signature · 2 lines

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

Peter Maydell

2011-10-26 15:09 UTC #14 lore raw

Quoted 28 lines · Jan Kiszka

Quoted 17 lines · Peter Maydell
Quoted 9 lines · Paolo Bonzini
Quoted 8 lines · Peter Maydell
For the record (since I think we only talked about this on IRC):
* the POSIX TLS fallback code doesn't work on Linux hosts for linux-user emulation (the constructor is never called to set up the TLS for the main thread, probably something to do with our custom linker script, since it does work OK for system emulation)
* if I recall correctly from IRC it doesn't compile on OpenBSD
Quoted 4 lines
Quoted 2 lines
so for 1.0 perhaps we need to fall back to a simpler set of patches that just avoid the regression in thread support for linux-user ?
I agree.
I was thinking something like a trivial qemu-tls.h which does
Quoted 9 lines
#ifdef __linux__
#define DECLARE_TLS(type, x) extern DEFINE_TLS(type, x) #define DEFINE_TLS(type, x) __thread __typeof__(type) tls__##x #define get_tls(x) tls__##x
#else
/* Dummy implementations -- we can get away with this because system
* mode is effectively single-threaded for our current limited use of
* TLS, and the only -user mode which supports multiple threads is
* linux-user.

Quoted 5 lines

We will use it in system mode as well, e.g. when I'll push away the global lock from KVM entry/exits. So let's state that it is only interesting on Linux hosts for now.
Otherwise, this looks good.

The point of the comment is that you can't use this facility in system mode until we've added the non-Linux support (because you'd break Win32 &c). So fixing and committing the non-Linux support is a prerequisite for anything like the global lock moves. I can be a bit more explicit about that if you like:

/* Dummy implementations -- we can get away with this because system
* mode is effectively single-threaded for our current limited use of
* TLS, and the only -user mode which supports multiple threads is
* linux-user.
* This means you cannot use this for any variables which will
* actually be accessed by more than one thread in system mode
* until the implementations for Win32 and POSIX systems without
* __thread have been added!
*
* TODO: add implementations via Win32 .tls sections and
* POSIX pthread_getspecific.
*/

-- PMM

Jan Kiszka

2011-10-26 15:13 UTC #15 lore raw

Quoted 54 lines · Peter Maydell

Quoted 28 lines · Jan Kiszka
Quoted 17 lines · Peter Maydell
Quoted 9 lines · Paolo Bonzini
Quoted 8 lines · Peter Maydell
For the record (since I think we only talked about this on IRC):
* the POSIX TLS fallback code doesn't work on Linux hosts for linux-user emulation (the constructor is never called to set up the TLS for the main thread, probably something to do with our custom linker script, since it does work OK for system emulation)
* if I recall correctly from IRC it doesn't compile on OpenBSD
Quoted 4 lines
Quoted 2 lines
so for 1.0 perhaps we need to fall back to a simpler set of patches that just avoid the regression in thread support for linux-user ?
I agree.
I was thinking something like a trivial qemu-tls.h which does
Quoted 9 lines
#ifdef __linux__
#define DECLARE_TLS(type, x) extern DEFINE_TLS(type, x) #define DEFINE_TLS(type, x) __thread __typeof__(type) tls__##x
#define get_tls(x) tls__##x
#else
/* Dummy implementations -- we can get away with this because system
* mode is effectively single-threaded for our current limited use of
* TLS, and the only -user mode which supports multiple threads is
* linux-user.
Quoted 5 lines
We will use it in system mode as well, e.g. when I'll push away the global lock from KVM entry/exits. So let's state that it is only interesting on Linux hosts for now.
Otherwise, this looks good.
The point of the comment is that you can't use this facility in system mode until we've added the non-Linux support (because you'd break Win32 &c). So fixing and committing the non-Linux support is a prerequisite for anything like the global lock moves. I can be a bit more explicit about that if you like:
/* Dummy implementations -- we can get away with this because system
* mode is effectively single-threaded for our current limited use of
* TLS, and the only -user mode which supports multiple threads is
* linux-user.
* This means you cannot use this for any variables which will
* actually be accessed by more than one thread in system mode
* until the implementations for Win32 and POSIX systems without
* __thread have been added!
*
* TODO: add implementations via Win32 .tls sections and
* POSIX pthread_getspecific.
*/

Quoted 1 line

-- PMM

My point is that it is fine to use for per-vcpu variables because:
- they are single-threaded in TCG mode
- they are multi-threaded in KVM mode, but that's only affecting Linux hosts for which this TLS wrapper is already usable

Jan

Signature · 2 lines

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

Peter Maydell

2011-10-26 15:19 UTC #16 lore raw

Quoted 4 lines · Jan Kiszka

My point is that it is fine to use for per-vcpu variables because:
- they are single-threaded in TCG mode
- they are multi-threaded in KVM mode, but that's only affecting Linux hosts for which this TLS wrapper is already usable

Oh, I see. Feel free to suggest reworded comment text :-)

-- PMM

Jan Kiszka

2011-10-26 16:03 UTC #17 lore raw

Quoted 7 lines · Peter Maydell

Quoted 4 lines · Jan Kiszka
My point is that it is fine to use for per-vcpu variables because:
- they are single-threaded in TCG mode
- they are multi-threaded in KVM mode, but that's only affecting Linux hosts for which this TLS wrapper is already usable
Oh, I see. Feel free to suggest reworded comment text :-)

/* Dummy implementations -- we can get away with this because

 *  - we confine the use to per-VCPU variables
 *  - only linux-user supports multiple VCPU threads
 *  - TCG system mode is single-threaded regarding VCPUs
 *  - KVM system mode is multi-threaded but limited to Linux

OK?

Jan

Signature · 2 lines

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

Andreas Färber

2011-10-26 16:27 UTC #18 lore raw

Am 26.10.2011 18:02, schrieb Jan Kiszka:

Quoted 7 lines · Peter Maydell

Quoted 6 lines · Jan Kiszka
Quoted 4 lines
My point is that it is fine to use for per-vcpu variables because:
- they are single-threaded in TCG mode
- they are multi-threaded in KVM mode, but that's only affecting Linux hosts for which this TLS wrapper is already usable

Quoted 8 lines

Quoted 1 line
Oh, I see. Feel free to suggest reworded comment text :-)
/* Dummy implementations -- we can get away with this because
 *  - we confine the use to per-VCPU variables
 *  - only linux-user supports multiple VCPU threads
 *  - TCG system mode is single-threaded regarding VCPUs
* - KVM system mode is multi-threaded but limited to Linux

There was a port of KVM to illumos-derived SmartOS. But I don't see an easy way to error out on non-Linux KVM hosts without breaking TCG, too.

Andreas

Signature · 2 lines

SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Jan Kiszka

2011-10-26 16:31 UTC #19 lore raw

Quoted 15 lines · Andreas Färber

Am 26.10.2011 18:02, schrieb Jan Kiszka:
Quoted 13 lines · Peter Maydell
Quoted 7 lines · Jan Kiszka
Quoted 4 lines
My point is that it is fine to use for per-vcpu variables because:
- they are single-threaded in TCG mode
- they are multi-threaded in KVM mode, but that's only affecting Linux hosts for which this TLS wrapper is already usable
Oh, I see. Feel free to suggest reworded comment text :-)
/* Dummy implementations -- we can get away with this because
 *  - we confine the use to per-VCPU variables
 *  - only linux-user supports multiple VCPU threads
 *  - TCG system mode is single-threaded regarding VCPUs

Quoted 4 lines

Quoted 1 line
* - KVM system mode is multi-threaded but limited to Linux
There was a port of KVM to illumos-derived SmartOS. But I don't see an easy way to error out on non-Linux KVM hosts without breaking TCG, too.

And I would be surprised to see them running with zero QEMU patches. So they are not upstream, thus slightly outside our radar.

Jan

Signature · 2 lines

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