Cannot get IRQs enabled

barahir · **Joined:** Wed Apr 10, 2019 2:02 am **Posts:** 5

Hello,

I am currently trying to build my first OS with the help of JamesM's tutorial and the OSDev wiki. I run my code with qemu-system-i386.

I have managed to load the GDT and the IDT (and so far they seem to be correct), but, although I have remapped the PIC and filled the IDT accordingly, I cannot get the IRQs to work. As advised on the wiki, I have checked that I receive software interrupts, and I have enabled all the IRQs on the PIC mask.

Here is my code :

Code:

/* ---------- kernel.c ---------- */
void kernel_main(void) {
  terminal_initialize();

  gdt_init();
  idt_init();
  timer_init(50);
}

Code:

/* ---------- descr_tbl.c ---------- */
#include <stdint.h>
#include <stddef.h>
#include <stdio.h>
#include <string.h>

#include <kernel/descrtbl.h>

// ------------------------------ GDT ------------------------------ //
gdt_entry_t gdt[5];
gdt_ptr_t   gdt_ptr;

void gdt_setentry(size_t   num,
        uint32_t base,
        uint32_t limit,
        uint8_t  access,
        uint8_t  granularity) {
  // See https://wiki.osdev.org/GDT
  gdt[num].base_low    = (base & 0xFFFF);
  gdt[num].base_middle = (base >> 16) & 0xFF;
  gdt[num].base_high   = (base >> 24) & 0xFF;

  gdt[num].limit_low   = (limit & 0xFFFF);
  gdt[num].granularity = (limit >> 16) & 0x0F;

  gdt[num].granularity |= granularity & 0xF0;
  gdt[num].access      = access;
}

// In descrtbl_lib.S
extern void gdt_flush(uint32_t);

void gdt_init(void) {
  gdt_ptr.limit = (sizeof(gdt_entry_t) * 5) - 1;
  gdt_ptr.base  = (uint32_t) &gdt;

  gdt_setentry(0, 0, 0, 0, 0);                // Null segment
  gdt_setentry(1, 0, 0xFFFFFFFF, 0x9A, 0xCF); // Code segment
  gdt_setentry(2, 0, 0xFFFFFFFF, 0x92, 0xCF); // Data segment
  gdt_setentry(3, 0, 0xFFFFFFFF, 0xFA, 0xCF); // User mode code segment
  gdt_setentry(4, 0, 0xFFFFFFFF, 0xF2, 0xCF); // User mode data segment

  gdt_flush((uint32_t) &gdt_ptr);
}

// ------------------------------ IDT ------------------------------ //

#define MASTER_PIC_CMD 0x20
#define MASTER_PIC_DATA 0x21
#define SLAVE_PIC_CMD 0xA0
#define SLAVE_PIC_DATA 0xA1

idt_entry_t idt[256];
idt_ptr_t   idt_ptr;

void idt_setentry(uint8_t num,
        uint32_t base,
        uint16_t selector,
        uint8_t flags) {
  // See https://wiki.osdev.org/IDT
  idt[num].base_lo = base & 0xFFFF;
  idt[num].base_hi = (base >> 16) & 0xFFFF;

  idt[num].selector = selector;
  idt[num].always0  = 0;
  idt[num].flags    = flags /* | 0x60 in user mode */;
}

// In descrtbl_lib.S
extern void idt_flush(uint32_t);

void idt_init(void) {
  idt_ptr.limit = sizeof(idt_entry_t) * 256 - 1;
  idt_ptr.base  = (uint32_t) &idt;

  memset(&idt, 0, sizeof(idt_entry_t) * 256);

  // Load the exceptions : see https://wiki.osdev.org/Exceptions
  idt_setentry(0, (uint32_t) isr0, 0x08, 0x8E);
  ...
  idt_setentry(31, (uint32_t) isr31, 0x08, 0x8E);

  // Initialize the PICs
  outb(MASTER_PIC_CMD,  0x11);
  io_wait();
  outb(SLAVE_PIC_CMD,   0x11);
  io_wait();
  // Give the vector offset
  outb(MASTER_PIC_DATA, 0x20);
  io_wait();
  outb(SLAVE_PIC_DATA,  0x28);
  io_wait();
  // Acknowledge the other PIC
  outb(MASTER_PIC_DATA, 0x04);
  io_wait();
  outb(SLAVE_PIC_DATA,  0x02);
  io_wait();
  // 8086/88 mode
  outb(MASTER_PIC_DATA, 0x01);
  io_wait();
  outb(SLAVE_PIC_DATA,  0x01);
  io_wait();
  // Set the PICs' masks
  outb(MASTER_PIC_DATA, 0x00);
  outb(SLAVE_PIC_DATA,  0x00);

  // Fill the IDT with the IRQs
  idt_setentry(32, (uint32_t) irq0, 0x08, 0x8E);
  ...
  idt_setentry(47, (uint32_t) irq15, 0x08, 0x8E);
  
  idt_flush((uint32_t) &idt_ptr);
}

Code:

/* ---------- descr_lib.S ---------- */
.global gdt_flush   
gdt_flush:
   mov 4(%esp), %eax
   lgdt (%eax)

   mov $0x10, %ax
   mov %ax, %ds
   mov %ax, %es
   mov %ax, %fs
   mov %ax, %gs
   mov %ax, %ss
   #jmp $0x08, $.reload_cs
.reload_cs:
   ret
   
.global idt_flush
idt_flush:
   mov 4(%esp), %eax
   lidt (%eax)
   ret

.global isr0
isr0:   
   cli
   push $0
   push $0
   jmp isr_common_stub

...

.global isr31
isr31:   
   cli
   push $0
   push $31
   jmp isr_common_stub

.global irq0
irq0:   
   cli
   push $0
   push $32
   jmp irq_common_stub

...

.global irq15
irq15:   
   cli
   push $0
   push $47
   jmp irq_common_stub

.extern isr_handler
isr_common_stub:
   pusha          # Push the registers
   
   mov %ds, %ax   
   push %eax       # Push the current ds descriptor
   mov $0x10, %ax      # Load the kernel ds descriptor
   mov %ax, %ds
   mov %ax, %es
   mov %ax, %fs
   mov %ax, %gs

   call isr_handler    

   cli
   pop %eax      # Reload the original ds descriptor
   hlt
   mov %ax, %ds
   mov %ax, %es
   mov %ax, %fs
   mov %ax, %gs

   popa         # Reload the registers
   add $8, %esp      # Clean the stack
   sti
   iret         # Pop cs, eip, eflags, ss and esp

irq_common_stub:
   pusha         # Push the registers

   mov %ds, %ax
   push %eax      # Save the current ds descriptor

   mov $0x10, %ax      # Load the kernel ds descriptor
   mov %ax, %ds
   mov %ax, %es
   mov %ax, %fs
   mov %ax, %gs

   call irq_handler

   pop %ebx      # Reload the original data segment descriptor
   mov %bx, %ds
   mov %bx, %es
   mov %bx, %fs
   mov %bx, %gs

   popa          # Reload the registers
   add $8, %esp
   sti
   iret         # Pop cs, eip, eflags, ss and esp

Code:

/* ---------- isr.c ---------- */
#include <stdio.h>

#include <kernel/isr.h>
#include <kernel/tty.h>

isr_t interrupt_handlers[256];

void register_interrupt_handler(uint8_t n, isr_t handler) {
  interrupt_handlers[n] = handler;
}

// Called by the isr_common_stub when the kernel is interrupted by an exception
void isr_handler(registers_t regs) {
  // Handle the exceptions
  if (regs.int_no <= 31) {
    char* exceptions[] = {"Division By Zero",
         ...
         "Unknown"};
      printf("\nException %d: %s", regs.int_no, exceptions[regs.int_no]);
  }
}

// Called by irq_common_stub when the kernel is interrupted by an IRQ
void irq_handler(registers_t regs) {
  // Handle the IRQs
  
  // Send an EOI
  if (regs.int_no >= 40) {
    // Send the EOI to the slave PIC
    outb(0xA0, 0x20);
  }
  // Send the EOI to the master PIC
  outb(0x20, 0x20);

  if (interrupt_handlers[regs.int_no] != 0) {
    isr_t handler = interrupt_handlers[regs.int_no];
    handler(regs);
  }
}

