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");
}

@Mat 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" — im so confused, Oct 26 '12 at 16:26
A clone of what implementation of printf? Does it include macros or is it a proper variadic function? — Mat, Oct 26 '12 at 16:27
granted it is not an exact clone as in `printf`, `%x` would generate 0xa0 , not 0xA0 — im so confused, Oct 26 '12 at 16:28
Sorry, I posted the code, I suppose it isn't exactly the same as `printf`. — bre, Oct 26 '12 at 16:29
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. — bre, Oct 26 '12 at 16:32
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...). — twalberg, Oct 26 '12 at 18:52
Could it be that the memory referenced by `pde` (in `set_dir_entry()`) had not been properly allocated? — alk, Oct 27 '12 at 07:26
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. — Aniket Inge, Oct 27 '12 at 10:01
@alk: The memory should be getting allocated, and the comparison `(pde == NULL)` returns false... — bre, Oct 27 '12 at 14:18
@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`. — bre, Oct 27 '12 at 14:20
my only other advice to you @bre 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. — Aniket Inge, Oct 27 '12 at 15:14

score 1 · Answer 1 · edited May 23 '17 at 12:03

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.

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_t`s and see what happens... — bre, Oct 27 '12 at 14:27
@bre: updated my answer to suggest a way to use both a `uint32_t` as well as your struct without extra memory overhead. — slashmais, Oct 27 '12 at 17:25

score 0 · Answer 2 · answered Oct 27 '12 at 09:49

0

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?

answered Oct 27 '12 at 09:49

piscianemperor

99
2
9

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. – bre Oct 27 '12 at 14:26

C Pointer not getting set

2 Answers2