PIT not working

[viruss33]

Hi. I have problem with PIT and IRQs. I followed the meaty skeleton on wiki and james molloy tutorial for setting up gdt, idt, irq and pit. The problem is that my "tick" is not displayed on screen.
I remapped the PIC and when I send int 0x20, it works. I'm aware that this tutorial has bugs in it, but I was hoping that the code at least works. I have no idea how to debug it, I'm using qemu with the flags -d int,cpu_reset -D qemu2.log, but it says nothing about interrupts nor irqs. Here's my code
interrupt_requests.c

Code: Select all

void IRQ_set_mask(unsigned char IRQline) {
    u16int port;
    u8int value;
 
    if(IRQline < 8) {
        port = PIC1_DATA; 
    } else {
        port = PIC2_DATA;
        IRQline -= 8;
    }
    value = inbyte(port) | (1 << IRQline);
    outbyte(port, value);        
}
 
void IRQ_clear_mask(unsigned char IRQline) {
    u16int port;
    u8int value;
 
    if(IRQline < 8) {
        port = PIC1_DATA;
    } else {
        port = PIC2_DATA;
        IRQline -= 8;
    }
    value = inbyte(port) & ~(1 << IRQline);
    outbyte(port, value);        
}

interrupt_tables.c

Code: Select all

#include <kernel/common.h>
#include <kernel/interrupt_tables.h>
#include <string.h>

extern void idt_flush(u32int);


idt_entry idt_entries[256];
idtr_t  idt_ptr;


void init_idt()
{
   idt_ptr.limit = sizeof(idt_entry) * 256 -1;
   idt_ptr.base  = (u32int)&idt_entries;

   memset(&idt_entries, 0, sizeof(idt_entry)*256);

   idt_set_gate( 0, (u32int)isr0 , 0x08, 0x8E);
   idt_set_gate( 1, (u32int)isr1 , 0x08, 0x8E);
   idt_set_gate( 2, (u32int)isr2 , 0x08, 0x8E);
   idt_set_gate( 3, (u32int)isr3 , 0x08, 0x8E);
   idt_set_gate( 4, (u32int)isr4 , 0x08, 0x8E);
   idt_set_gate( 5, (u32int)isr5 , 0x08, 0x8E);
   idt_set_gate( 6, (u32int)isr6 , 0x08, 0x8E);
   idt_set_gate( 7, (u32int)isr7 , 0x08, 0x8E);
   idt_set_gate( 8, (u32int)isr8 , 0x08, 0x8E);
   idt_set_gate( 9, (u32int)isr9 , 0x08, 0x8E);
   idt_set_gate( 10, (u32int)isr10 , 0x08, 0x8E);
   idt_set_gate( 11, (u32int)isr11 , 0x08, 0x8E);
   idt_set_gate( 12, (u32int)isr12 , 0x08, 0x8E);
   idt_set_gate( 13, (u32int)isr13 , 0x08, 0x8E);
   idt_set_gate( 14, (u32int)isr14 , 0x08, 0x8E);
   idt_set_gate( 15, (u32int)isr15 , 0x08, 0x8E);
   idt_set_gate( 16, (u32int)isr16 , 0x08, 0x8E);
   idt_set_gate( 17, (u32int)isr17 , 0x08, 0x8E);
   idt_set_gate( 18, (u32int)isr18 , 0x08, 0x8E);
   idt_set_gate( 19, (u32int)isr19 , 0x08, 0x8E);
   idt_set_gate( 20, (u32int)isr20 , 0x08, 0x8E);
   idt_set_gate( 21, (u32int)isr21 , 0x08, 0x8E);
   idt_set_gate( 22, (u32int)isr22 , 0x08, 0x8E);
   idt_set_gate( 23, (u32int)isr23 , 0x08, 0x8E);
   idt_set_gate( 24, (u32int)isr24 , 0x08, 0x8E);
   idt_set_gate( 25, (u32int)isr25 , 0x08, 0x8E);
   idt_set_gate( 26, (u32int)isr26 , 0x08, 0x8E);
   idt_set_gate( 27, (u32int)isr27 , 0x08, 0x8E);
   idt_set_gate( 28, (u32int)isr28 , 0x08, 0x8E);
   idt_set_gate( 29, (u32int)isr29 , 0x08, 0x8E);
   idt_set_gate( 30, (u32int)isr30 , 0x08, 0x8E);
   idt_set_gate( 31, (u32int)isr31 , 0x08, 0x8E);

   outbyte(0x20, 0x11);
  outbyte(0xA0, 0x11);
  outbyte(0x21, 0x20);
  outbyte(0xA1, 0x28);
  outbyte(0x21, 0x04);
  outbyte(0xA1, 0x02);
  outbyte(0x21, 0x01);
  outbyte(0xA1, 0x01);
  outbyte(0x21, 0x0);
  outbyte(0xA1, 0x0);

   idt_set_gate(32, (u32int)irq0, 0x08, 0x8E);
   idt_set_gate(33, (u32int)irq1, 0x08, 0x8E);
   idt_set_gate(34, (u32int)irq2, 0x08, 0x8E);
   idt_set_gate(35, (u32int)irq3, 0x08, 0x8E);
   idt_set_gate(36, (u32int)irq4, 0x08, 0x8E);
   idt_set_gate(37, (u32int)irq5, 0x08, 0x8E);
   idt_set_gate(38, (u32int)irq6, 0x08, 0x8E);
   idt_set_gate(39, (u32int)irq7, 0x08, 0x8E);
   idt_set_gate(40, (u32int)irq8, 0x08, 0x8E);
   idt_set_gate(41, (u32int)irq9, 0x08, 0x8E);
   idt_set_gate(42, (u32int)irq10, 0x08, 0x8E);
   idt_set_gate(43, (u32int)irq11, 0x08, 0x8E);
   idt_set_gate(44, (u32int)irq12, 0x08, 0x8E);
   idt_set_gate(45, (u32int)irq13, 0x08, 0x8E);
   idt_set_gate(46, (u32int)irq14, 0x08, 0x8E);
   idt_set_gate(47, (u32int)irq15, 0x08, 0x8E);
   

   idt_flush((u32int)&idt_ptr);
}

