Update lab3 answers

answers-lab2.txt

@@ -0,0 +1,22 @@
+1. The entry given by PDX(UVPT) is mapped to the page directory itself,
+to allow programs to read existing memory mappings. The entry at PDX(UPAGES)
+is mapped to the pages variable in memory so that the kernel (and potentially other ring 0 programs) can access it. The entry pointed to by PDX(KSTACKTOP-KSTACKSIZE) is mapped
+to the bootstack location. Finally, both the memory pointed to by 0 and PDX(KERNBASE) are mapped to kernel memory. However, the mappings at VA 0 are read-only, so user programs can't touch them, while the mappings at PDX(KERNBASE) are kernel-private and RW.
+
+A table could be as follows:
+
+400 - Kernel memory
+... - Kernel memory
+3c0 - Kernel memory
+3bf - Kernel Stack
+3bd - UVPT
+3bc - UPAGES
+
+
+2. The kernel memory is mapped as kernel read write. This means there are several flags set in the kernel page directory and page table entries, which indicate that this is restricted memory. The lower 3 bits of the CS register will be checked when an access to this memory is made, and, if they are not 0 or 1 (indicating kernel code), the CPU generates a fault, and the user program gives up control back to the OS. Thus, unless a program is started in ring 0, it will not be able to read or write kernel memory, as it should.
+
+3. The absolute maximum is 4GB, since we use 32-bit integers for referencing memory. Some of this memory is used for the kernel itself, as well as for the page directory and tables.
+
+4. The page directory and the corresponding pages are all 4kb. The page directory can have 1024 entries, and each of these point to a page table. Thus, we use approximatey 4MB (slightly more than that, actually, due to the size of the page directory itself) of memory. Additionally, the "pages" structs (which are used to keep track of available physical pages), will require ~1000000 entries, each of which is between 6 and 8 bytes (depending on whether GCC aligns struct sizes). This means another 8MB is used to keep track of free pages, to a total of around 12MB.
+
+5. We switch to high EIP when we jump to "relocated". Relocated is a label, and a symbol that's inserted by the linker. Since the linker is configured to link the kernel high, relocated points to the upper portion of memory, where KERNBASE is. However, the entry page directory, just like our full page directory later on, sets up two mappings, one starting at 0 (creating a one to one mapping between some of the virtual addresses and their physical counterparts), and one starting at KERNBASE. Thus, we can continue to run at a low EIP. The only reason I can think of as to why we NEED to make the switch, besides the elementary "the kernel links high", is that we need to be able to write to various symbols, also linked above KERNBASE.

answers-lab3.txt

@@ -0,0 +1,18 @@
+1. Some interrupts receive a different "shape" of stack frame - the kernel pushes
+an error code for some, but not for the others. We thus need individual handlers
+that would make sure that what we have on the stack is consistent, and
+then call one centralized function for that. The handlers additionally
+are able to pass in a trap number to that centrallized function. This way, a lot
+of code can be re-used.
+
+Conceptually, each trap has a different semantic meaning. Thus, it makes no sense
+to perform the same action for both traps - they don't mean the same thing.
+
+2. The general protection fault occurs when a process can access memory, but not do something else with it. We do not want page faults to be trigger-able by users (this is the case for many other traps). Thus, when the user tries to invoke int 14,
+it's not allowed to execute it from userspace, so it is thrown a general protection fault, which it then prints.
+
+3. Unlike most other traps, the debugging trap should be possible to trigger from user code. If the "privillege level" is set to 0 in the IDT, when the INT3 occurs, the user does not have permission to call that interrupt,
+and thus, a general protection fault occurs. In order to make it work correctly, the privillege level needs to be 3 (for userland).
+
+4. It's probably so that users cannot deliberately trigger certain faults for malicious purposes. It doesn't make sense to trigger a page fault without properly setting up CR2, and even then, though I can't think
+of anything right now, there's likely security issues that could arise if users can deliberately page fault their own process.

inc/x86.h

@@ -3,6 +3,10 @@
 
 #include <inc/types.h>
 
+#define MSR_IA32_SYSENTER_CS 0x174
+#define MSR_IA32_SYSENTER_EIP 0x176
+#define MSR_IA32_SYSENTER_ESP 0x175
+
 static inline void
 breakpoint(void)
 {
@@ -261,4 +265,19 @@ xchg(volatile uint32_t *addr, uint32_t newval)
 	return result;
 }
 
+static inline void
+write_msr(uint32_t reg, uint32_t low, uint32_t high) {
+	asm volatile("wrmsr\n\t"
+			:: "c" (reg), "a" (low), "d" (high));
+}
+
+static inline void
+read_msr(uint32_t reg, uint32_t* low, uint32_t* high) {
+	uint32_t eax, edx;
+	asm volatile("rdmsr\n\t"
+			: "=a" (eax), "=d" (edx) : "c" (reg));
+	*low = eax;
+	*high = edx;
+}
+
 #endif /* !JOS_INC_X86_H */

kern/Makefrag

@@ -50,7 +50,8 @@ KERN_BINFILES :=	user/hello \
 			user/faultread \
 			user/faultreadkernel \
 			user/faultwrite \
-			user/faultwritekernel
+			user/faultwritekernel \
+			user/getc
 
 KERN_OBJFILES := $(patsubst %.c, $(OBJDIR)/%.o, $(KERN_SRCFILES))
 KERN_OBJFILES := $(patsubst %.S, $(OBJDIR)/%.o, $(KERN_OBJFILES))

kern/init.c

@@ -3,6 +3,7 @@
 #include <inc/stdio.h>
 #include <inc/string.h>
 #include <inc/assert.h>
+#include <inc/x86.h>
 
 #include <kern/monitor.h>
 #include <kern/console.h>
@@ -11,6 +12,7 @@
 #include <kern/env.h>
 #include <kern/trap.h>
 
+void sysenter_handler();
 
 void
 i386_init(void)
@@ -34,6 +36,10 @@ i386_init(void)
 		"\33[34m" "r"
 		"\33[0m"  " Works!" "\n");
 
+	write_msr(MSR_IA32_SYSENTER_EIP, (uint32_t) sysenter_handler, 0);
+	write_msr(MSR_IA32_SYSENTER_ESP, KSTACKTOP, 0);
+	write_msr(MSR_IA32_SYSENTER_CS, GD_KT, 0);
+
 	// Lab 2 memory management initialization functions
 	mem_init();
 

kern/monitor.c

@@ -7,6 +7,7 @@
 #include <inc/assert.h>
 #include <inc/x86.h>
 
+#include <kern/env.h>
 #include <kern/ansi.h>
 #include <kern/console.h>
 #include <kern/monitor.h>
@@ -31,7 +32,9 @@ static struct Command commands[] = {
 	{ "kerninfo", "Display information about the kernel", mon_kerninfo },
 	{ "backtrace", "Display current backtrace", mon_backtrace },
 	{ "showmappings", "Display the physical mappings for range", mon_showmappings },
-	{ "mperms", "Change the permissions of a memory range", mon_mperms }
+	{ "mperms", "Change the permissions of a memory range", mon_mperms },
+	{ "resume", "Resume from a breakpoint", mon_resume },
+	{ "step", "Step to next instruction", mon_step }
 	
 };
 
@@ -188,6 +191,43 @@ int mon_mperms(int argc, char** argv, struct Trapframe* tf) {
 	return 0;
 }
 
+// This is a nice thought and all...
+// But we should modify the trap frame.
+// I feel stupid.
+static inline void
+set_eflag(uint16_t flagno, int value) {
+	uint32_t temp;
+	uint32_t mask = ~(1 << flagno);
+	uint32_t regv = value << flagno;
+	asm volatile("pushfl\n\t"
+			"pop %w0\n\t"
+			"and %w1, %w0\n\t"
+			"or %w2, %w0\n\t"
+			"push %w0\n\t"
+			"popfl\n\t"
+			: "=r" (temp)
+			: "r" (mask), "r" (regv));
+}
+
+#define EXPECT_BRKPT if(!(tf->tf_trapno == T_BRKPT || tf->tf_trapno == T_DEBUG)) { \
+	cprintf(ACOL_ERR("I don't think I should resume from this.\n")); \
+	return 0; }
+#define EFLAGS_TF 0x8
+
+int mon_resume(int argc, char** argv, struct Trapframe* tf) {
+	EXPECT_BRKPT;
+	tf->tf_eflags &= ~(1 << EFLAGS_TF);
+	env_pop_tf(tf);
+	return 0;
+}
+
+int mon_step(int argc, char** argv, struct Trapframe* tf) {
+	EXPECT_BRKPT;
+	tf->tf_eflags |= 1 << EFLAGS_TF;
+	env_pop_tf(tf);
+	return 0;
+}
+
 /***** Kernel monitor command interpreter *****/
 
 #define WHITESPACE "\t\r\n "

kern/monitor.h

@@ -17,5 +17,7 @@ int mon_kerninfo(int argc, char **argv, struct Trapframe *tf);
 int mon_backtrace(int argc, char **argv, struct Trapframe *tf);
 int mon_showmappings(int argc, char **argv, struct Trapframe *tf);
 int mon_mperms(int argc, char** argv, struct Trapframe* tf);
+int mon_resume(int argc, char** argv, struct Trapframe* tf);
+int mon_step(int argc, char** argv, struct Trapframe* tf);
 
 #endif	// !JOS_KERN_MONITOR_H

kern/pmap.c

