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

Re: [Minios-devel] [UNIKRAFT PATCH v4 5/8] plat: Add global struct to keep x86 CPU information



Hi,

looks good to me.

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

BR, Yuri.

Florian Schmidt <florian.schmidt@xxxxxxxxx> writes:

> Currently, all information relates to the additional registers that can
> be available on x86 CPUs, and how to save and restore them.
>
> For Arm, there is only a dummy function so far that can be filled later
> when we support extended registers on Arm.
>
> Signed-off-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>
> ---
>  plat/common/include/arm/arm64/cpu.h |  4 +++
>  plat/common/include/x86/cpu.h       | 56 ++++++++++++++++++++++++++---
>  plat/common/x86/cpu_features.c      | 37 +++++++++++++++++++
>  plat/kvm/Makefile.uk                |  1 +
>  plat/kvm/x86/setup.c                |  2 ++
>  plat/linuxu/Makefile.uk             |  1 +
>  plat/linuxu/setup.c                 |  5 +++
>  plat/xen/Makefile.uk                |  1 +
>  plat/xen/x86/setup.c                |  2 ++
>  9 files changed, 104 insertions(+), 5 deletions(-)
>  create mode 100644 plat/common/x86/cpu_features.c
>
> diff --git a/plat/common/include/arm/arm64/cpu.h 
> b/plat/common/include/arm/arm64/cpu.h
> index bfea4617..503bc68b 100644
> --- a/plat/common/include/arm/arm64/cpu.h
> +++ b/plat/common/include/arm/arm64/cpu.h
> @@ -74,6 +74,10 @@ static inline void ioreg_write64(volatile uint64_t *addr, 
> uint64_t value)
>       *addr = value;
>  }
>  
> +static inline void _init_cpufeatures(void)
> +{
> +}
> +
>  /* Define compatibility IO macros */
>  #define outb(addr, v)   UK_BUG()
>  #define outw(addr, v)   UK_BUG()
> diff --git a/plat/common/include/x86/cpu.h b/plat/common/include/x86/cpu.h
> index 001e9cac..6de62dc0 100644
> --- a/plat/common/include/x86/cpu.h
> +++ b/plat/common/include/x86/cpu.h
> @@ -31,16 +31,62 @@
>  #define __PLAT_COMMON_X86_CPU_H__
>  
>  #include <uk/arch/types.h>
> +#include <x86/cpu_defs.h>
> +#include <stdint.h>
>  
>  void halt(void);
>  void system_off(void);
>  
> -static inline void cpuid(__u32 leaf, __u32 *eax, __u32 *ebx,
> -             __u32 *ecx, __u32 *edx)
> +enum save_cmd {
> +     X86_SAVE_NONE,
> +     X86_SAVE_FSAVE,
> +     X86_SAVE_FXSAVE,
> +     X86_SAVE_XSAVE,
> +     X86_SAVE_XSAVEOPT
> +};
> +
> +struct _x86_features {
> +     unsigned long extregs_size;     /* Size of the extregs area */
> +     unsigned long extregs_align;    /* Alignment of the extregs area */
> +     enum save_cmd save;             /* which CPU instruction to use for
> +                                      * saving/restoring extregs.
> +                                      */
> +};
> +
> +extern struct _x86_features x86_cpu_features;
> +
> +static inline void _init_cpufeatures(void)
>  {
> -     asm volatile("cpuid"
> -                  : "=a"(*eax), "=b"(*ebx), "=c"(*ecx), "=d"(*edx)
> -                  : "0"(leaf));
> +     __u32 eax, ebx, ecx, edx;
> +
> +     /* Why are we saving the eax register content to the eax variable with
> +      * "=a(eax)", but then never use it?
> +      * Because gcc otherwise will assume that the eax register still
> +      * contains "1" after this asm expression. See the "Warning" note at
> +      * https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#InputOperands
> +      */
> +     asm volatile("cpuid" : "=a"(eax), "=c"(ecx), "=d"(edx) : "a"(1)
> +                     : "ebx");
> +     if (ecx & X86_CPUID1_ECX_OSXSAVE) {
> +             asm volatile("cpuid" : "=a"(eax), "=c"(ecx) : "a"(0xd), "c"(1)
> +                             : "ebx", "edx");
> +             if (eax & X86_CPUIDD1_EAX_XSAVEOPT)
> +                     x86_cpu_features.save = X86_SAVE_XSAVEOPT;
> +             else
> +                     x86_cpu_features.save = X86_SAVE_XSAVE;
> +             asm volatile("cpuid" : "=a"(eax), "=b"(ebx), "=c"(ecx)
> +                             : "a"(0xd), "c"(0) : "edx");
> +             x86_cpu_features.extregs_size = ebx;
> +             x86_cpu_features.extregs_align = 64;
> +     } else if (edx & X86_CPUID1_EDX_FXSR) {
> +             x86_cpu_features.save = X86_SAVE_FXSAVE;
> +             x86_cpu_features.extregs_size = 512;
> +             x86_cpu_features.extregs_align = 16;
> +     } else {
> +             x86_cpu_features.save = X86_SAVE_FSAVE;
> +             x86_cpu_features.extregs_size = 108;
> +             x86_cpu_features.extregs_align = 1;
> +     }
>  }
>  
>  unsigned long read_cr2(void);
> diff --git a/plat/common/x86/cpu_features.c b/plat/common/x86/cpu_features.c
> new file mode 100644
> index 00000000..07097397
> --- /dev/null
> +++ b/plat/common/x86/cpu_features.c
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: BSD-3-Clause */
> +/*
> + * Authors: Florian Schmidt <florian.schmidt@xxxxxxxxx>
> + *
> + * Copyright (c) 2018, 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.
> + */
> +
> +#include <x86/cpu.h>
> +
> +struct _x86_features x86_cpu_features;
> diff --git a/plat/kvm/Makefile.uk b/plat/kvm/Makefile.uk
> index 72dd8a30..5fb56ee9 100644
> --- a/plat/kvm/Makefile.uk
> +++ b/plat/kvm/Makefile.uk
> @@ -27,6 +27,7 @@ LIBKVMPLAT_CXXFLAGS            += -DKVMPLAT
>  ##
>  LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
> $(UK_PLAT_COMMON_BASE)/x86/trace.c|common
>  LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
> $(UK_PLAT_COMMON_BASE)/x86/traps.c|common
> +LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
> $(UK_PLAT_COMMON_BASE)/x86/cpu_features.c|common
>  LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
> $(UK_PLAT_COMMON_BASE)/x86/cpu_native.c|common
>  ifeq ($(CONFIG_HAVE_SCHED),y)
>  LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
> $(UK_PLAT_COMMON_BASE)/x86/thread_start.S|common
> diff --git a/plat/kvm/x86/setup.c b/plat/kvm/x86/setup.c
> index 47a78dcf..c17a7dd5 100644
> --- a/plat/kvm/x86/setup.c
> +++ b/plat/kvm/x86/setup.c
> @@ -27,6 +27,7 @@
>   */
>  
>  #include <string.h>
> +#include <x86/cpu.h>
>  #include <x86/traps.h>
>  #include <kvm/console.h>
>  #include <kvm/intctrl.h>
> @@ -118,6 +119,7 @@ void _libkvmplat_entry(void *arg)
>  {
>       struct multiboot_info *mi = (struct multiboot_info *)arg;
>  
> +     _init_cpufeatures();
>       _libkvmplat_init_console();
>       traps_init();
>       intctrl_init();
> diff --git a/plat/linuxu/Makefile.uk b/plat/linuxu/Makefile.uk
> index e70b4b7a..2c0de76c 100644
> --- a/plat/linuxu/Makefile.uk
> +++ b/plat/linuxu/Makefile.uk
> @@ -20,6 +20,7 @@ LIBLINUXUPLAT_ASFLAGS             += -DLINUXUPLAT
>  LIBLINUXUPLAT_CFLAGS              += -DLINUXUPLAT
>  LIBLINUXUPLAT_CXXFLAGS            += -DLINUXUPLAT
>  
> +LIBLINUXUPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
> $(UK_PLAT_COMMON_BASE)/x86/cpu_features.c|common
>  LIBLINUXUPLAT_SRCS-$(CONFIG_ARCH_X86_32) += 
> $(LIBLINUXUPLAT_BASE)/x86/entry32.S
>  LIBLINUXUPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
> $(LIBLINUXUPLAT_BASE)/x86/entry64.S
>  LIBLINUXUPLAT_SRCS-$(CONFIG_ARCH_ARM_32) += 
> $(LIBLINUXUPLAT_BASE)/arm/entry32.S
> diff --git a/plat/linuxu/setup.c b/plat/linuxu/setup.c
> index 5fbf54b1..7849f682 100644
> --- a/plat/linuxu/setup.c
> +++ b/plat/linuxu/setup.c
> @@ -45,6 +45,9 @@
>  #include <uk/plat/bootstrap.h>
>  #include <uk/assert.h>
>  #include <uk/errptr.h>
> +#if defined __X86_64__
> +#include <x86/cpu.h>
> +#endif
>  
>  struct liblinuxuplat_opts _liblinuxuplat_opts = { 0 };
>  
> @@ -150,6 +153,8 @@ void _liblinuxuplat_entry(int argc, char *argv[])
>       int ret;
>       void *pret;
>  
> +     _init_cpufeatures();
> +
>       /*
>        * Initialize platform console
>        */
> diff --git a/plat/xen/Makefile.uk b/plat/xen/Makefile.uk
> index 20d1e5af..38b510ad 100644
> --- a/plat/xen/Makefile.uk
> +++ b/plat/xen/Makefile.uk
> @@ -33,6 +33,7 @@ LIBXENPLAT_SRCS-y              += 
> $(UK_PLAT_COMMON_BASE)/memory.c|common
>  
>  LIBXENPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
> $(UK_PLAT_COMMON_BASE)/x86/trace.c|common
>  LIBXENPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
> $(UK_PLAT_COMMON_BASE)/x86/traps.c|common
> +LIBXENPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
> $(UK_PLAT_COMMON_BASE)/x86/cpu_features.c|common
>  ifeq ($(CONFIG_HAVE_SCHED),y)
>  LIBXENPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
> $(UK_PLAT_COMMON_BASE)/x86/thread_start.S|common
>  LIBXENPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
> $(UK_PLAT_COMMON_BASE)/thread.c|common
> diff --git a/plat/xen/x86/setup.c b/plat/xen/x86/setup.c
> index a41d5cb3..60a9f9e6 100644
> --- a/plat/xen/x86/setup.c
> +++ b/plat/xen/x86/setup.c
> @@ -74,6 +74,7 @@
>  #include <uk/plat/config.h>
>  #include <uk/plat/console.h>
>  #include <uk/plat/bootstrap.h>
> +#include <x86/cpu.h>
>  
>  #include <xen/xen.h>
>  #include <common/console.h>
> @@ -170,6 +171,7 @@ void _libxenplat_x86entry(void *start_info) __noreturn;
>  void _libxenplat_x86entry(void *start_info)
>  {
>       _init_traps();
> +     _init_cpufeatures();
>       HYPERVISOR_start_info = (start_info_t *)start_info;
>       _libxenplat_prepare_console(); /* enables buffering for console */
>  
> -- 
> 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

 


Rackspace

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