Reading bad data from keyboard

[iansjack]

It seems to me that your problem is not just to clean up your project but a lack of knowledge. I do hope your "clean up" involves all the project members taking some time off to read about OS development and the x86 processor. I don't mean reading tutorials from the internet, or cut and pasting code; I mean read some real books on the subject.

[austanss]

It seems to me that your problem is not just to clean up your project but a lack of knowledge. I do hope your "clean up" involves all the project members taking some time off to read about OS development and the x86 processor. I don't mean reading tutorials from the internet, or cut and pasting code; I mean read some real books on the subject.

We switched build systems, then one of our most knowledgable (relatively speaking) left the project. Seems we are going to have to do that. xD

[MichaelPetch]

I mentioned this problem in another thread and I still see in the latest version of the branch in Github you haven't resolved it. But your interrupt handlers (irq0 to irq15) are not being called because you don't actually have them in your IDT! You need to have an entry for each of IRQs like:

Code: Select all

idt_set_gate(32, (uint32_t)irq0, 0x08, 0x8E);
idt_set_gate(33, (uint32_t)irq1, 0x08, 0x8E);
// And a similar entry for all 16 irq handlers ...
idt_set_gate(47, (uint32_t)irq15, 0x08, 0x8E);

The numbering for the IRQs starts at 32(0x20) because that is where you remapped them to (0x20 and 0x28). As it is you are likely receiving some kind of fault on each interrupt because you haven't set up any handlers for the IRQs.

I also happened to mention the following bug previously that still hasn't been resolved:

Code: Select all

void ISRHandler(Registers registers)
void IRQHandler(Registers registers)

may cause issues with this code clobbering the saved state of the registers on the stack potentially causing the handler to fail when it tries to exit from the handler back to the interrupted code. You really should amend your assembly code to pass the `registers` struct by reference and not by value. I happened to show how that can be done in a previous comment under your original question here: viewtopic.php?p=309795#p309795 . These should be handled this way:

Code: Select all

void ISRHandler(Registers &registers)
void IRQHandler(Registers &registers)

with the needed changes to the assembly code that calls these handlers.

[austanss]

I mentioned this problem in another thread and I still see in the latest version of the branch in Github you haven't resolved it. But your interrupt handlers (irq0 to irq15) are not being called because you don't actually have them in your IDT! You need to have an entry for each of IRQs like:

Code: Select all

idt_set_gate(32, (uint32_t)irq0, 0x08, 0x8E);
idt_set_gate(33, (uint32_t)irq1, 0x08, 0x8E);
// And a similar entry for all 16 irq handlers ...
idt_set_gate(47, (uint32_t)irq15, 0x08, 0x8E);

The numbering for the IRQs starts at 32(0x20) because that is where you remapped them to (0x20 and 0x28). As it is you are likely receiving some kind of fault on each interrupt because you haven't set up any handlers for the IRQs.

I also happened to mention the following bug previously that still hasn't been resolved:

Code: Select all

void ISRHandler(Registers registers)
void IRQHandler(Registers registers)

may cause issues with this code clobbering the saved state of the registers on the stack potentially causing the handler to fail when it tries to exit from the handler back to the interrupted code. You really should amend your assembly code to pass the `registers` struct by reference and not by value. I happened to show how that can be done in a previous comment under your original question here: viewtopic.php?p=309795#p309795 . These should be handled this way:

Code: Select all

void ISRHandler(Registers &registers)
void IRQHandler(Registers &registers)

with the needed changes to the assembly code that calls these handlers.

yea i did all that but my interrupts are still throwing constant exceptions

[Octocontrabass]

You should only get one exception, because you should halt the CPU as soon as you get the first exception.

Then you can check the registers you've printed on the screen to see what's wrong.

[MichaelPetch]

Your current GitHub doesn't have IDT entries for any of the IRQs, only the exceptions and interrupts less than 32 are present.

[austanss]

Your current GitHub doesn't have IDT entries for any of the IRQs, only the exceptions and interrupts less than 32 are present.

I've left the Novix project, I've created a clone in order to maintain my progress.

Why, you might ask? After all, I did state that it would be hard for me to leave them. However, as we were developing plans and such to move forward with the project in a singular direction, the owner of the repository, the person who founded the project was asked if he would prefer a microkernel or a monolithic kernel.

He doesn't know the difference.

Therefore, I've packed up and left. However, my current repo is can be found at https://github.com/microNET-OS/microCORE

[austanss]

I feel like the OS is trolling me at this point:

[MichaelPetch]

I saw your screenshot and I was thinking the interrupt numbers looked like you were printing the wrong memory location where the interrupt number was stored on the stack. I reviewed your new GitHub repository and see that you hooked all 16 of the IRQS into the interrupt table inside GDT.CPP. I also noticed you specified Registers being a reference & rather than by value, but you seem to have missed in my original comment on the matter that a change was needed in the assembly language function that calls the handler. You have to push the address of the structure on the stack. If you review my comment viewtopic.php?p=309795#p309795 you will see I added a `push esp` before calling IsrHandler:

Code: Select all

        cld
        push esp
        call ISRHandler
        pop eax

        pop eax

I also cleared the direction flag (CLD) to be compliant with the System V i386 C/C++ ABI requiring forward string direction. You will also note that there is an extra POP to throw away the the pointer(reference) pushed as the 1st parameter. You could replace the first POP EAX with ADD ESP, 4 but the end result is the same - the top dword on the stack is discarded.

At the time you didn't have the IRQ common stub. The same change has to be made there as well.

[austanss]

I saw your screenshot and I was thinking the interrupt numbers looked like you were printing the wrong memory location where the interrupt number was stored on the stack. I reviewed your new GitHub repository and see that you hooked all 16 of the IRQS into the interrupt table inside GDT.CPP. I also noticed you specified Registers being a reference & rather than by value, but you seem to have missed in my original comment on the matter that a change was needed in the assembly language function that calls the handler. You have to push the address of the structure on the stack. If you review my comment viewtopic.php?p=309795#p309795 you will see I added a `push esp` before calling IsrHandler:

Code: Select all

        cld
        push esp
        call ISRHandler
        pop eax

        pop eax

I also cleared the direction flag (CLD) to be compliant with the System V i386 C/C++ ABI requiring forward string direction. You will also note that there is an extra POP to throw away the the pointer(reference) pushed as the 1st parameter. You could replace the first POP EAX with ADD ESP, 4 but the end result is the same - the top dword on the stack is discarded.

At the time you didn't have the IRQ common stub. The same change has to be made there as well.

I fixed the code, and here is the debugging info (i finally printed it in hex).

EDIT: I fixed the error code to print in hex: it prints as 0xB82C.

[iansjack]

Your SS value is invalid.

[austanss]

Your SS value is invalid.

Hm.

Screenshot_20201019_124812.png (14.23 KiB) Viewed 1467 times

I noticed when I let the output spam enough, it breaks:

[MichaelPetch]

It isn't just SS but CS as well. It appears you are pointing to the data on the stack but your `Registers` structure doesn't map to what is on the stack. Can you update you Github project with whatever is the latest code you are using?

[austanss]

It isn't just SS but CS as well. It appears you are pointing to the data on the stack but your `Registers` structure doesn't map to what is on the stack. Can you update you Github project with whatever is the latest code you are using?

Done and done.

[MichaelPetch]

When I posted the assembly code changes it was just a snippet. The second POP EAX was just the one that already existed. You have 3 `POP EAX` where there should just be 2. It should be:

Code: Select all

isr_common_stub:
    ; 1. Save CPU state
	pusha ; Pushes edi,esi,ebp,esp,ebx,edx,ecx,eax
	mov ax, ds ; Lower 16-bits of eax = ds.
	push eax ; save the data segment descriptor
	mov ax, 0x10  ; kernel data segment descriptor
	mov ds, ax
	mov es, ax
	mov fs, ax
	mov gs, ax

    ; 2. Call C handler
    cld
    push esp
    call ISRHandler
    pop eax
    ; pop eax <--------------- removed this extra one
    ; 3. Restore state
	pop eax
	mov ds, ax
	mov es, ax
	mov fs, ax
	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

As well in one of my followup questions I pointed out you have to make the same change to your irq common stub routine as well which appears you haven't done yet. My original post on the matter was at a time when you only had a common stub for ISRs only.