OSDev.org

The Place to Start for Operating System Developers
http://f.osdev.org/

General Protection Fault upon return from Intterupt handler

http://f.osdev.org/viewtopic.php?t=27857

Page 2 of 2

Re: General Protection Fault upon return from Intterupt hand

Posted: Thu Mar 27, 2014 3:36 pm
by kemosparc
Dear Sortie,

Thanks a lot for your thorough reply, although I did not get from it except that I need to clean my code.

This code I am experimenting with so I can learn more about kernels and get more in depth knowledge, so what I have is a sandbox that I am trying things out with, and one day when I am confident enought I might sit down and write a cleaner kernel; so it is not meant to be as clean as you expect.

Nevertheless, I agree that the code was not clean, but as you said I was following the tutorial by James Molloy trying to learn, and I did not have a clue that there is a complete page with bugs/problems regarding the tutroial, and I would like to thank you for refering to that; I totally missed the link even though it is just to the right of the tutorial link.

I just have one comment regarding the atitude and the language being used in communication, kindly I would prefer that generally people choose their language and select appropriate words when trying to transfer knowledge to others. Words that starts with "F" and using stupid a lot, I don't think it is very appropriate in general, at the end of the day I think that people hangout here on this forum to share information and to help each other rather than enumerating problems without really much of a help; you should consider that you are describing stuff to a range of people who are not all at the same level of understanding of the topic and that is why they are seeking help in the first place; so explaining to someone a subject as if he already understand is totally defeating the purpose. You might be storng in a specific area but this really does not matter for people like me, what really matters is the quality of the help. At the end of the day I think that James Molloy did a lot of people like me a favour by spending some of his time providing people like me such tutorial which at least introduced a lot to me even though it has some bugs (like any piece of software), so I think it is not fair using the word "stupid" in that context, rather I would expect you to show us your elegant code and do the intiative of updating the tutorial, correcting it, and providing V2 of it.


Now back to the tech stuff, regarding the problem I have, I followed your reply as much as I can and I sat down for the past three hours trying to clean the code as much as possible; At the end of my post you will find two code sections with the newer version after cleaning it. (The Full code is the code for the First Experiment: read below and you will understand)

Obvioiusly there is something wrong with my understanding of the interrupt handler code, so I did three experiments.

The first experiment, I removed completely all the parameters to my interrupt handler and all what it does is to print a hello world message with a counter to ensure that it is being fired contineously, it is not even aware of the interrupt number and all what I aimed at is to make it return without crashing the VM. Here is the code of my interrupt related code for that experiment, and it worked fine. One thing to mention here is as you can see I had to add 16 to the rsp and not 8, and it did not work with 8 (VM crashes), so it would be good if you have an explanation for that because I don't.

Code: Select all

extern "C" void idt_handler()
{
    tick++;
    video.setPosition(10,10);
    video.putString("Hello World:  ",COLOR_BLUE,COLOR_WHITE);
    video.putDecimal(tick,COLOR_BLUE,COLOR_WHITE);
    video.putString("      ",COLOR_BLUE,COLOR_WHITE);
    Ports::outportb(PIC1_COMMAND, 0x20);
}



idt_common_stub:
    mov ax, ds               ; Lower 16-bits of eax = ds.
    push rax                 ; save the data segment descriptor

    mov ax, 0x10  ; load the kernel data segment descriptor
    mov ds, ax
    mov es, ax
    mov fs, ax
    mov gs, ax

    call idt_handler

    pop rbx        ; reload the original data segment descriptor
    mov ds, bx
    mov es, bx
    mov fs, bx
    mov gs, bx
    add rsp,16
    iretq           ; pops 5 things at once: CS, EIP, EFLAGS, SS, and ESP

%macro ISR_NOERRCODE 1
  [GLOBAL isr%1]
  isr%1:
    cli
    push byte 0
    push byte %1
    jmp idt_common_stub
%endmacro

%macro ISR_ERRCODE 1
  [GLOBAL isr%1]
  isr%1:
    cli
    push byte %1
    jmp idt_common_stub
%endmacro

%macro IRQ 2
  global irq%1
  irq%1:
    cli
    push byte 0
    push byte %2
    jmp idt_common_stub
%endmacro

In the second experiment, I aimed at passing the interrupt number and the error code originally pushed on the stack in the isr and irq macros. I did that by passing them by value, and to do that without messing up with the ds register being pushed on the stack I had to move the pushing of the ds into the macros to take place before pushing the interrupt number and the error code, and this worked and displayed the interrupt number of PIC "32" on the screen, and I also added a counter to make sure that it is being fired contineously.One thing I would like to mention is the parameters to the interrupt handler which are two uint64_t and not uint8_t. Below is the code.

Code: Select all

extern "C" void idt_handler(uint64_t v1,uint64_t v2)
{
    tick++;
    video.setPosition(10,10);
    video.putString("interupt:  ",COLOR_BLUE,COLOR_WHITE);
    video.putDecimal(v1,COLOR_BLUE,COLOR_WHITE);
    video.putString(", Counter:  ",COLOR_BLUE,COLOR_WHITE);
    video.putDecimal(tick,COLOR_BLUE,COLOR_WHITE);
    video.putString("      ",COLOR_BLUE,COLOR_WHITE);

    // Send an EOI (end of interrupt) signal to the PICs.
    // If this interrupt involved the slave.

    if (v1 >= 40)
    {
        // Send reset signal to slave.
        Ports::outportb(PIC2_COMMAND, 0x20);
    }
    // Send reset signal to master. (As well as slave, if necessary).
    Ports::outportb(PIC1_COMMAND, 0x20);
}


idt_common_stub:

    mov ax, 0x10  ; load the kernel data segment descriptor
    mov ds, ax
    mov es, ax
    mov fs, ax
    mov gs, ax

    call idt_handler

    pop rbx        ; reload the original data segment descriptor
    mov ds, bx
    mov es, bx
    mov fs, bx
    mov gs, bx
    add rsp,16
    iretq           ; pops 5 things at once: CS, EIP, EFLAGS, SS, and ESP

%macro ISR_NOERRCODE 1
  [GLOBAL isr%1]

  isr%1:
    cli
    mov ax, ds               ; Lower 16-bits of eax = ds.
    push rax                 ; save the data segment descriptor
    push byte 0
    push byte %1
    jmp idt_common_stub
%endmacro

%macro ISR_ERRCODE 1
  [GLOBAL isr%1]
  isr%1:
    cli
    mov ax, ds               ; Lower 16-bits of eax = ds.
    push rax                 ; save the data segment descriptor

    push byte %1
    jmp idt_common_stub
%endmacro

%macro IRQ 2
  global irq%1
  irq%1:
    cli
    mov ax, ds               ; Lower 16-bits of eax = ds.
    push rax                 ; save the data segment descriptor
    push byte 0
    push byte %2
    jmp idt_common_stub
%endmacro
In the third experiment, I created a new structure for the interrupt number and the error code and tried to pass it by reference as per the code below and I could not make it work (VM Crashes)

Code: Select all


struct interrupts{
    uint64_t intNo, errCode;
} ;


extern "C" void idt_handler(struct interrupts * _interrupts)
{
    tick++;
    video.setPosition(10,10);
    video.putString("interupt:  ",COLOR_BLUE,COLOR_WHITE);
    video.putDecimal(_interrupts->intNo,COLOR_BLUE,COLOR_WHITE);
    video.putString(", Counter:  ",COLOR_BLUE,COLOR_WHITE);
    video.putDecimal(tick,COLOR_BLUE,COLOR_WHITE);
    video.putString("      ",COLOR_BLUE,COLOR_WHITE);

    // Send an EOI (end of interrupt) signal to the PICs.
    // If this interrupt involved the slave.

    if (_interrupts->intNo >= 40)
    {
        // Send reset signal to slave.
        Ports::outportb(PIC2_COMMAND, 0x20);
    }
    // Send reset signal to master. (As well as slave, if necessary).
    Ports::outportb(PIC1_COMMAND, 0x20);
}

; This is our common stub for both irqs and isrs
idt_common_stub:

    mov ax, 0x10  ; load the kernel data segment descriptor
    mov ds, ax
    mov es, ax
    mov fs, ax
    mov gs, ax

    push rsp
    call idt_handler

    pop rbx        ; reload the original data segment descriptor
    mov ds, bx
    mov es, bx
    mov fs, bx
    mov gs, bx
    add rsp,16
    iretq           ; pops 5 things at once: CS, EIP, EFLAGS, SS, and ESP

%macro ISR_NOERRCODE 1
  [GLOBAL isr%1]
  isr%1:
    cli
    mov ax, ds               ; Lower 16-bits of eax = ds.
    push rax                 ; save the data segment descriptor
    push byte 0
    push byte %1
    jmp idt_common_stub
%endmacro

%macro ISR_ERRCODE 1
  [GLOBAL isr%1]
  isr%1:
    cli
    mov ax, ds               ; Lower 16-bits of eax = ds.
    push rax                 ; save the data segment descriptor

    push byte %1
    jmp idt_common_stub
%endmacro

%macro IRQ 2
  global irq%1
  irq%1:
    cli
    mov ax, ds               ; Lower 16-bits of eax = ds.
    push rax                 ; save the data segment descriptor
    push byte 0
    push byte %2
    jmp idt_common_stub
%endmacro

I also attached 2 screen shots for the first two experiments


Finally I have a couple of questions regarding your post


First what did you mean by the following:
You are pushing the general purpose registers onto the kernel stack before you have changed the the data segment. You don't need to change the stack segment, since the CPU does that for you. I forgot if any of this is even needed on x86_64.

I googled a lot trying to find the bits definition of the IDT flag to set the cli/sti per interrupt but I could not find it, so can you please send me any reference for that?
You clear interrupts when you enter the interrupt handler, but as described in my above link, that's stupid and you should just decide whether you want interrupts on when you set the IDT entries.

Code: Select all

[BITS 64]
global idtInit
extern idtP
idtInit:
   lidt [idtP]
   iretq

extern idt_handler

; This is our common stub for both irqs and isrs
idt_common_stub:
    mov ax, ds               ; Lower 16-bits of eax = ds.
    push rax                 ; save the data segment descriptor

    mov ax, 0x10  ; load the kernel data segment descriptor
    mov ds, ax
    mov es, ax
    mov fs, ax
    mov gs, ax

    call idt_handler

    pop rbx        ; reload the original data segment descriptor
    mov ds, bx
    mov es, bx
    mov fs, bx
    mov gs, bx
    add rsp,16
    iretq           ; pops 5 things at once: CS, EIP, EFLAGS, SS, and ESP

