Triple fault on enabling paging

[viruss33]

Hi, I'm writing a code for paging and now it triple faults as soon as I enable paging. Qemu shows nothing when I type info mem or info tlb, so my paging structures are empty so that's the problem, but I checked it on debug that the address that I pass to cpu contains address of my page_directory structure. The page_table entries are also filled with correct values.

Code: Select all

#include <kernel/page_table_entry.h>
#include <stdint.h>
#include <stdbool.h>
#include <kernel/physical_memory_manager.h>
#include <string.h>
#include <kernel/math.h>

page_directory* current_directory=0;

extern void loadPageDirectory(unsigned int*);
extern void enablePaging();

void set_bit(uint32_t* pte, uint8_t bit_number){
    uint32_t pte_value = *pte;
    pte_value |= (1 << bit_number);
    *pte = pte_value;

}

void unset_bit (uint32_t* pte, int bit_number){
    uint32_t pte_value = *pte;
    pte_value &= ~(1 << bit_number);
    *pte = pte_value;
}

bool is_bit_set (uint32_t* pte, int bit_number) {
    uint32_t pte_value = *pte;
    return pte_value &  (1 << bit_number);
}

void set_frame (uint32_t* pte, physical_address physical_address){
    uint32_t pte_value = *pte;
    pte_value |= (physical_address << 12);
    *pte = pte_value;
}

uint32_t get_frame (uint32_t pte){
    return pte >> 12;
}

bool allocate_page (pt_entry* e) {
 
	void* block = allocate_block ();
	if (!block)
		return false;
 
	set_frame (e, (physical_address)block);
	set_bit (e, IS_PRESENT);
 
	return true;
}

void vmmngr_free_page (pt_entry* e) {
 
	void* p = (void*)get_frame (*e);
	if (p)
		free_block (p);
 
	unset_bit (e, IS_PRESENT);
}

inline pt_entry* get_page_table_entry_from_address (page_table* page_table,virtual_address address) {
 
	if (page_table)
		return &page_table->entries[ PAGE_TABLE_INDEX (address) ];
	return 0;
}

inline pd_entry* get_page_directory_entry_from_address (page_directory* p, virtual_address addr) {
 
	if (p)
		return &p->entries[ PAGE_DIRECTORY_INDEX (addr) ];
	return 0;
}



bool switch_pdirectory (page_directory* directory) {
 
	if (!directory)
		return false;
 
	current_directory = directory;
	loadPageDirectory (&current_directory[0].entries);
	return true;
}


 
page_directory* get_directory () {
 
	return current_directory;
}

void flush_tlb_entry (virtual_address addr) {
 
#ifdef _MSC_VER
	_asm {
		cli
		invlpg	addr
		sti
	}
#endif
}

void map_page (void* physical_addr, void* virtual_address) {

   page_directory* page_directory = get_directory ();

   pd_entry* e = &page_directory->entries [PAGE_DIRECTORY_INDEX ((uint32_t) virtual_address) ];
   if ( !is_bit_set(e, IS_PRESENT)) {
        page_table* table = (page_table*) allocate_block();
        if (!table){
            return;
        }

        memset (table, 0, sizeof(page_table));
        set_bit(e, IS_PRESENT);
        set_bit(e, IS_WRITABLE);
        set_frame(e, (physical_address) table);

   }
    page_table* table = (page_table*) get_frame ( *e );
    pt_entry* page = &table->entries [ PAGE_TABLE_INDEX ( (uint32_t) virtual_address) ];
    set_frame(page, (physical_address)physical_addr);
    set_bit(page, IS_PRESENT);


}




