OSDev.org
https://forum.osdev.org/

Unable to set up GDT properly
https://forum.osdev.org/viewtopic.php?f=1&t=33680
Page 1 of 1

Author:  KSMb [ Fri May 03, 2019 1:00 pm ]
Post subject:  Unable to set up GDT properly

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.

Author:  quirck [ Fri May 03, 2019 1:33 pm ]
Post subject:  Re: Unable to set up GDT properly

In gdt_set_gate limit_low is assigned twice in a row

Author:  MichaelPetch [ Fri May 03, 2019 4:55 pm ]
Post subject:  Re: Unable to set up GDT properly

Quirck is correct. You have this line in gdt.c:
Code:
gdt_entries[num].limit_low = (limit >> 16) & 0x0F
when it should be
Code:
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.

Author:  KSMb [ Sat May 04, 2019 9:31 am ]
Post subject:  Re: Unable to set up GDT properly

Thanks for the replies!

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

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.

Author:  Octocontrabass [ Sat May 04, 2019 11:22 am ]
Post subject:  Re: Unable to set up GDT properly

KSMb wrote:
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.)

KSMb wrote:
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?

Author:  MichaelPetch [ Sat May 04, 2019 12:53 pm ]
Post subject:  Re: Unable to set up GDT properly

Octo beat me to it, and you can find a discussion/solution about the issue here: viewtopic.php?f=1&t=33636&p=289580&hilit=+registers_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:
void isr_handler(registers_t *regs);
and in kernel/arch/i386/isrs.c your isr_handler function should be:
Code:
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:
// 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:
    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:
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;

Author:  KSMb [ Sat May 04, 2019 12:58 pm ]
Post subject:  Re: Unable to set up GDT properly

Octocontrabass wrote:
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.

Octocontrabass wrote:
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:
isr_handler
does no longer get called. :(

Octocontrabass wrote:
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:
atoi()
function and now i can see decimal outputs correctly(thanks osdev.org wiki).

As usual, this is the repository

Author:  MichaelPetch [ Sat May 04, 2019 1:16 pm ]
Post subject:  Re: Unable to set up GDT properly

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

Author:  KSMb [ Sat May 04, 2019 5:08 pm ]
Post subject:  Re: Unable to set up GDT properly

MichaelPetch wrote:
Octo beat me to it, and you can find a discussion/solution about the issue here: viewtopic.php?f=1&t=33636&p=289580&hilit=+registers_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:
void isr_handler(registers_t *regs);
and in kernel/arch/i386/isrs.c your isr_handler function should be:
Code:
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:
// 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:
    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:
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.

Page 1 of 1 All times are UTC - 6 hours
Powered by phpBB © 2000, 2002, 2005, 2007 phpBB Group
http://www.phpbb.com/