%macro ISR_NOERRCODE 1
  [GLOBAL isr%1]
  isr%1:
    cli
    push byte 0
    push byte %1
    jmp idt_common_stub
%endmacro

%macro ISR_ERRCODE 1
  [GLOBAL isr%1]
  isr%1:
    cli
    push byte %1
    jmp idt_common_stub
%endmacro

%macro IRQ 2
  global irq%1
  irq%1:
    cli
    push byte 0
    push byte %2
    jmp idt_common_stub
%endmacro

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_ERRCODE   8
ISR_NOERRCODE 9
ISR_ERRCODE   10
ISR_ERRCODE   11
ISR_ERRCODE   12
ISR_ERRCODE   13
ISR_ERRCODE   14
ISR_NOERRCODE 15
ISR_NOERRCODE 16
ISR_NOERRCODE 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_NOERRCODE 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

Code: Select all

#include "icxxabi.h"
#include "Video.h"
#include "Disk.h"
#include "CPUID.h"
#include "PageManager.h"
#include "MemoryManager.h"
#include "HeapIndexItem.h"
#include "PCIConfigHeaderManager.h"
#include "new.h"
#include "E1000.h"
#include "rtl8139.h"

/*
* Interrupt Descriptor Structure
*/
#define IDT_SIZE    256

#define MAIN_FILE

#include "globals.h"
const char * exception_messages[32] =
{
    "Division By Zero",
    "Debug",
    "Non Maskable Interrupt",
    "Breakpoint Exception",
    "Into Detected Overflow Exception",
    "Out of Bounds Exception",
    "Invalid Opcode Exception",
    "No Coprocessor Exception",
    "Double Fault Exception",
    "Coprocessor Segment Overrun Exception",
    "Bad TSS Exception",
    "Segment Not Present Exception",
    "Stack Fault Exception",
    "General Protection Fault Exception",
    "Page Fault Exception",
    "Unknown Interrupt Exception",
    "Coprocessor Fault Exception",
    "Alignment Check Exception (486+)",
    "Machine Check Exception (Pentium/586+)",
    "Reserved Exceptions",
    "Reserved Exceptions",
    "Reserved Exceptions",
    "Reserved Exceptions",
    "Reserved Exceptions",
    "Reserved Exceptions",
    "Reserved Exceptions",
    "Reserved Exceptions",
    "Reserved Exceptions",
    "Reserved Exceptions",
    "Reserved Exceptions",
    "Reserved Exceptions",
    "Reserved Exceptions"
};

char kbdus[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 */
};



/*
* Prototypes
*/
void idtStart(void);
void idtSet(uint8_t, uint64_t, uint16_t, uint8_t);

extern "C" void isrHandler(struct registers *);
extern "C" void idtInit();

extern "C" void isr0();
extern "C" void isr1();
extern "C" void isr2();
extern "C" void isr3();
extern "C" void isr4();
extern "C" void isr5();
extern "C" void isr6();
extern "C" void isr7();
extern "C" void isr8();
extern "C" void isr9();
extern "C" void isr10();
extern "C" void isr11();
extern "C" void isr12();
extern "C" void isr13();
extern "C" void isr14();
extern "C" void isr15();
extern "C" void isr16();
extern "C" void isr17();
extern "C" void isr18();
extern "C" void isr19();
extern "C" void isr20();
extern "C" void isr21();
extern "C" void isr22();
extern "C" void isr23();
extern "C" void isr24();
extern "C" void isr25();
extern "C" void isr26();
extern "C" void isr27();
extern "C" void isr28();
extern "C" void isr29();
extern "C" void isr30();
extern "C" void isr31();
extern "C" void irq0 ();
extern "C" void irq1 ();
extern "C" void irq2 ();
extern "C" void irq3 ();
extern "C" void irq4 ();
extern "C" void irq5 ();
extern "C" void irq6 ();
extern "C" void irq7 ();
extern "C" void irq8 ();
extern "C" void irq9 ();
extern "C" void irq10();
extern "C" void irq11();
extern "C" void irq12();
extern "C" void irq13();
extern "C" void irq14();
extern "C" void irq15();


/* Setup Table and Pointer */

idtEntry idt[IDT_SIZE];
idtPointer idtP;



void IRQ_set_mask(unsigned char IRQline) {
    uint16_t port;
    uint8_t value;

    if(IRQline < 8) {
        port = PIC1_DATA;
    } else {
        port = PIC2_DATA;
        IRQline -= 8;
    }
    value = Ports::inportb(port) | (1 << IRQline);
    Ports::outportb(port, value);
}

void IRQ_clear_mask(unsigned char IRQline) {
    uint16_t port;
    uint8_t value;

    if(IRQline < 8) {
        port = PIC1_DATA;
    } else {
        port = PIC2_DATA;
        IRQline -= 8;
    }
    value = Ports::inportb(port) & ~(1 << IRQline);
    Ports::outportb(port, value);
}
void register_interrupt_handler(uint8_t n, isr_t handler)
{
    interrupt_handlers[n] = handler;
    if ( n >= IRQ0)
        IRQ_clear_mask(n-IRQ0);
}

void idtStart(void) {
   /* Set IDT Pointer */
   idtP.limit = (sizeof(idtEntry) * IDT_SIZE) - 1;
   idtP.base = (uint64_t)&idt;
   /* Clear Memory for IDT's */
   Utils::memset((uint8_t *)&idt, 0, sizeof(idtEntry) * IDT_SIZE);

   Utils::memset((uint8_t *)&interrupt_handlers, 0, sizeof(isr_t)*256);

    Ports::outportb(0x20, 0x11);
    Ports::outportb(0xA0, 0x11);
    Ports::outportb(0x21, 0x20);
    Ports::outportb(0xA1, 0x28);
    Ports::outportb(0x21, 0x04);
    Ports::outportb(0xA1, 0x02);
    Ports::outportb(0x21, 0x01);
    Ports::outportb(0xA1, 0x01);
    Ports::outportb(0x21, 0x0);
    Ports::outportb(0xA1, 0x0);
    for ( unsigned char c = 0 ; c < 16 ; c++)
        IRQ_set_mask(c);

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

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

   /* Load IDT Table */
   idtInit();

}

void idtSet(uint8_t number, uint64_t base, uint16_t selector, uint8_t flags) {
   /* Set Base Address */
   idt[number].baseLow = base & 0xFFFF;
   idt[number].baseMid = (base >> 16) & 0xFFFF;
   idt[number].baseHigh = (base >> 32) & 0xFFFFFFFF;

   /* Set Selector */
   idt[number].selector = selector;
   idt[number].flags = flags;

   /* Set Reserved Areas to Zero */
   idt[number].reservedIst = 0;
   idt[number].reserved = 0;
}


extern "C" void idt_handler()
{
    tick++;
    video.setPosition(10,10);
    video.putString("Hello World:  ",COLOR_BLUE,COLOR_WHITE);
    video.putDecimal(tick,COLOR_BLUE,COLOR_WHITE);
    video.putString("      ",COLOR_BLUE,COLOR_WHITE);
    Ports::outportb(PIC1_COMMAND, 0x20);
}

static void timer_callback()
{
    tick++;
    video.setPosition(50,12);
    video.putString("Tick: ",COLOR_BLUE,COLOR_WHITE);
    video.putDecimal(tick,COLOR_BLUE,COLOR_WHITE);
    video.putString("\n",COLOR_BLUE,COLOR_WHITE);
}

static void keyboard_callback()
{
    unsigned char status = Ports::inportb(0x64);
    unsigned char sc = Ports::inportb(0x60);

    if ( sc&0x80)
    {
    }
    else
    {
        video.setPosition(40,15);
        char mychar = kbdus[sc];
        video.putString("Got Character: ",COLOR_BLUE,COLOR_WHITE);
        video.putCharacter((char)mychar,COLOR_BLUE,COLOR_WHITE);
        video.putString("\n",COLOR_BLUE,COLOR_WHITE);
    }
}


void init_timer(uint32_t frequency)
{
    // Firstly, register our timer callback.
    register_interrupt_handler(IRQ0, &timer_callback);
    register_interrupt_handler(IRQ1, &keyboard_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.
    uint32_t divisor = 1193180 / frequency;

    // Send the command byte.
    Ports::outportb(0x43, 0x34);

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

    // Send the frequency divisor.
    Ports::outportb(0x40, l);
    Ports::outportb(0x40, h);
}

extern "C" void kernel_main()
{
    video.initialize();
    video.clearScreen(COLOR_BLACK);
    idtStart();
    asm volatile ("sti");
    init_timer(50);
}

Re: General Protection Fault upon return from Intterupt hand

Posted: Thu Mar 27, 2014 7:09 pm
by sortie
Hi kemosparc,

I apologize if you felt offended by my language. I was merely expressing how the code surprised me in a bad way (see sites like thedailywtf.com - note that sites like that only show samples from professional developers that should have known better). I view the developers and code as two very separate things and I do understand and appreciate how you are a newcomer trying to learn. If I happen to say `wtf' when I see your code or happen to call the code stupid, that means nothing about you, but rather that I see some flaws that newcomers can have a hard time spotting. Indeed, I tend to even call my own code stupid when it is. This can be very confusing and offensive if you are not used to this culture. I'm a bit too used to it, and it tends to cause friction when I meet people that react badly to it. I'll attempt to avoid these confusing phrases in the future.

Please note that I took the time to give your code a read-through and note all the flaws I spotted. I'm not trying to insult you, but appreciate you are a newcomer, and attempted to help you resolve the issues. It is important to realize that osdev is very much about fully understanding everything that is going on, you cannot expect to succeed when you don't. It is therefore not unreasonable to expect you already have done careful research about interrupt handling on x86_64 when you attempt to write an interrupt handler in your custom operating system. If you need any explanation, please ask, but it is important you don't rely on the partial (and potentially wrong) knowledge you get from tutorials. For the record, I began my own osdev work using the JamesM tutorial and I very much appreciate its existence, even though I got hit hard by the bugs in it. I'm attempting to help you not make the same mistakes I did. I personally took the initiative to write the wiki.osdev.org article about the known bugs in the James Molloy Tutorial that I linked you to. The author is aware of my errata, I don't think he updated his tutorial yet (he is writing a new one), but unfortunately it is not a peer-edited tutorial unlike wiki.osdev.org so I am powerless to edit the tutorial. I would link to my `superior' code, but I have not yet fully cleansed it of my early design mistakes and it would hardly serve as a good example (I am considering writing a better tutorial kernel down the road, though, that would reflect what I consider good practices).

I find it slightly confusing or hard to help you when all we have is small code samples, which might not give us enough information. I would prefer if you would upload your full source code somewhere such that we have all the information we could possibly need. That is useful in cases like this where many files cooperate in handling interrupts.

Beware of using the experimental method in operating system's development. You should ideally understand the relevant parts of the processor environment in depth such that you can reason whether code is correct or not.

(I need to brush up on my segmentation knowledge. However, as far as I recall from previous discussions, it is somewhat bad that your interrupt handler thinks that ds, es, gs and gs are all the same value. It would probably be better if you treat each as different value and preserve them in your interrupt handlers.)

Your first experiment is flawed because it doesn't preserve the registers of the code that was interrupted, but you know this. Note that when I said adding 8 to rsp, that was unrelated to the part where you pop 16 bytes at the end of the interrupt handler. Note how the stack grows downwards on x86_64. The stack pointer always points to the last value that was pushed, or rather, to the value that would be popped if you pop. Executing a pop instruction on x86_64 means that the value pointed to by rsp is loaded into a register and the value 8 is added to rsp, such that it points to the next value on the stack. If you don't care about the value on the top of the stack, you can disregard it by simply adding 8 to rsp. Naturally, if you add 16 to rsp, that disregards the two top (physically bottom as the stack grows down) values on the stack. Note how you add 16 to rsp before the iretq because you pushed two values interrupt_number and error_code during the interrupt entry. The push byte part is somewhat misleading as on x86_64 you can only push 64-bit values, so each of those push instructions actually pushes 8 bytes.

I can see your interrupt handlers still run the cli instruction as the first thing to disable interrupts. The author of the tutorial you followed didn't do his research carefully and didn't know that the kernel programmer decides whether interrupt handlers run with interrupts disabled. You can set a bit in the interrupt descriptor table that controls whether interrupts are disabled or unaffected when the particular interrupt handler is run. Thus, you don't have a subtle race condition between entering the interrupt handler and disabling interrupts.

Your second experiment is flawed because it doesn't preserve the registers of the code that was interrupted, but you know this. It is also flawed because it unconditionally sends and EOI to the PIC even though the interrupt wasn't an interrupt request (such as a page fault). This can cause your kernel to loose interrupts. I can see you have combined the isr and irq handlers into a single assembly piece, which is good, but it is important to remember they are very different things when you get to high-level code. I have no idea how your second experiment happened to print 32 to the screen: It is not following the calling convention (wiki article that isn't fully finished)! The System V ABI for x86_64 passes parameters to functions in registers, rather than on the stack, so unless you are using an unexpected ABI somehow, you must simply accidentally have had the value `32' in the rdi register.

This is a very important error actually that I totally overlooked earlier. You don't actually follow (or know?) the ABI you are using: Note how you adapted James Molloy's 32-bit operating system to different processor and didn't properly adapt the code to the different calling convention. You need to spend some time carefully researching the System V ABI for x86_64 calling convention (assuming that's what you are using) (If you are not using a x86_64 gcc cross-compiler, you need to let us know immediately or this discussion are somewhat irrelevant details). This will be constantly useful in your future osdev endeavours. The immediately needed details (but not comprehensive!) is that parameters are passed in the registers rdi, rsi, rdx, rcx, r8, r9 (more arguments than this go on the stack), return values go in rax (rdx is also used if returning a pair of 64-bit values) and that the stack must be 16-byte aligned when you invoke a call instruction.

Don't move the saving of the segment registers into the macro entry points. This makes each of them larger, their only existence is to work around the fact that the CPU never tells you which interrupt handler was invoked. (My previous advise to load the kernel data segment early on was slightly misguided:) The push and pop instructions (anything addressing relative to the stack pointer) work relative to the stack segment. The CPU automatically loads the kernel stack segment when it invokes the interrupt handler, so push and pop operations are safe at all times. However, you should save the interrupted data segment value on the stack and load the kernel data segment (at least when you get a user-space) such that regular memory accesses are safe before doing any memory accesses outside of push and pop operations. (My segmentation knowledge is old and I have forgotten details, don't trust me too much on this.)

The third experiment is flawed for the above reasons: You don't follow the ABI, you acknowledge the PIC for ISRs, you don't save the registers of what you interrupted. Additionally, notice how you push rsp before calling idt_handler (which does not pass the structure you built on the stack to idt_handler function according to the ABI), but you never removed parameter from the stack: The parameters that actually are passed on the stack belongs to the function you called (and may be changed), but it is up to the caller to remove them from the stack. Therefore the stack is 8 bytes wrong when you attempt to do the iretq and thus you won't get it to return successfully. You could attempt to pop rsp, but I told you that any parameter given by the stack should now be considered changed, so that's not safe. Instead you should add 8 to rsp after calling idt_handler to remove that value. That's getting off track, though, because you should never have pushed the value in the first place but passed the pointer in rdi.

I notice your code has a parameter `struct interrupts* _interrupts'. This is bad style. First of all, the structure doesn't describe an interrupt, but rather an interrupted context. You should call it something more fitting such as struct interrupt_context. (Beware calling it struct registers, because you don't actually save all the registers when doing interrupt handlers. For instance, you normally don't save floating point registers, as the kernel doesn't use those, so it's not a structure of all the registers, and not all things in the structure are actually registers.) Secondly, notice how structure names and and variable names reside in their own separate namespaces. You can actually declare a variable with the same name as its structure `struct foo foo;' (Even function names are in their own namespace. You can actually in C use the stat(2) function as such `struct stat stat; stat(&stat);' if I am not mistaken). Thirdly, beware leading underscores in your identifiers, a practice you might have learned from other programming languages. In C and C++, anything starting with an underscore and a capital letter or two underscores are reserved for the implementation (and the usual namespace pollution rules don't apply). You are the implementation now, though, so you know whether it collides with anything, but it is still bad practice. Fortunately _interrupts is not in the reserved namespace since i is a lower case letter, but that's still not that pretty and signifies you may not know these namespace rules.

Alright, onto your questions:

I answered your first question above, when I tried to clarify what I meant about the segment registers when discussing the stack segment and push/pop.

Your second question is about where you can learn more about the interrupt descriptor table entries, I briefly mentioned that above before I saw your question (long post!). I recommend you get yourself a copy of the AMD CPU documentation. It documents your x86-64 system (as well as its 32-bit capabilities) in depth. It comes as a series of volumes that discuss everything from system programming to instruction encoding. This is what you should study carefully rather than wiki.osdev.org and tutorials. The system programming volume should cover exactly how interrupts work in the various modes (real mode, protected, long mode, ...) that are available on your x86-64 system. You'll want to find the section on the interrupt descriptor table (for long mode) and see the exact definitions and official explainations for yourself.


Looking at the code you appended to your code, why does the idtInit function return with an iretq? That seems wrong. Is this an interrupt handler or something that tells the processor where IDT is located? Functions return to the calling function using the ret instructions (that pops a value from the stack and jumps there, the call instruction pushes the location of where it should return and jumps to the requested function).

(Your kernel seems very oddly structured and in a single file, or perhaps you combined stuff for our pleasure.)

(Have you programming the GDT yet? You should do that before interrupt handlers as you need to tell in the IDT which segments it should jump to! Or wait, judging from the headers at the top, you seem fairly advanced. What stage are you actually at? I am pretty confused at this point without a full copy of your real source code.)

Alright, I think that concludes the stuff I can note about your code. Please ask if you have further questions. Remember that you should never osdev from incomplete knowledge. You need to immediately rectify your missing knowledge about the interrupt descriptor table, interrupt handling, and the ABI that you are using. Finally, be more sceptical of the knowledge you find online on wikis, tutorials, forums, and even of what I say. This is not an easy discipline, you need to truly understand everything and have paranormal information seeking skills and fantastic debugging skills. Fortunately, these things come with practice. :-)

Re: General Protection Fault upon return from Intterupt hand

Posted: Fri Mar 28, 2014 12:12 am
by Bender
Hi,
@sortie: +5 ('cause +1 is too less)
@kemosparc: Are you waiting for the auspicious moment here?
Can we now have the Bochs log? :) And perhaps some debugging?
I don't know what others opinions are but it's better to attach the log than post a screenshot or the
whole log, long posts filled with logs/code don't look good and when you're on your phone the experience is worse. :wink:
-Bender

Re: General Protection Fault upon return from Intterupt hand

Posted: Fri Mar 28, 2014 6:21 am
by kemosparc
Dear Sortie,

Thanks a lot for your reply, I really appreciate spending the time replying to my post.

Very detailed reply that will need thorough reading.

So what I am going to do is:

Read carefully your comments
Will read the ABI and the AMD manual sections related to my problem.
Will try to update the code and solve the problems.
Will spend some time cleaning the whole code as I have my own boot loader.
I will then zip everything and upload it to the forum so you guys can have a look at it one more time.

I might come back in the middle and ask transient questions to be able to continue.

Thanks a lot for the help, much appreciated.

Regarding Bender's reply,
I think I have a problem with my current qemu version as whenever I try to remote debug remotely it by passes the break points and I cannot perform proper debugging, it used to work before upgrading mu Ubuntu; if you have came across this and have a solution please let me know.

Anyways, mean while I will try to install Bochs to generate more rich logs and debug info.

Finally, I agree with the phone comment

Thanks for you all

Karim.

Re: General Protection Fault upon return from Intterupt hand

Posted: Fri Mar 28, 2014 9:30 am
by bwat
kemosparc wrote:Dear Sortie,

Thanks a lot for your thorough reply, although I did not get from it except that I need to clean my code.

This code I am experimenting with so I can learn more about kernels and get more in depth knowledge, so what I have is a sandbox that I am trying things out with, ...
Messy code is the result of messy thinking. Understand your problem better and your code will end up cleaner, and you'll have fewer bugs.

Re: General Protection Fault upon return from Intterupt hand

Posted: Fri Mar 28, 2014 9:51 am
by sortie
Note that the System V ABI is split into a core document and supplements (some of which unofficial) for each processor architecture. It's meant for operating system's developers and compiler developers as a specification, it's not meant to be a guide for those new to the ABI (though it is surprisingly readable, but as any ABI, it is large).

You can learn a lot about how C and C++ is translated to assembly by passing the -S option to your compiler. It makes the compiler stop before invoking the assembler, so you can see exactly what assembly your code is translated to (you are using nasm rather than the GNU assembler, so it is in another syntax, the same used by your cross objdump). I recommend you write very short test files with a few functions that call each other and see what assembly it is translated into. While this doesn't teach you the full ABI, you'll see it in practice and understand some common cases. If you don't understand the instructions used, you can look them up in the CPU documentation. (The compiler tend to generate good assembly, rather than readable assembly. The assembly generate without any optimization is particularly unreadable, but it gets better if you pass optimization options, though that can make the compiler very clever and also make it hard to read. Additionally, debug information and other auxiliary information is interleaved into your assembly, this is especially bad if C++ exception handling is supported. You can pass -fno-rtti and -fno-exceptions to your C++ compiler (or use C) to filter some of this out as long as you don't need to know about the C++ ABI specifics.)

Re: General Protection Fault upon return from Intterupt hand

Posted: Fri Mar 28, 2014 4:11 pm
by kemosparc
Hi,

First of all, I got bochs to start and run my kernel, but I am having a problem generating logs related to my GPF, any pointers to documentations are much appreciated.

So I am sorry for posting long posts, I will try to make bochs logging work to be able to attach the logs

Okay now, I have read the ABI and the AMD Architetcure Programmers Manual Volume 2 (Not the whole thing, I just read the parts related to my problem).

Accordingly, I have removed the CLI command from the irq and isr macros as I already define the type of gate descriptor as an interrupt gate, so the IFLAGS.IF is being cleared during the interrupt control transfer.

Also, according to the ABI manual, as you said I can pass the pointer to interrupt handle through the rdi register stated by the ABI page 21 figure 3.4.

I updated my code as shown here:

Code: Select all

idt_common_stub:
    mov ax, ds               ; Lower 16-bits of eax = ds.
    push rax                 ; save the data segment descriptor
    mov ax, 0x10  ; load the kernel data segment descriptor
    mov ds, ax
    mov es, ax
    mov fs, ax
    mov gs, ax

    mov rdi,rsp
    call idt_handler

    pop rbx        ; reload the original data segment descriptor
    mov ds, bx
    mov es, bx
    mov fs, bx
    mov gs, bx

    add rsp,16
    iretq           ; pops 5 things at once: CS, EIP, EFLAGS, SS, and ESP

%macro ISR_NOERRCODE 1
  [GLOBAL isr%1]
  isr%1:
    push byte 0
    push byte %1
    jmp idt_common_stub
%endmacro

%macro ISR_ERRCODE 1
  [GLOBAL isr%1]
  isr%1:
    push byte %1
    jmp idt_common_stub
%endmacro

%macro IRQ 2
  global irq%1
  irq%1:
    push byte 0
    push byte %2
    jmp idt_common_stub
%endmacro
Okay, now notice that I have moved into rdi the rsp pointer and did not add to it 8 to bypass the ds on the stack, and I meant that for a purpose.

Basically, although I have done all those updates, yet the same GPF code still fires. so I did the following to try to understand more. What I needed, is to see the stack when the GPF occur. so I did the following:

As I stated before I moved the rsp pointer to the rdi register as shown above right before the call to idt_handler.
Then I defined my inetrrupt context structure like this

Code: Select all

struct interrupted_context{
    uint64_t ds;
    uint64_t intNo, errCode;
    uint64_t return_address;
} ;
So now I can access the values on the stack throught my interrupt context struct inside my handler.

I modified my handler to look like this

Code: Select all

extern "C" void idt_handler(struct interrupted_context * ic)
{
    tick++;

    if ( ic->intNo == 32)
        video.setPosition(10,2);
    else video.setPosition(60,2);
    video.putString("int: ",COLOR_BLUE,COLOR_WHITE);
    video.putDecimal(ic->intNo,COLOR_BLUE,COLOR_WHITE);
    video.putString("\nC: ",COLOR_BLUE,COLOR_WHITE);
    video.putDecimal(tick,COLOR_BLUE,COLOR_WHITE);
    video.putString("\nds: ",COLOR_BLUE,COLOR_WHITE);
    video.putHexa(ic->ds,COLOR_BLUE,COLOR_WHITE);
    video.putString("\nintno: ",COLOR_BLUE,COLOR_WHITE);
    video.putDecimal(ic->intNo,COLOR_BLUE,COLOR_WHITE);
    video.putString("\nerrCode: ",COLOR_BLUE,COLOR_WHITE);
    video.putHexa(ic->errCode,COLOR_BLUE,COLOR_WHITE);
    video.putString("\nret: ",COLOR_BLUE,COLOR_WHITE);
    video.putHexa(ic->return_address,COLOR_BLUE,COLOR_WHITE);

    if ( ic->intNo != 32)
        for(;;);


    // Send an EOI (end of interrupt) signal to the PICs.
    // If this interrupt involved the slave.

    if (ic->intNo >= 40)
    {
        // Send reset signal to slave.
        Ports::outportb(PIC2_COMMAND, 0x20);
    }
    // Send reset signal to master. (As well as slave, if necessary).
    Ports::outportb(PIC1_COMMAND, 0x20);
}
This function will check the interrupt number and if it is 32 (PIC), it will set the cursor position on the left of the screen else it will set it on the right (Just a way to be able to view data on the screen obviously).

Then it will print the last four addresses on the stack (top -> bottom) which should be the ds register value pushed on the stack prior to calling the handler, the interrupt number, the error code, and finally the return address.

It also print a counter to show me the sequence of the interrupt occurrence

It will then check the interrupt number again and if it is not 32 it will go to infinite loop to stop and allow me to inspect the screen.

Here is the surprise.

The first interrupt was 32 and it went through successfully (I spent some time this morning under the impression that what generates the GPF is something related to the return of this interrupt). I inspected the return address and it was correct; basically cross checked with objdump.

The second interrupt is the GPF (13) and the return address for that interrupt was 0x1004b. So through the objdump command I located this line of code and it appeared to be that:

Code: Select all

int __cxa_atexit(void (*f)(void *), void *objptr, void *dso)
{
        if (__atexit_func_count >= ATEXIT_MAX_FUNCS) {return -1;};
        __atexit_funcs[__atexit_func_count].destructor_func = f;
   1003c:       4a 8d 0c c1             lea    (%rcx,%r8,8),%rcx
   10040:       48 89 39                mov    %rdi,(%rcx)
        __atexit_funcs[__atexit_func_count].obj_ptr = objptr;
   10043:       48 89 71 08             mov    %rsi,0x8(%rcx)
        __atexit_funcs[__atexit_func_count].dso_handle = dso;
   10047:       48 89 51 10             mov    %rdx,0x10(%rcx)
        __atexit_func_count++;
        return 0; /*I would prefer if functions returned 1 on success, but the ABI says...*/
->>>>>>  1004b:       c3                      retq   
   1004c:       0f 1f 40 00             nopl   0x0(%rax)

void *__dso_handle = 0; //Attention! Optimally, you should remove the '= 0' part and define this in your asm script.

int __cxa_atexit(void (*f)(void *), void *objptr, void *dso)
{
        if (__atexit_func_count >= ATEXIT_MAX_FUNCS) {return -1;};
   10050:       b8 ff ff ff ff          mov    $0xffffffff,%eax
        __atexit_funcs[__atexit_func_count].destructor_func = f;
        __atexit_funcs[__atexit_func_count].obj_ptr = objptr;
        __atexit_funcs[__atexit_func_count].dso_handle = dso;
        __atexit_func_count++;
        return 0; /*I would prefer if functions returned 1 on success, but the ABI says...*/
};

Notice the line above beginning with ->>>>>>

So this piece of code I got from OSDev website which enabled me to create new objects, and since I removed all the code that instantiate new objects in the process of cleaning up (basically removed the memory management functionality) I was kind of surprised.

After some testing I found out that this function is being called upon exiting my kernel_main function

Code: Select all

extern "C" void kernel_main()
{
    video.initialize();
    video.clearScreen(COLOR_BLACK);
    idtStart();
    init_timer(50);
    asm volatile ("sti");
->>>>>>   for(;;);
}
So to make sure that this is the problem, I decided to put a for loop at the end of my kernel main function as shown by the line above beginning with ->>>>>>, and the GPF disappeared and the PIC interrupt kept on firing without any problems.

I got this code from the following link http://wiki.osdev.org/C%2B%2B and I don't know if it is buggy or not. Basically this is the only place that I could find some sort of a tutorial on creating the new/delete operators for instantiation and disposal of objects.

Any suggestions will be much appreciated, and I will continue to research this on my side.

Thanks
Karim

Re: General Protection Fault upon return from Intterupt hand

Posted: Sat Mar 29, 2014 9:01 am
by kemosparc
Hi,

I have some updates.

I think that I have misinterpreted some of the symptoms in my previous reply.

After looking a little bit thoroughly, I found the foillowing:

This is how I call my kernel main function:

Code: Select all

[BITS 64]
SECTION .text
global _start
_start:
jmp Main64

Main64:
	cli                           ; Clear the interrupt flag.

        extern kernel_main
        call kernel_main

        hlt
SECTION .bss
_stack_end:
    resb    8192
_stack_start:
So when the first interrupt fires, which is PIC (32) the rip is at the halt statement, but when I dumped the stack just after the interrupt as stated in my previous post I found that the return address is 0x10009, and using objdump as shown below I found that this is the address just below the hlt statement.

Code: Select all

Main64:
        cli                           ; Clear the interrupt flag.
   10002:       fa                      cli    

    extern kernel_main
    call kernel_main
   10003:       e8 88 63 00 00          callq  16390 <kernel_main>

    hlt
   10008:       f4                      hlt    
   10009:       0f 1f 80 00 00 00 00    nopl   0x0(%rax)
So most probably the first interrupt return to that location which is nopl and this generates the GPF (second interrupt).

I looked at the source code of James Molly "boot.s" and I found out that it uses "jmp $" instead of hlt.
I tried that and it worked well, neverless the CPU is drained in the infinite loop.

I check another OS "ChickenOS" and I found out the it uses

Code: Select all

halt:
        hlt                          ; halt machine should kernel return
        jmp halt
Which makes sence as when the interrupt returns it will execute the jmp statment and hence go to halt again.

I used this and it worked good, and the CPU usage went down as well.

I don't know if the results I ended up with are correct or not ???

Any comments are appreciated
Thanks
Karim.

Re: General Protection Fault upon return from Intterupt hand

Posted: Sat Mar 29, 2014 9:16 am
by Nable
kemosparc wrote:Which makes sence as when the interrupt returns it will execute the jmp statment and hence go to halt again.

I used this and it worked good, and the CPU usage went down as well.

I don't know if the results I ended up with are correct or not ???

Any comments are appreciated
Thanks
Karim.
This post may help you: http://forum.osdev.org/viewtopic.php?f= ... lt#p230192
ot: shame on rarely-sleeping me: one should read "It's well known" instead of "It's know"
All times are UTC-06:00
Page 2 of 2

Powered by phpBB® Forum Software © phpBB Limited