Porting memory to aarch64
#701
Conversation
…i-rs is fully merged
…uld be set in descriptors.
…alization to `memory`
NathanRoyer
force-pushed
the
uefi-aarch64-memory
branch
from
f26d9c1 to
697e844
Compare
…plemented the creation and filling of a new page table; Implemented the switch to the new page table
…MMU configuration
…nto uefi-aarch64-memory
There was a problem hiding this comment.
A few things:
- Can you incorporate the latest changes to the source files in
memory(from the main branch) into your copied versions of them inaarch64/kernel/memory/? That will make it a lot easier to see what has changed. - A lot of the arch-specific things in
memoryhave been recently improved intheseus_mainso you can probably avoid or move lots of those cfg(target_arch) parts. - In
page_table_entry, you're directly modifying PTE flags. Instead of doing it there, move it elsewhere (e.g., intoPteFlagsArch::adjust_for_higher_level_pte()or somewhere else in thePteFlagsAarch64module). Ideally the entirepage_table_entrycrate would be arch-independent, and any required flags would be handled in thePteFlags<Arch>module.
| flags: PteFlags, | ||
| active: bool, | ||
| ) -> Result<(MappedPages, AllocatedFrames), &'static str> { | ||
| // On aarch64, P4/P3/P2 should only contain valid |
There was a problem hiding this comment.
some new changes in the main branch should address this. I've added a function called PteFlagsArch::adjust_for_higher_level_pte() that is the proper place to set these higher_level_flags (previously called top_level_flags)
There was a problem hiding this comment.
I changed the aarch64 version of adjust_for_higher_level_pte so that it works; my implementation discards any flag set by the caller, which isn't great
| } | ||
|
|
||
| /// Installs a page table in the TTBR CPU register | ||
| pub fn set_page_table_addr(page_table: PhysicalAddress) { |
There was a problem hiding this comment.
- I think this should be called
set_as_active_page_table_rootor something like that - It should accept a reference to a
PageTableto ensure that nobody can just directly call it with some randomPhysicalAddress - Add a similar function to
memory_x86_64and use it inPageTable::switch()so that the structure mimics that of aarch64
There was a problem hiding this comment.
One problem with 2: PageTable is defined in memory, which depends on memory_aarch64 and memory_x86_64; we can't use PageTable in them. I'd keep it the way it is for simplicity (I think it would be a bit cumbersome to pull PageTable from memory while keeping the rest of paging there).
There was a problem hiding this comment.
ah, cyclic dependencies strike again. Okay, that's fine for now, we can always change it later.
There was a problem hiding this comment.
I left a comment about the adjust_higher_level_pte_flags() function on aarch64.
One major issue is that I don't want to merge in the concept of NextLevelAccess to the memory crate in the main repo. I understand this is necessary because you're currently re-using the UEFI-provided page table, but after we finish merging in support for UEFI on x86, this won't be necessary. Therefore, I don't want to merge in such wide-sweeping, intrusive changes to all of the page translation code, and then have to undo it all later.
So I think it's best to keep those changes in your /aarch64/kernel crates only. We can integrate aarch64 stuff into /kernel/memory after UEFI is complete, and that will also make it easier to implement memory in a mostly arch-agnostic manner, since I don't think most of it will need to change.
If you can remove those changes, I'm happy to accept this PR; everything else looks good.
NathanRoyer
force-pushed
the
uefi-aarch64-memory
branch
from
141b701 to
2e66ab1
Compare
There was a problem hiding this comment.
Looking all good now! Thanks.
One thing for the future: the set_as_active_page_table_root() fn being both public and safe is not sound, since it allows anyone to change the root page table to any arbitrary PhysicalAddress. All other pub functions in memory_<arch> are safe to expose publicly.
There are a few easy solutions to this:
- move
memory_x86_64andmemory_aarch64crates into modules within thememorycrate and make that fn private orpub(crate) - move the logic back into
PageTable::switch()and have arch-specific cfg blocks directly in that function- This is currently what I have done in this PR
- any other idea you may have, etc
* aarch64 support for basic paging configuration code, page table creation, populating page table entries, and switching page tables. * Ensure that the recursive P4 table address is canonical; on aarch64 this means having the 16 MSBs cleared, on x86_64 this means the typical sign-extended canonicalization. * Ensure MAIR profiles match the index order expected by `pte_flags`. * Force the correct flags for the recursive P4 entry and all higher-level PTEs on aarch64. * Properly handle `PAGE_DESCRIPTOR` and `ACCESSED` PTE flags in all aarch64 PTEs according to Theseus's expectations of a 48-bit virtual address and an ASID of 0. Co-authored-by: Kevin Boos <[email protected]> cc62b18
* aarch64 support for basic paging configuration code, page table creation, populating page table entries, and switching page tables. * Ensure that the recursive P4 table address is canonical; on aarch64 this means having the 16 MSBs cleared, on x86_64 this means the typical sign-extended canonicalization. * Ensure MAIR profiles match the index order expected by `pte_flags`. * Force the correct flags for the recursive P4 entry and all higher-level PTEs on aarch64. * Properly handle `PAGE_DESCRIPTOR` and `ACCESSED` PTE flags in all aarch64 PTEs according to Theseus's expectations of a 48-bit virtual address and an ASID of 0. Co-authored-by: Kevin Boos <[email protected]> cc62b18
This adds aarch64 support to the
memoryas well asmemory_aarch64which contains utility functions for MMU management under aarch64.