On Mon, Oct 25, 2021 at 10:19:04AM -0700, Richard Henderson wrote:
> On 10/25/21 7:07 AM, Stefan Hajnoczi wrote:
> > Compiler optimizations can cache TLS values across coroutine yield
> > points, resulting in stale values from the previous thread when a
> > coroutine is re-entered by a new thread.
> ...
> >   include/qemu/tls.h | 142 +++++++++++++++++++++++++++++++++++++++++++++
> 
> Better as qemu/coroutine-tls.h, since it is needed for no other purpose.
> 
> > +#define QEMU_DEFINE_TLS(type, var) \
> > +    __thread type qemu_tls_##var; \
> > +    type get_##var(void) { return qemu_tls_##var; } \
> > +    void set_##var(type v) { qemu_tls_##var = v; }
> 
> You might as well make the variable static, since it may only be referenced
> by these two functions.

Oops, that's a bug. It should be declared extern. QEMU_DEFINE_TLS() is
meant for use in header files.

> 
> > +#define QEMU_DEFINE_STATIC_TLS(type, var) \
> > +    static __thread type qemu_tls_##var; \
> > +    static __attribute__((noinline)) type get_##var(void); \
> > +    static type get_##var(void) { return qemu_tls_##var; } \
> > +    static __attribute__((noinline)) void set_##var(type v); \
> > +    static void set_##var(type v) { qemu_tls_##var = v; }
> 
> You don't need separate function declarations; you can fold them together.
> 
> If would be nice to inline this when possible,
> 
> #if defined(__aarch64__)
> #define QEMU_COROUTINE_TLS_ADDR(RET, VAR)                       \
>     asm volatile("mrs %0, tpidr_el0\n\t"                        \
>                  "add %0, %0, #:tprel_hi12:"#VAR", lsl #12\n\t" \
>                  "add %0, %0, #:tprel_lo12_nc:"#VAR             \
>                  : "=r"(RET))
> #elif defined(__powerpc64__)
> #define QEMU_COROUTINE_TLS_ADDR(RET, VAR)                       \
>     asm volatile("addis %0,13,"#VAR"@tprel@ha\n\t"              \
>                  "add   %0,%0,"#VAR"@tprel@l"                   \
>                  : "=r"(RET))
> #elif defined(__riscv)
> #define QEMU_COROUTINE_TLS_ADDR(RET, VAR)                       \
>     asm volatile("lui  %0,%%tprel_hi("#VAR")\n\t"               \
>                  "add  %0,%0,%%tprel_add("#VAR")\n\t"           \
>                  "addi %0,%0,%%tprel_lo("#VAR")"                \
>                  : "=r"(RET))
> #elif defined(__x86_64__)
> #define QEMU_COROUTINE_TLS_ADDR(RET, VAR)                       \
>     asm volatile("mov %%fs:"#VAR"@tpoff, %0" : "=r"(RET))
> #endif
> 
> #ifdef QEMU_COROUTINE_TLS_ADDR
> #define QEMU_COROUTINE_TLS_DECLARE(TYPE, VAR)                   \
>     extern __thread TYPE co_tls_##VAR;                          \
>     static inline TYPE get_##VAR(void)                          \
>     { TYPE *p; QEMU_COROUTINE_TLS_ADDR(p, co_tls_##VAR); return *p; } \
>     static inline void set_##VAR(TYPE v)                        \
>     { TYPE *p; QEMU_COROUTINE_TLS_ADDR(p, co_tls_##VAR); *p = v; }
> #else
>     etc
> #endif

Nice. That makes me wonder if it's possible to write a portable version:

  static inline TYPE get_##VAR(void) \
  { volatile TYPE *p = &co_tls_##VAR; return *p; }

But the trouble is we need &co_tls_##VAR to be "volatile" and I don't
think there is a way to express that?

Stefan