Re: [Minios-devel] [UNIKRAFT PATCH v3 6/8] plat/common: Add functionality to save and restore extended (x86) registers

[Yuri Volchkov <yuri.volchkov@xxxxxxxxx>]

Reviewed-by: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>

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  |  8 +++++--
>  plat/common/include/x86/cpu.h | 43 +++++++++++++++++++++++++++++++++++
>  plat/common/sw_ctx.c          | 18 +++++++++++++--
>  3 files changed, 65 insertions(+), 4 deletions(-)
>
> diff --git a/plat/common/include/sw_ctx.h b/plat/common/include/sw_ctx.h
> index fae96beb..d52fe65b 100644
> --- a/plat/common/include/sw_ctx.h
> +++ b/plat/common/include/sw_ctx.h
> @@ -35,11 +35,15 @@
>  #define __PLAT_CMN_SW_CTX_H__
>  
>  #ifndef __ASSEMBLY__
> +#include <stdint.h>
>  #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 */
> +     uintptr_t extregs;      /* Pointer to an area to which extended
> +                              * registers are saved on context switch.
> +                              */
>  };
>  
>  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 6de62dc0..8acd71e4 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,48 @@ struct _x86_features {
>  
>  extern struct _x86_features x86_cpu_features;
>  
> +static inline void save_extregs(struct sw_ctx *ctx)
> +{
> +     switch (x86_cpu_features.save) {
> +     case X86_SAVE_NONE:
> +             /* nothing to do */
> +             break;
> +     case X86_SAVE_FSAVE:
> +             asm volatile("fsave (%0)" :: "r"(ctx->extregs) : "memory");
> +             break;
> +     case X86_SAVE_FXSAVE:
> +             asm volatile("fxsave (%0)" :: "r"(ctx->extregs) : "memory");
> +             break;
> +     case X86_SAVE_XSAVE:
> +             asm volatile("xsave (%0)" :: "r"(ctx->extregs),
> +                             "a"(0xffffffff), "d"(0xffffffff) : "memory");
> +             break;
> +     case X86_SAVE_XSAVEOPT:
> +             asm volatile("xsaveopt (%0)" :: "r"(ctx->extregs),
> +                             "a"(0xffffffff), "d"(0xffffffff) : "memory");
> +             break;
> +     }
> +}
> +static inline void restore_extregs(struct sw_ctx *ctx)
> +{
> +     switch (x86_cpu_features.save) {
> +     case X86_SAVE_NONE:
> +             /* nothing to do */
> +             break;
> +     case X86_SAVE_FSAVE:
> +             asm volatile("frstor (%0)" :: "r"(ctx->extregs));
> +             break;
> +     case X86_SAVE_FXSAVE:
> +             asm volatile("fxrstor (%0)" :: "r"(ctx->extregs));
> +             break;
> +     case X86_SAVE_XSAVE:
> +     case X86_SAVE_XSAVEOPT:
> +             asm volatile("xrstor (%0)" :: "r"(ctx->extregs),
> +                             "a"(0xffffffff), "d"(0xffffffff));
> +             break;
> +     }
> +}
> +
>  static inline void _init_cpufeatures(void)
>  {
>       __u32 eax, ebx, ecx, edx;
> diff --git a/plat/common/sw_ctx.c b/plat/common/sw_ctx.c
> index a477753b..c69f5fde 100644
> --- a/plat/common/sw_ctx.c
> +++ b/plat/common/sw_ctx.c
> @@ -32,12 +32,13 @@
>   * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
>   */
>  
> +#include <stdint.h>
>  #include <stdlib.h>
>  #include <uk/plat/thread.h>
>  #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 +53,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 +68,10 @@ static void *sw_ctx_create(struct uk_alloc *allocator, 
> unsigned long sp)
>  
>       ctx->sp = sp;
>       ctx->ip = (unsigned long) asm_thread_starter;
> +     ctx->extregs = ALIGN_UP(((uintptr_t)ctx + sizeof(struct sw_ctx)),
> +                             x86_cpu_features.extregs_align);
> +     // Initialize the extregs area by saving a valid register layout to it
> +     save_extregs(ctx);
>  
>       return ctx;
>  }
> @@ -85,6 +94,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.20.1
>

-- 
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