kernel/isr: improve interrupt handling code

On the assembly side, ensure the stack frame looks always the same, by pushing
a fake "error code" for the interrupts that don't generate one.

On the C side, use a struct instead of magic indices into an "array", and
make it consistent with the current style.

3 files changed, 56 insertions, 29 deletions

diff --git a/src/kernel/arch/amd64/interrupts.h b/src/kernel/arch/amd64/interrupts.h
index dfc523a..288bd78 100644
--- a/src/kernel/arch/amd64/interrupts.h
+++ b/src/kernel/arch/amd64/interrupts.h
@@ -14,4 +14,4 @@ extern const char _isr_stubs;
 
 void irq_init(void);
 void irq_eoi(uint8_t line);
-void isr_stage3(uint8_t interrupt, uint64_t *stackframe);
+void isr_stage3(uint8_t interrupt, void *stackframe);
diff --git a/src/kernel/arch/amd64/interrupts/isr.c b/src/kernel/arch/amd64/interrupts/isr.c
index dd97fd4..bf84c5c 100644
--- a/src/kernel/arch/amd64/interrupts/isr.c
+++ b/src/kernel/arch/amd64/interrupts/isr.c
@@ -8,29 +8,48 @@
 #include <stdint.h>
 
 enum {
-	NMI = 0x02,
-	GP_FAULT = 0x0d,
-	PAGE_FAULT = 0x0e,
+	Nmi = 0x02,
+	GpFault = 0x0d,
+	PageFault = 0x0e,
 };
 
+typedef struct {
+	uint64_t ip, cs, flags;
+	/* only valid if switching from/into user mode */
+	uint64_t sp, ss;
+} IretFrame;
+
 void (*irq_fn[16])(void) = {0};
 
-static void log_interrupt(int interrupt, uint64_t *stackframe) {
+static uint64_t
+getcr2(void)
+{
+	uint64_t cr2;
+	asm("mov %%cr2, %0" : "=r"(cr2));
+	return cr2;
+}
+
+static void
+log_interrupt(uint8_t inr, void *stackframe)
+{
+	IretFrame *iret = stackframe;
+	uint64_t *errcode = ((uint64_t*)stackframe) - 1;
 	kprintf("interrupt %d, rip = k/%08x, cs 0x%x, code 0x%x\n",
-			interrupt, stackframe[0], stackframe[1], stackframe[-1]);
-	if ((stackframe[1] & 0x3) == 0) {
-		uint64_t *stack = (void*)stackframe[3];
-		kprintf("kernel rip = %p, *rip = %p\n", stack, *stack);
+			inr, iret->ip, iret->cs, *errcode);
+	if ((iret->cs & 0x3) == 0) { /* ring 0? */
+		uint64_t *stack = (void*)iret->sp;
+		kprintf("kernel rsp = %p, *rsp = %p\n", stack, *stack);
 	}
-	if (interrupt == PAGE_FAULT) {
-		uint64_t addr = 0x69;
-		asm("mov %%cr2, %0" : "=r"(addr));
-		kprintf("addr 0x%x\n", addr);
+	if (inr == PageFault) {
+		kprintf("addr 0x%x\n", getcr2());
 	}
 }
 
-void isr_stage3(uint8_t interrupt, uint64_t *stackframe) {
-	uint8_t irqn = interrupt - IRQ_IBASE;
+void
+isr_stage3(uint8_t inr, void *stackframe)
+{
+	IretFrame *iret = stackframe;
+	uint8_t irqn = inr - IRQ_IBASE;
 	if (irqn < 16) {
 		if (irq_fn[irqn]) {
 			irq_fn[irqn]();
@@ -39,21 +58,17 @@ void isr_stage3(uint8_t interrupt, uint64_t *stackframe) {
 		}
 	}
 
-	if (interrupt == NMI) { /* print some debugging information */
-		log_interrupt(interrupt, stackframe);
+	if (inr == Nmi) { /* print some debugging information */
+		log_interrupt(inr, stackframe);
 		mem_debugprint();
 		return;
 	}
 
-	if (interrupt == PAGE_FAULT || interrupt == GP_FAULT) {
-		stackframe++;
-	}
-
-	if ((stackframe[1] & 0x3) == 0) { /* in kernel */
-		log_interrupt(interrupt, stackframe);
+	if ((iret->cs & 0x3) == 0) { /* ring 0? */
+		log_interrupt(inr, stackframe);
 		cpu_halt();
 	} else { /* in user */
-		proc_kill(proc_cur, interrupt);
+		proc_kill(proc_cur, inr);
 		proc_switch_any();
 	}
 }
diff --git a/src/kernel/arch/amd64/interrupts/isr_stub.s b/src/kernel/arch/amd64/interrupts/isr_stub.s
index 75934d5..3134808 100644
--- a/src/kernel/arch/amd64/interrupts/isr_stub.s
+++ b/src/kernel/arch/amd64/interrupts/isr_stub.s
@@ -2,21 +2,27 @@
 
 .global _isr_stubs
 _isr_stubs:
+.set i, 0
 .rept 256
 	.set _stub_start, .
 
 	cli
+	.if i !=  8 && i != 10 && i != 11 && i != 12 && i != 13 && i != 14 && i != 17 && i != 21 && i != 29 && i != 30
+		/* no error code was pushed - push anything, just to line up the stack */
+		push %rbx
+	.endif
 	call _isr_stage2
 
 	.if . - _stub_start > 8
 		.error "isr stubs over maximum size"
 		.abort
 	.endif
-	.align 8
+	.align 8, 0xCC
+	.set i, i + 1
 .endr
 
 _isr_stage2:
-	// pushal order, without %esp
+	/* pushal order, without %esp. 15 in total */
 	push %rax
 	push %rcx
 	push %rdx
@@ -33,13 +39,19 @@ _isr_stage2:
 	push %r14
 	push %r15
 	// TODO FXSAVE might be required on interrupts too?
+	// on interrupts - no
+	// on syscalls - yes
+	// have fun figuring that one out
+	// basically if you're context switching, FXSAVE
 
-	// convert the return address into the vector nr
+	/* first argument: vector nr. computed from the return address at offset
+	 * 15*8 = 120 */
 	mov 120(%rsp), %rdi
 	sub $_isr_stubs, %rdi
 	shr $3, %rdi
 
-	lea 128(%rsp), %rsi // second argument - IRET stack frame
+	/* second argument: IRET stack frame */
+	lea 136(%rsp), %rsi
 
 	// load kernel paging
 	mov %cr3, %rbx
@@ -72,7 +84,7 @@ _isr_stage2:
 	pop %rcx
 	pop %rax
 
-	add $8, %rsp // skip call's return address
+	add $16, %rsp /* skip return address from call and the error code */
 	iretq
 
 .align 8