[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCH 3/4] plat: switch thread-local storage area on context switch



Hi Costin,


On 4/16/19 3:22 PM, Costin Lupu wrote:
Hi Florian,

Please see my comments inline.

On 4/15/19 1:03 PM, Florian Schmidt wrote:
On x86, the standard way is writing to an MSR to switch the fs register,
which is used as the pointer to the TLS area. However, this is a
privileged instruction, so for the linuxu platform, use a system call
provided by Linux that does exactly that instead.

Signed-off-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>
---
  arch/x86/x86_64/include/uk/asm/thread.h     |  5 +++
  include/uk/plat/thread.h                    |  8 ++--
  lib/uksched/thread.c                        |  5 ++-
  plat/common/include/sw_ctx.h                |  1 +
  plat/common/include/thread_switch.h         | 43 +++++++++++++++++++
  plat/common/include/x86/thread_switch.h     | 41 ++++++++++++++++++
  plat/common/sw_ctx.c                        | 10 ++++-
  plat/linuxu/include/linuxu/syscall-x86_64.h |  1 +
  plat/linuxu/include/linuxu/syscall.h        |  7 ++++
  plat/linuxu/include/thread_switch.h         | 46 +++++++++++++++++++++
  10 files changed, 160 insertions(+), 7 deletions(-)
  create mode 100644 plat/common/include/thread_switch.h
  create mode 100644 plat/common/include/x86/thread_switch.h
  create mode 100644 plat/linuxu/include/thread_switch.h

diff --git a/arch/x86/x86_64/include/uk/asm/thread.h 
b/arch/x86/x86_64/include/uk/asm/thread.h
index 6e95848d..f07eb521 100644
--- a/arch/x86/x86_64/include/uk/asm/thread.h
+++ b/arch/x86/x86_64/include/uk/asm/thread.h
@@ -62,3 +62,8 @@ static inline void tls_copy(void *tls_area)
        memset(tls_area+tls_data_len, 0, tls_bss_len);
        *((uintptr_t *)(tls_area+tls_len)) = (uintptr_t)(tls_area+tls_len);
  }
