Keyboard IRQ within an x86 kernel

I'm trying to program a very simple kernel for learning purposes. After reading a bunch of articles about the PIC and IRQs in the x86 architecture, I've figured out that IRQ1 is the keyboard handler. I'm using the following code to print the keys being pressed:

#include "port_io.h"

#define IDT_SIZE 256
#define PIC_1_CTRL 0x20
#define PIC_2_CTRL 0xA0
#define PIC_1_DATA 0x21
#define PIC_2_DATA 0xA1

void keyboard_handler();
void load_idt(void*);

struct idt_entry
{
    unsigned short int offset_lowerbits;
    unsigned short int selector;
    unsigned char zero;
    unsigned char flags;
    unsigned short int offset_higherbits;
};

struct idt_pointer
{
    unsigned short limit;
    unsigned int base;
};

struct idt_entry idt_table[IDT_SIZE];
struct idt_pointer idt_ptr;

void load_idt_entry(char isr_number, unsigned long base, short int selector, char flags)
{
    idt_table[isr_number].offset_lowerbits = base & 0xFFFF;
    idt_table[isr_number].offset_higherbits = (base >> 16) & 0xFFFF;
    idt_table[isr_number].selector = selector;
    idt_table[isr_number].flags = flags;
    idt_table[isr_number].zero = 0;
}

static void initialize_idt_pointer()
{
    idt_ptr.limit = (sizeof(struct idt_entry) * IDT_SIZE) - 1;
    idt_ptr.base = (unsigned int)&idt_table;
}

static void initialize_pic()
{
    /* ICW1 - begin initialization */
    write_port(PIC_1_CTRL, 0x11);
    write_port(PIC_2_CTRL, 0x11);

    /* ICW2 - remap offset address of idt_table */
    /*
    * In x86 protected mode, we have to remap the PICs beyond 0x20 because
    * Intel have designated the first 32 interrupts as "reserved" for cpu exceptions
    */
    write_port(PIC_1_DATA, 0x20);
    write_port(PIC_2_DATA, 0x28);

    /* ICW3 - setup cascading */
    write_port(PIC_1_DATA, 0x00);
    write_port(PIC_2_DATA, 0x00);

    /* ICW4 - environment info */
    write_port(PIC_1_DATA, 0x01);
    write_port(PIC_2_DATA, 0x01);
    /* Initialization finished */

    /* mask interrupts */
    write_port(0x21 , 0xff);
    write_port(0xA1 , 0xff);
}

void idt_init()
{
    initialize_pic();
    initialize_idt_pointer();
    load_idt(&idt_ptr);
}

load_idt just uses the lidt x86 instruction. Afterwards I'm loading the keyboard handler:

void kmain(void)
{
    //Using grub bootloader..
    idt_init();
    kb_init();
    load_idt_entry(0x21, (unsigned long) keyboard_handler, 0x08, 0x8e);
}

This is the implementation:

#include "kprintf.h"
#include "port_io.h"
#include "keyboard_map.h"

void kb_init(void)
{
    /* 0xFD is 11111101 - enables only IRQ1 (keyboard)*/
    write_port(0x21 , 0xFD);
}

void keyboard_handler(void)
{
    unsigned char status;
    char keycode;
    char *vidptr = (char*)0xb8000;  //video mem begins here.
    /* Acknownlegment */

    int current_loc = 0;
    status = read_port(0x64);
    /* Lowest bit of status will be set if buffer is not empty */
    if (status & 0x01) {
        keycode = read_port(0x60);
        if(keycode < 0)
            return;
        vidptr[current_loc++] = keyboard_map[keycode];
        vidptr[current_loc++] = 0x07;
    }

    write_port(0x20, 0x20);
}

This is the extra code I'm using:

section .text

global load_idt
global keyboard_handler

extern kprintf
extern keyboard_handler_main

load_idt:
    sti
    mov edx, [esp + 4]
    lidt [edx]
    ret

global read_port
global write_port

; arg: int, port number.
read_port:
    mov edx, [esp + 4]
    in al, dx   
    ret

