C Pointer not getting set

Question

I'm creating a sort of toy OS and I'm trying to set up paging. The boot loader maps the kernel into the higher half first, and then I'm trying to set up a new page directory.

I have some structures defined like this:

typedef struct pd_entry pd_entry_t;
struct pd_entry
{
    uint32_t present    : 1;
    uint32_t writable   : 1;
    uint32_t user       : 1;
    uint32_t writethru  : 1;
    uint32_t nocache    : 1;
    uint32_t accessed   : 1;
    uint32_t _reserved  : 1;
    uint32_t size_4mb   : 1;
    uint32_t global     : 1;
    uint32_t available  : 3;
    uint32_t frame      : 20;
}__attribute__((packed));

typedef struct pt_entry pt_entry_t;
struct pt_entry
{
    uint32_t present    : 1;
    uint32_t writable   : 1;
    uint32_t user       : 1;
    uint32_t writethru  : 1;
    uint32_t nocache    : 1;
    uint32_t accessed   : 1;
    uint32_t dirty      : 1;
    uint32_t attr_index : 1;
    uint32_t global     : 1;
    uint32_t available  : 3;
    uint32_t frame      : 20;
} __attribute__((packed));

typedef struct page_dir page_dir_t;
struct page_dir
{
    pd_entry_t entries[TABLES_PER_DIR];
};

typedef struct page_table page_table_t;
struct page_table
{
    pt_entry_t entries[PAGES_PER_TABLE];
};

And I use this code to set entries:

// I store the page directory pointer in the last directory entry
uint32_t map_quick(uint32_t addr)
{
    GET_PTE(QUICKMAP_ADDR)->frame = PAGE_ALIGN(addr) >> 12; // QUICKMAP_ADDR is 0xC0000000
    paging_flush_tlb_entry(QUICKMAP_ADDR); // asm volatile ("invlpg (%0)" :: "r"(addr));
    return QUICKMAP_ADDR;
}

pd_entry_t* get_dir_entry(page_dir_t *dir, uint32_t virt)
{
    dir = (page_dir_t *)map_quick((uint32_t)dir);
    return &dir->entries[PAGE_DIR_INDEX(virt)];
}

void set_dir_entry(page_dir_t *dir, uint32_t virt, page_table_t *pt)
{
    pd_entry_t *pde = get_dir_entry(dir, virt);

    pde->present   = 1;
    pde->writable  = 1;
    pde->user      = 0;
    pde->writethru = 0;
    pde->nocache   = 0;
    pde->accessed  = 0;
    pde->size_4mb  = 0;
    pde->global    = 0;
    pde->frame     = (uint32_t)pt >> 12;
    kprintf("0x%x:0x%x ", (uint32_t)pt >> 12, pde->frame); // Just a 'printf' clone
}

However, when set_dir_entry is called, kprintf prints 0xA0:0x0. I can't figure out why pde->frame isn't getting set? I'm not getting any page-faults or exceptions, the directory entry just always equals 0 no matter what I set it to.

EDIT: Here's the code for kprintf:

size_t kprintf(const char *str, ...)
{
    if (!str) return 0;

    va_list args;
    va_start(args, str);

    unsigned int i;
    for (i = 0; i < strlen(str); i++)
    {
        if (str[i] == '%')
        {
            switch (str[i+1])
            {
                case 'c':
                {
                    char c = va_arg(args, char);
                    kputc(c);
                    i++;
                    break;
                }
                case 's':
                {
                    char *s = va_arg(args, char*);
                    kputs(s);
                    i++;
                    break;
                }
                case 'd':
                case 'i':
                {
                    int c = va_arg(args, int);
                    char s[32] = {0};
                    itoa_s(c, s, 10);
                    kputs(s);
                    i++;
                    break;
                }
                case 'X':
                case 'x':
                {
                    int c = va_arg(args, int);
                    char s[32] = {0};
                    itoa_s(c, s, 16);
                    kputs(s);
                    i++;
                    break;
                }
            }
        }
        else
        {
            kputc(str[i]);
        }
    }

    va_end(args);
    return (size_t)i;
}

