OSDev.org

Posted: **Sun May 23, 2021 9:42 am**

nullplan wrote:A page table entry is essentially just the physical address of the next page table or the page with a few bits ORed in, so just print it in hex.

Thanks for the advice, it was simpler to track the issue.
Whenever I attach a new page I assign the address of the next entry-level to an entry and then to populate that entry-level I retrieve it back with bit-shifting. This was the part I was doing wrong.
My code was:

Code: Select all

uint64_t* pdp = (uint64_t*)((pml4[pml4_offset] & PML_ADDRESS >> 12));

so just removing the shift fixes the issue.
Kinda.
Now `info mem` returns some readable results, but they are for the most part wrong. Some pages have only the read flag and not the write and most of them are with a totally wrong size:

Code: Select all

0000000000200000-0000000000400000 0000000000200000 -rw
0000000003c00000-0000000003e00000 0000000000200000 -rw
0000000004400000-0000000004600000 0000000000200000 -rw
0000000004a05000-0000000004a06000 0000000000001000 -r-
0000000004a07000-0000000004a08000 0000000000001000 -rw
0000000004a08000-0000000004a0b000 0000000000003000 -r-
0000000004a0c000-0000000004a0d000 0000000000001000 -rw
0000000004a0f000-0000000004a10000 0000000000001000 -r-
0000000004a10000-0000000004a11000 0000000000001000 -rw
0000000004a12000-0000000004a13000 0000000000001000 -r-
0000000004a14000-0000000004a15000 0000000000001000 -rw
0000000004a15000-0000000004a16000 0000000000001000 -r-
0000000004a19000-0000000004a1c000 0000000000003000 -r-
0000000004a1e000-0000000004a20000 0000000000002000 -rw
0000000004a25000-0000000004a27000 0000000000002000 -r-
...

Now I think that the issue could be with the loop I use to attach the pages. Do you see anything weird with the calculation?

Code: Select all

    for(uint64_t i = 0; i < memorySize / 1024; i += 2) // size in KB
        paging_attach_2mb_page(paging_data, i * 0x200000, i * 0x200000); // pml4_addr, physical, virtual

Posted: **Sun May 23, 2021 2:22 pm**

Bonfra wrote:Now I think that the issue could be with the loop I use to attach the pages. Do you see anything weird with the calculation?

Yeah, it does look weird. Loops work better if you iterate either over the correct series, or you iterate over the integers and map all the values correctly. But this hybrid approach where you increase i by 2 each time and then still multiply it with some number seems weird to me. I would probably just write it like this:

Code: Select all

for (size_t i = 0; i < memorySize; i += 2 << 20)
  paging_attach_2mb_page(paging_data, i, i);

Let's see, what values do you calculate in your loop?

Code: Select all

i | i * 0x200000
--+-------------
0 | 0
2 | 0x400000
4 | 0x800000
6 | 0xC00000
8 | 0x1000000
...

Pretty sure those were not the inputs you were looking for. But beyond that the output of your info mem also does not fit these values if paging_attach_2mb_page was working correctly, so something weird is going on there as well.

Posted: **Sun May 23, 2021 3:22 pm**

nullplan wrote:Pretty sure those were not the inputs you were looking for.

Yeah, I don't know what I was thinking when I wrote that loop. with your code `info mem` returns just one line

Code: Select all

0000000000000000-0000000000200000 0000000000200000 -rw

Which is correct but it's only the first page. the others are missing so something weird must be going on in the attach function.

nullplan wrote: But beyond that, the output of your info mem also does not fit these values if paging_attach_2mb_page was working correctly, so something weird is going on there as well.

Exactly, I'm posting here the code for the new attach function with the correction you suggested before. Maybe you can spot something I missed.

Code: Select all

#define PML_PRESENT (1ull << 0)
#define PML_READWRITE (1ull << 1)
#define PML_USER (1ull << 2)
#define PML_WRITETHROUGH (1ull << 3)
#define PML_CACHEDISABLE (1ull << 4)
#define PML_ACCESSED (1ull << 5)
#define PML_SIZE (1ull << 7)
#define PML_AVAILABLE (0b111ull << 9)
#define PML_ADDRESS (0xFFFFFFFFFFull << 12)
#define PML_EXECDISABLE (1ull << 63)

