On 07.06.19 18:12, Ralf Ramsauer wrote:
On 6/7/19 5:52 PM, Jan Kiszka wrote:
On 07.06.19 17:26, Ralf Ramsauer wrote:
On 5/31/19 8:39 PM, Ralf Ramsauer wrote:
On 5/31/19 12:04 PM, Jan Kiszka wrote:
On 31.05.19 17:52, Ralf Ramsauer wrote:
On 5/30/19 3:00 AM, Jan Kiszka wrote:
On 21.05.19 16:34, Ralf Ramsauer wrote:
Recent gcc versions emit SSE instructions for 32-bit inmates
(e.g., in
hex2str or cmdline_parse routines). Inmates aren't able to execute
those
instructions as SSE is not enabled and will crash.
Enabling SSE is the same code for 32 and 64 bit x86 and straight
forward: Lookup SSE availability via cpuid and enable OSFXSR in
cr4. If
SSE is not available, stop the inmate.
If AVX is available, activate it (XCR0).
Lookup features that need no explicit activation.
Signed-off-by: Ralf Ramsauer <[email protected]>
---
inmates/lib/x86/Makefile | 6 +-
inmates/lib/x86/cpu-features.c | 103
+++++++++++++++++++++++++++++
inmates/lib/x86/header-32.S | 9 ++-
inmates/lib/x86/header-64.S | 9 ++-
inmates/lib/x86/include/asm/regs.h | 36 ++++++++++
5 files changed, 158 insertions(+), 5 deletions(-)
create mode 100644 inmates/lib/x86/cpu-features.c
diff --git a/inmates/lib/x86/Makefile b/inmates/lib/x86/Makefile
index e474ffd0..ed3b04d5 100644
--- a/inmates/lib/x86/Makefile
+++ b/inmates/lib/x86/Makefile
@@ -40,7 +40,7 @@ include $(INMATES_LIB)/Makefile.lib
always := lib.a lib32.a
-TARGETS := header-common.o ioapic.o printk.o setup.o smp.o uart.o
+TARGETS := cpu-features.o header-common.o ioapic.o printk.o setup.o
smp.o uart.o
TARGETS += ../alloc.o ../pci.o ../string.o ../cmdline.o
../setup.o
TARGETS += ../uart-8250.o ../printk.o
TARGETS_32_ONLY := header-32.o
@@ -57,6 +57,10 @@ $(obj)/lib32.a: $(addprefix $(obj)/,$(lib32-y))
targets += header-32.o
+# Code of this object is called before SSE/AVX is available. Ensure
that the
+# compiler won't generate unsupported instructions for this file.
+CFLAGS_cpu-features.o += -mno-sse
+
$(obj)/%-32.o: c_flags += -m32
$(obj)/%-32.o: $(src)/%.c
$(call if_changed_rule,cc_o_c)
diff --git a/inmates/lib/x86/cpu-features.c
b/inmates/lib/x86/cpu-features.c
new file mode 100644
index 00000000..9cf98543
--- /dev/null
+++ b/inmates/lib/x86/cpu-features.c
@@ -0,0 +1,103 @@
+/*
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) OTH Regensburg, 2019
+ *
+ * Authors:
+ * Ralf Ramsauer <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version
2. See
+ * the COPYING file in the top-level directory.
+ *
+ * Alternatively, you can use or redistribute this file under the
following
+ * BSD license:
+ *
+ * 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.
+ *
+ * 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.
+ */
+
+#include <inmate.h>
+#include <asm/regs.h>
+
+/* Must only be called from assembler via jmp */
+void arch_init_features(void);
+
+struct x86_cpu_features x86_cpu_features
__attribute__((section(".data")));
+
+/*
+ * We arrive here very early, and we don't have a stack. Take care.
+ *
+ * Every booting CPU will call this function. We make the
assumption
that all
+ * CPUs have the same feature set. So we don't need any locks when
writing to
+ * x86_cpu_features.
Any comments on this? Is it okay that this code will run on each CPU,
even if only required on the primary CPU?
Running once on all CPUs is harmless - we are not in a hurry.
Running without stack is a problem, see below.
Depends. It's perfectly fine to have no stack if we don't use it (even
if we had an corrupt stack…).
+ */
+void __attribute__((naked, noreturn, section(".boot")))
What's "naked" supposed to do here? The compilers says it will ignore
it.
Huh? Does it produce a warning?
Yes, gcc 7.4 said so.
naked ensures that there's no pro- and epilogue sequences. And as the
comment states: We won't have a stack that early.
BTW: That raises a question: Where do we set the stack for secondary
CPUs on x86?
In header.S, always to stack_top... Looks like the ARM issue is still
present here.
I remember that I once fixed some corrupted stack things for ARM
inmates, but it was a bit different there.
Trying to interpret your words: We're currently running on an
overlapping stack for secondary (all) CPUs, right? Should probably be
fixed first.
Hmm. On x86 we're hlting all secondary CPUs very early and wait for a
SIPI. Besides setting ap_entry, we could allocate a valid stack on the
primary CPU inside smp_start_cpu() and pass it.
MMIO stuff is done, let's get back to this topic. :-)
Yeah, right... I was close to pushing "--no-sse" today when I recalled
that you are on this ;)
The problem that I initially wanted to solve was missing SSE support,
turns out secondary CPUs in x86 have a broken stack.
There might be a clean solution to solve all issues, including the dirty
"naked jmp around"-hack:
Let's reconsider the early boot phase of x86 inmates. In comparison to
ARM, for example, we start all CPUs but halt secondary CPUs and wait for
SIPIs.
Now, we could provide a stack symbol that points to a free, valid stack
that a CPU can grab. Initially, (by compile time) the symbol holds the
address of the stack for the primary CPU. It's yet unclear who will win
the race to be the primary CPU.
We could cmpxchg the content of this symbol *very* early. Whoever wins
is the primary CPU, everyone else will read zero and simply put the CPU
in hlt state.
That's what we do today already: Only the first CPU gets the static
stack, the rest enter a hlt loop until their inmate-triggered wakeup
without touching that stack. No problem here.
Hmm. We go through start16 start32 and start64 until we reach the point
where we check the CPU ID, and setup the stack. And everything we do
during this path (lgdtl, cr0/4 setup, …) will be executed twice: On the
initial startup, and on the SIPI.
We do NOT set up the stack: if the CPU is no the first one, we stop:
cmp $0,%edi
jne stop
Yes, we do the initialization twice - but please do not optimize the slow-path
here.
When we receive the SIPI, can't we simply assume that everything but the
stack is already set up and in place? So all we would have to do is
setup the stack, get the entry point and call it. Or am I missing
something here?
INIT/SIPI implies CPU reset on x86 -> back to real-mode etc.
When starting secondary CPUs, smp_start_cpu() alloc()s a new stack from
the heap, sets the stack symbol (and sets ap_entry) and kicks the CPU.
Now the same startup code will run, but the secondary CPU grabs the
stack and boots.
Right, we need to make "mov $stack_top,%rsp" configurable, and the
problem should be solved. For the first CPU, it can remain that static one.
This has some advantages:
- We have a valid stack very early, and can jump to C back and forth
with regular calls instead of the hacky hack above.
- Startup code will only run once when needed, and not multiple times
as it is the case at the moment
Startup should happen via INIT/SIPI, so some bits have to be run again.
Ok, maybe I lack some architectural knowledge: in which state do we
arrive when we receive the SIPI? And why is it written twice?
INIT does the reset, SIPI defines the startup address and kicks off. Calling it
twice is due to some timing things on real HW - IIRC. We just copied the common
pattern.
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
--
You received this message because you are subscribed to the Google Groups
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/jailhouse-dev/23803b48-aef5-781d-b6de-6fdf0ad17dcf%40siemens.com.
For more options, visit https://groups.google.com/d/optout.
