Merge pull request #370 from rust-osdev/addr

VirtAddr improvements

Author: josephlr

src/addr.rs

@@ -22,7 +22,7 @@ const ADDRESS_SPACE_SIZE: u64 = 0x1_0000_0000_0000;
 /// between `u64` and `usize`.
 ///
 /// On `x86_64`, only the 48 lower bits of a virtual address can be used. The top 16 bits need
-/// to be copies of bit 47, i.e. the most significant bit. Addresses that fulfil this criterium
+/// to be copies of bit 47, i.e. the most significant bit. Addresses that fulfil this criterion
 /// are called “canonical”. This type guarantees that it always represents a canonical address.
 #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[repr(transparent)]
@@ -62,39 +62,43 @@ impl core::fmt::Debug for VirtAddrNotValid {
 impl VirtAddr {
     /// Creates a new canonical virtual address.
     ///
-    /// This function performs sign extension of bit 47 to make the address canonical.
+    /// The provided address should already be canonical. If you want to check
+    /// whether an address is canonical, use [`try_new`](Self::try_new).
     ///
     /// ## Panics
     ///
-    /// This function panics if the bits in the range 48 to 64 contain data (i.e. are not null and no sign extension).
-    #[inline]
-    pub fn new(addr: u64) -> VirtAddr {
-        Self::try_new(addr).expect(
-            "address passed to VirtAddr::new must not contain any data \
-             in bits 48 to 64",
-        )
+    /// This function panics if the bits in the range 48 to 64 are invalid
+    /// (i.e. are not a proper sign extension of bit 47).
+    #[inline]
+    pub const fn new(addr: u64) -> VirtAddr {
+        // TODO: Replace with .ok().expect(msg) when that works on stable.
+        match Self::try_new(addr) {
+            Ok(v) => v,
+            Err(_) => panic!("virtual address must be sign extended in bits 48 to 64"),
+        }
     }
 
     /// Tries to create a new canonical virtual address.
     ///
-    /// This function tries to performs sign
-    /// extension of bit 47 to make the address canonical. It succeeds if bits 48 to 64 are
-    /// either a correct sign extension (i.e. copies of bit 47) or all null. Else, an error
-    /// is returned.
-    #[inline]
-    pub fn try_new(addr: u64) -> Result<VirtAddr, VirtAddrNotValid> {
-        match addr.get_bits(47..64) {
-            0 | 0x1ffff => Ok(VirtAddr(addr)),     // address is canonical
-            1 => Ok(VirtAddr::new_truncate(addr)), // address needs sign extension
-            _ => Err(VirtAddrNotValid(addr)),
+    /// This function checks wether the given address is canonical
+    /// and returns an error otherwise. An address is canonical
+    /// if bits 48 to 64 are a correct sign
+    /// extension (i.e. copies of bit 47).
+    #[inline]
+    pub const fn try_new(addr: u64) -> Result<VirtAddr, VirtAddrNotValid> {
+        let v = Self::new_truncate(addr);
+        if v.0 == addr {
+            Ok(v)
+        } else {
+            Err(VirtAddrNotValid(addr))
         }
     }
 
     /// Creates a new canonical virtual address, throwing out bits 48..64.
     ///
-    /// This function performs sign extension of bit 47 to make the address canonical, so
-    /// bits 48 to 64 are overwritten. If you want to check that these bits contain no data,
-    /// use `new` or `try_new`.
+    /// This function performs sign extension of bit 47 to make the address
+    /// canonical, overwriting bits 48 to 64. If you want to check whether an
+    /// address is canonical, use [`new`](Self::new) or [`try_new`](Self::try_new).
     #[inline]
     pub const fn new_truncate(addr: u64) -> VirtAddr {
         // By doing the right shift as a signed operation (on a i64), it will
@@ -125,11 +129,7 @@ impl VirtAddr {
     }
 
     /// Creates a virtual address from the given pointer
-    // cfg(target_pointer_width = "32") is only here for backwards
-    // compatibility: Earlier versions of this crate did not have any `cfg()`
-    // on this function. At least for 32- and 64-bit we know the `as u64` cast
-    // doesn't truncate.
-    #[cfg(any(target_pointer_width = "32", target_pointer_width = "64"))]
+    #[cfg(target_pointer_width = "64")]
     #[inline]
     pub fn from_ptr<T>(ptr: *const T) -> Self {
         Self::new(ptr as u64)

src/instructions/interrupts.rs

@@ -109,10 +109,10 @@ where
 /// On some processors, the interrupt shadow of `sti` does not apply to
 /// non-maskable interrupts (NMIs). This means that an NMI can occur between
 /// the `sti` and `hlt` instruction, with the result that the CPU is put to
-/// sleep even though a new interrupt occured.
+/// sleep even though a new interrupt occurred.
 ///
 /// To work around this, it is recommended to check in the NMI handler if
-/// the interrupt occured between `sti` and `hlt` instructions. If this is the
+/// the interrupt occurred between `sti` and `hlt` instructions. If this is the
 /// case, the handler should increase the instruction pointer stored in the
 /// interrupt stack frame so that the `hlt` instruction is skipped.
 ///

src/structures/idt.rs

@@ -42,7 +42,7 @@ use super::gdt::SegmentSelector;
 /// first entry, the entry for the `divide_error` exception. Note that the index access is
 /// not possible for entries for which an error code is pushed.
 ///
-/// The remaining entries are used for interrupts. They can be accesed through index
+/// The remaining entries are used for interrupts. They can be accessed through index
 /// operations on the idt, e.g. `idt[32]` returns the first interrupt entry, which is the 32nd IDT
 /// entry).
 ///
@@ -1263,7 +1263,7 @@ macro_rules! set_general_handler {
 #[macro_export]
 #[doc(hidden)]
 /// We can't loop in macros, but we can use recursion.
-/// This macro recursivly adds one more bit to it's arguments until we have 8 bits so that we can call set_general_handler_entry.
+/// This macro recursively adds one more bit to it's arguments until we have 8 bits so that we can call set_general_handler_entry.
 macro_rules! set_general_handler_recursive_bits {
     // if we have 8 all bits, construct the index from the bits, check if the entry is in range and invoke the macro that sets the handler
     ($idt:expr, $handler:ident, $range:expr, $bit7:tt, $bit6:tt, $bit5:tt, $bit4:tt, $bit3:tt, $bit2:tt, $bit1:tt, $bit0:tt) => {{
@@ -1274,7 +1274,7 @@ macro_rules! set_general_handler_recursive_bits {
             $crate::set_general_handler_entry!($idt, $handler, IDX, $bit7, $bit6, $bit5, $bit4, $bit3, $bit2, $bit1, $bit0);
         }
     }};
-    // otherwise recursivly invoke the macro adding one more bit
+    // otherwise recursively invoke the macro adding one more bit
     ($idt:expr, $handler:ident, $range:expr $(, $bits:tt)*) => {
         $crate::set_general_handler_recursive_bits!($idt, $handler, $range $(, $bits)*, 0);
         $crate::set_general_handler_recursive_bits!($idt, $handler, $range $(, $bits)*, 1);