Strange multitasking bugs

[Agola]

Hello, after implementing higher half paging, I implemented multitasking. But my implementation has some strange bugs...

If I don't add some delay to tasks, it page faults after a while, with a strange faulting address like 0, 10, 12, 34, but then I fixed that with adding schedule_noirq(); before printf()s. Is that correct?

And bugs, firstly, if I dump tasks with:

Code: Select all

printf("kernel_idle: %x, %s\ntest1: %x, %s\ntest2: %x, %s\n", task_getpid("kernel_idle"), task_getname(0), task_getpid("test1"), task_getname(1), task_getpid("test2"), task_getname(2));

When I start tasking, it immediately triggers isr1 (debug).
If i some delay after printf, it works, but os freezes after a short while.

I think something in my implementation messes stack, but I don't know what's causing that

That is irq0 handler:

Code: Select all

.macro IRQ ident byte
    .global _irq\ident
    .type _irq\ident, @function
    _irq\ident:
        cli
        push $0x00
        push $\byte
        jmp irq_common
.endm

/* Interrupt Requests */
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
.type irq_handler, @function

irq_common:
    pusha

    /* Save segment registers */
    push %ds
    push %es
    push %fs
    push %gs

    /* Call interrupt handler */
    push %esp
    call irq_handler
    add $4, %esp

    /* Restore segment registers */
    pop %gs
    pop %fs
    pop %es
    pop %ds

    /* Restore all registers */
    popa
    /* Cleanup error code and IRQ # */
    add $8, %esp
    /* pop CS, EIP, EFLAGS, SS and ESP */
    iret

That is my multitasking code:

Code: Select all

volatile bool tasking = false;

typedef struct task
{
    registers_t regs;
    uintptr_t page_directory;
    char* name;
    uint32_t pid;

    struct task* next;
} task_t;

task_t* task_idle;
task_t* current_task;

uint32_t last_pid = 1;

task_t* create_task(char* name, void (*func)(), uint32_t flags, uintptr_t page_directory)
{
    task_t *task = (task_t*) calloc(1, sizeof(task_t));
    task->name = name;
    task->regs.cs = 0x08;
    task->regs.gs = 0x10;
    task->regs.fs = 0x10;
    task->regs.es = 0x10;
    task->regs.ds = 0x10;
    task->regs.ss = 0x10;
    task->regs.eflags = flags;
    task->regs.eip = (uint32_t) func;
    task->regs.useresp = (uint32_t) (malloc(0x1000) + 0x1000);
    task->page_directory = page_directory;
    task->next = NULL;

    return task;
}

uint32_t add_task(task_t* task)
{
    task->pid = last_pid++;

    task_t* last_task = task_idle;
    while (last_task->next != NULL) last_task = last_task->next;

    last_task->next = task;

    return task->pid;
}

void remove_task(uint32_t pid)
{
    if (pid == 0) return;

    task_t* task = task_idle;
    for (uint32_t i = 0; i < pid - 1; i++) task = task->next;

    task_t* temp = task->next;
    task->next = temp->next;

    free(temp);
}

uint32_t task_getpid(char* name)
{
    task_t* task = task_idle;

    while (task)
    {
        if (strcmp(task->name, name) == 0) break;
        task = task->next;
    }

    if (task == NULL) return 0xFFFFFFFF;
    return task->pid;
}

char* task_getname(uint32_t pid)
{
    task_t* task = task_idle;

    while (task)
    {
        if (task->pid == pid) break;
        task = task->next;
    }

    if (task == NULL) return "(error)";
    return task->name;
}

void schedule(registers_t* regs)
{
    if (tasking)
    {
        memcpy(&current_task->regs, regs, sizeof(registers_t));

        if (current_task->next != NULL) current_task = current_task->next;
        else current_task = task_idle;

        memcpy(regs, &current_task->regs, sizeof(registers_t));
    }
}

void timer(registers_t* regs)
{
    schedule(regs);
}

void schedule_noirq()
{
    asm volatile("int $0x20");
    return;
}

void idle()
{
   for(;;)
    {
        schedule_noirq();
        printf("a");
    }
}

void test1()
{
   for(;;)
    {
        schedule_noirq();
        printf("b");
    }
}

void test2()
{
    for(;;)
    {
        schedule_noirq();
        printf("c");
    }
}

void init_tasking()
{
    uint32_t eflags;
    asm volatile("pushfl; movl (%%esp), %%eax; movl %%eax, %0; popfl;":"=m"(eflags)::"%eax");

    task_idle = create_task("kernel_idle", &idle, eflags, read_cr3());
    current_task = task_idle;

    add_task(create_task("test1", &test1, eflags, read_cr3()));
    add_task(create_task("test2", &test2, eflags, read_cr3()));

    idle();
}

Struct registers_t is:

Code: Select all

typedef struct regs
{
    unsigned int gs, fs, es, ds;
    unsigned int edi, esi, ebp, esp, ebx, edx, ecx, eax;
    unsigned int int_no, err_code;
    unsigned int eip, cs, eflags, useresp, ss;    
} registers_t;

Please help, I have tried fix that for 2 days, I really got tired.
I also tried changing "task->regs.useresp = (uint32_t) (malloc(0x1000) + 0x1000);" with "task->regs.esp = (uint32_t) (malloc(0x1000) + 0x1000);" but didn't change anything.

Thanks

Notes:

void timer(); is IRQ0 handler, calls schedule in 100Hz.
void schedule_noirq() calls IRQ0 without waiting PIT.