+
+static inline void *tls_pointer(void *tls_area)
+{
+       return tls_area + (_tls_end - _tls_start);
+}
diff --git a/include/uk/plat/thread.h b/include/uk/plat/thread.h
index 69fc5e28..4b349ea7 100644
--- a/include/uk/plat/thread.h
+++ b/include/uk/plat/thread.h
@@ -51,7 +51,8 @@ enum ukplat_ctx_type {
  struct uk_alloc;
typedef void *(*ukplat_ctx_create_func_t)
-               (struct uk_alloc *allocator, unsigned long sp);
+               (struct uk_alloc *allocator, unsigned long sp,
+                unsigned long tlsp);
  typedef void  (*ukplat_ctx_start_func_t)
                (void *ctx);
  typedef void  (*ukplat_ctx_switch_func_t)
@@ -72,12 +73,13 @@ int ukplat_ctx_callbacks_init(struct ukplat_ctx_callbacks 
*ctx_cbs,
static inline
  void *ukplat_thread_ctx_create(struct ukplat_ctx_callbacks *cbs,
-               struct uk_alloc *allocator, unsigned long sp)
+               struct uk_alloc *allocator, unsigned long sp,
+               unsigned long tlsp)
  {
        UK_ASSERT(cbs != NULL);
        UK_ASSERT(allocator != NULL);
- return cbs->create_cb(allocator, sp);
+       return cbs->create_cb(allocator, sp, tlsp);
  }
void ukplat_thread_ctx_destroy(struct uk_alloc *allocator, void *ctx);
diff --git a/lib/uksched/thread.c b/lib/uksched/thread.c
index a97fad9e..45f79206 100644
--- a/lib/uksched/thread.c
+++ b/lib/uksched/thread.c
@@ -39,7 +39,7 @@
  #include <uk/wait.h>
  #include <uk/print.h>
  #include <uk/assert.h>
-
+#include <uk/arch/thread.h>
/* Pushes the specified value onto the stack of the specified thread */
  static void stack_push(unsigned long *sp, unsigned long value)
@@ -103,7 +103,8 @@ int uk_thread_init(struct uk_thread *thread,
        init_sp(&sp, stack, function, arg);
/* Call platform specific setup. */
-       thread->ctx = ukplat_thread_ctx_create(cbs, allocator, sp);
+       thread->ctx = ukplat_thread_ctx_create(cbs, allocator, sp,
+                       (uintptr_t)tls_pointer(tls));
        if (thread->ctx == NULL)
                return -1;
diff --git a/plat/common/include/sw_ctx.h b/plat/common/include/sw_ctx.h
index d52fe65b..366c6a8f 100644
--- a/plat/common/include/sw_ctx.h
+++ b/plat/common/include/sw_ctx.h
@@ -41,6 +41,7 @@
  struct sw_ctx {
        unsigned long sp;       /* Stack pointer */
        unsigned long ip;       /* Instruction pointer */
+       unsigned long tlsp;     /* thread-local storage pointer */
        uintptr_t extregs;      /* Pointer to an area to which extended
                                 * registers are saved on context switch.
                                 */
diff --git a/plat/common/include/thread_switch.h 
b/plat/common/include/thread_switch.h
new file mode 100644
index 00000000..b04c38c5
--- /dev/null
+++ b/plat/common/include/thread_switch.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Florian Schmidt <florian.schmidt@xxxxxxxxx>
+ *
+ * Copyright (c) 2019, NEC Europe Ltd., NEC Corporation. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ *    contributors may be used to endorse or promote products derived from
+ *    this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
+ */
+#ifndef __PLAT_CMN_THREAD_SWITCH_H__
+#define __PLAT_CMN_THREAD_SWITCH_H__
+
+#if defined(__X86_64__)
+#include <x86/thread_switch.h>
+#else
+#error "Add thread_switch.h for current architecture."
+#endif
+
+#endif /* __PLAT_CMN_THREAD_SWITCH_H__ */
diff --git a/plat/common/include/x86/thread_switch.h 
b/plat/common/include/x86/thread_switch.h
new file mode 100644
index 00000000..f8252916
--- /dev/null
+++ b/plat/common/include/x86/thread_switch.h

For this file the location is correct, according to my comments for the
previous patch. However, I would use the same header for both sets of
changes and rename it to tls.h .

I'll make it consistent with the file in the previous patch.



@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Florian Schmidt <florian.schmidt@xxxxxxxxx>
+ *
+ * Copyright (c) 2019, NEC Europe Ltd., NEC Corporation. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ *    contributors may be used to endorse or promote products derived from
+ *    this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
+ */
+#ifndef __PLAT_CMN_X86_THREAD_SWITCH_H__
+#define __PLAT_CMN_X86_THREAD_SWITCH_H__
+
+#define MSR_FS_BASE 0xc0000100

This is a CPU related macro, please move it to
plat/common/include/x86/cpu_defs.h where all the CPU related macros are
defined.

OK, I'll do that (and maybe change the name according to the scheme in that file).



+
+#define set_tls_pointer(ptr) wrmsrl(MSR_FS_BASE, ptr)
+
+#endif /* __PLAT_CMN_X86_THREAD_SWITCH_H__ */
diff --git a/plat/common/sw_ctx.c b/plat/common/sw_ctx.c
index 06795244..088d1233 100644
--- a/plat/common/sw_ctx.c
+++ b/plat/common/sw_ctx.c
@@ -39,9 +39,11 @@
  #include <uk/alloc.h>
  #include <sw_ctx.h>
  #include <uk/assert.h>
+#include <thread_switch.h>
  #include <x86/cpu.h>
-static void *sw_ctx_create(struct uk_alloc *allocator, unsigned long sp);
+static void *sw_ctx_create(struct uk_alloc *allocator, unsigned long sp,
+                               unsigned long tlsp);
  static void  sw_ctx_start(void *ctx) __noreturn;
  static void  sw_ctx_switch(void *prevctx, void *nextctx);
@@ -51,7 +53,8 @@ static void sw_ctx_switch(void *prevctx, void *nextctx);
   */
  extern void asm_thread_starter(void);
-static void *sw_ctx_create(struct uk_alloc *allocator, unsigned long sp)
+static void *sw_ctx_create(struct uk_alloc *allocator, unsigned long sp,
+                               unsigned long tlsp)
  {
        struct sw_ctx *ctx;
        size_t sz;
@@ -68,6 +71,7 @@ static void *sw_ctx_create(struct uk_alloc *allocator, 
unsigned long sp)
        }
ctx->sp = sp;
+       ctx->tlsp = tlsp;
        ctx->ip = (unsigned long) asm_thread_starter;
        ctx->extregs = ALIGN_UP(((uintptr_t)ctx + sizeof(struct sw_ctx)),
                                x86_cpu_features.extregs_align);
@@ -86,6 +90,7 @@ static void sw_ctx_start(void *ctx)
UK_ASSERT(sw_ctx != NULL); + set_tls_pointer(sw_ctx->tlsp);
        /* Switch stacks and run the thread */
        asm_ctx_start(sw_ctx->sp, sw_ctx->ip);
@@ -101,6 +106,7 @@ static void sw_ctx_switch(void *prevctx, void *nextctx) save_extregs(p);
        restore_extregs(n);
+       set_tls_pointer(n->tlsp);
        asm_sw_ctx_switch(prevctx, nextctx);
  }
diff --git a/plat/linuxu/include/linuxu/syscall-x86_64.h b/plat/linuxu/include/linuxu/syscall-x86_64.h
index 26820dcd..553f0ba4 100644
--- a/plat/linuxu/include/linuxu/syscall-x86_64.h
+++ b/plat/linuxu/include/linuxu/syscall-x86_64.h
@@ -48,6 +48,7 @@
  #define __SC_RT_SIGPROCMASK 14
  #define __SC_IOCTL  16
  #define __SC_EXIT   60
+#define __SC_ARCH_PRCTL       158
  #define __SC_TIMER_CREATE     222
  #define __SC_TIMER_SETTIME    223
  #define __SC_TIMER_GETTIME    224
diff --git a/plat/linuxu/include/linuxu/syscall.h 
b/plat/linuxu/include/linuxu/syscall.h
index d378d265..d5b66698 100644
--- a/plat/linuxu/include/linuxu/syscall.h
+++ b/plat/linuxu/include/linuxu/syscall.h
@@ -123,6 +123,13 @@ static inline int sys_sigprocmask(int how,
                              sizeof(k_sigset_t));
  }
+static inline int sys_arch_prctl(int code, unsigned long addr)
+{
+       return (int) syscall2(__SC_ARCH_PRCTL,
+                             (long) code,
+                             (long) addr);
+}
+
  static inline int sys_pselect6(int nfds,
                k_fd_set *readfds, k_fd_set *writefds, k_fd_set *exceptfds,
                const struct k_timespec *timeout, const void *sigmask)
diff --git a/plat/linuxu/include/thread_switch.h 
b/plat/linuxu/include/thread_switch.h
new file mode 100644
index 00000000..3a0f962f
--- /dev/null
+++ b/plat/linuxu/include/thread_switch.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Florian Schmidt <florian.schmidt@xxxxxxxxx>
+ *
+ * Copyright (c) 2019, NEC Europe Ltd., NEC Corporation. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ *    contributors may be used to endorse or promote products derived from
+ *    this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
+ */
+#ifndef __PLAT_THREAD_SWITCH_H__
+#define __PLAT_THREAD_SWITCH_H__
+
+#include <linuxu/syscall.h>
+
+#define ARCH_SET_FS 0x1002
+
+static inline void set_tls_pointer(unsigned long arg)
+{
+       sys_arch_prctl(ARCH_SET_FS, arg);
+}
+
+#endif /* __PLAT_THREAD_SWITCH_H__ */


--
Dr. Florian Schmidt
フローリアン・シュミット
Research Scientist,
Systems and Machine Learning Group
NEC Laboratories Europe
Kurfürsten-Anlage 36, D-69115 Heidelberg
Tel.     +49 (0)6221 4342-265
Fax:     +49 (0)6221 4342-155
e-mail:  florian.schmidt@xxxxxxxxx
============================================================
Registered at Amtsgericht Mannheim, Germany, HRB728558

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.