@@ -185,8 +185,6 @@ mem_init(void)
 	boot_map_region(kern_pgdir,
 			UPAGES, ROUNDUP(pages_size, PGSIZE),
 			PADDR(pages), PTE_U);
-	kern_pgdir[PDX(UPAGES)] |= PTE_U;
-	kern_pgdir[PDX(UPAGES)] &= ~PTE_W;
 
 	//////////////////////////////////////////////////////////////////////
 	// Map the 'envs' array read-only by the user at linear address UENVS
@@ -198,8 +196,6 @@ mem_init(void)
 	boot_map_region(kern_pgdir,
 		UENVS, ROUNDUP(envs_size, PGSIZE),
 		PADDR(envs), PTE_U);
-	kern_pgdir[PDX(UENVS)] |= PTE_U;
-	kern_pgdir[PDX(UPAGES)] &= ~PTE_W;
 
 	//////////////////////////////////////////////////////////////////////
 	// Use the physical memory that 'bootstack' refers to as the kernel
@@ -215,8 +211,6 @@ mem_init(void)
 	boot_map_region(kern_pgdir,
 			KSTACKTOP-KSTKSIZE, KSTKSIZE,
 			PADDR(bootstack), PTE_W);
-	kern_pgdir[PDX(KSTACKTOP-KSTKSIZE)] |= PTE_W;
-	kern_pgdir[PDX(KSTACKTOP-KSTKSIZE)] &= ~PTE_U;
 
 	//////////////////////////////////////////////////////////////////////
 	// Map all of physical memory at KERNBASE.
@@ -229,9 +223,6 @@ mem_init(void)
 	boot_map_region(kern_pgdir,
 			KERNBASE, 0x100000000 - KERNBASE,
 			0, PTE_W);
-	kern_pgdir[PDX(KERNBASE)] |= PTE_W | PTE_P;
-	kern_pgdir[PDX(KERNBASE)] &= ~PTE_U;
-
 
 	// Check that the initial page directory has been set up correctly.
 	check_kern_pgdir();

kern/trap.c

@@ -199,7 +199,7 @@ trap_dispatch(struct Trapframe *tf)
 	if (tf->tf_trapno == T_PGFLT) {
 		page_fault_handler(tf);
 		return;
-	} else if (tf->tf_trapno == T_BRKPT) {
+	} else if (tf->tf_trapno == T_BRKPT || tf->tf_trapno == T_DEBUG) {
 		monitor(tf);
 		return;
 	} else if (tf->tf_trapno == T_SYSCALL) {

kern/trapentry.S

@@ -43,6 +43,21 @@
 
 .text
 
+.globl sysenter_handler
+sysenter_handler:
+	push %ebp // holds env's stack pointer
+	push %esi // holds the env's return addr
+	push %edi
+	push %ebx
+	push %ecx
+	push %edx
+	push %eax
+	call syscall
+	add $0x14, %esp
+	pop %edx
+	pop %ecx
+	sysexit
+	
 /*
  * Lab 3: Your code here for generating entry points for the different traps.
  */

lib/syscall.c

@@ -2,6 +2,7 @@
 
 #include <inc/syscall.h>
 #include <inc/lib.h>
+#include <inc/x86.h>
 
 static inline int32_t
 syscall(int num, int check, uint32_t a1, uint32_t a2, uint32_t a3, uint32_t a4, uint32_t a5)
@@ -37,16 +38,30 @@ syscall(int num, int check, uint32_t a1, uint32_t a2, uint32_t a3, uint32_t a4,
 	return ret;
 }
 
+int32_t
+fast_syscall(int num, uint32_t a1, uint32_t a2, uint32_t a3, uint32_t a4) {
+	asm volatile(
+			"push %%ebp\n\t"
+			"mov %%esp, %%ebp\n\t"
+			"lea syscall_ret_%=, %%esi\n\t"
+			"sysenter\n\t"
+			"syscall_ret_%=: pop %%ebp\n\t"
+			: "+a" (num)
+			: "d" (a1), "c" (a2), "b" (a3), "D" (a4)
+			: "esi");
+	return num;
+}
+
 void
 sys_cputs(const char *s, size_t len)
 {
-	syscall(SYS_cputs, 0, (uint32_t)s, len, 0, 0, 0);
+	fast_syscall(SYS_cputs, (uint32_t)s, len, 0, 0);
 }
 
 int
 sys_cgetc(void)
 {
-	return syscall(SYS_cgetc, 0, 0, 0, 0, 0, 0);
+	return fast_syscall(SYS_cgetc, 0, 0, 0, 0);
 }
 
 int

user/getc.c

@@ -0,0 +1,9 @@
+#include <inc/lib.h>
+
+void
+umain(int argc, char **argv)
+{
+	char c;
+	while(!(c = sys_cgetc()));
+	cprintf("got character %c\n", c);
+}