void kputc(unsigned char c)
{
    uint16_t attr = color << 8;

    if (c == 0x8 && posx)
        posx--;
    else if (c == 0x9)
        posx = (posx + 8) & ~(8-1);
    else if (c == '\r')
        posx = 0;
    else if (c == '\n')
    {
        posx = 0;
        posy++;
    }
    else if (c >= ' ')
    {
        uint16_t *loc = videomem + (posy*80 + posx);
        *loc = c | attr;
        posx++;
    }

    if (posx >= 80)
    {
        posx = 0;
        posy++;
    }
    if (posy >= 25)
        kscroll();
}

void kputs(const char *str)
{
    if (!str) return;

    unsigned int i;
    for (i = 0; i < strlen(str); i++)
        kputc(str[i]);
}

EDIT 2: I don't know if this is useful, and I don't really like just dumping a huge amount of code for people to read through, but this is the entire paging code. All of the function calls that aren't part of this code work well independently.

virtual.h

#ifndef _VIRTUAL_H_
#define _VIRTUAL_H_

#include <x86/paging.h>
#include <stdbool.h>

#define VIRTUAL_TO_PHYSICAL(x) ((uint32_t)(x) - 0xC0001000 + 0x00101000)
#define PHYSICAL_TO_VIRTUAL(x) ((uint32_t)(x) + 0xC0001000 - 0x00101000)

#define KERNEL_CODE_VADDRESS 0xC0001000
#define KERNEL_HEAP_VADDRESS 0xD0000000

void virtual_init();

void virtual_create_dir(page_dir_t **dir);
void virtual_destroy_dir(page_dir_t *dir);
void virtual_set_dir(page_dir_t *dir);

void virtual_map_page(page_dir_t *dir, uint32_t phys, uint32_t virt, bool present);
void virtual_map_kernel(page_dir_t *dir);

#endif

virtual.c

#include <system/memory/physical.h>
#include <x86/idt.h>
#include <utils/kernio.h>
#include <utils/kernpanic.h>
#include <string.h>
#include "virtual.h"

#define QUICKMAP_ADDR 0xC0000000

#define GET_PDE(x) ((pd_entry_t *)(0xFFFFF000 + ((x) >> 22) * 4))
#define GET_PTE(x) ((pt_entry_t *)(0xFFC00000 + ((x) >> 12) * 4))

extern uint32_t __kernel_start, __kernel_end;

page_dir_t *kern_dir;

pd_entry_t* get_dir_entry(page_dir_t *dir, uint32_t virt);
void set_dir_entry(page_dir_t *dir, uint32_t virt, page_table_t *pt);
pt_entry_t* get_table_entry(page_dir_t *dir, uint32_t virt);
void set_table_entry(page_dir_t *dir, uint32_t phys, uint32_t virt, bool present);

uint32_t map_quick(uint32_t addr);

void interrupt page_fault(registers_t *regs);

void virtual_init()
{
    int_enable(14, page_fault);

    virtual_create_dir(&kern_dir);

    uint32_t phys, virt;
    for (phys = 0; phys < 0x400000; phys += PAGE_SIZE)
        virtual_map_page(kern_dir, phys, phys, true);

    phys = VIRTUAL_TO_PHYSICAL(&__kernel_start);
    virt = (uint32_t)&__kernel_start;

    uint32_t end = VIRTUAL_TO_PHYSICAL(&__kernel_end) + physical_get_bitmap_size();
    for (; phys < end; phys += PAGE_SIZE, virt += PAGE_SIZE)
        virtual_map_page(kern_dir, phys, virt, true);

    virtual_map_page(kern_dir, 0x0, QUICKMAP_ADDR, true);

    virtual_set_dir(kern_dir);
}

