Cannot get IRQs enabled

Question about which tools to use, bugs, the best way to implement a function, etc should go here. Don't forget to see if your question is answered in the wiki first! When in doubt post here.
Post Reply
barahir
Posts: 5
Joined: Wed Apr 10, 2019 2:02 am

Cannot get IRQs enabled

Post by barahir »

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: Select all

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

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

Code: Select all

/* ---------- 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: Select all

/* ---------- 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: Select all

/* ---------- 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: Select all

/* ---------- 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
Member
Member
Posts: 1000
Joined: Thu May 17, 2007 1:27 pm
Contact:

Re: Cannot get IRQs enabled

Post by Korona »

sti before iret is pointless, as iret reloads rflags. Do you ever set the interrupt flag (sti)?
managarm: Microkernel-based OS capable of running a Wayland desktop (Discord: https://discord.gg/7WB6Ur3). My OS-dev projects: [mlibc: Portable C library for managarm, qword, Linux, Sigma, ...] [LAI: AML interpreter] [xbstrap: Build system for OS distributions].
barahir
Posts: 5
Joined: Wed Apr 10, 2019 2:02 am

Re: Cannot get IRQs enabled

Post by barahir »

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: Select all

.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: Select all

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: Select all

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: Select all

#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
Member
Member
Posts: 798
Joined: Fri Aug 26, 2016 1:41 pm
Libera.chat IRC: mpetch

Re: Cannot get IRQs enabled

Post by MichaelPetch »

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: Select all

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
Posts: 5
Joined: Wed Apr 10, 2019 2:02 am

Re: Cannot get IRQs enabled

Post by barahir »

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: Select all

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 :
  • ds = 0x10
    edi = 0x0
    esi = 0x0
    ebp = 0x0
    ebx = 05D
    edx = 0x40
    ecx = 0x40
    eax = 0x5D
    eip = 0x10100
    cs = 0x8
    eflags = 0x12
    esp = 0x10133
    ss = 0x10
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: Select all

void kernel_main(void) {
  terminal_initialize();

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

Code: Select all

_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
Member
Member
Posts: 798
Joined: Fri Aug 26, 2016 1:41 pm
Libera.chat IRC: mpetch

Re: Cannot get IRQs enabled

Post by MichaelPetch »

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
Posts: 15
Joined: Tue Feb 19, 2019 8:30 pm

Re: Cannot get IRQs enabled

Post by pistachio »

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 ... Known_Bugs for more about this tutorial. It has quite a few issues.
barahir
Posts: 5
Joined: Wed Apr 10, 2019 2:02 am

Re: Cannot get IRQs enabled

Post by barahir »

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
Member
Member
Posts: 798
Joined: Fri Aug 26, 2016 1:41 pm
Libera.chat IRC: mpetch

Re: Cannot get IRQs enabled

Post by MichaelPetch »

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: Select all

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: Select all

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: Select all

	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: Select all

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
Posts: 5
Joined: Wed Apr 10, 2019 2:02 am

Re: Cannot get IRQs enabled

Post by barahir »

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
Member
Member
Posts: 501
Joined: Wed Jun 17, 2015 9:40 am
Libera.chat IRC: glauxosdever
Location: Athens, Greece

Re: Cannot get IRQs enabled

Post by glauxosdever »

Hi,

pistachio wrote:Refer to https://wiki.osdev.org/James_Molloy%27s ... 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
Post Reply