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

fix signature of Step::steps_between implementations #513

Merged
merged 5 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 5 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ jobs:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@nightly
with:
targets: x86_64-unknown-linux-musl, i686-unknown-linux-gnu, thumbv7em-none-eabihf
targets: x86_64-unknown-linux-musl, i686-unknown-linux-musl, thumbv7em-none-eabihf

- run: cargo build

Expand All @@ -72,9 +72,12 @@ jobs:

- name: "Build on non x86_64 platforms"
run: |
cargo build --target i686-unknown-linux-gnu --no-default-features --features nightly
cargo build --target i686-unknown-linux-musl --no-default-features --features nightly
cargo build --target thumbv7em-none-eabihf --no-default-features --features nightly

- run: cargo test --target i686-unknown-linux-musl --no-default-features --features nightly
if: runner.os == 'Linux'

bootloader-test:
name: "Bootloader Integration Test"

Expand Down
132 changes: 95 additions & 37 deletions src/addr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,25 +240,43 @@ impl VirtAddr {
}

// FIXME: Move this into the `Step` impl, once `Step` is stabilized.
#[cfg(feature = "step_trait")]
pub(crate) fn steps_between_impl(start: &Self, end: &Self) -> (usize, Option<usize>) {
if let Some(steps) = Self::steps_between_u64(start, end) {
let steps = usize::try_from(steps).ok();
(steps.unwrap_or(usize::MAX), steps)
} else {
(0, None)
}
}

