|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT PATCH v3 5/8] plat: Add global struct to keep x86 CPU information
Hi,
looks good to me. Just small note inline
Reviewed-by: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
Cheers, 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.
>
> Signed-off-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>
> ---
> 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 | 7 +++++
> plat/xen/Makefile.uk | 1 +
> plat/xen/x86/setup.c | 2 ++
> 8 files changed, 102 insertions(+), 5 deletions(-)
> create mode 100644 plat/common/x86/cpu_features.c
>
> 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..c6b910fa 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,10 @@ void _liblinuxuplat_entry(int argc, char *argv[])
> int ret;
> void *pret;
>
> +#if defined __X86_64__
> + _init_cpufeatures();
> +#endif
Would it make sense to add a stub-function _init_cpufeatures for arm?
> +
> /*
> * 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
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |