|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT PATCH v2 8/9] plat/common: Add functionality to save and restore extended (x86) registers
Hi,
see my notes inline
-Yuri.
Florian Schmidt <florian.schmidt@xxxxxxxxx> writes:
> On creation of a sw_ctx struct, allocate an area sufficiently large to
> save all extended registers. On context switch, use the appropriate CPU
> instruction to save and restore those registers to/from that area.
>
> Signed-off-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>
> ---
> plat/common/include/sw_ctx.h | 9 +++++--
> plat/common/include/x86/cpu.h | 49 +++++++++++++++++++++++++++++++++++
> plat/common/sw_ctx.c | 14 ++++++++--
> 3 files changed, 68 insertions(+), 4 deletions(-)
>
> diff --git a/plat/common/include/sw_ctx.h b/plat/common/include/sw_ctx.h
> index fae96beb..1b279b25 100644
> --- a/plat/common/include/sw_ctx.h
> +++ b/plat/common/include/sw_ctx.h
> @@ -38,8 +38,13 @@
> #include <uk/plat/thread.h>
>
> struct sw_ctx {
> - unsigned long sp; /* Stack pointer */
> - unsigned long ip; /* Instruction pointer */
> + unsigned long sp; /* Stack pointer */
> + unsigned long ip; /* Instruction pointer */
> + unsigned char extregs[]; /* NB, this will typically NOT point to
> + * the beginning of the extregs area,
> + * because the extregs area needs to
> + * be aligned.
> + */
You can easily achieve that extregs always pointing to the begining of
extregs area. If extregs is a pointer, you can just assign it the
address of this area. E.g.
ctx = uk_malloc(allocator, sz);
ctx.extregs = ALIGN_UP(((uintptr_t)ctx + sizeof(struct sw_ctx)),
x86_cpu_features.extregs_align);
> };
>
> void sw_ctx_callbacks_init(struct ukplat_ctx_callbacks *ctx_cbs);
> diff --git a/plat/common/include/x86/cpu.h b/plat/common/include/x86/cpu.h
> index fbc229d9..f2a8f0d5 100644
> --- a/plat/common/include/x86/cpu.h
> +++ b/plat/common/include/x86/cpu.h
> @@ -32,6 +32,7 @@
>
> #include <uk/arch/types.h>
> #include <x86/cpu_defs.h>
> +#include <sw_ctx.h>
> #include <stdint.h>
>
> void halt(void);
> @@ -55,6 +56,54 @@ struct _x86_features {
>
> extern struct _x86_features x86_cpu_features;
>
> +static inline void save_extregs(struct sw_ctx *ctx)
> +{
> + uintptr_t r = ALIGN_UP(((uintptr_t)ctx + sizeof(struct sw_ctx)),
> + x86_cpu_features.extregs_align);
> +
> + switch (x86_cpu_features.save) {
> + case X86_SAVE_NONE:
> + /* nothing to do */
> + break;
> + case X86_SAVE_FSAVE:
> + asm volatile("fsave (%0)" :: "r"(r) : "memory");
> + break;
> + case X86_SAVE_FXSAVE:
> + asm volatile("fxsave (%0)" :: "r"(r) : "memory");
> + break;
> + case X86_SAVE_XSAVE:
> + asm volatile("xsave (%0)" :: "r"(r), "a"(0xffffffff),
> + "d"(0xffffffff) : "memory");
> + break;
> + case X86_SAVE_XSAVEOPT:
> + asm volatile("xsaveopt (%0)" :: "r"(r), "a"(0xffffffff),
> + "d"(0xffffffff) : "memory");
> + break;
> + }
> +}
> +static inline void restore_extregs(struct sw_ctx *ctx)
> +{
> + uintptr_t r = ALIGN_UP(((uintptr_t)ctx + sizeof(struct sw_ctx)),
> + x86_cpu_features.extregs_align);
> +
> + switch (x86_cpu_features.save) {
> + case X86_SAVE_NONE:
> + /* nothing to do */
> + break;
> + case X86_SAVE_FSAVE:
> + asm volatile("frstor (%0)" :: "r"(r));
> + break;
> + case X86_SAVE_FXSAVE:
> + asm volatile("fxrstor (%0)" :: "r"(r));
> + break;
> + case X86_SAVE_XSAVE:
> + case X86_SAVE_XSAVEOPT:
> + asm volatile("xrstor (%0)" :: "r"(r), "a"(0xffffffff),
> + "d"(0xffffffff));
> + break;
> + }
> +}
> +
> static inline void _init_cpufeatures(void)
> {
> #if LINUXUPLAT
> diff --git a/plat/common/sw_ctx.c b/plat/common/sw_ctx.c
> index a477753b..79935776 100644
> --- a/plat/common/sw_ctx.c
> +++ b/plat/common/sw_ctx.c
> @@ -37,7 +37,7 @@
> #include <uk/alloc.h>
> #include <sw_ctx.h>
> #include <uk/assert.h>
> -
> +#include <x86/cpu.h>
>
> static void *sw_ctx_create(struct uk_alloc *allocator, unsigned long sp);
> static void sw_ctx_start(void *ctx) __noreturn;
> @@ -52,10 +52,14 @@ extern void asm_thread_starter(void);
> static void *sw_ctx_create(struct uk_alloc *allocator, unsigned long sp)
> {
> struct sw_ctx *ctx;
> + size_t sz;
>
> UK_ASSERT(allocator != NULL);
>
> - ctx = uk_malloc(allocator, sizeof(struct sw_ctx));
> + sz = ALIGN_UP(sizeof(struct sw_ctx), x86_cpu_features.extregs_align)
> + + x86_cpu_features.extregs_size;
> + ctx = uk_malloc(allocator, sz);
> + uk_pr_debug("Allocating %lu bytes for sw ctx at %p\n", sz, ctx);
> if (ctx == NULL) {
> uk_pr_warn("Error allocating software context.");
> return NULL;
> @@ -63,6 +67,7 @@ static void *sw_ctx_create(struct uk_alloc *allocator,
> unsigned long sp)
>
> ctx->sp = sp;
> ctx->ip = (unsigned long) asm_thread_starter;
> + save_extregs(ctx);
Maybe a small note why we are saving registers without actual context
switch here? So people like me get less confusion :)
>
> return ctx;
> }
> @@ -85,6 +90,11 @@ extern void asm_sw_ctx_switch(void *prevctx, void
> *nextctx);
>
> static void sw_ctx_switch(void *prevctx, void *nextctx)
> {
> + struct sw_ctx *p = prevctx;
> + struct sw_ctx *n = nextctx;
> +
> + save_extregs(p);
> + restore_extregs(n);
> asm_sw_ctx_switch(prevctx, nextctx);
> }
>
> --
> 2.19.2
>
--
Yuri Volchkov
Software Specialist
NEC Europe Ltd
Kurfürsten-Anlage 36
D-69115 Heidelberg
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |