From 56767c59ed4ca45ca1a090912ad68cc1f2df1dd9 Mon Sep 17 00:00:00 2001
From: fef <owo@fef.moe>
Date: Fri, 31 Dec 2021 00:02:00 +0100
Subject: [PATCH] x86/ktrace: improve traceability

---
 arch/x86/boot/setup32.S     |  3 +++
 arch/x86/boot/setup64.S     | 17 ++++++++++++-----
 arch/x86/sys/amd64/ktrace.c | 19 ++++++++++++++++++-
 arch/x86/sys/i386/ktrace.c  | 20 ++++++++++++++++++--
 4 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/arch/x86/boot/setup32.S b/arch/x86/boot/setup32.S
index b44c524..97b5313 100644
--- a/arch/x86/boot/setup32.S
+++ b/arch/x86/boot/setup32.S
@@ -53,6 +53,9 @@ ENTRY(_setup)
 	 * set up initial stack frame
 	 */
 	movl	$PADDR(stack_top), %esp
+	/* "previous" %eip for ktrace */
+	pushl	$0
+	/* "previous" %ebp for ktrace */
 	pushl	$0
 	movl	%esp, %ebp
 
diff --git a/arch/x86/boot/setup64.S b/arch/x86/boot/setup64.S
index e0e1928..988b91e 100644
--- a/arch/x86/boot/setup64.S
+++ b/arch/x86/boot/setup64.S
@@ -4,6 +4,7 @@
 
 #include <arch/multiboot.h>
 #include <arch/page.h>
+#include <arch/sched.h>
 #include <arch/segment.h>
 
 /*
@@ -69,9 +70,12 @@ ENTRY(_setup)
 	 * set up the stack
 	 */
 	movl	$PADDR(stack_top), %esp
-	/* XXX use the APIC for reading the CPU number */
-	pushl	$0 /* CPU number -- see include/arch/smp.h */
-	pushl	$0 /* two longwords to keep the stack 8 byte aligned */
+	/* "previous" %rip (for ktrace) */
+	pushl	$0
+	pushl	$0
+	/* "previous" %rbp (for ktrace) */
+	pushl	$0
+	pushl	$0
 	movl	%esp, %ebp
 
 	push	%ebx /* temporarily stash multiboot tag address away */
@@ -107,9 +111,12 @@ ENTRY(_setup)
 	 */
 	pushl	$PADDR(_x86_kern_tss)
 	pushl	$PADDR(_x86_gdt + X86_KERN_TSS)
+	/* _x86_write_tss_base(&_x86_gdt[X86_KERN_TSS / sizeof(_x86_gdt[0])], &_x86_kern_tss); */
 	call	_x86_write_tss_base
+
 	pushl	$PADDR(_x86_user_tss)
 	pushl	$PADDR(_x86_gdt + X86_USER_TSS)
+	/* _x86_write_tss_base(&_x86_gdt[X86_USER_TSS / sizeof(_x86_gdt[0])], &_x86_user_tss); */
 	call	_x86_write_tss_base
 	addl	$16, %esp
 
@@ -266,7 +273,7 @@ L_END(_setup_highmem)
 	.section .multiboot.data, "a", @progbits
 
 	.section .bootstrap_stack, "aw", @nobits
-	.align 16384
+	.align KERN_STACK_SIZE
 stack_bottom:
-	.skip 16384 /* 16 K for the stack should be plenty for now */
+	.skip KERN_STACK_SIZE
 stack_top:
diff --git a/arch/x86/sys/amd64/ktrace.c b/arch/x86/sys/amd64/ktrace.c
index 040b32e..ad85ff8 100644
--- a/arch/x86/sys/amd64/ktrace.c
+++ b/arch/x86/sys/amd64/ktrace.c
@@ -4,6 +4,19 @@
 #include <gay/ktrace.h>
 #include <gay/linker.h>
 
+/*
+ * amd64 stack frame layout:
+ *
+ *  24(%rbp) ...
+ *  16(%rbp) argument 7 (if applicable, args 1-6 are passed in registers)
+ *   8(%rbp) caller %rip (after the call instruction, pushed by hardware)
+ * ------------------------------------------------------------
+ *    (%rbp) caller %rbp (pushed in callee prologue)
+ *  -8(%rbp) local variable 1
+ * -16(%rbp) local variable 2
+ * -24(%rbp) ...
+ */
+
 __naked void ktrace_print(void)
 {
 	__asm__ volatile(
@@ -20,7 +33,11 @@ void ktrace_print_from(void *frame)
 	/* XXX Rather than spitting out raw addresses, parse the kernel image's
 	 *     ELF sections to figure out what the address actually belongs to */
 	while (rbp >= (void **)image_start && rbp < (void **)image_end) {
-		/* caller return address is immediately above the stack frame */
+		/* arch/x86/boot/setup64.S pushes two `nil's as the initial stack frame */
+		if (rbp[1] == nil)
+			break;
+		/* Caller return address is immediately above the stack frame.
+		 * XXX This prints the position *after* the call instruction. */
 		kprintf("  %p\n", rbp[1]);
 		if (rbp[1] >= isr_start && rbp[1] < isr_end)
 			break;
diff --git a/arch/x86/sys/i386/ktrace.c b/arch/x86/sys/i386/ktrace.c
index 780af5f..7980a62 100644
--- a/arch/x86/sys/i386/ktrace.c
+++ b/arch/x86/sys/i386/ktrace.c
@@ -4,6 +4,19 @@
 #include <gay/ktrace.h>
 #include <gay/linker.h>
 
+/*
+ * i386 stack frame layout:
+ *
+ *  12(%ebp) ...
+ *   8(%ebp) argument 1 (if applicable)
+ *   4(%ebp) caller %eip (after the call instruction, pushed by hardware)
+ * ------------------------------------------------------------
+ *    (%ebp) caller %ebp (pushed in callee prologue)
+ *  -4(%ebp) local variable 1
+ *  -8(%ebp) local variable 2
+ * -12(%ebp) ...
+ */
+
 __naked void ktrace_print(void)
 {
 	__asm__ volatile(
@@ -21,11 +34,14 @@ void ktrace_print_from(void *frame)
 	/* XXX Rather than spitting out raw addresses, parse the kernel image's
 	 *     ELF sections to figure out what the address actually belongs to */
 	while (ebp >= (void **)image_start && ebp < (void **)image_end) {
+		/* arch/x86/boot/setup32.S pushes two `nil's as the initial stack frame */
+		if (ebp[1] == nil)
+			break;
 		/* caller return address is immediately above the stack frame */
 		kprintf("  %p\n", ebp[1]);
-		if (rbp[1] >= isr_start && rbp[1] < isr_end)
+		if (ebp[1] >= isr_start && ebp[1] < isr_end)
 			break;
-		rbp = *rbp;
+		ebp = *ebp;
 	}
 }