Unable to set up GDT properly

[KSMb]

Hi!
In my Operating System i have just implemented a GDT, an IDT and some ISRs; however i'm stuck trying to get GDT working.
The only thing i can see is that the OS keeps rebooting forever; i know there are many other threads in this forum about problems similar to this case, but i can't find any reply that solve my problem.
This is the GitHub repository where the OS is located...the folder structure is based on this Meaty Skeleton and i'm following James Molloy's tutorial to implements the whole project.

What i'm doing wrong? I've already checked the code and i haven't seen any obvious bug.

[quirck]

In gdt_set_gate limit_low is assigned twice in a row

[MichaelPetch]

Quirck is correct. You have this line in gdt.c:

Code: Select all

gdt_entries[num].limit_low = (limit >> 16) & 0x0F

when it should be

Code: Select all

gdt_entries[num].gran = (limit >> 16) & 0x0F

Your bug actually ends up restricting the limit to 64KiB and when you do the FAR JMP to set CS the memory above 0x100000 is outside the CS limit and you tripe fault. Of course any memory address through the other affected descriptors will fault on any memory address above 0xFFFF for the same reason.

[KSMb]

Thanks for the replies!

...so it seems that i didn't gave enough time looking for typing errors

However now i have another "problem": i've also implemented IDT and ISRs, but when i try to fire some interrupts(from the kernel) this function does not print the related message(from the array). If i try to print only the interrupt number it gaves me strange symbols and not the correct number, so i think it's a problem with the IDT itself.

[Octocontrabass]

when i try to fire some interrupts(from the kernel) this function does not print the related message(from the array).

Your isr_common_stub passes a pointer to a struct, but your isr_handler expects a struct. You won't get anything useful until you reconcile this difference. Keep in mind that the ABI allows C functions to clobber their arguments, so you probably want to use a pointer to a struct.

Also, there's no need for "call eax", the linker will ensure "call isr_handler" works correctly. (And while we're at it, save an instruction and use "push esp" to put the pointer on the stack.)

If i try to print only the interrupt number it gaves me strange symbols and not the correct number, so i think it's a problem with the IDT itself.

Even after you fix the above issue, this still won't work since you're not converting the integer to a string. Don't you get a compiler warning about passing an integer type to a function expecting a pointer?

[MichaelPetch]

Octo beat me to it, and you can find a discussion/solution about the issue here: viewtopic.php?f=1&t=33636&p=289580&hili ... _t#p289580

Upon further review of your code there is a mixture of a fix already in place. The parameters to irs_handler is passed properly, just your delcaration and prototype in the C code are wrong. In kernel/include/kernel/isrs.h it should be:

Code: Select all

void isr_handler(registers_t *regs);

and in kernel/arch/i386/isrs.c your isr_handler function should be:

Code: Select all

void isr_handler(registers_t *regs) {
   terminal_writestring("Received interrupt: ");
   terminal_writestring(interrupts_messages[(uint8_t)regs->int_no]); // Print message associated to num
   // terminal_writestring(regs->int_no);
   terminal_writestring("\n");
}

As Octo points out to use terminal_writestring to print an integer you'd have to write a function to do that (ie: itoa).

There is one other significant issue even if you fix the code above will still prevent the right interrupt number from being accessed properly. In kernel/include/kernel/isrs.h you have:

Code: Select all

 // This struct will be used to store all registers we've pushed
typedef struct registers {
    uint32_t ds; // Data segment selector
    uint32_t edi, esi, ebp, esp, ebx, edx, ecx, eax; // Pushed by pusha
    uint32_t int_no, err_code; // Interrupt number and error code
    uint32_t eip, cs, eflags, useresp, ss; // Pushed by the CPU itself
} registers_t;

The problem is that in kernel/arch/i386/boot.asm you do:

Code: Select all

    pusha ; Pushes EDI, ESI, EBP, ESP, EBX, EDX, ECX and EAX registers
    push ds
    push es
    push fs
    push gs

What you push has to exactly match the registers_t structure (in reverse order). You push DS, ES, FS, and GS and not just DS. Because these two things don't match you will be accessing data in registers_t at incorrect offsets. registers_t should look like:

Code: Select all

typedef struct registers {
    uint32_t gs, fs, es, ds; // Segment selectors
    uint32_t edi, esi, ebp, esp, ebx, edx, ecx, eax; // Pushed by pusha
    uint32_t int_no, err_code; // Interrupt number and error code
    uint32_t eip, cs, eflags, useresp, ss; // Pushed by the CPU itself
} registers_t;

[KSMb]

Your isr_common_stub passes a pointer to a struct, but your isr_handler expects a struct. You won't get anything useful until you reconcile this difference. Keep in mind that the ABI allows C functions to clobber their arguments, so you probably want to use a pointer to a struct.

Ok...i've changed but it fire the same interrupt every time.

Also, there's no need for "call eax", the linker will ensure "call isr_handler" works correctly. (And while we're at it, save an instruction and use "push esp" to put the pointer on the stack.)

I've tried to remove it, but

Code: Select all

isr_handler

does no longer get called.

Even after you fix the above issue, this still won't work since you're not converting the integer to a string. Don't you get a compiler warning about passing an integer type to a function expecting a pointer?

I've imported a

Code: Select all

atoi()

function and now i can see decimal outputs correctly(thanks osdev.org wiki).

As usual, this is the repository

[MichaelPetch]

Please see the edit in my last post for other issues that need resolving.

[KSMb]

Octo beat me to it, and you can find a discussion/solution about the issue here: viewtopic.php?f=1&t=33636&p=289580&hili ... _t#p289580

Upon further review of your code there is a mixture of a fix already in place. The parameters to irs_handler is passed properly, just your delcaration and prototype in the C code are wrong. In kernel/include/kernel/isrs.h it should be:

Code: Select all

void isr_handler(registers_t *regs);

and in kernel/arch/i386/isrs.c your isr_handler function should be:

Code: Select all

void isr_handler(registers_t *regs) {
   terminal_writestring("Received interrupt: ");
   terminal_writestring(interrupts_messages[(uint8_t)regs->int_no]); // Print message associated to num
   // terminal_writestring(regs->int_no);
   terminal_writestring("\n");
}

As Octo points out to use terminal_writestring to print an integer you'd have to write a function to do that (ie: itoa).

There is one other significant issue even if you fix the code above will still prevent the right interrupt number from being accessed properly. In kernel/include/kernel/isrs.h you have:

Code: Select all

 // This struct will be used to store all registers we've pushed
typedef struct registers {
    uint32_t ds; // Data segment selector
    uint32_t edi, esi, ebp, esp, ebx, edx, ecx, eax; // Pushed by pusha
    uint32_t int_no, err_code; // Interrupt number and error code
    uint32_t eip, cs, eflags, useresp, ss; // Pushed by the CPU itself
} registers_t;

The problem is that in kernel/arch/i386/boot.asm you do:

Code: Select all

    pusha ; Pushes EDI, ESI, EBP, ESP, EBX, EDX, ECX and EAX registers
    push ds
    push es
    push fs
    push gs

What you push has to exactly match the registers_t structure (in reverse order). You push DS, ES, FS, and GS and not just DS. Because these two things don't match you will be accessing data in registers_t at incorrect offsets. registers_t should look like:

Code: Select all

typedef struct registers {
    uint32_t gs, fs, es, ds; // Segment selectors
    uint32_t edi, esi, ebp, esp, ebx, edx, ecx, eax; // Pushed by pusha
    uint32_t int_no, err_code; // Interrupt number and error code
    uint32_t eip, cs, eflags, useresp, ss; // Pushed by the CPU itself
} registers_t;

Thanks for your time...now everything works fine! Really appreciated.