Keyboard IRQ fires only once

Question

I'm developing a toy unix clone and I'm trying to wire up my interrupts properly. I've run into a problem where my Keyboard IRQ (IRQ 1) fires just once even after I properly acknowledge it and so on. I've enabled the PIT interrupt as well, to double check if my ACKs are going to the PIC okay, and that seems to work fine. (fires multiple times)

One catch with interrup.s is that I'm passing the struct register_t on the stack (by value) and the compiler was trashing that after it returns from the C interrupt handler. Surprisingly, the only value that was getting trashed was the value on the top of the stack (The data segment register in this case) and I've verified that the rest of the values in the stack look okay by printing the stack before and after the call to the interrupt handler occurs. I've added a temporary work-around to fix this issue, but I will clean this up later.

I've also verified that software interrupts work fine by triggering int $3 multiple times.

Any advice is appreciated! Here's the code:

interrupt.s

.macro ISR_NOERRCODE int_no # A macro for ISRs that don't push an error code
.global isr\int_no
isr\int_no:
  cli
  push $0
  push $\int_no
  jmp isr_irq_common_stub
.endm

.macro ISR_ERRORCODE int_no # A macro for ISRs that do push an error code
.global isr\int_no
isr\int_no:
  cli
  push $\int_no
  jmp isr_irq_common_stub
.endm

.macro IRQ irq_no, isr_map # A macro for IRQs from the PIC
.global irq\irq_no
irq\irq_no:
  xchgw %bx, %bx
  cli
  push $0 # Error code
  push $\isr_map # Interrupt number
  jmp isr_irq_common_stub
.endm

ISR_NOERRCODE 0
ISR_NOERRCODE 1
ISR_NOERRCODE 2
ISR_NOERRCODE 3
ISR_NOERRCODE 4
ISR_NOERRCODE 5
ISR_NOERRCODE 6
ISR_NOERRCODE 7
ISR_ERRORCODE 8  # ISR 8 pushes error code onto stack
ISR_NOERRCODE 9
ISR_ERRORCODE 10 # ISR 10 - 14 push error codes onto stack
ISR_ERRORCODE 11
ISR_ERRORCODE 12
ISR_ERRORCODE 13
ISR_ERRORCODE 14
ISR_NOERRCODE 15
ISR_NOERRCODE 16
ISR_ERRORCODE 17
ISR_NOERRCODE 18
ISR_NOERRCODE 19
ISR_NOERRCODE 20
ISR_NOERRCODE 21
ISR_NOERRCODE 22
ISR_NOERRCODE 23
ISR_NOERRCODE 24
ISR_NOERRCODE 25
ISR_NOERRCODE 26
ISR_NOERRCODE 27
ISR_NOERRCODE 28
ISR_NOERRCODE 29
ISR_ERRORCODE 30
ISR_NOERRCODE 31
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

# This is in isr.c
.extern isr_irq_handler

# This is our common isr stub. It saves the processor state, sets up for kernel
# mode segments, calls the C-level fault handler, and finally restores the stack
# frame
isr_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             # Right now, we dont really have to do this
    mov %ax, %es             # but after we enter the user mode, the segment
    mov %ax, %fs             # registers will be different (0x18? and 0x20?)
    mov %ax, %gs

    call isr_irq_handler

    # This does not work because the structure value we passed earlier
    # is being messed up by the compiler. It does not preserve the previous eax
    # we pushed on to the stack.
    pop %eax
    mov $0x10, %ax           # reload the original data segment descriptor
    mov %ax, %ds
    mov %ax, %es
    mov %ax, %fs
    mov %ax, %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

isr.h

#ifndef __isr_h
#define __isr_h

#include <stdint.h>

struct Registers {
  uint32_t ds;
  uint32_t edi, esi, ebp, esp, ebx, edx, ecx, eax;
  uint32_t int_no, err_code;
  uint32_t eip, cs, eflags, useresp, ss;
} __attribute__((packed));

typedef struct Registers register_t;
typedef void (*isr_t)(registers_t);

void register_interrupt_handler(uint8_t n, isr_t handler);
void isr_irq_handler(register_t regs);

#endif

isr.c

#include <kernel/isr.h>
#include <kernel/pic.h>
#include <stdio.h>
#include <log.h>
#include <kernel/tty.h>

isr_t interrupt_handlers[256];

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

void isr_irq_handler(register_t regs)
{
  printf("Received ISR/IRQ: %d\n", regs.int_no);

  if (interrupt_handlers[regs.int_no]) {
    interrupt_handlers[regs.int_no]();
  }

  if (regs.int_no >= 32 && regs.int_no <= 47) {
    pic_send_eoi(regs.int_no - 32);
  }
  return;
}

pic.c

#include <kernel/pic.h>
#include <asm.h>

#define PIC1        0x20        /* IO base address for master PIC */
#define PIC2        0xA0        /* IO base address for slave PIC */
#define PIC1_COMMAND    PIC1
#define PIC1_DATA   (PIC1+1)
#define PIC2_COMMAND    PIC2
#define PIC2_DATA   (PIC2+1)
#define PIC_EOI     0x20        /* End-of-interrupt command code */

#define ICW1_ICW4      0x01 /* ICW4 (not) needed */
#define ICW1_SINGLE    0x02 /* Single (cascade) mode */
#define ICW1_INTERVAL4 0x04 /* Call address interval 4 (8) */
#define ICW1_LEVEL     0x08 /* Level triggered (edge) mode */
#define ICW1_INIT      0x10 /* Initialization - required! */

#define ICW4_8086       0x01 /* 8086/88 (MCS-80/85) mode */
#define ICW4_AUTO       0x02 /* Auto (normal) EOI */
#define ICW4_BUF_SLAVE  0x08 /* Buffered mode/slave */
#define ICW4_BUF_MASTER 0x0C /* Buffered mode/master */
#define ICW4_SFNM       0x10 /* Special fully nested (not) */
#define PIC_READ_IRR                0x0a    /* OCW3 irq ready next CMD read */
#define PIC_READ_ISR                0x0b    /* OCW3 irq service next CMD read */

#define PIC1_OFFSET 0x20
#define PIC2_OFFSET 0x28

static void pic_mask(int pic_num, uint16_t mask);
static uint16_t pic_get_irq_reg(int ocw3);
static uint16_t pic_get_irr(void);
static uint16_t pic_get_isr(void);

void setup_remap_pics()
{
  uint8_t a1, a2;

  // Save existing masks
  a1 = inb(PIC1_DATA);
  a2 = inb(PIC2_DATA);

  outb(PIC1_COMMAND, ICW1_INIT);
  io_wait();
  outb(PIC2_COMMAND, ICW1_INIT);
  io_wait();
  outb(PIC1_DATA, PIC1_OFFSET); // We're remapping the PICs interrupt codes from (0x07-0x7F) to (offset, offset + 8)
  io_wait();
  outb(PIC2_DATA, PIC2_OFFSET);
  io_wait();
  outb(PIC1_DATA, 4); // Tell the master PIC that there is a slave PIC at IRQ2 (00000100)
  io_wait();
  outb(PIC2_DATA, 2); // Tell the slave pic it's cascade identity (00000010)
  io_wait();

  outb(PIC1_DATA, ICW4_8086);
  io_wait();
  outb(PIC2_DATA, ICW4_8086);
  io_wait();

  // Restore saved masks
  outb(PIC1_DATA, a1);
  outb(PIC2_DATA, a2);

  enable_interrupts();

  // Mask everything except the keyboard, timer
  pic_mask(1, 0xFD);
  pic_mask(2, 0xFF);
}

static void pic_mask(int pic_num, uint16_t mask) {
    uint16_t port = (pic_num == 1) ? PIC1_DATA : PIC2_DATA;
    outb(port, mask);
}

// MARK :- Helpers
void pic_send_eoi(uint8_t irq)
{
  if (irq >= 8) {
    outb(PIC2_COMMAND, PIC_EOI);
  }

  printf("Sending EOI for IRQ: %d, EOI: %x, to CMD: %x\n", irq, PIC_EOI, PIC1_COMMAND);

  // Always signal PIC1 that an interrupt has been handled
  // because it's the PIC that forwards PIC2's irqs as well.
  outb(PIC1_COMMAND, PIC_EOI);
}

static uint16_t pic_get_irq_reg(int ocw3)
{
    /* OCW3 to PIC CMD to get the register values.  PIC2 is chained, and
     * represents IRQs 8-15.  PIC1 is IRQs 0-7, with 2 being the chain */
    outb(PIC1_COMMAND, ocw3);
    outb(PIC2_COMMAND, ocw3);
    return (inb(PIC2_COMMAND) << 8) | inb(PIC1_COMMAND);
}

/* Returns the combined value of the cascaded PICs irq request register */
static uint16_t pic_get_irr(void)
{
    return pic_get_irq_reg(PIC_READ_IRR);
}

/* Returns the combined value of the cascaded PICs in-service register */
static uint16_t pic_get_isr(void)
{
    return pic_get_irq_reg(PIC_READ_ISR);
}

Answer 1

I don't see any keyboard ISR code, and that could well be where your problem is. I'm guessing your are using the PC/AT style interface to interact with the KB controller (ports 60, 61, 64 if I remember correctly). There are one or more OUTs to one (or more) of these ports before they'll generate another interrupt - if memory serves.

In general any hardware device will require attention after it generates an interrupt. The PIC generating a time tick interrupt is an exception.

In the way of advice, I have a few suggestions:

1. Inspect the compiler code that you think is incorrectly trashing the AX register and/or the register_t structure (I'm not entirely sure from the OP which you are saying is being trashed). Identify exactly how you think the compiler is incorrect. I suspect the compiler is not incorrect, but rather that it is just tricky to call a C function from an ISR. In doing that in the past I had to go through a number of iterations to get it right.

2. Come up with a debugging scheme where you can view recent events.

   a. One method is to use the fixed locations in the VGA screen memory (I'm assuming you are working in text mode at this point) where you can write single characters that indicate what your program is doing. A few characters at the end of the top line in particular, let's say 4. The 4 characters represent the 4 most recent events. When an event (an interrupt or a call from a user mode program to the kernel as examples) occurs you move the 3 most recent chars to the left (or right) and then put the newest char in the "most recent" slot. When you're program is working normally you'll see those locations on the screen as kind of a blur as they rapidly get updated due to various events. However if your program hangs the characters will stop changing and you'll see the 4 most recent events, even if you can't break in with a debugger.

   b. Create a circular in memory trace log. Each entry in the log represents an event but can include details about the event like register values, or program state. If you break in with a debugger you can view the log as either bytes, words, dwords, and decode the events in your head. Or if you have a debugger that allows custom extensions, then write an extensions that supports displaying and querying the log for events of interest.

FWIW, I implemented a V86 monitor (old Intel terminology for virtualized real mode hypervisor) and so these suggestions come from experience. These suggestions apply to dev testing your kernel on both bare metal and to a somewhat lesser extent using a VM as your dev test platform.

With a VM and the right hypervisor, you will have access to a debugger specifically designed to debug a VM. Virtual Box, for example, has such a debugger. Windbg can be used on Hyper-V, although it was very slow when I tried it last. However someone wrote an extension that made kernel debugging of Hyper-V VMs much quicker.