feat(hal-core): address type improvements #64
Conversation
hawkw
added
kind/enhancement
Signed-off-by: Eliza Weisman <[email protected]>
im a loser, and a fool
Signed-off-by: Eliza Weisman <[email protected]>
| if u & mask == 0 { | ||
| return self; | ||
| } | ||
| let aligned = (u | mask) + 1; |
There was a problem hiding this comment.
Is this more efficient than self.align_down(align) + align?
There was a problem hiding this comment.
(but probably very slightly)
There was a problem hiding this comment.
that's fair. Let's stick with this.
Signed-off-by: Eliza Weisman <[email protected]>
|
changed the |
hal-core/src/addr.rs
Outdated
| #[inline] | ||
| #[cfg(target_arch = "x86_64")] | ||
| fn from_usize(u: usize) -> Self { | ||
| debug_assert!( |
There was a problem hiding this comment.
Should this be a debug_assert_eq!?
| /// # Panics | ||
| /// | ||
| /// - If `align` is not a power of two. | ||
| fn align_down<A: Into<usize>>(self, align: A) -> Self { |
There was a problem hiding this comment.
We can totally do this in a different patch, but it might be nice to have align_up_for and align_down_for helper methods like the is_aligned_for method.
hal-core/src/addr.rs
Outdated
| debug_assert!( | ||
| u & MASK == 0, | ||
| "x86_64 physical addresses may not have the 12 most significant bits set!" | ||
| ); |
There was a problem hiding this comment.
Might be worth #[cfg(target_arch = "x86_64")]-ing only this assertion, and keeping the rest of the from_usize impl common?
| /// | ||
| /// - If `self` is not aligned for a `T`-typed value. | ||
| fn as_ptr<T>(self) -> *mut T { | ||
| assert!(self.is_aligned_for::<T>()); |
There was a problem hiding this comment.
I suppose we need this restriction for rust, even though there's no aligned read requirement on x64?
There was a problem hiding this comment.
that's correct, will add a note
hal-core/src/addr.rs
Outdated
| // #[cfg(target_arch = "x86_64")] | ||
| // #[test] | ||
| // fn sign_extend_vaddr() { | ||
| // let actual = VAddr::from_usize(123 | (1 << 47)).as_usize(); | ||
| // let expected = (0xFFFFF << 47) | 123; | ||
| // assert_eq!( | ||
| // actual, expected, | ||
| // "\n left: `{:064b}`\n right: `{:064b}`", | ||
| // actual, expected | ||
| // ); | ||
| // let actual = VAddr::from_usize(123 | (1010 << 47)).as_usize(); | ||
| // let expected = 123; | ||
| // assert_eq!( | ||
| // actual, expected, | ||
| // "\n left: `{:064b}`\n right: `{:064b}`", | ||
| // actual, expected | ||
| // ); | ||
| // } |
There was a problem hiding this comment.
We probably want to remove this.
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
This way, they can be used even when the Address trait is not in scope, but Address allows writing code that's generic over PAddr/VAddr. Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
This branch makes the following changes:
feat(hal-core): implement Address::{align_up, align_down}
Add generic default implementations of
align_upandalign_downtoall types implementing Address. These are implemented using a new
required
from_usizeconstructor.feat(hal-core):
Address::as_ptrreturns typed mut ptrAs discussed in [this comment][1],
as_ptrnow returns a*mut Trather than a
*const (). It will now assert that the address iscorrectly aligned for a
T, using a newis_aligned_for::<T>convenience method.
feat(hal-core): validate/canonicalize x86_64 addrs
Add
#[cfg(target_arch = "x86_64")]implementations ofAddress::from_usizeforPAddr/VAddrthat assert the addressis sign-extended/canonicalized when compiled in debug mode.
Additionally, add
from_usize_checkedconstructors that returnResultand always validate that the constructed addresses arevalid.
feat(hal-core): add inherent versions of address trait methods