void idt_set_gate(u8int index, u32int base, u16int sel, u8int flags)
{
   idt_entries[index].isr_low = base & 0xFFFF;
   idt_entries[index].isr_high = (base >> 16) & 0xFFFF;

   idt_entries[index].kernel_cs     = sel;
   idt_entries[index].reserved = 0;
   // We must uncomment the OR below when we get to using user-mode.
   // It sets the interrupt gate's privilege level to 3.
   idt_entries[index].attributes   = flags /* | 0x60 */;
}

irq_handler.c

Code: Select all

#include <kernel/common.h>
#include <kernel/irq_handler.h>


isr_t interrupt_handlers[256];

// This gets called from our ASM interrupt handler stub.
void irq_handler(registers_t regs)
{
   // Send an EOI (end of interrupt) signal to the PICs.
   // If this interrupt involved the slave.
   if (regs.int_no >= 40)
   {
       // Send reset signal to slave.
       outbyte(0xA0, 0x20);
   }
   // Send reset signal to master. (As well as slave, if necessary).
   outbyte(0x20, 0x20);

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

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

kernel.c

Code: Select all

#include <stdio.h>
#include <kernel/descriptor_tables.h>
#include <kernel/interrupt_tables.h>
#include <kernel/tty.h>
#include <kernel/timer.h>
#include <kernel/interrupt_requests.h>
#include <stdint.h>

void kernel_main(void) {
	terminal_initialize();
	init_descriptor_tables();
	init_idt();
	IRQ_clear_mask(0);
	init_timer(50);
	asm volatile ("int $0x20");
}

timer.c

Code: Select all

#include <kernel/timer.h>
#include <stdio.h>
#include <kernel/irq_handler.h>
#include <kernel/common.h>

u32int tick = 0;

static void timer_callback(registers_t regs)
{
   tick++;
   printf("Tick: ");
}

void init_timer(u32int frequency)
{
   register_interrupt_handler(IRQ0, &timer_callback);   

   // The value we send to the PIT is the value to divide it's input clock
   // (1193180 Hz) by, to get our required frequency. Important to note is
   // that the divisor must be small enough to fit into 16-bits.
   u32int divisor = 1193180 / frequency;

   // Send the command byte.
   outbyte(0x43, 0x36);

   // Divisor has to be sent byte-wise, so split here into upper/lower bytes.
   u8int l = (u8int)(divisor & 0xFF);
   u8int h = (u8int)( (divisor>>8) & 0xFF );

   // Send the frequency divisor.
   outbyte(0x40, l);
   outbyte(0x40, h);
}

irq.S

Code: Select all

.macro IRQ i, j
.global irq\i
irq\i:
    cli
    push $0
    push \j
    jmp irq_common_stub 
    iret 
.endm

IRQ   0,    32
IRQ   1,    33
IRQ   2,    34
IRQ   3,    35
IRQ   4,    36
IRQ   5,    37
IRQ   6,    38
IRQ   7,    39
IRQ   8,    40
IRQ   9,    41
IRQ   10,    42
IRQ   11,    43
IRQ   12,    44
IRQ   13,    45
IRQ   14,    46
IRQ  15,    47

.extern irq_handler

irq_common_stub:
   pusha                    # Pushes edi,esi,ebp,esp,ebx,edx,ecx,eax

   mov %ds, %ax            # Lower 16-bits of eax = ds.
   push %eax                 # save the data segment descriptor

   mov $0x10, %ax  # load the kernel data segment 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                     # Pops edi,esi,ebp...
   add $8, %esp     # Cleans up the pushed error code and pushed ISR number
   sti
   iret           # pops 5 things at once: CS, EIP, EFLAGS, SS, and ESP

[Demindiro]

Are you enabling interrupts (sti) after initializing the IDT?

Also, you shouldn't use cli when entering the IRQ handler nor should you use sti before exiting with iretq. The interrupt flag will already have been cleared if you set up your IDT properly and iretq will automatically restore the FLAGS register, so sti will do nothing in the best case, in the worst case the stack may overflow if many interrupts are pending.

[nullplan]

Since you are not getting any messages about interrupts, it is very likely that Demindiro's suggestion is correct. However, a few more suggestions:

1. Your kernel_main() returns. It is very likely not supposed to. You can just add an infinite loop to the bottom like so:

   Code: Select all

   for(;;) asm("hlt");

   This will make the kernel wait for interrupts infinitely.
2. You are passing the registers into the IRQ handler by value. Likely you are doing the same for the exception handlers. Semantically, this is wrong because those parameters are local variables of the handler functions and can end up trashed. You should change the argument type to a pointer to register_t, and push ESP before calling the handler function, and increase ESP by four afterwards. This ensures the compiler will not trash the registers, which is needed to ensure your interrupt code is bullet-proof. Because if it isn't, you will chase very strange and hard to find bugs.

That should get you running at least, although the code still has massive design problems. For example, you should probably mask out all IRQs by default (except IRQ 2 in case of the PIC pair), and then only unmask IRQs as drivers request them. And contrary to Demindiro's advice, you should STI only after initializing the PIC, or in general, all interrupt controllers in the system (and not after the IDT, because that is too early). You are also sending EOI to the PIC before calling the handler, which you should probably not (may cause spurious interrupts). And the PIC driver ought not to depend on the exact interrupt numbers it gets. But those design defects can be worked out later. The two points above are more important right now.

[viruss33]

Thanks guys, your remarks helped to solve the problem. Please review my changes:
isr.S - I removed cli and sti.
At interrupt_request.c I added a method

Code: Select all

void mask_all_IRQs(){
  for (int8_t i =0; i< 16; i++){
    IRQ_set_mask(i);
  }
  
}

At irq_handler.c

Code: Select all

#include <stdint.h>
#include <kernel/irq_handler.h>


isr_t irq_handlers[16];

// This gets called from our ASM interrupt handler stub.
void irq_handler(registers_t* registers_pointer)
{
   // Send an EOI (end of interrupt) signal to the PICs.
   // If this interrupt involved the slave.
   registers_t registers_value = registers_pointer[0];
   if (registers_value.irq_number >= 8)
   {
       // Send reset signal to slave.
       outbyte(0xA0, 0x20);
   }

   if (irq_handlers[registers_value.irq_number] != 0)
   {
       isr_t handler = irq_handlers[registers_value.irq_number];
       handler(registers_value);
   }

   // Send reset signal to master. (As well as slave, if necessary).
   outbyte(0x20, 0x20);
}

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

At kernel.c

Code: Select all

extern void enable_interrupts(); <-- assembly method that does sti and ret

void handleIRQs (){
	mask_all_IRQs();
	IRQ_clear_mask(2);	
}

void kernel_main(void) {
	terminal_initialize();
	init_descriptor_tables();
	init_idt();
	handleIRQs();
	enable_interrupts();
	init_timer(50);
	for(;;) asm("hlt");
}

in timer.c I added "IRQ_clear_mask(0);"

irq.S

Code: Select all

irq_common_stub:
   pusha                    # Pushes edi,esi,ebp,esp,ebx,edx,ecx,eax

   mov %ds, %ax            # Lower 16-bits of eax = ds.
   push %eax                 # save the data segment descriptor

   mov $0x10, %ax  # load the kernel data segment descriptor
   mov %ax, %ds
   mov %ax, %es
   mov %ax, %fs
   mov %ax, %gs

   push %esp

   call irq_handler

   add $4, %esp

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

   popa                     # Pops edi,esi,ebp...
   add $8, %esp     # Cleans up the pushed error code and pushed ISR number   
   iret           # pops 5 things at once: CS, EIP, EFLAGS, SS, and ESP

[Octocontrabass]

At interrupt_request.c I added a method

Why? Just change the masks you program into the PICs at the end of initializing them:

Code: Select all

  outbyte(0x21, 0xfb); // leave IRQ2 enabled
  outbyte(0xA1, 0xff);

Code: Select all

   registers_t registers_value = registers_pointer[0];

That's an odd choice. Why not access the individual struct elements through the pointer? Like this:

Code: Select all

   if (registers_pointer->irq_number >= 8)

Also, just in case you haven't seen it yet, we have a list of known problems in James Molloy's tutorial.

[viruss33]

At interrupt_request.c I added a method

Why? Just change the masks you program into the PICs at the end of initializing them

Ok will do it.

At interrupt_request.c I added a method

That's an odd choice. Why not access the individual struct elements through the pointer?

Cause I'm a C newbie Will fix that.

Yeah, I knew about the page with James Malloy tutorial issues, but I didn't know how to fix the issues with registers struct. Now that I look again at that page, I can see sti and cli mentioned there so my bad, for not paying much attention there. Also there's still some points to be fixed in my code.