void set_up_paging () {
 
	page_table* default_page_table = (page_table*) allocate_block ();
	if (!default_page_table)
		return;
 
	page_table* kernel_page_table = (page_table*) allocate_block ();
	if (!kernel_page_table)
		return;

	memset(default_page_table, 0, sizeof(page_table));
	memset(kernel_page_table, 0, sizeof(page_table));

    for (int i=0, frame=0x0, virtual=0x00000000; i<1024; i++, frame+=4096, virtual+=4096) {

		pt_entry page=0;
		set_bit (&page, IS_PRESENT);
 		set_frame (&page, frame);

		default_page_table->entries [PAGE_TABLE_INDEX (virtual) ] = page;
	}

    for (int i=0, frame=0x100000, virtual=0xc0000000; i<1024; i++, frame+=4096, virtual+=4096) {

		pt_entry page=0;
		set_bit (&page, IS_PRESENT);
		set_frame (&page, frame);

		kernel_page_table->entries [PAGE_TABLE_INDEX (virtual) ] = page;
	}
    page_directory* page_dir = (page_directory*) allocate_blocks(divide_round_up(sizeof(page_directory), BLOCKS_SIZE));
    if (!page_dir){
        return;
    }
    memset(page_dir, 0, sizeof(page_dir));

    pd_entry* entry = &page_dir->entries [PAGE_DIRECTORY_INDEX (0xc0000000) ];
	set_bit (entry, IS_PRESENT);
	set_bit (entry, IS_WRITABLE);
	set_frame (entry, (physical_address)kernel_page_table);

	pd_entry* entry2 = &page_dir->entries [PAGE_DIRECTORY_INDEX (0x00000000) ];
	set_bit (entry2, IS_PRESENT);
	set_bit (entry2, IS_WRITABLE);
	set_frame (entry2, (physical_address)default_page_table);

	switch_pdirectory (page_dir);
 
	enablePaging ();


}

Code: Select all

#ifndef PAGE_TABLE_ENTRY
#define PAGE_TABLE_ENTRY

#include <stdint.h>
#include <stdbool.h>

#define PAGES_PER_TABLE 1024
#define PAGE_TABLES_PER_DIRECTORY	1024

#define PAGE_DIRECTORY_INDEX(x) (((x) >> 22) & 0x3ff)
#define PAGE_TABLE_INDEX(x) (((x) >> 12) & 0x3ff)

#define PAGE_TABLE_ADDRESS_SPACE_SIZE 0x400000

#define DIRECTORY_TABLE_ADDRESS_SPACE_SIZE 0x100000000

#define PAGE_SIZE 4096

typedef uint32_t pt_entry;
typedef uint32_t pd_entry;

typedef uint32_t virtual_address;
typedef uint32_t physical_address;

typedef struct page_table {
 
	pt_entry entries[PAGES_PER_TABLE];
} page_table;
 
//! page directory
typedef struct pdirectory {
 
	pd_entry entries[PAGE_TABLES_PER_DIRECTORY];
} page_directory;


enum PTE_BIT_NUMBERS {
 
	IS_PRESENT		=	0,		
	IS_WRITABLE		=	1,		
	USER_MODE		=	2,		
	WRITETHOUGH		=	3,		
	NOT_CACHEABLE	=	4,		
	IS_ACCESSED		=	5,		
	IS_DIRTY		=	6,		
	PAT			    =	7,		
	CPU_GLOBAL		=	8,		
	LV4_GLOBAL		=	9,		
};
void set_frame (uint32_t* pte, uint32_t physical_address);
uint32_t get_frame (uint32_t pte);
void set_up_paging ();


#endif

Code: Select all

.text
.globl loadPageDirectory
loadPageDirectory:
push %ebp
mov %esp, %ebp
mov 8(%esp), %eax
mov %eax, %cr3
mov %ebp, %esp
pop %ebp
ret

.text
.globl enablePaging
enablePaging:
push %ebp
mov %esp, %ebp
mov %cr0, %eax
or $0x80000001, %eax
mov %eax, %cr0
mov %ebp, %esp
pop %ebp
ret

[nexos]

There is no reason to have your arrays in a structure like that. That's asking for the compiler to do weird things, and it simply adds complexity. Try removing those structures and using an array directly instead

