make VirtAddr generic over address validity#586
Draft
Freax13 wants to merge 1 commit intorust-osdev:masterfrom
Draft
make VirtAddr generic over address validity#586Freax13 wants to merge 1 commit intorust-osdev:masterfrom
Freax13 wants to merge 1 commit intorust-osdev:masterfrom
Conversation
As proposed by Joe and Philipp, this patch adds a generic parameter for VirtAddr. This generic parameter can be used to enforce validity for addresses with 4 level paging or 5 level paging. This patch is still a work in progress because I wanted to make that this is what you had in mind before I spend more time on it. I slightly modified the VirtValidity interface to because check_virt_addr by itself cannot be used to implement truncation, but truncation can be used to implement check_virt_addr. That's just a minor implementation detail though, we can discuss this later. TODO: - Add a feature to gate enabling the const_trait_impl feature. - Fix up the remaining code. I didn't adjust all of the code to take a generic parameter for the validity yet. As of right now, there are a ton of compilation errors. - Figure out how to make the mapper impls work with VirtValidity. VirtValidity by itself doesn't tell the mappers how many levels there are. I don't expect this to be difficult to solve. - Default the validity generic parameter to Virt48. Currently, I intentionally left the default out, so that the compiler would tell me about all the code that needed to become aware of it. Rust doesn't allow defaulted generic parameters after non-defaulted ones, so I currently put the validity parameter before the page size parameter for Page. We'll probably want to swap that once we set the default. - Test that the Add/Sub/Step impls work and fix them if they don't. Up until now I mostly focused on fixing compilation errors and not so much on the code actually working. - Add type aliases? User would probably benefit from VirtAddr48, VirtAddr57, Page48, and Page57. - Do the same thing for PhysAddr. - Rebase this on next. This will very likely be a breaking change.
phil-opp
reviewed
Apr 14, 2026
Member
phil-opp
left a comment
phil-opp
left a comment
There was a problem hiding this comment.
Sorry for the slow reply!
I think the approach in general looks good, it just adds quite a bit of noise.
How about we add a non-generic RawVirtAddr type to avoid the generic read methods? This RawVirtAddr could implement TryInto for the two possible VirtAddr<V> types so that you can do the conversion yourself.
For the corresponding write methods, we could take an Into<RawVirtAddr> as argument and implement this trait for all VirtAddr variants.
| unsafe { | ||
| let val: u64; | ||
| asm!(concat!("rd", $name, "base {}"), out(reg) val, options(nomem, nostack, preserves_flags)); | ||
| VirtAddr::new_unsafe(val) |
Member
There was a problem hiding this comment.
This might be problematic if the value is a 5-level address and V is a 48 bit address.
| #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] | ||
| #[repr(transparent)] | ||
| pub struct VirtAddr(u64); | ||
| pub struct VirtAddr<T: VirtValidity>(u64, PhantomData<T>); |
Member
There was a problem hiding this comment.
Can we set the default for T to Virt48?
| #![warn(missing_docs)] | ||
| #![deny(missing_debug_implementations)] | ||
| #![deny(unsafe_op_in_unsafe_fn)] | ||
| #![feature(const_trait_impl)] |
Member
There was a problem hiding this comment.
Is there a way to implement this feature on stable?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As proposed by Joe and Philipp, this patch adds a generic parameter for VirtAddr. This generic parameter can be used to enforce validity for addresses with 4 level paging or 5 level paging.
This patch is still a work in progress because I wanted to make that this is what you had in mind before I spend more time on it.
I slightly modified the VirtValidity interface to because check_virt_addr by itself cannot be used to implement truncation, but truncation can be used to implement check_virt_addr. That's just a minor implementation detail though, we can discuss this later.
TODO:
Closes #435