#define PML_CLEAR_AVAILABLE(entry) (entry &= ~PML_AVAILABLE)
#define PML_SET_AVAILABLE(entry, val) (entry |= (val & PML_AVAILABLE))
#define PML_UPDATE_AVAILABLE(entry, val) (PML_CLEAR_AVAILABLE(entry), PML_SET_AVAILABLE(entry, val))

#define PML_CLEAR_ADDRESS(entry) (entry &= ~PML_ADDRESS)
#define PML_SET_ADDRESS(entry, val) (entry |= (val & PML_ADDRESS))
#define PML_UPDATE_ADDRESS(entry, val) (PML_CLEAR_ADDRESS(entry), PML_SET_ADDRESS(entry, val))

void paging_attach_2mb_page(paging_data_t data, void* physical_addr, void* virtual_addr)
{
    uint64_t pml4_offset = ((uint64_t)virtual_addr >> 39) & 0x01FF;
    uint64_t pdp_offset = ((uint64_t)virtual_addr >> 30) & 0x01FF;
    uint64_t pd_offset = ((uint64_t)virtual_addr >> 21) & 0x01FF;

    uint64_t* pml4 = data;
    if((pml4[pml4_offset] & PML_PRESENT) == 0)
    {
        uint64_t* pdp = pfa_alloc_page();
        memset(pdp, 0, pfa_page_size());

        pml4[pml4_offset] = (PML_PRESENT | PML_READWRITE) & ~PML_USER;
        PML_UPDATE_ADDRESS(pml4[pml4_offset], (uint64_t)pdp);
    }

    uint64_t* pdp = (uint64_t*)((pml4[pml4_offset] & PML_ADDRESS));
    if((pdp[pdp_offset] & PML_PRESENT) == 0)
    {
        uint64_t* pd = pfa_alloc_page();
        memset(pd, 0, pfa_page_size());

        pdp[pdp_offset] = (PML_PRESENT | PML_READWRITE) & ~PML_USER;
        PML_UPDATE_ADDRESS(pdp[pdp_offset], (uint64_t)pd);
    }

    uint64_t* pd = (uint64_t*)((pdp[pdp_offset] & PML_ADDRESS));
    pd[pd_offset] = (PML_PRESENT | PML_READWRITE | PML_SIZE) & ~PML_USER;
    PML_UPDATE_ADDRESS(pd[pd_offset], (uint64_t)physical_addr);
}

Posted: **Mon May 24, 2021 1:21 am**

Inspecting the page table by hand it is in fact not populated:

Code: Select all

x /gx 0xd07000 // pml4 address
0xd07000:	0x0000000000d08003

x /gx 0xd08000
0xd08000:	0x0000000000d09003

x /3gx 0xd09000
0xd09000:	0x0000000000000083	0x0000000000000000
0xd09010:	0x0000000000000000

So the stepping through the code I noticed that for loop you suggested iterates only one time

Code: Select all

for(uint64_t i = 0; i < memorySize; i += 2 << 20)
    paging_attach_2mb_page(paging_data, i, i);

I modified the for and ensured that it printed the right values but in the `info mem` these values aren't the same

So it must be something in the `paging_attach_2mb_page` function

Posted: **Mon May 24, 2021 4:03 am**

Turns out if I step line by line in the code, calling `info mem` the line right after the line where I edit the content of cr3 returns

Code: Select all

0000000000000000-0000000020000000 0000000020000000 -rw

So with 512 MB of memory, this is exactly what I need.
But whenever I execute any line after the page table is correctly loaded everything goes funky: printf function prints a rainbow instead of characters, ISRs are triggered without any reasons and I get SIGQUIT in random parts of the code. also sometimes the content of the page table is altered when a SIGQUI halts the execution

Posted: **Mon May 24, 2021 4:49 am**

Final post here, the function works, I just didn't map the frame buffer. It isn't in memory so obviously, it didn't work.
I'm an idiot now it works.
Thanks for helping me with the code

OSDev.org

Setting up paging

Re: Setting up paging

Re: Setting up paging

Re: Setting up paging

Re: Setting up paging

Re: Setting up paging

Re: Setting up paging