Can writing a separate irq0 handler to keep the stack safe instead of irq_common work?

[bauen1]

Whilst i can't see your irq_handler method, when working on interrupts my code page faulted too because gcc likes to modify the arguments passed to it to save some stack space, try adding

Code: Select all

__attribute__((optimize("-O0")))

infront of the function declaration likes this:

Code: Select all

// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=27234
__attribute__((optimize("-O0"))) void irq_handler(volatile struct registers registers) {
  // your code here
}

I also had to do the same thing to my isr handler (in C)

This disables optimization and iirc there is a better attribute especially for this case but i have to search for it again.

[Agola]

Whilst i can't see your irq_handler method, when working on interrupts my code page faulted too because gcc likes to modify the arguments passed to it to save some stack space, try adding

Code: Select all

__attribute__((optimize("-O0")))

infront of the function declaration likes this:

Code: Select all

// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=27234
__attribute__((optimize("-O0"))) void irq_handler(volatile struct registers registers) {
  // your code here
}

I also had to do the same thing to my isr handler (in C)

This disables optimization and iirc there is a better attribute especially for this case but i have to search for it again.

IRQ handler is called from assembly code that i posted first:

Code: Select all

void irq_handler(registers_t *r)
{
    void (*handler)(registers_t *r);

    handler = irq_routines[r->int_no - 32];
    if (handler)
    {
        handler(r);
    }

    if (r->int_no >= 40)
    {
        outb(0xA0, 0x20);
    }
    outb(0x20, 0x20);
}

After adding __attribute__((optimize("-O0"))) and volatile it also causes isr1 without using printf before idle(); too. Really strange.

[bauen1]

So i don't see you adding the scheduler method as IRQ0 handler in your code, maybe thats it ?
And another thing, your causing IRQ0 using int 20, but still send a IRQ acknowledge to the PCI.

[Octocontrabass]

Whilst i can't see your irq_handler method, when working on interrupts my code page faulted too because gcc likes to modify the arguments passed to it to save some stack space

It sounds like you've copied one of the nastier bugs in James Molloy's tutorial.

Code: Select all

__attribute__((optimize("-O0"))) void irq_handler(volatile struct registers registers) {

Use this instead:

Code: Select all

void irq_handler(struct registers * registers) {

You'll have to modify your assembly code to pass a pointer to the struct instead of the struct itself.

[Agola]

Whilst i can't see your irq_handler method, when working on interrupts my code page faulted too because gcc likes to modify the arguments passed to it to save some stack space

It sounds like you've copied one of the nastier bugs in James Molloy's tutorial.

Code: Select all

__attribute__((optimize("-O0"))) void irq_handler(volatile struct registers registers) {

Use this instead:

Code: Select all

void irq_handler(struct registers * registers) {

You'll have to modify your assembly code to pass a pointer to the struct instead of the struct itself.

I was copied irq handler from Bran's tutorial, and irq_handler is defined as

Code: Select all

void irq_handler(registers_t *r),

the

Code: Select all

__attribute__((optimize("-O0"))) void irq_handler(volatile struct registers registers) {

is from bauen's example.

[Octocontrabass]

Both of the quote boxes in my post are correctly attributed.

[bauen1]

I'm also changing to Octoncontrabass's method.

[Agola]

Still don't work, even changing

Code: Select all

registers_t* r

with

Code: Select all

struct regs* r

And a note, I use newlib with int 0x74 syscall handler, can interrupting the interrupt cause that problem?

[bauen1]

Hm, one thing you could try is using

Code: Select all

pushd
popd

instead, because

Code: Select all

push

only pushes ax, bx and not eax and ebb

EDIT: you also need to change

Code: Select all

unsigned int

to

Code: Select all

unsigned long

because int is only 16 bit wide but push etc operate on 4 byte = 32 bits

[Agola]

Hm, one thing you could try is using

Code: Select all

pushd
popd

instead, because

Code: Select all

push

only pushes ax, bx and not eax and ebb

EDIT: you also need to change

Code: Select all

unsigned int

to

Code: Select all

unsigned long

because int is only 16 bit wide but push etc operate on 4 byte = 32 bits

Agola is an 32 bit operating system and I've already built an i686-agola target cross compiler. So sizeof(int) is 4 bytes, and pusha pushes all 32-bit GPRs

[bauen1]

Well, i'm out of ideas then

[Boris]

Hi,
you should write your switch_task(task*from,task*to) method that does not use interrupts.
You just have to save and restore registers non preserved by your ABI ( segments,eip, eax, ecx, edx, eflags, esp, I guess..)
the rest will be saved/restored by your compiler before/after the switche_task method.

What I usually do:
- use non preserved registers ( ebx for example ) for swapping registers.
- back up the return EIP ( which is saved by the compiler on the stack before it calls switch_task ) to from_task
- back up current preserved registers to from_task
- change paging pointer
- push stuff on the current stack which iretd expects
- call iretd

[Agola]

Hi,
you should write your switch_task(task*from,task*to) method that does not use interrupts.
You just have to save and restore registers non preserved by your ABI ( segments,eip, eax, ecx, edx, eflags, esp, I guess..)
the rest will be saved/restored by your compiler before/after the switche_task method.

What I usually do:
- use non preserved registers ( ebx for example ) for swapping registers.
- back up the return EIP ( which is saved by the compiler on the stack before it calls switch_task ) to from_task
- back up current preserved registers to from_task
- change paging pointer
- push stuff on the current stack which iretd expects
- call iretd

Yay, that fixed it. Finally multitasking works great!
I started adding priorities to tasks.