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

dont modify the bootloader's pagetables #465

Closed
wants to merge 4 commits into from

Conversation

Freax13
Copy link
Member

@Freax13 Freax13 commented Oct 26, 2024

Don't modify the bootloader's page tables.
Previously we assumed that the page tables set up by UEFI would only contain relevant mappings in the first PML4 entry, but it turns out that some implementations map the frame buffer into address space ranges mapped by later PML4 entries. As a result, we can no longer just remove all but the first PML4 entry. The problem is that because we've assumed that only the first PML4 entry is used, we've been mapping memory into the address ranges covered by other PML4 entries, but that may no longer work if UEFI already happened to have mapped some memory there.
Fortunately, there's only a single location where we've been mapping memory into the bootloader page tables: We mapped the boot info and memory map into both the bootloader and kernel address spaces so that we can simply create references to them and pass them directly from the bootloader to the kernel. Instead, we now only map memory into the bootloader's address space. When we need to access the memory from the bootloader, we traverse the kernel's page tables to look up the physical address and use the identify mappings to access the physical memory. This is mostly consistent with how we access the kernel's memory in other places.

@Colepng Could you please test that this resolves the issue on your system?

Supersedes #464
Closes #462

This has the advantage that we don't need to map the memory map into
the bootloader's page tables. Up until now, we've assumed that UEFI
only maps memory into regions covered by the first PML4 entry, but it
turns out that some implementations map the frame buffer into memory
mapped by other PML4 entries (this is totally legal, the frame buffer
need not be identity mapped). Previously we removed all but the first
PML4 entry, so that we can map our own memory there, but it's now clear
that this can lead to problems. The solution to this is to not modify
the page tables constructed by UEFI and keep all its PML4 entries, but
this is problematic because the code constructing the boot info and
memory map assumes that it can map them into the same addresses used
for the kernel's address space, but UEFI may have already mapped some
memory in there. Instead of mapping the boot info and memory map into
the bootloader's address space, we now only map them into the kernel's
address space. When we want to write to them, we first look up the
physical addresses in the kernel's page table and write to the identity
mapping. RemoteMemoryRegion implements this.
We have some unit tests for constructing the memory map and we want
those unit tests to be able to run outside the bootloader, so we can't
use RemoteMemoryRegion. To solve this, we introduce a trait
MemoryRegionSlice that only requires implementing a method for writing
a memory region and implement that slice for MemoryRegionSlice as well
as MemoryRegionSlice.
In the future, we'll no longer map the BootInfo into the bootloader's
page tables, so we can't/shouldn't create a mutable reference to it.
We don't need to use a &'static mut anyway using VirtAddr works just as
well.
Modifying the bootloaders page tables by mapping addresses that we
don't know for sure are available can lead to all sorts of problems.
Simply stop doing that.
We don't need to do this anymore because we no longer modify the
bootloader's page tables. This also finally makes it possible to access
all memory mapped in by UEFI and not just the memory accessible through
the first PML4 entry. Some UEFI implementations map the frame buffer
into ranges not accessible through the first PML4 entry.
@Freax13
Copy link
Member Author

Freax13 commented Oct 26, 2024

I implemented an alternative fix in #466 based on @Colepng's idea.

@Colepng
Copy link

Colepng commented Oct 26, 2024

I just tested both pull requests, they both work great.

@phil-opp phil-opp mentioned this pull request Nov 1, 2024
@Freax13
Copy link
Member Author

Freax13 commented Nov 14, 2024

Closed in favor of #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