From b1096911798788421a4d5c44fc7f2da138c8e24f Mon Sep 17 00:00:00 2001 From: Carlos Date: Fri, 28 Aug 2015 10:52:05 +0200 Subject: [PATCH] Lesson 23, section 6 --- 23-fixes/Makefile | 4 ++-- 23-fixes/README.md | 21 +++++++++++++++++---- 23-fixes/cpu/interrupt.asm | 37 ++++--------------------------------- 23-fixes/cpu/isr.c | 14 +++++++------- 23-fixes/cpu/isr.h | 12 +++++++++--- 23-fixes/cpu/timer.c | 2 +- 23-fixes/drivers/keyboard.c | 2 +- 23-fixes/kernel/kernel.c | 3 +++ 8 files changed, 44 insertions(+), 51 deletions(-) diff --git a/23-fixes/Makefile b/23-fixes/Makefile index 6b679a7..65e577b 100644 --- a/23-fixes/Makefile +++ b/23-fixes/Makefile @@ -7,7 +7,7 @@ OBJ = ${C_SOURCES:.c=.o cpu/interrupt.o} CC = /usr/local/i386elfgcc/bin/i386-elf-gcc GDB = /usr/local/i386elfgcc/bin/i386-elf-gdb # -g: Use debugging symbols in gcc -CFLAGS = -g -ffreestanding -Wall -Wextra -fno-exceptions +CFLAGS = -g -ffreestanding -Wall -Wextra -fno-exceptions -m32 # First rule is run by default os-image.bin: boot/bootsect.bin kernel.bin @@ -33,7 +33,7 @@ debug: os-image.bin kernel.elf # Generic rules for wildcards # To make an object, always compile from its .c %.o: %.c ${HEADERS} - ${CC} ${CFLAGS} -ffreestanding -c $< -o $@ + ${CC} ${CFLAGS} -c $< -o $@ %.o: %.asm nasm $< -f elf -o $@ diff --git a/23-fixes/README.md b/23-fixes/README.md index 6797b20..5d9451c 100644 --- a/23-fixes/README.md +++ b/23-fixes/README.md @@ -1,4 +1,4 @@ -*Concepts you may want to Google beforehand: XX* +*Concepts you may want to Google beforehand: freestanding, uint32_t, size_t,* **Goal: Fix miscellaneous issues with our code** @@ -16,7 +16,8 @@ We add `-ffreestanding` when compiling `.o` files, which includes `kernel_entry Before, we disabled libgcc (not libc) through the use of `-nostdlib` and we didn't re-enable it for linking. Since this is tricky, we'll delete `-nostdlib` -`-nostdinc` was also pased to gcc, but we will need it for step 3. +`-nostdinc` was also pased to gcc, but we will need it for step 3, so let's delete it. + 2. kernel.c `main()` function ----------------------------- @@ -69,11 +70,23 @@ bit telling whether interrupts are on or off. In other words the interrupt handler automatically restores interrupts whether or not interrupts were enabled before this interrupt - On `cpu/isr.h`, `struct registers_t` has a couple issues. First, the alleged `esp` is renamed to `useless`. The value is useless because it has to do with the current stack context, not what was interrupted. Then, we rename `useresp` to `esp` -Finally, we add `cld` just before `call isr_handler` on `cpu/interrupt.asm` +We add `cld` just before `call isr_handler` on `cpu/interrupt.asm` as suggested +by the osdev wiki. + +There is a final, important issue with `cpu/interrupt.asm`. The common stubs create an instance +of `struct registers` on the stack and then call the C handler. But that breaks the ABI, since +the stack belongs to the called function and they may change them as they please. It is needed +to pass the struct as a pointer. + +To achieve this, edit `cpu/isr.h` and `cpu/isr.c` and change `registers_t r` into `registers_t *t`, +then, instead of accessing the fields of the struct via `.`, access the fields of the pointer via `->`. +Finally, in `cpu/interrupt.asm`, and add a `push esp` before calling both `isr_handler` and +`irq_handler` -- remember to also `pop eax` to clear the pointer afterwards. +Both current callbacks, the timer and the keyboard, also need to be changed to use a pointer to +`registers_t`. diff --git a/23-fixes/cpu/interrupt.asm b/23-fixes/cpu/interrupt.asm index a5d9636..4266b4e 100644 --- a/23-fixes/cpu/interrupt.asm +++ b/23-fixes/cpu/interrupt.asm @@ -13,13 +13,14 @@ isr_common_stub: mov es, ax mov fs, ax mov gs, ax - + push esp ; registers_t *r ; 2. Call C handler cld ; C code following the sysV ABI requires DF to be clear on function entry call isr_handler ; 3. Restore state pop eax + pop eax mov ds, ax mov es, ax mov fs, ax @@ -39,9 +40,11 @@ irq_common_stub: mov es, ax mov fs, ax mov gs, ax + push esp cld call irq_handler ; Different than the ISR code pop ebx ; Different than the ISR code + pop ebx mov ds, bx mov es, bx mov fs, bx @@ -110,218 +113,186 @@ global irq15 ; 0: Divide By Zero Exception isr0: - cli push byte 0 push byte 0 jmp isr_common_stub ; 1: Debug Exception isr1: - cli push byte 0 push byte 1 jmp isr_common_stub ; 2: Non Maskable Interrupt Exception isr2: - cli push byte 0 push byte 2 jmp isr_common_stub ; 3: Int 3 Exception isr3: - cli push byte 0 push byte 3 jmp isr_common_stub ; 4: INTO Exception isr4: - cli push byte 0 push byte 4 jmp isr_common_stub ; 5: Out of Bounds Exception isr5: - cli push byte 0 push byte 5 jmp isr_common_stub ; 6: Invalid Opcode Exception isr6: - cli push byte 0 push byte 6 jmp isr_common_stub ; 7: Coprocessor Not Available Exception isr7: - cli push byte 0 push byte 7 jmp isr_common_stub ; 8: Double Fault Exception (With Error Code!) isr8: - cli push byte 8 jmp isr_common_stub ; 9: Coprocessor Segment Overrun Exception isr9: - cli push byte 0 push byte 9 jmp isr_common_stub ; 10: Bad TSS Exception (With Error Code!) isr10: - cli push byte 10 jmp isr_common_stub ; 11: Segment Not Present Exception (With Error Code!) isr11: - cli push byte 11 jmp isr_common_stub ; 12: Stack Fault Exception (With Error Code!) isr12: - cli push byte 12 jmp isr_common_stub ; 13: General Protection Fault Exception (With Error Code!) isr13: - cli push byte 13 jmp isr_common_stub ; 14: Page Fault Exception (With Error Code!) isr14: - cli push byte 14 jmp isr_common_stub ; 15: Reserved Exception isr15: - cli push byte 0 push byte 15 jmp isr_common_stub ; 16: Floating Point Exception isr16: - cli push byte 0 push byte 16 jmp isr_common_stub ; 17: Alignment Check Exception isr17: - cli push byte 0 push byte 17 jmp isr_common_stub ; 18: Machine Check Exception isr18: - cli push byte 0 push byte 18 jmp isr_common_stub ; 19: Reserved isr19: - cli push byte 0 push byte 19 jmp isr_common_stub ; 20: Reserved isr20: - cli push byte 0 push byte 20 jmp isr_common_stub ; 21: Reserved isr21: - cli push byte 0 push byte 21 jmp isr_common_stub ; 22: Reserved isr22: - cli push byte 0 push byte 22 jmp isr_common_stub ; 23: Reserved isr23: - cli push byte 0 push byte 23 jmp isr_common_stub ; 24: Reserved isr24: - cli push byte 0 push byte 24 jmp isr_common_stub ; 25: Reserved isr25: - cli push byte 0 push byte 25 jmp isr_common_stub ; 26: Reserved isr26: - cli push byte 0 push byte 26 jmp isr_common_stub ; 27: Reserved isr27: - cli push byte 0 push byte 27 jmp isr_common_stub ; 28: Reserved isr28: - cli push byte 0 push byte 28 jmp isr_common_stub ; 29: Reserved isr29: - cli push byte 0 push byte 29 jmp isr_common_stub ; 30: Reserved isr30: - cli push byte 0 push byte 30 jmp isr_common_stub ; 31: Reserved isr31: - cli push byte 0 push byte 31 jmp isr_common_stub diff --git a/23-fixes/cpu/isr.c b/23-fixes/cpu/isr.c index 6e43fb2..72eee46 100644 --- a/23-fixes/cpu/isr.c +++ b/23-fixes/cpu/isr.c @@ -116,13 +116,13 @@ char *exception_messages[] = { "Reserved" }; -void isr_handler(registers_t r) { +void isr_handler(registers_t *r) { kprint("received interrupt: "); char s[3]; - int_to_ascii(r.int_no, s); + int_to_ascii(r->int_no, s); kprint(s); kprint("\n"); - kprint(exception_messages[r.int_no]); + kprint(exception_messages[r->int_no]); kprint("\n"); } @@ -130,15 +130,15 @@ void register_interrupt_handler(uint8_t n, isr_t handler) { interrupt_handlers[n] = handler; } -void irq_handler(registers_t r) { +void irq_handler(registers_t *r) { /* After every interrupt we need to send an EOI to the PICs * or they will not send another interrupt again */ - if (r.int_no >= 40) port_byte_out(0xA0, 0x20); /* slave */ + if (r->int_no >= 40) port_byte_out(0xA0, 0x20); /* slave */ port_byte_out(0x20, 0x20); /* master */ /* Handle the interrupt in a more modular way */ - if (interrupt_handlers[r.int_no] != 0) { - isr_t handler = interrupt_handlers[r.int_no]; + if (interrupt_handlers[r->int_no] != 0) { + isr_t handler = interrupt_handlers[r->int_no]; handler(r); } } diff --git a/23-fixes/cpu/isr.h b/23-fixes/cpu/isr.h index 03a16d6..88fff58 100644 --- a/23-fixes/cpu/isr.h +++ b/23-fixes/cpu/isr.h @@ -71,7 +71,13 @@ extern void irq15(); #define IRQ14 46 #define IRQ15 47 -/* Struct which aggregates many registers */ +/* Struct which aggregates many registers. + * It matches exactly the pushes on interrupt.asm. From the bottom: + * - Pushed by the processor automatically + * - `push byte`s on the isr-specific code: error code, then int number + * - All the registers by pusha + * - `push eax` whose lower 16-bits contain DS + */ typedef struct { uint32_t ds; /* Data segment selector */ uint32_t edi, esi, ebp, useless, ebx, edx, ecx, eax; /* Pushed by pusha. */ @@ -80,10 +86,10 @@ typedef struct { } registers_t; void isr_install(); -void isr_handler(registers_t r); +void isr_handler(registers_t *r); void irq_install(); -typedef void (*isr_t)(registers_t); +typedef void (*isr_t)(registers_t*); void register_interrupt_handler(uint8_t n, isr_t handler); #endif diff --git a/23-fixes/cpu/timer.c b/23-fixes/cpu/timer.c index 2203463..d241b88 100644 --- a/23-fixes/cpu/timer.c +++ b/23-fixes/cpu/timer.c @@ -5,7 +5,7 @@ uint32_t tick = 0; -static void timer_callback(registers_t regs) { +static void timer_callback(registers_t *regs) { tick++; UNUSED(regs); } diff --git a/23-fixes/drivers/keyboard.c b/23-fixes/drivers/keyboard.c index 31461f2..9ec77c0 100644 --- a/23-fixes/drivers/keyboard.c +++ b/23-fixes/drivers/keyboard.c @@ -25,7 +25,7 @@ const char sc_ascii[] = { '?', '?', '1', '2', '3', '4', '5', '6', 'H', 'J', 'K', 'L', ';', '\'', '`', '?', '\\', 'Z', 'X', 'C', 'V', 'B', 'N', 'M', ',', '.', '/', '?', '?', '?', ' '}; -static void keyboard_callback(registers_t regs) { +static void keyboard_callback(registers_t *regs) { /* The PIC leaves us the scancode in port 0x60 */ uint8_t scancode = port_byte_in(0x60); diff --git a/23-fixes/kernel/kernel.c b/23-fixes/kernel/kernel.c index 9d44cd2..3904c26 100644 --- a/23-fixes/kernel/kernel.c +++ b/23-fixes/kernel/kernel.c @@ -9,6 +9,9 @@ void kernel_main() { isr_install(); irq_install(); + asm("int $2"); + asm("int $3"); + kprint("Type something, it will go through the kernel\n" "Type END to halt the CPU or PAGE to request a kmalloc()\n> "); }