OSDev.org

The Place to Start for Operating System Developers
http://f.osdev.org/

Reading the disk with AHCI.

http://f.osdev.org/viewtopic.php?t=40718

Page 2 of 4

Re: Reading the disk with AHCI.

Posted: Fri Feb 12, 2021 6:56 pm
by foliagecanine
Bonfra wrote:Reading all of the discussion above I assume that I'm not going anywhere without correctly enabling paging...
This is what I love/hate about os-dev, you never run out of things to implement.
So, ok, I'll study paging and I'll implement it. Then I'll come back and implement all of the steps listed above. I hope it doesn't take too long XD
Have you tried using the code in my previous post? It should identity map everything up to 4GiB.

Re: Reading the disk with AHCI.

Posted: Sat Feb 13, 2021 3:43 pm
by Bonfra
foliagecanine wrote:Have you tried using the code in my previous post? It should identity map everything up to 4GiB.
I spent all day studying in deep the concepts of paging and now I just need to implement it.
I've copied your code to start from but now I'm trying to convert to intel syntax that little line of inline assembly.
This is the original line you posted:

Code: Select all

asm volatile("mov %0, %%cr3":: "r"(new_cr3));
and this is how I translated:

Code: Select all

asm volatile("mov cr3, %0":: "r"(new_cr3));
but it does not work, actually, if I run the code with the debugger line by line after executing that line it halts forever. Maybe I'm doing it in the wrong moment. I'm starting the kernel with the "week paging" I implemented the bootloader and then I call this new function in the kernel at some point. Is it wrong?

Re: Reading the disk with AHCI.

Posted: Sat Feb 13, 2021 8:02 pm
by xeyes
Bonfra wrote:
foliagecanine wrote:Have you tried using the code in my previous post? It should identity map everything up to 4GiB.
I spent all day studying in deep the concepts of paging and now I just need to implement it.
I've copied your code to start from but now I'm trying to convert to intel syntax that little line of inline assembly.
This is the original line you posted:

Code: Select all

asm volatile("mov %0, %%cr3":: "r"(new_cr3));
and this is how I translated:

Code: Select all

asm volatile("mov cr3, %0":: "r"(new_cr3));
but it does not work, actually, if I run the code with the debugger line by line after executing that line it halts forever. Maybe I'm doing it in the wrong moment. I'm starting the kernel with the "week paging" I implemented the bootloader and then I call this new function in the kernel at some point. Is it wrong?
If you use GCC inline assembly the syntax is ATT not Intel unless you are somehow forcing things in Makefiles?

Maybe print CR3's value after you step through this move to be sure that it has the value you have in mind.
Bonfra wrote:This is what I love/hate about os-dev, you never run out of things to implement.
Good news is that since there are so many things, you can choose to focus on what interests you the most.

Although some does have prerequisites and IMHO since AHCI involves juggling around multiple ring buffers and data buffers as well as the need to access MMIO registers, a working understanding of paging is needed.

Re: Reading the disk with AHCI.

Posted: Sun Feb 14, 2021 4:25 am
by Bonfra
xeyes wrote: If you use GCC inline assembly the syntax is ATT not Intel unless you are somehow forcing things in Makefiles?
Yes, this flags switch GCC to Intels syntax -masm=intel
xeyes wrote: Maybe print CR3's value after you step through this move to be sure that it has the value you have in mind.
I couldn't print it because it crashes right after the instruction but with GDB I managed to ensure it contained the right value. Being not this line the problem I also randomly started talking about this problem with paging on this discord server and they pointed out this thing:"your page tables are of the wrong size. all page table levels are 512 entries in long mode" but not being my code I don't know exactly what to change to follow their suggestion. Is it just the size of the arrays or something else?

Re: Reading the disk with AHCI.

Posted: Sun Feb 14, 2021 10:26 am
by foliagecanine
Bonfra wrote:Being not this line the problem I also randomly started talking about this problem with paging on this discord server and they pointed out this thing:"your page tables are of the wrong size. all page table levels are 512 entries in long mode" but not being my code I don't know exactly what to change to follow their suggestion.
Hmm... This was my error in the code. I assumed that long mode simply extended the protected mode paging method, but it appears that ALL page tables are converted to 64-bit. Therefore, there can only be 512 entries of any table entry per page. I guess I didn't read far enough in the AMD64 Manual. If you want to look at it, it's the AMD64 Manual, Section 5.3.3

Try this code:

Code: Select all

