Re: [Qemu-devel] [PATCH 05/30] exec: do not use qemu/tls.h

[Peter Maydell]

On 28 June 2013 19:26, Paolo Bonzini <address@hidden> wrote:
> The next patch will change qemu/tls.h to support more platforms, but at
> some performance cost.  Declare cpu_single_env directly instead of using
> the tls.h abstractions.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  exec.c                 | 10 ++++++++--
>  include/exec/cpu-all.h | 14 +++++++++++---
>  include/qemu/tls.h     | 52 
> --------------------------------------------------
>  3 files changed, 19 insertions(+), 57 deletions(-)
>  delete mode 100644 include/qemu/tls.h
>
> diff --git a/exec.c b/exec.c
> index d28403b..a788981 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -70,9 +70,15 @@ static MemoryRegion io_mem_unassigned;
>  #endif
>
>  CPUArchState *first_cpu;
> +
>  /* current CPU in the current thread. It is only valid inside
> -   cpu_exec() */
> -DEFINE_TLS(CPUArchState *,cpu_single_env);
> + * cpu_exec().  See comment in include/exec/cpu-all.h.  */
> +#if defined CONFIG_KVM || (defined CONFIG_USER_ONLY && defined 
> CONFIG_USE_NPTL)
> +__thread CPUArchState *cpu_single_env;
> +#else
> +CPUArchState *cpu_single_env;
> +#endif

I don't like having the semantics of this variable differ
depending on whether CONFIG_KVM was defined. In particular
this means that the variable is per-thread if you're running
TCG on a QEMU that was configured with KVM support, but
not per-thread if you're running TCG on a QEMU that was
configured without per-thread support. That's just bizarre
and a recipe for confusion and for bugs creeping in in the
less-well-tested config combinations.

We should just be consistent and always make this be
per-thread.

thanks
-- PMM