; arg: int, (dx)port number
;      int, (al)value to write
write_port:
    mov   edx, [esp + 4]    
    mov   al, [esp + 4 + 4]  
    out   dx, al  
    ret

This is my entry point:

bits 32
section .text
;grub bootloader header
        align 4
        dd 0x1BADB002            ;magic
        dd 0x00                  ;flags
        dd - (0x1BADB002 + 0x00) ;checksum. m+f+c should be zero

global start
extern kmain            

start:
;  cli          ;block interrupts
  mov esp, stack_space  ;set stack pointer
  call kmain
  hlt           ;halt the CPU

section .bss
resb 8192       ;8KB for stack
stack_space:

I'm using QEMU to run the kernel:

qemu-system-i386 -kernel kernel

The problem is that I'm not getting any character on the screen. Instead, I still get the same output:

SeaBIOS (version Ubuntu-1.8.2-1-ubuntu1)
Booting from ROM...

How do I solve this problem? Any suggestions?

Solution

You have a number of issues with your code. The main ones are discussed individually below.

The HLT instruction will halt the current CPU waiting for the next interrupt. You do have interrupts enabled by this point. After the first interrupt (keystroke) the code after HLT will be executed. It will start executing whatever random data is in memory. You could modify your kmain to do an infinite loop with a HLT instruction. Something like this should work:

while(1) __asm__("hlt\n\t");

In this code:

load_idt:
    sti
    mov edx, [esp + 4]
    lidt [edx]
    ret

It is generally a better idea to use STI after you update the interrupt table, not before it. This would be better:

load_idt:
    mov edx, [esp + 4]
    lidt [edx]
    sti
    ret

Your interrupt handler needs to perform an iretd to properly return from an interrupt. Your function keyboard_handler will do a ret to return. To resolve this you could create an assembly wrapper that calls the C keyboard_handler function and then does an IRETD.

In a NASM assembly file you could define a global function called keyboard_handler_int like this:

extern keyboard_handler
global keyboard_handler_int

keyboard_handler_int:
    call keyboard_handler
    iretd

The code to setup the IDT entry would look like this:

load_idt_entry(0x21, (unsigned long) keyboard_handler_int, 0x08, 0x8e);

Your kb_init function eventually enables (via a mask) the keyboard interrupt. Unfortunately, you set up the keyboard handler after you enable that interrupt. It is possible for a keystroke to be pressed after the interrupt is enabled and before the entry is placed in the IDT. A quick fix is to set your keyboard handler up before the call to kb_init with something like:

void kmain(void)
{
    //Using grub bootloader..
    idt_init();
    load_idt_entry(0x21, (unsigned long) keyboard_handler_int, 0x08, 0x8e);
    kb_init();
    while(1) __asm__("hlt\n\t");
}

The most serious problem that is likely causing your kernel to triple fault (and effectively rebooting the virtual machine) is the way you defined the idt_pointer structure. You used:

struct idt_pointer
{
    unsigned short limit;
    unsigned int base;
};

The problem is that default alignment rules will place 2 bytes of padding after limit and before base so that the unsigned int will be aligned at a 4 byte offset within the structure. To alter this behavior and pack the data without padding, you can use __attribute__((packed)) on the structure. The definition would look like this:

struct idt_pointer
{
    unsigned short limit;
    unsigned int base;
} __attribute__((packed));

Doing it this way means that there are no extra bytes placed between limit and base for alignment purposes. Failure to deal with the alignment issue effectively yields a base address that is incorrectly placed in the structure. The IDT pointer needs a 16-bit value representing the size of the IDT followed immediately by a 32-bit value representing the base address of your IDT.

More information on structure alignment and padding can be found in one of Eric Raymond's blogs. Because of the way that members of struct idt_entry are placed there are no extra padding bytes. If you are creating structs that you never want padded I recommend using __attribute__((packed));. This is generally the case when you are mapping a C data structure with a system defined structure. With that in mind I'd also pack struct idt_entry for clarity.

Other considerations

