JamesM tutorial bugs? Help me with paging please!

[alebian]

Hi everyone! I'm a beginner in OS development, and as many of you I started reading osdev.org, osdever.net, Brans and JamesM tutorials, and of course this forum (and some other material out there). I am implementing physical memory management and paging, which I find a bit difficult, so I decided to read as much as I can to understand how those things work in the material mentioned before. I started reading the JamesM tutorial but then I read at osdev that It had well known bugs (http://wiki.osdev.org/James_Molloy%27s_ ... Known_Bugs). So I looked more info about that and found there was a fixed version of it here :https://code.google.com/p/jamesm-tutorials/.

Following the code and tutorials I found some things that called my attention. I wanted to ask you if it is right to say this things are bugs or it's just something I am not understanding correctly (If that is the case I would be grateful if you could explain me, please!). I've read a few times in this forum that those tutorials have some bugs left on purpose, so these might be some. If they are I think this post will be helpful for people who are struggling with errors that these may cause, or people who have missed them in a copy paste situation!

Since I am trying to implement pmm and paging I will talk about a bit of what I'm doing so you can give me your thoughts about that too.

First) The author declared arbitrary memory position for the memory manager stack in PMM_STACK_ADDR which starts at 0xFF000000. What would happen if the computer does not have that much memory if we haven't activated paging (like this case)?
In my OS I am trying that the pmm stack and paging directory are right after the kernel instead of this. I guess is just a design decision.

Anyways the problem comes here:

Code: Select all

void init_pmm (uint32_t start)
{
  // Ensure the initial page allocation location is page-aligned.
  pmm_location = (start + 0x1000) & PAGE_MASK;
}

uint32_t pmm_alloc_page ()
{
  if (pmm_paging_active)
  {
    // Quick sanity check.
    if (pmm_stack_loc == PMM_STACK_ADDR)
      panic ("Error:out of memory.");

    // Pop off the stack.
    pmm_stack_loc -= sizeof (uint32_t);
    uint32_t *stack = (uint32_t*)pmm_stack_loc;
  
    return *stack;
  }
  else
  {
    return pmm_location += 0x1000;
  }
}

This is the first part of pmm.c In main.c init_pmm is called with mboot_ptr->mem_upper as parameter. With this value, he tries to find the next page aligned address and make the pmm work from there.
The thing here is:
In my OS if I print the value of mem_upper it shows me this value 0xFBC0, now for what I've seen in other examples, this value of mem_upper and the mem_lower values are used to get the total memory like this:

Code: Select all

(mboot->mem_lower + mboot->mem_upper) * 1024

And for me that works great, so I don't know if I'm being lucky. But if I'm not and I'm using the mem_upper value correctly, this means that in the tutorials he is using the value in a wrong way, and starts to assign pages in that location which is below the 1Mb mark! This means that if for some reason in the paging init he asks for more pages eventually the ppm will give pages where the kernel is

Second) After pmm and vmm are initialized in main.c he tries to tell the pmm to free all the pages available like this:

Code: Select all

// Find all the usable areas of memory and inform the physical memory manager about them.
  uint32_t i = mboot_ptr->mmap_addr;
  while (i < mboot_ptr->mmap_addr + mboot_ptr->mmap_length)
  {
    mmap_entry_t *me = (mmap_entry_t*) i;

    // Does this entry specify usable RAM?
    if (me->type == 1)
    {
      uint32_t j;
      // For every page in this entry, add to the free page stack.
      for (j = me->base_addr_low; j < me->base_addr_low+me->length_low; j += 0x1000)
      {
        pmm_free_page (j);
      }
    }

    // The multiboot specification is strange in this respect - the size member does not include "size" itself in its calculations,
    // so we must add sizeof (uint32_t).
    i += me->size + sizeof (uint32_t);
 }

I worked a bit with multiboot in my OS and found that it gives you free spaces before your kernel is loaded so you could find that from address 0x100000 to the end of memory will be free (type 1), but I know that I have loaded the kernel at 1Mb and therefore not free! (in my case mboot tells me that address from 0x100000 to 63MB is free (I am using a virtual machine with 64MB)).
So he is actually telling the pmm that the address where the kernel, page directory, page tables and pmm stack are located are free! So I beleive this is working just because since it is a stack, the first few pops give you high addresses far from the tables and the stack, but if you make a program that asks for more pages this will make the OS crash.

In my OS I use the end tag of the linker script and wherever the pmm finished the allocation of the pages asked by the vmm to calculate the real available space and then push in the stack.

Third) In the vmm.c file he does something like this a few times:

Code: Select all

