OSDev.org

Octocontrabass wrote:Then the answer is yes, you did set the interrupt flag.

Are you receiving IRQs from any hardware other than the keyboard controller? For example, the timer is probably configured to raise IRQ0.

Octocontrabass wrote:Does your IRQ handler get called at all? Try running it in a debugger. You'll be able to set a breakpoint to see.

MichaelPetch wrote:You need to add entries to your IDT for the IRQs from 0x20 to 0x2f (32 to 47). Right now your interrupts aren't being redirected into a handler like the ISRs you already have. As well your going to run into problems if you build with optimizations. `CommonISRStub` builds the register structure by value and not by reference. This can cause the compiler to generate code that can corrupt the saved interrupt stack. `CommonISRStub` should push the address of the register structure as the first parameter to ISRHandler. Something like:

Code: Select all

CommonISRStub:
        pusha

        mov ax, ds
        push eax

        mov ax, 0x10
        mov ds, ax
        mov es, ax
        mov fs, ax
        mov gs, ax

        cld
        push esp
        call ISRHandler

        pop eax
        pop eax
        mov ds, ax
        mov es, ax
        mov fs, ax
        mov gs, ax

        popa
        add esp, 8
;       sti
        iret

Then ISRHandler should be defined this way:

Code: Select all

void ISRHandler(Registers& registers)

instead of

Code: Select all

void ISRHandler(Registers registers)

.

You also don't handle the EOIs properly in ISRHandler. You shouldn't put an STI at the end of CommonISRStub. And one serious concern I have is that your Linker script and startup code doesn't call the global constructors (the constructors of objects at global scope). This could cause serious problems for any objects that were supposed to have constructors initialize objects.

MichaelPetch wrote:I should have looked closer at the code. I made a false assumption looking at your GDT.cpp file and boot.S. I assumed that there is a global Terminal object in your system but you create it on the stack inside kernel_main. My assumption that there was a global `Terminal` that was already initialized was because in boot.S you call `loadGDT` that will setup the ISRs (and eventually a proper one for the timer and the keyboard), and then before you call `kernel_main` from boot.S you enable interrupts with `STI`.

If someone were to hit a key on the keyboard after you did `STI` (in boot.S) and before the constructor of `Terminal` is called in `kernel_main` then this code in ISRHandlerImpl:

Code: Select all

Terminal::instance->write(keycode);

will attempt to use `instance` before it has been set to anything which could cause problems.

Usually for a C++ singleton pattern most people create something like a static `instance()` function inside the class which checks to see if the static instance variable in the object is nullptr and if it is it internally creates a new copy of the object and returns that new pointer (and sets the instance variable with it) otherwise it just returns the instance variable. There is of course a cleaner way and that is for the `instance()` function to simply create a static object of itself and return that object reference. Ultimately you call `instance()` this way:

Code: Select all

Terminal::instance()->write(keycode);

This way you would be guaranteed to have a valid instance returned. This does have the disadvantage of potentially having more overhead on each call.

rizxt wrote:Would it also be possible, since kernel_main does stack-allocate the Terminal instance, that the Terminal instance is being destroyed after kernel_main ends?

Would it also be possible, since kernel_main does stack-allocate the Terminal instance, that the Terminal instance is being destroyed after kernel_main ends? Or do we not have luxuries like that in freestanding environments?

MichaelPetch wrote:

Would it also be possible, since kernel_main does stack-allocate the Terminal instance, that the Terminal instance is being destroyed after kernel_main ends? Or do we not have luxuries like that in freestanding environments?

The object on the stack would go out of scope as soon as `kernel_main` returns. Which means the destructor will be called and the space occupied by that object on the stack will be released. You won't be able to rely on any of the non static member variables in that object being valid anymore nor will the instance pointer be pointing to a valid object anymore (it would be pointing at the one that was on the stack). That will pose a problem in your code since you don't put an endless `hlt` loop in your `kernel_main` but let it return back to Boot.S to do the loop there.

There is a good chance that if you got interrupts going that and you don't fix the Terminal object and how you use it you may not see the output you are expecting in the ISRHandlerImpl (or it may just crash).

rizxt wrote:I don't know how I would run it in a debugger, and fwiw, some jackass stripped the build of it's debug symbols.

env CXX=clang++ CC=clang meson _build --buildtype debug

qemu-system-i386 -cdrom _build/NovaVita.iso -S -s -no-reboot -no-shutdown &

gdb ./_build/NovaCor/Kernel/NovaVita.kernel.debug \
        -ex 'target remote localhost:1234' \
        -ex 'layout src' \
        -ex 'layout regs' \
        -ex 'break _start' \
        -ex 'continue'