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

Copy all old pages table entries #464

Closed
wants to merge 1 commit into from
Closed

Conversation

Colepng
Copy link

@Colepng Colepng commented Oct 25, 2024

Corrects the assumption that only the first entry is the active page table is used.

closes #462

@Freax13
Copy link
Member

Freax13 commented Oct 25, 2024

Does this panic you saw in #462 still happen with these changes?

@Colepng
Copy link
Author

Colepng commented Oct 25, 2024

Yeah it still panics when trying to allocate boot info. I will try and do some more digging when I have a chance. It's panicking here.

Image.

@Freax13
Copy link
Member

Freax13 commented Oct 25, 2024

Hmm, so the problem is that up until now we've assumed that all existing memory is only in the first pml4 entry and so when we map in more memory, we assume that all other memory is unused:

used.entry_state[0] = true; // TODO: Can we do this dynamically?

@Colepng
Copy link
Author

Colepng commented Oct 26, 2024

Hmm. An simple fix would be to check what entry the framebuffer is in and mark it as used. I can try this patch out but any other devices that are mapped would also cause a similar issue.

@Colepng
Copy link
Author

Colepng commented Oct 26, 2024

Wait my bad there seems to already be a check for the framebuffer address.

@Colepng
Copy link
Author

Colepng commented Oct 26, 2024

So I got it working. All I added was cloning the second entry in the level 4 and marking it used. I don't know how to go about writing a general solution but I would love to help.

@Freax13
Copy link
Member

Freax13 commented Oct 26, 2024

Hmm. An simple fix would be to check what entry the framebuffer is in and mark it as used. I can try this patch out but any other devices that are mapped would also cause a similar issue.

Wait my bad there seems to already be a check for the framebuffer address.

I assume you mean this check, right? That check operates on the address where the frame buffer will be mapped into the kernel which is likely not the same address where it's mapped in the bootloader.

So I got it working. All I added was cloning the second entry in the level 4 and marking it used. I don't know how to go about writing a general solution but I would love to help.

We could mark all entries covered by the frame buffer as used and copy them over when creating the new PML4, but I think the cleaner solution is to just never modify the page tables used by the bootloader. Still, I opened a PR based on your idea as well in #466. The big upside is that it's much simpler than my original implementation in #465.

@Freax13
Copy link
Member

Freax13 commented Nov 14, 2024

Superseeded by #466

@Freax13 Freax13 closed this Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hang after loading first level 4 table on uefi
2 participants