[Minios-devel] [UNIKRAFT PATCH v2] plat/kvm: Fix current thread retrieval in interrupt context on x86_64

[Costin Lupu <costin.lupu@xxxxxxxxx>]

Commit 017fffd5 introduced support for setting the current thread pointer on top
of interrupt stacks in order to retrieve the current thread in interrupt context
as well. Unfortunately, the wrong stack was picked for KVM platform. This patch
fixes that and sets the thread on cpu_intr_stack instead.

cpu_intr_stack was resized to STACK_SIZE because this is a mandatory condition
when saving threads on top. However, given that it also needs a STACK_SIZE
alignment, a new section was created for it, .intrstack, in order to avoid
breaking the entire binary image layout. Without this new section, the entire
.text section would have a STACK_SIZE alignment (i.e. 64KB) and this would imply
that the multiboot header, which is included in .text section, would also be
moved at an address higher than STACK_SIZE, even though it must stay in the
first 8KB of the binary.

We apply the same solution for the traps stack in order to be able to print
the entire stack trace on faults.

Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
---
 plat/kvm/Makefile.uk      |  1 +
 plat/kvm/memory.c         |  6 -----
 plat/kvm/x86/link64.lds.S | 10 +++++++
 plat/kvm/x86/memory.c     | 57 +++++++++++++++++++++++++++++++++++++++
 plat/kvm/x86/traps.c      |  8 ++++--
 5 files changed, 74 insertions(+), 8 deletions(-)
 create mode 100644 plat/kvm/x86/memory.c

diff --git a/plat/kvm/Makefile.uk b/plat/kvm/Makefile.uk
index 3560939d..4758f035 100644
--- a/plat/kvm/Makefile.uk
+++ b/plat/kvm/Makefile.uk
@@ -58,6 +58,7 @@ LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
$(LIBKVMPLAT_BASE)/x86/lcpu.c
 LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += $(LIBKVMPLAT_BASE)/x86/intctrl.c
 LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += $(LIBKVMPLAT_BASE)/x86/tscclock.c
 LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += $(LIBKVMPLAT_BASE)/x86/time.c
+LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += $(LIBKVMPLAT_BASE)/x86/memory.c|x86
 ifeq ($(findstring y,$(CONFIG_KVM_KERNEL_VGA_CONSOLE) 
$(CONFIG_KVM_DEBUG_VGA_CONSOLE)),y)
 LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += $(LIBKVMPLAT_BASE)/x86/vga_console.c
 endif
diff --git a/plat/kvm/memory.c b/plat/kvm/memory.c
index e96bad2c..7f2fb46a 100644
--- a/plat/kvm/memory.c
+++ b/plat/kvm/memory.c
@@ -178,9 +178,3 @@ int _ukplat_mem_mappings_init(void)
 {
        return 0;
 }
-
-void ukplat_stack_set_current_thread(void *thread_addr)
-{
-       *((unsigned long *) _libkvmplat_cfg.bstack.end) =
-               (unsigned long) thread_addr;
-}
diff --git a/plat/kvm/x86/link64.lds.S b/plat/kvm/x86/link64.lds.S
index 362ba3e6..6103fc2d 100644
--- a/plat/kvm/x86/link64.lds.S
+++ b/plat/kvm/x86/link64.lds.S
@@ -99,6 +99,16 @@ SECTIONS
                . = ALIGN(__PAGE_SIZE);
        }
 
+       /* We keep the interrupt stack on a different section
+        * given that it may have a big alignment and it would
+        * change the entire binary layout
+        */
+       .intrstack :
+       {
+               *(.intrstack)
+               . = ALIGN(__PAGE_SIZE);
+       }
+
        _end = .;
 
        .comment       0 : { *(.comment) }
diff --git a/plat/kvm/x86/memory.c b/plat/kvm/x86/memory.c
new file mode 100644
index 00000000..ed58c1a4
--- /dev/null
+++ b/plat/kvm/x86/memory.c
@@ -0,0 +1,57 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Costin Lupu <costin.lupu@xxxxxxxxx>
+ *
+ * Copyright (c) 2019, University Politehnica of Bucharest. 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 <uk/plat/memory.h>
+
+
+extern char cpu_intr_stack[];
+extern char cpu_trap_stack[];
+
+void ukplat_stack_set_current_thread(void *thread_addr)
+{
+       /*
+        * TODO We set the current thread on interrupt and traps stack
+        * as well in order to be consistent when retrieving the current
+        * thread which is saved on the running thread stack.
+        *
+        * This is just a temporary solution and it should be removed
+        * when we will support stacks of various sizes. The current
+        * thread will be saved on some global variable, accessible from
+        * both thread and exception contexts.
+        */
+       *((unsigned long *) cpu_intr_stack) =
+               (unsigned long) thread_addr;
+       *((unsigned long *) cpu_trap_stack) =
+               (unsigned long) thread_addr;
+}
diff --git a/plat/kvm/x86/traps.c b/plat/kvm/x86/traps.c
index 27ef6d93..3a631d41 100644
--- a/plat/kvm/x86/traps.c
+++ b/plat/kvm/x86/traps.c
@@ -25,7 +25,9 @@
  */
 
 #include <string.h>
+#include <uk/essentials.h>
 #include <uk/arch/lcpu.h>
+#include <uk/plat/config.h>
 #include <x86/desc.h>
 #include <kvm-x86/traps.h>
 
@@ -59,8 +61,10 @@ static void gdt_init(void)
 
 static struct tss64 cpu_tss;
 
-static char cpu_intr_stack[4096]; /* IST1 */
-static char cpu_trap_stack[4096]; /* IST2 */
+__section(".intrstack")  __align(STACK_SIZE)
+char cpu_intr_stack[STACK_SIZE];  /* IST1 */
+__section(".intrstack")  __align(STACK_SIZE)
+char cpu_trap_stack[STACK_SIZE];  /* IST2 */
 static char cpu_nmi_stack[4096];  /* IST3 */
 
 static void tss_init(void)
-- 
2.20.1


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