uint32_t pt_idx = PAGE_DIR_IDX((PMM_STACK_ADDR>>12);
page_directory[pt_idx] = pmm_alloc_page () | PAGE_PRESENT | PAGE_WRITE;
memset (page_tables[pt_idx*1024], 0, 0x1000);

Where #define PAGE_DIR_IDX(x) ((uint32_t)x/1024)

I think it's much better if the macro does everything, something like PAGE_DIR_IDX(x) ((uint32_t)x/4096/1024)

This post is by no means trying to affirm that these are bugs (just question) and it is not meant to be a criticism of the tutorial, which I find very useful for beginners like me.

This being said I would take your responses to learn and fix my code!

Also I want to ask you something so I can keep going and make my OS better.
So far I have a physical memory manager similar to the JamesM tutorial (but changing the parts I mentioned earlier), a paging mechanism that only creates the page directory and the first page table using identity paging. Very simple.
My next goals for the OS are implementing the heap and multitasking. I know that for those things I need to use the pmm and vmm to assign the memory they need. So what should my paging and physical memory manager need to have (interface) in order to give space to the heap and every task?

Thank you very much!

You are invited to read my code and tell me what you think. Is always nice to have suggestions and if you find bugs or error would be great too!
I have spend quite a lot of time in the video driver so that it gives useful tools that can also be useful to you!

[JAAman]

In the vmm.c file he does something like this a few times:

Code: Select all

uint32_t pt_idx = PAGE_DIR_IDX((PMM_STACK_ADDR>>12);
page_directory[pt_idx] = pmm_alloc_page () | PAGE_PRESENT | PAGE_WRITE;
memset (page_tables[pt_idx*1024], 0, 0x1000);

Where #define PAGE_DIR_IDX(x) ((uint32_t)x/1024)

it looks like this is creating page tables for the PMM_STACK:
that first line locates the appropriate entry in the page directory that matches the address in PMM_STACK_ADDR -- first the ">>12" shifts it by 12 bits (that is: divide by 4k -- the size of a page) to find the page it is located in, then the PAGE_DIR_IDX macro divides by 1024 (same as ">>10" shifting another 10 bits to find the page table matching the address)

the second line is indexing the page directory by the calculated index and allocating a page of physical memory to go into it
the third line is then clearing the page table so that parts of it that are not yet allocated will be empty


this is really just basic page table initialization

Let's say that shifting and using the macro as it is works. I find the next line:

Code: Select all

page_directory[pt_idx] = pmm_alloc_page () | PAGE_PRESENT | PAGE_WRITE;

Very disturbing to me because pmm_alloc_page() returns a physical address

that is exactly right... that is why it works, we retrieve a physical address from pmm_alloc_page(), since the addresses returned from this function are all page-aligned, they will always have 0s in the lowest bits, this was ensured when we built the physical memory stack, so this will always be the case, and will always be safe to use in this way -- the | then sets the necessary flags in those lower bits (which will always be 0 since the address is always page-aligned)

My next goals for the OS are implementing the heap and multitasking. I know that for those things I need to use the pmm and vmm to assign the memory they need. So what should my paging and physical memory manager need to have (interface) in order to give space to the heap and every task?

you need a function that maps a virtual page to a physical page, a function that gets an available physical page (you already have that), and a function that maps a virtual page to physical memory

like this:
void MapXtoY(uintptr_t virtualAddr, uintptr_t physAddr, unsigned flags) //Map virtual address to a given physical address
uintptr_t GetPhysicalPage() //Retrieve available physical address -- you should already have a function like this
void MapX(uintptr_t virtualAddress) //Map virtual address to physical memory

with that last function implemented something like:

Code: Select all

void MapX(uintptr_t virtualAddress, unsigned flags)
{
     MapXtoY(virtualAddress, GetPhysicalPage(), flags)
}

everything else should be built on top of those functions fairly easily

[alebian]

Thanks for your fast reply! Now the shifting thing makes real sense to me!
I would change the macro to divide the 2 times just for clarity purposes then. Because when I read the name of the macro I thought that It received any address.
Anyways thank you!

[makerimages]

There's a JamesM tutorial that works? Wow!

[BasdP]

Terribly sorry for digging up this article, but I happen to have the same exact question as the original poster, but that's the only part of this post that hasn't been answered yet:

First) The author declared arbitrary memory position for the memory manager stack in PMM_STACK_ADDR which starts at 0xFF000000. What would happen if the computer does not have that much memory if we haven't activated paging (like this case)?

Could anyone please help me out?

For reference, this is what the (updated) JamesM code is for the PMM:
https://github.com/berkus/jamesm-tutori ... /src/pmm.c
https://github.com/berkus/jamesm-tutori ... /src/pmm.h