Remove vcoreid from get/set_tls_desc
authorKevin Klues <klueska@cs.berkeley.edu>
Sat, 18 Jul 2015 15:56:37 +0000 (08:56 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Wed, 22 Jul 2015 15:08:33 +0000 (11:08 -0400)
The presence of vcoreid as a parameter to these functions is remnants of
the old i386 way of using the LDT for TLS management.  Moreover, we only
really passed vcoreid in the first place because we were trying to avoid
a call to sys_getvcoreid() (a very antiquated way of getting the vcore
id).  These were the days before we standardized on *always* having
__vcoreid available in everyone's TLS (including uthreads).

Nowadays passing this parameter is completely unnecessary (and actually
meaningless on x86_64 and riscv). It is actually just ignored if you
pass any value in at all. Removing it for these systems is completely
benign. For i386 (which we don't even really support anymore), it is
sufficient to just call vcore_id() internally and use that to index into
the proper ldt.  In the (near) future, we should consider removing i386
support completely. I leave it here now for reference.

This change has been possible for quite some time, but was finally
motivated by the desire to get the tls_desc of the main thread in an
early SCP before any notion of vcores or vcore management has been set
up. In a future commit, we will use this to temporarily save off the
main thread's tls_desc during vcore initialization itself.
It would probably have been OK to just assume vcoreid 0 for the main
thread (especially since passing it on x86_64 was completely benign),
but it is an unecessary parameter to begin with, and better to just
clean things up the right way as we move forward.

[brho: added a void to a func definition ]

12 files changed:
tests/microb_test.c
tests/old/eth_audio.c
tests/old/msr_get_cores.c
tests/old/msr_get_singlecore.c
tools/compilers/gcc-glibc/glibc-2.19-akaros/sysdeps/akaros/i386/tls.h
tools/compilers/gcc-glibc/glibc-2.19-akaros/sysdeps/akaros/init.c
tools/compilers/gcc-glibc/glibc-2.19-akaros/sysdeps/akaros/riscv/tls.h
tools/compilers/gcc-glibc/glibc-2.19-akaros/sysdeps/akaros/sys/vcore-tls.h
tools/compilers/gcc-glibc/glibc-2.19-akaros/sysdeps/akaros/vcore-tls.c
tools/compilers/gcc-glibc/glibc-2.19-akaros/sysdeps/akaros/x86_64/tls.h
user/parlib/include/vcore.h
user/parlib/uthread.c

index a0222b5..b2dac1d 100644 (file)
@@ -57,7 +57,7 @@ void set_tlsdesc_test(unsigned long nr_loops)
 {
 #ifdef __i386__
        uint32_t vcoreid = vcore_id();
-       void *mytls = get_tls_desc(vcoreid);
+       void *mytls = get_tls_desc();
        void *vctls = get_vcpd_tls_desc(vcoreid);
        segdesc_t tmp = SEG(STA_W, (uint32_t)vctls, 0xffffffff, 3);
        uint32_t gs = (vcoreid << 3) | 0x07;
@@ -66,7 +66,7 @@ void set_tlsdesc_test(unsigned long nr_loops)
                cmb();
                asm volatile("movl %0,%%gs" : : "r" (gs) : "memory");
     }
-       set_tls_desc(mytls, vcoreid);
+       set_tls_desc(mytls);
 #endif
 }
 
index 7a42950..0447bde 100644 (file)
@@ -58,7 +58,7 @@ int main()
 
        /* Need to save this somewhere that you can find it again when restarting
         * core0 */
-       core0_tls = get_tls_desc(0);
+       core0_tls = get_tls_desc();
        /* Need to save our floating point state somewhere (like in the
         * user_thread_tcb so it can be restarted too */
 
@@ -118,7 +118,7 @@ void vcore_entry(void)
         * entry for this vcore to point to the TCB of the new user-thread. */
        if (vcoreid == 0) {
                handle_events(vcoreid);
-               set_tls_desc(core0_tls, 0);
+               set_tls_desc(core0_tls);
                assert(__vcoreid == 0); /* in case anyone uses this */
                /* Load silly state (Floating point) too */
                pop_user_ctx(&vcpd->uthread_ctx, vcoreid);
index 4e18566..70a853c 100644 (file)
@@ -50,7 +50,7 @@ int main(int argc, char** argv)
        #endif
        /* Need to save this somewhere that you can find it again when restarting
         * core0 */
-       core0_tls = get_tls_desc(0);
+       core0_tls = get_tls_desc();
        /* Need to save our floating point state somewhere (like in the
         * user_thread_tcb so it can be restarted too */
 /* end: stuff userspace needs to do before switching to multi-mode */
@@ -95,7 +95,7 @@ void vcore_entry(void)
         * entry for this vcore to point to the TCB of the new user-thread. */
        if (vcoreid == 0) {
                handle_events(vcoreid);
-               set_tls_desc(core0_tls, 0);
+               set_tls_desc(core0_tls);
                assert(__vcoreid == 0); /* in case anyone uses this */
                /* Load silly state (Floating point) too */
                pop_user_ctx(&vcpd->uthread_ctx, vcoreid);
index 3d0f6ae..9fe4c8a 100644 (file)
@@ -43,7 +43,7 @@ int main(int argc, char** argv)
        #endif
        /* Need to save this somewhere that you can find it again when restarting
         * core0 */
-       core0_tls = get_tls_desc(0);
+       core0_tls = get_tls_desc();
        /* Need to save our floating point state somewhere (like in the
         * user_thread_tcb so it can be restarted too */
 /* end: stuff userspace needs to do before switching to multi-mode */
@@ -96,7 +96,7 @@ void vcore_entry(void)
         * entry for this vcore to point to the TCB of the new user-thread. */
        if (vcoreid == 0) {
                handle_events(vcoreid);
-               set_tls_desc(core0_tls, 0);
+               set_tls_desc(core0_tls);
                assert(__vcoreid == 0); /* in case anyone uses this */
                /* Load silly state (Floating point) too */
                pop_user_ctx(&vcpd->uthread_ctx, vcoreid);
index 8af07c4..65f3776 100644 (file)
@@ -429,16 +429,18 @@ union user_desc_init
  * rely on it too much.  The intent of it is vcoreid is the caller's vcoreid,
  * and that vcoreid might be in the TLS of the caller (it will be for transition
  * stacks) and we could avoid a trap on x86 to sys_getvcoreid(). */
-static inline void *__get_tls_desc(uint32_t vcoreid)
+static inline void *__get_tls_desc(void)
 {
+       int vcoreid = vcore_id();
        return (void*)(__procdata.ldt[vcoreid].sd_base_31_24 << 24 |
                       __procdata.ldt[vcoreid].sd_base_23_16 << 16 |
                       __procdata.ldt[vcoreid].sd_base_15_0);
 }
 
 /* passing in the vcoreid, since it'll be in TLS of the caller */
-static inline void __set_tls_desc(void *tls_desc, uint32_t vcoreid)
+static inline void __set_tls_desc(void *tls_desc)
 {
+       int vcoreid = vcore_id();
        /* Keep this technique in sync with sysdeps/akaros/i386/tls.h */
        segdesc_t tmp = SEG(STA_W, (uint32_t)tls_desc, 0xffffffff, 3);
        __procdata.ldt[vcoreid] = tmp;
index fe064a0..7637342 100644 (file)
@@ -53,8 +53,7 @@ void __libc_vcore_entry(int id)
                 * AFAIK, riscv's TLS changes are really cheap, and they don't do it in
                 * the kernel (yet/ever), so they can set their TLS here too. */
                #ifndef __x86_64__
-               set_tls_desc((void*)__procdata.vcore_preempt_data[id].vcore_tls_desc,
-                            id);
+               set_tls_desc((void*)__procdata.vcore_preempt_data[id].vcore_tls_desc);
                #endif
                /* These TLS setters actually only need to happen once, at init time */
                __vcoreid = id;
index 2f2777d..4d281bd 100644 (file)
@@ -164,12 +164,12 @@ typedef struct rthread {
 #define THREAD_GSCOPE_WAIT() \
   GL(dl_wait_lookup_done) ()
 
-static inline void *__get_tls_desc(uint32_t vcoreid)
+static inline void *__get_tls_desc(void)
 {
        return READ_THREAD_POINTER() - TLS_TCB_OFFSET;
 }
 
-static inline void __set_tls_desc(void *tls_desc, uint32_t vcoreid)
+static inline void __set_tls_desc(void *tls_desc)
 {
        TLS_INIT_TP(tls_desc, 0);
 }
index 3ba2671..c74909f 100644 (file)
@@ -11,8 +11,8 @@
 extern "C" {
 #endif
 
-void set_tls_desc(void* addr, int vcoreid);
-void *get_tls_desc(int vcoreid);
+void set_tls_desc(void* addr);
+void *get_tls_desc(void);
 
 void *allocate_tls(void);
 void free_tls(void *tcb);
index abca6b3..96725cc 100644 (file)
@@ -2,14 +2,14 @@
 #include <parlib/vcore.h>
 #include <ldsodefs.h>
 
-void set_tls_desc(void* addr, int vcoreid)
+void set_tls_desc(void* addr)
 {
-       __set_tls_desc(addr, vcoreid);
+       __set_tls_desc(addr);
 }
 
-void *get_tls_desc(int vcoreid)
+void *get_tls_desc(void)
 {
-       return __get_tls_desc(vcoreid);
+       return __get_tls_desc();
 }
 
 /* Get a TLS, returns 0 on failure.  Vcores have their own TLS, and any thread
index 23c8eff..2afe19c 100644 (file)
@@ -404,13 +404,13 @@ extern void _dl_x86_64_restore_sse (void);
   } while (0)
 # endif
 
-static inline void *__get_tls_desc(uint32_t vcoreid)
+static inline void *__get_tls_desc(void)
 {
        /* the tcb->self pointer is set to the TLS base address */
        return THREAD_SELF;
 }
 
-static inline void __set_tls_desc(void *tls_desc, uint32_t vcoreid)
+static inline void __set_tls_desc(void *tls_desc)
 {
        /* TODO: check for and use WRFSBASE */
        __fastcall_setfsbase((uintptr_t)tls_desc);
@@ -424,7 +424,7 @@ static inline const char* tls_init_tp(void* thrdescr)
   head->tcb = thrdescr;
   head->self = thrdescr;
 
-  __set_tls_desc(thrdescr, 0x00dead00);        /* we ignore vcoreid, pass gibberish */
+  __set_tls_desc(thrdescr);
   return NULL;
 }
 
index 59b14b8..59a1562 100644 (file)
@@ -70,7 +70,7 @@ bool check_vcoreid(const char *str, uint32_t vcoreid);
 #define get_tlsvar_linaddr(_vcoreid, _var)                                     \
 ({                                                                             \
        uintptr_t vc_tls_desc = (uintptr_t)get_vcpd_tls_desc(_vcoreid);            \
-       uintptr_t var_off = (uintptr_t)&_var - (uintptr_t)get_tls_desc(vcore_id());\
+       uintptr_t var_off = (uintptr_t)&_var - (uintptr_t)get_tls_desc();          \
        (typeof(_var) *)(vc_tls_desc + var_off);                                   \
 })
 
@@ -265,13 +265,13 @@ static inline uint64_t vcore_account_uptime_nsec(uint32_t vcoreid)
        } else { /* vcore context */                                               \
                vcoreid = vcore_id();                                                  \
        }                                                                          \
-       temp_tls_desc = get_tls_desc(vcoreid);                                     \
-       set_tls_desc(tls_desc, vcoreid);                                           \
+       temp_tls_desc = get_tls_desc();                                            \
+       set_tls_desc(tls_desc);                                                    \
        begin_safe_access_tls_vars();
 
 #define end_access_tls_vars()                                                  \
        end_safe_access_tls_vars();                                                \
-       set_tls_desc(temp_tls_desc, vcoreid);                                      \
+       set_tls_desc(temp_tls_desc);                                               \
        if (!invcore) {                                                            \
                /* Note we reenable migration before enabling notifs, which is reverse
                 * from how we disabled notifs.  We must enabling migration before
index d5baeb6..7e11bda 100644 (file)
@@ -48,7 +48,7 @@ static void uthread_manage_thread0(struct uthread *uthread)
 {
        assert(uthread);
        /* Save a pointer to thread0's tls region (the glibc one) into its tcb */
-       uthread->tls_desc = get_tls_desc(0);
+       uthread->tls_desc = get_tls_desc();
        /* Save a pointer to the uthread in its own TLS */
        current_uthread = uthread;
        /* Thread is currently running (it is 'us') */
@@ -63,7 +63,7 @@ static void uthread_manage_thread0(struct uthread *uthread)
         * issue is that vcore0's transition-TLS isn't TLS_INITed yet.  Until it is
         * (right before vcore_entry(), don't try and take the address of any of
         * its TLS vars. */
-       set_tls_desc(get_vcpd_tls_desc(0), 0);
+       set_tls_desc(get_vcpd_tls_desc(0));
        begin_safe_access_tls_vars();
        /* We might have a basic uthread already installed (from lib_init), so
         * free it before installing the new one. */
@@ -78,7 +78,7 @@ static void uthread_manage_thread0(struct uthread *uthread)
         * vcore_context when running in scheduler_context for the SCP. */
        __vcore_context = TRUE;
        end_safe_access_tls_vars();
-       set_tls_desc(uthread->tls_desc, 0);
+       set_tls_desc(uthread->tls_desc);
        begin_safe_access_tls_vars();
        __vcoreid = 0;  /* setting the uthread's TLS var */
        assert(!in_vcore_context());
@@ -393,7 +393,7 @@ void uthread_yield(bool save_state, void (*yield_func)(struct uthread*, void*),
        }
        /* Change to the transition context (both TLS (if applicable) and stack). */
        if (__uthread_has_tls(uthread)) {
-               set_tls_desc(get_vcpd_tls_desc(vcoreid), vcoreid);
+               set_tls_desc(get_vcpd_tls_desc(vcoreid));
                begin_safe_access_tls_vars();
                assert(current_uthread == uthread);
                /* If this assert fails, see the note in uthread_manage_thread0 */
@@ -513,7 +513,7 @@ void highjack_current_uthread(struct uthread *uthread)
        /* and make sure we are using the correct TLS for the new uthread */
        if (__uthread_has_tls(uthread)) {
                assert(uthread->tls_desc);
-               set_tls_desc(uthread->tls_desc, vcoreid);
+               set_tls_desc(uthread->tls_desc);
                begin_safe_access_tls_vars();
                __vcoreid = vcoreid;    /* setting the uthread's TLS var */
                end_safe_access_tls_vars();
@@ -526,7 +526,7 @@ void highjack_current_uthread(struct uthread *uthread)
 static void set_uthread_tls(struct uthread *uthread, uint32_t vcoreid)
 {
        if (__uthread_has_tls(uthread)) {
-               set_tls_desc(uthread->tls_desc, vcoreid);
+               set_tls_desc(uthread->tls_desc);
                begin_safe_access_tls_vars();
                __vcoreid = vcoreid;    /* setting the uthread's TLS var */
                end_safe_access_tls_vars();