In the interrupt handler, although I suggested an IRETD, there is another issue. As your kernel grows and you add more interrupts you'll discover another problem. Your kernel may act erratically and registers may change values unexpectedly. The issue is that C functions acting as interrupt handlers will destroy the contents of some registers, but we don't save and restore them. Secondly, the direction flag (per the 32-bit ABI) is required to be cleared (CLD) before a function is called. You can't assume the direction flag is cleared upon entry to the interrupt routine. The ABI says:

EFLAGS The flags register contains the system flags, such as the direction flag and the carry flag. The direction flag must be set to the ‘‘forward’’ (that is, zero) direction before entry and upon exit from a function. Other user flags have no specified role in the standard calling sequence and are not preserved

You could push all the volatile registers individually but for brevity you can use the PUSHAD and POPAD instructions. An interrupt handler would be better if it looked like:

keyboard_handler_int:
    pushad                 ; Push all general purpose registers
    cld                    ; Clear direction flag (forward movement)
    call keyboard_handler
    popad                  ; Restore all general purpose registers
    iretd                  ; IRET will restore required parts of EFLAGS
                           ;   including the direction flag

If you were to save and restore all the volatile registers manually you'd have to save and restore EAX, ECX, and EDX as they don't need to be preserved across C function calls. It generally isn't a good idea to to use x87 FPU instructions in an interrupt handler (mostly for performance), but if you did you'd have to save and restore the x87 FPU state as well.

Sample Code

You didn't provide a complete example, so I filled in some of the gaps (including a simple keyboard map) and slight change to your keyboard handler. The revised keyboard handler only displays key down events and skips over characters that had no mapping. In all cases the code drops through to the end of the handler so that the PIC is sent an EOI (End Of Interrupt). The current cursor location is a static integer that will retain its value across interrupt calls. This allows the position to advance between each character press.

My kprintd.h file is empty, and I put ALL the assembler prototypes into your port_io.h. The prototypes should be divided properly into multiple headers. I only did it this way to reduce the number of files. My file lowlevel.asm defines all the low level assembly routines. The final code is as follows:

kernel.asm:

bits 32
section .text
;grub bootloader header
        align 4
        dd 0x1BADB002            ;magic
        dd 0x00                  ;flags
        dd - (0x1BADB002 + 0x00) ;checksum. m+f+c should be zero

global start
extern kmain

start:
    lgdt [gdtr]                 ; Load our own GDT, the GDTR of Grub may be invalid

    jmp CODE32_SEL:.setcs       ; Set CS to our 32-bit flat code selector
.setcs:
    mov ax, DATA32_SEL          ; Setup the segment registers with our flat data selector
    mov ds, ax
    mov es, ax
    mov fs, ax
    mov gs, ax
    mov ss, ax
    mov esp, stack_space        ; set stack pointer

    call kmain

; If we get here just enter an infinite loop
endloop:
    hlt                         ; halt the CPU
    jmp endloop

; Macro to build a GDT descriptor entry
%define MAKE_GDT_DESC(base, limit, access, flags) \
    (((base & 0x00FFFFFF) << 16) | \
    ((base & 0xFF000000) << 32) | \
    (limit & 0x0000FFFF) | \
    ((limit & 0x000F0000) << 32) | \
    ((access & 0xFF) << 40) | \
    ((flags & 0x0F) << 52))

section .data
align 4
gdt_start:
    dq MAKE_GDT_DESC(0, 0, 0, 0); null descriptor
gdt32_code:
    dq MAKE_GDT_DESC(0, 0x00ffffff, 10011010b, 1100b)
                                ; 32-bit code, 4kb gran, limit 0xffffffff bytes, base=0
gdt32_data:
    dq MAKE_GDT_DESC(0, 0x00ffffff, 10010010b, 1100b)
                                ; 32-bit data, 4kb gran, limit 0xffffffff bytes, base=0
end_of_gdt:

gdtr:
    dw end_of_gdt - gdt_start - 1
                                ; limit (Size of GDT - 1)
    dd gdt_start                ; base of GDT

CODE32_SEL equ gdt32_code - gdt_start
DATA32_SEL equ gdt32_data - gdt_start

section .bss
resb 8192                       ; 8KB for stack
stack_space:

