Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

map the P4 table recursively before setting paging #246

Merged
merged 5 commits into from Oct 23, 2016

Conversation

@matanelevi
Copy link
Contributor

commented Oct 22, 2016

We have to map the P4 table recursively before setting paging - can't do this wherever we'd like.

We have to map the P4 table recursively before setting paging - can't do this wherever we'd like.
Copy link
Owner

left a comment

Thanks a lot for the PR! I've left some suggestions.

@@ -253,15 +253,21 @@ By using recursive mapping, each page table is accessible through an unique virt
Of course recursive mapping has some disadvantages, too. It occupies a P4 entry and thus 512GiB of the virtual address space. But since we're in long mode and have a 48-bit address space, there are still 225.5TiB left. The bigger problem is that only the active table can be modified by default. To access another table, the recursive entry needs to be replaced temporary. We will tackle this problem in the next post when we switch to a new page table.

### Implementation
To map the P4 table recursively, we just need to point the 511th entry to the table itself. Of course we could do it in Rust, but it would require some fiddling with unsafe pointers. It's easier to just add some lines to our boot assembly:
To map the P4 table recursively, we just need to point the 511th entry to the table itself. It's very easy to just add some lines to our boot assembly:

This comment has been minimized.

Copy link
@phil-opp

phil-opp Oct 22, 2016

Owner

I would prefer something like "We can do so by adding...", since it might not be easy for everyone.


```nasm
mov eax, p4_table
or eax, 0b11 ; present + writable
mov [p4_table + 511 * 8], eax
```
I put it right after the `set_up_page_tables` label, but you can add it wherever you like.
We could do it also in Rust, but it would require some fiddling with unsafe pointers.

This comment has been minimized.

Copy link
@phil-opp

phil-opp Oct 22, 2016

Owner

Maybe ", but it would be more complicated since it requires some fiddling..."?

@@ -253,15 +253,21 @@ By using recursive mapping, each page table is accessible through an unique virt
Of course recursive mapping has some disadvantages, too. It occupies a P4 entry and thus 512GiB of the virtual address space. But since we're in long mode and have a 48-bit address space, there are still 225.5TiB left. The bigger problem is that only the active table can be modified by default. To access another table, the recursive entry needs to be replaced temporary. We will tackle this problem in the next post when we switch to a new page table.

### Implementation
To map the P4 table recursively, we just need to point the 511th entry to the table itself. Of course we could do it in Rust, but it would require some fiddling with unsafe pointers. It's easier to just add some lines to our boot assembly:
To map the P4 table recursively, we just need to point the 511th entry to the table itself. It's very easy to just add some lines to our boot assembly:

```nasm
mov eax, p4_table
or eax, 0b11 ; present + writable
mov [p4_table + 511 * 8], eax
```
I put it right after the `set_up_page_tables` label, but you can add it wherever you like.

This comment has been minimized.

Copy link
@phil-opp

phil-opp Oct 22, 2016

Owner

Maybe extend this sentence with "as long it's before paging gets enabled" and then explain why?


Can we really do that? add it wherever we want or do this in Rust?
Once we've set paging on, the p4_table will be treatd as a virtual address - so it won't work!

This comment has been minimized.

Copy link
@phil-opp

phil-opp Oct 22, 2016

Owner

I think that this explanation would better fit right after line 263. This way, we wouldn't need to come back to the problem again.

matanelevi added 2 commits Oct 22, 2016
@matanelevi

This comment has been minimized.

Copy link
Contributor Author

commented Oct 22, 2016

@phil-opp Thanks for your review, check this now :)


```nasm
mov eax, p4_table
or eax, 0b11 ; present + writable
mov [p4_table + 511 * 8], eax
```
I put it right after the `set_up_page_tables` label, but you can add it wherever you like.
I put it right after the `set_up_page_tables` label, but you can add it wherever you like, **as long it's before paging gets enabled**. Why? since once we've set paging on, the p4_table will be treated as a **virtual address** - so it won't work!

This comment has been minimized.

Copy link
@phil-opp

phil-opp Oct 23, 2016

Owner

I would prefer italics for emphasis instead of bold, since we don't use it in the rest of the post.


Let's get back to our business.

This comment has been minimized.

Copy link
@phil-opp

phil-opp Oct 23, 2016

Owner

I don't think that we need this transition anymore, since we didn't really switch topics. Could you just remove this sentence?


```nasm
mov eax, p4_table
or eax, 0b11 ; present + writable
mov [p4_table + 511 * 8], eax
```
I put it right after the `set_up_page_tables` label, but you can add it wherever you like.
I put it right after the `set_up_page_tables` label, but you can add it wherever you like, **as long it's before paging gets enabled**. Why? since once we've set paging on, the p4_table will be treated as a **virtual address** - so it won't work!
We have to map the P4 table recursively before setting paging.

This comment has been minimized.

Copy link
@phil-opp

phil-opp Oct 23, 2016

Owner

Hmm, do we really need this sentence? It feels kind of duplicate…

I put it right after the `set_up_page_tables` label, but you can add it wherever you like.
I put it right after the `set_up_page_tables` label, but you can add it wherever you like, **as long it's before paging gets enabled**. Why? since once we've set paging on, the p4_table will be treated as a **virtual address** - so it won't work!
We have to map the P4 table recursively before setting paging.
We could do it also in Rust, but it would be more complicated since it requires some fiddling with unsafe pointers.

This comment has been minimized.

Copy link
@phil-opp

phil-opp Oct 23, 2016

Owner

Could you make this line a new paragraph (by adding a line break before it)?

@matanelevi

This comment has been minimized.

Copy link
Contributor Author

commented Oct 23, 2016

@phil-opp Done :)

@phil-opp phil-opp merged commit f35ea94 into phil-opp:master Oct 23, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@phil-opp

This comment has been minimized.

Copy link
Owner

commented Oct 23, 2016

Thanks a lot for the PR and the quick adjustments!

@matanelevi matanelevi deleted the matanelevi:patch-1 branch Oct 23, 2016
@matanelevi matanelevi restored the matanelevi:patch-1 branch Oct 23, 2016
@matanelevi

This comment has been minimized.

Copy link
Contributor Author

commented Oct 23, 2016

No problem :)

@phil-opp

This comment has been minimized.

Copy link
Owner

commented Nov 22, 2016

Gil left this comment regarding this PR on the blog:

You said that the P4 recursive loop must be set before paging is enabled.
But I wonder - the memory is currently identity mapped, so what difference is there in P4_table address in with/without paging?

Sounds reasonable to me…

@matanelevi What do you think about this? Do you still know what the problem here was?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.