From 3bfcaeaf015ffe0d92937c023e9a0086909a0161 Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Sat, 29 Sep 2018 08:30:50 -0400 Subject: [PATCH] Make sysexit and trapret paths the same, so that forkret can return through either path. This helped tracking down a bug: use 144 instead of 32 to find cs in trapframe so that gs is correctly saved and restored. For good measure update linker script, because newer versions of GCC sometimes places symbols passed end. --- kernel.ld | 7 ++++--- proc.c | 16 ++++++++++++---- trapasm.S | 11 ++++++----- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/kernel.ld b/kernel.ld index e78fd38..11dc98f 100644 --- a/kernel.ld +++ b/kernel.ld @@ -41,9 +41,10 @@ SECTIONS .data : { *(.data) } - PROVIDE(edata = .); - .bss : { + bss : { + PROVIDE(edata = .); *(.bss) + *(COMMON) + PROVIDE(end = .); } - PROVIDE(end = .); } diff --git a/proc.c b/proc.c index 58ae948..3ab23f4 100644 --- a/proc.c +++ b/proc.c @@ -6,7 +6,6 @@ #include "x86.h" #include "proc.h" #include "spinlock.h" -#include "msr.h" struct { struct spinlock lock; @@ -17,7 +16,11 @@ static struct proc *initproc; int nextpid = 1; extern void forkret(void); + +// we can return two ways out of the kernel and +// for new processes we can choose either way extern void sysexit(void); +extern void trapret(void); static void wakeup1(void *chan); @@ -101,6 +104,10 @@ found: // Leave room for trap frame. sp -= sizeof *p->tf; + + if ((uint64) sp % 16) + panic("misaligned sp"); + p->tf = (struct trapframe*)sp; // Set up new context to start executing at forkret, @@ -132,6 +139,8 @@ userinit(void) inituvm(p->pgdir, _binary_initcode_start, (uint64)_binary_initcode_size); p->sz = PGSIZE; memset(p->tf, 0, sizeof(*p->tf)); + p->tf->cs = UCSEG | 0x3; + p->tf->ss = UDSEG | 0x3; p->tf->r11 = FL_IF; p->tf->rsp = PGSIZE; p->tf->rcx = 0; // beginning of initcode.S @@ -321,12 +330,12 @@ scheduler(void) { struct proc *p; struct cpu *c = mycpu(); + c->proc = 0; - for(;;){ // Enable interrupts on this processor. sti(); - + // Loop over process table looking for process to run. acquire(&ptable.lock); for(p = ptable.proc; p < &ptable.proc[NPROC]; p++){ @@ -336,7 +345,6 @@ scheduler(void) // Switch to chosen process. It is the process's job // to release ptable.lock and then reacquire it // before jumping back to us. - c->proc = p; switchuvm(p); p->state = RUNNING; diff --git a/trapasm.S b/trapasm.S index b6dbb1a..d160ff2 100644 --- a/trapasm.S +++ b/trapasm.S @@ -22,7 +22,7 @@ alltraps: push %rbx push %rax - cmpw $KCSEG, 32(%rsp) # compare to saved cs + cmpw $KCSEG, 144(%rsp) # compare to saved cs jz 1f swapgs @@ -33,7 +33,7 @@ alltraps: .globl trapret trapret: cli - cmpw $KCSEG, 32(%rsp) # compare to saved cs + cmpw $KCSEG, 144(%rsp) # compare to saved cs jz 1f swapgs @@ -72,12 +72,12 @@ sysentry: # Build trap frame. movq %rax, %rsp movq %gs:0, %rax // restore rax - // push usp - push $0 + // push usp to make a valid trapframe + push $(UDSEG|0x3) push %gs:8 // safe eflags and eip push %r11 - push $UCSEG + push $(UCSEG|0x3) push %rcx // push errno and trapno to make stack look like a trap push $0 @@ -130,6 +130,7 @@ sysexit: add $(5*8), %rsp # discard trapnum, errorcode, rip, cs and rflags mov (%rsp),%rsp # switch to the user stack + # there are two more values on the stack, but we don't care about them swapgs sysretq