/// An implementation of steps_between that returns u64. Note that this
/// function always returns the exact bound, so it doesn't need to return a
/// lower and upper bound like steps_between does.
#[cfg(any(feature = "instructions", feature = "step_trait"))]
pub(crate) fn steps_between_impl(start: &Self, end: &Self) -> Option<usize> {
pub(crate) fn steps_between_u64(start: &Self, end: &Self) -> Option<u64> {
let mut steps = end.0.checked_sub(start.0)?;

// Mask away extra bits that appear while jumping the gap.
steps &= 0xffff_ffff_ffff;

usize::try_from(steps).ok()
Some(steps)
}

// FIXME: Move this into the `Step` impl, once `Step` is stabilized.
#[inline]
pub(crate) fn forward_checked_impl(start: Self, count: usize) -> Option<Self> {
let offset = u64::try_from(count).ok()?;
if offset > ADDRESS_SPACE_SIZE {
Self::forward_checked_u64(start, u64::try_from(count).ok()?)
}

/// An implementation of forward_checked that takes u64 instead of usize.
#[inline]
pub(crate) fn forward_checked_u64(start: Self, count: u64) -> Option<Self> {
if count > ADDRESS_SPACE_SIZE {
return None;
}

let mut addr = start.0.checked_add(offset)?;
let mut addr = start.0.checked_add(count)?;

match addr.get_bits(47..) {
0x1 => {
Expand All @@ -274,6 +292,31 @@ impl VirtAddr {

Some(unsafe { Self::new_unsafe(addr) })
}

/// An implementation of backward_checked that takes u64 instead of usize.
#[cfg(feature = "step_trait")]
#[inline]
pub(crate) fn backward_checked_u64(start: Self, count: u64) -> Option<Self> {
if count > ADDRESS_SPACE_SIZE {
return None;
}

let mut addr = start.0.checked_sub(count)?;

match addr.get_bits(47..) {
0x1fffe => {
// Jump the gap by sign extending the 47th bit.
addr.set_bits(47.., 0);
}
0x1fffd => {
// Address underflow
return None;
}
_ => {}
}

Some(unsafe { Self::new_unsafe(addr) })
}
}

impl fmt::Debug for VirtAddr {
Expand Down Expand Up @@ -360,7 +403,7 @@ impl Sub<VirtAddr> for VirtAddr {
#[cfg(feature = "step_trait")]
impl Step for VirtAddr {
#[inline]
fn steps_between(start: &Self, end: &Self) -> Option<usize> {
fn steps_between(start: &Self, end: &Self) -> (usize, Option<usize>) {
Self::steps_between_impl(start, end)
}

Expand All @@ -371,26 +414,7 @@ impl Step for VirtAddr {

#[inline]
fn backward_checked(start: Self, count: usize) -> Option<Self> {
let offset = u64::try_from(count).ok()?;
if offset > ADDRESS_SPACE_SIZE {
return None;
}

let mut addr = start.0.checked_sub(offset)?;

match addr.get_bits(47..) {
0x1fffe => {
// Jump the gap by sign extending the 47th bit.
addr.set_bits(47.., 0);
}
0x1fffd => {
// Address underflow
return None;
}
_ => {}
}

Some(unsafe { Self::new_unsafe(addr) })
Self::backward_checked_u64(start, u64::try_from(count).ok()?)
}
}

Expand Down Expand Up @@ -664,22 +688,27 @@ mod tests {
Step::forward_checked(VirtAddr(0xffff_ffff_ffff_ffff), 1),
None
);
#[cfg(target_pointer_width = "64")]
assert_eq!(
Step::forward(VirtAddr(0x7fff_ffff_ffff), 0x1234_5678_9abd),
VirtAddr(0xffff_9234_5678_9abc)
);
#[cfg(target_pointer_width = "64")]
assert_eq!(
Step::forward(VirtAddr(0x7fff_ffff_ffff), 0x8000_0000_0000),
VirtAddr(0xffff_ffff_ffff_ffff)
);
#[cfg(target_pointer_width = "64")]
assert_eq!(
Step::forward(VirtAddr(0x7fff_ffff_ff00), 0x8000_0000_00ff),
VirtAddr(0xffff_ffff_ffff_ffff)
);
#[cfg(target_pointer_width = "64")]
assert_eq!(
Step::forward_checked(VirtAddr(0x7fff_ffff_ff00), 0x8000_0000_0100),
None
);
#[cfg(target_pointer_width = "64")]
assert_eq!(
Step::forward_checked(VirtAddr(0x7fff_ffff_ffff), 0x8000_0000_0001),
None
Expand All @@ -700,18 +729,22 @@ mod tests {
Step::backward(VirtAddr(0xffff_8000_0000_0001), 1),
VirtAddr(0xffff_8000_0000_0000)
);
#[cfg(target_pointer_width = "64")]
assert_eq!(
Step::backward(VirtAddr(0xffff_9234_5678_9abc), 0x1234_5678_9abd),
VirtAddr(0x7fff_ffff_ffff)
);
#[cfg(target_pointer_width = "64")]
assert_eq!(
Step::backward(VirtAddr(0xffff_8000_0000_0000), 0x8000_0000_0000),
VirtAddr(0)
);
#[cfg(target_pointer_width = "64")]
assert_eq!(
Step::backward(VirtAddr(0xffff_8000_0000_0000), 0x7fff_ffff_ff01),
VirtAddr(0xff)
);
#[cfg(target_pointer_width = "64")]
assert_eq!(
Step::backward_checked(VirtAddr(0xffff_8000_0000_0000), 0x8000_0000_0001),
None
Expand All @@ -721,43 +754,64 @@ mod tests {
#[test]
#[cfg(feature = "step_trait")]
fn virtaddr_steps_between() {
assert_eq!(Step::steps_between(&VirtAddr(0), &VirtAddr(0)), Some(0));
assert_eq!(Step::steps_between(&VirtAddr(0), &VirtAddr(1)), Some(1));
assert_eq!(Step::steps_between(&VirtAddr(1), &VirtAddr(0)), None);
assert_eq!(
Step::steps_between(&VirtAddr(0), &VirtAddr(0)),
(0, Some(0))
);
assert_eq!(
Step::steps_between(&VirtAddr(0), &VirtAddr(1)),
(1, Some(1))
);
assert_eq!(Step::steps_between(&VirtAddr(1), &VirtAddr(0)), (0, None));
assert_eq!(
Step::steps_between(
&VirtAddr(0x7fff_ffff_ffff),
&VirtAddr(0xffff_8000_0000_0000)
),
Some(1)
(1, Some(1))
);
assert_eq!(
Step::steps_between(
&VirtAddr(0xffff_8000_0000_0000),
&VirtAddr(0x7fff_ffff_ffff)
),
None
(0, None)
);
assert_eq!(
Step::steps_between(
&VirtAddr(0xffff_8000_0000_0000),
&VirtAddr(0xffff_8000_0000_0000)
),
Some(0)
(0, Some(0))
);
assert_eq!(
Step::steps_between(
&VirtAddr(0xffff_8000_0000_0000),
&VirtAddr(0xffff_8000_0000_0001)
),
Some(1)
(1, Some(1))
);
assert_eq!(
Step::steps_between(
&VirtAddr(0xffff_8000_0000_0001),
&VirtAddr(0xffff_8000_0000_0000)
),
None
(0, None)
);
// Make sure that we handle `steps > u32::MAX` correctly on 32-bit
// targets. On 64-bit targets, `0x1_0000_0000` fits into `usize`, so we
// can return exact lower and upper bounds. On 32-bit targets,
// `0x1_0000_0000` doesn't fit into `usize`, so we only return an lower
// bound of `usize::MAX` and don't return an upper bound.
#[cfg(target_pointer_width = "64")]
assert_eq!(
Step::steps_between(&VirtAddr(0), &VirtAddr(0x1_0000_0000)),
(0x1_0000_0000, Some(0x1_0000_0000))
);
#[cfg(not(target_pointer_width = "64"))]
assert_eq!(
Step::steps_between(&VirtAddr(0), &VirtAddr(0x1_0000_0000)),
(usize::MAX, None)
);
}

Expand Down Expand Up @@ -809,10 +863,14 @@ mod tests {
}

#[test]
#[cfg(target_pointer_width = "64")]
fn test_from_ptr_array() {
let slice = &[1, 2, 3, 4, 5];
// Make sure that from_ptr(slice) is the address of the first element
assert_eq!(VirtAddr::from_ptr(slice), VirtAddr::from_ptr(&slice[0]));
assert_eq!(
VirtAddr::from_ptr(slice.as_slice()),
VirtAddr::from_ptr(&slice[0])
);
}
}

Expand Down Expand Up @@ -951,7 +1009,7 @@ mod proofs {
};

// ...then `steps_between` succeeds as well.
assert!(Step::steps_between(&start, &end) == Some(count));
assert!(Step::steps_between(&start, &end) == (count, Some(count)));
}

// This harness proves that for all inputs for which `steps_between`
Expand All @@ -968,7 +1026,7 @@ mod proofs {
};

// If `steps_between` succeeds...
let Some(count) = Step::steps_between(&start, &end) else {
let Some(count) = Step::steps_between(&start, &end).1 else {
return;
};

Expand Down
4 changes: 2 additions & 2 deletions src/instructions/tlb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,14 +315,14 @@ where
if let Some(mut pages) = self.page_range {
while !pages.is_empty() {
// Calculate out how many pages we still need to flush.
let count = Page::<S>::steps_between_impl(&pages.start, &pages.end).unwrap();
let count = Page::<S>::steps_between_impl(&pages.start, &pages.end).0;

// Make sure that we never jump the gap in the address space when flushing.
let second_half_start =
Page::<S>::containing_address(VirtAddr::new(0xffff_8000_0000_0000));
let count = if pages.start < second_half_start {
let count_to_second_half =
Page::steps_between_impl(&pages.start, &second_half_start).unwrap();
Page::steps_between_impl(&pages.start, &second_half_start).0;
cmp::min(count, count_to_second_half)
} else {
count
Expand Down
Loading
Loading