You are directly dereferencing the paging structures. That works fine before paging is enabled, but afterwards, you eventually will be accessing un-mapped memory.

mov 8(%esp) probably should be mov 4(%esp), as addresses are 4 bytes in size with 32 bit paging.

flush_tlb_entry has a few problems. There's no need to disable interrupts before flushing a page. That operation is atomic. Also, it is doing nothing as it requires _MSC_VER to be defined, which clearly won't be defined on GNU toolchains! Move that with your other assembly helpers

or $0x80000001 can be $0x80000000, as the PE bit is (presumably) already set.

To be honest, it looks like you based that code off of Brokenthorn, but they don't have good paging code. It's a ticking time bomb. The proper way to do paging would be to use recursive paging, which a guide to is at https://wiki.osdev.org/User:Neon/Recursive_Paging

Also, those bit setting functions should be inline (or even macros) for performance reasons.

[neon]

Hi,

So...yeah. I am in agreement with above on that paging tutorial and have been for quite some time. I always encourage recursive paging as well if the target is 32 bit or just linear mapping if 64 bit. Many here can provide input if you decide to go that route and it would solve an issue you will run into later. The tutorial code works because Identity Space is not removed -- but in practice it would be. In any case, the code was written with MSVC in mind and we cannot guarantee its correctness on other tool chains without modification. I.e. We assume the structures have no alignment or padding added.

I mentioned about a week ago but it just keeps coming up...copying and pasting paging code never, ever works. It just doesn't. It needs to be carefully designed around your specific needs and if you aren't sure how a core component that effects the whole system works...

However, as you said the page tables looked correct... Do you happen to have a log for when it triple faults and are you able to show the paging tables that show this faulting address should be mapped?

[nexos]

Oh, one thing I forgot: try running your OS in the Bochs emulator. The Bochs log gives pretty detailed info about crashes, more so than QEMU.

[viruss33]