lowlevel.asm:

section .text

extern keyboard_handler
global read_port
global write_port
global load_idt
global keyboard_handler_int

keyboard_handler_int:
    pushad
    cld
    call keyboard_handler
    popad
    iretd

load_idt:
    mov edx, [esp + 4]
    lidt [edx]
    sti
    ret

; arg: int, port number.
read_port:
    mov edx, [esp + 4]
    in al, dx
    ret

; arg: int, (dx)port number
;      int, (al)value to write
write_port:
    mov   edx, [esp + 4]
    mov   al, [esp + 4 + 4]
    out   dx, al
    ret

port_io.h:

extern unsigned char read_port (int port);
extern void write_port (int port, unsigned char val);
extern void kb_init(void);

kprintf.h:

/* Empty file */

keyboard_map.h:

unsigned char keyboard_map[128] =
{
    0,  27, '1', '2', '3', '4', '5', '6', '7', '8',     /* 9 */
  '9', '0', '-', '=', '\b',     /* Backspace */
  '\t',                 /* Tab */
  'q', 'w', 'e', 'r',   /* 19 */
  't', 'y', 'u', 'i', 'o', 'p', '[', ']', '\n', /* Enter key */
    0,                  /* 29   - Control */
  'a', 's', 'd', 'f', 'g', 'h', 'j', 'k', 'l', ';',     /* 39 */
 '\'', '`',   0,                /* Left shift */
 '\\', 'z', 'x', 'c', 'v', 'b', 'n',                    /* 49 */
  'm', ',', '.', '/',   0,                              /* Right shift */
  '*',
    0,  /* Alt */
  ' ',  /* Space bar */
    0,  /* Caps lock */
    0,  /* 59 - F1 key ... > */
    0,   0,   0,   0,   0,   0,   0,   0,
    0,  /* < ... F10 */
    0,  /* 69 - Num lock*/
    0,  /* Scroll Lock */
    0,  /* Home key */
    0,  /* Up Arrow */
    0,  /* Page Up */
  '-',
    0,  /* Left Arrow */
    0,
    0,  /* Right Arrow */
  '+',
    0,  /* 79 - End key*/
    0,  /* Down Arrow */
    0,  /* Page Down */
    0,  /* Insert Key */
    0,  /* Delete Key */
    0,   0,   0,
    0,  /* F11 Key */
    0,  /* F12 Key */
    0,  /* All other keys are undefined */
};

keyb.c:

#include "kprintf.h"
#include "port_io.h"
#include "keyboard_map.h"

void kb_init(void)
{
    /* This is a very basic keyboard initialization. The assumption is we have a
     * PS/2 keyboard and it is already in a proper state. This may not be the case
     * on real hardware. We simply enable the keyboard interupt */

    /* Get current master PIC interrupt mask */
    unsigned char curmask_master = read_port (0x21);

    /* 0xFD is 11111101 - enables only IRQ1 (keyboard) on master pic
       by clearing bit 1. bit is clear for enabled and bit is set for disabled */
    write_port(0x21, curmask_master & 0xFD);
}

/* Maintain a global location for the current video memory to write to */
static int current_loc = 0;
/* Video memory starts at 0xb8000. Make it a constant pointer to
   characters as this can improve compiler optimization since it
   is a hint that the value of the pointer won't change */
static volatile char *const vidptr = (char*)0xb8000;

void keyboard_handler(void)
{
    signed char keycode;

    keycode = read_port(0x60);
    /* Only print characters on keydown event that have
     * a non-zero mapping */
    if(keycode >= 0 && keyboard_map[keycode]) {
        vidptr[current_loc++] = keyboard_map[keycode];
        /* Attribute 0x07 is white on black characters */
            vidptr[current_loc++] = 0x07;
    }

    /* Send End of Interrupt (EOI) to master PIC */
    write_port(0x20, 0x20);
}

main.c:

#include "port_io.h"

#define IDT_SIZE 256
#define PIC_1_CTRL 0x20
#define PIC_2_CTRL 0xA0
#define PIC_1_DATA 0x21
#define PIC_2_DATA 0xA1

