Re: [Minios-devel] [UNIKRAFT PATCH v4 2/7] plat/common: Implement a few extra registers stub helpers on arm64

[Santiago Pagani <Santiago.Pagani@xxxxxxxxx>]

Hi Justin,

Thanks for the patch. It looks good.

Best,
Santiago

On 09.10.19, 15:30, "Jia He" <justin.he@xxxxxxx> wrote:

    On arm64, we don't need the extra registers during context switching so
    far. This patch decouple the arch specific structures/functions info arch
    related files.
    
    Without this patch, it will report compilation error as follows:
    plat/common/include/x86/cpu.h/:: In function 'save_extregs':
    plat/common/include/x86/cpu.h:72:3: error: impossible constraint in 'asm'
       asm volatile("xsave (%0)" :: "r"(ctx->extregs),
       ^~~
    plat/common/include/x86/cpu.h:76:3: error: impossible constraint in 'asm'
       asm volatile("xsaveopt (%0)" :: "r"(ctx->extregs),
    
    Signed-off-by: Jia He <justin.he@xxxxxxx>
    ---
     plat/common/include/arm/arm64/cpu.h | 28 ++++++++++++++++++++++++++++
     plat/common/include/x86/cpu.h       | 24 ++++++++++++++++++++++++
     plat/common/sw_ctx.c                | 16 ++++------------
     3 files changed, 56 insertions(+), 12 deletions(-)
    
    diff --git a/plat/common/include/arm/arm64/cpu.h 
b/plat/common/include/arm/arm64/cpu.h
    index 1495192..23adf8d 100644
    --- a/plat/common/include/arm/arm64/cpu.h
    +++ b/plat/common/include/arm/arm64/cpu.h
    @@ -33,6 +33,10 @@
      */
     
     #include <inttypes.h>
    +#include <uk/essentials.h>
    +#include <sw_ctx.h>
    +#include <uk/alloc.h>
    +#include <uk/assert.h>
     
     /* Define macros to access IO registers */
     #define __IOREG_READ(bits) \
    @@ -108,3 +112,27 @@ int32_t smcc_psci_smc_call(uint32_t, uint64_t, 
uint64_t, uint64_t);
     void halt(void);
     void reset(void);
     void system_off(void);
    +
    +static inline void save_extregs(struct sw_ctx *ctx __unused)
    +{
    +}
    +
    +static inline void restore_extregs(struct sw_ctx *ctx __unused)
    +{
    +}
    +
    +static inline struct sw_ctx *arch_alloc_sw_ctx(struct uk_alloc *allocator)
    +{
    +   struct sw_ctx *ctx;
    +
    +   ctx = uk_malloc(allocator, sizeof(struct sw_ctx));
    +   uk_pr_debug("Allocating %lu bytes for sw ctx at %p\n",
    +              sizeof(struct sw_ctx), ctx);
    +
    +   return ctx;
    +}
    +
    +static inline void arch_init_extregs(struct sw_ctx *ctx)
    +{
    +   ctx->extregs = (uintptr_t)ctx + sizeof(struct sw_ctx);
    +}
    diff --git a/plat/common/include/x86/cpu.h b/plat/common/include/x86/cpu.h
    index 8acd71e..d2cf53a 100644
    --- a/plat/common/include/x86/cpu.h
    +++ b/plat/common/include/x86/cpu.h
    @@ -34,6 +34,9 @@
     #include <x86/cpu_defs.h>
     #include <sw_ctx.h>
     #include <stdint.h>
    +#include <uk/assert.h>
    +#include <uk/alloc.h>
    +#include <string.h>
     
     void halt(void);
     void system_off(void);
    @@ -98,6 +101,27 @@ static inline void restore_extregs(struct sw_ctx *ctx)
        }
     }
     
    +static inline struct sw_ctx *arch_alloc_sw_ctx(struct uk_alloc *allocator)
    +{
    +   struct sw_ctx *ctx;
    +   size_t sz;
    +
    +   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);
    +
    +   return ctx;
    +}
    +
    +static inline void arch_init_extregs(struct sw_ctx *ctx)
    +{
    +   ctx->extregs = ALIGN_UP(((uintptr_t)ctx + sizeof(struct sw_ctx)),
    +                           x86_cpu_features.extregs_align);
    +   // Initialize extregs area: zero out, then save a valid layout to it.
    +   memset((void *)ctx->extregs, 0, x86_cpu_features.extregs_size);
    +}
    +
     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 88a377f..2c78abe 100644
    --- a/plat/common/sw_ctx.c
    +++ b/plat/common/sw_ctx.c
    @@ -34,13 +34,11 @@
     
     #include <stdint.h>
     #include <stdlib.h>
    -#include <string.h>
     #include <uk/plat/thread.h>
    -#include <uk/alloc.h>
     #include <sw_ctx.h>
     #include <uk/assert.h>
     #include <tls.h>
    -#include <x86/cpu.h>
    +#include <cpu.h>
     
     static void *sw_ctx_create(struct uk_alloc *allocator, unsigned long sp,
                                unsigned long tlsp);
    @@ -57,14 +55,10 @@ static void *sw_ctx_create(struct uk_alloc *allocator, 
unsigned long sp,
                                unsigned long tlsp)
     {
        struct sw_ctx *ctx;
    -   size_t sz;
     
        UK_ASSERT(allocator != NULL);
     
    -   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);
    +   ctx = arch_alloc_sw_ctx(allocator);
        if (ctx == NULL) {
                uk_pr_warn("Error allocating software context.");
                return NULL;
    @@ -73,10 +67,8 @@ 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);
    -   // Initialize extregs area: zero out, then save a valid layout to it.
    -   memset((void *)ctx->extregs, 0, x86_cpu_features.extregs_size);
    +   arch_init_extregs(ctx);
    +
        save_extregs(ctx);
     
        return ctx;
    -- 
    2.17.1
    
    

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