I fixed all the remarks and now I fixed the issue. The problem I think was that I had bug in set_frame method which shifted the address left by 12 bits, but it should just do a bitwise or (I overlooked that in pd entry frame address consists of bits 31-12 of the address, I was thinking it contains bits 0-20. I implemented the recursive paging but the code didn't change much, so please review

Code: Select all

#ifndef PAGE_TABLE_ENTRY
#define PAGE_TABLE_ENTRY

#include <stdint.h>
#include <stdbool.h>

#define PAGES_PER_TABLE 1024
#define PAGE_TABLES_PER_DIRECTORY	1024

#define PAGE_DIRECTORY_INDEX(x) (((x) >> 22) & 0x3ff)	
#define PAGE_TABLE_INDEX(x) (((x) >> 12) & 0x3ff)

#define PAGE_TABLE_ADDRESS_SPACE_SIZE 0x400000

#define DIRECTORY_TABLE_ADDRESS_SPACE_SIZE 0x100000000

#define SET_BIT(pte,bit) ((pte) | (1 << (bit) ))
#define UNSET_BIT(pte,bit) ((pte) & ~(1 << (bit) ))
#define IS_BIT_SET(pte,bit) ((pte) & (1 <<(bit)))
#define SET_FRAME(pte,address) ((pte) | (address) )
#define GET_FRAME(pte) ((pte) & 0xFFFFF000)


#define PAGE_SIZE 4096

typedef uint32_t pt_entry;
typedef uint32_t pd_entry;

typedef uint32_t virtual_address;
typedef uint32_t physical_address;

enum PTE_BIT_NUMBERS {
 
	IS_PRESENT		=	0,		
	IS_WRITABLE		=	1,		
	USER_MODE		=	2,		
	WRITETHOUGH		=	3,		
	NOT_CACHEABLE	=	4,		
	IS_ACCESSED		=	5,		
	IS_DIRTY		=	6,		
	PAT			    =	7,		
	CPU_GLOBAL		=	8,		
	LV4_GLOBAL		=	9,		
};
void set_frame (uint32_t* pte, uint32_t physical_address);
uint32_t get_frame (uint32_t pte);
void set_up_paging ();
void set_bit(uint32_t* pte, uint8_t bit_number);
void unset_bit (uint32_t* pte, int bit_number);
bool is_bit_set (uint32_t* pte, int bit_number) ;
void set_frame (uint32_t* pte, physical_address physical_address);

#endif

Code: Select all

#include <kernel/page_table_entry.h>
#include <stdint.h>
#include <stdbool.h>
#include <kernel/physical_memory_manager.h>
#include <string.h>
#include <kernel/math.h>
#include <stdio.h>

uint32_t* current_directory = 0;

extern void loadPageDirectory(uint32_t*);
extern void enablePaging();
extern void flush_tlb_entry(uint32_t*);

bool allocate_page (uint32_t pt_entry) {
 
	void* block = allocate_block ();
	if (!block)
		return false;
 
	pt_entry= SET_FRAME (pt_entry, (physical_address)block);
	pt_entry= SET_BIT (pt_entry, IS_PRESENT);
 
	return true;
}

void free_page (uint32_t pt_entry) {
 
	void* p = (void*)GET_FRAME (pt_entry);
	if (p)
		free_block (p);
 
	pt_entry= UNSET_BIT (pt_entry, IS_PRESENT);
}

inline uint32_t get_page_table_entry_from_address (uint32_t* page_table,virtual_address address) {
 
	if (page_table)
		return page_table[ PAGE_TABLE_INDEX (address) ];
	return 0;
}

inline uint32_t get_page_directory_entry_from_address (uint32_t* page_directory, virtual_address addr) {
 
	if (page_directory)
		return page_directory[ PAGE_DIRECTORY_INDEX (addr) ];
	return 0;
}



bool switch_page_directory (uint32_t* directory) {
 
	if (!directory)
		return false;
 
	current_directory = directory;
	loadPageDirectory (directory);
	return true;
}


 
uint32_t* get_directory () {
 
	return current_directory;
}


void map_page (void* physical_addr, void* virtual_address) {


   pd_entry entry = current_directory[PAGE_DIRECTORY_INDEX ((uint32_t) virtual_address) ];
   if ( !IS_BIT_SET(entry, IS_PRESENT)) {
        uint32_t* table = (uint32_t*) allocate_block();
        if (!table){
            return;
        }

        memset (table, 0, PAGES_PER_TABLE * sizeof(uint32_t));
        entry = SET_BIT(entry, IS_PRESENT);
        entry = SET_BIT(entry, IS_WRITABLE);
        entry = SET_FRAME(entry, (physical_address) table);

   }
    uint32_t* page_table = (uint32_t*) GET_FRAME ( entry );
    pt_entry page = page_table [ PAGE_TABLE_INDEX ( (uint32_t) virtual_address) ];
    page = SET_FRAME(page, (physical_address)physical_addr);
    page = SET_BIT(page, IS_PRESENT);
}

void set_up_paging () {
 
	uint32_t* default_page_table = (uint32_t*) allocate_block ();
	if (!default_page_table)
		return;
 
	uint32_t* kernel_page_table = (uint32_t*) allocate_block ();
	if (!kernel_page_table)
		return;

	memset(default_page_table, 0, PAGES_PER_TABLE*4);
	memset(kernel_page_table, 0, PAGES_PER_TABLE*4);

    for (uint32_t i=0, frame=0x0, virtual=0x00000000; i<1024; i++, frame+=4096, virtual+=4096) {

		pt_entry page=0;
		page = SET_BIT (page, IS_PRESENT);
 		page = SET_FRAME (page, frame);

		default_page_table[PAGE_TABLE_INDEX (virtual) ] = page;
	}

    for (uint32_t i=0, frame=0x100000, virtual=0xc0000000; i<1024; i++, frame+=4096, virtual+=4096) {

		pt_entry page=0;
		page = SET_BIT (page, IS_PRESENT);
		page = SET_FRAME (page, frame);

		kernel_page_table[PAGE_TABLE_INDEX (virtual) ] = page;
	}
    uint32_t* page_dir = (uint32_t*) allocate_blocks(divide_round_up(4 * PAGE_TABLES_PER_DIRECTORY, BLOCKS_SIZE));
    if (!page_dir){
        return;
    }
    memset(page_dir, 0, PAGE_TABLES_PER_DIRECTORY*4);

    uint32_t pd_entry = 0;
	pd_entry = SET_BIT (pd_entry, IS_PRESENT);
	pd_entry= SET_BIT (pd_entry, IS_WRITABLE);
	pd_entry = SET_FRAME (pd_entry, (physical_address)page_dir);
	page_dir[1023]= pd_entry;
	printf(" pd entry address: %d ", GET_FRAME(pd_entry));

	uint32_t default_entry = 0;
	default_entry = SET_BIT (default_entry, IS_PRESENT);
	default_entry= SET_BIT (default_entry, IS_WRITABLE);
	default_entry = SET_FRAME (default_entry, (physical_address)default_page_table);
	page_dir[PAGE_DIRECTORY_INDEX(0) ] = default_entry;
	printf(" default entry address: %d", GET_FRAME(default_entry));


	uint32_t kernel_entry = 0;
	kernel_entry = SET_BIT (kernel_entry, IS_PRESENT);
	kernel_entry= SET_BIT (kernel_entry, IS_WRITABLE);
	kernel_entry = SET_FRAME (kernel_entry, (physical_address)kernel_page_table);
	page_dir[PAGE_DIRECTORY_INDEX(0xc0000000)] = kernel_entry;
	printf(" kernel entry address: %d", GET_FRAME(kernel_entry));

	switch_page_directory (page_dir);
 
	enablePaging ();


}

Code: Select all

.text
.globl loadPageDirectory
loadPageDirectory:
push %ebp
mov %esp, %ebp
mov 8(%esp), %eax
mov %eax, %cr3
mov %ebp, %esp
pop %ebp
ret

.text
.globl enablePaging
enablePaging:
push %ebp
mov %esp, %ebp
mov %cr0, %eax
or $0x80000000, %eax
mov %eax, %cr0
mov %ebp, %esp
pop %ebp
ret

.text
.globl flush_tlb_entry
flush_tlb_entry:
push %ebp
mov %esp, %ebp
mov 8(%esp), %eax
invlpg	(%eax)
mov %ebp, %esp
pop %ebp
ret

Also I dont understand why do we identity map first 4 mb of memory. And why is paging working, when I map only 1st mb -> 5mb to 3gb + 4mb, when my kernel img is around 12mb. I would expect that it's needed to map 1mb -> 13 mb to 3gb + 12mb, but then it would be stupid design, because with the growing kernel image the code will need to be adjusted.
Also, is my kernel now higher half kernel? Because on wiki in the higher half kernel on bare bones, there are some changes to linker script and also changes in the boot file where we set up paging. Do I need to make these changes?

[nexos]

Shouldn't

Code: Select all

#define SET_FRAME(pte,address) ((pte) | (address) )

be

Code: Select all

#define SET_FRAME(pte,address) ((pte) | ((address) >> 12) )

[neon]

Hi,

Also I dont understand why do we identity map first 4 mb of memory. And why is paging working, when I map only 1st mb -> 5mb to 3gb + 4mb, when my kernel img is around 12mb. I would expect that it's needed to map 1mb -> 13 mb to 3gb + 12mb, but then it would be stupid design, because with the growing kernel image the code will need to be adjusted.
Also, is my kernel now higher half kernel? Because on wiki in the higher half kernel on bare bones, there are some changes to linker script and also changes in the boot file where we set up paging. Do I need to make these changes?

We keep Identity Space mapped only for simplicity; this allows us to freely access video RAM for text output and continue to use the kernel stack without issue. You can remove Identity Space at a later stage (or remove it entirely if the boot loader enables paging for you). I advise not to map a part of an image file into memory. The kernel produced in the tutorial is relatively small. If you have a larger kernel i'd advise to map all of it.

As for the higher half question... well... are you currently executing code at 3GB? That is, if you were to remove Identity Space such that the only code executing is at 3GB, would it work? Getting a higher half kernel isn't really that difficult. You can either (1) get the boot loader to do it for you or, (2) have the kernel do it and relocate its code to 3GB. Most people go with (1). The tutorial uses option (1) with a custom boot loader. My OS uses option (2) but I have used (1) in the past as well. If you change the entry point address in the linker script to be 3GB it is probably using option (1) unless the initial code is position independent.

[viruss33]

Thanks for replies. I checked the addresses and they are correct.


I did a check and tried to put value to address that is unmapped and I get an infinite loop calling my isr handler as a page fault. The code just tries to write value to that address then calls the isr handler then again tries to write value to that address and so on. Is it normal behaviour? Or a problem with my code?

Code: Select all

.global idt_flush

idt_flush:
    movl 4(%esp), %eax  # Get the pointer to the IDT, passed as a parameter.
    lidt (%eax)        # Load the IDT pointer.
    ret

.global enable_interrupts
enable_interrupts:
    sti
    ret

.extern isr_handler

.macro isr_err_stub p
.global isr\p
isr\p:
    push $\p
    jmp isr_common_stub  
    iret 
.endm

.macro isr_no_err_stub p
.global isr\p
isr\p:
    push $0
    push $\p
     jmp  isr_common_stub  
    iret
.endm


isr_no_err_stub 0
isr_no_err_stub 1
isr_no_err_stub 2
isr_no_err_stub 3
isr_no_err_stub 4
isr_no_err_stub 5
isr_no_err_stub 6
isr_no_err_stub 7
isr_err_stub    8
isr_no_err_stub 9
isr_err_stub    10
isr_err_stub    11
isr_err_stub    12
isr_err_stub    13
isr_err_stub    14
isr_no_err_stub 15
isr_no_err_stub 16
isr_no_err_stub 17
isr_no_err_stub 18
isr_no_err_stub 19
isr_no_err_stub 20
isr_no_err_stub 21
isr_no_err_stub 22
isr_no_err_stub 23
isr_no_err_stub 24
isr_no_err_stub 25
isr_no_err_stub 26
isr_no_err_stub 27
isr_no_err_stub 28
isr_no_err_stub 29
isr_err_stub    30
isr_no_err_stub 31


isr_common_stub:
   pusha                    # Pushes edi,esi,ebp,esp,ebx,edx,ecx,eax

   mov %ds, %ax               # Lower 16-bits of eax = ds.
   push %eax                 # save the data segment descriptor

   mov $0x10, %ax # load the kernel data segment descriptor
   mov %ax, %es
   mov %ax, %fs
   mov %ax, %ds
   mov %ax, %gs

    push %esp
   call isr_handler
   add $4, %esp 

   pop %eax        # reload the original data segment descriptor
   mov %ax, %ds
   mov %ax, %es
   mov %ax, %fs
   mov %ax, %gs

   popa                     # Pops edi,esi,ebp...
   add $8, %esp    # Cleans up the pushed error code and pushed ISR number   
   iret           # pops 5 things at once: CS, EIP, EFLAGS, SS, and ESP

Code: Select all

void isr_handler(registers_t* registers)
{
   registers_t reg_value = registers[0];
   printf("Received interrupt %d ", reg_value.irq_number);   
   
}

[nexos]

You are returning from the ISR. This means that the CPU will repeat the faulting instructions, and hence trigger another fault. So yes, there is nothing wrong.