Code:

/* ---------- timer.c ---------- */
#include <stdio.h>

#include <kernel/isr.h>
#include <kernel/timer.h>

#define PIT_CHAN_0_DATA_PORT 0x40
#define PIT_CHAN_1_DATA_PORT 0x41
#define PIT_CHAN_2_DATA_PORT 0x42
#define PIT_CMD_PORT         0x43

uint32_t tick = 0;

static void timer_callback(registers_t regs) {
  tick++;
  printf("%d\n", tick);
}

void timer_init(uint32_t frequency) {
  // Register timer_callback as the interrupt handler for IRQ0
  register_interrupt_handler(32, &timer_callback);
  
  uint32_t divisor = 1193180 / frequency;

  // Set the PIT in repeating mode and prepare it for initialization
  outb(PIT_CMD_PORT, 0x36);

  // Send the frequency divisor
  uint8_t lo = (uint8_t) (divisor & 0xFF);
  uint8_t hi = (uint8_t) ((divisor >> 8) & 0xFF);
  outb(PIT_CHAN_0_DATA_PORT, lo);
  outb(PIT_CHAN_0_DATA_PORT, hi);
}

I believe the problem lies with the PIC since I receive software interrupts and since the are_interrupts_enabled function reports that the IRQs are disabled even after I load the IDT with idt_load.

Would anyone happen to have an idea of what is wrong in my code ?

Thank you for reading.

Korona · **Joined:** Thu May 17, 2007 1:27 pm **Posts:** 999

sti before iret is pointless, as iret reloads rflags. Do you ever set the interrupt flag (sti)?

barahir · **Joined:** Wed Apr 10, 2019 2:02 am **Posts:** 5

Korona wrote:

sti before iret is pointless, as iret reloads rflags. Do you ever set the interrupt flag (sti)?

That was it, thank you. I never thought of doing that since I don't believe JamesM ever does it in his tutorial.
Just to be sure, is it correct if I put sti right after lidt ?

Code:

.global idt_flush
idt_flush:
   mov 4(%esp), %eax
   lidt (%eax)
   sti
   ret

Anyway, this looks like its working, but I am getting a General Protection Fault now.
I managed to pinpoint the problem : the irq_handler function overwrites the stack (where the old value of the ds descriptor is stored) when it is called by irq_common_stub :

Code:

irq_common_stub:
   pusha         

   mov %ds, %ax
   push %eax   

        # 0x10 is loaded at 0x108c08 = %esp

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

   call irq_handler

        # Now 0x00102641 is loaded at 0x108c08 = %esp

   pop %ebx      
   mov %bx, %ds          # General Protection Fault
   mov %bx, %es
   mov %bx, %fs
   mov %bx, %gs

   popa          
   add $8, %esp
   iret         

More precisely the stack gets overwritten in timer_callback (the function that handles IRQ0) :

Code:

static void timer_callback(registers_t regs) {
  tick++;                    
  printf("%d\n", tick);
}

Before tick++ is called, %esp = 0x108c04.
After it is called, we still have %esp = 0x108c04 but the value at %esp + 4 = 0x108c08 (where we stored %ds) was overwritten with another value (0x00102641, I don't really know what it could be).
Then when printf is called, the value at %esp + 8 = 0x108c0c is overwritten with another value (0x1) but %esp still hasn't moved.

Then the stack's behavior goes back to normal and values don't get overwritten anymore. Thus a General Protection Fault occurs because we then load %ds with 0x00102641.

Here is my code for printf :

Code:

#include <limits.h>
#include <stdbool.h>
#include <stdarg.h>
#include <stdio.h>
#include <string.h>

#include <kernel/descrtbl.h>

typedef struct word {
  const char* data;
  size_t size;
} word_t;

static bool print(word_t word) {
  const unsigned char* bytes = (const unsigned char*) word.data;
  for (size_t i = 0; i < word.size; i++)
    if (putchar(bytes[i]) == EOF)
      return false;
  return true;
}

static word_t word_of_string(const char* str) {
  word_t word;
  word.data = str;
  word.size = strlen(str);
  return word;
}

static word_t word_of_int(uint8_t base, int n) {
  if (n == 0) {
    return word_of_string("0");
  } else {
    size_t len = 0;
    int m = n;
    while (m > 0) {
      m /= base;
      len ++;
    }
    char digits[16] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'};
    char data[len];
    for(int i = len - 1; i >= 0; i--) {
      data[i] = digits[n % base];
      n /= base;
    }
    word_t word;
    word.data = (const char*) data;
    word.size = len;
    return word;
  }
}

int printf(const char* __restrict__ format, ...) {
  va_list parameters;
  va_start(parameters, format);

  int written = 0;

  while (*format != '\0') {
    word_t word;
    size_t maxrem = INT_MAX - written;
    
    if (format[0] != '%' || format[1] == '%') {
      if (format[0] == '%')
   format++;
      size_t amount = 1;
      while (format[amount] && format[amount] != '%')
   amount++;
      if (maxrem < amount) {
        isr4(); // Into Detected Overflow Exception
   return -1;
      }
      word.data = format;
      word.size = amount;
      if (!print(word))
   return -1;
      format += amount;
      written += amount;
      continue;
    }

    const char* format_begun_at = format++;
    switch(*format) {
    case 's':
      format++;
      word = word_of_string(va_arg(parameters, const char*));
      break;
    case 'x':
      format++;
      word = word_of_int(16, va_arg(parameters, int));
      break;
    case 'd':
      format++;
      word = word_of_int(10, va_arg(parameters, int));
      break;
    case 'b':
      format++;
      word = word_of_int(2, va_arg(parameters, int));
      break;
    default:
      format = format_begun_at;
      word = word_of_string(format);
      format += word.size;
    }
    if (maxrem < word.size) {
        isr4(); // Into Detected Overflow Exception
      return -1;
    }
    if (!print(word)) {
      return -1;
    }
    written += word.size;
    continue;
  }

  va_end(parameters);
  return written;
}

It is basically the same as the one from the Meaty Skeleton but I added a few stuff to print numeric values.

I don't really understand how the stack could be rewritten at this point since I am not fiddling the memory and the compiler should take care of moving the top of the stack.

Any idea where this could come from ?

MichaelPetch · **Joined:** Fri Aug 26, 2016 1:41 pm **Posts:** 692

The 32-bit System V ABI doesn't guarantee that structures and data passed by value will be unchanged in a function. The compiler is free to reuse that stack space for its own data. Arguments passed (on the stack) to a function are owned by the called function, not the caller. This is a bug in the Malloy's tutorial from which that code comes from:

Code:

void irq_handler(registers_t regs) {

is the issue. you want to pass registers_t *regs instead. That will require passing the address of the registers structure into irq_handler in the assembly stub code. You should also be passing registers_t *regs into the interrupt handlers as well. I wrote a Reddit post recently about this issue and a fix that can be applied: https://www.reddit.com/r/osdev/comments ... pt/ef4bxnk

barahir · **Joined:** Wed Apr 10, 2019 2:02 am **Posts:** 5

MichaelPetch wrote:

The 32-bit System V ABI doesn't guarantee that structures and data passed by value will be unchanged in a function. The compiler is free to reuse that stack space for its own data. Arguments passed (on the stack) to a function are owned by the called function, not the caller. This is a bug in the Malloy's tutorial from which that code comes from:

Code:

void irq_handler(registers_t regs) {

is the issue. you want to pass registers_t *regs instead. That will require passing the address of the registers structure into irq_handler in the assembly stub code. You should also be passing registers_t *regs into the interrupt handlers as well. I wrote a Reddit post recently about this issue and a fix that can be applied: https://www.reddit.com/r/osdev/comments ... pt/ef4bxnk

Thank you! It almost works now.
Indeed, if I launch my OS through GDB, it seems to run flawlessly. The problem is that if I launch it directly through QEMU, I still get a General Protection Fault, and I cannot pinpoint the problem now that GDB thinks there aren't any problem.
If I print the values of the registers when the exception is raised, I get the following values :

In particular, eip = 0x10100 but nothing's written at this address. The fault disappears if I comment out timer_init in kernel_main and add a cli at the end of _start :

Code:

void kernel_main(void) {
  terminal_initialize();

  gdt_init();
  idt_init();
  // General Protection Fault when uncommented
  // timer_init(50);

Code:

_start:
   cli
   
   # Set up the stack.
   mov $stack_top, %esp

   # Enter the high-level kernel.
   call kernel_main

   # General Protection Fault without this line
   cli

   # Infinite loop if the system has nothing more to do.
1:   
   jmp 1b

MichaelPetch · **Joined:** Fri Aug 26, 2016 1:41 pm **Posts:** 692

If you put your entire project online somewhere (like github) it might be easier. Could be any number of things. Hard to say with info available.

pistachio · **Joined:** Tue Feb 19, 2019 8:30 pm **Posts:** 15

Are you re-enabling interrupts following the setup of the IDT table? If not, it would be silly to expect an interrupt!

Try "sti" after you do the sensitive stuff or expecting an interrupt from your timer.

Refer to https://wiki.osdev.org/James_Molloy%27s_Tutorial_Known_Bugs for more about this tutorial. It has quite a few issues.

barahir · **Joined:** Wed Apr 10, 2019 2:02 am **Posts:** 5

MichaelPetch wrote:

If you put your entire project online somewhere (like github) it might be easier. Could be any number of things. Hard to say with info available.

Here it is : https://framagit.org/barahir/os-debug

MichaelPetch · **Joined:** Fri Aug 26, 2016 1:41 pm **Posts:** 692

The multiboot spec doesn't guarantee that the CS, DS, ES, SS segment selectors have any particular value. So in one environment CS could be 0x08 or it could 0x10 in another etc. You'll actually find the values differ if you run your kernel with QEMU via the -kernel option (CS is usually 0x08) versus the -cdrom option (CS via real GRUB may be 0x10). This all matters because in your gdt_flush code you have commented out an important line:

Code:

gdt_flush:
   mov 4(%esp), %eax
   lgdt (%eax)

   mov $0x10, %ax
   mov %ax, %ds
   mov %ax, %es
   mov %ax, %fs
   mov %ax, %gs
   mov %ax, %ss
   #jmp $0x08, $.reload_cs
.reload_cs:
   ret

It should be:

Code:

gdt_flush:
   mov 4(%esp), %eax
   lgdt (%eax)

   mov $0x10, %ax
   mov %ax, %ds
   mov %ax, %es
   mov %ax, %fs
   mov %ax, %gs
   mov %ax, %ss
   jmp $0x08, $.reload_cs
.reload_cs:
   ret

You also have CLI/HLT in isr_common_stub

Code:

   cli
   pop %eax      # Reload the original ds descriptor
   hlt

The CLI/HLT should be removed. I assume this may have been placed there for testing purposes. You should also put HLT in an infinite loop at the end of kernel_main where interrupts are still enabled so that your kernel can continue to process interrupts without leaving your kernel. Add something like this to the end of kernel_main:

Code:

while(1) asm ("hlt");

Before calling your irq or ISR handler the common stub should clear the direction flag with CLD before calling into the C/C++ function. This is required by the 32-bit System V ABI as well.

barahir · **Joined:** Wed Apr 10, 2019 2:02 am **Posts:** 5

MichaelPetch wrote:

I assume this may have been placed there for testing purposes.

Yeah, I should have looked closer when going over my code. Thanks a lot!

glauxosdever · **Posted:** Fri Apr 12, 2019 2:19 am

Hi,

pistachio wrote:

Refer to https://wiki.osdev.org/James_Molloy%27s_Tutorial_Known_Bugs for more about this tutorial. It has quite a few issues.

I second this. It's been pointed out to JamesM multiple times that his tutorial has many such bugs, but he didn't bother to fix them or take the tutorial down.

Regards,
glauxsodever

OSDev.org

Cannot get IRQs enabled

Who is online