gcc - Cannot modify data segment register. When tried General Protection Error is thrown

Question

I have been trying to create an ISR handler following this tutorial by James Molloy but I got stuck. Whenever I throw a software interrupt, general purpose registers and the data segment register is pushed onto the stack with the variables automatically pushed by the CPU. Then the data segment is changed to the value of 0x10 (Kernel Data Segment Descriptor) so the privilege levels are changed. Then after the handler returns those values are poped. But whenever the value in ds is changed a GPE is thrown with the error code 0x2544 and after a few seconds the VM restarts. (linker and compiler i386-elf-gcc , assembler nasm)

I tried placing hlt instructions in between instructions to locate which instruction was throwing the GPE. After that I was able to find out that the the `mov ds,ax' instruction. I tried various things like removing the stack which was initialized by the bootstrap code to deleting the privilege changing parts of the code. The only way I can return from the common stub is to remove the parts of my code which change the privilege levels but as I want to move towards user mode I still want them to stay.

Here is my common stub:

isr_common_stub:
    pusha                    ; Pushes edi,esi,ebp,esp,ebx,edx,ecx,eax
    xor eax,eax
    mov ax, ds               ; Lower 16-bits of eax = ds.
    push eax                 ; 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 isr_handler

    xor eax,eax
    pop eax
    mov ds, ax ; This is the instruction everything fails;
    mov es, ax
    mov fs, ax
    mov gs, ax
    popa
    iret

My ISR handler macros:

extern isr_handler

%macro ISR_NOERRCODE 1
  global isr%1        ; %1 accesses the first parameter.
  isr%1:
    cli
    push byte 0
    push %1
    jmp isr_common_stub
%endmacro

%macro ISR_ERRCODE 1
  global isr%1
  isr%1:
    cli
    push byte %1
    jmp isr_common_stub
%endmacro
ISR_NOERRCODE 0
ISR_NOERRCODE 1
ISR_NOERRCODE 2
ISR_NOERRCODE 3
...

My C handler which results in "Received interrupt: 0xD err. code 0x2544"

#include <stdio.h>
#include <isr.h>
#include <tty.h>

void isr_handler(registers_t regs) {
    printf("ds: %x 
" ,regs.ds);
    printf("Received interrupt: %x with err. code: %x 
", regs.int_no, regs.err_code);
}

And my main function:

void kmain(struct multiboot *mboot_ptr) {
    descinit(); // Sets up IDT and GDT
    ttyinit(TTY0); // Sets up the VGA Framebuffer
    asm volatile ("int $0x1"); // Triggers a software interrupt
    printf("Wow"); // After that its supposed to print this
}

As you can see the code was supposed to output,

ds: 0x10
Received interrupt: 0x1 with err. code: 0

but results in,

...
ds: 0x10
Received interrupt: 0xD with err. code: 0x2544

ds: 0x10
Received interrupt: 0xD with err. code: 0x2544
...

Which goes on until the VM restarts itself.

What am I doing wrong?

Answer 1

The code isn't complete but I'm going to guess what you are seeing is a result of a well known bug in James Molloy's OSDev tutorial. The OSDev community has compiled a list of known bugs in an errata list. I recommend reviewing and fixing all the bugs mentioned there. Specifically in this case I believe the bug that is causing problems is this one:

Problem: Interrupt handlers corrupt interrupted state

This article previously told you to know the ABI. If you do you will see a huge problem in the interrupt.s suggested by the tutorial: It breaks the ABI for structure passing! It creates an instance of the struct registers on the stack and then passes it by value to the isr_handler function and then assumes the structure is intact afterwards. However, the function parameters on the stack belongs to the function and it is allowed to trash these values as it sees fit (if you need to know whether the compiler actually does this, you are thinking the wrong way, but it actually does). There are two ways around this. The most practical method is to pass the structure as a pointer instead, which allows you to explicitly edit the register state when needed - very useful for system calls, without having the compiler randomly doing it for you. The compiler can still edit the pointer on the stack when it's not specifically needed. The second option is to make another copy the structure and pass that

The problem is that the 32-bit System V ABI doesn't guarantee that data passed by value will be unmodified on the stack! The compiler is free to reuse that memory for whatever purposes it chooses. The compiler probably generated code that trashed the area on the stack where DS is stored. When DS was set with the bogus value it crashed. What you should be doing is passing by reference rather than value. I'd recommend these code changes in the assembly code:

irq_common_stub:
    pusha
    mov ax, ds
    push eax
    mov ax, 0x10 ;0x10
    mov ds, ax
    mov es, ax
    mov fs, ax
    mov gs, ax
    push esp                 ; At this point ESP is a pointer to where GS (and the rest
                             ; of the interrupt handler state resides)
                             ; Push ESP as 1st parameter as it's a 
                             ; pointer to a registers_t  
    call irq_handler
    pop ebx                  ; Remove the saved ESP on the stack. Efficient to just pop it 
                             ; into any register. You could have done: add esp, 4 as well
    pop ebx
    mov ds, bx
    mov es, bx
    mov fs, bx
    mov gs, bx
    popa
    add esp, 8
    sti
    iret

And then modify irq_handler to use registers_t *regs instead of registers_t regs :

void irq_handler(registers_t *regs) {
    if (regs->int_no >= 40) port_byte_out(0xA0, 0x20);
    port_byte_out(0x20, 0x20);

    if (interrupt_handlers[regs->int_no] != 0) {
        interrupt_handlers[regs->int_no](*regs);
    }
    else
    {
        klog("ISR: Unhandled IRQ%u!
", regs->int_no);
    }
}

I'd actually recommend each interrupt handler take a pointer to registers_t to avoid unnecessary copying. If your interrupt handlers and the interrupt_handlers array used function that took registers_t * as the parameter (instead of registers_t) then you'd modify the code:

interrupt_handlers[r->int_no](*regs); 

to be:

interrupt_handlers[r->int_no](regs);

Important: You have to make these same type of changes for your ISR handlers as well. Both the IRQ and ISR handlers and associated code have this same problem.