Skip to content
Open
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 114 additions & 1 deletion src/addr.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Physical and virtual addresses manipulation

#[cfg(feature = "step_trait")]
use core::convert::TryFrom;
use core::convert::TryInto;
use core::fmt;
#[cfg(feature = "step_trait")]
use core::iter::Step;
Expand Down Expand Up @@ -403,6 +403,96 @@ impl Step for VirtAddr {
}
}

impl TryFrom<u64> for VirtAddr {
type Error = VirtAddrNotValid;

#[inline]
fn try_from(addr: u64) -> Result<Self, Self::Error> {
Self::try_new(addr)
}
}

// if `target_pointer_width > 64`, we would need a different error type
#[cfg(any(target_pointer_width = "32", target_pointer_width = "64"))]
impl TryFrom<usize> for VirtAddr {
type Error = VirtAddrNotValid;

#[inline]
fn try_from(addr: usize) -> Result<Self, Self::Error> {
Self::try_new(addr.try_into().unwrap())
}
}

// if `target_pointer_width > 64`, we would need a different error type
// (and `as` would truncate)
#[cfg(any(target_pointer_width = "32", target_pointer_width = "64"))]
impl<T> TryFrom<*const T> for VirtAddr {
type Error = VirtAddrNotValid;

#[inline]
fn try_from(addr: *const T) -> Result<Self, Self::Error> {
Self::try_new(addr as u64)
}
}

// if target_pointer_width is 128, we would need a different error type
// (and `as` would truncate)
#[cfg(any(target_pointer_width = "32", target_pointer_width = "64"))]
impl<T> TryFrom<*mut T> for VirtAddr {
type Error = VirtAddrNotValid;

#[inline]
fn try_from(addr: *mut T) -> Result<Self, Self::Error> {
Self::try_new(addr as u64)
}
}

impl From<u32> for VirtAddr {
#[inline]
fn from(addr: u32) -> Self {
Self::new(addr.into())
}
}

#[cfg(any(target_pointer_width = "32", target_pointer_width = "64"))]
impl<T> From<&T> for VirtAddr {
#[inline]
fn from(addr: &T) -> Self {
Self::new(addr as *const _ as u64)
}
}

#[cfg(any(target_pointer_width = "32", target_pointer_width = "64"))]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also target_pointer_width = "16".

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added target_pointer_width = "16" where applicable.

impl<T> From<&mut T> for VirtAddr {
#[inline]
fn from(addr: &mut T) -> Self {
Self::new(addr as *mut _ as u64)
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These could fail on different architectures. Do we want to care about that given that most users will probably run on x86?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, good point! I guess we could a target_arch cfg-gate to avoid this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that'd work

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes these to target_arch. The target_pointer_width ought to be reasonable if the target_arch is x86 or x86_64, so I just removed those.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also wondering if we ought to use target_arch for the raw pointer conversions. I left them for now, since they implement TryFrom instead of From, but those really don't make a whole lot of sense on non-x86 based architectures.

Comment on lines +469 to +483
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These could technically fail if Intel's 5 level paging is active.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This possibility was mentioned in issue #293, here and here.

The last comment on the subject, by @josephlr was:

Personally, I think the panic is fine (for now). Before we (potentially) support 5-level paging, we can just have a disclaimer that is like: "if you use 5-level paging, some methods might panic, but safety will not be violated".

No disclaimer has been added yet, but I can add one if that's the course we want to take.

Copy link
Copy Markdown
Member

@Freax13 Freax13 Nov 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I think the panic is fine (for now). Before we (potentially) support 5-level paging, we can just have a disclaimer that is like: "if you use 5-level paging, some methods might panic, but safety will not be violated".

That seems like a good pragmatic option to me. Intel's LAM (Linear Address Masking) and AMD's UAI (Upper Address Ignore) can also cause problems because they relax the requirements on pointers to be canonical.

No disclaimer has been added yet, but I can add one if that's the course we want to take.

That'd be great, thanks!


impl From<VirtAddr> for u64 {
#[inline]
fn from(addr: VirtAddr) -> Self {
addr.0
}
}

#[cfg(target_pointer_width = "64")]
impl<T> From<VirtAddr> for *const T {
#[inline]
fn from(addr: VirtAddr) -> Self {
addr.0 as *const T
}
}

#[cfg(target_pointer_width = "64")]
impl<T> From<VirtAddr> for *mut T {
#[inline]
fn from(addr: VirtAddr) -> Self {
addr.0 as *mut T
}
}

/// A passed `u64` was not a valid physical address.
///
/// This means that bits 52 to 64 were not all null.
Expand Down Expand Up @@ -632,6 +722,29 @@ impl Sub<PhysAddr> for PhysAddr {
}
}

impl TryFrom<u64> for PhysAddr {
type Error = PhysAddrNotValid;

#[inline]
fn try_from(addr: u64) -> Result<Self, Self::Error> {
Self::try_new(addr)
}
}

impl From<u32> for PhysAddr {
#[inline]
fn from(addr: u32) -> Self {
Self::new(addr.into())
}
}

impl From<PhysAddr> for u64 {
#[inline]
fn from(addr: PhysAddr) -> Self {
addr.0
}
}

/// Align address downwards.
///
/// Returns the greatest `x` with alignment `align` so that `x <= addr`.
Expand Down