klange / toaruos

A completely-from-scratch hobby operating system: bootloader, kernel, drivers, C library, and userspace including a composited graphical UI, dynamic linker, syntax-highlighting text editor, network stack, etc.
https://toaruos.org/
University of Illinois/NCSA Open Source License
6.13k stars 487 forks source link

Suboptimal assembly #75

Closed sortie closed 9 years ago

sortie commented 9 years ago

A few minor nits of the assembly in start.s:

cli in start is unnecessary. The multiboot standard requires interrupts to be disabled when execution is transferred to the kernel.

The ISR_NOERR, ISR_ERR, IRQ_ENTRY macros disable interrupts immediately. However, the IDT has a bit that controls whether interrupts are left alone or disabled. Every IDT entry in the kernel are set to disabled, as they should be. Therefore the cli instructions are unnecessary as interrupts are already disabled at this point. This seems to be a leftover from the original tutorial the code was made from.

The isr_common_stub and irq_common_stub common stubs are identical except the higher level C function. It would be nicer to combine them into a single common stub that calls a single C function and let that function do the switch to the proper handler.

There's a few odd suboptimal things in the common stubs, such as using a function pointer, and not just pushing esp directly. They can be simplified as such:

-       mov eax, esp
-       push eax
+       push esp
        ; Call the C kernel fault handler
-       mov eax, fault_handler
-       call eax
-       pop eax
+       call fault_handler
+       add esp, 4