void keyboard_handler_int();
void load_idt(void*);

struct idt_entry
{
    unsigned short int offset_lowerbits;
    unsigned short int selector;
    unsigned char zero;
    unsigned char flags;
    unsigned short int offset_higherbits;
} __attribute__((packed));

struct idt_pointer
{
    unsigned short limit;
    unsigned int base;
} __attribute__((packed));

struct idt_entry idt_table[IDT_SIZE];
struct idt_pointer idt_ptr;

void load_idt_entry(int isr_number, unsigned long base, short int selector, unsigned char flags)
{
    idt_table[isr_number].offset_lowerbits = base & 0xFFFF;
    idt_table[isr_number].offset_higherbits = (base >> 16) & 0xFFFF;
    idt_table[isr_number].selector = selector;
    idt_table[isr_number].flags = flags;
    idt_table[isr_number].zero = 0;
}

static void initialize_idt_pointer()
{
    idt_ptr.limit = (sizeof(struct idt_entry) * IDT_SIZE) - 1;
    idt_ptr.base = (unsigned int)&idt_table;
}

static void initialize_pic()
{
    /* ICW1 - begin initialization */
    write_port(PIC_1_CTRL, 0x11);
    write_port(PIC_2_CTRL, 0x11);

    /* ICW2 - remap offset address of idt_table */
    /*
    * In x86 protected mode, we have to remap the PICs beyond 0x20 because
    * Intel have designated the first 32 interrupts as "reserved" for cpu exceptions
    */
    write_port(PIC_1_DATA, 0x20);
    write_port(PIC_2_DATA, 0x28);

    /* ICW3 - setup cascading */
    write_port(PIC_1_DATA, 0x04);
    write_port(PIC_2_DATA, 0x02);

    /* ICW4 - environment info */
    write_port(PIC_1_DATA, 0x01);
    write_port(PIC_2_DATA, 0x01);
    /* Initialization finished */

    /* mask interrupts */
    write_port(0x21 , 0xff);
    write_port(0xA1 , 0xff);
}

void idt_init()
{
    initialize_pic();
    initialize_idt_pointer();
    load_idt(&idt_ptr);
}

void kmain(void)
{
    //Using grub bootloader..
    idt_init();
    load_idt_entry(0x21, (unsigned long) keyboard_handler_int, 0x08, 0x8e);
    kb_init();
    while(1) __asm__("hlt\n\t");
}

In order to link this kernel I use a file link.ld with this definition:

/*
*  link.ld
*/
OUTPUT_FORMAT(elf32-i386)
ENTRY(start)
SECTIONS
 {
   . = 0x100000;
   .text : { *(.text) }
   .rodata : { *(.rodata) }
   .data : { *(.data) }
   .bss  : { *(.bss)  }
 }

I compile and link this code using a GCC i686 cross compiler with these commands:

nasm -f elf32 -g -F dwarf kernel.asm -o kernel.o
nasm -f elf32 -g -F dwarf lowlevel.asm -o lowlevel.o
i686-elf-gcc -g -m32  -c main.c -o main.o -ffreestanding -O3 -Wall -Wextra -pedantic
i686-elf-gcc -g -m32  -c keyb.c -o keyb.o -ffreestanding -O3 -Wall -Wextra -pedantic
i686-elf-gcc -g -m32  -Wl,--build-id=none -T link.ld -o kernel.elf -ffreestanding -nostdlib lowlevel.o main.o keyb.o kernel.o -lgcc

The result is a kernel called kernel.elf with debug information. I prefer an optimization level of -O3 rather than a default of -O0. Debug information makes it easier to debug with QEMU and GDB. The kernel can be debugged with these commands:

qemu-system-i386 -kernel kernel.elf -S -s &

gdb kernel.elf \
        -ex 'target remote localhost:1234' \
        -ex 'layout src' \
        -ex 'layout regs' \
        -ex 'break kmain' \
        -ex 'continue'

If you wish to debug at the assembly code level replace layout src with layout asm. When run with the input the quick brown fox jumps over the lazy dog 01234567890 QEMU displayed this: