From 94770e6354578485b2aed68dcc3064e5d6862645 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Duje=20Mihanovi=C4=87?= <duje.mihanovic@skole.hr>
Date: Sat, 7 May 2022 16:11:19 +0200
Subject: [PATCH] Refactor Makefile, linker script, properly set up stack

---
 .gitignore       |  1 +
 Makefile         | 19 +++++++++++--------
 kernel/entry.s   | 21 ++++++++++-----------
 kernel/kernel.c  |  2 +-
 kernel/linker.ld | 27 ++++++++++++++++++++++++---
 5 files changed, 47 insertions(+), 23 deletions(-)

diff --git a/.gitignore b/.gitignore
index a45d17d..9fc5bb7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,3 +4,4 @@ boot/x86/boot
 **/*.o
 **/*.bin
 **/*.elf
+**/*.dbg
diff --git a/Makefile b/Makefile
index 738fb0c..8ffcdf8 100644
--- a/Makefile
+++ b/Makefile
@@ -7,24 +7,24 @@ GIT_REV = $(shell git rev-parse --short HEAD)
 CFLAGS = -std=gnu89 -g -Iinclude/arch/x86 -ffreestanding -DGIT_COMMIT=\"$(GIT_REV)\"
 
 KERNEL_OBJ = kernel/entry.o kernel/arch/x86/tty/tty.o kernel/drivers/irq/i8259a.o kernel/arch/x86/irq/idt.o kernel/arch/x86/irq/sample_handler.o kernel/kernel.o
+BOOTLOADER_OBJ = boot/x86/mbr boot/x86/vbr-fat32 boot/x86/stage3/LOADER.BIN
 
 default: kernel/kernel.elf bootloader
 
 all: default boot/x86/disk.img
 
-bootloader: boot/x86/mbr boot/x86/vbr-fat32 boot/x86/stage3/LOADER.BIN
+bootloader: $(BOOTLOADER_OBJ)
 
 run: all
 	$(QEMU) boot/x86/disk.img
 
 boot/x86/mbr: boot/x86/mbr.s
-	$(MAKE) -C boot/x86
 boot/x86/vbr-fat32: boot/x86/vbr-fat32.s boot/x86/fat32/*.s 
-	$(MAKE) -C boot/x86
 boot/x86/stage3/LOADER.BIN: boot/x86/stage3/*.s boot/x86/fat32/*.s
+$(BOOTLOADER_OBJ):
 	$(MAKE) -C boot/x86
 
-boot/x86/disk.img: boot/x86/mbr boot/x86/vbr-fat32 boot/x86/stage3/LOADER.BIN boot/x86/disk.dump 
+boot/x86/disk.img: boot/x86/mbr boot/x86/vbr-fat32 boot/x86/stage3/LOADER.BIN boot/x86/disk.dump kernel/kernel.bin
 	truncate -s1G boot/x86/disk.img
 	sfdisk boot/x86/disk.img < boot/x86/disk.dump
 	mkfs.fat -F 32 --offset 2048 boot/x86/disk.img
@@ -33,20 +33,23 @@ boot/x86/disk.img: boot/x86/mbr boot/x86/vbr-fat32 boot/x86/stage3/LOADER.BIN bo
 	mcopy -i boot/x86/disk.img@@1M boot/x86/stage3/LOADER.BIN ::.
 	mcopy -i boot/x86/disk.img@@1M kernel/kernel.bin ::./KERNEL.BIN
 
-kernel/kernel.bin: ${KERNEL_OBJ}
+kernel/kernel.bin: ${KERNEL_OBJ} kernel/linker.ld
 	$(CC) -ffreestanding -nostdlib -o $@ -T kernel/linker.ld ${KERNEL_OBJ}
 
 kernel/entry.o: kernel/entry.s
-	$(AS) -f elf kernel/entry.s -o $@
+	$(AS) -f elf $< -o $@
 
 kernel/arch/x86/irq/sample_handler.o: kernel/arch/x86/irq/sample_handler.c
-	$(CC) $(CFLAGS) -mgeneral-regs-only -c kernel/arch/x86/irq/sample_handler.c -o $@
+	$(CC) $(CFLAGS) -mgeneral-regs-only -c $< -o $@
 
 kernel/kernel.elf: kernel/kernel.bin
 	$(CC) -ffreestanding -nostdlib -o $@ -T kernel/linker.ld ${KERNEL_OBJ} -Wl,--oformat=elf32-i386
+	i686-elf-objcopy --only-keep-debug kernel/kernel.elf kernel/kernel.dbg
+	i686-elf-objcopy --add-gnu-debuglink=kernel/kernel.dbg kernel/kernel.elf
+	i686-elf-strip --strip-unneeded kernel/kernel.elf
 
 clean:
-	-rm kernel/kernel.bin kernel/kernel.elf ${KERNEL_OBJ}
+	-rm kernel/kernel.{bin,dbg,elf} ${KERNEL_OBJ}
 	cd boot/x86 && $(MAKE) clean
 
 .PHONY: default all clean run bootloader
diff --git a/kernel/entry.s b/kernel/entry.s
index a02df92..10c95e8 100644
--- a/kernel/entry.s
+++ b/kernel/entry.s
@@ -1,12 +1,11 @@
-; when our kernel source has functions before _start and we blindly transfer control to the beginning
-; of the kernel binary, it will wrongly execute the other functions
-; the solution is to link a small assembly routine (this) which executes a known label within the kernel
-; so that this routine comes before the kernel in the resulting binary
-; we cannot link the boot sector code and the kernel because the former needs to be a raw binary, while the
-; kernel is compiled into an ELF object file which contains some metadata on the kernel code
+bits 32
+extern kmain
+extern __STACK_BOTTOM__
 
-	bits 32 ; this is started in protected mode
-	extern _start ; the known label in the kernel source we will execute is, well, not in this assembly routine
-
-	call _start ; call the known label
-	jmp $ ; it should never return, but hang forever if it does
+global _start
+_start:
+	mov ebp, __STACK_BOTTOM__
+	mov esp, ebp
+	call kmain
+	hlt
+	jmp $-1
diff --git a/kernel/kernel.c b/kernel/kernel.c
index e1a0df5..4c6efd8 100644
--- a/kernel/kernel.c
+++ b/kernel/kernel.c
@@ -9,7 +9,7 @@ extern void keyb_handler(struct interrupt_frame *frame);
 struct idt_descriptor idt[256] __attribute__((aligned(0x10)));
 struct idtr idtr __attribute__((aligned(0x10)));
 
-void _start(void)
+void kmain(void)
 {
 	screen_clear();
 	kprint("Welcome to Nameless OS!\nRunning revision: ", 0);
diff --git a/kernel/linker.ld b/kernel/linker.ld
index 59d0381..645a824 100644
--- a/kernel/linker.ld
+++ b/kernel/linker.ld
@@ -4,8 +4,29 @@ OUTPUT_FORMAT(binary)
 SECTIONS
 {
 	. = 0x100000;
+	__KERNEL_BASE__ = .;
 
-	.text : ALIGN(0x1000) { *(.text) }
-	.data : ALIGN(0x1000) { *(.data) }
-	.bss : ALIGN(0x1000) { *(.bss) }
+	.text : ALIGN(4K) { 
+		kernel/entry.o (.text)
+		*(.text) 
+	}
+	.rodata : ALIGN(4K) { *(.rodata) }
+
+	/* .rodata is put after .text so that the read-only entry in the program
+	 * header covers .rodata as well. When .rodata is after .data, that entry
+	 * only covers .text, leaving .rodata in the writable program header
+	 * entry. The problem with this approach is that the program header
+	 * entry where .text and .rodata are is marked as executable, which
+	 * would mean that .rodata should be executable too, but that doesn't
+	 * matter yet because we (currently) assume that the NX bit is not
+	 * supported anyway. */
+
+	.data : ALIGN(4K) { *(.data) }
+	.bss : ALIGN(4K) { 
+		*(.bss)
+		/* Reserving 16KiB for the stack. A __STACK_TOP__ is not really
+		 * needed (yet). */
+		. += 16K; 
+		__STACK_BOTTOM__ = .;
+	}
 }
-- 
2.39.5