From aa7a78349b50009962972ded5ac4e2c917cea13d Mon Sep 17 00:00:00 2001 From: Carlos Date: Tue, 18 Aug 2015 11:44:24 +0200 Subject: [PATCH] Lesson 23, step 6 --- 23-fixes/README.md | 21 ++++++++++++++++++--- 23-fixes/cpu/idt.c | 2 +- 23-fixes/cpu/interrupt.asm | 19 +------------------ 23-fixes/cpu/isr.h | 4 ++-- 23-fixes/cpu/ports.c | 8 ++++---- 5 files changed, 26 insertions(+), 28 deletions(-) diff --git a/23-fixes/README.md b/23-fixes/README.md index a64b32c..6797b20 100644 --- a/23-fixes/README.md +++ b/23-fixes/README.md @@ -39,6 +39,9 @@ C99 introduces standard fixed-width data types like `uint32_t` We need to include `` which works even in `-ffreestanding` (but requires stdlibs) and use those data types instead of our own, then delete them on `type.h` +We also delete the underscores around `__asm__` and `__volatile__` since they aren't needed. + + 4. Improperly aligned `kmalloc` ------------------------------- @@ -53,12 +56,24 @@ For now, it will always return a new page-aligned memory block. 5. Missing functions -------------------- +We will implement the missing `mem*` functions in following lessons + + 6. Interrupt handlers --------------------- -- also cli, sti in interrupt handlers +`cli` is redundant, since we already established on the IDT entries if interrupts +are enabled within a handler using the `idt_gate_t` flags. + +`sti` is also redundant, as `iret` loads the eflags value from the stack, which contains a +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 -7. Structs and attributes -------------------------- +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` diff --git a/23-fixes/cpu/idt.c b/23-fixes/cpu/idt.c index ee08cca..4d19108 100644 --- a/23-fixes/cpu/idt.c +++ b/23-fixes/cpu/idt.c @@ -13,5 +13,5 @@ void set_idt() { idt_reg.base = (uint32_t) &idt; idt_reg.limit = IDT_ENTRIES * sizeof(idt_gate_t) - 1; /* Don't make the mistake of loading &idt -- always load &idt_reg */ - __asm__ __volatile__("lidtl (%0)" : : "r" (&idt_reg)); + asm volatile("lidtl (%0)" : : "r" (&idt_reg)); } diff --git a/23-fixes/cpu/interrupt.asm b/23-fixes/cpu/interrupt.asm index bf83b7a..895914f 100644 --- a/23-fixes/cpu/interrupt.asm +++ b/23-fixes/cpu/interrupt.asm @@ -15,6 +15,7 @@ isr_common_stub: mov gs, ax ; 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 @@ -25,7 +26,6 @@ isr_common_stub: mov gs, ax popa add esp, 8 ; Cleans up the pushed error code and pushed ISR number - sti iret ; pops 5 things at once: CS, EIP, EFLAGS, SS, and ESP ; Common IRQ code. Identical to ISR code except for the 'call' @@ -47,7 +47,6 @@ irq_common_stub: mov gs, bx popa add esp, 8 - sti iret ; We don't get information about which interrupt was caller @@ -328,97 +327,81 @@ isr31: ; IRQ handlers irq0: - cli push byte 0 push byte 32 jmp irq_common_stub irq1: - cli push byte 1 push byte 33 jmp irq_common_stub irq2: - cli push byte 2 push byte 34 jmp irq_common_stub irq3: - cli push byte 3 push byte 35 jmp irq_common_stub irq4: - cli push byte 4 push byte 36 jmp irq_common_stub irq5: - cli push byte 5 push byte 37 jmp irq_common_stub irq6: - cli push byte 6 push byte 38 jmp irq_common_stub irq7: - cli push byte 7 push byte 39 jmp irq_common_stub irq8: - cli push byte 8 push byte 40 jmp irq_common_stub irq9: - cli push byte 9 push byte 41 jmp irq_common_stub irq10: - cli push byte 10 push byte 42 jmp irq_common_stub irq11: - cli push byte 11 push byte 43 jmp irq_common_stub irq12: - cli push byte 12 push byte 44 jmp irq_common_stub irq13: - cli push byte 13 push byte 45 jmp irq_common_stub irq14: - cli push byte 14 push byte 46 jmp irq_common_stub irq15: - cli push byte 15 push byte 47 jmp irq_common_stub diff --git a/23-fixes/cpu/isr.h b/23-fixes/cpu/isr.h index 6ef6789..03a16d6 100644 --- a/23-fixes/cpu/isr.h +++ b/23-fixes/cpu/isr.h @@ -74,9 +74,9 @@ extern void irq15(); /* Struct which aggregates many registers */ typedef struct { uint32_t ds; /* Data segment selector */ - uint32_t edi, esi, ebp, esp, ebx, edx, ecx, eax; /* Pushed by pusha. */ + uint32_t edi, esi, ebp, useless, ebx, edx, ecx, eax; /* Pushed by pusha. */ uint32_t int_no, err_code; /* Interrupt number and error code (if applicable) */ - uint32_t eip, cs, eflags, useresp, ss; /* Pushed by the processor automatically */ + uint32_t eip, cs, eflags, esp, ss; /* Pushed by the processor automatically */ } registers_t; void isr_install(); diff --git a/23-fixes/cpu/ports.c b/23-fixes/cpu/ports.c index 5dde2be..9582dde 100644 --- a/23-fixes/cpu/ports.c +++ b/23-fixes/cpu/ports.c @@ -13,7 +13,7 @@ uint8_t port_byte_in (uint16_t port) { * * Inputs and outputs are separated by colons */ - __asm__("in %%dx, %%al" : "=a" (result) : "d" (port)); + asm("in %%dx, %%al" : "=a" (result) : "d" (port)); return result; } @@ -23,15 +23,15 @@ void port_byte_out (uint16_t port, uint8_t data) { * However we see a comma since there are two variables in the input area * and none in the 'return' area */ - __asm__ __volatile__("out %%al, %%dx" : : "a" (data), "d" (port)); + asm volatile("out %%al, %%dx" : : "a" (data), "d" (port)); } uint16_t port_word_in (uint16_t port) { uint16_t result; - __asm__("in %%dx, %%ax" : "=a" (result) : "d" (port)); + asm("in %%dx, %%ax" : "=a" (result) : "d" (port)); return result; } void port_word_out (uint16_t port, uint16_t data) { - __asm__ __volatile__("out %%ax, %%dx" : : "a" (data), "d" (port)); + asm volatile("out %%ax, %%dx" : : "a" (data), "d" (port)); }