void virtual_create_dir(page_dir_t **dir)
{
    *dir = (page_dir_t *)physical_alloc_block();
    page_dir_t *virt_dir = (page_dir_t *)map_quick((uint32_t)*dir);
    memset(virt_dir, 0, sizeof(page_dir_t));

    pd_entry_t *last_pde = &virt_dir->entries[TABLES_PER_DIR - 1];
    last_pde->present = 1;
    last_pde->writable = 1;
    last_pde->frame = (uint32_t)*dir >> 12;
}

void virtual_destroy_dir(page_dir_t *dir)
{
    page_dir_t *virt_dir = (page_dir_t *)map_quick((uint32_t)dir);

    unsigned int i, j;
    for (i = 1; i < KERNEL_CODE_VADDRESS / (PAGE_SIZE * TABLES_PER_DIR); i++)
    {
        pd_entry_t *pde = &virt_dir->entries[i];
        page_table_t *table = (page_table_t *)(pde->frame << 12);

        if (table != NULL && pde->present)
        {
            page_table_t *virt_table = (page_table_t *)map_quick((uint32_t)table);
            for (j = 0; j < PAGES_PER_TABLE; j++)
            {
                pt_entry_t *pte = &virt_table->entries[j];
                uint32_t phys = pte->frame << 12;
                if (phys != NULL && pte->present)
                    physical_free_block((void *)phys);
            }

            physical_free_block(table);
        }
    }

    physical_free_block(dir);
}

void virtual_set_dir(page_dir_t *dir)
{
    paging_load_pdbr((uint32_t)dir);
    paging_enable(true);
}

void virtual_map_page(page_dir_t *dir, uint32_t phys, uint32_t virt, bool present)
{
    set_table_entry(dir, phys, virt, present);
}

void virtual_map_kernel(page_dir_t *dir)
{
    pd_entry_t *pde;

    pde = get_dir_entry(kern_dir, 0x0);
    set_dir_entry(dir, 0x0, (page_table_t *)(pde->frame << 12));

    pde = get_dir_entry(kern_dir, KERNEL_CODE_VADDRESS);
    set_dir_entry(dir, KERNEL_CODE_VADDRESS, (page_table_t *)(pde->frame << 12));
}

pd_entry_t* get_dir_entry(page_dir_t *dir, uint32_t virt)
{
    dir = (page_dir_t *)map_quick((uint32_t)dir);
    pd_entry_t *e = &dir->entries[PAGE_DIR_INDEX(virt)];
    return &dir->entries[PAGE_DIR_INDEX(virt)];
}

void set_dir_entry(page_dir_t *dir, uint32_t virt, page_table_t *pt)
{
    pd_entry_t *pde = get_dir_entry(dir, virt);

    pde->present = 1;
    pde->writable = 1;
    pde->user = 0;
    pde->writethru = 0;
    pde->nocache = 0;
    pde->accessed = 0;
    pde->size_4mb = 0;
    pde->global = 0;
    pde->frame = (uint32_t)pt >> 12;
}

pt_entry_t* get_table_entry(page_dir_t *dir, uint32_t virt)
{
    pd_entry_t *pde = get_dir_entry(dir, virt);
    page_table_t *table = (page_table_t *)(pde->frame << 12);

    if (table == NULL)
    {
        table = (page_table_t *)physical_alloc_block();
        set_dir_entry(dir, virt, table);

        unsigned int i;
        for (i = 0; i < PAGES_PER_TABLE; i++)
            set_table_entry(dir, 0x0, virt + (i * PAGE_SIZE), false);
    }

    table = (page_table_t *)map_quick((uint32_t)table);
    return (pt_entry_t *)&table->entries[PAGE_TABLE_INDEX(virt)];
}

void set_table_entry(page_dir_t *dir, uint32_t phys, uint32_t virt, bool present)
{
    pt_entry_t *pte = get_table_entry(dir, PAGE_ALIGN(virt));

    pte->present = present;
    pte->writable = 1;
    pte->user = 0;
    pte->writethru = 0;
    pte->nocache = 0;
    pte->dirty = 0;
    pte->attr_index = 0;
    pte->global = 0;
    pte->frame = PAGE_ALIGN(phys) >> 12;
}

