[SOLVED] Exception 13 thrown when initializing my kernel

[nullplan]

0x102 shifted right 3 bits would be index 0x20 which would be right if they had remapped the master pic to 0x20 and they got a timer interrupt.

Right, that does make more sense. Yeah, I was mixing up offset and index.

As for the size of the IDT, between APIC and MSI, any remotely modern OS should just completely fill the IDT (possibly sans the reserved exception vectors, but that doesn't save any memory). In 64-bit mode, that means the IDT fills up an entire page (which I know because I did the calculations for my own OS). In 32-bit mode, the entries are only half as long, so that comes out to half a page, or 0x800 bytes (so limit ought to be 0x7ff). 0x4000 is excessive in any mode, since the architecture cannot support more than 256 interrupts.

It's a good idea to align the GDT, but it's not required. The GDTR is only used by the LGDT instruction, so there's no reason to bother aligning it.

True enough. Honestly, there is little reason to store the GDTR in the static data section at all. In 32-bit mode, it might even be a good idea to store it in the first descriptor, since it is shorter than a descriptor and the first one is never read by the CPU, so it can contain anything. Though personally I just construct the GDTR on stack for the lgdt instruction, but then, in 64-bit mode the GDTR no longer fits into a descriptor entry.

The standard suggestion for the OP remains the same: Keep the interrupt flag clear until you have found and initialized all interrupt controllers in the system (this requires reading ACPI tables). While initializing, mask out all interrupts (except IRQ 2 on the PIC pair, obviously). Only then is it safe to set the interrupt flag. And then unmask interrupts only as drivers request them.

[Octocontrabass]

Code: Select all

Program received signal SIGSEGV, Segmentation fault.

There's no such thing as SIGSEGV on bare metal. Are you sure you've attached GDB to QEMU's GDB stub?

[Kaius]

It looks like a lot of the confusing issues originated from the fact that I was using regular GDB, rather than attaching to Qemu (fixed now of course).

Changing that fixed the `lgdt` and `outb` exceptions, but the original problem still remains.

My Github page is up-to-date as of this post, and I've also added in some debugging methods. Right now, I'm trying to locate the exact instruction that's causing the exception.

Edit: It looks like the issue is originating from the `while (true)` loop in `main`. It will run a number of several hundred or thousand instructions before finally throwing the exception. I've been trying to pin it down (I took out the `keyboard_input` from the `while` loop) but it seems very random.

[MichaelPetch]

As per my earlier comment you still haven't initialized the IRQ handlers from 0x20 (32) to 0x2f (47). Without handlers for the timer, keyboard and all the other interrupts you have enabled any interrupt you do get will fault with #GP. As well, you need to do `init_pic` before `init_idt` since your `init_idt` does an STI. Your `init_pic` has a bug.

Code: Select all

  outb(PIC1_DATA, 4);
  io_wait();
  outb(PIC1_DATA, 2);
  io_wait();

should have been

Code: Select all

  outb(PIC1_DATA, 4);
  io_wait();
  outb(PIC2_DATA, 2);
  io_wait();

Octo identified some other issues earlier about the EOI function being wrong among other things.

[Kaius]

• You enable interrupts before you've set up your IDT.
• Your interrupt stubs have several problems. You're clobbering the interrupted program's state, you're not clearing the direction flag or aligning the stack, and you don't remove the error code before IRET.
• Your EOI function doesn't work.
• You enable all interrupt sources when you initialize the interrupt controllers, even though you're not prepared to handle any of them.
• Your compiler options seem wrong. You're passing -Wno-return-type even though that warning should be enabled to tell you about bugs in your code. You're not passing -mgeneral-regs-only even though you haven't initialized any extended registers.

I think I've fixed most of these, there's just one that I don't quite know how to fix right off the bat.

What do you mean by "Your interrupt stubs have several problems. You're clobbering the interrupted program's state, you're not clearing the direction flag or aligning the stack, and you don't remove the error code before IRET."? I got the stubs from The Wiki I think, and I don't see anything immediately wrong with them. I'm also adding to the stack pointer to offset the parameter that I add. I don't know what you mean by aligning the stack in this scenario, and I don't understand how I'm clobbering the program's state.

Could someone explain what this is supposed to mean? I don't quite understand that point. Other than that, though, I fixed all of the other issues pointed out, I appreciate the help seeing where I went wrong.

[nullplan]

Your interrupt stubs do not save the call-clobbered registers (which for the i386 SysV ABI would be EAX, ECX, and EDX), which means they get lost. If the main program outside of the interrupt handler was using those, the registers are now gone. Stuff like this leads to hard-to-debug problems. Therefore it is important to fix them immediately. I would suggest to just save all registers, which a handy "pusha" will do for you in 32-bit mode.

Another issue with those stubs is that they do not switch or save segments. In 32-bit mode that is actually somewhat important. Right now you only have two segments, but that will change, right? The interrupt transition will set CS to the value from the IDT entry, and, if a privilege change happened, SS from the TSS. But DS, ES, FS, and GS remain unchanged

Another issue is that your macro "isr_err_stub" is not actually different from "is_noerr_stub". Seems to me like the latter should push a dummy value of 0 before anything else. Both should increase ESP by 8 at the end, to get rid of the error code, lest the IRET read wrong data.

Another issue is the direction flag: The interrupted code might have set it. Since the interrupt image on stack contains the old flags register, you can just issue a CLD instruction before calling the C handler, and the DF will return to its prior state after the IRET. The direction flag must be cleared before calling any C code, since the C ABI requires that the flag be cleared on entry and exit of any function.

Another issue is stack alignment. In 32-bit mode, the stack is not automatically aligned in an interrupt. The C ABI requires a stack alignment of 16 before the call instruction to enter another function.

It is great that you got your code from the Wiki, however, Wiki code ought to be treated as all other internet code: Untrusted until proven otherwise. It is entirely possible for Wiki code to be bad.

[MichaelPetch]

Nullplan pretty much covered it. Your idt.asm could be fixed up to look like

Code: Select all

%macro isr_err_stub 1
isr_stub_%+%1:
  push %1
  pusha
  push gs
  push fs
  push es
  push ds

  mov eax, 0x10                ; Set Segment selectors to kernel data selectors
  mov ds, eax
  mov es, eax
  mov fs, eax
  mov gs, eax

  cld
  mov ebx, esp                 ; EBX=Pointer to ISR state
  and esp, -16                 ; Align stack on 16 byte boundary
  sub esp, 12                  ; One param to handler. Must subtract additional 12 bytes
                               ;     so that at point of calling handler the stack
                               ;     is still on 16 byte boundary
  push ebx                     ; Param1 - Pointer to ISR state
  call exception_handler
  mov esp, ebx                 ; EBX is preserved, restore stack pointer
  pop ds
  pop es
  pop fs
  pop gs
  popa
  add esp, 8
  iret
%endmacro

%macro isr_no_err_stub 1
isr_stub_%+%1:
  push 0
  push %1
  pusha
  push gs
  push fs
  push es
  push ds

  mov eax, 0x10                ; Set Segment selectors to kernel data selectors
  mov ds, eax
  mov es, eax
  mov fs, eax
  mov gs, eax

  cld
  mov ebx, esp                 ; EBX=Pointer to ISR state
  and esp, -16                 ; Align stack on 16 byte boundary
  sub esp, 12                  ; One param to handler. Must subtract additional 12 bytes
                               ;     so that at point of calling handler the stack
                               ;     is still on 16 byte boundary
  push ebx                     ; Param1 - Pointer to ISR state
  call exception_handler
  mov esp, ebx                 ; EBX is preserved, restore stack pointer
  pop ds
  pop es
  pop fs
  pop gs
  popa
  add esp, 8
  iret
%endmacro

%macro irq_stub 1
isr_stub_%+%1:
  push 0
  push %1
  pusha
  push gs
  push fs
  push es
  push ds

  mov eax, 0x10                ; Set Segment selectors to kernel data selectors
  mov ds, eax
  mov es, eax
  mov fs, eax
  mov gs, eax

  cld
  mov ebx, esp                 ; EBX=Pointer to ISR state
  and esp, -16                 ; Align stack on 16 byte boundary
  sub esp, 12                  ; One param to handler. Must subtract additional 12 bytes
                               ;     so that at point of calling handler the stack
                               ;     is still on 16 byte boundary
  push ebx                     ; Param1 - Pointer to ISR state
  call irq_handler
  mov esp, ebx                 ; EBX is preserved, restore stack pointer
  pop ds
  pop es
  pop fs
  pop gs
  popa
  add esp, 8
  iret
%endmacro

extern exception_handler
isr_no_err_stub 0
isr_no_err_stub 1
isr_no_err_stub 2
isr_no_err_stub 3
isr_no_err_stub 4
isr_no_err_stub 5
isr_no_err_stub 6
isr_no_err_stub 7
isr_err_stub    8
isr_no_err_stub 9
isr_err_stub    10
isr_err_stub    11
isr_err_stub    12
isr_err_stub    13
isr_err_stub    14
isr_no_err_stub 15
isr_no_err_stub 16
isr_err_stub    17
isr_no_err_stub 18
isr_no_err_stub 19
isr_no_err_stub 20
isr_no_err_stub 21
isr_no_err_stub 22
isr_no_err_stub 23
isr_no_err_stub 24
isr_no_err_stub 25
isr_no_err_stub 26
isr_no_err_stub 27
isr_no_err_stub 28
isr_no_err_stub 29
isr_err_stub    30
isr_no_err_stub 31

extern irq_handler


%assign i 32
%rep 16
  irq_stub i
%assign i i+1
%endrep

; for performance i think
global isr_stub_table
isr_stub_table:
%assign i 0
%rep 48
    dd isr_stub_%+i
%assign i i+1
%endrep

The lone parameter passed to the handlers is a pointer to the stack state which includes the value of the interrupt no, error code, all the segment and general purpose registers etc. your idt.h would be amended to look like:

Code: Select all

#ifndef LIB_IDT_H
#define LIB_IDT_H

#include <stdint.h>

#define IDT_SIZE 256

typedef struct registers
{
    uint32_t ds, es, fs, gs;                         // 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 (if applicable)
    uint32_t eip, cs, eflags, useresp, ss;   // Pushed by the processor automatically.
} registers_t;

typedef void (*isr_t)(registers_t *);

typedef struct {
  uint16_t offset_low;  // lower 16 bits of handler func addr
  uint16_t selector;    // kernel segment selector
  uint8_t  reserved;    // this musts be zero
  uint8_t  attributes;  // type and attributes
  uint16_t offset_high; // higher 16 bits of handler func addr
} __attribute__((packed)) idt_entry_t;

typedef struct {
  uint16_t limit;
  uint32_t base;
} __attribute__((packed)) idtr_t;

extern idt_entry_t idt[IDT_SIZE];
extern idtr_t idtr;

void set_idt_entry(uint8_t index, uintptr_t handler_addr);
void idt_init();

void exception_handler(registers_t *state);
void irq_handler(registers_t *state);

#endif

Your idt.c would look like:

Code: Select all

#include <stdbool.h>
#include <stdint.h>
#include <lib/panic.h>
#include <lib/idt.h>
#include <drivers/pic.h>
#include <drivers/vga.h>
#include <drivers/keyboard.h>

__attribute__((aligned(0x10))) idt_entry_t idt[IDT_SIZE];
idtr_t idtr;

void set_idt_entry(uint8_t index, uintptr_t handler_addr) {
  idt[index].offset_low  = handler_addr & 0xffff;
  idt[index].selector    = 0x08; // kernel code segment
  idt[index].reserved    = 0;
  idt[index].attributes  = 0x8e; // interrupt gate
  idt[index].offset_high = (handler_addr >> 16) & 0xffff;
}

extern uintptr_t isr_stub_table[];

void idt_init() {
  idtr.base = (uint32_t) (uint8_t*) &idt[0];
  idtr.limit = (uint16_t) sizeof(idt) - 1;

  for (uint8_t i = 0; i < 48; i++) {
    idt_set_entry(i, isr_stub_table[i]);
  }

  __asm__ volatile ("lidt %0" : : "m"(idtr)); // load idt
  __asm__ volatile ("sti"); // enable interrupts
}

void exception_handler(registers_t *state) {
  panic(state->int_no, state->err_code);
}

void irq_handler(registers_t *state) {
  if (state->int_no == 33) keyboard_input();

  pic_end_int(state->int_no);
}

There is a bit of a bug in your EOI function. It should look like:

Code: Select all

void pic_end_int(uint8_t irq) {
  if (irq >= 40) outb(PIC2_CMD, 0x20);
  outb(PIC1_CMD, 0x20);
}

[Kaius]

Thank you for all of that, it was super helpful in giving me a good understanding of the way all of this works on a deeper level. This also helps a ton with my kernel panic function so I can dump registers and give more info for debugging.

[MichaelPetch]

No problem at all.

There was one subtle bug introduced in your commit. In the ISR stub handlers I did this:

Code: Select all

and esp, -16

The AND instruction is correct. -16 was the same as writing 0xfffffff0 which effectively rounds ESP down to the nearest 16 byte boundary by setting the last 4 bits to 0. I noticed you changed it to

Code: Select all

add esp, -16

which is not the same thing. As of GCC ~4.1 the ABI changed where the default 16 byte alignment (previously 4 byte) was needed at the point a function was called. More on this alignment can be found here: https://wiki.osdev.org/System_V_ABI#i386 .

[Kaius]

No problem at all.

There was one subtle bug introduced in your commit. In the ISR stub handlers I did this:

Code: Select all

and esp, -16

The AND instruction is correct. -16 was the same as writing 0xfffffff0 which effectively rounds ESP down to the nearest 16 byte boundary by setting the last 4 bits to 0. I noticed you changed it to

Code: Select all

add esp, -16

which is not the same thing. As of GCC ~4.1 the ABI changed where the default 16 byte alignment (previously 4 byte) was needed at the point a function was called. More on this alignment can be found here: https://wiki.osdev.org/System_V_ABI#i386 .

Yes, that was a mistake on my part - good catch!

On an unrelated note, in case anybody is interested, I think I'm going to implement a ring 0 terminal with hard-coded commands, then implement a filesystem, then probably move to ring 3 / start working with executables. If you have any tips on that process, I'd love to hear it, but for now I'm just doing tons of reading on the wiki and from external sources.

[Octocontrabass]

You can pass "-mpreferred-stack-boundary=2" to GCC if you want to use 4-byte alignment for your stack. You don't need 16-byte stack alignment when you're not using SSE in your kernel.

[nullplan]

You can pass "-mpreferred-stack-boundary=2" to GCC if you want to use 4-byte alignment for your stack. You don't need 16-byte stack alignment when you're not using SSE in your kernel.

OK, but the difference between "and esp, -16" and "and esp, -4" is too small for me to start investigating nonstandard GCC options.

[Octocontrabass]

It's standard enough that Linux uses it, and it saves up to 12 bytes of stack space per function call. (Linux also uses "-mregparm=3" and "-freg-struct-return" to further reduce stack usage.)