typedef struct {
	uint8_t present:1;
	uint8_t readwrite:1;
	uint8_t user:1;
	uint8_t writethru:1;
	uint8_t cachedisable:1;
	uint8_t access:1;
	uint8_t reserved:3;
	uint8_t avl:3;
	uint64_t address:36;
	uint16_t ignore:15;
	uint8_t execdisable:1;
} __attribute__((packed)) pml4t_entry;

typedef struct {
	uint8_t present:1;
	uint8_t readwrite:1;
	uint8_t user:1;
	uint8_t writethru:1;
	uint8_t cachedisable:1;
	uint8_t access:1;
	uint8_t reserved:3;
	uint8_t avl:3;
	uint64_t address:36;
	uint16_t ignore:15;
	uint8_t execdisable:1;
} __attribute__((packed)) pdpt_entry;

typedef struct {
	uint8_t present:1;
	uint8_t readwrite:1;
	uint8_t user:1;
	uint8_t writethru:1;
	uint8_t cachedisable:1;
	uint8_t access:1;
	uint8_t dirty:1;
	uint8_t size:1;
	uint8_t global:1;
	uint8_t avl:3;
	uint64_t address:36;
	uint16_t ignore:15;
	uint8_t execdisable:1;
} __attribute__((packed)) page_dir_entry;

typedef struct {
	uint8_t present:1;
	uint8_t readwrite:1;
	uint8_t user:1;
	uint8_t writethru:1;
	uint8_t cachedisable:1;
	uint8_t access:1;
	uint8_t dirty:1;
	uint8_t pat:1;
	uint8_t global:1;
	uint8_t avl:3;
	uint64_t address:36;
	uint16_t ignore:11;
	uint8_t: pke:4;
	uint8_t execdisable:1;
} __attribute__((packed)) page_table_entry;

pml4t_entry pml4_table[512] __attribute__((aligned(4096)));
pdpt_entry pdp_table[512] __attribute__((aligned(4096)));
page_dir_entry page_dir[512*4] __attribute__((aligned(4096)));
page_table_entry page_table[512*512*4] __attribute__((aligned(4096)));

void identity_map_everything() {
	pml4_table[0].present = 1;
	pml4_table[0].readwrite = 1;
	pml4_table[0].user = 0; // You'll need to set this to 1 if you are using usermode
	pml4_table[0].execdisable = 0;
	pml4_table[0].address = ((intptr_t)&pdp_table[0])>>12;
	
	for (int i = 0; i < 4; i++) {
		pdp_table[0].present = 1;
		pdp_table[0].readwrite = 1;
		pdp_table[0].user = 0; // 1 if using usermode
		pdp_table[0].execdisable = 0;
		pdp_table[0].address = ((intptr_t)&page_dir[i*512])>>12;
	}

	for (int i = 0; i < 512; i++) {
		page_dir[i].present = 1;
		page_dir[i].readwrite = 1;
		page_dir[i].user = 0; // 1 if using usermode
		page_dir[i].address = ((intptr_t)&page_table[i*512])>>12;
	}

	for (int i = 0; i < 512*512; i++) {
		page_table[i].present = 1;
		page_table[i].readwrite = 1;
		page_table[i].user = 0; // 1 if using usermode
		page_table[i].address = i;
	}
	
	uint64_t new_cr3 = (uint64_t)(intptr_t)&pml4_table[0];
	asm volatile("mov cr3, %0":: "r"(new_cr3)); // (Since you said you were using Intel Syntax)
}
Also, where is the base address that your kernel loaded at?

Re: Reading the disk with AHCI.

Posted: Sun Feb 14, 2021 11:30 am
by nullplan
foliagecanine wrote:I assumed that long mode simply extended the protected mode paging method,
It does, but they extended the PAE method of paging, not the non-PAE method. Therefore all entries are 64-bits and there are 512 entries per page.

Re: Reading the disk with AHCI.

Posted: Sun Feb 14, 2021 12:22 pm
by Bonfra
foliagecanine wrote:Also, where is the base address that your kernel loaded at?
the compiled elf file is loaded at 0x00100000 so the code starts at 0x00101000.

The code above has the same problem as before.. after the cr3 update, it does not work anymore...
I pushed all the code to a git repo so you can check if I messed up something somewhere else

Re: Reading the disk with AHCI.

Posted: Sun Feb 14, 2021 12:23 pm
by foliagecanine
Bonfra wrote:I pushed all the code to a git repo so you can check if I messed up something somewhere else
Thank you. I'll see if I can get it to work.

Re: Reading the disk with AHCI.

Posted: Sun Feb 14, 2021 2:12 pm
by foliagecanine
Alright! I got the paging thing to work. I haven't tested any AHCI code, but at least we're getting somewhere.
It was mostly a problem of forgetting to zero out the data and forgetting to index pdp_table.

Rather than posting the code here, here's a link to a GitHub Gist:
https://gist.github.com/foliagecanine/6 ... 3ba2c9755b

(May I suggest adding some sort of license to BonsOS? I don't know exactly your goals for this project, so it's up to you.)

Re: Reading the disk with AHCI.

Posted: Sun Feb 14, 2021 5:00 pm
by Bonfra
foliagecanine wrote:Alright! I got the paging thing to work.
Ok paging works! I just needed to move the stack because theese big data structures overlapped with it.
I tried to access that field in the AHCI code but I still get a page fault. I've checked the ABAR of the device I'm detecting and it is at 0xFEBD5000. Is this mapped by the paging code we discussed before?
foliagecanine wrote: (May I suggest adding some sort of license to BonsOS? I don't know exactly your goals for this project, so it's up to you.)
First or then I need to add it you are right

Re: Reading the disk with AHCI.

Posted: Sun Feb 14, 2021 5:23 pm
by foliagecanine
Bonfra wrote:Uhm.. yea paging setup works but for some reason it never returns from the function...
Yep. I figured that out too. It's because the stack is located at 0x200000.
Unfortunately, due to the amount of space needed for the page tables, your kernel now requires more than 1MiB of memory, so it overwrites the stack.
Good news is this is fixable.

boot/include/memory_layout.inc:

Code: Select all

... Replace Line 31-32:
Mem.Kernel.Stack.Bottom             equ 0x0E000000
Mem.Kernel.Stack.Top                equ 0x01000000
...
boot/include/paging.inc:

Code: Select all

... Insert after Line 60:
		mov dword [es:di + 0x28], 0x00A00000 | .StdBits | .LargePage
		mov dword [es:di + 0x30], 0x00C00000 | .StdBits | .LargePage
		mov dword [es:di + 0x38], 0x00E00000 | .StdBits | .LargePage
...
That should fix your problem. :D

EDIT:
I've updated the Gist to use 2MiB pages instead of 4KiB pages to make the kernel smaller. This code works with the existing stack but I would still recommend increasing the stack to prevent errors once your kernel grows beyond 1MiB.

Re: Reading the disk with AHCI.

Posted: Sun Feb 14, 2021 5:31 pm
by Bonfra
foliagecanine wrote: It's because the stack is located at 0x200000.
Unfortunately, due to the amount of space needed for the page tables, your kernel now requires more than 1MiB of memory, so it overwrites the stack.
Uh, I'm starting to understand bugs XD
foliagecanine wrote: Good news is this is fixable.
...
That should fix your problem. :D
This solves the problem with paging and it works perfectly, but I still get a page fault accessing the AHCI device memory.
I tried to access that field in the AHCI code but I still get a page fault. I've checked the ABAR of the device I'm detecting and it is at 0xFEBD5000. Is this mapped by the paging code we discussed before?

Re: Reading the disk with AHCI.

Posted: Sun Feb 14, 2021 5:59 pm
by foliagecanine
Okay. There was one mistake (I forgot to multiply the number of page directories to fill by 4).
This caused only the first 1GiB to be mapped.
It's fixed and the Gist is updated to now use 2MiB pages (makes kernel smaller).

Also, I would recommend changing
Mem.Kernel.Stack.Bottom to 0x00400000 and
Mem.Kernel.Stack.Top to 0x00500000

and

Code: Select all

pfa_init((void *)0x00500001);
...
pfa_deinit_region(0, 0x00500000);
instead of what you have currently (I assume that the memory layout should be Paging stuff, then Kernel, then stack, then heap)

Re: Reading the disk with AHCI.

Posted: Sun Feb 14, 2021 6:05 pm
by Octocontrabass
foliagecanine wrote:2MiB pages
Be careful: behavior is undefined when a 2MiB page crosses a memory type boundary.

Re: Reading the disk with AHCI.

Posted: Sun Feb 14, 2021 6:13 pm
by foliagecanine
Octocontrabass wrote:Be careful: behavior is undefined when a 2MiB page crosses a memory type boundary.
Is this referring to AMD64 Architecture Programmer's Manual section 7.7.4 (MTRRs and Page Cache Controls)?

sigh

Edit:
Are there specific paging flags (Cachedisable, writethrough) that need to be set on memory used by AHCI (and others)?
How do you know which flags are necessary?
All times are UTC-06:00
Page 2 of 4

Powered by phpBB® Forum Software © phpBB Limited