uint32_t map_quick(uint32_t addr)
{
    GET_PTE(QUICKMAP_ADDR)->frame = PAGE_ALIGN(addr) >> 12;
    paging_flush_tlb_entry(QUICKMAP_ADDR);
    return QUICKMAP_ADDR;
}

void interrupt page_fault(registers_t *regs)
{
    uint32_t virt;
    asm volatile ("movl %%cr2, %0" : "=r" (virt));
    kprintf("\nError accessing address 0x%x", virt);
    kpanic("Page Fault");
}

Casimiracasimire

Could you show how kprintf is coded? – Unwish

@Unwish that could certainly be the culprit, but i don't know if you saw the comment on that line that it is "just a printf clone" – Humane

A clone of what implementation of printf? Does it include macros or is it a proper variadic function? – Unwish

granted it is not an exact clone as in printf, %x would generate 0xa0 , not 0xA0 – Humane

Sorry, I posted the code, I suppose it isn't exactly the same as printf. – Casimiracasimire

Also, I use the kprintf function a lot elsewhere, and I haven't seen any problems with it. If I compare the value of (page_table_t *)(pde->frame << 12), it is always NULL as well. – Casimiracasimire

Is this a 32-bit platform, or 64-bit? Might kprintf be pulling the wrong-size int from the stack compared to what was pushed? In particular, since pde->frame is not cast to uint32_t like the previous argument is, it may get different treatment with respect to integral promotions for a function with an ellipsis in the prototype (I may be off on that, though - it's been a while since I studied what the standard has to say about that...). – Hydrocephalus

It is targeted for 32-bit x86. – Jocose

Could it be that the memory referenced by pde (in set_dir_entry()) had not been properly allocated? – Calli

did you print the data of pde->frame in binary? And try manually setting a value for pde->frame = 0xA0 and print it back out. – Plash

@alk: The memory should be getting allocated, and the comparison (pde == NULL) returns false... – Casimiracasimire

@Aniket: I haven't tried printing it in binary - I'll try that now. I have tried directly setting pde->frame to a value, but it still reads 0. Also, if I try to print, say, pde->present, it also is 0, even though it is explicitly set to 1. – Casimiracasimire

my only other advice to you @Casimiracasimire would be to check if data is loaded beyond the first sector of your drive. I have had the same problems when I wrote my own OS. Its probably written beyond the first 512 bytes. See for the mapping mate. – Plash

Answer 1

See this question re __attribute__(packed)
Quote:
... __attribute__((packed)) is potentially unsafe on some systems ... There have also been systems where a misaligned access quietly ignores the low-order bits of the address, causing it to access the wrong chunk of memory.

Try using pragma (pack) (and here) instead:

#pragma pack(1) // exact fit - no padding; placed at top of pragma pack stack
    struct pd_entry
    {
        ...
    };
    struct pt_entry
    {
        ...
    };
#pragma pack(pop) //restore previous pragma pack stack

[edit]
Based on your comment for using a uint32_t, if you want to retain the ability to access the individual bits you can make the structs part of a union with the uint31_t:

#pragma pack(1) // exact fit - no padding; placed at top of pragma pack stack
union
{
    uint32_t ui32;
    struct pd_entry
    {
        ...
    };
 }
    ...
#pragma pack(pop) //restore previous pragma pack stack

and use or and and with the uint32_tto set, and check the values using the struct's members.

South

Hi - thanks for the suggestion! I just tried it though, and I'm getting the same result... Since I just want the two structs to pack down to 32-bits, I might try replacing them with normal uint32_ts and see what happens... – Casimiracasimire

@bre: updated my answer to suggest a way to use both a uint32_t as well as your struct without extra memory overhead. – South

Answer 2

pt is a pointer, which means it holds the address of the page_table_t variable that it is pointing to. Why are you rightshifting the address by 12?? Is this intentional?

Noyade

It is intentional - the rightshift by 12 is short for ((uint32_t)pt % 0x400000) / 0x1000, which is determining the page number for the directory member, for x86 paging. – Casimiracasimire