Enabling interrupts is crashing OS

[Coalby]

Hello! This is my first post with this community and I am very excited to be surrounded by folks with a serious in-depth knowledge of this computing domain.

I am having issues with enabling interrupts for my operating system, IcarusOS. It seems to be only crashing when I enable interrupts. I have checked thoroughly through my GDT, IDT, and IRQ handlers and I cannot seem to find anything that is definitively wrong with the initialization. I compiled a brief list of some things I have gone through in order to debug the system with no success.

• Everything runs fine when stepping through QEMU with GDB. Hangs on the infinite loop in kernel_main indefinitely. When I use the command continue without any breakpoints, however, QEMU crashes.
• I have stepped through and made sure that GDT is set up properly. While I believe the setup is accurate, I'm open to feedback if any adjustments are needed. I've included a screenshot of the GDT setup for reference

  

  Capture.PNG (2.67 KiB) Viewed 5805 times

• The "I can't get interrupts working" page refers to accidentally setting up timer interrupt on vector 8, https://wiki.osdev.org/I_Can't_Get_Inte ... interrupts. I do not think this is my case, however, because the PIC remapping I did was very much identical to the wiki page's remapping.
• The irq routines seems to point to the handler function for the timer, as shown by GDB.

  

  Capture2.PNG (2.8 KiB) Viewed 5805 times

• It does not seem to crash QEMU until an interrupt is called for the PIT.

The source code can be found at https://github.com/Coalby/IcarusOS. I added the object files and iso image before posting this because I thought it would save you guys some time if you want to run it locally. I apologize in advance for the messy code. I have been trying to write cleaner code lately and will be actively trying to fix it up this weekend. Any help would be greatly appreciated! Please let me know if I can give any more information that can help you.

[nullplan]

So I had a look at your interrupt code, because I bet the problem's there. Sure enough, _pushalgen, _pushald, _popalgen, and _popald cannot work. You defined them as functions, but they push or pop values and then return. Return is not a magic instruction that knows where the corresponding call was, it is essentially just "pop EIP". So stack manip will not work this way. The things you have written might work as macros, though.

Also, to help you out with your "idt stub table", where you are apparently trying to list all ISRs in order: Well, I wouldn't do that, exactly. If you insist on handling all ISRs generically, one way would be to actually have the code pointers constant offsets from each other. Let's see here... You can install all gates as interrupt gates (which you should do anyway, and I notice you are already doing) then there is no need for the CLI instruction. So the stub for each ISR is just possibly a push 0, then a push vector and jmp isr. I'd suggest you actually completely fill up the IDT, which will make it easier later on to support APIC and MSI. For now, handle all unknown interrupts by panicking.

So the jmp isr is two bytes if isr is within 128 bytes of the jmp instruction. Since there are going to be 256 entries in our table, each at least one byte large, this isn't going to happen. In protected mode, the only alternative is jmp with a 32-bit offset, which is five bytes. So that comes out to up to nine bytes per entry. Which is awkward to work with, so let's make it sixteen bytes. The you can define something along these lines:

Code: Select all

.p2align 4
.global isr0
isr0:
num=0
.rept 32
.p2align 4
.if ((1 << num) & 0x22ff00)
pushl $0
.endif
pushl $num-128
jmp isr
num=num+1
.endr

.rept 256-32
.p2align 4
pushl $0
pushl $num-128
jmp isr
num=num+1
.endr

Then in C you can define something like

Code: Select all

extern const char isr0[];

And then you know you have one ISR every 16 bytes from there. Easy to loop over.

[Coalby]

Hi nullplan! Thanks for responding so fast. I was able to set up the interrupt.s file like you had recommended and loaded the idt properly. When it calls interrupt_handler in handlers.c, however, the value of frame.interrupt is some ridiculously large number that I am positive is not right. Am I implementing this incorrectly? I pushed the new code onto the github repo.

[nullplan]

Ah, that is another common bug, often seen from copying one particularly buggy tutorial off the internet: Your function interrupt_handler takes as argument a copy of an object of type interruptFrame. This is likely not what you want since parameters are memory of the function, and the compiler can emit code to re-use the memory as it sees fit once you are done using it. Also I never learned the ABI for handing large structures to functions. Better to declare your function to be taking a pointer (in C. In C++ you'd want to use a reference). Then you have to adjust the call, of course, you have to change it to

Code: Select all

pushl %esp
call interrupt_handler
addl $4, %esp

(To push a pointer to the struct onto the stack as first argument, then remove it afterwards)

Doing this would uncover another bug: You are registering isr0 as ISR for all interrupts, because one line in init_idt should read

Code: Select all

set_idt_descriptor(vector, isr + 16 * vector, INT_GATE_ATTRIBUTE);

Actually, looking at your code history, I see that you used to have the pointer there, but the call was never right for the pointer.

[Octocontrabass]

So that comes out to up to nine bytes per entry.

You can do it in seven bytes if you get rid of the dummy error code and jump to a "pushl (%esp)" instruction to push the vector twice.

the value of frame.interrupt is some ridiculously large number that I am positive is not right.

You're subtracting 128 from the vector here, so you need to add 128 somewhere else to get the number you expect. (And yes, that needs to be "frame->interrupt". Also, you need to align the stack and clear the direction flag.)

[nullplan]

I also noticed another issue that perfectly underscores why you should not copy code written by Internet strangers into your project if you do not understand it. I made a mistake writing the code for the exception handlers. Look at the condition for when a synthetic error code is pushed. Can you say what it does? And can you find the mistake I made? If not then a look into the AMD APM (or Intel SDM, you choice really) may help you.

I noticed it this morning while eating breakfast, but then it was too late; I had to get to work. So I'm writing this now that I am home again.

You can do it in seven bytes if you get rid of the dummy error code and jump to a "pushl (%esp)" instruction to push the vector twice.

That is also possible. Then the difference between exceptions with error code and those without is whether they jump to the pushl instruction or the one following.

As an adherent of the Torvalds school of interrupt handling, my interrupt frame struct only has a single augment slot (which contains the error code in exception handlers, the negated interrupt number in interrupt handlers, and the positive original syscall number in syscall handlers), and also I have completely different handlers for each exception and then a table for the interrupts, so I never had this problem to begin with. But I didn't want to overload the OP with too much too quickly, and also, my OS is 64-bit only, so some of the advice would not be applicable directly.