|
[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 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 .
> @@ -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.
> +
> +#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__ */
>
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |